SONAR-20317 fixed the issue where creating a portfolio would insert two entries into audit table

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<KeyWithUuidDto> 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<AuditDto> 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<UserPermissionChange> userPermissionUpdater = new PermissionUpdater(
     new IndexersImpl(new PermissionIndexer(db.getDbClient(), es.client())),
@@ -474,6 +477,26 @@ public class ComponentUpdaterIT {
   }
 
   @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();
     NewComponent newComponent = NewComponent.newComponentBuilder()
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");