From f007cde66df424fd24e0c6a9be8d61c9acf3108b Mon Sep 17 00:00:00 2001 From: Grant Ingersoll Date: Wed, 10 Jul 2013 10:55:38 +0000 Subject: [PATCH] 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 --- .../schema/CopyFieldCollectionResource.java | 16 ++----- .../rest/schema/FieldCollectionResource.java | 24 +--------- .../solr/rest/schema/FieldResource.java | 18 ++------ .../TestManagedSchemaFieldResource.java | 45 ++++++++----------- 4 files changed, 27 insertions(+), 76 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/rest/schema/CopyFieldCollectionResource.java b/solr/core/src/java/org/apache/solr/rest/schema/CopyFieldCollectionResource.java index 4813887fecc..c12fb601631 100644 --- a/solr/core/src/java/org/apache/solr/rest/schema/CopyFieldCollectionResource.java +++ b/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 destinations = (List)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 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){ diff --git a/solr/core/src/java/org/apache/solr/rest/schema/FieldCollectionResource.java b/solr/core/src/java/org/apache/solr/rest/schema/FieldCollectionResource.java index 6465d4080ff..110ed277062 100644 --- a/solr/core/src/java/org/apache/solr/rest/schema/FieldCollectionResource.java +++ b/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 copyTo = (List) map.get(IndexSchema.COPY_FIELDS); if (copyTo != null) { map.remove(IndexSchema.COPY_FIELDS); - String[] splits = copyTo.split(","); - Set 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); diff --git a/solr/core/src/java/org/apache/solr/rest/schema/FieldResource.java b/solr/core/src/java/org/apache/solr/rest/schema/FieldResource.java index cb8e463ae7c..0a3a32e25c6 100644 --- a/solr/core/src/java/org/apache/solr/rest/schema/FieldResource.java +++ b/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 copyFieldNames = Collections.emptySet(); - if (copyTo != null) { + List copyFieldNames = (List) 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); diff --git a/solr/core/src/test/org/apache/solr/rest/schema/TestManagedSchemaFieldResource.java b/solr/core/src/test/org/apache/solr/rest/schema/TestManagedSchemaFieldResource.java index b624016e9ca..cd408cf82f4 100644 --- a/solr/core/src/test/org/apache/solr/rest/schema/TestManagedSchemaFieldResource.java +++ b/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.\""); } }