From 3c1a38e93d1bc3b3bd1ebcbec21e355267d5003d Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Tue, 6 Apr 2021 11:53:21 -0500 Subject: [PATCH] Refactor BranchPersister to use cached project configuration --- .../component/BranchPersisterImpl.java | 26 +++++---------- .../component/BranchPersisterImplTest.java | 32 +++++++------------ 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImpl.java index c22489cd933..d234fd2e3ba 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImpl.java @@ -19,10 +19,9 @@ */ package org.sonar.ce.task.projectanalysis.component; -import java.util.List; +import java.util.Arrays; import java.util.regex.Pattern; import javax.annotation.Nullable; -import org.sonar.api.config.Configuration; import org.sonar.api.resources.Qualifiers; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.analysis.Branch; @@ -32,11 +31,7 @@ import org.sonar.db.component.BranchDto; import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.protobuf.DbProjectBranches; -import org.sonar.server.setting.ProjectConfigurationLoader; -import static java.lang.String.format; -import static java.util.Arrays.asList; -import static java.util.Optional.ofNullable; import static org.sonar.core.config.PurgeConstants.BRANCHES_TO_KEEP_WHEN_INACTIVE; /** @@ -46,14 +41,13 @@ public class BranchPersisterImpl implements BranchPersister { private final DbClient dbClient; private final TreeRootHolder treeRootHolder; private final AnalysisMetadataHolder analysisMetadataHolder; - private final ProjectConfigurationLoader projectConfigurationLoader; + private final ConfigurationRepository configurationRepository; - public BranchPersisterImpl(DbClient dbClient, TreeRootHolder treeRootHolder, AnalysisMetadataHolder analysisMetadataHolder, - ProjectConfigurationLoader projectConfigurationLoader) { + public BranchPersisterImpl(DbClient dbClient, TreeRootHolder treeRootHolder, AnalysisMetadataHolder analysisMetadataHolder, ConfigurationRepository configurationRepository) { this.dbClient = dbClient; this.treeRootHolder = treeRootHolder; this.analysisMetadataHolder = analysisMetadataHolder; - this.projectConfigurationLoader = projectConfigurationLoader; + this.configurationRepository = configurationRepository; } public void persist(DbSession dbSession) { @@ -64,10 +58,10 @@ public class BranchPersisterImpl implements BranchPersister { .orElseThrow(() -> new IllegalStateException("Component has been deleted by end-user during analysis")); // insert or update in table project_branches - dbClient.branchDao().upsert(dbSession, toBranchDto(dbSession, branchComponentDto, branch, checkIfExcludedFromPurge(dbSession))); + dbClient.branchDao().upsert(dbSession, toBranchDto(dbSession, branchComponentDto, branch, checkIfExcludedFromPurge())); } - private boolean checkIfExcludedFromPurge(DbSession dbSession) { + private boolean checkIfExcludedFromPurge() { if (analysisMetadataHolder.getBranch().isMain()) { return true; } @@ -76,12 +70,8 @@ public class BranchPersisterImpl implements BranchPersister { return false; } - ComponentDto projectDto = dbClient.componentDao().selectByUuid(dbSession, analysisMetadataHolder.getProject().getUuid()) - .orElseThrow(() -> new IllegalStateException(format("Component '%s' is missing.", analysisMetadataHolder.getProject().getKey()))); - Configuration projectConfiguration = projectConfigurationLoader.loadProjectConfiguration(dbSession, projectDto); - String[] branchesToKeep = projectConfiguration.getStringArray(BRANCHES_TO_KEEP_WHEN_INACTIVE); - List excludeFromPurgeEntries = asList(ofNullable(branchesToKeep).orElse(new String[0])); - return excludeFromPurgeEntries.stream() + String[] branchesToKeep = configurationRepository.getConfiguration().getStringArray(BRANCHES_TO_KEEP_WHEN_INACTIVE); + return Arrays.stream(branchesToKeep) .map(Pattern::compile) .anyMatch(excludePattern -> excludePattern.matcher(analysisMetadataHolder.getBranch().getName()).matches()); } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImplTest.java index e8f8aa2913b..9e3d18f4985 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/BranchPersisterImplTest.java @@ -29,6 +29,7 @@ import org.assertj.core.api.ThrowableAssert; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.sonar.api.config.internal.ConfigurationBridge; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.utils.System2; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; @@ -39,11 +40,8 @@ import org.sonar.db.component.BranchDto; import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentTesting; -import org.sonar.db.property.PropertyDto; import org.sonar.db.protobuf.DbProjectBranches; import org.sonar.server.project.Project; -import org.sonar.server.setting.ProjectConfigurationLoader; -import org.sonar.server.setting.ProjectConfigurationLoaderImpl; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; @@ -69,10 +67,9 @@ public class BranchPersisterImplTest { @Rule public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); - private final MapSettings globalSettings = new MapSettings(); - private final ProjectConfigurationLoader projectConfiguration = new ProjectConfigurationLoaderImpl(globalSettings, dbTester.getDbClient()); - private final BranchPersister underTest = new BranchPersisterImpl(dbTester.getDbClient(), treeRootHolder, analysisMetadataHolder, - projectConfiguration); + private final MapSettings settings = new MapSettings(); + private final ConfigurationRepository configurationRepository = new TestSettingsRepository(new ConfigurationBridge(settings)); + private final BranchPersister underTest = new BranchPersisterImpl(dbTester.getDbClient(), treeRootHolder, analysisMetadataHolder, configurationRepository); @Test public void persist_fails_with_ISE_if_no_component_for_main_branches() { @@ -146,7 +143,7 @@ public class BranchPersisterImplTest { @Test public void non_main_branch_is_excluded_from_branch_purge_if_matches_sonar_dbcleaner_keepFromPurge_property() { - globalSettings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "BRANCH.*"); + settings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "BRANCH.*"); analysisMetadataHolder.setProject(PROJECT); analysisMetadataHolder.setBranch(createBranch(BRANCH, false, "BRANCH_KEY")); treeRootHolder.setRoot(BRANCH1); @@ -164,7 +161,7 @@ public class BranchPersisterImplTest { } @Test - public void branch_is_excluded_from_purge_when_it_matches_project_level_but_not_global_level_keepFromPurge_setting() { + public void branch_is_excluded_from_purge_when_it_matches_setting() { analysisMetadataHolder.setProject(PROJECT); analysisMetadataHolder.setBranch(createBranch(BRANCH, false, "BRANCH_KEY")); treeRootHolder.setRoot(BRANCH1); @@ -172,8 +169,7 @@ public class BranchPersisterImplTest { ComponentDto component = ComponentTesting.newBranchComponent(mainComponent, new BranchDto().setUuid(BRANCH1.getUuid()).setKey(BRANCH1.getKey()).setBranchType(BRANCH)); dbTester.getDbClient().componentDao().insert(dbTester.getSession(), component); - globalSettings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "abc.*"); - insertProjectProperty(mainComponent, BRANCHES_TO_KEEP_WHEN_INACTIVE, "BRANCH.*"); + settings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "BRANCH.*"); dbTester.commit(); underTest.persist(dbTester.getSession()); @@ -184,7 +180,7 @@ public class BranchPersisterImplTest { } @Test - public void branch_is_not_excluded_from_purge_when_it_does_not_match_overriden_global_keepFromPurge_setting_on_project_level() { + public void branch_is_not_excluded_from_purge_when_it_does_not_match_setting() { analysisMetadataHolder.setProject(PROJECT); analysisMetadataHolder.setBranch(createBranch(BRANCH, false, "BRANCH_KEY")); treeRootHolder.setRoot(BRANCH1); @@ -192,8 +188,8 @@ public class BranchPersisterImplTest { ComponentDto component = ComponentTesting.newBranchComponent(mainComponent, new BranchDto().setUuid(BRANCH1.getUuid()).setKey(BRANCH1.getKey()).setBranchType(BRANCH)); dbTester.getDbClient().componentDao().insert(dbTester.getSession(), component); - globalSettings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "BRANCH.*"); - insertProjectProperty(mainComponent, BRANCHES_TO_KEEP_WHEN_INACTIVE, "abc.*"); + settings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "abc.*"); + dbTester.commit(); underTest.persist(dbTester.getSession()); @@ -205,7 +201,7 @@ public class BranchPersisterImplTest { @Test public void pull_request_is_never_excluded_from_branch_purge_even_if_its_source_branch_name_matches_sonar_dbcleaner_keepFromPurge_property() { - globalSettings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "develop"); + settings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "develop"); analysisMetadataHolder.setBranch(createPullRequest(PR1.getKey(), MAIN.getUuid())); analysisMetadataHolder.setPullRequestKey(PR1.getKey()); treeRootHolder.setRoot(PR1); @@ -228,7 +224,7 @@ public class BranchPersisterImplTest { @Test public void non_main_branch_is_included_in_branch_purge_if_branch_name_does_not_match_sonar_dbcleaner_keepFromPurge_property() { - globalSettings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "foobar-.*"); + settings.setProperty(BRANCHES_TO_KEEP_WHEN_INACTIVE, "foobar-.*"); analysisMetadataHolder.setProject(PROJECT); analysisMetadataHolder.setBranch(createBranch(BRANCH, false, "BRANCH_KEY")); treeRootHolder.setRoot(BRANCH1); @@ -311,8 +307,4 @@ public class BranchPersisterImplTest { .isInstanceOf(IllegalStateException.class) .hasMessage("Component has been deleted by end-user during analysis"); } - - private void insertProjectProperty(ComponentDto project, String propertyKey, String propertyValue) { - dbTester.properties().insertProperties(new PropertyDto().setKey(propertyKey).setValue(propertyValue).setComponentUuid(project.uuid())); - } } -- 2.39.5