SOLR-12482: Config API returns status 0 for failed operations

solr/CHANGES.txt

@@ -105,6 +105,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
 ----------------------
 

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);
+        throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing params", errs);
-        return;
       }
 
       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, ','));
+        throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing commands", errs);
-        resp.add(CommandOperation.ERR_MSGS, errs);
-        return;
       }
 
       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;
             }
 

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" +
@@ -501,6 +514,31 @@ public class TestSolrConfigHandler extends RestTestBase {
 
   }
   
+  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
     public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException {

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());