From 2d80a79c182283831b0b32047fa5f78580dd767e Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Thu, 17 Jan 2013 18:13:26 +0100 Subject: SONAR-4069 Minor improvements and fixes for tasks * Try to break package cycle * Add validation on TaskDefinition --- .../sonar/batch/bootstrap/InspectionModule.java | 131 ++++++++++++++++++++ .../java/org/sonar/batch/bootstrapper/Batch.java | 2 +- .../org/sonar/batch/tasks/InspectionModule.java | 135 --------------------- .../java/org/sonar/batch/tasks/InspectionTask.java | 2 + .../src/main/java/org/sonar/batch/tasks/Tasks.java | 74 ++++++++++- .../batch/bootstrap/InspectionModuleTest.java | 1 - .../batch/bootstrap/TaskBootstrapModuleTest.java | 2 +- .../test/java/org/sonar/batch/tasks/TasksTest.java | 94 ++++++++++++++ 8 files changed, 298 insertions(+), 143 deletions(-) create mode 100644 sonar-batch/src/main/java/org/sonar/batch/bootstrap/InspectionModule.java delete mode 100644 sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionModule.java (limited to 'sonar-batch') diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/InspectionModule.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/InspectionModule.java new file mode 100644 index 00000000000..c22a9132eff --- /dev/null +++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/InspectionModule.java @@ -0,0 +1,131 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2012 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.batch.bootstrap; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.sonar.api.batch.BatchExtensionDictionnary; +import org.sonar.api.batch.bootstrap.ProjectDefinition; +import org.sonar.api.profiles.RulesProfile; +import org.sonar.api.resources.Languages; +import org.sonar.api.resources.Project; +import org.sonar.api.tests.ProjectTestsImpl; +import org.sonar.batch.DefaultProfileLoader; +import org.sonar.batch.DefaultProjectClasspath; +import org.sonar.batch.DefaultProjectFileSystem2; +import org.sonar.batch.DefaultSensorContext; +import org.sonar.batch.DefaultTimeMachine; +import org.sonar.batch.ProfileProvider; +import org.sonar.batch.ProjectTree; +import org.sonar.batch.ResourceFilters; +import org.sonar.batch.ViolationFilters; +import org.sonar.batch.components.TimeMachineConfiguration; +import org.sonar.batch.events.EventBus; +import org.sonar.batch.index.DefaultIndex; +import org.sonar.batch.index.ResourcePersister; +import org.sonar.batch.local.DryRunExporter; +import org.sonar.batch.phases.Phases; +import org.sonar.batch.phases.PhasesTimeProfiler; +import org.sonar.core.qualitymodel.DefaultModelFinder; +import org.sonar.jpa.dao.ProfilesDao; +import org.sonar.jpa.dao.RulesDao; + +public class InspectionModule extends Module { + private static final Logger LOG = LoggerFactory.getLogger(InspectionModule.class); + private Project project; + + public InspectionModule(Project project) { + this.project = project; + } + + @Override + protected void configure() { + logSettings(); + addCoreComponents(); + addPluginExtensions(); + } + + private void addCoreComponents() { + ProjectDefinition projectDefinition = container.getComponentByType(ProjectTree.class).getProjectDefinition(project); + container.addSingleton(projectDefinition); + container.addSingleton(project.getConfiguration()); + container.addSingleton(project); + container.addSingleton(ProjectSettings.class); + + // hack to initialize commons-configuration before ExtensionProviders + container.getComponentByType(ProjectSettings.class); + + container.addSingleton(EventBus.class); + container.addSingleton(Phases.class); + container.addSingleton(PhasesTimeProfiler.class); + for (Class clazz : Phases.getPhaseClasses()) { + container.addSingleton(clazz); + } + container.addSingleton(UnsupportedProperties.class); + + for (Object component : projectDefinition.getContainerExtensions()) { + container.addSingleton(component); + } + container.addSingleton(Languages.class); + container.addSingleton(DefaultProjectClasspath.class); + container.addSingleton(DefaultProjectFileSystem2.class); + container.addSingleton(RulesDao.class); + + // the Snapshot component will be removed when asynchronous measures are improved (required for AsynchronousMeasureSensor) + container.addSingleton(container.getComponentByType(ResourcePersister.class).getSnapshot(project)); + + container.addSingleton(TimeMachineConfiguration.class); + container.addSingleton(org.sonar.api.database.daos.MeasuresDao.class); + container.addSingleton(ProfilesDao.class); + container.addSingleton(DefaultSensorContext.class); + container.addSingleton(BatchExtensionDictionnary.class); + container.addSingleton(DefaultTimeMachine.class); + container.addSingleton(ViolationFilters.class); + container.addSingleton(ResourceFilters.class); + container.addSingleton(DefaultModelFinder.class); + container.addSingleton(DefaultProfileLoader.class); + container.addSingleton(DryRunExporter.class); + container.addSingleton(ProjectTestsImpl.class); + container.addPicoAdapter(new ProfileProvider()); + } + + private void addPluginExtensions() { + ExtensionInstaller installer = container.getComponentByType(ExtensionInstaller.class); + installer.installInspectionExtensions(container); + } + + private void logSettings() { + LOG.info("------------- Inspecting {}", project.getName()); + } + + /** + * Analyze project + */ + @Override + protected void doStart() { + DefaultIndex index = container.getComponentByType(DefaultIndex.class); + index.setCurrentProject(project, + container.getComponentByType(ResourceFilters.class), + container.getComponentByType(ViolationFilters.class), + container.getComponentByType(RulesProfile.class)); + + container.getComponentByType(Phases.class).execute(project); + } +} diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrapper/Batch.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrapper/Batch.java index b890f440824..1918abaf85e 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/bootstrapper/Batch.java +++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrapper/Batch.java @@ -55,7 +55,7 @@ public final class Batch { this.taskCommand = builder.taskCommand; projectReactor = builder.projectReactor; if (builder.isEnableLoggingConfiguration()) { - logging = LoggingConfiguration.create().setProperties(Maps.fromProperties(projectReactor.getRoot().getProperties())); + logging = LoggingConfiguration.create().setProperties(globalProperties); } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionModule.java b/sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionModule.java deleted file mode 100644 index 116db16216d..00000000000 --- a/sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionModule.java +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Sonar, open source software quality management tool. - * Copyright (C) 2008-2012 SonarSource - * mailto:contact AT sonarsource DOT com - * - * Sonar is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * Sonar is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with Sonar; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 - */ -package org.sonar.batch.tasks; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.sonar.api.batch.BatchExtensionDictionnary; -import org.sonar.api.batch.bootstrap.ProjectDefinition; -import org.sonar.api.profiles.RulesProfile; -import org.sonar.api.resources.Languages; -import org.sonar.api.resources.Project; -import org.sonar.api.tests.ProjectTestsImpl; -import org.sonar.batch.DefaultProfileLoader; -import org.sonar.batch.DefaultProjectClasspath; -import org.sonar.batch.DefaultProjectFileSystem2; -import org.sonar.batch.DefaultSensorContext; -import org.sonar.batch.DefaultTimeMachine; -import org.sonar.batch.ProfileProvider; -import org.sonar.batch.ProjectTree; -import org.sonar.batch.ResourceFilters; -import org.sonar.batch.ViolationFilters; -import org.sonar.batch.bootstrap.ExtensionInstaller; -import org.sonar.batch.bootstrap.Module; -import org.sonar.batch.bootstrap.ProjectSettings; -import org.sonar.batch.bootstrap.UnsupportedProperties; -import org.sonar.batch.components.TimeMachineConfiguration; -import org.sonar.batch.events.EventBus; -import org.sonar.batch.index.DefaultIndex; -import org.sonar.batch.index.ResourcePersister; -import org.sonar.batch.local.DryRunExporter; -import org.sonar.batch.phases.Phases; -import org.sonar.batch.phases.PhasesTimeProfiler; -import org.sonar.core.qualitymodel.DefaultModelFinder; -import org.sonar.jpa.dao.ProfilesDao; -import org.sonar.jpa.dao.RulesDao; - -public class InspectionModule extends Module { - private static final Logger LOG = LoggerFactory.getLogger(InspectionModule.class); - private Project project; - - public InspectionModule(Project project) { - this.project = project; - } - - @Override - protected void configure() { - logSettings(); - addCoreComponents(); - addPluginExtensions(); - } - - private void addCoreComponents() { - ProjectDefinition projectDefinition = container.getComponentByType(ProjectTree.class).getProjectDefinition(project); - container.addSingleton(projectDefinition); - container.addSingleton(project.getConfiguration()); - container.addSingleton(project); - container.addSingleton(ProjectSettings.class); - - // hack to initialize commons-configuration before ExtensionProviders - container.getComponentByType(ProjectSettings.class); - - container.addSingleton(EventBus.class); - container.addSingleton(Phases.class); - container.addSingleton(PhasesTimeProfiler.class); - for (Class clazz : Phases.getPhaseClasses()) { - container.addSingleton(clazz); - } - container.addSingleton(UnsupportedProperties.class); - - for (Object component : projectDefinition.getContainerExtensions()) { - container.addSingleton(component); - } - container.addSingleton(Languages.class); - container.addSingleton(DefaultProjectClasspath.class); - container.addSingleton(DefaultProjectFileSystem2.class); - container.addSingleton(RulesDao.class); - - // the Snapshot component will be removed when asynchronous measures are improved (required for AsynchronousMeasureSensor) - container.addSingleton(container.getComponentByType(ResourcePersister.class).getSnapshot(project)); - - container.addSingleton(TimeMachineConfiguration.class); - container.addSingleton(org.sonar.api.database.daos.MeasuresDao.class); - container.addSingleton(ProfilesDao.class); - container.addSingleton(DefaultSensorContext.class); - container.addSingleton(BatchExtensionDictionnary.class); - container.addSingleton(DefaultTimeMachine.class); - container.addSingleton(ViolationFilters.class); - container.addSingleton(ResourceFilters.class); - container.addSingleton(DefaultModelFinder.class); - container.addSingleton(DefaultProfileLoader.class); - container.addSingleton(DryRunExporter.class); - container.addSingleton(ProjectTestsImpl.class); - container.addPicoAdapter(new ProfileProvider()); - } - - private void addPluginExtensions() { - ExtensionInstaller installer = container.getComponentByType(ExtensionInstaller.class); - installer.installInspectionExtensions(container); - } - - private void logSettings() { - LOG.info("------------- Inspecting {}", project.getName()); - } - - /** - * Analyze project - */ - @Override - protected void doStart() { - DefaultIndex index = container.getComponentByType(DefaultIndex.class); - index.setCurrentProject(project, - container.getComponentByType(ResourceFilters.class), - container.getComponentByType(ViolationFilters.class), - container.getComponentByType(RulesProfile.class)); - - container.getComponentByType(Phases.class).execute(project); - } -} diff --git a/sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionTask.java b/sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionTask.java index bc08ec3f9b4..05ead3d88c8 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionTask.java +++ b/sonar-batch/src/main/java/org/sonar/batch/tasks/InspectionTask.java @@ -19,6 +19,8 @@ */ package org.sonar.batch.tasks; +import org.sonar.batch.bootstrap.InspectionModule; + import org.sonar.api.platform.ComponentContainer; import org.sonar.api.resources.Project; import org.sonar.api.task.RequiresProject; diff --git a/sonar-batch/src/main/java/org/sonar/batch/tasks/Tasks.java b/sonar-batch/src/main/java/org/sonar/batch/tasks/Tasks.java index 9da1ad65271..8e5b8698eb4 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/tasks/Tasks.java +++ b/sonar-batch/src/main/java/org/sonar/batch/tasks/Tasks.java @@ -20,16 +20,30 @@ package org.sonar.batch.tasks; import org.apache.commons.lang.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.api.CoreProperties; import org.sonar.api.config.Settings; +import org.sonar.api.task.Task; +import org.sonar.api.task.TaskComponent; import org.sonar.api.task.TaskDefinition; import org.sonar.api.utils.SonarException; -public class Tasks { +import java.util.HashMap; +import java.util.Map; +import java.util.regex.Pattern; + +public class Tasks implements TaskComponent { + + private static final Logger LOG = LoggerFactory.getLogger(Tasks.class); + private static final String COMMAND_PATTERN = "[a-zA-Z0-9\\-\\_]+"; private final TaskDefinition[] taskDefinitions; private final Settings settings; + private final Map taskDefByCommand = new HashMap(); + private final Map, TaskDefinition> taskDefByTask = new HashMap, TaskDefinition>(); + public Tasks(Settings settings, TaskDefinition[] taskDefinitions) { this.settings = settings; this.taskDefinitions = taskDefinitions; @@ -43,10 +57,8 @@ public class Tasks { } // Default to inspection task finalCommand = StringUtils.isNotBlank(finalCommand) ? finalCommand : InspectionTask.COMMAND; - for (TaskDefinition taskDef : taskDefinitions) { - if (finalCommand.equals(taskDef.getCommand())) { - return taskDef; - } + if (taskDefByCommand.containsKey(finalCommand)) { + return taskDefByCommand.get(finalCommand); } throw new SonarException("No task found for command: " + finalCommand); } @@ -55,4 +67,56 @@ public class Tasks { return taskDefinitions; } + /** + * Perform validation of tasks definitions + */ + public void start() { + for (TaskDefinition def : taskDefinitions) { + validateTask(def); + validateName(def); + validateCommand(def); + validateDescription(def); + } + } + + private void validateName(TaskDefinition def) { + if (StringUtils.isBlank(def.getName())) { + throw new SonarException("Task definition for task '" + def.getTask().getName() + "' doesn't define task name"); + } + + } + + private void validateCommand(TaskDefinition def) { + String command = def.getCommand(); + if (StringUtils.isBlank(command)) { + throw new SonarException("Task definition '" + def.getName() + "' doesn't define task command"); + } + if (!Pattern.matches(COMMAND_PATTERN, command)) { + throw new SonarException("Command '" + command + "' for task definition '" + def.getName() + "' is not valid and should match " + COMMAND_PATTERN); + } + if (taskDefByCommand.containsKey(command)) { + throw new SonarException("Task '" + def.getName() + "' uses the same command than task '" + taskDefByCommand.get(command).getName() + "'"); + } + taskDefByCommand.put(command, def); + } + + private void validateDescription(TaskDefinition def) { + if (StringUtils.isBlank(def.getDescription())) { + LOG.warn("Task definition {} doesn't define a description. Using name as description.", def.getName()); + def.setDescription(def.getName()); + } + } + + private void validateTask(TaskDefinition def) { + Class taskClass = def.getTask(); + if (taskClass == null) { + throw new SonarException("Task definition '" + def.getName() + "' doesn't define the associated task class"); + } + if (taskDefByTask.containsKey(taskClass)) { + throw new SonarException("Task '" + def.getTask().getName() + "' is defined twice: first by '" + taskDefByTask.get(taskClass).getName() + "' and then by '" + def.getName() + + "'"); + } + taskDefByTask.put(taskClass, def); + } + } diff --git a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/InspectionModuleTest.java b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/InspectionModuleTest.java index 1bc61c5f990..6b8d3481580 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/InspectionModuleTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/InspectionModuleTest.java @@ -29,7 +29,6 @@ import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; import org.sonar.batch.ProjectTree; import org.sonar.batch.index.ResourcePersister; -import org.sonar.batch.tasks.InspectionModule; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Matchers.any; diff --git a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/TaskBootstrapModuleTest.java b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/TaskBootstrapModuleTest.java index 568f9ef9b3d..c47f0774a6a 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/TaskBootstrapModuleTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/TaskBootstrapModuleTest.java @@ -50,7 +50,7 @@ public class TaskBootstrapModuleTest { thrown.expect(SonarException.class); thrown.expectMessage("Task Sonar project inspection requires to be run on a project"); - module.doStart(); + module.start(); } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/tasks/TasksTest.java b/sonar-batch/src/test/java/org/sonar/batch/tasks/TasksTest.java index f95fa5c31c3..6c57398ae67 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/tasks/TasksTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/tasks/TasksTest.java @@ -25,6 +25,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.CoreProperties; import org.sonar.api.config.Settings; +import org.sonar.api.task.Task; import org.sonar.api.task.TaskDefinition; import org.sonar.api.utils.SonarException; @@ -51,18 +52,21 @@ public class TasksTest { @Test public void shouldReturnInspectionTask() { Tasks tasks = new Tasks(settings, new TaskDefinition[] {InspectionTask.DEFINITION, ListTasksTask.DEFINITION}); + tasks.start(); assertThat(tasks.getTaskDefinition(InspectionTask.COMMAND)).isEqualTo(InspectionTask.DEFINITION); } @Test public void shouldReturnInspectionTaskByDefault() { Tasks tasks = new Tasks(settings, new TaskDefinition[] {InspectionTask.DEFINITION, ListTasksTask.DEFINITION}); + tasks.start(); assertThat(tasks.getTaskDefinition(null)).isEqualTo(InspectionTask.DEFINITION); } @Test public void shouldReturnUsePropertyWhenNoCommand() { Tasks tasks = new Tasks(settings, new TaskDefinition[] {InspectionTask.DEFINITION, ListTasksTask.DEFINITION}); + tasks.start(); assertThat(tasks.getTaskDefinition(ListTasksTask.COMMAND)).isEqualTo(ListTasksTask.DEFINITION); assertThat(tasks.getTaskDefinition(null)).isEqualTo(InspectionTask.DEFINITION); @@ -80,4 +84,94 @@ public class TasksTest { tasks.getTaskDefinition("not-exists"); } + + @Test + public void shouldThrowWhenCommandMissing() { + Tasks tasks = new Tasks(settings, new TaskDefinition[] {TaskDefinition.create().setName("foo").setTask(FakeTask1.class)}); + + thrown.expect(SonarException.class); + thrown.expectMessage("Task definition 'foo' doesn't define task command"); + + tasks.start(); + } + + @Test + public void shouldThrowWhenCommandInvalid() { + Tasks tasks = new Tasks(settings, new TaskDefinition[] {TaskDefinition.create().setName("foo").setTask(FakeTask1.class).setCommand("Azc-12_bbC")}); + tasks.start(); + + tasks = new Tasks(settings, new TaskDefinition[] {TaskDefinition.create().setName("foo").setTask(FakeTask1.class).setCommand("with space")}); + + thrown.expect(SonarException.class); + thrown.expectMessage("Command 'with space' for task definition 'foo' is not valid and should match [a-zA-Z0-9\\-\\_]+"); + + tasks.start(); + } + + @Test + public void shouldThrowWhenDuplicateCommand() { + Tasks tasks = new Tasks(settings, new TaskDefinition[] { + TaskDefinition.create().setName("foo1").setTask(FakeTask1.class).setCommand("cmd"), + TaskDefinition.create().setName("foo2").setTask(FakeTask2.class).setCommand("cmd")}); + + thrown.expect(SonarException.class); + thrown.expectMessage("Task 'foo2' uses the same command than task 'foo1'"); + + tasks.start(); + } + + @Test + public void shouldThrowWhenNameMissing() { + Tasks tasks = new Tasks(settings, new TaskDefinition[] {TaskDefinition.create().setCommand("foo").setTask(FakeTask1.class)}); + + thrown.expect(SonarException.class); + thrown.expectMessage("Task definition for task 'org.sonar.batch.tasks.TasksTest$FakeTask1' doesn't define task name"); + + tasks.start(); + } + + @Test + public void shouldThrowWhenTaskMissing() { + Tasks tasks = new Tasks(settings, new TaskDefinition[] {TaskDefinition.create().setCommand("foo").setName("bar")}); + + thrown.expect(SonarException.class); + thrown.expectMessage("Task definition 'bar' doesn't define the associated task class"); + + tasks.start(); + } + + @Test + public void shouldThrowWhenDuplicateTask() { + Tasks tasks = new Tasks(settings, new TaskDefinition[] { + TaskDefinition.create().setName("foo1").setTask(FakeTask1.class).setCommand("cmd1"), + TaskDefinition.create().setName("foo2").setTask(FakeTask1.class).setCommand("cmd2")}); + + thrown.expect(SonarException.class); + thrown.expectMessage("Task 'org.sonar.batch.tasks.TasksTest$FakeTask1' is defined twice: first by 'foo1' and then by 'foo2'"); + + tasks.start(); + } + + @Test + public void shouldUseNameWhenDescriptionIsMissing() { + Tasks tasks = new Tasks(settings, new TaskDefinition[] {TaskDefinition.create().setName("foo").setCommand("cmd").setTask(FakeTask1.class)}); + tasks.start(); + + assertThat(tasks.getTaskDefinition("cmd").getDescription()).isEqualTo("foo"); + } + + private static class FakeTask1 implements Task { + + public void execute() { + } + + } + + private static class FakeTask2 implements Task { + + public void execute() { + } + + } + } -- cgit v1.2.3