From 46cdf6128ed90235d8c15c29519c97adb90bc30f Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 9 Nov 2015 11:03:48 +0100 Subject: [PATCH] SONAR-6990 implement check otherFileKey is not a project key --- .../DuplicationRepositoryImpl.java | 40 ++-- .../DuplicationRepositoryImplTest.java | 211 ++++++++++-------- .../DuplicationRepositoryRule.java | 21 +- 3 files changed, 144 insertions(+), 128 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepositoryImpl.java index db48356be6a..49162a9518d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/duplication/DuplicationRepositoryImpl.java @@ -30,6 +30,7 @@ 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.collect.FluentIterable.from; @@ -40,16 +41,21 @@ import static java.util.Objects.requireNonNull; * In-memory implementation of {@link DuplicationRepository}. */ public class DuplicationRepositoryImpl implements DuplicationRepository { + private final TreeRootHolder treeRootHolder; private final Map duplicationsByComponentUuid = new HashMap<>(); + public DuplicationRepositoryImpl(TreeRootHolder treeRootHolder) { + this.treeRootHolder = treeRootHolder; + } + @Override public Set getDuplications(Component file) { checkFileComponentArgument(file); if (duplicationsByComponentUuid.containsKey(file.getUuid())) { return from(duplicationsByComponentUuid.get(file.getUuid()).getDuplicates()) - .transform(DuplicatesEntryToDuplication.INSTANCE) - .toSet(); + .transform(DuplicatesEntryToDuplication.INSTANCE) + .toSet(); } return Collections.emptySet(); } @@ -97,6 +103,7 @@ public class DuplicationRepositoryImpl implements DuplicationRepository { 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 = getDuplicates(file, original); if (duplicates.isPresent()) { @@ -118,25 +125,25 @@ public class DuplicationRepositoryImpl implements DuplicationRepository { private void checkDuplicationAlreadyExists(Optional 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()); + "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 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()); + "Inner duplicate %s is already associated to original %s in file %s", duplicate, original, file.getKey()); } } private static void checkDuplicationAlreadyExists(Optional 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()); + "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, 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()); + "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) { @@ -246,7 +253,7 @@ public class DuplicationRepositoryImpl implements DuplicationRepository { public Iterable getDuplicates() { return from(duplicatesByLocation.entrySet()) - .transformAndConcat(MapEntryToDuplicateWithLocation.INSTANCE); + .transformAndConcat(MapEntryToDuplicateWithLocation.INSTANCE); } public void add(TextBlock duplicate) { @@ -296,13 +303,13 @@ public class DuplicationRepositoryImpl implements DuplicationRepository { @Nonnull public Iterable apply(@Nonnull final Map.Entry> entry) { return from(entry.getValue()) - .transform(new Function() { - @Override - @Nonnull - public DuplicateWithLocation apply(@Nonnull TextBlock input) { - return new DuplicateWithLocation(entry.getKey(), input); - } - }); + .transform(new Function() { + @Override + @Nonnull + public DuplicateWithLocation apply(@Nonnull TextBlock input) { + return new DuplicateWithLocation(entry.getKey(), input); + } + }); } } } @@ -351,9 +358,8 @@ public class DuplicationRepositoryImpl implements DuplicationRepository { @Nonnull public Duplication apply(@Nonnull Map.Entry entry) { return new Duplication( - entry.getKey(), - from(entry.getValue().getDuplicates()).transform(DuplicateLocationEntryToDuplicate.INSTANCE) - ); + entry.getKey(), + from(entry.getValue().getDuplicates()).transform(DuplicateLocationEntryToDuplicate.INSTANCE)); } private enum DuplicateLocationEntryToDuplicate implements Function { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryImplTest.java index c86e2021d48..65cab2715b5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryImplTest.java @@ -24,11 +24,11 @@ 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.Ignore; 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; @@ -43,10 +43,12 @@ import static org.mockito.Mockito.when; @RunWith(DataProviderRunner.class) public class DuplicationRepositoryImplTest { + @Rule + public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); - private static final Component COMPONENT_1 = ReportComponent.builder(Component.Type.FILE, 1).build(); - private static final Component COMPONENT_2 = ReportComponent.builder(Component.Type.FILE, 2).build(); - private static final Component COMPONENT_3 = ReportComponent.builder(Component.Type.FILE, 3).build(); + 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); @@ -57,7 +59,7 @@ public class DuplicationRepositoryImplTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - private DuplicationRepository underTest = new DuplicationRepositoryImpl(); + private DuplicationRepository underTest = new DuplicationRepositoryImpl(treeRootHolder); @Test public void getDuplications_throws_NPE_if_Component_argument_is_null() { @@ -78,7 +80,7 @@ public class DuplicationRepositoryImplTest { @Test public void getDuplications_returns_empty_set_when_repository_is_empty() { - assertNoDuplication(COMPONENT_1); + assertNoDuplication(FILE_COMPONENT_1); } @Test @@ -92,14 +94,14 @@ public class DuplicationRepositoryImplTest { public void addDuplication_inner_throws_NPE_if_original_argument_is_null() { expectOriginalArgumentNPE(); - underTest.addDuplication(COMPONENT_1, null, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, null, DUPLICATE_TEXTBLOCK_1); } @Test public void addDuplication_inner_throws_NPE_if_duplicate_argument_is_null() { expectDuplicateArgumentNPE(); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, null); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, null); } @Test @@ -117,87 +119,87 @@ public class DuplicationRepositoryImplTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("original and duplicate TextBlocks can not be the same"); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COPY_OF_ORIGINAL_TEXTBLOCK); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, COPY_OF_ORIGINAL_TEXTBLOCK); } @Test public void addDuplication_inner_throws_IAE_if_duplication_already_exists() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); + 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, COMPONENT_1.getKey())); + DUPLICATE_TEXTBLOCK_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_1.getKey())); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); } @Test public void addDuplication_inner_throws_IAE_if_reverse_duplication_already_exists() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); + 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, COMPONENT_1.getKey())); + ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1, FILE_COMPONENT_1.getKey())); - underTest.addDuplication(COMPONENT_1, DUPLICATE_TEXTBLOCK_1, ORIGINAL_TEXTBLOCK); + 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(COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); - underTest.addDuplication(COMPONENT_1, DUPLICATE_TEXTBLOCK_1, DUPLICATE_TEXTBLOCK_2); + 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, COMPONENT_1.getKey())); + ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1, FILE_COMPONENT_1.getKey())); - underTest.addDuplication(COMPONENT_1, DUPLICATE_TEXTBLOCK_1, ORIGINAL_TEXTBLOCK); + underTest.addDuplication(FILE_COMPONENT_1, DUPLICATE_TEXTBLOCK_1, ORIGINAL_TEXTBLOCK); } @Test public void addDuplication_inner_is_returned_by_getDuplications() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); - Set duplications = underTest.getDuplications(COMPONENT_1); + Set duplications = underTest.getDuplications(FILE_COMPONENT_1); assertThat(duplications).hasSize(1); assertDuplication( - duplications.iterator().next(), - ORIGINAL_TEXTBLOCK, - new InnerDuplicate(DUPLICATE_TEXTBLOCK_1)); + duplications.iterator().next(), + ORIGINAL_TEXTBLOCK, + new InnerDuplicate(DUPLICATE_TEXTBLOCK_1)); - assertNoDuplication(COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_2); } @Test public void addDuplication_inner_called_multiple_times_populate_a_single_Duplication() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_2); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_2); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, DUPLICATE_TEXTBLOCK_1); - Set duplications = underTest.getDuplications(COMPONENT_1); + Set 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)); + duplications.iterator().next(), + ORIGINAL_TEXTBLOCK, + new InnerDuplicate(DUPLICATE_TEXTBLOCK_1), new InnerDuplicate(DUPLICATE_TEXTBLOCK_2)); - assertNoDuplication(COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_2); } @Test public void addDuplication_inProject_throws_NPE_if_file_argument_is_null() { expectFileArgumentNPE(); - underTest.addDuplication(null, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(null, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1); } @Test public void addDuplication_inProject_throws_NPE_if_original_argument_is_null() { expectOriginalArgumentNPE(); - underTest.addDuplication(COMPONENT_1, null, COMPONENT_2, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, null, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1); } @Test @@ -205,7 +207,7 @@ public class DuplicationRepositoryImplTest { expectedException.expect(NullPointerException.class); expectedException.expectMessage("otherFile can not be null"); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, (Component) null, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, (Component) null, DUPLICATE_TEXTBLOCK_1); } @Test @@ -216,14 +218,14 @@ public class DuplicationRepositoryImplTest { Component component = mockComponentGetType(type); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, component, DUPLICATE_TEXTBLOCK_1); + 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(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2, null); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, null); } @Test @@ -233,7 +235,7 @@ public class DuplicationRepositoryImplTest { Component component = mockComponentGetType(type); - underTest.addDuplication(component, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(component, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1); } @Test @@ -241,65 +243,66 @@ public class DuplicationRepositoryImplTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("file and otherFile Components can not be the same"); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_1, DUPLICATE_TEXTBLOCK_1); + 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(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_1); + 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, COMPONENT_2.getKey(), ORIGINAL_TEXTBLOCK, COMPONENT_1.getKey())); + DUPLICATE_TEXTBLOCK_1, FILE_COMPONENT_2.getKey(), ORIGINAL_TEXTBLOCK, FILE_COMPONENT_1.getKey())); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1); } @Test public void addDuplication_inProject_is_returned_by_getDuplications() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1); - Set duplications = underTest.getDuplications(COMPONENT_1); + Set duplications = underTest.getDuplications(FILE_COMPONENT_1); assertThat(duplications).hasSize(1); assertDuplication( - duplications.iterator().next(), - ORIGINAL_TEXTBLOCK, - new InProjectDuplicate(COMPONENT_2, DUPLICATE_TEXTBLOCK_1)); + duplications.iterator().next(), + ORIGINAL_TEXTBLOCK, + new InProjectDuplicate(FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1)); - assertNoDuplication(COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_2); } @Test public void addDuplication_inProject_called_multiple_times_populate_a_single_Duplication() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_2); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_1); + 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 duplications = underTest.getDuplications(COMPONENT_1); + Set duplications = underTest.getDuplications(FILE_COMPONENT_1); assertThat(duplications).hasSize(1); assertDuplication( - duplications.iterator().next(), - ORIGINAL_TEXTBLOCK, - new InProjectDuplicate(COMPONENT_2, DUPLICATE_TEXTBLOCK_1), new InProjectDuplicate(COMPONENT_2, DUPLICATE_TEXTBLOCK_2)); + duplications.iterator().next(), + ORIGINAL_TEXTBLOCK, + new InProjectDuplicate(FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_1), new InProjectDuplicate(FILE_COMPONENT_2, DUPLICATE_TEXTBLOCK_2)); - assertNoDuplication(COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_2); } @Test public void addDuplication_inProject_called_multiple_times_with_different_components_populate_a_single_Duplication() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2, DUPLICATE_TEXTBLOCK_2); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_3, DUPLICATE_TEXTBLOCK_2); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_3, DUPLICATE_TEXTBLOCK_1); + 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 duplications = underTest.getDuplications(COMPONENT_1); + Set duplications = underTest.getDuplications(FILE_COMPONENT_1); assertThat(duplications).hasSize(1); assertDuplication( - duplications.iterator().next(), - ORIGINAL_TEXTBLOCK, - new InProjectDuplicate(COMPONENT_2, DUPLICATE_TEXTBLOCK_2), new InProjectDuplicate(COMPONENT_3, DUPLICATE_TEXTBLOCK_1), new InProjectDuplicate(COMPONENT_3, DUPLICATE_TEXTBLOCK_2)); + 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(COMPONENT_2); - assertNoDuplication(COMPONENT_3); + assertNoDuplication(FILE_COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_3); } @Test @@ -313,7 +316,7 @@ public class DuplicationRepositoryImplTest { public void addDuplication_crossProject_throws_NPE_if_original_argument_is_null() { expectOriginalArgumentNPE(); - underTest.addDuplication(COMPONENT_1, null, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, null, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); } @Test @@ -321,23 +324,28 @@ public class DuplicationRepositoryImplTest { expectedException.expect(NullPointerException.class); expectedException.expectMessage("otherFileKey can not be null"); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, (String) null, DUPLICATE_TEXTBLOCK_1); + 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(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, null); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, null); } @Test - @Ignore 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("type of file argument must be FILE"); + expectedException.expectMessage(format("otherFileKey '%s' can not be the key of a Component in the project", FILE_COMPONENT_2.getKey())); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, COMPONENT_2.getKey(), DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_2.getKey(), DUPLICATE_TEXTBLOCK_1); } @Test @@ -352,67 +360,76 @@ public class DuplicationRepositoryImplTest { @Test public void addDuplication_crossProject_throws_IAE_if_duplication_already_exists() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); + 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, COMPONENT_1.getKey())); + DUPLICATE_TEXTBLOCK_1, FILE_KEY_1, ORIGINAL_TEXTBLOCK, FILE_COMPONENT_1.getKey())); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); } @Test public void addDuplication_crossProject_is_returned_by_getDuplications() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); + treeRootHolder.setRoot(FILE_COMPONENT_1); - Set duplications = underTest.getDuplications(COMPONENT_1); + underTest.addDuplication(FILE_COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); + + Set duplications = underTest.getDuplications(FILE_COMPONENT_1); assertThat(duplications).hasSize(1); assertDuplication( - duplications.iterator().next(), - ORIGINAL_TEXTBLOCK, - new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_1)); + duplications.iterator().next(), + ORIGINAL_TEXTBLOCK, + new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_1)); - assertNoDuplication(COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_2); } @Test public void addDuplication_crossProject_called_multiple_times_populate_a_single_Duplication() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_2); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_1); + 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 duplications = underTest.getDuplications(COMPONENT_1); + Set 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)); + duplications.iterator().next(), + ORIGINAL_TEXTBLOCK, + new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_1), new CrossProjectDuplicate(FILE_KEY_1, DUPLICATE_TEXTBLOCK_2)); - assertNoDuplication(COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_2); } @Test public void addDuplication_crossProject_called_multiple_times_with_different_fileKeys_populate_a_single_Duplication() { - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_1, DUPLICATE_TEXTBLOCK_2); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_2, DUPLICATE_TEXTBLOCK_2); - underTest.addDuplication(COMPONENT_1, ORIGINAL_TEXTBLOCK, FILE_KEY_2, DUPLICATE_TEXTBLOCK_1); + 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 duplications = underTest.getDuplications(COMPONENT_1); + Set 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)); + 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(COMPONENT_2); + assertNoDuplication(FILE_COMPONENT_2); } @DataProvider public static Object[][] allComponentTypesButFile() { return from(Arrays.asList(Component.Type.values())) - .filter(not(equalTo(Component.Type.FILE))) - .transform(WrapInSingleElementArray.INSTANCE) - .toArray(Object[].class); + .filter(not(equalTo(Component.Type.FILE))) + .transform(WrapInSingleElementArray.INSTANCE) + .toArray(Object[].class); } private static void assertDuplication(Duplication duplication, TextBlock original, Duplicate... duplicates) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryRule.java b/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryRule.java index 583fbd62410..9feb25dcbf8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryRule.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/duplication/DuplicationRepositoryRule.java @@ -24,33 +24,26 @@ import org.junit.rules.ExternalResource; import org.sonar.server.computation.batch.TreeRootHolderRule; import org.sonar.server.computation.component.Component; import org.sonar.server.computation.component.ComponentProvider; -import org.sonar.server.computation.component.NoComponentProvider; -import org.sonar.server.computation.component.TreeComponentProvider; +import org.sonar.server.computation.component.TreeRootHolder; import org.sonar.server.computation.component.TreeRootHolderComponentProvider; public class DuplicationRepositoryRule extends ExternalResource implements DuplicationRepository { + private final TreeRootHolder treeRootHolder; private final ComponentProvider componentProvider; private DuplicationRepositoryImpl delegate; - private DuplicationRepositoryRule(ComponentProvider componentProvider) { - this.componentProvider = componentProvider; - } - - public static DuplicationRepositoryRule standalone() { - return new DuplicationRepositoryRule(NoComponentProvider.INSTANCE); + private DuplicationRepositoryRule(TreeRootHolder treeRootHolder) { + this.treeRootHolder = treeRootHolder; + this.componentProvider = new TreeRootHolderComponentProvider(treeRootHolder); } public static DuplicationRepositoryRule create(TreeRootHolderRule treeRootHolder) { - return new DuplicationRepositoryRule(new TreeRootHolderComponentProvider(treeRootHolder)); - } - - public static DuplicationRepositoryRule create(Component root) { - return new DuplicationRepositoryRule(new TreeComponentProvider(root)); + return new DuplicationRepositoryRule(treeRootHolder); } @Override protected void before() throws Throwable { - this.delegate = new DuplicationRepositoryImpl(); + this.delegate = new DuplicationRepositoryImpl(treeRootHolder); } @Override -- 2.39.5