diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java index e31c7980e473e..435f507fc1e40 100644 --- 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. + *

+ * Note: Only settings registered in {@link SettingsModule} can be changed dynamically. + *

+ * @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 void addSettingsUpdateConsumer(Setting setting, Consumer consumer, Consumer 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 index 512b420369a43..3f1e6a88857fa 100644 --- 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 index 1210011591cd9..0c5cece4249ef 100644 --- 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 getListValue(final List value) { ); } - @Test public void testAddSettingsUpdateConsumer() { Setting testSetting = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope); Setting testSetting2 = Setting.intSetting("foo.bar.baz", 1, Property.Dynamic, Property.NodeScope);