From 9e5fe807e20689fef2d8936c6aa2667b1732461a Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Mon, 10 Jun 2013 18:37:09 +0200 Subject: [PATCH] SONAR-4221 Fix regression with property loading and module exclusions --- .../sonar/batch/bootstrap/BatchSettings.java | 12 +- .../sonar/batch/scan/ProjectExclusions.java | 9 +- .../batch/scan/ProjectScanContainer.java | 105 ++++++++++-------- .../batch/scan/ProjectSettingsReady.java | 42 +++++++ .../batch/bootstrap/BatchSettingsTest.java | 9 +- .../batch/scan/ProjectExclusionsTest.java | 16 +-- .../batch/scan/ProjectScanContainerTest.java | 35 ------ .../org/sonar/batch/scan/ScanTaskTest.java | 2 +- 8 files changed, 126 insertions(+), 104 deletions(-) create mode 100644 sonar-batch/src/main/java/org/sonar/batch/scan/ProjectSettingsReady.java diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java index 2bed616e8f6..a50b3d40eb0 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java +++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java @@ -26,6 +26,7 @@ import org.json.simple.JSONValue; import org.slf4j.LoggerFactory; import org.sonar.api.CoreProperties; import org.sonar.api.batch.bootstrap.ProjectDefinition; +import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.config.PropertyDefinitions; import org.sonar.api.config.Settings; @@ -54,13 +55,13 @@ public class BatchSettings extends Settings { init(null); } - public void init(@Nullable ProjectDefinition rootProject) { + public void init(@Nullable ProjectReactor reactor) { savedProperties = this.getProperties(); - if (rootProject != null) { + if (reactor != null) { LoggerFactory.getLogger(BatchSettings.class).info("Load project settings"); String branch = bootstrapSettings.property(CoreProperties.PROJECT_BRANCH_PROPERTY); - String projectKey = rootProject.getKey(); + String projectKey = reactor.getRoot().getKey(); if (StringUtils.isNotBlank(branch)) { projectKey = String.format("%s:%s", projectKey, branch); } @@ -71,9 +72,8 @@ public class BatchSettings extends Settings { } addProperties(bootstrapSettings.properties()); - // Reload reactor properties in case reactor has changed since bootstrap - if (rootProject != null) { - addProperties(rootProject.getProperties()); + if (reactor != null) { + addProperties(reactor.getRoot().getProperties()); } properties.putAll(System.getenv()); addProperties(System.getProperties()); diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectExclusions.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectExclusions.java index 468d2203a63..a408ec8d400 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectExclusions.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectExclusions.java @@ -19,8 +19,6 @@ */ package org.sonar.batch.scan; -import org.sonar.api.task.TaskComponent; - import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; @@ -30,6 +28,7 @@ import org.sonar.api.batch.bootstrap.ProjectBuilder; import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.config.Settings; +import org.sonar.api.task.TaskComponent; import javax.annotation.Nullable; @@ -46,14 +45,16 @@ public class ProjectExclusions implements TaskComponent { private ProjectReactor reactor; public ProjectExclusions(Settings settings, ProjectReactor reactor, + // exclusions are applied when settings are loaded from Sonar DB + ProjectSettingsReady settingsReady, // exclusions are applied when the project is completely defined by extensions @Nullable ProjectBuilder[] projectBuilders) { this.settings = settings; this.reactor = reactor; } - public ProjectExclusions(Settings settings, ProjectReactor reactor) { - this(settings, reactor, new ProjectBuilder[0]); + public ProjectExclusions(Settings settings, ProjectReactor reactor, ProjectSettingsReady settingsReady) { + this(settings, reactor, settingsReady, new ProjectBuilder[0]); } public void start() { diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java index 857ffb174ab..aaa1413b901 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java @@ -30,8 +30,25 @@ import org.sonar.batch.DefaultFileLinesContextFactory; import org.sonar.batch.DefaultResourceCreationLock; import org.sonar.batch.ProjectConfigurator; import org.sonar.batch.ProjectTree; -import org.sonar.batch.bootstrap.*; -import org.sonar.batch.index.*; +import org.sonar.batch.bootstrap.BatchSettings; +import org.sonar.batch.bootstrap.ExtensionInstaller; +import org.sonar.batch.bootstrap.ExtensionMatcher; +import org.sonar.batch.bootstrap.ExtensionUtils; +import org.sonar.batch.bootstrap.MetricProvider; +import org.sonar.batch.index.Caches; +import org.sonar.batch.index.ComponentDataCache; +import org.sonar.batch.index.ComponentDataPersister; +import org.sonar.batch.index.DefaultIndex; +import org.sonar.batch.index.DefaultPersistenceManager; +import org.sonar.batch.index.DefaultResourcePersister; +import org.sonar.batch.index.DependencyPersister; +import org.sonar.batch.index.EventPersister; +import org.sonar.batch.index.LinkPersister; +import org.sonar.batch.index.MeasurePersister; +import org.sonar.batch.index.MemoryOptimizer; +import org.sonar.batch.index.ResourceCache; +import org.sonar.batch.index.SnapshotCache; +import org.sonar.batch.index.SourcePersister; import org.sonar.batch.issue.DeprecatedViolations; import org.sonar.batch.issue.IssueCache; import org.sonar.batch.issue.IssuePersister; @@ -71,49 +88,51 @@ public class ProjectScanContainer extends ComponentContainer { private void addBatchComponents() { add( - DefaultResourceCreationLock.class, - DefaultPersistenceManager.class, - DependencyPersister.class, - EventPersister.class, - LinkPersister.class, - MeasurePersister.class, - MemoryOptimizer.class, - DefaultResourcePersister.class, - SourcePersister.class, - DefaultNotificationManager.class, - MetricProvider.class, - ProjectConfigurator.class, - DefaultIndex.class, - DefaultFileLinesContextFactory.class, - ProjectLock.class, - LastSnapshots.class, - Caches.class, - SnapshotCache.class, - ResourceCache.class, - ComponentDataCache.class, - ComponentDataPersister.class, + DefaultResourceCreationLock.class, + DefaultPersistenceManager.class, + DependencyPersister.class, + EventPersister.class, + LinkPersister.class, + MeasurePersister.class, + MemoryOptimizer.class, + DefaultResourcePersister.class, + SourcePersister.class, + DefaultNotificationManager.class, + MetricProvider.class, + ProjectConfigurator.class, + DefaultIndex.class, + DefaultFileLinesContextFactory.class, + ProjectLock.class, + LastSnapshots.class, + Caches.class, + SnapshotCache.class, + ResourceCache.class, + ComponentDataCache.class, + ComponentDataPersister.class, - // issues - IssueUpdater.class, - FunctionExecutor.class, - IssueWorkflow.class, - DeprecatedViolations.class, - IssueCache.class, - ScanIssueStorage.class, - IssuePersister.class, - IssueNotifications.class, + // issues + IssueUpdater.class, + FunctionExecutor.class, + IssueWorkflow.class, + DeprecatedViolations.class, + IssueCache.class, + ScanIssueStorage.class, + IssuePersister.class, + IssueNotifications.class, - // tests - TestPlanPerspectiveLoader.class, - TestablePerspectiveLoader.class, - TestPlanBuilder.class, - TestableBuilder.class, - ScanGraph.create(), - GraphPersister.class, + // tests + TestPlanPerspectiveLoader.class, + TestablePerspectiveLoader.class, + TestPlanBuilder.class, + TestableBuilder.class, + ScanGraph.create(), + GraphPersister.class, - // lang - HighlightableBuilder.class, - SymbolizableBuilder.class); + // lang + HighlightableBuilder.class, + SymbolizableBuilder.class, + + ProjectSettingsReady.class); } private void fixMavenExecutor() { @@ -129,8 +148,6 @@ public class ProjectScanContainer extends ComponentContainer { @Override protected void doAfterStart() { ProjectTree tree = getComponentByType(ProjectTree.class); - BatchSettings settings = getComponentByType(BatchSettings.class); - settings.init(tree.getProjectDefinition(tree.getRootProject())); scanRecursively(tree.getRootProject()); } diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectSettingsReady.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectSettingsReady.java new file mode 100644 index 00000000000..4a267d55196 --- /dev/null +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectSettingsReady.java @@ -0,0 +1,42 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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 this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.batch.scan; + +import org.sonar.api.batch.bootstrap.ProjectReactor; +import org.sonar.batch.bootstrap.BatchSettings; + +/** + * Barrier to control the project settings are loaded from Sonar DB before applying project exclusions + * + */ +public class ProjectSettingsReady { + + private final ProjectReactor reactor; + private final BatchSettings settings; + + public ProjectSettingsReady(ProjectReactor reactor, BatchSettings settings) { + this.reactor = reactor; + this.settings = settings; + } + + public void start() { + settings.init(reactor); + } +} diff --git a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java index 7ac5b3ba19b..bfcf3d12c6a 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java @@ -23,6 +23,7 @@ import org.apache.commons.configuration.BaseConfiguration; import org.apache.commons.configuration.Configuration; import org.junit.Test; import org.sonar.api.batch.bootstrap.ProjectDefinition; +import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.config.PropertyDefinitions; import java.util.Collections; @@ -64,7 +65,7 @@ public class BatchSettingsTest { project.setProperty("project.prop", "project"); BatchSettings batchSettings = new BatchSettings(bootstrapSettings, new PropertyDefinitions(), client, deprecatedConf); - batchSettings.init(project); + batchSettings.init(new ProjectReactor(project)); assertThat(batchSettings.getString("project.prop")).isEqualTo("project"); } @@ -84,7 +85,7 @@ public class BatchSettingsTest { when(client.request("/batch_bootstrap/properties?project=struts")).thenReturn(REACTOR_JSON_RESPONSE); BatchSettings batchSettings = new BatchSettings(bootstrapSettings, new PropertyDefinitions(), client, deprecatedConf); - batchSettings.init(project); + batchSettings.init(new ProjectReactor(project)); assertThat(batchSettings.getString("sonar.java.coveragePlugin")).isEqualTo("jacoco"); } @@ -95,7 +96,7 @@ public class BatchSettingsTest { when(client.request("/batch_bootstrap/properties?project=struts")).thenReturn(REACTOR_JSON_RESPONSE); BatchSettings batchSettings = new BatchSettings(bootstrapSettings, new PropertyDefinitions(), client, deprecatedConf); - batchSettings.init(project); + batchSettings.init(new ProjectReactor(project)); Map moduleSettings = batchSettings.getModuleProperties("struts-core"); @@ -120,7 +121,7 @@ public class BatchSettingsTest { when(client.request("/batch_bootstrap/properties?project=struts")).thenReturn(REACTOR_JSON_RESPONSE); BatchSettings batchSettings = new BatchSettings(bootstrapSettings, new PropertyDefinitions(), client, deprecatedConf); - batchSettings.init(project); + batchSettings.init(new ProjectReactor(project)); assertThat(deprecatedConf.getString("sonar.cpd.cross")).isEqualTo("true"); assertThat(deprecatedConf.getString("sonar.java.coveragePlugin")).isEqualTo("jacoco"); diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectExclusionsTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectExclusionsTest.java index f64a603aa18..9b709cf7bc1 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectExclusionsTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectExclusionsTest.java @@ -23,11 +23,9 @@ import org.junit.Test; import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.config.Settings; -import org.sonar.batch.scan.ProjectExclusions; import static org.fest.assertions.Assertions.assertThat; - public class ProjectExclusionsTest { ProjectReactor newReactor(String rootKey, String... moduleKeys) { @@ -46,7 +44,7 @@ public class ProjectExclusionsTest { ProjectReactor reactor = newReactor("root", "sub1", "sub2"); - ProjectExclusions exclusions = new ProjectExclusions(settings, reactor); + ProjectExclusions exclusions = new ProjectExclusions(settings, reactor, null); exclusions.start(); assertThat(reactor.getProject("root")).isNotNull(); @@ -58,7 +56,7 @@ public class ProjectExclusionsTest { public void testNoSkippedModules() { Settings settings = new Settings(); ProjectReactor reactor = newReactor("root", "sub1", "sub2"); - ProjectExclusions exclusions = new ProjectExclusions(settings, reactor); + ProjectExclusions exclusions = new ProjectExclusions(settings, reactor, null); exclusions.start(); assertThat(reactor.getProject("root")).isNotNull(); @@ -71,7 +69,7 @@ public class ProjectExclusionsTest { Settings settings = new Settings(); settings.setProperty("sonar.includedModules", "sub1"); ProjectReactor reactor = newReactor("root", "sub1", "sub2"); - ProjectExclusions exclusions = new ProjectExclusions(settings, reactor); + ProjectExclusions exclusions = new ProjectExclusions(settings, reactor, null); exclusions.start(); assertThat(reactor.getProject("root")).isNotNull(); @@ -89,10 +87,9 @@ public class ProjectExclusionsTest { settings.setProperty("sonar.skippedModules", "sub1"); ProjectReactor reactor = new ProjectReactor(root); - ProjectExclusions exclusions = new ProjectExclusions(settings, reactor); + ProjectExclusions exclusions = new ProjectExclusions(settings, reactor, null); exclusions.start(); - assertThat(reactor.getProject("root")).isNotNull(); assertThat(reactor.getProject("sub1")).isNull(); assertThat(reactor.getProject("sub11")).isNull(); @@ -104,7 +101,7 @@ public class ProjectExclusionsTest { settings.setProperty("sonar.skippedModules", "sub1,root"); ProjectReactor reactor = newReactor("root", "sub1", "sub2"); - ProjectExclusions exclusions = new ProjectExclusions(settings, reactor); + ProjectExclusions exclusions = new ProjectExclusions(settings, reactor, null); exclusions.start(); } @@ -115,10 +112,9 @@ public class ProjectExclusionsTest { Settings settings = new Settings(); settings.setProperty("sonar.skippedModules", "struts-taglib"); - ProjectExclusions exclusions = new ProjectExclusions(settings, reactor); + ProjectExclusions exclusions = new ProjectExclusions(settings, reactor, null); exclusions.start(); - assertThat(reactor.getProject("org.apache.struts:struts")).isNotNull(); assertThat(reactor.getProject("org.apache.struts:struts-core")).isNotNull(); assertThat(reactor.getProject("org.apache.struts:struts-taglib")).isNull(); diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectScanContainerTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectScanContainerTest.java index 1c5964e4e61..54e3e12ee9a 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectScanContainerTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectScanContainerTest.java @@ -19,30 +19,20 @@ */ package org.sonar.batch.scan; -import com.google.common.collect.Lists; import org.junit.Test; import org.sonar.api.BatchExtension; import org.sonar.api.CoreProperties; import org.sonar.api.ServerExtension; import org.sonar.api.batch.InstantiationStrategy; -import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.config.Settings; import org.sonar.api.platform.ComponentContainer; -import org.sonar.api.resources.Project; import org.sonar.api.task.TaskExtension; -import org.sonar.batch.ProjectTree; -import org.sonar.batch.bootstrap.BatchSettings; import org.sonar.batch.bootstrap.ExtensionInstaller; import org.sonar.batch.profiling.PhasesSumUpTimeProfiler; import org.sonar.batch.scan.maven.MavenPluginExecutor; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; public class ProjectScanContainerTest { @@ -102,31 +92,6 @@ public class ProjectScanContainerTest { assertThat(filter.accept(MyTaskExtension.class)).isFalse(); } - @Test - public void should_isolate_project_settings() { - ComponentContainer parentContainer = new ComponentContainer(); - BatchSettings settings = mock(BatchSettings.class); - parentContainer.add(settings); - ProjectTree projectTree = mock(ProjectTree.class); - Project project = mock(Project.class); - when(project.getModules()).thenReturn(Lists. newArrayList()); - when(projectTree.getRootProject()).thenReturn(project); - ProjectDefinition definition = mock(ProjectDefinition.class); - when(projectTree.getProjectDefinition(project)).thenReturn(definition); - parentContainer.add(projectTree); - ProjectScanContainer container = new ProjectScanContainer(parentContainer); - container.add(mock(ExtensionInstaller.class)); - container.doBeforeStart(); - - container = spy(container); - doNothing().when(container).scan(any(Project.class)); - container.doAfterStart(); - verify(settings).init(definition); - - container.stop(); - verify(settings).restore(); - } - @InstantiationStrategy(InstantiationStrategy.PER_BATCH) static class MyBatchExtension implements BatchExtension { diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/ScanTaskTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/ScanTaskTest.java index 0a17bdb5148..8be63457c64 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/ScanTaskTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/ScanTaskTest.java @@ -42,7 +42,7 @@ public class ScanTaskTest { public void should_enable_all_phases() { ScanTask task = new ScanTask(mock(TaskContainer.class)); ComponentContainer projectScanContainer = new ComponentContainer(); - projectScanContainer.add(mock(ProjectConfigurator.class), mock(ProjectReactor.class), mock(Settings.class)); + projectScanContainer.add(mock(ProjectConfigurator.class), mock(ProjectReactor.class), mock(Settings.class), mock(ProjectSettingsReady.class)); task.scan(projectScanContainer); Phases phases = projectScanContainer.getComponentByType(Phases.class); -- 2.39.5