Merge pull request #19024 from vespa-engine/balder/allow-sideways-rankprofile-resolving-until-enforced

Allow inheriting any rankprofile until enforcing is enabled explicit.

5 files changed, 40 insertions, 9 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
index b42686406b5..9690f00a209 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
@@ -51,6 +51,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private double feedConcurrency = 0.5;
     private boolean enableFeedBlockInDistributor = true;
     private boolean useExternalRankExpression = true;
+    private boolean enforceRankProfileInheritance = true;
     private int maxActivationInhibitedOutOfSyncGroups = 0;
     private List<TenantSecretStore> tenantSecretStores = Collections.emptyList();
     private String jvmOmitStackTraceInFastThrowOption;
@@ -107,8 +108,12 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public double resourceLimitMemory() { return resourceLimitMemory; }
     @Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; }
     @Override public int metricsproxyNumThreads() { return 1; }
-    @Override public boolean enforceRankProfileInheritance() { return true; }
+    @Override public boolean enforceRankProfileInheritance() { return enforceRankProfileInheritance; }
 
+    public TestProperties enforceRankProfileInheritance(boolean value) {
+        enforceRankProfileInheritance = value;
+        return this;
+    }
     public TestProperties useExternalRankExpression(boolean value) {
         useExternalRankExpression = value;
         return this;
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 97770b6ebb9..38f8275e209 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
@@ -3,6 +3,7 @@ package com.yahoo.searchdefinition;
 
 import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModels;
 import com.yahoo.config.application.api.ApplicationPackage;
+import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.search.query.profile.QueryProfileRegistry;
 import com.yahoo.search.query.profile.types.FieldDescription;
 import com.yahoo.search.query.profile.types.QueryProfileType;
@@ -44,7 +45,6 @@ import java.util.OptionalDouble;
 import java.util.Set;
 import java.util.function.Supplier;
 import java.util.logging.Level;
-import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -55,7 +55,6 @@ import java.util.stream.Stream;
  */
 public class RankProfile implements Cloneable {
 
-    private final static Logger log = Logger.getLogger(RankProfile.class.getName());
     public final static String FIRST_PHASE = "firstphase";
     public final static String SECOND_PHASE = "secondphase";
     /** The search definition-unique name of this rank profile */
@@ -130,6 +129,8 @@ public class RankProfile implements Cloneable {
     /** Global onnx models not tied to a search definition */
     private final OnnxModels onnxModels;
 
+    private final DeployLogger deployLogger;
+
     /**
      * Creates a new rank profile for a particular search definition
      *
@@ -144,6 +145,7 @@ public class RankProfile implements Cloneable {
         this.model = null;
         this.onnxModels = null;
         this.rankProfileRegistry = rankProfileRegistry;
+        this.deployLogger = search.getDeployLogger();
     }
 
     /**
@@ -152,12 +154,13 @@ public class RankProfile implements Cloneable {
      * @param name  the name of the new profile
      * @param model the model owning this profile
      */
-    public RankProfile(String name, VespaModel model, RankProfileRegistry rankProfileRegistry, OnnxModels onnxModels) {
+    public RankProfile(String name, VespaModel model, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, OnnxModels onnxModels) {
         this.name = Objects.requireNonNull(name, "name cannot be null");
         this.search = null;
         this.model = Objects.requireNonNull(model, "model cannot be null");
         this.rankProfileRegistry = rankProfileRegistry;
         this.onnxModels = onnxModels;
+        this.deployLogger = deployLogger;
     }
 
     public String getName() { return name; }
@@ -214,7 +217,8 @@ public class RankProfile implements Cloneable {
                 if (search.getDeployProperties().featureFlags().enforceRankProfileInheritance()) {
                     throw new IllegalArgumentException(msg);
                 } else {
-                    log.warning(msg);
+                    deployLogger.log(Level.WARNING, msg);
+                    inherited = resolveIndependentOfInheritance();
                 }
             } else {
                 List<String> children = new ArrayList<>();
@@ -224,6 +228,12 @@ public class RankProfile implements Cloneable {
         }
         return inherited;
     }
+    private RankProfile resolveIndependentOfInheritance() {
+        for (RankProfile rankProfile : rankProfileRegistry.all()) {
+            if (rankProfile.getName().equals(inheritedName)) return rankProfile;
+        }
+        return null;
+    }
     private String createFullyQualifiedName() {
         return (search != null)
                 ? (search.getName() + "." + getName())
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
index f49a477c66b..9cd2c4d3bfb 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
@@ -95,7 +95,10 @@ public class SearchBuilder {
 
     /** For testing only */
     public SearchBuilder(RankProfileRegistry rankProfileRegistry, QueryProfileRegistry queryProfileRegistry) {
-        this(MockApplicationPackage.createEmpty(), new MockFileRegistry(), new BaseDeployLogger(), new TestProperties(), rankProfileRegistry, queryProfileRegistry);
+        this(rankProfileRegistry, queryProfileRegistry, new TestProperties());
+    }
+    public SearchBuilder(RankProfileRegistry rankProfileRegistry, QueryProfileRegistry queryProfileRegistry, ModelContext.Properties properties) {
+        this(MockApplicationPackage.createEmpty(), new MockFileRegistry(), new BaseDeployLogger(), properties, rankProfileRegistry, queryProfileRegistry);
     }
 
     public SearchBuilder(ApplicationPackage app,
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java
index e6687ec8ac8..3f1cf130aff 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java
@@ -298,7 +298,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri
             for (ImportedMlModel model : importedModels.all()) {
                 // Due to automatic naming not guaranteeing unique names, there must be a 1-1 between OnnxModels and global RankProfiles.
                 OnnxModels onnxModels = onnxModelInfoFromSource(model);
-                RankProfile profile = new RankProfile(model.name(), this, rankProfileRegistry, onnxModels);
+                RankProfile profile = new RankProfile(model.name(), this, deployLogger, rankProfileRegistry, onnxModels);
                 rankProfileRegistry.add(profile);
                 ConvertedModel convertedModel = ConvertedModel.fromSource(new ModelName(model.name()),
                                                                           model.name(), profile, queryProfiles.getRegistry(), model);
@@ -312,7 +312,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri
                 if (modelName.contains(".")) continue; // Name space: Not a global profile
                 // Due to automatic naming not guaranteeing unique names, there must be a 1-1 between OnnxModels and global RankProfiles.
                 OnnxModels onnxModels = onnxModelInfoFromStore(modelName);
-                RankProfile profile = new RankProfile(modelName, this, rankProfileRegistry, onnxModels);
+                RankProfile profile = new RankProfile(modelName, this, deployLogger, rankProfileRegistry, onnxModels);
                 rankProfileRegistry.add(profile);
                 ConvertedModel convertedModel = ConvertedModel.fromStore(new ModelName(modelName), modelName, profile);
                 convertedModel.expressions().values().forEach(f -> profile.addFunction(f, false));
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 8f3fbfc9de9..69789d09dc2 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java
@@ -118,8 +118,13 @@ public class RankProfileTestCase extends SchemaTestCase {
 
     @Test
     public void requireThatSidewaysInheritanceIsImpossible() throws ParseException {
+        verifySidewaysInheritance(false);
+        verifySidewaysInheritance(true);
+    }
+    private void verifySidewaysInheritance(boolean enforce) throws ParseException {
         RankProfileRegistry registry = new RankProfileRegistry();
-        SearchBuilder builder = new SearchBuilder(registry, setupQueryProfileTypes());
+        SearchBuilder builder = new SearchBuilder(registry, setupQueryProfileTypes(),
+                                                  new TestProperties().enforceRankProfileInheritance(enforce));
         builder.importString(joinLines(
                 "schema child1 {",
                 "  document child1 {",
@@ -163,7 +168,15 @@ public class RankProfileTestCase extends SchemaTestCase {
                 "}"));
         try {
             builder.build(true);
+            if (enforce) {
+                fail("Sideways inheritance should have been enforced");
+            } else {
+                assertNotNull(builder.getSearch("child2"));
+                assertNotNull(builder.getSearch("child1"));
+                assertTrue(registry.get("child1", "child").inherits("parent"));
+            }
         } catch (IllegalArgumentException e) {
+            if (!enforce) fail("Sideways inheritance should have been allowed");
             assertEquals("rank-profile 'child' inherits 'parent', but it does not exist anywhere in the inheritance of search 'child1'.", e.getMessage());
         }
     }