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] 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());