You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/11/21 20:45:24 UTC
[GitHub] [kafka] C0urante commented on a diff in pull request #12805: KAFKA-12610: Implement PluginClassLoader::getResource
C0urante commented on code in PR #12805:
URL: https://github.com/apache/kafka/pull/12805#discussion_r1028470717
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoaderTest.java:
##########
@@ -118,7 +119,8 @@ public void testLoadingMixOfValidAndInvalidPlugins() throws Exception {
for (String sourceJar : TestPlugins.pluginPath()) {
Path source = new File(sourceJar).toPath();
- Files.copy(source, pluginPath.resolve(source.getFileName()));
+ Files.copy(source, pluginPath.resolve(source.getFileName()),
+ StandardCopyOption.REPLACE_EXISTING);
Review Comment:
OOC, why is this added? Shouldn't the temporary `pluginDir` folder be cleaned up after test runs?
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginClassLoader.java:
##########
@@ -81,6 +82,17 @@ public String toString() {
return "PluginClassLoader{pluginLocation=" + pluginLocation + "}";
}
+ @Override
+ public URL getResource(String name) {
+ Objects.requireNonNull(name);
+
+ URL url = findResource(name);
Review Comment:
Probably worth adding a brief comment about how this preserves classpath loading isolation by allowing plugins to package their own resources.
There's also an interesting note in the [ClassLoader::getResources Javadocs](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/ClassLoader.html#getResources(java.lang.String)):
> When overriding this method it is recommended that an implementation ensures that any delegation is consistent with the [getResource(String)](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String)) method. This should ensure that the first element returned by the Enumeration's nextElement method is the same resource that the getResource(String) method would return.
So, unless there's a good reason not to, we should also override the `getResources` method with the same child-first logic that we have in this method.
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -64,56 +63,95 @@
* the names of the different plugins directly via the exposed constants.
*/
public class TestPlugins {
+ public enum TestPlugin {
+ /**
+ * Resource dir and class name of a plugin which will always throw an exception during loading
+ */
+ ALWAYS_THROW_EXCEPTION("always-throw-exception", "test.plugins.AlwaysThrowException"),
+ /**
+ * Resource dir and class name of a plugin which samples information about its initialization.
+ */
+ ALIASED_STATIC_FIELD("aliased-static-field", "test.plugins.AliasedStaticField"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.Converter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONVERTER("sampling-converter", "test.plugins.SamplingConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.Configurable}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIGURABLE("sampling-configurable", "test.plugins.SamplingConfigurable"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.HeaderConverter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_HEADER_CONVERTER("sampling-header-converter", "test.plugins.SamplingHeaderConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.config.provider.ConfigProvider}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIG_PROVIDER("sampling-config-provider", "test.plugins.SamplingConfigProvider"),
+ /**
+ * Resource dir and class name of a plugin which uses a {@link java.util.ServiceLoader}
+ * to load internal classes, and samples information about their initialization.
+ */
+ SERVICE_LOADER("service-loader", "test.plugins.ServiceLoaderPlugin"),
+ /**
+ * Resource dir and class name of plugins which reads a version string from resource.
+ */
+ READ_VERSION_FROM_RESOURCE_V1("read-version-from-resource-v1", "test.plugins.ReadVersionFromResource"),
+ // The behavior of DelegatingClassLoader when two jars providing the same plugin are both
+ // on the plugin path is undeterministic. So, hide the v2 jar from the plugin path. This jar
+ // is only used in certain testcases that verify resources are read correctly when provided
+ // in both the PluginClassLoader and its parent class loader.
+ READ_VERSION_FROM_RESOURCE_V2("read-version-from-resource-v2",
+ "test.plugins.ReadVersionFromResource", true),
- /**
- * Class name of a plugin which will always throw an exception during loading
- */
- public static final String ALWAYS_THROW_EXCEPTION = "test.plugins.AlwaysThrowException";
- /**
- * Class name of a plugin which samples information about its initialization.
- */
- public static final String ALIASED_STATIC_FIELD = "test.plugins.AliasedStaticField";
- /**
- * Class name of a {@link org.apache.kafka.connect.storage.Converter}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONVERTER = "test.plugins.SamplingConverter";
- /**
- * Class name of a {@link org.apache.kafka.common.Configurable}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONFIGURABLE = "test.plugins.SamplingConfigurable";
- /**
- * Class name of a {@link org.apache.kafka.connect.storage.HeaderConverter}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_HEADER_CONVERTER = "test.plugins.SamplingHeaderConverter";
- /**
- * Class name of a {@link org.apache.kafka.common.config.provider.ConfigProvider}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONFIG_PROVIDER = "test.plugins.SamplingConfigProvider";
- /**
- * Class name of a plugin which uses a {@link java.util.ServiceLoader}
- * to load internal classes, and samples information about their initialization.
- */
- public static final String SERVICE_LOADER = "test.plugins.ServiceLoaderPlugin";
+ /**
+ * Resource dir and class names of plugins that are included in a single jar.
+ */
+ MULTIPLE_PLUGINS_IN_JAR_THING_ONE("multiple-plugins-in-jar", "test.plugins.ThingOne"),
+ MULTIPLE_PLUGINS_IN_JAR_THING_TWO("multiple-plugins-in-jar", "test.plugins.ThingTwo");
+
+ private final String resourceDir;
+ private final String className;
+ private final boolean hideFromPluginPath;
+
+ TestPlugin(String resourceDir, String className) {
+ this(resourceDir, className, false);
+ }
+
+ TestPlugin(String resourceDir, String className, boolean hideFromPluginPath) {
+ this.resourceDir = resourceDir;
+ this.className = className;
+ this.hideFromPluginPath = hideFromPluginPath;
+ }
+
+ public String getResourceDir() {
Review Comment:
Nit: we don't use the `get` prefix in this code base:
```suggestion
public String resourceDir() {
```
Also, this method is unused at the moment.
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -64,56 +63,95 @@
* the names of the different plugins directly via the exposed constants.
*/
public class TestPlugins {
+ public enum TestPlugin {
+ /**
+ * Resource dir and class name of a plugin which will always throw an exception during loading
+ */
+ ALWAYS_THROW_EXCEPTION("always-throw-exception", "test.plugins.AlwaysThrowException"),
+ /**
+ * Resource dir and class name of a plugin which samples information about its initialization.
+ */
+ ALIASED_STATIC_FIELD("aliased-static-field", "test.plugins.AliasedStaticField"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.Converter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONVERTER("sampling-converter", "test.plugins.SamplingConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.Configurable}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIGURABLE("sampling-configurable", "test.plugins.SamplingConfigurable"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.HeaderConverter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_HEADER_CONVERTER("sampling-header-converter", "test.plugins.SamplingHeaderConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.config.provider.ConfigProvider}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIG_PROVIDER("sampling-config-provider", "test.plugins.SamplingConfigProvider"),
+ /**
+ * Resource dir and class name of a plugin which uses a {@link java.util.ServiceLoader}
+ * to load internal classes, and samples information about their initialization.
+ */
+ SERVICE_LOADER("service-loader", "test.plugins.ServiceLoaderPlugin"),
+ /**
+ * Resource dir and class name of plugins which reads a version string from resource.
+ */
+ READ_VERSION_FROM_RESOURCE_V1("read-version-from-resource-v1", "test.plugins.ReadVersionFromResource"),
+ // The behavior of DelegatingClassLoader when two jars providing the same plugin are both
+ // on the plugin path is undeterministic. So, hide the v2 jar from the plugin path. This jar
+ // is only used in certain testcases that verify resources are read correctly when provided
+ // in both the PluginClassLoader and its parent class loader.
+ READ_VERSION_FROM_RESOURCE_V2("read-version-from-resource-v2",
+ "test.plugins.ReadVersionFromResource", true),
- /**
- * Class name of a plugin which will always throw an exception during loading
- */
- public static final String ALWAYS_THROW_EXCEPTION = "test.plugins.AlwaysThrowException";
- /**
- * Class name of a plugin which samples information about its initialization.
- */
- public static final String ALIASED_STATIC_FIELD = "test.plugins.AliasedStaticField";
- /**
- * Class name of a {@link org.apache.kafka.connect.storage.Converter}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONVERTER = "test.plugins.SamplingConverter";
- /**
- * Class name of a {@link org.apache.kafka.common.Configurable}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONFIGURABLE = "test.plugins.SamplingConfigurable";
- /**
- * Class name of a {@link org.apache.kafka.connect.storage.HeaderConverter}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_HEADER_CONVERTER = "test.plugins.SamplingHeaderConverter";
- /**
- * Class name of a {@link org.apache.kafka.common.config.provider.ConfigProvider}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONFIG_PROVIDER = "test.plugins.SamplingConfigProvider";
- /**
- * Class name of a plugin which uses a {@link java.util.ServiceLoader}
- * to load internal classes, and samples information about their initialization.
- */
- public static final String SERVICE_LOADER = "test.plugins.ServiceLoaderPlugin";
+ /**
+ * Resource dir and class names of plugins that are included in a single jar.
+ */
+ MULTIPLE_PLUGINS_IN_JAR_THING_ONE("multiple-plugins-in-jar", "test.plugins.ThingOne"),
+ MULTIPLE_PLUGINS_IN_JAR_THING_TWO("multiple-plugins-in-jar", "test.plugins.ThingTwo");
+
+ private final String resourceDir;
+ private final String className;
+ private final boolean hideFromPluginPath;
+
+ TestPlugin(String resourceDir, String className) {
+ this(resourceDir, className, false);
+ }
+
+ TestPlugin(String resourceDir, String className, boolean hideFromPluginPath) {
+ this.resourceDir = resourceDir;
+ this.className = className;
+ this.hideFromPluginPath = hideFromPluginPath;
+ }
+
+ public String getResourceDir() {
+ return resourceDir;
+ }
+
+ public String getClassName() {
Review Comment:
Nit: no `get`:
```suggestion
public String className() {
```
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginsTest.java:
##########
@@ -327,6 +333,63 @@ public void newPluginsShouldConfigureWithPluginClassLoader() {
assertPluginClassLoaderAlwaysActive(samples);
}
+ @Test
+ public void pluginClassLoaderReadVersionFromResourceExistingOnlyInChild() throws Exception {
+ TestPlugins.assertAvailable();
+
+ assertClassLoaderReadsVersionFromResource(
+ TestPlugin.ALIASED_STATIC_FIELD,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1.getClassName(),
+ "1.0.0");
+ }
+
+ @Test
+ public void pluginClassLoaderReadVersionFromResourceExistingOnlyInParent() throws Exception {
+ TestPlugins.assertAvailable();
+
+ assertClassLoaderReadsVersionFromResource(
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1,
+ TestPlugin.ALIASED_STATIC_FIELD,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1.getClassName(),
+ "1.0.0");
+ }
+
+ @Test
+ public void pluginClassLoaderReadVersionFromResourceExistingInParentAndChild() throws Exception {
+ TestPlugins.assertAvailable();
+
+ assertClassLoaderReadsVersionFromResource(
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V2,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V2.getClassName(),
+ "2.0.0");
+ }
+
+ private void assertClassLoaderReadsVersionFromResource(
+ TestPlugin parentResource, TestPlugin childResource, String className, String expectedVersion) throws MalformedURLException {
+ String pluginPath = TestPlugins.pluginPath(parentResource);
+ URLClassLoader parent = new URLClassLoader(
+ new URL[]{new File(pluginPath).toURI().toURL()});
+
Review Comment:
Nit: can remove this blank line
```suggestion
```
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginsTest.java:
##########
@@ -327,6 +333,63 @@ public void newPluginsShouldConfigureWithPluginClassLoader() {
assertPluginClassLoaderAlwaysActive(samples);
}
+ @Test
+ public void pluginClassLoaderReadVersionFromResourceExistingOnlyInChild() throws Exception {
+ TestPlugins.assertAvailable();
+
+ assertClassLoaderReadsVersionFromResource(
+ TestPlugin.ALIASED_STATIC_FIELD,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1.getClassName(),
+ "1.0.0");
+ }
+
+ @Test
+ public void pluginClassLoaderReadVersionFromResourceExistingOnlyInParent() throws Exception {
+ TestPlugins.assertAvailable();
+
+ assertClassLoaderReadsVersionFromResource(
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1,
+ TestPlugin.ALIASED_STATIC_FIELD,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1.getClassName(),
+ "1.0.0");
+ }
+
+ @Test
+ public void pluginClassLoaderReadVersionFromResourceExistingInParentAndChild() throws Exception {
+ TestPlugins.assertAvailable();
+
+ assertClassLoaderReadsVersionFromResource(
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V1,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V2,
+ TestPlugin.READ_VERSION_FROM_RESOURCE_V2.getClassName(),
+ "2.0.0");
+ }
+
+ private void assertClassLoaderReadsVersionFromResource(
+ TestPlugin parentResource, TestPlugin childResource, String className, String expectedVersion) throws MalformedURLException {
+ String pluginPath = TestPlugins.pluginPath(parentResource);
+ URLClassLoader parent = new URLClassLoader(
+ new URL[]{new File(pluginPath).toURI().toURL()});
+
+
+ // Initialize Plugins object with parent class loader in the class loader tree. This is
+ // to simulate the situation where jars exist on both system classpath and plugin path.
+ Map<String, String> pluginProps = new HashMap<>();
+ pluginProps.put(WorkerConfig.PLUGIN_PATH_CONFIG,
+ TestPlugins.pluginPath(childResource));
+ plugins = new Plugins(pluginProps, parent);
+
+ Converter converter = plugins.newPlugin(
+ className,
+ new AbstractConfig(new ConfigDef(), Collections.emptyMap()),
+ Converter.class
+ );
+ // Verify the version was read from the correct resource
+ assertEquals(expectedVersion,
+ new String(converter.fromConnectData(null, null, null)));
+ }
Review Comment:
I love these test cases.
Great work @ncliang, hope you're well!
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -64,56 +63,95 @@
* the names of the different plugins directly via the exposed constants.
Review Comment:
Should this be updated from `exposed constants` to `{@link TestPlugin} enum`?
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java:
##########
@@ -60,12 +60,27 @@ public Plugins(Map<String, String> props) {
delegatingLoader.initLoaders();
}
+ // VisibleForTesting
+ Plugins(Map<String, String> props, ClassLoader parent) {
+ List<String> pluginLocations = WorkerConfig.pluginLocations(props);
+ delegatingLoader = newDelegatingClassLoader(pluginLocations, parent);
+ delegatingLoader.initLoaders();
Review Comment:
Nit: can we reduce duplication by delegating to this method in the public constructor?
```java
public Plugins(Map<String, String> props) {
this(props, Plugins.class.getClassLoader());
}
```
And if we do that, we can also get rid of the single-parameter `newDelegatingClassLoader` method.
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -141,18 +179,37 @@ public static void assertAvailable() throws AssertionError {
* @return A list of plugin jar filenames
*/
public static List<String> pluginPath() {
- return PLUGIN_JARS.values()
+ return PLUGIN_JARS.entrySet()
.stream()
- .map(File::getPath)
+ .filter(e -> !e.getKey().hideFromPluginPath())
+ .map(e -> e.getValue().getPath())
.collect(Collectors.toList());
}
+ /**
+ * Gets the path to a single jar containing the test plugin.
+ * @param testPlugin the unique {@link TestPlugin} enum representing the plugin
+ * @return a string representing the plugin jar filename, or null if a plugin for the
Review Comment:
Any reason to return null instead of throwing an exception of some kind if the plugin cannot be found? It looks like the one place where we're currently using this method will fail with an NPE if it does end up returning null, and I can't think of any good cases where we'd want `null` here instead of an exception.
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -64,56 +63,95 @@
* the names of the different plugins directly via the exposed constants.
*/
public class TestPlugins {
+ public enum TestPlugin {
+ /**
+ * Resource dir and class name of a plugin which will always throw an exception during loading
+ */
+ ALWAYS_THROW_EXCEPTION("always-throw-exception", "test.plugins.AlwaysThrowException"),
+ /**
+ * Resource dir and class name of a plugin which samples information about its initialization.
+ */
+ ALIASED_STATIC_FIELD("aliased-static-field", "test.plugins.AliasedStaticField"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.Converter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONVERTER("sampling-converter", "test.plugins.SamplingConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.Configurable}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIGURABLE("sampling-configurable", "test.plugins.SamplingConfigurable"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.HeaderConverter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_HEADER_CONVERTER("sampling-header-converter", "test.plugins.SamplingHeaderConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.config.provider.ConfigProvider}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIG_PROVIDER("sampling-config-provider", "test.plugins.SamplingConfigProvider"),
+ /**
+ * Resource dir and class name of a plugin which uses a {@link java.util.ServiceLoader}
+ * to load internal classes, and samples information about their initialization.
+ */
+ SERVICE_LOADER("service-loader", "test.plugins.ServiceLoaderPlugin"),
+ /**
+ * Resource dir and class name of plugins which reads a version string from resource.
+ */
+ READ_VERSION_FROM_RESOURCE_V1("read-version-from-resource-v1", "test.plugins.ReadVersionFromResource"),
+ // The behavior of DelegatingClassLoader when two jars providing the same plugin are both
+ // on the plugin path is undeterministic. So, hide the v2 jar from the plugin path. This jar
+ // is only used in certain testcases that verify resources are read correctly when provided
+ // in both the PluginClassLoader and its parent class loader.
+ READ_VERSION_FROM_RESOURCE_V2("read-version-from-resource-v2",
+ "test.plugins.ReadVersionFromResource", true),
- /**
- * Class name of a plugin which will always throw an exception during loading
- */
- public static final String ALWAYS_THROW_EXCEPTION = "test.plugins.AlwaysThrowException";
- /**
- * Class name of a plugin which samples information about its initialization.
- */
- public static final String ALIASED_STATIC_FIELD = "test.plugins.AliasedStaticField";
- /**
- * Class name of a {@link org.apache.kafka.connect.storage.Converter}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONVERTER = "test.plugins.SamplingConverter";
- /**
- * Class name of a {@link org.apache.kafka.common.Configurable}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONFIGURABLE = "test.plugins.SamplingConfigurable";
- /**
- * Class name of a {@link org.apache.kafka.connect.storage.HeaderConverter}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_HEADER_CONVERTER = "test.plugins.SamplingHeaderConverter";
- /**
- * Class name of a {@link org.apache.kafka.common.config.provider.ConfigProvider}
- * which samples information about its method calls.
- */
- public static final String SAMPLING_CONFIG_PROVIDER = "test.plugins.SamplingConfigProvider";
- /**
- * Class name of a plugin which uses a {@link java.util.ServiceLoader}
- * to load internal classes, and samples information about their initialization.
- */
- public static final String SERVICE_LOADER = "test.plugins.ServiceLoaderPlugin";
+ /**
+ * Resource dir and class names of plugins that are included in a single jar.
+ */
+ MULTIPLE_PLUGINS_IN_JAR_THING_ONE("multiple-plugins-in-jar", "test.plugins.ThingOne"),
+ MULTIPLE_PLUGINS_IN_JAR_THING_TWO("multiple-plugins-in-jar", "test.plugins.ThingTwo");
+
+ private final String resourceDir;
+ private final String className;
+ private final boolean hideFromPluginPath;
Review Comment:
I was a little misled by this field and the effects it'd have later on when seeing uses of `TestPlugins::pluginPath` with `READ_VERSION_FROM_RESOURCE_V2` as an argument, since based on this field that test plugin is meant to be hidden from the plugin path.
Do you think `hideByDefault` would be clearer about how plugins with this field set to `true` aren't completely hidden, but you do have to opt in to seeing them?
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -64,56 +63,95 @@
* the names of the different plugins directly via the exposed constants.
*/
public class TestPlugins {
+ public enum TestPlugin {
+ /**
+ * Resource dir and class name of a plugin which will always throw an exception during loading
Review Comment:
The "Resource dir and class name of a..." prefix for each of these is a little redundant. Can we nix that and just use, e.g., `A plugin which will always throw an exception during loading`, `A {@link org.apache.kafka.common.config.provider.ConfigProvider} which samples information about its method calls.`, etc.?
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -64,56 +63,95 @@
* the names of the different plugins directly via the exposed constants.
*/
public class TestPlugins {
+ public enum TestPlugin {
+ /**
+ * Resource dir and class name of a plugin which will always throw an exception during loading
+ */
+ ALWAYS_THROW_EXCEPTION("always-throw-exception", "test.plugins.AlwaysThrowException"),
+ /**
+ * Resource dir and class name of a plugin which samples information about its initialization.
+ */
+ ALIASED_STATIC_FIELD("aliased-static-field", "test.plugins.AliasedStaticField"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.Converter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONVERTER("sampling-converter", "test.plugins.SamplingConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.Configurable}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIGURABLE("sampling-configurable", "test.plugins.SamplingConfigurable"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.connect.storage.HeaderConverter}
+ * which samples information about its method calls.
+ */
+ SAMPLING_HEADER_CONVERTER("sampling-header-converter", "test.plugins.SamplingHeaderConverter"),
+ /**
+ * Resource dir and class name of a {@link org.apache.kafka.common.config.provider.ConfigProvider}
+ * which samples information about its method calls.
+ */
+ SAMPLING_CONFIG_PROVIDER("sampling-config-provider", "test.plugins.SamplingConfigProvider"),
+ /**
+ * Resource dir and class name of a plugin which uses a {@link java.util.ServiceLoader}
+ * to load internal classes, and samples information about their initialization.
+ */
+ SERVICE_LOADER("service-loader", "test.plugins.ServiceLoaderPlugin"),
+ /**
+ * Resource dir and class name of plugins which reads a version string from resource.
+ */
+ READ_VERSION_FROM_RESOURCE_V1("read-version-from-resource-v1", "test.plugins.ReadVersionFromResource"),
+ // The behavior of DelegatingClassLoader when two jars providing the same plugin are both
+ // on the plugin path is undeterministic. So, hide the v2 jar from the plugin path. This jar
Review Comment:
Nit:
```suggestion
// on the plugin path is nondeterministic. So, hide the v2 jar from the plugin path. This jar
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org