Revert "Merge pull request #23910 from vespa-engine/revert-23907-bratseth/model-reference-cleanup"

This reverts commit 37f0d3a64511b0dd0d902053be3a6cffc21acee6, reversing
changes made to c23bba0b2cd6ab7699ee2b0d6be34c023159ffb8.

4 files changed, 77 insertions, 32 deletions

diff --git a/config-lib/src/main/java/com/yahoo/config/ModelNode.java b/config-lib/src/main/java/com/yahoo/config/ModelNode.java
index 2748ef8c7e9..67ea751d40e 100644
--- a/config-lib/src/main/java/com/yahoo/config/ModelNode.java
+++ b/config-lib/src/main/java/com/yahoo/config/ModelNode.java
@@ -23,13 +23,13 @@ public class ModelNode extends LeafNode<Path> {
 
     public ModelNode(ModelReference modelReference) {
         super(true);
-        this.value = modelReference.value();
+        this.value = modelReference.value(); // The resolved value, or null if not resolved
         this.reference = modelReference;
     }
 
     @Override
     public String getValue() {
-        return value.toString();
+        return reference.toString();
     }
 
     @Override
diff --git a/config-lib/src/main/java/com/yahoo/config/ModelReference.java b/config-lib/src/main/java/com/yahoo/config/ModelReference.java
index ba35812db4d..ecee42225db 100644
--- a/config-lib/src/main/java/com/yahoo/config/ModelReference.java
+++ b/config-lib/src/main/java/com/yahoo/config/ModelReference.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.config;
 
-import java.io.File;
 import java.nio.file.Path;
 import java.util.Objects;
 import java.util.Optional;
@@ -59,14 +58,7 @@ public class ModelReference {
 
     /** Returns the path to the file containing this model, or null if not available. */
     public Path value() {
-        if (resolved != null)
-            return resolved;
-
-        if (url.isPresent() && new File(url.get().value()).exists())
-            return Path.of(url.get().value());
-        if (path.isPresent())
-            return Path.of(path.get().value());
-        return null;
+        return resolved;
     }
 
     @Override
@@ -86,9 +78,10 @@ public class ModelReference {
     /** Returns this on the format accepted by valueOf */
     @Override
     public String toString() {
-        return modelId.orElse("") + " " +
-               url.map(v -> v.value()).orElse("") + " " +
-               path.map(v -> v.value()).orElse("");
+        if (resolved != null) return resolved.toString();
+        return modelId.orElse("\"\"") + " " +
+               url.map(v -> v.value()).orElse("\"\"") + " " +
+               path.map(v -> v.value()).orElse("\"\"");
     }
 
     /** Creates a model reference having a model id only. */
@@ -121,9 +114,9 @@ public class ModelReference {
         if (parts.length == 1)
             return new ModelReference(Path.of(s));
         else if (parts.length == 3)
-            return new ModelReference(parts[0].equals("") ? Optional.empty() : Optional.of(parts[0]),
-                                      parts[1].equals("") ? Optional.empty() : Optional.of(new UrlReference(parts[1])),
-                                      parts[2].equals("") ? Optional.empty() : Optional.of(new FileReference(parts[2])));
+            return new ModelReference(parts[0].equals("\"\"") ? Optional.empty() : Optional.of(parts[0]),
+                                      parts[1].equals("\"\"") ? Optional.empty() : Optional.of(new UrlReference(parts[1])),
+                                      parts[2].equals("\"\"") ? Optional.empty() : Optional.of(new FileReference(parts[2])));
         else
             throw new IllegalArgumentException("Unexpected model string '" + s + "'");
     }
diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java
index b0ce2291438..4f078059f46 100644
--- a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java
+++ b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java
@@ -250,10 +250,12 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> {
     private ModelReference resolveModel(String modelStringValue) {
         var model = ModelReference.valueOf(modelStringValue);
         // Resolve any of url and path present, in priority order
-        if (model.url().isPresent() && canResolveUrls())
-            model = model.withUrl(Optional.of(resolveUrl(model.url().get().value())));
-        else if (model.path().isPresent())
-            model = model.withPath(Optional.of(resolvePath(model.path().get().value())));
+        if (model.url().isPresent() && canResolveUrls()) {
+            model = new ModelReference(Path.of(resolveUrl(model.url().get().value()).value()));
+        }
+        else if (model.path().isPresent()) {
+            model = new ModelReference(Path.of(resolvePath(model.path().get().value()).value()));
+        }
         return model;
     }
 
diff --git a/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java b/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java
index 4d3b6b93b3b..aa517c943de 100644
--- a/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java
+++ b/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java
@@ -11,28 +11,78 @@ import java.io.File;
 import java.nio.file.Path;
 import java.util.Optional;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
 /**
  * @author bratseth
  */
 public class ConfigPayloadApplierTest {
 
     @Test
-    public void testConfigApplier() {
-        var applier = new ConfigPayloadApplier<>(new ResolvedTypesConfig.Builder(), new MockAcquirer(), new MockDownloader());
-        var config = new ResolvedTypesConfig.Builder();
-        config.myPath(new FileReference("mock/myPath.txt"));
-        config.myUrl(new UrlReference("mock/myUrl.txt"));
-        config.myModel(new ModelReference(Optional.empty(),
-                                          Optional.of(new UrlReference("mockPath/myPath.txt")),
-                                          Optional.of(new FileReference("mockUrl/myUrl.txt"))));
-        applier.applyPayload(ConfigPayload.fromInstance(config.build()));
+    public void testAllConfigApplierReferenceTypes() {
+        var configBuilder = new ResolvedTypesConfig.Builder();
+        var applier = new ConfigPayloadApplier<>(configBuilder, new MockAcquirer(), new MockDownloader());
+
+        var inputConfig = new ResolvedTypesConfig.Builder();
+        inputConfig.myPath(new FileReference("myPath.txt"));
+        inputConfig.myUrl(new UrlReference("myUrl.txt"));
+        inputConfig.myModel(new ModelReference(Optional.empty(),
+                                               Optional.of(new UrlReference("myUrl.txt")),
+                                               Optional.of(new FileReference("myPath.txt"))));
+        applier.applyPayload(ConfigPayload.fromInstance(inputConfig.build()));
+        var config = configBuilder.build();
+
+        assertEndsWith("resolvedPath/myPath.txt", config.myPath().toString());
+        assertEndsWith("resolvedUrl/myUrl.txt", config.myUrl().toString());
+        assertEndsWith("resolvedUrl/myUrl.txt", config.myModel().toString());
+    }
+
+    @Test
+    public void testModelWithUrlOnly() {
+        var configBuilder = new ResolvedTypesConfig.Builder();
+        var applier = new ConfigPayloadApplier<>(configBuilder, new MockAcquirer(), new MockDownloader());
+
+        var inputConfig = new ResolvedTypesConfig.Builder();
+        inputConfig.myPath(new FileReference("myPath.txt"));
+        inputConfig.myUrl(new UrlReference("myUrl.txt"));
+        inputConfig.myModel(ModelReference.valueOf("my-id myUrl.txt \"\""));
+        applier.applyPayload(ConfigPayload.fromInstance(inputConfig.build()));
+        var config = configBuilder.build();
+
+        assertEndsWith("resolvedUrl/myUrl.txt", config.myModel().toString());
+    }
+
+    @Test
+    public void testModelWithPathOnly() {
+        var configBuilder = new ResolvedTypesConfig.Builder();
+        var applier = new ConfigPayloadApplier<>(configBuilder, new MockAcquirer(), new MockDownloader());
+
+        var inputConfig = new ResolvedTypesConfig.Builder();
+        inputConfig.myPath(new FileReference("myPath.txt"));
+        inputConfig.myUrl(new UrlReference("myUrl.txt"));
+        inputConfig.myModel(ModelReference.valueOf("my-id \"\" myPath.txt"));
+        applier.applyPayload(ConfigPayload.fromInstance(inputConfig.build()));
+        var config = configBuilder.build();
+
+        assertEndsWith("resolvedPath/myPath.txt", config.myModel().toString());
+    }
+
+    private void assertEndsWith(String ending, String string) {
+        String assertingThat = "'" + string + "' ends with '" + ending + "'";
+        try {
+            assertEquals(assertingThat, ending, string.substring(string.length() - ending.length()));
+        }
+        catch (StringIndexOutOfBoundsException e) {
+            fail(assertingThat);
+        }
     }
 
     private static class MockAcquirer implements ConfigTransformer.PathAcquirer {
 
         @Override
         public Path getPath(FileReference fileReference) {
-            return Path.of("mockPath", fileReference.value());
+            return Path.of("resolvedPath", fileReference.value());
         }
 
     }
@@ -41,7 +91,7 @@ public class ConfigPayloadApplierTest {
 
         @Override
         public File waitFor(UrlReference urlReference, long timeout) {
-            return new File("mockUrl", urlReference.value());
+            return new File("resolvedUrl", urlReference.value());
         }
 
     }