Fix quality flaws

diff --git a/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java b/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java
index 58bf7be09174d982e040292854d2974230323762..0f9240c93f07602039ef86741ca00f9b00d376c2 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java
+++ b/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java
@@ -36,19 +36,21 @@ public class BadRequestException extends ServerException {
 
   private static final int BAD_REQUEST = 400;
 
-  private List<Message> errors = newArrayList();
+  private final List<Message> errors;
 
   public BadRequestException(String message) {
     super(BAD_REQUEST, message);
+    this.errors = newArrayList();
   }
 
-  public BadRequestException(@Nullable String message, List<Message> errors) {
+  public BadRequestException(String message, List<Message> errors) {
     super(BAD_REQUEST, message);
     this.errors = errors;
   }
 
   public BadRequestException(@Nullable String message, @Nullable String l10nKey, @Nullable Object[] l10nParams) {
     super(BAD_REQUEST, message, l10nKey, l10nParams);
+    this.errors = newArrayList();
   }
 
   public BadRequestException(@Nullable String message, @Nullable String l10nKey, @Nullable Object[] l10nParams, List<Message> errors) {

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java
index 1fa32a29f6af3d4f41607ef1d81e7beea8967c50..aee8e61c1be767f2341d5048ae9a618a201f41da 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java
@@ -139,14 +139,22 @@ public class IssueShowWsHandler implements RequestHandler {
   private void addCharacteristics(IssueQueryResult result, DefaultIssue issue, JsonWriter json) {
     Characteristic requirement = technicalDebtManager.findRequirementByRule(result.rule(issue));
     // Requirement can be null if it has been disabled
-    if (requirement != null && requirement.rootId() != null && requirement.parentId() != null) {
-      Characteristic characteristic = technicalDebtManager.findCharacteristicById(requirement.rootId());
+    if (requirement != null) {
+      Characteristic characteristic = findCharacteristicById(requirement.rootId());
       json.prop("characteristic", characteristic != null ? characteristic.name() : null);
-      Characteristic subCharacteristic = technicalDebtManager.findCharacteristicById(requirement.parentId());
+      Characteristic subCharacteristic = findCharacteristicById(requirement.parentId());
       json.prop("subCharacteristic", subCharacteristic != null ? subCharacteristic.name() : null);
     }
   }
 
+  @CheckForNull
+  private Characteristic findCharacteristicById(@Nullable Integer id){
+    if (id != null) {
+      return technicalDebtManager.findCharacteristicById(id);
+    }
+    return null;
+  }
+
   private void writeTransitions(Issue issue, JsonWriter json) {
     json.name("transitions").beginArray();
     if (UserSession.get().isLoggedIn()) {

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
index 99f5d1cc55550d8daa0a2209a7bc3ce96c0fa08a..8de29df6e3f1c69fecd09af05c019ec360582712 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
@@ -43,6 +43,7 @@ import org.sonar.server.util.TypeValidations;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.util.Date;
 import java.util.List;
 
@@ -325,7 +326,8 @@ public class QProfileActiveRuleOperations implements ServerComponent {
         activeRuleDao.insert(activeRuleParam, session);
         session.commit();
         newParams.add(activeRuleParam);
-        actions.add(profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), parentParam.getKey(), null, parentParam.getValue(), getLoggedName(userSession)));
+        actions.add(profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), parentParam.getKey(), null, parentParam.getValue(),
+          getLoggedName(userSession)));
       }
     }
     return newParams;

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java
index 7f40c7079aeda17982c7d6b055eaa61c3b193dcc..8759f9a0b1f549032472d4b743179c3675178a60 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java
@@ -36,6 +36,7 @@ import org.sonar.server.user.UserSession;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.util.List;
 import java.util.Map;
 
@@ -54,7 +55,8 @@ public class QProfileOperations implements ServerComponent {
   private final ProfilesManager profilesManager;
 
   public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, PropertiesDao propertiesDao,
-                            QProfileRepositoryExporter exporter, PreviewCache dryRunCache, ESActiveRule esActiveRule, QProfileLookup profileLookup, ProfilesManager profilesManager) {
+                            QProfileRepositoryExporter exporter, PreviewCache dryRunCache, ESActiveRule esActiveRule, QProfileLookup profileLookup,
+                            ProfilesManager profilesManager) {
     this.myBatis = myBatis;
     this.dao = dao;
     this.activeRuleDao = activeRuleDao;

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/Rule.java b/sonar-server/src/main/java/org/sonar/server/rule/Rule.java
index a6bb4e2e1b35af3579932ebc20858f33f91720c6..169cc179ddaa3d56c01d5950b63ebecb5b2575f3 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/Rule.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/Rule.java
@@ -77,6 +77,7 @@ public class Rule {
     return language;
   }
 
+  @CheckForNull
   public String name() {
     return name;
   }
@@ -175,7 +176,7 @@ public class Rule {
       return this;
     }
 
-    public Builder setName(String name) {
+    public Builder setName(@Nullable String name) {
       this.name = name;
       return this;
     }

diff --git a/sonar-server/src/main/java/org/sonar/server/source/HtmlSourceDecorator.java b/sonar-server/src/main/java/org/sonar/server/source/HtmlSourceDecorator.java
index e63bfa72ef63d6b9f17f3290d6bf505e74d8bc57..24f80e9b49ccd131c71ab413deb7c28c9fcef3b7 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/source/HtmlSourceDecorator.java
+++ b/sonar-server/src/main/java/org/sonar/server/source/HtmlSourceDecorator.java
@@ -33,6 +33,7 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 public class HtmlSourceDecorator implements ServerComponent {
@@ -48,7 +49,6 @@ public class HtmlSourceDecorator implements ServerComponent {
     this.snapshotDataDao = snapshotDataDao;
   }
 
-  @CheckForNull
   public List<String> getDecoratedSourceAsHtml(String componentKey, @Nullable Integer from, @Nullable Integer to) {
     SqlSession session = mybatis.openSession();
     try {
@@ -57,7 +57,7 @@ public class HtmlSourceDecorator implements ServerComponent {
         String snapshotSource = snapshotSourceDao.selectSnapshotSourceByComponentKey(componentKey, session);
         return decorate(snapshotSource, snapshotDataEntries, from, to);
       }
-      return null;
+      return Collections.emptyList();
     } finally {
       MyBatis.closeQuietly(session);
     }

diff --git a/sonar-server/src/main/java/org/sonar/server/source/SourceService.java b/sonar-server/src/main/java/org/sonar/server/source/SourceService.java
index 8c742b0f28643c08f58dc82a9dbaaced0c8c22c6..c2ce1faa8129b2a484f27c9a061f38dd9b6bad32 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/source/SourceService.java
+++ b/sonar-server/src/main/java/org/sonar/server/source/SourceService.java
@@ -66,7 +66,7 @@ public class SourceService implements ServerComponent {
     UserSession.get().checkProjectPermission(UserRole.CODEVIEWER, project.getKey());
 
     List<String> decoratedSource = sourceDecorator.getDecoratedSourceAsHtml(componentKey, from, to);
-    if (decoratedSource != null) {
+    if (!decoratedSource.isEmpty()) {
       return decoratedSource;
     } else {
       return deprecatedSourceDecorator.getSourceAsHtml(componentKey, from, to);

diff --git a/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java b/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java
index 8f8948c71896a3297c469de6eb4b2188f124aa60..c2ef0708d367ad61e55d5e4ec7a621ed86081191 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java
@@ -34,13 +34,30 @@ public class BadRequestExceptionTest {
   }
 
   @Test
-  public void text_error_with_message() throws Exception {
+  public void text_error_with_list_of_errors() throws Exception {
     BadRequestException exception = BadRequestException.of("error", newArrayList(BadRequestException.Message.of("new error")));
 
     assertThat(exception.errors()).hasSize(1);
     assertThat(exception.errors().get(0).text()).isEqualTo("new error");
   }
 
+  @Test
+  public void text_error_with_list_of_l18n_errors() throws Exception {
+    BadRequestException exception = BadRequestException.of("error", newArrayList(BadRequestException.Message.ofL10n("issue.error.123", "10")));
+
+    assertThat(exception.errors()).hasSize(1);
+    assertThat(exception.errors().get(0).l10nKey()).isEqualTo("issue.error.123");
+    assertThat(exception.errors().get(0).l10nParams()).containsOnly("10");
+  }
+
+  @Test
+  public void list_of_errors() throws Exception {
+    BadRequestException exception = BadRequestException.of(newArrayList(BadRequestException.Message.of("new error")));
+
+    assertThat(exception.errors()).hasSize(1);
+    assertThat(exception.errors().get(0).text()).isEqualTo("new error");
+  }
+
   @Test
   public void l10n_errors() throws Exception {
     BadRequestException exception = BadRequestException.ofL10n("issue.error.123", "10");
@@ -49,8 +66,33 @@ public class BadRequestExceptionTest {
     assertThat(exception.l10nParams()).containsOnly("10");
   }
 
+  @Test
+  public void l10n_errors_with_list_of_errors() throws Exception {
+    BadRequestException exception = BadRequestException.ofL10n(newArrayList(BadRequestException.Message.of("new error")), "issue.error.123", "10");
+    assertThat(exception.getMessage()).isNull();
+    assertThat(exception.l10nKey()).isEqualTo("issue.error.123");
+    assertThat(exception.l10nParams()).containsOnly("10");
+    assertThat(exception.errors()).hasSize(1);
+    assertThat(exception.errors().get(0).text()).isEqualTo("new error");
+  }
+
   @Test
   public void test_equals_and_hashcode() throws Exception {
+    BadRequestException.Message msg = BadRequestException.Message.of("error1");
+    BadRequestException.Message sameMsg = BadRequestException.Message.of("error1");
+    BadRequestException.Message msg2 = BadRequestException.Message.of("error2");
+
+    assertThat(msg.toString()).contains("error1");
+    assertThat(msg).isEqualTo(sameMsg);
+    assertThat(msg).isEqualTo(sameMsg);
+    assertThat(msg.hashCode()).isEqualTo(msg.hashCode());
+    assertThat(msg.hashCode()).isEqualTo(sameMsg.hashCode());
+
+    assertThat(msg).isNotEqualTo(msg2);
+  }
+
+  @Test
+  public void test_equals_and_hashcode_on_l10n() throws Exception {
     BadRequestException.Message msg = BadRequestException.Message.ofL10n("error.123", "10");
     BadRequestException.Message sameMsg = BadRequestException.Message.ofL10n("error.123", "10");
     BadRequestException.Message msg2 = BadRequestException.Message.ofL10n("error.123", "200");

diff --git a/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java b/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java
index 4bce70a3394c96a4e4b2dbed3a007e06f023ceb1..894fe7061e8782b5cbb0046f2c9f40922db88d73 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java
@@ -33,6 +33,8 @@ import org.sonar.core.resource.ResourceDto;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.user.MockUserSession;
 
+import java.util.Collections;
+
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
 import static org.mockito.Mockito.*;
@@ -106,7 +108,7 @@ public class SourceServiceTest {
     String componentKey = "org.sonar.sample:Sample";
     MockUserSession.set().addProjectPermissions(UserRole.CODEVIEWER, projectKey);
     when(resourceDao.getRootProjectByComponentKey(componentKey)).thenReturn(new ResourceDto().setKey(projectKey));
-    when(sourceDecorator.getDecoratedSourceAsHtml(eq(componentKey), anyInt(), anyInt())).thenReturn(null);
+    when(sourceDecorator.getDecoratedSourceAsHtml(eq(componentKey), anyInt(), anyInt())).thenReturn(Collections.<String>emptyList());
 
     service.getSourcesByComponent(componentKey, 1, 2);