]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9970, SONAR-9949 Do not copy 'file' issue changelog entries
authorJulien HENRY <julien.henry@sonarsource.com>
Mon, 16 Oct 2017 15:15:49 +0000 (17:15 +0200)
committerJulien HENRY <julien.henry@sonarsource.com>
Fri, 20 Oct 2017 08:45:15 +0000 (18:45 +1000)
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycle.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycleTest.java
sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java
sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java

index 0032206793591dbe1965a766509359a17ee52332..b8f51767ed09d4ccc83627365b76af6897d46f5b 100644 (file)
@@ -21,7 +21,9 @@ package org.sonar.server.computation.task.projectanalysis.issue;
 
 import com.google.common.annotations.VisibleForTesting;
 import java.util.Date;
+import java.util.Optional;
 import org.sonar.api.issue.Issue;
+import org.sonar.api.issue.IssueComment;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.DefaultIssueComment;
 import org.sonar.core.issue.FieldDiffs;
@@ -92,8 +94,39 @@ public class IssueLifecycle {
   }
 
   private static void copyChanges(DefaultIssue raw, DefaultIssue base) {
-    base.comments().forEach(c -> raw.addComment(DefaultIssueComment.copy(raw.key(), c)));
-    base.changes().forEach(c -> raw.addChange(FieldDiffs.copy(raw.key(), c)));
+    base.comments().forEach(c -> raw.addComment(copy(raw.key(), c)));
+    base.changes().forEach(c -> copy(raw.key(), c).ifPresent(raw::addChange));
+  }
+
+  /**
+   * Copy a comment from another issue
+   */
+  private static DefaultIssueComment copy(String issueKey, IssueComment c) {
+    DefaultIssueComment comment = new DefaultIssueComment();
+    comment.setIssueKey(issueKey);
+    comment.setKey(Uuids.create());
+    comment.setUserLogin(c.userLogin());
+    comment.setMarkdownText(c.markdownText());
+    comment.setCreatedAt(c.createdAt()).setUpdatedAt(c.updatedAt());
+    comment.setNew(true);
+    return comment;
+  }
+
+  /**
+   * Copy a diff from another issue
+   */
+  private static Optional<FieldDiffs> copy(String issueKey, FieldDiffs c) {
+    FieldDiffs result = new FieldDiffs();
+    result.setIssueKey(issueKey);
+    result.setUserLogin(c.userLogin());
+    result.setCreationDate(c.creationDate());
+    // Don't copy "file" changelogs as they refer to file uuids that might later be purged
+    c.diffs().entrySet().stream().filter(e -> !e.getKey().equals(IssueFieldsSetter.FILE))
+      .forEach(e -> result.setDiff(e.getKey(), e.getValue().oldValue(), e.getValue().newValue()));
+    if (result.diffs().isEmpty()) {
+      return Optional.empty();
+    }
+    return Optional.of(result);
   }
 
   public void mergeExistingOpenIssue(DefaultIssue raw, DefaultIssue base) {
index 5b13b2ea7f84c4d8a728488ccd3258e6062ed7dd..4bf057872a55be2f657bff3119776e1be35175b4 100644 (file)
@@ -23,9 +23,11 @@ import com.google.common.collect.ImmutableMap;
 import java.util.Date;
 import org.junit.Rule;
 import org.junit.Test;
+import org.sonar.api.issue.IssueComment;
 import org.sonar.api.utils.Duration;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.DefaultIssueComment;
+import org.sonar.core.issue.FieldDiffs;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.db.protobuf.DbCommons;
 import org.sonar.db.protobuf.DbIssues;
@@ -36,6 +38,7 @@ import org.sonar.server.issue.workflow.IssueWorkflow;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.groups.Tuple.tuple;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -84,8 +87,10 @@ public class IssueLifecycleTest {
 
   @Test
   public void mergeIssueFromShortLivingBranch() {
-    DefaultIssue raw = new DefaultIssue();
-    DefaultIssue fromShort = new DefaultIssue();
+    DefaultIssue raw = new DefaultIssue()
+      .setKey("raw");
+    DefaultIssue fromShort = new DefaultIssue()
+      .setKey("short");
     fromShort.setResolution("resolution");
     fromShort.setStatus("status");
 
@@ -96,15 +101,40 @@ public class IssueLifecycleTest {
       .setUserLogin("user")
       .setMarkdownText("A comment"));
 
+    Date diffDate = new Date();
+    // file diff alone
+    fromShort.addChange(new FieldDiffs()
+      .setCreationDate(diffDate)
+      .setIssueKey("short")
+      .setUserLogin("user")
+      .setDiff("file", "uuidA1", "uuidB1"));
+    // file diff with another field
+    fromShort.addChange(new FieldDiffs()
+      .setCreationDate(diffDate)
+      .setIssueKey("short")
+      .setUserLogin("user")
+      .setDiff("severity", "MINOR", "MAJOR")
+      .setDiff("file", "uuidA2", "uuidB2"));
+
     Branch branch = mock(Branch.class);
     when(branch.getName()).thenReturn("master");
     analysisMetadataHolder.setBranch(branch);
 
     underTest.mergeConfirmedOrResolvedFromShortLivingBranch(raw, fromShort, "feature/foo");
+
     assertThat(raw.resolution()).isEqualTo("resolution");
     assertThat(raw.status()).isEqualTo("status");
-    assertThat(raw.changes().get(0).get(IssueFieldsSetter.FROM_SHORT_BRANCH).oldValue()).isEqualTo("feature/foo");
-    assertThat(raw.changes().get(0).get(IssueFieldsSetter.FROM_SHORT_BRANCH).newValue()).isEqualTo("master");
+    assertThat(raw.comments()).extracting(IssueComment::issueKey, IssueComment::createdAt, IssueComment::userLogin, IssueComment::markdownText)
+      .containsOnly(tuple("raw", commentDate, "user", "A comment"));
+    assertThat(raw.changes()).hasSize(2);
+    assertThat(raw.changes().get(0).creationDate()).isEqualTo(diffDate);
+    assertThat(raw.changes().get(0).userLogin()).isEqualTo("user");
+    assertThat(raw.changes().get(0).issueKey()).isEqualTo("raw");
+    assertThat(raw.changes().get(0).diffs()).containsOnlyKeys("severity");
+    assertThat(raw.changes().get(1).userLogin()).isEqualTo("julien");
+    assertThat(raw.changes().get(1).diffs()).containsOnlyKeys(IssueFieldsSetter.FROM_SHORT_BRANCH);
+    assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_SHORT_BRANCH).oldValue()).isEqualTo("feature/foo");
+    assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_SHORT_BRANCH).newValue()).isEqualTo("master");
   }
 
   @Test
