diff --git a/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java b/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java index 6d969eae3a..3534b428cd 100644 --- a/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java +++ b/source/java/org/alfresco/repo/cache/lookup/EntityLookupCache.java @@ -19,12 +19,14 @@ package org.alfresco.repo.cache.lookup; import java.io.Serializable; +import java.sql.Savepoint; import org.alfresco.repo.cache.SimpleCache; +import org.alfresco.repo.domain.control.ControlDAO; import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.util.Pair; -import org.springframework.extensions.surf.util.ParameterCheck; import org.springframework.dao.ConcurrencyFailureException; +import org.springframework.extensions.surf.util.ParameterCheck; /** * A cache for two-way lookups of database entities. These are characterized by having a unique @@ -412,6 +414,48 @@ public class EntityLookupCache + * This method takes the opposite approach to {@link #getOrCreateByValue(Object)}, which assumes the entity's + * existence: in this case the entity is assumed to NOT exist. + * The {@link EntityLookupCallbackDAO#createValue(Object)} and {@link EntityLookupCallbackDAO#findByValue(Object)} + * will be used if necessary.
+ *

+ * Use this method when the data involved is seldom reused. + * + * @param value The entity value (null is allowed) + * @param controlDAO an essential DAO required in order to ensure a transactionally-safe attempt at data creation + * @return Returns the key-value pair (new or existing and never null) + */ + public Pair createOrGetByValue(V value, ControlDAO controlDAO) + { + if (controlDAO == null) + { + throw new IllegalArgumentException("The ControlDAO is required in order to perform a safe attempted insert."); + } + Savepoint savepoint = controlDAO.createSavepoint("EntityLookupCache.createOrGetByValue"); + try + { + Pair entityPair = entityLookup.createValue(value); + controlDAO.releaseSavepoint(savepoint); + // Cache it + if (cache != null) + { + cache.put( + new CacheRegionKey(cacheRegion, entityPair.getFirst()), + (entityPair.getSecond() == null ? VALUE_NULL : entityPair.getSecond())); + } + // It's been created and cached + return entityPair; + } + catch (Exception e) + { + controlDAO.rollbackToSavepoint(savepoint); + // Fall through to the usual way, which should find it if the failure cause was a duplicate key + return getOrCreateByValue(value); + } + } + /** * Find the entity associated with the given value and create it if it doesn't exist. * The {@link EntityLookupCallbackDAO#findByValue(Object)} and {@link EntityLookupCallbackDAO#createValue(Object)} diff --git a/source/java/org/alfresco/repo/domain/contentdata/ibatis/ContentDataDAOImpl.java b/source/java/org/alfresco/repo/domain/contentdata/ibatis/ContentDataDAOImpl.java index b4abd98aeb..27a667f0d7 100644 --- a/source/java/org/alfresco/repo/domain/contentdata/ibatis/ContentDataDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/contentdata/ibatis/ContentDataDAOImpl.java @@ -82,6 +82,7 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl this.template = sqlSessionTemplate; } + @Override public Pair createContentUrlOrphaned(String contentUrl, Date orphanTime) { ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); @@ -106,9 +107,7 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl if(contentUrlKeyEntity != null) { - template.insert(INSERT_SYMMETRIC_KEY, contentUrlKeyEntity); - -// contentUrlEntity.setContentUrlKey(contentUrlKeyEntity); + template.insert(INSERT_SYMMETRIC_KEY, contentUrlKeyEntity); } // Done @@ -139,6 +138,7 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl return contentUrlEntity; } + @Override public void getContentUrlsOrphaned( final ContentUrlHandler contentUrlHandler, final Long maxOrphanTimeExclusive, @@ -161,7 +161,7 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl } } - @SuppressWarnings("unchecked") + @Override public void getContentUrlsKeepOrphaned( final ContentUrlHandler contentUrlHandler, final int maxResults) @@ -178,6 +178,7 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl } } + @Override public int updateContentUrlOrphanTime(Long id, Long orphanTime, Long oldOrphanTime) { ContentUrlUpdateEntity contentUrlUpdateEntity = new ContentUrlUpdateEntity(); @@ -190,6 +191,7 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl /** * {@inheritDoc} */ + @Override public int deleteContentUrls(List ids) { template.delete(DELETE_CONTENT_URL_KEYS, ids); @@ -289,6 +291,7 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl return template.delete(DELETE_CONTENT_DATA, params); } + @Override public void deleteContentDataForNode(Long nodeId, Set qnameIds) { if (qnameIds.size() == 0) @@ -317,62 +320,62 @@ public class ContentDataDAOImpl extends AbstractContentDataDAOImpl } } - @Override - protected int updateContentUrlEntity(ContentUrlEntity existing, ContentUrlEntity entity) - { - int ret = 0; + @Override + protected int updateContentUrlEntity(ContentUrlEntity existing, ContentUrlEntity entity) + { + int ret = 0; - ContentUrlKeyEntity existingContentUrlKey = existing.getContentUrlKey(); - ContentUrlKeyEntity contentUrlKey = entity.getContentUrlKey(); - contentUrlKey.setContentUrlId(existing.getId()); - if(existingContentUrlKey == null) - { - ret = template.insert(INSERT_SYMMETRIC_KEY, contentUrlKey); - } - else if (!EqualsHelper.nullSafeEquals(existingContentUrlKey, contentUrlKey)) - { - ret = template.update(UPDATE_SYMMETRIC_KEY, contentUrlKey); - } + ContentUrlKeyEntity existingContentUrlKey = existing.getContentUrlKey(); + ContentUrlKeyEntity contentUrlKey = entity.getContentUrlKey(); + contentUrlKey.setContentUrlId(existing.getId()); + if(existingContentUrlKey == null) + { + ret = template.insert(INSERT_SYMMETRIC_KEY, contentUrlKey); + } + else if (!EqualsHelper.nullSafeEquals(existingContentUrlKey, contentUrlKey)) + { + ret = template.update(UPDATE_SYMMETRIC_KEY, contentUrlKey); + } - return ret; - } + return ret; + } - @Override - protected int deleteContentUrlEntity(long id) - { + @Override + protected int deleteContentUrlEntity(long id) + { Map params = new HashMap(11); params.put("id", id); - return template.delete(DELETE_SYMMETRIC_KEY, params); - } + return template.delete(DELETE_SYMMETRIC_KEY, params); + } - @Override - public List getSymmetricKeysByMasterKeyAlias(String masterKeyAlias, long fromId, int maxResults) - { - ContentUrlKeyEntity entity = new ContentUrlKeyEntity(); - entity.setMasterKeyAlias(masterKeyAlias); - entity.setId(fromId); - List results = template.selectList(SELECT_SYMMETRIC_KEYS_BY_MASTER_KEY, - entity, new RowBounds(0, maxResults)); - return results; - } + @Override + public List getSymmetricKeysByMasterKeyAlias(String masterKeyAlias, long fromId, int maxResults) + { + ContentUrlKeyEntity entity = new ContentUrlKeyEntity(); + entity.setMasterKeyAlias(masterKeyAlias); + entity.setId(fromId); + List results = template.selectList(SELECT_SYMMETRIC_KEYS_BY_MASTER_KEY, + entity, new RowBounds(0, maxResults)); + return results; + } - @Override - public Map countSymmetricKeysForMasterKeys() - { - Map counts = new HashMap<>(); + @Override + public Map countSymmetricKeysForMasterKeys() + { + Map counts = new HashMap<>(); - List res = template.selectList(COUNT_SYMMETRIC_KEYS_FOR_MASTER_KEYS); - for(SymmetricKeyCount count : res) - { - counts.put(count.getMasterKeyAlias(), count.getCount()); - } + List res = template.selectList(COUNT_SYMMETRIC_KEYS_FOR_MASTER_KEYS); + for(SymmetricKeyCount count : res) + { + counts.put(count.getMasterKeyAlias(), count.getCount()); + } - return counts; - } - - @Override - public int countSymmetricKeysForMasterKeyAlias(String masterKeyAlias) - { - return (Integer)template.selectOne(COUNT_SYMMETRIC_KEYS_BY_MASTER_KEY, masterKeyAlias); - } + return counts; + } + + @Override + public int countSymmetricKeysForMasterKeyAlias(String masterKeyAlias) + { + return (Integer)template.selectOne(COUNT_SYMMETRIC_KEYS_BY_MASTER_KEY, masterKeyAlias); + } } diff --git a/source/test-java/org/alfresco/repo/cache/lookup/EntityLookupCacheTest.java b/source/test-java/org/alfresco/repo/cache/lookup/EntityLookupCacheTest.java index 2504f8a93a..a2b06f7836 100644 --- a/source/test-java/org/alfresco/repo/cache/lookup/EntityLookupCacheTest.java +++ b/source/test-java/org/alfresco/repo/cache/lookup/EntityLookupCacheTest.java @@ -18,6 +18,7 @@ */ package org.alfresco.repo.cache.lookup; +import java.sql.Savepoint; import java.util.Map; import java.util.TreeMap; @@ -27,8 +28,11 @@ import junit.framework.TestCase; import org.alfresco.repo.cache.MemoryCache; import org.alfresco.repo.cache.SimpleCache; import org.alfresco.repo.cache.lookup.EntityLookupCache.EntityLookupCallbackDAO; +import org.alfresco.repo.domain.control.ControlDAO; import org.alfresco.util.EqualsHelper; import org.alfresco.util.Pair; +import org.mockito.Mockito; +import org.springframework.dao.DuplicateKeyException; /** * A cache for two-way lookups of database entities. These are characterized by having a unique @@ -46,6 +50,7 @@ public class EntityLookupCacheTest extends TestCase implements EntityLookupCallb private EntityLookupCache entityLookupCacheA; private EntityLookupCache entityLookupCacheB; private TreeMap database; + private ControlDAO controlDAO; @Override protected void setUp() throws Exception @@ -54,6 +59,9 @@ public class EntityLookupCacheTest extends TestCase implements EntityLookupCallb entityLookupCacheA = new EntityLookupCache(cache, "A", this); entityLookupCacheB = new EntityLookupCache(cache, "B", this); database = new TreeMap(); + + controlDAO = Mockito.mock(ControlDAO.class); + Mockito.when(controlDAO.createSavepoint(Mockito.anyString())).thenReturn(Mockito.mock(Savepoint.class)); } public void testLookupsUsingIncorrectValue() throws Exception @@ -159,6 +167,34 @@ public class EntityLookupCacheTest extends TestCase implements EntityLookupCallb assertEquals(entityPairNull, entityPairCheck); } + public void testGetOrCreate() throws Exception + { + TestValue valueOne = new TestValue(getName() + "-ONE"); + Pair entityPairOne = entityLookupCacheA.getOrCreateByValue(valueOne); + assertNotNull(entityPairOne); + Long id = entityPairOne.getFirst(); + assertEquals(valueOne.val, database.get(id)); + assertEquals(2, cache.getKeys().size()); + + Pair entityPairOneCheck = entityLookupCacheA.getOrCreateByValue(valueOne); + assertNotNull(entityPairOneCheck); + assertEquals(id, entityPairOneCheck.getFirst()); + } + + public void testCreateOrGet() throws Exception + { + TestValue valueOne = new TestValue(getName() + "-ONE"); + Pair entityPairOne = entityLookupCacheA.createOrGetByValue(valueOne, controlDAO); + assertNotNull(entityPairOne); + Long id = entityPairOne.getFirst(); + assertEquals(valueOne.val, database.get(id)); + assertEquals(1, cache.getKeys().size()); + + Pair entityPairOneCheck = entityLookupCacheA.createOrGetByValue(valueOne, controlDAO); + assertNotNull(entityPairOneCheck); + assertEquals(id, entityPairOneCheck.getFirst()); + } + public void testUpdate() throws Exception { TestValue valueOne = new TestValue(getName() + "-ONE"); @@ -295,6 +331,12 @@ public class EntityLookupCacheTest extends TestCase implements EntityLookupCallb assertTrue(value == null || value instanceof TestValue); String dbValue = (value == null) ? null : ((TestValue)value).val; + // Kick out any duplicate values + if (database.containsValue(dbValue)) + { + throw new DuplicateKeyException("Value is duplicated: " + value); + } + // Get the last key Long lastKey = database.isEmpty() ? null : database.lastKey(); Long newKey = null;