From 53d745f5ab9fee51e251d3f0c8998b18b1a34ab4 Mon Sep 17 00:00:00 2001 From: lukasz-jarocki-sonarsource Date: Fri, 13 Oct 2023 14:03:18 +0200 Subject: [PATCH] SONAR-20317 fixed the issue where creating a portfolio would insert two entries into audit table --- .../component/ComponentUuidFactoryImplIT.java | 4 +-- ...ticipatedTransitionRepositoryImplTest.java | 2 +- .../sonar/db/portfolio/PortfolioDaoIT.java | 8 ++--- .../org/sonar/db/component/ComponentDao.java | 6 ++-- .../org/sonar/db/portfolio/PortfolioDao.java | 10 +++++-- .../org/sonar/db/audit/AuditDbTester.java | 5 ++++ .../sonar/db/component/ComponentDbTester.java | 2 +- .../server/batch/ProjectDataLoaderIT.java | 2 +- .../server/component/ComponentUpdaterIT.java | 29 +++++++++++++++++-- .../sonar/server/source/ws/LinesActionIT.java | 3 +- .../sonar/server/source/ws/ScmActionIT.java | 2 +- .../server/component/ComponentUpdater.java | 2 +- 12 files changed, 54 insertions(+), 21 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryImplIT.java b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryImplIT.java index 6da6104b038..e3d7f74307a 100644 --- a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryImplIT.java +++ b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryImplIT.java @@ -100,9 +100,9 @@ public class ComponentUuidFactoryImplIT { @Test public void getOrCreateForKey_whenNoExistingComponentsInDbForPortfolioAndSubPortfolio_shouldLoadUuidFromPortfolioTable() { PortfolioDto portfolioDto = ComponentTesting.newPortfolioDto("uuid_ptf1", "ptf1", "Portfolio1", null); - db.getDbClient().portfolioDao().insert(db.getSession(), portfolioDto); + db.getDbClient().portfolioDao().insertWithAudit(db.getSession(), portfolioDto); PortfolioDto subPortfolio = ComponentTesting.newPortfolioDto("subPtf1", "sub_ptf_1", "portfolio", portfolioDto); - db.getDbClient().portfolioDao().insert(db.getSession(), subPortfolio); + db.getDbClient().portfolioDao().insertWithAudit(db.getSession(), subPortfolio); ComponentUuidFactory underTest = new ComponentUuidFactoryImpl(db.getDbClient(), db.getSession(), portfolioDto.getKey()); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AnticipatedTransitionRepositoryImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AnticipatedTransitionRepositoryImplTest.java index 1c225bdef74..72cdeadec99 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AnticipatedTransitionRepositoryImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AnticipatedTransitionRepositoryImplTest.java @@ -94,7 +94,7 @@ public class AnticipatedTransitionRepositoryImplTest { dbClient.branchDao().insert(db.getSession(), branchDto); ComponentDto fileDto = getComponentDto(projectKey + ":" + mainFile, branchDto.getUuid()); - dbClient.componentDao().insertOnMainBranch(db.getSession(), fileDto); + dbClient.componentDao().insertWithAudit(db.getSession(), fileDto); insertAnticipatedTransition(projectUuid, mainFile); insertAnticipatedTransition(projectUuid, "file2.js"); diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/portfolio/PortfolioDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/portfolio/PortfolioDaoIT.java index e66fa88cce4..fc1bcc576cf 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/portfolio/PortfolioDaoIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/portfolio/PortfolioDaoIT.java @@ -150,7 +150,7 @@ public class PortfolioDaoIT { PortfolioDto portfolio2 = ComponentTesting.newPortfolioDto("uuid2", "ptf2", "Portfolio 2", null); PortfolioDto subPortfolio2 = ComponentTesting.newPortfolioDto("sub_uuid21", "sub_ptd2", "SubPortfolio 2", portfolio2); Arrays.asList(portfolio1, subPortfolio1, subSubPortfolio1, portfolio2, subPortfolio2) - .forEach(portfolio -> portfolioDao.insert(db.getSession(), portfolio)); + .forEach(portfolio -> portfolioDao.insertWithAudit(db.getSession(), portfolio)); List keyWithUuidDtos = portfolioDao.selectUuidsByKey(db.getSession(), portfolio1.getKey()); @@ -169,14 +169,14 @@ public class PortfolioDaoIT { .setUuid("uuid") .setParentUuid(null) .setRootUuid("root"); - assertThatThrownBy(() -> portfolioDao.insert(session, portfolio)) + assertThatThrownBy(() -> portfolioDao.insertWithAudit(session, portfolio)) .isInstanceOf(IllegalArgumentException.class); PortfolioDto portfolio2 = new PortfolioDto() .setUuid("uuid") .setParentUuid("parent") .setRootUuid("uuid"); - assertThatThrownBy(() -> portfolioDao.insert(session, portfolio2)) + assertThatThrownBy(() -> portfolioDao.insertWithAudit(session, portfolio2)) .isInstanceOf(IllegalArgumentException.class); } @@ -746,7 +746,7 @@ public class PortfolioDaoIT { .setRootUuid(parent.getRootUuid()) .setParentUuid(parent.getUuid()) .setUuid(uuid); - db.getDbClient().portfolioDao().insert(session, portfolio); + db.getDbClient().portfolioDao().insertWithAudit(session, portfolio); session.commit(); return portfolio; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java index 0d64637e9e4..e4dbb188d5f 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java @@ -289,14 +289,14 @@ public class ComponentDao implements Dao { /* INSERT / UPDATE */ - public void insert(DbSession session, ComponentDto item, boolean isMainBranch) { + public void insert(DbSession session, ComponentDto item, boolean shouldPersistAudit) { mapper(session).insert(item); - if (isMainBranch) { + if (shouldPersistAudit) { auditPersister.addComponent(session, new ComponentNewValue(item)); } } - public void insertOnMainBranch(DbSession session, ComponentDto item) { + public void insertWithAudit(DbSession session, ComponentDto item) { insert(session, item, true); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDao.java index 2ab3b5aed1c..3558af5fd25 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDao.java @@ -98,10 +98,16 @@ public class PortfolioDao implements Dao { /* * Modify portfolios */ - public void insert(DbSession dbSession, PortfolioDto portfolio) { + public void insertWithAudit(DbSession dbSession, PortfolioDto portfolio) { + insert(dbSession, portfolio, true); + } + + public void insert(DbSession dbSession, PortfolioDto portfolio, boolean shouldPersistAudit) { checkArgument(portfolio.isRoot() == (portfolio.getUuid().equals(portfolio.getRootUuid()))); mapper(dbSession).insert(portfolio); - auditPersister.addComponent(dbSession, toComponentNewValue(portfolio)); + if(shouldPersistAudit) { + auditPersister.addComponent(dbSession, toComponentNewValue(portfolio)); + } } public void delete(DbSession dbSession, PortfolioDto portfolio) { diff --git a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/audit/AuditDbTester.java b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/audit/AuditDbTester.java index d38c1553be5..65e00d861fe 100644 --- a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/audit/AuditDbTester.java +++ b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/audit/AuditDbTester.java @@ -19,6 +19,7 @@ */ package org.sonar.db.audit; +import java.util.List; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; @@ -40,4 +41,8 @@ public class AuditDbTester { dbClient.auditDao().insert(dbSession, auditDto); db.commit(); } + + public final List selectAll() { + return dbClient.auditDao().selectOlderThan(dbSession, Long.MAX_VALUE); + } } diff --git a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentDbTester.java b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentDbTester.java index b9aa5d50735..10807e788b9 100644 --- a/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentDbTester.java +++ b/server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentDbTester.java @@ -270,7 +270,7 @@ public class ComponentDbTester { PortfolioDto portfolioDto = toPortfolioDto(componentDto, System2.INSTANCE.now()); portfolioPopulator.accept(portfolioDto); - dbClient.portfolioDao().insert(dbSession, portfolioDto); + dbClient.portfolioDao().insertWithAudit(dbSession, portfolioDto); db.commit(); return componentDto; } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/batch/ProjectDataLoaderIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/batch/ProjectDataLoaderIT.java index 3a82f135a32..badc7a0cffd 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/batch/ProjectDataLoaderIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/batch/ProjectDataLoaderIT.java @@ -136,7 +136,7 @@ public class ProjectDataLoaderIT { public void fails_with_BRE_if_component_is_not_root() { String uuid = "uuid"; String key = "key"; - dbClient.componentDao().insertOnMainBranch(dbSession, new ComponentDto() + dbClient.componentDao().insertWithAudit(dbSession, new ComponentDto() .setUuid(uuid) .setUuidPath(uuid + ".") .setBranchUuid("branchUuid") diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/component/ComponentUpdaterIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/component/ComponentUpdaterIT.java index 440e52bc4ce..09725004b85 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/component/ComponentUpdaterIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/component/ComponentUpdaterIT.java @@ -26,7 +26,6 @@ import org.apache.commons.lang3.StringUtils; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.config.Configuration; import org.sonar.api.resources.Qualifiers; import org.sonar.api.resources.Scopes; import org.sonar.api.utils.System2; @@ -34,6 +33,7 @@ import org.sonar.api.web.UserRole; import org.sonar.core.util.SequenceUuidFactory; import org.sonar.db.DbSession; import org.sonar.db.DbTester; +import org.sonar.db.audit.AuditPersister; import org.sonar.db.component.BranchDto; import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; @@ -64,9 +64,11 @@ import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; 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.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.sonar.api.resources.Qualifiers.APP; @@ -92,15 +94,16 @@ public class ComponentUpdaterIT { private final System2 system2 = System2.INSTANCE; + private final AuditPersister auditPersister = mock(); + @Rule - public final DbTester db = DbTester.create(system2); + public final DbTester db = DbTester.create(system2, auditPersister); @Rule public final I18nRule i18n = new I18nRule().put("qualifier.TRK", "Project"); private final TestIndexers projectIndexers = new TestIndexers(); private final PermissionTemplateService permissionTemplateService = mock(PermissionTemplateService.class); private final DefaultBranchNameResolver defaultBranchNameResolver = mock(DefaultBranchNameResolver.class); - private final Configuration config = mock(Configuration.class); public EsTester es = EsTester.createCustom(new FooIndexDefinition()); private final PermissionUpdater userPermissionUpdater = new PermissionUpdater( new IndexersImpl(new PermissionIndexer(db.getDbClient(), es.client())), @@ -473,6 +476,26 @@ public class ComponentUpdaterIT { verify(permissionTemplateService, never()).applyDefaultToNewComponent(any(), any(), any()); } + @Test + public void createWithoutCommit_whenInsertingPortfolio_shouldOnlyAddOneEntryToAuditLogs() { + String portfolioKey = "portfolio"; + NewComponent portfolio = NewComponent.newComponentBuilder() + .setKey(portfolioKey) + .setName(portfolioKey) + .setQualifier(VIEW) + .build(); + ComponentCreationParameters creationParameters = ComponentCreationParameters.builder() + .newComponent(portfolio) + .creationMethod(CreationMethod.LOCAL_API) + .build(); + + underTest.createWithoutCommit(db.getSession(), creationParameters); + db.commit(); + + verify(auditPersister, times(1)).addComponent(argThat(d -> d.equals(db.getSession())), + argThat(newValue -> newValue.getComponentKey().equals(portfolioKey))); + } + @Test public void createWithoutCommit_whenProjectIsManagedAndPrivate_applyPublicPermissionsToCreator() { UserDto userDto = db.users().insertUser(); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/LinesActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/LinesActionIT.java index 86f94d69c9e..6f56bacdb67 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/LinesActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/LinesActionIT.java @@ -34,7 +34,6 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ProjectData; import org.sonar.db.component.SnapshotDao; import org.sonar.db.component.SnapshotDto; -import org.sonar.db.project.ProjectDto; import org.sonar.db.protobuf.DbFileSources; import org.sonar.db.source.FileSourceDto; import org.sonar.db.user.UserDto; @@ -409,7 +408,7 @@ public class LinesActionIT { private ComponentDto insertFile(ComponentDto project) { ComponentDto file = newFileDto(project); - componentDao.insertOnMainBranch(db.getSession(), file); + componentDao.insertWithAudit(db.getSession(), file); db.getSession().commit(); return file; } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/ScmActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/ScmActionIT.java index 4f8a2722aaf..ffe3bc71cfe 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/ScmActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/ScmActionIT.java @@ -67,7 +67,7 @@ public class ScmActionIT { public void setUp() { project = dbTester.components().insertPrivateProject(PROJECT_UUID); file = ComponentTesting.newFileDto(project.getMainBranchComponent(), null, FILE_UUID).setKey(FILE_KEY); - dbClient.componentDao().insertOnMainBranch(dbTester.getSession(), file); + dbClient.componentDao().insertWithAudit(dbTester.getSession(), file); dbTester.getSession().commit(); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentUpdater.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentUpdater.java index 0eace6d0245..b7ec726cb18 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentUpdater.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentUpdater.java @@ -138,7 +138,7 @@ public class ComponentUpdater { } } else if (isPortfolio(componentDto)) { portfolioDto = toPortfolioDto(componentDto, now); - dbClient.portfolioDao().insert(dbSession, portfolioDto); + dbClient.portfolioDao().insert(dbSession, portfolioDto, false); permissionTemplateService.applyDefaultToNewComponent(dbSession, portfolioDto, componentCreationParameters.userUuid()); } else { throw new IllegalArgumentException("Component " + componentDto + " is not a top level entity"); -- 2.39.5