SONAR-6887 Make it clear in SCM API that date and revision are mandatory on line blame

2 files changed, 56 insertions, 17 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java b/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java
index 7ed72faabad..cf7c526d5c3 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java
@@ -21,7 +21,6 @@ package org.sonar.batch.scm;
 
 import com.google.common.base.Preconditions;
 import java.text.Normalizer;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -84,17 +83,14 @@ class DefaultBlameOutput implements BlameOutput {
     Map<String, Integer> changesetsIdByRevision = new HashMap<>();
 
     for (BlameLine line : lines) {
-      if (StringUtils.isNotBlank(line.revision())) {
-        Integer changesetId = changesetsIdByRevision.get(line.revision());
-        if (changesetId == null) {
-          addChangeset(scmBuilder, line);
-          changesetId = scmBuilder.getChangesetCount() - 1;
-          changesetsIdByRevision.put(line.revision(), changesetId);
-        }
-        scmBuilder.addChangesetIndexByLine(changesetId);
-      } else {
+      validateLine(line);
+      Integer changesetId = changesetsIdByRevision.get(line.revision());
+      if (changesetId == null) {
         addChangeset(scmBuilder, line);
+        changesetId = scmBuilder.getChangesetCount() - 1;
+        changesetsIdByRevision.put(line.revision(), changesetId);
       }
+      scmBuilder.addChangesetIndexByLine(changesetId);
     }
     writer.writeComponentChangesets(scmBuilder.build());
     allFilesToBlame.remove(file);
@@ -102,18 +98,19 @@ class DefaultBlameOutput implements BlameOutput {
     progressReport.message(count + "/" + total + " files analyzed, last one was " + file.absolutePath());
   }
 
+  private static void validateLine(BlameLine line) {
+    Preconditions.checkNotNull(line.revision(), "Blame revision cannot be null");
+    Preconditions.checkNotNull(line.date(), "Blame date cannot be null");
+  }
+
   private static void addChangeset(Builder scmBuilder, BlameLine line) {
     BatchReport.Changesets.Changeset.Builder changesetBuilder = BatchReport.Changesets.Changeset.newBuilder();
-    if (StringUtils.isNotBlank(line.revision())) {
-      changesetBuilder.setRevision(line.revision());
-    }
+    changesetBuilder.setRevision(line.revision());
+    changesetBuilder.setDate(line.date().getTime());
     if (StringUtils.isNotBlank(line.author())) {
       changesetBuilder.setAuthor(normalizeString(line.author()));
     }
-    Date date = line.date();
-    if (date != null) {
-      changesetBuilder.setDate(date.getTime());
-    }
+
     scmBuilder.addChangeset(changesetBuilder.build());
   }
 
diff --git a/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java b/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java
index a92b20382dd..e7f00149111 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java
@@ -19,20 +19,40 @@
  */
 package org.sonar.batch.scm;
 
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.sonar.api.batch.fs.InputComponent;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.scm.BlameLine;
+import org.sonar.batch.index.BatchComponent;
+import org.sonar.batch.index.BatchComponentCache;
 
 import java.util.Arrays;
+import java.util.Date;
+
+import static org.mockito.Matchers.any;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class DefaultBlameOutputTest {
 
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
+  private BatchComponentCache componentCache;
+
+  @Before
+  public void prepare() {
+    componentCache = mock(BatchComponentCache.class);
+    BatchComponent component = mock(BatchComponent.class);
+    when(component.batchId()).thenReturn(1);
+    when(componentCache.get(any(InputComponent.class))).thenReturn(component);
+  }
+
   @Test
   public void shouldNotFailIfNotSameNumberOfLines() {
     InputFile file = new DefaultInputFile("foo", "src/main/java/Foo.java").setLines(10);
@@ -51,4 +71,26 @@ public class DefaultBlameOutputTest {
       .blameResult(file, Arrays.asList(new BlameLine().revision("1").author("guy")));
   }
 
+  @Test
+  public void shouldFailIfNullDate() {
+    InputFile file = new DefaultInputFile("foo", "src/main/java/Foo.java").setLines(1);
+
+    thrown.expect(NullPointerException.class);
+    thrown.expectMessage("Blame date cannot be null");
+
+    new DefaultBlameOutput(null, componentCache, Arrays.<InputFile>asList(file))
+      .blameResult(file, Arrays.asList(new BlameLine().revision("1").author("guy")));
+  }
+
+  @Test
+  public void shouldFailIfNullRevision() {
+    InputFile file = new DefaultInputFile("foo", "src/main/java/Foo.java").setLines(1);
+
+    thrown.expect(NullPointerException.class);
+    thrown.expectMessage("Blame revision cannot be null");
+
+    new DefaultBlameOutput(null, componentCache, Arrays.<InputFile>asList(file))
+      .blameResult(file, Arrays.asList(new BlameLine().date(new Date()).author("guy")));
+  }
+
 }