Skip to content

Commit

Permalink
Make plugin config injectable (opensearch-project#5078)
Browse files Browse the repository at this point in the history
* making plugin config injectable

Signed-off-by: Santhosh Gandhe <[email protected]>

* End to End test case for plugin config injectable

Signed-off-by: Santhosh Gandhe <[email protected]>

* two additional test cases

Signed-off-by: Santhosh Gandhe <[email protected]>

* reorganized test classes to separate out different test plugins

Signed-off-by: Santhosh Gandhe <[email protected]>

* addressing review comments

Signed-off-by: Santhosh Gandhe <[email protected]>

* better assertions when no injected bean expected in the bean factory

Signed-off-by: Santhosh Gandhe <[email protected]>

---------

Signed-off-by: Santhosh Gandhe <[email protected]>
  • Loading branch information
san81 authored Oct 21, 2024
1 parent 87f0649 commit 3aecf2f
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
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.plugins.configtest.TestComponentWithConfigInject;
import org.opensearch.dataprepper.plugins.configtest.TestDISourceWithConfig;
import org.opensearch.dataprepper.validation.PluginErrorsHandler;
import org.opensearch.dataprepper.model.configuration.PipelinesDataFlowModel;
import org.opensearch.dataprepper.model.configuration.PluginSetting;
Expand Down Expand Up @@ -118,6 +120,32 @@ void loadPlugin_should_return_a_new_plugin_instance_with_DI_context_initialized(
assertThat(plugin.getTestComponent().getIdentifier(), equalTo("test-component"));
}

@Test
void loadPlugin_should_return_a_new_plugin_instance_with_DI_context_and_config_injected() {

final String requiredStringValue = UUID.randomUUID().toString();
final String optionalStringValue = UUID.randomUUID().toString();

final Map<String, Object> pluginSettingMap = new HashMap<>();
pluginSettingMap.put("required_string", requiredStringValue);
pluginSettingMap.put("optional_string", optionalStringValue);
final PluginSetting pluginSetting = new PluginSetting("test_di_source_with_config", pluginSettingMap);
pluginSetting.setPipelineName("test_di_source_with_config");

final Source sourcePlugin = createObjectUnderTest().loadPlugin(Source.class, pluginSetting);

assertThat(sourcePlugin, instanceOf(TestDISourceWithConfig.class));
TestDISourceWithConfig plugin = (TestDISourceWithConfig) sourcePlugin;
// Testing the auto wired been with the Dependency Injection
assertNotNull(plugin.getTestComponent());
assertInstanceOf(TestComponentWithConfigInject.class, plugin.getTestComponent());
TestPluginConfiguration pluginConfig = plugin.getTestComponent().getConfiguration();
assertInstanceOf(TestPluginConfiguration.class, pluginConfig);
assertThat(pluginConfig.getRequiredString(), equalTo(requiredStringValue));
assertThat(pluginConfig.getOptionalString(), equalTo(optionalStringValue));
assertThat(plugin.getTestComponent().getIdentifier(), equalTo("test-component-with-plugin-config-injected"));
}

@Test
void loadPlugin_should_return_a_new_plugin_instance_with_the_expected_configuration_variable_args() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private <T> ComponentPluginArgumentsContext getConstructionContext(final PluginS
.createDefaultPluginConfigObservable(pluginConfigurationConverter, pluginConfigurationType, pluginSetting);

Class[] markersToScan = pluginAnnotation.packagesToScan();
BeanFactory beanFactory = pluginBeanFactoryProvider.createPluginSpecificContext(markersToScan);
BeanFactory beanFactory = pluginBeanFactoryProvider.createPluginSpecificContext(markersToScan, configuration);

return new ComponentPluginArgumentsContext.Builder()
.withPluginSetting(pluginSetting)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@

package org.opensearch.dataprepper.plugin;

import org.opensearch.dataprepper.model.configuration.PluginSetting;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.support.GenericApplicationContext;
Expand Down Expand Up @@ -58,9 +60,13 @@ GenericApplicationContext getCoreApplicationContext() {
* instead, a new isolated {@link ApplicationContext} should be created.
* @return BeanFactory A BeanFactory that inherits from {@link PluginBeanFactoryProvider#sharedPluginApplicationContext}
*/
public BeanFactory createPluginSpecificContext(Class[] markersToScan) {
public BeanFactory createPluginSpecificContext(Class[] markersToScan, Object configuration) {
AnnotationConfigApplicationContext isolatedPluginApplicationContext = new AnnotationConfigApplicationContext();
DefaultListableBeanFactory beanFactory = (DefaultListableBeanFactory) isolatedPluginApplicationContext.getBeanFactory();
if(markersToScan !=null && markersToScan.length>0) {
if(configuration !=null && !(configuration instanceof PluginSetting)) {
beanFactory.registerSingleton(configuration.getClass().getName(), configuration);
}
// If packages to scan is provided in this plugin annotation, which indicates
// that this plugin is interested in using Dependency Injection isolated for its module
Arrays.stream(markersToScan)
Expand All @@ -69,6 +75,6 @@ public BeanFactory createPluginSpecificContext(Class[] markersToScan) {
isolatedPluginApplicationContext.refresh();
}
isolatedPluginApplicationContext.setParent(sharedPluginApplicationContext);
return isolatedPluginApplicationContext.getBeanFactory();
return beanFactory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ void loadPlugin_should_create_a_new_instance_of_the_plugin_with_di_initialized()
equalTo(expectedInstance));
verify(pluginConfigurationObservableFactory).createDefaultPluginConfigObservable(eq(pluginConfigurationConverter),
eq(PluginSetting.class), eq(pluginSetting));
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{TestDISource.class});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{TestDISource.class}, convertedConfiguration);
}

@Test
Expand All @@ -227,7 +227,7 @@ void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found() {
equalTo(expectedInstance));
verify(pluginConfigurationObservableFactory).createDefaultPluginConfigObservable(eq(pluginConfigurationConverter),
eq(PluginSetting.class), eq(pluginSetting));
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration);
}

@Test
Expand Down Expand Up @@ -261,7 +261,7 @@ void loadPlugins_should_return_an_empty_list_when_the_number_of_instances_is_0()
assertThat(plugins, notNullValue());
assertThat(plugins.size(), equalTo(0));

verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, null);
verifyNoInteractions(pluginCreator);
}

