From: Julien HENRY Date: Mon, 16 Oct 2017 15:15:49 +0000 (+0200) Subject: SONAR-9970, SONAR-9949 Do not copy 'file' issue changelog entries X-Git-Tag: 6.7-RC1~150 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=eb411c60d9fdce34ee921c75cce377d8779b046a;p=sonarqube.git SONAR-9970, SONAR-9949 Do not copy 'file' issue changelog entries --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycle.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycle.java index 00322067935..b8f51767ed0 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycle.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycle.java @@ -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 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) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycleTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycleTest.java index 5b13b2ea7f8..4bf057872a5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueLifecycleTest.java @@ -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 diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java index df964e68989..534306fde31 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java @@ -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; diff --git a/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java b/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java index 80c7b880fa0..1fad9b371ed 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java @@ -45,18 +45,6 @@ public class FieldDiffs implements Serializable { private final Map 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 diffs() { return diffs; }