From 3bfcfa0de67e7f00d7cd0dc74649fef7e5772298 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 4 Apr 2014 16:56:26 +0200 Subject: [PATCH] Fix some quality flaws --- .../org/sonar/batch/rule/RulesProvider.java | 4 +- .../sonar/batch/rule/RulesProviderTest.java | 2 +- .../org/sonar/core/measure/MeasureFilter.java | 7 +++- .../core/measure/MeasureFilterFactory.java | 41 +++++++++---------- .../java/org/sonar/api/batch/rule/Rule.java | 3 +- .../api/batch/rule/internal/DefaultRule.java | 8 ++-- .../api/batch/rule/internal/NewRule.java | 6 +-- .../api/server/rule/RulesDefinition.java | 19 +++++---- .../rule/RulesDefinitionAnnotationLoader.java | 24 +++++------ .../batch/rule/internal/RulesBuilderTest.java | 6 +-- .../platform/DefaultServerUpgradeStatus.java | 2 +- .../server/platform/RailsAppsDeployer.java | 12 ++++-- 12 files changed, 69 insertions(+), 65 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java b/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java index 49b4cf95986..3f1a487700d 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java +++ b/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java @@ -77,8 +77,8 @@ public class RulesProvider extends ProviderAdapter { .setName(ruleDto.getName()) .setSeverity(ruleDto.getSeverityString()) .setDescription(ruleDto.getDescription()) - .setStatus(RuleStatus.valueOf(ruleDto.getStatus())); - // TODO should we set metadata ? + .setStatus(RuleStatus.valueOf(ruleDto.getStatus())) + .setInternalKey(ruleDto.getConfigKey()); if (hasCharacteristic(ruleDto)) { newRule.setDebtSubCharacteristic(effectiveCharacteristic(ruleDto, ruleKey, debtModel).key()); diff --git a/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java b/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java index 3bf623aab5a..42ab22fed38 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java @@ -101,7 +101,7 @@ public class RulesProviderTest extends AbstractDaoTestCase { assertThat(rule.name()).isEqualTo("Avoid Null"); assertThat(rule.description()).isEqualTo("Should avoid NULL"); assertThat(rule.severity()).isEqualTo(Severity.MINOR); - assertThat(rule.metadata()).isNull(); + assertThat(rule.internalKey()).isNull(); assertThat(rule.params()).hasSize(1); RuleParam param = rule.param("myParameter"); diff --git a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java index 4bed3ca31d1..3b27ec55608 100644 --- a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java +++ b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java @@ -27,6 +27,7 @@ import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.sonar.api.measures.Metric; import org.sonar.api.resources.Qualifiers; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import java.util.Collections; @@ -141,20 +142,22 @@ public class MeasureFilter { return this; } - public MeasureFilter setFromDate(Date d) { + public MeasureFilter setFromDate(@Nullable Date d) { this.fromDate = d; return this; } - public MeasureFilter setToDate(Date d) { + public MeasureFilter setToDate(@Nullable Date d) { this.toDate = d; return this; } + @CheckForNull public Date getFromDate() { return fromDate; } + @CheckForNull public Date getToDate() { return toDate; } diff --git a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterFactory.java b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterFactory.java index 5abbe9340ed..45e78f61190 100644 --- a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterFactory.java +++ b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilterFactory.java @@ -33,8 +33,11 @@ import org.sonar.api.utils.System2; import javax.annotation.CheckForNull; import javax.annotation.Nullable; - -import java.util.*; +import java.util.Arrays; +import java.util.Calendar; +import java.util.Date; +import java.util.List; +import java.util.Map; public class MeasureFilterFactory implements ServerComponent { @@ -55,15 +58,15 @@ public class MeasureFilterFactory implements ServerComponent { if (condition != null) { filter.addCondition(condition); } - String onBaseComponents = "onBaseComponents"; - if (properties.containsKey(onBaseComponents)) { - filter.setOnBaseResourceChildren(Boolean.valueOf((String) properties.get(onBaseComponents))); + String onBaseComponents = (String) properties.get("onBaseComponents"); + if (onBaseComponents != null) { + filter.setOnBaseResourceChildren(Boolean.valueOf(onBaseComponents)); } filter.setResourceName((String) properties.get("nameSearch")); filter.setResourceKey((String) properties.get("keySearch")); - String onFavourites = "onFavourites"; - if (properties.containsKey(onFavourites)) { - filter.setUserFavourites(Boolean.valueOf((String) properties.get(onFavourites))); + String onFavourites = (String) properties.get("onFavourites"); + if (onFavourites != null) { + filter.setUserFavourites(Boolean.valueOf(onFavourites)); } fillDateConditions(filter, properties); fillSorting(filter, properties); @@ -72,23 +75,17 @@ public class MeasureFilterFactory implements ServerComponent { } private void fillDateConditions(MeasureFilter filter, Map properties) { - String fromDate = "fromDate"; - if (properties.containsKey(fromDate)) { - filter.setFromDate(toDate((String) properties.get(fromDate))); + String fromDate = (String) properties.get("fromDate"); + if (fromDate != null) { + filter.setFromDate(toDate(fromDate)); } else { - String ageMaxDays = "ageMaxDays"; - if (properties.containsKey(ageMaxDays)) { - filter.setFromDate(toDays((String) properties.get(ageMaxDays))); - } + filter.setFromDate(toDays((String) properties.get("ageMaxDays"))); } - String toDate = "toDate"; - if (properties.containsKey(toDate)) { - filter.setToDate(toDate((String) properties.get(toDate))); + String toDate = (String) properties.get("toDate"); + if (toDate != null) { + filter.setToDate(toDate(toDate)); } else { - String ageMinDays = "ageMinDays"; - if (properties.containsKey(ageMinDays)) { - filter.setToDate(toDays((String) properties.get(ageMinDays))); - } + filter.setToDate(toDays((String) properties.get("ageMinDays"))); } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rule.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rule.java index 66d42e2be5c..35653424eb8 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rule.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rule.java @@ -31,7 +31,6 @@ import java.util.Collection; /** * @since 4.2 */ -@Beta public interface Rule { RuleKey key(); @@ -42,7 +41,7 @@ public interface Rule { String description(); @CheckForNull - String metadata(); + String internalKey(); String severity(); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRule.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRule.java index 7e311e2901c..07034bbfe56 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRule.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRule.java @@ -37,7 +37,7 @@ public class DefaultRule implements Rule { private final RuleKey key; private final Integer id; - private final String name, severity, description, metadata, debtSubCharacteristic; + private final String name, severity, description, internalKey, debtSubCharacteristic; private final RuleStatus status; private final DebtRemediationFunction debtRemediationFunction; @@ -49,7 +49,7 @@ public class DefaultRule implements Rule { this.name = newRule.name; this.severity = newRule.severity; this.description = newRule.description; - this.metadata = newRule.metadata; + this.internalKey = newRule.internalKey; this.status = newRule.status; this.debtSubCharacteristic = newRule.debtSubCharacteristic; this.debtRemediationFunction = newRule.debtRemediationFunction; @@ -87,8 +87,8 @@ public class DefaultRule implements Rule { } @Override - public String metadata() { - return metadata; + public String internalKey() { + return internalKey; } @Override diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/NewRule.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/NewRule.java index 5843672ade0..213236247d2 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/NewRule.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/NewRule.java @@ -37,7 +37,7 @@ public class NewRule { final RuleKey key; Integer id; - String name, description, severity = DEFAULT_SEVERITY, metadata, debtSubCharacteristic; + String name, description, severity = DEFAULT_SEVERITY, internalKey, debtSubCharacteristic; DebtRemediationFunction debtRemediationFunction; RuleStatus status = RuleStatus.defaultStatus(); Map params = new HashMap(); @@ -71,8 +71,8 @@ public class NewRule { return this; } - public NewRule setMetadata(@Nullable String metadata) { - this.metadata = metadata; + public NewRule setInternalKey(@Nullable String s) { + this.internalKey = s; return this; } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java index d2470d6eabf..831a05f1155 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java @@ -48,11 +48,11 @@ import java.util.Set; *

*

How to use

* 
- * public class JsSquidRulesDefinition implements RulesDefinition {
+ * public class MyJsRulesDefinition implements RulesDefinition {
  *
  *   {@literal @}Override
  *   public void define(Context context) {
- *     NewRepository repository = context.createRepository("js_squid", "js").setName("Javascript Squid");
+ *     NewRepository repository = context.createRepository("my_js", "js").setName("My Javascript Analyzer");
  *
  *     // define a rule programmatically. Note that rules
  *     // could be loaded from files (JSON, XML, ...)
@@ -84,20 +84,21 @@ import java.util.Set;
  * }
  *
*

- * If rules are declared in a XML file with the standard SonarQube format, then it can be loaded by using : + * If rules are declared in a XML file with the standard SonarQube format (see + * {@link org.sonar.api.server.rule.RulesDefinitionXmlLoader}), then it can be loaded by using : *

* 

- * public class JsSquidRulesDefinition implements RulesDefinition {
+ * public class MyJsRulesDefinition implements RulesDefinition {
  *
  *   private final RulesDefinitionXmlLoader xmlLoader;
  *
- *   public JsSquidRulesDefinition(RulesDefinitionXmlLoader xmlLoader) {
+ *   public MyJsRulesDefinition(RulesDefinitionXmlLoader xmlLoader) {
  *     this.xmlLoader = xmlLoader;
  *   }
  *
  *   {@literal @}Override
  *   public void define(Context context) {
- *     NewRepository repository = context.createRepository("js_squid", "js").setName("Javascript Squid");
+ *     NewRepository repository = context.createRepository("my_js", "js").setName("My Javascript Analyzer");
  *     // see javadoc of RulesDefinitionXmlLoader for the format
  *     xmlLoader.load(repository, getClass().getResourceAsStream("/path/to/rules.xml"));
  *     repository.done();
@@ -109,19 +110,19 @@ import java.util.Set;
  * (deprecated) English bundles can be used :
  * 

  * 
- * public class JsSquidRulesDefinition implements RulesDefinition {
+ * public class MyJsRulesDefinition implements RulesDefinition {
  *
  *   private final RulesDefinitionXmlLoader xmlLoader;
  *   private final RulesDefinitionI18nLoader i18nLoader;
  *
- *   public JsSquidRulesDefinition(RulesDefinitionXmlLoader xmlLoader, RulesDefinitionI18nLoader i18nLoader) {
+ *   public MyJsRulesDefinition(RulesDefinitionXmlLoader xmlLoader, RulesDefinitionI18nLoader i18nLoader) {
  *     this.xmlLoader = xmlLoader;
  *     this.i18nLoader = i18nLoader;
  *   }
  *
  *   {@literal @}Override
  *   public void define(Context context) {
- *     NewRepository repository = context.createRepository("js_squid", "js").setName("Javascript Squid");
+ *     NewRepository repository = context.createRepository("my_js", "js").setName("My Javascript Analyzer");
  *     xmlLoader.load(repository, getClass().getResourceAsStream("/path/to/rules.xml"));
  *     i18nLoader.load(repository);
  *     repository.done();
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoader.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoader.java
index 2a2585110d7..5c455ebdbf6 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoader.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoader.java
@@ -45,6 +45,18 @@ public class RulesDefinitionAnnotationLoader {
 
   private static final Logger LOG = LoggerFactory.getLogger(RulesDefinitionAnnotationLoader.class);
 
+  private static final Function, RuleParamType> TYPE_FOR_CLASS = Functions.forMap(
+    ImmutableMap., RuleParamType>builder()
+      .put(Integer.class, RuleParamType.INTEGER)
+      .put(int.class, RuleParamType.INTEGER)
+      .put(Float.class, RuleParamType.FLOAT)
+      .put(float.class, RuleParamType.FLOAT)
+      .put(Boolean.class, RuleParamType.BOOLEAN)
+      .put(boolean.class, RuleParamType.BOOLEAN)
+      .build(),
+    RuleParamType.STRING
+  );
+
   public void load(RulesDefinition.NewExtendedRepository repo, Class... annotatedClasses) {
     for (Class annotatedClass : annotatedClasses) {
       loadRule(repo, annotatedClass);
@@ -102,18 +114,6 @@ public class RulesDefinitionAnnotationLoader {
     }
   }
 
-  private static final Function, RuleParamType> TYPE_FOR_CLASS = Functions.forMap(
-    ImmutableMap., RuleParamType>builder()
-      .put(Integer.class, RuleParamType.INTEGER)
-      .put(int.class, RuleParamType.INTEGER)
-      .put(Float.class, RuleParamType.FLOAT)
-      .put(float.class, RuleParamType.FLOAT)
-      .put(Boolean.class, RuleParamType.BOOLEAN)
-      .put(boolean.class, RuleParamType.BOOLEAN)
-      .build(),
-    RuleParamType.STRING
-  );
-
   @VisibleForTesting
   static RuleParamType guessType(Class type) {
     return TYPE_FOR_CLASS.apply(type);
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/RulesBuilderTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/RulesBuilderTest.java
index e7a445bf904..25e3649bbd1 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/RulesBuilderTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/RulesBuilderTest.java
@@ -45,7 +45,7 @@ public class RulesBuilderTest {
     NewRule newSquid1 = builder.add(RuleKey.of("squid", "S0001"));
     newSquid1.setName("Detect bug");
     newSquid1.setDescription("Detect potential bug");
-    newSquid1.setMetadata("foo=bar");
+    newSquid1.setInternalKey("foo=bar");
     newSquid1.setSeverity(Severity.CRITICAL);
     newSquid1.setStatus(RuleStatus.BETA);
     newSquid1.setDebtSubCharacteristic("COMPILER");
@@ -68,7 +68,7 @@ public class RulesBuilderTest {
     assertThat(squid1.key().rule()).isEqualTo("S0001");
     assertThat(squid1.name()).isEqualTo("Detect bug");
     assertThat(squid1.description()).isEqualTo("Detect potential bug");
-    assertThat(squid1.metadata()).isEqualTo("foo=bar");
+    assertThat(squid1.internalKey()).isEqualTo("foo=bar");
     assertThat(squid1.status()).isEqualTo(RuleStatus.BETA);
     assertThat(squid1.severity()).isEqualTo(Severity.CRITICAL);
     assertThat(squid1.debtSubCharacteristic()).isEqualTo("COMPILER");
@@ -86,7 +86,7 @@ public class RulesBuilderTest {
     assertThat(squid2.key().repository()).isEqualTo("squid");
     assertThat(squid2.key().rule()).isEqualTo("S0002");
     assertThat(squid2.description()).isNull();
-    assertThat(squid2.metadata()).isNull();
+    assertThat(squid2.internalKey()).isNull();
     assertThat(squid2.status()).isEqualTo(RuleStatus.defaultStatus());
     assertThat(squid2.severity()).isEqualTo(Severity.defaultSeverity());
     assertThat(squid2.debtSubCharacteristic()).isNull();
diff --git a/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java b/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java
index 29d421513b0..4d12df6dcf6 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java
@@ -47,7 +47,7 @@ public final class DefaultServerUpgradeStatus implements ServerUpgradeStatus, St
 
   @Override
   public void stop() {
-
+    // do nothing
   }
 
   @Override
diff --git a/sonar-server/src/main/java/org/sonar/server/platform/RailsAppsDeployer.java b/sonar-server/src/main/java/org/sonar/server/platform/RailsAppsDeployer.java
index 2ba3bd2d4c8..af91a4bad92 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/RailsAppsDeployer.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/RailsAppsDeployer.java
@@ -26,6 +26,7 @@ import org.apache.commons.lang.StringUtils;
 import org.picocontainer.Startable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.sonar.api.Plugin;
 import org.sonar.api.platform.PluginMetadata;
 import org.sonar.api.platform.PluginRepository;
 import org.sonar.api.platform.ServerFileSystem;
@@ -60,10 +61,13 @@ public class RailsAppsDeployer implements Startable {
 
     for (PluginMetadata pluginMetadata : pluginRepository.getMetadata()) {
       String pluginKey = pluginMetadata.getKey();
-      try {
-        deployRailsApp(appsDir, pluginKey, pluginRepository.getPlugin(pluginKey).getClass().getClassLoader());
-      } catch (Exception e) {
-        throw new IllegalStateException("Fail to deploy Ruby on Rails application: " + pluginKey, e);
+      Plugin plugin = pluginRepository.getPlugin(pluginKey);
+      if (plugin != null) {
+        try {
+          deployRailsApp(appsDir, pluginKey, plugin.getClass().getClassLoader());
+        } catch (Exception e) {
+          throw new IllegalStateException("Fail to deploy Ruby on Rails application: " + pluginKey, e);
+        }
       }
     }
   }
-- 
2.39.5