diff options
10 files changed, 193 insertions, 34 deletions
diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 5b86c3587cd..82d3223c8fe 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -5790,6 +5790,7 @@ "public com.yahoo.search.query.profile.compiled.CompiledQueryProfile getQueryProfile()", "public java.lang.Object get(com.yahoo.processing.request.CompoundName, java.util.Map, com.yahoo.processing.request.Properties)", "public void set(com.yahoo.processing.request.CompoundName, java.lang.Object, java.util.Map)", + "public void clearAll(com.yahoo.processing.request.CompoundName, java.util.Map)", "public java.util.Map listProperties(com.yahoo.processing.request.CompoundName, java.util.Map, com.yahoo.processing.request.Properties)", "public boolean isComplete(java.lang.StringBuilder, java.util.Map)", "public com.yahoo.search.query.profile.QueryProfileProperties clone()", diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java b/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java index 4dc9ade62e5..e32d2dc226d 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java @@ -22,8 +22,7 @@ public class ModelObjectMap extends PropertyMap { */ @Override protected boolean shouldSet(CompoundName name, Object value) { - if (value == null) return true; - return ! FieldType.isLegalFieldValue(value); + return value != null && ! FieldType.isLegalFieldValue(value); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java index 05e3c4fe9a0..701ea7690f4 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java @@ -32,7 +32,12 @@ public class QueryProfileProperties extends Properties { /** Values which has been overridden at runtime, or null if none */ private Map<CompoundName, Object> values = null; - /** Query profile references which has been overridden at runtime, or null if none. Earlier values has precedence */ + + /** + * Query profile references which has been overridden at runtime, possibly to the null value to clear values, + * or null if none (i.e this is lazy). + * Earlier values has precedence + */ private List<Pair<CompoundName, CompiledQueryProfile>> references = null; /** Creates an instance from a profile, throws an exception if the given profile is null */ @@ -49,20 +54,21 @@ public class QueryProfileProperties extends Properties { public Object get(CompoundName name, Map<String, String> context, com.yahoo.processing.request.Properties substitution) { name = unalias(name, context); - Object value = null; - if (values != null) - value = values.get(name); - if (value == null) { - Pair<CompoundName, CompiledQueryProfile> reference = findReference(name); - if (reference != null) - return reference.getSecond().get(name.rest(reference.getFirst().size()), context, substitution); // yes; even if null + if (values != null && values.containsKey(name)) + return values.get(name); // Returns this value, even if null + + Pair<CompoundName, CompiledQueryProfile> reference = findReference(name); + if (reference != null) { + if (reference.getSecond() == null) + return null; // cleared + else + return reference.getSecond().get(name.rest(reference.getFirst().size()), context, substitution); // even if null } - if (value == null) - value = profile.get(name, context, substitution); - if (value == null) - value = super.get(name, context, substitution); - return value; + Object value = profile.get(name, context, substitution); + if (value != null) + return value; + return super.get(name, context, substitution); } /** @@ -143,12 +149,26 @@ public class QueryProfileProperties extends Properties { } @Override - public Map<String, Object> listProperties(CompoundName path, Map<String,String> context, + public void clearAll(CompoundName name, Map<String, String> context) { + if (references == null) + references = new ArrayList<>(); + references.add(new Pair<>(name, null)); + + if (values != null) + values.keySet().removeIf(key -> key.hasPrefix(name)); + } + + @Override + public Map<String, Object> listProperties(CompoundName path, Map<String, String> context, com.yahoo.processing.request.Properties substitution) { path = unalias(path, context); if (context == null) context = Collections.emptyMap(); - Map<String, Object> properties = profile.listValues(path, context, substitution); + Map<String, Object> properties = new HashMap<>(); + for (var entry : profile.listValues(path, context, substitution).entrySet()) { + if (references != null && containsNullParentOf(path, references)) continue; + properties.put(entry.getKey(), entry.getValue()); + } properties.putAll(super.listProperties(path, context, substitution)); if (references != null) { @@ -165,8 +185,14 @@ public class QueryProfileProperties extends Properties { pathInReference = path.rest(refEntry.getFirst().size()); prefixToReferenceKeys = CompoundName.empty; } - for (Map.Entry<String, Object> valueEntry : refEntry.getSecond().listValues(pathInReference, context, substitution).entrySet()) { - properties.put(prefixToReferenceKeys.append(new CompoundName(valueEntry.getKey())).toString(), valueEntry.getValue()); + if (refEntry.getSecond() == null) { + if (refEntry.getFirst().hasPrefix(path)) + properties.put(prefixToReferenceKeys.toString(), null); + } + else { + for (Map.Entry<String, Object> valueEntry : refEntry.getSecond().listValues(pathInReference, context, substitution).entrySet()) { + properties.put(prefixToReferenceKeys.append(new CompoundName(valueEntry.getKey())).toString(), valueEntry.getValue()); + } } } @@ -241,6 +267,12 @@ public class QueryProfileProperties extends Properties { return null; } + private boolean containsNullParentOf(CompoundName path, List<Pair<CompoundName, CompiledQueryProfile>> properties) { + if (properties.contains(new Pair<>(path, (CompiledQueryProfile)null))) return true; + if (path.size() > 0 && containsNullParentOf(path.first(path.size() - 1), properties)) return true; + return false; + } + CompoundName unalias(CompoundName name, Map<String,String> context) { if (profile.getTypes().isEmpty()) return name; diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java index 644d366e7d0..94f4e4747a0 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java @@ -134,7 +134,6 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable public Map<String, Object> listValues(CompoundName prefix, Map<String, String> context, Properties substitution) { Map<String, Object> values = new HashMap<>(); for (Map.Entry<CompoundName, DimensionalValue<ValueWithSource>> entry : entries.entrySet()) { - if ( entry.getKey().size() <= prefix.size()) continue; if ( ! entry.getKey().hasPrefix(prefix)) continue; ValueWithSource valueWithSource = entry.getValue().get(context); diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java index 4f30331e738..643e215daef 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java @@ -26,7 +26,10 @@ public class PropertyMap extends Properties { /** The properties of this */ private Map<CompoundName, Object> properties = new LinkedHashMap<>(); - public void set(CompoundName name, Object value, Map<String,String> context) { + public void set(CompoundName name, Object value, Map<String, String> context) { + if (value == null) // Both clear and forward + properties.remove(name); + if (shouldSet(name, value)) properties.put(name, value); else diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java index dfe6c2af44b..96f73e925af 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java @@ -294,7 +294,7 @@ public class QueryProperties extends Properties { super.set(key,value,context); } catch (Exception e) { // Make sure error messages are informative. This should be moved out of this properties implementation - if (e.getMessage().startsWith("Could not set")) + if (e.getMessage() != null && e.getMessage().startsWith("Could not set")) throw e; else throw new IllegalArgumentException("Could not set '" + key + "' to '" + value + "'", e); diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java index bda191ee910..eb1584efe84 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java @@ -504,7 +504,8 @@ public class QueryProfileTestCase { p.set("a","a-value", null); p.set("a.b","a.b-value", null); Map<String, Object> values = p.compile(null).listValues("a"); - assertEquals(1, values.size()); + assertEquals(2, values.size()); + p.set("a","a-value", null); assertEquals("a.b-value", values.get("b")); } diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java index e71ed308aaf..34c3da395b7 100644 --- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java @@ -45,6 +45,8 @@ import org.junit.Test; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -345,6 +347,61 @@ public class QueryTestCase { } @Test + public void testQueryProfileClearAndSet() { + QueryProfile profile = new QueryProfile("myProfile"); + profile.set("b", "b-value", null); + Query q = new Query(QueryTestCase.httpEncode("/search?queryProfile=myProfile"), profile.compile(null)); + assertEquals("b-value", q.properties().get("b")); + assertContains(q.properties().listProperties("b"), "b-value"); + + q.properties().set("b", null, null); + assertContains(q.properties().listProperties("b"), (Object)null); + + q.properties().set("b", "b-value", null); + assertEquals("b-value", q.properties().get("b")); + assertContains(q.properties().listProperties("b"), "b-value"); + } + + @Test + public void testQueryProfileClearValue() { + QueryProfile profile = new QueryProfile("myProfile"); + profile.set("a", "a-value", null); + profile.set("b", "b-value", null); + profile.set("b.c", "b.c-value", null); + profile.set("b.d", "b.d-value", null); + Query q = new Query(QueryTestCase.httpEncode("/search?queryProfile=myProfile"), profile.compile(null)); + assertEquals("a-value", q.properties().get("a")); + assertEquals("b-value", q.properties().get("b")); + assertEquals("b.c-value", q.properties().get("b.c")); + assertEquals("b.d-value", q.properties().get("b.d")); + assertContains(q.properties().listProperties("b"), "b-value", "b.c-value", "b.d-value"); + + q.properties().set("a", null, null); + assertEquals(null, q.properties().get("a")); + + q.properties().set("b", null, null); + assertEquals(null, q.properties().get("b")); + assertEquals("b.c-value", q.properties().get("b.c")); + assertEquals("b.d-value", q.properties().get("b.d")); + assertContains(q.properties().listProperties("b"), null, "b.c-value", "b.d-value"); + + q.properties().set("b", "b-value", null); + q.properties().set("b.e", "b.e-value", null); + q.properties().set("b.f", "b.f-value", null); + assertEquals("b-value", q.properties().get("b")); + assertEquals("b.e-value", q.properties().get("b.e")); + assertContains(q.properties().listProperties("b"), "b-value", "b.c-value", "b.d-value", "b.e-value", "b.f-value"); + + q.properties().clearAll("b"); + assertEquals(null, q.properties().get("b")); + assertEquals(null, q.properties().get("b.c")); + assertEquals(null, q.properties().get("b.d")); + assertEquals(null, q.properties().get("b.e")); + assertEquals(null, q.properties().get("b.f")); + assertContains(q.properties().listProperties("b"), (Object)null); + } + + @Test public void testNotEqual() { Query q = new Query("/?query=something+test&nocache"); Query p = new Query("/?query=something+test"); @@ -985,6 +1042,19 @@ public class QueryTestCase { assertEquals(expectedDetectionText, mockLinguistics.detector.lastDetectionText); } + private void assertContains(Map<String, Object> properties, Object ... expectedValues) { + if (expectedValues == null) { + assertEquals(1, properties.size()); + assertTrue("Contains value null", properties.containsValue(null)); + } + else { + assertEquals(properties + " contains values " + Arrays.toString(expectedValues), + expectedValues.length, properties.size()); + for (Object expectedValue : expectedValues) + assertTrue("Contains value " + expectedValue, properties.containsValue(expectedValue)); + } + } + /** A linguistics instance which records the last language detection text passed to it */ private static class MockLinguistics extends SimpleLinguistics { diff --git a/processing/abi-spec.json b/processing/abi-spec.json index 78058f1a8b7..8f77672faec 100644 --- a/processing/abi-spec.json +++ b/processing/abi-spec.json @@ -329,6 +329,10 @@ "public final void set(java.lang.String, java.lang.Object, java.util.Map)", "public final void set(com.yahoo.processing.request.CompoundName, java.lang.Object)", "public final void set(java.lang.String, java.lang.Object)", + "public void clearAll(com.yahoo.processing.request.CompoundName, java.util.Map)", + "public final void clearAll(java.lang.String, java.lang.Object, java.util.Map)", + "public final void clearAll(com.yahoo.processing.request.CompoundName)", + "public final void clearAll(java.lang.String)", "public final boolean getBoolean(com.yahoo.processing.request.CompoundName)", "public final boolean getBoolean(java.lang.String)", "public final boolean getBoolean(com.yahoo.processing.request.CompoundName, boolean)", diff --git a/processing/src/main/java/com/yahoo/processing/request/Properties.java b/processing/src/main/java/com/yahoo/processing/request/Properties.java index 095a0e51ce0..cadc658417b 100644 --- a/processing/src/main/java/com/yahoo/processing/request/Properties.java +++ b/processing/src/main/java/com/yahoo/processing/request/Properties.java @@ -206,8 +206,8 @@ public class Properties implements Cloneable { * This default implementation forwards to the chained instance or throws * a RuntimeException if there is not chained instance. * - * @param name the name of the value - * @param value the value to set. Setting a name to null explicitly is legal. + * @param name the name of the property + * @param value the value to set. Setting a property to null clears it. * @param context the context used to resolve where the values should be set, or null if none * @throws RuntimeException if no instance in the chain accepted this name-value pair */ @@ -223,8 +223,8 @@ public class Properties implements Cloneable { * This default implementation forwards to the chained instance or throws * a RuntimeException if there is not chained instance. * - * @param name the name of the value - * @param value the value to set. Setting a name to null explicitly is legal. + * @param name the name of the property + * @param value the value to set. Setting a property to null clears it. * @param context the context used to resolve where the values should be set, or null if none * @throws RuntimeException if no instance in the chain accepted this name-value pair */ @@ -235,8 +235,8 @@ public class Properties implements Cloneable { /** * Sets a value to the first chained instance which accepts it by calling set(name,value,null). * - * @param name the name of the value - * @param value the value to set. Setting a name to null explicitly is legal. + * @param name the name of the property + * @param value the value to set. Setting a property to null clears it. * @throws RuntimeException if no instance in the chain accepted this name-value pair */ public final void set(CompoundName name, Object value) { @@ -246,8 +246,8 @@ public class Properties implements Cloneable { /** * Sets a value to the first chained instance which accepts it by calling set(name,value,null). * - * @param name the name of the value - * @param value the value to set. Setting a name to null explicitly is legal. + * @param name the name of the property + * @param value the value to set. Setting a property to null clears it. * @throws RuntimeException if no instance in the chain accepted this name-value pair */ public final void set(String name, Object value) { @@ -255,6 +255,56 @@ public class Properties implements Cloneable { } /** + * Sets all properties having this name as a compound prefix to null. + * I.e clearAll("a") will clear the value of "a" and "a.b" but not "ab". + * This default implementation forwards to the chained instance or throws + * a RuntimeException if there is not chained instance. + * + * @param name the compound prefix of the properties to clear + * @param context the context used to resolve where the values should be cleared, or null if none + * @throws RuntimeException if no instance in the chain accepted this name-value pair + */ + public void clearAll(CompoundName name, Map<String, String> context) { + if (chained == null) throw new RuntimeException("Property '" + name + + "' was not accepted in this property chain"); + chained.clearAll(name, context); + } + + /** + * Sets all properties having this name as a compound prefix to null. + * I.e clearAll("a") will clear the value of "a" and "a.b" but not "ab". + * + * @param name the compound prefix of the properties to clear + * @param context the context used to resolve where the values should be cleared, or null if none + * @throws RuntimeException if no instance in the chain accepted this name-value pair + */ + public final void clearAll(String name, Object value, Map<String, String> context) { + set(new CompoundName(name), value, context); + } + + /** + * Sets all properties having this name as a compound prefix to null. + * I.e clearAll("a") will clear the value of "a" and "a.b" but not "ab". + * + * @param name the compound prefix of the properties to clear + * @throws RuntimeException if no instance in the chain accepted this name-value pair + */ + public final void clearAll(CompoundName name) { + clearAll(name, null); + } + + /** + * Sets all properties having this name as a compound prefix to null. + * I.e clearAll("a") will clear the value of "a" and "a.b" but not "ab". + * + * @param name the compound prefix of the properties to clear + * @throws RuntimeException if no instance in the chain accepted this name-value pair + */ + public final void clearAll(String name) { + clearAll(new CompoundName(name), Collections.<String,String>emptyMap()); + } + + /** * Gets a property as a boolean - if this value can reasonably be interpreted as a boolean, this will return * the value. Returns false if this property is null. */ @@ -571,14 +621,14 @@ public class Properties implements Cloneable { } } - /** - * Clones a map by deep cloning each value which is cloneable and shallow copying all other values. - */ + /** Clones a map by deep cloning each value which is cloneable and shallow copying all other values. */ public static Map<CompoundName, Object> cloneMap(Map<CompoundName, Object> map) { return cloneHelper.cloneMap(map); } + /** Clones this object if it is clonable, and the clone is public. Returns null if not */ public static Object clone(Object object) { return cloneHelper.clone(object); } + } |