From: Duarte Meneses Date: Wed, 25 Apr 2018 15:19:28 +0000 (+0200) Subject: SONAR-10544 Apply feedback X-Git-Tag: 7.5~1299 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=557b62ed2730744c4493d164bf6d20a66e10bc49;p=sonarqube.git SONAR-10544 Apply feedback --- diff --git a/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/XooPluginTest.java b/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/XooPluginTest.java index 6384ffa58c6..0f1b2d2feb6 100644 --- a/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/XooPluginTest.java +++ b/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/XooPluginTest.java @@ -27,6 +27,7 @@ import org.sonar.api.internal.PluginContextImpl; import org.sonar.api.internal.SonarRuntimeImpl; import org.sonar.api.utils.Version; import org.sonar.xoo.lang.CpdTokenizerSensor; +import org.sonar.xoo.rule.OneExternalIssuePerLineSensor; import static org.assertj.core.api.Assertions.assertThat; @@ -61,6 +62,6 @@ public class XooPluginTest { SonarRuntime runtime = SonarRuntimeImpl.forSonarQube(Version.parse("7.2"), SonarQubeSide.SCANNER); Plugin.Context context = new PluginContextImpl.Builder().setSonarRuntime(runtime).build(); new XooPlugin().define(context); - assertThat(context.getExtensions()).hasSize(52).contains(CpdTokenizerSensor.class); + assertThat(context.getExtensions()).hasSize(52).contains(OneExternalIssuePerLineSensor.class); } } diff --git a/server/sonar-db-core/src/main/resources/org/sonar/db/version/schema-h2.ddl b/server/sonar-db-core/src/main/resources/org/sonar/db/version/schema-h2.ddl index 627c4631e43..35a9732f71a 100644 --- a/server/sonar-db-core/src/main/resources/org/sonar/db/version/schema-h2.ddl +++ b/server/sonar-db-core/src/main/resources/org/sonar/db/version/schema-h2.ddl @@ -188,7 +188,7 @@ CREATE TABLE "RULES" ( "DESCRIPTION_FORMAT" VARCHAR(20), "PRIORITY" INTEGER, "IS_TEMPLATE" BOOLEAN DEFAULT FALSE, - "IS_EXTERNAL" BOOLEAN DEFAULT FALSE, + "IS_EXTERNAL" BOOLEAN, "TEMPLATE_ID" INTEGER, "PLUGIN_CONFIG_KEY" VARCHAR(200), "NAME" VARCHAR(200), diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDaoTest.java index 17f82225c37..03bad9b6c8b 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDaoTest.java @@ -537,7 +537,6 @@ public class RuleDaoTest { .setConfigKey("NewConfigKey") .setSeverity(Severity.INFO) .setIsTemplate(true) - .setIsExternal(true) .setLanguage("dart") .setTemplateId(3) .setDefRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.toString()) @@ -563,7 +562,7 @@ public class RuleDaoTest { assertThat(ruleDto.getSeverity()).isEqualTo(0); assertThat(ruleDto.getLanguage()).isEqualTo("dart"); assertThat(ruleDto.isTemplate()).isTrue(); - assertThat(ruleDto.isExternal()).isTrue(); + assertThat(ruleDto.isExternal()).isFalse(); assertThat(ruleDto.getTemplateId()).isEqualTo(3); assertThat(ruleDto.getDefRemediationFunction()).isEqualTo("LINEAR_OFFSET"); assertThat(ruleDto.getDefRemediationGapMultiplier()).isEqualTo("5d"); @@ -834,10 +833,10 @@ public class RuleDaoTest { assertThat(firstRule.getType()).isEqualTo(r1.getType()); assertThat(firstRule.getCreatedAt()).isEqualTo(r1.getCreatedAt()); assertThat(firstRule.getUpdatedAt()).isEqualTo(r1.getUpdatedAt()); - + RuleForIndexingDto secondRule = it.next(); assertThat(secondRule.isExternal()).isTrue(); - + } @Test @@ -988,8 +987,7 @@ public class RuleDaoTest { .extracting(DeprecatedRuleKeyDto::getNewRepositoryKey, DeprecatedRuleKeyDto::getNewRuleKey) .containsExactly( tuple(null, null), - tuple(null, null) - ); + tuple(null, null)); } @Test @@ -1019,7 +1017,7 @@ public class RuleDaoTest { @Test public void deleteDeprecatedRuleKeys() { DeprecatedRuleKeyDto deprecatedRuleKeyDto1 = db.rules().insertDeprecatedKey(); - db.rules().insertDeprecatedKey();; + db.rules().insertDeprecatedKey(); assertThat(underTest.selectAllDeprecatedRuleKeys(db.getSession())).hasSize(2); diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/AddRuleExternal.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/AddRuleExternal.java index 20ef409ab14..89e217eee8d 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/AddRuleExternal.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/AddRuleExternal.java @@ -38,7 +38,6 @@ public class AddRuleExternal extends DdlChange { .addColumn(BooleanColumnDef.newBooleanColumnDefBuilder() .setColumnName("is_external") .setIsNullable(true) - .setDefaultValue(false) .build()) .build()); } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72.java index dd22e5fb441..cb3eb5ae31e 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72.java @@ -30,6 +30,7 @@ public class DbVersion72 implements DbVersion { .add(2100, "Increase size of USERS.CRYPTED_PASSWORD", IncreaseCryptedPasswordSize.class) .add(2101, "Add HASH_METHOD to table users", AddHashMethodToUsersTable.class) .add(2102, "Populate HASH_METHOD on table users", PopulateHashMethodOnUsers.class) - .add(2103, "Add isExternal boolean to rules", AddRuleExternal.class); + .add(2103, "Add isExternal boolean to rules", AddRuleExternal.class) + ; } } diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72Test.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72Test.java index 31fa3c4f6ee..7676fe52168 100644 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72Test.java +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/DbVersion72Test.java @@ -28,7 +28,7 @@ public class DbVersion72Test { private DbVersion72 underTest = new DbVersion72(); @Test - public void migrationNumber_starts_at_2000() { + public void migrationNumber_starts_at_2100() { verifyMinimumMigrationNumber(underTest, 2100); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRule.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRule.java index 8b85d1486f9..1be91e3b605 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRule.java @@ -20,6 +20,7 @@ package org.sonar.server.computation.task.projectanalysis.issue; import java.util.Collections; +import java.util.Objects; import java.util.Set; import javax.annotation.concurrent.Immutable; import org.sonar.api.rule.RuleKey; @@ -34,18 +35,12 @@ public class NewExternalRule implements Rule { private final String pluginKey; private NewExternalRule(Builder builder) { - this.key = checkNotNull(builder.key, "key"); + Objects.requireNonNull(builder.key, "'key' not expected to be null for an external rule"); + this.key = builder.key; this.pluginKey = builder.pluginKey; this.name = builder.name; } - private static T checkNotNull(T obj, String name) { - if (obj == null) { - throw new IllegalStateException("'" + name + "' not expected to be null for an external rule"); - } - return obj; - } - @Override public int getId() { return 0; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/Rule.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/Rule.java index a417dc95cdf..f5225d5f1db 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/Rule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/Rule.java @@ -36,6 +36,9 @@ public interface Rule { RuleStatus getStatus(); + /** + * Will be null for external rules + */ @CheckForNull RuleType getType(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistExternalRulesStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistExternalRulesStep.java index e51c1c895de..af997edf208 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistExternalRulesStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistExternalRulesStep.java @@ -39,8 +39,6 @@ public class PersistExternalRulesStep implements ComputationStep { try (DbSession dbSession = dbClient.openSession(false)) { ruleRepository.persistNewExternalRules(dbSession); - dbSession.flushStatements(); - dbSession.commit(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRuleTest.java index f973b28b7b8..16ee04142b7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRuleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/NewExternalRuleTest.java @@ -46,7 +46,7 @@ public class NewExternalRuleTest { @Test public void fail_if_rule_key_is_not_set() { - exception.expect(IllegalStateException.class); + exception.expect(NullPointerException.class); exception.expectMessage("'key' not expected to be null for an external rule"); new NewExternalRule.Builder() diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistIssuesStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistIssuesStepTest.java index 5e6ae3631eb..d25c96d6b20 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistIssuesStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistIssuesStepTest.java @@ -50,7 +50,6 @@ import org.sonar.server.computation.task.projectanalysis.issue.IssueCache; import org.sonar.server.computation.task.projectanalysis.issue.RuleRepositoryImpl; import org.sonar.server.computation.task.projectanalysis.issue.UpdateConflictResolver; import org.sonar.server.computation.task.step.ComputationStep; -import org.sonar.server.es.EsTester; import org.sonar.server.rule.ExternalRuleCreator; import org.sonar.server.util.cache.DiskCache; @@ -84,9 +83,6 @@ public class PersistIssuesStepTest extends BaseStepTest { private IssueCache issueCache; private ComputationStep step; - @org.junit.Rule - public EsTester es = EsTester.create(); - private ExternalRuleCreator externalRuleCreator = mock(ExternalRuleCreator.class); @Override diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java index cc5c1b113a0..be31ef5a637 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java @@ -111,17 +111,15 @@ public class TransitionActionTest { @Test public void do_not_allow_transitions_for_issues_from_external_rule_engine() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("No transition allowed on issue from externally define rule"); - loginAndAddProjectPermission("john", ISSUE_ADMIN); context.issue() .setIsFromExternalRuleEngine(true) .setStatus(STATUS_CLOSED); + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("No transition allowed on issue from externally define rule"); action.execute(ImmutableMap.of("transition", "close"), context); - } @Test diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/Issue.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/Issue.java index 0f752778775..cb4deefe53a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/Issue.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/Issue.java @@ -36,7 +36,7 @@ public interface Issue extends IIssue { */ List locations(); } - + /** * Effort to fix the issue. Used by technical debt model. * @deprecated since 5.5 use {@link #gap()} @@ -51,13 +51,13 @@ public interface Issue extends IIssue { */ @CheckForNull Double gap(); - + /** * Overridden severity. */ @CheckForNull Severity overriddenSeverity(); - + /** * Primary locations for this issue. * @since 5.2