From 9e158756e51480dcf145bddedb2286d2c4218850 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 24 Feb 2015 10:17:04 -0800 Subject: [PATCH] Convert plugin loading code to use Path All of the internal scanning/loading code now uses Path, as do the plugin and data directories in SitePaths. For backwards compatibility, we still inject the @PluginData directory as a File. We do break backwards compatibility of the ServerPluginProvider extension point, under the theory that there should be very few such extensions and they can handle the pain. Tested: -Installed and reloaded cookbook plugin multiple times. -Expanded dynamic *.ssh plugin loading in the cookbook, with monkey-testing. Change-Id: I5212d248d9288b88d08731c9854b42196999aa73 --- Documentation/dev-plugins.txt | 11 +- .../com/google/gerrit/common/PluginData.java | 14 +- .../extensions/annotations/PluginData.java | 8 +- .../pgm/init/InitPluginStepsLoader.java | 60 ++-- .../google/gerrit/pgm/init/InitPlugins.java | 43 +-- .../gerrit/server/config/SitePaths.java | 8 +- .../gerrit/server/plugins/CleanupHandle.java | 23 +- .../server/plugins/JarPluginProvider.java | 65 ++--- .../MultipleProvidersForPluginException.java | 6 +- .../google/gerrit/server/plugins/Plugin.java | 14 +- .../server/plugins/PluginContentScanner.java | 5 +- .../plugins/PluginDataAsFileProvider.java | 38 +++ .../gerrit/server/plugins/PluginLoader.java | 264 +++++++++--------- .../gerrit/server/plugins/ServerPlugin.java | 28 +- .../server/plugins/ServerPluginProvider.java | 23 +- .../UniversalServerPluginProvider.java | 31 +- plugins/cookbook-plugin | 2 +- 17 files changed, 350 insertions(+), 293 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginDataAsFileProvider.java diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt index a193df78da..a672b177ef 100644 --- a/Documentation/dev-plugins.txt +++ b/Documentation/dev-plugins.txt @@ -1717,17 +1717,18 @@ $ curl http://review.example.com/plugins/helloworld/print [[data-directory]] == Data Directory -Plugins can request a data directory with a `@PluginData` File -dependency. A data directory will be created automatically by the -server in `$site_path/data/$plugin_name` and passed to the plugin. +Plugins can request a data directory with a `@PluginData` Path (or File, +deprecated) dependency. A data directory will be created automatically +by the server in `$site_path/data/$plugin_name` and passed to the +plugin. Plugins can use this to store any data they want. [source,java] ---- @Inject -MyType(@PluginData java.io.File myDir) { - new FileInputStream(new File(myDir, "my.config")); +MyType(@PluginData java.nio.file.Path myDir) { + this.in = Files.newInputStream(myDir.resolve("my.config")); } ---- diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/PluginData.java b/gerrit-common/src/main/java/com/google/gerrit/common/PluginData.java index ffdae9d46b..27dc6398da 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/PluginData.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/PluginData.java @@ -14,18 +14,18 @@ package com.google.gerrit.common; -import java.io.File; +import java.nio.file.Path; import java.util.Objects; public class PluginData { public final String name; public final String version; - public final File pluginFile; + public final Path pluginPath; - public PluginData(String name, String version, File pluginFile) { + public PluginData(String name, String version, Path pluginPath) { this.name = name; this.version = version; - this.pluginFile = pluginFile; + this.pluginPath = pluginPath; } @Override @@ -33,13 +33,13 @@ public class PluginData { if (obj instanceof PluginData) { PluginData o = (PluginData) obj; return Objects.equals(name, o.name) && Objects.equals(version, o.version) - && Objects.equals(pluginFile, o.pluginFile); + && Objects.equals(pluginPath, o.pluginPath); } return super.equals(obj); } @Override public int hashCode() { - return Objects.hash(name, version, pluginFile); + return Objects.hash(name, version, pluginPath); } -} \ No newline at end of file +} diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/PluginData.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/PluginData.java index 75238a8fbc..1bcb5ab9fd 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/PluginData.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/PluginData.java @@ -27,11 +27,15 @@ import java.lang.annotation.Target; *

* A plugin or extension may receive this string by Guice injection to discover * a directory where it can store configuration or other data that is private: + *

+ * This binding is on both {@link java.io.File} and {@link java.nio.file.Path}, + * pointing to the same location. The {@code File} version should be considered + * deprecated and may be removed in a future version. * * 

  * {@literal @Inject}
