From e2699fa9dc347f2a54216d03b649ea37cd3777ad Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 11 Mar 2020 13:14:13 +0100 Subject: [PATCH] SONAR-13160 Fix display of Portfolio Admin page when project contains UTF-8 characters --- .../step/ValidateProjectStep.java | 37 ++++++++---- .../step/ValidateProjectStepTest.java | 59 ++++++++++++++----- .../component/ComponentKeyUpdaterDaoTest.java | 6 +- .../server/component/ComponentService.java | 9 +-- .../server/component/ComponentUpdater.java | 3 +- .../ComponentServiceUpdateKeyTest.java | 13 ++-- .../component/ComponentUpdaterTest.java | 10 ++-- .../project/ws/BulkUpdateKeyActionTest.java | 9 +-- .../server/project/ws/CreateActionTest.java | 6 +- .../sonar/core/component/ComponentKeys.java | 22 +++---- .../core/component/ComponentKeysTest.java | 29 +++++++-- .../scanner/scan/ProjectReactorValidator.java | 14 ++++- .../scan/ProjectReactorValidatorTest.java | 39 ++++++++++-- 13 files changed, 176 insertions(+), 80 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 441e9eefbb8..4595560277d 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; @@ -46,17 +48,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. short living branch or PR targets a branch that still contains modules
    6. - *
