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:23 UTC
[lucene-solr] 02/02: SOLR-14345: return correct err msg when
non-binary resp parser is used
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 {