SONAR-4688 remove loading of rule bundles

11 files changed, 33 insertions, 119 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 f69c1d59b5a..8ae6feeea41 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,6 +22,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.picocontainer.Startable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.BatchExtension;
@@ -44,7 +45,7 @@ import java.util.MissingResourceException;
 import java.util.ResourceBundle;
 import java.util.Set;
 
-public class I18nManager implements I18n, ServerExtension, BatchExtension {
+public class I18nManager implements I18n, ServerExtension, BatchExtension, Startable {
   private static final Logger LOG = LoggerFactory.getLogger(I18nManager.class);
 
   public static final String BUNDLE_PACKAGE = "org.sonar.l10n.";
@@ -52,7 +53,6 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
   private PluginRepository pluginRepository;
   private I18nClassloader i18nClassloader;
   private Map<String, String> propertyToBundles;
-  private Map<String, Map<Locale, String>> fileContentCache = Maps.newHashMap();
   private final ResourceBundle.Control control;
 
   public I18nManager(PluginRepository pluginRepository) {
@@ -70,6 +70,7 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
     };
   }
 
+  @Override
   public void start() {
     doStart(new I18nClassloader(pluginRepository));
   }
@@ -94,10 +95,10 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
     LOG.debug(String.format("Loaded %d properties from l10n bundles", propertyToBundles.size()));
   }
 
+  @Override
   public void stop() {
     i18nClassloader = null;
     propertyToBundles = null;
-    fileContentCache = null;
   }
 
   @CheckForNull
@@ -122,12 +123,7 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
    * 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, boolean keepInCache) {
-    Map<Locale, String> fileCache = fileContentCache.get(filename);
-    if (fileCache != null && fileCache.containsKey(locale)) {
-      return fileCache.get(locale);
-    }
-
+  String messageFromFile(Locale locale, String filename, String relatedProperty) {
     String result = null;
     String bundleBase = propertyToBundles.get(relatedProperty);
     if (bundleBase == null) {
@@ -144,15 +140,6 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
     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);
-    }
     return result;
   }
 
