Validate local substitutions during compile

4 files changed, 51 insertions, 33 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java b/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java
index 0059b761734..1fc1e19e3ee 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java
@@ -21,7 +21,7 @@ public class DimensionBinding {
     private DimensionValues values;
 
     /** The binding from those dimensions to values, and possibly other values */
-    private Map<String,String> context;
+    private Map<String, String> context;
 
     public static final DimensionBinding nullBinding =
         new DimensionBinding(Collections.unmodifiableList(Collections.emptyList()), DimensionValues.empty, null);
@@ -195,11 +195,11 @@ public class DimensionBinding {
     @Override
     public String toString() {
         if (isInvalid()) return "Invalid DimensionBinding";
-        if (dimensions==null) return "DimensionBinding []";
-        StringBuilder b=new StringBuilder("DimensionBinding [");
-        for (int i=0; i<dimensions.size(); i++) {
+        if (dimensions == null) return "DimensionBinding []";
+        StringBuilder b = new StringBuilder("DimensionBinding [");
+        for (int i = 0; i < dimensions.size(); i++) {
             b.append(dimensions.get(i)).append("=").append(values.get(i));
-            if (i<dimensions.size()-1)
+            if (i < dimensions.size()-1)
                 b.append(", ");
         }
         b.append("]");
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java
index accef7ba154..fd2852fda60 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java
@@ -33,31 +33,36 @@ public class QueryProfileCompiler {
     }
 
     public static CompiledQueryProfile compile(QueryProfile in, CompiledQueryProfileRegistry registry) {
-        DimensionalMap.Builder<CompoundName, Object> values = new DimensionalMap.Builder<>();
-        DimensionalMap.Builder<CompoundName, QueryProfileType> types = new DimensionalMap.Builder<>();
-        DimensionalMap.Builder<CompoundName, Object> references = new DimensionalMap.Builder<>();
-        DimensionalMap.Builder<CompoundName, Object> unoverridables = new DimensionalMap.Builder<>();
-
-        // Resolve values for each existing variant and combine into a single data structure
-        Set<DimensionBindingForPath> variants = collectVariants(CompoundName.empty, in, DimensionBinding.nullBinding);
-        variants.add(new DimensionBindingForPath(DimensionBinding.nullBinding, CompoundName.empty)); // if this contains no variants
-        log.fine(() -> "Compiling " + in.toString() + " having " + variants.size() + " variants");
-        for (DimensionBindingForPath variant : variants) {
-            log.finer(() -> "  Compiling variant " + variant);
-            for (Map.Entry<String, Object> entry : in.listValues(variant.path(), variant.binding().getContext(), null).entrySet()) {
-                values.put(variant.path().append(entry.getKey()), variant.binding(), entry.getValue());
+        try {
+            DimensionalMap.Builder<CompoundName, Object> values = new DimensionalMap.Builder<>();
+            DimensionalMap.Builder<CompoundName, QueryProfileType> types = new DimensionalMap.Builder<>();
+            DimensionalMap.Builder<CompoundName, Object> references = new DimensionalMap.Builder<>();
+            DimensionalMap.Builder<CompoundName, Object> unoverridables = new DimensionalMap.Builder<>();
+
+            // Resolve values for each existing variant and combine into a single data structure
+            Set<DimensionBindingForPath> variants = collectVariants(CompoundName.empty, in, DimensionBinding.nullBinding);
+            variants.add(new DimensionBindingForPath(DimensionBinding.nullBinding, CompoundName.empty)); // if this contains no variants
+            log.fine(() -> "Compiling " + in.toString() + " having " + variants.size() + " variants");
+            for (DimensionBindingForPath variant : variants) {
+                log.finer(() -> "  Compiling variant " + variant);
+                for (Map.Entry<String, Object> entry : in.listValues(variant.path(), variant.binding().getContext(), null).entrySet()) {
+                    values.put(variant.path().append(entry.getKey()), variant.binding(), entry.getValue());
+                }
+                for (Map.Entry<CompoundName, QueryProfileType> entry : in.listTypes(variant.path(), variant.binding().getContext()).entrySet())
+                    types.put(variant.path().append(entry.getKey()), variant.binding(), entry.getValue());
+                for (CompoundName reference : in.listReferences(variant.path(), variant.binding().getContext()))
+                    references.put(variant.path().append(reference), variant.binding(), Boolean.TRUE); // Used as a set; value is ignored
+                for (CompoundName name : in.listUnoverridable(variant.path(), variant.binding().getContext()))
+                    unoverridables.put(variant.path().append(name), variant.binding(), Boolean.TRUE); // Used as a set; value is ignored
             }
-            for (Map.Entry<CompoundName, QueryProfileType> entry : in.listTypes(variant.path(), variant.binding().getContext()).entrySet())
-                types.put(variant.path().append(entry.getKey()), variant.binding(), entry.getValue());
-            for (CompoundName reference : in.listReferences(variant.path(), variant.binding().getContext()))
-                references.put(variant.path().append(reference), variant.binding(), Boolean.TRUE); // Used as a set; value is ignored
-            for (CompoundName name : in.listUnoverridable(variant.path(), variant.binding().getContext()))
-                unoverridables.put(variant.path().append(name), variant.binding(), Boolean.TRUE); // Used as a set; value is ignored
-        }
 
-        return new CompiledQueryProfile(in.getId(), in.getType(),
-                                        values.build(), types.build(), references.build(), unoverridables.build(),
-                                        registry);
+            return new CompiledQueryProfile(in.getId(), in.getType(),
+                                            values.build(), types.build(), references.build(), unoverridables.build(),
+                                            registry);
+        }
+        catch (IllegalArgumentException e) {
+            throw new IllegalArgumentException("Invalid " + in, e);
+        }
     }
 
     /**
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java
index 5e7f2783393..50d0a2de46f 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java
@@ -180,7 +180,11 @@ public class DimensionalValue<VALUE> {
                             if (component instanceof SubstituteString.RelativePropertyComponent) {
                                 SubstituteString.RelativePropertyComponent relativeComponent = (SubstituteString.RelativePropertyComponent)component;
                                 var substituteValues = lookupByLocalName(relativeComponent.fieldName(), entries);
-                                String resolved = substituteValues == null ? "(null)" : substituteValues.valueFor(variant).toString();
+                                if (substituteValues == null)
+                                    throw new IllegalArgumentException("Could not resolve local substitution '" +
+                                                                       relativeComponent.fieldName() + "' in variant " +
+                                                                       variant);
+                                String resolved = substituteValues.valueFor(variant).toString();
                                 resolvedComponents.add(new SubstituteString.StringComponent(resolved));
                             }
                             else {
diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileSubstitutionTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileSubstitutionTestCase.java
index 1e51a4890be..b3b83b9c07e 100644
--- a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileSubstitutionTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileSubstitutionTestCase.java
@@ -43,11 +43,20 @@ public class QueryProfileSubstitutionTestCase {
         p.set("message","Hello %{.world}!", null);
         p.set("world", "world", null);
         assertEquals("Hello world!", p.compile(null).get("message"));
+    }
 
-        QueryProfile p2 = new QueryProfile("test2");
-        p2.addInherited(p);
-        p2.set("world", "universe", null);
-        assertEquals("Hello universe!", p2.compile(null).get("message"));
+    @Test
+    public void testRelativeSubstitutionNotFound() {
+        try {
+            QueryProfile p = new QueryProfile("test");
+            p.set("message", "Hello %{.world}!", null);
+            assertEquals("Hello world!", p.compile(null).get("message"));
+            fail("Expected exception");
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("Invalid query profile 'test': Could not resolve local substitution 'world' in variant DimensionBinding []",
+                         Exceptions.toMessageString(e));
+        }
     }
 
     @Test