Author: kwin
Date: Wed Jun 17 07:58:43 2015
New Revision: 1685933
URL: http://svn.apache.org/r1685933
Log:
SLING-4795 improve cache invalidation by only invalidate the affected resource
bundles
Modified:
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundle.java
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java
sling/trunk/bundles/extensions/i18n/src/test/java/org/apache/sling/i18n/impl/ConcurrentJcrResourceBundleLoadingTest.java
Modified:
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundle.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundle.java?rev=1685933&r1=1685932&r2=1685933&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundle.java
(original)
+++
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundle.java
Wed Jun 17 07:58:43 2015
@@ -64,11 +64,14 @@ public class JcrResourceBundle extends R
private final Locale locale;
+ private final String baseName;
+
private final Set<String> languageRoots = new HashSet<String>();
JcrResourceBundle(Locale locale, String baseName,
ResourceResolver resourceResolver) {
this.locale = locale;
+ this.baseName = baseName;
log.info("Finding all dictionaries for '{}' (basename: {}) ...",
locale, baseName == null ? "<none>" : baseName);
@@ -98,12 +101,20 @@ public class JcrResourceBundle extends R
protected void setParent(ResourceBundle parent) {
super.setParent(parent);
}
+
+ public ResourceBundle getParent() {
+ return parent;
+ }
@Override
public Locale getLocale() {
return locale;
}
+ public String getBaseName() {
+ return baseName;
+ }
+
/**
* Returns a Set of all resource keys provided by this resource bundle
only.
* <p>
@@ -337,4 +348,10 @@ public class JcrResourceBundle extends R
private static String toRFC4646String(Locale locale) {
return locale.toString().replace('_', '-');
}
+
+ @Override
+ public String toString() {
+ return "JcrResourceBundle [locale=" + locale + ", baseName=" +
baseName + ", languageRoots=" + languageRoots
+ + ", parent=" + parent + "]";
+ }
}
Modified:
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java?rev=1685933&r1=1685932&r2=1685933&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java
(original)
+++
sling/trunk/bundles/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java
Wed Jun 17 07:58:43 2015
@@ -22,6 +22,7 @@ import static org.apache.sling.i18n.impl
import static org.apache.sling.i18n.impl.JcrResourceBundle.PROP_LANGUAGE;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.Dictionary;
import java.util.HashMap;
@@ -111,14 +112,16 @@ public class JcrResourceBundleProvider i
private ResourceResolver resourceResolver;
/**
- * Map of cached resource bundles indexed by a key combined of the pertient
- * base name and <code>Locale</code> used to load and identify the
- * <code>ResourceBundle</code>.
+ * Map of cached resource bundles indexed by a key combined of the base
name
+ * and <code>Locale</code> used to load and identify the
<code>ResourceBundle</code>.
*/
private final ConcurrentHashMap<Key, JcrResourceBundle>
resourceBundleCache = new ConcurrentHashMap<Key, JcrResourceBundle>();
private final ConcurrentHashMap<Key, Semaphore> loadingGuards = new
ConcurrentHashMap<Key, Semaphore>();
+ /**
+ * paths from which JCR resource bundles have been loaded
+ */
private final Set<String> languageRootPaths =
Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>());
/**
@@ -129,7 +132,10 @@ public class JcrResourceBundleProvider i
private BundleContext bundleContext;
- private List<ServiceRegistration> bundleServiceRegistrations;
+ /**
+ * Each ResourceBundle is registered as a service. Each registration is
stored in this map with the locale & base name used as a key.
+ */
+ private Map<Key, ServiceRegistration> bundleServiceRegistrations;
private boolean preloadBundles;
@@ -174,27 +180,77 @@ public class JcrResourceBundleProvider i
@Override
public void handleEvent(final org.osgi.service.event.Event event) {
- final String path =
(String)event.getProperty(SlingConstants.PROPERTY_PATH);
- if ( path != null ) {
- boolean invalidate = false;
- if ( languageRootPaths.contains(path) ) {
- log.debug("handleEvent: Detected change of cached language
root {}, removing cached ResourceBundles", path);
- invalidate = true;
+ final String path = (String)
event.getProperty(SlingConstants.PROPERTY_PATH);
+ if (path != null) {
+ log.debug("handleEvent: Detecting event {} for path '{}'", event,
path);
+
+ // if this change was on languageRootPath level this might change
basename and locale as well, therefore
+ // invalidate everything
+ if (languageRootPaths.contains(path)) {
+ log.debug(
+ "handleEvent: Detected change of cached language root
'{}', removing all cached ResourceBundles",
+ path);
+ clearCache();
+ preloadBundles();
} else {
- for(final String root : languageRootPaths) {
- if ( path.startsWith(root) ) {
- log.debug("handleEvent: Resource changes, removing
cached ResourceBundles");
- invalidate = true;
+ // if it is only a change below a root path, only messages of
one resource bundle can be affected!
+ for (final String root : languageRootPaths) {
+ if (path.startsWith(root)) {
+ // figure out which JcrResourceBundle from the cached
ones is affected
+ for (JcrResourceBundle bundle :
resourceBundleCache.values()) {
+ if (bundle.getLanguageRootPaths().contains(root)) {
+ // reload it
+ log.debug("handleEvent: Resource changes below
'{}', reloading ResourceBundle '{}'",
+ root, bundle);
+ reloadBundle(bundle);
+ return;
+ }
+ }
+ log.warn("handleEvent: No cached resource bundle found
with root '{}'", root);
break;
}
}
}
+ }
+ }
- if ( invalidate ) {
- clearCache();
- preloadBundles();
+ synchronized void reloadBundle(JcrResourceBundle oldBundle) {
+ String baseName = oldBundle.getBaseName();
+ Locale locale = oldBundle.getLocale();
+ Key key = new Key(baseName, locale);
+
+ // remove bundle from cache
+ resourceBundleCache.remove(key);
+
+ // unregister bundle
+ ServiceRegistration serviceRegistration =
bundleServiceRegistrations.remove(key);
+ if (serviceRegistration != null) {
+ serviceRegistration.unregister();
+ } else {
+ log.warn("Could not find resource bundle service for key {}", key);
+ }
+
+ Collection<JcrResourceBundle> dependentBundles = new
ArrayList<JcrResourceBundle>();
+ // this bundle might be a parent of a cached bundle -> invalidate
those dependent bundles as well
+ for (JcrResourceBundle bundle : resourceBundleCache.values()) {
+ if (bundle.getParent() instanceof JcrResourceBundle) {
+ JcrResourceBundle parentBundle = (JcrResourceBundle)
bundle.getParent();
+ Key parentKey = new Key(parentBundle.getBaseName(),
parentBundle.getLocale());
+ if (parentKey.equals(key)) {
+ log.debug("Also invalidate dependent bundle {} which has
bundle {} as parent", bundle, parentBundle);
+ dependentBundles.add(bundle);
+ }
}
}
+ for (JcrResourceBundle dependentBundle : dependentBundles) {
+ reloadBundle(dependentBundle);
+ }
+
+ if (preloadBundles) {
+ // reload the bundle from the repository (will also fill cache and
register as a service)
+ getResourceBundle(oldBundle.getBaseName(), oldBundle.getLocale());
+ }
+
}
// ---------- SCR Integration
----------------------------------------------
@@ -223,7 +279,7 @@ public class JcrResourceBundleProvider i
this.preloadBundles =
PropertiesUtil.toBoolean(props.get(PROP_PRELOAD_BUNDLES),
DEFAULT_PRELOAD_BUNDLES);
this.bundleContext = context.getBundleContext();
- this.bundleServiceRegistrations = new ArrayList<ServiceRegistration>();
+ this.bundleServiceRegistrations = new HashMap<Key,
ServiceRegistration>();
if (this.resourceResolverFactory != null) {
final Thread t = new Thread() {
@Override
@@ -317,7 +373,7 @@ public class JcrResourceBundleProvider i
ServiceRegistration serviceReg =
bundleContext.registerService(ResourceBundle.class.getName(),
resourceBundle, serviceProps);
synchronized (this) {
- bundleServiceRegistrations.add(serviceReg);
+ bundleServiceRegistrations.put(key, serviceReg);
}
// register language root paths
@@ -449,15 +505,12 @@ public class JcrResourceBundleProvider i
resourceBundleCache.clear();
languageRootPaths.clear();
- ServiceRegistration[] serviceRegs;
synchronized (this) {
- serviceRegs = bundleServiceRegistrations.toArray(new
ServiceRegistration[bundleServiceRegistrations.size()]);
+ for (ServiceRegistration serviceReg :
bundleServiceRegistrations.values()) {
+ serviceReg.unregister();
+ }
bundleServiceRegistrations.clear();
}
-
- for (ServiceRegistration serviceReg : serviceRegs) {
- serviceReg.unregister();
- }
}
private void preloadBundles() {
Modified:
sling/trunk/bundles/extensions/i18n/src/test/java/org/apache/sling/i18n/impl/ConcurrentJcrResourceBundleLoadingTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/i18n/src/test/java/org/apache/sling/i18n/impl/ConcurrentJcrResourceBundleLoadingTest.java?rev=1685933&r1=1685932&r2=1685933&view=diff
==============================================================================
---
sling/trunk/bundles/extensions/i18n/src/test/java/org/apache/sling/i18n/impl/ConcurrentJcrResourceBundleLoadingTest.java
(original)
+++
sling/trunk/bundles/extensions/i18n/src/test/java/org/apache/sling/i18n/impl/ConcurrentJcrResourceBundleLoadingTest.java
Wed Jun 17 07:58:43 2015
@@ -18,6 +18,22 @@
*/
package org.apache.sling.i18n.impl;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.times;
+import static org.powermock.api.mockito.PowerMockito.doReturn;
+import static org.powermock.api.mockito.PowerMockito.spy;
+import static org.powermock.api.mockito.PowerMockito.verifyPrivate;
+
+import java.util.Hashtable;
+import java.util.Locale;
+import java.util.ResourceBundle;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
@@ -28,20 +44,6 @@ import org.powermock.api.mockito.PowerMo
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
-import java.util.Hashtable;
-import java.util.Locale;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
-
-import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.times;
-import static org.powermock.api.mockito.PowerMockito.doReturn;
-import static org.powermock.api.mockito.PowerMockito.spy;
-import static org.powermock.api.mockito.PowerMockito.verifyPrivate;
-
/**
* Test case to verify that each bundle is only loaded once, even
* if concurrent requests for the same bundle are made.
@@ -52,15 +54,24 @@ public class ConcurrentJcrResourceBundle
@Mock JcrResourceBundle english;
@Mock JcrResourceBundle german;
-
- @Test
- public void loadBundlesOnlyOncePerLocale() throws Exception {
- JcrResourceBundleProvider provider = spy(new
JcrResourceBundleProvider());
- provider.activate(createComponentContext(new Hashtable<String,
Object>()));
-
+
+ private JcrResourceBundleProvider provider;
+
+ @Before
+ public void setup() throws Exception {
+ provider = spy(new JcrResourceBundleProvider());
+ Hashtable<String, Object> properties = new Hashtable<String, Object>();
+ properties.put("locale.default", "en");
+ provider.activate(createComponentContext(properties));
doReturn(english).when(provider, "createResourceBundle", eq(null),
eq(Locale.ENGLISH));
doReturn(german).when(provider, "createResourceBundle", eq(null),
eq(Locale.GERMAN));
+ Mockito.when(german.getLocale()).thenReturn(Locale.GERMAN);
+ Mockito.when(english.getLocale()).thenReturn(Locale.ENGLISH);
+ Mockito.when(german.getParent()).thenReturn(english);
+ }
+ @Test
+ public void loadBundlesOnlyOncePerLocale() throws Exception {
assertEquals(english, provider.getResourceBundle(Locale.ENGLISH));
assertEquals(english, provider.getResourceBundle(Locale.ENGLISH));
assertEquals(german, provider.getResourceBundle(Locale.GERMAN));
@@ -71,12 +82,6 @@ public class ConcurrentJcrResourceBundle
@Test
public void loadBundlesOnlyOnceWithConcurrentRequests() throws Exception {
- final JcrResourceBundleProvider provider = spy(new
JcrResourceBundleProvider());
- provider.activate(createComponentContext(new Hashtable<String,
Object>()));
-
- doReturn(english).when(provider, "createResourceBundle", eq(null),
eq(Locale.ENGLISH));
- doReturn(german).when(provider, "createResourceBundle", eq(null),
eq(Locale.GERMAN));
-
final int numberOfThreads = 40;
final ExecutorService executor =
Executors.newFixedThreadPool(numberOfThreads / 2);
for (int i = 0; i < numberOfThreads; i++) {
@@ -94,6 +99,42 @@ public class ConcurrentJcrResourceBundle
verifyPrivate(provider, times(1)).invoke("createResourceBundle",
eq(null), eq(Locale.ENGLISH));
verifyPrivate(provider, times(1)).invoke("createResourceBundle",
eq(null), eq(Locale.GERMAN));
}
+
+ @Test
+ public void newBundleUsedAfterReload() throws Exception {
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+
+ // reloading german should not reload any other bundle
+ provider.reloadBundle(german);
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+
+ verifyPrivate(provider, times(1)).invoke("createResourceBundle",
eq(null), eq(Locale.ENGLISH));
+ verifyPrivate(provider, times(2)).invoke("createResourceBundle",
eq(null), eq(Locale.GERMAN));
+ }
+
+ @Test
+ public void newBundleUsedAsParentAfterReload() throws Exception {
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+
+ // reloading english should also reload german (because it has english
as a parent)
+ provider.reloadBundle(english);
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+ provider.getResourceBundle(Locale.ENGLISH);
+ provider.getResourceBundle(Locale.GERMAN);
+
+ verifyPrivate(provider, times(2)).invoke("createResourceBundle",
eq(null), eq(Locale.ENGLISH));
+ verifyPrivate(provider, times(2)).invoke("createResourceBundle",
eq(null), eq(Locale.GERMAN));
+ }
private ComponentContext createComponentContext(Hashtable<String, Object>
config) {
final ComponentContext componentContext =
PowerMockito.mock(ComponentContext.class);
