From 0bb02eb9f3255c853200a2120d587ec7e964db3d Mon Sep 17 00:00:00 2001 From: David Venable Date: Thu, 9 Jan 2025 12:07:55 -0600 Subject: [PATCH 1/2] Introduces the experimental plugin feature, allowing plugin developers to mark plugins as experimental. This PR adds a new annotation @Experimental which can be applied to plugin types. When loading plugins, the plugin framework will now check whether the plugin is experimental or not. If the plugin is experimental, Data Prepper will fail if experimental plugins are not enabled. This PR also adds a new configuration in data-prepper-config.yaml to enable all experimental plugins if they are desired. Additionally, I refactored the code that logs deprecated plugin names into a consumer to help the overall code structure and fit the code design I took for checking for experimental plugins. Resolves #2695 Signed-off-by: David Venable --- .../model/annotations/Experimental.java | 29 ++++++ .../plugin/DefaultPluginFactoryIT.java | 22 +++++ .../model/DataPrepperConfiguration.java | 13 ++- .../core/parser/PipelineTransformerTests.java | 1 + .../plugins/TestExperimentalPlugin.java | 15 +++ data-prepper-plugin-framework/build.gradle | 1 + .../plugin/DefaultPluginFactory.java | 18 ++-- .../dataprepper/plugin/DefinedPlugin.java | 26 +++++ .../plugin/DeprecatedPluginDetector.java | 31 ++++++ .../plugin/ExperimentalConfiguration.java | 31 ++++++ .../ExperimentalConfigurationContainer.java | 21 ++++ .../plugin/ExperimentalPluginValidator.java | 33 +++++++ .../plugin/DefaultPluginFactoryTest.java | 41 +++++++- .../plugin/DeprecatedPluginDetectorTest.java | 96 +++++++++++++++++++ .../plugin/ExperimentalConfigurationTest.java | 21 ++++ .../ExperimentalPluginValidatorTest.java | 88 +++++++++++++++++ 16 files changed, 477 insertions(+), 10 deletions(-) create mode 100644 data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java create mode 100644 data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java create mode 100644 data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java create mode 100644 data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java create mode 100644 data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java create mode 100644 data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java create mode 100644 data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java create mode 100644 data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java create mode 100644 data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java create mode 100644 data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java diff --git a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java new file mode 100644 index 0000000000..63c7f52471 --- /dev/null +++ b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java @@ -0,0 +1,29 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.model.annotations; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a Data Prepper plugin as experimental. + *

+ * Experimental plugins do not have the same compatibility guarantees as other plugins and may be unstable. + * They may have breaking changes between minor versions and may even be removed. + *

