Include inherited rank profiles

4 files changed, 49 insertions, 71 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java
index ec446e27670..08ae3d838ec 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java
@@ -6,7 +6,7 @@ import com.yahoo.searchdefinition.document.SDDocumentType;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -22,18 +22,15 @@ import java.util.Set;
  *
  * @author Ulf Lilleengen
  */
+// TODO: These should be stored in each schema as everything else
 public class RankProfileRegistry {
 
     private final Map<String, Map<String, RankProfile>> rankProfiles = new LinkedHashMap<>();
-    private static final String MAGIC_GLOBAL_RANKPROFILES = "[MAGIC_GLOBAL_RANKPROFILES]";
+    private static final String globalRankProfilesKey = "[global]";
 
     /* These rank profiles can be overridden: 'default' rank profile, as that is documented to work. And 'unranked'. */
     static final Set<String> overridableRankProfileNames = new HashSet<>(Arrays.asList("default", "unranked"));
 
-    public RankProfileRegistry() {
-
-    }
-
     public static RankProfileRegistry createRankProfileRegistryWithBuiltinRankProfiles(Schema schema) {
         RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
         rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry, schema.rankingConstants()));
@@ -42,14 +39,10 @@ public class RankProfileRegistry {
     }
 
     private String extractName(ImmutableSchema search) {
-        return search != null ? search.getName() : MAGIC_GLOBAL_RANKPROFILES;
+        return search != null ? search.getName() : globalRankProfilesKey;
     }
 
