From 5f8690fb87898f2f6bf001637e21d83db65da845 Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Mon, 2 Dec 2019 18:37:42 +1300 Subject: [PATCH] #1876 - findMap() does not use/hit natural key cache --- .../server/core/DefaultServer.java | 11 +- .../server/core/OrmQueryRequest.java | 62 +++++++++-- .../server/core/SpiOrmQueryRequest.java | 5 + .../cache/TestCacheViaComplexNaturalKey3.java | 101 +++++++++++++++++- 4 files changed, 160 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index 227b805d0..c504d4061 100644 --- a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -1245,16 +1245,13 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { public Set findSet(Query query, Transaction t) { SpiOrmQueryRequest request = createQueryRequest(Type.SET, query, t); - Object result = request.getFromQueryCache(); if (result != null) { return (Set) result; } - try { request.initTransIfRequired(); return request.findSet(); - } finally { request.endTransIfRequired(); } @@ -1265,16 +1262,18 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { public Map findMap(Query 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) result; } - try { request.initTransIfRequired(); return request.findMap(); - } finally { request.endTransIfRequired(); } diff --git a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java index 7394065dc..b6092bf81 100644 --- a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java +++ b/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 extends BeanRequest implements SpiOrmQuery public void mergeCacheHits(BeanCollection 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 orderBy = query.getOrderBy(); - if (orderBy != null) { - beanDescriptor.sort(((BeanList)result).getActualList(), orderBy.toStringFormat()); - } + } + } + + private void mergeCacheHitsToList(BeanCollection result) { + for (T hit : cacheBeans) { + result.internalAdd(hit); + } + if (result instanceof BeanList) { + OrderBy orderBy = query.getOrderBy(); + if (orderBy != null) { + // in memory sort after merging the cache hits with the DB hits + beanDescriptor.sort(((BeanList) result).getActualList(), orderBy.toStringFormat()); } } } + @SuppressWarnings({"rawtypes"}) + private void mergeCacheHitsToMap(BeanCollection result) { + BeanMap map = (BeanMap)result; + ElPropertyValue property = mapProperty(); + for (T bean : cacheBeans) { + map.internalPut(property.pathGet(bean), bean); + } + } + @Override public List getBeanCacheHits() { OrderBy orderBy = query.getOrderBy(); @@ -608,6 +627,33 @@ public final class OrmQueryRequest extends BeanRequest implements SpiOrmQuery return cacheBeans; } + @Override + public Map getBeanCacheHitsAsMap() { + OrderBy orderBy = query.getOrderBy(); + if (orderBy != null) { + beanDescriptor.sort(cacheBeans, orderBy.toStringFormat()); + } + return cacheBeansToMap(); + } + + @SuppressWarnings("unchecked") + private Map cacheBeansToMap() { + ElPropertyValue property = mapProperty(); + Map 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() { diff --git a/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java b/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java index 807407f4b..7085a1453 100644 --- a/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java +++ b/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java @@ -142,6 +142,11 @@ public interface SpiOrmQueryRequest extends BeanQueryRequest, DocQueryRequ */ List getBeanCacheHits(); + /** + * Return the bean cache hits for findMap (when all hits / no misses). + */ + Map getBeanCacheHitsAsMap(); + /** * Reset Bean cache mode AUTO - require explicit setting for bean cache use with findList(). */ diff --git a/src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java b/src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java index cb0ec0c58..a8112a2c6 100644 --- a/src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java +++ b/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 stores =Arrays.asList("abc", "def"); + List stores = asList("abc", "def"); for (String store : stores) { - List skus = Arrays.asList("1", "2", "3"); + List 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 codes = asList(1001); + + LoggedSqlCollector.start(); + + Map list = Ebean.find(OCachedNatKeyBean3.class) + .where() + .eq("store", "def") + .eq("sku", "2") + .in("code", codes) + .setUseCache(true) + .order().asc("code") + .setMapKey("sku") + .findMap(); + + List 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 codes = asList(1001, 1000); + + LoggedSqlCollector.start(); + + Map list = Ebean.find(OCachedNatKeyBean3.class) + .where() + .eq("store", "def") + .eq("sku", "2") + .in("code", codes) + .setUseCache(true) + .order().asc("code") + .setMapKey("sku") + .findMap(); + + List 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 map = Ebean.find(OCachedNatKeyBean3.class) + .where() + .eq("store", "def") + .eq("sku", "2") + .in("code", asList(1002, 1000)) + .setUseCache(true) + .setMapKey("code") + .findMap(); + + List 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 codes = Arrays.asList(1001, 1000, 1002, 1003); + List codes = asList(1001, 1000, 1002, 1003); LoggedSqlCollector.start(); @@ -131,7 +222,7 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase { setup(); loadSomeIntoCache(); - List skus = Arrays.asList("2", "3"); + List skus = asList("2", "3"); LoggedSqlCollector.start(); @@ -161,7 +252,7 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase { loadSomeIntoCache(); String storeId = "abc"; - List skus = Arrays.asList("3", "2", "4"); + List skus = asList("3", "2", "4"); LoggedSqlCollector.start();