#2387 - Hit the L2 bean cache via property IN expression (when property is the id property)

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

@@ -1,6 +1,6 @@
 package io.ebeaninternal.api;
 
-import io.ebeaninternal.server.expression.IdInExpression;
+import io.ebeaninternal.server.expression.IdInCommon;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -13,10 +13,10 @@ import java.util.Set;
  */
 public final class CacheIdLookupMany<T> implements CacheIdLookup<T> {
 
-  private final IdInExpression idInExpression;
+  private final IdInCommon idInExpression;
   private int remaining;
 
-  public CacheIdLookupMany(IdInExpression idInExpression) {
+  public CacheIdLookupMany(IdInCommon idInExpression) {
     this.idInExpression = idInExpression;
   }
 
@@ -34,15 +34,12 @@ public final class CacheIdLookupMany<T> implements CacheIdLookup<T> {
    */
   @Override
   public List<T> removeHits(BeanCacheResult<T> cacheResult) {
-
     Set<Object> hitIds = new HashSet<>();
     List<T> beans = new ArrayList<>(hitIds.size());
-
     for (BeanCacheResult.Entry<T> hit : cacheResult.hits()) {
       hitIds.add(hit.getKey());
       beans.add(hit.getBean());
     }
-
     this.remaining = idInExpression.removeIds(hitIds);
     return beans;
   }

ebean-core/src/main/java/io/ebeaninternal/server/expression/IdInCommon.java

@@ -0,0 +1,20 @@
+package io.ebeaninternal.server.expression;
+
+import java.util.Collection;
+import java.util.Set;
+
+/**
+ * Id IN expression common for cache handling.
+ */
+public interface IdInCommon {
+
+  /**
+   * Return the ids this expression is looking to fetch.
+   */
+  Collection<?> idValues();
+
+  /**
+   * Remove Ids that where obtained from l2 cache. Don't fetch these from DB.
+   */
+  int removeIds(Set<Object> hitIds);
+}

ebean-core/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java

@@ -20,7 +20,7 @@ import java.util.Set;
 /**
  * In a collection of Id values.
  */
-public final class IdInExpression extends NonPrepareExpression {
+public final class IdInExpression extends NonPrepareExpression implements IdInCommon {
 
   private final List<Object> idCollection;
   private boolean multiValueIdSupported;
@@ -29,16 +29,12 @@ public final class IdInExpression extends NonPrepareExpression {
     this.idCollection = new ArrayList<>(idCollection);
   }
 
-  /**
-   * Return the ids this expression is looking to fetch.
-   */
+  @Override
   public Collection<Object> idValues() {
     return idCollection;
   }
 
-  /**
-   * Remove Ids that where obtained from l2 cache. Don't fetch these from DB.
-   */
+  @Override
   public int removeIds(Set<Object> hitIds) {
     idCollection.removeAll(hitIds);
     return idCollection.size();

ebean-core/src/main/java/io/ebeaninternal/server/expression/InExpression.java

@@ -10,13 +10,9 @@ import io.ebeaninternal.server.el.ElPropertyValue;
 import io.ebeaninternal.server.persist.MultiValueWrapper;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
 
-final class InExpression extends AbstractExpression {
+public final class InExpression extends AbstractExpression implements IdInCommon {
 
   private final boolean not;
 
@@ -49,6 +45,24 @@ final class InExpression extends AbstractExpression {
     this.empty = false;
   }
 
+  public String property() {
+    return propName;
+  }
+
+  @Override
+  public Collection<?> idValues() {
+    if (bindValues == null) {
+      bindValues = new ArrayList<>(sourceValues);
+    }
+    return bindValues;
+  }
+
+  @Override
+  public int removeIds(Set<Object> hitIds) {
+    bindValues.removeAll(hitIds);
+    return bindValues.size();
+  }
+
   private List<Object> values() {
     if (empty || sourceValues == null) {
       return Collections.emptyList();

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

@@ -56,7 +56,6 @@ public final class CQueryPlanStats {
    * Return a Snapshot of the query execution statistics potentially resetting the internal counters.
    */
   Snapshot getSnapshot(boolean reset) {
-
     TimedMetricStats collect = timedMetric.collect(reset);
     Snapshot snapshot = new Snapshot(collected, queryPlan, collect);
     collected = true;

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

@@ -14,9 +14,7 @@ import io.ebeaninternal.server.autotune.ProfilingListener;
 import io.ebeaninternal.server.core.SpiOrmQueryRequest;
 import io.ebeaninternal.server.deploy.*;
 import io.ebeaninternal.server.el.ElPropertyDeploy;
-import io.ebeaninternal.server.expression.DefaultExpressionList;
-import io.ebeaninternal.server.expression.IdInExpression;
-import io.ebeaninternal.server.expression.SimpleExpression;
+import io.ebeaninternal.server.expression.*;
 import io.ebeaninternal.server.query.NativeSqlQueryPlanKey;
 import io.ebeaninternal.server.rawsql.SpiRawSql;
 import io.ebeaninternal.server.transaction.ExternalJdbcTransaction;
@@ -631,6 +629,11 @@ public final class DefaultOrmQuery<T> extends AbstractQuery implements SpiQuery<
       SpiExpression singleExpression = underlyingList.get(0);
       if (singleExpression instanceof IdInExpression) {
         return new CacheIdLookupMany<>((IdInExpression) singleExpression);
+      } else if (singleExpression instanceof InExpression) {
+        InExpression in = (InExpression)singleExpression;
+        if (in.property().equals(beanDescriptor.idName())) {
+          return new CacheIdLookupMany<>(in);
+        }
       }
     }
     return null;

ebean-redis/src/test/java/org/integration/IntegrationTest.java

@@ -19,7 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 class IntegrationTest {
 
   @Test
-  void mput() throws InterruptedException {
+  void mput_via_setIdIn() throws InterruptedException {
 
     ServerCache beanCache = DB.cacheManager().beanCache(RCust.class);
     beanCache.clear();
@@ -60,6 +60,49 @@ class IntegrationTest {
     assertThat(stats2.getHitCount()).isEqualTo(3);
   }
 
+
+  @Test
+  void mput_via_propertyInExpression() throws InterruptedException {
+
+    ServerCache beanCache = DB.cacheManager().beanCache(RCust.class);
+    beanCache.clear();
+    beanCache.statistics(true);
+
+    List<RCust> people = new ArrayList<>();
+    for (String name : new String[]{"mpx0", "mpx1", "mpx2"}) {
+      people.add(new RCust(name));
+    }
+    DB.saveAll(people);
+    List<Long> ids = people.stream().map(RCust::getId).collect(Collectors.toList());
+
+    List<RCust> f0 = new QRCust()
+      .id.in(ids)
+      .findList();
+
+    assertThat(f0).hasSize(3);
+    ServerCacheStatistics stats0 = beanCache.statistics(true);
+    assertThat(stats0.getHitCount()).isEqualTo(0);
+
+    Thread.sleep(5);
+
+    // we will hit the cache this time
+    List<RCust> f1 = new QRCust()
+      .id.in(ids)
+      .findList();
+
+    assertThat(f1).hasSize(3);
+    ServerCacheStatistics stats1 = beanCache.statistics(true);
+    assertThat(stats1.getHitCount()).isEqualTo(3);
+
+    // we will hit the cache again
+    List<RCust> f2 = new QRCust()
+      .id.in(ids)
+      .findList();
+    assertThat(f2).hasSize(3);
+    ServerCacheStatistics stats2 = beanCache.statistics(true);
+    assertThat(stats2.getHitCount()).isEqualTo(3);
+  }
+
   @Test
   void test() throws InterruptedException {
 

ebean-test/src/test/java/org/tests/query/TestWhereIn.java

@@ -23,6 +23,7 @@ public class TestWhereIn extends BaseTestCase {
     ResetBasicData.reset();
 
     Query<Country> query = DB.find(Country.class)
+      .setUseCache(false)
       .where().in("code", "NZ", "AU")
       .query();
 
@@ -100,6 +101,7 @@ public class TestWhereIn extends BaseTestCase {
     ResetBasicData.reset();
 
     Query<Country> query = DB.find(Country.class)
+      .setUseCache(false)
       .where().in("code", new ArrayList<>()).query();
 
     query.findList();
@@ -112,6 +114,7 @@ public class TestWhereIn extends BaseTestCase {
     ResetBasicData.reset();
 
     Query<Country> query = DB.find(Country.class)
+      .setUseCache(false)
       .where().in("code", (Collection)null).query();
 
     query.findList();
@@ -124,6 +127,7 @@ public class TestWhereIn extends BaseTestCase {
     ResetBasicData.reset();
 
     Query<Country> query = DB.find(Country.class)
+      .setUseCache(false)
       .where().notIn("code", new ArrayList<>()).query();
 
     query.findList();
@@ -136,6 +140,7 @@ public class TestWhereIn extends BaseTestCase {
     ResetBasicData.reset();
 
     Query<Country> query = DB.find(Country.class)
+      .setUseCache(false)
       .where().notIn("code", (Collection)null).query();
 
     query.findList();