SONAR-15091 Compute engine fails with NPE if an external issue has an empty message

3 files changed, 154 insertions, 5 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java
index 429017f1f05..220410a4daa 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java
@@ -74,6 +74,9 @@ public class AbstractTracker<RAW extends Trackable, BASE extends Trackable> {
       if (this == o) {
         return true;
       }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
       LineAndLineHashKey that = (LineAndLineHashKey) o;
       // start with most discriminant field
       return Objects.equals(line, that.line) && lineHash.equals(that.lineHash) && ruleKey.equals(that.ruleKey);
@@ -108,7 +111,7 @@ public class AbstractTracker<RAW extends Trackable, BASE extends Trackable> {
       }
       LineAndLineHashAndMessage that = (LineAndLineHashAndMessage) o;
       // start with most discriminant field
-      return Objects.equals(line, that.line) && lineHash.equals(that.lineHash) && message.equals(that.message) && ruleKey.equals(that.ruleKey);
+      return Objects.equals(line, that.line) && lineHash.equals(that.lineHash) && Objects.equals(message, that.message) && ruleKey.equals(that.ruleKey);
     }
 
     @Override
@@ -138,7 +141,7 @@ public class AbstractTracker<RAW extends Trackable, BASE extends Trackable> {
       }
       LineHashAndMessageKey that = (LineHashAndMessageKey) o;
       // start with most discriminant field
-      return lineHash.equals(that.lineHash) && message.equals(that.message) && ruleKey.equals(that.ruleKey);
+      return lineHash.equals(that.lineHash) && Objects.equals(message, that.message) && ruleKey.equals(that.ruleKey);
     }
 
     @Override
@@ -168,7 +171,7 @@ public class AbstractTracker<RAW extends Trackable, BASE extends Trackable> {
       }
       LineAndMessageKey that = (LineAndMessageKey) o;
       // start with most discriminant field
