sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From k...@apache.org
Subject svn commit: r1685933 - in /sling/trunk/bundles/extensions/i18n/src: main/java/org/apache/sling/i18n/impl/ test/java/org/apache/sling/i18n/impl/
Date Wed, 17 Jun 2015 07:58:43 GMT
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);



Mime
View raw message