code refactoring

Signed-off-by: Kuanysh Aimurzinov <[email protected]>
opensearch-project · Jun 21, 2023 · b335b84 · b335b84
1 parent a41e810
commit b335b84
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 22 deletions.
diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java
@@ -238,25 +238,18 @@ public synchronized Settings applySettings(Settings newSettings) {
 
     /**
      * Adds a settings consumer with a predicate that is only evaluated at update time.
-     * This method allows registering an additional validator that is only applied to updates of a specific setting.
-     * It is useful to add additional validation to settings at runtime compared to at startup time.
-     * Please note that only settings registered in the {@link SettingsModule} can be changed dynamically.
-     *
-     * @param setting The setting for which the consumer is registered.
-     * @param consumer The consumer to be invoked when the setting is updated.
-     * @param validator An additional validator that is only applied to updates of this setting.
-     * @throws SettingsException if the setting is not registered for the given key.
-     * @throws NullPointerException if the setting is not registered.
+     * <p>
+     * Note: Only settings registered in {@link SettingsModule} can be changed dynamically.
+     * </p>
+     * @param validator an additional validator that is only applied to updates of this setting.
+     *                  This is useful to add additional validation to settings at runtime compared to at startup time.
      */
     public synchronized <T> void addSettingsUpdateConsumer(Setting<T> setting, Consumer<T> consumer, Consumer<T> validator) {
-        if (setting.getKey() != null && !setting.equals(get(setting.getKey()))) {
+        if (setting != get(setting.getKey())) {
             throw new SettingsException("Setting is not registered for key [" + setting.getKey() + "]");
-        } else if (setting.getKey() == null) {
-            throw new NullPointerException("Setting is not registered");
-        } else {
-            addSettingsUpdater(setting.newUpdater(consumer, logger, validator));
         }
-    }
+        addSettingsUpdater(setting.newUpdater(consumer, logger, validator));
+        }
 
     /**
      * Adds a settings consumer that is only executed if any setting in the supplied list of settings is changed. In that case all the

diff --git a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java
@@ -75,11 +75,11 @@ TransportResponse handleAddSettingsUpdateConsumerRequest(AddSettingsUpdateConsum
 
                 // we need to get the actual setting from nodeSetting or indexsetting maps in SettingsModule
                 // use conditional based on setting properties
-                Setting<?> SettingForUpdateConsumer = null;
+                Setting<?> settingForUpdateConsumer = null;
                 if (setting.hasIndexScope()) {
-                    SettingForUpdateConsumer = settingsModule.getIndexScopedSettings().get(setting.getKey());
+                    settingForUpdateConsumer = settingsModule.getIndexScopedSettings().get(setting.getKey());
                 } else if (setting.hasNodeScope()) {
-                    SettingForUpdateConsumer = settingsModule.getClusterSettings().get(setting.getKey());
+                    settingForUpdateConsumer = settingsModule.getClusterSettings().get(setting.getKey());
                 }
                 // do a null check and throw IllegalArgument exception here if neither index or node scope
 
@@ -88,7 +88,7 @@ TransportResponse handleAddSettingsUpdateConsumerRequest(AddSettingsUpdateConsum
                 // Register setting update consumer with callback method to extension
                 if (setting.hasIndexScope()) {
                     clusterService.getClusterSettings()
-                        .addSettingsUpdateConsumer(SettingForUpdateConsumer, (data) -> {
+                        .addSettingsUpdateConsumer(settingForUpdateConsumer, (data) -> {
                             logger.debug("Sending extension request type: " + updateSettingsRequestType);
                             UpdateSettingsResponseHandler updateSettingsResponseHandler = new UpdateSettingsResponseHandler();
                             transportService.sendRequest(
@@ -102,7 +102,7 @@ TransportResponse handleAddSettingsUpdateConsumerRequest(AddSettingsUpdateConsum
                 if (setting.hasNodeScope()) {
                     clusterService.getClusterSettings()
                         // Register setting update consumer with callback method to extension
-                        .addSettingsUpdateConsumer(SettingForUpdateConsumer, (data) -> {
+                        .addSettingsUpdateConsumer(settingForUpdateConsumer, (data) -> {
                             logger.debug("Sending extension request type: " + updateSettingsRequestType);
                             UpdateSettingsResponseHandler updateSettingsResponseHandler = new UpdateSettingsResponseHandler();
                             transportService.sendRequest(

diff --git a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java
@@ -33,7 +33,6 @@
 
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
-import org.junit.Test;
 import org.opensearch.Version;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.routing.allocation.decider.FilterAllocationDecider;
@@ -1463,7 +1462,6 @@ public List<String> getListValue(final List<String> value) {
         );
     }
 
-    @Test
     public void testAddSettingsUpdateConsumer() {
         Setting<Integer> testSetting = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope);
         Setting<Integer> testSetting2 = Setting.intSetting("foo.bar.baz", 1, Property.Dynamic, Property.NodeScope);