Expand All @@ -277,7 +277,7 @@ void loadPlugins_should_return_a_single_instance_when_the_the_numberOfInstances_
final List<?> plugins = createObjectUnderTest().loadPlugins(
baseClass, pluginSetting, c -> 1);

verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration);
verify(pluginConfigurationObservableFactory).createDefaultPluginConfigObservable(eq(pluginConfigurationConverter),
eq(PluginSetting.class), eq(pluginSetting));
final ArgumentCaptor<ComponentPluginArgumentsContext> pluginArgumentsContextArgCapture = ArgumentCaptor.forClass(ComponentPluginArgumentsContext.class);
Expand Down Expand Up @@ -306,7 +306,7 @@ void loadPlugin_with_varargs_should_return_a_single_instance_when_the_the_number

final Object plugin = createObjectUnderTest().loadPlugin(baseClass, pluginSetting, object);

verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration);
verify(pluginConfigurationObservableFactory).createDefaultPluginConfigObservable(eq(pluginConfigurationConverter),
eq(PluginSetting.class), eq(pluginSetting));
final ArgumentCaptor<ComponentPluginArgumentsContext> pluginArgumentsContextArgCapture = ArgumentCaptor.forClass(ComponentPluginArgumentsContext.class);
Expand Down Expand Up @@ -341,7 +341,7 @@ void loadPlugins_should_return_an_instance_for_the_total_count() {
final List<?> plugins = createObjectUnderTest().loadPlugins(
baseClass, pluginSetting, c -> 3);

verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration);
final ArgumentCaptor<ComponentPluginArgumentsContext> pluginArgumentsContextArgCapture = ArgumentCaptor.forClass(ComponentPluginArgumentsContext.class);
verify(pluginCreator, times(3)).newPluginInstance(eq(expectedPluginClass), pluginArgumentsContextArgCapture.capture(), eq(pluginName));
final List<ComponentPluginArgumentsContext> actualPluginArgumentsContextList = pluginArgumentsContextArgCapture.getAllValues();
Expand Down Expand Up @@ -377,7 +377,7 @@ void loadPlugins_should_return_a_single_instance_with_values_from_ApplicationCon
final List<?> plugins = createObjectUnderTest().loadPlugins(
baseClass, pluginSetting, c -> 1);

verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration);
final ArgumentCaptor<ComponentPluginArgumentsContext> pluginArgumentsContextArgCapture = ArgumentCaptor.forClass(ComponentPluginArgumentsContext.class);
verify(pluginCreator).newPluginInstance(eq(expectedPluginClass), pluginArgumentsContextArgCapture.capture(), eq(pluginName));
final ComponentPluginArgumentsContext actualPluginArgumentsContext = pluginArgumentsContextArgCapture.getValue();
Expand Down Expand Up @@ -419,7 +419,7 @@ void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found_with_corr

assertThat(createObjectUnderTest().loadPlugin(baseClass, pluginSetting), equalTo(expectedInstance));
MatcherAssert.assertThat(expectedInstance.getClass().getAnnotation(DataPrepperPlugin.class).deprecatedName(), equalTo(TEST_SINK_DEPRECATED_NAME));
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration);
}
}

Expand Down Expand Up @@ -448,7 +448,7 @@ void loadPlugin_should_create_a_new_instance_of_the_first_plugin_found_with_corr

assertThat(createObjectUnderTest().loadPlugin(baseClass, pluginSetting), equalTo(expectedInstance));
MatcherAssert.assertThat(expectedInstance.getClass().getAnnotation(DataPrepperPlugin.class).alternateNames(), equalTo(new String[]{TEST_SINK_ALTERNATE_NAME}));
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{});
verify(beanFactoryProvider).createPluginSpecificContext(new Class[]{}, convertedConfiguration);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,21 @@

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.opensearch.dataprepper.model.configuration.PluginSetting;
import org.opensearch.dataprepper.plugins.test.TestComponent;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.ListableBeanFactory;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.context.support.GenericApplicationContext;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
Expand Down Expand Up @@ -68,16 +78,16 @@ void testPluginBeanFactoryProviderGetReturnsBeanFactory() {
final PluginBeanFactoryProvider beanFactoryProvider = createObjectUnderTest();

verify(context).getParent();
assertThat(beanFactoryProvider.createPluginSpecificContext(new Class[]{}), is(instanceOf(BeanFactory.class)));
assertThat(beanFactoryProvider.createPluginSpecificContext(new Class[]{}, null), is(instanceOf(BeanFactory.class)));
}

