SONAR-6517 apply second feedback

3 files changed, 61 insertions, 17 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/platform/PluginInfo.java b/sonar-core/src/main/java/org/sonar/core/platform/PluginInfo.java
index 317ced0ba21..66e1942ee58 100644
--- a/sonar-core/src/main/java/org/sonar/core/platform/PluginInfo.java
+++ b/sonar-core/src/main/java/org/sonar/core/platform/PluginInfo.java
@@ -24,6 +24,8 @@ import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ComparisonChain;
+import com.google.common.collect.Ordering;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.updatecenter.common.PluginManifest;
 import org.sonar.updatecenter.common.Version;
@@ -87,7 +89,7 @@ public class PluginInfo implements Comparable<PluginInfo> {
     }
   }
 
-  private String key;
+  private final String key;
   private String name;
 
   @CheckForNull
@@ -133,6 +135,7 @@ public class PluginInfo implements Comparable<PluginInfo> {
   private final Set<RequiredPlugin> requiredPlugins = new HashSet<>();
 
   public PluginInfo(String key) {
+    Preconditions.checkNotNull(key);
     this.key = key;
     this.name = key;
   }
@@ -143,7 +146,7 @@ public class PluginInfo implements Comparable<PluginInfo> {
   }
 
   @CheckForNull
-  public File getJarFile2() {
+  public File getJarFile() {
     return jarFile;
   }
 
@@ -327,12 +330,34 @@ public class PluginInfo implements Comparable<PluginInfo> {
   }
 
   @Override
-  public int compareTo(PluginInfo other) {
-    int cmp = name.compareTo(other.name);
-    if (cmp != 0) {
-      return cmp;
+  public boolean equals(@Nullable Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    PluginInfo info = (PluginInfo) o;
+    if (!key.equals(info.key)) {
+      return false;
     }
-    return version.compareTo(other.version);
+    return !(version != null ? !version.equals(info.version) : info.version != null);
+
+  }
+
+  @Override
+  public int hashCode() {
+    int result = key.hashCode();
+    result = 31 * result + (version != null ? version.hashCode() : 0);
+    return result;
+  }
+
+  @Override
+  public int compareTo(PluginInfo that) {
+    return ComparisonChain.start()
+      .compare(this.name, that.name)
+      .compare(this.version, that.version, Ordering.natural().nullsFirst())
+      .result();
   }
 
   public static PluginInfo create(File jarFile) {
@@ -386,7 +411,7 @@ public class PluginInfo implements Comparable<PluginInfo> {
     }
   }
 
-  public static Function<File, PluginInfo>  jarToPluginInfo() {
+  public static Function<File, PluginInfo> jarToPluginInfo() {
     return JarToPluginInfo.INSTANCE;
   }
 }
diff --git a/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java b/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java
index b758f12d423..7afda907021 100644
--- a/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java
+++ b/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java
@@ -145,7 +145,7 @@ public class PluginLoader implements BatchComponent, ServerComponent {
         } catch (UnsupportedClassVersionError e) {
           throw new IllegalStateException(String.format("The plugin [%s] does not support Java %s",
             pluginKey, SystemUtils.JAVA_VERSION_TRIMMED), e);
-        } catch (Exception e) {
+        } catch (ClassNotFoundException|IllegalAccessException|InstantiationException e) {
           throw new IllegalStateException(String.format(
             "Fail to instantiate class [%s] of plugin [%s]", mainClass, pluginKey), e);
         }
diff --git a/sonar-core/src/test/java/org/sonar/core/platform/PluginInfoTest.java b/sonar-core/src/test/java/org/sonar/core/platform/PluginInfoTest.java
index 198f9e0c9f1..fac38034eaf 100644
--- a/sonar-core/src/test/java/org/sonar/core/platform/PluginInfoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/platform/PluginInfoTest.java
@@ -31,7 +31,6 @@ import javax.annotation.Nullable;
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
 import static com.google.common.collect.Ordering.natural;
@@ -58,19 +57,39 @@ public class PluginInfoTest {
   }
 
   @Test
-  public void test_comparison() {
+   public void test_comparison() {
     PluginInfo java1 = new PluginInfo("java").setVersion(Version.create("1.0"));
     PluginInfo java2 = new PluginInfo("java").setVersion(Version.create("2.0"));
+    PluginInfo javaNoVersion = new PluginInfo("java");
     PluginInfo cobol = new PluginInfo("cobol").setVersion(Version.create("1.0"));
     PluginInfo noVersion = new PluginInfo("noVersion");
-    List<PluginInfo> plugins = Arrays.asList(java1, java2, cobol, noVersion);
-    Collections.shuffle(plugins);
+    List<PluginInfo> plugins = Arrays.asList(java1, cobol, javaNoVersion, noVersion, java2);
 
     List<PluginInfo> ordered = natural().sortedCopy(plugins);
     assertThat(ordered.get(0)).isSameAs(cobol);
-    assertThat(ordered.get(1)).isSameAs(java1);
-    assertThat(ordered.get(2)).isSameAs(java2);
-    assertThat(ordered.get(3)).isSameAs(noVersion);
+    assertThat(ordered.get(1)).isSameAs(javaNoVersion);
+    assertThat(ordered.get(2)).isSameAs(java1);
+    assertThat(ordered.get(3)).isSameAs(java2);
+    assertThat(ordered.get(4)).isSameAs(noVersion);
+  }
+
+  @Test
+  public void test_equals() {
+    PluginInfo java1 = new PluginInfo("java").setVersion(Version.create("1.0"));
+    PluginInfo java2 = new PluginInfo("java").setVersion(Version.create("2.0"));
+    PluginInfo javaNoVersion = new PluginInfo("java");
+    PluginInfo cobol = new PluginInfo("cobol").setVersion(Version.create("1.0"));
+
+    assertThat(java1.equals(java1)).isTrue();
+    assertThat(java1.equals(java2)).isFalse();
+    assertThat(java1.equals(javaNoVersion)).isFalse();
+    assertThat(java1.equals(cobol)).isFalse();
+    assertThat(java1.equals("java:1.0")).isFalse();
+    assertThat(java1.equals(null)).isFalse();
+    assertThat(javaNoVersion.equals(javaNoVersion)).isTrue();
+
+    assertThat(java1.hashCode()).isEqualTo(java1.hashCode());
+    assertThat(javaNoVersion.hashCode()).isEqualTo(javaNoVersion.hashCode());
   }
 
   @Test
@@ -118,7 +137,7 @@ public class PluginInfoTest {
     assertThat(pluginInfo.getKey()).isEqualTo("java");
     assertThat(pluginInfo.getName()).isEqualTo("Java");
     assertThat(pluginInfo.getVersion().getName()).isEqualTo("1.0");
-    assertThat(pluginInfo.getJarFile2()).isSameAs(jarFile);
+    assertThat(pluginInfo.getJarFile()).isSameAs(jarFile);
     assertThat(pluginInfo.getMainClass()).isEqualTo("org.foo.FooPlugin");
     assertThat(pluginInfo.isCore()).isFalse();
     assertThat(pluginInfo.getBasePlugin()).isNull();