You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2018/01/08 14:11:12 UTC

[GitHub] kwin closed pull request #1: Bugfix/fix resource bundles not all refreshed

kwin closed pull request #1: Bugfix/fix resource bundles not all refreshed
URL: https://github.com/apache/sling-org-apache-sling-i18n/pull/1
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java b/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java
index 43500b3..580ae4a 100644
--- a/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java
+++ b/src/main/java/org/apache/sling/i18n/impl/JcrResourceBundleProvider.java
@@ -210,48 +210,53 @@ public ResourceBundle getResourceBundle(String baseName, Locale locale) {
 
     @Override
     public void onChange(List<ResourceChange> changes) {
-        boolean refreshed = false;
-        for(final ResourceChange change : changes) {
-            log.trace("handleEvent: Detecting event {} for path '{}'", change.getType(), change.getPath());
-
-            // if this change was on languageRootPath level this might change basename and locale as well, therefore
-            // invalidate everything
-            if (languageRootPaths.contains(change.getPath())) {
-                log.debug(
-                        "handleEvent: Detected change of cached language root '{}', removing all cached ResourceBundles",
-                        change.getPath());
-                scheduleReloadBundles(true);
-            } else {
-                // 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 (change.getPath().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);
-                                scheduleReloadBundle(bundle);
-                                return;
-                            }
+        for (final ResourceChange change : changes) {
+             onChange(change);
+        }
+        // refresh at most once per onChange()
+        resourceResolver.refresh();
+    }
+
+    private void onChange(ResourceChange change) {
+        log.trace("handleChange: Detecting change {} for path '{}'", change.getType(), change.getPath());
+
+        // if this change was on languageRootPath level this might change basename and locale as well, therefore
+        // invalidate everything
+        if (languageRootPaths.contains(change.getPath())) {
+            log.debug(
+                    "handleChange: Detected change of cached language root '{}', removing all cached ResourceBundles",
+                    change.getPath());
+            scheduleReloadBundles(true);
+        } else {
+            for (final String root : languageRootPaths) {
+                if (change.getPath().startsWith(root)) {
+                    // figure out which JcrResourceBundles from the cached ones is affected
+                    boolean bundlesFound = false;
+                    for (JcrResourceBundle bundle : resourceBundleCache.values()) {
+                        if (bundle.getLanguageRootPaths().contains(root)) {
+                            // reload it
+                            log.debug("handleChange: Resource changes below '{}', reloading ResourceBundle '{}'",
+                                    root, bundle);
+                            scheduleReloadBundle(bundle);
+                            bundlesFound = true;
                         }
-                        log.debug("handleEvent: No cached resource bundle found with root '{}'", root);
-                        break;
                     }
-                }
-                // may be a completely new dictionary
-                if (!refreshed) {
-                    // refresh at most once per onChange()
-                    resourceResolver.refresh();
-                    refreshed = true;
-                }
-                if (isDictionaryResource(change)) {
-                    scheduleReloadBundles(true);
+                    if(bundlesFound) {
+                        log.debug("handleChange: Refreshed all affected resource bundles for path '{}'", change.getPath());
+                        return;
+                    }
+                    log.debug("handleChange: No cached resource bundle found with root '{}'", root);
+                    break;
                 }
             }
         }
+        // may be a completely new dictionary
+        if (isDictionaryResource(change)) {
+            scheduleReloadBundles(true);
+        }
     }
 
+
     private boolean isDictionaryResource(final ResourceChange change) {
         // language node changes happen quite frequently (https://issues.apache.org/jira/browse/SLING-2881)
         // therefore only consider changes either for sling:MessageEntry's
diff --git a/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java b/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java
index 7e1dfa2..d0704bd 100644
--- a/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java
+++ b/src/test/java/org/apache/sling/i18n/it/ResourceBundleProviderIT.java
@@ -18,26 +18,6 @@
  */
 package org.apache.sling.i18n.it;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.fail;
-import static org.ops4j.pax.exam.CoreOptions.frameworkProperty;
-import static org.ops4j.pax.exam.CoreOptions.junitBundles;
-import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
-import static org.ops4j.pax.exam.CoreOptions.options;
-import static org.ops4j.pax.exam.CoreOptions.systemProperty;
-import static org.ops4j.pax.exam.CoreOptions.when;
-
-import java.io.File;
-import java.net.URISyntaxException;
-import java.util.Locale;
-import java.util.ResourceBundle;
-
-import javax.inject.Inject;
-import javax.jcr.Node;
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-
 import org.apache.sling.i18n.ResourceBundleProvider;
 import org.apache.sling.jcr.api.SlingRepository;
 import org.junit.After;
@@ -52,6 +32,18 @@
 import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
 import org.ops4j.pax.exam.spi.reactors.PerClass;
 
+import javax.inject.Inject;
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import java.io.File;
+import java.net.URISyntaxException;
+import java.util.Locale;
+import java.util.ResourceBundle;
+
+import static org.junit.Assert.*;
+import static org.ops4j.pax.exam.CoreOptions.*;
+
 @RunWith(PaxExam.class)
 @ExamReactorStrategy(PerClass.class)
 public class ResourceBundleProviderIT {
@@ -68,6 +60,9 @@
     public static final int RETRY_TIMEOUT_MSEC = 50000;
     public static final String MSG_KEY1 = "foo";
     public static final String MSG_KEY2 = "foo2";
+    public static final String MSG_KEY3 = "foo3";
+
+    public static final String BASENAME = "basename";
 
     @Inject
     private SlingRepository repository;
@@ -81,6 +76,7 @@
     private Node deDeRoot;
     private Node frRoot;
     private Node enRoot;
+    private Node enBasenameRoot;
 
     @Configuration
     public Option[] config() {
@@ -295,6 +291,7 @@ public void setup() throws RepositoryException {
         frRoot = addLanguageNode(i18nRoot, "fr");
         deDeRoot = addLanguageNode(i18nRoot, "de_DE");
         enRoot = addLanguageNode(i18nRoot, "en");
+        enBasenameRoot = addLanguageNodeWithBasename(i18nRoot, "en",BASENAME);
         session.save();
     }
 
@@ -306,35 +303,41 @@ public void cleanup() throws RepositoryException {
     }
 
     private Node addLanguageNode(Node parent, String language) throws RepositoryException {
-        final Node child = parent.addNode(language, "nt:folder");
+        final Node child = parent.addNode(language, "sling:Folder");
         child.addMixin("mix:language");
         child.setProperty("jcr:language", language);
         return child;
     }
 
-    private void assertMessages(final String key, final String deMessage, final String deDeMessage, final String frMessage) {
+    private Node addLanguageNodeWithBasename(Node parent, String language, String basename) throws RepositoryException {
+        final Node child = parent.addNode(language + "-" + basename, "sling:Folder");
+        child.addMixin("mix:language");
+        child.setProperty("jcr:language", language);
+        if(basename != null) {
+            child.setProperty("sling:basename", basename);
+        }
+        return child;
+    }
+
+    private void assertMessage(final String key, final Locale locale, final String basename, final String value) {
         new Retry(RETRY_TIMEOUT_MSEC) {
             @Override
             protected void exec() {
                 {
-                    final ResourceBundle deDE = resourceBundleProvider.getResourceBundle(Locale.GERMANY); // this is the resource bundle for de_DE
-                    assertNotNull(deDE);
-                    assertEquals(deDeMessage, deDE.getString(key));
-                }
-                {
-                    final ResourceBundle de = resourceBundleProvider.getResourceBundle(Locale.GERMAN);
-                    assertNotNull(de);
-                    assertEquals(deMessage, de.getString(key));
-                }
-                {
-                    final ResourceBundle fr = resourceBundleProvider.getResourceBundle(Locale.FRENCH);
-                    assertNotNull(fr);
-                    assertEquals(frMessage, fr.getString(key));
+                    final ResourceBundle resourceBundle = resourceBundleProvider.getResourceBundle(basename, locale); // this is the resource bundle for de_DE
+                    assertNotNull(resourceBundle);
+                    assertEquals(value, resourceBundle.getString(key));
                 }
             }
         };
     }
 
+    private void assertMessages(final String key, final String deMessage, final String deDeMessage, final String frMessage) {
+        assertMessage(key, Locale.GERMAN, null, deMessage);
+        assertMessage(key, Locale.GERMANY, null, deDeMessage);
+        assertMessage(key, Locale.FRENCH, null, frMessage);
+    }
+
     private void setMessage(final Node rootNode, final String key, final String message) throws RepositoryException {
         final String nodeName = "node_" + key;
         final Node node;
@@ -376,6 +379,18 @@ public void testChangesDetection() throws RepositoryException {
         setMessage(enRoot, MSG_KEY2, "EN_changed");
         session.save();
         assertMessages(MSG_KEY2, "EN_changed", "EN_changed", "EN_changed");
+
+        // set a message and fetch it so that it is cached in the resourcebundle cache
+        setMessage(enBasenameRoot, MSG_KEY3, "EN_basename_message");
+        session.save();
+        assertMessage(MSG_KEY3, Locale.ENGLISH, null, "EN_basename_message");
+        assertMessage(MSG_KEY3, Locale.ENGLISH, BASENAME, "EN_basename_message");
+
+        // see that both resource bundles with and without basename are changed
+        setMessage(enBasenameRoot, MSG_KEY3, "EN_basename_changed");
+        session.save();
+        assertMessage(MSG_KEY3, Locale.ENGLISH, null, "EN_basename_changed");
+        assertMessage(MSG_KEY3, Locale.ENGLISH, BASENAME, "EN_basename_changed");
     }
 
     private String references() {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services