Validate constants in profiles

7 files changed, 54 insertions, 49 deletions

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 24f6b1390fd..95e022fca9e 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
@@ -412,10 +412,10 @@ public class RankProfile implements Cloneable {
     }
 
     /** Returns an unmodifiable view of the constants available in this */
-    public Map<Reference, Constant> getConstants() {
+    public Map<Reference, Constant> constants() {
         Map<Reference, Constant> allConstants = new HashMap<>();
         for (var inheritedProfile : inherited()) {
-            for (var constant : inheritedProfile.getConstants().values()) {
+            for (var constant : inheritedProfile.constants().values()) {
                 if (allConstants.containsKey(constant.name()))
                     throw new IllegalArgumentException(constant + "' is present in " +
                                                        inheritedProfile + " inherited by " +
@@ -935,7 +935,7 @@ public class RankProfile implements Cloneable {
     }
 
     private void compileThis(QueryProfileRegistry queryProfiles, ImportedMlModels importedModels) {
-        checkNameCollisions(getFunctions(), getConstants());
+        checkNameCollisions(getFunctions(), constants());
         ExpressionTransforms expressionTransforms = new ExpressionTransforms();
 
         Map<Reference, TensorType> featureTypes = featureTypes();
@@ -943,8 +943,8 @@ public class RankProfile implements Cloneable {
         Map<String, RankingExpressionFunction> inlineFunctions =
                 compileFunctions(this::getInlineFunctions, queryProfiles, featureTypes, importedModels, Collections.emptyMap(), expressionTransforms);
 
-        firstPhaseRanking = compile(this.getFirstPhase(), queryProfiles, featureTypes, importedModels, getConstants(), inlineFunctions, expressionTransforms);
-        secondPhaseRanking = compile(this.getSecondPhase(), queryProfiles, featureTypes, importedModels, getConstants(), inlineFunctions, expressionTransforms);
+        firstPhaseRanking = compile(this.getFirstPhase(), queryProfiles, featureTypes, importedModels, constants(), inlineFunctions, expressionTransforms);
+        secondPhaseRanking = compile(this.getSecondPhase(), queryProfiles, featureTypes, importedModels, constants(), inlineFunctions, expressionTransforms);
 
         // Function compiling second pass: compile all functions and insert previously compiled inline functions
         // TODO: This merges all functions from inherited profiles too and erases inheritance information. Not good.
@@ -979,7 +979,7 @@ public class RankProfile implements Cloneable {
         while (null != (entry = findUncompiledFunction(functions.get(), compiledFunctions.keySet()))) {
             RankingExpressionFunction rankingExpressionFunction = entry.getValue();
             RankingExpressionFunction compiled = compile(rankingExpressionFunction, queryProfiles, featureTypes,
-                                                         importedModels, getConstants(), inlineFunctions,
+                                                         importedModels, constants(), inlineFunctions,
                                                          expressionTransforms);
             compiledFunctions.put(entry.getKey(), compiled);
         }
@@ -1039,7 +1039,7 @@ public class RankProfile implements Cloneable {
                                                 Map<Reference, TensorType> featureTypes) {
         MapEvaluationTypeContext context = new MapEvaluationTypeContext(getExpressionFunctions(), featureTypes);
 
-        getConstants().forEach((k, v) -> context.setType(k, v.type()));
+        constants().forEach((k, v) -> context.setType(k, v.type()));
 
         // Add query features from all rank profile types
         for (QueryProfileType queryProfileType : queryProfiles.getTypeRegistry().allComponents()) {
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java
index f9c1f76a35d..770ca358ea4 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java
@@ -76,9 +76,9 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ
         Map<Reference, RankProfile.Constant> allFileConstants = new HashMap<>();
         addFileConstants(constantsFromSchema.values(), allFileConstants, schema != null ? schema.toString() : "[global]");
         for (var profile : deployState.rankProfileRegistry().rankProfilesOf(schema))
-            addFileConstants(profile.getConstants().values(), allFileConstants, profile.toString());
+            addFileConstants(profile.constants().values(), allFileConstants, profile.toString());
         for (var profile : deployState.rankProfileRegistry().rankProfilesOf(null))
-            addFileConstants(profile.getConstants().values(), allFileConstants, profile.toString());
+            addFileConstants(profile.constants().values(), allFileConstants, profile.toString());
         return new FileDistributedConstants(deployState.getFileRegistry(), allFileConstants.values());
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java
index af2a781a70c..0f87effcc82 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java
@@ -19,51 +19,35 @@ import java.io.FileNotFoundException;
  *
  * @author Vegard Sjonfjell
  */
-public class RankingConstantsValidator extends Validator {
-
-    private static class ExceptionMessageCollector {
-
-        String combinedMessage;
-        boolean exceptionsOccurred = false;
-
-        ExceptionMessageCollector(String messagePrelude) {
-            this.combinedMessage = messagePrelude;
-        }
-
-        public ExceptionMessageCollector add(Throwable throwable, String rcName, String rcFilename) {
-            exceptionsOccurred = true;
-            combinedMessage += String.format("\nRanking constant '%s' (%s): %s", rcName, rcFilename, throwable.getMessage());
-            return this;
-        }
-    }
-
-    static class TensorValidationException extends IllegalArgumentException {
-        TensorValidationException(String message) {
-            super(message);
-        }
-    }
+public class ConstantValidator extends Validator {
 
     @Override
     public void validate(VespaModel model, DeployState deployState) {
-        ApplicationPackage applicationPackage = deployState.getApplicationPackage();
-        ExceptionMessageCollector exceptionMessageCollector = new ExceptionMessageCollector("Invalid constant tensor file(s):");
-
+        var exceptionMessageCollector = new ExceptionMessageCollector("Invalid constant tensor file(s):");
         for (Schema schema : deployState.getSchemas()) {
-            for (RankProfile.Constant rc : schema.constants().values()) {
-                try {
-                    validateRankingConstant(rc, applicationPackage);
-                } catch (InvalidConstantTensorException | FileNotFoundException ex) {
-                    exceptionMessageCollector.add(ex, rc.name().toString(), rc.valuePath().get());
-                }
+            for (var constant : schema.constants().values())
+                validate(constant, deployState.getApplicationPackage(), exceptionMessageCollector);
+            for (var profile : deployState.rankProfileRegistry().rankProfilesOf(schema)) {
+                for (var constant : profile.constants().values())
+                    validate(constant, deployState.getApplicationPackage(), exceptionMessageCollector);
             }
         }
 
-        if (exceptionMessageCollector.exceptionsOccurred) {
+        if (exceptionMessageCollector.exceptionsOccurred)
             throw new TensorValidationException(exceptionMessageCollector.combinedMessage);
+    }
+
+    private void validate(RankProfile.Constant constant,
+                          ApplicationPackage applicationPackage,
+                          ExceptionMessageCollector exceptionMessageCollector) {
+        try {
+            validate(constant, applicationPackage);
+        } catch (InvalidConstantTensorException | FileNotFoundException ex) {
+            exceptionMessageCollector.add(ex, constant.name().toString(), constant.valuePath().get());
         }
     }
 
-    private void validateRankingConstant(RankProfile.Constant rankingConstant, ApplicationPackage application) throws FileNotFoundException {
+    private void validate(RankProfile.Constant rankingConstant, ApplicationPackage application) throws FileNotFoundException {
         // Only validate file backed constants:
         if (rankingConstant.valuePath().isEmpty()) return;
         if (rankingConstant.pathType().get() != DistributableResource.PathType.FILE) return;
@@ -80,4 +64,26 @@ public class RankingConstantsValidator extends Validator {
                                                    tensorApplicationFile.createReader());
     }
 
+    private static class ExceptionMessageCollector {
+
+        String combinedMessage;
+        boolean exceptionsOccurred = false;
+
+        ExceptionMessageCollector(String messagePrelude) {
+            this.combinedMessage = messagePrelude;
+        }
+
+        public ExceptionMessageCollector add(Throwable throwable, String rcName, String rcFilename) {
+            exceptionsOccurred = true;
+            combinedMessage += String.format("\nRanking constant '%s' (%s): %s", rcName, rcFilename, throwable.getMessage());
+            return this;
+        }
+    }
+
+    static class TensorValidationException extends IllegalArgumentException {
+        TensorValidationException(String message) {
+            super(message);
+        }
+    }
+
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
index 4ba809a58ba..ce61d3edc3b 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
@@ -77,7 +77,7 @@ public class Validation {
         new NoPrefixForIndexes().validate(model, deployState);
         new DeploymentSpecValidator().validate(model, deployState);
         new ValidationOverridesValidator().validate(model, deployState);
-        new RankingConstantsValidator().validate(model, deployState);
+        new ConstantValidator().validate(model, deployState);
         new SecretStoreValidator().validate(model, deployState);
         new EndpointCertificateSecretsValidator().validate(model, deployState);
         new AccessControlFilterValidator().validate(model, deployState);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java
index 89d85a9b417..255ce3a95f6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java
@@ -318,7 +318,7 @@ public class ConvertedModel {
         }
         else {
             var constantReference = FeatureNames.asConstantFeature(constantName);
-            if ( ! profile.getConstants().containsKey(constantReference)) {
+            if ( ! profile.constants().containsKey(constantReference)) {
                 Path constantPath = store.writeLargeConstant(constantName, constantValue);
                 profile.add(new RankProfile.Constant(constantReference, constantValue.type(), constantPath.toString()));
             }
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java
index 44857835ccf..bd6310ffd48 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java
@@ -323,7 +323,7 @@ public class RankingExpressionWithOnnxTestCase {
     }
 
     private void assertSmallConstant(String name, TensorType type, RankProfileSearchFixture search) {
-        var value = search.compiledRankProfile("my_profile").getConstants().get(FeatureNames.asConstantFeature(name));
+        var value = search.compiledRankProfile("my_profile").constants().get(FeatureNames.asConstantFeature(name));
         assertNotNull(value);
         assertEquals(type, value.type());
     }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ConstantValidatorTest.java
index efb6969d6c7..e6657d4f061 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ConstantValidatorTest.java
@@ -2,14 +2,13 @@
 package com.yahoo.vespa.model.application.validation;
 
 import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg;
-import com.yahoo.yolean.Exceptions;
 import org.junit.Test;
 
-import static com.yahoo.vespa.model.application.validation.RankingConstantsValidator.TensorValidationException;
+import static com.yahoo.vespa.model.application.validation.ConstantValidator.TensorValidationException;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-public class RankingConstantsValidatorTest {
+public class ConstantValidatorTest {
 
     @Test
     public void ensure_that_valid_ranking_constants_do_not_fail() {