Prevent overriding built-in similarity and allow defining index global similarity

This merge commit merges #16682 which adds the ability to define a index-global
default similarity but at the same time prevents overriding built-in similarities
for new indices. Old indices where able to do this int the past which should not
be punished even though this is not possible anymore.

Closes #16594
Closes #16682

core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java

@@ -35,6 +35,7 @@ import org.elasticsearch.index.fielddata.IndexFieldDataService;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.percolator.PercolatorQueriesRegistry;
+import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.index.store.FsDirectoryService;
 import org.elasticsearch.index.store.IndexStore;
 import org.elasticsearch.index.store.Store;
@@ -44,6 +45,7 @@ import org.elasticsearch.indices.IndicesRequestCache;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Predicate;
 
@@ -133,8 +135,15 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
         FsDirectoryService.INDEX_LOCK_FACTOR_SETTING,
         EngineConfig.INDEX_CODEC_SETTING,
         IndexWarmer.INDEX_NORMS_LOADING_SETTING,
-        // this sucks but we can't really validate all the analyzers/similarity in here
+        // validate that built-in similarities don't get redefined
-        Setting.groupSetting("index.similarity.", false, Setting.Scope.INDEX), // this allows similarity settings to be passed
+        Setting.groupSetting("index.similarity.", false, Setting.Scope.INDEX, (s) -> {
+            Map<String, Settings> groups = s.getAsGroups();
+            for (String key : SimilarityService.BUILT_IN.keySet()) {
+                if (groups.containsKey(key)) {
+                    throw new IllegalArgumentException("illegal value for [index.similarity."+ key + "] cannot redefine built-in similarity");
+                }
+            }
+        }), // this allows similarity settings to be passed
         Setting.groupSetting("index.analysis.", false, Setting.Scope.INDEX) // this allows analysis settings to be passed
 
     )));

