SOLR-11631: The Schema API should return non-zero status when there are failures

This commit is contained in:
Steve Rowe 2018-01-08 21:25:14 -05:00
parent d189b58708
commit 9f221796fe
5 changed files with 131 additions and 47 deletions

View File

@ -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
----------------------

View File

@ -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);

View File

@ -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

View File

@ -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);

View File

@ -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.") );
}
}