SONAR-5007 fix some quality flaws

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java
index 4995e17ec6effab8a34a21600d9ede9f2c2e47c3..ee8eadc2947b5b3e53e886f83079215fd98db829 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java
@@ -93,10 +93,10 @@ public class RegisterQualityProfiles implements ServerComponent {
     try {
       ListMultimap<String, RulesProfile> profilesByLanguage = profilesByLanguage();
       for (String language : profilesByLanguage.keySet()) {
-        List<RulesProfile> profiles = profilesByLanguage.get(language);
-        verifyLanguage(language, profiles);
+        List<RulesProfile> profileDefs = profilesByLanguage.get(language);
+        verifyLanguage(language, profileDefs);
 
-        for (Map.Entry<String, Collection<RulesProfile>> entry : profilesByName(profiles).entrySet()) {
+        for (Map.Entry<String, Collection<RulesProfile>> entry : profilesByName(profileDefs).entrySet()) {
           String profileName = entry.getKey();
           QualityProfileKey profileKey = QualityProfileKey.of(profileName, language);
           if (shouldRegister(profileKey, session)) {
@@ -104,7 +104,7 @@ public class RegisterQualityProfiles implements ServerComponent {
           }
           defaultProfilesCache.put(language, profileName);
         }
-        setDefault(language, profiles, session);
+        setDefault(language, profileDefs, session);
       }
       session.commit();
     } finally {
@@ -161,21 +161,21 @@ public class RegisterQualityProfiles implements ServerComponent {
     session.commit();
   }
 
-  private void setDefault(String language, List<RulesProfile> profiles, DbSession session) {
+  private void setDefault(String language, List<RulesProfile> profileDefs, DbSession session) {
     String propertyKey = "sonar.profile." + language;
 
     boolean upToDate = false;
     String currentDefault = settings.getString(propertyKey);
     if (currentDefault != null) {
-      for (RulesProfile profile : profiles) {
-        if (StringUtils.equals(currentDefault, profile.getName())) {
-          upToDate = true;
-        }
+      // check validity
+      QualityProfileDto profile = dbClient.qualityProfileDao().getByKey(QualityProfileKey.of(currentDefault, language), session);
+      if (profile != null) {
+        upToDate = true;
       }
     }
 
     if (!upToDate) {
-      String defaultProfileName = defaultProfileName(profiles);
+      String defaultProfileName = defaultProfileName(profileDefs);
       LOGGER.info("Set default " + language + " profile: " + defaultProfileName);
       settings.saveProperty(propertyKey, defaultProfileName);
     }

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleDoc.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleDoc.java
index e6555b786ebd6aa08820acb253fc5709d6b0b01f..bccc4735f493ab934c63a8ccb3abe1a20a2c099a 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleDoc.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleDoc.java
@@ -35,7 +35,7 @@ public class ActiveRuleDoc extends BaseDoc implements ActiveRule {
 
   public ActiveRuleDoc(Map<String, Object> fields) {
     super(fields);
-    this.key = ActiveRuleKey.parse((String) getField(ActiveRuleNormalizer.ActiveRuleField.KEY.key()));
+    this.key = ActiveRuleKey.parse((String) getNullableField(ActiveRuleNormalizer.ActiveRuleField.KEY.key()));
     Preconditions.checkArgument(key!=null, "Invalid ActiveRuleKey!");
   }
 
@@ -46,12 +46,12 @@ public class ActiveRuleDoc extends BaseDoc implements ActiveRule {
 
   @Override
   public String severity() {
-    return (String) getField(ActiveRuleNormalizer.ActiveRuleField.SEVERITY.key());
+    return (String) getNullableField(ActiveRuleNormalizer.ActiveRuleField.SEVERITY.key());
   }
 
   @Override
   public ActiveRule.Inheritance inheritance() {
-    String inheritance = getField(ActiveRuleNormalizer.ActiveRuleField.INHERITANCE.key());
+    String inheritance = getNullableField(ActiveRuleNormalizer.ActiveRuleField.INHERITANCE.key());
     if (inheritance == null || inheritance.isEmpty() ||
       inheritance.toLowerCase().contains("none")) {
       return Inheritance.NONE;
@@ -67,7 +67,7 @@ public class ActiveRuleDoc extends BaseDoc implements ActiveRule {
   @Override
   @CheckForNull
   public ActiveRuleKey parentKey() {
-    String data = getField(ActiveRuleNormalizer.ActiveRuleField.PARENT_KEY.key());
+    String data = getNullableField(ActiveRuleNormalizer.ActiveRuleField.PARENT_KEY.key());
     if (data != null && !data.isEmpty()) {
       return ActiveRuleKey.parse(data);
     }
@@ -77,7 +77,7 @@ public class ActiveRuleDoc extends BaseDoc implements ActiveRule {
   @Override
   public Map<String, String> params() {
     Map<String, String> params = new HashMap<String, String>();
-    List<Map<String, String>> allParams = getField(ActiveRuleNormalizer.ActiveRuleField.PARAMS.key());
+    List<Map<String, String>> allParams = getNullableField(ActiveRuleNormalizer.ActiveRuleField.PARAMS.key());
     if (allParams != null) {
       for (Map<String, String> param : allParams) {
         params.put(param.get(ActiveRuleNormalizer.ActiveRuleParamField.NAME.key()),

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java b/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java
index f8e1f351935c5e9c0ebd24c0c09847c944cd6aad..ed72f4c20e72d953bac8adb55b02cff351fdc034 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java
@@ -230,12 +230,15 @@ public class RegisterRules implements Startable {
     // Debt definitions are set to null if the sub-characteristic and the remediation function are null
     DebtRemediationFunction debtRemediationFunction = subCharacteristic != null ? def.debtRemediationFunction() : null;
     boolean hasDebt = subCharacteristic != null && debtRemediationFunction != null;
-    return mergeDebtDefinitions(def, dto,
-      hasDebt ? subCharacteristic.getId() : null,
-      debtRemediationFunction != null ? debtRemediationFunction.type().name() : null,
-      hasDebt ? debtRemediationFunction.coefficient() : null,
-      hasDebt ? debtRemediationFunction.offset() : null,
-      hasDebt ? def.effortToFixDescription() : null);
+    if (hasDebt) {
+      return mergeDebtDefinitions(def, dto,
+        subCharacteristic.getId(),
+        debtRemediationFunction.type().name(),
+        debtRemediationFunction.coefficient(),
+        debtRemediationFunction.offset(),
+        def.effortToFixDescription());
+    }
+    return mergeDebtDefinitions(def, dto, null, null, null, null, null);
   }
 
   private boolean mergeDebtDefinitions(RulesDefinition.Rule def, RuleDto dto, @Nullable Integer characteristicId, @Nullable String remediationFunction,
@@ -354,8 +357,8 @@ public class RegisterRules implements Startable {
       if (toBeRemoved && !RuleStatus.REMOVED.toString().equals(ruleDto.getStatus())) {
         LOG.info(String.format("Disable rule %s", ruleDto.getKey()));
         ruleDto.setStatus(Rule.STATUS_REMOVED);
-        ruleDto.setSystemTags(Collections.EMPTY_SET);
-        ruleDto.setTags(Collections.EMPTY_SET);
+        ruleDto.setSystemTags(Collections.<String>emptySet());
+        ruleDto.setTags(Collections.<String>emptySet());
         dbClient.ruleDao().update(session, ruleDto);
         removedRules.add(ruleDto);
         if (removedRules.size() % 100 == 0) {

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java b/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java
index 285922d9c41694523dc81f37698d95b1cdcff095..12605c979c649dcd5982394c72f44d52d9224e33 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java
@@ -78,6 +78,9 @@ public class RuleDao extends BaseDao<RuleMapper, RuleDto, RuleKey> {
     throw new UnsupportedOperationException("Rules cannot be deleted");
   }
 
+  /**
+   * @deprecated in 4.4. Use keys.
+   */
   @CheckForNull
   @Deprecated
   public RuleDto getById(int id, DbSession session) {

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java b/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java
index 3a4f0c721cb0868d846e803989dff4efcbf052f2..0367203ad16909a89421de9362c9bab67438190e 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java
@@ -29,7 +29,6 @@ import org.sonar.server.rule.RuleParam;
 import org.sonar.server.search.BaseDoc;
 import org.sonar.server.search.IndexUtils;
 
-import javax.annotation.CheckForNull;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
@@ -46,12 +45,7 @@ public class RuleDoc extends BaseDoc implements Rule {
 
   @Override
   public RuleKey key() {
-    String key = getField(RuleNormalizer.RuleField.KEY.key());
-    if (key == null || key.isEmpty()) {
-      throw new IllegalStateException("Missing values for RuleKey in RuleDoc");
-    } else {
-      return RuleKey.parse(key);
-    }
+    return RuleKey.parse(this.<String>getField(RuleNormalizer.RuleField.KEY.key()));
   }
 
   /**
@@ -62,97 +56,82 @@ public class RuleDoc extends BaseDoc implements Rule {
   }
 
   @Override
-  @CheckForNull
   public String internalKey() {
-    return getField(RuleNormalizer.RuleField.INTERNAL_KEY.key());
-  }
-
-  @Override
-  public String markdownNote() {
-    return getField(RuleNormalizer.RuleField.NOTE.key());
+    return getNullableField(RuleNormalizer.RuleField.INTERNAL_KEY.key());
   }
 
   @Override
-  @CheckForNull
   public String language() {
     return getField(RuleNormalizer.RuleField.LANGUAGE.key());
   }
 
   @Override
-  @CheckForNull
   public String name() {
     return getField(RuleNormalizer.RuleField.NAME.key());
   }
 
   @Override
-  @CheckForNull
   public String htmlDescription() {
-    return getField(RuleNormalizer.RuleField.HTML_DESCRIPTION.key());
+    return getNullableField(RuleNormalizer.RuleField.HTML_DESCRIPTION.key());
   }
 
   @Override
-  @CheckForNull
   public String severity() {
     return (String) getField(RuleNormalizer.RuleField.SEVERITY.key());
   }
 
   @Override
-  @CheckForNull
   public RuleStatus status() {
     return RuleStatus.valueOf((String) getField(RuleNormalizer.RuleField.STATUS.key()));
   }
 
   @Override
-  @CheckForNull
   public boolean template() {
     return (Boolean) getField(RuleNormalizer.RuleField.TEMPLATE.key());
   }
 
   @Override
-  @CheckForNull
   public List<String> tags() {
     return (List<String>) getField(RuleNormalizer.RuleField.TAGS.key());
   }
 
   @Override
-  @CheckForNull
   public List<String> systemTags() {
     return (List<String>) getField(RuleNormalizer.RuleField.SYSTEM_TAGS.key());
   }
 
   @Override
-  @CheckForNull
   public List<RuleParam> params() {
     List<RuleParam> params = new ArrayList<RuleParam>();
-    if (this.getField(RuleNormalizer.RuleField.PARAMS.key()) != null) {
-      List<Map<String, Object>> esParams = this.getField(RuleNormalizer.RuleField.PARAMS.key());
-      for (final Map<String, Object> param : esParams) {
+    List<Map<String, Object>> esParams = getNullableField(RuleNormalizer.RuleField.PARAMS.key());
+    if (esParams != null) {
+      for (final Map<String, Object> esParam : esParams) {
         params.add(new RuleParam() {
           {
-            this.fields = param;
+            this.fields = esParam;
           }
 
           Map<String, Object> fields;
 
           @Override
           public String key() {
-            return (String) param.get(RuleNormalizer.RuleParamField.NAME.key());
+            return (String) esParam.get(RuleNormalizer.RuleParamField.NAME.key());
           }
 
           @Override
           public String description() {
-            return (String) param.get(RuleNormalizer.RuleParamField.DESCRIPTION.key());
+            return (String) esParam.get(RuleNormalizer.RuleParamField.DESCRIPTION.key());
           }
 
           @Override
           public String defaultValue() {
-            return (String) param.get(RuleNormalizer.RuleParamField.DEFAULT_VALUE.key());
+            return (String) esParam.get(RuleNormalizer.RuleParamField.DEFAULT_VALUE.key());
           }
 
           @Override
           public RuleParamType type() {
             return RuleParamType
-              .parse((String) param.get(RuleNormalizer.RuleParamField.TYPE.key()));
+              .parse((String) esParam.get(RuleNormalizer.RuleParamField.TYPE.key()));
           }
         });
       }
@@ -161,21 +140,18 @@ public class RuleDoc extends BaseDoc implements Rule {
   }
 
   @Override
-  @CheckForNull
   public String debtCharacteristicKey() {
-    return (String) getField(RuleNormalizer.RuleField.CHARACTERISTIC.key());
+    return (String) getNullableField(RuleNormalizer.RuleField.CHARACTERISTIC.key());
   }
 
   @Override
-  @CheckForNull
   public String debtSubCharacteristicKey() {
-    return (String) getField(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.key());
+    return (String) getNullableField(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.key());
   }
 
   @Override
-  @CheckForNull
   public DebtRemediationFunction debtRemediationFunction() {
-    final String function = this.getField(RuleNormalizer.RuleField.DEBT_FUNCTION_TYPE.key());
+    final String function = getNullableField(RuleNormalizer.RuleField.DEBT_FUNCTION_TYPE.key());
     if (function == null || function.isEmpty()) {
       return null;
     } else {
@@ -187,43 +163,45 @@ public class RuleDoc extends BaseDoc implements Rule {
 
         @Override
         public String coefficient() {
-          return (String) getField(RuleNormalizer.RuleField.DEBT_FUNCTION_COEFFICIENT.key());
+          return (String) getNullableField(RuleNormalizer.RuleField.DEBT_FUNCTION_COEFFICIENT.key());
         }
 
         @Override
         public String offset() {
-          return (String) getField(RuleNormalizer.RuleField.DEBT_FUNCTION_OFFSET.key());
+          return (String) getNullableField(RuleNormalizer.RuleField.DEBT_FUNCTION_OFFSET.key());
         }
       };
     }
   }
 
   @Override
-  @CheckForNull
+  public String markdownNote() {
+    return getNullableField(RuleNormalizer.RuleField.NOTE.key());
+  }
+
+  @Override
   public String noteLogin() {
-    return (String) getField(RuleNormalizer.RuleField.NOTE_LOGIN.key());
+    return (String) getNullableField(RuleNormalizer.RuleField.NOTE_LOGIN.key());
   }
 
   @Override
   public Date noteCreatedAt() {
-    return IndexUtils.parseDateTime((String) getField(RuleNormalizer.RuleField.NOTE_CREATED_AT.key()));
+    return IndexUtils.parseDateTime((String) getNullableField(RuleNormalizer.RuleField.NOTE_CREATED_AT.key()));
   }
 
   @Override
   public Date noteUpdatedAt() {
-    return IndexUtils.parseDateTime((String) getField(RuleNormalizer.RuleField.NOTE_UPDATED_AT.key()));
+    return IndexUtils.parseDateTime((String) getNullableField(RuleNormalizer.RuleField.NOTE_UPDATED_AT.key()));
   }
 
   @Override
-  @CheckForNull
   public Date createdAt() {
-    return IndexUtils.parseDateTime((String) getField(RuleNormalizer.RuleField.CREATED_AT.key()));
+    return IndexUtils.parseDateTime((String) getNullableField(RuleNormalizer.RuleField.CREATED_AT.key()));
   }
 
   @Override
-  @CheckForNull
   public Date updatedAt() {
-    return IndexUtils.parseDateTime((String) getField(RuleNormalizer.RuleField.UPDATED_AT.key()));
+    return IndexUtils.parseDateTime((String) getNullableField(RuleNormalizer.RuleField.UPDATED_AT.key()));
   }
 
   @Override

diff --git a/sonar-server/src/main/java/org/sonar/server/search/BaseDoc.java b/sonar-server/src/main/java/org/sonar/server/search/BaseDoc.java
index 966e714ea44d0a61e14c90385443f36309ed0048..d7b930532fce61266886f7dab7d48d757dba28d1 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/search/BaseDoc.java
+++ b/sonar-server/src/main/java/org/sonar/server/search/BaseDoc.java
@@ -27,21 +27,35 @@ import java.util.Map;
  */
 public abstract class BaseDoc {
 
-  private final Map<String,Object> fields;
+  private final Map<String, Object> fields;
 
-  protected BaseDoc(Map<String,Object> fields) {
+  protected BaseDoc(Map<String, Object> fields) {
     this.fields = fields;
   }
 
   public String keyField() {
-    return (String)fields.get("key");
+    return (String) fields.get("key");
   }
 
+  /**
+   * Use this method when field value can be null
+   */
   @CheckForNull
-  public <K> K getField(String key) {
+  public <K> K getNullableField(String key) {
     if (!fields.containsKey(key)) {
       throw new IllegalStateException(String.format("Field %s not specified in query options", key));
     }
-    return (K)fields.get(key);
+    return (K) fields.get(key);
+  }
+
+  /**
+   * Use this method when you are sure that the value can't be null in ES document
+   */
+  public <K> K getField(String key) {
+    K value = getNullableField(key);
+    if (value == null) {
+      throw new IllegalStateException("Value of index field is null: " + key);
+    }
+    return value;
   }
 }

diff --git a/sonar-server/src/main/java/org/sonar/server/search/IndexUtils.java b/sonar-server/src/main/java/org/sonar/server/search/IndexUtils.java
index febc8dcfcff4082c7478c3cd661ba7ce97c96141..02d69bb96ce26c1df990a3ad671774a9d7786a6e 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/search/IndexUtils.java
+++ b/sonar-server/src/main/java/org/sonar/server/search/IndexUtils.java
@@ -34,11 +34,11 @@ public class IndexUtils {
 
   @CheckForNull
   public static Date parseDateTime(@Nullable String s) {
-    DateFormat sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");
     if (s == null) {
       return null;
     }
     try {
+      DateFormat sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");
       return sdf.parse(s);
     } catch (ParseException e) {
       throw new IllegalArgumentException("Cannot parse ES date: " + s, e);

diff --git a/sonar-server/src/main/java/org/sonar/server/search/ws/BaseMapping.java b/sonar-server/src/main/java/org/sonar/server/search/ws/BaseMapping.java
index 28aedaa8ff2027e88d7802c2e1a19bebd0c4c949..584df5f31a06af6115093aa602049643de590e47 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/search/ws/BaseMapping.java
+++ b/sonar-server/src/main/java/org/sonar/server/search/ws/BaseMapping.java
@@ -132,7 +132,7 @@ public abstract class BaseMapping implements ServerComponent {
 
     @Override
     public void write(JsonWriter json, BaseDoc doc) {
-      Object val = doc.getField(indexFields[0]);
+      Object val = doc.getNullableField(indexFields[0]);
       json.prop(key, val != null ? val.toString() : null);
     }
   }
@@ -147,7 +147,7 @@ public abstract class BaseMapping implements ServerComponent {
 
     @Override
     public void write(JsonWriter json, BaseDoc doc) {
-      Boolean val = doc.getField(indexFields[0]);
+      Boolean val = doc.getNullableField(indexFields[0]);
       json.prop(key, val != null ? val.booleanValue() : null);
     }
   }
@@ -162,7 +162,7 @@ public abstract class BaseMapping implements ServerComponent {
 
     @Override
     public void write(JsonWriter json, BaseDoc doc) {
-      Iterable<String> values = doc.getField(indexFields[0]);
+      Iterable<String> values = doc.getNullableField(indexFields[0]);
       json.name(key).beginArray().values(values).endArray();
     }
   }
@@ -177,7 +177,7 @@ public abstract class BaseMapping implements ServerComponent {
 
     @Override
     public void write(JsonWriter json, BaseDoc doc) {
-      String val = doc.getField(indexFields[0]);
+      String val = doc.getNullableField(indexFields[0]);
       if (val != null) {
         json.propDateTime(key, IndexUtils.parseDateTime(val));
       }

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb
index 360b2c7afd46b53931b655c0aa90586337cb067c..588ecab7ef57fe060e9dd8e2bcf0a7d8fb34f4c6 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb
@@ -8,9 +8,9 @@
   <% end %>
 </div>
 
-<% if @rule.noteAsMarkdown() %>
+<% if @rule.markdownNote() %>
   <div class="marginbottom10">
-    <%= Api::Utils.markdown_to_html(@rule.noteAsMarkdown()) -%>
+    <%= Api::Utils.markdown_to_html(@rule.markdownNote()) -%>
   </div>
 <% end %>
 

diff --git a/sonar-server/src/test/java/org/sonar/server/search/BaseDocTest.java b/sonar-server/src/test/java/org/sonar/server/search/BaseDocTest.java
index 41ad2775902cb7091788a1171603c04c02bfeb08..2b22562bddffb4f697cb2c341d9a4882e24dc2fb 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/search/BaseDocTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/search/BaseDocTest.java
@@ -39,9 +39,9 @@ public class BaseDocTest {
     BaseDoc doc = new BaseDoc(fields) {
     };
 
-    assertThat(doc.getField("a_string")).isEqualTo("foo");
-    assertThat(doc.getField("a_int")).isEqualTo(42);
-    assertThat(doc.getField("a_null")).isNull();
+    assertThat(doc.getNullableField("a_string")).isEqualTo("foo");
+    assertThat(doc.getNullableField("a_int")).isEqualTo(42);
+    assertThat(doc.getNullableField("a_null")).isNull();
   }
 
   @Test
@@ -51,7 +51,7 @@ public class BaseDocTest {
     };
 
     try {
-      doc.getField("a_string");
+      doc.getNullableField("a_string");
       fail();
     } catch (IllegalStateException e) {
       assertThat(e).hasMessage("Field a_string not specified in query options");