]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-20317 fixed the issue where creating a portfolio would insert two entries into...
authorlukasz-jarocki-sonarsource <lukasz.jarocki@sonarsource.com>
Fri, 13 Oct 2023 12:03:18 +0000 (14:03 +0200)
committersonartech <sonartech@sonarsource.com>
Mon, 16 Oct 2023 20:02:49 +0000 (20:02 +0000)
12 files changed:
server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryImplIT.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AnticipatedTransitionRepositoryImplTest.java
server/sonar-db-dao/src/it/java/org/sonar/db/portfolio/PortfolioDaoIT.java
server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/portfolio/PortfolioDao.java
server/sonar-db-dao/src/testFixtures/java/org/sonar/db/audit/AuditDbTester.java
server/sonar-db-dao/src/testFixtures/java/org/sonar/db/component/ComponentDbTester.java
server/sonar-webserver-webapi/src/it/java/org/sonar/server/batch/ProjectDataLoaderIT.java
server/sonar-webserver-webapi/src/it/java/org/sonar/server/component/ComponentUpdaterIT.java
server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/LinesActionIT.java
server/sonar-webserver-webapi/src/it/java/org/sonar/server/source/ws/ScmActionIT.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentUpdater.java

index 6da6104b03880a48e10b87eec033b94977b75f0e..e3d7f74307a9d5b72dfdf5d3875609c312d73fc7 100644 (file)
@@ -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());
 
index 1c225bdef7449fd248857dd9271c24137ae40c85..72cdeadec99ffb2faaf6dbf591810a9e5e3c3499 100644 (file)
@@ -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");
index e66fa88cce49d563d409d9881ef44641ed6c0746..fc1bcc576cffec65f1ebc7d093c7db3a8caf8b69 100644 (file)
@@ -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;
   }
index 0d64637e9e4621419406112c17f86931e7968392..e4dbb188d5f2ec61bc7207276b5785f4ec1bd84b 100644 (file)
@@ -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);
   }
 
index 2ab3b5aed1cb710ec9ae6742a4d62ede41a7d71a..3558af5fd2542beb88480dd6009fda9e05e8b6b1 100644 (file)
@@ -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) {
index d38c1553be5358b146fe51456a8eda0fdc8c64f0..65e00d861fe3d74e3511f9c7ebf4f0bc9e77ec96 100644 (file)
@@ -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);
+  }
 }
index b9aa5d50735b9b75bd6e49ebc526f46ecb608cae..10807e788b97cc2b3522727386b9fb800603ed46 100644 (file)
@@ -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;
   }
index 3a82f135a32962fe77a5ce84696a513e6333fd66..badc7a0cffd070992948d61ccdacc6820d702217 100644 (file)
@@ -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")
index 440e52bc4ce695787689cd285ebc9b70ba1adf38..09725004b850db8ddb79c97610767f6d771e1396 100644 (file)
@@ -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())),
@@ -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();
index 86f94d69c9ee4b12eed69e413177ca360ad404f0..6f56bacdb6741b806563287ddec63bdbf4b64307 100644 (file)
@@ -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;
   }
index 4f8a2722aafc616b96377d167424b7ca1237e00e..ffe3bc71cfec970ea49e7f1ea4a92567603e688d 100644 (file)
@@ -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();
   }
 
index 0eace6d024577db9801175c88017d52d70811474..b7ec726cb188aba0293ad6738fe6395203fece8b 100644 (file)
@@ -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");