From f3191d8b198d71a263b273271194992b47b3b84a Mon Sep 17 00:00:00 2001 From: Klaudio Sinani Date: Thu, 28 Jul 2022 20:53:58 +0200 Subject: [PATCH] SONAR-12077 Do not record file move events in issue changelog --- .../issue/MovedIssueVisitor.java | 12 ++--- .../issue/ProjectTrackerBaseLazyInput.java | 17 +++---- .../issue/MovedIssueVisitorTest.java | 14 ++---- .../sonar/db/issue/IssueChangeDaoTest.java | 2 +- .../sonar/server/issue/IssueFieldsSetter.java | 11 ++--- .../server/issue/IssueFieldsSetterTest.java | 33 +++++++------- .../server/issue/IssueChangeWSSupport.java | 2 +- .../server/issue/ws/ChangelogAction.java | 1 - .../server/issue/ws/ChangelogActionTest.java | 44 ++----------------- 9 files changed, 41 insertions(+), 95 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitor.java index 9a11b4078ae..e1091282746 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitor.java @@ -21,12 +21,11 @@ package org.sonar.ce.task.projectanalysis.issue; import java.util.Date; import java.util.Optional; -import org.sonar.ce.task.projectanalysis.component.Component; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.IssueChangeContext; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; +import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository.OriginalFile; +import org.sonar.core.issue.DefaultIssue; import org.sonar.server.issue.IssueFieldsSetter; import static com.google.common.base.Preconditions.checkState; @@ -56,10 +55,7 @@ public class MovedIssueVisitor extends IssueVisitor { "Issue %s doesn't belong to file %s registered as original file of current file %s", issue, originalFile.getUuid(), component); - // changes the issue's component uuid, add a change and set issue as changed to enforce it is persisted to DB - issueUpdater.setIssueMoved(issue, component.getUuid(), IssueChangeContext.createUser(new Date(analysisMetadataHolder.getAnalysisDate()), null)); - // other fields (such as module, modulePath, componentKey) are read-only and set/reset for consistency only - issue.setComponentKey(component.getKey()); - issue.setModuleUuidPath(null); + // changes the issue's component uuid, and set issue as changed to enforce it is persisted to DB + issueUpdater.setIssueComponent(issue, component.getUuid(), component.getKey(), new Date(analysisMetadataHolder.getAnalysisDate())); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInput.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInput.java index 9de670a009c..3939ef14ee1 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInput.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInput.java @@ -45,12 +45,12 @@ import static org.apache.commons.lang.StringUtils.trimToEmpty; class ProjectTrackerBaseLazyInput extends BaseInputFactory.BaseLazyInput { - private AnalysisMetadataHolder analysisMetadataHolder; - private ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues; - private DbClient dbClient; - private IssueFieldsSetter issueUpdater; - private ComponentIssuesLoader issuesLoader; - private ReportModulesPath reportModulesPath; + private final AnalysisMetadataHolder analysisMetadataHolder; + private final ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues; + private final DbClient dbClient; + private final IssueFieldsSetter issueUpdater; + private final ComponentIssuesLoader issuesLoader; + private final ReportModulesPath reportModulesPath; ProjectTrackerBaseLazyInput(AnalysisMetadataHolder analysisMetadataHolder, ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues, DbClient dbClient, IssueFieldsSetter issueUpdater, ComponentIssuesLoader issuesLoader, ReportModulesPath reportModulesPath, Component component) { @@ -108,10 +108,7 @@ class ProjectTrackerBaseLazyInput extends BaseInputFactory.BaseLazyInput { if (StringUtils.isNotBlank(modulePath)) { issueUpdater.setMessage(i, "[" + modulePath + "] " + i.getMessage(), context); } - issueUpdater.setIssueMoved(i, component.getUuid(), context); - // other fields (such as module, modulePath, componentKey) are read-only and set/reset for consistency only - i.setComponentKey(component.getKey()); - i.setModuleUuidPath(null); + issueUpdater.setIssueComponent(i, component.getUuid(), component.getKey(), context.date()); } return issuesOnModule; } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitorTest.java index 75c5edb936a..15f88d5a3cd 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MovedIssueVisitorTest.java @@ -23,19 +23,15 @@ import java.util.Date; import java.util.Optional; import org.junit.Before; import org.junit.Test; -import org.mockito.ArgumentCaptor; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.ReportComponent; import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.IssueChangeContext; import org.sonar.server.issue.IssueFieldsSetter; -import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -54,8 +50,8 @@ public class MovedIssueVisitorTest { @org.junit.Rule public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule(); - private MovedFilesRepository movedFilesRepository = mock(MovedFilesRepository.class); - private MovedIssueVisitor underTest = new MovedIssueVisitor(analysisMetadataHolder, movedFilesRepository, new IssueFieldsSetter()); + private final MovedFilesRepository movedFilesRepository = mock(MovedFilesRepository.class); + private final MovedIssueVisitor underTest = new MovedIssueVisitor(analysisMetadataHolder, movedFilesRepository, new IssueFieldsSetter()); @Before public void setUp() { @@ -117,12 +113,8 @@ public class MovedIssueVisitorTest { verify(issue).setComponentUuid(FILE.getUuid()); verify(issue).setComponentKey(FILE.getKey()); verify(issue).setModuleUuidPath(null); + verify(issue).setUpdateDate(new Date(ANALYSIS_DATE)); verify(issue).setChanged(true); - ArgumentCaptor issueChangeContextCaptor = ArgumentCaptor.forClass(IssueChangeContext.class); - verify(issue).setFieldChange(issueChangeContextCaptor.capture(), eq("file"), eq(originalFile.getUuid()), eq(FILE.getUuid())); - assertThat(issueChangeContextCaptor.getValue().date()).isEqualTo(new Date(ANALYSIS_DATE)); - assertThat(issueChangeContextCaptor.getValue().userUuid()).isNull(); - assertThat(issueChangeContextCaptor.getValue().scan()).isFalse(); } private DefaultIssue mockIssue(String fileUuid) { diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java index 40879d7a512..17908dcaa96 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java @@ -48,7 +48,7 @@ public class IssueChangeDaoTest { @Rule public DbTester db = DbTester.create(System2.INSTANCE); - private IssueChangeDao underTest = db.getDbClient().issueChangeDao(); + private final IssueChangeDao underTest = db.getDbClient().issueChangeDao(); @Test public void select_issue_changelog_from_issue_key() { diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java index bdcc5837f13..2b027eb37ac 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java @@ -329,15 +329,16 @@ public class IssueFieldsSetter { return false; } - public boolean setIssueMoved(DefaultIssue issue, String newComponentUuid, IssueChangeContext context) { + public void setIssueComponent(DefaultIssue issue, String newComponentUuid, String newComponentKey, Date updateDate) { if (!Objects.equals(newComponentUuid, issue.componentUuid())) { - issue.setFieldChange(context, FILE, issue.componentUuid(), newComponentUuid); issue.setComponentUuid(newComponentUuid); - issue.setUpdateDate(context.date()); + issue.setUpdateDate(updateDate); issue.setChanged(true); - return true; } - return false; + + // other fields (such as module, modulePath, componentKey) are read-only and set/reset for consistency only + issue.setComponentKey(newComponentKey); + issue.setModuleUuidPath(null); } private static boolean relevantDateDifference(@Nullable Date left, @Nullable Date right) { diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java index b70ea011845..c0835a1b701 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java @@ -21,7 +21,6 @@ package org.sonar.server.issue; import java.util.Calendar; import java.util.Date; -import java.util.Map; import java.util.Random; import org.apache.commons.lang.time.DateUtils; import org.junit.Test; @@ -45,9 +44,9 @@ public class IssueFieldsSetterTest { private final String DEFAULT_RULE_DESCRIPTION_CONTEXT_KEY = "spring"; - private DefaultIssue issue = new DefaultIssue(); - private IssueChangeContext context = IssueChangeContext.createUser(new Date(), "user_uuid"); - private IssueFieldsSetter underTest = new IssueFieldsSetter(); + private final DefaultIssue issue = new DefaultIssue(); + private final IssueChangeContext context = IssueChangeContext.createUser(new Date(), "user_uuid"); + private final IssueFieldsSetter underTest = new IssueFieldsSetter(); @Test public void assign() { @@ -479,36 +478,34 @@ public class IssueFieldsSetterTest { } @Test - public void setIssueMoved_has_no_effect_if_component_uuid_is_not_changed() { - String componentUuid = "a"; + public void setIssueComponent_has_no_effect_if_component_uuid_is_not_changed() { + String componentKey = "key"; + String componentUuid = "uuid"; + issue.setComponentUuid(componentUuid); + issue.setComponentKey(componentKey); - underTest.setIssueMoved(issue, componentUuid, context); + underTest.setIssueComponent(issue, componentUuid, componentKey, context.date()); - assertThat(issue.changes()).isEmpty(); assertThat(issue.componentUuid()).isEqualTo(componentUuid); + assertThat(issue.componentKey()).isEqualTo(componentKey); assertThat(issue.isChanged()).isFalse(); assertThat(issue.updateDate()).isNull(); assertThat(issue.mustSendNotifications()).isFalse(); } @Test - public void setIssueMoved_changes_componentUuid_adds_a_change() { + public void setIssueComponent_changes_component_uuid() { String oldComponentUuid = "a"; String newComponentUuid = "b"; + String componentKey = "key"; + issue.setComponentUuid(oldComponentUuid); - underTest.setIssueMoved(issue, newComponentUuid, context); + underTest.setIssueComponent(issue, newComponentUuid, componentKey, context.date()); - assertThat(issue.changes()).hasSize(1); - FieldDiffs fieldDiffs = issue.changes().get(0); - assertThat(fieldDiffs.creationDate()).isEqualTo(context.date()); - assertThat(fieldDiffs.diffs()).hasSize(1); - Map.Entry entry = fieldDiffs.diffs().entrySet().iterator().next(); - assertThat(entry.getKey()).isEqualTo("file"); - assertThat(entry.getValue().oldValue()).isEqualTo(oldComponentUuid); - assertThat(entry.getValue().newValue()).isEqualTo(newComponentUuid); assertThat(issue.componentUuid()).isEqualTo(newComponentUuid); + assertThat(issue.componentKey()).isEqualTo(componentKey); assertThat(issue.isChanged()).isTrue(); assertThat(issue.updateDate()).isEqualTo(DateUtils.truncate(context.date(), Calendar.SECOND)); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/IssueChangeWSSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/IssueChangeWSSupport.java index 2233608a075..373841895e0 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/IssueChangeWSSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/IssueChangeWSSupport.java @@ -96,7 +96,7 @@ public class IssueChangeWSSupport { } public FormattingContext newFormattingContext(DbSession dbSession, Set dtos, Load load) { - return newFormattingContext(dbSession, dtos, load, ImmutableSet.of(), ImmutableSet.of()); + return newFormattingContext(dbSession, dtos, load, Set.of(), Set.of()); } public FormattingContext newFormattingContext(DbSession dbSession, Set dtos, Load load, Set preloadedUsers, Set preloadedComponents) { diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/ChangelogAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/ChangelogAction.java index 96de378b70a..848b32b3f65 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/ChangelogAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/ChangelogAction.java @@ -39,7 +39,6 @@ import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_CHANGELOG; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE; public class ChangelogAction implements IssuesWsAction { - private final DbClient dbClient; private final IssueFinder issueFinder; private final IssueChangeWSSupport issueChangeSupport; diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/ChangelogActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/ChangelogActionTest.java index 0e8a52b1bf1..fedb3a435e1 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/ChangelogActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/ChangelogActionTest.java @@ -52,8 +52,6 @@ import static org.assertj.core.groups.Tuple.tuple; import static org.sonar.api.web.UserRole.CODEVIEWER; import static org.sonar.api.web.UserRole.USER; import static org.sonar.db.component.ComponentTesting.newFileDto; -import static org.sonar.db.issue.IssueTesting.newDto; -import static org.sonar.db.rule.RuleTesting.newRuleDto; import static org.sonar.db.user.UserTesting.newUserDto; import static org.sonar.test.JsonAssert.assertJson; @@ -68,10 +66,10 @@ public class ChangelogActionTest { private ComponentDto project; private ComponentDto file; - private IssueFinder issueFinder = new IssueFinder(db.getDbClient(), userSession); - private IssueChangeWSSupport issueChangeSupport = new IssueChangeWSSupport(db.getDbClient(), new AvatarResolverImpl(), userSession); - private ChangelogAction underTest = new ChangelogAction(db.getDbClient(), issueFinder, issueChangeSupport); - private WsActionTester tester = new WsActionTester(underTest); + private final IssueFinder issueFinder = new IssueFinder(db.getDbClient(), userSession); + private final IssueChangeWSSupport issueChangeSupport = new IssueChangeWSSupport(db.getDbClient(), new AvatarResolverImpl(), userSession); + private final ChangelogAction underTest = new ChangelogAction(db.getDbClient(), issueFinder, issueChangeSupport); + private final WsActionTester tester = new WsActionTester(underTest); @Before public void setUp() { @@ -98,40 +96,6 @@ public class ChangelogActionTest { assertThat(result.getChangelogList().get(0).getDiffsList()).extracting(Diff::getKey, Diff::getOldValue, Diff::getNewValue).containsOnly(tuple("severity", "MAJOR", "BLOCKER")); } - @Test - public void changelog_of_file_move_contains_file_names() { - RuleDto rule = db.rules().insert(newRuleDto()); - ComponentDto project = db.components().insertPrivateProject(); - ComponentDto file1 = db.components().insertComponent(newFileDto(project)); - ComponentDto file2 = db.components().insertComponent(newFileDto(project)); - IssueDto issueDto = db.issues().insertIssue(newDto(rule, file2, project)); - userSession.logIn("john") - .addProjectPermission(USER, project, file); - db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setDiff("file", file1.uuid(), file2.uuid()).setCreationDate(new Date())); - - ChangelogWsResponse result = call(issueDto.getKey()); - - assertThat(result.getChangelogList()).hasSize(1); - assertThat(result.getChangelogList().get(0).hasUser()).isFalse(); - assertThat(result.getChangelogList().get(0).getCreationDate()).isNotEmpty(); - assertThat(result.getChangelogList().get(0).getDiffsList()).extracting(Diff::getKey, Diff::getOldValue, Diff::getNewValue) - .containsOnly(tuple("file", file1.longName(), file2.longName())); - } - - @Test - public void changelog_of_file_move_is_empty_when_files_does_not_exists() { - IssueDto issueDto = insertNewIssue(); - userSession.logIn("john") - .addProjectPermission(USER, project, file); - db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setDiff("file", "UNKNOWN_1", "UNKNOWN_2").setCreationDate(new Date())); - - ChangelogWsResponse result = call(issueDto.getKey()); - - assertThat(result.getChangelogList()).hasSize(1); - assertThat(result.getChangelogList().get(0).getDiffsList()).extracting(Diff::getKey, Diff::hasOldValue, Diff::hasNewValue) - .containsOnly(tuple("file", false, false)); - } - @Test public void return_changelog_on_user_without_email() { UserDto user = db.users().insertUser(UserTesting.newUserDto("john", "John", null)); -- 2.39.5