SONAR-1450: Remove unused methods, improve unit tests

4 files changed, 46 insertions, 46 deletions

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastViolationsLoader.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastViolationsLoader.java
index e4217bbc5c2..c6a8a503256 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastViolationsLoader.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastViolationsLoader.java
@@ -21,12 +21,9 @@ public class PastViolationsLoader implements BatchExtension {
     this.resourcePersister = resourcePersister;
   }
 
-  public Snapshot getPreviousLastSnapshot(Resource resource) {
+  public List<RuleFailureModel> getPastViolations(Resource resource) {
     Snapshot snapshot = resourcePersister.getSnapshot(resource);
-    return resourcePersister.getLastSnapshot(snapshot, true);
-  }
-
-  public List<RuleFailureModel> getPastViolations(Snapshot previousLastSnapshot) {
+    Snapshot previousLastSnapshot = resourcePersister.getLastSnapshot(snapshot, true);
     if (previousLastSnapshot == null) {
       return Collections.emptyList();
     }
@@ -34,14 +31,6 @@ public class PastViolationsLoader implements BatchExtension {
         "snapshotId", previousLastSnapshot.getId());
   }
 
-  public SnapshotSource getPastSource(Snapshot previousLastSnapshot) {
-    if (previousLastSnapshot == null) {
-      return null;
-    }
-    return session.getSingleResult(SnapshotSource.class,
-        "snapshotId", previousLastSnapshot.getId());
-  }
-
   public SnapshotSource getSource(Resource resource) {
     Snapshot snapshot = resourcePersister.getSnapshot(resource);
     return session.getSingleResult(SnapshotSource.class,
diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java
index 2ec1661f72b..dbbddcc27e7 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecorator.java
@@ -12,7 +12,6 @@ import org.sonar.api.batch.DecoratorBarriers;
 import org.sonar.api.batch.DecoratorContext;
 import org.sonar.api.batch.DependsUpon;
 import org.sonar.api.database.model.RuleFailureModel;
-import org.sonar.api.database.model.Snapshot;
 import org.sonar.api.database.model.SnapshotSource;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
@@ -52,14 +51,13 @@ public class ViolationPersisterDecorator implements Decorator {
   }
 
   public void decorate(Resource resource, DecoratorContext context) {
-    Snapshot previousLastSnapshot = pastViolationsLoader.getPreviousLastSnapshot(resource);
     // Load past violations
-    List<RuleFailureModel> pastViolations = pastViolationsLoader.getPastViolations(previousLastSnapshot);
+    List<RuleFailureModel> pastViolations = pastViolationsLoader.getPastViolations(resource);
     // Load current source and calculate checksums
     checksums = getChecksums(pastViolationsLoader.getSource(resource));
     // Save violations
     compareWithPastViolations(context, pastViolations);
-    // Clear caches
+    // Clear cache
     checksums.clear();
   }
 
@@ -93,8 +91,7 @@ public class ViolationPersisterDecorator implements Decorator {
     try {
       List<String> lines = IOUtils.readLines(new StringInputStream(data));
       for (String line : lines) {
-        String reducedLine = StringUtils.replaceChars(line, SPACE_CHARS, "");
-        result.add(DigestUtils.md5Hex(reducedLine));
+        result.add(getChecksum(line));
       }
     } catch (IOException e) {
       throw new SonarException("Unable to calculate checksums", e);
@@ -102,6 +99,11 @@ public class ViolationPersisterDecorator implements Decorator {
     return result;
   }
 
+  static String getChecksum(String line) {
+    String reducedLine = StringUtils.replaceChars(line, SPACE_CHARS, "");
+    return DigestUtils.md5Hex(reducedLine);
+  }
+
   /**
    * @return checksum or null if checksum not exists for line
    */
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/PastViolationsLoaderTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/PastViolationsLoaderTest.java
index ba0891da475..5f7bc8e0b34 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/PastViolationsLoaderTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/PastViolationsLoaderTest.java
@@ -1,31 +1,42 @@
 package org.sonar.plugins.core.timemachine;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.notNullValue;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.database.model.RuleFailureModel;
 import org.sonar.api.database.model.Snapshot;
+import org.sonar.api.resources.JavaFile;
+import org.sonar.batch.index.ResourcePersister;
 import org.sonar.jpa.test.AbstractDbUnitTestCase;
 
 import java.util.List;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
 public class PastViolationsLoaderTest extends AbstractDbUnitTestCase {
 
+  private ResourcePersister resourcePersister;
   private PastViolationsLoader loader;
 
   @Before
   public void setUp() {
     setupData("shared");
-    loader = new PastViolationsLoader(getSession(), null);
+    resourcePersister = mock(ResourcePersister.class);
+    loader = new PastViolationsLoader(getSession(), resourcePersister);
   }
 
   @Test
   public void shouldGetPastResourceViolations() {
     Snapshot snapshot = getSession().getSingleResult(Snapshot.class, "id", 1000);
-    List<RuleFailureModel> violations = loader.getPastViolations(snapshot);
+    doReturn(snapshot).when(resourcePersister)
+        .getLastSnapshot(any(Snapshot.class), anyBoolean());
+
+    List<RuleFailureModel> violations = loader.getPastViolations(new JavaFile("file"));
 
     assertThat(violations.size(), is(2));
   }
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java
index c8c543af067..12feac164f6 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationPersisterDecoratorTest.java
@@ -26,17 +26,15 @@ public class ViolationPersisterDecoratorTest {
   }
 
   @Test
-  public void testGetChecksums() {
+  public void shouldGenerateCorrectChecksums() {
     List<String> crlf = ViolationPersisterDecorator.getChecksums("Hello\r\nWorld");
     List<String> lf = ViolationPersisterDecorator.getChecksums("Hello\nWorld");
     assertThat(crlf.size(), is(2));
     assertThat(crlf.get(0), not(equalTo(crlf.get(1))));
     assertThat(lf, equalTo(crlf));
 
-    List<String> spaces = ViolationPersisterDecorator.getChecksums("" +
-        "\tvoid  method()  {\n" +
-        "  void method() {");
-    assertThat(spaces.get(0), equalTo(spaces.get(1)));
+    assertThat(ViolationPersisterDecorator.getChecksum("\tvoid  method()  {\n"),
+        equalTo(ViolationPersisterDecorator.getChecksum("  void method() {")));
   }
 
   @Test
@@ -54,22 +52,22 @@ public class ViolationPersisterDecoratorTest {
     assertThat(found, equalTo(pastViolation));
   }
 
-  // @Test
-  // public void sameRuleAndMessageButDifferentLine() {
-  // Rule rule = Rule.create().setKey("rule");
-  // Violation violation = Violation.create(rule, null)
-  // .setLineId(1).setMessage("message");
-  // decorator.checksums = ViolationPersisterDecorator.getChecksums("violation");
-  //
-  // RuleFailureModel pastViolation = newPastViolation(rule, 2, "message");
-  // decorator.pastChecksums = ViolationPersisterDecorator.getChecksums("line\nviolation");
-  //
-  // Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
-  // pastViolationsByRule.put(rule, pastViolation);
-  //
-  // RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
-  // assertThat(found, equalTo(pastViolation));
-  // }
+  @Test
+  public void sameRuleAndMessageButDifferentLine() {
+    Rule rule = Rule.create().setKey("rule");
+    Violation violation = Violation.create(rule, null)
+        .setLineId(1).setMessage("message");
+    decorator.checksums = ViolationPersisterDecorator.getChecksums("violation");
+
+    RuleFailureModel pastViolation = newPastViolation(rule, 2, "message");
+    pastViolation.setChecksum(ViolationPersisterDecorator.getChecksum("violation"));
+
+    Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
+    pastViolationsByRule.put(rule, pastViolation);
+
+    RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
+    assertThat(found, equalTo(pastViolation));
+  }
 
   @Test
   public void newViolation() {