From ba3556f6fb778a03f04a91cc1104409cd7466171 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 7 May 2015 17:11:07 +0200 Subject: [PATCH] SONAR-6517 apply second feedback --- .../plugins/StaticResourcesServlet.java | 1 - .../plugins/ws/UninstallPluginsWsAction.java | 2 +- .../server/startup/GeneratePluginIndex.java | 6 ++- .../bootstrap/BatchPluginRepository.java | 1 - .../org/sonar/core/platform/PluginInfo.java | 41 +++++++++++++++---- .../org/sonar/core/platform/PluginLoader.java | 2 +- .../sonar/core/platform/PluginInfoTest.java | 35 ++++++++++++---- 7 files changed, 66 insertions(+), 22 deletions(-) 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 9a7ab9c3690..e3f1306d8ab 100644 --- 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 98399499acd..1c8f2dcfe07 100644 --- 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 4fe91fb829f..eb50d9a0732 100644 --- 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 37658fe8b1e..bf1d7d12519 100644 --- 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 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 { } } - private String key; + private final String key; private String name; @CheckForNull @@ -133,6 +135,7 @@ public class PluginInfo implements Comparable { private final Set 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 { } @CheckForNull - public File getJarFile2() { + public File getJarFile() { return jarFile; } @@ -327,12 +330,34 @@ public class PluginInfo implements Comparable { } @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 { } } - public static Function jarToPluginInfo() { + public static Function 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 plugins = Arrays.asList(java1, java2, cobol, noVersion); - Collections.shuffle(plugins); + List plugins = Arrays.asList(java1, cobol, javaNoVersion, noVersion, java2); List 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(); -- 2.39.5