-      return Objects.equals(line, that.line) && message.equals(that.message) && ruleKey.equals(that.ruleKey);
+      return Objects.equals(line, that.line) && Objects.equals(message, that.message) && ruleKey.equals(that.ruleKey);
     }
 
     @Override
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/tracking/AbstractTrackerTest.java b/sonar-core/src/test/java/org/sonar/core/issue/tracking/AbstractTrackerTest.java
new file mode 100644
index 00000000000..726da91c4cc
--- /dev/null
+++ b/sonar-core/src/test/java/org/sonar/core/issue/tracking/AbstractTrackerTest.java
@@ -0,0 +1,131 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2021 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.core.issue.tracking;
+
+import java.util.function.Function;
+import org.junit.Test;
+import org.sonar.api.rule.RuleKey;
+import org.sonar.core.issue.tracking.AbstractTracker.LineAndLineHashAndMessage;
+import org.sonar.core.issue.tracking.AbstractTracker.LineAndLineHashKey;
+import org.sonar.core.issue.tracking.AbstractTracker.LineAndMessageKey;
+import org.sonar.core.issue.tracking.AbstractTracker.LineHashAndMessageKey;
+import org.sonar.core.issue.tracking.AbstractTracker.LineHashKey;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class AbstractTrackerTest {
+  private final Trackable base = trackable(RuleKey.of("r1", "r1"), 0, "m1", "hash1");
+  private final Trackable same = trackable(RuleKey.of("r1", "r1"), 0, "m1", "hash1");
+  private final Trackable diffRule = trackable(RuleKey.of("r1", "r2"), 0, "m1", "hash1");
+  private final Trackable diffMessage = trackable(RuleKey.of("r1", "r1"), 0, null, "hash1");
+  private final Trackable diffLineHash = trackable(RuleKey.of("r1", "r1"), 0, "m1", null);
+  private final Trackable diffLine = trackable(RuleKey.of("r1", "r1"), null, "m1", "hash1");
+
+  @Test
+  public void lineAndLineHashKey() {
+    Comparator comparator = new Comparator(LineAndLineHashKey::new);
+    comparator.sameEqualsAndHashcode(base, same);
+    comparator.sameEqualsAndHashcode(base, diffMessage);
+    comparator.differentEquals(base, diffRule);
+    comparator.differentEquals(base, diffLineHash);
+    comparator.differentEquals(base, diffLine);
+  }
+
+  @Test
+  public void lineAndLineHashAndMessage() {
+    Comparator comparator = new Comparator(LineAndLineHashAndMessage::new);
+    comparator.sameEqualsAndHashcode(base, same);
+    comparator.differentEquals(base, diffMessage);
+    comparator.differentEquals(base, diffRule);
+    comparator.differentEquals(base, diffLineHash);
+    comparator.differentEquals(base, diffLine);
+  }
+
+  @Test
+  public void lineHashAndMessageKey() {
+    Comparator comparator = new Comparator(LineHashAndMessageKey::new);
+    comparator.sameEqualsAndHashcode(base, same);
+    comparator.sameEqualsAndHashcode(base, diffLine);
+    comparator.differentEquals(base, diffMessage);
+    comparator.differentEquals(base, diffRule);
+    comparator.differentEquals(base, diffLineHash);
+  }
+
+  @Test
+  public void lineAndMessageKey() {
+    Comparator comparator = new Comparator(LineAndMessageKey::new);
+    comparator.sameEqualsAndHashcode(base, same);
+    comparator.sameEqualsAndHashcode(base, diffLineHash);
+    comparator.differentEquals(base, diffMessage);
+    comparator.differentEquals(base, diffRule);
+    comparator.differentEquals(base, diffLine);
+  }
+
+  @Test
+  public void lineHashKey() {
+    Comparator comparator = new Comparator(LineHashKey::new);
+    comparator.sameEqualsAndHashcode(base, same);
+    comparator.sameEqualsAndHashcode(base, diffLine);
+    comparator.sameEqualsAndHashcode(base, diffMessage);
+    comparator.differentEquals(base, diffRule);
+    comparator.differentEquals(base, diffLineHash);
+  }
+
+  private static Trackable trackable(RuleKey ruleKey, Integer line, String message, String lineHash) {
+    Trackable trackable = mock(Trackable.class);
+    when(trackable.getRuleKey()).thenReturn(ruleKey);
+    when(trackable.getLine()).thenReturn(line);
+    when(trackable.getMessage()).thenReturn(message);
+    when(trackable.getLineHash()).thenReturn(lineHash);
+    return trackable;
+  }
+
+  private static class Comparator {
+    private final Function<Trackable, AbstractTracker.SearchKey> searchKeyFactory;
+
+    private Comparator(Function<Trackable, AbstractTracker.SearchKey> searchKeyFactory) {
+      this.searchKeyFactory = searchKeyFactory;
+    }
+
+    private void sameEqualsAndHashcode(Trackable t1, Trackable t2) {
+      AbstractTracker.SearchKey k1 = searchKeyFactory.apply(t1);
+      AbstractTracker.SearchKey k2 = searchKeyFactory.apply(t2);
+
+      assertThat(k1).isEqualTo(k1);
+      assertThat(k1).isEqualTo(k2);
+      assertThat(k2).isEqualTo(k1);
+      assertThat(k1).hasSameHashCodeAs(k1);
+      assertThat(k1).hasSameHashCodeAs(k2);
+      assertThat(k2).hasSameHashCodeAs(k1);
+    }
+
+    private void differentEquals(Trackable t1, Trackable t2) {
+      AbstractTracker.SearchKey k1 = searchKeyFactory.apply(t1);
+      AbstractTracker.SearchKey k2 = searchKeyFactory.apply(t2);
+
+      assertThat(k1).isNotEqualTo(k2);
+      assertThat(k2).isNotEqualTo(k1);
+      assertThat(k1).isNotEqualTo(new Object());
+      assertThat(k1).isNotEqualTo(null);
+    }
+  }
+}
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java
index a34fc6f6de1..61d8511f519 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java
@@ -23,6 +23,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
 import java.util.List;
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.junit.Rule;
@@ -185,6 +186,19 @@ public class TrackerTest {
     assertThat(tracking.getUnmatchedBases()).containsOnly(base);
   }
 
+  // SONAR-15091
+  @Test
+  public void do_not_fail_if_message_is_null() {
+    FakeInput baseInput = new FakeInput("H1", "H2");
+    Issue base = baseInput.createIssueOnLine(1, RULE_UNUSED_LOCAL_VARIABLE, null);
+
+    FakeInput rawInput = new FakeInput("H1", "H2");
+    Issue raw = rawInput.createIssueOnLine(1, RULE_UNUSED_LOCAL_VARIABLE, null);
+
+    Tracking<Issue, Issue> tracking = tracker.trackNonClosed(rawInput, baseInput);
+    assertThat(tracking.baseFor(raw)).isNotNull();
+  }
+
   @Test
   public void do_not_fail_if_raw_line_does_not_exist() {
     FakeInput baseInput = new FakeInput();
@@ -456,7 +470,7 @@ public class TrackerTest {
     private final String status;
     private final Date updateDate;
 
-    Issue(@Nullable Integer line, String lineHash, RuleKey ruleKey, String message, String status, Date updateDate) {
+    Issue(@Nullable Integer line, String lineHash, RuleKey ruleKey, @Nullable String message, String status, Date updateDate) {
       this.line = line;
       this.lineHash = lineHash;
       this.ruleKey = ruleKey;
@@ -470,6 +484,7 @@ public class TrackerTest {
       return line;
     }
 
+    @CheckForNull
     @Override
     public String getMessage() {
       return message;
@@ -521,7 +536,7 @@ public class TrackerTest {
     /**
      * No line (line 0)
      */
-    Issue createIssue(RuleKey ruleKey, String message) {
+    Issue createIssue(RuleKey ruleKey, @Nullable String message) {
       Issue issue = new Issue(null, "", ruleKey, message, org.sonar.api.issue.Issue.STATUS_OPEN, new Date());
       issues.add(issue);
       return issue;