From ec384a1e6926c801dc5167807f26b1f2ef6baf84 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 4 Mar 2020 18:06:32 +0100 Subject: [PATCH] SONAR-13160 Fix display of Portfolio Admin page when project contains UTF-8 characters --- .../step/ValidateProjectStep.java | 40 ++++--- .../step/ValidateProjectStepTest.java | 106 +++++++++++------- .../db/component/ComponentKeyUpdaterDao.java | 1 + .../component/ComponentKeyUpdaterDaoTest.java | 7 +- .../server/component/ComponentService.java | 9 +- .../server/component/ComponentUpdater.java | 4 +- .../ComponentServiceUpdateKeyTest.java | 13 +-- .../component/ComponentUpdaterTest.java | 10 +- .../project/ws/BulkUpdateKeyActionTest.java | 9 +- .../server/project/ws/CreateActionTest.java | 6 +- .../sonar/core/component/ComponentKeys.java | 23 ++-- .../core/component/ComponentKeysTest.java | 29 ++++- .../scanner/scan/ProjectReactorValidator.java | 19 +++- .../scan/ProjectReactorValidatorTest.java | 39 ++++++- 14 files changed, 202 insertions(+), 113 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 ddee476f008..47e75233334 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 @@ -27,6 +27,8 @@ import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; import org.sonar.api.utils.MessageException; +import org.sonar.api.utils.System2; +import org.sonar.ce.task.log.CeTaskMessages; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.ComponentVisitor; @@ -45,17 +47,9 @@ import org.sonar.db.component.SnapshotDto; import static com.google.common.base.Preconditions.checkState; import static java.lang.String.format; import static org.sonar.api.utils.DateUtils.formatDateTime; +import static org.sonar.core.component.ComponentKeys.ALLOWED_CHARACTERS_MESSAGE; +import static org.sonar.core.component.ComponentKeys.isValidProjectKey; -/** - * Validate project and modules. It will fail in the following cases : - *
    - *
  1. module key is not valid
    2. - *
  2. module key already exists in another project (same module key cannot exists in different projects)
    3. - *
  3. module key is already used as a project key
    4. - *
  4. date of the analysis is before last analysis
    5. - *
  5. PR targets a branch that still contains modules
    6. - *
- */ public class ValidateProjectStep implements ComputationStep { private static final Joiner MESSAGES_JOINER = Joiner.on("\n o "); @@ -63,11 +57,15 @@ 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) { + 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 @@ -75,6 +73,7 @@ public class ValidateProjectStep implements ComputationStep { try (DbSession dbSession = dbClient.openSession(false)) { validateTargetBranch(dbSession); Component root = treeRootHolder.getRoot(); + // FIXME if module have really be dropped, no more need to load them List baseModules = dbClient.componentDao().selectEnabledModulesFromProjectKey(dbSession, root.getDbKey()); Map baseModulesByKey = baseModules.stream().collect(Collectors.toMap(ComponentDto::getDbKey, x -> x)); ValidateProjectsVisitor visitor = new ValidateProjectsVisitor(dbSession, dbClient.componentDao(), baseModulesByKey); @@ -121,8 +120,21 @@ public class ValidateProjectStep implements ComputationStep { @Override public void visitProject(Component rawProject) { String rawProjectKey = rawProject.getDbKey(); - Optional baseProject = loadBaseComponent(rawProjectKey); - validateAnalysisDate(baseProject); + Optional baseProjectOpt = loadBaseComponent(rawProjectKey); + validateAnalysisDate(baseProjectOpt); + if (!baseProjectOpt.isPresent()) { + return; + } + if (!isValidProjectKey(baseProjectOpt.get().getKey())) { + 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())); + } } private void validateAnalysisDate(Optional baseProject) { @@ -132,7 +144,7 @@ public class ValidateProjectStep implements ComputationStep { 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.", + "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 42a52d6d3a5..0a35b16968e 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 @@ -24,9 +24,11 @@ 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; import org.sonar.api.utils.System2; +import org.sonar.ce.task.log.CeTaskMessages; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; import org.sonar.ce.task.projectanalysis.analysis.Branch; import org.sonar.ce.task.projectanalysis.component.Component; @@ -42,78 +44,78 @@ import org.sonar.db.component.ComponentTesting; import org.sonar.db.component.SnapshotTesting; 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; public class ValidateProjectStepTest { - static long DEFAULT_ANALYSIS_TIME = 1433131200000L; // 2015-06-01 + static long PAST_ANALYSIS_TIME = 1_420_088_400_000L; // 2015-01-01 + static long DEFAULT_ANALYSIS_TIME = 1_433_131_200_000L; // 2015-06-01 + static long NOW = 1_500_000_000_000L; + static final String PROJECT_KEY = "PROJECT_KEY"; static final Branch DEFAULT_BRANCH = new DefaultBranchImpl(); @Rule - public DbTester dbTester = DbTester.create(System2.INSTANCE); - + 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 CeTaskMessages taskMessages = mock(CeTaskMessages.class); + private DbClient dbClient = db.getDbClient(); - private DbClient dbClient = dbTester.getDbClient(); - private ValidateProjectStep underTest = new ValidateProjectStep(dbClient, treeRootHolder, analysisMetadataHolder); + private ValidateProjectStep underTest = new ValidateProjectStep(dbClient, treeRootHolder, analysisMetadataHolder, taskMessages, system2); @Test - public void fail_if_pr_is_targeting_branch_with_modules() { - ComponentDto masterProject = dbTester.components().insertPublicProject(); - ComponentDto mergeBranch = dbTester.components().insertProjectBranch(masterProject, b -> b.setKey("mergeBranch")); - dbClient.componentDao().insert(dbTester.getSession(), ComponentTesting.newModuleDto(mergeBranch)); - setBranch(BranchType.PULL_REQUEST, mergeBranch.uuid()); - dbTester.getSession().commit(); + public void dont_fail_for_long_forked_from_master_with_modules() { + ComponentDto masterProject = db.components().insertPublicProject(); + dbClient.componentDao().insert(db.getSession(), ComponentTesting.newModuleDto(masterProject)); + setBranch(BranchType.BRANCH, masterProject.uuid()); + db.getSession().commit(); treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid("DEFG") .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()); + verifyNoInteractions(taskMessages); } @Test - public void dont_fail_for_long_forked_from_master_with_modules() { - ComponentDto masterProject = dbTester.components().insertPublicProject(); - dbClient.componentDao().insert(dbTester.getSession(), ComponentTesting.newModuleDto(masterProject)); - setBranch(BranchType.BRANCH, masterProject.uuid()); - dbTester.getSession().commit(); + public void not_fail_if_analysis_date_is_after_last_analysis() { + ComponentDto project = ComponentTesting.newPrivateProjectDto(db.organizations().insert(), "ABCD").setDbKey(PROJECT_KEY); + dbClient.componentDao().insert(db.getSession(), project); + dbClient.snapshotDao().insert(db.getSession(), SnapshotTesting.newAnalysis(project).setCreatedAt(PAST_ANALYSIS_TIME)); + db.getSession().commit(); - treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid("DEFG") - .setKey("branch") - .build()); + treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid("ABCD").setKey(PROJECT_KEY).build()); underTest.execute(new TestComputationStepContext()); } - private void setBranch(BranchType type, @Nullable String mergeBranchUuid) { - Branch branch = mock(Branch.class); - when(branch.getType()).thenReturn(type); - when(branch.getReferenceBranchUuid()).thenReturn(mergeBranchUuid); - analysisMetadataHolder.setBranch(branch); - } - @Test - public void not_fail_if_analysis_date_is_after_last_analysis() { - ComponentDto project = ComponentTesting.newPrivateProjectDto(dbTester.organizations().insert(), "ABCD").setDbKey(PROJECT_KEY); - dbClient.componentDao().insert(dbTester.getSession(), project); - dbClient.snapshotDao().insert(dbTester.getSession(), SnapshotTesting.newAnalysis(project).setCreatedAt(1420088400000L)); // 2015-01-01 - dbTester.getSession().commit(); + public void fail_if_pr_is_targeting_branch_with_modules() { + ComponentDto masterProject = db.components().insertPublicProject(); + ComponentDto mergeBranch = db.components().insertProjectBranch(masterProject, b -> b.setKey("mergeBranch")); + dbClient.componentDao().insert(db.getSession(), ComponentTesting.newModuleDto(mergeBranch)); + setBranch(BranchType.PULL_REQUEST, mergeBranch.uuid()); + db.getSession().commit(); - treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid("ABCD").setKey(PROJECT_KEY).build()); + treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid("DEFG") + .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()); } @@ -121,10 +123,10 @@ public class ValidateProjectStepTest { public void fail_if_analysis_date_is_before_last_analysis() { analysisMetadataHolder.setAnalysisDate(DateUtils.parseDate("2015-01-01")); - ComponentDto project = ComponentTesting.newPrivateProjectDto(dbTester.organizations().insert(), "ABCD").setDbKey(PROJECT_KEY); - dbClient.componentDao().insert(dbTester.getSession(), project); - dbClient.snapshotDao().insert(dbTester.getSession(), SnapshotTesting.newAnalysis(project).setCreatedAt(1433131200000L)); // 2015-06-01 - dbTester.getSession().commit(); + ComponentDto project = ComponentTesting.newPrivateProjectDto(db.organizations().insert(), "ABCD").setDbKey(PROJECT_KEY); + dbClient.componentDao().insert(db.getSession(), project); + dbClient.snapshotDao().insert(db.getSession(), SnapshotTesting.newAnalysis(project).setCreatedAt(1433131200000L)); // 2015-06-01 + db.getSession().commit(); treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid("ABCD").setKey(PROJECT_KEY).build()); @@ -136,4 +138,28 @@ public class ValidateProjectStepTest { underTest.execute(new TestComputationStepContext()); } + @Test + public void add_warning_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) + .setUuid(project.uuid()) + .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)); + } + + private void setBranch(BranchType type, @Nullable String mergeBranchUuid) { + Branch branch = mock(Branch.class); + when(branch.getType()).thenReturn(type); + when(branch.getReferenceBranchUuid()).thenReturn(mergeBranchUuid); + analysisMetadataHolder.setBranch(branch); + } } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentKeyUpdaterDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentKeyUpdaterDao.java index 61dcb2242b6..902f569766d 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentKeyUpdaterDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentKeyUpdaterDao.java @@ -47,6 +47,7 @@ import static org.sonar.core.component.ComponentKeys.checkProjectKey; * @since 3.2 */ public class ComponentKeyUpdaterDao implements Dao { + public void updateKey(DbSession dbSession, String projectUuid, String newKey) { ComponentKeyUpdaterMapper mapper = dbSession.getMapper(ComponentKeyUpdaterMapper.class); if (mapper.countResourceByKey(newKey) > 0) { diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentKeyUpdaterDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentKeyUpdaterDaoTest.java index 085f07a8139..c37af6ba61c 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentKeyUpdaterDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentKeyUpdaterDaoTest.java @@ -20,7 +20,6 @@ package org.sonar.db.component; import com.google.common.base.Strings; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -368,9 +367,9 @@ public class ComponentKeyUpdaterDaoTest { ComponentDto project = db.components().insertPrivateProject(); thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("Malformed key for ' '. Project key cannot be empty nor contain whitespaces."); + thrown.expectMessage("Malformed key for 'my?project?key'. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); - underTest.bulkUpdateKey(dbSession, project.uuid(), project.getDbKey(), " ", doNotReturnAnyRekeyedResource()); + underTest.bulkUpdateKey(dbSession, project.uuid(), project.getDbKey(), "my?project?key", doNotReturnAnyRekeyedResource()); } @Test @@ -431,7 +430,7 @@ public class ComponentKeyUpdaterDaoTest { thrown.expect(IllegalArgumentException.class); - underTest.simulateBulkUpdateKey(dbSession, "A", "project", " "); + underTest.simulateBulkUpdateKey(dbSession, "A", "project", "project?"); } @Test diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentService.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentService.java index 064d94f81a2..7a32281a2f5 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentService.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ComponentService.java @@ -41,8 +41,7 @@ import org.sonar.server.user.UserSession; import static java.util.Collections.emptyList; import static java.util.Collections.singleton; import static java.util.Collections.singletonList; -import static org.sonar.core.component.ComponentKeys.isValidProjectKey; -import static org.sonar.server.exceptions.BadRequestException.checkRequest; +import static org.sonar.core.component.ComponentKeys.checkProjectKey; @ServerSide public class ComponentService { @@ -60,7 +59,7 @@ public class ComponentService { public void updateKey(DbSession dbSession, ProjectDto project, String newKey) { userSession.checkProjectPermission(UserRole.ADMIN, project); - checkProjectKeyFormat(newKey); + checkProjectKey(newKey); dbClient.componentKeyUpdaterDao().updateKey(dbSession, project.getUuid(), newKey); projectIndexers.commitAndIndexProjects(dbSession, singletonList(project), ProjectIndexer.Cause.PROJECT_KEY_UPDATE); Project newProject = new Project(project.getUuid(), newKey, project.getName(), project.getDescription(), project.getTags()); @@ -94,8 +93,4 @@ public class ComponentService { return new RekeyedProject(project, rekeyedResource.getOldKey()); } - private static void checkProjectKeyFormat(String key) { - checkRequest(isValidProjectKey(key), "Malformed key for '%s'. It cannot be empty nor contain whitespaces.", key); - } - } 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 5bba07896c8..0ae333ec5ee 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 @@ -42,8 +42,8 @@ import org.sonar.server.favorite.FavoriteUpdater; import org.sonar.server.permission.PermissionTemplateService; import static java.util.Collections.singletonList; -import static org.sonar.api.resources.Qualifiers.APP; import static org.sonar.api.resources.Qualifiers.PROJECT; +import static org.sonar.core.component.ComponentKeys.ALLOWED_CHARACTERS_MESSAGE; import static org.sonar.core.component.ComponentKeys.isValidProjectKey; import static org.sonar.server.exceptions.BadRequestException.checkRequest; @@ -170,7 +170,7 @@ public class ComponentUpdater { } private void checkKeyFormat(String qualifier, String key) { - checkRequest(isValidProjectKey(key), "Malformed key for %s: '%s'. It cannot be empty nor contain whitespaces.", getQualifierToDisplay(qualifier), key); + checkRequest(isValidProjectKey(key), "Malformed key for %s: '%s'. %s.", getQualifierToDisplay(qualifier), key, ALLOWED_CHARACTERS_MESSAGE); } private String getQualifierToDisplay(String qualifier) { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentServiceUpdateKeyTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentServiceUpdateKeyTest.java index 18772de9c9c..43b0450498d 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentServiceUpdateKeyTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentServiceUpdateKeyTest.java @@ -33,7 +33,6 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentTesting; import org.sonar.server.es.ProjectIndexer; import org.sonar.server.es.TestProjectIndexers; -import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.project.Project; import org.sonar.server.project.ProjectLifeCycleListeners; @@ -132,21 +131,21 @@ public class ComponentServiceUpdateKeyTest { ComponentDto project = insertSampleProject(); logInAsProjectAdministrator(project); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Malformed key for ''. It cannot be empty nor contain whitespaces."); + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Malformed key for ''. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); underTest.updateKey(dbSession, componentDb.getProjectDto(project), ""); } @Test - public void fail_if_new_key_is_invalid() { + public void fail_if_new_key_is_not_formatted_correctly() { ComponentDto project = insertSampleProject(); logInAsProjectAdministrator(project); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Malformed key for 'sample root'. It cannot be empty nor contain whitespaces."); + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Malformed key for 'sample?root'. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); - underTest.updateKey(dbSession, componentDb.getProjectDto(project), "sample root"); + underTest.updateKey(dbSession, componentDb.getProjectDto(project), "sample?root"); } @Test diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java index c592065c9aa..bb2ea365fff 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java @@ -290,11 +290,11 @@ public class ComponentUpdaterTest { @Test public void fail_when_key_has_bad_format() { expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Malformed key for Project: ' '"); + expectedException.expectMessage("Malformed key for Project: '1234'"); underTest.create(db.getSession(), NewComponent.newComponentBuilder() - .setKey(" ") + .setKey("1234") .setName(DEFAULT_PROJECT_NAME) .setOrganizationUuid(db.getDefaultOrganization().getUuid()) .build(), @@ -302,13 +302,13 @@ public class ComponentUpdaterTest { } @Test - public void properly_fail_when_key_contains_percent_character() { + public void fail_when_key_contains_percent_character() { expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Malformed key for Project: ' '"); + expectedException.expectMessage("Malformed key for Project: 'roject%Key'"); underTest.create(db.getSession(), NewComponent.newComponentBuilder() - .setKey(" ") + .setKey("roject%Key") .setName(DEFAULT_PROJECT_NAME) .setOrganizationUuid(db.getDefaultOrganization().getUuid()) .build(), diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java index a85345e71ce..638c71b45d1 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java @@ -60,6 +60,7 @@ import static org.sonarqube.ws.client.project.ProjectsWsParameters.PARAM_PROJECT import static org.sonarqube.ws.client.project.ProjectsWsParameters.PARAM_TO; public class BulkUpdateKeyActionTest { + private static final String MY_PROJECT_KEY = "my_project"; private static final String FROM = "my_"; private static final String TO = "your_"; @@ -169,9 +170,9 @@ public class BulkUpdateKeyActionTest { insertMyProject(); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Malformed key for 'my aproject'. Project key cannot be empty nor contain whitespaces."); + expectedException.expectMessage("Malformed key for 'my?project'. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); - callByKey(MY_PROJECT_KEY, FROM, "my a"); + callByKey(MY_PROJECT_KEY, FROM, "my?"); } @Test @@ -179,9 +180,9 @@ public class BulkUpdateKeyActionTest { insertMyProject(); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Malformed key for 'my aproject'. Project key cannot be empty nor contain whitespaces."); + expectedException.expectMessage("Malformed key for 'my?project'. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); - callDryRunByKey(MY_PROJECT_KEY, FROM, "my a"); + callDryRunByKey(MY_PROJECT_KEY, FROM, "my?"); } @Test diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/CreateActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/CreateActionTest.java index def3cc53b02..4b23370bc14 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/CreateActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/project/ws/CreateActionTest.java @@ -273,14 +273,14 @@ public class CreateActionTest { } @Test - public void properly_fail_when_invalid_project_key() { + public void fail_when_invalid_project_key() { userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization()); expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Malformed key for Project: 'project Key'. It cannot be empty nor contain whitespaces."); + expectedException.expectMessage("Malformed key for Project: 'project%Key'. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); call(CreateRequest.builder() - .setProjectKey("project Key") + .setProjectKey("project%Key") .setName(DEFAULT_PROJECT_NAME) .build()); } diff --git a/sonar-core/src/main/java/org/sonar/core/component/ComponentKeys.java b/sonar-core/src/main/java/org/sonar/core/component/ComponentKeys.java index 8378a30bab1..5eb95b14534 100644 --- a/sonar-core/src/main/java/org/sonar/core/component/ComponentKeys.java +++ b/sonar-core/src/main/java/org/sonar/core/component/ComponentKeys.java @@ -19,6 +19,7 @@ */ package org.sonar.core.component; +import java.util.regex.Pattern; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; @@ -28,15 +29,14 @@ public final class ComponentKeys { public static final int MAX_COMPONENT_KEY_LENGTH = 400; - /* - * Must not be blank or empty - */ - private static final String VALID_PROJECT_KEY_REGEXP = "[^\\p{javaWhitespace}]+"; + public static final String ALLOWED_CHARACTERS_MESSAGE = "Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit"; + + public static final String MALFORMED_KEY_MESSAGE = "Malformed key for '%s'. %s."; - /* - * Allowed characters are alphanumeric, '-', '_', '.' and '/' + /** + * Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit */ - private static final String VALID_BRANCH_REGEXP = "[\\p{Alnum}\\-_./]*"; + private static final Pattern VALID_PROJECT_KEY_REGEXP = Pattern.compile("[\\p{Alnum}\\-_.:]*[\\p{Alpha}\\-_.:]+[\\p{Alnum}\\-_.:]*"); private static final String KEY_WITH_BRANCH_FORMAT = "%s:%s"; @@ -53,13 +53,8 @@ public final class ComponentKeys { return sb.toString(); } - /** - * Test if given parameter is valid for a project. A key is valid if it doesn't contain whitespaces. - * - * @return true if keyCandidate can be used for a project - */ public static boolean isValidProjectKey(String keyCandidate) { - return keyCandidate.matches(VALID_PROJECT_KEY_REGEXP); + return VALID_PROJECT_KEY_REGEXP.matcher(keyCandidate).matches(); } /** @@ -68,7 +63,7 @@ public final class ComponentKeys { * @throws IllegalArgumentException if the format is incorrect */ public static void checkProjectKey(String keyCandidate) { - checkArgument(isValidProjectKey(keyCandidate), "Malformed key for '%s'. %s", keyCandidate, "Project key cannot be empty nor contain whitespaces."); + checkArgument(isValidProjectKey(keyCandidate), MALFORMED_KEY_MESSAGE, keyCandidate, ALLOWED_CHARACTERS_MESSAGE); } /** diff --git a/sonar-core/src/test/java/org/sonar/core/component/ComponentKeysTest.java b/sonar-core/src/test/java/org/sonar/core/component/ComponentKeysTest.java index 05848ac1c56..eef16fac229 100644 --- a/sonar-core/src/test/java/org/sonar/core/component/ComponentKeysTest.java +++ b/sonar-core/src/test/java/org/sonar/core/component/ComponentKeysTest.java @@ -37,12 +37,18 @@ public class ComponentKeysTest { } @Test - public void isValidProjectKey() { + public void valid_project_key() { assertThat(ComponentKeys.isValidProjectKey("abc")).isTrue(); - assertThat(ComponentKeys.isValidProjectKey("0123")).isTrue(); assertThat(ComponentKeys.isValidProjectKey("ab_12")).isTrue(); - assertThat(ComponentKeys.isValidProjectKey("ab/12")).isTrue(); - assertThat(ComponentKeys.isValidProjectKey("코드품질")).isTrue(); + } + + @Test + public void invalid_project_key() { + assertThat(ComponentKeys.isValidProjectKey("0123")).isFalse(); + + assertThat(ComponentKeys.isValidProjectKey("ab/12")).isFalse(); + assertThat(ComponentKeys.isValidProjectKey("코드품질")).isFalse(); + assertThat(ComponentKeys.isValidProjectKey("")).isFalse(); assertThat(ComponentKeys.isValidProjectKey(" ")).isFalse(); assertThat(ComponentKeys.isValidProjectKey("ab 12")).isFalse(); @@ -69,4 +75,19 @@ public class ComponentKeysTest { ComponentKeys.checkProjectKey("ab 12"); } + + @Test + public void checkProjectKey_fail_if_only_digit() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Malformed key for '0123'. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit."); + + ComponentKeys.checkProjectKey("0123"); + } + + @Test + public void checkProjectKey_fail_if_special_characters_not_allowed() { + expectedException.expect(IllegalArgumentException.class); + + ComponentKeys.checkProjectKey("ab/12"); + } } 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 319d5eabf63..ccb7173b572 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 @@ -20,20 +20,23 @@ package org.sonar.scanner.scan; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.stream.Stream; 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; import static org.sonar.core.config.ScannerProperties.BRANCHES_DOC_LINK; import static org.sonar.core.config.ScannerProperties.BRANCH_NAME; import static org.sonar.core.config.ScannerProperties.PULL_REQUEST_BASE; @@ -46,6 +49,9 @@ 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 @@ -65,7 +71,7 @@ public class ProjectReactorValidator { List validationMessages = new ArrayList<>(); for (ProjectDefinition moduleDef : reactor.getProjects()) { - validateModule(moduleDef, validationMessages); + validateModule(moduleDef); } if (isBranchFeatureAvailable()) { @@ -82,7 +88,7 @@ public class ProjectReactorValidator { } private void validateBranchParamsWhenPluginAbsent(List validationMessages) { - for (String param : Arrays.asList(BRANCH_NAME)) { + 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)); @@ -97,9 +103,12 @@ public class ProjectReactorValidator { + "See %s for more information.", param, BRANCHES_DOC_LINK))); } - private static void validateModule(ProjectDefinition moduleDef, List validationMessages) { + private static void validateModule(ProjectDefinition moduleDef) { if (!ComponentKeys.isValidProjectKey(moduleDef.getKey())) { - validationMessages.add(format("\"%s\" is not a valid project or module key. It cannot be empty nor contain whitespaces.", 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); } } 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 91993c340c5..f74dd8df5bd 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 @@ -34,11 +34,15 @@ 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.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -50,6 +54,9 @@ 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); @@ -86,12 +93,36 @@ public class ProjectReactorValidatorTest { } @Test - public void fail_with_invalid_key() { - ProjectReactor reactor = createProjectReactor(" "); + public void log_warning_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."); + } + + @Test + public void log_warning_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."); + } + + @Test + public void log_warning_when_backslash_in_key() { + ProjectReactor reactor = createProjectReactor("foo\\bar"); - thrown.expect(MessageException.class); - thrown.expectMessage("\" \" is not a valid project or module key"); 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."); } @Test -- 2.39.5