From 17aeea97bd1a04f84d6ed0b2f532d5597d300ca1 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 17 Aug 2011 17:53:18 +0200 Subject: [PATCH] SONAR-75 Add cache of rule descriptions --- .../java/org/sonar/core/i18n/GwtI18n.java | 3 ++ .../java/org/sonar/core/i18n/I18nManager.java | 26 ++++++++++++--- .../org/sonar/core/i18n/RuleI18nManager.java | 8 ++--- .../org/sonar/core/i18n/I18nManagerTest.java | 32 ++++++++++++++++--- .../sonar/core/i18n/RuleI18nManagerTest.java | 17 ++++++++-- 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 bundleToClassloaders; private Map propertyToBundles; private ClassLoader languagePackClassLoader; + private Map> 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 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> 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> 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,10 +65,21 @@ 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); @@ -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); } -- 2.39.5