Revert "Revert "Added a object structure for GroupingRequest objects, accessable from…""

6 files changed, 36 insertions, 48 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 8ce0d90dfc5..b023166fe9e 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,7 +22,6 @@ 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;
@@ -134,10 +133,6 @@ 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;
@@ -151,14 +146,8 @@ public class GroupingRequest {
      * @return The list of grouping requests.
      */
     @SuppressWarnings({ "unchecked" })
+    @Deprecated
     public static List<GroupingRequest> getRequests(Query query) {
-        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;
+        return query.getSelect().getGrouping();
     }
 }
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 bf7eb8dc12e..2b3f80a4b8f 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,7 +46,8 @@ import com.yahoo.vespa.objects.ObjectPredicate;
 public class GroupingExecutor extends Searcher {
 
     public final static String COMPONENT_NAME = "GroupingExecutor";
-    private final static CompoundName PROP_GROUPINGLIST = newCompoundName("GroupingList");
+    private final static String GROUPING_LIST = "GroupingList";
+    private final static CompoundName PROP_GROUPINGLIST = newCompoundName(GROUPING_LIST);
     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 3ffc6bddb24..ef6a7fe8272 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,6 +10,9 @@ 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;
 
 
 /**
@@ -27,11 +30,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);
@@ -67,8 +70,10 @@ 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 - https://docs.vespa.ai/documentation/reference/select-reference.html. */
-    public void setWhere(String where) {
+    /** 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) {
         this.where = where;
         model.setType(SELECT);
 
@@ -78,13 +83,13 @@ public class Select implements Cloneable {
 
 
     /** Returns the where-clause in the query */
-    public String getWhereString(){
-        return this.where;
-    }
+    public String getWhereString(){ return where; }
 
 
-    /** 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){
+    /** 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){
         this.grouping = grouping;
         SelectParser parser = (SelectParser) ParserFactory.newInstance(Query.Type.SELECT, new ParserEnvironment());
 
@@ -98,10 +103,14 @@ public class Select implements Cloneable {
 
     /** Returns the grouping in the query */
     public String getGroupingString(){
-        return this.grouping;
+        return 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 71002166b11..559a7279f83 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,10 +3,11 @@ 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;
@@ -14,11 +15,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,
@@ -139,7 +140,9 @@ 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);
     }
 
@@ -253,9 +256,9 @@ public class QueryProperties extends Properties {
             }
             else if (key.size()==2 && key.first().equals(Select.SELECT)) {
                 if (key.last().equals(Select.WHERE)){
-                    query.getSelect().setWhere(asString(value, ""));
+                    query.getSelect().setWhereString(asString(value, ""));
                 } else if (key.last().equals(Select.GROUPING)) {
-                    query.getSelect().setGrouping(asString(value, ""));
+                    query.getSelect().setGroupingString(asString(value, ""));
                 }
             }
             else if (key.first().equals("rankfeature") || key.first().equals("featureoverride") ) { // featureoverride is deprecated
@@ -277,8 +280,7 @@ 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 494602be7b3..12f4067ba1f 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,8 +10,10 @@ 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.*;
 
@@ -113,20 +115,7 @@ 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 031ba386ad4..b19743a1c60 100644
--- a/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java
+++ b/container-search/src/test/java/com/yahoo/select/SelectParserTestCase.java
@@ -15,14 +15,12 @@ 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;
@@ -669,7 +667,7 @@ public class SelectParserTestCase {
         assertEquals("default:query", query.getModel().getQueryTree().toString());
         assertEquals(Query.Type.ALL, query.getModel().getType());
 
-        query.getSelect().setWhere("{\"contains\" : [\"default\", \"select\"] }");
+        query.getSelect().setWhereString("{\"contains\" : [\"default\", \"select\"] }");
         assertEquals("default:select", query.getModel().getQueryTree().toString());
         assertEquals(Query.Type.SELECT, query.getModel().getType());
     }
@@ -678,7 +676,7 @@ public class SelectParserTestCase {
     @Test
     public void testOverridingWhereQueryTree() {
         Query query = new Query();
-        query.getSelect().setWhere("{\"contains\" : [\"default\", \"select\"] }");
+        query.getSelect().setWhereString("{\"contains\" : [\"default\", \"select\"] }");
         assertEquals("default:select", query.getModel().getQueryTree().toString());
         assertEquals(Query.Type.SELECT, query.getModel().getType());