Fix some quality flaws

7 files changed, 202 insertions, 24 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultPersistenceManager.java b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultPersistenceManager.java
index 39b9d8d472a..6b42c7184bf 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultPersistenceManager.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultPersistenceManager.java
@@ -27,6 +27,8 @@ import org.sonar.api.resources.ProjectLink;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.resources.ResourceUtils;
 
+import javax.annotation.Nullable;
+
 import java.util.List;
 
 public final class DefaultPersistenceManager implements PersistenceManager {
@@ -51,11 +53,11 @@ public final class DefaultPersistenceManager implements PersistenceManager {
     sourcePersister.clear();
   }
 
-  public void saveProject(Project project, Project parent) {
+  public void saveProject(Project project, @Nullable Project parent) {
     resourcePersister.saveProject(project, parent);
   }
 
-  public Snapshot saveResource(Project project, Resource resource, Resource parent) {
+  public Snapshot saveResource(Project project, Resource resource, @Nullable Resource parent) {
     if (ResourceUtils.isPersistable(resource)) {
       return resourcePersister.saveResource(project, resource, parent);
     }
diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultResourcePersister.java b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultResourcePersister.java
index 4e9082bc458..689b6a0e4cd 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultResourcePersister.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultResourcePersister.java
@@ -36,6 +36,7 @@ import org.sonar.api.resources.Scopes;
 import org.sonar.api.security.ResourcePermissions;
 import org.sonar.api.utils.SonarException;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import javax.persistence.NonUniqueResultException;
 import javax.persistence.Query;
@@ -126,7 +127,8 @@ public final class DefaultResourcePersister implements ResourcePersister {
     return snapshot;
   }
 
-  public Snapshot getSnapshot(Resource reference) {
+  @CheckForNull
+  public Snapshot getSnapshot(@Nullable Resource reference) {
     return snapshotsByResource.get(reference);
   }
 
@@ -241,6 +243,7 @@ public final class DefaultResourcePersister implements ResourcePersister {
     return snapshot;
   }
 
+  @CheckForNull
   public Snapshot getLastSnapshot(Snapshot snapshot, boolean onlyOlder) {
     String hql = "SELECT s FROM " + Snapshot.class.getSimpleName() + " s WHERE s.last=:last AND s.resourceId=:resourceId";
     if (onlyOlder) {
diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/PersistenceManager.java b/sonar-batch/src/main/java/org/sonar/batch/index/PersistenceManager.java
index b97baa1b8ea..a4dfcb74ffd 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/index/PersistenceManager.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/PersistenceManager.java
@@ -35,7 +35,7 @@ public interface PersistenceManager {
 
   void saveProject(Project project, @Nullable Project parent);
 
-  Snapshot saveResource(Project project, Resource resource, Resource parent);
+  Snapshot saveResource(Project project, Resource resource, @Nullable Resource parent);
 
   void setSource(Resource file, String source);
 
diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java b/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java
index af78d773c8d..d5a535dc27a 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java
@@ -24,6 +24,7 @@ import org.sonar.api.database.model.Snapshot;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
 public interface ResourcePersister {
@@ -33,13 +34,14 @@ public interface ResourcePersister {
   /**
    * Persist a resource in database. Returns null if the resource must not be persisted (scope lower than file)
    */
-  Snapshot saveResource(Project project, Resource resource, Resource parent);
+  Snapshot saveResource(Project project, Resource resource, @Nullable Resource parent);
 
   /**
    * Persist a resource in database. Returns null if the resource must not be persisted (scope lower than file)
    */
   Snapshot saveResource(Project project, Resource resource);
 
+  @CheckForNull
   Snapshot getSnapshot(Resource resource);
 
   Snapshot getSnapshotOrFail(Resource resource);
@@ -50,6 +52,7 @@ public interface ResourcePersister {
    * The current snapshot which is flagged as "last", different than the current analysis.
    * @param onlyOlder true if the result must be anterior to the snapshot parameter
    */
+  @CheckForNull
   Snapshot getLastSnapshot(Snapshot snapshot, boolean onlyOlder);
 
   void clear();
diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/AnalyzerContextAdaptor.java b/sonar-batch/src/main/java/org/sonar/batch/scan/AnalyzerContextAdaptor.java
index eb76d3a269d..4cddc459f4c 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/scan/AnalyzerContextAdaptor.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/scan/AnalyzerContextAdaptor.java
@@ -91,17 +91,14 @@ public class AnalyzerContextAdaptor implements AnalyzerContext {
   }
 
   @Override
-  public AnalyzerMeasure<?> getMeasure(String metricKey) {
-    Metric<?> m = metricFinder.findByKey(metricKey);
-    if (m == null) {
-      throw new IllegalStateException("Unknow metric with key: " + metricKey);
-    }
+  public AnalyzerMeasure getMeasure(String metricKey) {
+    Metric<?> m = findMetricOrFail(metricKey);
     return getMeasure(m);
   }
 
   @Override
   public <G extends Serializable> AnalyzerMeasure<G> getMeasure(Metric<G> metric) {
-    org.sonar.api.measures.Metric<G> m = metricFinder.findByKey(metric.key());
+    org.sonar.api.measures.Metric<G> m = (org.sonar.api.measures.Metric<G>) findMetricOrFail(metric.key());
     Measure<G> measure = sensorContext.getMeasure(m);
     if (measure == null) {
       return null;
@@ -114,18 +111,23 @@ public class AnalyzerContextAdaptor implements AnalyzerContext {
   }
 
   @Override
-  public AnalyzerMeasure<?> getMeasure(InputFile file, String metricKey) {
+  public AnalyzerMeasure getMeasure(InputFile file, String metricKey) {
+    Metric<?> m = findMetricOrFail(metricKey);
+    return getMeasure(file, m);
+  }
+
+  private Metric<?> findMetricOrFail(String metricKey) {
     Metric<?> m = metricFinder.findByKey(metricKey);
     if (m == null) {
       throw new IllegalStateException("Unknow metric with key: " + metricKey);
     }
-    return getMeasure(file, m);
+    return m;
   }
 
   @Override
   public <G extends Serializable> AnalyzerMeasure<G> getMeasure(InputFile file, Metric<G> metric) {
-    File fileRes = File.fromIOFile(file.file(), project);
-    org.sonar.api.measures.Metric<G> m = metricFinder.findByKey(metric.key());
+    File fileRes = File.create(file.relativePath());
+    org.sonar.api.measures.Metric<G> m = (org.sonar.api.measures.Metric<G>) findMetricOrFail(metric.key());
     Measure<G> measure = sensorContext.getMeasure(fileRes, m);
     if (measure == null) {
       return null;
@@ -174,8 +176,7 @@ public class AnalyzerContextAdaptor implements AnalyzerContext {
         }
     }
     if (measure.inputFile() != null) {
-      File fileRes = File.fromIOFile(measure.inputFile().file(), project);
-      sensorContext.saveMeasure(fileRes, measureToSave);
+      sensorContext.saveMeasure(measure.inputFile(), measureToSave);
     } else {
       sensorContext.saveMeasure(measureToSave);
     }
@@ -187,15 +188,16 @@ public class AnalyzerContextAdaptor implements AnalyzerContext {
   }
 
   @Override
-  public void addIssue(AnalyzerIssue issue) {
+  public boolean addIssue(AnalyzerIssue issue) {
     Resource r;
-    if (issue.inputFile() != null) {
-      r = File.fromIOFile(issue.inputFile().file(), project);
+    InputFile inputFile = issue.inputFile();
+    if (inputFile != null) {
+      r = File.create(inputFile.relativePath());
     } else {
       r = project;
     }
     Issuable issuable = perspectives.as(Issuable.class, r);
-    issuable.addIssue(toDefaultIssue(project.getKey(), r.getKey(), issue));
+    return issuable.addIssue(toDefaultIssue(project.getKey(), r.getKey(), issue));
   }
 
   public static DefaultIssue toDefaultIssue(String projectKey, String componentKey, AnalyzerIssue issue) {
diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan2/DefaultAnalyzerContext.java b/sonar-batch/src/main/java/org/sonar/batch/scan2/DefaultAnalyzerContext.java
index 8e16ccd1f97..19188321bf0 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/scan2/DefaultAnalyzerContext.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/scan2/DefaultAnalyzerContext.java
@@ -33,6 +33,7 @@ import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.measure.Metric;
 import org.sonar.api.batch.rule.ActiveRules;
+import org.sonar.api.batch.rule.Rules;
 import org.sonar.api.config.Settings;
 import org.sonar.batch.issue.IssueFilters;
 import org.sonar.batch.scan.AnalyzerContextAdaptor;
@@ -49,9 +50,10 @@ public class DefaultAnalyzerContext implements AnalyzerContext {
   private final FileSystem fs;
   private final ActiveRules activeRules;
   private final IssueFilters issueFilters;
+  private Rules rules;
 
   public DefaultAnalyzerContext(ProjectDefinition def, AnalyzerMeasureCache measureCache, AnalyzerIssueCache issueCache,
-    Settings settings, FileSystem fs, ActiveRules activeRules, IssueFilters issueFilters) {
+    Settings settings, FileSystem fs, ActiveRules activeRules, IssueFilters issueFilters, Rules rules) {
     this.def = def;
     this.measureCache = measureCache;
     this.issueCache = issueCache;
@@ -59,6 +61,7 @@ public class DefaultAnalyzerContext implements AnalyzerContext {
     this.fs = fs;
     this.activeRules = activeRules;
     this.issueFilters = issueFilters;
+    this.rules = rules;
   }
 
   @Override
@@ -116,16 +119,19 @@ public class DefaultAnalyzerContext implements AnalyzerContext {
   }
 
   @Override
-  public void addIssue(AnalyzerIssue issue) {
+  public boolean addIssue(AnalyzerIssue issue) {
     String resourceKey;
     if (issue.inputFile() != null) {
       resourceKey = ComponentKeys.createEffectiveKey(def.getKey(), issue.inputFile());
     } else {
       resourceKey = def.getKey();
     }
-
+    // TODO Lot of things to do. See ModuleIssues::initAndAddIssue
     if (issueFilters.accept(AnalyzerContextAdaptor.toDefaultIssue(def.getKey(), resourceKey, issue), null)) {
       issueCache.put(def.getKey(), resourceKey, (DefaultAnalyzerIssue) issue);
+      return true;
     }
+
+    return false;
   }
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/AnalyzerContextAdapterTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/AnalyzerContextAdapterTest.java
new file mode 100644
index 00000000000..f20f9304baa
--- /dev/null
+++ b/sonar-batch/src/test/java/org/sonar/batch/scan/AnalyzerContextAdapterTest.java
@@ -0,0 +1,162 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 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.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.ArgumentCaptor;
+import org.sonar.api.batch.SensorContext;
+import org.sonar.api.batch.analyzer.issue.internal.DefaultAnalyzerIssueBuilder;
+import org.sonar.api.batch.analyzer.measure.AnalyzerMeasure;
+import org.sonar.api.batch.analyzer.measure.internal.DefaultAnalyzerMeasureBuilder;
+import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.batch.fs.internal.DefaultFileSystem;
+import org.sonar.api.batch.fs.internal.DefaultInputFile;
+import org.sonar.api.batch.rule.ActiveRules;
+import org.sonar.api.batch.rule.internal.ActiveRulesBuilder;
+import org.sonar.api.component.ResourcePerspectives;
+import org.sonar.api.config.Settings;
+import org.sonar.api.issue.Issuable;
+import org.sonar.api.issue.Issue;
+import org.sonar.api.measures.CoreMetrics;
+import org.sonar.api.measures.Measure;
+import org.sonar.api.measures.MetricFinder;
+import org.sonar.api.resources.File;
+import org.sonar.api.resources.Project;
+import org.sonar.api.rule.RuleKey;
+
+import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class AnalyzerContextAdapterTest {
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
+  private ActiveRules activeRules;
+  private DefaultFileSystem fs;
+  private AnalyzerContextAdaptor adaptor;
+  private SensorContext sensorContext;
+  private Settings settings;
+  private ResourcePerspectives resourcePerspectives;
+
+  @Before
+  public void prepare() {
+    activeRules = new ActiveRulesBuilder().build();
+    fs = new DefaultFileSystem();
+    MetricFinder metricFinder = mock(MetricFinder.class);
+    when(metricFinder.findByKey(CoreMetrics.NCLOC_KEY)).thenReturn(CoreMetrics.NCLOC);
+    sensorContext = mock(SensorContext.class);
+    settings = new Settings();
+    resourcePerspectives = mock(ResourcePerspectives.class);
+    adaptor = new AnalyzerContextAdaptor(sensorContext, metricFinder, new Project("myProject"),
+      resourcePerspectives, settings, fs, activeRules);
+  }
+
+  @Test
+  public void shouldProvideComponents() {
+    assertThat(adaptor.activeRules()).isEqualTo(activeRules);
+    assertThat(adaptor.fileSystem()).isEqualTo(fs);
+    assertThat(adaptor.settings()).isEqualTo(settings);
+
+    assertThat(adaptor.issueBuilder()).isNotNull();
+    assertThat(adaptor.measureBuilder()).isNotNull();
+  }
+
+  @Test
+  public void shouldRedirectProjectMeasuresToSensorContext() {
+    AnalyzerMeasure<Integer> measure = adaptor.getMeasure(CoreMetrics.NCLOC_KEY);
+    assertThat(measure).isNull();
+
+    when(sensorContext.getMeasure(CoreMetrics.NCLOC)).thenReturn(new Measure<Integer>(CoreMetrics.NCLOC, 10.0));
+
+    measure = adaptor.getMeasure(CoreMetrics.NCLOC);
+    assertThat(measure.metric()).isEqualTo(CoreMetrics.NCLOC);
+    assertThat(measure.inputFile()).isNull();
+    assertThat(measure.value()).isEqualTo(10);
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Unknow metric with key: lines");
+    adaptor.getMeasure(CoreMetrics.LINES);
+  }
+
+  @Test
+  public void shouldRedirectFileMeasuresToSensorContext() {
+    InputFile file = new DefaultInputFile("src/Foo.php");
+
+    AnalyzerMeasure<Integer> measure = adaptor.getMeasure(file, CoreMetrics.NCLOC_KEY);
+    assertThat(measure).isNull();
+
+    when(sensorContext.getMeasure(File.create("src/Foo.php"), CoreMetrics.NCLOC)).thenReturn(new Measure<Integer>(CoreMetrics.NCLOC, 10.0));
+    measure = adaptor.getMeasure(file, CoreMetrics.NCLOC);
+
+    assertThat(measure.metric()).isEqualTo(CoreMetrics.NCLOC);
+    assertThat(measure.inputFile()).isEqualTo(file);
+    assertThat(measure.value()).isEqualTo(10);
+  }
+
+  @Test
+  public void shouldAddMeasureToSensorContext() {
+    InputFile file = new DefaultInputFile("src/Foo.php");
+
+    ArgumentCaptor<Measure> argumentCaptor = ArgumentCaptor.forClass(Measure.class);
+    when(sensorContext.saveMeasure(eq(file), argumentCaptor.capture())).thenReturn(null);
+
+    adaptor.addMeasure(new DefaultAnalyzerMeasureBuilder()
+      .onFile(file)
+      .forMetric(CoreMetrics.NCLOC)
+      .withValue(10)
+      .build());
+
+    Measure m = argumentCaptor.getValue();
+    assertThat(m.getValue()).isEqualTo(10.0);
+    assertThat(m.getMetric()).isEqualTo(CoreMetrics.NCLOC);
+  }
+
+  @Test
+  public void shouldAddIssue() {
+    InputFile file = new DefaultInputFile("src/Foo.php");
+
+    ArgumentCaptor<Issue> argumentCaptor = ArgumentCaptor.forClass(Issue.class);
+
+    Issuable issuable = mock(Issuable.class);
+    when(resourcePerspectives.as(Issuable.class, File.create("src/Foo.php"))).thenReturn(issuable);
+
+    when(issuable.addIssue(argumentCaptor.capture())).thenReturn(true);
+
+    adaptor.addIssue(new DefaultAnalyzerIssueBuilder()
+      .onFile(file)
+      .ruleKey(RuleKey.of("foo", "bar"))
+      .message("Foo")
+      .atLine(3)
+      .effortToFix(10.0)
+      .build());
+
+    Issue issue = argumentCaptor.getValue();
+    assertThat(issue.ruleKey()).isEqualTo(RuleKey.of("foo", "bar"));
+    assertThat(issue.message()).isEqualTo("Foo");
+    assertThat(issue.line()).isEqualTo(3);
+    assertThat(issue.effortToFix()).isEqualTo(10.0);
+  }
+}