From 2a2179be2a9c480eb7be55000d87bcad9cba0a71 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Fri, 9 Sep 2022 11:42:20 +0200 Subject: [PATCH 1/8] Scanning for Stable Plugin components Stable plugins are using @Extensible and @NamedComponents annotations to mark components to be loaded. This commit is loading extensible classNames from extensibles.json and named components from named_components.json relates #88980 --- libs/build.gradle | 2 +- modules/analysis-common/build.gradle | 2 + .../src/main/java/module-info.java | 2 + .../org/elasticsearch/analysis/common/XX.java | 26 +++++ .../org/elasticsearch/tracing/apm/YY.java | 26 +++++ server/build.gradle | 2 + server/src/main/java/module-info.java | 2 + .../elasticsearch/plugins/PluginBundle.java | 9 +- .../elasticsearch/plugins/PluginsService.java | 46 +++++---- .../scanners/ExtensibleFileReader.java | 57 +++++++++++ .../plugins/scanners/ExtensiblesRegistry.java | 37 ++++++++ .../plugins/scanners/NameToPluginInfo.java | 58 ++++++++++++ .../scanners/NamedComponentScanner.java | 91 ++++++++++++++++++ .../plugins/scanners/PluginInfo.java | 13 +++ .../scanners/StablePluginsRegistry.java | 56 +++++++++++ server/src/main/resources/extensibles.json | 6 ++ .../scanners/ExtensibleFileReaderTests.java | 36 +++++++ .../scanners/NamedComponentScannerTests.java | 94 +++++++++++++++++++ .../scanners/StablePluginsRegistryTests.java | 64 +++++++++++++ .../src/test/resources/named_components.json | 6 ++ .../src/test/resources/test_extensible.json | 6 ++ 21 files changed, 620 insertions(+), 21 deletions(-) create mode 100644 modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java create mode 100644 modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java create mode 100644 server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java create mode 100644 server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java create mode 100644 server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java create mode 100644 server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentScanner.java create mode 100644 server/src/main/java/org/elasticsearch/plugins/scanners/PluginInfo.java create mode 100644 server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java create mode 100644 server/src/main/resources/extensibles.json create mode 100644 server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java create mode 100644 server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentScannerTests.java create mode 100644 server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java create mode 100644 server/src/test/resources/named_components.json create mode 100644 server/src/test/resources/test_extensible.json diff --git a/libs/build.gradle b/libs/build.gradle index fbc946166fb83..f2b7e247fabd8 100644 --- a/libs/build.gradle +++ b/libs/build.gradle @@ -39,5 +39,5 @@ configure(subprojects - project('elasticsearch-log4j')) { } boolean isPluginApi(Project project, Project depProject) { - return project.path.matches(".*elasticsearch-plugin-.*-api") && depProject.path.equals(':libs:elasticsearch-plugin-api') + return project.path.matches(".*elasticsearch-plugin-.*api") } diff --git a/modules/analysis-common/build.gradle b/modules/analysis-common/build.gradle index d1345695d8a13..60a1ebfbbc7d7 100644 --- a/modules/analysis-common/build.gradle +++ b/modules/analysis-common/build.gradle @@ -23,6 +23,8 @@ restResources { dependencies { compileOnly project(':modules:lang-painless:spi') + compileOnly project(':libs:elasticsearch-plugin-analysis-api') + compileOnly project(':libs:elasticsearch-plugin-api') } tasks.named("yamlRestTestV7CompatTransform").configure { task -> diff --git a/modules/analysis-common/src/main/java/module-info.java b/modules/analysis-common/src/main/java/module-info.java index c2cc2b5f2ada7..ef08ba353a99a 100644 --- a/modules/analysis-common/src/main/java/module-info.java +++ b/modules/analysis-common/src/main/java/module-info.java @@ -16,6 +16,8 @@ requires org.apache.logging.log4j; requires org.apache.lucene.core; requires org.apache.lucene.analysis.common; + requires org.elasticsearch.plugin.analysis.api; + requires org.elasticsearch.plugin.api; exports org.elasticsearch.analysis.common; // for painless diff --git a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java new file mode 100644 index 0000000000000..007e97ed4cf8d --- /dev/null +++ b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.analysis.common; + +import org.apache.lucene.analysis.TokenStream; +import org.elasticsearch.plugin.analysis.api.TokenFilterFactory; +import org.elasticsearch.plugin.api.NamedComponent; + +@NamedComponent(name = "xx")// TODO to be removed. test class to see this being picked up by server on startup. +public class XX implements TokenFilterFactory { + @Override + public String name() { + return null; + } + + @Override + public TokenStream create(TokenStream tokenStream) { + return null; + } +} diff --git a/modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java b/modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java new file mode 100644 index 0000000000000..0fd0ba439cd6d --- /dev/null +++ b/modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.tracing.apm; + +import org.apache.lucene.analysis.TokenStream; +import org.elasticsearch.plugin.analysis.api.TokenFilterFactory; +import org.elasticsearch.plugin.api.NamedComponent; + +@NamedComponent(name = "yy")// TODO to be removed. test class to see this being picked up by server on startup. +public class YY implements TokenFilterFactory { + @Override + public String name() { + return null; + } + + @Override + public TokenStream create(TokenStream tokenStream) { + return null; + } +} diff --git a/server/build.gradle b/server/build.gradle index a082e30afbba5..9e1985d5643c5 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -30,6 +30,8 @@ dependencies { api project(':libs:elasticsearch-x-content') api project(":libs:elasticsearch-geo") api project(":libs:elasticsearch-lz4") + api project(":libs:elasticsearch-plugin-api") + api project(":libs:elasticsearch-plugin-analysis-api") implementation project(':libs:elasticsearch-plugin-classloader') diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java index 28391730f7e14..61b28a509d730 100644 --- a/server/src/main/java/module-info.java +++ b/server/src/main/java/module-info.java @@ -22,6 +22,8 @@ requires org.elasticsearch.securesm; requires org.elasticsearch.xcontent; requires org.elasticsearch.logging; + requires org.elasticsearch.plugin.api; + requires org.elasticsearch.plugin.analysis.api; requires com.sun.jna; requires hppc; diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginBundle.java b/server/src/main/java/org/elasticsearch/plugins/PluginBundle.java index cd454015a7f9f..154ffce6ba05f 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginBundle.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginBundle.java @@ -20,14 +20,16 @@ /** * A "bundle" is a group of jars that will be loaded in their own classloader */ -class PluginBundle { +public class PluginBundle { public final PluginDescriptor plugin; + private final Path dir; public final Set urls; public final Set spiUrls; public final Set allUrls; PluginBundle(PluginDescriptor plugin, Path dir) throws IOException { this.plugin = Objects.requireNonNull(plugin); + this.dir = dir; Path spiDir = dir.resolve("spi"); // plugin has defined an explicit api for extension @@ -40,6 +42,10 @@ class PluginBundle { this.allUrls = allUrls; } + public Path getDir() { + return dir; + } + public PluginDescriptor pluginDescriptor() { return this.plugin; } @@ -82,4 +88,5 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(plugin); } + } diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index a15c86b86993d..b294e5febbd5b 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -28,6 +28,8 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.jdk.JarHell; import org.elasticsearch.node.ReportingService; +import org.elasticsearch.plugins.scanners.NameToPluginInfo; +import org.elasticsearch.plugins.scanners.StablePluginsRegistry; import org.elasticsearch.plugins.spi.SPIClassIterator; import java.io.IOException; @@ -102,6 +104,8 @@ record LoadedPlugin(PluginDescriptor descriptor, Plugin instance, ClassLoader lo private final List plugins; private final PluginsAndModules info; + private final StablePluginsRegistry stablePluginsRegistry; + public static final Setting> MANDATORY_SETTING = Setting.listSetting( "plugin.mandatory", Collections.emptyList(), @@ -111,14 +115,14 @@ record LoadedPlugin(PluginDescriptor descriptor, Plugin instance, ClassLoader lo /** * Constructs a new PluginService - * - * @param settings The settings of the system + * @param settings The settings of the system * @param modulesDirectory The directory modules exist in, or null if modules should not be loaded from the filesystem * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem */ public PluginsService(Settings settings, Path configPath, Path modulesDirectory, Path pluginsDirectory) { this.settings = settings; this.configPath = configPath; + this.stablePluginsRegistry = new StablePluginsRegistry(); Set seenBundles = new LinkedHashSet<>(); @@ -409,7 +413,7 @@ private static String extensionConstructorMessage(Class extensi return "constructor for extension [" + extensionClass.getName() + "] of type [" + extensionPointType.getName() + "]"; } - private Plugin loadBundle(PluginBundle bundle, Map loaded) { + private void loadBundle(PluginBundle bundle, Map loaded) { String name = bundle.plugin.getName(); logger.debug(() -> "Loading bundle: " + name); @@ -457,22 +461,26 @@ private Plugin loadBundle(PluginBundle bundle, Map loaded) // that have dependencies with their own SPI endpoints have a chance to load // and initialize them appropriately. privilegedSetContextClassLoader(pluginClassLoader); - - Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), pluginClassLoader); - if (pluginClassLoader != pluginClass.getClassLoader()) { - throw new IllegalStateException( - "Plugin [" - + name - + "] must reference a class loader local Plugin class [" - + bundle.plugin.getClassname() - + "] (class loader [" - + pluginClass.getClassLoader() - + "])" - ); + stablePluginsRegistry.scanBundleForStablePlugins(bundle, pluginClassLoader); + Map namedComponents = stablePluginsRegistry.getNamedComponents(); + // System.out.println(namedComponents); some interim assertions would be good here.. + if (bundle.pluginDescriptor().isStable() == false) { + Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), pluginClassLoader); + if (pluginClassLoader != pluginClass.getClassLoader()) { + throw new IllegalStateException( + "Plugin [" + + name + + "] must reference a class loader local Plugin class [" + + bundle.plugin.getClassname() + + "] (class loader [" + + pluginClass.getClassLoader() + + "])" + ); + } + Plugin plugin = loadPlugin(pluginClass, settings, configPath); + loaded.put(name, new LoadedPlugin(bundle.plugin, plugin, spiLayerAndLoader.loader(), spiLayerAndLoader.layer())); + // return plugin; } - Plugin plugin = loadPlugin(pluginClass, settings, configPath); - loaded.put(name, new LoadedPlugin(bundle.plugin, plugin, spiLayerAndLoader.loader(), spiLayerAndLoader.layer())); - return plugin; } finally { privilegedSetContextClassLoader(cl); } @@ -755,7 +763,7 @@ static final Path[] urlsToPaths(Set urls) { return urls.stream().map(PluginsService::uncheckedToURI).map(PathUtils::get).toArray(Path[]::new); } - static final URI uncheckedToURI(URL url) { + public static final URI uncheckedToURI(URL url) { try { return url.toURI(); } catch (URISyntaxException e) { diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java new file mode 100644 index 0000000000000..a41f35a63bf2d --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import org.elasticsearch.logging.LogManager; +import org.elasticsearch.logging.Logger; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.elasticsearch.xcontent.XContentType.JSON; + +public class ExtensibleFileReader { + private static final Logger logger = LogManager.getLogger(ExtensibleFileReader.class); + + private String extensibleFile; + + public ExtensibleFileReader(String extensibleFile) { + this.extensibleFile = extensibleFile; + } + + public Map readFromFile() { + Map res = new HashMap<>(); + try (InputStream in = /*new BufferedInputStream(*/getClass().getClassLoader().getResourceAsStream(extensibleFile))/*)*/ { + if (in != null) { + try (XContentParser parser = JSON.xContent().createParser(XContentParserConfiguration.EMPTY, in)) { + // validation class exist?? + return parser.mapStrings() + .entrySet() + .stream() + .collect(Collectors.toMap(e -> classNameToSlashes(e.getKey()), e -> classNameToSlashes(e.getValue()))); + // todo decide if dot or /classes names should be used + } + } + + } catch (IOException e) { + logger.error("failed reading extensible file", e); + } + return res; + } + + // todo duplication + private static String classNameToSlashes(String className) { + return className.replace('.', '/'); + } +} diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java new file mode 100644 index 0000000000000..9c5a666ae1f8a --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java @@ -0,0 +1,37 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.util.Map; + +import static org.elasticsearch.core.Strings.format; + +public class ExtensiblesRegistry { + + private static final Logger logger = LogManager.getLogger(ExtensiblesRegistry.class); + + private static final String EXTENSIBLES_FILE = "extensibles.json"; + public static final ExtensiblesRegistry INSTANCE = new ExtensiblesRegistry(EXTENSIBLES_FILE); + + private final ExtensibleFileReader extensibleFileReader; + + ExtensiblesRegistry(String extensiblesFile) { + extensibleFileReader = new ExtensibleFileReader(extensiblesFile); + + Map fromFile = extensibleFileReader.readFromFile(); + if (fromFile.size() > 0) { + logger.debug(() -> format("Loaded extensible from cache file %s", fromFile)); + } + + } + +} diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java b/server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java new file mode 100644 index 0000000000000..e634284233ec8 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java @@ -0,0 +1,58 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +public class NameToPluginInfo { + + // plugin name to NamedPluginInfo + private Map namedPluginInfoMap = new HashMap<>(); + + public void put(String name, PluginInfo pluginInfo) { + namedPluginInfoMap.put(name, pluginInfo); + } + + public void putAll(Map namedPluginInfoMap) { + this.namedPluginInfoMap.putAll(namedPluginInfoMap); + } + + public void put(NameToPluginInfo nameToPluginInfo) { + this.namedPluginInfoMap.putAll(nameToPluginInfo.namedPluginInfoMap); + } + + public PluginInfo getForPluginName(String pluginName) { + return namedPluginInfoMap.get(pluginName); + } + + @Override + public String toString() { + return "NameToPluginInfo{" + "namedPluginInfoMap=" + namedPluginInfoMap + '}'; + } + + public NameToPluginInfo with(String name, PluginInfo pluginInfo) { + put(name, pluginInfo); + return this; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + NameToPluginInfo that = (NameToPluginInfo) o; + return Objects.equals(namedPluginInfoMap, that.namedPluginInfoMap); + } + + @Override + public int hashCode() { + return Objects.hash(namedPluginInfoMap); + } +} diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentScanner.java b/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentScanner.java new file mode 100644 index 0000000000000..0a480b79f720b --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentScanner.java @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.plugins.PluginBundle; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; + +import java.io.BufferedInputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; + +import static java.util.Collections.emptyMap; +import static org.elasticsearch.xcontent.XContentType.JSON; + +public class NamedComponentScanner { + + private Logger logger = LogManager.getLogger(NamedComponentScanner.class); + private static final String NAMED_COMPONENTS_FILE_NAME = "named_components.json"; + private final ExtensiblesRegistry extensiblesRegistry; + + public NamedComponentScanner() { + this.extensiblesRegistry = ExtensiblesRegistry.INSTANCE; + } + + NamedComponentScanner(ExtensiblesRegistry extensiblesRegistry) { + this.extensiblesRegistry = extensiblesRegistry; + } + + public Map findNamedComponents(PluginBundle bundle, ClassLoader pluginClassLoader) { + Path pluginDir = bundle.getDir(); + return findNamedComponents(pluginDir, pluginClassLoader); + } + + // scope for testing + Map findNamedComponents(Path pluginDir, ClassLoader pluginClassLoader) { + try { + Path namedComponent = findNamedComponentCacheFile(pluginDir); + return readFromFile(namedComponent, pluginClassLoader); + } catch (IOException e) { + logger.error("unable to read named components", e); + return emptyMap(); + } + } + + private Path findNamedComponentCacheFile(Path pluginDir) throws IOException { + try (Stream list = Files.list(pluginDir)) { + return list.filter(p -> p.getFileName().equals(NAMED_COMPONENTS_FILE_NAME)).findFirst().get(); + } + } + + private String pathToClassName(String classWithSlashes) { + return classWithSlashes.replace('/', '.'); + } + + @SuppressWarnings("unchecked") + Map readFromFile(Path namedComponent, ClassLoader pluginClassLoader) throws IOException { + Map res = new HashMap<>(); + + try (var json = new BufferedInputStream(Files.newInputStream(namedComponent))) { + try (XContentParser parser = JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { + Map map = parser.map(); + for (Map.Entry fileAsMap : map.entrySet()) { + String extensibleInterface = fileAsMap.getKey(); + + Map components = (Map) fileAsMap.getValue(); + for (Map.Entry nameToComponent : components.entrySet()) { + String name = nameToComponent.getKey(); + String value = (String) nameToComponent.getValue(); + + res.computeIfAbsent(extensibleInterface, k -> new NameToPluginInfo()) + .put(name, new PluginInfo(name, value, pluginClassLoader)); + } + } + } + } + return res; + } +} diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/PluginInfo.java b/server/src/main/java/org/elasticsearch/plugins/scanners/PluginInfo.java new file mode 100644 index 0000000000000..0a4e84e244890 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/PluginInfo.java @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +record PluginInfo(String name, String className, ClassLoader loader) { + +} diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java new file mode 100644 index 0000000000000..ac4d319405edb --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java @@ -0,0 +1,56 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import org.elasticsearch.plugins.PluginBundle; + +import java.util.HashMap; +import java.util.Map; + +public class StablePluginsRegistry { + + /* + A map of an interface/class name marked (effectively) with @Extensible to NameToPluginInfo map + effectively means that an interface which extends another interface marked with @Extensible is also extensible + NameToPluginInfo map is a map of Name to PluginInfo + i.e. + org.elasticsearch.plugin.analysis.api.TokenFilterFactory -> + {"nori" -> {nori, org.elasticserach.plugin.analysis.new_nori.NoriReadingFormFilterFactory, classloaderInstance} + */ + private final Map namedComponents; + private final NamedComponentScanner namedComponentsScanner; + + public StablePluginsRegistry() { + this(new NamedComponentScanner(), new HashMap<>()); + } + + // for testing + StablePluginsRegistry(NamedComponentScanner namedComponentScanner, HashMap namedComponents) { + this.namedComponentsScanner = namedComponentScanner; + this.namedComponents = namedComponents; + } + + public void scanBundleForStablePlugins(PluginBundle bundle, ClassLoader pluginClassLoader) { + Map namedComponentsFromPlugin = namedComponentsScanner.findNamedComponents(bundle, pluginClassLoader); + for (Map.Entry entry : namedComponentsFromPlugin.entrySet()) { + if (namedComponents.containsKey(entry.getKey())) { + NameToPluginInfo nameToPluginInfo = namedComponents.get(entry.getKey()); + nameToPluginInfo.put(entry.getValue()); + } else { + namedComponents.put(entry.getKey(), entry.getValue()); + } + } + } + + // TODO this will be removed. getPluginForName or similar wil be created + public Map getNamedComponents() { + return namedComponents; + } + +} diff --git a/server/src/main/resources/extensibles.json b/server/src/main/resources/extensibles.json new file mode 100644 index 0000000000000..64d123055df92 --- /dev/null +++ b/server/src/main/resources/extensibles.json @@ -0,0 +1,6 @@ +{ + "org.elasticsearch.plugin.analysis.api.AnalyzerFactory":"org.elasticsearch.plugin.analysis.api.AnalyzerFactory", + "org.elasticsearch.plugin.analysis.api.CharFilterFactory":"org.elasticsearch.plugin.analysis.api.CharFilterFactory", + "org.elasticsearch.plugin.analysis.api.TokenFilterFactory":"org.elasticsearch.plugin.analysis.api.TokenFilterFactory", + "org.elasticsearch.plugin.analysis.api.TokenizerFactory":"org.elasticsearch.plugin.analysis.api.TokenizerFactory" +} diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java new file mode 100644 index 0000000000000..28864a0c4b58c --- /dev/null +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import org.elasticsearch.test.ESTestCase; + +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; + +public class ExtensibleFileReaderTests extends ESTestCase { + + public void testLoadingFromFile() { + ExtensibleFileReader extensibleFileReader = new ExtensibleFileReader("test_extensible.json"); + + Map stringStringMap = extensibleFileReader.readFromFile(); + assertThat( + stringStringMap, + equalTo( + Map.of( + "org/elasticsearch/plugins/scanners/extensible_test_classes/ExtensibleClass", + "org/elasticsearch/plugins/scanners/extensible_test_classes/ExtensibleClass", + "org/elasticsearch/plugins/scanners/extensible_test_classes/SubClass", + "org/elasticsearch/plugins/scanners/extensible_test_classes/ExtensibleClass" + ) + ) + ); + + } +} diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentScannerTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentScannerTests.java new file mode 100644 index 0000000000000..0120f7b2be728 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentScannerTests.java @@ -0,0 +1,94 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import org.elasticsearch.core.PathUtils; +import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.Matchers.equalTo; + +public class NamedComponentScannerTests extends ESTestCase { + ExtensiblesRegistry extensiblesRegistry = new ExtensiblesRegistry("file_does_not_exist.txt");// forcing to do classpath scan + NamedComponentScanner namedComponentScanner = new NamedComponentScanner(extensiblesRegistry); + + @SuppressForbidden(reason = "test resource") + public void testReadNamedComponentsFromFile() throws IOException { + final String resource = this.getClass().getClassLoader().getResource("named_components.json").getPath(); + Path namedComponentPath = PathUtils.get(resource); + + Map namedComponents = namedComponentScanner.readFromFile( + namedComponentPath, + NamedComponentScannerTests.class.getClassLoader() + ); + + assertThat( + namedComponents.get("org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface") + .getForPluginName("test_named_component"), + equalTo( + new PluginInfo( + "test_named_component", + "org.elasticsearch.plugins.scanners.named_components_test_classes.TestNamedComponent", + NamedComponentScannerTests.class.getClassLoader() + ) + ) + ); + } + + static byte[] bytes(String str) { + return str.getBytes(UTF_8); + } + + public void testFindNamedComponentInJarWithNamedComponentscacheFile() throws IOException { + final Path tmp = createTempDir(); + final Path pluginDir = tmp.resolve("plugin-dir"); + Files.createDirectories(pluginDir); + Path namedComponentFile = pluginDir.resolve("named_components.json"); + Files.writeString(namedComponentFile, """ + { + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface": { + "a_component": "p.A", + "b_component": "p.B" + } + } + """); + + // jar can be ignored.. cached file is only read atm, verification maybe later? + + ClassLoader classLoader = NamedComponentScannerTests.class.getClassLoader(); + Map namedComponents = namedComponentScanner.findNamedComponents(pluginDir, classLoader); + + assertThat( + namedComponents.get("org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface") + .getForPluginName("b_component"), + equalTo(new PluginInfo("b_component", "p.B", classLoader)) + ); + assertThat( + namedComponents.get("org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface") + .getForPluginName("a_component"), + equalTo(new PluginInfo("a_component", "p.A", classLoader)) + ); + } + + private URL toURL(Path p) { + try { + return p.toUri().toURL(); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + } +} diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java new file mode 100644 index 0000000000000..883b53c08a48c --- /dev/null +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins.scanners; + +import org.elasticsearch.plugins.PluginBundle; +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matchers; +import org.mockito.Mockito; + +import java.util.HashMap; +import java.util.Map; + +import static org.mockito.ArgumentMatchers.any; + +public class StablePluginsRegistryTests extends ESTestCase { + + public void testAddingNamedComponentsFromMultiplePlugins() { + NamedComponentScanner scanner = Mockito.mock(NamedComponentScanner.class); + ClassLoader loader = Mockito.mock(ClassLoader.class); + ClassLoader loader2 = Mockito.mock(ClassLoader.class); + + NameToPluginInfo pluginInfo1 = new NameToPluginInfo().with( + "namedComponentName1", + new PluginInfo("namedComponentName1", "XXClassName", loader) + ); + NameToPluginInfo pluginInfo2 = new NameToPluginInfo().with( + "namedComponentName2", + new PluginInfo("namedComponentName2", "YYClassName", loader) + ); + NameToPluginInfo pluginInfo3 = new NameToPluginInfo().with( + "namedComponentName3", + new PluginInfo("namedComponentName3", "ZZClassName", loader2) + ); + + Mockito.when(scanner.findNamedComponents(any(PluginBundle.class), any(ClassLoader.class))) + .thenReturn(Map.of("extensibleInterfaceName", pluginInfo1)) + .thenReturn(Map.of("extensibleInterfaceName", pluginInfo2)) + .thenReturn(Map.of("extensibleInterfaceName2", pluginInfo3)); + + StablePluginsRegistry registry = new StablePluginsRegistry(scanner, new HashMap<>()); + registry.scanBundleForStablePlugins(Mockito.mock(PluginBundle.class), loader); // bundle 1 + registry.scanBundleForStablePlugins(Mockito.mock(PluginBundle.class), loader); // bundle 2 + registry.scanBundleForStablePlugins(Mockito.mock(PluginBundle.class), loader2); // bundle 3 + + assertThat( + registry.getNamedComponents(), + Matchers.equalTo( + Map.of( + "extensibleInterfaceName", + new NameToPluginInfo().with("namedComponentName1", new PluginInfo("namedComponentName1", "XXClassName", loader)) + .with("namedComponentName2", new PluginInfo("namedComponentName2", "YYClassName", loader)), + "extensibleInterfaceName2", + new NameToPluginInfo().with("namedComponentName3", new PluginInfo("namedComponentName3", "ZZClassName", loader2)) + ) + ) + ); + } +} diff --git a/server/src/test/resources/named_components.json b/server/src/test/resources/named_components.json new file mode 100644 index 0000000000000..087f2410dc097 --- /dev/null +++ b/server/src/test/resources/named_components.json @@ -0,0 +1,6 @@ +{ + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface": { + "test_named_component": "org.elasticsearch.plugins.scanners.named_components_test_classes.TestNamedComponent", + "test_named_component2": "org.elasticsearch.plugins.scanners.named_components_test_classes.TestNamedComponent2" + } +} diff --git a/server/src/test/resources/test_extensible.json b/server/src/test/resources/test_extensible.json new file mode 100644 index 0000000000000..0b3a52bb293fd --- /dev/null +++ b/server/src/test/resources/test_extensible.json @@ -0,0 +1,6 @@ +{ + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass": + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", + "org.elasticsearch.plugins.scanners.extensible_test_classes.SubClass": + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass" +} From f59692a3314060a6597d80841b8e0919bdf268b8 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Fri, 9 Sep 2022 16:14:24 +0200 Subject: [PATCH 2/8] small cleanup --- .../elasticsearch/plugins/PluginsService.java | 9 ++-- .../scanners/ExtensibleFileReader.java | 17 ++---- .../plugins/scanners/ExtensiblesRegistry.java | 52 +++++++++++++++++-- ...Scanner.java => NamedComponentReader.java} | 36 +++++++++---- .../scanners/StablePluginsRegistry.java | 13 +++-- .../scanners/ExtensibleFileReaderTests.java | 8 +-- ...ts.java => NamedComponentReaderTests.java} | 14 ++--- .../scanners/StablePluginsRegistryTests.java | 2 +- 8 files changed, 105 insertions(+), 46 deletions(-) rename server/src/main/java/org/elasticsearch/plugins/scanners/{NamedComponentScanner.java => NamedComponentReader.java} (70%) rename server/src/test/java/org/elasticsearch/plugins/scanners/{NamedComponentScannerTests.java => NamedComponentReaderTests.java} (87%) diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index b294e5febbd5b..f961372638055 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -461,9 +461,12 @@ private void loadBundle(PluginBundle bundle, Map loaded) { // that have dependencies with their own SPI endpoints have a chance to load // and initialize them appropriately. privilegedSetContextClassLoader(pluginClassLoader); - stablePluginsRegistry.scanBundleForStablePlugins(bundle, pluginClassLoader); - Map namedComponents = stablePluginsRegistry.getNamedComponents(); - // System.out.println(namedComponents); some interim assertions would be good here.. + // TODO to be removed - just for debugging + if (bundle.pluginDescriptor().isStable()) { + stablePluginsRegistry.scanBundleForStablePlugins(bundle, pluginClassLoader); + Map namedComponents = stablePluginsRegistry.getNamedComponents(); + // System.out.println(namedComponents); some interim assertions would be good here.. + } if (bundle.pluginDescriptor().isStable() == false) { Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), pluginClassLoader); if (pluginClassLoader != pluginClass.getClassLoader()) { diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java index a41f35a63bf2d..038c7d8b086f2 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java @@ -17,7 +17,6 @@ import java.io.InputStream; import java.util.HashMap; import java.util.Map; -import java.util.stream.Collectors; import static org.elasticsearch.xcontent.XContentType.JSON; @@ -32,26 +31,18 @@ public ExtensibleFileReader(String extensibleFile) { public Map readFromFile() { Map res = new HashMap<>(); - try (InputStream in = /*new BufferedInputStream(*/getClass().getClassLoader().getResourceAsStream(extensibleFile))/*)*/ { + // todo should it be BufferedInputStream ? + try (InputStream in = getClass().getClassLoader().getResourceAsStream(extensibleFile)) { if (in != null) { try (XContentParser parser = JSON.xContent().createParser(XContentParserConfiguration.EMPTY, in)) { - // validation class exist?? - return parser.mapStrings() - .entrySet() - .stream() - .collect(Collectors.toMap(e -> classNameToSlashes(e.getKey()), e -> classNameToSlashes(e.getValue()))); - // todo decide if dot or /classes names should be used + // TODO should we validate the classes actually exist? + return parser.mapStrings(); } } - } catch (IOException e) { logger.error("failed reading extensible file", e); } return res; } - // todo duplication - private static String classNameToSlashes(String className) { - return className.replace('.', '/'); - } } diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java index 9c5a666ae1f8a..c513cb6ae7378 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java @@ -15,6 +15,50 @@ import static org.elasticsearch.core.Strings.format; +/** + * A registry of Extensible interfaces/classes read from extensibles.json file. + * The file is generated during Elasticsearch built time (or commited) + * basing on the classes declared in stable plugins api (i.e. plugin-analysis-api) + * + * This file is present in server jar. + * a class/interface is directly extensible when is marked with @Extensible annotation + * a class/interface can be indirectly extensible when it extends/implements a directly extensible class + * + * Information about extensible interfaces/classes are stored in a map where: + * key and value are the same cannonical name of the class that is directly marked with @Extensible + * or + * key: a cannonical name of the class that is indirectly extensible but extends another extensible class (directly/indirectly) + * value: cannonical name of the class that is directly extensible + * + * The reason for indirectly extensible classes is to allow stable plugin apis to create hierarchies + * + * Example: + * @Extensible + * interface E{ + * public void foo(); + * } + * interface Eprim extends E{ + * } + * + * class Aclass implements E{ + * + * } + * + * @Extensible + * class E2 { + * public void bar(){} + * } + * + * the content of extensibles.json should be + * { + * "E" : "E", + * "Eprim" : "E", + * "A" : "E", + * "E2" : "E2" + * } + * + * @see org.elasticsearch.plugin.api.Extensible + */ public class ExtensiblesRegistry { private static final Logger logger = LogManager.getLogger(ExtensiblesRegistry.class); @@ -23,13 +67,15 @@ public class ExtensiblesRegistry { public static final ExtensiblesRegistry INSTANCE = new ExtensiblesRegistry(EXTENSIBLES_FILE); private final ExtensibleFileReader extensibleFileReader; + // classname (potentially extending/implementing extensible) to interface/class annotated with extensible + private final Map loadedExtensible; ExtensiblesRegistry(String extensiblesFile) { extensibleFileReader = new ExtensibleFileReader(extensiblesFile); - Map fromFile = extensibleFileReader.readFromFile(); - if (fromFile.size() > 0) { - logger.debug(() -> format("Loaded extensible from cache file %s", fromFile)); + this.loadedExtensible = extensibleFileReader.readFromFile(); + if (loadedExtensible.size() > 0) { + logger.debug(() -> format("Loaded extensible from cache file %s", loadedExtensible)); } } diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentScanner.java b/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentReader.java similarity index 70% rename from server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentScanner.java rename to server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentReader.java index 0a480b79f720b..4e26e2b4d9978 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentScanner.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentReader.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.core.Strings; import org.elasticsearch.plugins.PluginBundle; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; @@ -25,17 +26,28 @@ import static java.util.Collections.emptyMap; import static org.elasticsearch.xcontent.XContentType.JSON; -public class NamedComponentScanner { +/** + * Reads named components declared by a plugin in a cache file. + * Cache file is expected to be present in plugin's lib directory + * + * The content of a cache file is a JSON representation of a map where: + * keys -> name of the extensible interface (a class/interface marked with @Extensible) + * values -> a map of name to implementation class name + */ +public class NamedComponentReader { - private Logger logger = LogManager.getLogger(NamedComponentScanner.class); + private Logger logger = LogManager.getLogger(NamedComponentReader.class); private static final String NAMED_COMPONENTS_FILE_NAME = "named_components.json"; + /** + * a registry of known classes marked or indirectly marked (extending marked class) with @Extensible + */ private final ExtensiblesRegistry extensiblesRegistry; - public NamedComponentScanner() { + public NamedComponentReader() { this.extensiblesRegistry = ExtensiblesRegistry.INSTANCE; } - NamedComponentScanner(ExtensiblesRegistry extensiblesRegistry) { + NamedComponentReader(ExtensiblesRegistry extensiblesRegistry) { this.extensiblesRegistry = extensiblesRegistry; } @@ -48,23 +60,25 @@ public Map findNamedComponents(PluginBundle bundle, Cl Map findNamedComponents(Path pluginDir, ClassLoader pluginClassLoader) { try { Path namedComponent = findNamedComponentCacheFile(pluginDir); - return readFromFile(namedComponent, pluginClassLoader); + if (namedComponent != null) { + Map namedComponents = readFromFile(namedComponent, pluginClassLoader); + logger.debug(() -> Strings.format("Plugin in dir %s declared named components %s.", pluginDir, namedComponents)); + + return namedComponents; + } + logger.debug(() -> Strings.format("No named component defined in plugin dir %s", pluginDir)); } catch (IOException e) { logger.error("unable to read named components", e); - return emptyMap(); } + return emptyMap(); } private Path findNamedComponentCacheFile(Path pluginDir) throws IOException { try (Stream list = Files.list(pluginDir)) { - return list.filter(p -> p.getFileName().equals(NAMED_COMPONENTS_FILE_NAME)).findFirst().get(); + return list.filter(p -> p.getFileName().toString().equals(NAMED_COMPONENTS_FILE_NAME)).findFirst().orElse(null); } } - private String pathToClassName(String classWithSlashes) { - return classWithSlashes.replace('/', '.'); - } - @SuppressWarnings("unchecked") Map readFromFile(Path namedComponent, ClassLoader pluginClassLoader) throws IOException { Map res = new HashMap<>(); diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java index ac4d319405edb..4a80809e67905 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java @@ -13,6 +13,11 @@ import java.util.HashMap; import java.util.Map; +/** + * A registry of classes declared by plugins as named components. + * Named components are classes annotated with @NamedComponent(name) and can be referred later by a name given in this annotation. + * Named components implement/extend Extensibles (classes/interfaces marked with @Extensible) + */ public class StablePluginsRegistry { /* @@ -24,15 +29,15 @@ effectively means that an interface which extends another interface marked with {"nori" -> {nori, org.elasticserach.plugin.analysis.new_nori.NoriReadingFormFilterFactory, classloaderInstance} */ private final Map namedComponents; - private final NamedComponentScanner namedComponentsScanner; + private final NamedComponentReader namedComponentsScanner; public StablePluginsRegistry() { - this(new NamedComponentScanner(), new HashMap<>()); + this(new NamedComponentReader(), new HashMap<>()); } // for testing - StablePluginsRegistry(NamedComponentScanner namedComponentScanner, HashMap namedComponents) { - this.namedComponentsScanner = namedComponentScanner; + StablePluginsRegistry(NamedComponentReader namedComponentReader, HashMap namedComponents) { + this.namedComponentsScanner = namedComponentReader; this.namedComponents = namedComponents; } diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java index 28864a0c4b58c..0c6c1940722e0 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java @@ -24,10 +24,10 @@ public void testLoadingFromFile() { stringStringMap, equalTo( Map.of( - "org/elasticsearch/plugins/scanners/extensible_test_classes/ExtensibleClass", - "org/elasticsearch/plugins/scanners/extensible_test_classes/ExtensibleClass", - "org/elasticsearch/plugins/scanners/extensible_test_classes/SubClass", - "org/elasticsearch/plugins/scanners/extensible_test_classes/ExtensibleClass" + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", + "org.elasticsearch.plugins.scanners.extensible_test_classes.SubClass", + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass" ) ) ); diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentScannerTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java similarity index 87% rename from server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentScannerTests.java rename to server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java index 0120f7b2be728..19a0de373bdd1 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentScannerTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java @@ -22,18 +22,18 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.Matchers.equalTo; -public class NamedComponentScannerTests extends ESTestCase { +public class NamedComponentReaderTests extends ESTestCase { ExtensiblesRegistry extensiblesRegistry = new ExtensiblesRegistry("file_does_not_exist.txt");// forcing to do classpath scan - NamedComponentScanner namedComponentScanner = new NamedComponentScanner(extensiblesRegistry); + NamedComponentReader namedComponentReader = new NamedComponentReader(extensiblesRegistry); @SuppressForbidden(reason = "test resource") public void testReadNamedComponentsFromFile() throws IOException { final String resource = this.getClass().getClassLoader().getResource("named_components.json").getPath(); Path namedComponentPath = PathUtils.get(resource); - Map namedComponents = namedComponentScanner.readFromFile( + Map namedComponents = namedComponentReader.readFromFile( namedComponentPath, - NamedComponentScannerTests.class.getClassLoader() + NamedComponentReaderTests.class.getClassLoader() ); assertThat( @@ -43,7 +43,7 @@ public void testReadNamedComponentsFromFile() throws IOException { new PluginInfo( "test_named_component", "org.elasticsearch.plugins.scanners.named_components_test_classes.TestNamedComponent", - NamedComponentScannerTests.class.getClassLoader() + NamedComponentReaderTests.class.getClassLoader() ) ) ); @@ -69,8 +69,8 @@ public void testFindNamedComponentInJarWithNamedComponentscacheFile() throws IOE // jar can be ignored.. cached file is only read atm, verification maybe later? - ClassLoader classLoader = NamedComponentScannerTests.class.getClassLoader(); - Map namedComponents = namedComponentScanner.findNamedComponents(pluginDir, classLoader); + ClassLoader classLoader = NamedComponentReaderTests.class.getClassLoader(); + Map namedComponents = namedComponentReader.findNamedComponents(pluginDir, classLoader); assertThat( namedComponents.get("org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface") diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java index 883b53c08a48c..aa3d918dffa1f 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java @@ -21,7 +21,7 @@ public class StablePluginsRegistryTests extends ESTestCase { public void testAddingNamedComponentsFromMultiplePlugins() { - NamedComponentScanner scanner = Mockito.mock(NamedComponentScanner.class); + NamedComponentReader scanner = Mockito.mock(NamedComponentReader.class); ClassLoader loader = Mockito.mock(ClassLoader.class); ClassLoader loader2 = Mockito.mock(ClassLoader.class); From c5089772d33f04087fc53d84b5fbaec71870a194 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Fri, 9 Sep 2022 16:24:37 +0200 Subject: [PATCH 3/8] remove example use --- modules/analysis-common/build.gradle | 2 - .../src/main/java/module-info.java | 2 - .../org/elasticsearch/analysis/common/XX.java | 26 ---------- .../org/elasticsearch/tracing/apm/YY.java | 26 ---------- .../elasticsearch/plugins/PluginsService.java | 49 +++++++------------ 5 files changed, 19 insertions(+), 86 deletions(-) delete mode 100644 modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java delete mode 100644 modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java diff --git a/modules/analysis-common/build.gradle b/modules/analysis-common/build.gradle index 60a1ebfbbc7d7..d1345695d8a13 100644 --- a/modules/analysis-common/build.gradle +++ b/modules/analysis-common/build.gradle @@ -23,8 +23,6 @@ restResources { dependencies { compileOnly project(':modules:lang-painless:spi') - compileOnly project(':libs:elasticsearch-plugin-analysis-api') - compileOnly project(':libs:elasticsearch-plugin-api') } tasks.named("yamlRestTestV7CompatTransform").configure { task -> diff --git a/modules/analysis-common/src/main/java/module-info.java b/modules/analysis-common/src/main/java/module-info.java index ef08ba353a99a..c2cc2b5f2ada7 100644 --- a/modules/analysis-common/src/main/java/module-info.java +++ b/modules/analysis-common/src/main/java/module-info.java @@ -16,8 +16,6 @@ requires org.apache.logging.log4j; requires org.apache.lucene.core; requires org.apache.lucene.analysis.common; - requires org.elasticsearch.plugin.analysis.api; - requires org.elasticsearch.plugin.api; exports org.elasticsearch.analysis.common; // for painless diff --git a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java deleted file mode 100644 index 007e97ed4cf8d..0000000000000 --- a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/XX.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.analysis.common; - -import org.apache.lucene.analysis.TokenStream; -import org.elasticsearch.plugin.analysis.api.TokenFilterFactory; -import org.elasticsearch.plugin.api.NamedComponent; - -@NamedComponent(name = "xx")// TODO to be removed. test class to see this being picked up by server on startup. -public class XX implements TokenFilterFactory { - @Override - public String name() { - return null; - } - - @Override - public TokenStream create(TokenStream tokenStream) { - return null; - } -} diff --git a/modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java b/modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java deleted file mode 100644 index 0fd0ba439cd6d..0000000000000 --- a/modules/apm/src/main/java/org/elasticsearch/tracing/apm/YY.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.tracing.apm; - -import org.apache.lucene.analysis.TokenStream; -import org.elasticsearch.plugin.analysis.api.TokenFilterFactory; -import org.elasticsearch.plugin.api.NamedComponent; - -@NamedComponent(name = "yy")// TODO to be removed. test class to see this being picked up by server on startup. -public class YY implements TokenFilterFactory { - @Override - public String name() { - return null; - } - - @Override - public TokenStream create(TokenStream tokenStream) { - return null; - } -} diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index f961372638055..a15c86b86993d 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -28,8 +28,6 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.jdk.JarHell; import org.elasticsearch.node.ReportingService; -import org.elasticsearch.plugins.scanners.NameToPluginInfo; -import org.elasticsearch.plugins.scanners.StablePluginsRegistry; import org.elasticsearch.plugins.spi.SPIClassIterator; import java.io.IOException; @@ -104,8 +102,6 @@ record LoadedPlugin(PluginDescriptor descriptor, Plugin instance, ClassLoader lo private final List plugins; private final PluginsAndModules info; - private final StablePluginsRegistry stablePluginsRegistry; - public static final Setting> MANDATORY_SETTING = Setting.listSetting( "plugin.mandatory", Collections.emptyList(), @@ -115,14 +111,14 @@ record LoadedPlugin(PluginDescriptor descriptor, Plugin instance, ClassLoader lo /** * Constructs a new PluginService - * @param settings The settings of the system + * + * @param settings The settings of the system * @param modulesDirectory The directory modules exist in, or null if modules should not be loaded from the filesystem * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem */ public PluginsService(Settings settings, Path configPath, Path modulesDirectory, Path pluginsDirectory) { this.settings = settings; this.configPath = configPath; - this.stablePluginsRegistry = new StablePluginsRegistry(); Set seenBundles = new LinkedHashSet<>(); @@ -413,7 +409,7 @@ private static String extensionConstructorMessage(Class extensi return "constructor for extension [" + extensionClass.getName() + "] of type [" + extensionPointType.getName() + "]"; } - private void loadBundle(PluginBundle bundle, Map loaded) { + private Plugin loadBundle(PluginBundle bundle, Map loaded) { String name = bundle.plugin.getName(); logger.debug(() -> "Loading bundle: " + name); @@ -461,29 +457,22 @@ private void loadBundle(PluginBundle bundle, Map loaded) { // that have dependencies with their own SPI endpoints have a chance to load // and initialize them appropriately. privilegedSetContextClassLoader(pluginClassLoader); - // TODO to be removed - just for debugging - if (bundle.pluginDescriptor().isStable()) { - stablePluginsRegistry.scanBundleForStablePlugins(bundle, pluginClassLoader); - Map namedComponents = stablePluginsRegistry.getNamedComponents(); - // System.out.println(namedComponents); some interim assertions would be good here.. - } - if (bundle.pluginDescriptor().isStable() == false) { - Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), pluginClassLoader); - if (pluginClassLoader != pluginClass.getClassLoader()) { - throw new IllegalStateException( - "Plugin [" - + name - + "] must reference a class loader local Plugin class [" - + bundle.plugin.getClassname() - + "] (class loader [" - + pluginClass.getClassLoader() - + "])" - ); - } - Plugin plugin = loadPlugin(pluginClass, settings, configPath); - loaded.put(name, new LoadedPlugin(bundle.plugin, plugin, spiLayerAndLoader.loader(), spiLayerAndLoader.layer())); - // return plugin; + + Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), pluginClassLoader); + if (pluginClassLoader != pluginClass.getClassLoader()) { + throw new IllegalStateException( + "Plugin [" + + name + + "] must reference a class loader local Plugin class [" + + bundle.plugin.getClassname() + + "] (class loader [" + + pluginClass.getClassLoader() + + "])" + ); } + Plugin plugin = loadPlugin(pluginClass, settings, configPath); + loaded.put(name, new LoadedPlugin(bundle.plugin, plugin, spiLayerAndLoader.loader(), spiLayerAndLoader.layer())); + return plugin; } finally { privilegedSetContextClassLoader(cl); } @@ -766,7 +755,7 @@ static final Path[] urlsToPaths(Set urls) { return urls.stream().map(PluginsService::uncheckedToURI).map(PathUtils::get).toArray(Path[]::new); } - public static final URI uncheckedToURI(URL url) { + static final URI uncheckedToURI(URL url) { try { return url.toURI(); } catch (URISyntaxException e) { From 515071b21b6293e0ff0737eaf88b9d1ad1bdcd36 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Fri, 9 Sep 2022 16:27:13 +0200 Subject: [PATCH 4/8] Update docs/changelog/89969.yaml --- docs/changelog/89969.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/89969.yaml diff --git a/docs/changelog/89969.yaml b/docs/changelog/89969.yaml new file mode 100644 index 0000000000000..513f516cc59a9 --- /dev/null +++ b/docs/changelog/89969.yaml @@ -0,0 +1,5 @@ +pr: 89969 +summary: "[Stable plugin API] Load plugin named components" +area: Infra/Plugins +type: enhancement +issues: [] From 58726df0cd6647d5fbbddc04a5d5ba3f9975405e Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Fri, 9 Sep 2022 16:40:19 +0200 Subject: [PATCH 5/8] fix javadoc --- .../plugins/scanners/ExtensiblesRegistry.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java index c513cb6ae7378..1ca516c675a32 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java @@ -33,7 +33,8 @@ * The reason for indirectly extensible classes is to allow stable plugin apis to create hierarchies * * Example: - * @Extensible + * 
+ * @Extensible
  * interface E{
  *     public void foo();
  * }
