From 469ecd7eb9b89351fd10d7e8d033a5292a683f0e Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Tue, 17 Sep 2013 18:17:18 +0200 Subject: [PATCH] SONAR-4682 Start decoupling hash computation from issue matching Create SourceHashHolder to take care of the heavy hash computations --- .../plugins/core/issue/IssueTracking.java | 62 ++++++------ .../core/issue/IssueTrackingDecorator.java | 11 ++- .../plugins/core/issue/SourceHashHolder.java | 94 +++++++++++++++++++ .../issue/IssueTrackingDecoratorTest.java | 17 +++- .../plugins/core/issue/IssueTrackingTest.java | 79 +++++++++++----- .../core/issue/SourceHashHolderTest.java | 82 ++++++++++++++++ 6 files changed, 282 insertions(+), 63 deletions(-) create mode 100644 plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java create mode 100644 plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/SourceHashHolderTest.java diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java index 871239ef6d6..00a6f32fc1e 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java @@ -20,6 +20,8 @@ package org.sonar.plugins.core.issue; +import org.sonar.plugins.core.issue.tracking.SourceChecksum; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Objects; import com.google.common.collect.LinkedHashMultimap; @@ -27,48 +29,47 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import org.sonar.api.BatchExtension; -import org.sonar.api.batch.SonarIndex; import org.sonar.api.issue.internal.DefaultIssue; -import org.sonar.api.resources.Resource; import org.sonar.api.rule.RuleKey; -import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.db.IssueDto; -import org.sonar.plugins.core.issue.tracking.*; +import org.sonar.plugins.core.issue.tracking.HashedSequence; +import org.sonar.plugins.core.issue.tracking.HashedSequenceComparator; +import org.sonar.plugins.core.issue.tracking.IssueTrackingBlocksRecognizer; +import org.sonar.plugins.core.issue.tracking.RollingHashSequence; +import org.sonar.plugins.core.issue.tracking.RollingHashSequenceComparator; +import org.sonar.plugins.core.issue.tracking.StringText; +import org.sonar.plugins.core.issue.tracking.StringTextComparator; import javax.annotation.Nullable; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Map; public class IssueTracking implements BatchExtension { - private final LastSnapshots lastSnapshots; - private final SonarIndex index; - - public IssueTracking(LastSnapshots lastSnapshots, SonarIndex index) { - this.lastSnapshots = lastSnapshots; - this.index = index; - } - - public IssueTrackingResult track(Resource resource, Collection dbIssues, Collection newIssues) { + public IssueTrackingResult track(SourceHashHolder sourceHashHolder, Collection dbIssues, Collection newIssues) { IssueTrackingResult result = new IssueTrackingResult(); - String source = index.getSource(resource); - setChecksumOnNewIssues(newIssues, source); + setChecksumOnNewIssues(newIssues, sourceHashHolder); // Map new issues with old ones - mapIssues(newIssues, dbIssues, source, resource, result); + mapIssues(newIssues, dbIssues, sourceHashHolder, result); return result; } - private void setChecksumOnNewIssues(Collection issues, String source) { - List checksums = SourceChecksum.lineChecksumsOfFile(source); + private void setChecksumOnNewIssues(Collection issues, SourceHashHolder sourceHashHolder) { + List checksums = SourceChecksum.lineChecksumsOfFile(sourceHashHolder.getSource()); for (DefaultIssue issue : issues) { issue.setChecksum(SourceChecksum.getChecksumForLine(checksums, issue.line())); } } + @VisibleForTesting - void mapIssues(Collection newIssues, @Nullable Collection lastIssues, @Nullable String source, @Nullable Resource resource, IssueTrackingResult result) { + void mapIssues(Collection newIssues, @Nullable Collection lastIssues, SourceHashHolder sourceHashHolder, IssueTrackingResult result) { boolean hasLastScan = false; if (lastIssues != null) { @@ -78,11 +79,8 @@ public class IssueTracking implements BatchExtension { // If each new issue matches an old one we can stop the matching mechanism if (result.matched().size() != newIssues.size()) { - if (source != null && resource != null && hasLastScan) { - String referenceSource = lastSnapshots.getSource(resource); - if (referenceSource != null) { - mapNewissues(referenceSource, newIssues, source, result); - } + if (sourceHashHolder.hasBothReferenceAndCurrentSource() && hasLastScan) { + mapNewissues(sourceHashHolder, newIssues, result); } mapIssuesOnSameRule(newIssues, result); } @@ -109,20 +107,18 @@ public class IssueTracking implements BatchExtension { } } - private void mapNewissues(String referenceSource, Collection newIssues, String source, IssueTrackingResult result) { - HashedSequence hashedReference = HashedSequence.wrap(new StringText(referenceSource), StringTextComparator.IGNORE_WHITESPACE); - HashedSequence hashedSource = HashedSequence.wrap(new StringText(source), StringTextComparator.IGNORE_WHITESPACE); + private void mapNewissues(SourceHashHolder sourceHashHolder, Collection newIssues, IssueTrackingResult result) { + HashedSequenceComparator hashedComparator = new HashedSequenceComparator(StringTextComparator.IGNORE_WHITESPACE); + IssueTrackingBlocksRecognizer rec = new IssueTrackingBlocksRecognizer(sourceHashHolder.getHashedReference(), sourceHashHolder.getHashedSource(), hashedComparator); - IssueTrackingBlocksRecognizer rec = new IssueTrackingBlocksRecognizer(hashedReference, hashedSource, hashedComparator); + RollingHashSequence> a = RollingHashSequence.wrap(sourceHashHolder.getHashedReference(), hashedComparator, 5); + RollingHashSequence> b = RollingHashSequence.wrap(sourceHashHolder.getHashedSource(), hashedComparator, 5); + RollingHashSequenceComparator> cmp = new RollingHashSequenceComparator>(hashedComparator); Multimap newIssuesByLines = newIssuesByLines(newIssues, rec, result); Multimap lastIssuesByLines = lastIssuesByLines(result.unmatched(), rec); - RollingHashSequence> a = RollingHashSequence.wrap(hashedReference, hashedComparator, 5); - RollingHashSequence> b = RollingHashSequence.wrap(hashedSource, hashedComparator, 5); - RollingHashSequenceComparator> cmp = new RollingHashSequenceComparator>(hashedComparator); - Map map = Maps.newHashMap(); for (Integer line : lastIssuesByLines.keySet()) { diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index 33efa59f1d6..23a61828953 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -19,6 +19,8 @@ */ package org.sonar.plugins.core.issue; +import org.sonar.api.batch.SonarIndex; +import org.sonar.batch.scan.LastSnapshots; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.collect.Lists; @@ -54,6 +56,8 @@ public class IssueTrackingDecorator implements Decorator { private final IssueCache issueCache; private final InitialOpenIssuesStack initialOpenIssues; private final IssueTracking tracking; + private final LastSnapshots lastSnapshots; + private final SonarIndex index; private final IssueHandlers handlers; private final IssueWorkflow workflow; private final IssueUpdater updater; @@ -63,6 +67,7 @@ public class IssueTrackingDecorator implements Decorator { private final RuleFinder ruleFinder; public IssueTrackingDecorator(IssueCache issueCache, InitialOpenIssuesStack initialOpenIssues, IssueTracking tracking, + LastSnapshots lastSnapshots, SonarIndex index, IssueHandlers handlers, IssueWorkflow workflow, IssueUpdater updater, Project project, @@ -72,6 +77,8 @@ public class IssueTrackingDecorator implements Decorator { this.issueCache = issueCache; this.initialOpenIssues = initialOpenIssues; this.tracking = tracking; + this.lastSnapshots = lastSnapshots; + this.index = index; this.handlers = handlers; this.workflow = workflow; this.updater = updater; @@ -104,7 +111,9 @@ public class IssueTrackingDecorator implements Decorator { // all the issues that are not closed in db before starting this module scan, including manual issues Collection dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getEffectiveKey()); - IssueTrackingResult trackingResult = tracking.track(resource, dbOpenIssues, issues); + SourceHashHolder sourceHashHolder = new SourceHashHolder(index, lastSnapshots, resource); + + IssueTrackingResult trackingResult = tracking.track(sourceHashHolder, dbOpenIssues, issues); // unmatched = issues that have been resolved + issues on disabled/removed rules + manual issues addUnmatched(trackingResult.unmatched(), issues); diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java new file mode 100644 index 00000000000..15633db972e --- /dev/null +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/SourceHashHolder.java @@ -0,0 +1,94 @@ +/* + * 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.plugins.core.issue; + +import org.sonar.api.batch.SonarIndex; +import org.sonar.api.resources.Resource; +import org.sonar.batch.scan.LastSnapshots; +import org.sonar.plugins.core.issue.tracking.HashedSequence; +import org.sonar.plugins.core.issue.tracking.StringText; +import org.sonar.plugins.core.issue.tracking.StringTextComparator; + + + +public class SourceHashHolder { + + private final SonarIndex index; + private final LastSnapshots lastSnapshots; + private final Resource resource; + + private String source; + private boolean sourceInitialized; + private String referenceSource; + private boolean referenceSourceInitialized; + + private HashedSequence hashedReference; + private HashedSequence hashedSource; + + public SourceHashHolder(SonarIndex index, LastSnapshots lastSnapshots, Resource resource) { + this.index = index; + this.lastSnapshots = lastSnapshots; + this.resource = resource; + } + + private void initHashes() { + hashedReference = HashedSequence.wrap(new StringText(referenceSource), StringTextComparator.IGNORE_WHITESPACE); + hashedSource = HashedSequence.wrap(new StringText(source), StringTextComparator.IGNORE_WHITESPACE); + } + + public HashedSequence getHashedReference() { + initHashesIfNull(hashedReference); + return hashedReference; + } + + public HashedSequence getHashedSource() { + initHashesIfNull(hashedSource); + return hashedSource; + } + + public String getSource() { + if (! sourceInitialized) { + source = index.getSource(resource); + sourceInitialized = true; + } + return source; + } + + public String getReferenceSource() { + if (! referenceSourceInitialized) { + if (resource != null) { + referenceSource = lastSnapshots.getSource(resource); + } + referenceSourceInitialized = true; + } + return referenceSource; + } + + public boolean hasBothReferenceAndCurrentSource() { + return getSource() != null && getReferenceSource() != null; + } + + private void initHashesIfNull(Object required) { + if(required == null) { + initHashes(); + } + } +} + diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java index d6940cbb483..8c742eaef33 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java @@ -19,6 +19,9 @@ */ package org.sonar.plugins.core.issue; +import org.sonar.api.batch.SonarIndex; + +import org.sonar.batch.scan.LastSnapshots; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -55,6 +58,8 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { IssueCache issueCache = mock(IssueCache.class, RETURNS_MOCKS); InitialOpenIssuesStack initialOpenIssues = mock(InitialOpenIssuesStack.class); IssueTracking tracking = mock(IssueTracking.class, RETURNS_MOCKS); + LastSnapshots lastSnapshots = mock(LastSnapshots.class); + SonarIndex index = mock(SonarIndex.class); IssueHandlers handlers = mock(IssueHandlers.class); IssueWorkflow workflow = mock(IssueWorkflow.class); IssueUpdater updater = mock(IssueUpdater.class); @@ -68,6 +73,8 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { issueCache, initialOpenIssues, tracking, + lastSnapshots, + index, handlers, workflow, updater, @@ -103,7 +110,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { decorator.doDecorate(file); // Apply filters, track, apply transitions, notify extensions then update cache - verify(tracking).track(eq(file), eq(dbIssues), argThat(new ArgumentMatcher>() { + verify(tracking).track(isA(SourceHashHolder.class), eq(dbIssues), argThat(new ArgumentMatcher>() { @Override public boolean matches(Object o) { List issues = (List) o; @@ -126,7 +133,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { IssueTrackingResult trackingResult = new IssueTrackingResult(); trackingResult.addUnmatched(unmatchedIssue); - when(tracking.track(eq(file), anyCollection(), anyCollection())).thenReturn(trackingResult); + when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult); decorator.doDecorate(file); @@ -154,7 +161,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { IssueTrackingResult trackingResult = new IssueTrackingResult(); trackingResult.addUnmatched(unmatchedIssue); - when(tracking.track(eq(file), anyCollection(), anyCollection())).thenReturn(trackingResult); + when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult); decorator.doDecorate(file); @@ -183,7 +190,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { IssueTrackingResult trackingResult = new IssueTrackingResult(); trackingResult.addUnmatched(unmatchedIssue); - when(tracking.track(eq(file), anyCollection(), anyCollection())).thenReturn(trackingResult); + when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult); decorator.doDecorate(file); @@ -212,7 +219,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { IssueTrackingResult trackingResult = new IssueTrackingResult(); trackingResult.addUnmatched(unmatchedIssue); - when(tracking.track(eq(file), anyCollection(), anyCollection())).thenReturn(trackingResult); + when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult); decorator.doDecorate(file); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java index cd2867c0d2f..3fe4080a746 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java @@ -20,6 +20,8 @@ package org.sonar.plugins.core.issue; +import org.sonar.api.batch.SonarIndex; + import com.google.common.base.Charsets; import com.google.common.io.Resources; import org.junit.Before; @@ -44,15 +46,18 @@ public class IssueTrackingTest { IssueTracking tracking; Resource project; + SourceHashHolder sourceHashHolder; + SonarIndex index; LastSnapshots lastSnapshots; long violationId = 0; @Before public void before() { + index = mock(SonarIndex.class); lastSnapshots = mock(LastSnapshots.class); project = mock(Project.class); - tracking = new IssueTracking(lastSnapshots, null); + tracking = new IssueTracking(); } @Test @@ -64,13 +69,15 @@ public class IssueTrackingTest { DefaultIssue newIssue = newDefaultIssue("message", 10, RuleKey.of("squid", "AvoidCycle"), "checksum1").setKey("200"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue1, referenceIssue2), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue1, referenceIssue2), null, result); // same key assertThat(result.matching(newIssue)).isSameAs(referenceIssue2); } @Test public void checksum_should_have_greater_priority_than_line() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + IssueDto referenceIssue1 = newReferenceIssue("message", 1, "squid", "AvoidCycle", "checksum1"); IssueDto referenceIssue2 = newReferenceIssue("message", 3, "squid", "AvoidCycle", "checksum2"); @@ -78,7 +85,7 @@ public class IssueTrackingTest { DefaultIssue newIssue2 = newDefaultIssue("message", 5, RuleKey.of("squid", "AvoidCycle"), "checksum2"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue1, newIssue2), newArrayList(referenceIssue1, referenceIssue2), null, null, result); + tracking.mapIssues(newArrayList(newIssue1, newIssue2), newArrayList(referenceIssue1, referenceIssue2), sourceHashHolder, result); assertThat(result.matching(newIssue1)).isSameAs(referenceIssue1); assertThat(result.matching(newIssue2)).isSameAs(referenceIssue2); } @@ -88,51 +95,61 @@ public class IssueTrackingTest { */ @Test public void same_rule_and_null_line_and_checksum_but_different_messages() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("new message", null, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("old message", null, "squid", "AvoidCycle", "checksum1"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void same_rule_and_line_and_checksum_but_different_messages() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("new message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("old message", 1, "squid", "AvoidCycle", "checksum1"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void same_rule_and_line_message() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "AvoidCycle", "checksum2"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void should_ignore_reference_measure_without_checksum() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), null); IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "NullDeref", null); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isNull(); } @Test public void same_rule_and_message_and_checksum_but_different_line() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("message", 2, "squid", "AvoidCycle", "checksum1"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @@ -141,56 +158,65 @@ public class IssueTrackingTest { */ @Test public void same_checksum_and_rule_but_different_line_and_different_message() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("new message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("old message", 2, "squid", "AvoidCycle", "checksum1"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void should_create_new_issue_when_same_rule_same_message_but_different_line_and_checksum() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("message", 2, "squid", "AvoidCycle", "checksum2"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isNull(); } @Test public void should_not_track_issue_if_different_rule() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "NullDeref", "checksum1"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isNull(); } @Test public void should_compare_issues_with_database_format() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + // issue messages are trimmed and can be abbreviated when persisted in database. // Comparing issue messages must use the same format. DefaultIssue newIssue = newDefaultIssue(" message ", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "AvoidCycle", "checksum2"); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void past_issue_not_associated_with_line_should_not_cause_npe() throws Exception { when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - String source = load("example2-v2"); + when(index.getSource(project)).thenReturn(load("example2-v2")); + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, project); DefaultIssue newIssue = newDefaultIssue("Indentation", 9, RuleKey.of("squid", "AvoidCycle"), "foo"); IssueDto referenceIssue = newReferenceIssue("2 branches need to be covered", null, "squid", "AvoidCycle", null); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), source, project, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matched()).isEmpty(); } @@ -198,13 +224,14 @@ public class IssueTrackingTest { @Test public void new_issue_not_associated_with_line_should_not_cause_npe() throws Exception { when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - String source = load("example2-v2"); + when(index.getSource(project)).thenReturn(load("example2-v2")); + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, project); DefaultIssue newIssue = newDefaultIssue("1 branch need to be covered", null, RuleKey.of("squid", "AvoidCycle"), "foo"); IssueDto referenceIssue = newReferenceIssue("Indentationd", 7, "squid", "AvoidCycle", null); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), source, project, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matched()).isEmpty(); } @@ -215,13 +242,14 @@ public class IssueTrackingTest { @Test public void issue_not_associated_with_line() throws Exception { when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - String source = load("example2-v2"); + when(index.getSource(project)).thenReturn(load("example2-v2")); + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, project); DefaultIssue newIssue = newDefaultIssue("1 branch need to be covered", null, RuleKey.of("squid", "AvoidCycle"), null); IssueDto referenceIssue = newReferenceIssue("2 branches need to be covered", null, "squid", "AvoidCycle", null); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), source, project, result); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), sourceHashHolder, result); assertThat(result.matching(newIssue)).isEqualTo(referenceIssue); } @@ -232,7 +260,8 @@ public class IssueTrackingTest { @Test public void should_track_issues_based_on_blocks_recognition_on_example1() throws Exception { when(lastSnapshots.getSource(project)).thenReturn(load("example1-v1")); - String source = load("example1-v2"); + when(index.getSource(project)).thenReturn(load("example1-v2")); + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, project); IssueDto referenceIssue1 = newReferenceIssue("Indentation", 7, "squid", "AvoidCycle", null); IssueDto referenceIssue2 = newReferenceIssue("Indentation", 11, "squid", "AvoidCycle", null); @@ -243,7 +272,7 @@ public class IssueTrackingTest { DefaultIssue newIssue4 = newDefaultIssue("Indentation", 21, RuleKey.of("squid", "AvoidCycle"), null); IssueTrackingResult result = new IssueTrackingResult(); - tracking.mapIssues(Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4), Arrays.asList(referenceIssue1, referenceIssue2), source, project, result); + tracking.mapIssues(Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4), Arrays.asList(referenceIssue1, referenceIssue2), sourceHashHolder, result); assertThat(result.matching(newIssue1)).isNull(); assertThat(result.matching(newIssue2)).isNull(); @@ -257,7 +286,8 @@ public class IssueTrackingTest { @Test public void should_track_issues_based_on_blocks_recognition_on_example2() throws Exception { when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - String source = load("example2-v2"); + when(index.getSource(project)).thenReturn(load("example2-v2")); + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, project); IssueDto referenceIssue1 = newReferenceIssue("SystemPrintln", 5, "squid", "AvoidCycle", null); @@ -269,7 +299,7 @@ public class IssueTrackingTest { tracking.mapIssues( Arrays.asList(newIssue1, newIssue2, newIssue3), Arrays.asList(referenceIssue1), - source, project, result); + sourceHashHolder, result); assertThat(result.matching(newIssue1)).isNull(); assertThat(result.matching(newIssue2)).isSameAs(referenceIssue1); @@ -279,7 +309,8 @@ public class IssueTrackingTest { @Test public void should_track_issues_based_on_blocks_recognition_on_example3() throws Exception { when(lastSnapshots.getSource(project)).thenReturn(load("example3-v1")); - String source = load("example3-v2"); + when(index.getSource(project)).thenReturn(load("example3-v2")); + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, project); IssueDto referenceIssue1 = newReferenceIssue("Avoid unused local variables such as 'j'.", 6, "squid", "AvoidCycle", "63c11570fc0a76434156be5f8138fa03"); IssueDto referenceIssue2 = newReferenceIssue("Avoid unused private methods such as 'myMethod()'.", 13, "squid", "NullDeref", "ef23288705d1ef1e512448ace287586e"); @@ -300,7 +331,7 @@ public class IssueTrackingTest { tracking.mapIssues( Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4, newIssue5), Arrays.asList(referenceIssue1, referenceIssue2, referenceIssue3), - source, project, result); + sourceHashHolder, result); assertThat(result.matching(newIssue1)).isNull(); assertThat(result.matching(newIssue2)).isSameAs(referenceIssue2); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/SourceHashHolderTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/SourceHashHolderTest.java new file mode 100644 index 00000000000..f8fac6119ac --- /dev/null +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/SourceHashHolderTest.java @@ -0,0 +1,82 @@ +/* + * 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.plugins.core.issue; + +import static org.mockito.Mockito.verify; + +import org.mockito.Mockito; +import org.junit.Before; +import org.junit.Test; +import org.sonar.api.batch.SonarIndex; +import org.sonar.api.resources.Resource; +import org.sonar.batch.scan.LastSnapshots; +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class SourceHashHolderTest { + + SourceHashHolder sourceHashHolder; + + SonarIndex index; + LastSnapshots lastSnapshots; + Resource resource; + + @Before + public void setUp() { + index = mock(SonarIndex.class); + lastSnapshots = mock(LastSnapshots.class); + resource = mock(Resource.class); + + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, resource); + } + + @Test + public void should_lazy_load_source() { + final String source = "source"; + when(index.getSource(resource)).thenReturn(source); + + assertThat(sourceHashHolder.getSource()).isEqualTo(source); + verify(index).getSource(resource); + + assertThat(sourceHashHolder.getSource()).isEqualTo(source); + Mockito.verifyNoMoreInteractions(index); + } + + @Test + public void should_lazy_load_reference_source() { + final String source = "source"; + when(lastSnapshots.getSource(resource)).thenReturn(source); + + assertThat(sourceHashHolder.getReferenceSource()).isEqualTo(source); + verify(lastSnapshots).getSource(resource); + + assertThat(sourceHashHolder.getReferenceSource()).isEqualTo(source); + Mockito.verifyNoMoreInteractions(lastSnapshots); + } + + @Test + public void should_have_null_reference_source_for_null_resource() { + sourceHashHolder = new SourceHashHolder(index, lastSnapshots, null); + + assertThat(sourceHashHolder.getReferenceSource()).isNull(); + Mockito.verifyNoMoreInteractions(lastSnapshots); + } +} -- 2.39.5