From 8308fe2d21a5f588d18c975aecaaf4b999d51125 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 11 Nov 2022 15:31:16 -0800 Subject: [PATCH 1/4] Make NamedXContent available to extensions Signed-off-by: Daniel Widdis --- .../org/opensearch/sdk/BaseExtension.java | 19 ++-- .../java/org/opensearch/sdk/Extension.java | 19 +++- .../sdk/ExtensionNamedXContentRegistry.java | 74 +++++++++++++++ .../org/opensearch/sdk/ExtensionsRunner.java | 90 +++++++++++++++++-- .../ExtensionsInitRequestHandler.java | 14 ++- 5 files changed, 200 insertions(+), 16 deletions(-) create mode 100644 src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java diff --git a/src/main/java/org/opensearch/sdk/BaseExtension.java b/src/main/java/org/opensearch/sdk/BaseExtension.java index 9e50eb8f..eea8f2ad 100644 --- a/src/main/java/org/opensearch/sdk/BaseExtension.java +++ b/src/main/java/org/opensearch/sdk/BaseExtension.java @@ -20,6 +20,11 @@ * An abstract class that provides sample methods required by extensions */ public abstract class BaseExtension implements Extension { + /** + * The {@link ExtensionsRunner} instance running this extension + */ + protected ExtensionsRunner extensionsRunner; + /** * A client to make requests to the system */ @@ -66,14 +71,12 @@ public ExtensionSettings getExtensionSettings() { return this.settings; } - /** - * Returns components added by this extension. - * - * @param client A client to make requests to the system - * @param clusterService A service to allow watching and updating cluster state - * @param threadPool A service to allow retrieving an executor to run an async action - * @return A collection of objects - */ + @Override + public void setExtensionsRunner(ExtensionsRunner extensionsRunner) { + this.extensionsRunner = extensionsRunner; + } + + @Override public Collection createComponents(SDKClient client, ClusterService clusterService, ThreadPool threadPool) { this.client = client; this.clusterService = clusterService; diff --git a/src/main/java/org/opensearch/sdk/Extension.java b/src/main/java/org/opensearch/sdk/Extension.java index 010d4202..b61ab6a0 100644 --- a/src/main/java/org/opensearch/sdk/Extension.java +++ b/src/main/java/org/opensearch/sdk/Extension.java @@ -24,6 +24,7 @@ import org.opensearch.action.ActionResponse; import org.opensearch.action.support.TransportAction; import org.opensearch.common.settings.Setting; +import org.opensearch.common.xcontent.NamedXContentRegistry; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; @@ -58,6 +59,22 @@ default List> getSettings() { return Collections.emptyList(); } + /** + * Gets an optional list of custom {@link NamedXContentRegistry.Entry} for the extension to combine with OpenSearch NamedXConent. + * + * @return a list of custom NamedXConent this extension uses. + */ + default List getNamedXContent() { + return Collections.emptyList(); + } + + /** + * Set the Extension's instance of its corresponding {@link ExtensionsRunner}. + * + * @param extensionsRunner The ExtensionsRunner running this extension. + */ + void setExtensionsRunner(ExtensionsRunner extensionsRunner); + /** * Returns components added by this extension. * @@ -66,7 +83,7 @@ default List> getSettings() { * @param threadPool A service to allow retrieving an executor to run an async action * @return A collection of objects */ - public Collection createComponents(SDKClient client, ClusterService clusterService, ThreadPool threadPool); + Collection createComponents(SDKClient client, ClusterService clusterService, ThreadPool threadPool); /** * Gets an optional list of custom {@link TransportAction} for the extension to register with OpenSearch. diff --git a/src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java b/src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java new file mode 100644 index 00000000..e1e5eb21 --- /dev/null +++ b/src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java @@ -0,0 +1,74 @@ +/* + * 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.sdk; + +import static java.util.stream.Collectors.toList; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Function; +import java.util.stream.Stream; + +import org.opensearch.cluster.ClusterModule; +import org.opensearch.common.network.NetworkModule; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.NamedXContentRegistry; +import org.opensearch.indices.IndicesModule; +import org.opensearch.search.SearchModule; + +/** + * API used to handle named XContent registry requests from OpenSearch + */ +public class ExtensionNamedXContentRegistry { + private final List namedXContent = new ArrayList<>(); + private final NamedXContentRegistry namedXContentRegistry; + + /** + * Creates and populates a NamedXContentRegistry with the given NamedXContentRegistry entries for this extension and + * locally defined content. + * + * @param settings Combined OpenSearch and Extension settings + * @param extensionNamedXContent List of NamedXContentRegistry.Entry to be registered + */ + public ExtensionNamedXContentRegistry(Settings settings, List extensionNamedXContent) { + // Save a local copy of just the extension-added entries + this.namedXContent.addAll(extensionNamedXContent); + // Now include those entries plus core XContent in the registry + this.namedXContentRegistry = new NamedXContentRegistry( + Stream.of( + extensionNamedXContent.stream(), + NetworkModule.getNamedXContents().stream(), + IndicesModule.getNamedXContents().stream(), + new SearchModule(settings, Collections.emptyList()).getNamedXContents().stream(), + ClusterModule.getNamedXWriteables().stream() + ).flatMap(Function.identity()).collect(toList()) + ); + } + + /** + * Gets the NamedXContentRegistry. + * + * @return The NamedXContentRegistry of this API. Includes both extension-defined XContent and core OpenSearch + * XContent. + */ + public NamedXContentRegistry getRegistry() { + return this.namedXContentRegistry; + } + + /** + * Gets the extension's namedXContent. + * + * @return a list of extension-defined XContent. Does not include core OpenSearch XContent. + */ + public List getNamedXContent() { + return namedXContent; + } +} diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 3df77b5d..11ff9fcb 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -20,6 +20,7 @@ import org.opensearch.extensions.settings.RegisterCustomSettingsRequest; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.NamedXContentRegistry; import org.opensearch.discovery.InitializeExtensionsRequest; import org.opensearch.extensions.ExtensionActionListenerOnFailureRequest; import org.opensearch.extensions.DiscoveryExtension; @@ -50,6 +51,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.Consumer; @@ -64,6 +66,9 @@ public class ExtensionsRunner { private static final Logger logger = LogManager.getLogger(ExtensionsRunner.class); private static final String NODE_NAME_SETTING = "node.name"; + // Set when initialization is complete + private boolean initialized = false; + private String uniqueId; /** * This field is initialized by a call from {@link ExtensionsInitRequestHandler}. @@ -77,16 +82,27 @@ public class ExtensionsRunner { // The routes and classes which handle the REST requests private final ExtensionRestPathRegistry extensionRestPathRegistry = new ExtensionRestPathRegistry(); - // Custom settings from the extension's getSettings /** - * This field is initialized by a call from {@link ExtensionsInitRequestHandler}. + * Custom namedXContent from the extension's getNamedXContent. This field is initialized in the constructor. + */ + private final List customNamedXContent; + /** + * Custom settings from the extension's getSettings. This field is initialized in the constructor. */ private final List> customSettings; - // Node name, host, and port /** - * This field is initialized by a call from {@link ExtensionsInitRequestHandler}. + * Environment settings from OpenSearch. This field is initialized by a call from + * {@link ExtensionsInitRequestHandler}. + */ + private Settings environmentSettings = Settings.EMPTY; + /** + * Node name, host, and port. This field is initialized by a call from {@link ExtensionsInitRequestHandler}. */ public final Settings settings; + private ExtensionNamedXContentRegistry extensionNamedXContentRegistry = new ExtensionNamedXContentRegistry( + Settings.EMPTY, + Collections.emptyList() + ); private ExtensionNamedWriteableRegistry namedWriteableRegistry = new ExtensionNamedWriteableRegistry(); private ExtensionsInitRequestHandler extensionsInitRequestHandler = new ExtensionsInitRequestHandler(this); private OpensearchRequestHandler opensearchRequestHandler = new OpensearchRequestHandler(namedWriteableRegistry); @@ -114,10 +130,11 @@ public class ExtensionsRunner { /** * Instantiates a new Extensions Runner using the specified extension. * - * @param extension The settings with which to start the runner. + * @param extension The settings with which to start the runner. * @throws IOException if the runner failed to read settings or API. */ protected ExtensionsRunner(Extension extension) throws IOException { + extension.setExtensionsRunner(this); ExtensionSettings extensionSettings = extension.getExtensionSettings(); this.settings = Settings.builder() .put(NODE_NAME_SETTING, extensionSettings.getExtensionName()) @@ -132,6 +149,8 @@ protected ExtensionsRunner(Extension extension) throws IOException { } // save custom settings this.customSettings = extension.getSettings(); + // save custom namedXContent + this.customNamedXContent = extension.getNamedXContent(); // save custom transport actions this.transportActions = new TransportActions(extension.getActions()); @@ -142,15 +161,70 @@ protected ExtensionsRunner(Extension extension) throws IOException { } /** - * This method is call from {@link ExtensionsInitRequestHandler}. + * Marks the extension initialized. + */ + public void setInitialized() { + this.initialized = true; + logger.info("Extension initialization is complete!"); + } + + /** + * Reports if the extension has finished initializing. + * + * @return true if the extension has been initialized + */ + boolean isInitialized() { + return this.initialized; + } + + /** + * Sets the TransportService. Called from {@link ExtensionsInitRequestHandler}. + * * @param extensionTransportService assign value for extensionTransportService */ void setExtensionTransportService(TransportService extensionTransportService) { this.extensionTransportService = extensionTransportService; } + /** + * Sets the Environment Settings. Called from {@link ExtensionsInitRequestHandler}. + * + * @param settings assign value for environmentSettings + */ + public void setEnvironmentSettings(Settings settings) { + this.environmentSettings = settings; + } + + /** + * Gets the Environment Settings. Only valid if {@link #isInitialized()} returns true. + * + * @return the environment settings if initialized, an empty settings object otherwise. + */ + public Settings getEnvironmentSettings() { + return this.environmentSettings; + } + + /** + * Sets the NamedXContentRegistry. Called from {@link ExtensionsInitRequestHandler}. + * + * @param registry assign value for namedXContentRegistry + */ + public void setNamedXContentRegistry(ExtensionNamedXContentRegistry registry) { + this.extensionNamedXContentRegistry = registry; + } + + /** + * Gets the NamedXContentRegistry. Only valid if {@link #isInitialized()} returns true. + * + * @return the NamedXContentRegistry if initialized, an empty registry otherwise. + */ + public ExtensionNamedXContentRegistry getNamedXContentRegistry() { + return this.extensionNamedXContentRegistry; + } + /** * Sets the Unique ID, used in REST requests to uniquely identify this extension + * * @param id assign value for id */ public void setUniqueId(String id) { @@ -173,6 +247,10 @@ public DiscoveryNode getOpensearchNode() { return this.opensearchNode; } + public List getCustomNamedXContent() { + return this.customNamedXContent; + } + /** * Starts a TransportService. * diff --git a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java index e528f772..db086743 100644 --- a/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java +++ b/src/main/java/org/opensearch/sdk/handlers/ExtensionsInitRequestHandler.java @@ -11,8 +11,10 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.common.settings.Settings; import org.opensearch.discovery.InitializeExtensionsRequest; import org.opensearch.discovery.InitializeExtensionsResponse; +import org.opensearch.sdk.ExtensionNamedXContentRegistry; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.transport.TransportService; @@ -28,6 +30,7 @@ public class ExtensionsInitRequestHandler { /** * Instantiate this object with a reference to the ExtensionsRunner + * * @param extensionsRunner the ExtensionsRunner instance */ public ExtensionsInitRequestHandler(ExtensionsRunner extensionsRunner) { @@ -35,7 +38,7 @@ public ExtensionsInitRequestHandler(ExtensionsRunner extensionsRunner) { } /** - * Handles a extension request from OpenSearch. This is the first request for the transport communication and will initialize the extension and will be a part of OpenSearch bootstrap. + * Handles a extension request from OpenSearch. This is the first request for the transport communication and will initialize the extension and will be a part of OpenSearch bootstrap. * * @param extensionInitRequest The request to handle. * @return A response to OpenSearch validating that this is an extension. @@ -60,6 +63,15 @@ public InitializeExtensionsResponse handleExtensionInitRequest(InitializeExtensi extensionsRunner.opensearchNode, extensionsRunner.getUniqueId() ); + // Get OpenSearch Settings and set values on ExtensionsRunner + Settings settings = extensionsRunner.sendEnvironmentSettingsRequest(extensionTransportService); + extensionsRunner.setEnvironmentSettings(settings); + extensionsRunner.setNamedXContentRegistry( + new ExtensionNamedXContentRegistry(settings, extensionsRunner.getCustomNamedXContent()) + ); + + // Last step of initialization + extensionsRunner.setInitialized(); } } } From 96295206b808bfb0d79c342560cc82b630aca17c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 11 Nov 2022 20:32:34 -0800 Subject: [PATCH 2/4] Add tests Signed-off-by: Daniel Widdis --- .../TestExtensionNamedXContentRegistry.java | 146 ++++++++++++++++++ .../opensearch/sdk/TestExtensionsRunner.java | 20 +++ 2 files changed, 166 insertions(+) create mode 100644 src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java diff --git a/src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java b/src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java new file mode 100644 index 00000000..6edc0337 --- /dev/null +++ b/src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java @@ -0,0 +1,146 @@ +/* + * 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.sdk; + +import org.opensearch.cluster.ClusterModule; +import org.opensearch.common.ParseField; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.network.NetworkModule; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.NamedXContentRegistry; +import org.opensearch.common.xcontent.ToXContentObject; +import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.indices.IndicesModule; +import org.opensearch.search.SearchModule; +import org.opensearch.test.OpenSearchTestCase; +import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class TestExtensionNamedXContentRegistry extends OpenSearchTestCase { + private List namedXContents; + private ExtensionNamedXContentRegistry extensionNamedXContentRegistry; + + private static class Example implements ToXContentObject { + public static final String NAME = "Example"; + public static final NamedXContentRegistry.Entry XCONTENT_REGISTRY = new NamedXContentRegistry.Entry( + Example.class, + new ParseField(NAME), + it -> parse(it) + ); + public static final String NAME_FIELD = "name"; + + private final String name; + + public Example(String name) { + this.name = name; + } + + public String getName() { + return this.name; + } + + public static Example parse(XContentParser parser) throws IOException { + String name = null; + + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + + switch (fieldName) { + case NAME_FIELD: + name = parser.text(); + break; + default: + parser.skipChildren(); + break; + } + } + return new Example(name); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + XContentBuilder xContentBuilder = builder.startObject().field(NAME_FIELD, name); + return xContentBuilder.endObject(); + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + Example that = (Example) o; + return Objects.equals(name, that.name); + } + + @Override + public int hashCode() { + return Objects.hash(name); + } + } + + @Override + @BeforeEach + public void setUp() { + this.namedXContents = Collections.singletonList(Example.XCONTENT_REGISTRY); + this.extensionNamedXContentRegistry = new ExtensionNamedXContentRegistry(Settings.EMPTY, namedXContents); + } + + @Test + public void testNamedXContentRegistryParse() throws IOException { + // Tests parsing the custom namedXContent + BytesReference bytes = BytesReference.bytes( + JsonXContent.contentBuilder().startObject().field(Example.NAME_FIELD, Example.NAME).endObject() + ); + + NamedXContentRegistry registry = this.extensionNamedXContentRegistry.getRegistry(); + XContentParser parser = XContentType.JSON.xContent() + .createParser(registry, LoggingDeprecationHandler.INSTANCE, bytes.streamInput()); + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + + Example example = registry.parseNamedObject(Example.class, Example.NAME, parser, null); + assertEquals(Example.NAME, example.getName()); + } + + @Test + public void testNamedXContentRegistryExceptions() { + // Tests that the registry includes module contents and generates conflicts when adding + assertThrows( + IllegalArgumentException.class, + () -> new ExtensionNamedXContentRegistry(Settings.EMPTY, NetworkModule.getNamedXContents()) + ); + assertThrows( + IllegalArgumentException.class, + () -> new ExtensionNamedXContentRegistry(Settings.EMPTY, IndicesModule.getNamedXContents()) + ); + assertThrows( + IllegalArgumentException.class, + () -> new ExtensionNamedXContentRegistry( + Settings.EMPTY, + new SearchModule(Settings.EMPTY, Collections.emptyList()).getNamedXContents() + ) + ); + assertThrows( + IllegalArgumentException.class, + () -> new ExtensionNamedXContentRegistry(Settings.EMPTY, ClusterModule.getNamedXWriteables()) + ); + } +} diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index 8a276af8..138b7e40 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; import java.nio.charset.StandardCharsets; @@ -38,6 +39,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.transport.TransportAddress; +import org.opensearch.common.xcontent.NamedXContentRegistry; import org.opensearch.discovery.InitializeExtensionsRequest; import org.opensearch.discovery.InitializeExtensionsResponse; import org.opensearch.extensions.DiscoveryExtension; @@ -243,4 +245,22 @@ public void testRegisterCustomSettingsRequest() { verify(transportService, times(1)).sendRequest(any(), anyString(), any(), any(ExtensionStringResponseHandler.class)); } + + @Test + public void testGettersAndSetters() throws IOException { + assertFalse(extensionsRunner.isInitialized()); + extensionsRunner.setInitialized(); + assertTrue(extensionsRunner.isInitialized()); + + Settings settings = Settings.builder().put("test.key", "test.value").build(); + assertTrue(extensionsRunner.getEnvironmentSettings().isEmpty()); + extensionsRunner.setEnvironmentSettings(settings); + assertEquals("test.value", extensionsRunner.getEnvironmentSettings().get("test.key")); + + assertTrue(extensionsRunner.getCustomNamedXContent().isEmpty()); + assertTrue(extensionsRunner.getNamedXContentRegistry().getRegistry() instanceof NamedXContentRegistry); + assertTrue(extensionsRunner.getNamedXContentRegistry().getNamedXContent().isEmpty()); + extensionsRunner.setNamedXContentRegistry(null); + assertNull(extensionsRunner.getNamedXContentRegistry()); + } } From 1fce412dccd2286e85f011778d78c4cae425f955 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sat, 12 Nov 2022 11:21:17 -0800 Subject: [PATCH 3/4] No need to save custom list in registry as it's on ExtensionsRunner Signed-off-by: Daniel Widdis --- .../sdk/ExtensionNamedXContentRegistry.java | 21 +++---------------- .../TestExtensionNamedXContentRegistry.java | 6 +++--- .../opensearch/sdk/TestExtensionsRunner.java | 1 - 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java b/src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java index e1e5eb21..432f0367 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java +++ b/src/main/java/org/opensearch/sdk/ExtensionNamedXContentRegistry.java @@ -11,7 +11,6 @@ import static java.util.stream.Collectors.toList; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.function.Function; @@ -25,23 +24,19 @@ import org.opensearch.search.SearchModule; /** - * API used to handle named XContent registry requests from OpenSearch + * Combines Extension NamedXContent with core OpenSearch NamedXContent */ public class ExtensionNamedXContentRegistry { - private final List namedXContent = new ArrayList<>(); private final NamedXContentRegistry namedXContentRegistry; /** * Creates and populates a NamedXContentRegistry with the given NamedXContentRegistry entries for this extension and * locally defined content. * - * @param settings Combined OpenSearch and Extension settings + * @param settings OpenSearch environment settings * @param extensionNamedXContent List of NamedXContentRegistry.Entry to be registered */ public ExtensionNamedXContentRegistry(Settings settings, List extensionNamedXContent) { - // Save a local copy of just the extension-added entries - this.namedXContent.addAll(extensionNamedXContent); - // Now include those entries plus core XContent in the registry this.namedXContentRegistry = new NamedXContentRegistry( Stream.of( extensionNamedXContent.stream(), @@ -56,19 +51,9 @@ public ExtensionNamedXContentRegistry(Settings settings, List getNamedXContent() { - return namedXContent; - } } diff --git a/src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java b/src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java index 6edc0337..2fd8af93 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionNamedXContentRegistry.java @@ -9,6 +9,8 @@ package org.opensearch.sdk; +import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; + import org.opensearch.cluster.ClusterModule; import org.opensearch.common.ParseField; import org.opensearch.common.bytes.BytesReference; @@ -24,7 +26,6 @@ import org.opensearch.indices.IndicesModule; import org.opensearch.search.SearchModule; import org.opensearch.test.OpenSearchTestCase; -import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; import java.io.IOException; import java.util.Collections; import java.util.List; @@ -34,7 +35,6 @@ import org.junit.jupiter.api.Test; public class TestExtensionNamedXContentRegistry extends OpenSearchTestCase { - private List namedXContents; private ExtensionNamedXContentRegistry extensionNamedXContentRegistry; private static class Example implements ToXContentObject { @@ -100,7 +100,7 @@ public int hashCode() { @Override @BeforeEach public void setUp() { - this.namedXContents = Collections.singletonList(Example.XCONTENT_REGISTRY); + List namedXContents = Collections.singletonList(Example.XCONTENT_REGISTRY); this.extensionNamedXContentRegistry = new ExtensionNamedXContentRegistry(Settings.EMPTY, namedXContents); } diff --git a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java index 138b7e40..99ad245b 100644 --- a/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java +++ b/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java @@ -259,7 +259,6 @@ public void testGettersAndSetters() throws IOException { assertTrue(extensionsRunner.getCustomNamedXContent().isEmpty()); assertTrue(extensionsRunner.getNamedXContentRegistry().getRegistry() instanceof NamedXContentRegistry); - assertTrue(extensionsRunner.getNamedXContentRegistry().getNamedXContent().isEmpty()); extensionsRunner.setNamedXContentRegistry(null); assertNull(extensionsRunner.getNamedXContentRegistry()); } From 9285fe19a85de72252e7b54deb1f5f01b0eacacc Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 15 Nov 2022 18:27:51 -0800 Subject: [PATCH 4/4] Add sequence diagram to DESIGN.md Signed-off-by: Daniel Widdis --- DESIGN.md | 32 +++++++++++++++++++++++++++++--- Docs/NamedXContent.svg | 1 + 2 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 Docs/NamedXContent.svg diff --git a/DESIGN.md b/DESIGN.md index 34cddf25..b5958d18 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -74,10 +74,10 @@ The sequence diagram below shows the process of initializing an Extension, regis The `org.opensearch.sdk.sample` package contains a sample `HelloWorldExtension` implementing the below steps. It is executed following the steps in the [`DEVELOPER_GUIDE`](DEVELOPER_GUIDE.md). -![](Docs/ExtensionRestActions.svg) - #### Extension REST Actions Walk Through +![](Docs/ExtensionRestActions.svg) + ##### Extension Startup (1) Extensions must implement the `Extension` interface which requires them to define their settings (name, host address and port) and a list of `ExtensionRestHandler` implementations they will handle. They are started up using a `main()` method which passes an instance of the extension to the `ExtensionsRunner` using `ExtensionsRunner.run(this)`. @@ -116,9 +116,35 @@ The `ExtensionsOrchestrator` reads a list of extensions present in `extensions.y (27) The User receives the response. +#### Extension Point Implementation Walk Through + +An example of a more complex extension point, `getNamedXContent()` is shown below. A similar pattern can be followed for most extension points. + +![](Docs/NamedXContent.svg) + +##### Extension Startup + +(1, 2) Extensions initialize by passing an instance of themselves to the `ExtensionsRunner`. The first step in the constructor is for the `ExtensionsRunner` to pass its own instance back to the Extension via setter. + +(3, 4) The `Extension` interface includes extensions points such as `getNamedXContent()` (returning a default empty list). If overridden, the Extension will return a list of `NamedXContentRegistry.Entry` which will be saved as `customNamedXContent`. Other extension points operate in a similar manner. + +(5) The `ExtensionsRunner` registers an `ExtensionInitRequestHandler` which will complete the initialization process on OpenSearch startup. + +##### OpenSearch Startup, Extension Initialization, and NamedXContent Registration + +(6) Upon receipt of an `InitializeExtensionsRequest` (among other actions): + +(7, 8) Obtains Environment Settings from OpenSearch, necessary for some core XContent. + +(9, 10) Instantiates a new `ExtensionNamedXContentRegistry` which is set on the ExtensionsRunner. +This uses the OpenSearch environment settings along with NamedXContent from several OpenSearch modules, +and combines it the custom Extension NamedXContent. + +Since the Extension has an instance of the ExtensionsRunner, it can now access the registry via getter and pass it to Extension Rest Handlers as needed. + ## FAQ - Will extensions replace plugins? - Plugins will continue to be supported and extensions are preferred as they will be easier to develop, deploy, and operate. + Plugins will continue to be supported in the near term but are on a path to deprecation. New development should consider using extensions, as they will be easier to develop, deploy, and operate. - How is the latency going to be for extensions? https://github.com/opensearch-project/OpenSearch/issues/3012#issuecomment-1122682444 diff --git a/Docs/NamedXContent.svg b/Docs/NamedXContent.svg new file mode 100644 index 00000000..6d4ce533 --- /dev/null +++ b/Docs/NamedXContent.svg @@ -0,0 +1 @@ +%23%20File%20comapible%20with%20https%3A%2F%2Fsequencediagram.org%2F%0Atitle%20NamedXContent%20Registration%0A%0Aentryspacing%200.6%0Aparticipantgroup%20%23lightgreen%20**Extension%20Node**%0Aparticipant%20Extension%0Aparticipant%20ExtensionsRunner%0Aparticipant%20ExtensionNamedXContentRegistry%0Aend%0A%0Aparticipantgroup%20%23lightblue%20**OpenSearch%20Node**%0Aparticipant%20ExtensionsOrchestrator%0Aend%0Aautonumber%201%0AExtension-%3EExtensionsRunner%3Arun(this)%0AExtension%3C-ExtensionsRunner%3AsetExtensionsRunner(this)%0AExtension%3C-ExtensionsRunner%3AgetNamedXContent()%0AExtension-%3EExtensionsRunner%3AcustomNamedXContent%0AExtensionsRunner%3C-ExtensionsRunner%3Aregister%20ExtensionsInitRequestHandler%0A%0AExtensionsRunner%3C-ExtensionsOrchestrator%3AInitializeExtensionsRequest%0Anote%20over%20ExtensionsRunner%2CExtensionNamedXContentRegistry%3A**ExtensionsInitRequestHandler**%5CnCombine%20customNamedXContent%5Cnand%20class-loaded%20XContent%20from%5CnIndicesModule%2C%20NetworkModule%2C%20and%5CnSearchModule%20(with%20Settings%20as%20a%5Cnparameter)%0A%0AOpenSearch%20Environment%20Settings%5Cnto%20create%20a%20combined%20registry%0AExtensionsRunner-%3EExtensionsOrchestrator%3AsendEnvironmentSettingsRequest()%0AExtensionsRunner%3C-ExtensionsOrchestrator%3AEnvironmentSettingsResponse%0AExtensionsRunner-%3E*ExtensionNamedXContentRegistry%3Anew%20instance%0AExtensionsRunner%3C-ExtensionNamedXContentRegistry%3AsetNamedXContentRegistry()%0Adestroysilent%20ExtensionNamedXContentRegistry%0Anote%20over%20Extension%2CExtensionNamedXContentRegistry%3AExtension%20can%20now%20access%20NamedXContentRegistry%0A%0ANamedXContent RegistrationExtension NodeOpenSearch NodeExtensionExtensionsRunnerExtensionsOrchestratorrun(this)setExtensionsRunner(this)getNamedXContent()customNamedXContentregister ExtensionsInitRequestHandlerInitializeExtensionsRequestExtensionsInitRequestHandlerCombine customNamedXContentand class-loaded XContent fromIndicesModule, NetworkModule, andSearchModule (with Settings as aparameter)sendEnvironmentSettingsRequest()EnvironmentSettingsResponseExtensionNamedXContentRegistrynew instancesetNamedXContentRegistry()10 Extension can now access NamedXContentRegistry