You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sa...@apache.org on 2018/01/09 02:25:52 UTC
[1/2] lucene-solr:master: SOLR-11631: The Schema API should return
non-zero status when there are failures
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7x 9ac376a0c -> 34b30da60
refs/heads/master d189b5870 -> 9f221796f
SOLR-11631: The Schema API should return non-zero status when there are failures
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/9f221796
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/9f221796
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/9f221796
Branch: refs/heads/master
Commit: 9f221796fe1b79ead6509efdcaa0a17c5a382c65
Parents: d189b58
Author: Steve Rowe <sa...@apache.org>
Authored: Mon Jan 8 21:25:14 2018 -0500
Committer: Steve Rowe <sa...@apache.org>
Committed: Mon Jan 8 21:25:14 2018 -0500
----------------------------------------------------------------------
solr/CHANGES.txt | 3 +
.../org/apache/solr/handler/SchemaHandler.java | 12 ++-
.../solr/core/TestConfigSetImmutable.java | 6 +-
.../solr/rest/schema/TestBulkSchemaAPI.java | 91 ++++++++++++--------
.../solr/schema/SchemaApiFailureTest.java | 66 ++++++++++++++
5 files changed, 131 insertions(+), 47 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9f221796/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 31197a0..533748f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -92,6 +92,9 @@ Bug Fixes
* SOLR-11821: ConcurrentModificationException in SimSolrCloudTestCase.tearDown (shalin)
+* SOLR-11631: The Schema API should return non-zero status when there are failures.
+ (Noble Paul, Steve Rowe)
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9f221796/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
index e3e292b..fb84e84 100644
--- a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
@@ -80,20 +80,18 @@ public class SchemaHandler extends RequestHandlerBase implements SolrCoreAware,
String httpMethod = (String) req.getContext().get("httpMethod");
if ("POST".equals(httpMethod)) {
if (isImmutableConfigSet) {
- rsp.add("errors", "ConfigSet is immutable");
- return;
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "ConfigSet is immutable");
}
if (req.getContentStreams() == null) {
- rsp.add("errors", "no stream");
- return;
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "no stream");
}
try {
List errs = new SchemaManager(req).performOperations();
- if (!errs.isEmpty()) rsp.add("errors", errs);
+ if (!errs.isEmpty())
+ throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing commands", errs);
} catch (IOException e) {
- rsp.add("errors", Collections.singletonList("Error reading input String " + e.getMessage()));
- rsp.setException(e);
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Error reading input String " + e.getMessage(), e);
}
} else {
handleGET(req, rsp);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9f221796/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java b/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
index 2388b3d..b9f456c 100644
--- a/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
+++ b/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
@@ -93,8 +93,10 @@ public class TestConfigSetImmutable extends RestTestBase {
String response = restTestHarness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(map.get("errors"));
- assertTrue(map.get("errors").toString().contains("immutable"));
+ Map error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ String msg = (String)error.get("msg");
+ assertTrue(msg.contains("immutable"));
}
@Test
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9f221796/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java b/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
index 4dcfc66..4bebeca 100644
--- a/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
+++ b/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
@@ -109,14 +109,17 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = restTestHarness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- List l = (List) map.get("errors");
- assertNotNull("No errors", l);
- List errorList = (List) ((Map) l.get(0)).get("errorMessages");
- assertEquals(1, errorList.size());
- assertTrue (((String)errorList.get(0)).contains("Field 'a1': Field type 'string1' not found.\n"));
- errorList = (List) ((Map) l.get(1)).get("errorMessages");
- assertEquals(1, errorList.size());
- assertTrue (((String)errorList.get(0)).contains("is a required field"));
+ Map error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ List details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 2, details.size());
+ List firstErrorList = (List)((Map)details.get(0)).get("errorMessages");
+ assertEquals(1, firstErrorList.size());
+ assertTrue (((String)firstErrorList.get(0)).contains("Field 'a1': Field type 'string1' not found.\n"));
+ List secondErrorList = (List)((Map)details.get(1)).get("errorMessages");
+ assertEquals(1, secondErrorList.size());
+ assertTrue (((String)secondErrorList.get(0)).contains("is a required field"));
}
public void testAnalyzerClass() throws Exception {
@@ -145,8 +148,12 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = restTestHarness.post("/schema",
json(addFieldTypeAnalyzerWithClass + ',' + charFilters + tokenizer + filters + suffix));
Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- List list = (List)map.get("errors");
- List errorList = (List)((Map)list.get(0)).get("errorMessages");
+ Map error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ List details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 1, details.size());
+ List errorList = (List)((Map)details.get(0)).get("errorMessages");
assertEquals(1, errorList.size());
assertTrue (((String)errorList.get(0)).contains
("An analyzer with a class property may not define any char filters!"));
@@ -154,8 +161,12 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = restTestHarness.post("/schema",
json(addFieldTypeAnalyzerWithClass + ',' + tokenizer + filters + suffix));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- list = (List)map.get("errors");
- errorList = (List)((Map)list.get(0)).get("errorMessages");
+ error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 1, details.size());
+ errorList = (List)((Map)details.get(0)).get("errorMessages");
assertEquals(1, errorList.size());
assertTrue (((String)errorList.get(0)).contains
("An analyzer with a class property may not define a tokenizer!"));
@@ -163,15 +174,19 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = restTestHarness.post("/schema",
json(addFieldTypeAnalyzerWithClass + ',' + filters + suffix));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- list = (List)map.get("errors");
- errorList = (List)((Map)list.get(0)).get("errorMessages");
+ error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 1, details.size());
+ errorList = (List)((Map)details.get(0)).get("errorMessages");
assertEquals(1, errorList.size());
assertTrue (((String)errorList.get(0)).contains
("An analyzer with a class property may not define any filters!"));
response = restTestHarness.post("/schema", json(addFieldTypeAnalyzerWithClass + suffix));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(restTestHarness, "myNewTextFieldWithAnalyzerClass", "fieldTypes");
assertNotNull(map);
@@ -206,7 +221,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, newFieldName, "fields");
assertNotNull("Field '" + newFieldName + "' is not in the schema", map);
@@ -228,7 +243,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(response, map.get("errors"));
+ assertNotNull(response, map.get("error"));
map = getObj(harness, newFieldName, "dynamicFields");
assertNull(newFieldName + " illegal dynamic field should not have been added to schema", map);
@@ -251,7 +266,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(response, map.get("errors"));
+ assertNotNull(response, map.get("error"));
map = getObj(harness, newFieldName, "fields");
assertNull(newFieldName + " illegal dynamic field should not have been added to schema", map);
@@ -273,7 +288,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(response, map.get("errors"));
+ assertNotNull(response, map.get("error"));
}
public void testAddFieldWithExistingCatchallDynamicField() throws Exception {
@@ -305,7 +320,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, "*", "dynamicFields");
assertNotNull("Dynamic field '*' is not in the schema", map);
@@ -322,7 +337,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, newFieldName, "fields");
assertNotNull("Field '" + newFieldName + "' is not in the schema", map);
@@ -502,7 +517,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
m = getObj(harness, "a1", "fields");
assertNotNull("field a1 not created", m);
@@ -639,7 +654,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, "NewFieldType", "fieldTypes");
assertNotNull("'NewFieldType' is not in the schema", map);
@@ -693,14 +708,14 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-field-type' : {'name':'NewFieldType'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- Object errors = map.get("errors");
+ Object errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains("Can't delete 'NewFieldType' because it's the field type of "));
cmds = "{'delete-field' : {'name':'NewField1'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains
("Can't delete field 'NewField1' because it's referred to by at least one copy field directive"));
@@ -708,7 +723,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-field' : {'name':'NewField2'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains
("Can't delete field 'NewField2' because it's referred to by at least one copy field directive"));
@@ -716,7 +731,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-field' : {'name':'NewField1', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
// Make sure the copy field directives with source NewField1 are preserved
list = getSourceCopyFields(harness, "NewField1");
set = new HashSet();
@@ -730,7 +745,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-dynamic-field' : {'name':'NewDynamicField1*'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains
("copyField dest :'NewDynamicField1A' is not an explicit field and doesn't match a dynamicField."));
@@ -738,7 +753,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-field' : {'name':'NewField2', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNull(errors);
// Make sure the copy field directives with destination NewField2 are preserved
list = getDestCopyFields(harness, "NewField2");
@@ -755,7 +770,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-dynamic-field' : {'name':'NewDynamicField2*', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNull(errors);
// Make sure the copy field directives with source NewDynamicField2* are preserved
list = getSourceCopyFields(harness, "NewDynamicField2*");
@@ -765,7 +780,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-dynamic-field' : {'name':'NewDynamicField1*', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNull(errors);
// Make sure the copy field directives with destinations matching NewDynamicField1* are preserved
list = getDestCopyFields(harness, "NewDynamicField1A");
@@ -775,7 +790,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-field-type': {'name':'NewFieldType', 'class':'solr.BinaryField'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
// Make sure the copy field directives with sources and destinations of type NewFieldType are preserved
list = getDestCopyFields(harness, "NewField3");
assertEquals(2, list.size());
@@ -795,7 +810,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
"}\n";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
list = getSourceCopyFields(harness, "NewField1");
assertEquals(0, list.size());
list = getSourceCopyFields(harness, "NewDynamicField1*");
@@ -810,7 +825,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-field': [{'name':'NewField1'},{'name':'NewField2'},{'name':'NewField3'},{'name':'NewField4'}]}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
cmds = "{'delete-dynamic-field': [{'name':'NewDynamicField1*'}," +
" {'name':'NewDynamicField2*'},\n" +
@@ -818,12 +833,12 @@ public class TestBulkSchemaAPI extends RestTestBase {
"}\n";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
cmds = "{'delete-field-type':{'name':'NewFieldType'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
}
public void testSimilarityParser() throws Exception {
@@ -852,7 +867,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
Map fields = getObj(harness, fieldName, "fields");
assertNotNull("field " + fieldName + " not created", fields);
@@ -879,7 +894,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
fields = getObj(harness, fieldName, "fields");
assertNotNull("field " + fieldName + " not created", fields);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9f221796/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java b/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java
new file mode 100644
index 0000000..8456787
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.schema;
+
+import java.lang.invoke.MethodHandles;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.schema.SchemaRequest;
+import org.apache.solr.client.solrj.response.schema.SchemaResponse;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.util.Utils;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SchemaApiFailureTest extends SolrCloudTestCase {
+ private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String COLLECTION = "schema-api-failure";
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(1).configure();
+ CollectionAdminRequest.createCollection(COLLECTION, 2, 1) // _default configset
+ .setMaxShardsPerNode(2)
+ .process(cluster.getSolrClient());
+ cluster.getSolrClient().waitForState(COLLECTION, DEFAULT_TIMEOUT, TimeUnit.SECONDS,
+ (n, c) -> DocCollection.isFullyActive(n, c, 2, 1));
+ }
+
+ @Test
+ public void testAddTheSameFieldTwice() throws Exception {
+ CloudSolrClient client = cluster.getSolrClient();
+ SchemaRequest.Update fieldAddition = new SchemaRequest.AddField
+ (Utils.makeMap("name","myfield", "type","string"));
+ SchemaResponse.UpdateResponse updateResponse = fieldAddition.process(client, COLLECTION);
+
+ HttpSolrClient.RemoteExecutionException ex = expectThrows(HttpSolrClient.RemoteExecutionException.class,
+ () -> fieldAddition.process(client, COLLECTION));
+
+ assertTrue("expected error message 'Field 'myfield' already exists'.",Utils.getObjectByPath(ex.getMetaData(), false, "error/details[0]/errorMessages[0]").toString().contains("Field 'myfield' already exists.") );
+
+ }
+
+
+}
[2/2] lucene-solr:branch_7x: SOLR-11631: The Schema API should return
non-zero status when there are failures
Posted by sa...@apache.org.
SOLR-11631: The Schema API should return non-zero status when there are failures
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/34b30da6
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/34b30da6
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/34b30da6
Branch: refs/heads/branch_7x
Commit: 34b30da60cc4b6f9ed0a528d470eb075871db6f7
Parents: 9ac376a
Author: Steve Rowe <sa...@apache.org>
Authored: Mon Jan 8 21:25:14 2018 -0500
Committer: Steve Rowe <sa...@apache.org>
Committed: Mon Jan 8 21:25:42 2018 -0500
----------------------------------------------------------------------
solr/CHANGES.txt | 3 +
.../org/apache/solr/handler/SchemaHandler.java | 12 ++-
.../solr/core/TestConfigSetImmutable.java | 6 +-
.../solr/rest/schema/TestBulkSchemaAPI.java | 91 ++++++++++++--------
.../solr/schema/SchemaApiFailureTest.java | 66 ++++++++++++++
5 files changed, 131 insertions(+), 47 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34b30da6/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 2d676eb..753d6dc 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -72,6 +72,9 @@ Bug Fixes
* SOLR-11821: ConcurrentModificationException in SimSolrCloudTestCase.tearDown (shalin)
+* SOLR-11631: The Schema API should return non-zero status when there are failures.
+ (Noble Paul, Steve Rowe)
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34b30da6/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
index e3e292b..fb84e84 100644
--- a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
@@ -80,20 +80,18 @@ public class SchemaHandler extends RequestHandlerBase implements SolrCoreAware,
String httpMethod = (String) req.getContext().get("httpMethod");
if ("POST".equals(httpMethod)) {
if (isImmutableConfigSet) {
- rsp.add("errors", "ConfigSet is immutable");
- return;
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "ConfigSet is immutable");
}
if (req.getContentStreams() == null) {
- rsp.add("errors", "no stream");
- return;
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "no stream");
}
try {
List errs = new SchemaManager(req).performOperations();
- if (!errs.isEmpty()) rsp.add("errors", errs);
+ if (!errs.isEmpty())
+ throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing commands", errs);
} catch (IOException e) {
- rsp.add("errors", Collections.singletonList("Error reading input String " + e.getMessage()));
- rsp.setException(e);
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Error reading input String " + e.getMessage(), e);
}
} else {
handleGET(req, rsp);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34b30da6/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java b/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
index 2388b3d..b9f456c 100644
--- a/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
+++ b/solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java
@@ -93,8 +93,10 @@ public class TestConfigSetImmutable extends RestTestBase {
String response = restTestHarness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(map.get("errors"));
- assertTrue(map.get("errors").toString().contains("immutable"));
+ Map error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ String msg = (String)error.get("msg");
+ assertTrue(msg.contains("immutable"));
}
@Test
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34b30da6/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java b/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
index 4dcfc66..4bebeca 100644
--- a/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
+++ b/solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java
@@ -109,14 +109,17 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = restTestHarness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- List l = (List) map.get("errors");
- assertNotNull("No errors", l);
- List errorList = (List) ((Map) l.get(0)).get("errorMessages");
- assertEquals(1, errorList.size());
- assertTrue (((String)errorList.get(0)).contains("Field 'a1': Field type 'string1' not found.\n"));
- errorList = (List) ((Map) l.get(1)).get("errorMessages");
- assertEquals(1, errorList.size());
- assertTrue (((String)errorList.get(0)).contains("is a required field"));
+ Map error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ List details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 2, details.size());
+ List firstErrorList = (List)((Map)details.get(0)).get("errorMessages");
+ assertEquals(1, firstErrorList.size());
+ assertTrue (((String)firstErrorList.get(0)).contains("Field 'a1': Field type 'string1' not found.\n"));
+ List secondErrorList = (List)((Map)details.get(1)).get("errorMessages");
+ assertEquals(1, secondErrorList.size());
+ assertTrue (((String)secondErrorList.get(0)).contains("is a required field"));
}
public void testAnalyzerClass() throws Exception {
@@ -145,8 +148,12 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = restTestHarness.post("/schema",
json(addFieldTypeAnalyzerWithClass + ',' + charFilters + tokenizer + filters + suffix));
Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- List list = (List)map.get("errors");
- List errorList = (List)((Map)list.get(0)).get("errorMessages");
+ Map error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ List details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 1, details.size());
+ List errorList = (List)((Map)details.get(0)).get("errorMessages");
assertEquals(1, errorList.size());
assertTrue (((String)errorList.get(0)).contains
("An analyzer with a class property may not define any char filters!"));
@@ -154,8 +161,12 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = restTestHarness.post("/schema",
json(addFieldTypeAnalyzerWithClass + ',' + tokenizer + filters + suffix));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- list = (List)map.get("errors");
- errorList = (List)((Map)list.get(0)).get("errorMessages");
+ error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 1, details.size());
+ errorList = (List)((Map)details.get(0)).get("errorMessages");
assertEquals(1, errorList.size());
assertTrue (((String)errorList.get(0)).contains
("An analyzer with a class property may not define a tokenizer!"));
@@ -163,15 +174,19 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = restTestHarness.post("/schema",
json(addFieldTypeAnalyzerWithClass + ',' + filters + suffix));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- list = (List)map.get("errors");
- errorList = (List)((Map)list.get(0)).get("errorMessages");
+ error = (Map)map.get("error");
+ assertNotNull("No errors", error);
+ details = (List)error.get("details");
+ assertNotNull("No details", details);
+ assertEquals("Wrong number of details", 1, details.size());
+ errorList = (List)((Map)details.get(0)).get("errorMessages");
assertEquals(1, errorList.size());
assertTrue (((String)errorList.get(0)).contains
("An analyzer with a class property may not define any filters!"));
response = restTestHarness.post("/schema", json(addFieldTypeAnalyzerWithClass + suffix));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(restTestHarness, "myNewTextFieldWithAnalyzerClass", "fieldTypes");
assertNotNull(map);
@@ -206,7 +221,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, newFieldName, "fields");
assertNotNull("Field '" + newFieldName + "' is not in the schema", map);
@@ -228,7 +243,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(response, map.get("errors"));
+ assertNotNull(response, map.get("error"));
map = getObj(harness, newFieldName, "dynamicFields");
assertNull(newFieldName + " illegal dynamic field should not have been added to schema", map);
@@ -251,7 +266,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(response, map.get("errors"));
+ assertNotNull(response, map.get("error"));
map = getObj(harness, newFieldName, "fields");
assertNull(newFieldName + " illegal dynamic field should not have been added to schema", map);
@@ -273,7 +288,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNotNull(response, map.get("errors"));
+ assertNotNull(response, map.get("error"));
}
public void testAddFieldWithExistingCatchallDynamicField() throws Exception {
@@ -305,7 +320,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, "*", "dynamicFields");
assertNotNull("Dynamic field '*' is not in the schema", map);
@@ -322,7 +337,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, newFieldName, "fields");
assertNotNull("Field '" + newFieldName + "' is not in the schema", map);
@@ -502,7 +517,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
m = getObj(harness, "a1", "fields");
assertNotNull("field a1 not created", m);
@@ -639,7 +654,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
map = getObj(harness, "NewFieldType", "fieldTypes");
assertNotNull("'NewFieldType' is not in the schema", map);
@@ -693,14 +708,14 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-field-type' : {'name':'NewFieldType'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- Object errors = map.get("errors");
+ Object errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains("Can't delete 'NewFieldType' because it's the field type of "));
cmds = "{'delete-field' : {'name':'NewField1'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains
("Can't delete field 'NewField1' because it's referred to by at least one copy field directive"));
@@ -708,7 +723,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-field' : {'name':'NewField2'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains
("Can't delete field 'NewField2' because it's referred to by at least one copy field directive"));
@@ -716,7 +731,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-field' : {'name':'NewField1', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
// Make sure the copy field directives with source NewField1 are preserved
list = getSourceCopyFields(harness, "NewField1");
set = new HashSet();
@@ -730,7 +745,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-dynamic-field' : {'name':'NewDynamicField1*'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNotNull(errors);
assertTrue(errors.toString().contains
("copyField dest :'NewDynamicField1A' is not an explicit field and doesn't match a dynamicField."));
@@ -738,7 +753,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-field' : {'name':'NewField2', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNull(errors);
// Make sure the copy field directives with destination NewField2 are preserved
list = getDestCopyFields(harness, "NewField2");
@@ -755,7 +770,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-dynamic-field' : {'name':'NewDynamicField2*', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNull(errors);
// Make sure the copy field directives with source NewDynamicField2* are preserved
list = getSourceCopyFields(harness, "NewDynamicField2*");
@@ -765,7 +780,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-dynamic-field' : {'name':'NewDynamicField1*', 'type':'string'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- errors = map.get("errors");
+ errors = map.get("error");
assertNull(errors);
// Make sure the copy field directives with destinations matching NewDynamicField1* are preserved
list = getDestCopyFields(harness, "NewDynamicField1A");
@@ -775,7 +790,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'replace-field-type': {'name':'NewFieldType', 'class':'solr.BinaryField'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
// Make sure the copy field directives with sources and destinations of type NewFieldType are preserved
list = getDestCopyFields(harness, "NewField3");
assertEquals(2, list.size());
@@ -795,7 +810,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
"}\n";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
list = getSourceCopyFields(harness, "NewField1");
assertEquals(0, list.size());
list = getSourceCopyFields(harness, "NewDynamicField1*");
@@ -810,7 +825,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
cmds = "{'delete-field': [{'name':'NewField1'},{'name':'NewField2'},{'name':'NewField3'},{'name':'NewField4'}]}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
cmds = "{'delete-dynamic-field': [{'name':'NewDynamicField1*'}," +
" {'name':'NewDynamicField2*'},\n" +
@@ -818,12 +833,12 @@ public class TestBulkSchemaAPI extends RestTestBase {
"}\n";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
cmds = "{'delete-field-type':{'name':'NewFieldType'}}";
response = harness.post("/schema", json(cmds));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(map.get("errors"));
+ assertNull(map.get("error"));
}
public void testSimilarityParser() throws Exception {
@@ -852,7 +867,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
String response = harness.post("/schema", json(payload));
Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
Map fields = getObj(harness, fieldName, "fields");
assertNotNull("field " + fieldName + " not created", fields);
@@ -879,7 +894,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
response = harness.post("/schema", json(payload));
map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
- assertNull(response, map.get("errors"));
+ assertNull(response, map.get("error"));
fields = getObj(harness, fieldName, "fields");
assertNotNull("field " + fieldName + " not created", fields);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34b30da6/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java b/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java
new file mode 100644
index 0000000..8456787
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/schema/SchemaApiFailureTest.java
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.schema;
+
+import java.lang.invoke.MethodHandles;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.schema.SchemaRequest;
+import org.apache.solr.client.solrj.response.schema.SchemaResponse;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.util.Utils;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SchemaApiFailureTest extends SolrCloudTestCase {
+ private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String COLLECTION = "schema-api-failure";
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(1).configure();
+ CollectionAdminRequest.createCollection(COLLECTION, 2, 1) // _default configset
+ .setMaxShardsPerNode(2)
+ .process(cluster.getSolrClient());
+ cluster.getSolrClient().waitForState(COLLECTION, DEFAULT_TIMEOUT, TimeUnit.SECONDS,
+ (n, c) -> DocCollection.isFullyActive(n, c, 2, 1));
+ }
+
+ @Test
+ public void testAddTheSameFieldTwice() throws Exception {
+ CloudSolrClient client = cluster.getSolrClient();
+ SchemaRequest.Update fieldAddition = new SchemaRequest.AddField
+ (Utils.makeMap("name","myfield", "type","string"));
+ SchemaResponse.UpdateResponse updateResponse = fieldAddition.process(client, COLLECTION);
+
+ HttpSolrClient.RemoteExecutionException ex = expectThrows(HttpSolrClient.RemoteExecutionException.class,
+ () -> fieldAddition.process(client, COLLECTION));
+
+ assertTrue("expected error message 'Field 'myfield' already exists'.",Utils.getObjectByPath(ex.getMetaData(), false, "error/details[0]/errorMessages[0]").toString().contains("Field 'myfield' already exists.") );
+
+ }
+
+
+}