Skip to content

Commit

Permalink
Fix deprecated setting specializations (#33412)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jasontedor committed Sep 5, 2018
1 parent 81014e0 commit 1498b7c
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,19 @@ 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);
}

/**
* 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));
}

Expand Down Expand Up @@ -713,7 +724,7 @@ 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");
}
Expand Down Expand Up @@ -820,7 +831,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();
Expand Down Expand Up @@ -913,7 +924,7 @@ private ListSetting(String key, Function<Settings, List<String>> defaultStringVa
}

@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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,26 @@ public void testCompositeValidator() {

}

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);
Expand Down

0 comments on commit 1498b7c

Please sign in to comment.