-    /**
-     * Adds a rank profile to this registry
-     *
-     * @param rankProfile the rank profile to add
-     */
+    /** Adds a rank profile to this registry */
     public void add(RankProfile rankProfile) {
         String searchName = extractName(rankProfile.getSearch());
         if ( ! rankProfiles.containsKey(searchName)) {
@@ -91,7 +84,7 @@ public class RankProfileRegistry {
     }
 
     public RankProfile getGlobal(String name) {
-        Map<String, RankProfile> profiles = rankProfiles.get(MAGIC_GLOBAL_RANKPROFILES);
+        Map<String, RankProfile> profiles = rankProfiles.get(globalRankProfilesKey);
         if (profiles == null) return null;
         return profiles.get(name);
     }
@@ -103,12 +96,13 @@ public class RankProfileRegistry {
             RankProfile parentProfile = resolve(parent, name);
             if (parentProfile != null) return parentProfile;
         }
-        return get(MAGIC_GLOBAL_RANKPROFILES, name);
+        return get(globalRankProfilesKey, name);
     }
 
     /**
      * Rank profiles that are collected across clusters.
-     * @return A set of global {@link RankProfile} instances.
+     *
+     * @return a set of global {@link RankProfile} instances
      */
     public Collection<RankProfile> all() {
         List<RankProfile> all = new ArrayList<>();
@@ -119,26 +113,28 @@ public class RankProfileRegistry {
     }
 
     /**
-     * Returns the rank profiles of a given search definition.
+     * Retrieve all rank profiles for a schema
      *
-     * @param search the searchdefinition to get rank profiles for
+     * @param schema the schema to fetch rank profiles for, or null for the global ones
      * @return a collection of {@link RankProfile} instances
      */
-    public Collection<RankProfile> rankProfilesOf(String search) {
-        Map<String, RankProfile> mapping = rankProfiles.get(search);
-        if (mapping == null) {
-            return Collections.emptyList();
+    public Collection<RankProfile> rankProfilesOf(ImmutableSchema schema) {
+        String key = schema == null ? globalRankProfilesKey : schema.getName();
+
+        if ( ! rankProfiles.containsKey(key)) return List.of();
+
+        var profiles = new LinkedHashMap<>(rankProfiles.get(key));
+        // Add all profiles in inherited schemas, unless they are already present (overridden)
+        while (schema != null && schema.inherited().isPresent()) {
+            schema = schema.inherited().get();
+            var inheritedProfiles = rankProfiles.get(schema.getName());
+            if (inheritedProfiles != null) {
+                for (Map.Entry<String, RankProfile> inheritedProfile : inheritedProfiles.entrySet()) {
+                    profiles.putIfAbsent(inheritedProfile.getKey(), inheritedProfile.getValue());
+                }
+            }
         }
-        return mapping.values();
-    }
-
-    /**
-     * Retrieve all rank profiles for a search definition
-     * @param search search definition to fetch rank profiles for, or null for the global ones
-     * @return Collection of RankProfiles
-     */
-    public Collection<RankProfile> rankProfilesOf(ImmutableSchema search) {
-        return rankProfilesOf(extractName(search));
+        return profiles.values();
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
index c7a7ecd1d08..ddad67324ba 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
@@ -344,11 +344,12 @@ public class Schema implements ImmutableSchema {
     }
 
     /**
-     * Returns a field defined in one of the documents of this search definition. This does <b>not</b> include the extra
-     * fields defined outside of a document (those accessible through the getExtraField() method).
+     * Returns a field defined in one of the documents of this search definition.
+     * This does not include the extra fields defined outside the document
+     * (those accessible through the getExtraField() method).
      *
-     * @param name The name of the field to return.
-     * @return The named field, or null if not found.
+     * @param name the name of the field to return
+     * @return the named field, or null if not found
      */
     public SDField getDocumentField(String name) {
         return (SDField) documentType.getField(name);
@@ -458,7 +459,7 @@ public class Schema implements ImmutableSchema {
     /**
      * Consolidates a set of index settings for the same index into one
      *
-     * @param indices The list of indexes to consolidate
+     * @param indices the list of indexes to consolidate
      * @return the consolidated index
      */
     private Index consolidateIndices(List<Index> indices) {
@@ -477,13 +478,10 @@ public class Schema implements ImmutableSchema {
             if (consolidated.getRankType() == null) {
                 consolidated.setRankType(current.getRankType());
             } else {
-                if (current.getRankType() != null &&
-                    !consolidated.getRankType().equals(current.getRankType()))
-                {
+                if (current.getRankType() != null && consolidated.getRankType() != current.getRankType())
                     deployLogger.logApplicationPackage(Level.WARNING, "Conflicting rank type settings for " +
                                                                       first.getName() + " in " + this + ", using " +
                                                                       consolidated.getRankType());
-                }
             }
 
             for (Iterator<String> j = current.aliasIterator(); j.hasNext();) {
@@ -505,11 +503,8 @@ public class Schema implements ImmutableSchema {
             }
         }
 
-        for (ImmutableSDField field : allConcreteFields()) {
-            for (Index index : field.getIndices().values()) {
-                allIndices.add(index);
-            }
-        }
+        for (ImmutableSDField field : allConcreteFields())
+            allIndices.addAll(field.getIndices().values());
 
         return Collections.unmodifiableList(allIndices);
     }
@@ -618,17 +613,7 @@ public class Schema implements ImmutableSchema {
         return summaryFields;
     }
 
-    @Override
-    public int hashCode() {
-        return name.hashCode();
-    }
-
-    /**
-     * Returns the first occurrence of an attribute having this name, or null if none
-     *
-     * @param name Name of attribute
-     * @return The Attribute with given name.
-     */
+    /** Returns the first occurrence of an attribute having this name, or null if none */
     public Attribute getAttribute(String name) {
         for (ImmutableSDField field : allConcreteFields()) {
             Attribute attribute = field.getAttributes().get(name);
@@ -650,33 +635,29 @@ public class Schema implements ImmutableSchema {
     }
 
     @Override
+    public int hashCode() {
+        return name.hashCode();
+    }
+
+    @Override
     public String toString() {
         return "schema '" + getName() + "'";
     }
 
     public boolean isAccessingDiskSummary(SummaryField field) {
-        if (!field.getTransform().isInMemory()) {
-            return true;
-        }
-        if (field.getSources().size() == 0) {
-            return isAccessingDiskSummary(getName());
-        }
+        if (!field.getTransform().isInMemory()) return true;
+        if (field.getSources().size() == 0) return isAccessingDiskSummary(getName());
         for (SummaryField.Source source : field.getSources()) {
-            if (isAccessingDiskSummary(source.getName())) {
+            if (isAccessingDiskSummary(source.getName()))
                 return true;
-            }
         }
         return false;
     }
 
     private boolean isAccessingDiskSummary(String source) {
         SDField field = getConcreteField(source);
-        if (field == null) {
-            return false;
-        }
-        if (field.doesSummarying() && !field.doesAttributing()) {
-            return true;
-        }
+        if (field == null) return false;
+        if (field.doesSummarying() && !field.doesAttributing()) return true;
         return false;
     }
 
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java
index 40de25dbc76..5643bb660f1 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/DiversitySettingsValidator.java
@@ -22,7 +22,7 @@ public class DiversitySettingsValidator extends Processor {
         if ( ! validate) return;
         if (documentsOnly) return;
 
-        for (RankProfile rankProfile : rankProfileRegistry.rankProfilesOf(schema.getName())) {
+        for (RankProfile rankProfile : rankProfileRegistry.rankProfilesOf(schema)) {
             if (rankProfile.getMatchPhaseSettings() != null && rankProfile.getMatchPhaseSettings().getDiversity() != null) {
                 validate(rankProfile, rankProfile.getMatchPhaseSettings().getDiversity());
             }
diff --git a/config-model/src/test/derived/schemainheritance/rank-profiles.cfg b/config-model/src/test/derived/schemainheritance/rank-profiles.cfg
index 9e68045fab0..1e2ed46e696 100644
--- a/config-model/src/test/derived/schemainheritance/rank-profiles.cfg
+++ b/config-model/src/test/derived/schemainheritance/rank-profiles.cfg
@@ -9,3 +9,4 @@ rankprofile[].fef.property[].value "0"
 rankprofile[].fef.property[].name "vespa.dump.ignoredefaultfeatures"
 rankprofile[].fef.property[].value "true"
 rankprofile[].name "child_profile"
+rankprofile[].name "parent_profile"