@@ -184,8 +171,4 @@ public class I18nManager implements I18n, ServerExtension, BatchExtension {
   ClassLoader getBundleClassLoader() {
     return i18nClassloader;
   }
-
-  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 6b732add0fe..5c0a63317a1 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
@@ -100,7 +100,7 @@ public class RuleI18nManager implements RuleI18n, ServerExtension, BatchExtensio
     String relatedProperty = new StringBuilder().append(RULE_PREFIX).append(repositoryKey).append(".").append(ruleKey).append(NAME_SUFFIX).toString();
 
     String ruleDescriptionFilePath = "rules/" + repositoryKey + "/" + ruleKey + ".html";
-    String description = i18nManager.messageFromFile(Locale.ENGLISH, ruleDescriptionFilePath, relatedProperty, true);
+    String description = i18nManager.messageFromFile(Locale.ENGLISH, ruleDescriptionFilePath, relatedProperty);
     if (description == null) {
       // Following line is to ensure backward compatibility (SONAR-3319)
       description = lookUpDescriptionInFormerLocation(ruleKey, relatedProperty);
@@ -115,7 +115,7 @@ public class RuleI18nManager implements RuleI18n, ServerExtension, BatchExtensio
    * See http://jira.codehaus.org/browse/SONAR-3319
    */
   private String lookUpDescriptionInFormerLocation(String ruleKey, String relatedProperty) {
-    return i18nManager.messageFromFile(Locale.ENGLISH, ruleKey + ".html", relatedProperty, true);
+    return i18nManager.messageFromFile(Locale.ENGLISH, ruleKey + ".html", relatedProperty);
   }
 
   @CheckForNull
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 efed9596423..49558738dff 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
@@ -44,7 +44,7 @@ public class I18nManagerTest {
     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[] {
+    I18nClassloader i18nClassloader = new I18nClassloader(new ClassLoader[]{
       newCoreClassloader(), newFrenchPackClassloader(), newSqaleClassloader(), newCheckstyleClassloader()
     });
     manager = new I18nManager(pluginRepository);
@@ -131,60 +131,34 @@ public class I18nManagerTest {
 
   @Test
   public void should_locate_english_file() {
-    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name", false);
+    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name");
     assertThat(html).isEqualTo("This is the architecture rule");
   }
 
   @Test
   public void should_return_null_if_file_not_found() {
-    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "checkstyle.rule1.name", false);
+    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "checkstyle.rule1.name");
     assertThat(html).isNull();
   }
 
   @Test
   public void should_return_null_if_rule_not_internationalized() {
-    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "foo.rule1.name", false);
+    String html = manager.messageFromFile(Locale.ENGLISH, "UnknownRule.html", "foo.rule1.name");
     assertThat(html).isNull();
   }
 
   @Test
   public void should_locate_french_file() {
-    String html = manager.messageFromFile(Locale.FRENCH, "ArchitectureRule.html", "checkstyle.rule1.name", false);
+    String html = manager.messageFromFile(Locale.FRENCH, "ArchitectureRule.html", "checkstyle.rule1.name");
     assertThat(html).isEqualTo("Règle d'architecture");
   }
 
   @Test
   public void should_locate_file_with_missing_locale() {
-    String html = manager.messageFromFile(Locale.CHINA, "ArchitectureRule.html", "checkstyle.rule1.name", false);
+    String html = manager.messageFromFile(Locale.CHINA, "ArchitectureRule.html", "checkstyle.rule1.name");
     assertThat(html).isNull();
   }
 
-  @Test
-  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", keepInCache);
-
-    assertThat(html).isNotNull();
-    assertThat(manager.getFileContentCache()).isEmpty();
-  }
-
-  @Test
-  public void should_keep_in_cache() {
-    assertThat(manager.getFileContentCache()).isEmpty();
-    boolean keepInCache = true;
-    String html = manager.messageFromFile(Locale.ENGLISH, "ArchitectureRule.html", "checkstyle.rule1.name", keepInCache);
-    assertThat(html).isEqualTo("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);
-
-    html = manager.messageFromFile(Locale.FRENCH, "ArchitectureRule.html", "checkstyle.rule1.name", keepInCache);
-    assertThat(html).isEqualTo("Règle d'architecture");
-    assertThat(manager.getFileContentCache()).hasSize(1);
-  }
-
   static URLClassLoader newCoreClassloader() {
     return newClassLoader("/org/sonar/core/i18n/corePlugin/");
   }
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 6ecdb1078f1..dd8fa6ee566 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
@@ -69,13 +69,13 @@ public class RuleI18nManagerTest {
     String propertyKeyForName = "rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.name";
 
     I18nManager i18n = mock(I18nManager.class);
-    when(i18n.messageFromFile(Locale.ENGLISH, "rules/checkstyle/com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName, true)).thenReturn("Description");
+    when(i18n.messageFromFile(Locale.ENGLISH, "rules/checkstyle/com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName)).thenReturn("Description");
 
     RuleI18nManager ruleI18n = new RuleI18nManager(i18n);
     String description = ruleI18n.getDescription("checkstyle", "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck", Locale.ENGLISH);
     assertThat(description, is("Description"));
 
-    verify(i18n).messageFromFile(Locale.ENGLISH, "rules/checkstyle/com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName, true);
+    verify(i18n).messageFromFile(Locale.ENGLISH, "rules/checkstyle/com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName);
     verifyNoMoreInteractions(i18n);
   }
 
@@ -86,14 +86,14 @@ public class RuleI18nManagerTest {
 
     I18nManager i18n = mock(I18nManager.class);
     // this is the "old" way of storing HTML description files for rules (they are not in the "rules/<repo-key>" folder)
-    when(i18n.messageFromFile(Locale.ENGLISH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName, true)).thenReturn("Description");
+    when(i18n.messageFromFile(Locale.ENGLISH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName)).thenReturn("Description");
 
     RuleI18nManager ruleI18n = new RuleI18nManager(i18n);
     String description = ruleI18n.getDescription("checkstyle", "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck", Locale.ENGLISH);
     assertThat(description, is("Description"));
 
-    verify(i18n).messageFromFile(Locale.ENGLISH, "rules/checkstyle/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);
+    verify(i18n).messageFromFile(Locale.ENGLISH, "rules/checkstyle/com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName);
+    verify(i18n).messageFromFile(Locale.ENGLISH, "com.puppycrawl.tools.checkstyle.checks.annotation.AnnotationUseStyleCheck.html", propertyKeyForName);
     verifyNoMoreInteractions(i18n);
   }
 
diff --git a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
index 3951fd33f2f..103dec7edc8 100644
--- a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
+++ b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
@@ -161,7 +161,7 @@ public final class RegisterRules {
 
   private void validateRuleRepositoryName(Rule rule, String repositoryKey) {
     String nameFromBundle = ruleI18nManager.getName(repositoryKey, rule.getKey());
-    if(! Strings.isNullOrEmpty(nameFromBundle)) {
+    if (!Strings.isNullOrEmpty(nameFromBundle)) {
       rule.setName(nameFromBundle);
     }
     if (Strings.isNullOrEmpty(rule.getName())) {
@@ -171,7 +171,7 @@ public final class RegisterRules {
 
   private void validateRuleDescription(Rule rule, String repositoryKey) {
     String descriptionFromBundle = ruleI18nManager.getDescription(repositoryKey, rule.getKey());
-    if(! Strings.isNullOrEmpty(descriptionFromBundle)) {
+    if (!Strings.isNullOrEmpty(descriptionFromBundle)) {
       rule.setDescription(descriptionFromBundle);
     }
     if (Strings.isNullOrEmpty(rule.getDescription())) {
@@ -211,7 +211,11 @@ public final class RegisterRules {
         if (persistedParam == null) {
           persistedParam = persistedRule.createParameter(param.getKey());
         }
-        persistedParam.setDescription(param.getDescription());
+        String desc = StringUtils.defaultIfEmpty(
+          ruleI18nManager.getParamDescription(rule.getRepositoryKey(), rule.getKey(), param.getKey()),
+          param.getDescription()
+        );
+        persistedParam.setDescription(desc);
         persistedParam.setType(param.getType());
         persistedParam.setDefaultValue(param.getDefaultValue());
       }
diff --git a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java
index 26b17f67dc8..002d62c8b31 100644
--- a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java
+++ b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java
@@ -406,18 +406,6 @@ public final class JRubyFacade {
     return getJRubyI18n().message(rubyLocale, key, defaultValue, parameters);
   }
 
-  public String getRuleName(String repositoryKey, String key) {
-    return getJRubyI18n().getRuleName(repositoryKey, key);
-  }
-
-  public String getRuleDescription(String repositoryKey, String key) {
-    return getJRubyI18n().getRuleDescription(repositoryKey, key);
-  }
-
-  public String getRuleParamDescription(String repositoryKey, String key, String paramKey) {
-    return getJRubyI18n().getRuleParamDescription(repositoryKey, key, paramKey);
-  }
-
   public String getJsL10nDictionnary(String rubyLocale) {
     return getJRubyI18n().getJsDictionnary(rubyLocale);
   }
diff --git a/sonar-server/src/main/java/org/sonar/server/ui/JRubyI18n.java b/sonar-server/src/main/java/org/sonar/server/ui/JRubyI18n.java
index bf992bc6617..63bb8b30309 100644
--- a/sonar-server/src/main/java/org/sonar/server/ui/JRubyI18n.java
+++ b/sonar-server/src/main/java/org/sonar/server/ui/JRubyI18n.java
@@ -38,12 +38,10 @@ public class JRubyI18n implements ServerComponent {
 
   private I18n i18n;
   private Map<String, Locale> localesByRubyKey = Maps.newHashMap();
-  private RuleI18nManager ruleI18nManager;
   private GwtI18n gwtI18n;
 
-  public JRubyI18n(I18n i18n, RuleI18nManager ruleI18nManager, GwtI18n gwtI18n) {
+  public JRubyI18n(I18n i18n, GwtI18n gwtI18n) {
     this.i18n = i18n;
-    this.ruleI18nManager = ruleI18nManager;
     this.gwtI18n = gwtI18n;
   }
 
@@ -79,18 +77,6 @@ public class JRubyI18n implements ServerComponent {
     return StringUtils.defaultString(i18n.message(getLocale(rubyLocale), key, defaultValue, parameters), key);
   }
 
-  public String getRuleName(String repositoryKey, String key) {
-    return ruleI18nManager.getName(repositoryKey, key);
-  }
-
-  public String getRuleDescription(String repositoryKey, String key) {
-    return ruleI18nManager.getDescription(repositoryKey, key);
-  }
-
-  public String getRuleParamDescription(String repositoryKey, String ruleKey, String paramKey) {
-    return ruleI18nManager.getParamDescription(repositoryKey, ruleKey, paramKey);
-  }
-
   public String getJsDictionnary(String rubyLocale) {
     return gwtI18n.getJsDictionnary(toLocale(rubyLocale));
   }
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb
index f98251b994d..fb3f5ab8baa 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb
@@ -114,13 +114,9 @@ class Rule < ActiveRecord::Base
   def name(unused_deprecated_l10n=true)
     @raw_name ||=
       begin
-        result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleName(repository_key, plugin_rule_key)
-        # if no name present in the bundle, try to find it in the DB
-        result = read_attribute(:name) unless result
         # SONAR-4583
         # name should return an empty string instead of nil
-        result = '' unless result
-        result
+        read_attribute(:name) || ''
       end
   end
 
@@ -131,12 +127,9 @@ class Rule < ActiveRecord::Base
   def description
     @raw_description ||=
       begin
-        result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleDescription(repository_key, plugin_rule_key)
-        result = read_attribute(:description) unless result
         # SONAR-4583
         # description should return an empty string instead of nil
-        result = '' unless result
-        result
+        read_attribute(:description) || ''
       end
   end
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rules_parameter.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rules_parameter.rb
index 319fe18af09..f8c539a7002 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/rules_parameter.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rules_parameter.rb
@@ -33,19 +33,6 @@ class RulesParameter < ActiveRecord::Base
     param_type[2, param_type.length-3].split(",")
   end
 
-  def description
-    @raw_description ||=
-      begin
-        result = Java::OrgSonarServerUi::JRubyFacade.instance.getRuleParamDescription(rule.repository_key, rule.plugin_rule_key, name)
-        result = read_attribute(:description) unless result
-        result
-      end
-  end
-
-  def description=(value)
-    write_attribute(:description, value)
-  end
-
   def validate_value(attribute, errors, value)
     validate_rule_param(attribute, param_type, errors, value)
   end
diff --git a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
index a0d87abe5aa..673795d610f 100644
--- a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
@@ -43,10 +43,10 @@ import static org.mockito.Mockito.when;
 
 public class RegisterRulesTest extends AbstractDbUnitTestCase {
 
-  private RegisterRules task;
-  private ProfilesManager profilesManager;
-  private RuleRegistry ruleRegistry;
-  private RuleI18nManager ruleI18nManager;
+  RegisterRules task;
+  ProfilesManager profilesManager;
+  RuleRegistry ruleRegistry;
+  RuleI18nManager ruleI18nManager;
 
   @Before
   public void init() {
diff --git a/sonar-server/src/test/java/org/sonar/server/ui/JRubyI18nTest.java b/sonar-server/src/test/java/org/sonar/server/ui/JRubyI18nTest.java
index 160cdb03b00..094490dcdd3 100644
--- a/sonar-server/src/test/java/org/sonar/server/ui/JRubyI18nTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/ui/JRubyI18nTest.java
@@ -22,7 +22,6 @@ package org.sonar.server.ui;
 import org.junit.Test;
 import org.sonar.api.i18n.I18n;
 import org.sonar.core.i18n.GwtI18n;
-import org.sonar.core.i18n.RuleI18nManager;
 
 import java.util.Locale;
 
@@ -38,7 +37,7 @@ public class JRubyI18nTest {
 
   @Test
   public void shouldCacheLocales() {
-    JRubyI18n i18n = new JRubyI18n(mock(I18n.class), mock(RuleI18nManager.class), mock(GwtI18n.class));
+    JRubyI18n i18n = new JRubyI18n(mock(I18n.class), mock(GwtI18n.class));
     assertThat(i18n.getLocalesByRubyKey()).isEmpty();
 
     i18n.getLocale("fr");