This is an automated email from the ASF dual-hosted git repository. mdrob pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit a28d2337e31b7405084737950ee853589b0de762 Author: Mike Drob <[email protected]> AuthorDate: Fri Feb 18 12:22:56 2022 -0600 SOLR-15333 Fix SpatialRecursivePrefixTreeFieldType schema definition warnings (#645) Co-authored-by: Steffen Moldenhauer <[email protected]> Co-authored-by: David Smiley <[email protected]> (cherry picked from commit 7f620c7a10006f344ff660d28b17298372f102fd) --- solr/CHANGES.txt | 2 + .../schema/AbstractSpatialPrefixTreeFieldType.java | 30 +++++++------- .../java/org/apache/solr/schema/IndexSchema.java | 2 +- .../java/org/apache/solr/schema/SchemaManager.java | 19 +++++---- .../solr/util/plugin/AbstractPluginLoader.java | 6 +-- .../apache/solr/rest/schema/TestBulkSchemaAPI.java | 47 ++++++++++++++++++++++ 6 files changed, 80 insertions(+), 26 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 100be4b..094c9f6 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -647,6 +647,8 @@ Bug Fixes * SOLR-15983: Fix ClassCastException in UpdateLog$LogReplayer.doReplay. (Christine Poerschke, David Smiley) +* SOLR-15333: Reduced spurious warn logging by AbstractSpatialPrefixTreeFieldType field properties (Steffen Moldenhauer, David Smiley, Mike Drob) + ================== 8.11.2 ================== Bug Fixes diff --git a/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java b/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java index f7b9912..4f15e00 100644 --- a/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java +++ b/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java @@ -72,22 +72,19 @@ public abstract class AbstractSpatialPrefixTreeFieldType<T extends PrefixTreeStr for (Map.Entry<String, String> entry : FIELD_TYPE_INVARIANTS.entrySet()) { final String key = entry.getKey(); final String hardcodedValue = entry.getValue(); - final String userConfiguredValue = args.get(entry.getKey()); - - if (args.containsKey(key)) { - if (userConfiguredValue.equals(hardcodedValue)) { - log.warn("FieldType {} does not allow {} to be specified in schema, hardcoded behavior is {}={}", - getClass().getSimpleName(), key, key, hardcodedValue); - } else { - final String message = String.format(Locale.ROOT, "FieldType %s is incompatible with %s=%s; hardcoded " + - "behavior is %s=%s. Remove specification in schema", - getClass().getSimpleName(), key, userConfiguredValue, key, hardcodedValue); - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message); - } + final String userConfiguredValue = args.remove(key); + + if (hardcodedValue.equals(userConfiguredValue)) { + // If this is coming from the schema API, can we pass it back to the user without causing a failure? + log.warn("FieldType {} does not allow {} to be specified in schema, hardcoded behavior is {}={}", + getClass().getSimpleName(), key, key, hardcodedValue); + } else if (userConfiguredValue != null) { + final String message = String.format(Locale.ROOT, + "FieldType %s is incompatible with %s=%s; hardcoded behavior is %s=%s. Remove specification in schema", + getClass().getSimpleName(), key, userConfiguredValue, key, hardcodedValue); + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message); } } - args.putAll(FIELD_TYPE_INVARIANTS); - super.setArgs(schema, args); } @@ -95,6 +92,11 @@ public abstract class AbstractSpatialPrefixTreeFieldType<T extends PrefixTreeStr protected void init(IndexSchema schema, Map<String, String> args) { super.init(schema, args); + //set STORE_ property flags to be always false + properties &= ~(STORE_TERMPOSITIONS | STORE_TERMOFFSETS); + //set OMIT_ property flags to be always true + properties |= (OMIT_TF_POSITIONS | OMIT_NORMS | OMIT_POSITIONS); + args.putIfAbsent(SpatialPrefixTreeFactory.VERSION, schema.getDefaultLuceneMatchVersion().toString()); // Convert the maxDistErr to degrees (based on distanceUnits) since Lucene spatial layer depends on degrees 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 1fb0de7..ba1a565 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -765,7 +765,7 @@ public class IndexSchema { for (Map.Entry<SchemaField, Integer> entry : copyFieldTargetCounts.entrySet()) { if (entry.getValue() > 1 && !entry.getKey().multiValued()) { - log.warn("Field {} is not multivalued and destination for multiople {} ({})" + log.warn("Field {} is not multivalued and destination for multiple {} ({})" , entry.getKey().name, COPY_FIELDS, entry.getValue()); } } 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 03bdde2..da909bc 100644 --- a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java +++ b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java @@ -186,7 +186,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.addFieldTypes(singletonList(fieldType), false); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not add field type.", e); op.addError(getErrorStr(e)); return false; } @@ -221,7 +221,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.addCopyFields(src, dests, maxChars); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not add copy field", e); op.addError(getErrorStr(e)); return false; } @@ -239,7 +239,7 @@ public class SchemaManager { = mgr.managedIndexSchema.addFields(singletonList(field), Collections.emptyMap(), false); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not add field", e); op.addError(getErrorStr(e)); return false; } @@ -257,7 +257,7 @@ public class SchemaManager { = mgr.managedIndexSchema.addDynamicFields(singletonList(field), Collections.emptyMap(), false); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not add dynamic field", e); op.addError(getErrorStr(e)); return false; } @@ -276,7 +276,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.deleteFieldTypes(singleton(name)); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not delete field type", e); op.addError(getErrorStr(e)); return false; } @@ -297,6 +297,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.deleteCopyFields(singletonMap(source, dests)); return true; } catch (Exception e) { + log.error("Could not delete copy field", e); op.addError(getErrorStr(e)); return false; } @@ -315,6 +316,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.deleteFields(singleton(name)); return true; } catch (Exception e) { + log.error("Could not delete field", e); op.addError(getErrorStr(e)); return false; } @@ -333,7 +335,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.deleteDynamicFields(singleton(name)); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not delete dynamic field", e); op.addError(getErrorStr(e)); return false; } @@ -349,7 +351,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.replaceFieldType(name, className, op.getDataMap()); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not replace field type", e); op.addError(getErrorStr(e)); return false; } @@ -370,7 +372,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.replaceField(name, ft, op.getValuesExcluding(NAME, TYPE)); return true; } catch (Exception e) { - log.error("err", e); + log.error("Could not replace field", e); op.addError(getErrorStr(e)); return false; } @@ -391,6 +393,7 @@ public class SchemaManager { mgr.managedIndexSchema = mgr.managedIndexSchema.replaceDynamicField(name, ft, op.getValuesExcluding(NAME, TYPE)); return true; } catch (Exception e) { + log.error("Could not replace dynamic field", e); op.addError(getErrorStr(e)); return false; } diff --git a/solr/core/src/java/org/apache/solr/util/plugin/AbstractPluginLoader.java b/solr/core/src/java/org/apache/solr/util/plugin/AbstractPluginLoader.java index 4dc386d..73fc5b9 100644 --- a/solr/core/src/java/org/apache/solr/util/plugin/AbstractPluginLoader.java +++ b/solr/core/src/java/org/apache/solr/util/plugin/AbstractPluginLoader.java @@ -257,10 +257,10 @@ public abstract class AbstractPluginLoader<T> for (PluginInitInfo pinfo : info) { try { init(pinfo.plugin, pinfo.node); + } catch (SolrException e) { + throw new SolrException(ErrorCode.getErrorCode(e.code()), "Plugin init failure for " + type, e); } catch (Exception ex) { - SolrException e = new SolrException - (ErrorCode.SERVER_ERROR, "Plugin init failure for " + type, ex); - throw e; + throw new SolrException(ErrorCode.SERVER_ERROR, "Plugin init failure for " + type, ex); } } return plugin; 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 aa0b868..960b83a 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 @@ -38,8 +38,10 @@ 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; +import org.apache.solr.schema.AbstractSpatialPrefixTreeFieldType; import org.apache.solr.schema.SimilarityFactory; import org.apache.solr.search.similarities.SchemaSimilarityFactory; +import org.apache.solr.util.LogListener; import org.apache.solr.util.RESTfulServerProvider; import org.apache.solr.util.RestTestBase; import org.apache.solr.util.RestTestHarness; @@ -50,6 +52,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import static org.apache.solr.common.util.Utils.fromJSONString; +import static org.hamcrest.Matchers.containsString; public class TestBulkSchemaAPI extends RestTestBase { @@ -1179,6 +1182,50 @@ public class TestBulkSchemaAPI extends RestTestBase { assertEquals(1, size); } + @Test + public void testDateRangeFieldDefaults() throws Exception { + try (LogListener listener = LogListener.warn(AbstractSpatialPrefixTreeFieldType.class)) { + // Add a default date range field and verify success + assertJPost("/schema", "{\n" + + " \"add-field-type\":{\n" + + " \"name\":\"dr-field1\",\n" + + " \"class\":\"solr.DateRangeField\"}\n" + + "}", "/responseHeader/status==0"); + assertJQ( + "/schema/fieldtypes/dr-field1", + "fieldType=={\n" + + " \"name\":\"dr-field1\",\n" + + " \"class\":\"solr.DateRangeField\"\n" + + "}"); + + assertEquals(0, listener.getCount()); + + // Add a date range field with redundant invariants + assertJPost("/schema", "{\n" + + " \"add-field-type\":{\n" + + " \"name\":\"dr-redundant\",\n" + + " \"omitNorms\":\"true\",\n" + + " \"termOffsets\":\"false\",\n" + + " \"class\":\"solr.DateRangeField\"}\n" + + "}", "/responseHeader/status==0"); + + assertEquals(2, listener.getCount()); + assertThat(listener.pollMessage(), containsString("hardcoded behavior is omitNorms=true")); + assertThat(listener.pollMessage(), containsString("hardcoded behavior is termOffsets=false")); + + // Add a date range field with violated invariants + assertJPost("/schema", "{\n" + + " \"add-field-type\":{\n" + + " \"name\":\"dr-invalid\",\n" + + " \"omitNorms\":\"false\",\n" + + " \"termOffsets\":\"true\",\n" + + " \"class\":\"solr.DateRangeField\"}\n" + + "}", "/responseHeader/status==400"); + + // could assert no more listener events but listener's close() gives us a better error message + } + } + public void testSimilarityParser() throws Exception { RestTestHarness harness = restTestHarness;
