SONAR-75 Improve i18n API

- The extension point LanguagePack is not required anymore
- No error logs when rule description is not available in all locales
- Increase code coverage and decrease complexity

8 files changed, 148 insertions, 124 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
index 7b5ef8fac7f..6835c7f69dd 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
@@ -41,6 +41,8 @@ import org.sonar.core.components.DefaultMetricFinder;
 import org.sonar.core.components.DefaultModelFinder;
 import org.sonar.core.components.DefaultRuleFinder;
 import org.sonar.core.components.DefaultUserFinder;
+import org.sonar.core.i18n.I18nManager;
+import org.sonar.core.i18n.RuleI18nManager;
 import org.sonar.core.notifications.DefaultNotificationManager;
 import org.sonar.jpa.dao.DaoFacade;
 import org.sonar.jpa.dao.MeasuresDao;
@@ -187,6 +189,8 @@ public final class Platform {
     servicesContainer.as(Characteristics.CACHE).addComponent(RulesConsole.class);
     servicesContainer.as(Characteristics.CACHE).addComponent(JRubyI18n.class);
     servicesContainer.as(Characteristics.CACHE).addComponent(DefaultUserFinder.class);
+    servicesContainer.as(Characteristics.CACHE).addComponent(I18nManager.class);
+    servicesContainer.as(Characteristics.CACHE).addComponent(RuleI18nManager.class);
 
     // Notifications
     servicesContainer.as(Characteristics.CACHE).addComponent(NotificationService.class);
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 473bc679f65..e0ea1b1004f 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
@@ -302,13 +302,34 @@ public final class JRubyFacade {
     return component;
   }
 
-  public String getI18nMessage(String rubyLocale, String key, String defaultValue, Object... parameters) {
+  public String getMessage(String rubyLocale, String key, String defaultValue, Object... parameters) {
     if (i18n == null) {
       i18n = getContainer().getComponent(JRubyI18n.class);
     }
     return i18n.message(rubyLocale, key, defaultValue, parameters);
   }
 
+  public String getRuleName(String rubyLocale, String repositoryKey, String key) {
+    if (i18n == null) {
+      i18n = getContainer().getComponent(JRubyI18n.class);
+    }
+    return i18n.getRuleName(rubyLocale, repositoryKey, key);
+  }
+
+  public String getRuleDescription(String rubyLocale, String repositoryKey, String key) {
+    if (i18n == null) {
+      i18n = getContainer().getComponent(JRubyI18n.class);
+    }
+    return i18n.getRuleDescription(rubyLocale, repositoryKey, key);
+  }
+
+  public String getRuleParamDescription(String rubyLocale, String repositoryKey, String key, String paramKey) {
+    if (i18n == null) {
+      i18n = getContainer().getComponent(JRubyI18n.class);
+    }
+    return i18n.getRuleParamDescription(rubyLocale, repositoryKey, key, paramKey);
+  }
+
   public ReviewsNotificationManager getReviewsNotificationManager() {
     return getContainer().getComponent(ReviewsNotificationManager.class);
   }
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 72a1d0e39a0..e7d9804e3f3 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
@@ -23,6 +23,7 @@ import com.google.common.collect.Maps;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.i18n.I18n;
+import org.sonar.core.i18n.RuleI18nManager;
 
 import java.util.Locale;
 import java.util.Map;
@@ -33,10 +34,12 @@ import java.util.Map;
 public final class JRubyI18n implements ServerComponent {
 
   private I18n i18n;
-  private Map<String,Locale> localesByRubyKey = Maps.newHashMap();
+  private Map<String, Locale> localesByRubyKey = Maps.newHashMap();
+  private RuleI18nManager ruleI18nManager;
 
-  public JRubyI18n(I18n i18n) {
+  public JRubyI18n(I18n i18n, RuleI18nManager ruleI18nManager) {
     this.i18n = i18n;
+    this.ruleI18nManager = ruleI18nManager;
   }
 
   Locale getLocale(String rubyKey) {
@@ -55,7 +58,7 @@ public final class JRubyI18n implements ServerComponent {
   static Locale toLocale(String rubyKey) {
     Locale locale;
     String[] fields = StringUtils.split(rubyKey, "-");
-    if (fields.length==1) {
+    if (fields.length == 1) {
       locale = new Locale(fields[0]);
     } else {
       locale = new Locale(fields[0], fields[1]);
@@ -66,4 +69,16 @@ public final class JRubyI18n implements ServerComponent {
   public String message(String rubyLocale, String key, String defaultValue, Object... parameters) {
     return i18n.message(getLocale(rubyLocale), key, defaultValue, parameters);
   }
+
+  public String getRuleName(String rubyLocale, String repositoryKey, String key) {
+    return ruleI18nManager.getName(repositoryKey, key, toLocale(rubyLocale));
+  }
+
+  public String getRuleDescription(String rubyLocale, String repositoryKey, String key) {
+    return ruleI18nManager.getDescription(repositoryKey, key, toLocale(rubyLocale));
+  }
+
+  public String getRuleParamDescription(String rubyLocale, String repositoryKey, String ruleKey, String paramKey) {
+    return ruleI18nManager.getParamDescription(repositoryKey, ruleKey, paramKey, toLocale(rubyLocale));
+  }
 }
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
index 0c06549b0fd..9e8ff5e686b 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
@@ -97,12 +97,7 @@ class ApplicationController < ActionController::Base
   
   # i18n
   def message(key, options={})
-    default = options[:default]
-    params = options[:params]
-    if params.nil? 
-      params=[]
-    end
-    Java::OrgSonarServerUi::JRubyFacade.getInstance().getI18nMessage(I18n.locale, key, default, params.to_java)    
+    Api::Utils.message(key, options)
   end
   
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb
index 7ff974147af..8f5d33436a6 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb
@@ -67,6 +67,6 @@ class Api::Utils
     if params.nil?
       params=[]
     end
-    Java::OrgSonarServerUi::JRubyFacade.getInstance().getI18nMessage(I18n.locale, key, default, params.to_java)
+    Java::OrgSonarServerUi::JRubyFacade.getInstance().getMessage(I18n.locale, key, default, params.to_java)
   end
 end
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 0fbb2e78823..6724dcbeac8 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
@@ -26,6 +26,10 @@ class Rule < ActiveRecord::Base
   has_many :active_rules
   belongs_to :parent, :class_name => 'Rule', :foreign_key => 'parent_id'
 
+  def repository_key
+    plugin_name
+  end
+
   def parameters
     rules_parameters
   end
@@ -57,41 +61,31 @@ class Rule < ActiveRecord::Base
   def <=>(rule)
     name<=>rule.name
   end
-  
-  def name(translate=true)
-    default_string = read_attribute(:name)
-    return default_string unless translate
-    
-    rule_plugin_name = read_attribute(:plugin_name)
-    rule_plugin_rule_key = read_attribute(:plugin_rule_key)
-
-    return nil if (rule_plugin_name.nil? or rule_plugin_rule_key.nil?)
-    
-    i18n_key = 'rule.' + rule_plugin_name + '.' + rule_plugin_rule_key + '.name'   
-    result = Api::Utils.message(i18n_key, :default => default_string)
-    result
+
+  def name
+    @l10n_name ||=
+        begin
+          result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleName(I18n.locale, repository_key, plugin_rule_key)
+          result = read_attribute(:name) unless result
+          result
+        end
   end
-  
+
   def name=(value)
-    write_attribute(:name, value)    
+    write_attribute(:name, value)
   end
-  
-  def description(translate=true)
-    default_string = read_attribute(:description)
-    return default_string unless translate
-    
-    rule_plugin_name = read_attribute(:plugin_name)
-    rule_plugin_rule_key = read_attribute(:plugin_rule_key)
-    
-    return nil if (rule_plugin_name.nil? or rule_plugin_rule_key.nil?)
-    
-    i18n_key = 'rule.' + rule_plugin_name + '.' + rule_plugin_rule_key + '.description'   
-    result = Api::Utils.message(i18n_key, :default => default_string)
-    result
+
+  def description
+    @l10n_description ||=
+        begin
+          result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleDescription(I18n.locale, repository_key, plugin_rule_key)
+          result = read_attribute(:description) unless result
+          result
+        end
   end
 
   def description=(value)
-    write_attribute(:description, value)    
+    write_attribute(:description, value)
   end
 
   def config_key
@@ -142,7 +136,7 @@ class Rule < ActiveRecord::Base
     else
       json['priority'] = priority_text
     end
-    json['params'] = parameters.collect{|parameter| parameter.to_hash_json(active_rule)} unless parameters.empty?
+    json['params'] = parameters.collect { |parameter| parameter.to_hash_json(active_rule) } unless parameters.empty?
     json
   end
 
@@ -152,7 +146,7 @@ class Rule < ActiveRecord::Base
       xml.key(key)
       xml.config_key(config_key)
       xml.plugin(plugin_name)
-      xml.description {xml.cdata!(description)}
+      xml.description { xml.cdata!(description) }
       active_rule = nil
       if profile
         active_rule = profile.active_by_rule_id(id)
@@ -197,10 +191,10 @@ class Rule < ActiveRecord::Base
     if remove_blank(options[:plugins])
       plugins = options[:plugins]
       unless options[:language].blank?
-        plugins = plugins & java_facade.getRuleRepositoriesByLanguage(options[:language]).collect{ |repo| repo.getKey() }
+        plugins = plugins & java_facade.getRuleRepositoriesByLanguage(options[:language]).collect { |repo| repo.getKey() }
       end
     elsif !options[:language].blank?
-      plugins = java_facade.getRuleRepositoriesByLanguage(options[:language]).collect{ |repo| repo.getKey() }
+      plugins = java_facade.getRuleRepositoriesByLanguage(options[:language]).collect { |repo| repo.getKey() }
     end
 
     if plugins
@@ -220,8 +214,7 @@ class Rule < ActiveRecord::Base
     end
 
     includes=(options[:include_parameters] ? :rules_parameters : nil)
-    rules = Rule.find(:all, :order => 'rules.name', :include => includes,
-      :conditions => [conditions.join(" AND "), values])
+    rules = Rule.find(:all, :include => includes, :conditions => [conditions.join(" AND "), values]).sort_by { |rule| rule.name }
 
     filter(rules, options)
   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 efe684b8c58..360b4f988af 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
@@ -1,22 +1,22 @@
- #
- # Sonar, entreprise quality control tool.
- # Copyright (C) 2008-2011 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 {library}; if not, write to the Free Software
- # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
- #
+#
+# Sonar, entreprise quality control tool.
+# Copyright (C) 2008-2011 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 {library}; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+#
 class RulesParameter < ActiveRecord::Base
 
   validates_presence_of :name, :param_type
@@ -29,35 +29,30 @@ class RulesParameter < ActiveRecord::Base
   PARAM_TYPE_INTEGER_LIST = "i{}";
   PARAM_TYPE_BOOLEAN = "b";
   PARAM_TYPE_REGEXP = "r";
-	
+
   belongs_to :rule
 
   def is_set_type
-  	return param_type.at(1) == "[" && param_type.ends_with?( "]" )
+    return param_type.at(1) == "[" && param_type.ends_with?("]")
   end
-  
+
   def get_allowed_tokens
-	  return param_type[2,param_type.length-3].split( "," )
+    return param_type[2, param_type.length-3].split(",")
   end
 
-  def description(translate=true)
-    default_string = read_attribute(:description)
-    return default_string unless translate
-    
-    rule_plugin_name = rule.plugin_name
-    rule_plugin_rule_key = rule.plugin_rule_key
-    
-    return nil if (rule_plugin_name.nil? or rule_plugin_rule_key.nil?)
-    
-    i18n_key = 'rule.' + rule_plugin_name + '.' + rule_plugin_rule_key + '.param.' + read_attribute(:name)
-    result = Api::Utils.message(i18n_key, :default => default_string)
-    result
+  def description
+    @l10n_description ||=
+        begin
+          result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleParamDescription(I18n.locale, rule.repository_key, rule.plugin_rule_key, name())
+          result = read_attribute(:description) unless result
+          result
+        end
   end
-  
+
   def description=(value)
-    write_attribute(:description, value)    
+    write_attribute(:description, value)
   end
-  
+
   def readable_param_type
     return "String" if param_type == PARAM_TYPE_STRING
     return "Set of string (, as delimiter)" if param_type == PARAM_TYPE_STRING_LIST
@@ -67,55 +62,55 @@ class RulesParameter < ActiveRecord::Base
     return "Regular expression" if param_type == PARAM_TYPE_REGEXP
     return "Set of values (, as delimiter)" if is_set_type
   end
-  
+
   def input_box_size
     return 15 if param_type == PARAM_TYPE_STRING or param_type == PARAM_TYPE_STRING_LIST or param_type == PARAM_TYPE_REGEXP
     return 8 if param_type == PARAM_TYPE_INTEGER or param_type == PARAM_TYPE_INTEGER_LIST
     return 4 if param_type == PARAM_TYPE_BOOLEAN
     if is_set_type
-      size = ( param_type.length / 2 ).to_i
+      size = (param_type.length / 2).to_i
       size = 64 if size > 64
       return size
     end
   end
 
   def validate_value(attribute, errors, value)
-   return if attribute.nil? or attribute.length == 0
-   if is_set_type
-     provided_tokens = attribute.split( "," )
-     allowed_tokens = get_allowed_tokens
-     provided_tokens.each do |provided_token|
-       if !allowed_tokens.include?(provided_token)
-         errors.add( "#{value}", "Invalid value '" + provided_token + "'. Must be one of : " + allowed_tokens.join(", ") )
-       end
-     end
-   elsif param_type == RulesParameter::PARAM_TYPE_INTEGER
-     begin
-       Kernel.Integer(attribute)
-     rescue
-       errors.add( "#{value}", "Invalid value '" + attribute + "'. Must be an integer." )
-     end
-   elsif param_type == RulesParameter::PARAM_TYPE_INTEGER_LIST
-     provided_numbers = attribute.split( "," )
-     provided_numbers.each do |provided_number|
-       begin
-         Kernel.Integer(provided_number)
-       rescue
-         errors.add("#{value}", "Invalid value '" + provided_number + "'. Must be an integer." )
-         return
-       end
-     end
-   elsif param_type == RulesParameter::PARAM_TYPE_BOOLEAN
-     if attribute != "true" && attribute != "false"
-       errors.add( "#{value}", "Invalid value '" + attribute + "'. Must be one of : true,false" )
-     end
-   elsif param_type == RulesParameter::PARAM_TYPE_REGEXP
-     begin
-       Regexp.new(attribute)
-     rescue
-       errors.add( "#{value}", "Invalid regular expression '" + attribute + "'.")
-     end
-   end
+    return if attribute.nil? or attribute.length == 0
+    if is_set_type
+      provided_tokens = attribute.split(",")
+      allowed_tokens = get_allowed_tokens
+      provided_tokens.each do |provided_token|
+        if !allowed_tokens.include?(provided_token)
+          errors.add("#{value}", "Invalid value '" + provided_token + "'. Must be one of : " + allowed_tokens.join(", "))
+        end
+      end
+    elsif param_type == RulesParameter::PARAM_TYPE_INTEGER
+      begin
+        Kernel.Integer(attribute)
+      rescue
+        errors.add("#{value}", "Invalid value '" + attribute + "'. Must be an integer.")
+      end
+    elsif param_type == RulesParameter::PARAM_TYPE_INTEGER_LIST
+      provided_numbers = attribute.split(",")
+      provided_numbers.each do |provided_number|
+        begin
+          Kernel.Integer(provided_number)
+        rescue
+          errors.add("#{value}", "Invalid value '" + provided_number + "'. Must be an integer.")
+          return
+        end
+      end
+    elsif param_type == RulesParameter::PARAM_TYPE_BOOLEAN
+      if attribute != "true" && attribute != "false"
+        errors.add("#{value}", "Invalid value '" + attribute + "'. Must be one of : true,false")
+      end
+    elsif param_type == RulesParameter::PARAM_TYPE_REGEXP
+      begin
+        Regexp.new(attribute)
+      rescue
+        errors.add("#{value}", "Invalid regular expression '" + attribute + "'.")
+      end
+    end
   end
 
   def to_hash_json(active_rule)
@@ -130,7 +125,7 @@ class RulesParameter < ActiveRecord::Base
   def to_xml(active_rule, xml)
     xml.param do
       xml.name(name)
-      xml.description {xml.cdata!(description)}
+      xml.description { xml.cdata!(description) }
       if active_rule
         active_parameter = active_rule.active_param_by_param_id(id)
         xml.value(active_parameter.value) if active_parameter
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 fe150e2b754..c03934a3940 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,6 +22,7 @@ package org.sonar.server.ui;
 import org.hamcrest.core.Is;
 import org.junit.Test;
 import org.sonar.api.i18n.I18n;
+import org.sonar.core.i18n.RuleI18nManager;
 
 import java.util.Locale;
 
@@ -39,7 +40,7 @@ public class JRubyI18nTest {
 
   @Test
   public void shouldCacheLocales() {
-    JRubyI18n i18n = new JRubyI18n(mock(I18n.class));
+    JRubyI18n i18n = new JRubyI18n(mock(I18n.class), mock(RuleI18nManager.class));
     assertThat(i18n.getLocalesByRubyKey().size(), Is.is(0));
 
     i18n.getLocale("fr");