#566 - Refactor internals - SpiQuerySecondary and DLoadContext

src/main/java/com/avaje/ebeaninternal/api/LoadContext.java

@@ -25,11 +25,6 @@ public interface LoadContext {
 	 */
 	void executeSecondaryQueries(OrmQueryRequest<?> parentRequest);
 
-	/**
-	 * Register any secondary queries (+query or +lazy) with their appropriate LoadBeanContext or LoadManyContext.
-	 */
-  void registerSecondaryQueries(List<OrmQueryProperties> queryJoins, List<OrmQueryProperties> lazyJoins);
-
   /**
 	 * Return the node for a given path which is used by AutoTune profiling.
 	 */

src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java

@@ -314,22 +314,6 @@ public interface SpiQuery<T> extends Query<T> {
    */
   void setFilterMany(String prop, ExpressionList<?> filterMany);
 
-  /**
-   * Remove the query joins from query detail.
-   * <p>
-   * These are registered with the Load Context.
-   * </p>
-   */
-  List<OrmQueryProperties> removeQueryJoins();
-
-  /**
-   * Remove the lazy joins from query detail.
-   * <p>
-   * These are registered with the Load Context.
-   * </p>
-   */
-  List<OrmQueryProperties> removeLazyJoins();
-
   /**
    * Set the path of the many when +query/+lazy loading query is executed.
    */
@@ -338,7 +322,7 @@ public interface SpiQuery<T> extends Query<T> {
   /**
    * Convert joins as necessary to query joins etc.
    */
-  void convertJoins();
+  SpiQuerySecondary convertJoins();
 
   /**
    * Return the TransactionContext.

src/main/java/com/avaje/ebeaninternal/api/SpiQuerySecondary.java

@@ -0,0 +1,21 @@
+package com.avaje.ebeaninternal.api;
+
+import com.avaje.ebeaninternal.server.querydefn.OrmQueryProperties;
+
+import java.util.List;
+
+/**
+ * The secondary query paths for 'query joins' and 'lazy loading'.
+ */
+public interface SpiQuerySecondary {
+
+  /**
+   * Return a list of path/properties that are query join loaded.
+   */
+  List<OrmQueryProperties> getQueryJoins();
+
+  /**
+   * Return the list of path/properties that are lazy loaded.
+   */
+  List<OrmQueryProperties> getLazyJoins();
+}

src/main/java/com/avaje/ebeaninternal/server/core/OrmQueryRequest.java

@@ -19,6 +19,7 @@ import com.avaje.ebeaninternal.api.LoadContext;
 import com.avaje.ebeaninternal.api.SpiEbeanServer;
 import com.avaje.ebeaninternal.api.SpiQuery;
 import com.avaje.ebeaninternal.api.SpiQuery.Type;
+import com.avaje.ebeaninternal.api.SpiQuerySecondary;
 import com.avaje.ebeaninternal.api.SpiTransaction;
 import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
 import com.avaje.ebeaninternal.server.deploy.BeanProperty;
@@ -28,7 +29,6 @@ import com.avaje.ebeaninternal.server.deploy.DeployPropertyParserMap;
 import com.avaje.ebeaninternal.server.loadcontext.DLoadContext;
 import com.avaje.ebeaninternal.server.query.CQueryPlan;
 import com.avaje.ebeaninternal.server.query.CancelableQuery;
-import com.avaje.ebeaninternal.server.querydefn.OrmQueryProperties;
 import com.avaje.ebeaninternal.server.transaction.DefaultPersistenceContext;
 
 import javax.persistence.PersistenceException;
@@ -63,9 +63,7 @@ public final class OrmQueryRequest<T> extends BeanRequest implements BeanQueryRe
 
   private CQueryPlanKey queryPlanKey;
 
-  private List<OrmQueryProperties> queryJoins;
-
-  private List<OrmQueryProperties> lazyJoins;
+  private SpiQuerySecondary secondaryQueries;
 
   /**
    * Create the InternalQueryRequest.
@@ -152,9 +150,7 @@ public final class OrmQueryRequest<T> extends BeanRequest implements BeanQueryRe
 
     adapterPreQuery();
 
-    query.convertJoins();
-    this.queryJoins = query.removeQueryJoins();
-    this.lazyJoins = query.removeLazyJoins();
+    this.secondaryQueries = query.convertJoins();
     this.queryPlanKey = query.prepare(this);
   }
 
@@ -216,8 +212,7 @@ public final class OrmQueryRequest<T> extends BeanRequest implements BeanQueryRe
     }
     // initialise the persistenceContext and loadContext
     this.persistenceContext = getPersistenceContext(query, transaction);
-    this.loadContext = new DLoadContext(this);
-    this.loadContext.registerSecondaryQueries(queryJoins, lazyJoins);
+    this.loadContext = new DLoadContext(this, secondaryQueries);
   }
 
   /**

src/main/java/com/avaje/ebeaninternal/server/loadcontext/DLoadBaseContext.java

@@ -54,18 +54,7 @@ public abstract class DLoadBaseContext {
       return batchSize;
     }
 
-    int queryFetchBatch = queryProps.getQueryFetchBatch();
-    if (queryFetchBatch > 0) {
-      // property join was automatically set to a 'query join'
-      return queryFetchBatch;
-    }
-
-    FetchConfig fetchConfig = queryProps.getFetchConfig();
-    if (fetchConfig == null) {
-      return batchSize;
-    }
-
-    int queryBatchSize = fetchConfig.getQueryBatchSize();
+    int queryBatchSize = queryProps.getQueryFetchBatch();
     if (queryBatchSize == -1) {
       // not eager query fetch, just lazy loading
       return batchSize;
@@ -84,9 +73,6 @@ public abstract class DLoadBaseContext {
       return defaultBatchSize;
     }
     FetchConfig fetchConfig = queryProps.getFetchConfig();
-    if (fetchConfig == null) {
-      return defaultBatchSize;
-    }
     if (fetchConfig.isQueryAll()) {
       return firstBatchSize;
     }

src/main/java/com/avaje/ebeaninternal/server/loadcontext/DLoadBeanContext.java

@@ -58,7 +58,6 @@ public class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContex
     if (currentBuffer.isFull()) {
       currentBuffer = createBuffer(secondaryBatchSize);
     }
-    // set the persistenceContext on the bean first 
     ebi.setBeanLoader(currentBuffer, getPersistenceContext());
     currentBuffer.add(ebi);
   }
@@ -124,7 +123,7 @@ public class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContex
     }
 
     /**
-     * Return true if the buffer is full.
+     * Add the bean to the load buffer.
      */
     public void add(EntityBeanIntercept ebi) {
       if (persistenceContext == null) {
@@ -180,11 +179,10 @@ public class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContex
       }
 
       if (context.hitCache) {
-        // Check each of the beans in the batch to see if they are in the L2 cache.
+        // check each of the beans in the batch to see if they are in the L2 cache.
         Iterator<EntityBeanIntercept> iterator = list.iterator();
         while (iterator.hasNext()) {
-          EntityBeanIntercept bean = iterator.next();
-          if (context.desc.cacheBeanLoad(bean)) {
+          if (context.desc.cacheBeanLoad(iterator.next())) {
             iterator.remove();
           }
         }

src/main/java/com/avaje/ebeaninternal/server/loadcontext/DLoadContext.java

@@ -9,6 +9,7 @@ import com.avaje.ebeaninternal.api.LoadContext;
 import com.avaje.ebeaninternal.api.LoadSecondaryQuery;
 import com.avaje.ebeaninternal.api.SpiEbeanServer;
 import com.avaje.ebeaninternal.api.SpiQuery;
+import com.avaje.ebeaninternal.api.SpiQuerySecondary;
 import com.avaje.ebeaninternal.server.core.OrmQueryRequest;
 import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
 import com.avaje.ebeaninternal.server.deploy.BeanProperty;
@@ -58,7 +59,7 @@ public class DLoadContext implements LoadContext {
 
   private List<OrmQueryProperties> secQuery;
 
-  public DLoadContext(OrmQueryRequest<?> request) {
+  public DLoadContext(OrmQueryRequest<?> request, SpiQuerySecondary secondaryQueries) {
 
     this.persistenceContext = request.getPersistenceContext();
     this.ebeanServer = request.getServer();
@@ -86,6 +87,41 @@ public class DLoadContext implements LoadContext {
 
     // initialise rootBeanContext after origin and relativePath have been set
     this.rootBeanContext = new DLoadBeanContext(this, rootDescriptor, null, defaultBatchSize, null);
+
+    registerSecondaryQueries(secondaryQueries);
+  }
+
+
+  /**
+   * Register the +query and +lazy secondary queries with their appropriate LoadBeanContext or LoadManyContext.
+   */
+  private void registerSecondaryQueries(SpiQuerySecondary secondaryQueries) {
+
+    this.secQuery = secondaryQueries.getQueryJoins();
+    if (secQuery != null) {
+      for (OrmQueryProperties pathProperties : secQuery) {
+        registerSecondaryQuery(pathProperties);
+      }
+    }
+
+    List<OrmQueryProperties> lazyJoins = secondaryQueries.getLazyJoins();
+    if (lazyJoins != null) {
+      for (OrmQueryProperties lazyJoin : lazyJoins) {
+        registerSecondaryQuery(lazyJoin);
+      }
+    }
+  }
+
+  /**
+   * Setup the load context at this path with OrmQueryProperties which is
+   * used to build the appropriate query for +query or +lazy loading.
+   */
+  private void registerSecondaryQuery(OrmQueryProperties props) {
+
+    ElPropertyValue elGetValue = rootDescriptor.getElGetValue(props.getPath());
+
+    boolean many = elGetValue.getBeanProperty().containsMany();
+    registerSecondaryNode(many, props);
   }
 
   protected boolean isExcludeBeanCache() {
@@ -136,36 +172,6 @@ public class DLoadContext implements LoadContext {
     return beanLoad;
   }
 
-  /**
-   * Register the +query and +lazy secondary queries with their appropriate LoadBeanContext or LoadManyContext.
-   */
-  public void registerSecondaryQueries(List<OrmQueryProperties> queryJoins, List<OrmQueryProperties> lazyJoins) {
-
-    this.secQuery = queryJoins;
-    if (secQuery != null) {
-      for (int i = 0; i < secQuery.size(); i++) {
-        registerSecondaryQuery(secQuery.get(i));
-      }
-    }
-    if (lazyJoins != null) {
-      for (int i = 0; i < lazyJoins.size(); i++) {
-        registerSecondaryQuery(lazyJoins.get(i));
-      }
-    }
-  }
-
-  /**
-   * Setup the load context at this path with OrmQueryProperties which is
-   * used to build the appropriate query for +query or +lazy loading.
-   */
-  private void registerSecondaryQuery(OrmQueryProperties props) {
-
-    ElPropertyValue elGetValue = rootDescriptor.getElGetValue(props.getPath());
-
-    boolean many = elGetValue.getBeanProperty().containsMany();
-    registerSecondaryNode(many, props);
-  }
-
   public ObjectGraphNode getObjectGraphNode(String path) {
 
     ObjectGraphNode node = nodePathMap.get(path);
@@ -233,7 +239,7 @@ public class DLoadContext implements LoadContext {
     getManyContext(path).register(bc);
   }
 
-  private DLoadBeanContext getBeanContext(String path) {
+  protected DLoadBeanContext getBeanContext(String path) {
     if (path == null) {
       return rootBeanContext;
     }

src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java

@@ -19,6 +19,7 @@ import com.avaje.ebeaninternal.api.SpiExpression;
 import com.avaje.ebeaninternal.api.SpiExpressionList;
 import com.avaje.ebeaninternal.api.SpiExpressionValidation;
 import com.avaje.ebeaninternal.api.SpiQuery;
+import com.avaje.ebeaninternal.api.SpiQuerySecondary;
 import com.avaje.ebeaninternal.server.autotune.ProfilingListener;
 import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
 import com.avaje.ebeaninternal.server.deploy.BeanPropertyAssocMany;
@@ -406,7 +407,7 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
     return false;
   }
 
-  public List<OrmQueryProperties> removeQueryJoins() {
+  protected List<OrmQueryProperties> removeQueryJoins() {
     List<OrmQueryProperties> queryJoins = detail.removeSecondaryQueries();
     if (queryJoins != null) {
       if (orderBy != null) {
@@ -434,7 +435,7 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
     return queryJoins;
   }
 
-  public List<OrmQueryProperties> removeLazyJoins() {
+  protected List<OrmQueryProperties> removeLazyJoins() {
     return detail.removeSecondaryLazyQueries();
   }
 
@@ -443,10 +444,12 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
   }
 
   @Override
-  public void convertJoins() {
+  public SpiQuerySecondary convertJoins() {
 
     createExtraJoinsToSupportManyWhereClause();
     markQueryJoins();
+
+    return new OrmQuerySecondary(removeQueryJoins(), removeLazyJoins());
   }
 
   /**

src/main/java/com/avaje/ebeaninternal/server/querydefn/OrmQueryProperties.java

@@ -393,42 +393,72 @@ public class OrmQueryProperties implements Serializable {
     markForQueryJoin = true;
   }
 
-  public boolean isFetchJoin() {
-    return !isQueryFetch() && !isLazyFetch();
-  }
-
+  /**
+   * Return true if this path is a 'query join'.
+   */
   public boolean isQueryFetch() {
     return markForQueryJoin || getQueryFetchBatch() > -1;
   }
 
-  public int getQueryFetchBatch() {
-    return fetchConfig.getQueryBatchSize();
-  }
-
-  public boolean isQueryFetchAll() {
-    return fetchConfig.isQueryAll();
+  /**
+   * Return true if this path is a 'fetch join'.
+   */
+  public boolean isFetchJoin() {
+    return !isQueryFetch() && !isLazyFetch();
   }
 
+  /**
+   * Return true if this path is a lazy fetch.
+   */
   public boolean isLazyFetch() {
     return getLazyFetchBatch() > -1;
   }
 
+  /**
+   * Return the batch size to use for the query join.
+   */
+  public int getQueryFetchBatch() {
+    return fetchConfig.getQueryBatchSize();
+  }
+
+  /**
+   * Return true if a query join should eagerly fetch 'all' rather than the 'first'.
+   */
+  public boolean isQueryFetchAll() {
+    return fetchConfig.isQueryAll();
+  }
+
+  /**
+   * Return the batch size to use for lazy loading.
+   */
   public int getLazyFetchBatch() {
     return fetchConfig.getLazyBatchSize();
   }
 
+  /**
+   * Return true if this path has the +readonly option.
+   */
   public boolean isReadOnly() {
     return readOnly;
   }
 
+  /**
+   * Return true if this path has the +cache option to hit the cache.
+   */
   public boolean isCache() {
     return cache;
   }
 
+  /**
+   * Return the parent path.
+   */
   public String getParentPath() {
     return parentPath;
   }
 
+  /**
+   * Return the path relative to the root of the graph.
+   */
   public String getPath() {
     return path;
   }

src/main/java/com/avaje/ebeaninternal/server/querydefn/OrmQuerySecondary.java

@@ -0,0 +1,39 @@
+package com.avaje.ebeaninternal.server.querydefn;
+
+import com.avaje.ebeaninternal.api.SpiQuerySecondary;
+
+import java.util.List;
+
+/**
+ * The secondary query paths for 'query joins' and 'lazy loading'.
+ */
+public class OrmQuerySecondary implements SpiQuerySecondary {
+
+  private final List<OrmQueryProperties> queryJoins;
+
+  private final List<OrmQueryProperties> lazyJoins;
+
+  /**
+   * Construct with the 'query join' and 'lazy join' path properties.
+   */
+  public OrmQuerySecondary(List<OrmQueryProperties> queryJoins, List<OrmQueryProperties> lazyJoins) {
+    this.queryJoins = queryJoins;
+    this.lazyJoins = lazyJoins;
+  }
+
+  /**
+   * Return a list of path/properties that are query join loaded.
+   */
+  @Override
+  public List<OrmQueryProperties> getQueryJoins() {
+    return queryJoins;
+  }
+
+  /**
+   * Return the list of path/properties that are lazy loaded.
+   */
+  @Override
+  public List<OrmQueryProperties> getLazyJoins() {
+    return lazyJoins;
+  }
+}

src/test/java/com/avaje/ebeaninternal/server/core/DefaultServer_createOrmQueryRequestTest.java

@@ -5,7 +5,6 @@ import com.avaje.ebean.BaseTestCase;
 import com.avaje.ebean.Ebean;
 import com.avaje.ebean.FetchConfig;
 import com.avaje.ebean.Query;
-import com.avaje.ebeaninternal.api.SpiQuery;
 import com.avaje.ebeaninternal.server.querydefn.DefaultOrmQuery;
 import com.avaje.ebeaninternal.server.querydefn.OrmQueryDetail;
 import com.avaje.tests.model.basic.Order;
@@ -15,14 +14,12 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 public class DefaultServer_createOrmQueryRequestTest extends BaseTestCase {
 
-  static DefaultServer defaultServer = (DefaultServer) Ebean.getDefaultServer();
-
   Query<Order> query() {
-    return defaultServer.find(Order.class);
+    return server().find(Order.class);
   }
 
   OrmQueryRequest<Order> queryRequest(Query<Order> query) {
-    return (OrmQueryRequest<Order>) defaultServer.createQueryRequest(SpiQuery.Type.LIST, query, null);
+    return OrmQueryRequestTestHelper.queryRequest(query);
   }
 
   OrmQueryDetail detail(Query<Order> query) {

src/test/java/com/avaje/ebeaninternal/server/core/OrmQueryRequestTestHelper.java

@@ -0,0 +1,18 @@
+package com.avaje.ebeaninternal.server.core;
+
+import com.avaje.ebean.Ebean;
+import com.avaje.ebean.Query;
+import com.avaje.ebeaninternal.api.SpiQuery;
+
+public class OrmQueryRequestTestHelper {
+
+  static DefaultServer defaultServer = (DefaultServer) Ebean.getDefaultServer();
+
+  /**
+   * Create and return a OrmQueryRequest for the given query.
+   */
+  public static <T> OrmQueryRequest<T> queryRequest(Query<T> query) {
+    return (OrmQueryRequest<T>) defaultServer.createQueryRequest(SpiQuery.Type.LIST, query, null);
+  }
+
+}

src/test/java/com/avaje/ebeaninternal/server/loadcontext/DLoadContextTest.java

@@ -0,0 +1,75 @@
+package com.avaje.ebeaninternal.server.loadcontext;
+
+import com.avaje.ebean.BaseTestCase;
+import com.avaje.ebean.FetchConfig;
+import com.avaje.ebean.Query;
+import com.avaje.ebeaninternal.server.core.OrmQueryRequest;
+import com.avaje.ebeaninternal.server.core.OrmQueryRequestTestHelper;
+import com.avaje.tests.model.basic.Order;
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class DLoadContextTest extends BaseTestCase {
+
+  OrmQueryRequest<Order> queryRequest(Query<Order> query) {
+    return OrmQueryRequestTestHelper.queryRequest(query);
+  }
+
+  Query<Order> query() {
+    return server().find(Order.class);
+  }
+
+  @Test
+  public void construct_when_defaults_expect_defaultLazyBatchSizeServerDefaults() {
+
+    OrmQueryRequest<Order> queryRequest = queryRequest(query());
+    queryRequest.initTransIfRequired();
+
+    DLoadContext graphContext = (DLoadContext)queryRequest.getGraphContext();
+    DLoadBeanContext customer = graphContext.getBeanContext("customer");
+
+    assertThat(customer.firstBatchSize).isEqualTo(10);
+    assertThat(customer.secondaryBatchSize).isEqualTo(10);
+  }
+
+  @Test
+  public void construct_when_fetchQuery_expect_100_batchSize() {
+
+    OrmQueryRequest<Order> queryRequest = queryRequest(query().fetch("customer",new FetchConfig().query()));
+    queryRequest.initTransIfRequired();
+
+    DLoadContext graphContext = (DLoadContext)queryRequest.getGraphContext();
+    DLoadBeanContext customer = graphContext.getBeanContext("customer");
+
+    assertThat(customer.firstBatchSize).isEqualTo(100);
+    assertThat(customer.secondaryBatchSize).isEqualTo(100);
+  }
+
+  @Test
+  public void construct_when_fetchQuery50_expect_50_batchSize() {
+
+    OrmQueryRequest<Order> queryRequest = queryRequest(query().fetch("customer",new FetchConfig().query(50)));
+    queryRequest.initTransIfRequired();
+
+    DLoadContext graphContext = (DLoadContext)queryRequest.getGraphContext();
+    DLoadBeanContext customer = graphContext.getBeanContext("customer");
+
+    assertThat(customer.firstBatchSize).isEqualTo(50);
+    assertThat(customer.secondaryBatchSize).isEqualTo(50);
+  }
+
+  @Test
+  public void construct_when_fetchQueryFirst20Lazy5_expect_20_5_batchSize() {
+
+    OrmQueryRequest<Order> queryRequest = queryRequest(query().fetch("customer",new FetchConfig().queryFirst(20).lazy(5)));
+    queryRequest.initTransIfRequired();
+
+    DLoadContext graphContext = (DLoadContext)queryRequest.getGraphContext();
+    DLoadBeanContext customer = graphContext.getBeanContext("customer");
+
+    assertThat(customer.firstBatchSize).isEqualTo(20);
+    assertThat(customer.secondaryBatchSize).isEqualTo(5);
+  }
+
+}