SONAR-75 Add cache of rule descriptions

5 files changed, 70 insertions, 16 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 d3de526be98..ff8d010a940 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
@@ -53,6 +53,9 @@ public class GwtI18n implements ServerComponent {
     return propertyKeys;
   }
 
+  /**
+   * Used by the JRuby on Rails application
+   */
   public String getJsDictionnary(Locale locale) {
     ResourceBundle bundle = getBundle(locale);
     return getJsDictionnary(bundle);
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 9421f31da8d..d1a98db5599 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
@@ -45,6 +45,7 @@ public class I18nManager implements I18n, ServerExtension {
   private Map<String, ClassLoader> bundleToClassloaders;
   private Map<String, String> propertyToBundles;
   private ClassLoader languagePackClassLoader;
+  private Map<String,Map<Locale,String>> fileContentCache = Maps.newHashMap();
 
   public I18nManager(PluginRepository pluginRepository) {
     this.pluginRepository = pluginRepository;
@@ -106,22 +107,35 @@ public class I18nManager implements I18n, ServerExtension {
   }
 
   /**
-   * Results are not kept in cache.
+   * Only the given locale is searched. Contrary to java.util.ResourceBundle, no strategy for locating the bundle is implemented in
+   * this method. 
    */
-  String messageFromFile(Locale locale, String filename, String relatedProperty) {
+  String messageFromFile(Locale locale, String filename, String relatedProperty, boolean keepInCache) {
+    Map<Locale,String> fileCache = fileContentCache.get(filename);
+    if (fileCache!=null && fileCache.containsKey(locale)) {
+      return fileCache.get(locale);
+    }
+
     ClassLoader classloader = getClassLoaderForProperty(relatedProperty);
     String result = null;
     if (classloader != null) {
       String bundleBase = propertyToBundles.get(relatedProperty);
       String filePath = bundleBase.replace('.', '/');
-      if (locale != Locale.ENGLISH) {
-        filePath += "_" + locale.toString();
+      if (!"en".equals(locale.getLanguage())) {
+        filePath += "_" + locale.getLanguage();
       }
       filePath += "/" + filename;
       InputStream input = classloader.getResourceAsStream(filePath);
       if (input != null) {
         try {
           result = IOUtils.toString(input, "UTF-8");
+          if (keepInCache && result!=null) {
+            if (fileCache==null) {
+              fileCache = Maps.newHashMap();
+              fileContentCache.put(filename, fileCache);
+            }
+            fileCache.put(locale, result);
+          }
         } catch (IOException e) {
           throw new SonarException("Fail to load file: " + filePath, e);
         } finally {
@@ -183,4 +197,8 @@ public class I18nManager implements I18n, ServerExtension {
   ClassLoader getLanguagePackClassLoader() {
     return languagePackClassLoader;
   }
+
+  Map<String, Map<Locale, String>> getFileContentCache() {
+    return fileContentCache;
+  }
 }
diff --git a/sonar-core/src/main/java/org/sonar/core/i18n/RuleI18nManager.java b/sonar-core/src/main/java/org/sonar/core/i18n/RuleI18nManager.java
index da1a374329a..466805f54de 100644
--- a/sonar-core/src/main/java/org/sonar/core/i18n/RuleI18nManager.java
+++ b/sonar-core/src/main/java/org/sonar/core/i18n/RuleI18nManager.java
@@ -55,10 +55,10 @@ public class RuleI18nManager implements ServerComponent {
   public String getDescription(String repositoryKey, String ruleKey, Locale locale) {
     String relatedProperty = new StringBuilder().append(RULE_PREFIX).append(repositoryKey).append(".").append(ruleKey).append(NAME_SUFFIX).toString();
 
-    // TODO add cache
-    String description = i18nManager.messageFromFile(locale, ruleKey + ".html", relatedProperty);
-    if (description == null && !Locale.ENGLISH.equals(locale)) {
-      description = i18nManager.messageFromFile(Locale.ENGLISH, ruleKey + ".html", relatedProperty);
+    Locale localeWithoutCountry = (locale.getCountry()==null ? locale : new Locale(locale.getLanguage()));
+    String description = i18nManager.messageFromFile(localeWithoutCountry, ruleKey + ".html", relatedProperty, true);
+    if (description == null && !"en".equals(localeWithoutCountry.getLanguage())) {
+      description = i18nManager.messageFromFile(Locale.ENGLISH, ruleKey + ".html", relatedProperty, true);
     }
     return description;
   }
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 b2ef460c993..3fff2cd4b14 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
@@ -20,7 +20,6 @@
 package org.sonar.core.i18n;
 
 import com.google.common.collect.Maps;
-import org.hamcrest.CoreMatchers;
 import org.hamcrest.core.Is;
 import org.junit.Before;
 import org.junit.Test;
@@ -30,6 +29,7 @@ import java.net.URLClassLoader;
 import java.util.Locale;
 import java.util.Map;
 
+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;
@@ -123,28 +123,50 @@ public class I18nManagerTest {
 
   @Test
   public void shouldFindEnglishFile() {
-    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */);
+    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"));
   }
 
   @Test
   public void shouldNotFindFile() {
-    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "checkstyle.rule1.name" /* any property in the same bundle */);
+    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "checkstyle.rule1.name" /* any property in the same bundle */, false);
     assertThat(html, nullValue());
   }
 
   @Test
   public void shouldFindFrenchFile() {
-    String html = manager.messageFromFile(Locale.FRENCH, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */);
+    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"));
   }
 
   @Test
   public void shouldNotFindMissingLocale() {
-    String html = manager.messageFromFile(Locale.CHINA, "ArchitectureRule.html", "checkstyle.rule1.name" /* any property in the same bundle */);
+    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));
+    boolean keepInCache = false;
+    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));
+  }
+
+  @Test
+  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);
+
+    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"));
+  }
+
 
   private URLClassLoader newSqaleClassLoader() {
     return newClassLoader("/org/sonar/core/i18n/sqalePlugin/");
diff --git a/sonar-core/src/test/java/org/sonar/core/i18n/RuleI18nManagerTest.java b/sonar-core/src/test/java/org/sonar/core/i18n/RuleI18nManagerTest.java
index 448b3ade323..22e797298de 100644
--- a/sonar-core/src/test/java/org/sonar/core/i18n/RuleI18nManagerTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/i18n/RuleI18nManagerTest.java
@@ -65,11 +65,22 @@ public class RuleI18nManagerTest {
     ruleI18n.getDescription("checkstyle", "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck", Locale.ENGLISH);
 
     String propertyKeyForName = "rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.name";
-    verify(i18n).messageFromFile(Locale.ENGLISH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName);
+    verify(i18n).messageFromFile(Locale.ENGLISH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName, true);
     verifyNoMoreInteractions(i18n);
   }
 
   @Test
+  public void shouldUseOnlyLanguage() {
+    I18nManager i18n = mock(I18nManager.class);
+    RuleI18nManager ruleI18n = new RuleI18nManager(i18n);
+
+    ruleI18n.getDescription("checkstyle", "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck", new Locale("fr", "BE"));
+
+    String propertyKeyForName = "rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.name";
+    verify(i18n).messageFromFile(new Locale("fr"), "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName, true);
+  }
+
+  @Test
   public void shoudlReturnNullIfMissingDescription() {
     I18nManager i18n = mock(I18nManager.class);
     RuleI18nManager ruleI18n = new RuleI18nManager(i18n);
@@ -85,8 +96,8 @@ public class RuleI18nManagerTest {
     ruleI18n.getDescription("checkstyle", "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck", Locale.FRENCH);
 
     String propertyKeyForName = "rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.name";
-    verify(i18n).messageFromFile(Locale.FRENCH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName);
-    verify(i18n).messageFromFile(Locale.ENGLISH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName);
+    verify(i18n).messageFromFile(Locale.FRENCH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName, true);
+    verify(i18n).messageFromFile(Locale.ENGLISH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName, true);
     verifyNoMoreInteractions(i18n);
   }