Browse Source

SONAR-6990 strongly reduce HEAP usage of the DuplicationRepository

tags/5.3-RC2
Sébastien Lesaint 8 years ago
parent
commit
9c311c5591
14 changed files with 285 additions and 816 deletions
  1. 11
    3
      server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java
  2. 3
    42
      server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepository.java
  3. 14
    339
      server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepositoryImpl.java
  4. 47
    12
      server/sonar-server/src/main/java/org/sonar/server/computation/duplication/IntegrateCrossProjectDuplications.java
  5. 6
    7
      server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java
  6. 3
    3
      server/sonar-server/src/main/java/org/sonar/server/computation/step/DuplicationDataMeasuresStep.java
  7. 3
    2
      server/sonar-server/src/main/java/org/sonar/server/computation/step/DuplicationMeasuresStep.java
  8. 34
    8
      server/sonar-server/src/main/java/org/sonar/server/computation/step/LoadDuplicationsFromReportStep.java
  9. 29
    333
      server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryImplTest.java
  10. 91
    25
      server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryRule.java
  11. 30
    34
      server/sonar-server/src/test/java/org/sonar/server/computation/duplication/IntegrateCrossProjectDuplicationsTest.java
  12. 1
    1
      server/sonar-server/src/test/java/org/sonar/server/computation/step/LoadDuplicationsFromReportStepTest.java
  13. 8
    1
      server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java
  14. 5
    6
      server/sonar-server/src/test/java/org/sonar/server/computation/step/ReportDuplicationMeasuresStepTest.java

+ 11
- 3
server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java View File

