diff options
author | Jon Bratseth <bratseth@gmail.com> | 2021-04-28 15:49:57 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2021-04-28 15:49:57 +0200 |
commit | 2ee3e1d2e9fdda8d10a91e616e3a4f8d758d7dbe (patch) | |
tree | c20f09ad439cb4f2854a27b2a77dbd250c6b3b4d /container-search | |
parent | 357de578bc8d79c71ddb6bbbcf37d11d46902020 (diff) |
Use owner's dimensions in BackedOverridableQueryProfile
Use owner's dimensions rather than the backed's in
BackedObverridableQueryProfile. This matters when a profile
references a profile with different but overlapping dimensions
and both the owner and referred profile assigns values for
the same paths, within their respective dimension spaces.
Diffstat (limited to 'container-search')
5 files changed, 48 insertions, 17 deletions
diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index 8d1f8212dbf..f6bf91f5f85 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java @@ -303,7 +303,6 @@ public class FederationSearcher extends ForkingSearcher { Object value = getSourceOrProviderProperty(original, key, sourceName, providerName, window.get(key)); if (value != null) outgoing.properties().set(key, value); - if (value != null) System.out.println("Setting " + key + " = " + value); } } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java index 0bb36d87f64..9eab6629a45 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java @@ -31,7 +31,7 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple * @param backingProfile the backing profile, which is assumed read only, never null */ public BackedOverridableQueryProfile(QueryProfile backingProfile) { - Validator.ensureNotNull("An overridable query profile must be backed by a real query profile",backingProfile); + Validator.ensureNotNull("An overridable query profile must be backed by a real query profile", backingProfile); setType(backingProfile.getType()); this.backingProfile = backingProfile; } @@ -49,7 +49,7 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple protected Object localLookup(String localName, DimensionBinding dimensionBinding) { Object valueInThis = super.localLookup(localName, dimensionBinding); if (valueInThis != null) return valueInThis; - return backingProfile.localLookup(localName, dimensionBinding); + return backingProfile.localLookup(localName, dimensionBinding.createFor(backingProfile.getDimensions())); } protected Boolean isLocalInstanceOverridable(String localName) { @@ -88,17 +88,17 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple } @Override - protected void visitVariants(boolean allowContent,QueryProfileVisitor visitor,DimensionBinding dimensionBinding) { + protected void visitVariants(boolean allowContent, QueryProfileVisitor visitor, DimensionBinding dimensionBinding) { super.visitVariants(allowContent, visitor, dimensionBinding); if (visitor.isDone()) return; - backingProfile.visitVariants(allowContent, visitor, dimensionBinding); + backingProfile.visitVariants(allowContent, visitor, dimensionBinding.createFor(backingProfile.getDimensions())); } @Override protected void visitInherited(boolean allowContent, QueryProfileVisitor visitor, DimensionBinding dimensionBinding, QueryProfile owner) { super.visitInherited(allowContent, visitor, dimensionBinding, owner); if (visitor.isDone()) return; - backingProfile.visitInherited(allowContent, visitor, dimensionBinding,owner); + backingProfile.visitInherited(allowContent, visitor, dimensionBinding.createFor(backingProfile.getDimensions()), owner); } /** Returns a value from the content of this: The value in this, or the value from the backing if not set in this */ @@ -113,12 +113,12 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple * All the values in this, and all values in the backing where an overriding value is not set in this */ @Override - protected Map<String,Object> getContent() { - Map<String,Object> thisContent=super.getContent(); - Map<String,Object> backingContent=backingProfile.getContent(); + protected Map<String, Object> getContent() { + Map<String,Object> thisContent = super.getContent(); + Map<String,Object> backingContent = backingProfile.getContent(); if (thisContent.isEmpty()) return backingContent; // Shortcut if (backingContent.isEmpty()) return thisContent; // Shortcut - Map<String,Object> content=new HashMap<>(backingContent); + Map<String, Object> content = new HashMap<>(backingContent); content.putAll(thisContent); return content; } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java index 4371955ae63..d3da2fd076a 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java @@ -604,7 +604,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable } /** Sets the value of a node in <i>this</i> profile - the local name given must not be nested (contain dots) */ - protected QueryProfile setLocalNode(String localName, Object value,QueryProfileType parentType, + protected QueryProfile setLocalNode(String localName, Object value, QueryProfileType parentType, DimensionBinding dimensionBinding, QueryProfileRegistry registry) { if (parentType != null && type == null && ! isFrozen()) type = parentType; @@ -622,9 +622,10 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable static Object combineValues(Object newValue, Object existingValue) { if (newValue instanceof QueryProfile) { QueryProfile newProfile = (QueryProfile)newValue; - if ( existingValue == null || ! (existingValue instanceof QueryProfile)) { - if (!isModifiable(newProfile)) + if ( ! (existingValue instanceof QueryProfile)) { + if ( ! isModifiable(newProfile)) { newProfile = new BackedOverridableQueryProfile(newProfile); // Make the query profile reference overridable + } newProfile.value = existingValue; return newProfile; } @@ -829,7 +830,6 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable validateName(localName); value = convertToSubstitutionString(value); - if (dimensionBinding.isNull()) { Object combinedValue = value instanceof QueryProfile ? combineValues(value, content == null ? null : content.get(localName)) @@ -838,9 +838,8 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable content.put(localName, combinedValue); } else { - if (variants == null) { + if (variants == null) variants = new QueryProfileVariants(dimensionBinding.getDimensions(), this); - } variants.set(localName, dimensionBinding.getValues(), value); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariant.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariant.java index f6c43eab8a0..855befad658 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariant.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariant.java @@ -37,7 +37,7 @@ public class QueryProfileVariant implements Cloneable, Comparable<QueryProfileVa * Returns the live reference to the values of this. This may be modified * if this is not frozen. */ - public Map<String,Object> values() { + public Map<String, Object> values() { if (values == null) { if (frozen) return Collections.emptyMap(); @@ -68,6 +68,8 @@ public class QueryProfileVariant implements Cloneable, Comparable<QueryProfileVa Object oldValue = values.get(key); Object combinedOrNull = QueryProfile.combineValues(newValue, oldValue); + if (combinedOrNull instanceof BackedOverridableQueryProfile) // Use the owner's, not the referenced dimensions + ((QueryProfile) combinedOrNull).setDimensions(owner.getDimensions().toArray(new String[0])); if (combinedOrNull != null) values.put(key, combinedOrNull); return combinedOrNull; 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 89217bb7f0c..a592d40d2e9 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 @@ -10,6 +10,7 @@ import com.yahoo.search.query.profile.BackedOverridableQueryProfile; import com.yahoo.search.query.profile.QueryProfile; import com.yahoo.search.query.profile.QueryProfileProperties; import com.yahoo.search.query.profile.QueryProfileRegistry; +import com.yahoo.search.query.profile.QueryProfileVariant; import com.yahoo.search.query.profile.compiled.CompiledQueryProfile; import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry; import com.yahoo.search.query.profile.compiled.ValueWithSource; @@ -103,6 +104,36 @@ public class QueryProfileVariantsTestCase { cRegistry.getComponent("test").get("a.b", Map.of("d1", "d1v"))); } + /** + * Tests referencing a variant which modifies the dimension set, + * and also setting a value within that variants subspace. + */ + @Test + public void testVariantReference() { + QueryProfileRegistry registry = new QueryProfileRegistry(); + + QueryProfile parent = new QueryProfile("parent"); + parent.set("b", 48, registry); + registry.register(parent); + + QueryProfile referenced = new QueryProfile("referenced"); + referenced.addInherited(parent); + referenced.setDimensions(new String[] {"d2", "d3"}); + registry.register(referenced); + + QueryProfile base = new QueryProfile("base"); + base.setDimensions(new String[]{"d1", "d2", "d3"}); + base.set("a", referenced, new String[] {null, null, "d3-val"}, registry); + assertEquals("Variant dimensions are not overridden by the referenced dimensions", + "[d1, d2, d3]", + ((QueryProfile)base.getVariants().getVariants().get(0).values().get("a")).getDimensions().toString()); + base.set("a.b", 1, new String[] {null, null, "d3-val"}, registry); + QueryProfileVariant aVariants = base.getVariants().getVariants().get(0); + assertEquals("Variant dimensions are not overridden by the referenced dimensions", + "[d1, d2, d3]", + ((QueryProfile)base.getVariants().getVariants().get(0).values().get("a")).getDimensions().toString()); + } + @Test public void testReference() { QueryProfileRegistry registry = new QueryProfileRegistry(); |