SONAR-10544 Apply feedback

13 files changed, 23 insertions, 34 deletions

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> 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<IssueLocation> 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