Skip to content

Commit

Permalink
Revert "SONAR-6589 Fix project and module key validation"
Browse files Browse the repository at this point in the history 
This reverts commit df0e388.
  • Loading branch information
@julienlancelot
julienlancelot committed Jun 1, 2015
1 parent b99b802 commit 5fc1d3a
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 112 deletions.
23 changes: 9 additions & 14 deletions server/sonar-server/src/main/java/org/sonar/server/computation/step/ValidateProjectStep.java
View file
Expand Up @@ -31,7 +31,6 @@
import org.sonar.api.CoreProperties; import org.sonar.api.CoreProperties;
import org.sonar.api.config.Settings; import org.sonar.api.config.Settings;
import org.sonar.batch.protocol.output.BatchReport; import org.sonar.batch.protocol.output.BatchReport;
import org.sonar.batch.protocol.output.BatchReportReader;
import org.sonar.core.component.ComponentDto; import org.sonar.core.component.ComponentDto;
import org.sonar.core.component.ComponentKeys; import org.sonar.core.component.ComponentKeys;
import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbSession;
Expand Down Expand Up @@ -75,7 +74,7 @@ public String apply(@Nonnull ComponentDto input) {
} }
}); });
ValidateProjectsVisitor visitor = new ValidateProjectsVisitor(session, dbClient.componentDao(), context.getReportMetadata(), ValidateProjectsVisitor visitor = new ValidateProjectsVisitor(session, dbClient.componentDao(), context.getReportMetadata(),
context.getReportReader(), settings.getBoolean(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION), modulesByKey); settings.getBoolean(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION), modulesByKey);
visitor.visit(context.getRoot()); visitor.visit(context.getRoot());


if (!visitor.validationMessages.isEmpty()) { if (!visitor.validationMessages.isEmpty()) {
Expand All @@ -95,21 +94,18 @@ private static class ValidateProjectsVisitor extends DepthTraversalTypeAwareVisi
private final DbSession session; private final DbSession session;
private final ComponentDao componentDao; private final ComponentDao componentDao;
private final BatchReport.Metadata reportMetadata; private final BatchReport.Metadata reportMetadata;
private final BatchReportReader batchReportReader;
private final boolean preventAutomaticProjectCreation; private final boolean preventAutomaticProjectCreation;
private final Map<String, ComponentDto> modulesByKey; private final Map<String, ComponentDto> modulesByKey;
private final List<String> validationMessages = new ArrayList<>(); private final List<String> validationMessages = new ArrayList<>();


private Component root; private Component root;


public ValidateProjectsVisitor(DbSession session, ComponentDao componentDao, BatchReport.Metadata reportMetadata, BatchReportReader batchReportReader, public ValidateProjectsVisitor(DbSession session, ComponentDao componentDao, BatchReport.Metadata reportMetadata, boolean preventAutomaticProjectCreation,
boolean preventAutomaticProjectCreation, Map<String, ComponentDto> modulesByKey) { Map<String, ComponentDto> modulesByKey) {
super(Component.Type.MODULE, Order.PRE_ORDER); super(Component.Type.MODULE, Order.PRE_ORDER);
this.session = session; this.session = session;
this.componentDao = componentDao; this.componentDao = componentDao;
this.reportMetadata = reportMetadata; this.reportMetadata = reportMetadata;
this.batchReportReader = batchReportReader;

this.preventAutomaticProjectCreation = preventAutomaticProjectCreation; this.preventAutomaticProjectCreation = preventAutomaticProjectCreation;
this.modulesByKey = modulesByKey; this.modulesByKey = modulesByKey;
} }
Expand All @@ -118,7 +114,6 @@ public ValidateProjectsVisitor(DbSession session, ComponentDao componentDao, Bat
public void visitProject(Component project) { public void visitProject(Component project) {
this.root = project; this.root = project;
validateBranch(); validateBranch();
validateBatchKey(project);


String projectKey = project.getKey(); String projectKey = project.getKey();
ComponentDto projectDto = loadComponent(projectKey); ComponentDto projectDto = loadComponent(projectKey);
Expand All @@ -133,13 +128,14 @@ public void visitProject(Component project) {
+ "If you really want to stop directly analysing project \"%s\", please first delete it from SonarQube and then relaunch the analysis of project \"%s\".", + "If you really want to stop directly analysing project \"%s\", please first delete it from SonarQube and then relaunch the analysis of project \"%s\".",
projectKey, anotherProject.key(), anotherProject.key(), projectKey)); projectKey, anotherProject.key(), anotherProject.key(), projectKey));
} }
validateKey(projectKey);
} }