- * MyType(@PluginData java.io.File myDir) {
- *   new FileInputStream(new File(myDir, "my.config"));
+ * MyType(@PluginData java.nio.file.Path myDir) {
+ *   this.in = Files.newInputStream(myDir.resolve("my.config"));
  * }
  *
*/ diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java index 893f00db35..2cffe42804 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPluginStepsLoader.java @@ -15,6 +15,7 @@ package com.google.gerrit.pgm.init; import com.google.common.base.MoreObjects; +import com.google.common.collect.Ordering; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.pgm.init.api.ConsoleUI; import com.google.gerrit.pgm.init.api.InitStep; @@ -26,23 +27,22 @@ import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Singleton; -import java.io.File; -import java.io.FileFilter; import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.jar.Attributes; import java.util.jar.JarFile; @Singleton public class InitPluginStepsLoader { - private final File pluginsDir; + private final Path pluginsDir; private final Injector initInjector; final ConsoleUI ui; @@ -55,10 +55,10 @@ public class InitPluginStepsLoader { } public Collection getInitSteps() { - List jars = scanJarsInPluginsDirectory(); + List jars = scanJarsInPluginsDirectory(); ArrayList pluginsInitSteps = new ArrayList<>(); - for (File jar : jars) { + for (Path jar : jars) { InitStep init = loadInitStep(jar); if (init != null) { pluginsInitSteps.add(init); @@ -68,12 +68,12 @@ public class InitPluginStepsLoader { } @SuppressWarnings("resource") - private InitStep loadInitStep(File jar) { + private InitStep loadInitStep(Path jar) { try { URLClassLoader pluginLoader = - new URLClassLoader(new URL[] {jar.toURI().toURL()}, + new URLClassLoader(new URL[] {jar.toUri().toURL()}, InitPluginStepsLoader.class.getClassLoader()); - try (JarFile jarFile = new JarFile(jar)) { + try (JarFile jarFile = new JarFile(jar.toFile())) { Attributes jarFileAttributes = jarFile.getManifest().getMainAttributes(); String initClassName = jarFileAttributes.getValue("Gerrit-InitStep"); if (initClassName == null) { @@ -86,7 +86,7 @@ public class InitPluginStepsLoader { } catch (ClassCastException e) { ui.message( "WARN: InitStep from plugin %s does not implement %s (Exception: %s)", - jar.getName(), InitStep.class.getName(), e.getMessage()); + jar.getFileName(), InitStep.class.getName(), e.getMessage()); return null; } } catch (Exception e) { @@ -97,11 +97,10 @@ public class InitPluginStepsLoader { } } - private Injector getPluginInjector(final File jarFile) throws IOException { - final String pluginName = - MoreObjects.firstNonNull( - JarPluginProvider.getJarPluginName(jarFile), - PluginLoader.nameOf(jarFile)); + private Injector getPluginInjector(Path jarPath) throws IOException { + final String pluginName = MoreObjects.firstNonNull( + JarPluginProvider.getJarPluginName(jarPath), + PluginLoader.nameOf(jarPath)); return initInjector.createChildInjector(new AbstractModule() { @Override protected void configure() { @@ -111,27 +110,24 @@ public class InitPluginStepsLoader { }); } - private List scanJarsInPluginsDirectory() { - if (pluginsDir == null || !pluginsDir.exists()) { + private List scanJarsInPluginsDirectory() { + if (pluginsDir == null || !Files.isDirectory(pluginsDir)) { return Collections.emptyList(); } - File[] matches = pluginsDir.listFiles(new FileFilter() { + DirectoryStream.Filter filter = new DirectoryStream.Filter() { @Override - public boolean accept(File pathname) { - String n = pathname.getName(); - return (n.endsWith(".jar") && pathname.isFile()); + public boolean accept(Path entry) throws IOException { + return entry.getFileName().toString().endsWith(".jar") + && Files.isRegularFile(entry); } - }); - if (matches == null) { - ui.message("WARN: Cannot list %s", pluginsDir.getAbsolutePath()); + }; + try (DirectoryStream paths = + Files.newDirectoryStream(pluginsDir, filter)) { + return Ordering.natural().sortedCopy(paths); + } catch (IOException e) { + ui.message("WARN: Cannot list %s: %s", pluginsDir.toAbsolutePath(), + e.getMessage()); return Collections.emptyList(); } - Arrays.sort(matches, new Comparator() { - @Override - public int compare(File o1, File o2) { - return o1.getName().compareTo(o2.getName()); - } - }); - return Arrays.asList(matches); } } diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java index cc076b5dc3..ca4b94952d 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java @@ -25,9 +25,10 @@ import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Singleton; -import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.jar.Attributes; import java.util.jar.JarFile; @@ -55,10 +56,10 @@ public class InitPlugins implements InitStep { pluginsDistribution.foreach(new PluginsDistribution.Processor() { @Override public void process(String pluginName, InputStream in) throws IOException { - File tmpPlugin = JarPluginProvider.storeInTemp(pluginName, in, site); + Path tmpPlugin = JarPluginProvider.storeInTemp(pluginName, in, site); String pluginVersion = getVersion(tmpPlugin); if (deleteTempPluginFile) { - tmpPlugin.delete(); + Files.delete(tmpPlugin); } result.add(new PluginData(pluginName, pluginVersion, tmpPlugin)); } @@ -108,37 +109,39 @@ public class InitPlugins implements InitStep { for (PluginData plugin : plugins) { String pluginName = plugin.name; try { - final File tmpPlugin = plugin.pluginFile; + final Path tmpPlugin = plugin.pluginPath; if (!(initFlags.installPlugins.contains(pluginName) || ui.yesno(false, "Install plugin %s version %s", pluginName, plugin.version))) { - tmpPlugin.delete(); + Files.deleteIfExists(tmpPlugin); continue; } - final File p = new File(site.plugins_dir, plugin.name + ".jar"); - if (p.exists()) { + final Path p = site.plugins_dir.resolve(plugin.name + ".jar"); + if (Files.exists(p)) { final String installedPluginVersion = getVersion(p); if (!ui.yesno(false, "version %s is already installed, overwrite it", installedPluginVersion)) { - tmpPlugin.delete(); + Files.deleteIfExists(tmpPlugin); continue; } - if (!p.delete()) { + try { + Files.delete(p); + } catch (IOException e) { throw new IOException("Failed to delete plugin " + pluginName - + ": " + p.getAbsolutePath()); + + ": " + p.toAbsolutePath(), e); } } - if (!tmpPlugin.renameTo(p)) { + try { + Files.move(tmpPlugin, p); + } catch (IOException e) { throw new IOException("Failed to install plugin " + pluginName - + ": " + tmpPlugin.getAbsolutePath() + " -> " - + p.getAbsolutePath()); + + ": " + tmpPlugin.toAbsolutePath() + " -> " + + p.toAbsolutePath(), e); } } finally { - if (plugin.pluginFile.exists()) { - plugin.pluginFile.delete(); - } + Files.deleteIfExists(plugin.pluginPath); } } if (plugins.isEmpty()) { @@ -159,11 +162,11 @@ public class InitPlugins implements InitStep { } } - private static String getVersion(final File plugin) throws IOException { - final JarFile jarFile = new JarFile(plugin); + private static String getVersion(Path plugin) throws IOException { + JarFile jarFile = new JarFile(plugin.toFile()); try { - final Manifest manifest = jarFile.getManifest(); - final Attributes main = manifest.getMainAttributes(); + Manifest manifest = jarFile.getManifest(); + Attributes main = manifest.getMainAttributes(); return main.getValue(Attributes.Name.IMPLEMENTATION_VERSION); } finally { jarFile.close(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java index a3e7924834..5cfcc045f6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java @@ -35,8 +35,8 @@ public final class SitePaths { public final Path lib_dir; public final Path tmp_dir; public final File logs_dir; - public final File plugins_dir; - public final File data_dir; + public final Path plugins_dir; + public final Path data_dir; public final File mail_dir; public final File hooks_dir; public final File static_dir; @@ -74,8 +74,8 @@ public final class SitePaths { etc_dir = p.resolve("etc"); lib_dir = p.resolve("lib"); tmp_dir = p.resolve("tmp"); - plugins_dir = new File(site_path, "plugins"); - data_dir = new File(site_path, "data"); + plugins_dir = p.resolve("plugins"); + data_dir = p.resolve("data"); logs_dir = new File(site_path, "logs"); mail_dir = etc_dir.resolve("mail").toFile(); hooks_dir = new File(site_path, "hooks"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java index 593f2c94b4..98278124e4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java @@ -14,17 +14,17 @@ package com.google.gerrit.server.plugins; -import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.jar.JarFile; class CleanupHandle { - private final File tmpFile; + private final Path tmp; private final JarFile jarFile; - CleanupHandle(File tmpFile, - JarFile jarFile) { - this.tmpFile = tmpFile; + CleanupHandle(Path tmp, JarFile jarFile) { + this.tmp = tmp; this.jarFile = jarFile; } @@ -34,12 +34,13 @@ class CleanupHandle { } catch (IOException err) { PluginLoader.log.error("Cannot close " + jarFile.getName(), err); } - if (!tmpFile.delete() && tmpFile.exists()) { - PluginLoader.log.warn("Cannot delete " + tmpFile.getAbsolutePath() - + ", retrying to delete it on termination of the virtual machine"); - tmpFile.deleteOnExit(); - } else { - PluginLoader.log.info("Cleaned plugin " + tmpFile.getName()); + try { + Files.deleteIfExists(tmp); + PluginLoader.log.info("Cleaned plugin " + tmp.getFileName()); + } catch (IOException e) { + PluginLoader.log.warn("Cannot delete " + tmp.toAbsolutePath() + + ", retrying to delete it on termination of the virtual machine", e); + tmp.toFile().deleteOnExit(); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarPluginProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarPluginProvider.java index 37c05d556f..dcfb52cf9b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarPluginProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarPluginProvider.java @@ -24,8 +24,6 @@ import org.eclipse.jgit.internal.storage.file.FileSnapshot; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; @@ -33,6 +31,7 @@ import java.net.URL; import java.net.URLClassLoader; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Date; @@ -45,52 +44,50 @@ public class JarPluginProvider implements ServerPluginProvider { static final String JAR_EXTENSION = ".jar"; static final Logger log = LoggerFactory.getLogger(JarPluginProvider.class); - private final File tmpDir; + private final Path tmpDir; @Inject JarPluginProvider(SitePaths sitePaths) { - // TODO(dborowitz): Convert to NIO. - tmpDir = sitePaths.tmp_dir.toFile(); + tmpDir = sitePaths.tmp_dir; } @Override - public boolean handles(File srcFile) { - String fileName = srcFile.getName(); + public boolean handles(Path srcPath) { + String fileName = srcPath.getFileName().toString(); return fileName.endsWith(JAR_EXTENSION) || fileName.endsWith(JAR_EXTENSION + ".disabled"); } @Override - public String getPluginName(File srcFile) { + public String getPluginName(Path srcPath) { try { - return MoreObjects.firstNonNull(getJarPluginName(srcFile), - PluginLoader.nameOf(srcFile)); + return MoreObjects.firstNonNull(getJarPluginName(srcPath), + PluginLoader.nameOf(srcPath)); } catch (IOException e) { - throw new IllegalArgumentException("Invalid plugin file " + srcFile + throw new IllegalArgumentException("Invalid plugin file " + srcPath + ": cannot get plugin name", e); } } - public static String getJarPluginName(File srcFile) throws IOException { - try (JarFile jarFile = new JarFile(srcFile)) { + public static String getJarPluginName(Path srcPath) throws IOException { + try (JarFile jarFile = new JarFile(srcPath.toFile())) { return jarFile.getManifest().getMainAttributes() .getValue("Gerrit-PluginName"); } } @Override - public ServerPlugin get(File srcFile, FileSnapshot snapshot, + public ServerPlugin get(Path srcPath, FileSnapshot snapshot, PluginDescription description) throws InvalidPluginException { try { - String name = getPluginName(srcFile); - String extension = getExtension(srcFile); - try (FileInputStream in = new FileInputStream(srcFile)) { - File tmp = asTemp(in, tempNameFor(name), extension, tmpDir); - return loadJarPlugin(name, srcFile.toPath(), snapshot, tmp, - description); + String name = getPluginName(srcPath); + String extension = getExtension(srcPath); + try (InputStream in = Files.newInputStream(srcPath)) { + Path tmp = asTemp(in, tempNameFor(name), extension, tmpDir); + return loadJarPlugin(name, srcPath, snapshot, tmp, description); } } catch (IOException e) { - throw new InvalidPluginException("Cannot load Jar plugin " + srcFile, e); + throw new InvalidPluginException("Cannot load Jar plugin " + srcPath, e); } } @@ -99,8 +96,8 @@ public class JarPluginProvider implements ServerPluginProvider { return "gerrit"; } - private static String getExtension(File file) { - return getExtension(file.getName()); + private static String getExtension(Path path) { + return getExtension(path.getFileName().toString()); } private static String getExtension(String name) { @@ -113,19 +110,18 @@ public class JarPluginProvider implements ServerPluginProvider { return PLUGIN_TMP_PREFIX + name + "_" + fmt.format(new Date()) + "_"; } - public static File storeInTemp(String pluginName, InputStream in, + public static Path storeInTemp(String pluginName, InputStream in, SitePaths sitePaths) throws IOException { if (!Files.exists(sitePaths.tmp_dir)) { Files.createDirectories(sitePaths.tmp_dir); } - return asTemp(in, tempNameFor(pluginName), ".jar", - sitePaths.tmp_dir.toFile()); + return asTemp(in, tempNameFor(pluginName), ".jar", sitePaths.tmp_dir); } private ServerPlugin loadJarPlugin(String name, Path srcJar, - FileSnapshot snapshot, File tmp, PluginDescription description) + FileSnapshot snapshot, Path tmp, PluginDescription description) throws IOException, InvalidPluginException, MalformedURLException { - JarFile jarFile = new JarFile(tmp); + JarFile jarFile = new JarFile(tmp.toFile()); boolean keep = false; try { Manifest manifest = jarFile.getManifest(); @@ -134,14 +130,13 @@ public class JarPluginProvider implements ServerPluginProvider { List urls = new ArrayList<>(2); String overlay = System.getProperty("gerrit.plugin-classes"); if (overlay != null) { - File classes = new File(new File(new File(overlay), name), "main"); - if (classes.isDirectory()) { - log.info(String.format("plugin %s: including %s", name, - classes.getPath())); - urls.add(classes.toURI().toURL()); + Path classes = Paths.get(overlay).resolve(name).resolve("main"); + if (Files.isDirectory(classes)) { + log.info(String.format("plugin %s: including %s", name, classes)); + urls.add(classes.toUri().toURL()); } } - urls.add(tmp.toURI().toURL()); + urls.add(tmp.toUri().toURL()); ClassLoader pluginLoader = new URLClassLoader(urls.toArray(new URL[urls.size()]), @@ -149,7 +144,7 @@ public class JarPluginProvider implements ServerPluginProvider { JarScanner jarScanner = createJarScanner(srcJar); ServerPlugin plugin = new ServerPlugin(name, description.canonicalUrl, - description.user, srcJar.toFile(), snapshot, jarScanner, + description.user, srcJar, snapshot, jarScanner, description.dataDir, pluginLoader); plugin.setCleanupHandle(new CleanupHandle(tmp, jarFile)); keep = true; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/MultipleProvidersForPluginException.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/MultipleProvidersForPluginException.java index 82a6ad94ea..cf383102d4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/MultipleProvidersForPluginException.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/MultipleProvidersForPluginException.java @@ -18,14 +18,14 @@ import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.collect.Iterables; -import java.io.File; +import java.nio.file.Path; class MultipleProvidersForPluginException extends IllegalArgumentException { private static final long serialVersionUID = 1L; - MultipleProvidersForPluginException(File pluginSrcFile, + MultipleProvidersForPluginException(Path pluginSrcPath, Iterable providersHandlers) { - super(pluginSrcFile.getAbsolutePath() + super(pluginSrcPath.toAbsolutePath() + " is claimed to be handled by more than one plugin provider: " + providersListToString(providersHandlers)); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java index 1be65aa335..6b84c21c14 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java @@ -14,6 +14,8 @@ package com.google.gerrit.server.plugins; +import static com.google.gerrit.common.FileUtil.lastModified; + import com.google.common.base.Strings; import com.google.common.collect.Lists; import com.google.gerrit.common.Nullable; @@ -25,7 +27,6 @@ import com.google.inject.Injector; import org.eclipse.jgit.internal.storage.file.FileSnapshot; -import java.io.File; import java.nio.file.Path; import java.util.Collections; import java.util.List; @@ -81,18 +82,17 @@ public abstract class Plugin { private List> reloadableHandles; public Plugin(String name, - Path srcFile, + Path srcPath, PluginUser pluginUser, FileSnapshot snapshot, ApiType apiType) { this.name = name; - // TODO(dborowitz): Rename to srcPath or something. - this.srcFile = srcFile; + this.srcFile = srcPath; this.apiType = apiType; this.snapshot = snapshot; this.pluginUser = pluginUser; this.cacheKey = new Plugin.CacheKey(name); - this.disabled = srcFile.getFileName().toString().endsWith(".disabled"); + this.disabled = srcPath.getFileName().toString().endsWith(".disabled"); } public CleanupHandle getCleanupHandle() { @@ -170,7 +170,7 @@ public abstract class Plugin { abstract boolean canReload(); - boolean isModified(File jar) { - return snapshot.lastModified() != jar.lastModified(); + boolean isModified(Path jar) { + return snapshot.lastModified() != lastModified(jar); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java index 022850965e..1d9cd0e5c8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java @@ -11,14 +11,15 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package com.google.gerrit.server.plugins; import com.google.common.base.Optional; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.lang.annotation.Annotation; +import java.nio.file.NoSuchFileException; import java.util.Collections; import java.util.Enumeration; import java.util.Map; @@ -57,7 +58,7 @@ public interface PluginContentScanner { @Override public InputStream getInputStream(PluginEntry entry) throws IOException { - throw new FileNotFoundException("Empty plugin"); + throw new NoSuchFileException("Empty plugin"); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginDataAsFileProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginDataAsFileProvider.java new file mode 100644 index 0000000000..5e8242ba06 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginDataAsFileProvider.java @@ -0,0 +1,38 @@ +// Copyright (C) 2015 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.plugins; + +import com.google.gerrit.extensions.annotations.PluginData; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import java.io.File; +import java.nio.file.Path; + +@Singleton +class PluginDataAsFileProvider implements Provider { + private final Provider pathProvider; + + @Inject + PluginDataAsFileProvider(@PluginData Provider pathProvider) { + this.pathProvider = pathProvider; + } + + @Override + public File get() { + return pathProvider.get().toFile(); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java index 3fee7d7592..17bb63443c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java @@ -18,6 +18,8 @@ import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.base.MoreObjects; import com.google.common.base.Predicate; +import com.google.common.collect.ComparisonChain; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Lists; @@ -25,6 +27,7 @@ import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.Queues; import com.google.common.collect.Sets; +import com.google.common.io.ByteStreams; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.systemstatus.ServerInformation; @@ -45,15 +48,15 @@ import org.eclipse.jgit.lib.Config; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; -import java.io.FileFilter; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.AbstractMap; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -72,13 +75,13 @@ public class PluginLoader implements LifecycleListener { static final String PLUGIN_TMP_PREFIX = "plugin_"; static final Logger log = LoggerFactory.getLogger(PluginLoader.class); - public String getPluginName(File srcFile) { - return MoreObjects.firstNonNull(getGerritPluginName(srcFile), - nameOf(srcFile)); + public String getPluginName(Path srcPath) { + return MoreObjects.firstNonNull(getGerritPluginName(srcPath), + nameOf(srcPath)); } - private final File pluginsDir; - private final File dataDir; + private final Path pluginsDir; + private final Path dataDir; private final PluginGuiceEnvironment env; private final ServerInformationImpl srvInfoImpl; private final PluginUser.Factory pluginUserFactory; @@ -159,7 +162,7 @@ public class PluginLoader implements LifecycleListener { checkRemoteInstall(); String fileName = originalName; - File tmp = asTemp(in, ".next_" + fileName + "_", ".tmp", pluginsDir); + Path tmp = asTemp(in, ".next_" + fileName + "_", ".tmp", pluginsDir); String name = MoreObjects.firstNonNull(getGerritPluginName(tmp), nameOf(fileName)); if (!originalName.equals(name)) { @@ -169,26 +172,26 @@ public class PluginLoader implements LifecycleListener { } String fileExtension = getExtension(fileName); - File dst = new File(pluginsDir, name + fileExtension); + Path dst = pluginsDir.resolve(name + fileExtension); synchronized (this) { Plugin active = running.get(name); if (active != null) { fileName = active.getSrcFile().getFileName().toString(); log.info(String.format("Replacing plugin %s", active.getName())); - File old = new File(pluginsDir, ".last_" + fileName); - old.delete(); - active.getSrcFile().toFile().renameTo(old); + Path old = pluginsDir.resolve(".last_" + fileName); + Files.deleteIfExists(old); + Files.move(active.getSrcFile(), old); } - new File(pluginsDir, fileName + ".disabled").delete(); - tmp.renameTo(dst); + Files.deleteIfExists(pluginsDir.resolve(fileName + ".disabled")); + Files.move(tmp, dst); try { Plugin plugin = runPlugin(name, dst, active); if (active == null) { log.info(String.format("Installed plugin %s", plugin.getName())); } } catch (PluginInstallException e) { - dst.delete(); + Files.deleteIfExists(dst); throw e; } @@ -196,21 +199,17 @@ public class PluginLoader implements LifecycleListener { } } - static File asTemp(InputStream in, String prefix, String suffix, File dir) + static Path asTemp(InputStream in, String prefix, String suffix, Path dir) throws IOException { - File tmp = File.createTempFile(prefix, suffix, dir); + Path tmp = Files.createTempFile(dir, prefix, suffix); boolean keep = false; - try (FileOutputStream out = new FileOutputStream(tmp)) { - byte[] data = new byte[8192]; - int n; - while ((n = in.read(data)) > 0) { - out.write(data, 0, n); - } + try (OutputStream out = Files.newOutputStream(tmp)) { + ByteStreams.copy(in, out); keep = true; return tmp; } finally { if (!keep) { - tmp.delete(); + Files.delete(tmp); } } } @@ -241,12 +240,21 @@ public class PluginLoader implements LifecycleListener { } log.info(String.format("Disabling plugin %s", active.getName())); - File off = new File(active.getSrcFile() + ".disabled"); - active.getSrcFile().toFile().renameTo(off); + Path off = active.getSrcFile().resolveSibling( + active.getSrcFile().getFileName() + ".disabled"); + try { + Files.move(active.getSrcFile(), off); + } catch (IOException e) { + log.error("Failed to disable plugin", e); + // In theory we could still unload the plugin even if the rename + // failed. However, it would be reloaded on the next server startup, + // which is probably not what the user expects. + continue; + } unloadPlugin(active); try { - FileSnapshot snapshot = FileSnapshot.save(off); + FileSnapshot snapshot = FileSnapshot.save(off.toFile()); Plugin offPlugin = loadPlugin(name, off, snapshot); disabled.put(name, offPlugin); } catch (Throwable e) { @@ -279,9 +287,13 @@ public class PluginLoader implements LifecycleListener { if (n.endsWith(".disabled")) { n = n.substring(0, n.lastIndexOf('.')); } - File on = new File(pluginsDir, n); - off.getSrcFile().toFile().renameTo(on); - + Path on = pluginsDir.resolve(n); + try { + Files.move(off.getSrcFile(), on); + } catch (IOException e) { + log.error("Failed to move plugin " + name + " into place", e); + continue; + } disabled.remove(name); runPlugin(name, on, null); } @@ -291,7 +303,7 @@ public class PluginLoader implements LifecycleListener { @Override public synchronized void start() { - log.info("Loading plugins from " + pluginsDir.getAbsolutePath()); + log.info("Loading plugins from " + pluginsDir.toAbsolutePath()); srvInfoImpl.state = ServerInformation.State.STARTUP; rescan(); srvInfoImpl.state = ServerInformation.State.RUNNING; @@ -343,7 +355,7 @@ public class PluginLoader implements LifecycleListener { String name = active.getName(); try { log.info(String.format("Reloading plugin %s", name)); - runPlugin(name, active.getSrcFile().toFile(), active); + runPlugin(name, active.getSrcFile(), active); } catch (PluginInstallException e) { log.warn(String.format("Cannot reload plugin %s", name), e.getCause()); throw e; @@ -355,30 +367,30 @@ public class PluginLoader implements LifecycleListener { } public synchronized void rescan() { - Multimap pluginsFiles = prunePlugins(pluginsDir); + Multimap pluginsFiles = prunePlugins(pluginsDir); if (pluginsFiles.isEmpty()) { return; } syncDisabledPlugins(pluginsFiles); - Map activePlugins = filterDisabled(pluginsFiles); - for (Map.Entry entry : jarsFirstSortedPluginsSet(activePlugins)) { + Map activePlugins = filterDisabled(pluginsFiles); + for (Map.Entry entry : jarsFirstSortedPluginsSet(activePlugins)) { String name = entry.getKey(); - File file = entry.getValue(); - String fileName = file.getName(); - if (!isJsPlugin(fileName) && !serverPluginFactory.handles(file)) { + Path path = entry.getValue(); + String fileName = path.getFileName().toString(); + if (!isJsPlugin(fileName) && !serverPluginFactory.handles(path)) { log.warn("No Plugin provider was found that handles this file format: {}", fileName); continue; } FileSnapshot brokenTime = broken.get(name); - if (brokenTime != null && !brokenTime.isModified(file)) { + if (brokenTime != null && !brokenTime.isModified(path.toFile())) { continue; } Plugin active = running.get(name); - if (active != null && !active.isModified(file)) { + if (active != null && !active.isModified(path)) { continue; } @@ -388,7 +400,7 @@ public class PluginLoader implements LifecycleListener { } try { - Plugin loadedPlugin = runPlugin(name, file, active); + Plugin loadedPlugin = runPlugin(name, path, active); if (active == null && !loadedPlugin.isDisabled()) { log.info(String.format("Loaded plugin %s, version %s", loadedPlugin.getName(), loadedPlugin.getVersion())); @@ -401,31 +413,28 @@ public class PluginLoader implements LifecycleListener { cleanInBackground(); } - private void addAllEntries(Map from, - TreeSet> to) { - Iterator> it = from.entrySet().iterator(); + private void addAllEntries(Map from, + TreeSet> to) { + Iterator> it = from.entrySet().iterator(); while (it.hasNext()) { - Entry entry = it.next(); + Entry entry = it.next(); to.add(new AbstractMap.SimpleImmutableEntry<>( entry.getKey(), entry.getValue())); } } - private TreeSet> jarsFirstSortedPluginsSet( - Map activePlugins) { - TreeSet> sortedPlugins = - Sets.newTreeSet(new Comparator>() { + private TreeSet> jarsFirstSortedPluginsSet( + Map activePlugins) { + TreeSet> sortedPlugins = + Sets.newTreeSet(new Comparator>() { @Override - public int compare(Entry entry1, - Entry entry2) { - String file1 = entry1.getValue().getName(); - String file2 = entry2.getValue().getName(); - int cmp = file1.compareTo(file2); - if (file1.endsWith(".jar")) { - return (file2.endsWith(".jar") ? cmp : -1); - } else { - return (file2.endsWith(".jar") ? +1 : cmp); - } + public int compare(Entry e1, Entry e2) { + Path n1 = e1.getValue().getFileName(); + Path n2 = e2.getValue().getFileName(); + return ComparisonChain.start() + .compareTrueFirst(n1.endsWith(".jar"), n2.endsWith(".jar")) + .compare(n1, n2) + .result(); } }); @@ -433,14 +442,14 @@ public class PluginLoader implements LifecycleListener { return sortedPlugins; } - private void syncDisabledPlugins(Multimap jars) { + private void syncDisabledPlugins(Multimap jars) { stopRemovedPlugins(jars); dropRemovedDisabledPlugins(jars); } - private Plugin runPlugin(String name, File plugin, Plugin oldPlugin) + private Plugin runPlugin(String name, Path plugin, Plugin oldPlugin) throws PluginInstallException { - FileSnapshot snapshot = FileSnapshot.save(plugin); + FileSnapshot snapshot = FileSnapshot.save(plugin.toFile()); try { Plugin newPlugin = loadPlugin(name, plugin, snapshot); if (newPlugin.getCleanupHandle() != null) { @@ -480,11 +489,11 @@ public class PluginLoader implements LifecycleListener { } } - private void stopRemovedPlugins(Multimap jars) { + private void stopRemovedPlugins(Multimap jars) { Set unload = Sets.newHashSet(running.keySet()); - for (Map.Entry> entry : jars.asMap().entrySet()) { - for (File file : entry.getValue()) { - if (!file.getName().endsWith(".disabled")) { + for (Map.Entry> entry : jars.asMap().entrySet()) { + for (Path path : entry.getValue()) { + if (!path.getFileName().toString().endsWith(".disabled")) { unload.remove(entry.getKey()); } } @@ -494,11 +503,11 @@ public class PluginLoader implements LifecycleListener { } } - private void dropRemovedDisabledPlugins(Multimap jars) { + private void dropRemovedDisabledPlugins(Multimap jars) { Set unload = Sets.newHashSet(disabled.keySet()); - for (Map.Entry> entry : jars.asMap().entrySet()) { - for (File file : entry.getValue()) { - if (file.getName().endsWith(".disabled")) { + for (Map.Entry> entry : jars.asMap().entrySet()) { + for (Path path : entry.getValue()) { + if (path.getFileName().toString().endsWith(".disabled")) { unload.remove(entry.getKey()); } } @@ -529,8 +538,8 @@ public class PluginLoader implements LifecycleListener { } } - public static String nameOf(File plugin) { - return nameOf(plugin.getName()); + public static String nameOf(Path plugin) { + return nameOf(plugin.getFileName().toString()); } private static String nameOf(String name) { @@ -546,21 +555,21 @@ public class PluginLoader implements LifecycleListener { return 0 < ext ? name.substring(ext) : ""; } - private Plugin loadPlugin(String name, File srcPlugin, FileSnapshot snapshot) + private Plugin loadPlugin(String name, Path srcPlugin, FileSnapshot snapshot) throws InvalidPluginException { - String pluginName = srcPlugin.getName(); + String pluginName = srcPlugin.getFileName().toString(); if (isJsPlugin(pluginName)) { - return loadJsPlugin(name, srcPlugin.toPath(), snapshot); + return loadJsPlugin(name, srcPlugin, snapshot); } else if (serverPluginFactory.handles(srcPlugin)) { return loadServerPlugin(srcPlugin, snapshot); } else { throw new InvalidPluginException(String.format( - "Unsupported plugin type: %s", srcPlugin.getName())); + "Unsupported plugin type: %s", srcPlugin.getFileName())); } } - private File getPluginDataDir(String name) { - return new File(dataDir, name); + private Path getPluginDataDir(String name) { + return dataDir.resolve(name); } private String getPluginCanonicalWebUrl(String name) { @@ -574,7 +583,7 @@ public class PluginLoader implements LifecycleListener { return new JsPlugin(name, srcJar, pluginUserFactory.create(name), snapshot); } - private ServerPlugin loadServerPlugin(File scriptFile, + private ServerPlugin loadServerPlugin(Path scriptFile, FileSnapshot snapshot) throws InvalidPluginException { String name = serverPluginFactory.getPluginName(scriptFile); return serverPluginFactory.get(scriptFile, snapshot, new PluginDescription( @@ -598,15 +607,15 @@ public class PluginLoader implements LifecycleListener { // Only one active plugin per plugin name can exist for each plugin name. // Filter out disabled plugins and transform the multimap to a map - private static Map filterDisabled( - Multimap pluginFiles) { - Map activePlugins = Maps.newHashMapWithExpectedSize( - pluginFiles.keys().size()); - for (String name : pluginFiles.keys()) { - for (File pluginFile : pluginFiles.asMap().get(name)) { - if (!pluginFile.getName().endsWith(".disabled")) { + private static Map filterDisabled( + Multimap pluginPaths) { + Map activePlugins = Maps.newHashMapWithExpectedSize( + pluginPaths.keys().size()); + for (String name : pluginPaths.keys()) { + for (Path pluginPath : pluginPaths.asMap().get(name)) { + if (!pluginPath.getFileName().toString().endsWith(".disabled")) { assert(!activePlugins.containsKey(name)); - activePlugins.put(name, pluginFile); + activePlugins.put(name, pluginPath); } } } @@ -622,37 +631,40 @@ public class PluginLoader implements LifecycleListener { // // NOTE: Bear in mind that the plugin name can be reassigned after load by the // Server plugin provider. - public Multimap prunePlugins(File pluginsDir) { - List pluginFiles = scanFilesInPluginsDirectory(pluginsDir); - Multimap map; - map = asMultimap(pluginFiles); + public Multimap prunePlugins(Path pluginsDir) { + List pluginPaths = scanPathsInPluginsDirectory(pluginsDir); + Multimap map; + map = asMultimap(pluginPaths); for (String plugin : map.keySet()) { - Collection files = map.asMap().get(plugin); + Collection files = map.asMap().get(plugin); if (files.size() == 1) { continue; } // retrieve enabled plugins - Iterable enabled = filterDisabledPlugins( - files); + Iterable enabled = filterDisabledPlugins(files); // If we have only one (the winner) plugin, nothing to do if (!Iterables.skip(enabled, 1).iterator().hasNext()) { continue; } - File winner = Iterables.getFirst(enabled, null); + Path winner = Iterables.getFirst(enabled, null); assert(winner != null); // Disable all loser plugins by renaming their file names to // "file.disabled" and replace the disabled files in the multimap. - Collection elementsToRemove = Lists.newArrayList(); - Collection elementsToAdd = Lists.newArrayList(); - for (File loser : Iterables.skip(enabled, 1)) { + Collection elementsToRemove = Lists.newArrayList(); + Collection elementsToAdd = Lists.newArrayList(); + for (Path loser : Iterables.skip(enabled, 1)) { log.warn(String.format("Plugin <%s> was disabled, because" + " another plugin <%s>" + " with the same name <%s> already exists", loser, winner, plugin)); - File disabledPlugin = new File(loser + ".disabled"); + Path disabledPlugin = Paths.get(loser + ".disabled"); elementsToAdd.add(disabledPlugin); elementsToRemove.add(loser); - loser.renameTo(disabledPlugin); + try { + Files.move(loser, disabledPlugin); + } catch (IOException e) { + log.warn("Failed to fully disable plugin " + loser, e); + } } Iterables.removeAll(files, elementsToRemove); Iterables.addAll(files, elementsToAdd); @@ -660,50 +672,52 @@ public class PluginLoader implements LifecycleListener { return map; } - private List scanFilesInPluginsDirectory(File pluginsDir) { - if (pluginsDir == null || !pluginsDir.exists()) { + private List scanPathsInPluginsDirectory(Path pluginsDir) { + if (pluginsDir == null || !Files.exists(pluginsDir)) { return Collections.emptyList(); } - File[] matches = pluginsDir.listFiles(new FileFilter() { + DirectoryStream.Filter filter = new DirectoryStream.Filter() { @Override - public boolean accept(File pathname) { - String n = pathname.getName(); + public boolean accept(Path entry) throws IOException { + String n = entry.getFileName().toString(); return !n.startsWith(".last_") && !n.startsWith(".next_"); } - }); - if (matches == null) { - log.error("Cannot list " + pluginsDir.getAbsolutePath()); - return Collections.emptyList(); + }; + try (DirectoryStream files + = Files.newDirectoryStream(pluginsDir, filter)) { + return ImmutableList.copyOf(files); + } catch (IOException e) { + log.error("Cannot list " + pluginsDir.toAbsolutePath(), e); + return ImmutableList.of(); } - return Arrays.asList(matches); } - private static Iterable filterDisabledPlugins( - Collection files) { - return Iterables.filter(files, new Predicate() { + private static Iterable filterDisabledPlugins( + Collection paths) { + return Iterables.filter(paths, new Predicate() { @Override - public boolean apply(File file) { - return !file.getName().endsWith(".disabled"); + public boolean apply(Path p) { + return !p.getFileName().toString().endsWith(".disabled"); } }); } - public String getGerritPluginName(File srcFile) { - String fileName = srcFile.getName(); + public String getGerritPluginName(Path srcPath) { + String fileName = srcPath.getFileName().toString(); if (isJsPlugin(fileName)) { return fileName.substring(0, fileName.length() - 3); } - if (serverPluginFactory.handles(srcFile)) { - return serverPluginFactory.getPluginName(srcFile); + if (serverPluginFactory.handles(srcPath)) { + return serverPluginFactory.getPluginName(srcPath); } return null; } - private Multimap asMultimap(List plugins) { - Multimap map = LinkedHashMultimap.create(); - for (File srcFile : plugins) { - map.put(getPluginName(srcFile), srcFile); + private Multimap asMultimap(List plugins) { + Multimap map = LinkedHashMultimap.create(); + for (Path srcPath : plugins) { + map.put(getPluginName(srcPath), srcPath); } return map; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java index 15888008a2..7bb9a19a01 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java @@ -36,6 +36,8 @@ import org.eclipse.jgit.internal.storage.file.FileSnapshot; import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.jar.Attributes; import java.util.jar.Manifest; @@ -59,7 +61,7 @@ public class ServerPlugin extends Plugin { private final Manifest manifest; private final PluginContentScanner scanner; - private final File dataDir; + private final Path dataDir; private final String pluginCanonicalWebUrl; private final ClassLoader classLoader; private Class sysModule; @@ -75,12 +77,12 @@ public class ServerPlugin extends Plugin { public ServerPlugin(String name, String pluginCanonicalWebUrl, PluginUser pluginUser, - File srcJar, + Path srcJar, FileSnapshot snapshot, PluginContentScanner scanner, - File dataDir, + Path dataDir, ClassLoader classLoader) throws InvalidPluginException { - super(name, srcJar.toPath(), pluginUser, snapshot, + super(name, srcJar, pluginUser, snapshot, Plugin.getApiType(getPluginManifest(scanner))); this.pluginCanonicalWebUrl = pluginCanonicalWebUrl; this.scanner = scanner; @@ -128,8 +130,8 @@ public class ServerPlugin extends Plugin { return (Class) clazz; } - File getSrcJar() { - return getSrcFile().toFile(); + Path getSrcJar() { + return getSrcFile(); } private static Manifest getPluginManifest(PluginContentScanner scanner) @@ -245,20 +247,22 @@ public class ServerPlugin extends Plugin { .annotatedWith(PluginCanonicalWebUrl.class) .toInstance(pluginCanonicalWebUrl); - bind(File.class) + bind(Path.class) .annotatedWith(PluginData.class) - .toProvider(new Provider() { + .toProvider(new Provider() { private volatile boolean ready; @Override - public File get() { + public Path get() { if (!ready) { synchronized (dataDir) { if (!ready) { - if (!dataDir.exists() && !dataDir.mkdirs()) { + try { + Files.createDirectories(dataDir); + } catch (IOException e) { throw new ProvisionException(String.format( "Cannot create %s for plugin %s", - dataDir.getAbsolutePath(), getName())); + dataDir.toAbsolutePath(), getName()), e); } ready = true; } @@ -267,6 +271,8 @@ public class ServerPlugin extends Plugin { return dataDir; } }); + bind(File.class).annotatedWith(PluginData.class) + .toProvider(PluginDataAsFileProvider.class); } }); return Guice.createInjector(modules); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPluginProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPluginProvider.java index a529b6d814..bc2432bc3e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPluginProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPluginProvider.java @@ -19,7 +19,7 @@ import com.google.gerrit.server.PluginUser; import org.eclipse.jgit.internal.storage.file.FileSnapshot; -import java.io.File; +import java.nio.file.Path; /** * Provider of one Server plugin from one external file @@ -31,7 +31,6 @@ import java.io.File; * group them into a directory tree and then load the directory * root as a single plugin. */ -// TODO(dborowitz): Convert to NIO; ensure clients can migrate. @ExtensionPoint public interface ServerPluginProvider { @@ -41,7 +40,7 @@ public interface ServerPluginProvider { public class PluginDescription { public final PluginUser user; public final String canonicalUrl; - public final File dataDir; + public final Path dataDir; /** * Creates a new PluginDescription for ServerPluginProvider. @@ -50,7 +49,7 @@ public interface ServerPluginProvider { * @param canonicalUrl plugin root Web URL * @param dataDir directory for plugin data */ - public PluginDescription(PluginUser user, String canonicalUrl, File dataDir) { + public PluginDescription(PluginUser user, String canonicalUrl, Path dataDir) { this.user = user; this.canonicalUrl = canonicalUrl; this.dataDir = dataDir; @@ -60,39 +59,39 @@ public interface ServerPluginProvider { /** * Declares the availability to manage an external file or directory * - * @param srcFile the external file or directory + * @param srcPath the external file or directory * @return true if file or directory can be loaded into a Server Plugin */ - boolean handles(File srcFile); + boolean handles(Path srcPath); /** * Returns the plugin name of an external file or directory * - * Should be called only if {@link #handles(File) handles(srcFile)} + * Should be called only if {@link #handles(Path) handles(srcFile)} * returns true and thus srcFile is a supported plugin format. * An IllegalArgumentException is thrown otherwise as srcFile * is not a valid file format for extracting its plugin name. * - * @param srcFile external file or directory + * @param srcPath external file or directory * @return plugin name */ - String getPluginName(File srcFile); + String getPluginName(Path srcPath); /** * Loads an external file or directory into a Server plugin. * - * Should be called only if {@link #handles(File) handles(srcFile)} + * Should be called only if {@link #handles(Path) handles(srcFile)} * returns true and thus srcFile is a supported plugin format. * An IllegalArgumentException is thrown otherwise as srcFile * is not a valid file format for extracting its plugin name. * - * @param srcFile external file or directory + * @param srcPath external file or directory * @param snapshot snapshot of the external file * @param pluginDescriptor descriptor of the ServerPlugin to load * @throws InvalidPluginException if plugin is supposed to be handled * but cannot be loaded for any other reason */ - ServerPlugin get(File srcFile, FileSnapshot snapshot, + ServerPlugin get(Path srcPath, FileSnapshot snapshot, PluginDescription pluginDescriptor) throws InvalidPluginException; /** diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java index 0e8bd872ef..afdc5b3071 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java @@ -22,7 +22,7 @@ import org.eclipse.jgit.internal.storage.file.FileSnapshot; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; @@ -38,27 +38,26 @@ class UniversalServerPluginProvider implements ServerPluginProvider { } @Override - public ServerPlugin get(File srcFile, FileSnapshot snapshot, + public ServerPlugin get(Path srcPath, FileSnapshot snapshot, PluginDescription pluginDescription) throws InvalidPluginException { - return providerOf(srcFile).get(srcFile, snapshot, pluginDescription); + return providerOf(srcPath).get(srcPath, snapshot, pluginDescription); } @Override - public String getPluginName(File srcFile) { - return providerOf(srcFile).getPluginName(srcFile); + public String getPluginName(Path srcPath) { + return providerOf(srcPath).getPluginName(srcPath); } @Override - public boolean handles(File srcFile) { - List providers = - providersForHandlingPlugin(srcFile); + public boolean handles(Path srcPath) { + List providers = providersForHandlingPlugin(srcPath); switch (providers.size()) { case 1: return true; case 0: return false; default: - throw new MultipleProvidersForPluginException(srcFile, providers); + throw new MultipleProvidersForPluginException(srcPath, providers); } } @@ -67,27 +66,27 @@ class UniversalServerPluginProvider implements ServerPluginProvider { return "gerrit"; } - private ServerPluginProvider providerOf(File srcFile) { + private ServerPluginProvider providerOf(Path srcPath) { List providers = - providersForHandlingPlugin(srcFile); + providersForHandlingPlugin(srcPath); switch (providers.size()) { case 1: return providers.get(0); case 0: throw new IllegalArgumentException( "No ServerPluginProvider found/loaded to handle plugin file " - + srcFile.getAbsolutePath()); + + srcPath.toAbsolutePath()); default: - throw new MultipleProvidersForPluginException(srcFile, providers); + throw new MultipleProvidersForPluginException(srcPath, providers); } } private List providersForHandlingPlugin( - final File srcFile) { + final Path srcPath) { List providers = new ArrayList<>(); for (ServerPluginProvider serverPluginProvider : serverPluginProviders) { - boolean handles = serverPluginProvider.handles(srcFile); - log.debug("File {} handled by {} ? => {}", srcFile, + boolean handles = serverPluginProvider.handles(srcPath); + log.debug("File {} handled by {} ? => {}", srcPath, serverPluginProvider.getProviderPluginName(), handles); if (handles) { providers.add(serverPluginProvider); diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin index 4c295fd382..cbcc3cac14 160000 --- a/plugins/cookbook-plugin +++ b/plugins/cookbook-plugin @@ -1 +1 @@ -Subproject commit 4c295fd3829725af769dc3c37db9df1c562e24a3 +Subproject commit cbcc3cac14604ff2194473b38d268dbcfa8f1dcc