Fix some quality flaws

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java
index c4348fca5652c767d23a91d890c0f26908db4b4c..44391663eec10b91158a1dd8bc262219a30a8891 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java
@@ -27,6 +27,7 @@ import org.sonar.core.qualityprofile.db.ActiveRuleKey;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.util.HashMap;
 import java.util.Map;
 
@@ -60,22 +61,21 @@ public class ActiveRuleChange implements ActivityLog {
     return severity;
   }
 
-  public ActiveRuleChange setSeverity(@Nullable String severity) {
-    this.severity = severity;
+  public ActiveRuleChange setSeverity(@Nullable String s) {
+    this.severity = s;
     return this;
   }
 
-  public ActiveRuleChange setInheritance(@Nullable ActiveRule.Inheritance inheritance) {
-    this.inheritance = inheritance;
+  public ActiveRuleChange setInheritance(@Nullable ActiveRule.Inheritance i) {
+    this.inheritance = i;
     return this;
   }
 
   @CheckForNull
   public ActiveRule.Inheritance getInheritance() {
-    return this.inheritance;
+    return inheritance;
   }
 
-  @CheckForNull
   public Map<String, String> getParameters() {
     return parameters;
   }
@@ -95,16 +95,13 @@ public class ActiveRuleChange implements ActivityLog {
   public Map<String, String> getDetails() {
     HashMap<String, String> details = new HashMap<String, String>();
 
-    if (getKey() != null) {
-      details.put("key", getKey().toString());
-      details.put("ruleKey", getKey().ruleKey().toString());
-      details.put("profileKey", getKey().qProfile().toString());
-    }
-    if (!parameters.isEmpty()) {
-      for (Map.Entry<String, String> param : parameters.entrySet()) {
-        if (!param.getKey().isEmpty()) {
-          details.put("param_" + param.getKey(), param.getValue());
-        }
+    details.put("key", getKey().toString());
+    details.put("ruleKey", getKey().ruleKey().toString());
+    details.put("profileKey", getKey().qProfile().toString());
+
+    for (Map.Entry<String, String> param : parameters.entrySet()) {
+      if (!param.getKey().isEmpty()) {
+        details.put("param_" + param.getKey(), param.getValue());
       }
     }
     if (StringUtils.isNotEmpty(severity)) {
@@ -133,6 +130,6 @@ public class ActiveRuleChange implements ActivityLog {
 
   @Override
   public String getAction() {
-    return this.type.name();
+    return type.name();
   }
 }

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java
index 45d196b3f1b0e8d408af4194acdd47fe14905298..7f496b39ea3518dc9bfd787d8e8ac94462db388a 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java
@@ -218,8 +218,11 @@ public class QProfileService implements ServerComponent {
         RuleDto ruleDto = db.ruleDao().getNullableByKey(session, profileActivity.ruleKey());
         profileActivity.ruleName(ruleDto != null ? ruleDto.getName() : null);
 
-        UserDto user = db.userDao().selectActiveUserByLogin(profileActivity.login(), session);
-        profileActivity.authorName(user != null ? user.getName() : null);
+        String login = profileActivity.login();
+        if (login != null) {
+          UserDto user = db.userDao().selectActiveUserByLogin(login, session);
+          profileActivity.authorName(user != null ? user.getName() : null);
+        }
         result.getHits().add(profileActivity);
       }
       return result;

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
index 1400427ab422b30fa3b76fdc9c666633ff947a3b..81c470690bad126d27df3f5dd486e780fabab640 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
@@ -204,10 +204,12 @@ class RuleActivatorContext {
   }
 
   boolean isSame(ActiveRuleChange change) {
-    if (change.getInheritance() != null && !change.getInheritance().name().equals(activeRule.getInheritance())) {
+    ActiveRule.Inheritance inheritance = change.getInheritance();
+    if (inheritance != null && !inheritance.name().equals(activeRule.getInheritance())) {
       return false;
     }
-    if (change.getSeverity() != null && !change.getSeverity().equals(activeRule.getSeverityString())) {
+    String severity = change.getSeverity();
+    if (severity != null && !severity.equals(activeRule.getSeverityString())) {
       return false;
     }
     for (Map.Entry<String, String> changeParam : change.getParameters().entrySet()) {

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java
index bf8fec54f75466b1693f6dded410827565291da8..1854942476b36e777f3aec3abbcb956324254c03 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java
@@ -63,8 +63,9 @@ public class RuleActivatorContextFactory implements ServerComponent {
   private RuleActivatorContext create(QualityProfileDto profile, RuleKey ruleKey, DbSession session, RuleActivatorContext context) {
     initRule(ruleKey, context, session);
     initActiveRules(profile.getKey(), ruleKey, context, session, false);
-    if (profile.getParentKee() != null) {
-      initActiveRules(profile.getParentKee(), ruleKey, context, session, true);
+    String parentKee = profile.getParentKee();
+    if (parentKee != null) {
+      initActiveRules(parentKee, ruleKey, context, session, true);
     }
     return context;
   }

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 ca07838e2daa9efeeda4d4cf628e9eada3101a62..24ce64dd9e4603adcd51a62f0b10ec3709326c11 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) getNullableField(ActiveRuleNormalizer.ActiveRuleField.KEY.field()));
+    this.key = ActiveRuleKey.parse((String) getField(ActiveRuleNormalizer.ActiveRuleField.KEY.field()));
     Preconditions.checkArgument(key!=null, "Invalid ActiveRuleKey!");
   }
 

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java
index 133c066d56f12c4bd5dc3ab48c6dbb53f6058031..924490bcb4a496d0e5b3219368ed14081e1cb0e4 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java
@@ -22,6 +22,7 @@ package org.sonar.server.rule;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.core.permission.GlobalPermissions;
+import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.rule.index.RuleIndex;
 import org.sonar.server.rule.index.RuleNormalizer;
 import org.sonar.server.rule.index.RuleQuery;
@@ -51,10 +52,18 @@ public class RuleService implements ServerComponent {
   }
 
   @CheckForNull
-  public org.sonar.server.rule.Rule getByKey(RuleKey key) {
+  public Rule getByKey(RuleKey key) {
     return index.getByKey(key);
   }
 
+  public Rule getNonNullByKey(RuleKey key) {
+    Rule rule = index.getByKey(key);
+    if (rule == null) {
+      throw new NotFoundException("Rule not found: " + key);
+    }
+    return rule;
+  }
+
   public RuleQuery newRuleQuery() {
     return new RuleQuery();
   }

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java b/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java
index 898acb5d657a9dec99545b8425ceb82a4aa2d18a..57f186a1d89e2bfa8acf56b9285a399fc4a484e4 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java
@@ -65,8 +65,8 @@ public class RuleQuery {
     return qProfileKey;
   }
 
-  public RuleQuery setQProfileKey(String qProfileKey) {
-    this.qProfileKey = qProfileKey;
+  public RuleQuery setQProfileKey(@Nullable String s) {
+    this.qProfileKey = s;
     return this;
   }
 
@@ -229,8 +229,8 @@ public class RuleQuery {
     return this;
   }
 
-  public RuleQuery setAvailableSince(Date since) {
-    this.availableSince = since;
+  public RuleQuery setAvailableSince(@Nullable Date d) {
+    this.availableSince = d;
     return this;
   }
 
@@ -238,8 +238,8 @@ public class RuleQuery {
     return this.availableSince;
   }
 
-  public RuleQuery setInternalKey(String internalKey) {
-    this.internalKey = internalKey;
+  public RuleQuery setInternalKey(@Nullable String s) {
+    this.internalKey = s;
     return this;
   }
 
@@ -247,8 +247,8 @@ public class RuleQuery {
     return internalKey;
   }
 
-  public RuleQuery setRuleKey(String ruleKey) {
-    this.ruleKey = ruleKey;
+  public RuleQuery setRuleKey(@Nullable String s) {
+    this.ruleKey = s;
     return this;
   }
 

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java b/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java
index 5f90f91c96351e32b3d5d598b379611934fa3797..bb8f2da4ee3db938192fa86a1c15a546b05a34c6 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java
@@ -46,10 +46,11 @@ public class ActiveRuleCompleter implements ServerComponent {
   void completeSearch(RuleQuery query, Collection<Rule> rules, JsonWriter json) {
     json.name("actives").beginObject();
 
-    if (query.getQProfileKey() != null) {
+    String profileKey = query.getQProfileKey();
+    if (profileKey != null) {
       // Load details of active rules on the selected profile
       for (Rule rule : rules) {
-        ActiveRule activeRule = loader.getActiveRule(ActiveRuleKey.of(query.getQProfileKey(), rule.key()));
+        ActiveRule activeRule = loader.getActiveRule(ActiveRuleKey.of(profileKey, rule.key()));
         if (activeRule != null) {
           writeActiveRules(rule.key(), Arrays.asList(activeRule), json);
         }
@@ -88,8 +89,9 @@ public class ActiveRuleCompleter implements ServerComponent {
       .prop("qProfile", activeRule.key().qProfile().toString())
       .prop("inherit", activeRule.inheritance().toString())
       .prop("severity", activeRule.severity());
-    if (activeRule.parentKey() != null) {
-      json.prop("parent", activeRule.parentKey().toString());
+    ActiveRuleKey parentKey = activeRule.parentKey();
+    if (parentKey != null) {
+      json.prop("parent", parentKey.toString());
     }
     json.name("params").beginArray();
     for (Map.Entry<String, String> param : activeRule.params().entrySet()) {

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java b/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java
index 3eb19d41e9d1c41982104158dc4d77bb96f3c12a..58a2cf69cf06535a844447b87eb6853d44c1ad19 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java
@@ -156,14 +156,14 @@ public class CreateAction implements RequestHandler {
   }
 
   private void writeResponse(Response response, RuleKey ruleKey) {
-    Rule rule = service.getByKey(ruleKey);
+    Rule rule = service.getNonNullByKey(ruleKey);
     JsonWriter json = response.newJsonWriter().beginObject().name("rule");
     mapping.write((BaseDoc) rule, json);
     json.endObject().close();
   }
 
   private void write409(Response response, RuleKey ruleKey) {
-    Rule rule = service.getByKey(ruleKey);
+    Rule rule = service.getNonNullByKey(ruleKey);
 
     Response.Stream stream = response.stream();
     stream.setStatus(409);

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java b/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java
index cca752f610b3095f6f48eda13cd0371b5bbd3d45..553491f93dc1eba3a84c5b569de8dd7b01c84350 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java
@@ -213,7 +213,7 @@ public class UpdateAction implements RequestHandler {
   }
 
   private void writeResponse(Response response, RuleKey ruleKey) {
-    Rule rule = service.getByKey(ruleKey);
+    Rule rule = service.getNonNullByKey(ruleKey);
     JsonWriter json = response.newJsonWriter().beginObject().name("rule");
     mapping.write((BaseDoc) rule, json);
     json.endObject().close();

diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
index 0147e02f3e4c298d54915f7a0f7929eac7edd73f..9c6f692fae2ba9d60603d60e3c8462a32698066c 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
@@ -32,6 +32,7 @@ import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.rule.RuleDto;
 import org.sonar.server.db.DbClient;
+import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.exceptions.UnauthorizedException;
 import org.sonar.server.rule.db.RuleDao;
 import org.sonar.server.rule.index.RuleIndex;
@@ -43,6 +44,7 @@ import java.util.Collections;
 import java.util.Set;
 
 import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
 
 public class RuleServiceMediumTest {
 
@@ -79,6 +81,29 @@ public class RuleServiceMediumTest {
 
     Rule rule = service.getByKey(key);
     assertThat(rule).isNotNull();
+
+    assertThat(service.getByKey(RuleKey.of("un", "known"))).isNull();
+  }
+
+  @Test
+  public void get_non_null_rule_by_key() throws Exception {
+    MockUserSession.set()
+      .setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)
+      .setLogin("me");
+
+    RuleKey key = RuleKey.of("java", "S001");
+
+    dao.insert(dbSession, RuleTesting.newDto(key));
+    dbSession.commit();
+    dbSession.clearCache();
+
+    assertThat(service.getNonNullByKey(key)).isNotNull();
+    try {
+      service.getNonNullByKey(RuleKey.of("un", "known"));
+      fail();
+    } catch (NotFoundException e) {
+      assertThat(e).hasMessage("Rule not found: un:known");
+    }
   }
 
   @Test