From 1c1d85d052955ce3434228199761646a75e2c077 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Mon, 28 Mar 2022 11:39:43 -0500 Subject: [PATCH] SONAR-16050 Scanner fails with NPE if user doesn't have permission to analyze project --- ....java => ProjectRepositoriesProvider.java} | 22 ++++++++----------- .../scan/SpringProjectScanContainer.java | 4 ++-- .../scan/filesystem/StatusDetection.java | 10 ++++----- .../org/sonar/scanner/scm/ScmPublisher.java | 10 ++++----- ...a => ProjectRepositoriesProviderTest.java} | 18 +++++++-------- .../scan/filesystem/StatusDetectionTest.java | 18 +++++---------- 6 files changed, 35 insertions(+), 47 deletions(-) rename sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/{ProjectRepositoriesSupplier.java => ProjectRepositoriesProvider.java} (75%) rename sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/{ProjectRepositoriesSupplierTest.java => ProjectRepositoriesProviderTest.java} (77%) diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesSupplier.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesProvider.java similarity index 75% rename from sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesSupplier.java rename to sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesProvider.java index ea913b966ee..b1c951c4e5f 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesSupplier.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/repository/ProjectRepositoriesProvider.java @@ -19,35 +19,31 @@ */ package org.sonar.scanner.repository; -import java.util.function.Supplier; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Profiler; import org.sonar.scanner.bootstrap.ScannerProperties; import org.sonar.scanner.scan.branch.BranchConfiguration; +import org.springframework.context.annotation.Bean; -public class ProjectRepositoriesSupplier implements Supplier { - private static final Logger LOG = Loggers.get(ProjectRepositoriesSupplier.class); +public class ProjectRepositoriesProvider { + private static final Logger LOG = Loggers.get(ProjectRepositoriesProvider.class); private static final String LOG_MSG = "Load project repositories"; private final ProjectRepositoriesLoader loader; private final ScannerProperties scannerProperties; private final BranchConfiguration branchConfig; - private ProjectRepositories project = null; - - public ProjectRepositoriesSupplier(ProjectRepositoriesLoader loader, ScannerProperties scannerProperties, BranchConfiguration branchConfig) { + public ProjectRepositoriesProvider(ProjectRepositoriesLoader loader, ScannerProperties scannerProperties, BranchConfiguration branchConfig) { this.loader = loader; this.scannerProperties = scannerProperties; this.branchConfig = branchConfig; } - public ProjectRepositories get() { - if (project == null) { - Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG); - project = loader.load(scannerProperties.getProjectKey(), branchConfig.referenceBranchName()); - profiler.stopInfo(); - } - + @Bean + public ProjectRepositories projectRepositories() { + Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG); + ProjectRepositories project = loader.load(scannerProperties.getProjectKey(), branchConfig.referenceBranchName()); + profiler.stopInfo(); return project; } } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/SpringProjectScanContainer.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/SpringProjectScanContainer.java index 325032da593..da491cf9f5a 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/SpringProjectScanContainer.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/SpringProjectScanContainer.java @@ -95,7 +95,7 @@ import org.sonar.scanner.report.TestExecutionPublisher; import org.sonar.scanner.repository.ContextPropertiesCache; import org.sonar.scanner.repository.DefaultProjectRepositoriesLoader; import org.sonar.scanner.repository.DefaultQualityProfileLoader; -import org.sonar.scanner.repository.ProjectRepositoriesSupplier; +import org.sonar.scanner.repository.ProjectRepositoriesProvider; import org.sonar.scanner.repository.QualityProfilesProvider; import org.sonar.scanner.repository.ReferenceBranchSupplier; import org.sonar.scanner.repository.language.DefaultLanguagesRepository; @@ -169,7 +169,7 @@ public class SpringProjectScanContainer extends SpringComponentContainer { new BranchConfigurationProvider(), new ProjectBranchesProvider(), new ProjectPullRequestsProvider(), - ProjectRepositoriesSupplier.class, + ProjectRepositoriesProvider.class, new ProjectServerSettingsProvider(), AnalysisCacheEnabled.class, diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java index 54c50715e5e..a6240aa8215 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/StatusDetection.java @@ -24,7 +24,7 @@ import org.apache.commons.lang.StringUtils; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.scanner.repository.FileData; -import org.sonar.scanner.repository.ProjectRepositoriesSupplier; +import org.sonar.scanner.repository.ProjectRepositories; import org.sonar.scanner.scm.ScmChangedFiles; import static org.sonar.api.batch.fs.InputFile.Status.ADDED; @@ -33,11 +33,11 @@ import static org.sonar.api.batch.fs.InputFile.Status.SAME; @Immutable public class StatusDetection { - private final ProjectRepositoriesSupplier projectSettingsSupplier; + private final ProjectRepositories projectRepositories; private final ScmChangedFiles scmChangedFiles; - public StatusDetection(ProjectRepositoriesSupplier projectSettingsSupplier, ScmChangedFiles scmChangedFiles) { - this.projectSettingsSupplier = projectSettingsSupplier; + public StatusDetection(ProjectRepositories projectRepositories, ScmChangedFiles scmChangedFiles) { + this.projectRepositories = projectRepositories; this.scmChangedFiles = scmChangedFiles; } @@ -49,7 +49,7 @@ public class StatusDetection { } private InputFile.Status checkChangedWithProjectRepositories(String moduleKeyWithBranch, DefaultInputFile inputFile, String hash) { - FileData fileDataPerPath = projectSettingsSupplier.get().fileData(moduleKeyWithBranch, inputFile); + FileData fileDataPerPath = projectRepositories.fileData(moduleKeyWithBranch, inputFile); if (fileDataPerPath == null) { return ADDED; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmPublisher.java index bacd468050e..e773c0d3bcf 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmPublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scm/ScmPublisher.java @@ -36,7 +36,7 @@ import org.sonar.scanner.protocol.output.ScannerReport.Changesets.Builder; import org.sonar.scanner.protocol.output.ScannerReportWriter; import org.sonar.scanner.report.ReportPublisher; import org.sonar.scanner.repository.FileData; -import org.sonar.scanner.repository.ProjectRepositoriesSupplier; +import org.sonar.scanner.repository.ProjectRepositories; import org.sonar.scanner.scan.branch.BranchConfiguration; import org.sonar.scanner.scan.filesystem.InputComponentStore; @@ -45,17 +45,17 @@ public final class ScmPublisher { private static final Logger LOG = Loggers.get(ScmPublisher.class); private final ScmConfiguration configuration; - private final ProjectRepositoriesSupplier projectRepositoriesSupplier; + private final ProjectRepositories projectRepositories; private final InputComponentStore componentStore; private final FileSystem fs; private final ScannerReportWriter writer; private AnalysisWarnings analysisWarnings; private final BranchConfiguration branchConfiguration; - public ScmPublisher(ScmConfiguration configuration, ProjectRepositoriesSupplier projectRepositoriesSupplier, InputComponentStore componentStore, FileSystem fs, + public ScmPublisher(ScmConfiguration configuration, ProjectRepositories projectRepositories, InputComponentStore componentStore, FileSystem fs, ReportPublisher reportPublisher, BranchConfiguration branchConfiguration, AnalysisWarnings analysisWarnings) { this.configuration = configuration; - this.projectRepositoriesSupplier = projectRepositoriesSupplier; + this.projectRepositories = projectRepositories; this.componentStore = componentStore; this.fs = fs; this.branchConfiguration = branchConfiguration; @@ -99,7 +99,7 @@ public final class ScmPublisher { if (configuration.forceReloadAll() || f.status() != Status.SAME) { addIfNotEmpty(filesToBlame, f); } else if (!branchConfiguration.isPullRequest()) { - FileData fileData = projectRepositoriesSupplier.get().fileData(componentStore.findModule(f).key(), f); + FileData fileData = projectRepositories.fileData(componentStore.findModule(f).key(), f); if (fileData == null || StringUtils.isEmpty(fileData.revision())) { addIfNotEmpty(filesToBlame, f); } else { diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ProjectRepositoriesSupplierTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ProjectRepositoriesProviderTest.java similarity index 77% rename from sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ProjectRepositoriesSupplierTest.java rename to sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ProjectRepositoriesProviderTest.java index abe7adfdd5b..f603277d55a 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ProjectRepositoriesSupplierTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/repository/ProjectRepositoriesProviderTest.java @@ -34,17 +34,17 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -public class ProjectRepositoriesSupplierTest { - private ProjectRepositoriesSupplier underTest; +public class ProjectRepositoriesProviderTest { + private ProjectRepositoriesProvider underTest; private ProjectRepositories project; - private ProjectRepositoriesLoader loader = mock(ProjectRepositoriesLoader.class); - private ScannerProperties props = mock(ScannerProperties.class); - private BranchConfiguration branchConfiguration = mock(BranchConfiguration.class); + private final ProjectRepositoriesLoader loader = mock(ProjectRepositoriesLoader.class); + private final ScannerProperties props = mock(ScannerProperties.class); + private final BranchConfiguration branchConfiguration = mock(BranchConfiguration.class); @Before public void setUp() { - underTest = new ProjectRepositoriesSupplier(loader, props, branchConfiguration); + underTest = new ProjectRepositoriesProvider(loader, props, branchConfiguration); Map fileMap = emptyMap(); project = new SingleProjectRepository(fileMap); when(props.getProjectKey()).thenReturn("key"); @@ -53,18 +53,18 @@ public class ProjectRepositoriesSupplierTest { @Test public void testValidation() { when(loader.load(eq("key"), any())).thenReturn(project); - underTest.get(); + assertThat(underTest.projectRepositories()).isEqualTo(project); } @Test public void testAssociated() { when(loader.load(eq("key"), any())).thenReturn(project); - ProjectRepositories repo = underTest.get(); + ProjectRepositories repo = underTest.projectRepositories(); assertThat(repo.exists()).isTrue(); verify(props).getProjectKey(); - verify(loader).load(eq("key"), eq(null)); + verify(loader).load("key", null); verifyNoMoreInteractions(loader, props); } } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/filesystem/StatusDetectionTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/filesystem/StatusDetectionTest.java index 9c6e0a7f2a4..73c736b224b 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/filesystem/StatusDetectionTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/filesystem/StatusDetectionTest.java @@ -23,32 +23,24 @@ import java.nio.file.Paths; import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.junit.Before; import org.junit.Test; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.scanner.repository.FileData; -import org.sonar.scanner.repository.ProjectRepositoriesSupplier; +import org.sonar.scanner.repository.ProjectRepositories; import org.sonar.scanner.repository.SingleProjectRepository; import org.sonar.scanner.scm.ScmChangedFiles; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class StatusDetectionTest { - private ProjectRepositoriesSupplier projectRepositoriesSupplier = mock(ProjectRepositoriesSupplier.class); - - @Before - public void setUp() { - when(projectRepositoriesSupplier.get()).thenReturn(new SingleProjectRepository(createFileDataPerPathMap())); - } + private final ProjectRepositories projectRepositories = new SingleProjectRepository(createFileDataPerPathMap()); @Test public void detect_status() { ScmChangedFiles changedFiles = new ScmChangedFiles(null); - StatusDetection statusDetection = new StatusDetection(projectRepositoriesSupplier, changedFiles); + StatusDetection statusDetection = new StatusDetection(projectRepositories, changedFiles); assertThat(statusDetection.status("foo", createFile("src/Foo.java"), "ABCDE")).isEqualTo(InputFile.Status.SAME); assertThat(statusDetection.status("foo", createFile("src/Foo.java"), "XXXXX")).isEqualTo(InputFile.Status.CHANGED); @@ -58,7 +50,7 @@ public class StatusDetectionTest { @Test public void detect_status_branches_exclude() { ScmChangedFiles changedFiles = new ScmChangedFiles(Collections.emptyList()); - StatusDetection statusDetection = new StatusDetection(projectRepositoriesSupplier, changedFiles); + StatusDetection statusDetection = new StatusDetection(projectRepositories, changedFiles); // normally changed assertThat(statusDetection.status("foo", createFile("src/Foo.java"), "XXXXX")).isEqualTo(InputFile.Status.SAME); @@ -70,7 +62,7 @@ public class StatusDetectionTest { @Test public void detect_status_branches_confirm() { ScmChangedFiles changedFiles = new ScmChangedFiles(Collections.singletonList(Paths.get("module", "src", "Foo.java"))); - StatusDetection statusDetection = new StatusDetection(projectRepositoriesSupplier, changedFiles); + StatusDetection statusDetection = new StatusDetection(projectRepositories, changedFiles); assertThat(statusDetection.status("foo", createFile("src/Foo.java"), "XXXXX")).isEqualTo(InputFile.Status.CHANGED); } -- 2.39.5