SOLR-5010: switch to lists for copy fields

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1501715 13f79535-47bb-0310-9956-ffa450edef68

solr/core/src/java/org/apache/solr/rest/schema/CopyFieldCollectionResource.java

@@ -136,21 +136,11 @@ public class CopyFieldCollectionResource extends BaseFieldResource implements GE
                 log.error(message);
                 throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message);
               }
-              String destinations = (String)map.get(IndexSchema.DESTINATION);
+              List<String> destinations = (List<String>)map.get(IndexSchema.DESTINATION);
               if (destinations == null) {
-                String message = "Missing '" + IndexSchema.DESTINATION + "' mapping.";
-                log.error(message);
-                throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message);
-              }
-              String [] splits = destinations.split(",");
-              Set<String> destinationSet = new HashSet<>();
-              if (splits != null && splits.length > 0){
-                for (int i = 0; i < splits.length; i++) {
-                  destinationSet.add(splits[i].trim());
-                }
-                fieldsToCopy.put(fieldName, destinationSet);
-              } else {
                 malformed.add(fieldName);
+              } else {
+                fieldsToCopy.put(fieldName, destinations);
               }
             }
             if (malformed.size() > 0){

solr/core/src/java/org/apache/solr/rest/schema/FieldCollectionResource.java

@@ -155,33 +155,13 @@ public class FieldCollectionResource extends BaseFieldResource implements GETabl
                 throw new SolrException(ErrorCode.BAD_REQUEST, message);
               }
               // copyFields:"comma separated list of destination fields"
-              String copyTo = (String) map.get(IndexSchema.COPY_FIELDS);
+              List<String> copyTo = (List<String>) map.get(IndexSchema.COPY_FIELDS);
               if (copyTo != null) {
                 map.remove(IndexSchema.COPY_FIELDS);
-                String[] splits = copyTo.split(",");
-                Set<String> destinations = new HashSet<>();
-                if (splits != null && splits.length > 0) {
-                  for (int i = 0; i < splits.length; i++) {
-                    destinations.add(splits[i].trim());
-                  }
-                  copyFields.put(fieldName, destinations);
-                } else{
-                  malformed.add(fieldName);
-                }
+                copyFields.put(fieldName, copyTo);
               }
               newFields.add(oldSchema.newField(fieldName, fieldType, map));
             }
-            if (malformed.size() > 0){
-              StringBuilder message = new StringBuilder("Malformed destination(s) for: ");
-              for (String s : malformed) {
-                message.append(s).append(", ");
-              }
-              if (message.length() > 2) {
-                message.setLength(message.length() - 2);//drop the last ,
-              }
-              log.error(message.toString().trim());
-              throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message.toString().trim());
-            }
             IndexSchema newSchema = oldSchema.addFields(newFields, copyFields);
 
             getSolrCore().setLatestSchema(newSchema);

solr/core/src/java/org/apache/solr/rest/schema/FieldResource.java

