Address code review feedback

Signed-off-by: Craig Perkins <cwperx@amazon.com>
opensearch-project · May 23, 2023 · 1d1273f · 1d1273f
1 parent c53852c
commit 1d1273f
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 51 deletions.
diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java
@@ -467,9 +467,8 @@ protected Node(
 
             final IdentityService identityService = new IdentityService(settings, identityPlugins);
 
-            final List<ExtensionAwarePlugin> extensionAwarePlugins = new ArrayList<>();
             if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
-                extensionAwarePlugins.addAll(pluginsService.filterPlugins(ExtensionAwarePlugin.class));
+                final List<ExtensionAwarePlugin> extensionAwarePlugins = pluginsService.filterPlugins(ExtensionAwarePlugin.class);
                 Set<Setting<?>> additionalSettings = new HashSet<>();
                 for (ExtensionAwarePlugin extAwarePlugin : extensionAwarePlugins) {
                     additionalSettings.addAll(extAwarePlugin.getExtensionSettings());

diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java
@@ -873,65 +873,56 @@ public void testIncompatibleExtensionRegistration() throws IOException, IllegalA
         }
     }
 
-    public void testAdditionalExtensionSettings() throws Exception {
+    public void testAdditionalExtensionSettingsForExtensionWithCustomSettingSet() throws Exception {
         Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8);
 
         Set<Setting<?>> additionalSettings = extAwarePlugin.getExtensionSettings().stream().collect(Collectors.toSet());
 
         ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, additionalSettings);
 
-        List<DiscoveryExtensionNode> expectedExtensions = new ArrayList<DiscoveryExtensionNode>();
+        DiscoveryExtensionNode extension = new DiscoveryExtensionNode(
+            "firstExtension",
+            "uniqueid1",
+            new TransportAddress(InetAddress.getByName("127.0.0.1"), 9300),
+            new HashMap<String, String>(),
+            Version.fromString("3.0.0"),
+            Version.fromString("3.0.0"),
+            List.of()
+        );
+        DiscoveryExtensionNode initializedExtension = extensionsManager.getExtensionIdMap().get(extension.getId());
+        assertEquals(extension.getName(), initializedExtension.getName());
+        assertEquals(extension.getId(), initializedExtension.getId());
+        assertTrue(extensionsManager.lookupExtensionSettingsById(extension.getId()).isPresent());
+        assertEquals(
+            "custom_setting",
+            extensionsManager.lookupExtensionSettingsById(extension.getId()).get().getAdditionalSettings().get(customSetting)
+        );
+    }
 
-        String expectedUniqueId = "uniqueid0";
-        Version expectedVersion = Version.fromString("2.0.0");
-        ExtensionDependency expectedDependency = new ExtensionDependency(expectedUniqueId, expectedVersion);
+    public void testAdditionalExtensionSettingsForExtensionWithoutCustomSettingSet() throws Exception {
+        Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8);
 
-        expectedExtensions.add(
-            new DiscoveryExtensionNode(
-                "firstExtension",
-                "uniqueid1",
-                new TransportAddress(InetAddress.getByName("127.0.0.0"), 9300),
-                new HashMap<String, String>(),
-                Version.fromString("3.0.0"),
-                Version.fromString("3.0.0"),
-                Collections.emptyList()
-            )
-        );
+        Set<Setting<?>> additionalSettings = extAwarePlugin.getExtensionSettings().stream().collect(Collectors.toSet());
 
-        expectedExtensions.add(
-            new DiscoveryExtensionNode(
-                "secondExtension",
-                "uniqueid2",
-                new TransportAddress(InetAddress.getByName("127.0.0.1"), 9301),
-                new HashMap<String, String>(),
-                Version.fromString("2.0.0"),
-                Version.fromString("2.0.0"),
-                List.of(expectedDependency)
-            )
+        ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, additionalSettings);
+
+        DiscoveryExtensionNode extension = new DiscoveryExtensionNode(
+            "secondExtension",
+            "uniqueid2",
+            new TransportAddress(InetAddress.getByName("127.0.0.1"), 9301),
+            new HashMap<String, String>(),
+            Version.fromString("2.0.0"),
+            Version.fromString("2.0.0"),
+            List.of()
+        );
+        DiscoveryExtensionNode initializedExtension = extensionsManager.getExtensionIdMap().get(extension.getId());
+        assertEquals(extension.getName(), initializedExtension.getName());
+        assertEquals(extension.getId(), initializedExtension.getId());
+        assertTrue(extensionsManager.lookupExtensionSettingsById(extension.getId()).isPresent());
+        assertEquals(
+            "none",
+            extensionsManager.lookupExtensionSettingsById(extension.getId()).get().getAdditionalSettings().get(customSetting)
         );
-        assertEquals(expectedExtensions.size(), extensionsManager.getExtensionIdMap().values().size());
-        for (DiscoveryExtensionNode extension : expectedExtensions) {
-            DiscoveryExtensionNode initializedExtension = extensionsManager.getExtensionIdMap().get(extension.getId());
-            assertEquals(extension.getName(), initializedExtension.getName());
-            assertEquals(extension.getId(), initializedExtension.getId());
-            assertEquals(extension.getAddress(), initializedExtension.getAddress());
-            assertEquals(extension.getAttributes(), initializedExtension.getAttributes());
-            assertEquals(extension.getVersion(), initializedExtension.getVersion());
-            assertEquals(extension.getMinimumCompatibleVersion(), initializedExtension.getMinimumCompatibleVersion());
-            assertEquals(extension.getDependencies(), initializedExtension.getDependencies());
-            assertTrue(extensionsManager.lookupExtensionSettingsById(extension.getId()).isPresent());
-            if ("firstExtension".equals(extension.getName())) {
-                assertEquals(
-                    "custom_setting",
-                    extensionsManager.lookupExtensionSettingsById(extension.getId()).get().getAdditionalSettings().get(customSetting)
-                );
-            } else if ("secondExtension".equals(extension.getName())) {
-                assertEquals(
-                    "none",
-                    extensionsManager.lookupExtensionSettingsById(extension.getId()).get().getAdditionalSettings().get(customSetting)
-                );
-            }
-        }
     }
 
     private void initialize(ExtensionsManager extensionsManager) {