diff options
author | Jon Bratseth <bratseth@verizonmedia.com> | 2020-02-13 13:37:14 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@verizonmedia.com> | 2020-02-13 13:37:14 +0100 |
commit | 2f1e103ba33d32f60f0c2460ffe0de91247aaa5d (patch) | |
tree | 78edd88785ac1207e96b4220561393e30e2c7bfa /container-search | |
parent | 74c0354105f3a00ceb4465eb7cb2aa9c1780e7ef (diff) |
Support clearing values in query profiles
Diffstat (limited to 'container-search')
8 files changed, 128 insertions, 23 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 { |