SONAR-4221 Fix regression with property loading and module exclusions

8 files changed, 126 insertions, 104 deletions

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
+ * </ul>
+ */
+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<String, String> 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.<Project> 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);