SONAR-6517 apply second feedback

diff --git a/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java b/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java
index 9a7ab9c36908e93e81b6de98935253e336cc9c6b..e3f1306d8abfbcd3632fb69099c02174b00e3a01 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java
@@ -22,7 +22,6 @@ package org.sonar.server.plugins;
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
-import org.sonar.api.Plugin;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.core.platform.PluginRepository;

diff --git a/server/sonar-server/src/main/java/org/sonar/server/plugins/ws/UninstallPluginsWsAction.java b/server/sonar-server/src/main/java/org/sonar/server/plugins/ws/UninstallPluginsWsAction.java
index 98399499acd28d16957063b327cd8cbb46314bd7..1c8f2dcfe07606491b7d0ebd06cdc3a65516c45f 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/plugins/ws/UninstallPluginsWsAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/plugins/ws/UninstallPluginsWsAction.java
@@ -63,7 +63,7 @@ public class UninstallPluginsWsAction implements PluginsWsAction {
     response.noContent();
   }
 
-  // FIXME should be moved to {@link ServerPluginRepository#uninstall(String)}
+  // FIXME should be moved to ServerPluginRepository#uninstall(String)
   private void ensurePluginIsInstalled(String key) {
     if (!pluginRepository.hasPlugin(key)) {
       throw new IllegalArgumentException(format("Plugin [%s] is not installed", key));

diff --git a/server/sonar-server/src/main/java/org/sonar/server/startup/GeneratePluginIndex.java b/server/sonar-server/src/main/java/org/sonar/server/startup/GeneratePluginIndex.java
index 4fe91fb829f706d31862108925653dd36dd2f2a8..eb50d9a073212e858e979f9680307ac684848c16 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/startup/GeneratePluginIndex.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/startup/GeneratePluginIndex.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.startup;
 
+import org.apache.commons.io.Charsets;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.CharUtils;
@@ -29,8 +30,9 @@ import org.sonar.core.plugins.RemotePlugin;
 import org.sonar.server.platform.DefaultServerFileSystem;
 
 import java.io.File;
-import java.io.FileWriter;
+import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.OutputStreamWriter;
 import java.io.Writer;
 
 public final class GeneratePluginIndex implements ServerComponent {
@@ -49,7 +51,7 @@ public final class GeneratePluginIndex implements ServerComponent {
 
   void writeIndex(File indexFile) throws IOException {
     FileUtils.forceMkdir(indexFile.getParentFile());
-    Writer writer = new FileWriter(indexFile, false);
+    Writer writer = new OutputStreamWriter(new FileOutputStream(indexFile), Charsets.UTF_8);
     try {
       for (PluginInfo pluginInfo : repository.getPluginInfos()) {
         writer.append(RemotePlugin.create(pluginInfo).marshal());

diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchPluginRepository.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchPluginRepository.java
index 37658fe8b1ea73ebe8689f8f225f77f4cd8772a4..bf1d7d12519ec9481be467bf9384b8e369912642 100644 (file)
--- a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchPluginRepository.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchPluginRepository.java
@@ -21,7 +21,6 @@ package org.sonar.batch.bootstrap;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Maps;
-import java.util.HashMap;
 import org.picocontainer.Startable;
 import org.sonar.api.Plugin;
 import org.sonar.core.platform.PluginInfo;

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 317ced0ba21acce59cb3fa1e615b34f365a6cf13..66e1942ee58cd68f7666323ec62a052a54cd6521 100644 (file)
--- 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 b758f12d423c694bd2610593e44e3672380ac77d..7afda90702125826da045e5a21103755f474f6af 100644 (file)
--- 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 198f9e0c9f1d2f533ef1732f8b2146981be6d3be..fac38034eaf7a99263bffff6298e33598086bdc2 100644 (file)
--- 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();