diff --git a/src/main/java/io/ebeaninternal/api/NaturalKeyEntry.java b/src/main/java/io/ebeaninternal/api/NaturalKeyEntry.java index 9a979a6b4..25fae5444 100644 --- a/src/main/java/io/ebeaninternal/api/NaturalKeyEntry.java +++ b/src/main/java/io/ebeaninternal/api/NaturalKeyEntry.java @@ -2,6 +2,7 @@ package io.ebeaninternal.api; import io.ebean.Pairs; +import io.ebeaninternal.server.deploy.BeanNaturalKey; import java.util.HashMap; import java.util.List; @@ -19,7 +20,7 @@ public class NaturalKeyEntry { /** * Used when query query just has a series of EQ expressions (no IN clause). */ - public NaturalKeyEntry(String[] naturalKey, List eqList) { + NaturalKeyEntry(BeanNaturalKey naturalKey, List eqList) { load(eqList); this.key = calculateKey(naturalKey); } @@ -27,7 +28,7 @@ public class NaturalKeyEntry { /** * Create when query uses an IN clause. */ - public NaturalKeyEntry(String[] naturalKey, List eqList, String inProperty, Object inValue) { + NaturalKeyEntry(BeanNaturalKey naturalKey, List eqList, String inProperty, Object inValue) { load(eqList); if (inProperty != null) { map.put(inProperty, inValue); @@ -39,7 +40,7 @@ public class NaturalKeyEntry { /** * Create when query uses an IN PAIRS clause. */ - public NaturalKeyEntry(String[] naturalKey, List eqList, + NaturalKeyEntry(BeanNaturalKey naturalKey, List eqList, String inMapProperty0, String inMapProperty1, Pairs.Entry pair) { load(eqList); map.put(inMapProperty0, pair.getA()); @@ -56,18 +57,8 @@ public class NaturalKeyEntry { } } - private Object calculateKey(String[] naturalKey) { - - if (naturalKey.length == 1) { - return map.get(naturalKey[0]); - } - - StringBuilder sb = new StringBuilder(); - for (String key : naturalKey) { - sb.append(map.get(key)).append(";"); - } - - return sb.toString(); + private Object calculateKey(BeanNaturalKey naturalKey) { + return naturalKey.calculateKey(map); } /** @@ -80,7 +71,7 @@ public class NaturalKeyEntry { /** * Return the inValue (used to remove from IN clause of original query). */ - public Object getInValue() { + Object getInValue() { return inValue; } } diff --git a/src/main/java/io/ebeaninternal/api/NaturalKeyQueryData.java b/src/main/java/io/ebeaninternal/api/NaturalKeyQueryData.java index 30a1ff984..21e17269c 100644 --- a/src/main/java/io/ebeaninternal/api/NaturalKeyQueryData.java +++ b/src/main/java/io/ebeaninternal/api/NaturalKeyQueryData.java @@ -1,6 +1,7 @@ package io.ebeaninternal.api; import io.ebean.Pairs; +import io.ebeaninternal.server.deploy.BeanNaturalKey; import java.util.ArrayList; import java.util.HashSet; @@ -12,7 +13,7 @@ import java.util.Set; */ public class NaturalKeyQueryData { - private final String[] naturalKey; + private final BeanNaturalKey naturalKey; /** * Only one of IN or IN PAIRS is allowed. @@ -34,18 +35,12 @@ public class NaturalKeyQueryData { private int hitCount; - public NaturalKeyQueryData(String[] naturalKey) { + public NaturalKeyQueryData(BeanNaturalKey naturalKey) { this.naturalKey = naturalKey; } private boolean matchProperty(String propName) { - - for (String key : naturalKey) { - if (key.equals(propName)) { - return true; - } - } - return false; + return naturalKey.matchProperty(propName); } /** @@ -132,13 +127,8 @@ public class NaturalKeyQueryData { * Return true if the properties match the natural key properties. */ private boolean matchProperties() { - if (naturalKey.length == 1) { - // simple single property case - if (inProperty != null) { - return inProperty.equals(naturalKey[0]); - } else { - return eqList.get(0).property.equals(naturalKey[0]); - } + if (naturalKey.isSingleProperty()) { + naturalKey.matchSingleProperty((inProperty != null) ? inProperty : eqList.get(0).property); } // multiple properties case @@ -157,27 +147,17 @@ public class NaturalKeyQueryData { exprProps.add(eq.property); } } - if (exprProps.size() != naturalKey.length) { - return false; - } - for (String key : naturalKey) { - if (!exprProps.remove(key)) { - return false; - } - } - - return exprProps.isEmpty(); + return naturalKey.matchMultiProperties(exprProps); } /** * Check that all the natural key properties are defined. */ private boolean expressionCount() { - int defined = (inValues == null) ? 0 : 1; defined += (inPairs == null) ? 0 : 2; defined += (eqList == null) ? 0 : eqList.size(); - return defined == naturalKey.length; + return defined == naturalKey.length(); } /** diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index e2edf7c70..f76aa143b 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -375,6 +375,8 @@ public class BeanDescriptor implements BeanType, STreeType { private final BeanProperty[] propertiesGenUpdate; private final List propertiesUnique = new ArrayList<>(); + private BeanNaturalKey beanNaturalKey; + /** * The bean class name or the table name for MapBeans. */ @@ -752,6 +754,18 @@ public class BeanDescriptor implements BeanType, STreeType { softDeleteByIdSql = null; softDeleteByIdInSql = null; } + initNaturalKey(); + } + + private void initNaturalKey() { + final String[] naturalKey = cacheHelp.getNaturalKey(); + if (naturalKey != null && naturalKey.length != 0) { + BeanProperty[] props = new BeanProperty[naturalKey.length]; + for (int i = 0; i < naturalKey.length; i++) { + props[i] = getBeanProperty(naturalKey[i]); + } + this.beanNaturalKey = new BeanNaturalKey(naturalKey, props); + } } private boolean hasCircularImportedId() { @@ -1322,10 +1336,10 @@ public class BeanDescriptor implements BeanType, STreeType { } /** - * Return the natural key properties. + * Return the natural key. */ - public String[] getNaturalKey() { - return cacheHelp.getNaturalKey(); + public BeanNaturalKey getNaturalKey() { + return beanNaturalKey; } /** diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanNaturalKey.java b/src/main/java/io/ebeaninternal/server/deploy/BeanNaturalKey.java new file mode 100644 index 000000000..2318aac11 --- /dev/null +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanNaturalKey.java @@ -0,0 +1,80 @@ +package io.ebeaninternal.server.deploy; + +import java.util.Map; +import java.util.Set; + +/** + * Natural key for a bean type. + */ +public class BeanNaturalKey { + + private final String[] naturalKey; + private final BeanProperty[] props; + + BeanNaturalKey(String[] naturalKey, BeanProperty[] props) { + this.naturalKey = naturalKey; + this.props = props; + } + + public int length() { + return naturalKey.length; + } + + /** + * Return true if the property name is part of the natural key. + */ + public boolean matchProperty(String propName) { + for (String key : naturalKey) { + if (key.equals(propName)) { + return true; + } + } + return false; + } + + /** + * Return true if this is a single property natural key. + */ + public boolean isSingleProperty() { + return props.length == 1; + } + + /** + * Return true if the given propertyName is our natural key property. + */ + public boolean matchSingleProperty(String propertyName) { + return naturalKey[0].equals(propertyName); + } + + /** + * Return true if all the properties match our natural key. + */ + public boolean matchMultiProperties(Set expressionProperties) { + if (expressionProperties.size() != naturalKey.length) { + return false; + } + for (String key : naturalKey) { + if (!expressionProperties.remove(key)) { + return false; + } + } + return expressionProperties.isEmpty(); + } + + /** + * Return the cache key given the bind values. + * + * @param map The bind values for the properties. + */ + public Object calculateKey(Map map) { + if (naturalKey.length == 1) { + return map.get(naturalKey[0]); + } + + StringBuilder sb = new StringBuilder(); + for (BeanProperty prop : props) { + sb.append(prop.naturalKeyVal(map)).append(";"); + } + return sb.toString(); + } +} diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java index 4f03e8507..4af68241a 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java @@ -856,6 +856,13 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty { setValue(bean, cacheData); } + /** + * Return the cache key value for this property. + */ + Object naturalKeyVal(Map values) { + return values.get(name); + } + @Override public Object getVal(Object bean) { return getValue((EntityBean) bean); diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index ac64689be..a9919dce8 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -210,6 +210,14 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr } } + Object naturalKeyVal(Map values) { + EntityBean bean = (EntityBean) values.get(name); + if (bean == null) { + return null; + } + return targetIdBinder.cacheKeyFromBean(bean); + } + @Override public ElPropertyValue buildElPropertyValue(String propName, String remainder, ElPropertyChainBuilder chain, boolean propertyDeploy) { diff --git a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinder.java b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinder.java index c722af79e..01aec83e7 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinder.java +++ b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinder.java @@ -213,4 +213,8 @@ public interface IdBinder { */ String cacheKey(Object idValue); + /** + * Return a key to use for bean caches given the bean. + */ + String cacheKeyFromBean(EntityBean bean); } diff --git a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java index 599a8a97e..f4db0904e 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java +++ b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java @@ -501,4 +501,8 @@ public final class IdBinderEmbedded implements IdBinder { return sb.toString(); } + @Override + public String cacheKeyFromBean(EntityBean bean) { + return cacheKey(embIdProperty.getValue(bean)); + } } diff --git a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmpty.java b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmpty.java index 7992c7431..3c7785943 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmpty.java +++ b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmpty.java @@ -182,4 +182,8 @@ public final class IdBinderEmpty implements IdBinder { return null; } + @Override + public String cacheKeyFromBean(EntityBean bean) { + return null; + } } diff --git a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java index e76503b20..f9ba347e9 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java +++ b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java @@ -268,4 +268,9 @@ public final class IdBinderSimple implements IdBinder { return scalarType.format(value); } + @Override + public String cacheKeyFromBean(EntityBean bean) { + final Object value = idProperty.getValue(bean); + return scalarType.format(value); + } } diff --git a/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java index ab0824901..cf8f6b207 100644 --- a/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -48,6 +48,7 @@ import io.ebeaninternal.api.SpiTransaction; import io.ebeaninternal.server.autotune.ProfilingListener; import io.ebeaninternal.server.core.SpiOrmQueryRequest; import io.ebeaninternal.server.deploy.BeanDescriptor; +import io.ebeaninternal.server.deploy.BeanNaturalKey; import io.ebeaninternal.server.deploy.BeanPropertyAssocMany; import io.ebeaninternal.server.deploy.InheritInfo; import io.ebeaninternal.server.deploy.TableJoin; @@ -771,8 +772,8 @@ public class DefaultOrmQuery implements SpiQuery { if (whereExpressions == null) { return null; } - String[] naturalKey = beanDescriptor.getNaturalKey(); - if (naturalKey == null || naturalKey.length == 0) { + BeanNaturalKey naturalKey = beanDescriptor.getNaturalKey(); + if (naturalKey == null) { return null; } diff --git a/src/test/java/io/ebean/BaseTestCase.java b/src/test/java/io/ebean/BaseTestCase.java index a44de541d..ccdb8f561 100644 --- a/src/test/java/io/ebean/BaseTestCase.java +++ b/src/test/java/io/ebean/BaseTestCase.java @@ -2,6 +2,7 @@ package io.ebean; import io.ebean.annotation.PersistBatch; import io.ebean.annotation.Platform; +import io.ebean.cache.ServerCacheStatistics; import io.ebean.config.dbplatform.IdType; import io.ebean.meta.MetaTimedMetric; import io.ebean.meta.MetricType; @@ -76,6 +77,10 @@ public abstract class BaseTestCase { } } + protected void clearAllL2Cache() { + server().getServerCacheManager().clearAll(); + } + protected void resetAllMetrics() { server().getMetaInfoManager().resetAllMetrics(); } @@ -221,6 +226,10 @@ public abstract class BaseTestCase { return spiEbeanServer().getBeanDescriptor(cls); } + protected ServerCacheStatistics getBeanCacheStats(Class cls, boolean reset) { + return server().getServerCacheManager().getBeanCache(cls).getStatistics(reset); + } + protected Platform platform() { return spiEbeanServer().getDatabasePlatform().getPlatform(); } diff --git a/src/test/java/org/tests/model/basic/cache/OCacheBase.java b/src/test/java/org/tests/model/basic/cache/OCacheBase.java new file mode 100644 index 000000000..08fb12bf7 --- /dev/null +++ b/src/test/java/org/tests/model/basic/cache/OCacheBase.java @@ -0,0 +1,33 @@ +package org.tests.model.basic.cache; + +import io.ebean.Model; + +import javax.persistence.Id; +import javax.persistence.MappedSuperclass; +import javax.persistence.Version; + +@MappedSuperclass +public class OCacheBase extends Model { + + @Id + private long id; + + @Version + private long version; + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/model/basic/cache/OCachedApp.java b/src/test/java/org/tests/model/basic/cache/OCachedApp.java new file mode 100644 index 000000000..b903cf1a4 --- /dev/null +++ b/src/test/java/org/tests/model/basic/cache/OCachedApp.java @@ -0,0 +1,22 @@ +package org.tests.model.basic.cache; + +import io.ebean.annotation.Cache; + +import javax.persistence.Entity; +import javax.persistence.UniqueConstraint; + +@Cache(naturalKey = "appName") +@Entity +@UniqueConstraint(columnNames = "app_name") +public class OCachedApp extends OCacheBase { + + private final String appName; + + public OCachedApp(String appName) { + this.appName = appName; + } + + public String getAppName() { + return appName; + } +} diff --git a/src/test/java/org/tests/model/basic/cache/OCachedAppDetail.java b/src/test/java/org/tests/model/basic/cache/OCachedAppDetail.java new file mode 100644 index 000000000..d4a97b84d --- /dev/null +++ b/src/test/java/org/tests/model/basic/cache/OCachedAppDetail.java @@ -0,0 +1,31 @@ +package org.tests.model.basic.cache; + +import io.ebean.annotation.Cache; + +import javax.persistence.Entity; +import javax.persistence.ManyToOne; +import javax.persistence.UniqueConstraint; + +@Cache(naturalKey = {"app", "detail"}) +@Entity +@UniqueConstraint(columnNames = {"app_id", "detail"}) +public class OCachedAppDetail extends OCacheBase { + + @ManyToOne(optional = false) + private final OCachedApp app; + + private final String detail; + + public OCachedAppDetail(OCachedApp app, String detail) { + this.app = app; + this.detail = detail; + } + + public OCachedApp getApp() { + return app; + } + + public String getDetail() { + return detail; + } +} diff --git a/src/test/java/org/tests/model/basic/cache/TestNatKeyCacheWithForeignKey.java b/src/test/java/org/tests/model/basic/cache/TestNatKeyCacheWithForeignKey.java new file mode 100644 index 000000000..4f9117476 --- /dev/null +++ b/src/test/java/org/tests/model/basic/cache/TestNatKeyCacheWithForeignKey.java @@ -0,0 +1,126 @@ +package org.tests.model.basic.cache; + +import io.ebean.BaseTestCase; +import io.ebean.DB; +import io.ebean.cache.ServerCacheStatistics; +import org.ebeantest.LoggedSqlCollector; +import org.junit.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestNatKeyCacheWithForeignKey extends BaseTestCase { + + private static boolean seededData; + + private static final OCachedApp app0 = new OCachedApp("app0"); + private static final OCachedApp app1 = new OCachedApp("app1"); + + private ServerCacheStatistics getStats() { + return getBeanCacheStats(OCachedAppDetail.class, true); + } + + @Test + public void test_findOne() { + + setupData(); + clearAllL2Cache(); + + final OCachedAppDetail found0 = findDetail(app0, "detail0"); + assertThat(found0).isNotNull(); + assertThat(getStats().getHitCount()).isEqualTo(0); + + resetAllMetrics(); + + LoggedSqlCollector.start(); + + final OCachedAppDetail found1 = findDetail(app0, "detail0"); + assertThat(found1).isNotNull(); + + final List sql = LoggedSqlCollector.stop(); + assertThat(sql).as("Expected cache hit, no SQL query expected").isEmpty(); + assertThat(getStats().getHitCount()).isEqualTo(1); + } + + private OCachedAppDetail findDetail(OCachedApp app, String detail) { + + return DB.find(OCachedAppDetail.class) + .where() + .eq("app", app) + .eq("detail", detail) + .findOne(); + } + + + @Test + public void test_findList_details_expect_hitNatKeyCache() { + + setupData(); + clearAllL2Cache(); + + final List result0 = findListDetails(app0, "detail0", "detail1"); + assertThat(result0).hasSize(2); + assertThat(getStats().getHitCount()).isEqualTo(0); + + LoggedSqlCollector.start(); + final List result1 = findListDetails(app0, "detail0", "detail1"); + assertThat(result1).hasSize(2); + + final List sql = LoggedSqlCollector.stop(); + assertThat(sql).as("Expected cache hit, no SQL query expected").isEmpty(); + assertThat(getStats().getHitCount()).isEqualTo(2); + } + + private List findListDetails(OCachedApp app, String... details) { + + return DB.find(OCachedAppDetail.class) + .setUseCache(true) + .where() + .eq("app", app) + .in("detail", details) + .findList(); + } + + @Test + public void test_findList_foreignKey_expect_hitNatKeyCache() { + + setupData(); + clearAllL2Cache(); + + final List result0 = findListApps("detail0", app0, app1); + assertThat(result0).hasSize(2); + assertThat(getStats().getHitCount()).isEqualTo(0); + + LoggedSqlCollector.start(); + final List result1 = findListApps("detail0", app0, app1); + assertThat(result1).hasSize(2); + + final List sql = LoggedSqlCollector.stop(); + assertThat(sql).as("Expected cache hit, no SQL query expected").isEmpty(); + assertThat(getStats().getHitCount()).isEqualTo(2); + } + + private List findListApps(String detail, OCachedApp... apps) { + + return DB.find(OCachedAppDetail.class) + .setUseCache(true) + .where() + .in("app", apps) + .eq("detail", detail) + .findList(); + } + + private static void setupData() { + + if (!seededData) { + seededData = true; + app0.save(); + app1.save(); + new OCachedAppDetail(app0, "detail0").save(); + new OCachedAppDetail(app0, "detail1").save(); + new OCachedAppDetail(app1, "detail0").save(); + new OCachedAppDetail(app1, "detail1").save(); + } + } +}