Merge pull request #11379 from vespa-engine/bratseth/add-some-test-cases

Add some test cases: No functional changes

5 files changed, 91 insertions, 15 deletions

diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json
index 2a03614396b..66502b62ee8 100644
--- a/container-search/abi-spec.json
+++ b/container-search/abi-spec.json
@@ -5599,6 +5599,7 @@
       "public"
     ],
     "methods": [
+      "public static com.yahoo.search.query.profile.DimensionBinding createFrom(java.util.Map)",
       "public static com.yahoo.search.query.profile.DimensionBinding createFrom(java.util.List, java.util.Map)",
       "public static com.yahoo.search.query.profile.DimensionBinding createFrom(java.util.List, com.yahoo.search.query.profile.DimensionValues)",
       "public com.yahoo.search.query.profile.DimensionBinding createFor(java.util.List)",
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 1fc1e19e3ee..50bd2c58da8 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
@@ -4,6 +4,7 @@ package com.yahoo.search.query.profile;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -21,7 +22,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; // TODO: This is not needed any more
 
     public static final DimensionBinding nullBinding =
         new DimensionBinding(Collections.unmodifiableList(Collections.emptyList()), DimensionValues.empty, null);
@@ -32,7 +33,13 @@ public class DimensionBinding {
     /** Whether the value array contains only nulls */
     private boolean containsAllNulls;
 
+    // NOTE: Map must be ordered
+    public static DimensionBinding createFrom(Map<String,String> values) {
+        return createFrom(new ArrayList<>(values.keySet()), values);
+    }
+
     /** Creates a binding from a variant and a context. Any of the arguments may be null. */
+    // NOTE: Map must be ordered
     public static DimensionBinding createFrom(List<String> dimensions, Map<String,String> context) {
         if (dimensions == null || dimensions.size() == 0) {
             if (context == null) return nullBinding;
@@ -102,7 +109,7 @@ public class DimensionBinding {
      * in the corresponding order. The array is always of the same length as the number of dimensions.
      * Dimensions which are not set in this context get a null value.
      */
-    private static DimensionValues extractDimensionValues(List<String> dimensions,Map<String,String> context) {
+    private static DimensionValues extractDimensionValues(List<String> dimensions, Map<String,String> context) {
         String[] dimensionValues=new String[dimensions.size()];
         if (context==null || context.size()==0) return DimensionValues.createFrom(dimensionValues);
         for (int i=0; i<dimensions.size(); i++)
@@ -131,6 +138,16 @@ public class DimensionBinding {
         return DimensionBinding.createFrom(combinedDimensions, combinedValues);
     }
 
+    /** Returns the binding of this (dimension->value) as a map */
+    private Map<String, String> asMap() {
+        Map<String, String> map = new LinkedHashMap<>();
+        for (int i = 0; i < Math.min(dimensions.size(), values.size()); i++) {
+            if (values.getValues()[i] != null)
+                map.put(dimensions.get(i), values.getValues()[i]);
+        }
+        return map;
+    }
+
     /**
      * Returns a combined list of dimensions from two separate lists,
      * or null if they are incompatible.
@@ -169,7 +186,7 @@ public class DimensionBinding {
      * or null if they are incompatible.
      */
     private Map<String, String> combineValues(Map<String, String> m1, Map<String, String> m2) {
-        Map<String, String> combinedValues = new HashMap<>(m1);
+        Map<String, String> combinedValues = new LinkedHashMap<>(m1);
         for (Map.Entry<String, String> m2Entry : m2.entrySet()) {
             if (m2Entry.getValue() == null) continue;
             String m1Value = m1.get(m2Entry.getKey());
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 3cc7570576b..cffe941b912 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
@@ -82,7 +82,6 @@ public class QueryProfileCompiler {
             variants.addAll(parentVariants);
             variants.addAll(combined(variants, parentVariants)); // parents and children may have different variant dimensions
         }
-
         variants.addAll(wildcardExpanded(variants));
         return variants;
     }
@@ -152,8 +151,10 @@ public class QueryProfileCompiler {
         Set<DimensionBindingForPath> variants = new HashSet<>();
         if (profileVariants != null) {
             for (QueryProfileVariant variant : profile.getVariants().getVariants()) {
+                // Allow switching order since we're entering another profile
                 DimensionBinding combinedVariant =
                         DimensionBinding.createFrom(profile.getDimensions(), variant.getDimensionValues()).combineWith(currentVariant);
+
                 if (combinedVariant.isInvalid()) continue; // values at this point in the graph are unreachable
 
                 variants.addAll(collectVariantsFromValues(path, variant.values(), combinedVariant));
diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java
index 73b455c147b..7d3bec7cc9e 100644
--- a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java
@@ -14,6 +14,7 @@ import com.yahoo.search.query.profile.compiled.CompiledQueryProfile;
 import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry;
 import com.yahoo.search.query.profile.compiled.ValueWithSource;
 import com.yahoo.yolean.trace.TraceNode;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import java.util.Arrays;
@@ -86,15 +87,15 @@ public class QueryProfileVariantsTestCase {
         registry.register(parent);
         registry.register(profile);
         registry.register(referenced);
-        profile.set("feed.main", referenced, registry);
-        referenced.set("streams", "default_value", registry);
-        referenced.set("streams", "variant_value", new String[] { "x1", null, "z1" }, registry);
+        profile.set("a.b", referenced, registry);
+        referenced.set("c", "default_value", registry);
+        referenced.set("c", "variant_value", new String[] { "x1", null, "z1" }, registry);
 
         CompiledQueryProfileRegistry cRegistry = registry.compile();
         CompiledQueryProfile cTest = cRegistry.findQueryProfile("test");
 
-        assertEquals("default_value", new Query("?", cTest).properties().get("feed.main.streams"));
-        assertEquals("variant_value", new Query("?x=x1&y=y1&z=z1", cTest).properties().get("feed.main.streams"));
+        assertEquals("default_value", new Query("?", cTest).properties().get("a.b.c"));
+        assertEquals("variant_value", new Query("?x=x1&y=y1&z=z1", cTest).properties().get("a.b.c"));
 
         {
             Map<String, ValueWithSource> values = cRegistry.findQueryProfile("test")
@@ -102,9 +103,9 @@ public class QueryProfileVariantsTestCase {
                                                                                   new HashMap<>(),
                                                                                   null);
             assertEquals(1, values.size());
-            assertEquals("default_value", values.get("feed.main.streams").value());
-            assertEquals("referenced", values.get("feed.main.streams").source());
-            assertTrue(values.get("feed.main.streams").variant().isEmpty());
+            assertEquals("default_value", values.get("a.b.c").value());
+            assertEquals("referenced", values.get("a.b.c").source());
+            assertTrue(values.get("a.b.c").variant().isEmpty());
         }
 
         {
@@ -113,9 +114,9 @@ public class QueryProfileVariantsTestCase {
                                                                                   toMap("x=x1", "y=y1", "z=z1"),
                                                                                   null);
             assertEquals(2, values.size());
-            assertEquals("variant_value", values.get("feed.main.streams").value());
-            assertEquals("referenced", values.get("feed.main.streams").source());
-            assertEquals("[x1, *, z1]", values.get("feed.main.streams").variant().get().toString());
+            assertEquals("variant_value", values.get("a.b.c").value());
+            assertEquals("referenced", values.get("a.b.c").source());
+            assertEquals("[x1, *, z1]", values.get("a.b.c").variant().get().toString());
 
             assertEquals("otherValue", values.get("other").value());
             assertEquals("parent", values.get("other").source());
@@ -124,6 +125,61 @@ public class QueryProfileVariantsTestCase {
     }
 
     @Test
+    public void testSwitchingDimensionOrderInReferencedVariantWithFullOverlap() {
+        QueryProfileRegistry registry = new QueryProfileRegistry();
+        QueryProfile profile = new QueryProfile("test");
+        profile.setDimensions(new String[] { "x", "i", "j", "y" });
+        QueryProfile referenced = new QueryProfile("referenced");
+        referenced.setDimensions(new String[] { "y", "x" });
+        registry.register(profile);
+        registry.register(referenced);
+        profile.set("a.b", referenced, new String[] { "x1", "i1", "j1", "y1" }, registry);
+        referenced.set("c", "variant_value", new String[] { "y1" }, registry);
+
+        CompiledQueryProfileRegistry cRegistry = registry.compile();
+        CompiledQueryProfile cTest = cRegistry.findQueryProfile("test");
+
+        assertEquals("variant_value", new Query("?x=x1&i=i1&j=j1&y=y1", cTest).properties().get("a.b.c"));
+    }
+
+    @Test
+    public void testSameDimensionOrderInReferencedVariantWithPartialOverlap() {
+        QueryProfileRegistry registry = new QueryProfileRegistry();
+        QueryProfile profile = new QueryProfile("test");
+        profile.setDimensions(new String[] { "x", "i", "j", "y" });
+        QueryProfile referenced = new QueryProfile("referenced");
+        referenced.setDimensions(new String[] { "x", "y" });
+        registry.register(profile);
+        registry.register(referenced);
+        profile.set("a.b", referenced, new String[] { "x1", "i1", "j1" }, registry);
+        referenced.set("c", "variant_value", new String[] { null, "y1" }, registry);
+
+        CompiledQueryProfileRegistry cRegistry = registry.compile();
+        CompiledQueryProfile cTest = cRegistry.findQueryProfile("test");
+
+        assertEquals("variant_value", new Query("?x=x1&i=i1&j=j1&y=y1", cTest).properties().get("a.b.c"));
+    }
+
+    @Test
+    @Ignore // Switching order is not supported
+    public void testSwitchingDimensionOrderInReferencedVariantWithPartialOverlap() {
+        QueryProfileRegistry registry = new QueryProfileRegistry();
+        QueryProfile profile = new QueryProfile("test");
+        profile.setDimensions(new String[] { "x", "i", "j", "y" });
+        QueryProfile referenced = new QueryProfile("referenced");
+        referenced.setDimensions(new String[] { "y", "x" });
+        registry.register(profile);
+        registry.register(referenced);
+        profile.set("a.b", referenced, new String[] { "x1", "i1", "j1" }, registry);
+        referenced.set("c", "variant_value", new String[] { "y1" }, registry);
+
+        CompiledQueryProfileRegistry cRegistry = registry.compile();
+        CompiledQueryProfile cTest = cRegistry.findQueryProfile("test");
+
+        assertEquals("variant_value", new Query("?x=x1&i=i1&j=j1&y=y1", cTest).properties().get("a.b.c"));
+    }
+
+    @Test
     public void testMultipleMatchingVariantsAndDefault() {
         QueryProfile profile = new QueryProfile("test");
         profile.setDimensions(new String[] { "a", "b" });
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index f2aa186fe4d..0c6ee546166 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -41,6 +41,7 @@ import static com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl.Containe
  * @author bakksjo
  */
 public class NodeAgentImpl implements NodeAgent {
+
     // This is used as a definition of 1 GB when comparing flavor specs in node-repo
     private static final long BYTES_IN_GB = 1_000_000_000L;