Skip to content

Commit cfe0af4

Browse files
kkondakaJonah Calvo
kkondaka
authored and
Jonah Calvo
committed
Support alternative extension implementations (opensearch-project#5816)
* Support alternative extension implementations Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments and added tests Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments and added tests Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Added tests Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> --------- Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> Signed-off-by: Jonah Calvo <caljonah@amazon.com>
1 parent 49b2d05 commit cfe0af4

7 files changed

Lines changed: 168 additions & 5 deletions

File tree

‎data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/DataPrepperExtensionPoints.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,16 @@
1212

1313
import javax.inject.Inject;
1414
import javax.inject.Named;
15+
import java.util.HashSet;
16+
import java.util.Set;
1517
import java.util.Objects;
1618

1719
@Named
1820
public class DataPrepperExtensionPoints implements ExtensionPoints {
1921
private static final ExtensionProvider.Context EMPTY_CONTEXT = new EmptyContext();
2022
private final GenericApplicationContext sharedApplicationContext;
2123
private final GenericApplicationContext coreApplicationContext;
24+
private Set<Class> providerClassesSet;
2225

2326
@Inject
2427
public DataPrepperExtensionPoints(
@@ -28,10 +31,14 @@ public DataPrepperExtensionPoints(
2831
Objects.requireNonNull(pluginBeanFactoryProvider.getSharedPluginApplicationContext());
2932
this.sharedApplicationContext = pluginBeanFactoryProvider.getSharedPluginApplicationContext();
3033
this.coreApplicationContext = pluginBeanFactoryProvider.getCoreApplicationContext();
34+
this.providerClassesSet = new HashSet<>();
3135
}
3236

3337
@Override
3438
public void addExtensionProvider(final ExtensionProvider extensionProvider) {
39+
if (providerClassesSet.contains(extensionProvider.supportedClass())) {
40+
return;
41+
}
3542
coreApplicationContext.registerBean(
3643
extensionProvider.supportedClass(),
3744
() -> extensionProvider.provideInstance(EMPTY_CONTEXT).orElse(null),
@@ -40,6 +47,7 @@ public void addExtensionProvider(final ExtensionProvider extensionProvider) {
4047
extensionProvider.supportedClass(),
4148
() -> extensionProvider.provideInstance(EMPTY_CONTEXT),
4249
b -> b.setScope(BeanDefinition.SCOPE_PROTOTYPE));
50+
providerClassesSet.add(extensionProvider.supportedClass());
4351
}
4452

4553
private static class EmptyContext implements ExtensionProvider.Context {

‎data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ExtensionLoader.java‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,33 @@
1717
import javax.inject.Inject;
1818
import javax.inject.Named;
1919
import java.util.Arrays;
20+
import java.util.Comparator;
2021
import java.util.List;
2122
import java.util.Objects;
2223
import java.util.stream.Collectors;
2324

2425
@Named
2526
public class ExtensionLoader {
27+
public class ExtensionPluginWithContext {
28+
ExtensionPlugin extensionPlugin;
29+
boolean configured;
30+
31+
public ExtensionPluginWithContext(final ExtensionPlugin extensionPlugin, final boolean isConfigured) {
32+
this.extensionPlugin = extensionPlugin;
33+
this.configured = isConfigured;
34+
}
35+
36+
public ExtensionPlugin getExtensionPlugin() {
37+
return extensionPlugin;
38+
}
39+
40+
public boolean isConfigured() {
41+
return configured;
42+
}
43+
}
44+
45+
private Comparator<ExtensionLoader.ExtensionPluginWithContext> extensionsLoaderComparator;
46+
2647
private final ExtensionPluginConfigurationConverter extensionPluginConfigurationConverter;
2748
private final ExtensionClassProvider extensionClassProvider;
2849
private final PluginCreator extensionPluginCreator;
@@ -35,23 +56,26 @@ public class ExtensionLoader {
3556
final ExtensionClassProvider extensionClassProvider,
3657
@Named("extensionPluginCreator") final PluginCreator extensionPluginCreator,
3758
final PluginErrorCollector pluginErrorCollector,
38-
final PluginErrorsHandler pluginErrorsHandler) {
59+
final PluginErrorsHandler pluginErrorsHandler,
60+
@Named("extensionsLoaderComparator") final Comparator<ExtensionLoader.ExtensionPluginWithContext> extensionsLoaderComparator) {
3961
this.extensionPluginConfigurationConverter = extensionPluginConfigurationConverter;
4062
this.extensionClassProvider = extensionClassProvider;
4163
this.extensionPluginCreator = extensionPluginCreator;
4264
this.pluginErrorCollector = pluginErrorCollector;
4365
this.pluginErrorsHandler = pluginErrorsHandler;
66+
this.extensionsLoaderComparator = Objects.requireNonNull(extensionsLoaderComparator);
4467
}
4568

4669
public List<? extends ExtensionPlugin> loadExtensions() {
47-
final List<? extends ExtensionPlugin> result = extensionClassProvider.loadExtensionPluginClasses()
70+
final List<ExtensionPlugin> result = extensionClassProvider.loadExtensionPluginClasses()
4871
.stream()
4972
.map(extensionClass -> {
5073
final String pluginName = convertClassToName(extensionClass);
5174
try {
5275
final PluginArgumentsContext pluginArgumentsContext = getConstructionContext(extensionClass);
53-
return extensionPluginCreator.newPluginInstance(
54-
extensionClass, pluginArgumentsContext, pluginName);
76+
final Object config = pluginArgumentsContext.getArgument(0);
77+
return new ExtensionPluginWithContext(extensionPluginCreator.newPluginInstance(
78+
extensionClass, pluginArgumentsContext, pluginName), (config != null));
5579
} catch (Exception e) {
5680
final PluginError pluginError = PluginError.builder()
5781
.componentType(PipelinesDataFlowModel.EXTENSION_PLUGIN_TYPE)
@@ -62,7 +86,11 @@ public List<? extends ExtensionPlugin> loadExtensions() {
6286
return null;
6387
}
6488
})
89+
.filter(pluginWithContext -> pluginWithContext != null)
90+
.sorted(extensionsLoaderComparator)
91+
.map(extensionPluginWithContext -> extensionPluginWithContext.getExtensionPlugin())
6592
.collect(Collectors.toList());
93+
6694
final List<PluginError> extensionPluginErrors = pluginErrorCollector.getPluginErrors()
6795
.stream().filter(pluginError -> PipelinesDataFlowModel.EXTENSION_PLUGIN_TYPE
6896
.equals(pluginError.getComponentType()))
@@ -133,5 +161,10 @@ public Object[] createArguments(Class<?>[] parameterTypes, final Object ... args
133161
}
134162
return new Object[] { extensionPluginConfiguration };
135163
}
164+
165+
@Override
166+
public Object getArgument(int index) {
167+
return (index == 0) ? extensionPluginConfiguration : null;
168+
}
136169
}
137170
}

‎data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginArgumentsContext.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,8 @@
77

88
interface PluginArgumentsContext {
99
Object[] createArguments(final Class<?>[] parameterTypes, final Object ... args);
10+
11+
default Object getArgument(int index) {
12+
return null;
13+
}
1014
}

‎data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginCreatorContext.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import javax.inject.Named;
66

7+
import java.util.Comparator;
8+
79
@Named
810
public class PluginCreatorContext {
911
@Bean(name = "extensionPluginCreator")
@@ -16,4 +18,9 @@ public PluginCreator pluginCreator(
1618
final PluginConfigurationObservableRegister pluginConfigurationObservableRegister) {
1719
return new PluginCreator(pluginConfigurationObservableRegister);
1820
}
21+
22+
@Bean(name = "extensionsLoaderComparator")
23+
public Comparator<ExtensionLoader.ExtensionPluginWithContext> extensionsLoaderComparator() {
24+
return Comparator.comparing(ExtensionLoader.ExtensionPluginWithContext::isConfigured).reversed();
25+
}
1926
}

‎data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/DataPrepperExtensionPointsTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import static org.mockito.Mockito.never;
3131
import static org.mockito.Mockito.reset;
3232
import static org.mockito.Mockito.verify;
33+
import static org.mockito.Mockito.times;
3334
import static org.mockito.Mockito.when;
3435

3536
@ExtendWith(MockitoExtension.class)
@@ -97,6 +98,19 @@ void addExtensionProvider_should_registerBean() {
9798
verify(coreApplicationContext).registerBean(eq(extensionClass), any(Supplier.class), any(BeanDefinitionCustomizer.class));
9899
}
99100

101+
@Test
102+
void addExtensionProvider_should_not_registerBean_for_the_sameClass() {
103+
DataPrepperExtensionPoints dataPrepperExtensionPoints = createObjectUnderTest();
104+
dataPrepperExtensionPoints.addExtensionProvider(extensionProvider);
105+
106+
verify(sharedApplicationContext, times(1)).registerBean(eq(extensionClass), any(Supplier.class), any(BeanDefinitionCustomizer.class));
107+
verify(coreApplicationContext, times(1)).registerBean(eq(extensionClass), any(Supplier.class), any(BeanDefinitionCustomizer.class));
108+
109+
dataPrepperExtensionPoints.addExtensionProvider(extensionProvider);
110+
verify(sharedApplicationContext, times(1)).registerBean(eq(extensionClass), any(Supplier.class), any(BeanDefinitionCustomizer.class));
111+
verify(coreApplicationContext, times(1)).registerBean(eq(extensionClass), any(Supplier.class), any(BeanDefinitionCustomizer.class));
112+
}
113+
100114
@Test
101115
void addExtensionProvider_should_registerBean_which_calls_provideInstance() {
102116
createObjectUnderTest().addExtensionProvider(extensionProvider);

‎data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ExtensionLoaderTest.java‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.opensearch.dataprepper.model.plugin.ExtensionPlugin;
2323
import org.opensearch.dataprepper.model.plugin.InvalidPluginConfigurationException;
2424
import org.opensearch.dataprepper.model.plugin.InvalidPluginDefinitionException;
25+
import org.opensearch.dataprepper.plugins.test.TestExtension;
2526
import org.opensearch.dataprepper.plugins.test.TestExtensionConfig;
2627
import org.opensearch.dataprepper.plugins.test.TestExtensionWithConfig;
2728
import org.opensearch.dataprepper.validation.PluginError;
@@ -31,6 +32,7 @@
3132
import java.util.ArrayList;
3233
import java.util.Collection;
3334
import java.util.Collections;
35+
import java.util.Comparator;
3436
import java.util.HashSet;
3537
import java.util.List;
3638
import java.util.stream.Stream;
@@ -67,14 +69,18 @@ class ExtensionLoaderTest {
6769
private ArgumentCaptor<Collection<PluginError>> pluginErrorsArgumentCaptor;
6870
private PluginErrorCollector pluginErrorCollector;
6971

72+
private Comparator<ExtensionLoader.ExtensionPluginWithContext> extensionsLoaderComparator;
73+
7074
private ExtensionLoader createObjectUnderTest() {
7175
return new ExtensionLoader(extensionPluginConfigurationConverter, extensionClassProvider,
72-
extensionPluginCreator, pluginErrorCollector, pluginErrorsHandler);
76+
extensionPluginCreator, pluginErrorCollector, pluginErrorsHandler, extensionsLoaderComparator);
7377
}
7478

7579
@BeforeEach
7680
void setUp() {
7781
pluginErrorCollector = new PluginErrorCollector();
82+
PluginCreatorContext pluginCreatorContext = new PluginCreatorContext();
83+
extensionsLoaderComparator = pluginCreatorContext.extensionsLoaderComparator();
7884
}
7985

8086
@Test
@@ -109,6 +115,37 @@ void loadExtensions_returns_single_extension_for_single_plugin_class() {
109115
assertThat(pluginErrorCollector.getPluginErrors().isEmpty(), is(true));
110116
}
111117

118+
@Test
119+
void loadExtensions_returns_single_extension_with_config_for_multiple_plugin_class() {
120+
when(extensionClassProvider.loadExtensionPluginClasses())
121+
.thenReturn(List.of(TestExtension.class, TestExtensionWithConfig.class));
122+
123+
final TestExtensionWithConfig expectedPlugin = mock(TestExtensionWithConfig.class);
124+
final TestExtension testPlugin = mock(TestExtension.class);
125+
final String expectedPluginName = "test_extension_with_config";
126+
when(extensionPluginConfigurationConverter.convert(eq(true), eq(TestExtensionConfig.class),
127+
anyString())).thenReturn(new TestExtensionConfig());
128+
when(extensionPluginCreator.newPluginInstance(
129+
eq(TestExtensionWithConfig.class),
130+
any(PluginArgumentsContext.class),
131+
eq(expectedPluginName)))
132+
.thenReturn(expectedPlugin);
133+
134+
when(extensionPluginCreator.newPluginInstance(
135+
eq(TestExtension.class),
136+
any(PluginArgumentsContext.class),
137+
anyString()))
138+
.thenReturn(testPlugin);
139+
140+
final List<? extends ExtensionPlugin> extensionPlugins = createObjectUnderTest().loadExtensions();
141+
142+
assertThat(extensionPlugins, notNullValue());
143+
assertThat(extensionPlugins.size(), equalTo(2));
144+
assertThat(extensionPlugins.get(0), equalTo(expectedPlugin));
145+
assertThat(extensionPlugins.get(1), equalTo(testPlugin));
146+
assertThat(pluginErrorCollector.getPluginErrors().isEmpty(), is(true));
147+
}
148+
112149
@Test
113150
void loadExtensions_throws_InvalidPluginConfigurationException_when_invoking_newPluginInstance_throws_exception() {
114151
pluginErrorCollector.collectPluginError(

‎data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginCreatorContextTest.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.dataprepper.plugin;
7+
8+
import org.junit.jupiter.api.Test;
9+
import org.junit.jupiter.api.BeforeEach;
10+
import static org.junit.jupiter.api.Assertions.assertNotNull;
11+
12+
import static org.mockito.Mockito.mock;
13+
import static org.mockito.Mockito.when;
14+
import static org.hamcrest.Matchers.greaterThan;
15+
import static org.hamcrest.Matchers.equalTo;
16+
import static org.hamcrest.Matchers.lessThan;
17+
import static org.hamcrest.MatcherAssert.assertThat;
18+
import java.util.Comparator;
19+
20+
public class PluginCreatorContextTest {
21+
PluginCreatorContext pluginCreatorContext;
22+
23+
@BeforeEach
24+
void setUp() {
25+
pluginCreatorContext = new PluginCreatorContext();
26+
}
27+
28+
@Test
29+
public void test_observablePluginCreator() {
30+
PluginCreator pluginCreator = pluginCreatorContext.observablePluginCreator();
31+
assertNotNull(pluginCreator);
32+
}
33+
34+
@Test
35+
public void test_pluginCreator() {
36+
PluginConfigurationObservableRegister pluginConfigurationObservableRegister = mock(PluginConfigurationObservableRegister.class);
37+
PluginCreator pluginCreator = pluginCreatorContext.pluginCreator(pluginConfigurationObservableRegister);
38+
assertNotNull(pluginCreator);
39+
}
40+
41+
@Test
42+
public void test_extensionsLoaderComparator() {
43+
ExtensionLoader.ExtensionPluginWithContext context1 = mock(ExtensionLoader.ExtensionPluginWithContext.class);
44+
ExtensionLoader.ExtensionPluginWithContext context2 = mock(ExtensionLoader.ExtensionPluginWithContext.class);
45+
Comparator<ExtensionLoader.ExtensionPluginWithContext> extensionsLoaderComparator = pluginCreatorContext.extensionsLoaderComparator();
46+
assertNotNull(extensionsLoaderComparator);
47+
when(context1.isConfigured()).thenReturn(true);
48+
when(context2.isConfigured()).thenReturn(true);
49+
assertThat(extensionsLoaderComparator.compare(context1, context2), equalTo(0));
50+
when(context1.isConfigured()).thenReturn(false);
51+
when(context2.isConfigured()).thenReturn(false);
52+
assertThat(extensionsLoaderComparator.compare(context1, context2), equalTo(0));
53+
when(context1.isConfigured()).thenReturn(false);
54+
when(context2.isConfigured()).thenReturn(true);
55+
assertThat(extensionsLoaderComparator.compare(context1, context2), greaterThan(0));
56+
when(context1.isConfigured()).thenReturn(true);
57+
when(context2.isConfigured()).thenReturn(false);
58+
assertThat(extensionsLoaderComparator.compare(context1, context2), lessThan(0));
59+
}
60+
}

0 commit comments

Comments
 (0)