You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sa...@apache.org on 2018/06/22 03:21:39 UTC
[1/2] lucene-solr:master: SOLR-12482: Config API returns status 0 for
failed operations
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7x 5b5b09c83 -> 53d24f722
refs/heads/master 2ea416ee3 -> 25e7631b9
SOLR-12482: Config API returns status 0 for failed operations
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/25e7631b
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/25e7631b
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/25e7631b
Branch: refs/heads/master
Commit: 25e7631b9014a5d0729be7926313c498df1dc606
Parents: 2ea416e
Author: Steve Rowe <sa...@apache.org>
Authored: Thu Jun 21 23:21:13 2018 -0400
Committer: Steve Rowe <sa...@apache.org>
Committed: Thu Jun 21 23:21:13 2018 -0400
----------------------------------------------------------------------
solr/CHANGES.txt | 2 ++
.../apache/solr/handler/SolrConfigHandler.java | 11 +++---
.../apache/solr/core/TestSolrConfigHandler.java | 38 ++++++++++++++++++++
.../client/solrj/request/TestV2Request.java | 4 ++-
4 files changed, 47 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/25e7631b/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 960baf2..215c128 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -104,6 +104,8 @@ Bug Fixes
* SOLR-12413: If Zookeeper was pre-loaded with data before first-use, then the aliases information would be ignored.
(David Smiley, Gaël Jourdan, Gus Heck)
+
+* SOLR-12482: Config API returns status 0 for failed operations. (Steve Rowe)
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/25e7631b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
index ca9b69d..53d543f 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -425,8 +425,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
List errs = CommandOperation.captureErrors(ops);
if (!errs.isEmpty()) {
- resp.add(CommandOperation.ERR_MSGS, errs);
- return;
+ throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing params", errs);
}
SolrResourceLoader loader = req.getCore().getResourceLoader();
@@ -489,9 +488,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
}
List errs = CommandOperation.captureErrors(ops);
if (!errs.isEmpty()) {
- log.info("Failed to run commands. errors are {}", StrUtils.join(errs, ','));
- resp.add(CommandOperation.ERR_MSGS, errs);
- return;
+ throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing commands", errs);
}
SolrResourceLoader loader = req.getCore().getResourceLoader();
@@ -633,7 +630,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
try {
val = Integer.parseInt(val.toString());
} catch (Exception exp) {
- op.addError(formatString(typeErr, typ.getSimpleName()));
+ op.addError(formatString(typeErr, name, typ.getSimpleName()));
continue;
}
@@ -641,7 +638,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
try {
val = Float.parseFloat(val.toString());
} catch (Exception exp) {
- op.addError(formatString(typeErr, typ.getSimpleName()));
+ op.addError(formatString(typeErr, name, typ.getSimpleName()));
continue;
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/25e7631b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
index d2c9b80..88286e0 100644
--- a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
+++ b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
@@ -186,6 +186,19 @@ public class TestSolrConfigHandler extends RestTestBase {
assertNull(response, map.get("errors")); // Will this ever be returned?
}
+ public static void runConfigCommandExpectFailure(RestTestHarness harness, String uri, String payload, String expectedErrorMessage) throws Exception {
+ String json = SolrTestCaseJ4.json(payload);
+ log.info("going to send config command. path {} , payload: {}", uri, payload);
+ String response = harness.post(uri, json);
+ Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
+ assertNotNull(response, map.get("errorMessages"));
+ assertNotNull(response, map.get("error"));
+ assertTrue("Expected status != 0: " + response, 0L != (Long)((Map)map.get("responseHeader")).get("status"));
+ List errorDetails = (List)((Map)map.get("error")).get("details");
+ List errorMessages = (List)((Map)errorDetails.get(0)).get("errorMessages");
+ assertTrue("Expected '" + expectedErrorMessage + "': " + response,
+ errorMessages.get(0).toString().contains(expectedErrorMessage));
+ }
public static void reqhandlertests(RestTestHarness writeHarness, String testServerBaseUrl, CloudSolrClient cloudSolrClient) throws Exception {
String payload = "{\n" +
@@ -500,6 +513,31 @@ public class TestSolrConfigHandler extends RestTestBase {
assertEquals("Actual output "+ Utils.toJSONString(map), "org.apache.solr.search.LFUCache",getObjectByPath(map, true, ImmutableList.of( "caches", "lfuCacheDecayFalse")));
}
+
+ public void testFailures() throws Exception {
+ String payload = "{ not-a-real-command: { param1: value1, param2: value2 } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "Unknown operation 'not-a-real-command'");
+
+ payload = "{ set-property: { update.autoCreateFields: false } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "'update.autoCreateFields' is not an editable property");
+
+ payload = "{ set-property: { updateHandler.autoCommit.maxDocs: false } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "Property updateHandler.autoCommit.maxDocs must be of Integer type");
+
+ payload = "{ unset-property: not-an-editable-property }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "'[not-an-editable-property]' is not an editable property");
+
+ for (String component : new String[] {
+ "requesthandler", "searchcomponent", "initparams", "queryresponsewriter", "queryparser",
+ "valuesourceparser", "transformer", "updateprocessor", "queryconverter", "listener", "runtimelib"}) {
+ for (String operation : new String[] { "add", "update" }) {
+ payload = "{ " + operation + "-" + component + ": { param1: value1 } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "'name' is a required field");
+ }
+ payload = "{ delete-" + component + ": not-a-real-component-name }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "NO such ");
+ }
+ }
public static class CacheTest extends DumpRequestHandler {
@Override
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/25e7631b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
index 0280a9b..12b3271 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
@@ -85,9 +85,11 @@ public class TestV2Request extends SolrCloudTestCase {
assertSuccess(client, new V2Request.Builder("/c/_introspect").build());
+ String requestHandlerName = "/x" + random().nextInt();
assertSuccess(client, new V2Request.Builder("/c/test/config")
.withMethod(SolrRequest.METHOD.POST)
- .withPayload("{'create-requesthandler' : { 'name' : '/x', 'class': 'org.apache.solr.handler.DumpRequestHandler' , 'startup' : 'lazy'}}")
+ .withPayload("{'create-requesthandler' : { 'name' : '" + requestHandlerName +
+ "', 'class': 'org.apache.solr.handler.DumpRequestHandler' , 'startup' : 'lazy'}}")
.build());
assertSuccess(client, new V2Request.Builder("/c/test").withMethod(SolrRequest.METHOD.DELETE).build());
[2/2] lucene-solr:branch_7x: SOLR-12482: Config API returns status 0
for failed operations
Posted by sa...@apache.org.
SOLR-12482: Config API returns status 0 for failed operations
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/53d24f72
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/53d24f72
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/53d24f72
Branch: refs/heads/branch_7x
Commit: 53d24f722d30e43a276ffdcaae72f2fcbb289943
Parents: 5b5b09c
Author: Steve Rowe <sa...@apache.org>
Authored: Thu Jun 21 23:21:13 2018 -0400
Committer: Steve Rowe <sa...@apache.org>
Committed: Thu Jun 21 23:21:30 2018 -0400
----------------------------------------------------------------------
solr/CHANGES.txt | 2 ++
.../apache/solr/handler/SolrConfigHandler.java | 11 +++---
.../apache/solr/core/TestSolrConfigHandler.java | 38 ++++++++++++++++++++
.../client/solrj/request/TestV2Request.java | 4 ++-
4 files changed, 47 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/53d24f72/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fe34998..a556fd9 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -75,6 +75,8 @@ Bug Fixes
* SOLR-12413: If Zookeeper was pre-loaded with data before first-use, then the aliases information would be ignored.
(David Smiley, Gaël Jourdan, Gus Heck)
+
+* SOLR-12482: Config API returns status 0 for failed operations. (Steve Rowe)
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/53d24f72/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
index ca9b69d..53d543f 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -425,8 +425,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
List errs = CommandOperation.captureErrors(ops);
if (!errs.isEmpty()) {
- resp.add(CommandOperation.ERR_MSGS, errs);
- return;
+ throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing params", errs);
}
SolrResourceLoader loader = req.getCore().getResourceLoader();
@@ -489,9 +488,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
}
List errs = CommandOperation.captureErrors(ops);
if (!errs.isEmpty()) {
- log.info("Failed to run commands. errors are {}", StrUtils.join(errs, ','));
- resp.add(CommandOperation.ERR_MSGS, errs);
- return;
+ throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing commands", errs);
}
SolrResourceLoader loader = req.getCore().getResourceLoader();
@@ -633,7 +630,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
try {
val = Integer.parseInt(val.toString());
} catch (Exception exp) {
- op.addError(formatString(typeErr, typ.getSimpleName()));
+ op.addError(formatString(typeErr, name, typ.getSimpleName()));
continue;
}
@@ -641,7 +638,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
try {
val = Float.parseFloat(val.toString());
} catch (Exception exp) {
- op.addError(formatString(typeErr, typ.getSimpleName()));
+ op.addError(formatString(typeErr, name, typ.getSimpleName()));
continue;
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/53d24f72/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
index d2c9b80..88286e0 100644
--- a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
+++ b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
@@ -186,6 +186,19 @@ public class TestSolrConfigHandler extends RestTestBase {
assertNull(response, map.get("errors")); // Will this ever be returned?
}
+ public static void runConfigCommandExpectFailure(RestTestHarness harness, String uri, String payload, String expectedErrorMessage) throws Exception {
+ String json = SolrTestCaseJ4.json(payload);
+ log.info("going to send config command. path {} , payload: {}", uri, payload);
+ String response = harness.post(uri, json);
+ Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
+ assertNotNull(response, map.get("errorMessages"));
+ assertNotNull(response, map.get("error"));
+ assertTrue("Expected status != 0: " + response, 0L != (Long)((Map)map.get("responseHeader")).get("status"));
+ List errorDetails = (List)((Map)map.get("error")).get("details");
+ List errorMessages = (List)((Map)errorDetails.get(0)).get("errorMessages");
+ assertTrue("Expected '" + expectedErrorMessage + "': " + response,
+ errorMessages.get(0).toString().contains(expectedErrorMessage));
+ }
public static void reqhandlertests(RestTestHarness writeHarness, String testServerBaseUrl, CloudSolrClient cloudSolrClient) throws Exception {
String payload = "{\n" +
@@ -500,6 +513,31 @@ public class TestSolrConfigHandler extends RestTestBase {
assertEquals("Actual output "+ Utils.toJSONString(map), "org.apache.solr.search.LFUCache",getObjectByPath(map, true, ImmutableList.of( "caches", "lfuCacheDecayFalse")));
}
+
+ public void testFailures() throws Exception {
+ String payload = "{ not-a-real-command: { param1: value1, param2: value2 } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "Unknown operation 'not-a-real-command'");
+
+ payload = "{ set-property: { update.autoCreateFields: false } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "'update.autoCreateFields' is not an editable property");
+
+ payload = "{ set-property: { updateHandler.autoCommit.maxDocs: false } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "Property updateHandler.autoCommit.maxDocs must be of Integer type");
+
+ payload = "{ unset-property: not-an-editable-property }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "'[not-an-editable-property]' is not an editable property");
+
+ for (String component : new String[] {
+ "requesthandler", "searchcomponent", "initparams", "queryresponsewriter", "queryparser",
+ "valuesourceparser", "transformer", "updateprocessor", "queryconverter", "listener", "runtimelib"}) {
+ for (String operation : new String[] { "add", "update" }) {
+ payload = "{ " + operation + "-" + component + ": { param1: value1 } }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "'name' is a required field");
+ }
+ payload = "{ delete-" + component + ": not-a-real-component-name }";
+ runConfigCommandExpectFailure(restTestHarness, "/config", payload, "NO such ");
+ }
+ }
public static class CacheTest extends DumpRequestHandler {
@Override
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/53d24f72/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
index 0280a9b..12b3271 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
@@ -85,9 +85,11 @@ public class TestV2Request extends SolrCloudTestCase {
assertSuccess(client, new V2Request.Builder("/c/_introspect").build());
+ String requestHandlerName = "/x" + random().nextInt();
assertSuccess(client, new V2Request.Builder("/c/test/config")
.withMethod(SolrRequest.METHOD.POST)
- .withPayload("{'create-requesthandler' : { 'name' : '/x', 'class': 'org.apache.solr.handler.DumpRequestHandler' , 'startup' : 'lazy'}}")
+ .withPayload("{'create-requesthandler' : { 'name' : '" + requestHandlerName +
+ "', 'class': 'org.apache.solr.handler.DumpRequestHandler' , 'startup' : 'lazy'}}")
.build());
assertSuccess(client, new V2Request.Builder("/c/test").withMethod(SolrRequest.METHOD.DELETE).build());