You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2017/06/22 05:32:12 UTC
lucene-solr:master: SOLR-10406: SolrJ must throw exception if server
throws an error
Repository: lucene-solr
Updated Branches:
refs/heads/master d0d30464d -> ad2cb7784
SOLR-10406: SolrJ must throw exception if server throws an error
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/ad2cb778
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/ad2cb778
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/ad2cb778
Branch: refs/heads/master
Commit: ad2cb7784e2f2a508dfb2fdcb2de7baf8bd18d68
Parents: d0d3046
Author: Noble Paul <no...@apache.org>
Authored: Thu Jun 22 15:02:01 2017 +0930
Committer: Noble Paul <no...@apache.org>
Committed: Thu Jun 22 15:02:01 2017 +0930
----------------------------------------------------------------------
.../autoscaling/AutoScalingHandlerTest.java | 5 +--
.../solr/handler/V2ApiIntegrationTest.java | 10 ++++-
.../solr/client/solrj/impl/HttpSolrClient.java | 41 ++++++++++++++++++++
.../client/solrj/impl/LBHttpSolrClient.java | 5 ++-
.../java/org/apache/solr/common/util/Utils.java | 33 +++++++++++-----
5 files changed, 79 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad2cb778/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
index d9f0388..05cdb87 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
@@ -79,9 +79,8 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
SolrRequest req = createAutoScalingRequest(SolrRequest.METHOD.POST, setPolicyCommand);
NamedList<Object> response = null;
try {
- response = solrClient.request(req);
- String errorMsg = (String) ((NamedList)response.get("error")).get("msg");
- assertTrue(errorMsg.contains("cores is only allowed in 'cluster-policy'"));
+ solrClient.request(req);
+ fail("Adding a policy with 'cores' attribute should not have succeeded.");
} catch (SolrServerException e) {
// todo one of these catch blocks should not be needed after SOLR-10768
if (e.getRootCause() instanceof HttpSolrClient.RemoteSolrException) {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad2cb778/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
index dac495e..844c960 100644
--- a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
@@ -27,6 +27,7 @@ import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.BinaryResponseParser;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.impl.XMLResponseParser;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.V2Request;
@@ -67,8 +68,13 @@ public class V2ApiIntegrationTest extends SolrCloudTestCase {
.withPayload(payload)
.build();
v2Request.setResponseParser(responseParser);
- V2Response response = v2Request.process(cluster.getSolrClient());
- assertEquals(getStatus(response), expectedCode);
+ try {
+ v2Request.process(cluster.getSolrClient());
+ fail("expected an exception with error code: "+expectedCode);
+ } catch (HttpSolrClient.RemoteExecutionException e) {
+ assertEquals(expectedCode, e.code());
+
+ }
}
@Test
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad2cb778/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
----------------------------------------------------------------------
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 fa1ccf1..e73e08b 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
@@ -82,6 +82,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
+import static org.apache.solr.common.util.Utils.getObjectByPath;
+
/**
* A SolrClient implementation that talks directly to a Solr server via HTTP
*/
@@ -575,6 +577,12 @@ public class HttpSolrClient extends SolrClient {
} catch (Exception e) {
throw new RemoteSolrException(baseUrl, httpStatus, e.getMessage(), e);
}
+ if (isV2Api) {
+ Object err = rsp.get("error");
+ if (err != null) {
+ throw RemoteExecutionException.create(baseUrl, rsp);
+ }
+ }
if (httpStatus != HttpStatus.SC_OK && !isV2Api) {
NamedList<String> metadata = null;
String reason = null;
@@ -759,6 +767,39 @@ public class HttpSolrClient extends SolrClient {
}
/**
+ * This should be thrown when a server has an error in executing the request and
+ * it sends a proper payload back to the client
+ */
+ public static class RemoteExecutionException extends RemoteSolrException {
+ private NamedList meta;
+
+ public RemoteExecutionException(String remoteHost, int code, String msg, NamedList meta) {
+ super(remoteHost, code, msg, null);
+ this.meta = meta;
+ }
+
+
+ public static RemoteExecutionException create(String host, NamedList errResponse) {
+ Object errObj = errResponse.get("error");
+ if (errObj != null) {
+ Number code = (Number) getObjectByPath(errObj, true, Collections.singletonList("code"));
+ String msg = (String) getObjectByPath(errObj, true, Collections.singletonList("msg"));
+ return new RemoteExecutionException(host, code == null ? ErrorCode.UNKNOWN.code : code.intValue(),
+ msg == null ? "Unknown Error" : msg, errResponse);
+
+ } else {
+ throw new RuntimeException("No error");
+ }
+
+ }
+
+ public NamedList getMetaData() {
+
+ return meta;
+ }
+ }
+
+ /**
* Constructs {@link HttpSolrClient} instances from provided configuration.
*/
public static class Builder {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad2cb778/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
index 21816bb..2e51bbc 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
@@ -43,6 +43,7 @@ import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteExecutionException;
import org.apache.solr.client.solrj.request.IsUpdateRequest;
import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.client.solrj.response.QueryResponse;
@@ -460,7 +461,9 @@ public class LBHttpSolrClient extends SolrClient {
if (isZombie) {
zombieServers.remove(zombieKey);
}
- } catch (SolrException e) {
+ } catch (RemoteExecutionException e){
+ throw e;
+ } catch(SolrException e) {
// we retry on 404 or 403 or 503 or 500
// unless it's an update - then we only retry on connect exception
if (!isNonRetryable && RETRY_CODES.contains(e.code())) {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad2cb778/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 66e21c0..1da5af6 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -213,15 +213,20 @@ public class Utils {
}
}
- public static Object getObjectByPath(Map root, boolean onlyPrimitive, String hierarchy) {
+ public static Object getObjectByPath(Object root, boolean onlyPrimitive, String hierarchy) {
+ if(root == null) return null;
+ if(!isMapLike(root)) throw new RuntimeException("must be a Map or NamedList");
List<String> parts = StrUtils.splitSmart(hierarchy, '/');
if (parts.get(0).isEmpty()) parts.remove(0);
return getObjectByPath(root, onlyPrimitive, parts);
}
- public static Object getObjectByPath(Map root, boolean onlyPrimitive, List<String> hierarchy) {
+
+
+ public static Object getObjectByPath(Object root, boolean onlyPrimitive, List<String> hierarchy) {
if(root == null) return null;
- Map obj = root;
+ if(!isMapLike(root)) throw new RuntimeException("must be a Map or NamedList");
+ Object obj = root;
for (int i = 0; i < hierarchy.size(); i++) {
int idx = -1;
String s = hierarchy.get(i);
@@ -233,22 +238,22 @@ public class Utils {
}
}
if (i < hierarchy.size() - 1) {
- Object o = obj.get(s);
+ Object o = getVal(obj, s);
if (o == null) return null;
if (idx > -1) {
List l = (List) o;
o = idx < l.size() ? l.get(idx) : null;
}
- if (!(o instanceof Map)) return null;
- obj = (Map) o;
+ if (!isMapLike(o)) return null;
+ obj = o;
} else {
- Object val = obj.get(s);
+ Object val = getVal(obj, s);
if (val == null) return null;
if (idx > -1) {
List l = (List) val;
val = idx < l.size() ? l.get(idx) : null;
}
- if (onlyPrimitive && val instanceof Map) {
+ if (onlyPrimitive && isMapLike(val)) {
return null;
}
return val;
@@ -257,7 +262,17 @@ public class Utils {
return false;
}
-
+
+ private static boolean isMapLike(Object o) {
+ return o instanceof Map || o instanceof NamedList;
+ }
+
+ private static Object getVal(Object obj, String key) {
+ if(obj instanceof NamedList) return ((NamedList) obj).get(key);
+ else if (obj instanceof Map) return ((Map) obj).get(key);
+ else throw new RuntimeException("must be a NamedList or Map");
+ }
+
/**
* If the passed entity has content, make sure it is fully
* read and closed.