Code refactor to reduce duplicate code

Signed-off-by: Dinu John <86094133+dinujoh@users.noreply.github.com>
opensearch-project · Sep 21, 2024 · 37b1562 · 37b1562
1 parent d620d08
commit 37b1562
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 38 deletions.
diff --git a/...per-api/src/main/java/org/opensearch/dataprepper/model/annotations/DataPrepperPlugin.java b/...per-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/...in-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java b/...in-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<String, Map<Class<?>, Class<?>>> scanForPlugins() {
 
         final Map<String, Map<Class<?>, 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<?>, 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<String, Map<Class<?>, 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<?>, Class<?>> supportTypeToPluginTypeMap =
-                    pluginsMap.computeIfAbsent(deprecatedPluginName, k -> new HashMap<>());
-            supportTypeToPluginTypeMap.put(supportedType, concretePluginClass);
-        }
-    }
-
-    private void addOptionalAlternatePluginName(
+    private void addPossiblePluginName(
             final Map<String, Map<Class<?>, Class<?>>> pluginsMap,
-            final Class<?> concretePluginClass) {
+            final Class<?> concretePluginClass,
+            final Function<DataPrepperPlugin, String> possiblePluginNameFunction,
+            final Predicate<String> 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<?>, Class<?>> supportTypeToPluginTypeMap =
-                    pluginsMap.computeIfAbsent(alternatePluginName, k -> new HashMap<>());
+                    pluginsMap.computeIfAbsent(possiblePluginName, k -> new HashMap<>());
             supportTypeToPluginTypeMap.put(supportedType, concretePluginClass);
         }
     }

diff --git a/...ramework/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java b/...ramework/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<Class<? extends Source>> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, DEFAULT_ALTERNATE_NAME);
+        final Optional<Class<? extends Source>> 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<Class<? extends Source>> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, "test_source_alternate_name");
+        final Optional<Class<? extends Source>> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, alternateSourceName);
         assertThat(optionalPlugin, notNullValue());
         assertThat(optionalPlugin.isPresent(), equalTo(true));
         assertThat(optionalPlugin.get(), equalTo(TestSource.class));

diff --git a/...n-framework/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java b/...n-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/...pper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSink.java b/...pper-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<Record<String>> {
     public boolean isShutdown = false;
     private final List<Record<String>> collectedRecords;

diff --git a/...er-plugin-framework/src/test/java/org/opensearch/dataprepper/plugins/test/TestSource.java b/...er-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<Record<String>> {
     public static final List<Record<String>> TEST_DATA = Stream.of("TEST")
             .map(Record::new).collect(Collectors.toList());