From 23934e39d2911c987bd7c91cbf5e8281ced361a9 Mon Sep 17 00:00:00 2001
From: Jason Tedor <jason@tedor.me>
Date: Wed, 5 Sep 2018 11:01:58 -0400
Subject: [PATCH] Fix deprecated setting specializations (#33412)

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              | 19 ++++++++++++++----
 .../common/settings/SettingTests.java         | 20 +++++++++++++++++++
 3 files changed, 36 insertions(+), 5 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 c23a0bd42e3..33f4718aa45 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 abstract class SecureSetting<T> extends Setting<T> {
     }
 
     @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 ceeb60f8edd..ff6a5b8fe0f 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,19 @@ public class Setting<T> implements ToXContentObject {
      * 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);
+    }
+
+    /**
+     * 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));
     }
 
@@ -713,7 +724,7 @@ public class Setting<T> implements ToXContentObject {
         }
 
         @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");
         }
@@ -820,7 +831,7 @@ public class Setting<T> implements ToXContentObject {
         }
 
         @Override
-        public String getRaw(Settings settings) {
+        public String innerGetRaw(final Settings settings) {
             Settings subSettings = get(settings);
             try {
                 XContentBuilder builder = XContentFactory.jsonBuilder();
@@ -913,7 +924,7 @@ public class Setting<T> implements ToXContentObject {
         }
 
         @Override
-        public String getRaw(Settings settings) {
+        String innerGetRaw(final Settings settings) {
             List<String> 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 bedb2857b60..d82b6206602 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 class SettingTests extends ESTestCase {
 
     }
 
+    public void testListSettingsDeprecated() {
+        final Setting<List<String>> deprecatedListSetting =
+                Setting.listSetting(
+                        "foo.deprecated",
+                        Collections.singletonList("foo.deprecated"),
+                        Function.identity(),
+                        Property.Deprecated,
+                        Property.NodeScope);
+        final Setting<List<String>> 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<List<String>> listSetting = Setting.listSetting("foo.bar", Arrays.asList("foo,bar"), (s) -> s.toString(),
             Property.Dynamic, Property.NodeScope);