@Override @Override
public void visitModule(Component module) { public void visitModule(Component module) {
String projectKey = root.getKey();
String moduleKey = module.getKey(); String moduleKey = module.getKey();
validateBatchKey(module); String projectKey = root.getKey();
validateKey(moduleKey);


ComponentDto moduleDto = loadComponent(moduleKey); ComponentDto moduleDto = loadComponent(moduleKey);
if (moduleDto == null) { if (moduleDto == null) {
Expand All @@ -156,11 +152,10 @@ public void visitModule(Component module) {
} }
} }


private void validateBatchKey(Component component) { private void validateKey(String moduleKey) {
String batchKey = batchReportReader.readComponent(component.getRef()).getKey(); if (!ComponentKeys.isValidModuleKey(moduleKey)) {
if (!ComponentKeys.isValidModuleKey(batchKey)) {
validationMessages.add(String.format("\"%s\" is not a valid project or module key. " validationMessages.add(String.format("\"%s\" is not a valid project or module key. "
+ "Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.", batchKey)); + "Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.", moduleKey));
} }
} }


Expand Down
119 changes: 21 additions & 98 deletions ...sonar-server/src/test/java/org/sonar/server/computation/step/ValidateProjectStepTest.java
View file
Expand Up @@ -21,6 +21,7 @@
package org.sonar.server.computation.step; package org.sonar.server.computation.step;


import java.io.File; import java.io.File;
import java.io.IOException;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.ClassRule; import org.junit.ClassRule;
Expand All @@ -30,7 +31,6 @@
import org.junit.rules.TemporaryFolder; import org.junit.rules.TemporaryFolder;
import org.sonar.api.CoreProperties; import org.sonar.api.CoreProperties;
import org.sonar.api.config.Settings; import org.sonar.api.config.Settings;
import org.sonar.batch.protocol.Constants;
import org.sonar.batch.protocol.output.BatchReport; import org.sonar.batch.protocol.output.BatchReport;
import org.sonar.batch.protocol.output.BatchReportReader; import org.sonar.batch.protocol.output.BatchReportReader;
import org.sonar.batch.protocol.output.BatchReportWriter; import org.sonar.batch.protocol.output.BatchReportWriter;
Expand Down Expand Up @@ -65,9 +65,6 @@ public class ValidateProjectStepTest {


Settings settings; Settings settings;


File reportDir;
BatchReportWriter writer;

ValidateProjectStep sut; ValidateProjectStep sut;


@Before @Before
Expand All @@ -76,8 +73,6 @@ public void setUp() throws Exception {
dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new ComponentDao()); dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new ComponentDao());
dbSession = dbClient.openSession(false); dbSession = dbClient.openSession(false);
settings = new Settings(); settings = new Settings();
reportDir = temp.newFolder();
writer = new BatchReportWriter(reportDir);


sut = new ValidateProjectStep(dbClient, settings); sut = new ValidateProjectStep(dbClient, settings);
} }
Expand All @@ -89,18 +84,11 @@ public void tearDown() throws Exception {


@Test @Test
public void not_fail_if_provisioning_enforced_and_project_exists() throws Exception { public void not_fail_if_provisioning_enforced_and_project_exists() throws Exception {
writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.build());

settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "true"); settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "true");
dbClient.componentDao().insert(dbSession, ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY)); dbClient.componentDao().insert(dbSession, ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY));
dbSession.commit(); dbSession.commit();


sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null)); ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
} }


Expand All @@ -109,66 +97,46 @@ public void fail_if_provisioning_enforced_and_project_does_not_exists() throws E
thrown.expect(IllegalArgumentException.class); thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("Unable to scan non-existing project '" + PROJECT_KEY + "'"); thrown.expectMessage("Unable to scan non-existing project '" + PROJECT_KEY + "'");


writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.build());

settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "true"); settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "true");


sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null)); ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
} }


@Test @Test
public void fail_if_provisioning_not_enforced_and_project_does_not_exists() throws Exception { public void fail_if_provisioning_not_enforced_and_project_does_not_exists() throws Exception {
writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.build());

settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "false"); settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "false");


sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null)); ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
} }


@Test @Test
public void not_fail_on_valid_branch() throws Exception { public void not_fail_on_valid_branch() throws Exception {
File reportDir = temp.newFolder();
BatchReportWriter writer = new BatchReportWriter(reportDir);
writer.writeMetadata(BatchReport.Metadata.newBuilder() writer.writeMetadata(BatchReport.Metadata.newBuilder()
.setBranch("origin/master") .setBranch("origin/master")
.build()); .build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.build());


sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY + ":origin/master")), null)); ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
} }


@Test @Test
public void fail_on_invalid_branch() throws Exception { public void fail_on_invalid_branch() throws Exception {
File reportDir = temp.newFolder();
thrown.expect(IllegalArgumentException.class); thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("Validation of project failed:\n" + thrown.expectMessage("Validation of project failed:\n" +
" o \"bran#ch\" is not a valid branch name. Allowed characters are alphanumeric, '-', '_', '.' and '/'."); " o \"bran#ch\" is not a valid branch name. Allowed characters are alphanumeric, '-', '_', '.' and '/'.");


BatchReportWriter writer = new BatchReportWriter(reportDir);
writer.writeMetadata(BatchReport.Metadata.newBuilder() writer.writeMetadata(BatchReport.Metadata.newBuilder()
.setBranch("bran#ch") .setBranch("bran#ch")
.build()); .build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.build());


sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY + ":bran#ch")), null)); ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
} }