@Test
void testPluginBeanFactoryProviderGetReturnsUniqueBeanFactory() {
doReturn(context).when(context).getParent();

final PluginBeanFactoryProvider beanFactoryProvider = createObjectUnderTest();
final BeanFactory isolatedBeanFactoryA = beanFactoryProvider.createPluginSpecificContext(new Class[]{});
final BeanFactory isolatedBeanFactoryB = beanFactoryProvider.createPluginSpecificContext(new Class[]{});
final BeanFactory isolatedBeanFactoryA = beanFactoryProvider.createPluginSpecificContext(new Class[]{}, null);
final BeanFactory isolatedBeanFactoryB = beanFactoryProvider.createPluginSpecificContext(new Class[]{}, null);

verify(context).getParent();
assertThat(isolatedBeanFactoryA, not(sameInstance(isolatedBeanFactoryB)));
Expand All @@ -103,7 +113,7 @@ void getSharedPluginApplicationContext_called_multiple_times_returns_same_instan
void testCreatePluginSpecificContext() {
when(context.getParent()).thenReturn(context);
final PluginBeanFactoryProvider objectUnderTest = createObjectUnderTest();
BeanFactory beanFactory = objectUnderTest.createPluginSpecificContext(new Class[]{TestComponent.class});
BeanFactory beanFactory = objectUnderTest.createPluginSpecificContext(new Class[]{TestComponent.class}, null);
assertThat(beanFactory, notNullValue());
assertThat(beanFactory.getBean(TestComponent.class), notNullValue());
}
Expand All @@ -112,8 +122,48 @@ void testCreatePluginSpecificContext() {
void testCreatePluginSpecificContext_with_empty_array() {
when(context.getParent()).thenReturn(context);
final PluginBeanFactoryProvider objectUnderTest = createObjectUnderTest();
BeanFactory beanFactory = objectUnderTest.createPluginSpecificContext(new Class[]{});
BeanFactory beanFactory = objectUnderTest.createPluginSpecificContext(new Class[]{}, null);
assertThat(beanFactory, notNullValue());
assertThat(beanFactory, instanceOf(ListableBeanFactory.class));
ListableBeanFactory listableBeanFactory = (ListableBeanFactory) beanFactory;
List<String> nonSpringBeans = Arrays.stream(listableBeanFactory.getBeanDefinitionNames())
.filter(Predicate.not(name -> name.startsWith("org.springframework")))
.collect(Collectors.toList());
assertThat(nonSpringBeans, equalTo(Collections.emptyList()));
}

@Test
void testCreatePluginSpecificContext_with_pipeline_settings() {
when(context.getParent()).thenReturn(context);
final PluginBeanFactoryProvider objectUnderTest = createObjectUnderTest();
PluginSetting pipelineSettings = new PluginSetting(UUID.randomUUID().toString(), Map.of("key", "val"));
BeanFactory beanFactory = objectUnderTest.createPluginSpecificContext(new Class[]{}, pipelineSettings);
assertThat(beanFactory, notNullValue());
assertThrows(NoSuchBeanDefinitionException.class, ()->beanFactory.getBean(PluginSetting.class));
}

@Test
void testCreatePluginSpecificContext_with_empty_array_with_plugin_config() {
when(context.getParent()).thenReturn(context);
final PluginBeanFactoryProvider objectUnderTest = createObjectUnderTest();
TestPluginConfiguration config = new TestPluginConfiguration();
BeanFactory beanFactory = objectUnderTest.createPluginSpecificContext(new Class[]{}, config);
assertThat(beanFactory, notNullValue());
assertThrows(NoSuchBeanDefinitionException.class, ()->beanFactory.getBean(TestComponent.class));
assertThrows(NoSuchBeanDefinitionException.class, ()->beanFactory.getBean(TestPluginConfiguration.class));
}

@Test
void testCreatePluginSpecificContext_with_plugin_config() {
when(context.getParent()).thenReturn(context);
final PluginBeanFactoryProvider objectUnderTest = createObjectUnderTest();
TestPluginConfiguration config = new TestPluginConfiguration();
String requiredStringValue = UUID.randomUUID().toString();
config.setRequiredString(requiredStringValue);
BeanFactory beanFactory = objectUnderTest.createPluginSpecificContext(new Class[]{TestComponent.class}, config);
assertThat(beanFactory, notNullValue());
assertThat(beanFactory.getBean(TestComponent.class), notNullValue());
assertThat(beanFactory.getBean(TestPluginConfiguration.class), notNullValue());
assertThat(beanFactory.getBean(TestPluginConfiguration.class).getRequiredString(), equalTo(requiredStringValue));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.opensearch.dataprepper.plugins.configtest;

import org.opensearch.dataprepper.plugin.TestPluginConfiguration;

import javax.inject.Named;

@Named
public class TestComponentWithConfigInject {
private final TestPluginConfiguration configuration;

public TestComponentWithConfigInject(TestPluginConfiguration configuration) {
this.configuration = configuration;
}

public String getIdentifier() {
return "test-component-with-plugin-config-injected";
}

public TestPluginConfiguration getConfiguration() {
return configuration;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.dataprepper.plugins.configtest;

import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin;
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor;
import org.opensearch.dataprepper.model.buffer.Buffer;
import org.opensearch.dataprepper.model.record.Record;
import org.opensearch.dataprepper.model.source.Source;
import org.opensearch.dataprepper.plugin.TestPluggableInterface;
import org.opensearch.dataprepper.plugin.TestPluginConfiguration;

@DataPrepperPlugin(name = "test_di_source_with_config",
alternateNames = { "test_source_alternate_name1", "test_source_alternate_name2" },
deprecatedName = "test_source_deprecated_name",
pluginType = Source.class,
packagesToScan = {TestDISourceWithConfig.class},
pluginConfigurationType = TestPluginConfiguration.class
)
public class TestDISourceWithConfig implements Source<Record<String>>, TestPluggableInterface {

private final TestComponentWithConfigInject testComponent;

@DataPrepperPluginConstructor
public TestDISourceWithConfig(final TestComponentWithConfigInject testComponent) {
this.testComponent = testComponent;
}

@Override
public void start(Buffer<Record<String>> buffer) {
}

public TestComponentWithConfigInject getTestComponent() {
return testComponent;
}

@Override
public void stop() {}
}

0 comments on commit 3aecf2f

Please sign in to comment.