From 4291ac2ff3f20dc963949d49cf70542d7f4814f7 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 5 Sep 2018 04:58:26 -0400 Subject: [PATCH 1/3] Fix deprecated setting specializations Deprecating a some setting specializations (e.g., list settings) does not cause deprecation warning headers and deprecation log messages to appear. This is due to a missed check for deprecation. This commit fixes this for all setting specializations, and ensures that this can not be missed again. --- .../common/settings/SecureSetting.java | 2 +- .../common/settings/Setting.java | 14 ++++++++----- .../common/settings/SettingTests.java | 20 +++++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index c23a0bd42e3e0..33f4718aa45e4 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -69,7 +69,7 @@ public T getDefault(Settings settings) { } @Override - public String getRaw(Settings settings) { + String innerGetRaw(final Settings settings) { throw new UnsupportedOperationException("secure settings are not strings"); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index ceeb60f8edd04..a9e300060a0e6 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -426,8 +426,12 @@ public void diff(Settings.Builder builder, Settings source, Settings defaultSett * Returns the raw (string) settings value. If the setting is not present in the given settings object the default value is returned * instead. This is useful if the value can't be parsed due to an invalid value to access the actual value. */ - public String getRaw(Settings settings) { + public final String getRaw(final Settings settings) { checkDeprecation(settings); + return innerGetRaw(settings); + } + + String innerGetRaw(final Settings settings) { return settings.get(getKey(), defaultValue.apply(settings)); } @@ -713,9 +717,9 @@ public T get(Settings settings) { } @Override - public String getRaw(Settings settings) { + public String innerGetRaw(final Settings settings) { throw new UnsupportedOperationException("affix settings can't return values" + - " use #getConcreteSetting to obtain a concrete setting"); + " use #getConcreteSetting to obtain a concrete setting"); } @Override @@ -820,7 +824,7 @@ public boolean isGroupSetting() { } @Override - public String getRaw(Settings settings) { + public String innerGetRaw(final Settings settings) { Settings subSettings = get(settings); try { XContentBuilder builder = XContentFactory.jsonBuilder(); @@ -913,7 +917,7 @@ private ListSetting(String key, Function> defaultStringVa } @Override - public String getRaw(Settings settings) { + String innerGetRaw(final Settings settings) { List array = settings.getAsList(getKey(), null); return array == null ? defaultValue.apply(settings) : arrayToParsableString(array); } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index bedb2857b6055..d82b620660249 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -462,6 +462,26 @@ public void testCompositeValidator() { } + public void testListSettingsDeprecated() { + final Setting> deprecatedListSetting = + Setting.listSetting( + "foo.deprecated", + Collections.singletonList("foo.deprecated"), + Function.identity(), + Property.Deprecated, + Property.NodeScope); + final Setting> nonDeprecatedListSetting = + Setting.listSetting( + "foo.non_deprecated", Collections.singletonList("foo.non_deprecated"), Function.identity(), Property.NodeScope); + final Settings settings = Settings.builder() + .put("foo.deprecated", "foo.deprecated1,foo.deprecated2") + .put("foo.deprecated", "foo.non_deprecated1,foo.non_deprecated2") + .build(); + deprecatedListSetting.get(settings); + nonDeprecatedListSetting.get(settings); + assertSettingDeprecationsAndWarnings(new Setting[]{deprecatedListSetting}); + } + public void testListSettings() { Setting> listSetting = Setting.listSetting("foo.bar", Arrays.asList("foo,bar"), (s) -> s.toString(), Property.Dynamic, Property.NodeScope); From e25e1ba26e7d26ea0a79322580071ece428d9623 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 5 Sep 2018 05:06:45 -0400 Subject: [PATCH 2/3] Revert format change --- .../main/java/org/elasticsearch/common/settings/Setting.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index a9e300060a0e6..2d54819e0f3a1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -719,7 +719,7 @@ public T get(Settings settings) { @Override public String innerGetRaw(final Settings settings) { throw new UnsupportedOperationException("affix settings can't return values" + - " use #getConcreteSetting to obtain a concrete setting"); + " use #getConcreteSetting to obtain a concrete setting"); } @Override From 10dd9bb34fc83ca79a5baf854a01b33f15166813 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 5 Sep 2018 05:08:49 -0400 Subject: [PATCH 3/3] Javadocs --- .../java/org/elasticsearch/common/settings/Setting.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 2d54819e0f3a1..ff6a5b8fe0f96 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -431,6 +431,13 @@ public final String getRaw(final Settings settings) { return innerGetRaw(settings); } + /** + * The underlying implementation for {@link #getRaw(Settings)}. Setting specializations can override this as needed to convert the + * actual settings value to raw strings. + * + * @param settings the settings instance + * @return the raw string representation of the setting value + */ String innerGetRaw(final Settings settings) { return settings.get(getKey(), defaultValue.apply(settings)); }