SOLR-13827: fail on unknown operation in Request Parameters API

solr/CHANGES.txt

@@ -112,7 +112,7 @@ Optimizations
 
 Bug Fixes
 ---------------------
-(No changes)
+* SOLR-13827: Fail on unknown operation in Request Parameters API (Munendra S N, noble)
 
 Other Changes
 ---------------------

solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java

@@ -152,7 +152,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
     NamedList configSetProperties = core.getConfigSetProperties();
     if(configSetProperties == null) return false;
     Object immutable = configSetProperties.get(IMMUTABLE_CONFIGSET_ARG);
-    return immutable != null ? Boolean.parseBoolean(immutable.toString()) : false;
+    return immutable != null && Boolean.parseBoolean(immutable.toString());
   }
 
 
@@ -292,13 +292,13 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
         pluginInfo = ((PluginInfo) plugin).toMap(new LinkedHashMap<>());
       }
       String useParams = (String) pluginInfo.get(USEPARAM);
-      String useparamsInReq = req.getOriginalParams().get(USEPARAM);
+      String useParamsInReq = req.getOriginalParams().get(USEPARAM);
-      if (useParams != null || useparamsInReq != null) {
+      if (useParams != null || useParamsInReq != null) {
         Map m = new LinkedHashMap<>();
         pluginInfo.put("_useParamsExpanded_", m);
         List<String> params = new ArrayList<>();
         if (useParams != null) params.addAll(StrUtils.splitSmart(useParams, ','));
-        if (useparamsInReq != null) params.addAll(StrUtils.splitSmart(useparamsInReq, ','));
+        if (useParamsInReq != null) params.addAll(StrUtils.splitSmart(useParamsInReq, ','));
         for (String param : params) {
           RequestParams.ParamSet p = this.req.getCore().getSolrConfig().getRequestParams().getParams(param);
           if (p != null) {
@@ -414,10 +414,14 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
             if (op.hasError()) break;
             for (String s : name) {
               if (params.getParams(s) == null) {
-                op.addError(formatString("can't delete . No such params ''{0}'' exist", s));
+                op.addError(formatString("Could not delete. No such params ''{0}'' exist", s));
               }
               params = params.setParams(s, null);
             }
+            break;
+          }
+          default: {
+            op.unknownOperation();
           }
         }
       }
@@ -503,7 +507,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
       } else {
         SolrResourceLoader.persistConfLocally(loader, ConfigOverlay.RESOURCE_NAME, overlay.toByteArray());
         req.getCore().getCoreContainer().reload(req.getCore().getName());
-        log.info("Executed config commands successfully and persited to File System {}", ops);
+        log.info("Executed config commands successfully and persisted to File System {}", ops);
       }
 
     }
@@ -769,7 +773,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
 
         if (!success) {
           String coreUrl = concurrentTasks.get(f).coreUrl;
-          log.warn("Core " + coreUrl + "could not get the expected version " + expectedVersion);
+          log.warn("Core " + coreUrl + " could not get the expected version " + expectedVersion);
           if (failedList == null) failedList = new ArrayList<>();
           failedList.add(coreUrl);
         }

solr/core/src/test/org/apache/solr/handler/TestReqParamsAPI.java

@@ -297,5 +297,15 @@ public class TestReqParamsAPI extends SolrCloudTestCase {
         asList("response", "params", "y", "p"),
         null,
         10);
+
+    payload = " {'unset' : 'y'}";
+    TestSolrConfigHandler.runConfigCommandExpectFailure(
+        writeHarness,"/config/params", payload, "Unknown operation 'unset'");
+
+    // deleting already deleted one should fail
+    // error message should contain parameter set name
+    payload = " {'delete' : 'y'}";
+    TestSolrConfigHandler.runConfigCommandExpectFailure(
+        writeHarness,"/config/params", payload, "Could not delete. No such params 'y' exist");
   }
 }

solr/core/src/test/org/apache/solr/handler/TestSolrConfigHandlerCloud.java

@@ -262,6 +262,15 @@ public class TestSolrConfigHandlerCloud extends AbstractFullDistribZkTestBase {
         null,
         TIMEOUT_S);
 
+    payload = " {'unset' : 'y'}";
+    TestSolrConfigHandler.runConfigCommandExpectFailure(
+        writeHarness,"/config/params", payload, "Unknown operation 'unset'");
+
+    // deleting already deleted one should fail
+    // error message should contain parameter set name
+    payload = " {'delete' : 'y'}";
+    TestSolrConfigHandler.runConfigCommandExpectFailure(
+        writeHarness,"/config/params", payload, "Could not delete. No such params 'y' exist");
 
   }
 

solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java

@@ -368,7 +368,7 @@ public class StrUtils {
   }
 
   /**
-   * Format using MesssageFormat but with the ROOT locale
+   * Format using {@link MessageFormat} but with the ROOT locale
    */
   public static String formatString(String pattern, Object... args) {
     return new MessageFormat(pattern, Locale.ROOT).format(args);