You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mu...@apache.org on 2020/06/10 14:09:36 UTC
[lucene-solr] branch branch_8x updated (b8977d4 -> 6c77f46)
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a change to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.
from b8977d4 SOLR-14347: fix cached session update to not depend on Zookeeper state
new a74de7f SOLR-14550: fix duplicate issue in Atomic updates with add-distinct
new 6c77f46 SOLR-14345: return correct err msg when non-binary resp parser is used
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
solr/CHANGES.txt | 4 ++
.../processor/AtomicUpdateDocumentMerger.java | 46 +++++++++++-----------
.../apache/solr/search/json/TestJsonRequest.java | 31 ++++++++-------
.../solr/update/processor/AtomicUpdatesTest.java | 15 +++++--
.../solr/client/solrj/impl/Http2SolrClient.java | 25 +++++++++---
.../solr/client/solrj/impl/HttpSolrClient.java | 22 ++++++++---
.../src/java/org/apache/solr/SolrTestCaseHS.java | 17 +++++---
7 files changed, 102 insertions(+), 58 deletions(-)
[lucene-solr] 01/02: SOLR-14550: fix duplicate issue in Atomic
updates with add-distinct
Posted by mu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit a74de7f5a2f9fe1cd3024833c51f287125e54ea5
Author: Munendra S N <mu...@apache.org>
AuthorDate: Wed Jun 10 18:59:44 2020 +0530
SOLR-14550: fix duplicate issue in Atomic updates with add-distinct
* When add-distinct value is list, it can end up adding duplicate
entries
---
solr/CHANGES.txt | 2 +
.../processor/AtomicUpdateDocumentMerger.java | 46 +++++++++++-----------
.../solr/update/processor/AtomicUpdatesTest.java | 15 +++++--
3 files changed, 37 insertions(+), 26 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 5230eb6..10f34d1 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -158,6 +158,8 @@ Bug Fixes
* SOLR-13203: Return 400 status code on invalid dynamic field for Edismax's user Fields
(Johannes Kloos, mrsoong via Munendra S N)
+* SOLR-14550: Fix duplicates issue in Atomic updates with add-distinct (Thomas Corthals, Munendra S N)
+
Other Changes
---------------------
* SOLR-14197: SolrResourceLoader: marked many methods as deprecated, and in some cases rerouted exiting logic to avoid
diff --git a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
index 04f0ecd..673c4fa 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
@@ -302,7 +302,7 @@ public class AtomicUpdateDocumentMerger {
/**
*
* @param completeHierarchy SolrInputDocument that represents the nested document hierarchy from its root
- * @param fieldPath the path to fetch, seperated by a '/' e.g. /children/grandChildren
+ * @param fieldPath the path to fetch, separated by a '/' e.g. /children/grandChildren
* @return the SolrInputField of fieldPath
*/
public static SolrInputField getFieldFromHierarchy(SolrInputDocument completeHierarchy, String fieldPath) {
@@ -355,7 +355,7 @@ public class AtomicUpdateDocumentMerger {
// This can happen, despite requesting for these fields in the call to RTGC.getInputDocument, if the document was
// fetched from the tlog and had all these fields (possibly because it was a full document ADD operation).
if (updatedFields != null) {
- Collection<String> names = new HashSet<String>(oldDocument.getFieldNames());
+ Collection<String> names = new HashSet<>(oldDocument.getFieldNames());
for (String fieldName: names) {
if (fieldName.equals(CommonParams.VERSION_FIELD)==false && fieldName.equals(ID)==false && updatedFields.contains(fieldName)==false) {
oldDocument.remove(fieldName);
@@ -450,33 +450,33 @@ public class AtomicUpdateDocumentMerger {
final String name = sif.getName();
SolrInputField existingField = toDoc.get(name);
+ // throws exception if field doesn't exist
SchemaField sf = schema.getField(name);
- if (sf != null) {
- Collection<Object> original = existingField != null ?
- existingField.getValues() :
- new ArrayList<>();
+ Collection<Object> original = existingField != null ?
+ existingField.getValues() :
+ new ArrayList<>();
- int initialSize = original.size();
- if (fieldVal instanceof Collection) {
- for (Object object : (Collection) fieldVal) {
- if (!original.contains(object)) {
- original.add(object);
- }
- }
- } else {
- Object object = sf.getType().toNativeType(fieldVal);
- if (!original.contains(object)) {
- original.add(object);
+ int initialSize = original.size();
+ if (fieldVal instanceof Collection) {
+ for (Object object : (Collection) fieldVal) {
+ Object obj = sf.getType().toNativeType(object);
+ if (!original.contains(obj)) {
+ original.add(obj);
}
}
+ } else {
+ Object object = sf.getType().toNativeType(fieldVal);
+ if (!original.contains(object)) {
+ original.add(object);
+ }
+ }
- if (original.size() > initialSize) { // update only if more are added
- if (original.size() == 1) { // if single value, pass the value instead of List
- doAdd(toDoc, sif, original.toArray()[0]);
- } else {
- toDoc.setField(name, original);
- }
+ if (original.size() > initialSize) { // update only if more are added
+ if (original.size() == 1) { // if single value, pass the value instead of List
+ doAdd(toDoc, sif, original.toArray()[0]);
+ } else {
+ toDoc.setField(name, original);
}
}
}
diff --git a/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java b/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java
index 48c76b7..5a15e10 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java
@@ -989,6 +989,7 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
SolrInputDocument doc = new SolrInputDocument();
doc.setField("id", "3");
doc.setField("cat", new String[]{"aaa", "ccc"});
+ doc.setField("atomic_is", 10);
assertU(adoc(doc));
doc = new SolrInputDocument();
@@ -1005,22 +1006,30 @@ public class AtomicUpdatesTest extends SolrTestCaseJ4 {
doc = new SolrInputDocument();
doc.setField("id", "3");
doc.setField("cat", ImmutableMap.of("add-distinct", "bbb"));
+ doc.setField("atomic_is", ImmutableMap.of("add-distinct", 10));
assertU(adoc(doc));
assertU(commit());
assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']");
assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']");
- assertQ(req("q", "cat:bbb", "indent", "true"), "//doc/arr[@name='cat'][count(str)=3]");
+ assertQ(req("q", "cat:bbb", "indent", "true"),
+ "//doc/arr[@name='cat'][count(str)=3]",
+ "//doc/arr[@name='atomic_is'][count(int)=1]"
+ );
doc = new SolrInputDocument();
doc.setField("id", "3");
- doc.setField("cat", ImmutableMap.of("add-distinct", Arrays.asList(new String[]{"bbb", "bbb"})));
+ doc.setField("cat", ImmutableMap.of("add-distinct", Arrays.asList("bbb", "bbb")));
+ doc.setField("atomic_is", ImmutableMap.of("add-distinct", Arrays.asList(10, 34)));
assertU(adoc(doc));
assertU(commit());
assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']");
assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']");
- assertQ(req("q", "cat:bbb", "indent", "true"), "//doc/arr[@name='cat'][count(str)=3]"); //'bbb' already present will not be added again
+ assertQ(req("q", "cat:bbb", "indent", "true"),
+ "//doc/arr[@name='cat'][count(str)=3]", //'bbb' already present will not be added again
+ "//doc/arr[@name='atomic_is'][count(int)=2]"
+ );
doc = new SolrInputDocument();
doc.setField("id", "5");
[lucene-solr] 02/02: SOLR-14345: return correct err msg when
non-binary resp parser is used
Posted by mu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 6c77f4603cfd6d8566d4eae7d287e727edde78ed
Author: Munendra S N <mu...@apache.org>
AuthorDate: Wed Jun 10 19:11:52 2020 +0530
SOLR-14345: return correct err msg when non-binary resp parser is used
* This adds support to parse error properly in case of non-binary
resp parser but the problem still exists for noopResponseParser
---
solr/CHANGES.txt | 2 ++
.../apache/solr/search/json/TestJsonRequest.java | 31 +++++++++++-----------
.../solr/client/solrj/impl/Http2SolrClient.java | 25 ++++++++++++-----
.../solr/client/solrj/impl/HttpSolrClient.java | 22 +++++++++++----
.../src/java/org/apache/solr/SolrTestCaseHS.java | 17 +++++++-----
5 files changed, 65 insertions(+), 32 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 10f34d1..8223759 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -160,6 +160,8 @@ Bug Fixes
* SOLR-14550: Fix duplicates issue in Atomic updates with add-distinct (Thomas Corthals, Munendra S N)
+* SOLR-14345: Return proper error message when non-BinaryResponseParser is used in solrJ (Munendra S N)
+
Other Changes
---------------------
* SOLR-14197: SolrResourceLoader: marked many methods as deprecated, and in some cases rerouted exiting logic to avoid
diff --git a/solr/core/src/test/org/apache/solr/search/json/TestJsonRequest.java b/solr/core/src/test/org/apache/solr/search/json/TestJsonRequest.java
index 9224644..bf5a2b7 100644
--- a/solr/core/src/test/org/apache/solr/search/json/TestJsonRequest.java
+++ b/solr/core/src/test/org/apache/solr/search/json/TestJsonRequest.java
@@ -32,6 +32,8 @@ import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
+import static org.hamcrest.core.StringContains.containsString;
+
@SuppressWarnings("deprecation")
@LuceneTestCase.SuppressCodecs({"Lucene3x","Lucene40","Lucene41","Lucene42","Lucene45","Appending"})
@@ -83,6 +85,8 @@ public class TestJsonRequest extends SolrTestCaseHS {
public static void doJsonRequest(Client client, boolean isDistrib) throws Exception {
addDocs(client);
+ ignoreException("Expected JSON");
+
// test json param
client.testJQ( params("json","{query:'cat_s:A'}")
, "response/numFound==2"
@@ -91,6 +95,7 @@ public class TestJsonRequest extends SolrTestCaseHS {
// invalid value
SolrException ex = expectThrows(SolrException.class, () -> client.testJQ(params("q", "*:*", "json", "5")));
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, ex.code());
+ assertThat(ex.getMessage(), containsString("Expected JSON Object but got Long=5"));
// this is to verify other json params are not affected
client.testJQ( params("q", "cat_s:A", "json.limit", "1"),
@@ -388,23 +393,19 @@ public class TestJsonRequest extends SolrTestCaseHS {
, "response/numFound==3", isDistrib? "" : "response/docs==[{id:'4'},{id:'1'},{id:'5'}]"
);
- try {
- client.testJQ(params("json", "{query:{'lucene':'foo_s:ignore_exception'}}")); // TODO: this seems like a reasonable capability that we would want to support in the future. It should be OK to make this pass.
- fail();
- } catch (Exception e) {
- assertTrue(e.getMessage().contains("foo_s"));
- }
+ // TODO: this seems like a reasonable capability that we would want to support in the future. It should be OK to make this pass.
+ Exception e = expectThrows(Exception.class, () -> {
+ client.testJQ(params("json", "{query:{'lucene':'foo_s:ignore_exception'}}"));
+ });
+ assertThat(e.getMessage(), containsString("foo_s"));
- try {
- // test failure on unknown parameter
- client.testJQ(params("json", "{query:'cat_s:A', foobar_ignore_exception:5}")
- , "response/numFound==2"
- );
- fail();
- } catch (Exception e) {
- assertTrue(e.getMessage().contains("foobar"));
- }
+ // test failure on unknown parameter
+ e = expectThrows(Exception.class, () -> {
+ client.testJQ(params("json", "{query:'cat_s:A', foobar_ignore_exception:5}"), "response/numFound==2");
+ });
+ assertThat(e.getMessage(), containsString("foobar"));
+ resetExceptionIgnores();
}
private static void doParamRefDslTest(Client client) throws Exception {
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index bdb4291..8034083 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -70,6 +70,7 @@ import org.apache.solr.common.util.ExecutorUtil;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.ObjectReleaseTracker;
import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.common.util.Utils;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpClientTransport;
import org.eclipse.jetty.client.ProtocolHandlers;
@@ -630,6 +631,7 @@ public class Http2SolrClient extends SolrClient {
return processor == null || processor instanceof InputStreamResponseParser;
}
+ @SuppressWarnings({"unchecked"})
private NamedList<Object> processErrorsAndResponse(Response response,
final ResponseParser processor,
InputStream is,
@@ -702,13 +704,24 @@ public class Http2SolrClient extends SolrClient {
NamedList<String> metadata = null;
String reason = null;
try {
- NamedList err = (NamedList) rsp.get("error");
- if (err != null) {
- reason = (String) err.get("msg");
- if (reason == null) {
- reason = (String) err.get("trace");
+ if (error != null) {
+ reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("msg"));
+ if(reason == null) {
+ reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("trace"));
+ }
+ Object metadataObj = Utils.getObjectByPath(error, false, Collections.singletonList("metadata"));
+ if (metadataObj instanceof NamedList) {
+ metadata = (NamedList<String>) metadataObj;
+ } else if (metadataObj instanceof List) {
+ // NamedList parsed as List convert to NamedList again
+ List<Object> list = (List<Object>) metadataObj;
+ metadata = new NamedList<>(list.size()/2);
+ for (int i = 0; i < list.size(); i+=2) {
+ metadata.add((String)list.get(i), (String) list.get(i+1));
+ }
+ } else if (metadataObj instanceof Map) {
+ metadata = new NamedList((Map) metadataObj);
}
- metadata = (NamedList<String>) err.get("metadata");
}
} catch (Exception ex) {}
if (reason == null) {
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
index c4b68f3..da6326d 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
@@ -533,6 +533,7 @@ public class HttpSolrClient extends BaseHttpSolrClient {
private static final List<String> errPath = Arrays.asList("metadata", "error-class");//Utils.getObjectByPath(err, false,"metadata/error-class")
+ @SuppressWarnings({"unchecked"})
protected NamedList<Object> executeMethod(HttpRequestBase method, Principal userPrincipal, final ResponseParser processor, final boolean isV2Api) throws SolrServerException {
method.addHeader("User-Agent", AGENT);
@@ -643,13 +644,24 @@ public class HttpSolrClient extends BaseHttpSolrClient {
NamedList<String> metadata = null;
String reason = null;
try {
- NamedList err = (NamedList) rsp.get("error");
- if (err != null) {
- reason = (String) err.get("msg");
+ if (error != null) {
+ reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("msg"));
if(reason == null) {
- reason = (String) err.get("trace");
+ reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("trace"));
+ }
+ Object metadataObj = Utils.getObjectByPath(error, false, Collections.singletonList("metadata"));
+ if (metadataObj instanceof NamedList) {
+ metadata = (NamedList<String>) metadataObj;
+ } else if (metadataObj instanceof List) {
+ // NamedList parsed as List convert to NamedList again
+ List<Object> list = (List<Object>) metadataObj;
+ metadata = new NamedList<>(list.size()/2);
+ for (int i = 0; i < list.size(); i+=2) {
+ metadata.add((String)list.get(i), (String) list.get(i+1));
+ }
+ } else if (metadataObj instanceof Map) {
+ metadata = new NamedList((Map) metadataObj);
}
- metadata = (NamedList<String>)err.get("metadata");
}
} catch (Exception ex) {}
if (reason == null) {
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java
index cca39739..5ef9185 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java
@@ -44,11 +44,13 @@ import org.apache.solr.client.solrj.embedded.JettySolrRunner;
import org.apache.solr.client.solrj.impl.NoOpResponseParser;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.response.DelegationTokenResponse;
import org.apache.solr.client.solrj.response.UpdateResponse;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.Utils;
import org.apache.solr.core.CoreDescriptor;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.schema.IndexSchema;
@@ -197,12 +199,15 @@ public class SolrTestCaseHS extends SolrTestCaseJ4 {
query.setPath(path);
}
- query.setResponseParser(new NoOpResponseParser(wt));
- NamedList<Object> rsp = client.request(query);
-
- String raw = (String)rsp.get("response");
-
- return raw;
+ if ("json".equals(wt)) {
+ query.setResponseParser(new DelegationTokenResponse.JsonMapResponseParser());
+ NamedList<Object> rsp = client.request(query);
+ return Utils.toJSONString(rsp);
+ } else {
+ query.setResponseParser(new NoOpResponseParser(wt));
+ NamedList<Object> rsp = client.request(query);
+ return (String)rsp.get("response");
+ }
}
public static String getQueryResponse(String wt, SolrParams params) throws Exception {