core/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -36,10 +36,12 @@ import org.elasticsearch.common.xcontent.XContentType;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -177,7 +179,7 @@ public class Setting<T> extends ToXContentToBytes {
     /**
      * Returns <code>true</code> iff this setting is present in the given settings object. Otherwise <code>false</code>
      */
-    public final boolean exists(Settings settings) {
+    public boolean exists(Settings settings) {
         return settings.get(getKey()) != null;
     }
 
@@ -505,17 +507,36 @@ public class Setting<T> extends ToXContentToBytes {
             throw new ElasticsearchException(ex);
         }
     }
-
     public static Setting<Settings> groupSetting(String key, boolean dynamic, Scope scope) {
+        return groupSetting(key, dynamic, scope, (s) -> {});
+    }
+    public static Setting<Settings> groupSetting(String key, boolean dynamic, Scope scope, Consumer<Settings> validator) {
         return new Setting<Settings>(new GroupKey(key), (s) -> "", (s) -> null, dynamic, scope) {
             @Override
             public boolean isGroupSetting() {
                 return true;
             }
 
+            @Override
+            public String getRaw(Settings settings) {
+                throw new UnsupportedOperationException("group settings don't support raw values");
+            }
+
             @Override
             public Settings get(Settings settings) {
-                return settings.getByPrefix(getKey());
+                Settings byPrefix = settings.getByPrefix(getKey());
+                validator.accept(byPrefix);
+                return byPrefix;
+            }
+
+            @Override
+            public boolean exists(Settings settings) {
+                for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {
+                    if (entry.getKey().startsWith(key)) {
+                        return true;
+                    }
+                }
+                return false;
             }
 
             @Override

core/src/main/java/org/elasticsearch/index/similarity/SimilarityService.java

@@ -21,6 +21,7 @@ package org.elasticsearch.index.similarity;
 
 import org.apache.lucene.search.similarities.PerFieldSimilarityWrapper;
 import org.apache.lucene.search.similarities.Similarity;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.AbstractIndexComponent;
 import org.elasticsearch.index.IndexModule;
@@ -63,6 +64,10 @@ public final class SimilarityService extends AbstractIndexComponent {
         Map<String, Settings> similaritySettings = this.indexSettings.getSettings().getGroups(IndexModule.SIMILARITY_SETTINGS_PREFIX);
         for (Map.Entry<String, Settings> entry : similaritySettings.entrySet()) {
             String name = entry.getKey();
+            // Starting with v5.0 indices, it should no longer be possible to redefine built-in similarities
+            if(BUILT_IN.containsKey(name) && indexSettings.getIndexVersionCreated().onOrAfter(Version.V_5_0_0)) {
+                throw new IllegalArgumentException("Cannot redefine built-in Similarity [" + name + "]");
+            }
             Settings settings = entry.getValue();
             String typeName = settings.get("type");
             if (typeName == null) {
@@ -76,9 +81,16 @@ public final class SimilarityService extends AbstractIndexComponent {
             }
             providers.put(name, factory.apply(name, settings));
         }
-        addSimilarities(similaritySettings, providers, DEFAULTS);
+        for (Map.Entry<String, SimilarityProvider> entry : addSimilarities(similaritySettings, DEFAULTS).entrySet()) {
+            // Avoid overwriting custom providers for indices older that v5.0
+            if (providers.containsKey(entry.getKey()) && indexSettings.getIndexVersionCreated().before(Version.V_5_0_0)) {
+                continue;
+            }
+            providers.put(entry.getKey(), entry.getValue());
+        }
         this.similarities = providers;
-        defaultSimilarity = providers.get(SimilarityService.DEFAULT_SIMILARITY).get();
+        defaultSimilarity = (providers.get("default") != null) ? providers.get("default").get()
+                                                              : providers.get(SimilarityService.DEFAULT_SIMILARITY).get();
         // Expert users can configure the base type as being different to default, but out-of-box we use default.
         baseSimilarity = (providers.get("base") != null) ? providers.get("base").get() :
                 defaultSimilarity;
@@ -90,7 +102,9 @@ public final class SimilarityService extends AbstractIndexComponent {
                 defaultSimilarity;
     }
 
-    private void addSimilarities(Map<String, Settings>  similaritySettings, Map<String, SimilarityProvider> providers, Map<String, BiFunction<String, Settings, SimilarityProvider>> similarities)  {
+    private Map<String, SimilarityProvider> addSimilarities(Map<String, Settings>  similaritySettings,
+                                 Map<String, BiFunction<String, Settings, SimilarityProvider>> similarities)  {
+        Map<String, SimilarityProvider> providers = new HashMap<>(similarities.size());
         for (Map.Entry<String, BiFunction<String, Settings, SimilarityProvider>> entry : similarities.entrySet()) {
             String name = entry.getKey();
             BiFunction<String, Settings, SimilarityProvider> factory = entry.getValue();
@@ -100,12 +114,17 @@ public final class SimilarityService extends AbstractIndexComponent {
             }
             providers.put(name, factory.apply(name, settings));
         }
+        return providers;
     }
 
     public SimilarityProvider getSimilarity(String name) {
         return similarities.get(name);
     }
 
+    public SimilarityProvider getDefaultSimilarity() {
+        return similarities.get("default");
+    }
+
     static class PerFieldSimilarity extends PerFieldSimilarityWrapper {
 
         private final Similarity defaultSimilarity;

core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -216,6 +216,13 @@ public class ScopedSettingsTests extends ESTestCase {
         } catch (IllegalArgumentException e) {
             assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage());
         }
+
+        try {
+            settings.validate("index.similarity.classic.type", Settings.builder().put("index.similarity.classic.type", "mine").build());
+            fail();
+        } catch (IllegalArgumentException e) {
+            assertEquals("illegal value for [index.similarity.classic] cannot redefine built-in similarity", e.getMessage());
+        }
     }
 
 

core/src/test/java/org/elasticsearch/index/similarity/SimilarityServiceTests.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.elasticsearch.index.similarity;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.IndexSettings;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.IndexSettingsModule;
+
+import java.util.Collections;
+
+public class SimilarityServiceTests extends ESTestCase {
+
+    // Tests #16594
+    public void testOverrideBuiltInSimilarity() {
+        Settings settings = Settings.builder().put("index.similarity.BM25.type", "classic").build();
+        IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", settings);
+        try {
+            new SimilarityService(indexSettings, Collections.emptyMap());
+            fail("can't override bm25");
+        } catch (IllegalArgumentException ex) {
+            assertEquals(ex.getMessage(), "Cannot redefine built-in Similarity [BM25]");
+        }
+    }
+
+    // Pre v3 indices could override built-in similarities
+    public void testOverrideBuiltInSimilarityPreV3() {
+        Settings settings = Settings.builder()
+                                    .put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_2_0_0)
+                                    .put("index.similarity.BM25.type", "classic")
+                                    .build();
+        IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", settings);
+        SimilarityService service = new SimilarityService(indexSettings, Collections.emptyMap());
+        assertTrue(service.getSimilarity("BM25") instanceof ClassicSimilarityProvider);
+    }
+
+    // Tests #16594
+    public void testDefaultSimilarity() {
+        Settings settings = Settings.builder().put("index.similarity.default.type", "BM25").build();
+        IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("test", settings);
+        SimilarityService service = new SimilarityService(indexSettings, Collections.emptyMap());
+        assertTrue(service.getDefaultSimilarity() instanceof BM25SimilarityProvider);
+    }
+}