From 4a4134ee93480da59800d97b716f93564e1fffce Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Wed, 13 Dec 2017 22:19:25 +1300 Subject: [PATCH] #1227 - NPE when using L2 Query cache with findOne() or findOneOrEmpty() --- .../java/io/ebeaninternal/api/SpiQuery.java | 2 +- .../server/core/DefaultServer.java | 10 ++--- .../server/core/OrmQueryRequest.java | 10 ++--- .../server/core/SpiOrmQueryRequest.java | 2 +- .../server/querydefn/DefaultOrmQuery.java | 10 +++-- .../org/tests/cache/TestQueryCacheInsert.java | 40 ++++++++++++++++++- 6 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/main/java/io/ebeaninternal/api/SpiQuery.java b/src/main/java/io/ebeaninternal/api/SpiQuery.java index b89544ca1..9d96b7ef1 100644 --- a/src/main/java/io/ebeaninternal/api/SpiQuery.java +++ b/src/main/java/io/ebeaninternal/api/SpiQuery.java @@ -369,7 +369,7 @@ public interface SpiQuery extends Query, TxnProfileEventCodes { /** * Reset AUTO mode to OFF for findList(). Expect explicit cache use with findList(). */ - void resetBeanCacheAutoMode(); + void resetBeanCacheAutoMode(boolean findOne); /** * Collect natural key data for this query or null if the query does not match diff --git a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index 64cb8cec7..3b9d78425 100644 --- a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -1518,12 +1518,10 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { private List findList(Query query, Transaction t, boolean findOne) { SpiOrmQueryRequest request = createQueryRequest(Type.LIST, query, t); - if (!findOne) { - request.resetBeanCacheAutoMode(); - Object result = request.getFromQueryCache(); - if (result != null) { - return (List) result; - } + request.resetBeanCacheAutoMode(findOne); + Object result = request.getFromQueryCache(); + if (result != null) { + return (List) result; } if ((t == null || !t.isSkipCache()) && request.getFromBeanCache()) { return request.getBeanCacheHits(); diff --git a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java index 63a1fd224..a488c8d9d 100644 --- a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java +++ b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java @@ -14,9 +14,12 @@ import io.ebean.event.BeanFindController; import io.ebean.event.BeanQueryAdapter; import io.ebean.event.BeanQueryRequest; import io.ebean.text.json.JsonReadOptions; +import io.ebeaninternal.api.BeanCacheResult; import io.ebeaninternal.api.CQueryPlanKey; import io.ebeaninternal.api.HashQuery; import io.ebeaninternal.api.LoadContext; +import io.ebeaninternal.api.NaturalKeyQueryData; +import io.ebeaninternal.api.NaturalKeySet; import io.ebeaninternal.api.SpiEbeanServer; import io.ebeaninternal.api.SpiQuery; import io.ebeaninternal.api.SpiQuery.Type; @@ -30,9 +33,6 @@ import io.ebeaninternal.server.deploy.DeployPropertyParserMap; import io.ebeaninternal.server.loadcontext.DLoadContext; import io.ebeaninternal.server.query.CQueryPlan; import io.ebeaninternal.server.query.CancelableQuery; -import io.ebeaninternal.api.BeanCacheResult; -import io.ebeaninternal.api.NaturalKeyQueryData; -import io.ebeaninternal.api.NaturalKeySet; import io.ebeaninternal.server.transaction.DefaultPersistenceContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -503,8 +503,8 @@ public final class OrmQueryRequest extends BeanRequest implements BeanQueryRe } @Override - public void resetBeanCacheAutoMode() { - query.resetBeanCacheAutoMode(); + public void resetBeanCacheAutoMode(boolean findOne) { + query.resetBeanCacheAutoMode(findOne); } public boolean isBeanCachePut() { diff --git a/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java b/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java index 281758b9d..49f7080ce 100644 --- a/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java +++ b/src/main/java/io/ebeaninternal/server/core/SpiOrmQueryRequest.java @@ -134,7 +134,7 @@ public interface SpiOrmQueryRequest extends DocQueryRequest { /** * Reset Bean cache mode AUTO - require explicit setting for bean cache use with findList(). */ - void resetBeanCacheAutoMode(); + void resetBeanCacheAutoMode(boolean findOne); /** * Return the Database platform like clause. diff --git a/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java index 89765da9e..2f77afc5f 100644 --- a/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -1104,12 +1104,12 @@ public class DefaultOrmQuery implements SpiQuery { @Override public boolean isBeanCachePut() { - return beanDescriptor.isBeanCaching() && useBeanCache.isPut(); + return useBeanCache.isPut() && beanDescriptor.isBeanCaching(); } @Override public boolean isBeanCacheGet() { - return beanDescriptor.isBeanCaching() && useBeanCache.isGet(); + return useBeanCache.isGet() && beanDescriptor.isBeanCaching() ; } @Override @@ -1118,9 +1118,11 @@ public class DefaultOrmQuery implements SpiQuery { } @Override - public void resetBeanCacheAutoMode() { + public void resetBeanCacheAutoMode(boolean findOne) { if (useBeanCache == CacheMode.AUTO) { - useBeanCache = CacheMode.OFF; + if (!findOne || useQueryCache != CacheMode.OFF) { + useBeanCache = CacheMode.OFF; + } } } diff --git a/src/test/java/org/tests/cache/TestQueryCacheInsert.java b/src/test/java/org/tests/cache/TestQueryCacheInsert.java index e74acbce6..dad6caf1f 100644 --- a/src/test/java/org/tests/cache/TestQueryCacheInsert.java +++ b/src/test/java/org/tests/cache/TestQueryCacheInsert.java @@ -3,12 +3,17 @@ package org.tests.cache; import io.ebean.BaseTestCase; import io.ebean.Ebean; import io.ebean.EbeanServer; -import org.tests.model.basic.EBasicVer; +import io.ebean.cache.ServerCache; +import io.ebean.cache.ServerCacheStatistics; import org.junit.Test; +import org.tests.model.basic.EBasicVer; import java.util.List; +import java.util.Optional; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; public class TestQueryCacheInsert extends BaseTestCase { @@ -30,4 +35,37 @@ public class TestQueryCacheInsert extends BaseTestCase { assertEquals(alist0.size() + 1, alist1.size()); } + + @Test + public void findOne() { + + EBasicVer doda = new EBasicVer("doda"); + doda.setDescription("OddButUniqueSillyExample"); + + Ebean.save(doda); + + ServerCache queryCache = Ebean.getServerCacheManager().getQueryCache(EBasicVer.class); + + Optional found0 = Ebean.find(EBasicVer.class) + .where().eq("description", "OddButUniqueSillyExample") + .setUseQueryCache(true) + .findOneOrEmpty(); + + assertTrue(found0.isPresent()); + assertHitMiss(0, 1, queryCache); + + EBasicVer found1 = Ebean.find(EBasicVer.class) + .where().eq("description", "OddButUniqueSillyExample") + .setUseQueryCache(true) + .findOne(); + + assertNotNull(found1); + assertHitMiss(1, 0, queryCache); + } + + private void assertHitMiss(int hits, int miss, ServerCache queryCache) { + ServerCacheStatistics stats = queryCache.getStatistics(true); + assertEquals(hits, stats.getHitCount()); + assertEquals(miss, stats.getMissCount()); + } }