]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5286 Fix issue when duplications on removed file
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 26 Jun 2014 11:42:51 +0000 (13:42 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 26 Jun 2014 12:06:24 +0000 (14:06 +0200)
sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java
sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsWriter.java
sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsParserTest.java
sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsWriterTest.java
sonar-server/src/test/resources/org/sonar/server/duplication/ws/DuplicationsParserTest/duplication_on_removed_file.xml [new file with mode: 0644]

index ef74d034ed576aa37ba55f637935106ec632dfc8..bec04c2fa8c0d4cbe622614fae9f6df0b9252aa8 100644 (file)
@@ -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;
     }
index b3e8b8afe5613e035f33e31234edb102a8ae0775..1f08101134332460516deaee14c81cbda065d280 100644 (file)
@@ -63,11 +63,15 @@ public class DuplicationsWriter implements ServerComponent {
   }
 
   private void writeDuplication(Map<String, String> 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();
index edc35fc1bcc5adf86c1b2bba0d81257ad44853b1..1c9cec59858e9df751e7946c37bf833376587e8d 100644 (file)
@@ -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<DuplicationsParser.Block> blocks = parser.parse(currentFile, getData("duplication_on_removed_file.xml"), session);
+    assertThat(blocks).hasSize(1);
+
+    List<DuplicationsParser.Duplication> 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 {
index ec0338ed34709f767e9c172861c8c459e00faf54..918185364eefb2f1bb123274bb14c01b73742d23 100644 (file)
@@ -123,7 +123,7 @@ public class DuplicationsWriterTest {
     List<DuplicationsParser.Block> 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 (file)
index 0000000..4ccf8b8
--- /dev/null
@@ -0,0 +1,6 @@
+<duplications>
+  <g>
+    <b s="20" l="5" r="org.codehaus.sonar:sonar-plugin-api:src/main/java/com/sonar/orchestrator/util/CommandExecutor.java"/>
+    <b s="31" l="5" r="org.codehaus.sonar:sonar-plugin-api:src/main/java/com/sonar/orchestrator/util/RemovedFile.java"/>
+  </g>
+</duplications>