@@ -34,6 +34,7 @@ import java.io.UnsupportedEncodingException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -145,22 +146,9 @@ public class FieldResource extends BaseFieldResource implements GETable, PUTable
                 throw new SolrException(ErrorCode.BAD_REQUEST, message);
               } else {
                 ManagedIndexSchema oldSchema = (ManagedIndexSchema) getSchema();
-                String copyTo = (String) map.get(IndexSchema.COPY_FIELDS);
-                Collection<String> copyFieldNames = Collections.emptySet();
-                if (copyTo != null) {
+                List<String> copyFieldNames = (List<String>) map.get(IndexSchema.COPY_FIELDS);
+                if (copyFieldNames != null) {
                   map.remove(IndexSchema.COPY_FIELDS);
-                  String [] tmp = copyTo.split(",");
-                  if (tmp != null && tmp.length > 0) {
-                    copyFieldNames = new HashSet<>(tmp.length);
-                    for (int i = 0; i < tmp.length; i++) {
-                      copyFieldNames.add(tmp[i].trim());
-                    }
-                  } else {
-                    //the user specified copy fields, but then passed in something invalid
-                    String msg = "Invalid " + IndexSchema.COPY_FIELDS + " for field: " + fieldName;
-                    log.error(msg);
-                    throw new SolrException(ErrorCode.BAD_REQUEST, msg);
-                  }
                 }
                 SchemaField newField = oldSchema.newField(fieldName, fieldType, map);
                 IndexSchema newSchema = oldSchema.addField(newField, copyFieldNames);

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

@@ -113,7 +113,7 @@ public class TestManagedSchemaFieldResource extends RestTestBase {
         "{\"type\":\"text\",\"stored\":\"false\"}",
         "/responseHeader/status==0");
     assertJPut("/schema/fields/fieldB",
-        "{\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\"fieldA\"}",
+        "{\"type\":\"text\",\"stored\":\"false\", \"copyFields\":[\"fieldA\"]}",
         "/responseHeader/status==0");
 
     assertQ("/schema/fields/fieldB?indent=on&wt=xml",
@@ -122,12 +122,13 @@ public class TestManagedSchemaFieldResource extends RestTestBase {
     assertQ("/schema/copyfields/?indent=on&wt=xml&source.fl=fieldB",
         "count(/response/arr[@name='copyFields']/lst) = 1"
     );
+    //fine to pass in empty list, just won't do anything
+    assertJPut("/schema/fields/fieldD",
+        "{\"type\":\"text\",\"stored\":\"false\", \"copyFields\":[]}",
+        "/responseHeader/status==0");
     //some bad usages
-    assertJPut("/schema/fields/fieldB",
-        "{\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\",,,\"}",
-        "/error/msg==\"Invalid copyFields for field: fieldB\"");
     assertJPut("/schema/fields/fieldC",
-        "{\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\"some_nonexistent_field_ignore_exception\"}",
+        "{\"type\":\"text\",\"stored\":\"false\", \"copyFields\":[\"some_nonexistent_field_ignore_exception\"]}",
         "/error/msg==\"copyField dest :\\'some_nonexistent_field_ignore_exception\\' is not an explicit field and doesn\\'t match a dynamicField.\"");
   }
 
@@ -177,7 +178,7 @@ public class TestManagedSchemaFieldResource extends RestTestBase {
     assertJPost("/schema/fields",
               "[{\"name\":\"fieldA\",\"type\":\"text\",\"stored\":\"false\"},"
                + "{\"name\":\"fieldB\",\"type\":\"text\",\"stored\":\"false\"},"
-               + " {\"name\":\"fieldC\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\"fieldB\"}]",
+               + " {\"name\":\"fieldC\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":[\"fieldB\"]}]",
                 "/responseHeader/status==0");
     assertQ("/schema/copyfields/?indent=on&wt=xml&source.fl=fieldC",
         "count(/response/arr[@name='copyFields']/lst) = 1"
@@ -185,30 +186,23 @@ public class TestManagedSchemaFieldResource extends RestTestBase {
     assertJPost("/schema/fields",
               "[{\"name\":\"fieldD\",\"type\":\"text\",\"stored\":\"false\"},"
                + "{\"name\":\"fieldE\",\"type\":\"text\",\"stored\":\"false\"},"
-               + " {\"name\":\"fieldF\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\"fieldD,fieldE\"}]",
+               + " {\"name\":\"fieldF\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":[\"fieldD\",\"fieldE\"]}]",
                 "/responseHeader/status==0");
     assertQ("/schema/copyfields/?indent=on&wt=xml&source.fl=fieldF",
         "count(/response/arr[@name='copyFields']/lst) = 2"
     );
+    //passing in an empty list is perfectly acceptable, it just won't do anything
     assertJPost("/schema/fields",
-              "[{\"name\":\"fieldG\",\"type\":\"text\",\"stored\":\"false\"},"
-               + "{\"name\":\"fieldH\",\"type\":\"text\",\"stored\":\"false\"},"
-               + " {\"name\":\"fieldI\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\"fieldG,   fieldH   \"}]",
-                "/responseHeader/status==0");
-    assertQ("/schema/copyfields/?indent=on&wt=xml&source.fl=fieldF",
-        "count(/response/arr[@name='copyFields']/lst) = 2"
-    );
+        "[{\"name\":\"fieldX\",\"type\":\"text\",\"stored\":\"false\"},"
+            + "{\"name\":\"fieldY\",\"type\":\"text\",\"stored\":\"false\"},"
+            + " {\"name\":\"fieldZ\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":[]}]",
+        "/responseHeader/status==0");
     //some bad usages
-    assertJPost("/schema/fields",
-              "[{\"name\":\"fieldX\",\"type\":\"text\",\"stored\":\"false\"},"
-               + "{\"name\":\"fieldY\",\"type\":\"text\",\"stored\":\"false\"},"
-               + " {\"name\":\"fieldZ\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\",,,\"}]",
-                "/error/msg==\"Malformed destination(s) for: fieldZ\"");
 
     assertJPost("/schema/fields",
-              "[{\"name\":\"fieldX\",\"type\":\"text\",\"stored\":\"false\"},"
-               + "{\"name\":\"fieldY\",\"type\":\"text\",\"stored\":\"false\"},"
-               + " {\"name\":\"fieldZ\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":\"some_nonexistent_field_ignore_exception\"}]",
+              "[{\"name\":\"fieldH\",\"type\":\"text\",\"stored\":\"false\"},"
+               + "{\"name\":\"fieldI\",\"type\":\"text\",\"stored\":\"false\"},"
+               + " {\"name\":\"fieldJ\",\"type\":\"text\",\"stored\":\"false\", \"copyFields\":[\"some_nonexistent_field_ignore_exception\"]}]",
                 "/error/msg==\"copyField dest :\\'some_nonexistent_field_ignore_exception\\' is not an explicit field and doesn\\'t match a dynamicField.\"");
   }
 
@@ -221,14 +215,13 @@ public class TestManagedSchemaFieldResource extends RestTestBase {
                   + "{\"name\":\"fieldD\",\"type\":\"text\",\"stored\":\"false\"},"
                + " {\"name\":\"fieldE\",\"type\":\"text\",\"stored\":\"false\"}]",
                 "/responseHeader/status==0");
-    assertJPost("/schema/copyfields", "[{\"source\":\"fieldA\", \"dest\":\"fieldB\"},{\"source\":\"fieldD\", \"dest\":\"fieldC,   fieldE\"}]", "/responseHeader/status==0");
+    assertJPost("/schema/copyfields", "[{\"source\":\"fieldA\", \"dest\":[\"fieldB\"]},{\"source\":\"fieldD\", \"dest\":[\"fieldC\", \"fieldE\"]}]", "/responseHeader/status==0");
     assertQ("/schema/copyfields/?indent=on&wt=xml&source.fl=fieldA",
         "count(/response/arr[@name='copyFields']/lst) = 1");
     assertQ("/schema/copyfields/?indent=on&wt=xml&source.fl=fieldD",
         "count(/response/arr[@name='copyFields']/lst) = 2");
-    assertJPost("/schema/copyfields", "[{\"source\":\"fieldD\", \"dest\":\",,,\"}]", "/error/msg==\"Malformed destination(s) for: fieldD\"");
-    assertJPost("/schema/copyfields", "[{\"source\":\"some_nonexistent_field_ignore_exception\", \"dest\":\"fieldA\"}]", "/error/msg==\"copyField source :\\'some_nonexistent_field_ignore_exception\\' is not a glob and doesn\\'t match any explicit field or dynamicField.\"");
-    assertJPost("/schema/copyfields", "[{\"source\":\"fieldD\", \"dest\":\"some_nonexistent_field_ignore_exception\"}]", "/error/msg==\"copyField dest :\\'some_nonexistent_field_ignore_exception\\' is not an explicit field and doesn\\'t match a dynamicField.\"");
+    assertJPost("/schema/copyfields", "[{\"source\":\"some_nonexistent_field_ignore_exception\", \"dest\":[\"fieldA\"]}]", "/error/msg==\"copyField source :\\'some_nonexistent_field_ignore_exception\\' is not a glob and doesn\\'t match any explicit field or dynamicField.\"");
+    assertJPost("/schema/copyfields", "[{\"source\":\"fieldD\", \"dest\":[\"some_nonexistent_field_ignore_exception\"]}]", "/error/msg==\"copyField dest :\\'some_nonexistent_field_ignore_exception\\' is not an explicit field and doesn\\'t match a dynamicField.\"");
   }
 
 }