Skip to content

Plugins: Consolidate plugin and module loading code #28815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 9, 2018
Merged

Plugins: Consolidate plugin and module loading code #28815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd9ed71
Plugins: Consolidate plugin and module loading code
rjernst Feb 25, 2018
4d6da50
Merge branch 'master' into plugins_module_dedup
rjernst Mar 8, 2018
fc65c54
fix tests
rjernst Mar 9, 2018
9016a0e
fix another test
rjernst Mar 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 23 additions & 39 deletions server/src/main/java/org/elasticsearch/plugins/PluginsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ public PluginsService(Settings settings, Path configPath, Path modulesDirectory,
// TODO: remove this leniency, but tests bogusly rely on it
if (isAccessibleDirectory(pluginsDirectory, logger)) {
checkForFailedPluginRemovals(pluginsDirectory);
List<BundleCollection> plugins = getPluginBundleCollections(pluginsDirectory);
// call findBundles directly to get the meta plugin names
List<BundleCollection> plugins = findBundles(pluginsDirectory, "plugin");
for (final BundleCollection plugin : plugins) {
final Collection<Bundle> bundles = plugin.bundles();
for (final Bundle bundle : bundles) {
Expand Down Expand Up @@ -173,8 +174,9 @@ public PluginsService(Settings settings, Path configPath, Path modulesDirectory,
if (!missingPlugins.isEmpty()) {
final String message = String.format(
Locale.ROOT,
"missing mandatory plugins [%s]",
Strings.collectionToDelimitedString(missingPlugins, ", "));
"missing mandatory plugins [%s], found plugins [%s]",
Strings.collectionToDelimitedString(missingPlugins, ", "),
Strings.collectionToDelimitedString(pluginsNames, ", "));
throw new IllegalStateException(message);
}
}
Expand Down Expand Up @@ -400,25 +402,6 @@ static void verifyCompatibility(PluginInfo info) {
JarHell.checkJavaVersion(info.getName(), info.getJavaVersion());
}

// similar in impl to getPluginBundles, but DO NOT try to make them share code.
// we don't need to inherit all the leniency, and things are different enough.
static Set<Bundle> getModuleBundles(Path modulesDirectory) throws IOException {
// damn leniency
if (Files.notExists(modulesDirectory)) {
return Collections.emptySet();
}
Set<Bundle> bundles = new LinkedHashSet<>();
try (DirectoryStream<Path> stream = Files.newDirectoryStream(modulesDirectory)) {
for (Path module : stream) {
PluginInfo info = PluginInfo.readFromProperties(module);
if (bundles.add(new Bundle(info, module)) == false) {
throw new IllegalStateException("duplicate module: " + info);
}
}
}
return bundles;
}

static void checkForFailedPluginRemovals(final Path pluginsDirectory) throws IOException {
/*
* Check for the existence of a marker file that indicates any plugins are in a garbage state from a failed attempt to remove the
Expand All @@ -440,29 +423,29 @@ static void checkForFailedPluginRemovals(final Path pluginsDirectory) throws IOE
}
}

/**
* Get the plugin bundles from the specified directory.
*
* @param pluginsDirectory the directory
* @return the set of plugin bundles in the specified directory
* @throws IOException if an I/O exception occurs reading the plugin bundles
*/
/** Get bundles for plugins installed in the given modules directory. */
static Set<Bundle> getModuleBundles(Path modulesDirectory) throws IOException {
return findBundles(modulesDirectory, "module").stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet());
}

/** Get bundles for plugins installed in the given plugins directory. */
static Set<Bundle> getPluginBundles(final Path pluginsDirectory) throws IOException {
return getPluginBundleCollections(pluginsDirectory).stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet());
return findBundles(pluginsDirectory, "plugin").stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet());
}

private static List<BundleCollection> getPluginBundleCollections(final Path pluginsDirectory) throws IOException {
// searches subdirectories under the given directory for plugin directories
private static List<BundleCollection> findBundles(final Path directory, String type) throws IOException {
final List<BundleCollection> bundles = new ArrayList<>();
final Set<Bundle> seenBundles = new HashSet<>();
final Tuple<List<Path>, Map<String, List<Path>>> groupedPluginDirs = findGroupedPluginDirs(pluginsDirectory);
final Tuple<List<Path>, Map<String, List<Path>>> groupedPluginDirs = findGroupedPluginDirs(directory);
for (final Path plugin : groupedPluginDirs.v1()) {
final Bundle bundle = bundle(seenBundles, plugin);
final Bundle bundle = readPluginBundle(seenBundles, plugin, type);
bundles.add(bundle);
}
for (final Map.Entry<String, List<Path>> metaPlugin : groupedPluginDirs.v2().entrySet()) {
final List<Bundle> metaPluginBundles = new ArrayList<>();
for (final Path metaPluginPlugin : metaPlugin.getValue()) {
final Bundle bundle = bundle(seenBundles, metaPluginPlugin);
final Bundle bundle = readPluginBundle(seenBundles, metaPluginPlugin, type);
metaPluginBundles.add(bundle);
}
final MetaBundle metaBundle = new MetaBundle(metaPlugin.getKey(), metaPluginBundles);
Expand All @@ -472,18 +455,19 @@ private static List<BundleCollection> getPluginBundleCollections(final Path plug
return bundles;
}

private static Bundle bundle(final Set<Bundle> bundles, final Path plugin) throws IOException {
Loggers.getLogger(PluginsService.class).trace("--- adding plugin [{}]", plugin.toAbsolutePath());
// get a bundle for a single plugin dir
private static Bundle readPluginBundle(final Set<Bundle> bundles, final Path plugin, String type) throws IOException {
Loggers.getLogger(PluginsService.class).trace("--- adding [{}] [{}]", type, plugin.toAbsolutePath());
final PluginInfo info;
try {
info = PluginInfo.readFromProperties(plugin);
} catch (final IOException e) {
throw new IllegalStateException("Could not load plugin descriptor for existing plugin ["
+ plugin.getFileName() + "]. Was the plugin built before 2.0?", e);
throw new IllegalStateException("Could not load plugin descriptor for " + type +
" directory [" + plugin.getFileName() + "]", e);
}
final Bundle bundle = new Bundle(info, plugin);
if (bundles.add(bundle) == false) {
throw new IllegalStateException("duplicate plugin: " + info);
throw new IllegalStateException("duplicate " + type + ": " + info);
}
return bundle;
}
Expand Down
13 changes: 5 additions & 8 deletions server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,9 @@ public void testAdditionalSettingsClash() {
public void testExistingPluginMissingDescriptor() throws Exception {
Path pluginsDir = createTempDir();
Files.createDirectory(pluginsDir.resolve("plugin-missing-descriptor"));
try {
PluginsService.getPluginBundles(pluginsDir);
fail();
} catch (IllegalStateException e) {
assertTrue(e.getMessage(), e.getMessage().contains("Could not load plugin descriptor for existing plugin [plugin-missing-descriptor]"));
}
IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginsService.getPluginBundles(pluginsDir));
assertThat(e.getMessage(),
containsString("Could not load plugin descriptor for plugin directory [plugin-missing-descriptor]"));
}

public void testFilterPlugins() {
Expand All @@ -139,7 +136,7 @@ public void testHiddenFiles() throws IOException {
IllegalStateException.class,
() -> newPluginsService(settings));

final String expected = "Could not load plugin descriptor for existing plugin [.hidden]";
final String expected = "Could not load plugin descriptor for plugin directory [.hidden]";
assertThat(e, hasToString(containsString(expected)));
}

Expand All @@ -158,7 +155,7 @@ public void testDesktopServicesStoreFiles() throws IOException {
assertNotNull(pluginsService);
} else {
final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings));
assertThat(e, hasToString(containsString("Could not load plugin descriptor for existing plugin [.DS_Store]")));
assertThat(e.getMessage(), containsString("Could not load plugin descriptor for plugin directory [.DS_Store]"));
assertNotNull(e.getCause());
assertThat(e.getCause(), instanceOf(FileSystemException.class));
if (Constants.WINDOWS) {
Expand Down