diff --git a/config/alfresco/caches.properties b/config/alfresco/caches.properties index 75aec744f8..865d6b08e5 100644 --- a/config/alfresco/caches.properties +++ b/config/alfresco/caches.properties @@ -337,7 +337,7 @@ cache.resourceBundleBaseNamesSharedCache.tx.maxItems=1000 cache.resourceBundleBaseNamesSharedCache.maxItems=1000 cache.resourceBundleBaseNamesSharedCache.timeToLiveSeconds=0 cache.resourceBundleBaseNamesSharedCache.maxIdleSeconds=0 -cache.resourceBundleBaseNamesSharedCache.cluster.type=fully-distributed +cache.resourceBundleBaseNamesSharedCache.cluster.type=invalidating cache.resourceBundleBaseNamesSharedCache.backup-count=1 cache.resourceBundleBaseNamesSharedCache.eviction-policy=LRU cache.resourceBundleBaseNamesSharedCache.eviction-percentage=25 @@ -347,7 +347,7 @@ cache.loadedResourceBundlesSharedCache.tx.maxItems=1000 cache.loadedResourceBundlesSharedCache.maxItems=1000 cache.loadedResourceBundlesSharedCache.timeToLiveSeconds=0 cache.loadedResourceBundlesSharedCache.maxIdleSeconds=0 -cache.loadedResourceBundlesSharedCache.cluster.type=fully-distributed +cache.loadedResourceBundlesSharedCache.cluster.type=invalidating cache.loadedResourceBundlesSharedCache.backup-count=1 cache.loadedResourceBundlesSharedCache.eviction-policy=LRU cache.loadedResourceBundlesSharedCache.eviction-percentage=25 @@ -357,7 +357,7 @@ cache.messagesSharedCache.tx.maxItems=1000 cache.messagesSharedCache.maxItems=1000 cache.messagesSharedCache.timeToLiveSeconds=0 cache.messagesSharedCache.maxIdleSeconds=0 -cache.messagesSharedCache.cluster.type=fully-distributed +cache.messagesSharedCache.cluster.type=invalidating cache.messagesSharedCache.backup-count=1 cache.messagesSharedCache.eviction-policy=LRU cache.messagesSharedCache.eviction-percentage=25 diff --git a/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java b/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java index d756b7893c..eb523526b3 100644 --- a/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java +++ b/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java @@ -24,6 +24,7 @@ import java.io.InputStreamReader; import java.io.Reader; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; @@ -184,7 +185,7 @@ public class MessageServiceImpl implements MessageService LockHelper.tryLock(readLock, tryLockTimeout, "getting resource bundle base names in 'MessageServiceImpl.registerResourceBundle()'"); try { - tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain, false); + tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain, false, true); } finally { @@ -197,6 +198,7 @@ public class MessageServiceImpl implements MessageService if (! tenantResourceBundleBaseNames.contains(resBundlePath)) { tenantResourceBundleBaseNames.add(resBundlePath); + putResourceBundleBaseNames(tenantDomain, tenantResourceBundleBaseNames); } logger.info("Registered message bundle '" + resBundlePath + "'"); @@ -285,9 +287,9 @@ public class MessageServiceImpl implements MessageService try { // all locales - loadedResourceBundlesForAllLocales = getLoadedResourceBundles(tenantDomain); - cachedMessagesForAllLocales = getMessages(tenantDomain); - resourceBundleBaseNamesForAllLocales = getResourceBundleBaseNames(tenantDomain, false); + loadedResourceBundlesForAllLocales = getLoadedResourceBundles(tenantDomain, false); + cachedMessagesForAllLocales = getMessages(tenantDomain, false); + resourceBundleBaseNamesForAllLocales = getResourceBundleBaseNames(tenantDomain, false, true); } finally { @@ -399,13 +401,13 @@ public class MessageServiceImpl implements MessageService LockHelper.tryLock(readLock, tryLockTimeout, "getting loaded resource bundles, messages and base names in 'MessageServiceImpl.getLocaleProperties()'"); try { - tenantLoadedResourceBundles = getLoadedResourceBundles(tenantDomain); + tenantLoadedResourceBundles = getLoadedResourceBundles(tenantDomain, true); loadedBundles = tenantLoadedResourceBundles.get(locale); - tenantCachedMessages = getMessages(tenantDomain); + tenantCachedMessages = getMessages(tenantDomain, true); props = tenantCachedMessages.get(locale); - tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain, false); + tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain, false, false); loadedBundleCount = tenantResourceBundleBaseNames.size(); } finally @@ -420,6 +422,7 @@ public class MessageServiceImpl implements MessageService { loadedBundles = new HashSet(); tenantLoadedResourceBundles.put(locale, loadedBundles); + putLoadedResourceBundles(tenantDomain, tenantLoadedResourceBundles); init = true; } finally @@ -436,6 +439,7 @@ public class MessageServiceImpl implements MessageService { props = new HashMap(); tenantCachedMessages.put(locale, props); + putMessages(tenantDomain, tenantCachedMessages); init = true; } finally @@ -451,7 +455,7 @@ public class MessageServiceImpl implements MessageService try { // get registered resource bundles - Set resBundleBaseNames = getResourceBundleBaseNames(tenantDomain, true); + Set resBundleBaseNames = getResourceBundleBaseNames(tenantDomain, true, false); int count = 0; @@ -595,7 +599,7 @@ public class MessageServiceImpl implements MessageService LockHelper.tryLock(readLock, tryLockTimeout, "getting resource bundle base names in 'MessageServiceImpl.getRegisteredBundles()'"); try { - return getResourceBundleBaseNames(getTenantDomain(), false); + return getResourceBundleBaseNames(getTenantDomain(), false, false); } finally { @@ -603,13 +607,13 @@ public class MessageServiceImpl implements MessageService } } - private Set getResourceBundleBaseNames(String tenantDomain, boolean haveWriteLock) + private Set getResourceBundleBaseNames(String tenantDomain, boolean haveWriteLock, boolean forWrite) { // Assume a read lock is present Set resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain); if (resourceBundleBaseNames != null) { - return resourceBundleBaseNames; + return getOrCopyResourceBundleBaseNames(resourceBundleBaseNames, forWrite); } if (!haveWriteLock) @@ -624,7 +628,7 @@ public class MessageServiceImpl implements MessageService resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain); if (resourceBundleBaseNames != null) { - return resourceBundleBaseNames; + return getOrCopyResourceBundleBaseNames(resourceBundleBaseNames, forWrite); } reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster) resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain); @@ -644,11 +648,17 @@ public class MessageServiceImpl implements MessageService throw new AlfrescoRuntimeException("Failed to re-initialise resourceBundleBaseNamesCache " + tenantDomain); } // Done - return resourceBundleBaseNames; + return getOrCopyResourceBundleBaseNames(resourceBundleBaseNames, forWrite); } + private Set getOrCopyResourceBundleBaseNames(Set inbound, boolean createNew) + { + return createNew ? new HashSet(inbound) : inbound; + } + private void putResourceBundleBaseNames(String tenantDomain, Set resourceBundleBaseNames) { + resourceBundleBaseNames = Collections.unmodifiableSet(new HashSet(resourceBundleBaseNames)); resourceBundleBaseNamesCache.put(tenantDomain, resourceBundleBaseNames); } @@ -656,18 +666,17 @@ public class MessageServiceImpl implements MessageService { if (resourceBundleBaseNamesCache.get(tenantDomain) != null) { - resourceBundleBaseNamesCache.get(tenantDomain).clear(); resourceBundleBaseNamesCache.remove(tenantDomain); } } - private Map> getLoadedResourceBundles(String tenantDomain) + private Map> getLoadedResourceBundles(String tenantDomain, boolean forWrite) { // Assume a read lock is present Map> loadedResourceBundles = loadedResourceBundlesCache.get(tenantDomain); if (loadedResourceBundles != null) { - return loadedResourceBundles; + return getOrCopyResourceBundles(loadedResourceBundles, forWrite); } // Not present. Upgrade to write lock. @@ -678,7 +687,7 @@ public class MessageServiceImpl implements MessageService loadedResourceBundles = loadedResourceBundlesCache.get(tenantDomain); if (loadedResourceBundles != null) { - return loadedResourceBundles; + return getOrCopyResourceBundles(loadedResourceBundles, forWrite); } reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster) loadedResourceBundles = loadedResourceBundlesCache.get(tenantDomain); @@ -695,11 +704,17 @@ public class MessageServiceImpl implements MessageService throw new AlfrescoRuntimeException("Failed to re-initialise loadedResourceBundlesCache " + tenantDomain); } // Done - return loadedResourceBundles; - } + return getOrCopyResourceBundles(loadedResourceBundles, forWrite); + } + + private Map> getOrCopyResourceBundles(Map> inbound, boolean createNew) + { + return createNew ? new HashMap>(inbound) : inbound; + } private void putLoadedResourceBundles(String tenantDomain, Map> loadedResourceBundles) { + loadedResourceBundles = Collections.unmodifiableMap(new HashMap>(loadedResourceBundles)); loadedResourceBundlesCache.put(tenantDomain, loadedResourceBundles); } @@ -707,7 +722,6 @@ public class MessageServiceImpl implements MessageService { if (loadedResourceBundlesCache.get(tenantDomain) != null) { - loadedResourceBundlesCache.get(tenantDomain).clear(); loadedResourceBundlesCache.remove(tenantDomain); } } @@ -716,17 +730,17 @@ public class MessageServiceImpl implements MessageService { if (loadedResourceBundlesCache.get(tenantDomain) != null) { - loadedResourceBundlesCache.get(tenantDomain).clear(); + putLoadedResourceBundles(tenantDomain, new HashMap>()); } } - private Map> getMessages(String tenantDomain) + private Map> getMessages(String tenantDomain, boolean forWrite) { // Assume a read lock Map> messages = messagesCache.get(tenantDomain); if (messages != null) { - return messages; + return getOrCopyMessages(messages, forWrite); } // Need to create it. Upgrade to write lock. @@ -737,7 +751,7 @@ public class MessageServiceImpl implements MessageService messages = messagesCache.get(tenantDomain); if (messages != null) { - return messages; + return getOrCopyMessages(messages, forWrite); } reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster) messages = messagesCache.get(tenantDomain); @@ -754,11 +768,18 @@ public class MessageServiceImpl implements MessageService throw new AlfrescoRuntimeException("Failed to re-initialise messagesCache " + tenantDomain); } // Done - return messages; + return getOrCopyMessages(messages, forWrite); } + private Map> getOrCopyMessages(Map> inbound, boolean createNew) + { + return createNew ? new HashMap>(inbound) : inbound; + } + + private void putMessages(String tenantDomain, Map> messages) { + messages = Collections.unmodifiableMap(new HashMap>(messages)); messagesCache.put(tenantDomain, messages); } @@ -766,7 +787,6 @@ public class MessageServiceImpl implements MessageService { if (messagesCache.get(tenantDomain) != null) { - messagesCache.get(tenantDomain).clear(); messagesCache.remove(tenantDomain); } } diff --git a/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java b/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java index c04f34f5be..aa0b85ff3f 100644 --- a/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java @@ -395,6 +395,32 @@ public class MessageServiceImplTest extends TestCase implements MessageDeployer assertEquals(Locale.getDefault(), messageService.parseLocale("")); } + public void testRegisteredBundlesSetDirectModification() + { + String bad_key = "BAD_KEY" + System.currentTimeMillis(); + + Set bundles = messageService.getRegisteredBundles(); + + assertNotNull(bundles); + assertTrue(!bundles.contains(bad_key)); + + try + { + // put entry directly + bundles.add(bad_key); + fail("Shouldn't be modified"); + } + catch (UnsupportedOperationException e) + { + // it's ok + } + + Set anotherTryBundles = messageService.getRegisteredBundles(); + + assertNotNull(anotherTryBundles); + assertTrue(!bundles.contains(bad_key)); + } + private void getMessages() { // Check default values