From ba864aa618db80424f76908dc880d7af826dad94 Mon Sep 17 00:00:00 2001 From: Godin Date: Fri, 3 Dec 2010 12:34:20 +0000 Subject: [PATCH] SONAR-1450: Remove unused methods, improve unit tests --- .../timemachine/PastViolationsLoader.java | 15 +------ .../ViolationPersisterDecorator.java | 14 ++++--- .../timemachine/PastViolationsLoaderTest.java | 23 ++++++++--- .../ViolationPersisterDecoratorTest.java | 40 +++++++++---------- 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 getPastViolations(Resource resource) { Snapshot snapshot = resourcePersister.getSnapshot(resource); - return resourcePersister.getLastSnapshot(snapshot, true); - } - - public List 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 pastViolations = pastViolationsLoader.getPastViolations(previousLastSnapshot); + List 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 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 violations = loader.getPastViolations(snapshot); + doReturn(snapshot).when(resourcePersister) + .getLastSnapshot(any(Snapshot.class), anyBoolean()); + + List 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 crlf = ViolationPersisterDecorator.getChecksums("Hello\r\nWorld"); List lf = ViolationPersisterDecorator.getChecksums("Hello\nWorld"); assertThat(crlf.size(), is(2)); assertThat(crlf.get(0), not(equalTo(crlf.get(1)))); assertThat(lf, equalTo(crlf)); - List 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 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 pastViolationsByRule = LinkedHashMultimap.create(); + pastViolationsByRule.put(rule, pastViolation); + + RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule); + assertThat(found, equalTo(pastViolation)); + } @Test public void newViolation() { -- 2.39.5