@@ -44,11 +45,11 @@
  *
  * }
  *
- * @Extensible
+ * @Extensible
  * class E2 {
  *     public void bar(){}
  * }
- *
+ *
* the content of extensibles.json should be * { * "E" : "E", From e722af2450ae6b4f2fe094527a17a1ce298b4d33 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Mon, 12 Sep 2022 11:26:53 +0200 Subject: [PATCH 6/8] code review followup --- ...alDistributionModuleCheckTaskProvider.java | 2 + .../plugins/scanners/ExtensiblesRegistry.java | 8 ++-- .../plugins/scanners/NameToPluginInfo.java | 44 +++++-------------- .../scanners/NamedComponentReader.java | 40 ++++++++++------- .../scanners/StablePluginsRegistry.java | 9 +--- .../plugins/scanner}/extensibles.json | 0 .../scanners/ExtensibleFileReaderTests.java | 5 ++- .../scanners/NamedComponentReaderTests.java | 23 +++++++--- .../scanners/StablePluginsRegistryTests.java | 12 ++--- .../src/test/resources/test_extensible.json | 4 +- 10 files changed, 73 insertions(+), 74 deletions(-) rename server/src/main/resources/{ => org/elasticsearch/plugins/scanner}/extensibles.json (100%) diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java index b659f1efeece8..0a29741be8937 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java @@ -54,6 +54,8 @@ public class InternalDistributionModuleCheckTaskProvider { "org.elasticsearch.geo", "org.elasticsearch.logging", "org.elasticsearch.lz4", + "org.elasticsearch.plugin.analysis.api", + "org.elasticsearch.plugin.api", "org.elasticsearch.pluginclassloader", "org.elasticsearch.securesm", "org.elasticsearch.server", diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java index 1ca516c675a32..49e2aeb5aa862 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java @@ -64,21 +64,23 @@ public class ExtensiblesRegistry { private static final Logger logger = LogManager.getLogger(ExtensiblesRegistry.class); - private static final String EXTENSIBLES_FILE = "extensibles.json"; + private static final String EXTENSIBLES_FILE = "org/elasticsearch/plugins/scanner/extensibles.json"; public static final ExtensiblesRegistry INSTANCE = new ExtensiblesRegistry(EXTENSIBLES_FILE); - private final ExtensibleFileReader extensibleFileReader; // classname (potentially extending/implementing extensible) to interface/class annotated with extensible private final Map loadedExtensible; ExtensiblesRegistry(String extensiblesFile) { - extensibleFileReader = new ExtensibleFileReader(extensiblesFile); + ExtensibleFileReader extensibleFileReader = new ExtensibleFileReader(extensiblesFile); this.loadedExtensible = extensibleFileReader.readFromFile(); if (loadedExtensible.size() > 0) { logger.debug(() -> format("Loaded extensible from cache file %s", loadedExtensible)); } + } + public boolean hasExtensible(String extensibleName) { + return loadedExtensible.containsKey(extensibleName); } } diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java b/server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java index e634284233ec8..b90b704b784f7 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/NameToPluginInfo.java @@ -10,49 +10,29 @@ import java.util.HashMap; import java.util.Map; -import java.util.Objects; -public class NameToPluginInfo { +public record NameToPluginInfo(Map nameToPluginInfoMap) { - // plugin name to NamedPluginInfo - private Map namedPluginInfoMap = new HashMap<>(); - - public void put(String name, PluginInfo pluginInfo) { - namedPluginInfoMap.put(name, pluginInfo); - } - - public void putAll(Map namedPluginInfoMap) { - this.namedPluginInfoMap.putAll(namedPluginInfoMap); - } - - public void put(NameToPluginInfo nameToPluginInfo) { - this.namedPluginInfoMap.putAll(nameToPluginInfo.namedPluginInfoMap); + public NameToPluginInfo() { + this(new HashMap<>()); } - public PluginInfo getForPluginName(String pluginName) { - return namedPluginInfoMap.get(pluginName); + public NameToPluginInfo put(String name, PluginInfo pluginInfo) { + nameToPluginInfoMap.put(name, pluginInfo); + return this; } - @Override - public String toString() { - return "NameToPluginInfo{" + "namedPluginInfoMap=" + namedPluginInfoMap + '}'; + public void putAll(Map namedPluginInfoMap) { + this.nameToPluginInfoMap.putAll(namedPluginInfoMap); } - public NameToPluginInfo with(String name, PluginInfo pluginInfo) { - put(name, pluginInfo); + public NameToPluginInfo put(NameToPluginInfo nameToPluginInfo) { + putAll(nameToPluginInfo.nameToPluginInfoMap); return this; } - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - NameToPluginInfo that = (NameToPluginInfo) o; - return Objects.equals(namedPluginInfoMap, that.namedPluginInfoMap); + public PluginInfo getForPluginName(String pluginName) { + return nameToPluginInfoMap.get(pluginName); } - @Override - public int hashCode() { - return Objects.hash(namedPluginInfoMap); - } } diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentReader.java b/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentReader.java index 4e26e2b4d9978..609a7b104f271 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentReader.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/NamedComponentReader.java @@ -12,7 +12,6 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.core.Strings; import org.elasticsearch.plugins.PluginBundle; -import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; import java.io.BufferedInputStream; @@ -29,7 +28,7 @@ /** * Reads named components declared by a plugin in a cache file. * Cache file is expected to be present in plugin's lib directory - * + *

* The content of a cache file is a JSON representation of a map where: * keys -> name of the extensible interface (a class/interface marked with @Extensible) * values -> a map of name to implementation class name @@ -44,7 +43,7 @@ public class NamedComponentReader { private final ExtensiblesRegistry extensiblesRegistry; public NamedComponentReader() { - this.extensiblesRegistry = ExtensiblesRegistry.INSTANCE; + this(ExtensiblesRegistry.INSTANCE); } NamedComponentReader(ExtensiblesRegistry extensiblesRegistry) { @@ -83,23 +82,30 @@ private Path findNamedComponentCacheFile(Path pluginDir) throws IOException { Map readFromFile(Path namedComponent, ClassLoader pluginClassLoader) throws IOException { Map res = new HashMap<>(); - try (var json = new BufferedInputStream(Files.newInputStream(namedComponent))) { - try (XContentParser parser = JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { - Map map = parser.map(); - for (Map.Entry fileAsMap : map.entrySet()) { - String extensibleInterface = fileAsMap.getKey(); - - Map components = (Map) fileAsMap.getValue(); - for (Map.Entry nameToComponent : components.entrySet()) { - String name = nameToComponent.getKey(); - String value = (String) nameToComponent.getValue(); - - res.computeIfAbsent(extensibleInterface, k -> new NameToPluginInfo()) - .put(name, new PluginInfo(name, value, pluginClassLoader)); - } + try ( + var json = new BufferedInputStream(Files.newInputStream(namedComponent)); + var parser = JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json) + ) { + Map map = parser.map(); + for (Map.Entry fileAsMap : map.entrySet()) { + String extensibleInterface = fileAsMap.getKey(); + validateExtensible(extensibleInterface); + Map components = (Map) fileAsMap.getValue(); + for (Map.Entry nameToComponent : components.entrySet()) { + String name = nameToComponent.getKey(); + String value = (String) nameToComponent.getValue(); + + res.computeIfAbsent(extensibleInterface, k -> new NameToPluginInfo()) + .put(name, new PluginInfo(name, value, pluginClassLoader)); } } } return res; } + + private void validateExtensible(String extensibleInterface) { + if (extensiblesRegistry.hasExtensible(extensibleInterface) == false) { + throw new IllegalStateException("Unknown extensible name " + extensibleInterface); + } + } } diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java index 4a80809e67905..1ab1bf825a372 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/StablePluginsRegistry.java @@ -44,16 +44,11 @@ public StablePluginsRegistry() { public void scanBundleForStablePlugins(PluginBundle bundle, ClassLoader pluginClassLoader) { Map namedComponentsFromPlugin = namedComponentsScanner.findNamedComponents(bundle, pluginClassLoader); for (Map.Entry entry : namedComponentsFromPlugin.entrySet()) { - if (namedComponents.containsKey(entry.getKey())) { - NameToPluginInfo nameToPluginInfo = namedComponents.get(entry.getKey()); - nameToPluginInfo.put(entry.getValue()); - } else { - namedComponents.put(entry.getKey(), entry.getValue()); - } + namedComponents.compute(entry.getKey(), (k, v) -> v != null ? v.put(entry.getValue()) : entry.getValue()); } } - // TODO this will be removed. getPluginForName or similar wil be created + // TODO this will be removed. getPluginForName or similar will be created public Map getNamedComponents() { return namedComponents; } diff --git a/server/src/main/resources/extensibles.json b/server/src/main/resources/org/elasticsearch/plugins/scanner/extensibles.json similarity index 100% rename from server/src/main/resources/extensibles.json rename to server/src/main/resources/org/elasticsearch/plugins/scanner/extensibles.json diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java index 0c6c1940722e0..f69372e7f58e4 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java @@ -27,10 +27,11 @@ public void testLoadingFromFile() { "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", "org.elasticsearch.plugins.scanners.extensible_test_classes.SubClass", - "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass" + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface" , + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface" ) ) ); - } } diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java index 19a0de373bdd1..597ebea670920 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java @@ -19,11 +19,10 @@ import java.nio.file.Path; import java.util.Map; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.Matchers.equalTo; public class NamedComponentReaderTests extends ESTestCase { - ExtensiblesRegistry extensiblesRegistry = new ExtensiblesRegistry("file_does_not_exist.txt");// forcing to do classpath scan + ExtensiblesRegistry extensiblesRegistry = new ExtensiblesRegistry("test_extensible.json"); NamedComponentReader namedComponentReader = new NamedComponentReader(extensiblesRegistry); @SuppressForbidden(reason = "test resource") @@ -49,8 +48,22 @@ public void testReadNamedComponentsFromFile() throws IOException { ); } - static byte[] bytes(String str) { - return str.getBytes(UTF_8); + public void testUnknownExtensible() throws IOException { + final Path tmp = createTempDir(); + final Path pluginDir = tmp.resolve("plugin-dir"); + Files.createDirectories(pluginDir); + Path namedComponentFile = pluginDir.resolve("named_components.json"); + Files.writeString(namedComponentFile, """ + { + "org.elasticsearch.plugins.scanners.extensible_test_classes.UnknownExtensible": { + "a_component": "p.A", + "b_component": "p.B" + } + } + """); + + ClassLoader classLoader = NamedComponentReaderTests.class.getClassLoader(); + expectThrows(IllegalStateException.class, () -> namedComponentReader.findNamedComponents(pluginDir, classLoader)); } public void testFindNamedComponentInJarWithNamedComponentscacheFile() throws IOException { @@ -67,8 +80,6 @@ public void testFindNamedComponentInJarWithNamedComponentscacheFile() throws IOE } """); - // jar can be ignored.. cached file is only read atm, verification maybe later? - ClassLoader classLoader = NamedComponentReaderTests.class.getClassLoader(); Map namedComponents = namedComponentReader.findNamedComponents(pluginDir, classLoader); diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java index aa3d918dffa1f..38b1e955cbc6c 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/StablePluginsRegistryTests.java @@ -25,15 +25,15 @@ public void testAddingNamedComponentsFromMultiplePlugins() { ClassLoader loader = Mockito.mock(ClassLoader.class); ClassLoader loader2 = Mockito.mock(ClassLoader.class); - NameToPluginInfo pluginInfo1 = new NameToPluginInfo().with( + NameToPluginInfo pluginInfo1 = new NameToPluginInfo().put( "namedComponentName1", new PluginInfo("namedComponentName1", "XXClassName", loader) ); - NameToPluginInfo pluginInfo2 = new NameToPluginInfo().with( + NameToPluginInfo pluginInfo2 = new NameToPluginInfo().put( "namedComponentName2", new PluginInfo("namedComponentName2", "YYClassName", loader) ); - NameToPluginInfo pluginInfo3 = new NameToPluginInfo().with( + NameToPluginInfo pluginInfo3 = new NameToPluginInfo().put( "namedComponentName3", new PluginInfo("namedComponentName3", "ZZClassName", loader2) ); @@ -53,10 +53,10 @@ public void testAddingNamedComponentsFromMultiplePlugins() { Matchers.equalTo( Map.of( "extensibleInterfaceName", - new NameToPluginInfo().with("namedComponentName1", new PluginInfo("namedComponentName1", "XXClassName", loader)) - .with("namedComponentName2", new PluginInfo("namedComponentName2", "YYClassName", loader)), + new NameToPluginInfo().put("namedComponentName1", new PluginInfo("namedComponentName1", "XXClassName", loader)) + .put("namedComponentName2", new PluginInfo("namedComponentName2", "YYClassName", loader)), "extensibleInterfaceName2", - new NameToPluginInfo().with("namedComponentName3", new PluginInfo("namedComponentName3", "ZZClassName", loader2)) + new NameToPluginInfo().put("namedComponentName3", new PluginInfo("namedComponentName3", "ZZClassName", loader2)) ) ) ); diff --git a/server/src/test/resources/test_extensible.json b/server/src/test/resources/test_extensible.json index 0b3a52bb293fd..0910cbadfd3a2 100644 --- a/server/src/test/resources/test_extensible.json +++ b/server/src/test/resources/test_extensible.json @@ -2,5 +2,7 @@ "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass": "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", "org.elasticsearch.plugins.scanners.extensible_test_classes.SubClass": - "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass" + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface" : + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface" } From fadaf34df73ffa886834071ba1bb7c3b21431aa8 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Mon, 12 Sep 2022 12:14:18 +0200 Subject: [PATCH 7/8] path to resources --- .../elasticsearch/plugins/scanners/ExtensibleFileReader.java | 2 +- .../elasticsearch/plugins/scanners/ExtensiblesRegistry.java | 2 +- .../plugins/scanners/ExtensibleFileReaderTests.java | 4 ++-- .../plugins/scanners/NamedComponentReaderTests.java | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java index 038c7d8b086f2..558dd723266ca 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensibleFileReader.java @@ -32,7 +32,7 @@ public ExtensibleFileReader(String extensibleFile) { public Map readFromFile() { Map res = new HashMap<>(); // todo should it be BufferedInputStream ? - try (InputStream in = getClass().getClassLoader().getResourceAsStream(extensibleFile)) { + try (InputStream in = getClass().getResourceAsStream(extensibleFile)) { if (in != null) { try (XContentParser parser = JSON.xContent().createParser(XContentParserConfiguration.EMPTY, in)) { // TODO should we validate the classes actually exist? diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java index 49e2aeb5aa862..ef561a5bed7c7 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java @@ -64,7 +64,7 @@ public class ExtensiblesRegistry { private static final Logger logger = LogManager.getLogger(ExtensiblesRegistry.class); - private static final String EXTENSIBLES_FILE = "org/elasticsearch/plugins/scanner/extensibles.json"; + private static final String EXTENSIBLES_FILE = "/org/elasticsearch/plugins/scanner/extensibles.json"; public static final ExtensiblesRegistry INSTANCE = new ExtensiblesRegistry(EXTENSIBLES_FILE); // classname (potentially extending/implementing extensible) to interface/class annotated with extensible diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java index f69372e7f58e4..c177b2474df48 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/ExtensibleFileReaderTests.java @@ -17,7 +17,7 @@ public class ExtensibleFileReaderTests extends ESTestCase { public void testLoadingFromFile() { - ExtensibleFileReader extensibleFileReader = new ExtensibleFileReader("test_extensible.json"); + ExtensibleFileReader extensibleFileReader = new ExtensibleFileReader("/test_extensible.json"); Map stringStringMap = extensibleFileReader.readFromFile(); assertThat( @@ -28,7 +28,7 @@ public void testLoadingFromFile() { "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", "org.elasticsearch.plugins.scanners.extensible_test_classes.SubClass", "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleClass", - "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface" , + "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface", "org.elasticsearch.plugins.scanners.extensible_test_classes.ExtensibleInterface" ) ) diff --git a/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java index 597ebea670920..428f0239d97ff 100644 --- a/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/scanners/NamedComponentReaderTests.java @@ -22,7 +22,7 @@ import static org.hamcrest.Matchers.equalTo; public class NamedComponentReaderTests extends ESTestCase { - ExtensiblesRegistry extensiblesRegistry = new ExtensiblesRegistry("test_extensible.json"); + ExtensiblesRegistry extensiblesRegistry = new ExtensiblesRegistry("/test_extensible.json"); NamedComponentReader namedComponentReader = new NamedComponentReader(extensiblesRegistry); @SuppressForbidden(reason = "test resource") From 6534f3448d4ceb46157ddad6a734ec620407dafa Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Mon, 12 Sep 2022 15:20:36 +0200 Subject: [PATCH 8/8] move file --- .../org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java | 2 +- .../plugins/{scanner => scanners}/extensibles.json | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename server/src/main/resources/org/elasticsearch/plugins/{scanner => scanners}/extensibles.json (100%) diff --git a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java index ef561a5bed7c7..dc40bd6fcd8e2 100644 --- a/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java +++ b/server/src/main/java/org/elasticsearch/plugins/scanners/ExtensiblesRegistry.java @@ -64,7 +64,7 @@ public class ExtensiblesRegistry { private static final Logger logger = LogManager.getLogger(ExtensiblesRegistry.class); - private static final String EXTENSIBLES_FILE = "/org/elasticsearch/plugins/scanner/extensibles.json"; + private static final String EXTENSIBLES_FILE = "/org/elasticsearch/plugins/scanners/extensibles.json"; public static final ExtensiblesRegistry INSTANCE = new ExtensiblesRegistry(EXTENSIBLES_FILE); // classname (potentially extending/implementing extensible) to interface/class annotated with extensible diff --git a/server/src/main/resources/org/elasticsearch/plugins/scanner/extensibles.json b/server/src/main/resources/org/elasticsearch/plugins/scanners/extensibles.json similarity index 100% rename from server/src/main/resources/org/elasticsearch/plugins/scanner/extensibles.json rename to server/src/main/resources/org/elasticsearch/plugins/scanners/extensibles.json