Refactor query internals to reduce property parsing for partial select and fetch (#2167)

* Update test only - update TestBasicClobNoVer to use DB and AssertJ

* Refactor OrmQueryProperties internals to reduce raw property parsing

- Remove String properties
- Reduces parsing of properties via  fetchProperties() selectProperties() methods

* No effective change - tidy pom.xml for ebean-querybean

* Refactor query internals adding fetchProperties() selectProperties()

- Adds SpiQueryFetch
- Used by query beans with Set<String> properties passed (effectively skipping any parsing)

ebean-core/src/main/java/io/ebeaninternal/api/SpiQuery.java

@@ -20,6 +20,7 @@ import io.ebeaninternal.server.deploy.TableJoin;
 import io.ebeaninternal.server.query.CancelableQuery;
 import io.ebeaninternal.server.querydefn.NaturalKeyBindParam;
 import io.ebeaninternal.server.querydefn.OrmQueryDetail;
+import io.ebeaninternal.server.querydefn.OrmQueryProperties;
 import io.ebeaninternal.server.querydefn.OrmUpdateProperties;
 import io.ebeaninternal.server.rawsql.SpiRawSql;
 
@@ -30,7 +31,7 @@ import java.util.Set;
 /**
  * Object Relational query - Internal extension to Query object.
  */
-public interface SpiQuery<T> extends Query<T>, TxnProfileEventCodes {
+public interface SpiQuery<T> extends Query<T>, SpiQueryFetch, TxnProfileEventCodes {
 
   enum Mode {
     NORMAL(false), LAZYLOAD_MANY(false), LAZYLOAD_BEAN(true), REFRESH_BEAN(true);
@@ -289,6 +290,16 @@ public interface SpiQuery<T> extends Query<T>, TxnProfileEventCodes {
    */
   boolean selectAllForLazyLoadProperty();
 
+  /**
+   * Set the select properties.
+   */
+  void selectProperties(OrmQueryProperties other);
+
+  /**
+   * Set the fetch properties for the given path.
+   */
+  void fetchProperties(String path, OrmQueryProperties other);
+
   /**
    * Set the on a secondary query given the label, relativePath and profile location of the parent query.
    */

ebean-core/src/main/java/io/ebeaninternal/api/SpiQueryFetch.java

@@ -0,0 +1,22 @@
+package io.ebeaninternal.api;
+
+import io.ebean.FetchConfig;
+
+import java.util.Set;
+
+/**
+ * Query select and fetch properties (that avoids parsing).
+ */
+public interface SpiQueryFetch {
+
+  /**
+   * Specify the select properties.
+   */
+  void selectProperties(Set<String> properties);
+
+  /**
+   * Specify the fetch properties for the given path.
+   */
+  void fetchProperties(String name, Set<String> properties, FetchConfig config);
+
+}

ebean-core/src/main/java/io/ebeaninternal/server/query/DFetchGroupBuilder.java

@@ -11,6 +11,8 @@ import io.ebeaninternal.server.querydefn.SpiFetchGroup;
  */
 class DFetchGroupBuilder<T> implements FetchGroupBuilder<T> {
 
+  private static final FetchConfig DEFAULT_FETCH = FetchConfig.ofDefault();
+
   private static final FetchConfig FETCH_CACHE = FetchConfig.ofCache();
 
   private static final FetchConfig FETCH_QUERY = FetchConfig.ofQuery();
@@ -31,13 +33,13 @@ class DFetchGroupBuilder<T> implements FetchGroupBuilder<T> {
 
   @Override
   public FetchGroupBuilder<T> fetch(String path) {
-    detail.fetch(path, null, null);
+    detail.fetchProperties(path, null, DEFAULT_FETCH);
     return this;
   }
 
   @Override
   public FetchGroupBuilder<T> fetch(String path, FetchGroup nestedGroup) {
-    return fetchNested(path, nestedGroup, null);
+    return fetchNested(path, nestedGroup, DEFAULT_FETCH);
   }
 
   @Override
@@ -51,7 +53,6 @@ class DFetchGroupBuilder<T> implements FetchGroupBuilder<T> {
   }
 
   private FetchGroupBuilder<T> fetchNested(String path, FetchGroup nestedGroup, FetchConfig fetchConfig) {
-
     OrmQueryDetail nestedDetail = ((SpiFetchGroup) nestedGroup).underlying();
     detail.addNested(path, nestedDetail, fetchConfig);
     return this;
@@ -59,25 +60,25 @@ class DFetchGroupBuilder<T> implements FetchGroupBuilder<T> {
 
   @Override
   public FetchGroupBuilder<T> fetchQuery(String path) {
-    detail.fetch(path, null, FETCH_QUERY);
+    detail.fetchProperties(path, null, FETCH_QUERY);
     return this;
   }
 
   @Override
   public FetchGroupBuilder<T> fetchCache(String path) {
-    detail.fetch(path, null, FETCH_CACHE);
+    detail.fetchProperties(path, null, FETCH_CACHE);
     return this;
   }
 
   @Override
   public FetchGroupBuilder<T> fetchLazy(String path) {
-    detail.fetch(path, null, FETCH_LAZY);
+    detail.fetchProperties(path, null, FETCH_LAZY);
     return this;
   }
 
   @Override
   public FetchGroupBuilder<T> fetch(String path, String properties) {
-    detail.fetch(path, properties, null);
+    detail.fetch(path, properties, DEFAULT_FETCH);
     return this;
   }
 

ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultFetchGroupQuery.java

@@ -25,6 +25,7 @@ import io.ebean.Transaction;
 import io.ebean.UpdateQuery;
 import io.ebean.Version;
 import io.ebean.service.SpiFetchGroupQuery;
+import io.ebeaninternal.api.SpiQueryFetch;
 import io.ebeaninternal.server.querydefn.OrmQueryDetail;
 import io.ebeaninternal.server.querydefn.SpiFetchGroup;
 
@@ -43,7 +44,7 @@ import java.util.stream.Stream;
 /**
  * Implementation of FetchGroup query for use to create FetchGroup via query beans.
  */
-class DefaultFetchGroupQuery<T> implements SpiFetchGroupQuery<T> {
+class DefaultFetchGroupQuery<T> implements SpiFetchGroupQuery<T>, SpiQueryFetch {
 
   private static final FetchConfig FETCH_CACHE = FetchConfig.ofCache();
 
@@ -628,4 +629,14 @@ class DefaultFetchGroupQuery<T> implements SpiFetchGroupQuery<T> {
   public Query<T> orderById(boolean orderById) {
     throw new RuntimeException("EB102: Only select() and fetch() clause is allowed on FetchGroup");
   }
+
+  @Override
+  public void selectProperties(Set<String> props) {
+    detail.selectProperties(props);
+  }
+
+  @Override
+  public void fetchProperties(String property, Set<String> columns, FetchConfig config) {
+    detail.fetchProperties(property, columns, config);
+  }
 }

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java

@@ -1392,6 +1392,26 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
     return this;
   }
 
+  @Override
+  public void selectProperties(Set<String> props) {
+    detail.selectProperties(props);
+  }
+
+  @Override
+  public void fetchProperties(String property, Set<String> columns, FetchConfig config) {
+    detail.fetchProperties(property, columns, config);
+  }
+
+  @Override
+  public void selectProperties(OrmQueryProperties properties) {
+    detail.selectProperties(properties);
+  }
+
+  @Override
+  public void fetchProperties(String path, OrmQueryProperties other) {
+    detail.fetchProperties(path, other);
+  }
+
   @Override
   public DefaultOrmQuery<T> select(String columns) {
     detail.select(columns);

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryDetail.java

@@ -59,9 +59,9 @@ public class OrmQueryDetail implements Serializable {
    * Add a nested OrmQueryDetail to this detail.
    */
   public void addNested(String path, OrmQueryDetail other, FetchConfig config) {
-    fetch(path, other.baseProps.getProperties(), config);
+    fetchProperties(path, other.baseProps, config);
     for (Map.Entry<String, OrmQueryProperties> entry : other.fetchPaths.entrySet()) {
-      fetch(path + "." + entry.getKey(), entry.getValue().getProperties(), entry.getValue().getFetchConfig());
+      fetchProperties(path + "." + entry.getKey(), entry.getValue(), entry.getValue().getFetchConfig());
     }
   }
 
@@ -133,8 +133,19 @@ public class OrmQueryDetail implements Serializable {
   /**
    * set the properties to include on the base / root entity.
    */
-  public void select(String columns) {
-    baseProps = new OrmQueryProperties(null, columns, null);
+  public void select(String properties) {
+    baseProps = new OrmQueryProperties(null, properties, null);
+  }
+
+  /**
+   * Set select properties that are already parsed.
+   */
+  public void selectProperties(Set<String> properties) {
+    baseProps = new OrmQueryProperties(null, properties, OrmQueryProperties.DEFAULT_FETCH);
+  }
+
+  void selectProperties(OrmQueryProperties other) {
+    baseProps = new OrmQueryProperties(null, other, OrmQueryProperties.DEFAULT_FETCH);
   }
 
   boolean containsProperty(String property) {
@@ -262,10 +273,24 @@ public class OrmQueryDetail implements Serializable {
    * @param partialProps the properties on the join property to include
    */
   public void fetch(String path, String partialProps, FetchConfig fetchConfig) {
-
     fetch(new OrmQueryProperties(path, partialProps, fetchConfig));
   }
 
+  /**
+   * Set fetch properties that are already parsed.
+   */
+  public void fetchProperties(String path, Set<String> properties, FetchConfig fetchConfig) {
+    fetch(new OrmQueryProperties(path, properties, fetchConfig));
+  }
+
+  void fetchProperties(String path, OrmQueryProperties other) {
+    fetchProperties(path, other, other.getFetchConfig());
+  }
+
+  void fetchProperties(String path, OrmQueryProperties other, FetchConfig fetchConfig) {
+    fetch(new OrmQueryProperties(path, other, fetchConfig));
+  }
+
   /**
    * Add for raw sql etc when the properties are already parsed into a set.
    */

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java

@@ -30,7 +30,7 @@ public class OrmQueryProperties implements Serializable {
 
   private final String parentPath;
   private final String path;
-  private final String properties;
+  private final boolean allProperties;
   private final Set<String> included;
   private final FetchConfig fetchConfig;
   private final boolean cache;
@@ -77,7 +77,7 @@ public class OrmQueryProperties implements Serializable {
   public OrmQueryProperties(String path) {
     this.path = path;
     this.parentPath = SplitName.parent(path);
-    this.properties = null;
+    this.allProperties = false;
     this.included = null;
     this.cache = false;
     this.fetchConfig = DEFAULT_FETCH;
@@ -91,7 +91,7 @@ public class OrmQueryProperties implements Serializable {
     this.path = path;
     this.parentPath = SplitName.parent(path);
     OrmQueryPropertiesParser.Response response = OrmQueryPropertiesParser.parse(rawProperties);
-    this.properties = response.properties;
+    this.allProperties = response.allProperties;
     this.included = response.included;
     if (fetchConfig != null) {
       this.fetchConfig = fetchConfig;
@@ -103,12 +103,25 @@ public class OrmQueryProperties implements Serializable {
   }
 
   public OrmQueryProperties(String path, Set<String> included) {
+    this(path, included, DEFAULT_FETCH);
+  }
+
+  OrmQueryProperties(String path, Set<String> included, FetchConfig fetchConfig) {
     this.path = path;
     this.parentPath = SplitName.parent(path);
     this.included = included;
-    this.properties = String.join(",", included);
-    this.cache = false;
-    this.fetchConfig = DEFAULT_FETCH;
+    this.allProperties = false;
+    this.fetchConfig = fetchConfig;
+    this.cache = fetchConfig.isCache();
+  }
+
+  OrmQueryProperties(String path, OrmQueryProperties other, FetchConfig fetchConfig) {
+    this.path = path;
+    this.parentPath = SplitName.parent(path);
+    this.allProperties = other.allProperties;
+    this.included = other.included;
+    this.cache = other.cache;
+    this.fetchConfig = fetchConfig;
   }
 
   /**
@@ -118,7 +131,7 @@ public class OrmQueryProperties implements Serializable {
     this.fetchConfig = sourceFetchConfig;
     this.parentPath = source.parentPath;
     this.path = source.path;
-    this.properties = source.properties;
+    this.allProperties = source.allProperties;
     this.cache = source.cache;
     this.filterMany = source.filterMany;
     this.markForQueryJoin = source.markForQueryJoin;
@@ -158,7 +171,7 @@ public class OrmQueryProperties implements Serializable {
    * Return the expressions used to filter on this path. This should be a many path to use this
    * method.
    */
-  @SuppressWarnings({"rawtypes","unchecked"})
+  @SuppressWarnings({"rawtypes", "unchecked"})
   public <T> SpiExpressionList<T> filterMany(Query<T> rootQuery) {
     if (filterMany == null) {
       FilterExprPath exprPath = new FilterExprPath(path);
@@ -201,9 +214,8 @@ public class OrmQueryProperties implements Serializable {
    */
   @SuppressWarnings("unchecked")
   public void configureBeanQuery(SpiQuery<?> query) {
-
-    if (properties != null && !properties.isEmpty()) {
-      query.select(properties);
+    if (!isEmpty()) {
+      query.selectProperties(this);
     }
 
     if (filterMany != null) {
@@ -219,7 +231,7 @@ public class OrmQueryProperties implements Serializable {
       for (OrmQueryProperties p : secondaryChildren) {
         String path = p.getPath();
         path = path.substring(trimPath);
-        query.fetch(path, p.getProperties(), p.getFetchConfig());
+        query.fetchProperties(path, p);
         query.setFilterMany(path, p.getFilterManyTrimPath(trimPath));
       }
     }
@@ -230,8 +242,7 @@ public class OrmQueryProperties implements Serializable {
   }
 
   public boolean hasSelectClause() {
-    if ("*".equals(properties)) {
-      // explicitly selected all properties
+    if (allProperties) {
       return true;
     }
     // explicitly selected some properties
@@ -241,8 +252,8 @@ public class OrmQueryProperties implements Serializable {
   /**
    * Return true if the properties and configuration are empty.
    */
-  public boolean isEmpty() {
-    return properties == null || properties.isEmpty();
+  boolean isEmpty() {
+    return !allProperties && included == null;
   }
 
   public void asStringDebug(String prefix, StringBuilder sb) {
@@ -250,8 +261,10 @@ public class OrmQueryProperties implements Serializable {
     if (path != null) {
       sb.append(path).append(" ");
     }
-    if (!isEmpty()) {
-      sb.append("(").append(properties).append(")");
+    if (allProperties) {
+      sb.append("(*)");
+    } else if (included != null) {
+      sb.append("(").append(String.join(",", included)).append(")");
     }
   }
 
@@ -269,17 +282,11 @@ public class OrmQueryProperties implements Serializable {
     secondaryChildren.add(child);
   }
 
-  /**
-   * Return the raw properties.
-   */
-  public String getProperties() {
-    return properties;
-  }
-
   /**
    * Return true if this includes all properties on the path.
    */
   public boolean allProperties() {
+    // this is really "default" properties
     return included == null;
   }
 
@@ -326,7 +333,6 @@ public class OrmQueryProperties implements Serializable {
     if (includedBeanJoin != null && includedBeanJoin.contains(propName)) {
       return false;
     }
-    // all properties included
     return included == null || included.contains(propName);
   }
 
@@ -403,7 +409,7 @@ public class OrmQueryProperties implements Serializable {
     if (path != null) {
       builder.append(path);
     }
-    if (included != null){
+    if (included != null) {
       builder.append("/i").append(included);
     }
     if (secondaryQueryJoins != null) {

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryPropertiesParser.java

@@ -7,72 +7,43 @@ import java.util.Set;
 /**
  * Parses the path properties string.
  */
-class OrmQueryPropertiesParser {
+final class OrmQueryPropertiesParser {
 
-  private static final Response EMPTY = new Response();
+  private static final Response EMPTY = new Response(false, null);
+  private static final Response ALL = new Response(true, null);
 
   /**
    * Immutable response of the parsed properties and options.
    */
   static class Response {
 
-    final String properties;
+    final boolean allProperties;
     final Set<String> included;
 
-    private Response(String properties, Set<String> included) {
-      this.properties = properties;
+    private Response(boolean allProperties, Set<String> included) {
+      this.allProperties = allProperties;
       this.included = included;
     }
-
-    private Response() {
-      this.properties = "";
-      this.included = null;
-    }
   }
 
   /**
    * Parses the path properties string returning the parsed properties and options.
    * In general it is comma delimited with some special strings like +lazy(20).
    */
-  public static Response parse(String rawProperties) {
-    return new OrmQueryPropertiesParser(rawProperties).parse();
-  }
-
-  private final String inputProperties;
-  private boolean allProperties;
-
-  private OrmQueryPropertiesParser(String inputProperties) {
-    this.inputProperties = inputProperties;
-  }
-
-  /**
-   * Parse the raw string properties input.
-   */
-  private Response parse() {
-    if (inputProperties == null || inputProperties.isEmpty()) {
+  static Response parse(String rawProperties) {
+    if (rawProperties == null || rawProperties.isEmpty()) {
       return EMPTY;
     }
-    if (inputProperties.equals("*")) {
-      return new Response("*", null);
+    if (rawProperties.equals("*")) {
+      return ALL;
     }
-    Set<String> fields = splitRawSelect(inputProperties);
-    for (String val : fields) {
-      if (val.equals("*")) {
-        allProperties = true;
-        break;
-      }
-    }
-    String properties = allProperties ? "*" : inputProperties;
-    if (fields.isEmpty()) {
-      fields = null;
-    }
-    return new Response(properties, fields);
+    return new Response(false, splitRawSelect(rawProperties));
   }
 
   /**
    * Split allowing 'dynamic function based properties'.
    */
-  private Set<String> splitRawSelect(String inputProperties) {
+  private static Set<String> splitRawSelect(String inputProperties) {
     return DSelectColumnsParser.parse(inputProperties);
   }