From 95e54196fc327a454d757bfee59be29f242caa5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20W=C3=B6ckinger?= Date: Fri, 4 Oct 2019 13:05:01 -0400 Subject: [PATCH] SOLR-13795: Managed schema should do a core reload in standalone mode. Fixes #902 (cherry picked from commit 22e96697de1d9bc710f6e68e94885460106528bc) --- solr/CHANGES.txt | 3 + .../org/apache/solr/schema/IndexSchema.java | 108 ++++++++---------- .../solr/schema/ManagedIndexSchema.java | 40 +++---- .../org/apache/solr/schema/SchemaManager.java | 1 + .../solr/rest/schema/TestBulkSchemaAPI.java | 24 +++- .../solr/client/solrj/request/SchemaTest.java | 10 +- 6 files changed, 97 insertions(+), 89 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 85fd954fe1b..52557b88ce3 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -135,6 +135,9 @@ Improvements * SOLR-13771: Add -v and -m to ulimit section of reference guide and bin/solr checks (Erick Erickson) +* SOLR-13795: Managed schema operations should do a core reload in Solr standalone mode. + (Thomas Wöckinger via David Smiley) + Bug Fixes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index 980be8d6007..02168f180a7 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -138,7 +138,7 @@ public class IndexSchema { protected List fieldsWithDefaultValue = new ArrayList<>(); protected Collection requiredFields = new HashSet<>(); - protected volatile DynamicField[] dynamicFields; + protected DynamicField[] dynamicFields = new DynamicField[] {}; public DynamicField[] getDynamicFields() { return dynamicFields; } protected Map dynamicFieldCache = new ConcurrentHashMap<>(); @@ -151,7 +151,7 @@ public class IndexSchema { protected Map> copyFieldsMap = new HashMap<>(); public Map> getCopyFieldsMap() { return Collections.unmodifiableMap(copyFieldsMap); } - protected DynamicCopy[] dynamicCopyFields; + protected DynamicCopy[] dynamicCopyFields = new DynamicCopy[] {}; public DynamicCopy[] getDynamicCopyFields() { return dynamicCopyFields; } private Map decoders = new HashMap<>(); // cache to avoid scanning token filters repeatedly, unnecessarily @@ -962,18 +962,12 @@ public class IndexSchema { private void incrementCopyFieldTargetCount(SchemaField dest) { copyFieldTargetCounts.put(dest, copyFieldTargetCounts.containsKey(dest) ? copyFieldTargetCounts.get(dest) + 1 : 1); } - - private void registerDynamicCopyField( DynamicCopy dcopy ) { - if( dynamicCopyFields == null ) { - dynamicCopyFields = new DynamicCopy[] {dcopy}; - } - else { - DynamicCopy[] temp = new DynamicCopy[dynamicCopyFields.length+1]; - System.arraycopy(dynamicCopyFields,0,temp,0,dynamicCopyFields.length); - temp[temp.length -1] = dcopy; - dynamicCopyFields = temp; - } - log.trace("Dynamic Copy Field:" + dcopy); + + private void registerDynamicCopyField(DynamicCopy dcopy) { + DynamicCopy[] temp = new DynamicCopy[dynamicCopyFields.length + 1]; + System.arraycopy(dynamicCopyFields, 0, temp, 0, dynamicCopyFields.length); + temp[temp.length - 1] = dcopy; + dynamicCopyFields = temp; } static SimilarityFactory readSimilarity(SolrResourceLoader loader, Node node) { @@ -1337,11 +1331,9 @@ public class IndexSchema { } } } - if (null != dynamicCopyFields) { - for (DynamicCopy dynamicCopy : dynamicCopyFields) { - if (dynamicCopy.getDestFieldName().equals(destField)) { - fieldNames.add(dynamicCopy.getRegex()); - } + for (DynamicCopy dynamicCopy : dynamicCopyFields) { + if (dynamicCopy.getDestFieldName().equals(destField)) { + fieldNames.add(dynamicCopy.getRegex()); } } return fieldNames; @@ -1356,11 +1348,9 @@ public class IndexSchema { // This is useful when we need the maxSize param of each CopyField public List getCopyFieldsList(final String sourceField){ final List result = new ArrayList<>(); - if (null != dynamicCopyFields) { - for (DynamicCopy dynamicCopy : dynamicCopyFields) { - if (dynamicCopy.matches(sourceField)) { - result.add(new CopyField(getField(sourceField), dynamicCopy.getTargetField(sourceField), dynamicCopy.maxChars)); - } + for (DynamicCopy dynamicCopy : dynamicCopyFields) { + if (dynamicCopy.matches(sourceField)) { + result.add(new CopyField(getField(sourceField), dynamicCopy.getTargetField(sourceField), dynamicCopy.maxChars)); } } List fixedCopyFields = copyFieldsMap.get(sourceField); @@ -1556,48 +1546,46 @@ public class IndexSchema { } } } - if (null != dynamicCopyFields) { - for (IndexSchema.DynamicCopy dynamicCopy : dynamicCopyFields) { - final String source = dynamicCopy.getRegex(); - final String destination = dynamicCopy.getDestFieldName(); - if ((null == requestedSourceFields || requestedSourceFields.contains(source)) - && (null == requestedDestinationFields || requestedDestinationFields.contains(destination))) { - SimpleOrderedMap dynamicCopyProps = new SimpleOrderedMap<>(); + for (IndexSchema.DynamicCopy dynamicCopy : dynamicCopyFields) { + final String source = dynamicCopy.getRegex(); + final String destination = dynamicCopy.getDestFieldName(); + if ((null == requestedSourceFields || requestedSourceFields.contains(source)) + && (null == requestedDestinationFields || requestedDestinationFields.contains(destination))) { + SimpleOrderedMap dynamicCopyProps = new SimpleOrderedMap<>(); - dynamicCopyProps.add(SOURCE, dynamicCopy.getRegex()); - if (showDetails) { - IndexSchema.DynamicField sourceDynamicBase = dynamicCopy.getSourceDynamicBase(); - if (null != sourceDynamicBase) { - dynamicCopyProps.add(SOURCE_DYNAMIC_BASE, sourceDynamicBase.getRegex()); - } else if (source.contains("*")) { - List sourceExplicitFields = new ArrayList<>(); - Pattern pattern = Pattern.compile(source.replace("*", ".*")); // glob->regex - for (String field : fields.keySet()) { - if (pattern.matcher(field).matches()) { - sourceExplicitFields.add(field); - } - } - if (sourceExplicitFields.size() > 0) { - Collections.sort(sourceExplicitFields); - dynamicCopyProps.add(SOURCE_EXPLICIT_FIELDS, sourceExplicitFields); + dynamicCopyProps.add(SOURCE, dynamicCopy.getRegex()); + if (showDetails) { + IndexSchema.DynamicField sourceDynamicBase = dynamicCopy.getSourceDynamicBase(); + if (null != sourceDynamicBase) { + dynamicCopyProps.add(SOURCE_DYNAMIC_BASE, sourceDynamicBase.getRegex()); + } else if (source.contains("*")) { + List sourceExplicitFields = new ArrayList<>(); + Pattern pattern = Pattern.compile(source.replace("*", ".*")); // glob->regex + for (String field : fields.keySet()) { + if (pattern.matcher(field).matches()) { + sourceExplicitFields.add(field); } } - } - - dynamicCopyProps.add(DESTINATION, dynamicCopy.getDestFieldName()); - if (showDetails) { - IndexSchema.DynamicField destDynamicBase = dynamicCopy.getDestDynamicBase(); - if (null != destDynamicBase) { - dynamicCopyProps.add(DESTINATION_DYNAMIC_BASE, destDynamicBase.getRegex()); + if (sourceExplicitFields.size() > 0) { + Collections.sort(sourceExplicitFields); + dynamicCopyProps.add(SOURCE_EXPLICIT_FIELDS, sourceExplicitFields); } } - - if (0 != dynamicCopy.getMaxChars()) { - dynamicCopyProps.add(MAX_CHARS, dynamicCopy.getMaxChars()); - } - - copyFieldProperties.add(dynamicCopyProps); } + + dynamicCopyProps.add(DESTINATION, dynamicCopy.getDestFieldName()); + if (showDetails) { + IndexSchema.DynamicField destDynamicBase = dynamicCopy.getDestDynamicBase(); + if (null != destDynamicBase) { + dynamicCopyProps.add(DESTINATION_DYNAMIC_BASE, destDynamicBase.getRegex()); + } + } + + if (0 != dynamicCopy.getMaxChars()) { + dynamicCopyProps.add(MAX_CHARS, dynamicCopy.getMaxChars()); + } + + copyFieldProperties.add(dynamicCopyProps); } } return copyFieldProperties; diff --git a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java index c7fbf276be5..57b0c90e90b 100644 --- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java @@ -81,7 +81,7 @@ import org.xml.sax.InputSource; /** Solr-managed schema - non-user-editable, but can be mutable via internal and external REST API requests. */ public final class ManagedIndexSchema extends IndexSchema { - private boolean isMutable = false; + private final boolean isMutable; @Override public boolean isMutable() { return isMutable; } @@ -654,7 +654,7 @@ public final class ManagedIndexSchema extends IndexSchema { System.arraycopy(newSchema.dynamicFields, dfPos + 1, temp, dfPos, newSchema.dynamicFields.length - dfPos - 1); newSchema.dynamicFields = temp; } else { - newSchema.dynamicFields = new DynamicField[0]; + newSchema.dynamicFields = new DynamicField[] {}; } } // After removing all dynamic fields, rebuild affected dynamic copy fields. @@ -840,26 +840,24 @@ public final class ManagedIndexSchema extends IndexSchema { boolean found = false; if (null == destSchemaField || null == sourceSchemaField) { // Must be dynamic copy field - if (dynamicCopyFields != null) { - for (int i = 0 ; i < dynamicCopyFields.length ; ++i) { - DynamicCopy dynamicCopy = dynamicCopyFields[i]; - if (source.equals(dynamicCopy.getRegex()) && dest.equals(dynamicCopy.getDestFieldName())) { - found = true; - SchemaField destinationPrototype = dynamicCopy.getDestination().getPrototype(); - if (copyFieldTargetCounts.containsKey(destinationPrototype)) { - decrementCopyFieldTargetCount(destinationPrototype); - } - if (dynamicCopyFields.length > 1) { - DynamicCopy[] temp = new DynamicCopy[dynamicCopyFields.length - 1]; - System.arraycopy(dynamicCopyFields, 0, temp, 0, i); - // skip over the dynamic copy field to be deleted - System.arraycopy(dynamicCopyFields, i + 1, temp, i, dynamicCopyFields.length - i - 1); - dynamicCopyFields = temp; - } else { - dynamicCopyFields = null; - } - break; + for (int i = 0; i < dynamicCopyFields.length; ++i) { + DynamicCopy dynamicCopy = dynamicCopyFields[i]; + if (source.equals(dynamicCopy.getRegex()) && dest.equals(dynamicCopy.getDestFieldName())) { + found = true; + SchemaField destinationPrototype = dynamicCopy.getDestination().getPrototype(); + if (copyFieldTargetCounts.containsKey(destinationPrototype)) { + decrementCopyFieldTargetCount(destinationPrototype); } + if (dynamicCopyFields.length > 1) { + DynamicCopy[] temp = new DynamicCopy[dynamicCopyFields.length - 1]; + System.arraycopy(dynamicCopyFields, 0, temp, 0, i); + // skip over the dynamic copy field to be deleted + System.arraycopy(dynamicCopyFields, i + 1, temp, i, dynamicCopyFields.length - i - 1); + dynamicCopyFields = temp; + } else { + dynamicCopyFields = new DynamicCopy[] {}; + } + break; } } } else { // non-dynamic copy field directive diff --git a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java index afc3b04fe89..31a7206ed8e 100644 --- a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java +++ b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java @@ -138,6 +138,7 @@ public class SchemaManager { //only for non cloud stuff managedIndexSchema.persistManagedSchema(false); core.setLatestSchema(managedIndexSchema); + core.getCoreContainer().reload(core.getName()); } catch (SolrException e) { log.warn(errorMsg); errors = singletonList(errorMsg + e.getMessage()); 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 95132e9c38a..81cc005bd9a 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 @@ -20,6 +20,7 @@ import java.io.File; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -32,6 +33,8 @@ import org.apache.lucene.search.similarities.BM25Similarity; import org.apache.lucene.search.similarities.DFISimilarity; import org.apache.lucene.search.similarities.PerFieldSimilarityWrapper; import org.apache.lucene.search.similarities.Similarity; +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.request.schema.SchemaRequest; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; @@ -43,6 +46,7 @@ import org.apache.solr.util.RestTestBase; import org.apache.solr.util.RestTestHarness; import org.junit.After; import org.junit.Before; +import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -184,8 +188,6 @@ public class TestBulkSchemaAPI extends RestTestBase { response = restTestHarness.post("/schema", json(addFieldTypeAnalyzerWithClass + suffix)); map = (Map) fromJSONString(response); assertNull(response, map.get("error")); - - restTestHarness.checkAdminResponseStatus("/admin/cores?wt=xml&action=RELOAD&core=" + coreName, "0"); map = getObj(restTestHarness, "myNewTextFieldWithAnalyzerClass", "fieldTypes"); assertNotNull(map); @@ -901,6 +903,24 @@ public class TestBulkSchemaAPI extends RestTestBase { } + @Test + public void testAddNewFieldAndQuery() throws Exception { + getSolrClient().add(Arrays.asList( + sdoc("id", "1", "term_s", "tux"))); + + getSolrClient().commit(true, true); + Map attrs = new HashMap<>(); + attrs.put("name", "newstringtestfield"); + attrs.put("type", "string"); + + new SchemaRequest.AddField(attrs).process(getSolrClient()); + + SolrQuery query = new SolrQuery("*:*"); + query.addFacetField("newstringtestfield"); + int size = getSolrClient().query(query).getResults().size(); + assertEquals(1, size); + } + public void testSimilarityParser() throws Exception { RestTestHarness harness = restTestHarness; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/SchemaTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/SchemaTest.java index 3f08722ee6f..09235bcd480 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/request/SchemaTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/request/SchemaTest.java @@ -16,6 +16,10 @@ */ package org.apache.solr.client.solrj.request; +import static org.hamcrest.CoreMatchers.anyOf; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; + import java.io.File; import java.util.ArrayList; import java.util.Arrays; @@ -47,10 +51,6 @@ import org.junit.Before; import org.junit.Test; import org.restlet.ext.servlet.ServerServlet; -import static org.hamcrest.CoreMatchers.anyOf; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.is; - /** * Test the functionality (accuracy and failure) of the methods exposed by the classes * {@link SchemaRequest} and {@link SchemaResponse}. @@ -622,8 +622,6 @@ public class SchemaTest extends RestTestBase { SchemaResponse.UpdateResponse addFieldTypeResponse = addFieldTypeRequest.process(getSolrClient()); assertValidSchemaResponse(addFieldTypeResponse); - restTestHarness.reload(); - SchemaRequest.FieldType fieldTypeRequest = new SchemaRequest.FieldType(fieldTypeName); SchemaResponse.FieldTypeResponse newFieldTypeResponse = fieldTypeRequest.process(getSolrClient()); assertValidSchemaResponse(newFieldTypeResponse);