Verify that rank profile inheritance is correct and sound. The rank profile must exist and be visible in the inheritance tree of the searchdefinition.

7 files changed, 122 insertions, 28 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSearch.java b/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSearch.java
index 24bc081aded..c56606adafc 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSearch.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSearch.java
@@ -5,6 +5,7 @@ import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.api.ModelContext;
 import com.yahoo.searchdefinition.document.ImmutableSDField;
+import com.yahoo.searchdefinition.document.SDDocumentType;
 import com.yahoo.searchdefinition.document.SDField;
 import com.yahoo.vespa.documentmodel.SummaryField;
 
@@ -35,7 +36,7 @@ public interface ImmutableSearch {
     LargeRankExpressions rankExpressionFiles();
     OnnxModels onnxModels();
     Stream<ImmutableSDField> allImportedFields();
-
+    SDDocumentType getDocument();
     ImmutableSDField getField(String name);
 
     default Stream<ImmutableSDField> allFields() {
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
index 47d95cba516..12578a9da3b 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
@@ -65,6 +65,7 @@ public class RankProfile implements Cloneable {
 
     /** The name of the rank profile inherited by this */
     private String inheritedName = null;
+    private RankProfile inherited = null;
 
     /** The match settings of this profile */
     private MatchPhaseSettings matchPhaseSettings = null;
@@ -199,20 +200,25 @@ public class RankProfile implements Cloneable {
 
     /** Returns the inherited rank profile, or null if there is none */
     public RankProfile getInherited() {
-        if (getSearch() == null) return getInheritedFromRegistry(inheritedName);
-
-        RankProfile inheritedInThisSearch = rankProfileRegistry.get(search, inheritedName);
-        if (inheritedInThisSearch != null) return inheritedInThisSearch;
-        return getInheritedFromRegistry(inheritedName);
-    }
-
-    private RankProfile getInheritedFromRegistry(String inheritedName) {
-        for (RankProfile r : rankProfileRegistry.all()) {
-            if (r.getName().equals(inheritedName)) {
-                return r;
+        if (inheritedName == null) return null;
+        if (inherited == null) {
+            inherited = resolveInherited();
+            if (inherited == null) {
+                throw new IllegalArgumentException("rank-profile '" + getName() + "' inherits '" + inheritedName +
+                        "', but it does not exist anywhere in the inheritance of search '" +
+                        ((getSearch() != null) ? getSearch().getName() : " global rank profiles") + "'.");
             }
         }
-        return null;
+        return inherited;
+    }
+
+    private RankProfile resolveInherited() {
+        if (inheritedName == null) return null;
+        return (getSearch() != null)
+                ? (search.getDocument() != null)
+                ? rankProfileRegistry.resolve(search.getDocument(), inheritedName)
+                : rankProfileRegistry.get(search.getName(), inheritedName)
+                : rankProfileRegistry.getGlobal(inheritedName);
     }
 
     /**
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 271442768a8..d8fca37c6b7 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java
@@ -1,11 +1,15 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.searchdefinition;
 
+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.HashSet;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -20,8 +24,8 @@ import java.util.Set;
  */
 public class RankProfileRegistry {
 
-    private final Map<RankProfile, ImmutableSearch> rankProfileToSearch = new LinkedHashMap<>();
-    private final Map<ImmutableSearch, Map<String, RankProfile>> rankProfiles = new LinkedHashMap<>();
+    private final Map<String, Map<String, RankProfile>> rankProfiles = new LinkedHashMap<>();
+    private final String MAGIC_GLOBAL_RANKPROFILES = "[MAGIC_GLOBAL_RANKPROFILES]";
 
     /* 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"));
@@ -33,24 +37,28 @@ public class RankProfileRegistry {
         return rankProfileRegistry;
     }
 
+    private String extractName(ImmutableSearch search) {
+        return search != null ? search.getName() : MAGIC_GLOBAL_RANKPROFILES;
+    }
+
     /**
      * Adds a rank profile to this registry
      *
      * @param rankProfile the rank profile to add
      */
     public void add(RankProfile rankProfile) {
-        if ( ! rankProfiles.containsKey(rankProfile.getSearch())) {
-            rankProfiles.put(rankProfile.getSearch(), new LinkedHashMap<>());
+        String searchName = extractName(rankProfile.getSearch());
+        if ( ! rankProfiles.containsKey(searchName)) {
+            rankProfiles.put(searchName, new LinkedHashMap<>());
         }
         checkForDuplicate(rankProfile);
-        rankProfiles.get(rankProfile.getSearch()).put(rankProfile.getName(), rankProfile);
-        rankProfileToSearch.put(rankProfile, rankProfile.getSearch());
+        rankProfiles.get(searchName).put(rankProfile.getName(), rankProfile);
     }
 
     private void checkForDuplicate(RankProfile rankProfile) {
         String rankProfileName = rankProfile.getName();
-        RankProfile existingRangProfileWithSameName = rankProfiles.get(rankProfile.getSearch()).get(rankProfileName);
-        if (existingRangProfileWithSameName == null) return;
+        RankProfile existingRankProfileWithSameName = rankProfiles.get(extractName(rankProfile.getSearch())).get(rankProfileName);
+        if (existingRankProfileWithSameName == null) return;
 
         if ( ! overridableRankProfileNames.contains(rankProfileName)) {
             throw new IllegalArgumentException("Duplicate rank profile '" + rankProfileName + "' in " +
@@ -65,27 +73,50 @@ public class RankProfileRegistry {
      * @param name the name of the rank profile
      * @return the RankProfile to return.
      */
-    public RankProfile get(ImmutableSearch search, String name) {
+    public RankProfile get(String search, String name) {
         Map<String, RankProfile> profiles = rankProfiles.get(search);
         if (profiles == null) return null;
         return profiles.get(name);
     }
+    public RankProfile get(ImmutableSearch search, String name) {
+        return get(search.getName(), name);
+    }
+
+    public RankProfile getGlobal(String name) {
+        Map<String, RankProfile> profiles = rankProfiles.get(MAGIC_GLOBAL_RANKPROFILES);
+        if (profiles == null) return null;
+        return profiles.get(name);
+    }
+
+    public RankProfile resolve(SDDocumentType docType, String name) {
+        RankProfile rankProfile = get(docType.getName(), name);
+        if (rankProfile != null) return rankProfile;
+        for (var parent : docType.getInheritedTypes()) {
+            RankProfile parentProfile = resolve(parent, name);
+            if (parentProfile != null) return parentProfile;
+        }
+        return get(MAGIC_GLOBAL_RANKPROFILES, name);
+    }
 
     /**
      * Rank profiles that are collected across clusters.
      * @return A set of global {@link RankProfile} instances.
      */
-    public Set<RankProfile> all() {
-        return rankProfileToSearch.keySet();
+    public Collection<RankProfile> all() {
+        List<RankProfile> all = new ArrayList<>();
+        for (var entry : rankProfiles.values()) {
+            all.addAll(entry.values());
+        }
+        return all;
     }
 
     /**
      * Returns the rank profiles of a given search definition.
      *
-     * @param search {@link Search} to get rank profiles for
+     * @param search the searchdefinition to get rank profiles for
      * @return a collection of {@link RankProfile} instances
      */
-    public Collection<RankProfile> rankProfilesOf(ImmutableSearch search) {
+    public Collection<RankProfile> rankProfilesOf(String search) {
         Map<String, RankProfile> mapping = rankProfiles.get(search);
         if (mapping == null) {
             return Collections.emptyList();
@@ -93,4 +124,13 @@ public class RankProfileRegistry {
         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(ImmutableSearch search) {
+        return rankProfilesOf(extractName(search));
+    }
+
 }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Search.java b/config-model/src/main/java/com/yahoo/searchdefinition/Search.java
index f3d6e2f78a2..dd1e65f78f9 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/Search.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/Search.java
@@ -274,6 +274,7 @@ public class Search implements ImmutableSearch {
     /**
      * @return The document in this search.
      */
+    @Override
     public SDDocumentType getDocument() {
         return docType;
     }
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 029892cba1c..b18bebb4d7a 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(search)) {
+        for (RankProfile rankProfile : rankProfileRegistry.rankProfilesOf(search.getName())) {
             if (rankProfile.getMatchPhaseSettings() != null && rankProfile.getMatchPhaseSettings().getDiversity() != null) {
                 validate(rankProfile, rankProfile.getMatchPhaseSettings().getDiversity());
             }
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java
index b4f4608facf..d96052cd8e0 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java
@@ -4,7 +4,6 @@ package com.yahoo.searchdefinition;
 import com.yahoo.collections.Pair;
 import com.yahoo.component.ComponentId;
 import com.yahoo.config.model.api.ModelContext;
-import com.yahoo.config.model.application.provider.BaseDeployLogger;
 import com.yahoo.config.model.application.provider.MockFileRegistry;
 import com.yahoo.config.model.deploy.TestProperties;
 import com.yahoo.document.DataType;
@@ -28,7 +27,9 @@ import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Tests rank profiles
@@ -61,6 +62,40 @@ public class RankProfileTestCase extends SchemaTestCase {
         assertEquals(RankType.DEFAULT, setting.getValue());
     }
 
+    @Test
+    public void requireThatIllegalInheritanceIsChecked() throws ParseException {
+        try {
+            RankProfileRegistry registry = new RankProfileRegistry();
+            SearchBuilder builder = new SearchBuilder(registry, setupQueryProfileTypes());
+            builder.importString(
+                    "search test {\n" +
+                    "  document test { } \n" +
+                    "  rank-profile p1 inherits notexist {}\n" +
+                    "}");
+            builder.build(true);
+            fail();
+        } catch (IllegalArgumentException e) {
+            assertEquals("rank-profile 'p1' inherits 'notexist', but it does not exist anywhere in the inheritance of search 'test'.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void requireThatRankProfilesCanInheritNotYetSeenProfiles() throws ParseException
+    {
+        RankProfileRegistry registry = new RankProfileRegistry();
+        SearchBuilder builder = new SearchBuilder(registry, setupQueryProfileTypes());
+        builder.importString(
+                "search test {\n" +
+                        "  document test { } \n" +
+                        "  rank-profile p1 inherits not_yet_defined {}\n" +
+                        "  rank-profile not_yet_defined inherits not_yet_defined {}\n" +
+                        "}");
+        builder.build(true);
+        assertNotNull(registry.get("test","p1"));
+        assertTrue(registry.get("test","p1").inherits("not_yet_defined"));
+        assertNotNull(registry.get("test","not_yet_defined"));
+    }
+
     private String createSD(Double termwiseLimit) {
         return "search test {\n" +
                 "    document test { \n" +
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/ExportingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/ExportingTestCase.java
index 138fb333621..41c5c919a62 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/ExportingTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/ExportingTestCase.java
@@ -9,6 +9,7 @@ import org.junit.Test;
 import java.io.IOException;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 /**
  * Tests exporting
@@ -165,6 +166,16 @@ public class ExportingTestCase extends AbstractExportingTestCase {
     }
 
     @Test
+    public void testIllegalSidewaysRankProfileInheritance() throws IOException, ParseException {
+        try {
+            assertCorrectDeriving("illegal_sideways_inheritance", "child1", new TestableDeployLogger());
+            fail();
+        } catch (IllegalArgumentException e) {
+            assertEquals("rank-profile 'child' inherits 'parent', but it does not exist anywhere in the inheritance of search 'child1'.", e.getMessage());
+        }
+    }
+
+    @Test
     public void testLanguage() throws IOException, ParseException {
         TestableDeployLogger logger = new TestableDeployLogger();
         assertCorrectDeriving("language", logger);