diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-05-13 09:44:30 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2022-05-13 09:44:30 +0200 |
commit | 1ac0e7b95b48ea75a1796856ea8e8ebac04f6b8d (patch) | |
tree | c82703cb1632fd5e2b82f8f038a54ccf41c3961c /config-model | |
parent | efc33e32e7a3afe91d50b963a2adce00b799c223 (diff) |
Validate constants in profiles
Diffstat (limited to 'config-model')
-rw-r--r-- | config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java | 14 | ||||
-rw-r--r-- | config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java | 4 | ||||
-rw-r--r-- | config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java (renamed from config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java) | 74 | ||||
-rw-r--r-- | config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java | 2 | ||||
-rw-r--r-- | config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java | 2 | ||||
-rw-r--r-- | config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java | 2 | ||||
-rw-r--r-- | config-model/src/test/java/com/yahoo/vespa/model/application/validation/ConstantValidatorTest.java (renamed from config-model/src/test/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidatorTest.java) | 5 |
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() { |