Allow deprecated plugin names (#2508)

Allow deprecated plugin names and update otel plugin names Signed-off-by: Asif Sohail Mohammed <[email protected]> --------- Signed-off-by: Asif Sohail Mohammed <[email protected]>
opensearch-project · Apr 18, 2023 · da14b74 · da14b74
1 parent 0f130c1
commit da14b74
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 5 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
@@ -31,12 +31,21 @@
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.TYPE})
 public @interface DataPrepperPlugin {
+    String DEFAULT_DEPRECATED_NAME = "";
+
     /**
      *
      * @return Name of the plugin which should be unique for the type
      */
     String name();
 
+    /**
+     *
+     * @return Deprecated name of the plugin which should be unique for the type
+     * @since 2.2
+     */
+    String deprecatedName() default DEFAULT_DEPRECATED_NAME;
+
     /**
      * The class type for this plugin.
      *

diff --git a/...prepper-core/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java b/...prepper-core/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java
@@ -14,6 +14,8 @@
 import java.util.Optional;
 import java.util.Set;
 
+import static org.opensearch.dataprepper.model.annotations.DataPrepperPlugin.DEFAULT_DEPRECATED_NAME;
+
 /**
  * The implementation of {@link PluginProvider} which loads plugins from the
  * current Java classpath.
@@ -58,16 +60,31 @@ 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 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);
         }
 
         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);
+        }
+    }
 }
diff --git a/data-prepper-core/src/main/java/org/opensearch/dataprepper/plugin/DefaultPluginFactory.java b/data-prepper-core/src/main/java/org/opensearch/dataprepper/plugin/DefaultPluginFactory.java
@@ -11,6 +11,8 @@
 import org.opensearch.dataprepper.model.plugin.PluginFactory;
 import org.opensearch.dataprepper.event.DefaultEventFactory;
 import org.opensearch.dataprepper.acknowledgements.DefaultAcknowledgementSetManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -28,6 +30,8 @@
  */
 @Named
 public class DefaultPluginFactory implements PluginFactory {
+    private static final Logger LOG = LoggerFactory.getLogger(DefaultPluginFactory.class);
+
     private final Collection<PluginProvider> pluginProviders;
     private final PluginCreator pluginCreator;
     private final PluginConfigurationConverter pluginConfigurationConverter;
@@ -109,12 +113,23 @@ private <T> PluginArgumentsContext getConstructionContext(final PluginSetting pl
     }
 
     private <T> Class<? extends T> getPluginClass(final Class<T> baseClass, final String pluginName) {
-        return pluginProviders.stream()
+        final Class<? extends T> pluginClass = pluginProviders.stream()
                 .map(pluginProvider -> pluginProvider.findPluginClass(baseClass, pluginName))
                 .filter(Optional::isPresent)
                 .map(Optional::get)
                 .findFirst()
                 .orElseThrow(() -> new NoPluginFoundException(
                         "Unable to find a plugin named '" + pluginName + "'. Please ensure that plugin is annotated with appropriate values."));
+
+        logDeprecatedPluginsNames(pluginClass, pluginName);
+        return pluginClass;
+    }
+
+    private <T> void logDeprecatedPluginsNames(final Class<? extends T> pluginClass, final String pluginName) {
+        final String deprecatedName = pluginClass.getAnnotation(DataPrepperPlugin.class).deprecatedName();
+        final String name = pluginClass.getAnnotation(DataPrepperPlugin.class).name();
+        if (deprecatedName.equals(pluginName)) {
+            LOG.warn("Plugin name '{}' is deprecated and will be removed in the next major release. Consider using the updated plugin name '{}'.", deprecatedName, name);
+        }
     }
 }
diff --git a/...per-core/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java b/...per-core/src/test/java/org/opensearch/dataprepper/plugin/ClasspathPluginProviderTest.java
@@ -18,6 +18,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 
@@ -27,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_DEPRECATED_NAME;
 
 class ClasspathPluginProviderTest {
 
@@ -82,6 +84,27 @@ void findPlugin_should_return_empty_if_no_plugins_found() {
         assertThat(optionalPlugin.isPresent(), equalTo(false));
     }
 
+    @Test
+    void findPlugin_should_return_empty_for_default_deprecated_name() {
+        given(reflections.getTypesAnnotatedWith(DataPrepperPlugin.class))
+                .willReturn(new HashSet<>(List.of(TestSource.class)));
+
+        final Optional<Class<? extends Source>> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, DEFAULT_DEPRECATED_NAME);
+        assertThat(optionalPlugin, notNullValue());
+        assertThat(optionalPlugin.isPresent(), equalTo(false));
+    }
+
+    @Test
+    void findPlugin_should_return_plugin_if_found_for_deprecated_name_and_type_using_pluginType() {
+        given(reflections.getTypesAnnotatedWith(DataPrepperPlugin.class))
+                .willReturn(new HashSet<>(List.of(TestSource.class)));
+
+        final Optional<Class<? extends Source>> optionalPlugin = createObjectUnderTest().findPluginClass(Source.class, "test_source_deprecated_name");
+        assertThat(optionalPlugin, notNullValue());
+        assertThat(optionalPlugin.isPresent(), equalTo(true));
+        assertThat(optionalPlugin.get(), equalTo(TestSource.class));
+    }
+
     @Nested
     class WithPredefinedPlugins {
 

diff --git a/...repper-core/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java b/...repper-core/src/test/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryTest.java
@@ -6,6 +6,7 @@
 package org.opensearch.dataprepper.plugin;
 
 import org.mockito.ArgumentCaptor;
+import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin;
 import org.opensearch.dataprepper.model.configuration.PipelineDescription;
 import org.opensearch.dataprepper.model.configuration.PluginSetting;
 import org.opensearch.dataprepper.model.plugin.NoPluginFoundException;
@@ -284,4 +285,33 @@ void loadPlugins_should_return_an_instance_for_the_total_count() {
             assertThat(plugins.get(2), equalTo(expectedInstance3));
         }
     }
+
+    @Nested
+    class WithFoundDeprecatedPluginName {
+        private static final String TEST_SINK_DEPRECATED_NAME = "test_sink_deprecated_name";
+        private Class expectedPluginClass;
+
+        @BeforeEach
+        void setUp() {
+            expectedPluginClass = TestSink.class;
+            given(pluginSetting.getName()).willReturn(TEST_SINK_DEPRECATED_NAME);
+
+            given(firstPluginProvider.findPluginClass(baseClass, TEST_SINK_DEPRECATED_NAME))
+                    .willReturn(Optional.of(expectedPluginClass));
+        }
+
+        @Test
+        void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found_with_correct_name_and_deprecated_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(PluginArgumentsContext.class), eq(TEST_SINK_DEPRECATED_NAME)))
+                    .willReturn(expectedInstance);
+
+            assertThat(createObjectUnderTest().loadPlugin(baseClass, pluginSetting), equalTo(expectedInstance));
+            assertThat(expectedInstance.getClass().getAnnotation(DataPrepperPlugin.class).deprecatedName(), equalTo(TEST_SINK_DEPRECATED_NAME));
+            verify(beanFactoryProvider).get();
+        }
+    }
 }
diff --git a/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestSink.java b/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestSink.java
@@ -14,7 +14,7 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-@DataPrepperPlugin(name = "test_sink", pluginType = Sink.class)
+@DataPrepperPlugin(name = "test_sink", deprecatedName = "test_sink_deprecated_name", pluginType = Sink.class)
 public class TestSink implements Sink<Record<String>> {
     private final List<Record<String>> collectedRecords;
     private final boolean failSinkForTest;

diff --git a/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestSource.java b/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestSource.java
@@ -16,7 +16,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-@DataPrepperPlugin(name = "test_source", pluginType = Source.class)
+@DataPrepperPlugin(name = "test_source", 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());