SOLR-9411: Better validation of dynamic field for Schema API

solr/CHANGES.txt

@@ -129,6 +129,8 @@ Bug Fixes
 
 * SOLR-9330: Fix AlreadyClosedException on admin/mbeans?stats=true (Mikhail Khludnev)
 
+* SOLR-9411: Better validation for Schema REST API add-dynamic-field (janhoy)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/schema/SchemaManager.java

@@ -254,7 +254,7 @@ public class SchemaManager {
           return  false;
         }
         try {
-          SchemaField field = SchemaField.create(name, ft, op.getValuesExcluding(NAME, TYPE));
+          SchemaField field = mgr.managedIndexSchema.newDynamicField(name, type, op.getValuesExcluding(NAME, TYPE));
           mgr.managedIndexSchema
               = mgr.managedIndexSchema.addDynamicFields(singletonList(field), Collections.emptyMap(), false);
           return true;

solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java

@@ -195,6 +195,28 @@ public class TestBulkSchemaAPI extends RestTestBase {
     assertNotNull("Field '" + newFieldName + "' is not in the schema", map);
   }
 
+  public void testAddIllegalDynamicField() throws Exception {
+    RestTestHarness harness = restTestHarness;
+
+    String newFieldName = "illegal";
+
+    String payload = "{\n" +
+        "    'add-dynamic-field' : {\n" +
+        "                 'name':'" + newFieldName + "',\n" +
+        "                 'type':'string',\n" +
+        "                 'stored':true,\n" +
+        "                 'indexed':true\n" +
+        "                 }\n" +
+        "    }";
+
+    String response = harness.post("/schema?wt=json", json(payload));
+    Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
+    assertNotNull(response, map.get("errors"));
+
+    map = getObj(harness, newFieldName, "dynamicFields");
+    assertNull(newFieldName + " illegal dynamic field should not have been added to schema", map);
+  }
+
   public void testAddFieldWithExistingCatchallDynamicField() throws Exception {
     RestTestHarness harness = restTestHarness;
 

solr/solrj/src/test/org/apache/solr/client/solrj/request/SchemaTest.java

@@ -54,8 +54,8 @@ import static org.hamcrest.CoreMatchers.is;
  */
 public class SchemaTest extends RestTestBase {
   private static void assertValidSchemaResponse(SolrResponseBase schemaResponse) {
-    assertEquals(0, schemaResponse.getStatus());
+    assertEquals("Response contained errors: " + schemaResponse.toString(), 0, schemaResponse.getStatus());
-    assertNull(schemaResponse.getResponse().get("errors"));
+    assertNull("Response contained errors: " + schemaResponse.toString(), schemaResponse.getResponse().get("errors"));
   }
 
   private static void createStoredStringField(String fieldName, SolrClient solrClient) throws Exception {
@@ -391,8 +391,7 @@ public class SchemaTest extends RestTestBase {
     fieldAttributes.put("type", "string");
     fieldAttributes.put("stored", false);
     fieldAttributes.put("indexed", true);
-    fieldAttributes.put("default", "accuracy");
+    // Dynamic fields cannot be required or have a default value
-    fieldAttributes.put("required", true);
     SchemaRequest.AddDynamicField addFieldUpdateSchemaRequest =
         new SchemaRequest.AddDynamicField(fieldAttributes);
     SchemaResponse.UpdateResponse addFieldResponse = addFieldUpdateSchemaRequest.process(getSolrClient());
@@ -412,8 +411,6 @@ public class SchemaTest extends RestTestBase {
     assertThat("string", is(equalTo(newFieldAttributes.get("type"))));
     assertThat(false, is(equalTo(newFieldAttributes.get("stored"))));
     assertThat(true, is(equalTo(newFieldAttributes.get("indexed"))));
-    assertThat("accuracy", is(equalTo(newFieldAttributes.get("default"))));
-    assertThat(true, is(equalTo(newFieldAttributes.get("required"))));
   }
 
   @Test
@@ -481,7 +478,6 @@ public class SchemaTest extends RestTestBase {
     fieldAttributes.put("type", "string");
     fieldAttributes.put("stored", false);
     fieldAttributes.put("indexed", true);
-    fieldAttributes.put("required", true);
     SchemaRequest.AddDynamicField addDFieldUpdateSchemaRequest =
         new SchemaRequest.AddDynamicField(fieldAttributes);
     SchemaResponse.UpdateResponse addFieldResponse = addDFieldUpdateSchemaRequest.process(getSolrClient());
@@ -506,7 +502,6 @@ public class SchemaTest extends RestTestBase {
     assertThat("string", is(equalTo(newFieldAttributes.get("type"))));
     assertThat(true, is(equalTo(newFieldAttributes.get("stored"))));
     assertThat(false, is(equalTo(newFieldAttributes.get("indexed"))));
-    assertThat(true, is(equalTo(newFieldAttributes.get("required"))));
   }
 
   @Test