From f7bb6cc1a95b3c54d5371e2b37b97821648ff2b7 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 26 Jun 2014 13:42:51 +0200 Subject: [PATCH] SONAR-5286 Fix issue when duplications on removed file --- .../duplication/ws/DuplicationsParser.java | 13 ++++++++-- .../duplication/ws/DuplicationsWriter.java | 14 ++++++---- .../ws/DuplicationsParserTest.java | 26 ++++++++++++++++++- .../ws/DuplicationsWriterTest.java | 6 ++--- .../duplication_on_removed_file.xml | 6 +++++ 5 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 sonar-server/src/test/resources/org/sonar/server/duplication/ws/DuplicationsParserTest/duplication_on_removed_file.xml diff --git a/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java b/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java index ef74d034ed5..bec04c2fa8c 100644 --- a/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java +++ b/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java @@ -29,6 +29,7 @@ import org.sonar.core.component.ComponentDto; import org.sonar.core.persistence.DbSession; import org.sonar.server.component.persistence.ComponentDao; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; @@ -116,9 +117,13 @@ public class DuplicationsParser implements ServerComponent { if (d1 == null || d2 == null) { return -1; } - ComponentDto file1 = d1.file(); ComponentDto file2 = d2.file(); + + if (file1 == null || file2 == null) { + return -1; + } + if (file1.equals(d2.file())) { // if duplication on same file => order by starting line return d1.from().compareTo(d2.from()); @@ -161,12 +166,16 @@ public class DuplicationsParser implements ServerComponent { private final ComponentDto file; private final Integer from, size; - Duplication(ComponentDto file, Integer from, Integer size) { + Duplication(@Nullable ComponentDto file, Integer from, Integer size) { this.file = file; this.from = from; this.size = size; } + /** + * File can be null when duplication is linked on a removed file + */ + @CheckForNull ComponentDto file() { return file; } diff --git a/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsWriter.java b/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsWriter.java index b3e8b8afe56..1f081011343 100644 --- a/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsWriter.java +++ b/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsWriter.java @@ -63,11 +63,15 @@ public class DuplicationsWriter implements ServerComponent { } private void writeDuplication(Map refByComponentKey, DuplicationsParser.Duplication duplication, JsonWriter json) { - String componentKey = duplication.file().key(); - String ref = refByComponentKey.get(componentKey); - if (ref == null) { - ref = Integer.toString(refByComponentKey.size() + 1); - refByComponentKey.put(componentKey, ref); + String ref = null; + ComponentDto componentDto = duplication.file(); + if (componentDto != null) { + String componentKey = componentDto.key(); + ref = refByComponentKey.get(componentKey); + if (ref == null) { + ref = Integer.toString(refByComponentKey.size() + 1); + refByComponentKey.put(componentKey, ref); + } } json.beginObject(); diff --git a/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsParserTest.java b/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsParserTest.java index edc35fc1bcc..1c9cec59858 100644 --- a/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsParserTest.java +++ b/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsParserTest.java @@ -161,6 +161,26 @@ public class DuplicationsParserTest { assertThat(blocks.get(1).duplications().get(1).from()).isEqualTo(83); } + @Test + public void duplication_on_removed_file() throws Exception { + List blocks = parser.parse(currentFile, getData("duplication_on_removed_file.xml"), session); + assertThat(blocks).hasSize(1); + + List duplications = blocks.get(0).duplications(); + assertThat(duplications).hasSize(2); + + // Duplications on removed file + DuplicationsParser.Duplication duplication1 = duplications.get(0); + assertThat(duplication1.file()).isNull(); + assertThat(duplication1.from()).isEqualTo(31); + assertThat(duplication1.size()).isEqualTo(5); + + DuplicationsParser.Duplication duplication2 = duplications.get(1); + assertThat(duplication2.file()).isEqualTo(fileOnSameProject); + assertThat(duplication2.from()).isEqualTo(20); + assertThat(duplication2.size()).isEqualTo(5); + } + @Test public void compare_duplications() throws Exception { ComponentDto currentFile = new ComponentDto().setId(11L).setProjectId(1L); @@ -181,10 +201,14 @@ public class DuplicationsParserTest { assertThat(comparator.compare(new DuplicationsParser.Duplication(fileOnDifferentProject, 5, 2), new DuplicationsParser.Duplication(new ComponentDto().setId(30L).setProjectId(3L), 2, 2))).isEqualTo(1); - // At leat one null files + // With null duplications assertThat(comparator.compare(null, new DuplicationsParser.Duplication(fileOnSameProject, 2, 2))).isEqualTo(-1); assertThat(comparator.compare(new DuplicationsParser.Duplication(fileOnSameProject, 2, 2), null)).isEqualTo(-1); assertThat(comparator.compare(null, null)).isEqualTo(-1); + + // On some removed file + assertThat(comparator.compare(new DuplicationsParser.Duplication(currentFile, 2, 2), new DuplicationsParser.Duplication(null, 5, 2))).isEqualTo(-1); + assertThat(comparator.compare(new DuplicationsParser.Duplication(null, 2, 2), new DuplicationsParser.Duplication(currentFile, 5, 2))).isEqualTo(-1); } private String getData(String file) throws IOException { diff --git a/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsWriterTest.java b/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsWriterTest.java index ec0338ed347..918185364ee 100644 --- a/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsWriterTest.java +++ b/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsWriterTest.java @@ -123,7 +123,7 @@ public class DuplicationsWriterTest { List blocks = newArrayList(); blocks.add(new DuplicationsParser.Block(newArrayList( new DuplicationsParser.Duplication(file1, 57, 12), - new DuplicationsParser.Duplication(file2, 73, 12) + new DuplicationsParser.Duplication(null, 73, 12) ))); test(blocks, @@ -135,7 +135,7 @@ public class DuplicationsWriterTest { " \"from\": 57, \"size\": 12, \"_ref\": \"1\"\n" + " },\n" + " {\n" + - " \"from\": 73, \"size\": 12, \"_ref\": \"2\"\n" + + " \"from\": 73, \"size\": 12\n" + " }\n" + " ]\n" + " }," + @@ -150,7 +150,7 @@ public class DuplicationsWriterTest { "}" ); - verify(componentDao, times(2)).getNullableByKey(eq(session), anyString()); + verify(componentDao, times(1)).getNullableByKey(eq(session), anyString()); verify(componentDao, times(1)).getById(anyLong(), eq(session)); } diff --git a/sonar-server/src/test/resources/org/sonar/server/duplication/ws/DuplicationsParserTest/duplication_on_removed_file.xml b/sonar-server/src/test/resources/org/sonar/server/duplication/ws/DuplicationsParserTest/duplication_on_removed_file.xml new file mode 100644 index 00000000000..4ccf8b84d04 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/duplication/ws/DuplicationsParserTest/duplication_on_removed_file.xml @@ -0,0 +1,6 @@ + + + + + + -- 2.39.5