- */ public class ValidateProjectStep implements ComputationStep { private static final Joiner MESSAGES_JOINER = Joiner.on("\n o "); @@ -64,11 +58,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 @@ -76,6 +74,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); @@ -123,8 +122,20 @@ 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. + 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) { 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 05e3136e39f..13e4ba22ffe 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 @@ -27,6 +27,8 @@ import org.junit.rules.ExpectedException; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.MessageException; import org.sonar.api.utils.System2; +import org.sonar.api.utils.internal.TestSystem2; +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,31 +44,38 @@ 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.verifyZeroInteractions; import static org.mockito.Mockito.when; public class ValidateProjectStepTest { - static long DEFAULT_ANALYSIS_TIME = 1433131200000L; // 2015-06-01 - static final String PROJECT_KEY = "PROJECT_KEY"; - static final Branch DEFAULT_BRANCH = new DefaultBranchImpl(); + private static final String PROJECT_KEY = "PROJECT_KEY"; + + private static long PAST_ANALYSIS_TIME = 1_420_088_400_000L; // 2015-01-01 + private static long DEFAULT_ANALYSIS_TIME = 1_433_131_200_000L; // 2015-06-01 + private static long NOW = 1_500_000_000_000L; + + private static final Branch DEFAULT_BRANCH = new DefaultBranchImpl(); @Rule public DbTester dbTester = 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); - DbClient dbClient = dbTester.getDbClient(); + private System2 system2 = new TestSystem2().setNow(NOW); + + private CeTaskMessages taskMessages = mock(CeTaskMessages.class); + private DbClient dbClient = dbTester.getDbClient(); - ValidateProjectStep underTest = new ValidateProjectStep(dbClient, treeRootHolder, analysisMetadataHolder); + private ValidateProjectStep underTest = new ValidateProjectStep(dbClient, treeRootHolder, analysisMetadataHolder, taskMessages, system2); @Test public void fail_if_slb_is_targeting_master_with_modules() { @@ -112,6 +121,7 @@ public class ValidateProjectStepTest { .build()); underTest.execute(new TestComputationStepContext()); + verifyZeroInteractions(taskMessages); } @Test @@ -126,13 +136,7 @@ public class ValidateProjectStepTest { .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.getMergeBranchUuid()).thenReturn(mergeBranchUuid); - analysisMetadataHolder.setBranch(branch); + verifyZeroInteractions(taskMessages); } @Test @@ -166,4 +170,29 @@ public class ValidateProjectStepTest { underTest.execute(new TestComputationStepContext()); } + @Test + public void add_warning_when_project_key_is_invalid() { + ComponentDto project = dbTester.components().insertPrivateProject(p -> p.setDbKey("inv$lid!")); + dbTester.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.getMergeBranchUuid()).thenReturn(mergeBranchUuid); + analysisMetadataHolder.setBranch(branch); + } + } 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 faf276cf947..d41c321949e 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 @@ -314,9 +314,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 @@ -377,7 +377,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-server/src/main/java/org/sonar/server/component/ComponentService.java b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java index 2b25909072c..9448328a578 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java @@ -40,9 +40,8 @@ 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.core.component.ComponentKeys.checkProjectKey; import static org.sonar.db.component.ComponentKeyUpdaterDao.checkIsProjectOrModule; -import static org.sonar.server.ws.WsUtils.checkRequest; @ServerSide public class ComponentService { @@ -61,7 +60,7 @@ public class ComponentService { public void updateKey(DbSession dbSession, ComponentDto projectOrModule, String newKey) { userSession.checkComponentPermission(UserRole.ADMIN, projectOrModule); checkIsProjectOrModule(projectOrModule); - checkProjectOrModuleKeyFormat(newKey); + checkProjectKey(newKey); dbClient.componentKeyUpdaterDao().updateKey(dbSession, projectOrModule.uuid(), newKey); projectIndexers.commitAndIndex(dbSession, singletonList(projectOrModule), ProjectIndexer.Cause.PROJECT_KEY_UPDATE); if (isMainProject(projectOrModule)) { @@ -101,8 +100,4 @@ public class ComponentService { return new RekeyedProject(project, rekeyedResource.getOldKey()); } - private static void checkProjectOrModuleKeyFormat(String key) { - checkRequest(isValidProjectKey(key), "Malformed key for '%s'. It cannot be empty nor contain whitespaces.", key); - } - } diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentUpdater.java index da4c1c3cb93..5a9d21623ed 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentUpdater.java @@ -43,6 +43,7 @@ import org.sonar.server.permission.PermissionTemplateService; import static java.util.Collections.singletonList; 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.ws.WsUtils.checkRequest; @@ -163,7 +164,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 void checkLegacyBranchFormat(String qualifier, @Nullable String branch) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceUpdateKeyTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceUpdateKeyTest.java index 66583ddfbf2..6ca997fb929 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceUpdateKeyTest.java +++ b/server/sonar-server/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; @@ -152,21 +151,21 @@ public class ComponentServiceUpdateKeyTest { ComponentDto project = insertSampleRootProject(); 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, project, ""); } @Test - public void fail_if_new_key_is_invalid() { + public void fail_if_new_key_is_not_formatted_correctly() { ComponentDto project = insertSampleRootProject(); 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, project, "sample root"); + underTest.updateKey(dbSession, project, "sample?root"); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java index 9739cfae408..2b4e8908f78 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java @@ -304,11 +304,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(), @@ -316,13 +316,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: 'project%Key'"); underTest.create(db.getSession(), NewComponent.newComponentBuilder() - .setKey(" ") + .setKey("project%Key") .setName(DEFAULT_PROJECT_NAME) .setOrganizationUuid(db.getDefaultOrganization().getUuid()) .build(), diff --git a/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java index aa3e20bf131..ba997276472 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkUpdateKeyActionTest.java @@ -62,6 +62,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_"; @@ -183,9 +184,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 @@ -193,9 +194,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-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java index cddc20157bd..8db71a6b7d9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java @@ -305,14 +305,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() - .setKey("project Key") + .setKey("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 79330b39fe1..38a67c2642e 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; import org.sonar.api.batch.fs.internal.DefaultInputFile; @@ -29,12 +30,16 @@ public final class ComponentKeys { public static final int MAX_COMPONENT_KEY_LENGTH = 400; - /* - * Must not be blank or empty + 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 ':', with at least one non-digit */ - private static final String VALID_PROJECT_KEY_REGEXP = "[^\\p{javaWhitespace}]+"; + private static final Pattern VALID_PROJECT_KEY_REGEXP = Pattern.compile("[\\p{Alnum}\\-_.:]*[\\p{Alpha}\\-_.:]+[\\p{Alnum}\\-_.:]*"); - /* + /** * Allowed characters are alphanumeric, '-', '_', '.' and '/' */ private static final String VALID_BRANCH_REGEXP = "[\\p{Alnum}\\-_./]*"; @@ -58,13 +63,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(); } /** @@ -73,7 +73,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 5187d09bd24..2f90cd67452 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 @@ -47,12 +47,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(); @@ -91,4 +97,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 f639e0f02fd..cd55e517e97 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 @@ -28,6 +28,8 @@ 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; @@ -35,6 +37,7 @@ import org.sonar.scanner.scan.branch.BranchParamsValidator; import static java.lang.String.format; 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.BRANCH_TARGET; @@ -48,6 +51,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 @@ -67,7 +73,7 @@ public class ProjectReactorValidator { List validationMessages = new ArrayList<>(); for (ProjectDefinition moduleDef : reactor.getProjects()) { - validateModule(moduleDef, validationMessages); + validateModule(moduleDef); } String deprecatedBranchName = reactor.getRoot().getBranch(); @@ -102,9 +108,11 @@ 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. + 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 464ea32f912..43ce7869847 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