diff options
author | Jon Bratseth <bratseth@gmail.com> | 2020-11-04 21:23:21 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2020-11-04 21:23:21 +0100 |
commit | 3c49543bb514abd8c3040fc4355f6ccc537468e1 (patch) | |
tree | e9b48215a89c2170853a3fd16e69b5b70846c8e3 | |
parent | 95ca6fdcb45b1d3ae805c5c9f3d20cc7972f136d (diff) |
Revert "Merge pull request #15181 from vespa-engine/revert-15180-bratseth/override-in-variant-references"
This reverts commit 95ca6fdcb45b1d3ae805c5c9f3d20cc7972f136d, reversing
changes made to 19f9396e6b60e49c7830d78d504a14c015600f2b.
8 files changed, 97 insertions, 50 deletions
diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 86e7ddc7e52..113760b85cb 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -5868,7 +5868,7 @@ "public com.yahoo.search.query.profile.DimensionValues getDimensionValues()", "public java.util.Map values()", "public java.util.List inherited()", - "public void set(java.lang.String, java.lang.Object)", + "public java.lang.Object set(java.lang.String, java.lang.Object)", "public void inherit(com.yahoo.search.query.profile.QueryProfile)", "public int compareTo(com.yahoo.search.query.profile.QueryProfileVariant)", "public boolean matches(com.yahoo.search.query.profile.DimensionValues)", @@ -5899,6 +5899,7 @@ "public int compareTo(com.yahoo.search.query.profile.QueryProfileVariants$FieldValue)", "public com.yahoo.search.query.profile.QueryProfileVariants$FieldValue clone(java.lang.String, java.util.List)", "public com.yahoo.search.query.profile.QueryProfileVariants$FieldValue clone()", + "public java.lang.String toString()", "public bridge synthetic java.lang.Object clone()", "public bridge synthetic int compareTo(java.lang.Object)" ], @@ -5954,6 +5955,7 @@ "public com.yahoo.search.query.profile.QueryProfileVariants clone()", "protected void ensureNotFrozen()", "public com.yahoo.search.query.profile.QueryProfileVariant getVariant(com.yahoo.search.query.profile.DimensionValues, boolean)", + "public java.lang.String toString()", "public bridge synthetic java.lang.Object clone()" ], "fields": [] diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java b/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java index 3c336c80d37..68bf112133a 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/AllValuesQueryProfileVisitor.java @@ -3,9 +3,6 @@ package com.yahoo.search.query.profile; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.query.profile.compiled.ValueWithSource; -import com.yahoo.search.query.profile.types.FieldDescription; -import com.yahoo.search.query.profile.types.QueryProfileFieldType; -import com.yahoo.search.query.profile.types.QueryProfileType; import java.util.Collections; import java.util.HashMap; @@ -16,7 +13,7 @@ import java.util.Map; */ final class AllValuesQueryProfileVisitor extends PrefixQueryProfileVisitor { - private Map<String, ValueWithSource> values = new HashMap<>(); + private final Map<String, ValueWithSource> values = new HashMap<>(); /* Lists all values starting at prefix */ public AllValuesQueryProfileVisitor(CompoundName prefix) { 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 11864e60cec..0bb36d87f64 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 @@ -22,7 +22,7 @@ import java.util.Map; public class BackedOverridableQueryProfile extends OverridableQueryProfile implements Cloneable { /** The backing read only query profile, or null if this is not backed */ - private QueryProfile backingProfile; + private final QueryProfile backingProfile; /** * Creates an overridable profile from the given backing profile. The backing profile will never be @@ -95,16 +95,16 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple } @Override - protected void visitInherited(boolean allowContent,QueryProfileVisitor visitor,DimensionBinding dimensionBinding, QueryProfile owner) { - super.visitInherited(allowContent,visitor,dimensionBinding, owner); + 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,owner); } /** Returns a value from the content of this: The value in this, or the value from the backing if not set in this */ protected Object getContent(String localKey) { - Object value=super.getContent(localKey); - if (value!=null) return value; + Object value = super.getContent(localKey); + if (value != null) return value; return backingProfile.getContent(localKey); } @@ -125,7 +125,7 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple @Override public String toString() { - return "overridable wrapper of " + backingProfile.toString(); + return "overridable wrapper of " + backingProfile; } @Override 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 6008b046d1a..d0a42e8a1f9 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 @@ -366,7 +366,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable * @throws IllegalArgumentException if the given name is illegal given the types of this or any nested query profile * @throws IllegalStateException if this query profile is frozen */ - public final void set(String name,Object value, DimensionValues dimensionValues, QueryProfileRegistry registry) { + public final void set(String name, Object value, DimensionValues dimensionValues, QueryProfileRegistry registry) { set(new CompoundName(name), value, DimensionBinding.createFrom(getDimensions(), dimensionValues), registry); } @@ -527,7 +527,6 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable QueryProfileVisitor visitor, DimensionBinding dimensionBinding, QueryProfile owner) { - //System.out.println(" visiting " + this); visitor.onQueryProfile(this, dimensionBinding, owner, null); if (visitor.isDone()) return; @@ -541,7 +540,6 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable if (visitor.visitInherited()) visitInherited(allowContent, visitor, dimensionBinding, owner); - //System.out.println(" done visiting " + this); } protected void visitVariants(boolean allowContent, QueryProfileVisitor visitor, DimensionBinding dimensionBinding) { @@ -738,7 +736,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable parent.overridable.put(fieldName.last(), overridable); } - /** Sets a value to a (possibly non-local) node. The parent query profile holding the value is returned */ + /** Sets a value to a (possibly non-local) node. */ private void setNode(CompoundName name, Object value, QueryProfileType parentType, DimensionBinding dimensionBinding, QueryProfileRegistry registry) { ensureNotFrozen(); @@ -811,19 +809,18 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable validateName(localName); value = convertToSubstitutionString(value); - if (dimensionBinding.isNull()) { - Object combinedValue; - if (value instanceof QueryProfile) - combinedValue = combineValues(value, content == null ? null : content.get(localName)); - else - combinedValue = combineValues(value, localLookup(localName, dimensionBinding)); - if (combinedValue!=null) + if (dimensionBinding.isNull()) { + Object combinedValue = value instanceof QueryProfile + ? combineValues(value, content == null ? null : content.get(localName)) + : combineValues(value, localLookup(localName, dimensionBinding)); + if (combinedValue != null) 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 3f70ff98373..a33ee33b652 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 @@ -16,13 +16,13 @@ public class QueryProfileVariant implements Cloneable, Comparable<QueryProfileVa private List<QueryProfile> inherited = null; - private DimensionValues dimensionValues; + private final DimensionValues dimensionValues; private Map<String, Object> values; private boolean frozen = false; - private QueryProfile owner; + private final QueryProfile owner; public QueryProfileVariant(DimensionValues dimensionValues, QueryProfile owner) { this.dimensionValues = dimensionValues; @@ -59,20 +59,16 @@ public class QueryProfileVariant implements Cloneable, Comparable<QueryProfileVa return inherited; } - public void set(String key, Object newValue) { + public Object set(String key, Object newValue) { if (values == null) values = new HashMap<>(); Object oldValue = values.get(key); - if (oldValue == null) { - values.put(key, newValue); - } else { - Object combinedOrNull = QueryProfile.combineValues(newValue, oldValue); - if (combinedOrNull != null) { - values.put(key, combinedOrNull); - } - } + Object combinedOrNull = QueryProfile.combineValues(newValue, oldValue); + if (combinedOrNull != null) + values.put(key, combinedOrNull); + return combinedOrNull; } public void inherit(QueryProfile profile) { @@ -138,6 +134,7 @@ public class QueryProfileVariant implements Cloneable, Comparable<QueryProfileVa frozen=true; } + @Override public QueryProfileVariant clone() { if (frozen) return this; try { @@ -156,7 +153,7 @@ public class QueryProfileVariant implements Cloneable, Comparable<QueryProfileVa @Override public String toString() { - return "query profile variant for " + dimensionValues; + return "query profile variant of " + owner + " for " + dimensionValues; } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariants.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariants.java index 4c4d6778d86..062b9d8c6e4 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariants.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileVariants.java @@ -43,10 +43,10 @@ public class QueryProfileVariants implements Freezable, Cloneable { * Order matters - more specific values to the left in this list are more significant than more specific values * to the right */ - private List<String> dimensions; + private final List<String> dimensions; /** The query profile this variants of */ - private QueryProfile owner; + private final QueryProfile owner; /** * Creates a set of virtual query profiles which may return varying values over the set of dimensions given. @@ -105,14 +105,10 @@ public class QueryProfileVariants implements Freezable, Cloneable { if (contentName != null) { if (type != null) contentName = type.unalias(contentName); - //System.out.println(" accepting single value in " + this + " for local key " + contentName); acceptSingleValue(contentName, allowContent, visitor, dimensionBinding); // Special cased for performance - //System.out.println(" done accepting single value in " + this + " for local key " + contentName); } else { - //System.out.println(" accepting all values in " + this); acceptAllValues(allowContent, visitor, type, dimensionBinding); - //System.out.println(" done accepting all values in " + this); } } @@ -223,7 +219,7 @@ public class QueryProfileVariants implements Freezable, Cloneable { ensureNotFrozen(); // Update variant - getVariant(dimensionValues, true).set(fieldName, value); + Object combinedValue = getVariant(dimensionValues, true).set(fieldName, value); // Update per-variable optimized structure FieldValues fieldValues = fieldValuesByName.get(fieldName); @@ -232,7 +228,6 @@ public class QueryProfileVariants implements Freezable, Cloneable { fieldValuesByName.put(fieldName, fieldValues); } - Object combinedValue = QueryProfile.combineValues(value, fieldValues.getExact(dimensionValues)); if (combinedValue != null) fieldValues.put(dimensionValues, combinedValue); } @@ -261,6 +256,7 @@ public class QueryProfileVariants implements Freezable, Cloneable { return Collections.unmodifiableList(variants); } + @Override public QueryProfileVariants clone() { try { if (frozen) return this; @@ -308,9 +304,12 @@ public class QueryProfileVariants implements Freezable, Cloneable { return variant; } + @Override + public String toString() { return "variants of " + owner; } + public static class FieldValues implements Freezable, Cloneable { - private List<FieldValue> resolutionList=null; + private List<FieldValue> resolutionList = null; private boolean frozen = false; @@ -424,7 +423,7 @@ public class QueryProfileVariants implements Freezable, Cloneable { public static class FieldValue implements Comparable<FieldValue>, Cloneable { - private DimensionValues dimensionValues; + private final DimensionValues dimensionValues; private Object value; public FieldValue(DimensionValues dimensionValues, Object value) { @@ -462,7 +461,7 @@ public class QueryProfileVariants implements Freezable, Cloneable { } /** Clone by filling in the value from the given variants */ - public FieldValue clone(String fieldName,List<QueryProfileVariant> clonedVariants) { + public FieldValue clone(String fieldName, List<QueryProfileVariant> clonedVariants) { try { FieldValue clone = (FieldValue)super.clone(); if (this.value instanceof QueryProfile) @@ -475,6 +474,7 @@ public class QueryProfileVariants implements Freezable, Cloneable { } } + @Override public FieldValue clone() { try { FieldValue clone = (FieldValue)super.clone(); @@ -494,6 +494,9 @@ public class QueryProfileVariants implements Freezable, Cloneable { return null; } + @Override + public String toString() { return "field value " + value + " for " + dimensionValues; } + } } 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 03faeaae8a0..89217bb7f0c 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 @@ -23,6 +23,7 @@ import java.util.Map; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; /** @@ -74,6 +75,57 @@ public class QueryProfileVariantsTestCase { } @Test + public void testReferenceInVariant() { + QueryProfileRegistry registry = new QueryProfileRegistry(); + QueryProfile test = new QueryProfile("test"); + test.setDimensions(new String[] { "d1" }); + registry.register(test); + + QueryProfile references = new QueryProfile("referenced"); + references.setDimensions(new String[] { "d1" }); + registry.register(references); + + QueryProfile other = new QueryProfile("other"); + other.setDimensions(new String[] { "d1" }); + registry.register(other); + + test.set( "a", references, new String[] { "d1v"}, registry); + test.set( "a.b", "test-value", new String[] { "d1v"}, registry); + other.set( "a", references, new String[] { "d1v"}, registry); + other.set("a.b", "other-value", new String[] { "d1v"}, registry); + + assertEquals("test-value", test.get("a.b", new String[] { "d1v"})); + assertEquals("other-value", other.get("a.b", new String[] { "d1v"})); + assertNull(references.get("b", new String[] { "d1v"})); + + var cRegistry = registry.compile(); + assertEquals("test-value", + cRegistry.getComponent("test").get("a.b", Map.of("d1", "d1v"))); + } + + @Test + public void testReference() { + QueryProfileRegistry registry = new QueryProfileRegistry(); + QueryProfile test = new QueryProfile("test"); + registry.register(test); + + QueryProfile references = new QueryProfile("referenced"); + registry.register(references); + + QueryProfile other = new QueryProfile("other"); + registry.register(other); + + test.set( "a", references, registry); + test.set( "a.b", "test-value", registry); + other.set( "a", references, registry); + other.set("a.b", "other-value", registry); + + assertEquals("test-value", test.get("a.b")); + assertEquals("other-value", other.get("a.b")); + assertNull(references.get("b")); + } + + @Test public void testVariantInReferencedAndParentWithOtherMatchingVariant() { QueryProfileRegistry registry = new QueryProfileRegistry(); QueryProfile parent = new QueryProfile("parent"); diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogManager.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogManager.java index 5b0367c5a55..ee1939d9bbe 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogManager.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/OsgiLogManager.java @@ -26,12 +26,11 @@ class OsgiLogManager implements LogService { this.configureLogLevel = configureLogLevel; } - @SuppressWarnings("unchecked") - public void install(final BundleContext osgiContext) { + public void install(BundleContext osgiContext) { if (tracker != null) { throw new IllegalStateException("OsgiLogManager already installed."); } - tracker = new ServiceTracker<>(osgiContext, LogService.class, new ServiceTrackerCustomizer<LogService,LogService>() { + tracker = new ServiceTracker<>(osgiContext, LogService.class, new ServiceTrackerCustomizer<>() { @Override public LogService addingService(ServiceReference<LogService> reference) { |