Merge pull request #22611 from vespa-engine/revert-22600-bratseth/constants-cleanup-2

Revert "Bratseth/constants cleanup 2"

7 files changed, 48 insertions, 35 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
index 3cd2499f968..74d92bcfd02 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
@@ -41,7 +41,6 @@ import com.yahoo.vespa.model.container.search.QueryProfiles;
 import com.yahoo.vespa.model.container.search.QueryProfilesBuilder;
 import com.yahoo.vespa.model.container.search.SemanticRuleBuilder;
 import com.yahoo.vespa.model.container.search.SemanticRules;
-import com.yahoo.yolean.Exceptions;
 
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -218,7 +217,7 @@ public class DeployState implements ConfigDefinitionStore {
         for (var entry : importedModels.getSkippedModels().entrySet()) {
             // TODO: Vespa 8: Throw IllegalArgumentException instead
             deployLogger.logApplicationPackage(Level.WARNING, "Skipping import of model " + entry.getKey() + " as an exception " +
-                                                              "occurred during import: " + entry.getValue());
+                                                              "occurred during import. Error: " + entry.getValue());
         }
         return importedModels;
     }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java
index 8de86beacdb..c9b0ed2f628 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java
@@ -20,30 +20,54 @@ import java.util.function.Function;
  */
 public class FileDistributedConstants {
 
-    private final Map<String, DistributableConstant> constants;
+    private final FileRegistry fileRegistry;
+
+    private final Map<String, DistributableConstant> constants = new LinkedHashMap<>();
 
     public FileDistributedConstants(FileRegistry fileRegistry, Collection<RankProfile.Constant> constants) {
-        Map<String, DistributableConstant> distributableConstants = new LinkedHashMap<>();
+        this.fileRegistry = fileRegistry;
         for (var constant : constants) {
-            if ( ! constant.valuePath().isPresent()) continue;
-
-            var distributableConstant = new DistributableConstant(constant.name().simpleArgument().get(),
-                                                                  constant.type(),
-                                                                  constant.valuePath().get(),
-                                                                  constant.pathType().get());
-            distributableConstant.validate();
-            distributableConstant.register(fileRegistry);
-            distributableConstants.put(distributableConstant.getName(), distributableConstant);
+            if (constant.valuePath().isPresent())
+                add(new DistributableConstant(constant.name().simpleArgument().get(),
+                                              constant.type(),
+                                              constant.valuePath().get(),
+                                              constant.pathType().get()));
         }
-        this.constants = Collections.unmodifiableMap(distributableConstants);
+    }
+
+    public void add(DistributableConstant constant) {
+        constant.validate();
+        constant.register(fileRegistry);
+        String name = constant.getName();
+        DistributableConstant prev = constants.putIfAbsent(name, constant);
+        if ( prev != null )
+            throw new IllegalArgumentException("Constant '" + name + "' defined twice");
+    }
+
+    public void putIfAbsent(DistributableConstant constant) {
+        constant.validate();
+        constant.register(fileRegistry);
+        String name = constant.getName();
+        constants.putIfAbsent(name, constant);
+    }
+
+    public void computeIfAbsent(String name, Function<? super String, ? extends DistributableConstant> createConstant) {
+        constants.computeIfAbsent(name, key -> {
+            DistributableConstant constant = createConstant.apply(key);
+            constant.validate();
+            constant.register(fileRegistry);
+            return constant;
+        });
     }
 
     /** Returns a read-only map of the constants in this indexed by name. */
-    public Map<String, DistributableConstant> asMap() { return constants; }
+    public Map<String, DistributableConstant> asMap() {
+        return Collections.unmodifiableMap(constants);
+    }
 
     public static class DistributableConstant extends DistributableResource {
 
-        private final TensorType tensorType;
+        private TensorType tensorType;
 
         public DistributableConstant(String name, TensorType type, String fileName) {
             this(name, type, fileName, PathType.FILE);
@@ -55,6 +79,10 @@ public class FileDistributedConstants {
             validate();
         }
 
+        public void setType(TensorType type) {
+            this.tensorType = type;
+        }
+
         public TensorType getTensorType() { return tensorType; }
         public String getType() { return tensorType.toString(); }
 
diff --git a/config-model/src/test/cfg/application/stateless_eval/example.model b/config-model/src/test/cfg/application/stateless_eval/example.model
index 1d2db15c3ba..af1c85be4f0 100644
--- a/config-model/src/test/cfg/application/stateless_eval/example.model
+++ b/config-model/src/test/cfg/application/stateless_eval/example.model
@@ -7,7 +7,6 @@ model example {
     constants {
         constant1: tensor(x[3]):{{x:0}:0.5, {x:1}:1.5, {x:2}:2.5}
         constant2: 3.0
-        #constant1asLarge tensor(x[3]): file:constant1asLarge.json
     }
 
     constant constant1asLarge {
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java
index 1c23950d972..2057bcaba04 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java
@@ -8,7 +8,6 @@ import com.yahoo.io.IOUtils;
 import com.yahoo.path.Path;
 import com.yahoo.vespa.config.search.RankProfilesConfig;
 import com.yahoo.vespa.config.search.core.OnnxModelsConfig;
-import com.yahoo.vespa.config.search.core.RankingConstantsConfig;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.search.DocumentDatabase;
 import com.yahoo.vespa.model.search.IndexedSearchCluster;
@@ -58,15 +57,6 @@ public class RankingExpressionWithOnnxModelTestCase {
 
     private void assertGeneratedConfig(VespaModel vespaModel) {
         DocumentDatabase db = ((IndexedSearchCluster)vespaModel.getSearchClusters().get(0)).getDocumentDbs().get(0);
-
-        RankingConstantsConfig.Builder rankingConstantsConfigBuilder = new RankingConstantsConfig.Builder();
-        db.getConfig(rankingConstantsConfigBuilder);
-        var rankingConstantsConfig = rankingConstantsConfigBuilder.build();
-        assertEquals(1, rankingConstantsConfig.constant().size());
-        assertEquals("my_constant", rankingConstantsConfig.constant(0).name());
-        assertEquals("tensor(d0[2])", rankingConstantsConfig.constant(0).type());
-        assertEquals("files/constant.json", rankingConstantsConfig.constant(0).fileref().value());
-
         OnnxModelsConfig.Builder builder = new OnnxModelsConfig.Builder();
         ((OnnxModelsConfig.Producer) db).getConfig(builder);
         OnnxModelsConfig config = new OnnxModelsConfig(builder);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java
index bec72f78a65..f246b87d9bf 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java
@@ -18,7 +18,8 @@ import static org.junit.Assume.assumeTrue;
 public class ModelsEvaluatorTest {
 
     @Test
-    public void testModelsEvaluator() {
+    public void testModelsEvaluatorTester() {
+        assumeTrue(OnnxEvaluator.isRuntimeAvailable());
         ModelsEvaluator modelsEvaluator = ModelsEvaluatorTester.create("src/test/cfg/application/stateless_eval");
         assertEquals(3, modelsEvaluator.models().size());
 
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java
index bd07513b704..556d91acb70 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java
@@ -1,7 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.ml;
 
-import com.yahoo.config.FileReference;
 import com.yahoo.config.model.ApplicationPackageTester;
 import ai.vespa.rankingexpression.importer.configmodelview.MlModelImporter;
 import com.yahoo.config.model.deploy.DeployState;
@@ -23,9 +22,9 @@ import java.io.UncheckedIOException;
 import java.util.List;
 import java.util.Optional;
 
+import static junit.framework.TestCase.assertTrue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 
 /**
  * Helper for testing of imported models.
@@ -75,8 +74,6 @@ public class ImportedModelTester {
             assertEquals(constantName, constant.getName());
             assertTrue(constant.getFileName().endsWith(constantApplicationPackagePath.toString()));
 
-            assertTrue(model.fileReferences().contains(new FileReference(constant.getFileName())));
-
             if (expectedSize.isPresent()) {
                 Path constantPath = applicationDir.append(constantApplicationPackagePath);
                 assertTrue("Constant file '" + constantPath + "' has been written",
diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java
index 1003ebd8f3f..367b840a728 100644
--- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java
+++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java
@@ -3,7 +3,6 @@ package ai.vespa.rankingexpression.importer.configmodelview;
 
 import com.yahoo.concurrent.InThreadExecutorService;
 import com.yahoo.path.Path;
-import com.yahoo.yolean.Exceptions;
 
 import java.io.File;
 import java.util.Arrays;
@@ -56,7 +55,7 @@ public class ImportedMlModels {
                     models.put(name, model);
                 }
             } catch (InterruptedException | ExecutionException e) {
-                skippedModels.put(name, Exceptions.toMessageString(e));
+                skippedModels.put(name, e.getMessage());
             }
         });
         importedModels = Collections.unmodifiableMap(models);
@@ -98,7 +97,7 @@ public class ImportedMlModels {
                 if (existing != null) {
                     try {
                         throw new IllegalArgumentException("The models in " + child + " and " + existing.get().source() +
-                                                           " both resolve to the model name '" + name + "'");
+                                " both resolve to the model name '" + name + "'");
                     } catch (InterruptedException | ExecutionException e) {}
                 }