SONAR-3596 I18n can search for plugin bundles inside language packs

4 files changed, 78 insertions, 19 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/i18n/I18nManager.java b/sonar-core/src/main/java/org/sonar/core/i18n/I18nManager.java
index e582685d6ea..a61509e71cf 100644
--- a/sonar-core/src/main/java/org/sonar/core/i18n/I18nManager.java
+++ b/sonar-core/src/main/java/org/sonar/core/i18n/I18nManager.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.core.i18n;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Maps;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
@@ -58,8 +59,10 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
     this.pluginRepository = pluginRepository;
   }
 
-  I18nManager(Map<String, ClassLoader> bundleToClassloaders) {
+  @VisibleForTesting
+  I18nManager(Map<String, ClassLoader> bundleToClassloaders, ClassLoader languagePackClassLoader) {
     this.bundleToClassloaders = bundleToClassloaders;
+    this.languagePackClassLoader = languagePackClassLoader;
   }
 
   public void start() {
@@ -109,7 +112,10 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
 
   public String message(Locale locale, String key, String defaultValue, Object... parameters) {
     String bundleKey = propertyToBundles.get(key);
-    ResourceBundle resourceBundle = getBundle(bundleKey, locale);
+    ResourceBundle resourceBundle = null;
+    if (bundleKey != null) {
+      resourceBundle = getBundle(bundleKey, locale);
+    }
     return message(resourceBundle, key, defaultValue, parameters);
   }
 
@@ -123,7 +129,7 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
       return fileCache.get(locale);
     }
 
-    ClassLoader classloader = getClassLoaderForProperty(relatedProperty);
+    ClassLoader classloader = getClassLoaderForProperty(relatedProperty, locale);
     String result = null;
     if (classloader != null) {
       String bundleBase = propertyToBundles.get(relatedProperty);
@@ -161,20 +167,38 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
   }
 
   ResourceBundle getBundle(String bundleKey, Locale locale) {
+    ResourceBundle bundle = null;
     try {
+      // First, we check if the bundle exists in the language pack classloader
+      bundle = ResourceBundle.getBundle(bundleKey, locale, languagePackClassLoader);
+    } catch (MissingResourceException e1) {
+      // well, maybe the plugin has specified its own bundles, let's see
       ClassLoader classloader = bundleToClassloaders.get(bundleKey);
       if (classloader != null) {
-        return ResourceBundle.getBundle(bundleKey, locale, classloader);
+        try {
+          bundle = ResourceBundle.getBundle(bundleKey, locale, classloader);
+        } catch (MissingResourceException e2) {
+          // Well, here, there's nothing much we can do...
+        }
       }
-    } catch (MissingResourceException e) {
-      // ignore
     }
-    return null;
+    return bundle;
   }
 
-  ClassLoader getClassLoaderForProperty(String propertyKey) {
+  ClassLoader getClassLoaderForProperty(String propertyKey, Locale locale) {
     String bundleKey = propertyToBundles.get(propertyKey);
-    return (bundleKey != null ? bundleToClassloaders.get(bundleKey) : null);
+    if (bundleKey == null) {
+      return null;
+    }
+
+    try {
+      // First, we check if the bundle exists in the language pack classloader
+      ResourceBundle.getBundle(bundleKey, locale, languagePackClassLoader);
+      return languagePackClassLoader;
+    } catch (MissingResourceException e) {
+      // the plugin has specified its own bundles
+      return bundleToClassloaders.get(bundleKey);
+    }
   }
 
   String message(ResourceBundle resourceBundle, String key, String defaultValue, Object... parameters) {
diff --git a/sonar-core/src/test/java/org/sonar/core/i18n/I18nManagerTest.java b/sonar-core/src/test/java/org/sonar/core/i18n/I18nManagerTest.java
index 121f0bd9f1b..be28db66446 100644
--- a/sonar-core/src/test/java/org/sonar/core/i18n/I18nManagerTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/i18n/I18nManagerTest.java
@@ -31,6 +31,7 @@ import java.net.URLClassLoader;
 import java.util.Locale;
 import java.util.Map;
 
+import static org.fest.assertions.Assertions.assertThat;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertThat;
@@ -42,6 +43,7 @@ public class I18nManagerTest {
   private I18nManager manager;
   private ClassLoader coreClassLoader;
   private ClassLoader sqaleClassLoader;
+  private ClassLoader forgeClassLoader;
 
   /**
    * See http://jira.codehaus.org/browse/SONAR-2927
@@ -59,13 +61,20 @@ public class I18nManagerTest {
 
   @Before
   public void init() {
-    coreClassLoader = newCoreClassLoader();
-    sqaleClassLoader = newSqaleClassLoader();
     Map<String, ClassLoader> bundleToClassLoaders = Maps.newHashMap();
+    // following represents the English language pack + a core plugin : they use the same classloader
+    coreClassLoader = newCoreClassLoader();
     bundleToClassLoaders.put(BUNDLE_PACKAGE + "core", coreClassLoader);
     bundleToClassLoaders.put(BUNDLE_PACKAGE + "checkstyle", coreClassLoader);
+    // following represents a commercial plugin that must embed all its bundles, whatever the language
+    sqaleClassLoader = newSqaleClassLoader();
     bundleToClassLoaders.put(BUNDLE_PACKAGE + "sqale", sqaleClassLoader);
-    manager = new I18nManager(bundleToClassLoaders);
+    // following represents a forge plugin that embeds only the english bundle, and lets the language
+    // packs embed all the bundles for the other languages
+    forgeClassLoader = newForgeClassLoader();
+    bundleToClassLoaders.put(BUNDLE_PACKAGE + "forge", forgeClassLoader);
+
+    manager = new I18nManager(bundleToClassLoaders, coreClassLoader);
     manager.start();
   }
 
@@ -75,7 +84,7 @@ public class I18nManagerTest {
     bundleToClassLoaders.put(BUNDLE_PACKAGE + "core", getClass().getClassLoader());
     bundleToClassLoaders.put(BUNDLE_PACKAGE + "checkstyle", getClass().getClassLoader());
     bundleToClassLoaders.put(BUNDLE_PACKAGE + "sqale", getClass().getClassLoader());
-    I18nManager i18n = new I18nManager(bundleToClassLoaders);
+    I18nManager i18n = new I18nManager(bundleToClassLoaders, coreClassLoader);
     i18n.start();
 
     assertThat(i18n.extractBundleFromKey("by"), Is.is(BUNDLE_PACKAGE + "core"));
@@ -134,14 +143,22 @@ public class I18nManagerTest {
 
   @Test
   public void shouldGetClassLoaderByProperty() {
-    assertThat(manager.getClassLoaderForProperty("foo.unknown"), nullValue());
-    assertThat(manager.getClassLoaderForProperty("by"), Is.is(coreClassLoader));
-    assertThat(manager.getClassLoaderForProperty("sqale.page"), Is.is(sqaleClassLoader));
+    assertThat(manager.getClassLoaderForProperty("foo.unknown", Locale.ENGLISH), nullValue());
+    assertThat(manager.getClassLoaderForProperty("by", Locale.ENGLISH), Is.is(coreClassLoader));
+    // The following plugin defines its own bundles, whatever the language
+    assertThat(manager.getClassLoaderForProperty("sqale.page", Locale.ENGLISH), Is.is(sqaleClassLoader));
+    assertThat(manager.getClassLoaderForProperty("sqale.page", Locale.FRENCH), Is.is(sqaleClassLoader));
+    // The following plugin defines only the English bundle, and lets the language packs handle the translations
+    assertThat(manager.getClassLoaderForProperty("forge_plugin.page", Locale.ENGLISH), Is.is(forgeClassLoader));
+    assertThat(manager.getClassLoaderForProperty("forge_plugin.page", Locale.FRENCH), Is.is(coreClassLoader));
   }
 
   @Test
   public void shouldFindEnglishFile() {
-    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */, false);
+    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name" /*
+                                                                                                            * any property in the same
+                                                                                                            * bundle
+                                                                                                            */, false);
     assertThat(html, Is.is("This is the architecture rule"));
   }
 
@@ -167,7 +184,10 @@ public class I18nManagerTest {
   public void shouldNotKeepInCache() {
     assertThat(manager.getFileContentCache().size(), Is.is(0));
     boolean keepInCache = false;
-    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */, keepInCache);
+    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name" /*
+                                                                                                            * any property in the same
+                                                                                                            * bundle
+                                                                                                            */, keepInCache);
 
     assertThat(html, not(nullValue()));
     assertThat(manager.getFileContentCache().size(), Is.is(0));
@@ -177,7 +197,10 @@ public class I18nManagerTest {
   public void shouldKeepInCache() {
     assertThat(manager.getFileContentCache().size(), Is.is(0));
     boolean keepInCache = true;
-    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */, keepInCache);
+    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name" /*
+                                                                                                            * any property in the same
+                                                                                                            * bundle
+                                                                                                            */, keepInCache);
 
     assertThat(html, not(nullValue()));
     Map<String, Map<Locale, String>> cache = manager.getFileContentCache();
@@ -185,6 +208,16 @@ public class I18nManagerTest {
     assertThat(cache.get("ArchitectureRule.html").get(Locale.ENGLISH), Is.is("This is the architecture rule"));
   }
 
+  // see SONAR-3596
+  @Test
+  public void shouldLookInCoreClassloaderForPluginsThatDontEmbedAllLanguages() {
+    assertThat(manager.message(Locale.ENGLISH, "forge_plugin.page", null)).isEqualTo("This is my plugin");
+    assertThat(manager.message(Locale.FRENCH, "forge_plugin.page", null)).isEqualTo("C'est mon plugin");
+  }
+
+  private URLClassLoader newForgeClassLoader() {
+    return newClassLoader("/org/sonar/core/i18n/forgePlugin/");
+  }
 
   private URLClassLoader newSqaleClassLoader() {
     return newClassLoader("/org/sonar/core/i18n/sqalePlugin/");
diff --git a/sonar-core/src/test/resources/org/sonar/core/i18n/forgePlugin/org/sonar/l10n/forge.properties b/sonar-core/src/test/resources/org/sonar/core/i18n/forgePlugin/org/sonar/l10n/forge.properties
new file mode 100644
index 00000000000..f2ccf6fc16f
--- /dev/null
+++ b/sonar-core/src/test/resources/org/sonar/core/i18n/forgePlugin/org/sonar/l10n/forge.properties
@@ -0,0 +1 @@
+forge_plugin.page=This is my plugin
\ No newline at end of file
diff --git a/sonar-core/src/test/resources/org/sonar/core/i18n/frenchPack/org/sonar/l10n/forge_fr.properties b/sonar-core/src/test/resources/org/sonar/core/i18n/frenchPack/org/sonar/l10n/forge_fr.properties
new file mode 100644
index 00000000000..96fb089d580
--- /dev/null
+++ b/sonar-core/src/test/resources/org/sonar/core/i18n/frenchPack/org/sonar/l10n/forge_fr.properties
@@ -0,0 +1 @@
+forge_plugin.page=C'est mon plugin
\ No newline at end of file