diff --git a/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java b/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java
index 47af7166eb..06a46f4ae2 100644
--- a/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java
+++ b/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java
@@ -40,6 +40,14 @@ import org.alfresco.util.ParameterCheck;
*
* All keys will be unique to the given cache region, allowing the cache to be shared
* between instances of this class.
+ *
+ * Generics:
+ *
+ * - K: The database unique identifier.
+ * - V: The value stored against K.
+ * - VK: The a value-derived key that will be used as a cache key when caching K for lookups by V.
+ * This can be the value itself if it is itself a good key.
+ *
*
* @author Derek Hulley
* @since 3.3
@@ -53,19 +61,27 @@ public class EntityLookupCache
+ * A return value should be small and efficient; don't return a value if this is not possible.
+ *
* Implementations will often return the value itself, provided that the value is both
* serializable and has a good equals and hashCode.
+ *
+ * Were no adequate key can be generated for the value, then null can be returned.
+ * In this case, the {@link #findByValue(Object) findByValue} method might not even do a search
+ * and just return null itself i.e. if it is difficult to look the value up in storage
+ * then it is probably difficult to generate a cache key from it, too.. In this scenario, the
+ * cache will be purely for key-based lookups
*
- * @param value the full value being keyed
- * @return Returns the business key representing the entity
+ * @param value the full value being keyed (never null)
+ * @return Returns the business key representing the entity, or null
+ * if an economical key cannot be generated.
*/
VK1 getValueKey(V1 value);
/**
* Find an entity for a given key.
*
- * @param key the key (ID) used to identify the entity
+ * @param key the key (ID) used to identify the entity (never null)
* @return Return the entity or null if no entity is exists for the ID
*/
Pair findByKey(K1 key);
@@ -74,18 +90,46 @@ public class EntityLookupCacheequals and hashCode
* methods of the value object should respect case-sensitivity in the same way that this
* lookup treats case-sensitivity i.e. if the equals method is case-sensitive
- * then this method should look the entity up using a case-sensitive search. Where the
- * behaviour is configurable,
+ * then this method should look the entity up using a case-sensitive search.
+ *
+ * Since this is a cache backed by some sort of database, null values are allowed by the
+ * cache. The implementation of this method can throw an exception if null is not
+ * appropriate for the use-case.
+ *
+ * If the search is impossible or expensive, this method should just return null. This
+ * would usually be the case if the {@link #getValueKey(Object) getValueKey} method also returned
+ * null i.e. if it is difficult to look the value up in storage then it is probably
+ * difficult to generate a cache key from it, too.
*
- * @param value the value (business object) used to identify the entity
+ * @param value the value (business object) used to identify the entity (null allowed).
* @return Return the entity or null if no entity matches the given value
*/
Pair findByValue(V1 value);
+ /**
+ * Create an entity using the given values. It is valid to assume that the entity does not exist
+ * within the current transaction at least.
+ *
+ * Since persistence mechanisms often allow null values, these can be expected here. The
+ * implementation must throw an exception if null is not allowed for the specific use-case.
+ *
+ * @param value the value (business object) used to identify the entity (null allowed).
+ * @return Return the newly-created entity ID-value pair
+ */
Pair createValue(V1 value);
}
- private static final String NULL_VALUE = "@@NULL_VALUE@@";
+ /**
+ * A valid null value i.e. a value that has been persisted as null.
+ */
+ private static final Serializable VALUE_NULL = "@@VALUE_NULL@@";
+ /**
+ * A value that was not found or persisted.
+ */
+ private static final Serializable VALUE_NOT_FOUND = "@@VALUE_NOT_FOUND@@";
+ /**
+ * The cache region that will be used (see {@link CacheRegionKey}) in all the cache keys
+ */
private static final String CACHE_REGION_DEFAULT = "DEFAULT";
private final SimpleCache cache;
@@ -139,6 +183,10 @@ public class EntityLookupCache getByKey(K key)
{
+ if (key == null)
+ {
+ throw new IllegalArgumentException("An entity lookup key may not be null");
+ }
// Handle missing cache
if (cache == null)
{
@@ -148,26 +196,36 @@ public class EntityLookupCache(key, value);
+ if (value.equals(VALUE_NOT_FOUND))
+ {
+ // We checked before
+ return null;
+ }
+ else if (value.equals(VALUE_NULL))
+ {
+ return new Pair(key, null);
+ }
+ else
+ {
+ return new Pair(key, value);
+ }
}
// Resolve it
Pair entityPair = entityLookup.findByKey(key);
if (entityPair == null)
{
- // Cache nulls
- cache.put(cacheKey, NULL_VALUE);
+ // Cache "not found"
+ cache.put(cacheKey, VALUE_NOT_FOUND);
}
else
{
+ value = entityPair.getSecond();
// Cache the value
- cache.put(cacheKey, entityPair.getSecond());
+ cache.put(
+ cacheKey,
+ (value == null ? VALUE_NULL : value));
}
// Done
return entityPair;
@@ -182,33 +240,49 @@ public class EntityLookupCache(key, value);
+ // We checked before and ...
+ if (key.equals(VALUE_NOT_FOUND))
+ {
+ // ... it didn't exist
+ return null;
+ }
+ else
+ {
+ // ... it did exist
+ return new Pair(key, value);
+ }
}
// Resolve it
Pair entityPair = entityLookup.findByValue(value);
if (entityPair == null)
{
- // Cache a null
- cache.put(cacheKey, NULL_VALUE);
+ // Cache "not found"
+ cache.put(valueCacheKey, VALUE_NOT_FOUND);
}
else
{
key = entityPair.getFirst();
// Cache the key
- cache.put(cacheKey, key);
+ cache.put(valueCacheKey, key);
+ cache.put(
+ new CacheRegionKey(cacheRegion, key),
+ (value == null ? VALUE_NULL : value));
}
// Done
return entityPair;
@@ -229,12 +303,25 @@ public class EntityLookupCache entityPair = entityLookup.findByValue(value);
+ if (entityPair == null)
+ {
+ entityPair = entityLookup.createValue(value);
+ }
+ return entityPair;
+ }
+
// Look in the cache
- K key = (K) cache.get(cacheKey);
+ CacheRegionKey valueCacheKey = new CacheRegionKey(cacheRegion, valueKey);
+ K key = (K) cache.get(valueCacheKey);
// Check if the value is already mapped to a key
- if (key != null && !key.equals(NULL_VALUE))
+ if (key != null && !key.equals(VALUE_NOT_FOUND))
{
return new Pair(key, value);
}
@@ -247,8 +334,10 @@ public class EntityLookupCache entityPairNull = entityLookupCacheA.getOrCreateByValue(valueNull);
+ assertNotNull(entityPairNull);
+ assertTrue(database.containsKey(entityPairNull.getFirst()));
+ assertNull(database.get(entityPairNull.getFirst()));
+ assertEquals(2, cache.getKeys().size());
+
+ // Look it up again
+ Pair entityPairCheck = entityLookupCacheA.getOrCreateByValue(valueNull);
+ assertNotNull(entityPairNull);
+ assertTrue(database.containsKey(entityPairNull.getFirst()));
+ assertNull(database.get(entityPairNull.getFirst()));
+ assertEquals(entityPairNull, entityPairCheck);
+ }
+
/**
* Helper class to represent business object
*/
@@ -182,30 +200,6 @@ public class EntityLookupCacheTest extends TestCase implements EntityLookupCallb
return dbValue;
}
- /**
- * Simulate creation of a new database entry
- */
- public Pair createValue(Object value)
- {
- assertNotNull(value);
- assertTrue(value instanceof TestValue);
- String dbValue = ((TestValue)value).val;
-
- // Get the last key
- Long lastKey = database.isEmpty() ? null : database.lastKey();
- Long newKey = null;
- if (lastKey == null)
- {
- newKey = new Long(1);
- }
- else
- {
- newKey = new Long(lastKey.longValue() + 1);
- }
- database.put(newKey, dbValue);
- return new Pair(newKey, value);
- }
-
public Pair findByKey(Long key)
{
assertNotNull(key);
@@ -222,17 +216,39 @@ public class EntityLookupCacheTest extends TestCase implements EntityLookupCallb
public Pair findByValue(Object value)
{
- assertNotNull(value);
- assertTrue(value instanceof TestValue);
- String dbValue = ((TestValue)value).val;
+ assertTrue(value == null || value instanceof TestValue);
+ String dbValue = (value == null) ? null : ((TestValue)value).val;
for (Map.Entry entry : database.entrySet())
{
- if (entry.getValue().equals(dbValue))
+ if (EqualsHelper.nullSafeEquals(entry.getValue(), dbValue))
{
return new Pair(entry.getKey(), entry.getValue());
}
}
return null;
}
+
+ /**
+ * Simulate creation of a new database entry
+ */
+ public Pair createValue(Object value)
+ {
+ assertTrue(value == null || value instanceof TestValue);
+ String dbValue = (value == null) ? null : ((TestValue)value).val;
+
+ // Get the last key
+ Long lastKey = database.isEmpty() ? null : database.lastKey();
+ Long newKey = null;
+ if (lastKey == null)
+ {
+ newKey = new Long(1);
+ }
+ else
+ {
+ newKey = new Long(lastKey.longValue() + 1);
+ }
+ database.put(newKey, dbValue);
+ return new Pair(newKey, value);
+ }
}