index df964e689899f7dc60319015f5b08d031b867fd7..534306fde319b0557d5cec81cb58cfa6cbe995f7 100644 (file)
@@ -53,20 +53,6 @@ public class DefaultIssueComment implements Serializable, IssueComment {
     return comment;
   }
 
-  /**
-   * Copy a comment from another issue
-   */
-  public static DefaultIssueComment copy(String issueKey, IssueComment c) {
-    DefaultIssueComment comment = new DefaultIssueComment();
-    comment.setIssueKey(issueKey);
-    comment.setKey(Uuids.create());
-    comment.setUserLogin(c.userLogin());
-    comment.setMarkdownText(c.markdownText());
-    comment.setCreatedAt(c.createdAt()).setUpdatedAt(c.updatedAt());
-    comment.setNew(true);
-    return comment;
-  }
-
   @Override
   public String markdownText() {
     return markdownText;
index 80c7b880fa0a81fc13ac9662f6ba5a410e9547b1..1fad9b371ed7faa90604c8ca3924c404cca42863 100644 (file)
@@ -45,18 +45,6 @@ public class FieldDiffs implements Serializable {
 
   private final Map<String, Diff> diffs = Maps.newLinkedHashMap();
 
-  /**
-   * Copy a diff from another issue
-   */
-  public static FieldDiffs copy(String issueKey, FieldDiffs c) {
-    FieldDiffs result = new FieldDiffs();
-    result.issueKey = issueKey;
-    result.userLogin = c.userLogin;
-    result.creationDate = c.creationDate;
-    result.diffs.putAll(c.diffs);
-    return result;
-  }
-
   public Map<String, Diff> diffs() {
     return diffs;
   }