honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Plugins: Consolidate plugin and module loading code (#28815) 

At one point, modules and plugins were very different. But effectively
now they are the same, just from different directories. This commit
unifies the loading methods so they are simply two different
directories. Note that the main codepath to load plugin bundles had
duplication (was not calling getPluginBundles) since previous
refactorings to add meta plugins. Note this change also rewords the
primary exception message when a plugin descriptor is missing, as the
wording asking if the plugin was built before 2.0 isn't really
applicable anymore (it is highly unlikely someone tries to install a 1.x
plugin on any modern version).
This commit is contained in:
Ryan Ernst 2018-03-08 22:49:27 -08:00 committed by GitHub
parent beb22d89c8
commit 62293ec1c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 29 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
server/src/main/java/org/elasticsearch/plugins/PluginsService.java
View File

@ -140,7 +140,8 @@ public class PluginsService extends AbstractComponent {
// 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) {
@ -173,8 +174,9 @@ public class PluginsService extends AbstractComponent {
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);
}
}
@ -400,25 +402,6 @@ public class PluginsService extends AbstractComponent {
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
@ -440,29 +423,29 @@ public class PluginsService extends AbstractComponent {
}
}
/**
* 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
*/
static Set<Bundle> getPluginBundles(final Path pluginsDirectory) throws IOException {
return getPluginBundleCollections(pluginsDirectory).stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet());
/** 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());
}
private static List<BundleCollection> getPluginBundleCollections(final Path pluginsDirectory) throws IOException {
/** Get bundles for plugins installed in the given plugins directory. */
static Set<Bundle> getPluginBundles(final Path pluginsDirectory) throws IOException {
return findBundles(pluginsDirectory, "plugin").stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet());
}
// 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);
@ -472,18 +455,19 @@ public class PluginsService extends AbstractComponent {
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;
}

13
server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java
View File

@ -107,12 +107,9 @@ public class PluginsServiceTests extends ESTestCase {
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() {
@ -139,7 +136,7 @@ public class PluginsServiceTests extends ESTestCase {
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)));
}
@ -158,7 +155,7 @@ public class PluginsServiceTests extends ESTestCase {
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) {