]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9664 Backdate new issues of custom rules plugin when base plugin was changed
authorJulien HENRY <julien.henry@sonarsource.com>
Wed, 9 Aug 2017 16:16:51 +0000 (18:16 +0200)
committerJulien HENRY <julien.henry@sonarsource.com>
Thu, 7 Sep 2017 06:33:31 +0000 (08:33 +0200)
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/analysis/ScannerPlugin.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueCreationDateCalculator.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStep.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/analysis/ScannerPluginTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueCreationDateCalculatorTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/LoadReportAnalysisMetadataHolderStepTest.java

index d4eee35e405d1e08d4ed6029d597653b358efdcc..4ac961229ce2e63f698f9e3ea95b72da2d8dd3f7 100644 (file)
@@ -19,6 +19,8 @@
  */
 package org.sonar.server.computation.task.projectanalysis.analysis;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 import static java.util.Objects.requireNonNull;
@@ -26,10 +28,12 @@ import static java.util.Objects.requireNonNull;
 @Immutable
 public class ScannerPlugin {
   private final String key;
+  private final String basePluginKey;
   private final long updatedAt;
 
-  public ScannerPlugin(String key, long updatedAt) {
+  public ScannerPlugin(String key, @Nullable String basePluginKey, long updatedAt) {
     this.key = requireNonNull(key, "key can't be null");
+    this.basePluginKey = basePluginKey;
     this.updatedAt = updatedAt;
   }
 
@@ -37,6 +41,11 @@ public class ScannerPlugin {
     return key;
   }
 
+  @CheckForNull
+  public String getBasePluginKey() {
+    return basePluginKey;
+  }
+
   public long getUpdatedAt() {
     return updatedAt;
   }
@@ -62,6 +71,7 @@ public class ScannerPlugin {
   public String toString() {
     return "ScannerPlugin{" +
       "key='" + key + '\'' +
+      ", basePluginKey='" + basePluginKey + '\'' +
       ", updatedAt='" + updatedAt + '\'' +
       '}';
   }
index 8352a4e54a8d687fdbbf55c971e30fcd258007f0..92d4c4b9015d0aa35db7dd4c214c87425c957eac 100644 (file)
@@ -30,6 +30,7 @@ import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.server.computation.task.projectanalysis.analysis.Analysis;
 import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolder;
+import org.sonar.server.computation.task.projectanalysis.analysis.ScannerPlugin;
 import org.sonar.server.computation.task.projectanalysis.component.Component;
 import org.sonar.server.computation.task.projectanalysis.qualityprofile.ActiveRule;
 import org.sonar.server.computation.task.projectanalysis.qualityprofile.ActiveRulesHolder;
@@ -73,22 +74,35 @@ public class IssueCreationDateCalculator extends IssueVisitor {
     ActiveRule activeRule = toJavaUtilOptional(activeRulesHolder.get(issue.getRuleKey()))
       .orElseThrow(illegalStateException("The rule %s raised an issue, but is not one of the active rules.", issue.getRuleKey()));
     if (firstAnalysis
-      || ruleIsNew(activeRule, lastAnalysisOptional.get())
-      || pluginIsNew(activeRule, lastAnalysisOptional.get())) {
+      || activeRuleIsNew(activeRule, lastAnalysisOptional.get())
+      || ruleImplementationChanged(activeRule, lastAnalysisOptional.get())) {
       getScmChangeDate(component, issue)
         .ifPresent(changeDate -> updateDate(issue, changeDate));
     }
   }
 
-  private boolean pluginIsNew(ActiveRule activeRule, Long lastAnalysisDate) {
+  private boolean ruleImplementationChanged(ActiveRule activeRule, long lastAnalysisDate) {
     String pluginKey = activeRule.getPluginKey();
-    long pluginUpdateDate = Optional.ofNullable(analysisMetadataHolder.getScannerPluginsByKey().get(pluginKey))
-      .orElseThrow(illegalStateException("The rule %s is declared to come from plugin %s, but this plugin was not used by scanner.", activeRule.getRuleKey(), pluginKey))
-      .getUpdatedAt();
-    return lastAnalysisDate < pluginUpdateDate;
+    ScannerPlugin scannerPlugin = Optional.ofNullable(analysisMetadataHolder.getScannerPluginsByKey().get(pluginKey))
+      .orElseThrow(illegalStateException("The rule %s is declared to come from plugin %s, but this plugin was not used by scanner.", activeRule.getRuleKey(), pluginKey));
+    return pluginIsNew(scannerPlugin, lastAnalysisDate)
+      || basePluginIsNew(scannerPlugin, lastAnalysisDate);
   }
 
-  private static boolean ruleIsNew(ActiveRule activeRule, Long lastAnalysisDate) {
+  private boolean basePluginIsNew(ScannerPlugin scannerPlugin, long lastAnalysisDate) {
+    String basePluginKey = scannerPlugin.getBasePluginKey();
+    if (basePluginKey == null) {
+      return false;
+    }
+    ScannerPlugin basePlugin = analysisMetadataHolder.getScannerPluginsByKey().get(basePluginKey);
+    return lastAnalysisDate < basePlugin.getUpdatedAt();
+  }
+
+  private static boolean pluginIsNew(ScannerPlugin scannerPlugin, long lastAnalysisDate) {
+    return lastAnalysisDate < scannerPlugin.getUpdatedAt();
+  }
+
+  private static boolean activeRuleIsNew(ActiveRule activeRule, Long lastAnalysisDate) {
     long ruleCreationDate = activeRule.getCreatedAt();
     return lastAnalysisDate < ruleCreationDate;
   }
index 18658bda389f5cda598519ec343d7dea4f7b05ae..664fb463b9587b8fc76828bbe9f6fc5a45be19a6 100644 (file)
@@ -23,9 +23,12 @@ import com.google.common.base.Joiner;
 import java.util.Date;
 import java.util.List;
 import java.util.Optional;
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonar.api.utils.MessageException;
 import org.sonar.ce.queue.CeTask;
+import org.sonar.core.platform.PluginInfo;
+import org.sonar.core.platform.PluginRepository;
 import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
@@ -62,15 +65,17 @@ public class LoadReportAnalysisMetadataHolderStep implements ComputationStep {
   private final DefaultOrganizationProvider defaultOrganizationProvider;
   private final DbClient dbClient;
   private final BillingValidations billingValidations;
+  private final PluginRepository pluginRepository;
 
   public LoadReportAnalysisMetadataHolderStep(CeTask ceTask, BatchReportReader reportReader, MutableAnalysisMetadataHolder mutableAnalysisMetadataHolder,
-    DefaultOrganizationProvider defaultOrganizationProvider, DbClient dbClient, BillingValidationsProxy billingValidations) {
+    DefaultOrganizationProvider defaultOrganizationProvider, DbClient dbClient, BillingValidationsProxy billingValidations, PluginRepository pluginRepository) {
     this.ceTask = ceTask;
     this.reportReader = reportReader;
     this.mutableAnalysisMetadataHolder = mutableAnalysisMetadataHolder;
     this.defaultOrganizationProvider = defaultOrganizationProvider;
     this.dbClient = dbClient;
     this.billingValidations = billingValidations;
+    this.pluginRepository = pluginRepository;
   }
 
   @Override
@@ -95,10 +100,21 @@ public class LoadReportAnalysisMetadataHolderStep implements ComputationStep {
     mutableAnalysisMetadataHolder.setScannerPluginsByKey(reportMetadata.getPluginsByKey().values().stream()
       .collect(toMap(
         Plugin::getKey,
-        p -> new ScannerPlugin(p.getKey(), p.getUpdatedAt()))));
+        p -> new ScannerPlugin(p.getKey(), getBasePluginKey(p), p.getUpdatedAt()))));
     mutableAnalysisMetadataHolder.setOrganization(organization);
   }
 
+  @CheckForNull
+  private String getBasePluginKey(Plugin p) {
+    PluginInfo pluginInfo = pluginRepository.getPluginInfo(p.getKey());
+    if (pluginInfo == null) {
+      // May happen if plugin was uninstalled between start of scanner analysis and now.
+      // But it doesn't matter since all active rules are removed anyway, so no issues will be reported
+      return null;
+    }
+    return pluginInfo.getBasePlugin();
+  }
+
   /**
    * Check that the Quality profiles sent by scanner correctly relate to the project organization.
    */
index 1322d2526e7ba33a257afd163fc32cdf0635247d..101d8d2fca8195f4472ab62a44c3457632a2b2f3 100644 (file)
@@ -27,33 +27,34 @@ public class ScannerPluginTest {
 
   @Test
   public void verify_getters() {
-    ScannerPlugin plugin = new ScannerPlugin("key", 12345L);
+    ScannerPlugin plugin = new ScannerPlugin("key", "base", 12345L);
 
     assertThat(plugin.getKey()).isEqualTo("key");
+    assertThat(plugin.getBasePluginKey()).isEqualTo("base");
     assertThat(plugin.getUpdatedAt()).isEqualTo(12345L);
   }
 
   @Test
   public void verify_toString() {
-    ScannerPlugin plugin = new ScannerPlugin("key", 12345L);
+    ScannerPlugin plugin = new ScannerPlugin("key", "base", 12345L);
 
-    assertThat(plugin.toString()).isEqualTo("ScannerPlugin{key='key', updatedAt='12345'}");
+    assertThat(plugin.toString()).isEqualTo("ScannerPlugin{key='key', basePluginKey='base', updatedAt='12345'}");
   }
 
   @Test
   public void equals_is_based_on_key_only() {
-    ScannerPlugin plugin = new ScannerPlugin("key", 12345L);
+    ScannerPlugin plugin = new ScannerPlugin("key", "base", 12345L);
 
     assertThat(plugin).isEqualTo(plugin);
-    assertThat(plugin).isEqualTo(new ScannerPlugin("key", 45678L));
-    assertThat(plugin).isNotEqualTo(new ScannerPlugin("key2", 12345L));
+    assertThat(plugin).isEqualTo(new ScannerPlugin("key", null, 45678L));
+    assertThat(plugin).isNotEqualTo(new ScannerPlugin("key2", "base", 12345L));
     assertThat(plugin).isNotEqualTo(null);
     assertThat(plugin).isNotEqualTo("toto");
   }
 
   @Test
   public void hashcode_is_based_on_key_only() {
-    ScannerPlugin plugin = new ScannerPlugin("key", 12345L);
+    ScannerPlugin plugin = new ScannerPlugin("key", "base", 12345L);
 
     assertThat(plugin.hashCode()).isEqualTo("key".hashCode());
   }
index aef9e5af5b0d24ec6103cd396b4ffd536023a97d..26103d3462dab50cbfd8bdb3e7688381f078c2d3 100644 (file)
@@ -99,7 +99,24 @@ public class IssueCreationDateCalculatorTest {
   }
 
   @Test
-  public void should_not_change_date_if_rule_is_old() {
+  public void should_not_change_date_if_rule_and_plugin_and_base_plugin_are_old() {
+    previousAnalysisWas(2000L);
+    currentAnalysisIs(3000L);
+
+    newIssue();
+    withScm(1200L);
+    ruleCreatedAt(1500L);
+    rulePlugin("customjava");
+    pluginUpdatedAt("customjava", "java", 1700L);
+    pluginUpdatedAt("java", 1700L);
+
+    run();
+
+    assertNoChangeOfCreationDate();
+  }
+
+  @Test
+  public void should_not_change_date_if_rule_and_plugin_are_old_and_no_base_plugin() {
     previousAnalysisWas(2000L);
     currentAnalysisIs(3000L);
 
@@ -184,6 +201,23 @@ public class IssueCreationDateCalculatorTest {
     assertChangeOfCreationDateTo(1200L);
   }
 
+  @Test
+  public void should_change_date_if_scm_is_available_and_base_plugin_is_new() {
+    previousAnalysisWas(2000L);
+    currentAnalysisIs(3000L);
+
+    newIssue();
+    withScm(1200L);
+    ruleCreatedAt(1500L);
+    rulePlugin("customjava");
+    pluginUpdatedAt("customjava", "java", 1000L);
+    pluginUpdatedAt("java", 2500L);
+
+    run();
+
+    assertChangeOfCreationDateTo(1200L);
+  }
+
   private void previousAnalysisWas(long analysisDate) {
     when(analysisMetadataHolder.getBaseAnalysis())
       .thenReturn(baseAnalysis);
@@ -192,7 +226,11 @@ public class IssueCreationDateCalculatorTest {
   }
 
   private void pluginUpdatedAt(String pluginKey, long updatedAt) {
-    scannerPlugins.put(pluginKey, new ScannerPlugin(pluginKey, updatedAt));
+    scannerPlugins.put(pluginKey, new ScannerPlugin(pluginKey, null, updatedAt));
+  }
+
+  private void pluginUpdatedAt(String pluginKey, String basePluginKey, long updatedAt) {
+    scannerPlugins.put(pluginKey, new ScannerPlugin(pluginKey, basePluginKey, updatedAt));
   }
 
   private void currentAnalysisIs(long analysisDate) {
index 589c6d5f2a68110e5edec1406daa0be7d004e04c..2b1798b11a64475baff66960bb81da79b5a2a98e 100644 (file)
@@ -27,12 +27,15 @@ import org.mockito.ArgumentCaptor;
 import org.sonar.api.utils.MessageException;
 import org.sonar.api.utils.System2;
 import org.sonar.ce.queue.CeTask;
+import org.sonar.core.platform.PluginInfo;
+import org.sonar.core.platform.PluginRepository;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.scanner.protocol.output.ScannerReport;
 import org.sonar.server.computation.task.projectanalysis.analysis.MutableAnalysisMetadataHolderRule;
 import org.sonar.server.computation.task.projectanalysis.analysis.Organization;
+import org.sonar.server.computation.task.projectanalysis.analysis.ScannerPlugin;
 import org.sonar.server.computation.task.projectanalysis.batch.BatchReportReaderRule;
 import org.sonar.server.computation.task.step.ComputationStep;
 import org.sonar.server.organization.BillingValidations;
@@ -67,6 +70,7 @@ public class LoadReportAnalysisMetadataHolderStepTest {
   private DbClient dbClient = dbTester.getDbClient();
   private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(dbTester);
   private BillingValidationsProxy billingValidations = mock(BillingValidationsProxy.class);
+  private PluginRepository pluginRepository = mock(PluginRepository.class);
   private ComputationStep underTest;
 
   @Before
@@ -378,18 +382,23 @@ public class LoadReportAnalysisMetadataHolderStepTest {
     ScannerReport.Metadata.Builder metadataBuilder = newBatchReportBuilder();
     metadataBuilder.getMutablePluginsByKey().put("java", ScannerReport.Metadata.Plugin.newBuilder().setKey("java").setUpdatedAt(12345L).build());
     metadataBuilder.getMutablePluginsByKey().put("php", ScannerReport.Metadata.Plugin.newBuilder().setKey("php").setUpdatedAt(678910L).build());
+    metadataBuilder.getMutablePluginsByKey().put("customjava", ScannerReport.Metadata.Plugin.newBuilder().setKey("customjava").setUpdatedAt(111111L).build());
+    when(pluginRepository.getPluginInfo("customjava")).thenReturn(new PluginInfo("customjava").setBasePlugin("java"));
+
     reportReader.setMetadata(metadataBuilder.build());
 
     underTest.execute();
 
-    assertThat(analysisMetadataHolder.getScannerPluginsByKey()).containsOnlyKeys("java", "php");
-    assertThat(analysisMetadataHolder.getScannerPluginsByKey().values()).extracting("key", "updatedAt").containsOnly(
-      tuple("java", 12345L),
-      tuple("php", 678910L));
+    assertThat(analysisMetadataHolder.getScannerPluginsByKey()).containsOnlyKeys("java", "php", "customjava");
+    assertThat(analysisMetadataHolder.getScannerPluginsByKey().values()).extracting(ScannerPlugin::getKey, ScannerPlugin::getBasePluginKey, ScannerPlugin::getUpdatedAt)
+      .containsOnly(
+        tuple("java", null, 12345L),
+        tuple("customjava", "java", 111111L),
+        tuple("php", null, 678910L));
   }
 
   private LoadReportAnalysisMetadataHolderStep createStep(CeTask ceTask) {
-    return new LoadReportAnalysisMetadataHolderStep(ceTask, reportReader, analysisMetadataHolder, defaultOrganizationProvider, dbClient, billingValidations);
+    return new LoadReportAnalysisMetadataHolderStep(ceTask, reportReader, analysisMetadataHolder, defaultOrganizationProvider, dbClient, billingValidations, pluginRepository);
   }
 
   private static ScannerReport.Metadata.Builder newBatchReportBuilder() {