]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-19197 Add code variant logic to compute engine
authorAlain Kermis <alain.kermis@sonarsource.com>
Fri, 5 May 2023 14:27:04 +0000 (16:27 +0200)
committersonartech <sonartech@sonarsource.com>
Tue, 16 May 2023 20:02:49 +0000 (20:02 +0000)
server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorIT.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java
server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java
server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java
server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java

index bd142496fc7d7e146cd96c142daaf2f500171c67..fd2da91f154c7ed1aa7e0c92cace6d96b9307487 100644 (file)
@@ -22,16 +22,16 @@ package org.sonar.ce.task.projectanalysis.issue;
 import java.util.Date;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
+import org.jetbrains.annotations.NotNull;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.ArgumentCaptor;
 import org.sonar.api.config.internal.MapSettings;
-import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
-import org.sonar.api.utils.DateUtils;
 import org.sonar.api.utils.System2;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
 import org.sonar.ce.task.projectanalysis.analysis.Branch;
@@ -169,19 +169,16 @@ public class IntegrateIssuesVisitorIT {
 
   @Test
   public void process_new_issue() {
-    ruleRepositoryRule.add(RuleKey.of("xoo", "S001"));
+    ruleRepositoryRule.add(RuleTesting.XOO_X1);
     when(analysisMetadataHolder.isBranch()).thenReturn(true);
-    ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder()
-      .setMsg("the message")
-      .setRuleRepository("xoo")
-      .setRuleKey("S001")
-      .setSeverity(Constants.Severity.BLOCKER)
-      .build();
+    ScannerReport.Issue reportIssue = getReportIssue(RuleTesting.XOO_X1);
     reportReader.putIssues(FILE_REF, singletonList(reportIssue));
 
     underTest.visitAny(FILE);
 
-    assertThat(newArrayList(protoIssueCache.traverse())).hasSize(1);
+    List<DefaultIssue> issues = newArrayList(protoIssueCache.traverse());
+    assertThat(issues).hasSize(1);
+    assertThat(issues.get(0).codeVariants()).containsExactlyInAnyOrder("foo", "bar");
   }
 
   @Test
@@ -191,12 +188,7 @@ public class IntegrateIssuesVisitorIT {
     addBaseIssue(ruleKey);
 
     // Issue from report has severity blocker
-    ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder()
-      .setMsg("new message")
-      .setRuleRepository(ruleKey.repository())
-      .setRuleKey(ruleKey.rule())
-      .setSeverity(Constants.Severity.BLOCKER)
-      .build();
+    ScannerReport.Issue reportIssue = getReportIssue(ruleKey);
     reportReader.putIssues(FILE_REF, singletonList(reportIssue));
 
     underTest.visitAny(FILE);
@@ -213,12 +205,7 @@ public class IntegrateIssuesVisitorIT {
     addBaseIssue(ruleKey);
 
     // Issue from report has severity blocker
-    ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder()
-      .setMsg("the message")
-      .setRuleRepository(ruleKey.repository())
-      .setRuleKey(ruleKey.rule())
-      .setSeverity(Constants.Severity.BLOCKER)
-      .build();
+    ScannerReport.Issue reportIssue = getReportIssue(ruleKey);
     reportReader.putIssues(FILE_REF, singletonList(reportIssue));
 
     underTest.visitAny(FILE);
@@ -230,13 +217,8 @@ public class IntegrateIssuesVisitorIT {
 
   @Test
   public void execute_issue_visitors() {
-    ruleRepositoryRule.add(RuleKey.of("xoo", "S001"));
-    ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder()
-      .setMsg("the message")
-      .setRuleRepository("xoo")
-      .setRuleKey("S001")
-      .setSeverity(Constants.Severity.BLOCKER)
-      .build();
+    ruleRepositoryRule.add(RuleTesting.XOO_X1);
+    ScannerReport.Issue reportIssue = getReportIssue(RuleTesting.XOO_X1);
     reportReader.putIssues(FILE_REF, singletonList(reportIssue));
 
     underTest.visitAny(FILE);
@@ -244,7 +226,7 @@ public class IntegrateIssuesVisitorIT {
     verify(issueVisitor).beforeComponent(FILE);
     verify(issueVisitor).afterComponent(FILE);
     verify(issueVisitor).onIssue(eq(FILE), defaultIssueCaptor.capture());
-    assertThat(defaultIssueCaptor.getValue().ruleKey().rule()).isEqualTo("S001");
+    assertThat(defaultIssueCaptor.getValue().ruleKey().rule()).isEqualTo("x1");
   }
 
   @Test
@@ -275,12 +257,7 @@ public class IntegrateIssuesVisitorIT {
     addBaseIssue(ruleKey);
 
     // Issue from report has severity blocker
-    ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder()
-      .setMsg("new message")
-      .setRuleRepository(ruleKey.repository())
-      .setRuleKey(ruleKey.rule())
-      .setSeverity(Constants.Severity.BLOCKER)
-      .build();
+    ScannerReport.Issue reportIssue = getReportIssue(ruleKey);
     reportReader.putIssues(FILE_REF, singletonList(reportIssue));
     when(fileStatuses.isDataUnchanged(FILE)).thenReturn(true);
 
@@ -312,12 +289,7 @@ public class IntegrateIssuesVisitorIT {
     addBaseIssueOnBranch(ruleKey);
 
     // Issue from report has severity blocker
-    ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder()
-      .setMsg("the message")
-      .setRuleRepository(ruleKey.repository())
-      .setRuleKey(ruleKey.rule())
-      .setSeverity(Constants.Severity.BLOCKER)
-      .build();
+    ScannerReport.Issue reportIssue = getReportIssue(ruleKey);
     reportReader.putIssues(FILE_REF, singletonList(reportIssue));
 
     underTest.visitAny(FILE);
@@ -363,4 +335,15 @@ public class IntegrateIssuesVisitorIT {
     dbTester.getDbClient().issueDao().insert(dbTester.getSession(), issue);
     dbTester.getSession().commit();
   }
+
+  @NotNull
+  private static ScannerReport.Issue getReportIssue(RuleKey ruleKey) {
+    return ScannerReport.Issue.newBuilder()
+      .setMsg("the message")
+      .setRuleRepository(ruleKey.repository())
+      .setRuleKey(ruleKey.rule())
+      .addAllCodeVariants(Set.of("foo", "bar"))
+      .setSeverity(Constants.Severity.BLOCKER)
+      .build();
+  }
 }
index d628e1ddc00e9f7120c1fcd283c0d86bb19b8236..d3c5317cbf9a89ce2c9b69b35cb8c11f298ce07d 100644 (file)
@@ -201,6 +201,7 @@ public class IssueLifecycle {
     updater.setPastMessage(raw, base.getMessage(), base.getMessageFormattings(), changeContext);
     updater.setPastGap(raw, base.gap(), changeContext);
     updater.setPastEffort(raw, base.effort(), changeContext);
+    updater.setCodeVariants(raw, requireNonNull(base.codeVariants()), changeContext);
   }
 
   public void doAutomaticTransition(DefaultIssue issue) {
index a5583f1013de34cb697046704a58830e5f7a863b..27ca7fbbf2f94b749a646c1dd703783072103709 100644 (file)
@@ -191,6 +191,7 @@ public class TrackerRawInputFactory {
       issue.setLocations(dbLocationsBuilder.build());
       issue.setQuickFixAvailable(reportIssue.getQuickFixAvailable());
       issue.setRuleDescriptionContextKey(reportIssue.hasRuleDescriptionContextKey() ? reportIssue.getRuleDescriptionContextKey() : null);
+      issue.setCodeVariants(reportIssue.getCodeVariantsList());
       return issue;
     }
 
index d67e46c0c1a6848d371cdbc9168fc06ff2cbf4d4..a9334b82c2075b42fb2fc6f55905e9e1bfcf90a8 100644 (file)
@@ -47,7 +47,7 @@ import static java.util.Optional.ofNullable;
 
 public class ProtobufIssueDiskCache implements DiskCache<DefaultIssue> {
   private static final String TAGS_SEPARATOR = ",";
-  private static final Splitter TAGS_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings();
+  private static final Splitter STRING_LIST_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings();
 
   private final File file;
   private final System2 system2;
@@ -115,7 +115,8 @@ public class ProtobufIssueDiskCache implements DiskCache<DefaultIssue> {
     defaultIssue.setChecksum(next.hasChecksum() ? next.getChecksum() : null);
     defaultIssue.setAuthorLogin(next.hasAuthorLogin() ? next.getAuthorLogin() : null);
     next.getCommentsList().forEach(c -> defaultIssue.addComment(toDefaultIssueComment(c)));
-    defaultIssue.setTags(ImmutableSet.copyOf(TAGS_SPLITTER.split(next.getTags())));
+    defaultIssue.setTags(ImmutableSet.copyOf(STRING_LIST_SPLITTER.split(next.getTags())));
+    defaultIssue.setCodeVariants(ImmutableSet.copyOf(STRING_LIST_SPLITTER.split(next.getCodeVariants())));
     defaultIssue.setRuleDescriptionContextKey(next.hasRuleDescriptionContextKey() ? next.getRuleDescriptionContextKey() : null);
     defaultIssue.setLocations(next.hasLocations() ? next.getLocations() : null);
     defaultIssue.setIsFromExternalRuleEngine(next.getIsFromExternalRuleEngine());
@@ -167,6 +168,7 @@ public class ProtobufIssueDiskCache implements DiskCache<DefaultIssue> {
     ofNullable(defaultIssue.authorLogin()).ifPresent(builder::setAuthorLogin);
     defaultIssue.defaultIssueComments().forEach(c -> builder.addComments(toProtoComment(c)));
     ofNullable(defaultIssue.tags()).ifPresent(t -> builder.setTags(String.join(TAGS_SEPARATOR, t)));
+    ofNullable(defaultIssue.codeVariants()).ifPresent(codeVariant -> builder.setCodeVariants(String.join(TAGS_SEPARATOR, codeVariant)));
     ofNullable(defaultIssue.getLocations()).ifPresent(l -> builder.setLocations((DbIssues.Locations) l));
     defaultIssue.getRuleDescriptionContextKey().ifPresent(builder::setRuleDescriptionContextKey);
     builder.setIsFromExternalRuleEngine(defaultIssue.isFromExternalRuleEngine());
index 9a0e93bf5a57b7f6d2cd974424ac1b2ada6ab096..8f6456d11ba0a6f35ce0c366d209b269d3741731 100644 (file)
@@ -81,6 +81,7 @@ message Issue {
   optional bool isNoLongerNewCodeReferenceIssue = 43;
   optional string ruleDescriptionContextKey = 44;
   optional sonarqube.db.issues.MessageFormattings messageFormattings = 45;
+  optional string codeVariants = 46;
 }
 
 message Comment {
index c6697f626e79758f4393dbfe465cadfa41dae99a..924ccc81f6db8bf6929356ca48e4ffde635dfc0f 100644 (file)
@@ -20,6 +20,7 @@
 package org.sonar.ce.task.projectanalysis.issue;
 
 import java.util.Date;
+import java.util.Set;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.rules.RuleType;
@@ -353,6 +354,7 @@ public class IssueLifecycleTest {
       .setKey("RAW_KEY")
       .setRuleKey(XOO_X1)
       .setRuleDescriptionContextKey("spring")
+      .setCodeVariants(Set.of("foo", "bar"))
       .setCreationDate(parseDate("2015-10-01"))
       .setUpdateDate(parseDate("2015-10-02"))
       .setCloseDate(parseDate("2015-10-03"));
@@ -390,6 +392,7 @@ public class IssueLifecycleTest {
       .setMessageFormattings(messageFormattings)
       .setGap(15d)
       .setRuleDescriptionContextKey("hibernate")
+      .setCodeVariants(Set.of("donut"))
       .setEffort(Duration.create(15L))
       .setManualSeverity(false)
       .setLocations(issueLocations)
@@ -409,6 +412,7 @@ public class IssueLifecycleTest {
     assertThat(raw.assignee()).isEqualTo("base assignee uuid");
     assertThat(raw.authorLogin()).isEqualTo("base author");
     assertThat(raw.tags()).containsOnly("base tag");
+    assertThat(raw.codeVariants()).containsOnly("foo", "bar");
     assertThat(raw.effort()).isEqualTo(DEFAULT_DURATION);
     assertThat(raw.isOnDisabledRule()).isTrue();
     assertThat(raw.selectedAt()).isEqualTo(1000L);
@@ -422,6 +426,7 @@ public class IssueLifecycleTest {
     verify(updater).setPastSeverity(raw, BLOCKER, issueChangeContext);
     verify(updater).setPastLine(raw, 10);
     verify(updater).setRuleDescriptionContextKey(raw, "hibernate");
+    verify(updater).setCodeVariants(raw, Set.of("donut"), issueChangeContext);
     verify(updater).setPastMessage(raw, "message with code", messageFormattings, issueChangeContext);
     verify(updater).setPastEffort(raw, Duration.create(15L), issueChangeContext);
     verify(updater).setPastLocations(raw, issueLocations);
index 5f0512d311ea6a8a0b1a552b16949f8b1ab70a7d..32b915be9a04fe2f843c25143c0bba82d6d80f71 100644 (file)
@@ -26,6 +26,7 @@ import java.util.Date;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.sonar.api.ce.ComputeEngineSide;
 import org.sonar.api.rules.RuleType;
@@ -64,8 +65,9 @@ public class IssueFieldsSetter {
   public static final String TECHNICAL_DEBT = "technicalDebt";
   public static final String LINE = "line";
   public static final String TAGS = "tags";
+  public static final String CODE_VARIANTS = "code_variants";
 
-  private static final Joiner CHANGELOG_TAG_JOINER = Joiner.on(" ").skipNulls();
+  private static final Joiner CHANGELOG_LIST_JOINER = Joiner.on(" ").skipNulls();
 
   public boolean setType(DefaultIssue issue, RuleType type, IssueChangeContext context) {
     if (!Objects.equals(type, issue.type())) {
@@ -396,8 +398,8 @@ public class IssueFieldsSetter {
     Set<String> oldTags = new HashSet<>(issue.tags());
     if (!oldTags.equals(newTags)) {
       issue.setFieldChange(context, TAGS,
-        oldTags.isEmpty() ? null : CHANGELOG_TAG_JOINER.join(oldTags),
-        newTags.isEmpty() ? null : CHANGELOG_TAG_JOINER.join(newTags));
+        oldTags.isEmpty() ? null : CHANGELOG_LIST_JOINER.join(oldTags),
+        newTags.isEmpty() ? null : CHANGELOG_LIST_JOINER.join(newTags));
       issue.setTags(newTags);
       issue.setUpdateDate(context.date());
       issue.setChanged(true);
@@ -407,6 +409,32 @@ public class IssueFieldsSetter {
     return false;
   }
 
+  public boolean setCodeVariants(DefaultIssue issue, Set<String> currentCodeVariants, IssueChangeContext context) {
+    Set<String> newCodeVariants = getNewCodeVariants(issue);
+    if (!currentCodeVariants.equals(newCodeVariants)) {
+      issue.setFieldChange(context, CODE_VARIANTS,
+        currentCodeVariants.isEmpty() ? null : CHANGELOG_LIST_JOINER.join(currentCodeVariants),
+        newCodeVariants.isEmpty() ? null : CHANGELOG_LIST_JOINER.join(newCodeVariants));
+      issue.setCodeVariants(newCodeVariants);
+      issue.setUpdateDate(context.date());
+      issue.setChanged(true);
+      issue.setSendNotifications(true);
+      return true;
+    }
+    return false;
+  }
+
+  private static Set<String> getNewCodeVariants(DefaultIssue issue) {
+    Set<String> issueCodeVariants = issue.codeVariants();
+    if (issueCodeVariants == null) {
+      return Set.of();
+    }
+    return issueCodeVariants.stream()
+      .map(String::trim)
+      .filter(s -> !s.isEmpty())
+      .collect(Collectors.toSet());
+  }
+
   public void setIssueComponent(DefaultIssue issue, String newComponentUuid, String newComponentKey, Date updateDate) {
     if (!Objects.equals(newComponentUuid, issue.componentUuid())) {
       issue.setComponentUuid(newComponentUuid);
index 375ec976e73c0bf0e3823ec5c9959bc580378cff..29fb552eff342ce19d34943f8af6906a7461ee08 100644 (file)
  */
 package org.sonar.server.issue;
 
+import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Random;
-import java.util.function.Predicate;
+import java.util.Set;
 import org.apache.commons.lang.time.DateUtils;
 import org.junit.Test;
+import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.Duration;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.FieldDiffs;
@@ -45,6 +48,7 @@ import static org.sonar.server.issue.IssueFieldsSetter.RESOLUTION;
 import static org.sonar.server.issue.IssueFieldsSetter.SEVERITY;
 import static org.sonar.server.issue.IssueFieldsSetter.STATUS;
 import static org.sonar.server.issue.IssueFieldsSetter.TECHNICAL_DEBT;
+import static org.sonar.server.issue.IssueFieldsSetter.TYPE;
 import static org.sonar.server.issue.IssueFieldsSetter.UNUSED;
 
 public class IssueFieldsSetterTest {
@@ -133,6 +137,20 @@ public class IssueFieldsSetterTest {
       .hasMessage("It's not possible to update the assignee with this method, please use assign()");
   }
 
+  @Test
+  public void set_type() {
+    issue.setType(RuleType.CODE_SMELL);
+    boolean updated = underTest.setType(issue, RuleType.BUG, context);
+    assertThat(updated).isTrue();
+    assertThat(issue.type()).isEqualTo(RuleType.BUG);
+    assertThat(issue.manualSeverity()).isFalse();
+    assertThat(issue.mustSendNotifications()).isFalse();
+
+    FieldDiffs.Diff diff = issue.currentChange().get(TYPE);
+    assertThat(diff.oldValue()).isEqualTo(RuleType.CODE_SMELL);
+    assertThat(diff.newValue()).isEqualTo(RuleType.BUG);
+  }
+
   @Test
   public void set_severity() {
     boolean updated = underTest.setSeverity(issue, "BLOCKER", context);
@@ -492,6 +510,33 @@ public class IssueFieldsSetterTest {
     assertThat(diff.newValue()).isNull();
   }
 
+  @Test
+  public void setCodeVariants_whenCodeVariantAdded_shouldBeUpdated() {
+    Set<String> currentCodeVariants = new HashSet<>(Arrays.asList("linux"));
+    Set<String> newCodeVariants = new HashSet<>(Arrays.asList("linux", "windows"));
+
+    issue.setCodeVariants(newCodeVariants);
+    boolean updated = underTest.setCodeVariants(issue, currentCodeVariants, context);
+    assertThat(updated).isTrue();
+    assertThat(issue.codeVariants()).contains("linux", "windows");
+
+    FieldDiffs.Diff diff = issue.currentChange().get("code_variants");
+    assertThat(diff.oldValue()).isEqualTo("linux");
+    assertThat(diff.newValue()).isEqualTo("linux windows");
+    assertThat(issue.mustSendNotifications()).isTrue();
+  }
+
+  @Test
+  public void setCodeVariants_whenCodeVariantsUnchanged_shouldNotBeUpdated() {
+    Set<String> currentCodeVariants = new HashSet<>(Arrays.asList("linux", "windows"));
+    Set<String> newCodeVariants = new HashSet<>(Arrays.asList("windows", "linux"));
+
+    issue.setCodeVariants(newCodeVariants);
+    boolean updated = underTest.setCodeVariants(issue, currentCodeVariants, context);
+    assertThat(updated).isFalse();
+    assertThat(issue.currentChange()).isNull();
+  }
+
   @Test
   public void set_message() {
     boolean updated = underTest.setMessage(issue, "the message", context);