@@ -23,6 +23,7 @@ package org.sonar.server.benchmark;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.apache.commons.io.FileUtils;
import org.junit.Rule;
@@ -43,7 +44,10 @@ import org.sonar.server.computation.batch.BatchReportReaderImpl;
import org.sonar.server.computation.batch.TreeRootHolderRule;
import org.sonar.server.computation.component.Component;
import org.sonar.server.computation.component.ReportComponent;
import org.sonar.server.computation.duplication.Duplicate;
import org.sonar.server.computation.duplication.Duplication;
import org.sonar.server.computation.duplication.DuplicationRepositoryRule;
import org.sonar.server.computation.duplication.InnerDuplicate;
import org.sonar.server.computation.duplication.TextBlock;
import org.sonar.server.computation.scm.ScmInfoRepositoryImpl;
import org.sonar.server.computation.source.SourceHashRepositoryImpl;
@@ -141,9 +145,13 @@ public class PersistFileSourcesStepTest {
LineData lineData = new LineData();
for (int line = 1; line <= NUMBER_OF_LINES; line++) {
lineData.generateLineData(line);

duplicationRepository.addDuplication(fileRef, new TextBlock(line, line), new TextBlock(line + 1, line + 1));

duplicationRepository.add(
fileRef,
new Duplication(
new TextBlock(line, line),
Arrays.<Duplicate>asList(new InnerDuplicate(new TextBlock(line + 1, line + 1)))
)
);
}
writer.writeComponent(BatchReport.Component.newBuilder()
.setRef(fileRef)

+ 3
- 42
server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepository.java View File

@@ -19,7 +19,6 @@
*/
package org.sonar.server.computation.duplication;

import java.util.Set;
import org.sonar.server.computation.component.Component;

/**
@@ -34,58 +33,20 @@ import org.sonar.server.computation.component.Component;
* </p>
*/
public interface DuplicationRepository {

/**
* Returns the duplications in the specified file {@link Component}, if any.
*
* @throws NullPointerException if {@code file} is {@code null}
* @throws IllegalArgumentException if the type of the {@link Component} argument is not {@link Component.Type#FILE}
*/
Set<Duplication> getDuplications(Component file);
Iterable<Duplication> getDuplications(Component file);

/**
* Adds a project duplication of the specified original {@link TextBlock} in the specified file {@link Component}
* which is duplicated by the specified duplicate {@link TextBlock} in the same file.
* <p>
* This method can be called multiple times with the same {@code file} and {@code original} but a different
* {@code duplicate} to add multiple duplications of the same block.
* </p>
* <p>
* It must not, however, be called twice with {@code original} and {@code duplicate} swapped, this would raise
* an {@link IllegalArgumentException} as the duplication already exists.
* </p>
* Adds a project duplication in the specified file {@link Component} to the repository.
*
* @throws NullPointerException if any argument is {@code null}
* @throws IllegalArgumentException if the type of the {@link Component} argument is not {@link Component.Type#FILE}
* @throws IllegalStateException if {@code original} and {@code duplicate} are the same
* @throws IllegalStateException if the specified duplication already exists in the repository
*/
void addDuplication(Component file, TextBlock original, TextBlock duplicate);

/**
* Adds a project duplication of the specified original {@link TextBlock} in the specified file {@link Component} as
* duplicated by the duplicate {@link TextBlock} in the other file {@link Component}.
* <p>
* Note: the reverse duplication relationship between files is not added automatically (which leaves open the
* possibility of inconsistent duplication information). This means that it is the responsibility of the repository's
* user to call this method again with the {@link Component} arguments and the {@link TextBlock} arguments swapped.
* </p>
*
* @throws NullPointerException if any argument is {@code null}
* @throws IllegalArgumentException if the type of any of the {@link Component} arguments is not {@link Component.Type#FILE}
* @throws IllegalArgumentException if {@code file} and {@code otherFile} are the same
* @throws IllegalStateException if the specified duplication already exists in the repository
*/
void addDuplication(Component file, TextBlock original, Component otherFile, TextBlock duplicate);
void add(Component file, Duplication duplication);

/**
* Adds a cross-project duplication of the specified original {@link TextBlock} in the specified file {@link Component},
* as duplicated by the specified duplicate {@link TextBlock} in a file of another project identified by its key.
*
* @throws NullPointerException if any argument is {@code null}
* @throws IllegalArgumentException if the type of the {@link Component} argument is not {@link Component.Type#FILE}
* @throws IllegalArgumentException if {@code otherFileKey} is the key of a file in the project
* @throws IllegalStateException if the specified duplication already exists in the repository
*/
void addDuplication(Component file, TextBlock original, String otherFileKey, TextBlock duplicate);
}

+ 14
- 339
server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepositoryImpl.java View File

@@ -19,369 +19,44 @@
*/
package org.sonar.server.computation.duplication;

import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.Multimap;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import org.sonar.server.computation.component.Component;
import org.sonar.server.computation.component.TreeRootHolder;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.FluentIterable.from;
import static java.util.Objects.requireNonNull;

/**
* In-memory implementation of {@link DuplicationRepository}.
*/
public class DuplicationRepositoryImpl implements DuplicationRepository {
private final TreeRootHolder treeRootHolder;
private final Map<String, Duplications> duplicationsByComponentUuid = new HashMap<>();

public DuplicationRepositoryImpl(TreeRootHolder treeRootHolder) {
this.treeRootHolder = treeRootHolder;
}

@Override
public Set<Duplication> getDuplications(Component file) {
checkFileComponentArgument(file);

Duplications duplications = duplicationsByComponentUuid.get(file.getUuid());
if (duplications != null) {
return from(duplications.getDuplicates())
.transform(DuplicatesEntryToDuplication.INSTANCE)
.toSet();
}
return Collections.emptySet();
}

@Override
public void addDuplication(Component file, TextBlock original, TextBlock duplicate) {
checkFileComponentArgument(file);
checkOriginalTextBlockArgument(original);
checkDuplicateTextBlockArgument(duplicate);
checkArgument(!original.equals(duplicate), "original and duplicate TextBlocks can not be the same");

Optional<Duplicates> duplicates = getDuplicates(file, original);
if (duplicates.isPresent()) {
checkDuplicationAlreadyExists(duplicates, file, original, duplicate);
checkReverseDuplicationAlreadyExists(file, original, duplicate);

duplicates.get().add(duplicate);
} else {
checkReverseDuplicationAlreadyExists(file, original, duplicate);
getOrCreate(file, original).add(duplicate);
}
}
private Multimap<String, Duplication> duplications = HashMultimap.create();

@Override
public void addDuplication(Component file, TextBlock original, Component otherFile, TextBlock duplicate) {
public Iterable<Duplication> getDuplications(Component file) {
checkFileComponentArgument(file);
checkOriginalTextBlockArgument(original);
checkComponentArgument(otherFile, "otherFile");
checkDuplicateTextBlockArgument(duplicate);
checkArgument(!file.equals(otherFile), "file and otherFile Components can not be the same");

Optional<Duplicates> duplicates = getDuplicates(file, original);
if (duplicates.isPresent()) {
checkDuplicationAlreadyExists(duplicates, file, original, otherFile, duplicate);

duplicates.get().add(otherFile, duplicate);
} else {
getOrCreate(file, original).add(otherFile, duplicate);
Collection<Duplication> duplications = this.duplications.asMap().get(file.getKey());
if (duplications == null) {
return Collections.emptyList();
}
return duplications;
}

@Override
public void addDuplication(Component file, TextBlock original, String otherFileKey, TextBlock duplicate) {
public void add(Component file, Duplication duplication) {
checkFileComponentArgument(file);
checkOriginalTextBlockArgument(original);
requireNonNull(otherFileKey, "otherFileKey can not be null");
checkDuplicateTextBlockArgument(duplicate);
checkArgument(!treeRootHolder.hasComponentWithKey(otherFileKey), "otherFileKey '%s' can not be the key of a Component in the project", otherFileKey);

Optional<Duplicates> duplicates = getDuplicates(file, original);
if (duplicates.isPresent()) {
checkDuplicationAlreadyExists(duplicates, file, original, otherFileKey, duplicate);

duplicates.get().add(otherFileKey, duplicate);
} else {
getOrCreate(file, original).add(otherFileKey, duplicate);
}
}

private Optional<Duplicates> getDuplicates(Component file, TextBlock original) {
Duplications duplications = duplicationsByComponentUuid.get(file.getUuid());
if (duplications == null) {
return Optional.absent();
}
return duplications.get(original);
}

private void checkDuplicationAlreadyExists(Optional<Duplicates> duplicates, Component file, TextBlock original, TextBlock duplicate) {
checkArgument(!duplicates.get().hasDuplicate(duplicate),
"Inner duplicate %s is already associated to original %s in file %s", duplicate, original, file.getKey());
}

private void checkReverseDuplicationAlreadyExists(Component file, TextBlock original, TextBlock duplicate) {
Optional<Duplicates> reverseDuplication = getDuplicates(file, duplicate);
if (reverseDuplication.isPresent()) {
checkArgument(!reverseDuplication.get().hasDuplicate(original),
"Inner duplicate %s is already associated to original %s in file %s", duplicate, original, file.getKey());
}
}

private static void checkDuplicationAlreadyExists(Optional<Duplicates> duplicates, Component file, TextBlock original, Component otherFile, TextBlock duplicate) {
checkArgument(!duplicates.get().hasDuplicate(otherFile, duplicate),
"In-project duplicate %s in file %s is already associated to original %s in file %s", duplicate, otherFile.getKey(), original, file.getKey());
}

private static void checkDuplicationAlreadyExists(Optional<Duplicates> duplicates, Component file, TextBlock original, String otherFileKey, TextBlock duplicate) {
checkArgument(!duplicates.get().hasDuplicate(otherFileKey, duplicate),
"Cross-project duplicate %s in file %s is already associated to original %s in file %s", duplicate, otherFileKey, original, file.getKey());
}

private Duplicates getOrCreate(Component file, TextBlock original) {
Duplications duplications = duplicationsByComponentUuid.get(file.getUuid());
if (duplications == null) {
duplications = new Duplications();
duplicationsByComponentUuid.put(file.getUuid(), duplications);
}

return duplications.getOrCreate(original);
}

/**
* Represents the location of the file of one or more duplicate {@link TextBlock}.
*/
private interface DuplicateLocation {

}

private enum InnerDuplicationLocation implements DuplicateLocation {
INSTANCE
}

@Immutable
private static final class InProjectDuplicationLocation implements DuplicateLocation {
private final Component component;

public InProjectDuplicationLocation(Component component) {
this.component = component;
}

public Component getComponent() {
return component;
}

@Override
public boolean equals(@Nullable Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
InProjectDuplicationLocation that = (InProjectDuplicationLocation) o;
return component.equals(that.component);
}

@Override
public int hashCode() {
return component.hashCode();
}
}

@Immutable
private static final class CrossProjectDuplicationLocation implements DuplicateLocation {
private final String fileKey;

private CrossProjectDuplicationLocation(String fileKey) {
this.fileKey = fileKey;
}

public String getFileKey() {
return fileKey;
}

@Override
public boolean equals(@Nullable Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
CrossProjectDuplicationLocation that = (CrossProjectDuplicationLocation) o;
return fileKey.equals(that.fileKey);
}

@Override
public int hashCode() {
return fileKey.hashCode();
}
}

private static final class Duplications {
private final Map<TextBlock, Duplicates> duplicatesByTextBlock = new HashMap<>();

public Duplicates getOrCreate(TextBlock textBlock) {
Duplicates duplicates = duplicatesByTextBlock.get(textBlock);
if (duplicates != null) {
return duplicates;
}
Duplicates res = new Duplicates();
duplicatesByTextBlock.put(textBlock, res);
return res;
}

public Set<Map.Entry<TextBlock, Duplicates>> getDuplicates() {
return duplicatesByTextBlock.entrySet();
}

public Optional<Duplicates> get(TextBlock original) {
return Optional.fromNullable(duplicatesByTextBlock.get(original));
}
}

private static final class Duplicates {
private final Map<DuplicateLocation, Set<TextBlock>> duplicatesByLocation = new HashMap<>();

public Iterable<DuplicateWithLocation> getDuplicates() {
return from(duplicatesByLocation.entrySet())
.transformAndConcat(MapEntryToDuplicateWithLocation.INSTANCE);
}

public void add(TextBlock duplicate) {
add(InnerDuplicationLocation.INSTANCE, duplicate);
}

public void add(Component otherFile, TextBlock duplicate) {
InProjectDuplicationLocation key = new InProjectDuplicationLocation(otherFile);
add(key, duplicate);
}

public void add(String otherFileKey, TextBlock duplicate) {
CrossProjectDuplicationLocation key = new CrossProjectDuplicationLocation(otherFileKey);
add(key, duplicate);
}

private void add(DuplicateLocation duplicateLocation, TextBlock duplicate) {
Set<TextBlock> textBlocks = duplicatesByLocation.get(duplicateLocation);
if (textBlocks == null) {
textBlocks = new HashSet<>(1);
duplicatesByLocation.put(duplicateLocation, textBlocks);
}
textBlocks.add(duplicate);
}

public boolean hasDuplicate(TextBlock duplicate) {
return containsEntry(InnerDuplicationLocation.INSTANCE, duplicate);
}

public boolean hasDuplicate(Component otherFile, TextBlock duplicate) {
return containsEntry(new InProjectDuplicationLocation(otherFile), duplicate);
}

public boolean hasDuplicate(String otherFileKey, TextBlock duplicate) {
return containsEntry(new CrossProjectDuplicationLocation(otherFileKey), duplicate);
}

private boolean containsEntry(DuplicateLocation duplicateLocation, TextBlock duplicate) {
Set<TextBlock> textBlocks = duplicatesByLocation.get(duplicateLocation);
return textBlocks != null && textBlocks.contains(duplicate);
}

private enum MapEntryToDuplicateWithLocation implements Function<Map.Entry<DuplicateLocation, Set<TextBlock>>, Iterable<DuplicateWithLocation>> {
INSTANCE;

@Override
@Nonnull
public Iterable<DuplicateWithLocation> apply(@Nonnull final Map.Entry<DuplicateLocation, Set<TextBlock>> entry) {
return from(entry.getValue())
.transform(new Function<TextBlock, DuplicateWithLocation>() {
@Override
@Nonnull
public DuplicateWithLocation apply(@Nonnull TextBlock input) {
return new DuplicateWithLocation(entry.getKey(), input);
}
});
}
}
}

@Immutable
private static final class DuplicateWithLocation {
private final DuplicateLocation location;
private final TextBlock duplicate;

private DuplicateWithLocation(DuplicateLocation location, TextBlock duplicate) {
this.location = location;
this.duplicate = duplicate;
}

public DuplicateLocation getLocation() {
return location;
}

public TextBlock getDuplicate() {
return duplicate;
}
checkNotNull(duplication, "duplication can not be null");

duplications.put(file.getKey(), duplication);
}

private static void checkFileComponentArgument(Component file) {
checkComponentArgument(file, "file");
}

private static void checkComponentArgument(Component file, String argName) {
checkNotNull(file, "%s can not be null", argName);
checkArgument(file.getType() == Component.Type.FILE, "type of %s argument must be FILE", argName);
}

private static void checkDuplicateTextBlockArgument(TextBlock duplicate) {
requireNonNull(duplicate, "duplicate can not be null");
}

private static void checkOriginalTextBlockArgument(TextBlock original) {
requireNonNull(original, "original can not be null");
requireNonNull(file, "file can not be null");
checkArgument(file.getType() == Component.Type.FILE, "type of file must be FILE");
}

private enum DuplicatesEntryToDuplication implements Function<Map.Entry<TextBlock, Duplicates>, Duplication> {
INSTANCE;

@Override
@Nonnull
public Duplication apply(@Nonnull Map.Entry<TextBlock, Duplicates> entry) {
return new Duplication(
entry.getKey(),
from(entry.getValue().getDuplicates()).transform(DuplicateLocationEntryToDuplicate.INSTANCE));
}

private enum DuplicateLocationEntryToDuplicate implements Function<DuplicateWithLocation, Duplicate> {
INSTANCE;

@Override
@Nonnull
public Duplicate apply(@Nonnull DuplicateWithLocation input) {
DuplicateLocation duplicateLocation = input.getLocation();
if (duplicateLocation instanceof InnerDuplicationLocation) {
return new InnerDuplicate(input.getDuplicate());
}
if (duplicateLocation instanceof InProjectDuplicationLocation) {
return new InProjectDuplicate(((InProjectDuplicationLocation) duplicateLocation).getComponent(), input.getDuplicate());
}
if (duplicateLocation instanceof CrossProjectDuplicationLocation) {
return new CrossProjectDuplicate(((CrossProjectDuplicationLocation) duplicateLocation).getFileKey(), input.getDuplicate());
}
throw new IllegalArgumentException("Unsupported DuplicationLocation type " + duplicateLocation.getClass());
}
}
}
}

+ 47
- 12
server/sonar-server/src/main/java/org/sonar/server/computation/duplication/IntegrateCrossProjectDuplications.java View File

@@ -20,7 +20,9 @@

package org.sonar.server.computation.duplication;

import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
@@ -91,21 +93,23 @@ public class IntegrateCrossProjectDuplications {

private void addDuplication(Component file, CloneGroup duplication) {
ClonePart originPart = duplication.getOriginPart();
TextBlock originTextBlock = new TextBlock(originPart.getStartLine(), originPart.getEndLine());
int clonePartCount = 0;
for (ClonePart part : from(duplication.getCloneParts())
.filter(new DoesNotMatchSameComponentKey(originPart.getResourceId()))) {
clonePartCount++;
if (clonePartCount > MAX_CLONE_PART_PER_GROUP) {
LOGGER.warn("Too many duplication references on file {} for block at line {}. Keeping only the first {} references.",
file.getKey(), originPart.getStartLine(), MAX_CLONE_PART_PER_GROUP);
break;
}
duplicationRepository.addDuplication(file, originTextBlock, part.getResourceId(),
new TextBlock(part.getStartLine(), part.getEndLine()));
Iterable<Duplicate> duplicates = convertClonePartsToDuplicates(file, duplication);
if (!Iterables.isEmpty(duplicates)) {
duplicationRepository.add(
file,
new Duplication(new TextBlock(originPart.getStartLine(), originPart.getEndLine()), duplicates)
);
}
}

private static Iterable<Duplicate> convertClonePartsToDuplicates(final Component file, CloneGroup duplication) {
final ClonePart originPart = duplication.getOriginPart();
return from(duplication.getCloneParts())
.filter(new DoesNotMatchSameComponentKey(originPart.getResourceId()))
.filter(new DuplicateLimiter(file, originPart))
.transform(ClonePartToCrossProjectDuplicate.INSTANCE);
}

private NumberOfUnitsNotLessThan getNumberOfUnitsNotLessThan(String language) {
NumberOfUnitsNotLessThan numberOfUnitsNotLessThan = numberOfUnitsByLanguage.get(language);
if (numberOfUnitsNotLessThan == null) {
@@ -153,4 +157,35 @@ public class IntegrateCrossProjectDuplications {
}
}

private static class DuplicateLimiter implements Predicate<ClonePart> {
private final Component file;
private final ClonePart originPart;
private int counter = 0;

public DuplicateLimiter(Component file, ClonePart originPart) {
this.file = file;
this.originPart = originPart;
}

@Override
public boolean apply(@Nonnull ClonePart input) {
if (counter == MAX_CLONE_PART_PER_GROUP) {
LOGGER.warn("Too many duplication references on file {} for block at line {}. Keeping only the first {} references.",
file.getKey(), originPart.getStartLine(), MAX_CLONE_PART_PER_GROUP);
}
return counter++ <= MAX_CLONE_GROUP_PER_FILE;
}
}

private enum ClonePartToCrossProjectDuplicate implements Function<ClonePart, Duplicate> {
INSTANCE;

@Override
@Nonnull
public Duplicate apply(@Nonnull ClonePart input) {
return new CrossProjectDuplicate(
input.getResourceId(),
new TextBlock(input.getStartLine(), input.getEndLine()));
}
}
}

+ 6
- 7
server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java View File

@@ -24,11 +24,9 @@ import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.collect.Ordering;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.sonar.db.protobuf.DbFileSources;
@@ -37,12 +35,13 @@ import org.sonar.server.computation.duplication.InnerDuplicate;
import org.sonar.server.computation.duplication.TextBlock;

import static com.google.common.collect.FluentIterable.from;
import static com.google.common.collect.Iterables.size;

public class DuplicationLineReader implements LineReader {

private final Map<TextBlock, Integer> duplicatedTextBlockIndexByTextBlock;

public DuplicationLineReader(Set<Duplication> duplications) {
public DuplicationLineReader(Iterable<Duplication> duplications) {
this.duplicatedTextBlockIndexByTextBlock = createIndexOfDuplicatedTextBlocks(duplications);
}

@@ -70,7 +69,7 @@ public class DuplicationLineReader implements LineReader {
* index. It avoids false detections of changes in {@link DbFileSources.Line#getDuplicationList()}.
* </p>
*/
private static Map<TextBlock, Integer> createIndexOfDuplicatedTextBlocks(Collection<Duplication> duplications) {
private static Map<TextBlock, Integer> createIndexOfDuplicatedTextBlocks(Iterable<Duplication> duplications) {
List<TextBlock> duplicatedTextBlocks = extractAllDuplicatedTextBlocks(duplications);
Collections.sort(duplicatedTextBlocks);
return from(duplicatedTextBlocks)
@@ -84,10 +83,10 @@ public class DuplicationLineReader implements LineReader {
* The returned list is mutable on purpose because it will be sorted.
* </p>
*
* @see {@link #createIndexOfDuplicatedTextBlocks(Collection)}
* @see {@link #createIndexOfDuplicatedTextBlocks(Iterable)}
*/
private static List<TextBlock> extractAllDuplicatedTextBlocks(Collection<Duplication> duplications) {
List<TextBlock> duplicatedBlock = new ArrayList<>(duplications.size());
private static List<TextBlock> extractAllDuplicatedTextBlocks(Iterable<Duplication> duplications) {
List<TextBlock> duplicatedBlock = new ArrayList<>(size(duplications));
for (Duplication duplication : duplications) {
duplicatedBlock.add(duplication.getOriginal());
for (InnerDuplicate duplicate : from(duplication.getDuplicates()).filter(InnerDuplicate.class)) {

+ 3
- 3
server/sonar-server/src/main/java/org/sonar/server/computation/step/DuplicationDataMeasuresStep.java View File

@@ -20,7 +20,6 @@

package org.sonar.server.computation.step;

import java.util.Set;
import org.apache.commons.lang.StringEscapeUtils;
import org.sonar.server.computation.component.Component;
import org.sonar.server.computation.component.CrawlerDepthLimit;
@@ -39,6 +38,7 @@ import org.sonar.server.computation.measure.MeasureRepository;
import org.sonar.server.computation.metric.Metric;
import org.sonar.server.computation.metric.MetricRepository;

import static com.google.common.collect.Iterables.isEmpty;
import static org.sonar.api.measures.CoreMetrics.DUPLICATIONS_DATA_KEY;
import static org.sonar.server.computation.component.ComponentVisitor.Order.PRE_ORDER;

@@ -75,8 +75,8 @@ public class DuplicationDataMeasuresStep implements ComputationStep {

@Override
public void visitFile(Component file) {
Set<Duplication> duplications = duplicationRepository.getDuplications(file);
if (!duplications.isEmpty()) {
Iterable<Duplication> duplications = duplicationRepository.getDuplications(file);
if (!isEmpty(duplications)) {
computeDuplications(file, duplications);
}
}

+ 3
- 2
server/sonar-server/src/main/java/org/sonar/server/computation/step/DuplicationMeasuresStep.java View File

@@ -45,6 +45,7 @@ import org.sonar.server.computation.metric.Metric;
import org.sonar.server.computation.metric.MetricRepository;

import static com.google.common.collect.FluentIterable.from;
import static com.google.common.collect.Iterables.isEmpty;
import static java.util.Objects.requireNonNull;
import static org.sonar.api.measures.CoreMetrics.COMMENT_LINES_KEY;
import static org.sonar.api.measures.CoreMetrics.DUPLICATED_BLOCKS_KEY;
@@ -123,9 +124,9 @@ public class DuplicationMeasuresStep implements ComputationStep {
}

private void initializeForFile(Component file) {
Set<Duplication> duplications = requireNonNull(this.duplicationRepository, "DuplicationRepository missing")
Iterable<Duplication> duplications = requireNonNull(this.duplicationRepository, "DuplicationRepository missing")
.getDuplications(file);
if (duplications.isEmpty()) {
if (isEmpty(duplications)) {
return;
}


+ 34
- 8
server/sonar-server/src/main/java/org/sonar/server/computation/step/LoadDuplicationsFromReportStep.java View File

@@ -19,6 +19,8 @@
*/
package org.sonar.server.computation.step;

import com.google.common.base.Function;
import javax.annotation.Nonnull;
import org.sonar.batch.protocol.output.BatchReport;
import org.sonar.core.util.CloseableIterator;
import org.sonar.server.computation.batch.BatchReportReader;
@@ -28,9 +30,15 @@ import org.sonar.server.computation.component.DepthTraversalTypeAwareCrawler;
import org.sonar.server.computation.component.TreeRootHolder;
import org.sonar.server.computation.component.TypeAwareVisitorAdapter;
import org.sonar.server.computation.duplication.DetailedTextBlock;
import org.sonar.server.computation.duplication.Duplicate;
import org.sonar.server.computation.duplication.Duplication;
import org.sonar.server.computation.duplication.DuplicationRepository;
import org.sonar.server.computation.duplication.InProjectDuplicate;
import org.sonar.server.computation.duplication.InnerDuplicate;
import org.sonar.server.computation.duplication.TextBlock;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.FluentIterable.from;
import static org.sonar.server.computation.component.ComponentVisitor.Order.POST_ORDER;

/**
@@ -72,14 +80,12 @@ public class LoadDuplicationsFromReportStep implements ComputationStep {
}

private void loadDuplications(Component file, BatchReport.Duplication duplication, int id) {
DetailedTextBlock original = convert(duplication.getOriginPosition(), id);
for (BatchReport.Duplicate duplicate : duplication.getDuplicateList()) {
if (duplicate.hasOtherFileRef()) {
duplicationRepository.addDuplication(file, original, treeRootHolder.getComponentByRef(duplicate.getOtherFileRef()), convert(duplicate.getRange()));
} else {
duplicationRepository.addDuplication(file, original, convert(duplicate.getRange()));
}
}
duplicationRepository.add(file,
new Duplication(
convert(duplication.getOriginPosition(), id),
from(duplication.getDuplicateList())
.transform(new BatchDuplicateToCeDuplicate(file))
));
}

private static TextBlock convert(BatchReport.TextRange textRange) {
@@ -89,4 +95,24 @@ public class LoadDuplicationsFromReportStep implements ComputationStep {
private static DetailedTextBlock convert(BatchReport.TextRange textRange, int id) {
return new DetailedTextBlock(id, textRange.getStartLine(), textRange.getEndLine());
}

private class BatchDuplicateToCeDuplicate implements Function<BatchReport.Duplicate, Duplicate> {
private final Component file;

private BatchDuplicateToCeDuplicate(Component file) {
this.file = file;
}

@Override
@Nonnull
public Duplicate apply(@Nonnull BatchReport.Duplicate input) {
if (input.hasOtherFileRef()) {
checkArgument(input.getOtherFileRef() != file.getReportAttributes().getRef(), "file and otherFile references can not be the same");
return new InProjectDuplicate(
treeRootHolder.getComponentByRef(input.getOtherFileRef()),
convert(input.getRange()));
}
return new InnerDuplicate(convert(input.getRange()));
}
}
}

+ 29
- 333
server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryImplTest.java View File

@@ -23,12 +23,10 @@ import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
import java.util.Arrays;
import java.util.Set;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.sonar.server.computation.batch.TreeRootHolderRule;
import org.sonar.server.computation.component.Component;
import org.sonar.server.computation.component.ReportComponent;
import org.sonar.server.util.WrapInSingleElementArray;
@@ -36,30 +34,20 @@ import org.sonar.server.util.WrapInSingleElementArray;
import static com.google.common.base.Predicates.equalTo;
import static com.google.common.base.Predicates.not;
import static com.google.common.collect.FluentIterable.from;
import static java.lang.String.format;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

@RunWith(DataProviderRunner.class)
public class DuplicationRepositoryImplTest {
@Rule
public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule();

private static final Component FILE_COMPONENT_1 = ReportComponent.builder(Component.Type.FILE, 1).build();
private static final Component FILE_COMPONENT_2 = ReportComponent.builder(Component.Type.FILE, 2).build();
private static final Component FILE_COMPONENT_3 = ReportComponent.builder(Component.Type.FILE, 3).build();
private static final TextBlock ORIGINAL_TEXTBLOCK = new TextBlock(1, 2);
private static final TextBlock COPY_OF_ORIGINAL_TEXTBLOCK = new TextBlock(1, 2);
private static final TextBlock DUPLICATE_TEXTBLOCK_1 = new TextBlock(15, 15);
private static final TextBlock DUPLICATE_TEXTBLOCK_2 = new TextBlock(15, 16);
private static final String FILE_KEY_1 = "1";
private static final String FILE_KEY_2 = "2";
private static final Duplication SOME_DUPLICATION = createDuplication(1, 2);

@Rule
public ExpectedException expectedException = ExpectedException.none();

private DuplicationRepository underTest = new DuplicationRepositoryImpl(treeRootHolder);
private DuplicationRepository underTest = new DuplicationRepositoryImpl();

@Test
public void getDuplications_throws_NPE_if_Component_argument_is_null() {
@@ -84,24 +72,18 @@ public class DuplicationRepositoryImplTest {
}

@Test
public void addDuplication_inner_throws_NPE_if_file_argument_is_null() {
public void add_throws_NPE_if_file_argument_is_null() {
expectFileArgumentNPE();

underTest.addDuplication(null, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_inner_throws_NPE_if_original_argument_is_null() {
expectOriginalArgumentNPE();

underTest.addDuplication(FILE_COMPONENT_1, null, DUPLICATE_TEXTBLOCK_1);
underTest.add(null, SOME_DUPLICATION);
}

@Test
public void addDuplication_inner_throws_NPE_if_duplicate_argument_is_null() {
expectDuplicateArgumentNPE();
public void addDuplication_inner_throws_NPE_if_duplication_argument_is_null() {
expectedException.expect(NullPointerException.class);
expectedException.expectMessage("duplication can not be null");

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, null);
underTest.add(FILE_COMPONENT_1, null);
}

@Test
@@ -111,317 +93,46 @@ public class DuplicationRepositoryImplTest {

Component component = mockComponentGetType(type);

underTest.addDuplication(component, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);
underTest.add(component, SOME_DUPLICATION);
}

@Test
public void addDuplication_inner_throws_IAE_if_original_and_duplicate_are_equal() {
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("original and duplicate TextBlocks can not be the same");
public void added_duplication_is_returned_as_is_by_getDuplications() {
underTest.add(FILE_COMPONENT_1, SOME_DUPLICATION);

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, COPY_OF_ORIGINAL_TEXTBLOCK);
}

@Test
public void addDuplication_inner_throws_IAE_if_duplication_already_exists() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);

expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(format(
"Inner duplicate %s is already associated to original %s in file %s",
DUPLICATE_TEXTBLOCK_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_1.getKey()));

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_inner_throws_IAE_if_reverse_duplication_already_exists() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);

expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(format(
"Inner duplicate %s is already associated to original %s in file %s",
ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1, FILE_COMPONENT_1.getKey()));

underTest.addDuplication(FILE_COMPONENT_1, DUPLICATE_TEXTBLOCK_1, ORIGINAL_TEXTBLOCK);
}

@Test
public void addDuplication_inner_throws_IAE_if_reverse_duplication_already_exists_and_duplicate_has_duplicates_of_its_own() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);
underTest.addDuplication(FILE_COMPONENT_1, DUPLICATE_TEXTBLOCK_1, DUPLICATE_TEXTBLOCK_2);

expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(format(
"Inner duplicate %s is already associated to original %s in file %s",
ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1, FILE_COMPONENT_1.getKey()));

underTest.addDuplication(FILE_COMPONENT_1, DUPLICATE_TEXTBLOCK_1, ORIGINAL_TEXTBLOCK);
}

@Test
public void addDuplication_inner_is_returned_by_getDuplications() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
Iterable<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new InnerDuplicate(DUPLICATE_TEXTBLOCK_1));
assertThat(duplications.iterator().next()).isSameAs(SOME_DUPLICATION);

assertNoDuplication(FILE_COMPONENT_2);
}

@Test
public void addDuplication_inner_called_multiple_times_populate_a_single_Duplication() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_2);
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1);
public void added_duplication_does_not_avoid_same_duplication_inserted_twice_but_only_one_is_returned() {
underTest.add(FILE_COMPONENT_1, SOME_DUPLICATION);
underTest.add(FILE_COMPONENT_1, SOME_DUPLICATION);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
Iterable<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new InnerDuplicate(DUPLICATE_TEXTBLOCK_1), new InnerDuplicate(DUPLICATE_TEXTBLOCK_2));
assertThat(duplications.iterator().next()).isSameAs(SOME_DUPLICATION);

assertNoDuplication(FILE_COMPONENT_2);
}

@Test
public void addDuplication_inProject_throws_NPE_if_file_argument_is_null() {
expectFileArgumentNPE();

underTest.addDuplication(null, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_inProject_throws_NPE_if_original_argument_is_null() {
expectOriginalArgumentNPE();
public void added_duplications_are_returned_in_any_order_and_associated_to_the_right_file() {
underTest.add(FILE_COMPONENT_1, SOME_DUPLICATION);
underTest.add(FILE_COMPONENT_1, createDuplication(2, 4));
underTest.add(FILE_COMPONENT_1, createDuplication(2, 3));
underTest.add(FILE_COMPONENT_2, createDuplication(2, 3));
underTest.add(FILE_COMPONENT_2, createDuplication(1, 2));

underTest.addDuplication(FILE_COMPONENT_1, null, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1);
assertThat(underTest.getDuplications(FILE_COMPONENT_1)).containsOnly(SOME_DUPLICATION, createDuplication(2, 3), createDuplication(2, 4));
assertThat(underTest.getDuplications(FILE_COMPONENT_2)).containsOnly(createDuplication(1, 2), createDuplication(2, 3));
}

@Test
public void addDuplication_inProject_throws_NPE_if_otherFile_argument_is_null() {
expectedException.expect(NullPointerException.class);
expectedException.expectMessage("otherFile can not be null");

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, (Component) null, DUPLICATE_TEXTBLOCK_1);
}

@Test
@UseDataProvider("allComponentTypesButFile")
public void addDuplication_inProject_throws_NPE_if_otherFile_type_is_not_FILE(Component.Type type) {
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("type of otherFile argument must be FILE");

Component component = mockComponentGetType(type);

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, component, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_inProject_throws_NPE_if_duplicate_argument_is_null() {
expectDuplicateArgumentNPE();

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, null);
}

@Test
@UseDataProvider("allComponentTypesButFile")
public void addDuplication_inProject_throws_NPE_if_file_type_is_not_FILE(Component.Type type) {
expectFileTypeIAE();

Component component = mockComponentGetType(type);

underTest.addDuplication(component, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_inProject_throws_NPE_if_file_and_otherFile_are_the_same() {
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("file and otherFile Components can not be the same");

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_1, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_inProject_throws_IAE_if_duplication_already_exists() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1);

expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(format(
"In-project duplicate %s in file %s is already associated to original %s in file %s",
DUPLICATE_TEXTBLOCK_1, FILE_COMPONENT_2.getKey(), ORIGINAL_TEXTBLOCK, FILE_COMPONENT_1.getKey()));

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_inProject_is_returned_by_getDuplications() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new InProjectDuplicate(FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1));

assertNoDuplication(FILE_COMPONENT_2);
}

@Test
public void addDuplication_inProject_called_multiple_times_populate_a_single_Duplication() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_2);
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new InProjectDuplicate(FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1), new InProjectDuplicate(FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_2));

assertNoDuplication(FILE_COMPONENT_2);
}

@Test
public void addDuplication_inProject_called_multiple_times_with_different_components_populate_a_single_Duplication() {
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_2);
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_3, DUPLICATE_TEXTBLOCK_2);
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_3, DUPLICATE_TEXTBLOCK_1);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new InProjectDuplicate(FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_2), new InProjectDuplicate(FILE_COMPONENT_3, DUPLICATE_TEXTBLOCK_1), new InProjectDuplicate(FILE_COMPONENT_3,
DUPLICATE_TEXTBLOCK_2));

assertNoDuplication(FILE_COMPONENT_2);
assertNoDuplication(FILE_COMPONENT_3);
}

@Test
public void addDuplication_crossProject_throws_NPE_if_file_argument_is_null() {
expectFileArgumentNPE();

underTest.addDuplication(null, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_crossProject_throws_NPE_if_original_argument_is_null() {
expectOriginalArgumentNPE();

underTest.addDuplication(FILE_COMPONENT_1, null, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_crossProject_throws_NPE_if_otherFileKey_argument_is_null() {
expectedException.expect(NullPointerException.class);
expectedException.expectMessage("otherFileKey can not be null");

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, (String) null, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_crossProject_throws_NPE_if_duplicate_argument_is_null() {
expectDuplicateArgumentNPE();

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, null);
}

@Test
public void addDuplication_crossProject_throws_IAE_if_otherFileKey_is_key_of_Component_in_the_project() {
treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 999)
.addChildren(
FILE_COMPONENT_1, FILE_COMPONENT_2
)
.build());

expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(format("otherFileKey '%s' can not be the key of a Component in the project", FILE_COMPONENT_2.getKey()));

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2.getKey(), DUPLICATE_TEXTBLOCK_1);
}

@Test
@UseDataProvider("allComponentTypesButFile")
public void addDuplication_crossProject_throws_NPE_if_file_type_is_not_FILE(Component.Type type) {
expectFileTypeIAE();

Component component = mockComponentGetType(type);

underTest.addDuplication(component, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_crossProject_throws_IAE_if_duplication_already_exists() {
treeRootHolder.setRoot(FILE_COMPONENT_1);

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1);

expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage(format(
"Cross-project duplicate %s in file %s is already associated to original %s in file %s",
DUPLICATE_TEXTBLOCK_1, FILE_KEY_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_1.getKey()));

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1);
}

@Test
public void addDuplication_crossProject_is_returned_by_getDuplications() {
treeRootHolder.setRoot(FILE_COMPONENT_1);

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_1));

assertNoDuplication(FILE_COMPONENT_2);
}

@Test
public void addDuplication_crossProject_called_multiple_times_populate_a_single_Duplication() {
treeRootHolder.setRoot(FILE_COMPONENT_1);

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_2);
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_1), new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_2));

assertNoDuplication(FILE_COMPONENT_2);
}

@Test
public void addDuplication_crossProject_called_multiple_times_with_different_fileKeys_populate_a_single_Duplication() {
treeRootHolder.setRoot(FILE_COMPONENT_1);

underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_2);
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_2, DUPLICATE_TEXTBLOCK_2);
underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_2, DUPLICATE_TEXTBLOCK_1);

Set<Duplication> duplications = underTest.getDuplications(FILE_COMPONENT_1);
assertThat(duplications).hasSize(1);
assertDuplication(
duplications.iterator().next(),
ORIGINAL_TEXTBLOCK,
new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_2), new CrossProjectDuplicate(FILE_KEY_2, DUPLICATE_TEXTBLOCK_1), new CrossProjectDuplicate(FILE_KEY_2,
DUPLICATE_TEXTBLOCK_2));

assertNoDuplication(FILE_COMPONENT_2);
private static Duplication createDuplication(int originalLine, int duplicateLine) {
return new Duplication(new TextBlock(originalLine, originalLine), Arrays.<Duplicate>asList(new InnerDuplicate(new TextBlock(duplicateLine, duplicateLine))));
}

@DataProvider
@@ -432,11 +143,6 @@ public class DuplicationRepositoryImplTest {
.toArray(Object[].class);
}

private static void assertDuplication(Duplication duplication, TextBlock original, Duplicate... duplicates) {
assertThat(duplication.getOriginal()).isEqualTo(original);
assertThat(duplication.getDuplicates()).containsExactly(duplicates);
}

private void assertNoDuplication(Component component) {
assertThat(underTest.getDuplications(component)).isEmpty();
}
@@ -446,19 +152,9 @@ public class DuplicationRepositoryImplTest {
expectedException.expectMessage("file can not be null");
}

private void expectOriginalArgumentNPE() {
expectedException.expect(NullPointerException.class);
expectedException.expectMessage("original can not be null");
}

private void expectDuplicateArgumentNPE() {
expectedException.expect(NullPointerException.class);
expectedException.expectMessage("duplicate can not be null");
}

private void expectFileTypeIAE() {
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("type of file argument must be FILE");
expectedException.expectMessage("type of file must be FILE");
}

private Component mockComponentGetType(Component.Type type) {

+ 91
- 25
server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryRule.java View File

@@ -19,7 +19,12 @@
*/
package org.sonar.server.computation.duplication;

import java.util.Set;
import com.google.common.base.Function;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Multimap;
import java.util.Arrays;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import org.junit.rules.ExternalResource;
import org.sonar.server.computation.batch.TreeRootHolderRule;
import org.sonar.server.computation.component.Component;
@@ -27,78 +32,139 @@ import org.sonar.server.computation.component.ComponentProvider;
import org.sonar.server.computation.component.TreeRootHolder;
import org.sonar.server.computation.component.TreeRootHolderComponentProvider;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.FluentIterable.from;
import static java.util.Objects.requireNonNull;

public class DuplicationRepositoryRule extends ExternalResource implements DuplicationRepository {
private final TreeRootHolder treeRootHolder;
@CheckForNull
private final ComponentProvider componentProvider;
private DuplicationRepositoryImpl delegate;
private final Multimap<Component, TextBlock> componentRefsWithInnerDuplications = ArrayListMultimap.create();
private final Multimap<Component, TextBlock> componentRefsWithInProjectDuplications = ArrayListMultimap.create();
private final Multimap<Component, TextBlock> componentRefsWithCrossProjectDuplications = ArrayListMultimap.create();

private DuplicationRepositoryRule(TreeRootHolder treeRootHolder) {
this.treeRootHolder = treeRootHolder;
this.componentProvider = new TreeRootHolderComponentProvider(treeRootHolder);
}

public DuplicationRepositoryRule() {
this.componentProvider = null;
}

public static DuplicationRepositoryRule create(TreeRootHolderRule treeRootHolder) {
return new DuplicationRepositoryRule(treeRootHolder);
}

public static DuplicationRepositoryRule create() {
return new DuplicationRepositoryRule();
}

@Override
protected void before() throws Throwable {
this.delegate = new DuplicationRepositoryImpl(treeRootHolder);
this.delegate = new DuplicationRepositoryImpl();
}

@Override
protected void after() {
this.componentProvider.reset();
if (this.componentProvider != null) {
this.componentProvider.reset();
}
this.componentRefsWithInnerDuplications.clear();
this.componentRefsWithInProjectDuplications.clear();
this.componentRefsWithCrossProjectDuplications.clear();
this.delegate = null;
}

public Set<Duplication> getDuplications(int fileRef) {
componentProvider.ensureInitialized();
public Iterable<Duplication> getDuplications(int fileRef) {
ensureComponentProviderInitialized();

return delegate.getDuplications(componentProvider.getByRef(fileRef));
}

public DuplicationRepositoryRule addDuplication(int fileRef, TextBlock original, TextBlock duplicate) {
componentProvider.ensureInitialized();
public void add(int fileRef, Duplication duplication) {
ensureComponentProviderInitialized();

delegate.add(componentProvider.getByRef(fileRef), duplication);
}

public DuplicationRepositoryRule addDuplication(int fileRef, TextBlock original, TextBlock... duplicates) {
ensureComponentProviderInitialized();
Component component = componentProvider.getByRef(fileRef);
checkArgument(!componentRefsWithInnerDuplications.containsEntry(component, original), "Inner duplications for file %s and original %s already set", fileRef, original);
checkArgument(!componentRefsWithInProjectDuplications.containsEntry(component, original), "InProject duplications for file %s and original %s already set. Use add(int, Duplication) instead", fileRef, original);

delegate.addDuplication(componentProvider.getByRef(fileRef), original, duplicate);
componentRefsWithInnerDuplications.put(component, original);
delegate.add(
component,
new Duplication(
original,
from(Arrays.asList(duplicates)).transform(TextBlockToInnerDuplicate.INSTANCE))
);

return this;
}

public DuplicationRepositoryRule addDuplication(int fileRef, TextBlock original, int otherFileRef, TextBlock duplicate) {
componentProvider.ensureInitialized();

delegate.addDuplication(componentProvider.getByRef(fileRef), original, componentProvider.getByRef(otherFileRef), duplicate);
ensureComponentProviderInitialized();
Component component = componentProvider.getByRef(fileRef);
checkArgument(!componentRefsWithInProjectDuplications.containsEntry(component, original), "InProject duplications for file %s and original %s already set", fileRef, original);
checkArgument(!componentRefsWithInnerDuplications.containsEntry(component, original), "Inner duplications for file %s and original %s already set. Use add(int, Duplication) instead", fileRef, original);

componentRefsWithInProjectDuplications.put(component, original);
delegate.add(component,
new Duplication(
original,
Arrays.<Duplicate>asList(new InProjectDuplicate(componentProvider.getByRef(otherFileRef), duplicate))
)
);

return this;
}

public DuplicationRepositoryRule addDuplication(int fileRef, TextBlock original, String otherFileKey, TextBlock duplicate) {
componentProvider.ensureInitialized();

delegate.addDuplication(componentProvider.getByRef(fileRef), original, otherFileKey, duplicate);
ensureComponentProviderInitialized();
Component component = componentProvider.getByRef(fileRef);
checkArgument(!componentRefsWithCrossProjectDuplications.containsEntry(component, original), "CrossProject duplications for file %s and original %s already set", fileRef);

componentRefsWithCrossProjectDuplications.put(component, original);
delegate.add(componentProvider.getByRef(fileRef),
new Duplication(
original,
Arrays.<Duplicate>asList(new CrossProjectDuplicate(otherFileKey, duplicate))
)
);

return this;
}

@Override
public Set<Duplication> getDuplications(Component file) {
public Iterable<Duplication> getDuplications(Component file) {
return delegate.getDuplications(file);
}

@Override
public void addDuplication(Component file, TextBlock original, TextBlock duplicate) {
delegate.addDuplication(file, original, duplicate);
public void add(Component file, Duplication duplication) {
TextBlock original = duplication.getOriginal();
checkArgument(!componentRefsWithInnerDuplications.containsEntry(file, original), "Inner duplications for file %s and original %s already set", file, original);
checkArgument(!componentRefsWithInProjectDuplications.containsEntry(file, original), "InProject duplications for file %s and original %s already set", file, original);
checkArgument(!componentRefsWithCrossProjectDuplications.containsEntry(file, original), "CrossProject duplications for file %s and original %s already set", file, original);

delegate.add(file, duplication);
}

@Override
public void addDuplication(Component file, TextBlock original, Component otherFile, TextBlock duplicate) {
delegate.addDuplication(file, original, otherFile, duplicate);
private void ensureComponentProviderInitialized() {
requireNonNull(this.componentProvider, "Methods with component reference can not be used unless a TreeRootHolder has been provided when instantiating the rule");
this.componentProvider.ensureInitialized();
}

@Override
public void addDuplication(Component file, TextBlock original, String otherFileKey, TextBlock duplicate) {
delegate.addDuplication(file, original, otherFileKey, duplicate);
private enum TextBlockToInnerDuplicate implements Function<TextBlock, Duplicate> {
INSTANCE;

@Override
@Nonnull
public Duplicate apply(@Nonnull TextBlock input) {
return new InnerDuplicate(input);
}
}
}

+ 30
- 34
server/sonar-server/src/test/java/org/sonar/server/computation/duplication/IntegrateCrossProjectDuplicationsTest.java View File

@@ -21,6 +21,7 @@
package org.sonar.server.computation.duplication;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import org.junit.Rule;
@@ -38,13 +39,6 @@ import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.sonar.server.computation.component.Component.Type.FILE;
import static org.sonar.server.computation.component.ReportComponent.builder;

@@ -52,6 +46,8 @@ public class IntegrateCrossProjectDuplicationsTest {

@Rule
public LogTester logTester = new LogTester();
@Rule
public DuplicationRepositoryRule duplicationRepository = DuplicationRepositoryRule.create();

static final String XOO_LANGUAGE = "xoo";

@@ -65,8 +61,6 @@ public class IntegrateCrossProjectDuplicationsTest {

Settings settings = new Settings();

DuplicationRepository duplicationRepository = mock(DuplicationRepository.class);

IntegrateCrossProjectDuplications underTest = new IntegrateCrossProjectDuplications(settings, duplicationRepository);

@Test
@@ -106,11 +100,9 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(ORIGIN_FILE, originBlocks, duplicatedBlocks);

verify(duplicationRepository).addDuplication(
ORIGIN_FILE,
new TextBlock(30, 45),
OTHER_FILE_KEY,
new TextBlock(40, 55)
assertThat(duplicationRepository.getDuplications(ORIGIN_FILE))
.containsExactly(
crossProjectDuplication(new TextBlock(30, 45), OTHER_FILE_KEY, new TextBlock(40, 55))
);
}

@@ -140,11 +132,9 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(ORIGIN_FILE, originBlocks, duplicatedBlocks);

verify(duplicationRepository).addDuplication(
ORIGIN_FILE,
new TextBlock(30, 45),
OTHER_FILE_KEY,
new TextBlock(40, 55)
assertThat(duplicationRepository.getDuplications(ORIGIN_FILE))
.containsExactly(
crossProjectDuplication(new TextBlock(30, 45), OTHER_FILE_KEY, new TextBlock(40, 55))
);
}

@@ -181,7 +171,7 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(ORIGIN_FILE, originBlocks, duplicatedBlocks);

verifyNoMoreInteractions(duplicationRepository);
assertNoDuplicationAdded(ORIGIN_FILE);
}

@Test
@@ -210,7 +200,7 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(ORIGIN_FILE, originBlocks, duplicatedBlocks);

verifyNoMoreInteractions(duplicationRepository);
assertNoDuplicationAdded(ORIGIN_FILE);
}

@Test
@@ -229,7 +219,7 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(ORIGIN_FILE, originBlocks, Collections.<Block>emptyList());

verifyNoMoreInteractions(duplicationRepository);
assertNoDuplicationAdded(ORIGIN_FILE);
}

@Test
@@ -261,11 +251,9 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(javaFile, originBlocks, duplicatedBlocks);

verify(duplicationRepository).addDuplication(
ORIGIN_FILE,
new TextBlock(30, 45),
OTHER_FILE_KEY,
new TextBlock(40, 55)
assertThat(duplicationRepository.getDuplications(ORIGIN_FILE))
.containsExactly(
crossProjectDuplication(new TextBlock(30, 45), OTHER_FILE_KEY, new TextBlock(40, 55))
);
}

@@ -294,11 +282,9 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(ORIGIN_FILE, originBlocks, duplicatedBlocks);

verify(duplicationRepository).addDuplication(
ORIGIN_FILE,
new TextBlock(30, 45),
OTHER_FILE_KEY,
new TextBlock(40, 55)
assertThat(duplicationRepository.getDuplications(ORIGIN_FILE))
.containsExactly(
crossProjectDuplication(new TextBlock(30, 45), OTHER_FILE_KEY, new TextBlock(40, 55))
);
}

@@ -328,7 +314,9 @@ public class IntegrateCrossProjectDuplicationsTest {

assertThat(logTester.logs(LoggerLevel.WARN)).containsOnly(
"Too many duplication references on file " + ORIGIN_FILE_KEY + " for block at line 30. Keeping only the first 100 references.");
verify(duplicationRepository, times(100)).addDuplication(eq(ORIGIN_FILE), any(TextBlock.class), anyString(), any(TextBlock.class));
Iterable<Duplication> duplications = duplicationRepository.getDuplications(ORIGIN_FILE);
assertThat(duplications).hasSize(1);
assertThat(duplications.iterator().next().getDuplicates()).hasSize(100);
}

@Test
@@ -359,8 +347,16 @@ public class IntegrateCrossProjectDuplicationsTest {

underTest.computeCpd(ORIGIN_FILE, originBlocks, duplicatedBlocks);

verify(duplicationRepository, times(100)).addDuplication(eq(ORIGIN_FILE), any(TextBlock.class), anyString(), any(TextBlock.class));
assertThat(duplicationRepository.getDuplications(ORIGIN_FILE)).hasSize(100);
assertThat(logTester.logs(LoggerLevel.WARN)).containsOnly("Too many duplication groups on file " + ORIGIN_FILE_KEY + ". Keeping only the first 100 groups.");
}

private static Duplication crossProjectDuplication(TextBlock original, String otherFileKey, TextBlock duplicate) {
return new Duplication(original, Arrays.<Duplicate>asList(new CrossProjectDuplicate(otherFileKey, duplicate)));
}

private void assertNoDuplicationAdded(Component file) {
assertThat(duplicationRepository.getDuplications(file)).isEmpty();
}

}

+ 1
- 1
server/sonar-server/src/test/java/org/sonar/server/computation/step/LoadDuplicationsFromReportStepTest.java View File

@@ -179,7 +179,7 @@ public class LoadDuplicationsFromReportStepTest {
reportReader.putDuplications(FILE_1_REF, createDuplication(singleLineTextRange(line), createInProjectDuplicate(FILE_1_REF, line + 1)));

expectedException.expect(VisitException.class);
expectedException.expectCause(hasType(IllegalArgumentException.class).andMessage("file and otherFile Components can not be the same"));
expectedException.expectCause(hasType(IllegalArgumentException.class).andMessage("file and otherFile references can not be the same"));

underTest.execute();
}

+ 8
- 1
server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java View File

@@ -20,6 +20,7 @@

package org.sonar.server.computation.step;

import java.util.Arrays;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -38,7 +39,10 @@ import org.sonar.server.computation.batch.BatchReportReaderRule;
import org.sonar.server.computation.batch.TreeRootHolderRule;
import org.sonar.server.computation.component.Component;
import org.sonar.server.computation.component.ReportComponent;
import org.sonar.server.computation.duplication.Duplicate;
import org.sonar.server.computation.duplication.Duplication;
import org.sonar.server.computation.duplication.DuplicationRepositoryRule;
import org.sonar.server.computation.duplication.InnerDuplicate;
import org.sonar.server.computation.duplication.TextBlock;
import org.sonar.server.computation.scm.Changeset;
import org.sonar.server.computation.scm.ScmInfoRepositoryRule;
@@ -242,7 +246,10 @@ public class PersistFileSourcesStepTest extends BaseStepTest {
public void persist_duplication() {
initBasicReport(1);

duplicationRepository.addDuplication(FILE_REF, new TextBlock(1, 2), new TextBlock(3, 4));
duplicationRepository.add(
FILE_REF,
new Duplication(new TextBlock(1, 2), Arrays.<Duplicate>asList(new InnerDuplicate(new TextBlock(3, 4))))
);

underTest.execute();


+ 5
- 6
server/sonar-server/src/test/java/org/sonar/server/computation/step/ReportDuplicationMeasuresStepTest.java View File

@@ -107,9 +107,7 @@ public class ReportDuplicationMeasuresStepTest {
@Test
public void compute_duplicated_blocks_one_for_original_one_for_each_InnerDuplicate() {
TextBlock original = new TextBlock(1, 1);
duplicationRepository.addDuplication(FILE_1_REF, original, new TextBlock(2, 2));
duplicationRepository.addDuplication(FILE_1_REF, original, new TextBlock(3, 3));
duplicationRepository.addDuplication(FILE_1_REF, original, new TextBlock(2, 3));
duplicationRepository.addDuplication(FILE_1_REF, original, new TextBlock(2, 2), new TextBlock(3, 3), new TextBlock(2, 3));

underTest.execute();

@@ -213,8 +211,7 @@ public class ReportDuplicationMeasuresStepTest {
@Test
public void compute_duplicated_lines_counts_lines_from_original_and_InnerDuplicate_only_once() {
TextBlock original = new TextBlock(1, 12);
duplicationRepository.addDuplication(FILE_1_REF, original, new TextBlock(10, 11));
duplicationRepository.addDuplication(FILE_1_REF, original, new TextBlock(11, 15));
duplicationRepository.addDuplication(FILE_1_REF, original, new TextBlock(10, 11), new TextBlock(11, 15));
duplicationRepository.addDuplication(FILE_1_REF, new TextBlock(2, 2), new TextBlock(96, 96));

underTest.execute();
@@ -454,9 +451,11 @@ public class ReportDuplicationMeasuresStepTest {
private void addDuplicatedBlock(int fileRef, int blockCount) {
checkArgument(blockCount > 1, "BlockCount can not be less than 2");
TextBlock original = new TextBlock(1, 1);
TextBlock[] duplicates = new TextBlock[blockCount - 1];
for (int i = 10; i < blockCount + 9; i++) {
duplicationRepository.addDuplication(fileRef, original, new TextBlock(i, i));
duplicates[i - 10] = new TextBlock(i, i);
}
duplicationRepository.addDuplication(fileRef, original, duplicates);
}

private void addRawMeasure(int componentRef, String metricKey, int value) {

Loading…
Cancel
Save