+ * Data Prepper administrators must enable experimental plugins in order to use them. + * Otherwise, they are not available to use with pipelines. + * + * @since 2.11 + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE}) +public @interface Experimental { +} diff --git a/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryIT.java b/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryIT.java index 27dfa97cef..9230aa7ff8 100644 --- a/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryIT.java +++ b/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/plugin/DefaultPluginFactoryIT.java @@ -14,6 +14,7 @@ import org.opensearch.dataprepper.core.event.EventFactoryApplicationContextMarker; import org.opensearch.dataprepper.core.validation.LoggingPluginErrorsHandler; import org.opensearch.dataprepper.core.validation.PluginErrorCollector; +import org.opensearch.dataprepper.model.plugin.NoPluginFoundException; import org.opensearch.dataprepper.plugins.configtest.TestComponentWithConfigInject; import org.opensearch.dataprepper.plugins.configtest.TestDISourceWithConfig; import org.opensearch.dataprepper.validation.PluginErrorsHandler; @@ -27,6 +28,7 @@ import org.opensearch.dataprepper.plugins.test.TestPlugin; import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.UUID; @@ -38,6 +40,7 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; /** * Integration test of the plugin framework. These tests should not mock any portion @@ -49,6 +52,8 @@ class DefaultPluginFactoryIT { private PipelinesDataFlowModel pipelinesDataFlowModel; @Mock private ExtensionsConfiguration extensionsConfiguration; + @Mock + private ExperimentalConfigurationContainer experimentalConfigurationContainer; private String pluginName; private String objectPluginName; private String pipelineName; @@ -67,6 +72,8 @@ private DefaultPluginFactory createObjectUnderTest() { final AnnotationConfigApplicationContext coreContext = new AnnotationConfigApplicationContext(); coreContext.setParent(publicContext); + when(experimentalConfigurationContainer.getExperimental()).thenReturn(ExperimentalConfiguration.defaultConfiguration()); + coreContext.scan(EventFactoryApplicationContextMarker.class.getPackage().getName()); coreContext.scan(DefaultAcknowledgementSetManager.class.getPackage().getName()); coreContext.scan(DefaultPluginFactory.class.getPackage().getName()); @@ -75,6 +82,7 @@ private DefaultPluginFactory createObjectUnderTest() { coreContext.registerBean(PluginErrorsHandler.class, LoggingPluginErrorsHandler::new); coreContext.registerBean(ExtensionsConfiguration.class, () -> extensionsConfiguration); coreContext.registerBean(PipelinesDataFlowModel.class, () -> pipelinesDataFlowModel); + coreContext.registerBean(ExperimentalConfigurationContainer.class, () -> experimentalConfigurationContainer); coreContext.refresh(); return coreContext.getBean(DefaultPluginFactory.class); @@ -188,6 +196,20 @@ void loadPlugin_should_throw_when_a_plugin_configuration_is_invalid() { assertThat(actualException.getMessage(), equalTo("Plugin test_plugin in pipeline " + pipelineName + " is configured incorrectly: requiredString must not be null")); } + @Test + void loadPlugin_should_throw_when_a_plugin_is_experimental_by_default() { + pluginName = "test_experimental_plugin"; + final PluginSetting pluginSetting = createPluginSettings(Collections.emptyMap()); + + final DefaultPluginFactory objectUnderTest = createObjectUnderTest(); + + final NoPluginFoundException actualException = assertThrows(NoPluginFoundException.class, + () -> objectUnderTest.loadPlugin(TestPluggableInterface.class, pluginSetting)); + + assertThat(actualException.getMessage(), notNullValue()); + assertThat(actualException.getMessage(), equalTo("Unable to create experimental plugin test_experimental_plugin. You must enable experimental plugins in data-prepper-config.yaml in order to use them.")); + } + private PluginSetting createPluginSettings(final Map pluginSettingMap) { final PluginSetting pluginSetting = new PluginSetting(pluginName, pluginSettingMap); pluginSetting.setPipelineName(pipelineName); diff --git a/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/model/DataPrepperConfiguration.java b/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/model/DataPrepperConfiguration.java index 9212a9943b..046653dcd6 100644 --- a/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/model/DataPrepperConfiguration.java +++ b/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/model/DataPrepperConfiguration.java @@ -18,6 +18,8 @@ import org.opensearch.dataprepper.core.pipeline.PipelineShutdownOption; import org.opensearch.dataprepper.model.configuration.PipelineExtensions; import org.opensearch.dataprepper.model.configuration.PluginModel; +import org.opensearch.dataprepper.plugin.ExperimentalConfiguration; +import org.opensearch.dataprepper.plugin.ExperimentalConfigurationContainer; import org.opensearch.dataprepper.plugin.ExtensionsConfiguration; import java.time.Duration; @@ -31,7 +33,7 @@ /** * Class to hold configuration for DataPrepper, including server port and Log4j settings */ -public class DataPrepperConfiguration implements ExtensionsConfiguration, EventConfigurationContainer { +public class DataPrepperConfiguration implements ExtensionsConfiguration, EventConfigurationContainer, ExperimentalConfigurationContainer { static final Duration DEFAULT_SHUTDOWN_DURATION = Duration.ofSeconds(30L); private static final String DEFAULT_SOURCE_COORDINATION_STORE = "in_memory"; @@ -55,6 +57,7 @@ public class DataPrepperConfiguration implements ExtensionsConfiguration, EventC private PeerForwarderConfiguration peerForwarderConfiguration; private Duration processorShutdownTimeout; private Duration sinkShutdownTimeout; + private ExperimentalConfiguration experimental; private PipelineExtensions pipelineExtensions; public static final DataPrepperConfiguration DEFAULT_CONFIG = new DataPrepperConfiguration(); @@ -96,6 +99,7 @@ public DataPrepperConfiguration( @JsonProperty("source_coordination") final SourceCoordinationConfig sourceCoordinationConfig, @JsonProperty("pipeline_shutdown") final PipelineShutdownOption pipelineShutdown, @JsonProperty("event") final EventConfiguration eventConfiguration, + @JsonProperty("experimental") final ExperimentalConfiguration experimental, @JsonProperty("extensions") @JsonInclude(JsonInclude.Include.NON_NULL) @JsonSetter(nulls = Nulls.SKIP) @@ -126,6 +130,8 @@ public DataPrepperConfiguration( if (this.sinkShutdownTimeout.isNegative()) { throw new IllegalArgumentException("sinkShutdownTimeout must be non-negative."); } + this.experimental = experimental != null ? experimental : ExperimentalConfiguration.defaultConfiguration(); + this.pipelineExtensions = pipelineExtensions; } @@ -239,4 +245,9 @@ public EventConfiguration getEventConfiguration() { public PipelineExtensions getPipelineExtensions() { return pipelineExtensions; } + + @Override + public ExperimentalConfiguration getExperimental() { + return experimental; + } } diff --git a/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java b/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java index 9caa18820f..53888d6b66 100644 --- a/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java +++ b/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java @@ -140,6 +140,7 @@ void setUp() { @AfterEach void tearDown() { verify(dataPrepperConfiguration).getEventConfiguration(); + verify(dataPrepperConfiguration).getExperimental(); verifyNoMoreInteractions(dataPrepperConfiguration); } diff --git a/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java b/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java new file mode 100644 index 0000000000..cda130f889 --- /dev/null +++ b/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java @@ -0,0 +1,15 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugins; + +import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; +import org.opensearch.dataprepper.model.annotations.Experimental; +import org.opensearch.dataprepper.plugin.TestPluggableInterface; + +@DataPrepperPlugin(name = "test_experimental_plugin", pluginType = TestPluggableInterface.class) +@Experimental +public class TestExperimentalPlugin { +} diff --git a/data-prepper-plugin-framework/build.gradle b/data-prepper-plugin-framework/build.gradle index 8eff39b1dc..894132b473 100644 --- a/data-prepper-plugin-framework/build.gradle +++ b/data-prepper-plugin-framework/build.gradle @@ -25,4 +25,5 @@ dependencies { implementation libs.reflections.core implementation 'com.fasterxml.jackson.core:jackson-databind' implementation 'org.apache.commons:commons-text:1.10.0' + testImplementation 'ch.qos.logback:logback-classic:1.5.16' } \ No newline at end of file diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefaultPluginFactory.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefaultPluginFactory.java index 81fd1c2b5b..0ec3b5a953 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefaultPluginFactory.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefaultPluginFactory.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.function.Consumer; import java.util.function.Function; /** @@ -41,6 +42,7 @@ public class DefaultPluginFactory implements PluginFactory { private final PluginBeanFactoryProvider pluginBeanFactoryProvider; private final PluginConfigurationObservableFactory pluginConfigurationObservableFactory; private final ApplicationContextToTypedSuppliers applicationContextToTypedSuppliers; + private final List>> definedPluginConsumers; @Inject DefaultPluginFactory( @@ -49,8 +51,10 @@ public class DefaultPluginFactory implements PluginFactory { final PluginConfigurationConverter pluginConfigurationConverter, final PluginBeanFactoryProvider pluginBeanFactoryProvider, final PluginConfigurationObservableFactory pluginConfigurationObservableFactory, - final ApplicationContextToTypedSuppliers applicationContextToTypedSuppliers) { + final ApplicationContextToTypedSuppliers applicationContextToTypedSuppliers, + final List>> definedPluginConsumers) { this.applicationContextToTypedSuppliers = applicationContextToTypedSuppliers; + this.definedPluginConsumers = definedPluginConsumers; Objects.requireNonNull(pluginProviderLoader); Objects.requireNonNull(pluginConfigurationObservableFactory); this.pluginCreator = Objects.requireNonNull(pluginCreator); @@ -140,15 +144,13 @@ private Class getPluginClass(final Class baseClass, final St .orElseThrow(() -> new NoPluginFoundException( "Unable to find a plugin named '" + pluginName + "'. Please ensure that plugin is annotated with appropriate values.")); - logDeprecatedPluginsNames(pluginClass, pluginName); + handleDefinedPlugins(pluginClass, pluginName); return pluginClass; } - private void logDeprecatedPluginsNames(final Class 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); - } + private void handleDefinedPlugins(final Class pluginClass, final String pluginName) { + final DefinedPlugin definedPlugin = new DefinedPlugin<>(pluginClass, pluginName); + + definedPluginConsumers.forEach(definedPluginConsumer -> definedPluginConsumer.accept(definedPlugin)); } } diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java new file mode 100644 index 0000000000..959b5ba577 --- /dev/null +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java @@ -0,0 +1,26 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +import java.util.Objects; + +class DefinedPlugin { + private final Class pluginClass; + private final String pluginName; + + public DefinedPlugin(final Class pluginClass, final String pluginName) { + this.pluginClass = Objects.requireNonNull(pluginClass); + this.pluginName = Objects.requireNonNull(pluginName); + } + + public Class getPluginClass() { + return pluginClass; + } + + public String getPluginName() { + return pluginName; + } +} diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java new file mode 100644 index 0000000000..6740aff91e --- /dev/null +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java @@ -0,0 +1,31 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.inject.Named; +import java.util.function.Consumer; + +@Named +class DeprecatedPluginDetector implements Consumer> { + private static final Logger LOG = LoggerFactory.getLogger(DeprecatedPluginDetector.class); + + @Override + public void accept(final DefinedPlugin definedPlugin) { + logDeprecatedPluginsNames(definedPlugin.getPluginClass(), definedPlugin.getPluginName()); + } + + private void logDeprecatedPluginsNames(final Class 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/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java new file mode 100644 index 0000000000..0b7d9e2926 --- /dev/null +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java @@ -0,0 +1,31 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * Data Prepper configurations for experimental features. + * + * @since 2.11 + */ +public class ExperimentalConfiguration { + @JsonProperty("enable_all") + private boolean enableAll = false; + + public static ExperimentalConfiguration defaultConfiguration() { + return new ExperimentalConfiguration(); + } + + /** + * Gets whether all experimental features are enabled. + * @return true if all experimental features are enabled, false otherwise + * @since 2.11 + */ + public boolean isEnableAll() { + return enableAll; + } +} diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java new file mode 100644 index 0000000000..db4c1fd8a2 --- /dev/null +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java @@ -0,0 +1,21 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +/** + * Interface to decouple how an experimental configuration is defined from + * usage of those configurations. + * + * @since 2.11 + */ +public interface ExperimentalConfigurationContainer { + /** + * Gets the experimental configuration. + * @return the experimental configuration + * @since 2.11 + */ + ExperimentalConfiguration getExperimental(); +} diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java new file mode 100644 index 0000000000..0db808d8a4 --- /dev/null +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java @@ -0,0 +1,33 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +import org.opensearch.dataprepper.model.annotations.Experimental; +import org.opensearch.dataprepper.model.plugin.NoPluginFoundException; + +import javax.inject.Named; +import java.util.function.Consumer; + +@Named +class ExperimentalPluginValidator implements Consumer> { + private final ExperimentalConfiguration experimentalConfiguration; + + ExperimentalPluginValidator(final ExperimentalConfigurationContainer experimentalConfigurationContainer) { + this.experimentalConfiguration = experimentalConfigurationContainer.getExperimental(); + } + + @Override + public void accept(final DefinedPlugin definedPlugin) { + if(isPluginDisallowedAsExperimental(definedPlugin.getPluginClass())) { + throw new NoPluginFoundException("Unable to create experimental plugin " + definedPlugin.getPluginName() + + ". You must enable experimental plugins in data-prepper-config.yaml in order to use them."); + } + } + + private boolean isPluginDisallowedAsExperimental(final Class pluginClass) { + return pluginClass.isAnnotationPresent(Experimental.class) && !experimentalConfiguration.isEnableAll(); + } +} 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 6f54a55c95..2c1bf9e0fa 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 @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.function.Consumer; import java.util.function.Supplier; import static org.hamcrest.CoreMatchers.equalTo; @@ -38,6 +39,7 @@ import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.CoreMatchers.sameInstance; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -64,6 +66,7 @@ class DefaultPluginFactoryTest { private PluginConfigurationObservableFactory pluginConfigurationObservableFactory; private PluginConfigObservable pluginConfigObservable; private ApplicationContextToTypedSuppliers applicationContextToTypedSuppliers; + private List>> definedPluginConsumers; @BeforeEach void setUp() { @@ -92,6 +95,8 @@ void setUp() { )).willReturn(pluginConfigObservable); applicationContextToTypedSuppliers = mock(ApplicationContextToTypedSuppliers.class); + + definedPluginConsumers = List.of(mock(Consumer.class), mock(Consumer.class)); } private DefaultPluginFactory createObjectUnderTest() { @@ -99,7 +104,8 @@ private DefaultPluginFactory createObjectUnderTest() { pluginProviderLoader, pluginCreator, pluginConfigurationConverter, beanFactoryProvider, pluginConfigurationObservableFactory, - applicationContextToTypedSuppliers); + applicationContextToTypedSuppliers, + definedPluginConsumers); } @Test @@ -230,6 +236,22 @@ void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found() { verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration); } + @Test + void loadPlugin_should_call_all_definedPluginConsumers() { + createObjectUnderTest().loadPlugin(baseClass, pluginSetting); + + assertThat("This test is not valid if there are no defined plugin consumers.", + definedPluginConsumers.size(), greaterThanOrEqualTo(2)); + for (final Consumer> definedPluginConsumer : definedPluginConsumers) { + final ArgumentCaptor> definedPluginArgumentCaptor = ArgumentCaptor.forClass(DefinedPlugin.class); + verify(definedPluginConsumer).accept(definedPluginArgumentCaptor.capture()); + + final DefinedPlugin actualDefinedPlugin = definedPluginArgumentCaptor.getValue(); + assertThat(actualDefinedPlugin.getPluginClass(), equalTo(expectedPluginClass)); + assertThat(actualDefinedPlugin.getPluginName(), equalTo(pluginName)); + } + } + @Test void loadPlugins_should_throw_for_null_number_of_instances() { @@ -322,6 +344,23 @@ void loadPlugin_with_varargs_should_return_a_single_instance_when_the_the_number assertThat(plugin, equalTo(expectedInstance)); } + @Test + void loadPlugin_with_varargs_should_call_all_definedPluginConsumers() { + final Object vararg1 = new Object(); + createObjectUnderTest().loadPlugin(baseClass, pluginSetting, vararg1); + + assertThat("This test is not valid if there are no defined plugin consumers.", + definedPluginConsumers.size(), greaterThanOrEqualTo(2)); + for (final Consumer> definedPluginConsumer : definedPluginConsumers) { + final ArgumentCaptor> definedPluginArgumentCaptor = ArgumentCaptor.forClass(DefinedPlugin.class); + verify(definedPluginConsumer).accept(definedPluginArgumentCaptor.capture()); + + final DefinedPlugin actualDefinedPlugin = definedPluginArgumentCaptor.getValue(); + assertThat(actualDefinedPlugin.getPluginClass(), equalTo(expectedPluginClass)); + assertThat(actualDefinedPlugin.getPluginName(), equalTo(pluginName)); + } + } + @Test void loadPlugins_should_return_an_instance_for_the_total_count() { final TestSink expectedInstance1 = mock(TestSink.class); diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java new file mode 100644 index 0000000000..92bcd96414 --- /dev/null +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java @@ -0,0 +1,96 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.AppenderBase; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; +import org.opensearch.dataprepper.model.processor.Processor; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsEmptyCollection.empty; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DeprecatedPluginDetectorTest { + @Mock + private DefinedPlugin definedPlugin; + private TestLogAppender testAppender; + + @BeforeEach + void setUp() { + final Logger logger = (Logger) LoggerFactory.getLogger(DeprecatedPluginDetector.class); + + testAppender = new TestLogAppender(); + testAppender.start(); + logger.addAppender(testAppender); + } + + private DeprecatedPluginDetector createObjectUnderTest() { + return new DeprecatedPluginDetector(); + } + + @Test + void accept_on_plugin_without_deprecated_name_does_not_log() { + when(definedPlugin.getPluginClass()).thenReturn(PluginWithoutDeprecatedName.class); + createObjectUnderTest().accept(definedPlugin); + + assertThat(testAppender.getLoggedEvents(), empty()); + } + + @Test + void accept_on_plugin_with_deprecated_name_does_not_log_if_new_name_is_used() { + when(definedPlugin.getPluginClass()).thenReturn(PluginWithDeprecatedName.class); + when(definedPlugin.getPluginName()).thenReturn("test_for_deprecated_detection"); + createObjectUnderTest().accept(definedPlugin); + + assertThat(testAppender.getLoggedEvents(), empty()); + } + + @Test + void accept_on_plugin_with_deprecated_name_logs_if_deprecated_name_is_used() { + when(definedPlugin.getPluginClass()).thenReturn(PluginWithDeprecatedName.class); + when(definedPlugin.getPluginName()).thenReturn("test_for_deprecated_detection_deprecated_name"); + createObjectUnderTest().accept(definedPlugin); + + assertThat(testAppender.getLoggedEvents().stream() + .anyMatch(event -> event.getFormattedMessage().contains("Plugin name 'test_for_deprecated_detection_deprecated_name' is deprecated and will be removed in the next major release. Consider using the updated plugin name 'test_for_deprecated_detection'.")), + equalTo(true)); + } + + @DataPrepperPlugin(name = "test_for_deprecated_detection", pluginType = Processor.class) + private static class PluginWithoutDeprecatedName { + } + + @DataPrepperPlugin(name = "test_for_deprecated_detection", pluginType = Processor.class, deprecatedName = "test_for_deprecated_detection_deprecated_name") + private static class PluginWithDeprecatedName { + } + + public static class TestLogAppender extends AppenderBase { + private final List events = new ArrayList<>(); + + @Override + protected void append(final ILoggingEvent eventObject) { + events.add(eventObject); + } + + public List getLoggedEvents() { + return Collections.unmodifiableList(events); + } + } +} \ No newline at end of file diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java new file mode 100644 index 0000000000..8c3fad4023 --- /dev/null +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java @@ -0,0 +1,21 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; + +class ExperimentalConfigurationTest { + @Test + void defaultConfiguration_should_return_config_with_isEnableAll_false() { + final ExperimentalConfiguration objectUnderTest = ExperimentalConfiguration.defaultConfiguration(); + assertThat(objectUnderTest, notNullValue()); + assertThat(objectUnderTest.isEnableAll(), equalTo(false)); + } +} \ No newline at end of file diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java new file mode 100644 index 0000000000..f0a4c1b5cb --- /dev/null +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java @@ -0,0 +1,88 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.dataprepper.plugin; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.dataprepper.model.annotations.Experimental; +import org.opensearch.dataprepper.model.plugin.NoPluginFoundException; + +import java.util.UUID; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ExperimentalPluginValidatorTest { + + @Mock + private ExperimentalConfigurationContainer experimentalConfigurationContainer; + + @Mock + private ExperimentalConfiguration experimentalConfiguration; + + @Mock + private DefinedPlugin definedPlugin; + + @BeforeEach + void setUp() { + when(experimentalConfigurationContainer.getExperimental()).thenReturn(experimentalConfiguration); + } + + private ExperimentalPluginValidator createObjectUnderTest() { + return new ExperimentalPluginValidator(experimentalConfigurationContainer); + } + + @Test + void accept_with_non_Experimental_plugin_returns() { + when(definedPlugin.getPluginClass()).thenReturn(NonExperimentalPlugin.class); + + createObjectUnderTest().accept(definedPlugin); + } + + @Nested + class WithExperimentalPlugin { + @BeforeEach + void setUp() { + when(definedPlugin.getPluginClass()).thenReturn(ExperimentalPlugin.class); + } + + @Test + void accept_with_Experimental_plugin_throws_if_experimental_is_not_enabled() { + final String pluginName = UUID.randomUUID().toString(); + when(definedPlugin.getPluginName()).thenReturn(pluginName); + + final ExperimentalPluginValidator objectUnderTest = createObjectUnderTest(); + + final NoPluginFoundException actualException = assertThrows(NoPluginFoundException.class, () -> objectUnderTest.accept(definedPlugin)); + + assertThat(actualException.getMessage(), notNullValue()); + assertThat(actualException.getMessage(), containsString(pluginName)); + assertThat(actualException.getMessage(), containsString("experimental plugin")); + } + + @Test + void accept_with_Experimental_plugin_does_not_throw_if_experimental_is_enabled() { + when(experimentalConfiguration.isEnableAll()).thenReturn(true); + + createObjectUnderTest().accept(definedPlugin); + } + } + + private static class NonExperimentalPlugin { + } + + @Experimental + private static class ExperimentalPlugin { + } +} \ No newline at end of file From 23d0006c1ddaa4c9cba2524f31a1230ef6b0153f Mon Sep 17 00:00:00 2001 From: David Venable Date: Wed, 15 Jan 2025 09:35:35 -0600 Subject: [PATCH 2/2] Updated the copyright header to the newer version. Signed-off-by: David Venable --- .../dataprepper/model/annotations/Experimental.java | 4 ++++ .../dataprepper/plugins/TestExperimentalPlugin.java | 4 ++++ .../java/org/opensearch/dataprepper/plugin/DefinedPlugin.java | 4 ++++ .../dataprepper/plugin/DeprecatedPluginDetector.java | 4 ++++ .../dataprepper/plugin/ExperimentalConfiguration.java | 4 ++++ .../plugin/ExperimentalConfigurationContainer.java | 4 ++++ .../dataprepper/plugin/ExperimentalPluginValidator.java | 4 ++++ .../dataprepper/plugin/DeprecatedPluginDetectorTest.java | 4 ++++ .../dataprepper/plugin/ExperimentalConfigurationTest.java | 4 ++++ .../dataprepper/plugin/ExperimentalPluginValidatorTest.java | 4 ++++ 10 files changed, 40 insertions(+) diff --git a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java index 63c7f52471..8b97475f54 100644 --- a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java +++ b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/annotations/Experimental.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.model.annotations; diff --git a/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java b/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java index cda130f889..36f334e526 100644 --- a/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java +++ b/data-prepper-core/src/test/java/org/opensearch/dataprepper/plugins/TestExperimentalPlugin.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugins; diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java index 959b5ba577..7f0b550a89 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DefinedPlugin.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin; diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java index 6740aff91e..aab8a8120f 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetector.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin; diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java index 0b7d9e2926..5a30e529a4 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfiguration.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin; diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java index db4c1fd8a2..e2ba00657f 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationContainer.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin; diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java index 0db808d8a4..ffd835de41 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidator.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin; diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java index 92bcd96414..f5325968a2 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DeprecatedPluginDetectorTest.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin; diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java index 8c3fad4023..0a3df6a892 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalConfigurationTest.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin; diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java index f0a4c1b5cb..78d008b222 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExperimentalPluginValidatorTest.java @@ -1,6 +1,10 @@ /* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ package org.opensearch.dataprepper.plugin;