#1876 - findMap() does not use/hit natural key cache

src/main/java/io/ebeaninternal/server/core/DefaultServer.java

@@ -1245,16 +1245,13 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
   public <T> Set<T> findSet(Query<T> query, Transaction t) {
 
     SpiOrmQueryRequest request = createQueryRequest(Type.SET, query, t);
-
     Object result = request.getFromQueryCache();
     if (result != null) {
       return (Set<T>) result;
     }
-
     try {
       request.initTransIfRequired();
       return request.findSet();
-
     } finally {
       request.endTransIfRequired();
     }
@@ -1265,16 +1262,18 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
   public <K, T> Map<K, T> findMap(Query<T> query, Transaction t) {
 
     SpiOrmQueryRequest request = createQueryRequest(Type.MAP, query, t);
-
+    request.resetBeanCacheAutoMode(false);
+    if ((t == null || !t.isSkipCache()) && request.getFromBeanCache()) {
+      // hit bean cache and got all results from cache
+      return request.getBeanCacheHitsAsMap();
+    }
     Object result = request.getFromQueryCache();
     if (result != null) {
       return (Map<K, T>) result;
     }
-
     try {
       request.initTransIfRequired();
       return request.findMap();
-
     } finally {
       request.endTransIfRequired();
     }

src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java

@@ -10,6 +10,7 @@ import io.ebean.bean.EntityBean;
 import io.ebean.bean.PersistenceContext;
 import io.ebean.cache.QueryCacheEntry;
 import io.ebean.common.BeanList;
+import io.ebean.common.BeanMap;
 import io.ebean.common.CopyOnFirstWriteList;
 import io.ebean.event.BeanFindController;
 import io.ebean.event.BeanQueryAdapter;
@@ -31,6 +32,7 @@ import io.ebeaninternal.server.deploy.BeanProperty;
 import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
 import io.ebeaninternal.server.deploy.DeployParser;
 import io.ebeaninternal.server.deploy.DeployPropertyParserMap;
+import io.ebeaninternal.server.el.ElPropertyValue;
 import io.ebeaninternal.server.loadcontext.DLoadContext;
 import io.ebeaninternal.server.query.CQueryPlan;
 import io.ebeaninternal.server.query.CancelableQuery;
@@ -586,19 +588,36 @@ public final class OrmQueryRequest<T> extends BeanRequest implements SpiOrmQuery
   public void mergeCacheHits(BeanCollection<T> result) {
 
     if (cacheBeans != null && !cacheBeans.isEmpty()) {
-      for (T hit : cacheBeans) {
-        result.internalAdd(hit);
+      if (query.getType() == Type.MAP) {
+        mergeCacheHitsToMap(result);
+      } else {
+        mergeCacheHitsToList(result);
       }
-      // resort in memory here after merging the cache hits with the DB hits
-      if (result instanceof BeanList) {
-        OrderBy<T> orderBy = query.getOrderBy();
-        if (orderBy != null) {
-          beanDescriptor.sort(((BeanList<T>)result).getActualList(), orderBy.toStringFormat());
-        }
+    }
+  }
+
+  private void mergeCacheHitsToList(BeanCollection<T> result) {
+    for (T hit : cacheBeans) {
+      result.internalAdd(hit);
+    }
+    if (result instanceof BeanList) {
+      OrderBy<T> orderBy = query.getOrderBy();
+      if (orderBy != null) {
+        // in memory sort after merging the cache hits with the DB hits
+        beanDescriptor.sort(((BeanList<T>) result).getActualList(), orderBy.toStringFormat());
       }
     }
   }
 
+  @SuppressWarnings({"rawtypes"})
+  private void mergeCacheHitsToMap(BeanCollection<T> result) {
+    BeanMap map = (BeanMap)result;
+    ElPropertyValue property = mapProperty();
+    for (T bean : cacheBeans) {
+      map.internalPut(property.pathGet(bean), bean);
+    }
+  }
+
   @Override
   public List<T> getBeanCacheHits() {
     OrderBy<T> orderBy = query.getOrderBy();
@@ -608,6 +627,33 @@ public final class OrmQueryRequest<T> extends BeanRequest implements SpiOrmQuery
     return cacheBeans;
   }
 
+  @Override
+  public <K> Map<K, T> getBeanCacheHitsAsMap() {
+    OrderBy<T> orderBy = query.getOrderBy();
+    if (orderBy != null) {
+      beanDescriptor.sort(cacheBeans, orderBy.toStringFormat());
+    }
+    return cacheBeansToMap();
+  }
+
+  @SuppressWarnings("unchecked")
+  private <K> Map<K, T> cacheBeansToMap() {
+    ElPropertyValue property = mapProperty();
+    Map<K,T> map = new LinkedHashMap<>();
+    for (T bean : cacheBeans) {
+      map.put((K)property.pathGet(bean), bean);
+    }
+    return map;
+  }
+
+  private ElPropertyValue mapProperty() {
+    ElPropertyValue property = beanDescriptor.getElGetValue(query.getMapKey());
+    if (property == null) {
+      throw new IllegalStateException("Unknown map key property "+query.getMapKey());
+    }
+    return property;
+  }
+
   @Override
   public boolean getFromBeanCache() {
 

src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java

@@ -142,6 +142,11 @@ public interface SpiOrmQueryRequest<T> extends BeanQueryRequest<T>, DocQueryRequ
    */
   List<T> getBeanCacheHits();
 
+  /**
+   * Return the bean cache hits for findMap (when all hits / no misses).
+   */
+  <K> Map<K,T> getBeanCacheHitsAsMap();
+
   /**
    * Reset Bean cache mode AUTO - require explicit setting for bean cache use with findList().
    */

src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java

@@ -12,7 +12,9 @@ import org.junit.Test;
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
+import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class TestCacheViaComplexNaturalKey3 extends BaseTestCase {
@@ -30,9 +32,9 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase {
     if (!loadOnce) {
       Ebean.find(OCachedNatKeyBean3.class).delete();
 
-      List<String> stores =Arrays.asList("abc", "def");
+      List<String> stores = asList("abc", "def");
       for (String store : stores) {
-        List<String> skus = Arrays.asList("1", "2", "3");
+        List<String> skus = asList("1", "2", "3");
         for (String sku : skus) {
           int[] codes = {1000,1001,1002,1003,1004};
           for (int code : codes) {
@@ -91,6 +93,95 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase {
     clearStatistics();
   }
 
+  @Test
+  public void findMap_inClause_allHits() {
+
+    setup();
+    loadSomeIntoCache();
+
+    List<Integer> codes = asList(1001);
+
+    LoggedSqlCollector.start();
+
+    Map<String,OCachedNatKeyBean3> list = Ebean.find(OCachedNatKeyBean3.class)
+      .where()
+      .eq("store", "def")
+      .eq("sku", "2")
+      .in("code", codes)
+      .setUseCache(true)
+      .order().asc("code")
+      .setMapKey("sku")
+      .findMap();
+
+    List<String> sql = LoggedSqlCollector.stop();
+    assertThat(sql).isEmpty();
+
+    assertThat(list).hasSize(1);
+
+    assertNaturalKeyHitMiss(1, 0);
+    assertBeanCacheHitMiss(1, 0);
+  }
+
+  @Test
+  public void findMap_inClause_someHits() {
+
+    setup();
+    loadSomeIntoCache();
+
+    List<Integer> codes = asList(1001, 1000);
+
+    LoggedSqlCollector.start();
+
+    Map<String,OCachedNatKeyBean3> list = Ebean.find(OCachedNatKeyBean3.class)
+      .where()
+      .eq("store", "def")
+      .eq("sku", "2")
+      .in("code", codes)
+      .setUseCache(true)
+      .order().asc("code")
+      .setMapKey("sku")
+      .findMap();
+
+    List<String> sql = LoggedSqlCollector.stop();
+    assertThat(sql).hasSize(1);
+    if (isH2()) {
+      // in clause with only 1 bind param - miss on 1000
+      assertThat(sql.get(0)).contains("from o_cached_natkey3 t0 where t0.store = ? and t0.sku = ? and t0.code in (?) order by t0.code; --bind(def,2,Array[1]={1000})");
+    }
+    assertThat(list).hasSize(1);
+
+    assertNaturalKeyHitMiss(1, 1);
+    assertBeanCacheHitMiss(1, 0);
+  }
+
+  @Test
+  public void findMap_inClause_noHits() {
+
+    setup();
+
+    LoggedSqlCollector.start();
+
+    Map<String,OCachedNatKeyBean3> map = Ebean.find(OCachedNatKeyBean3.class)
+      .where()
+      .eq("store", "def")
+      .eq("sku", "2")
+      .in("code", asList(1002, 1000))
+      .setUseCache(true)
+      .setMapKey("code")
+      .findMap();
+
+    List<String> sql = LoggedSqlCollector.stop();
+    assertThat(sql).hasSize(1);
+    if (isH2()) {
+      // in clause with only 1 bind param - miss on 1000, 1002
+      assertThat(sql.get(0)).contains("from o_cached_natkey3 t0 where t0.store = ? and t0.sku = ? and t0.code in (?,?); --bind(def,2,Array[2]={1002,1000})");
+    }
+    assertThat(map).hasSize(2);
+
+    assertNaturalKeyHitMiss(0, 2);
+    assertBeanCacheHitMiss(0, 0);
+  }
+
   @Test
   public void findList_inClause_someHits() {
 
@@ -98,7 +189,7 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase {
     loadSomeIntoCache();
 
 
-    List<Integer> codes = Arrays.asList(1001, 1000, 1002, 1003);
+    List<Integer> codes = asList(1001, 1000, 1002, 1003);
 
     LoggedSqlCollector.start();
 
@@ -131,7 +222,7 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase {
     setup();
     loadSomeIntoCache();
 
-    List<String> skus = Arrays.asList("2", "3");
+    List<String> skus = asList("2", "3");
 
     LoggedSqlCollector.start();
 
@@ -161,7 +252,7 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase {
     loadSomeIntoCache();
 
     String storeId = "abc";
-    List<String> skus = Arrays.asList("3", "2", "4");
+    List<String> skus = asList("3", "2", "4");
 
     LoggedSqlCollector.start();