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 13:57:21 UTC
[lucene-solr] branch master updated (d4f7c90 -> 5304098)
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.
from d4f7c90 SOLR-14347: fix cached session update to not depend on Zookeeper state (#1542)
new 001c4e2 SOLR-14550: fix duplicate issue in Atomic updates with add-distinct
new 5304098 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] 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 master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 53040984f62eedd425c98eb6f60bc54a0e83258b
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 ef87d32..ee44d1b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -233,6 +233,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 9451ddf..4207305 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;
+
@LuceneTestCase.SuppressCodecs({"Lucene3x","Lucene40","Lucene41","Lucene42","Lucene45","Appending"})
public class TestJsonRequest extends SolrTestCaseHS {
@@ -84,6 +86,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"
@@ -92,6 +96,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"),
@@ -389,23 +394,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 3c32146..9e46a96 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
@@ -69,6 +69,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;
@@ -629,6 +630,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,
@@ -701,13 +703,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 4ee1698..a734831 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 7d3e7ca..cc572a9 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java
@@ -45,12 +45,14 @@ 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.SolrException;
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;
@@ -225,12 +227,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 {
[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 master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 001c4e2ddd515c47ec842efadad3e56c4e648c61
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 cd808ad..ef87d32 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -231,6 +231,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");