SONAR-3910 Sonar startup crashes due to French Translation Pack

11 files changed, 278 insertions, 245 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/i18n/GwtI18n.java b/sonar-core/src/main/java/org/sonar/core/i18n/GwtI18n.java
index bf208bff708..4e560726028 100644
--- a/sonar-core/src/main/java/org/sonar/core/i18n/GwtI18n.java
+++ b/sonar-core/src/main/java/org/sonar/core/i18n/GwtI18n.java
@@ -77,7 +77,7 @@ public class GwtI18n implements ServerComponent {
 
   ResourceBundle getBundle(Locale locale) {
     try {
-      return ResourceBundle.getBundle(GWT_BUNDLE, locale, manager.getLanguagePackClassLoader());
+      return ResourceBundle.getBundle(GWT_BUNDLE, locale, manager.getBundleClassLoader());
     } catch (MissingResourceException e) {
       throw new IllegalStateException("The English bundle for GWT extensions is not deployed", e);
     }
diff --git a/sonar-core/src/main/java/org/sonar/core/i18n/I18nClassloader.java b/sonar-core/src/main/java/org/sonar/core/i18n/I18nClassloader.java
new file mode 100644
index 00000000000..e57b9ad9eba
--- /dev/null
+++ b/sonar-core/src/main/java/org/sonar/core/i18n/I18nClassloader.java
@@ -0,0 +1,70 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.core.i18n;
+
+import com.google.common.collect.Lists;
+import org.sonar.api.Plugin;
+import org.sonar.api.platform.PluginRepository;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.List;
+
+class I18nClassloader extends URLClassLoader {
+
+  private ClassLoader[] pluginClassloaders;
+
+  public I18nClassloader(PluginRepository pluginRepository) {
+    super(new URL[0]);
+    List<ClassLoader> list = Lists.newArrayList();
+    for (Plugin plugin : pluginRepository.getPlugins()) {
+      ClassLoader classloader = plugin.getClass().getClassLoader();
+      if (classloader.getResource("org/sonar/l10n/") != null) {
+        list.add(classloader);
+      }
+    }
+    this.pluginClassloaders = list.toArray(new ClassLoader[list.size()]);
+  }
+
+  I18nClassloader(ClassLoader[] pluginClassloaders) {
+    super(new URL[0]);
+    this.pluginClassloaders = pluginClassloaders;
+  }
+
+  public URL getResource(String name) {
+    for (ClassLoader pluginClassloader : pluginClassloaders) {
+      URL url = pluginClassloader.getResource(name);
+      if (url != null) {
+        return url;
+      }
+    }
+    return null;
+  }
+
+  @Override
+  protected synchronized Class<?> loadClass(String s, boolean b) throws ClassNotFoundException {
+    throw new UnsupportedOperationException("I18n classloader does support only resources, but not classes");
+  }
+
+  @Override
+  public String toString() {
+    return "i18n-classloader";
+  }
+}
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 922710e4311..694536a23df 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
@@ -22,7 +22,6 @@ 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;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.BatchExtension;
@@ -32,10 +31,11 @@ import org.sonar.api.platform.PluginMetadata;
 import org.sonar.api.platform.PluginRepository;
 import org.sonar.api.utils.SonarException;
 
+import javax.annotation.Nullable;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.text.MessageFormat;
-import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Locale;
 import java.util.Map;
@@ -46,52 +46,29 @@ import java.util.Set;
 public class I18nManager implements I18n, ServerExtension, BatchExtension {
   private static final Logger LOG = LoggerFactory.getLogger(I18nManager.class);
 
-  public static final String ENGLISH_PACK_PLUGIN_KEY = "l10nen";
   public static final String BUNDLE_PACKAGE = "org.sonar.l10n.";
 
   private PluginRepository pluginRepository;
-  private Map<String, ClassLoader> bundleToClassloaders;
+  private I18nClassloader i18nClassloader;
   private Map<String, String> propertyToBundles;
-  private ClassLoader languagePackClassLoader;
   private Map<String, Map<Locale, String>> fileContentCache = Maps.newHashMap();
 
   public I18nManager(PluginRepository pluginRepository) {
     this.pluginRepository = pluginRepository;
   }
 
-  @VisibleForTesting
-  I18nManager(Map<String, ClassLoader> bundleToClassloaders, ClassLoader languagePackClassLoader) {
-    this.bundleToClassloaders = bundleToClassloaders;
-    this.languagePackClassLoader = languagePackClassLoader;
-  }
-
   public void start() {
-    initClassloaders();
-    initProperties();
+    doStart(new I18nClassloader(pluginRepository));
   }
 
-  private void initClassloaders() {
-    if (bundleToClassloaders == null) {
-      languagePackClassLoader = pluginRepository.getPlugin(ENGLISH_PACK_PLUGIN_KEY).getClass().getClassLoader();
-      bundleToClassloaders = Maps.newHashMap();
-      for (PluginMetadata metadata : pluginRepository.getMetadata()) {
-        if (!ENGLISH_PACK_PLUGIN_KEY.equals(metadata.getKey())
-          && !ENGLISH_PACK_PLUGIN_KEY.equals(metadata.getBasePlugin())) {
-          // This is a "simple" plugin, not a Language Pack
-          ClassLoader classLoader = pluginRepository.getPlugin(metadata.getKey()).getClass().getClassLoader();
-          bundleToClassloaders.put(BUNDLE_PACKAGE + metadata.getKey(), classLoader);
-        }
-      }
-    }
-    bundleToClassloaders = Collections.unmodifiableMap(bundleToClassloaders);
-  }
-
-  private void initProperties() {
+  @VisibleForTesting
+  void doStart(I18nClassloader classloader) {
+    this.i18nClassloader = classloader;
     propertyToBundles = Maps.newHashMap();
-    for (Map.Entry<String, ClassLoader> entry : bundleToClassloaders.entrySet()) {
+    for (PluginMetadata plugin : pluginRepository.getMetadata()) {
       try {
-        String bundleKey = entry.getKey();
-        ResourceBundle bundle = ResourceBundle.getBundle(bundleKey, Locale.ENGLISH, entry.getValue());
+        String bundleKey = BUNDLE_PACKAGE + plugin.getKey();
+        ResourceBundle bundle = ResourceBundle.getBundle(bundleKey, Locale.ENGLISH, i18nClassloader);
         Enumeration<String> keys = bundle.getKeys();
         while (keys.hasMoreElements()) {
           String key = keys.nextElement();
@@ -101,25 +78,24 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
         // ignore
       }
     }
-    propertyToBundles = Collections.unmodifiableMap(propertyToBundles);
-    LOG.debug(String.format("Loaded %d properties from English bundles", propertyToBundles.size()));
+    LOG.debug(String.format("Loaded %d properties from l10n bundles", propertyToBundles.size()));
   }
 
   public String message(Locale locale, String key, String defaultValue, Object... parameters) {
     String bundleKey = propertyToBundles.get(key);
-    ResourceBundle resourceBundle = null;
+    String value = null;
     if (bundleKey != null) {
       try {
-        // First, we check if the bundle exists in the language pack classloader
-        resourceBundle = ResourceBundle.getBundle(bundleKey, locale, languagePackClassLoader);
-        String message = resourceBundle.getString(key);
-        return formatMessage(message, parameters);
+        ResourceBundle resourceBundle = ResourceBundle.getBundle(bundleKey, locale, i18nClassloader);
+        value = resourceBundle.getString(key);
       } catch (MissingResourceException e1) {
-        // well, maybe the plugin has specified its own bundles, let's see
-        resourceBundle = getBundleFromCorrespondingPluginClassloader(bundleKey, locale);
+        // ignore
       }
     }
-    return message(resourceBundle, key, defaultValue, parameters);
+    if (value == null) {
+      value = defaultValue;
+    }
+    return formatMessage(value, parameters);
   }
 
   /**
@@ -132,28 +108,25 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
       return fileCache.get(locale);
     }
 
-    ClassLoader classloader = getClassLoaderForProperty(relatedProperty, locale);
     String result = null;
-    if (classloader != null) {
-      String bundleBase = propertyToBundles.get(relatedProperty);
-      String filePath = bundleBase.replace('.', '/');
-      if (!"en".equals(locale.getLanguage())) {
-        filePath += "_" + locale.getLanguage();
-      }
-      filePath += "/" + filename;
-      InputStream input = classloader.getResourceAsStream(filePath);
-      if (input != null) {
-        result = readInputStream(filePath, input);
-      }
+    String bundleBase = propertyToBundles.get(relatedProperty);
+    String filePath = bundleBase.replace('.', '/');
+    if (!"en".equals(locale.getLanguage())) {
+      filePath += "_" + locale.getLanguage();
+    }
+    filePath += "/" + filename;
+    InputStream input = i18nClassloader.getResourceAsStream(filePath);
+    if (input != null) {
+      result = readInputStream(filePath, input);
+    }
 
-      if (keepInCache) {
-        if (fileCache == null) {
-          fileCache = Maps.newHashMap();
-          fileContentCache.put(filename, fileCache);
-        }
-        // put null value for negative caching.
-        fileCache.put(locale, result);
+    if (keepInCache) {
+      if (fileCache == null) {
+        fileCache = Maps.newHashMap();
+        fileContentCache.put(filename, fileCache);
       }
+      // put null value for negative caching.
+      fileCache.put(locale, result);
     }
     return result;
   }
@@ -170,70 +143,21 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
     return result;
   }
 
+  @VisibleForTesting
   Set<String> getPropertyKeys() {
     return propertyToBundles.keySet();
   }
 
-  ResourceBundle getBundleFromCorrespondingPluginClassloader(String bundleKey, Locale locale) {
-    ClassLoader classloader = bundleToClassloaders.get(bundleKey);
-    if (classloader != null) {
-      try {
-        return ResourceBundle.getBundle(bundleKey, locale, classloader);
-      } catch (MissingResourceException e2) {
-        // Well, here, there's nothing much we can do...
-      }
-    }
-    return null;
-  }
-
-  ClassLoader getClassLoaderForProperty(String propertyKey, Locale locale) {
-    String bundleKey = propertyToBundles.get(propertyKey);
-    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) {
-    String value = null;
-    if (resourceBundle != null) {
-      try {
-        value = resourceBundle.getString(key);
-      } catch (MissingResourceException e) {
-        // ignore
-      }
-    }
-    if (value == null) {
-      value = defaultValue;
-    }
-    return formatMessage(value, parameters);
-  }
-
-  private String formatMessage(String message, Object... parameters) {
+  private String formatMessage(@Nullable String message, Object... parameters) {
     if (message == null || parameters.length == 0) {
       return message;
     }
     return MessageFormat.format(message.replaceAll("'", "''"), parameters);
   }
 
-  String extractBundleFromKey(String key) {
-    String bundleKey = BUNDLE_PACKAGE + StringUtils.substringBefore(key, ".");
-    if (bundleToClassloaders.containsKey(bundleKey)) {
-      return bundleKey;
-    }
-    return BUNDLE_PACKAGE + "core";
-  }
 
-  ClassLoader getLanguagePackClassLoader() {
-    return languagePackClassLoader;
+  ClassLoader getBundleClassLoader() {
+    return i18nClassloader;
   }
 
   Map<String, Map<Locale, String>> getFileContentCache() {
diff --git a/sonar-core/src/test/java/org/sonar/core/i18n/I18nClassloaderTest.java b/sonar-core/src/test/java/org/sonar/core/i18n/I18nClassloaderTest.java
new file mode 100644
index 00000000000..22210dc6be3
--- /dev/null
+++ b/sonar-core/src/test/java/org/sonar/core/i18n/I18nClassloaderTest.java
@@ -0,0 +1,67 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.core.i18n;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import java.net.URLClassLoader;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+public class I18nClassloaderTest {
+  private I18nClassloader i18nClassloader;
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
+  @Before
+  public void init() {
+    URLClassLoader sqale = I18nManagerTest.newSqaleClassloader();
+    URLClassLoader checkstyle = I18nManagerTest.newCheckstyleClassloader();
+
+    i18nClassloader = new I18nClassloader(new ClassLoader[]{sqale, checkstyle});
+  }
+
+  @Test
+  public void should_aggregate_plugin_classloaders() {
+    assertThat(i18nClassloader.getResource("org/sonar/l10n/checkstyle.properties")).isNotNull();
+    assertThat(i18nClassloader.getResource("org/sonar/l10n/checkstyle.properties").getFile()).endsWith("checkstyle.properties");
+    assertThat(i18nClassloader.getResource("org/sonar/l10n/checkstyle/ArchitectureRule.html").getFile()).endsWith("ArchitectureRule.html");
+  }
+
+  @Test
+  public void should_return_null_if_resource_not_found() {
+    assertThat(i18nClassloader.getResource("org/unknown.properties")).isNull();
+  }
+
+  @Test
+  public void should_not_support_lookup_of_java_classes() throws ClassNotFoundException {
+    thrown.expect(UnsupportedOperationException.class);
+    i18nClassloader.loadClass("java.lang.String");
+  }
+
+  @Test
+  public void should_override_toString() throws ClassNotFoundException {
+    assertThat(i18nClassloader.toString()).isEqualTo("i18n-classloader");
+  }
+}
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 19cb921cf75..2e7b527c66d 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
@@ -19,31 +19,27 @@
  */
 package org.sonar.core.i18n;
 
-import com.google.common.collect.Maps;
-import org.hamcrest.core.Is;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.sonar.api.platform.PluginMetadata;
+import org.sonar.api.platform.PluginRepository;
 
 import java.net.URL;
 import java.net.URLClassLoader;
+import java.util.Arrays;
+import java.util.List;
 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;
-import static org.sonar.core.i18n.I18nManager.BUNDLE_PACKAGE;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class I18nManagerTest {
 
   private static Locale defaultLocale;
   private I18nManager manager;
-  private ClassLoader coreClassLoader;
-  private ClassLoader sqaleClassLoader;
-  private ClassLoader forgeClassLoader;
 
   /**
    * See http://jira.codehaus.org/browse/SONAR-2927
@@ -61,186 +57,165 @@ public class I18nManagerTest {
 
   @Before
   public void init() {
-    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);
-    // 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();
+    PluginRepository pluginRepository = mock(PluginRepository.class);
+    List<PluginMetadata> plugins = Arrays.asList(newPlugin("sqale"), newPlugin("frpack"), newPlugin("core"), newPlugin("checkstyle"), newPlugin("other"));
+    when(pluginRepository.getMetadata()).thenReturn(plugins);
+
+    I18nClassloader i18nClassloader = new I18nClassloader(new ClassLoader[]{
+      newCoreClassloader(), newFrenchPackClassloader(), newSqaleClassloader(), newCheckstyleClassloader()
+    });
+    manager = new I18nManager(pluginRepository);
+    manager.doStart(i18nClassloader);
   }
 
   @Test
-  public void shouldExtractPluginFromKey() {
-    Map<String, ClassLoader> bundleToClassLoaders = Maps.newHashMap();
-    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, coreClassLoader);
-    i18n.start();
+  public void should_introspect_all_available_properties() {
+    assertThat(manager.getPropertyKeys().contains("by")).isTrue();
+    assertThat(manager.getPropertyKeys().contains("only.in.english")).isTrue();
+    assertThat(manager.getPropertyKeys().contains("sqale.page")).isTrue();
+    assertThat(manager.getPropertyKeys().contains("unknown")).isFalse();
+  }
 
-    assertThat(i18n.extractBundleFromKey("by"), Is.is(BUNDLE_PACKAGE + "core"));
-    assertThat(i18n.extractBundleFromKey("violations_drilldown.page"), Is.is(BUNDLE_PACKAGE + "core"));
-    assertThat(i18n.extractBundleFromKey("checkstyle.rule1.name"), Is.is(BUNDLE_PACKAGE + "checkstyle"));
-    assertThat(i18n.extractBundleFromKey("sqale.console.page"), Is.is(BUNDLE_PACKAGE + "sqale"));
+  @Test
+  public void should_get_english_labels() {
+    assertThat(manager.message(Locale.ENGLISH, "by", null)).isEqualTo("By");
+    assertThat(manager.message(Locale.ENGLISH, "sqale.page", null)).isEqualTo("Sqale page title");
+    assertThat(manager.message(Locale.ENGLISH, "checkstyle.rule1.name", null)).isEqualTo("Rule one");
   }
 
   @Test
-  public void shouldFindKeysInEnglishLanguagePack() {
-    assertThat(manager.message(Locale.ENGLISH, "checkstyle.rule1.name", null), Is.is("Rule one"));
-    assertThat(manager.message(Locale.ENGLISH, "by", null), Is.is("By"));
-    assertThat(manager.message(Locale.ENGLISH, "sqale.page", null), Is.is("Sqale page title"));
+  public void should_get_labels_from_french_pack() {
+    assertThat(manager.message(Locale.FRENCH, "checkstyle.rule1.name", null)).isEqualTo("Rule un");
+    assertThat(manager.message(Locale.FRENCH, "by", null)).isEqualTo("Par");
 
-    assertThat(manager.message(Locale.FRENCH, "checkstyle.rule1.name", null), Is.is("Rule un"));
-    assertThat(manager.message(Locale.FRENCH, "by", null), Is.is("Par"));
-    assertThat(manager.message(Locale.FRENCH, "sqale.page", null), Is.is("Titre de la page Sqale"));
+    // language pack
+    assertThat(manager.message(Locale.FRENCH, "sqale.page", null)).isEqualTo("Titre de la page Sqale");
   }
 
   @Test
-  public void shouldUseDefaultLocale() {
-    assertThat(manager.message(Locale.CHINA, "checkstyle.rule1.name", null), Is.is("Rule one"));
-    assertThat(manager.message(Locale.CHINA, "by", null), Is.is("By"));
-    assertThat(manager.message(Locale.CHINA, "sqale.page", null), Is.is("Sqale page title"));
+  public void should_get_french_label_if_swiss_country() {
+    Locale swiss = new Locale("fr", "CH");
+    assertThat(manager.message(swiss, "checkstyle.rule1.name", null)).isEqualTo("Rule un");
+    assertThat(manager.message(swiss, "by", null)).isEqualTo("Par");
+
+    // language pack
+    assertThat(manager.message(swiss, "sqale.page", null)).isEqualTo("Titre de la page Sqale");
   }
 
   @Test
-  public void shouldUseLanguagePack() {
-    assertThat(manager.message(Locale.FRENCH, "checkstyle.rule1.name", null), Is.is("Rule un"));
-    assertThat(manager.message(Locale.FRENCH, "by", null), Is.is("Par"));
-    assertThat(manager.message(Locale.FRENCH, "sqale.page", null), Is.is("Titre de la page Sqale"));
+  public void should_fallback_to_default_locale() {
+    assertThat(manager.message(Locale.CHINA, "checkstyle.rule1.name", null)).isEqualTo("Rule one");
+    assertThat(manager.message(Locale.CHINA, "by", null)).isEqualTo("By");
+    assertThat(manager.message(Locale.CHINA, "sqale.page", null)).isEqualTo("Sqale page title");
   }
 
+
   @Test
-  public void shouldReturnDefaultValueIfMissingKey() {
-    assertThat(manager.message(Locale.ENGLISH, "foo.unknown", "default"), Is.is("default"));
-    assertThat(manager.message(Locale.FRENCH, "foo.unknown", "default"), Is.is("default"));
+  public void should_return_default_value_if_missing_key() {
+    assertThat(manager.message(Locale.ENGLISH, "unknown", "default")).isEqualTo("default");
+    assertThat(manager.message(Locale.FRENCH, "unknown", "default")).isEqualTo("default");
   }
 
   @Test
-  public void shouldAcceptEmptyLabels() {
-    assertThat(manager.message(Locale.ENGLISH, "empty", "default"), Is.is(""));
-    assertThat(manager.message(Locale.FRENCH, "empty", "default"), Is.is(""));
+  public void should_accept_empty_labels() {
+    assertThat(manager.message(Locale.ENGLISH, "empty", "default")).isEqualTo("");
+    assertThat(manager.message(Locale.FRENCH, "empty", "default")).isEqualTo("");
   }
 
   @Test
   public void shouldFormatMessageWithParameters() {
-    assertThat(manager.message(Locale.ENGLISH, "with.parameters", null, "one", "two"), Is.is("First is one and second is two"));
+    assertThat(manager.message(Locale.ENGLISH, "with.parameters", null, "one", "two")).isEqualTo("First is one and second is two");
   }
 
   @Test
   public void shouldUseDefaultLocaleIfMissingValueInLocalizedBundle() {
-    assertThat(manager.message(Locale.FRENCH, "only.in.english", null), Is.is("Missing in French bundle"));
-    assertThat(manager.message(Locale.CHINA, "only.in.english", null), Is.is("Missing in French bundle"));
+    assertThat(manager.message(Locale.FRENCH, "only.in.english", null)).isEqualTo("Missing in French bundle");
+    assertThat(manager.message(Locale.CHINA, "only.in.english", null)).isEqualTo("Missing in French bundle");
   }
 
   @Test
-  public void shouldGetClassLoaderByProperty() {
-    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));
+  public void should_locate_english_file() {
+    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name", false);
+    assertThat(html).isEqualTo("This is the architecture rule");
   }
 
   @Test
-  public void shouldFindEnglishFile() {
-    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"));
+  public void should_return_null_if_file_not_found() {
+    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "checkstyle.rule1.name", false);
+    assertThat(html).isNull();
   }
 
   @Test
-  public void shouldNotFindFile() {
-    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "checkstyle.rule1.name" /* any property in the same bundle */, false);
-    assertThat(html, nullValue());
+  public void should_locate_french_file() {
+    String html = manager.messageFromFile(Locale.FRENCH, "ArchitectureRule.html", "checkstyle.rule1.name", false);
+    assertThat(html).isEqualTo("Règle d'architecture");
   }
 
   @Test
-  public void shouldFindFrenchFile() {
-    String html = manager.messageFromFile(Locale.FRENCH, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */, false);
-    assertThat(html, Is.is("Règle d'architecture"));
+  public void should_locate_file_with_missing_locale() {
+    String html = manager.messageFromFile(Locale.CHINA, "ArchitectureRule.html", "checkstyle.rule1.name", false);
+    assertThat(html).isNull();
   }
 
   @Test
-  public void shouldNotFindMissingLocale() {
-    String html = manager.messageFromFile(Locale.CHINA, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */, false);
-    assertThat(html, nullValue());
-  }
-
-  @Test
-  public void shouldNotKeepInCache() {
-    assertThat(manager.getFileContentCache().size(), Is.is(0));
+  public void should_not_keep_in_cache() {
+    assertThat(manager.getFileContentCache()).isEmpty();
     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", keepInCache);
 
-    assertThat(html, not(nullValue()));
-    assertThat(manager.getFileContentCache().size(), Is.is(0));
+    assertThat(html).isNotNull();
+    assertThat(manager.getFileContentCache()).isEmpty();
   }
 
   @Test
-  public void shouldKeepInCache() {
-    assertThat(manager.getFileContentCache().size(), Is.is(0));
+  public void should_keep_in_cache() {
+    assertThat(manager.getFileContentCache()).isEmpty();
     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", keepInCache);
+    assertThat(html).isEqualTo("This is the architecture rule");
 
-    assertThat(html, not(nullValue()));
-    Map<String, Map<Locale, String>> cache = manager.getFileContentCache();
-    assertThat(cache.size(), Is.is(1));
-    assertThat(cache.get("ArchitectureRule.html").get(Locale.ENGLISH), Is.is("This is the architecture rule"));
-  }
+    html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name", keepInCache);
+    assertThat(html).isEqualTo("This is the architecture rule");
+    assertThat(manager.getFileContentCache()).hasSize(1);
 
-  // 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");
+    html = manager.messageFromFile(Locale.FRENCH, "ArchitectureRule.html", "checkstyle.rule1.name", keepInCache);
+    assertThat(html).isEqualTo("Règle d'architecture");
+    assertThat(manager.getFileContentCache()).hasSize(1);
   }
 
-  // see SONAR-3783 => test that there will be no future regression on fallback for keys spread accross several classloaders
-  @Test
-  public void shouldFallbackOnOriginalPluginIfTranslationNotPresentInLanguagePack() {
-    // the "forge_plugin.page" has been translated in French
-    assertThat(manager.message(Locale.FRENCH, "forge_plugin.page", null)).isEqualTo("C'est mon plugin");
-    // but not the "forge_plugin.key_not_translated" key
-    assertThat(manager.message(Locale.FRENCH, "forge_plugin.key_not_translated", null)).isEqualTo("Key Not Translated");
+  static URLClassLoader newCoreClassloader() {
+    return newClassLoader("/org/sonar/core/i18n/corePlugin/");
   }
 
-  private URLClassLoader newForgeClassLoader() {
-    return newClassLoader("/org/sonar/core/i18n/forgePlugin/");
+  static URLClassLoader newCheckstyleClassloader() {
+    return newClassLoader("/org/sonar/core/i18n/checkstylePlugin/");
   }
 
-  private URLClassLoader newSqaleClassLoader() {
+  /**
+   * Example of plugin that embeds its own translations (English + French).
+   */
+  static URLClassLoader newSqaleClassloader() {
     return newClassLoader("/org/sonar/core/i18n/sqalePlugin/");
   }
 
-  private URLClassLoader newCoreClassLoader() {
-    return newClassLoader("/org/sonar/core/i18n/englishPack/", "/org/sonar/core/i18n/frenchPack/");
+  /**
+   * "Language Pack" contains various translations for different plugins.
+   */
+  static URLClassLoader newFrenchPackClassloader() {
+    return newClassLoader("/org/sonar/core/i18n/frenchPack/");
   }
 
-  private URLClassLoader newClassLoader(String... resourcePaths) {
+  private static URLClassLoader newClassLoader(String... resourcePaths) {
     URL[] urls = new URL[resourcePaths.length];
     for (int index = 0; index < resourcePaths.length; index++) {
-      urls[index] = getClass().getResource(resourcePaths[index]);
+      urls[index] = I18nManagerTest.class.getResource(resourcePaths[index]);
     }
     return new URLClassLoader(urls);
   }
+
+  private PluginMetadata newPlugin(String key) {
+    PluginMetadata plugin = mock(PluginMetadata.class);
+    when(plugin.getKey()).thenReturn(key);
+    return plugin;
+  }
 }
diff --git a/sonar-core/src/test/resources/org/sonar/core/i18n/englishPack/org/sonar/l10n/checkstyle.properties b/sonar-core/src/test/resources/org/sonar/core/i18n/checkstylePlugin/org/sonar/l10n/checkstyle.properties
index 10fa9295c44..10fa9295c44 100644
--- a/sonar-core/src/test/resources/org/sonar/core/i18n/englishPack/org/sonar/l10n/checkstyle.properties
+++ b/sonar-core/src/test/resources/org/sonar/core/i18n/checkstylePlugin/org/sonar/l10n/checkstyle.properties
diff --git a/sonar-core/src/test/resources/org/sonar/core/i18n/englishPack/org/sonar/l10n/checkstyle/ArchitectureRule.html b/sonar-core/src/test/resources/org/sonar/core/i18n/checkstylePlugin/org/sonar/l10n/checkstyle/ArchitectureRule.html
index a7cad9049d7..a7cad9049d7 100644
--- a/sonar-core/src/test/resources/org/sonar/core/i18n/englishPack/org/sonar/l10n/checkstyle/ArchitectureRule.html
+++ b/sonar-core/src/test/resources/org/sonar/core/i18n/checkstylePlugin/org/sonar/l10n/checkstyle/ArchitectureRule.html
diff --git a/sonar-core/src/test/resources/org/sonar/core/i18n/englishPack/org/sonar/l10n/core.properties b/sonar-core/src/test/resources/org/sonar/core/i18n/corePlugin/org/sonar/l10n/core.properties
index de205d651cb..de205d651cb 100644
--- a/sonar-core/src/test/resources/org/sonar/core/i18n/englishPack/org/sonar/l10n/core.properties
+++ b/sonar-core/src/test/resources/org/sonar/core/i18n/corePlugin/org/sonar/l10n/core.properties
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
deleted file mode 100644
index 4d907c9fbbb..00000000000
--- a/sonar-core/src/test/resources/org/sonar/core/i18n/forgePlugin/org/sonar/l10n/forge.properties
+++ /dev/null
@@ -1,2 +0,0 @@
-forge_plugin.page=This is my plugin
-forge_plugin.key_not_translated=Key Not Translated
\ 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
deleted file mode 100644
index 96fb089d580..00000000000
--- a/sonar-core/src/test/resources/org/sonar/core/i18n/frenchPack/org/sonar/l10n/forge_fr.properties
+++ /dev/null
@@ -1 +0,0 @@
-forge_plugin.page=C'est mon plugin
\ No newline at end of file
diff --git a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
index a969a12ef17..c1f79773e59 100644
--- a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
+++ b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
@@ -20,7 +20,7 @@
 package org.sonar.server.configuration;
 
 import org.apache.commons.lang.ObjectUtils;
-import org.codehaus.plexus.util.StringUtils;
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.database.DatabaseSession;
 import org.sonar.api.profiles.RulesProfile;
 import org.sonar.api.rules.*;