diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2018-08-10 22:46:55 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-08-10 22:46:55 +0200 |
commit | 7c33ec7cecbf65bd1019576d658af51cd104cab0 (patch) | |
tree | c3dcd730ebd17ad6a314424ff0d7edc7bfdc982a /container-search | |
parent | 047586da5f5889a5329b45c9059b2fc9fb5bdce6 (diff) |
Revert "Added a object structure for GroupingRequest objects, accessable from…"
Diffstat (limited to 'container-search')
6 files changed, 48 insertions, 36 deletions
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java index b023166fe9e..8ce0d90dfc5 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java @@ -22,6 +22,7 @@ import java.util.*; */ public class GroupingRequest { + private final static CompoundName PROP_REQUEST = new CompoundName(GroupingRequest.class.getName() + ".Request"); private final List<Continuation> continuations = new ArrayList<>(); private final int requestId; private GroupingOperation root; @@ -133,6 +134,10 @@ public class GroupingRequest { */ public static GroupingRequest newInstance(Query query) { List<GroupingRequest> lst = getRequests(query); + if (lst.isEmpty()) { + lst = new LinkedList<>(); + query.properties().set(PROP_REQUEST, lst); + } GroupingRequest ret = new GroupingRequest(lst.size()); lst.add(ret); return ret; @@ -146,8 +151,14 @@ public class GroupingRequest { * @return The list of grouping requests. */ @SuppressWarnings({ "unchecked" }) - @Deprecated public static List<GroupingRequest> getRequests(Query query) { - return query.getSelect().getGrouping(); + Object lst = query.properties().get(PROP_REQUEST); + if (lst == null) { + return Collections.emptyList(); + } + if (!(lst instanceof List)) { + throw new IllegalArgumentException("Expected " + GroupingRequest.class + ", got " + lst.getClass() + "."); + } + return (List<GroupingRequest>)lst; } } diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java index 2b3f80a4b8f..bf7eb8dc12e 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java @@ -46,8 +46,7 @@ import com.yahoo.vespa.objects.ObjectPredicate; public class GroupingExecutor extends Searcher { public final static String COMPONENT_NAME = "GroupingExecutor"; - private final static String GROUPING_LIST = "GroupingList"; - private final static CompoundName PROP_GROUPINGLIST = newCompoundName(GROUPING_LIST); + private final static CompoundName PROP_GROUPINGLIST = newCompoundName("GroupingList"); private final static Logger log = Logger.getLogger(GroupingExecutor.class.getName()); /** diff --git a/container-search/src/main/java/com/yahoo/search/query/Select.java b/container-search/src/main/java/com/yahoo/search/query/Select.java index ef6a7fe8272..3ffc6bddb24 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Select.java +++ b/container-search/src/main/java/com/yahoo/search/query/Select.java @@ -10,9 +10,6 @@ import com.yahoo.search.query.profile.types.FieldDescription; import com.yahoo.search.query.profile.types.QueryProfileType; import com.yahoo.search.yql.VespaGroupingStep; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; /** @@ -30,11 +27,11 @@ public class Select implements Cloneable { public static final String WHERE = "where"; public static final String GROUPING = "grouping"; + private static Model model; private Query parent; private String where = ""; private String grouping = ""; - private List<GroupingRequest> groupingRequests = new ArrayList<>(); static { argumentType = new QueryProfileType(SELECT); @@ -70,10 +67,8 @@ public class Select implements Cloneable { } - /** Set the where-clause for the query. Must be a JSON-string, with the format described in the Select Reference doc: - * @see <a href="https://docs.vespa.ai/documentation/reference/select-reference.html">https://docs.vespa.ai/documentation/reference/select-reference.html</a> - */ - public void setWhereString(String where) { + /** Set the where-clause for the query. Must be a JSON-string, with the format described in the Select Reference doc - https://docs.vespa.ai/documentation/reference/select-reference.html. */ + public void setWhere(String where) { this.where = where; model.setType(SELECT); @@ -83,13 +78,13 @@ public class Select implements Cloneable { /** Returns the where-clause in the query */ - public String getWhereString(){ return where; } + public String getWhereString(){ + return this.where; + } - /** Set the grouping-string for the query. Must be a JSON-string, with the format described in the Select Reference doc: - * @see <a href="https://docs.vespa.ai/documentation/reference/select-reference.html">https://docs.vespa.ai/documentation/reference/select-reference.html</a> - * */ - public void setGroupingString(String grouping){ + /** Set the grouping-string for the query. Must be a JSON-string, with the format described in the Select Reference doc - https://docs.vespa.ai/documentation/reference/select-reference.html. */ + public void setGrouping(String grouping){ this.grouping = grouping; SelectParser parser = (SelectParser) ParserFactory.newInstance(Query.Type.SELECT, new ParserEnvironment()); @@ -103,14 +98,10 @@ public class Select implements Cloneable { /** Returns the grouping in the query */ public String getGroupingString(){ - return grouping; + return this.grouping; } - /** Returns the query's {@link GroupingRequest} objects, as mutable list */ - public List<GroupingRequest> getGrouping(){ return groupingRequests; } - - @Override public String toString() { return "where: [" + where + "], grouping: [" + grouping+ "]"; 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 559a7279f83..71002166b11 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 @@ -3,11 +3,10 @@ package com.yahoo.search.query.properties; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; -import com.yahoo.search.grouping.GroupingRequest; -import com.yahoo.search.grouping.vespa.GroupingExecutor; import com.yahoo.search.query.*; import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry; 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 com.yahoo.search.query.ranking.Diversity; import com.yahoo.search.query.ranking.MatchPhase; @@ -15,11 +14,11 @@ import com.yahoo.search.query.ranking.Matching; import com.yahoo.search.query.ranking.SoftTimeout; import com.yahoo.tensor.Tensor; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; - - /** * Maps between the query model and text properties. * This can be done simpler by using reflection but the performance penalty was not worth it, @@ -140,9 +139,7 @@ public class QueryProperties extends Properties { if (key.toString().equals(Model.MODEL)) return query.getModel(); if (key.toString().equals(Ranking.RANKING)) return query.getRanking(); if (key.toString().equals(Presentation.PRESENTATION)) return query.getPresentation(); - } - return super.get(key, context, substitution); } @@ -256,9 +253,9 @@ public class QueryProperties extends Properties { } else if (key.size()==2 && key.first().equals(Select.SELECT)) { if (key.last().equals(Select.WHERE)){ - query.getSelect().setWhereString(asString(value, "")); + query.getSelect().setWhere(asString(value, "")); } else if (key.last().equals(Select.GROUPING)) { - query.getSelect().setGroupingString(asString(value, "")); + query.getSelect().setGrouping(asString(value, "")); } } else if (key.first().equals("rankfeature") || key.first().equals("featureoverride") ) { // featureoverride is deprecated @@ -280,7 +277,8 @@ public class QueryProperties extends Properties { query.setGroupingSessionCache(asBoolean(value, false)); else super.set(key,value,context); - } else + } + else super.set(key,value,context); } catch (Exception e) { // Make sure error messages are informative. This should be moved out of this properties implementation diff --git a/container-search/src/test/java/com/yahoo/search/grouping/GroupingRequestTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/GroupingRequestTestCase.java index 12f4067ba1f..494602be7b3 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/GroupingRequestTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/GroupingRequestTestCase.java @@ -10,10 +10,8 @@ import com.yahoo.search.result.Hit; import org.junit.Test; import java.lang.reflect.Field; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.List; import static org.junit.Assert.*; @@ -115,7 +113,20 @@ public class GroupingRequestTestCase { GroupingRequest bar = GroupingRequest.newInstance(query); assertEquals(Arrays.asList(foo, bar), GroupingRequest.getRequests(query)); } - + + @Test + public void requireThatGetRequestThrowsIllegalArgumentOnBadProperty() throws Exception { + Query query = new Query(); + Field propName = GroupingRequest.class.getDeclaredField("PROP_REQUEST"); + propName.setAccessible(true); + query.properties().set((CompoundName)propName.get(null), new Object()); + try { + GroupingRequest.getRequests(query); + fail(); + } catch (IllegalArgumentException e) { + + } + } private static RootGroup newRootGroup(int id) { return new RootGroup(id, new Continuation() { diff --git a/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java b/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java index b19743a1c60..031ba386ad4 100644 --- a/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java @@ -15,12 +15,14 @@ import com.yahoo.prelude.query.WeakAndItem; import com.yahoo.prelude.query.WordAlternativesItem; import com.yahoo.prelude.query.WordItem; import com.yahoo.search.Query; +import com.yahoo.search.federation.ProviderConfig; import com.yahoo.search.query.QueryTree; import com.yahoo.search.query.Select; import com.yahoo.search.query.SelectParser; import com.yahoo.search.query.parser.Parsable; import com.yahoo.search.query.parser.ParserEnvironment; import com.yahoo.search.yql.VespaGroupingStep; +import org.apache.http.client.utils.URIBuilder; import org.json.JSONException; import org.json.JSONObject; import org.junit.Test; @@ -667,7 +669,7 @@ public class SelectParserTestCase { assertEquals("default:query", query.getModel().getQueryTree().toString()); assertEquals(Query.Type.ALL, query.getModel().getType()); - query.getSelect().setWhereString("{\"contains\" : [\"default\", \"select\"] }"); + query.getSelect().setWhere("{\"contains\" : [\"default\", \"select\"] }"); assertEquals("default:select", query.getModel().getQueryTree().toString()); assertEquals(Query.Type.SELECT, query.getModel().getType()); } @@ -676,7 +678,7 @@ public class SelectParserTestCase { @Test public void testOverridingWhereQueryTree() { Query query = new Query(); - query.getSelect().setWhereString("{\"contains\" : [\"default\", \"select\"] }"); + query.getSelect().setWhere("{\"contains\" : [\"default\", \"select\"] }"); assertEquals("default:select", query.getModel().getQueryTree().toString()); assertEquals(Query.Type.SELECT, query.getModel().getType()); |