diff options
4 files changed, 90 insertions, 123 deletions
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStep.java index c2c336265fa..1189d83c164 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStep.java @@ -57,15 +57,11 @@ public class ValidateProjectStep implements ComputationStep { private final DbClient dbClient; private final TreeRootHolder treeRootHolder; private final AnalysisMetadataHolder analysisMetadataHolder; - private final CeTaskMessages taskMessages; - private final System2 system2; public ValidateProjectStep(DbClient dbClient, TreeRootHolder treeRootHolder, AnalysisMetadataHolder analysisMetadataHolder, CeTaskMessages taskMessages, System2 system2) { this.dbClient = dbClient; this.treeRootHolder = treeRootHolder; this.analysisMetadataHolder = analysisMetadataHolder; - this.taskMessages = taskMessages; - this.system2 = system2; } @Override @@ -121,32 +117,28 @@ public class ValidateProjectStep implements ComputationStep { public void visitProject(Component rawProject) { String rawProjectKey = rawProject.getDbKey(); Optional<ComponentDto> baseProjectOpt = loadBaseComponent(rawProjectKey); - validateAnalysisDate(baseProjectOpt); - if (!baseProjectOpt.isPresent()) { - return; - } - if (!isValidProjectKey(baseProjectOpt.get().getKey())) { + if (baseProjectOpt.isPresent()) { ComponentDto baseProject = baseProjectOpt.get(); - // As it was possible in the past to use project key with a format that is no more compatible, we need to display a warning to the user in - // order for him to update his project key. - // SONAR-13191 This warning should be removed in 9.0, and instead the analysis should fail - taskMessages.add(new CeTaskMessages.Message( - format("The project key ‘%s’ contains invalid characters. %s. You should update the project key with the expected format.", baseProject.getKey(), - ALLOWED_CHARACTERS_MESSAGE), - system2.now())); + validateAnalysisDate(baseProject); + validateProjectKey(baseProject); + } + } + + private void validateProjectKey(ComponentDto baseProject) { + if (!isValidProjectKey(baseProject.getKey())) { + validationMessages.add(format("The project key ‘%s’ contains invalid characters. %s. You should update the project key with the expected format.", baseProject.getKey(), + ALLOWED_CHARACTERS_MESSAGE)); } } - private void validateAnalysisDate(Optional<ComponentDto> baseProject) { - if (baseProject.isPresent()) { - Optional<SnapshotDto> snapshotDto = dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(session, baseProject.get().uuid()); - long currentAnalysisDate = analysisMetadataHolder.getAnalysisDate(); - Long lastAnalysisDate = snapshotDto.map(SnapshotDto::getCreatedAt).orElse(null); - if (lastAnalysisDate != null && currentAnalysisDate <= lastAnalysisDate) { - validationMessages.add(format("Date of analysis cannot be older than the date of the last known analysis on this project. Value: \"%s\". " + - "Latest analysis: \"%s\". It's only possible to rebuild the past in a chronological order.", - formatDateTime(new Date(currentAnalysisDate)), formatDateTime(new Date(lastAnalysisDate)))); - } + private void validateAnalysisDate(ComponentDto baseProject) { + Optional<SnapshotDto> snapshotDto = dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(session, baseProject.uuid()); + long currentAnalysisDate = analysisMetadataHolder.getAnalysisDate(); + Long lastAnalysisDate = snapshotDto.map(SnapshotDto::getCreatedAt).orElse(null); + if (lastAnalysisDate != null && currentAnalysisDate <= lastAnalysisDate) { + validationMessages.add(format("Date of analysis cannot be older than the date of the last known analysis on this project. Value: \"%s\". " + + "Latest analysis: \"%s\". It's only possible to rebuild the past in a chronological order.", + formatDateTime(new Date(currentAnalysisDate)), formatDateTime(new Date(lastAnalysisDate)))); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStepTest.java index 04378c1f61d..ed9087b10ea 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/ValidateProjectStepTest.java @@ -23,7 +23,6 @@ import java.util.Date; import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.api.impl.utils.TestSystem2; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.MessageException; @@ -43,9 +42,8 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentTesting; import org.sonar.db.component.SnapshotTesting; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @@ -61,19 +59,17 @@ public class ValidateProjectStepTest { @Rule public DbTester db = DbTester.create(System2.INSTANCE); @Rule - public ExpectedException thrown = ExpectedException.none(); - @Rule public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); @Rule public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule() .setAnalysisDate(new Date(DEFAULT_ANALYSIS_TIME)) .setBranch(DEFAULT_BRANCH); - private System2 system2 = new TestSystem2().setNow(NOW); + private final System2 system2 = new TestSystem2().setNow(NOW); - private CeTaskMessages taskMessages = mock(CeTaskMessages.class); - private DbClient dbClient = db.getDbClient(); + private final CeTaskMessages taskMessages = mock(CeTaskMessages.class); + private final DbClient dbClient = db.getDbClient(); - private ValidateProjectStep underTest = new ValidateProjectStep(dbClient, treeRootHolder, analysisMetadataHolder, taskMessages, system2); + private final ValidateProjectStep underTest = new ValidateProjectStep(dbClient, treeRootHolder, analysisMetadataHolder, taskMessages, system2); @Test public void dont_fail_for_long_forked_from_master_with_modules() { @@ -114,9 +110,10 @@ public class ValidateProjectStepTest { .setKey("branch") .build()); - thrown.expect(MessageException.class); - thrown.expectMessage("Due to an upgrade, you need first to re-analyze the target branch 'mergeBranch' before analyzing this pull request."); - underTest.execute(new TestComputationStepContext()); + var stepContext = new TestComputationStepContext(); + assertThatThrownBy(() -> underTest.execute(stepContext)) + .isInstanceOf(MessageException.class) + .hasMessage("Due to an upgrade, you need first to re-analyze the target branch 'mergeBranch' before analyzing this pull request."); } @Test @@ -130,16 +127,16 @@ public class ValidateProjectStepTest { treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid("ABCD").setKey(PROJECT_KEY).build()); - thrown.expect(MessageException.class); - thrown.expectMessage("Validation of project failed:"); - thrown.expectMessage("Date of analysis cannot be older than the date of the last known analysis on this project. Value: "); - thrown.expectMessage("Latest analysis: "); - - underTest.execute(new TestComputationStepContext()); + var stepContext = new TestComputationStepContext(); + assertThatThrownBy(() -> underTest.execute(stepContext)) + .isInstanceOf(MessageException.class) + .hasMessageContainingAll("Validation of project failed:", + "Date of analysis cannot be older than the date of the last known analysis on this project. Value: ", + "Latest analysis: "); } @Test - public void add_warning_when_project_key_is_invalid() { + public void fail_when_project_key_is_invalid() { ComponentDto project = db.components().insertPrivateProject(p -> p.setDbKey("inv$lid!")); db.components().insertSnapshot(project, a -> a.setCreatedAt(PAST_ANALYSIS_TIME)); treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1) @@ -147,13 +144,13 @@ public class ValidateProjectStepTest { .setKey(project.getKey()) .build()); - underTest.execute(new TestComputationStepContext()); - - verify(taskMessages, times(1)) - .add(new CeTaskMessages.Message( - "The project key ‘inv$lid!’ contains invalid characters. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit. " + - "You should update the project key with the expected format.", - NOW)); + var stepContext = new TestComputationStepContext(); + assertThatThrownBy(() -> underTest.execute(stepContext)) + .isInstanceOf(MessageException.class) + .hasMessageContainingAll("Validation of project failed:", + "The project key ‘inv$lid!’ contains invalid characters.", + "Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.", + "You should update the project key with the expected format."); } private void setBranch(BranchType type, @Nullable String mergeBranchUuid) { diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java index 9b50bb48a73..e009b3b1002 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java @@ -26,14 +26,11 @@ import javax.annotation.Nullable; import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.utils.MessageException; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; import org.sonar.core.component.ComponentKeys; import org.sonar.scanner.bootstrap.GlobalConfiguration; import org.sonar.scanner.scan.branch.BranchParamsValidator; import static java.lang.String.format; -import static java.util.Collections.singletonList; import static java.util.Objects.nonNull; import static org.apache.commons.lang.StringUtils.isNotEmpty; import static org.sonar.core.component.ComponentKeys.ALLOWED_CHARACTERS_MESSAGE; @@ -49,9 +46,6 @@ import static org.sonar.core.config.ScannerProperties.PULL_REQUEST_KEY; * @since 3.6 */ public class ProjectReactorValidator { - - private static final Logger LOG = Loggers.get(ProjectReactorValidator.class); - private final GlobalConfiguration settings; // null = branch plugin is not available @@ -71,7 +65,7 @@ public class ProjectReactorValidator { List<String> validationMessages = new ArrayList<>(); for (ProjectDefinition moduleDef : reactor.getProjects()) { - validateModule(moduleDef); + validateModule(moduleDef, validationMessages); } if (isBranchFeatureAvailable()) { @@ -82,17 +76,15 @@ public class ProjectReactorValidator { } if (!validationMessages.isEmpty()) { - throw MessageException.of("Validation of project reactor failed:\n o " + + throw MessageException.of("Validation of project failed:\n o " + String.join("\n o ", validationMessages)); } } private void validateBranchParamsWhenPluginAbsent(List<String> validationMessages) { - for (String param : singletonList(BRANCH_NAME)) { - if (isNotEmpty(settings.get(param).orElse(null))) { - validationMessages.add(format("To use the property \"%s\" and analyze branches, Developer Edition or above is required. " - + "See %s for more information.", param, BRANCHES_DOC_LINK)); - } + if (isNotEmpty(settings.get(BRANCH_NAME).orElse(null))) { + validationMessages.add(format("To use the property \"%s\" and analyze branches, Developer Edition or above is required. " + + "See %s for more information.", BRANCH_NAME, BRANCHES_DOC_LINK)); } } @@ -103,12 +95,9 @@ public class ProjectReactorValidator { + "See %s for more information.", param, BRANCHES_DOC_LINK))); } - private static void validateModule(ProjectDefinition moduleDef) { - if (!ComponentKeys.isValidProjectKey(moduleDef.getKey())) { - // As it was possible in the past to use project key with a format that is no more compatible, we need to display a warning to the user in - // order for him to update his project key. - // SONAR-13191 This warning should be removed in 9.0 - LOG.warn("\"{}\" is not a valid project or module key. {}.", moduleDef.getKey(), ALLOWED_CHARACTERS_MESSAGE); + private static void validateModule(ProjectDefinition projectDefinition, List<String> validationMessages) { + if (!ComponentKeys.isValidProjectKey(projectDefinition.getKey())) { + validationMessages.add(format("\"%s\" is not a valid project key. %s.", projectDefinition.getKey(), ALLOWED_CHARACTERS_MESSAGE)); } } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java index 5c984860484..1bdbac554a2 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java @@ -28,23 +28,19 @@ import java.util.function.Consumer; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.sonar.api.CoreProperties; import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.utils.MessageException; -import org.sonar.api.utils.log.LogAndArguments; import org.sonar.api.utils.log.LogTester; -import org.sonar.api.utils.log.LoggerLevel; import org.sonar.core.config.ScannerProperties; import org.sonar.scanner.ProjectInfo; import org.sonar.scanner.bootstrap.GlobalConfiguration; import static org.apache.commons.lang.RandomStringUtils.randomAscii; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -52,14 +48,11 @@ import static org.mockito.Mockito.when; public class ProjectReactorValidatorTest { @Rule - public ExpectedException thrown = ExpectedException.none(); - - @Rule public LogTester logTester = new LogTester(); - private GlobalConfiguration settings = mock(GlobalConfiguration.class); - private ProjectInfo projectInfo = mock(ProjectInfo.class); - private ProjectReactorValidator underTest = new ProjectReactorValidator(settings); + private final GlobalConfiguration settings = mock(GlobalConfiguration.class); + private final ProjectInfo projectInfo = mock(ProjectInfo.class); + private final ProjectReactorValidator underTest = new ProjectReactorValidator(settings); @Before public void prepare() { @@ -69,7 +62,8 @@ public class ProjectReactorValidatorTest { @Test @UseDataProvider("validKeys") public void not_fail_with_valid_key(String validKey) { - underTest.validate(createProjectReactor(validKey)); + ProjectReactor projectReactor = createProjectReactor(validKey); + underTest.validate(projectReactor); } @DataProvider @@ -93,36 +87,33 @@ public class ProjectReactorValidatorTest { } @Test - public void log_warning_when_invalid_key() { + public void failg_when_invalid_key() { ProjectReactor reactor = createProjectReactor("foo$bar"); - underTest.validate(reactor); - - assertThat(logTester.getLogs(LoggerLevel.WARN)) - .extracting(LogAndArguments::getFormattedMsg) - .containsOnly("\"foo$bar\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); + assertThatThrownBy(() -> underTest.validate(reactor)) + .isInstanceOf(MessageException.class) + .hasMessageContaining("\"foo$bar\" is not a valid project key. Allowed characters are alphanumeric," + + " '-', '_', '.' and ':', with at least one non-digit."); } @Test - public void log_warning_when_only_digits() { + public void fail_when_only_digits() { ProjectReactor reactor = createProjectReactor("12345"); - underTest.validate(reactor); - - assertThat(logTester.getLogs(LoggerLevel.WARN)) - .extracting(LogAndArguments::getFormattedMsg) - .containsOnly("\"12345\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); + assertThatThrownBy(() -> underTest.validate(reactor)) + .isInstanceOf(MessageException.class) + .hasMessageContaining("\"12345\" is not a valid project key. Allowed characters are alphanumeric, " + + "'-', '_', '.' and ':', with at least one non-digit."); } @Test - public void log_warning_when_backslash_in_key() { + public void fail_when_backslash_in_key() { ProjectReactor reactor = createProjectReactor("foo\\bar"); - underTest.validate(reactor); - - assertThat(logTester.getLogs(LoggerLevel.WARN)) - .extracting(LogAndArguments::getFormattedMsg) - .containsOnly("\"foo\\bar\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); + assertThatThrownBy(() -> underTest.validate(reactor)) + .isInstanceOf(MessageException.class) + .hasMessageContaining("\"foo\\bar\" is not a valid project key. Allowed characters are alphanumeric, " + + "'-', '_', '.' and ':', with at least one non-digit."); } @Test @@ -130,12 +121,11 @@ public class ProjectReactorValidatorTest { ProjectDefinition def = ProjectDefinition.create().setProperty(CoreProperties.PROJECT_KEY_PROPERTY, "foo"); ProjectReactor reactor = new ProjectReactor(def); - when(settings.get(eq(ScannerProperties.BRANCH_NAME))).thenReturn(Optional.of("feature1")); + when(settings.get(ScannerProperties.BRANCH_NAME)).thenReturn(Optional.of("feature1")); - thrown.expect(MessageException.class); - thrown.expectMessage("To use the property \"sonar.branch.name\" and analyze branches, Developer Edition or above is required"); - - underTest.validate(reactor); + assertThatThrownBy(() -> underTest.validate(reactor)) + .isInstanceOf(MessageException.class) + .hasMessageContaining("To use the property \"sonar.branch.name\" and analyze branches, Developer Edition or above is required"); } @Test @@ -143,12 +133,11 @@ public class ProjectReactorValidatorTest { ProjectDefinition def = ProjectDefinition.create().setProperty(CoreProperties.PROJECT_KEY_PROPERTY, "foo"); ProjectReactor reactor = new ProjectReactor(def); - when(settings.get(eq(ScannerProperties.PULL_REQUEST_KEY))).thenReturn(Optional.of("#1984")); - - thrown.expect(MessageException.class); - thrown.expectMessage("To use the property \"sonar.pullrequest.key\" and analyze pull requests, Developer Edition or above is required"); + when(settings.get(ScannerProperties.PULL_REQUEST_KEY)).thenReturn(Optional.of("#1984")); - underTest.validate(reactor); + assertThatThrownBy(() -> underTest.validate(reactor)) + .isInstanceOf(MessageException.class) + .hasMessageContaining("To use the property \"sonar.pullrequest.key\" and analyze pull requests, Developer Edition or above is required"); } @Test @@ -156,12 +145,11 @@ public class ProjectReactorValidatorTest { ProjectDefinition def = ProjectDefinition.create().setProperty(CoreProperties.PROJECT_KEY_PROPERTY, "foo"); ProjectReactor reactor = new ProjectReactor(def); - when(settings.get(eq(ScannerProperties.PULL_REQUEST_BRANCH))).thenReturn(Optional.of("feature1")); + when(settings.get(ScannerProperties.PULL_REQUEST_BRANCH)).thenReturn(Optional.of("feature1")); - thrown.expect(MessageException.class); - thrown.expectMessage("To use the property \"sonar.pullrequest.branch\" and analyze pull requests, Developer Edition or above is required"); - - underTest.validate(reactor); + assertThatThrownBy(() -> underTest.validate(reactor)) + .isInstanceOf(MessageException.class) + .hasMessageContaining("To use the property \"sonar.pullrequest.branch\" and analyze pull requests, Developer Edition or above is required"); } @Test @@ -169,12 +157,11 @@ public class ProjectReactorValidatorTest { ProjectDefinition def = ProjectDefinition.create().setProperty(CoreProperties.PROJECT_KEY_PROPERTY, "foo"); ProjectReactor reactor = new ProjectReactor(def); - when(settings.get(eq(ScannerProperties.PULL_REQUEST_BASE))).thenReturn(Optional.of("feature1")); - - thrown.expect(MessageException.class); - thrown.expectMessage("To use the property \"sonar.pullrequest.base\" and analyze pull requests, Developer Edition or above is required"); + when(settings.get(ScannerProperties.PULL_REQUEST_BASE)).thenReturn(Optional.of("feature1")); - underTest.validate(reactor); + assertThatThrownBy(() -> underTest.validate(reactor)) + .isInstanceOf(MessageException.class) + .hasMessageContaining("To use the property \"sonar.pullrequest.base\" and analyze pull requests, Developer Edition or above is required"); } @Test @@ -182,7 +169,8 @@ public class ProjectReactorValidatorTest { public void not_fail_with_valid_version(String validVersion) { when(projectInfo.getProjectVersion()).thenReturn(Optional.ofNullable(validVersion)); - underTest.validate(createProjectReactor("foo")); + ProjectReactor projectReactor = createProjectReactor("foo"); + underTest.validate(projectReactor); } @DataProvider @@ -200,7 +188,8 @@ public class ProjectReactorValidatorTest { public void not_fail_with_valid_buildString(String validBuildString) { when(projectInfo.getBuildString()).thenReturn(Optional.ofNullable(validBuildString)); - underTest.validate(createProjectReactor("foo")); + ProjectReactor projectReactor = createProjectReactor("foo"); + underTest.validate(projectReactor); } @DataProvider |