From: Julien Lancelot Date: Wed, 5 Feb 2014 09:35:03 +0000 (+0100) Subject: Fix quality flaws X-Git-Tag: 4.2~253 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=344057340c64db0087497615169112178323e48c;p=sonarqube.git 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 58bf7be0917..0f9240c93f0 100644 --- 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 errors = newArrayList(); + private final List errors; public BadRequestException(String message) { super(BAD_REQUEST, message); + this.errors = newArrayList(); } - public BadRequestException(@Nullable String message, List errors) { + public BadRequestException(String message, List 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 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 1fa32a29f6a..aee8e61c1be 100644 --- 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 99f5d1cc555..8de29df6e3f 100644 --- 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 7f40c7079ae..8759f9a0b1f 100644 --- 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 a6bb4e2e1b3..169cc179dda 100644 --- 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 e63bfa72ef6..24f80e9b49c 100644 --- 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 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 8c742b0f286..c2ce1faa812 100644 --- 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 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 8f8948c7189..c2ef0708d36 100644 --- 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 4bce70a3394..894fe7061e8 100644 --- 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.emptyList()); service.getSourcesByComponent(componentKey, 1, 2);