@Test @Test
Expand All @@ -180,22 +148,9 @@ public void fail_on_invalid_key() throws Exception {
" o \"Project\\Key\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.\n" + " o \"Project\\Key\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.\n" +
" o \"Module$Key\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit"); " o \"Module$Key\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit");


writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(invalidProjectKey)
.addChildRef(2)
.build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(2)
.setType(Constants.ComponentType.MODULE)
.setKey("Module$Key")
.build());

DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", invalidProjectKey, DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", invalidProjectKey,
new DumbComponent(Component.Type.MODULE, 2, "BCDE", "Module$Key")); new DumbComponent(Component.Type.MODULE, 2, "BCDE", "Module$Key"));
sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, ComponentTreeBuilders.from(root), null)); sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null, ComponentTreeBuilders.from(root), null));
} }


@Test @Test
Expand All @@ -206,26 +161,13 @@ public void fail_if_module_key_is_already_used_as_project_key() throws Exception
"If you really want to stop directly analysing project \"" + MODULE_KEY + "\", please first delete it from SonarQube and then relaunch the analysis of project \"" "If you really want to stop directly analysing project \"" + MODULE_KEY + "\", please first delete it from SonarQube and then relaunch the analysis of project \""
+ PROJECT_KEY + "\"."); + PROJECT_KEY + "\".");


writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.addChildRef(2)
.build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(2)
.setType(Constants.ComponentType.MODULE)
.setKey(MODULE_KEY)
.build());

ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(MODULE_KEY); ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(MODULE_KEY);
dbClient.componentDao().insert(dbSession, project); dbClient.componentDao().insert(dbSession, project);
dbSession.commit(); dbSession.commit();


DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY, DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY,
new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY)); new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY));
sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
ComponentTreeBuilders.from(root), null)); ComponentTreeBuilders.from(root), null));
} }


Expand All @@ -236,19 +178,6 @@ public void fail_if_module_key_already_exists_in_another_project() throws Except
thrown.expectMessage("Validation of project failed:\n" + thrown.expectMessage("Validation of project failed:\n" +
" o Module \"" + MODULE_KEY + "\" is already part of project \"" + anotherProjectKey + "\""); " o Module \"" + MODULE_KEY + "\" is already part of project \"" + anotherProjectKey + "\"");


writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.addChildRef(2)
.build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(2)
.setType(Constants.ComponentType.MODULE)
.setKey(MODULE_KEY)
.build());

ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY); ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY);
ComponentDto anotherProject = ComponentTesting.newProjectDto().setKey(anotherProjectKey); ComponentDto anotherProject = ComponentTesting.newProjectDto().setKey(anotherProjectKey);
dbClient.componentDao().insert(dbSession, project, anotherProject); dbClient.componentDao().insert(dbSession, project, anotherProject);
Expand All @@ -258,7 +187,7 @@ public void fail_if_module_key_already_exists_in_another_project() throws Except


DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY, DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY,
new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY)); new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY));
sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
ComponentTreeBuilders.from(root), null)); ComponentTreeBuilders.from(root), null));
} }


Expand All @@ -271,19 +200,6 @@ public void fail_if_project_key_already_exists_as_module() throws Exception {
"If you really want to stop directly analysing project \"" + anotherProjectKey + "\", please first delete it from SonarQube and then relaunch the analysis of project \"" "If you really want to stop directly analysing project \"" + anotherProjectKey + "\", please first delete it from SonarQube and then relaunch the analysis of project \""
+ PROJECT_KEY + "\"."); + PROJECT_KEY + "\".");


writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(1)
.setType(Constants.ComponentType.PROJECT)
.setKey(PROJECT_KEY)
.addChildRef(2)
.build());
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(2)
.setType(Constants.ComponentType.MODULE)
.setKey(MODULE_KEY)
.build());

ComponentDto anotherProject = ComponentTesting.newProjectDto().setKey(anotherProjectKey); ComponentDto anotherProject = ComponentTesting.newProjectDto().setKey(anotherProjectKey);
dbClient.componentDao().insert(dbSession, anotherProject); dbClient.componentDao().insert(dbSession, anotherProject);
ComponentDto module = ComponentTesting.newModuleDto("ABCD", anotherProject).setKey(PROJECT_KEY); ComponentDto module = ComponentTesting.newModuleDto("ABCD", anotherProject).setKey(PROJECT_KEY);
Expand All @@ -292,8 +208,15 @@ public void fail_if_project_key_already_exists_as_module() throws Exception {


DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY, DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY,
new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY)); new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY));
sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
ComponentTreeBuilders.from(root), null)); ComponentTreeBuilders.from(root), null));
} }


private BatchReportReader createBasicBatchReportReader() throws IOException {
File reportDir = temp.newFolder();
BatchReportWriter writer = new BatchReportWriter(reportDir);
writer.writeMetadata(BatchReport.Metadata.newBuilder()
.build());
return new BatchReportReader(reportDir);
}
} }

0 comments on commit 5fc1d3a

Please sign in to comment.