From d620d0854bca2bb9dda1b3a9235c94e1238d88cd Mon Sep 17 00:00:00 2001 From: Dinu John <86094133+dinujoh@users.noreply.github.com> Date: Sat, 21 Sep 2024 13:51:53 -0500 Subject: [PATCH 1/3] Add alternate name for Plugin Signed-off-by: Dinu John <86094133+dinujoh@users.noreply.github.com> --- .../model/annotations/DataPrepperPlugin.java | 8 +++++ .../plugin/ClasspathPluginProvider.java | 16 ++++++++++ .../plugin/ClasspathPluginProviderTest.java | 22 ++++++++++++++ .../plugin/DefaultPluginFactoryTest.java | 29 +++++++++++++++++++ .../dataprepper/plugins/test/TestSink.java | 2 +- .../dataprepper/plugins/test/TestSource.java | 2 +- 6 files changed, 77 insertions(+), 2 deletions(-) diff --git a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java index c9345385dc..7eab8190ca 100644 --- a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java +++ b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java @@ -33,6 +33,8 @@ public @interface DataPrepperPlugin { String DEFAULT_DEPRECATED_NAME = ""; + String DEFAULT_ALTERNATE_NAME = ""; + /** * * @return Name of the plugin which should be unique for the type @@ -46,6 +48,12 @@ */ String deprecatedName() default DEFAULT_DEPRECATED_NAME; + /** + * + * @return Alternate name of the plugin which should be unique for the type + */ + String alternateName() default DEFAULT_ALTERNATE_NAME; + /** * The class type for this plugin. * diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java index f5217ef8f5..0d60629951 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java @@ -17,6 +17,7 @@ import java.util.Set; import java.util.stream.Collectors; +import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_ALTERNATE_NAME; import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_DEPRECATED_NAME; /** @@ -78,6 +79,7 @@ private Map, Class>> scanForPlugins() { supportTypeToPluginTypeMap.put(supportedType, concretePluginClass); addOptionalDeprecatedPluginName(pluginsMap, concretePluginClass); + addOptionalAlternatePluginName(pluginsMap, concretePluginClass); } return pluginsMap; @@ -96,4 +98,18 @@ private void addOptionalDeprecatedPluginName( supportTypeToPluginTypeMap.put(supportedType, concretePluginClass); } } + + private void addOptionalAlternatePluginName( + final Map, Class>> pluginsMap, + final Class concretePluginClass) { + final DataPrepperPlugin dataPrepperPluginAnnotation = concretePluginClass.getAnnotation(DataPrepperPlugin.class); + final String alternatePluginName = dataPrepperPluginAnnotation.alternateName(); + final Class supportedType = dataPrepperPluginAnnotation.pluginType(); + + if (!alternatePluginName.equals(DEFAULT_ALTERNATE_NAME)) { + final Map, Class> supportTypeToPluginTypeMap = + pluginsMap.computeIfAbsent(alternatePluginName, k -> new HashMap<>()); + supportTypeToPluginTypeMap.put(supportedType, concretePluginClass); + } + } } diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java index 88763164dd..a5ea507cd8 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java @@ -28,6 +28,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; import static org.mockito.Mockito.mock; +import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_ALTERNATE_NAME; import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_DEPRECATED_NAME; class ClasspathPluginProviderTest { @@ -105,6 +106,27 @@ void findPlugin_should_return_plugin_if_found_for_deprecated_name_and_type_using assertThat(optionalPlugin.get(), equalTo(TestSource.class)); } + @Test + void findPlugin_should_return_empty_for_default_alternate_name() { + given(reflections.getTypesAnnotatedWith(DataPrepperPlugin.class)) + .willReturn(new HashSet<>(List.of(TestSource.class))); + + final Optional> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, DEFAULT_ALTERNATE_NAME); + assertThat(optionalPlugin, notNullValue()); + assertThat(optionalPlugin.isPresent(), equalTo(false)); + } + + @Test + void findPlugin_should_return_plugin_if_found_for_alternate_name_and_type_using_pluginType() { + given(reflections.getTypesAnnotatedWith(DataPrepperPlugin.class)) + .willReturn(new HashSet<>(List.of(TestSource.class))); + + final Optional> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, "test_source_alternate_name"); + assertThat(optionalPlugin, notNullValue()); + assertThat(optionalPlugin.isPresent(), equalTo(true)); + assertThat(optionalPlugin.get(), equalTo(TestSource.class)); + } + @Nested class WithPredefinedPlugins { diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java index 64b053a924..ec61763e61 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java @@ -401,4 +401,33 @@ void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found_with_corr verify(beanFactoryProvider).get(); } } + + @Nested + class WithAlternatePluginName { + private static final String TEST_SINK_ALTERNATE_NAME = "test_sink_alternate_name"; + private Class expectedPluginClass; + + @BeforeEach + void setUp() { + expectedPluginClass = TestSink.class; + given(pluginSetting.getName()).willReturn(TEST_SINK_ALTERNATE_NAME); + + given(firstPluginProvider.findPluginClass(baseClass, TEST_SINK_ALTERNATE_NAME)) + .willReturn(Optional.of(expectedPluginClass)); + } + + @Test + void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found_with_correct_name_and_alternate_name() { + final TestSink expectedInstance = mock(TestSink.class); + final Object convertedConfiguration = mock(Object.class); + given(pluginConfigurationConverter.convert(PluginSetting.class, pluginSetting)) + .willReturn(convertedConfiguration); + given(pluginCreator.newPluginInstance(eq(expectedPluginClass), any(ComponentPluginArgumentsContext.class), eq(TEST_SINK_ALTERNATE_NAME))) + .willReturn(expectedInstance); + + assertThat(createObjectUnderTest().loadPlugin(baseClass, pluginSetting), equalTo(expectedInstance)); + MatcherAssert.assertThat(expectedInstance.getClass().getAnnotation(DataPrepperPlugin.class).alternateName(), equalTo(TEST_SINK_ALTERNATE_NAME)); + verify(beanFactoryProvider).get(); + } + } } diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java index fc54428ba2..cc15f7320d 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java @@ -16,7 +16,7 @@ import java.util.stream.Collectors; import java.time.Instant; -@DataPrepperPlugin(name = "test_sink", deprecatedName = "test_sink_deprecated_name", pluginType = Sink.class) +@DataPrepperPlugin(name = "test_sink", alternateName = "test_sink_alternate_name", deprecatedName = "test_sink_deprecated_name", pluginType = Sink.class) public class TestSink implements Sink> { public boolean isShutdown = false; private final List> collectedRecords; diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java index 9a7192a370..eb94eef8e9 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java @@ -16,7 +16,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -@DataPrepperPlugin(name = "test_source", deprecatedName = "test_source_deprecated_name", pluginType = Source.class) +@DataPrepperPlugin(name = "test_source", alternateName = "test_source_alternate_name", deprecatedName = "test_source_deprecated_name", pluginType = Source.class) public class TestSource implements Source> { public static final List> TEST_DATA = Stream.of("TEST") .map(Record::new).collect(Collectors.toList()); From 37b15622adfd21826f62ecc0bc7607d2c1ea0b08 Mon Sep 17 00:00:00 2001 From: Dinu John <86094133+dinujoh@users.noreply.github.com> Date: Sat, 21 Sep 2024 15:38:20 -0500 Subject: [PATCH 2/3] Code refactor to reduce duplicate code Signed-off-by: Dinu John <86094133+dinujoh@users.noreply.github.com> --- .../model/annotations/DataPrepperPlugin.java | 2 +- .../plugin/ClasspathPluginProvider.java | 49 ++++++++----------- .../plugin/ClasspathPluginProviderTest.java | 13 +++-- .../plugin/DefaultPluginFactoryTest.java | 2 +- .../dataprepper/plugins/test/TestSink.java | 2 +- .../dataprepper/plugins/test/TestSource.java | 2 +- 6 files changed, 32 insertions(+), 38 deletions(-) diff --git a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java index 7eab8190ca..d94c0d8c19 100644 --- a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java +++ b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java @@ -52,7 +52,7 @@ * * @return Alternate name of the plugin which should be unique for the type */ - String alternateName() default DEFAULT_ALTERNATE_NAME; + String[] alternateNames() default {}; /** * The class type for this plugin. diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java index 0d60629951..764c83f4db 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java @@ -15,6 +15,8 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_ALTERNATE_NAME; @@ -70,45 +72,34 @@ private Map, Class>> scanForPlugins() { final Map, Class>> pluginsMap = new HashMap<>(dataPrepperPluginClasses.size()); for (final Class concretePluginClass : dataPrepperPluginClasses) { - final DataPrepperPlugin dataPrepperPluginAnnotation = concretePluginClass.getAnnotation(DataPrepperPlugin.class); - final String pluginName = dataPrepperPluginAnnotation.name(); - final Class supportedType = dataPrepperPluginAnnotation.pluginType(); - - final Map, Class> supportTypeToPluginTypeMap = - pluginsMap.computeIfAbsent(pluginName, k -> new HashMap<>()); - supportTypeToPluginTypeMap.put(supportedType, concretePluginClass); - - addOptionalDeprecatedPluginName(pluginsMap, concretePluginClass); - addOptionalAlternatePluginName(pluginsMap, concretePluginClass); + // plugin name + addPossiblePluginName(pluginsMap, concretePluginClass, DataPrepperPlugin::name, name -> true); + // deprecated plugin name + addPossiblePluginName(pluginsMap, concretePluginClass, DataPrepperPlugin::deprecatedName, + deprecatedPluginName -> !deprecatedPluginName.equals(DEFAULT_DEPRECATED_NAME)); + // alternate plugin names + for (final String alternateName: concretePluginClass.getAnnotation(DataPrepperPlugin.class).alternateNames()) { + addPossiblePluginName(pluginsMap, concretePluginClass, DataPrepperPlugin -> alternateName, + alternatePluginName -> !alternatePluginName.equals(DEFAULT_ALTERNATE_NAME)); + } } return pluginsMap; } - private void addOptionalDeprecatedPluginName( - final Map, Class>> pluginsMap, - final Class concretePluginClass) { - final DataPrepperPlugin dataPrepperPluginAnnotation = concretePluginClass.getAnnotation(DataPrepperPlugin.class); - final String deprecatedPluginName = dataPrepperPluginAnnotation.deprecatedName(); - final Class supportedType = dataPrepperPluginAnnotation.pluginType(); - - if (!deprecatedPluginName.equals(DEFAULT_DEPRECATED_NAME)) { - final Map, Class> supportTypeToPluginTypeMap = - pluginsMap.computeIfAbsent(deprecatedPluginName, k -> new HashMap<>()); - supportTypeToPluginTypeMap.put(supportedType, concretePluginClass); - } - } - - private void addOptionalAlternatePluginName( + private void addPossiblePluginName( final Map, Class>> pluginsMap, - final Class concretePluginClass) { + final Class concretePluginClass, + final Function possiblePluginNameFunction, + final Predicate possiblePluginNamePredicate + ) { final DataPrepperPlugin dataPrepperPluginAnnotation = concretePluginClass.getAnnotation(DataPrepperPlugin.class); - final String alternatePluginName = dataPrepperPluginAnnotation.alternateName(); + final String possiblePluginName = possiblePluginNameFunction.apply(dataPrepperPluginAnnotation); final Class supportedType = dataPrepperPluginAnnotation.pluginType(); - if (!alternatePluginName.equals(DEFAULT_ALTERNATE_NAME)) { + if (possiblePluginNamePredicate.test(possiblePluginName)) { final Map, Class> supportTypeToPluginTypeMap = - pluginsMap.computeIfAbsent(alternatePluginName, k -> new HashMap<>()); + pluginsMap.computeIfAbsent(possiblePluginName, k -> new HashMap<>()); supportTypeToPluginTypeMap.put(supportedType, concretePluginClass); } } diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java index a5ea507cd8..6cda169636 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java @@ -8,6 +8,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; import org.opensearch.dataprepper.model.sink.Sink; import org.opensearch.dataprepper.model.source.Source; @@ -28,7 +30,6 @@ import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; import static org.mockito.Mockito.mock; -import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_ALTERNATE_NAME; import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_DEPRECATED_NAME; class ClasspathPluginProviderTest { @@ -111,17 +112,19 @@ void findPlugin_should_return_empty_for_default_alternate_name() { given(reflections.getTypesAnnotatedWith(DataPrepperPlugin.class)) .willReturn(new HashSet<>(List.of(TestSource.class))); - final Optional> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, DEFAULT_ALTERNATE_NAME); + final Optional> optionalPlugin = createObjectUnderTest() + .findPluginClass(Source.class, UUID.randomUUID().toString()); assertThat(optionalPlugin, notNullValue()); assertThat(optionalPlugin.isPresent(), equalTo(false)); } - @Test - void findPlugin_should_return_plugin_if_found_for_alternate_name_and_type_using_pluginType() { + @ParameterizedTest + @ValueSource(strings = {"test_source_alternate_name1", "test_source_alternate_name2"}) + void findPlugin_should_return_plugin_if_found_for_alternate_name_and_type_using_pluginType(final String alternateSourceName) { given(reflections.getTypesAnnotatedWith(DataPrepperPlugin.class)) .willReturn(new HashSet<>(List.of(TestSource.class))); - final Optional> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, "test_source_alternate_name"); + final Optional> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, alternateSourceName); assertThat(optionalPlugin, notNullValue()); assertThat(optionalPlugin.isPresent(), equalTo(true)); assertThat(optionalPlugin.get(), equalTo(TestSource.class)); diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java index ec61763e61..495d003bb3 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java @@ -426,7 +426,7 @@ void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found_with_corr .willReturn(expectedInstance); assertThat(createObjectUnderTest().loadPlugin(baseClass, pluginSetting), equalTo(expectedInstance)); - MatcherAssert.assertThat(expectedInstance.getClass().getAnnotation(DataPrepperPlugin.class).alternateName(), equalTo(TEST_SINK_ALTERNATE_NAME)); + MatcherAssert.assertThat(expectedInstance.getClass().getAnnotation(DataPrepperPlugin.class).alternateNames(), equalTo(new String[]{TEST_SINK_ALTERNATE_NAME})); verify(beanFactoryProvider).get(); } } diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java index cc15f7320d..ea8196a07b 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java @@ -16,7 +16,7 @@ import java.util.stream.Collectors; import java.time.Instant; -@DataPrepperPlugin(name = "test_sink", alternateName = "test_sink_alternate_name", deprecatedName = "test_sink_deprecated_name", pluginType = Sink.class) +@DataPrepperPlugin(name = "test_sink", alternateNames = { "test_sink_alternate_name" }, deprecatedName = "test_sink_deprecated_name", pluginType = Sink.class) public class TestSink implements Sink> { public boolean isShutdown = false; private final List> collectedRecords; diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java index eb94eef8e9..2ad29f2650 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java @@ -16,7 +16,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -@DataPrepperPlugin(name = "test_source", alternateName = "test_source_alternate_name", deprecatedName = "test_source_deprecated_name", pluginType = Source.class) +@DataPrepperPlugin(name = "test_source", alternateNames = { "test_source_alternate_name1", "test_source_alternate_name2" }, deprecatedName = "test_source_deprecated_name", pluginType = Source.class) public class TestSource implements Source> { public static final List> TEST_DATA = Stream.of("TEST") .map(Record::new).collect(Collectors.toList()); From 477af25a7e5ac483d7bdc36ce84042f3104f2dba Mon Sep 17 00:00:00 2001 From: Dinu John <86094133+dinujoh@users.noreply.github.com> Date: Sat, 21 Sep 2024 15:44:40 -0500 Subject: [PATCH 3/3] Update TestSink with single alternate name Signed-off-by: Dinu John <86094133+dinujoh@users.noreply.github.com> --- .../java/org/opensearch/dataprepper/plugins/test/TestSink.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java index ea8196a07b..1e2742a0ba 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java @@ -16,7 +16,7 @@ import java.util.stream.Collectors; import java.time.Instant; -@DataPrepperPlugin(name = "test_sink", alternateNames = { "test_sink_alternate_name" }, deprecatedName = "test_sink_deprecated_name", pluginType = Sink.class) +@DataPrepperPlugin(name = "test_sink", alternateNames = "test_sink_alternate_name", deprecatedName = "test_sink_deprecated_name", pluginType = Sink.class) public class TestSink implements Sink> { public boolean isShutdown = false; private final List> collectedRecords;