From 256ca681fd40b0935d3a1927fc08c60d64e4ecd7 Mon Sep 17 00:00:00 2001 From: chang-chao Date: Fri, 26 Oct 2018 00:40:08 +0900 Subject: [PATCH 1/4] Consider aliases when checking descendants Update `AliasedConfigurationPropertySource` to consider aliases in `containsDescendantOf`. Prior to this commit, given a source containing `example.name` with a defined alias of `other.name -> example.name` calling `containsDescendantOf("other")` would incorrectly return `ConfigurationPropertyState.ABSENT`. Closes gh-14967 --- .../AliasedConfigurationPropertySource.java | 18 +++++++++---- .../ConfigurationPropertyNameAliases.java | 5 ++++ ...iasedConfigurationPropertySourceTests.java | 26 ++++++++++++++++++- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java index ea9496881b..6b2eefa708 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java @@ -16,6 +16,8 @@ package org.springframework.boot.context.properties.source; +import java.util.Set; + import org.springframework.util.Assert; /** @@ -58,11 +60,17 @@ class AliasedConfigurationPropertySource implements ConfigurationPropertySource if (result != ConfigurationPropertyState.ABSENT) { return result; } - for (ConfigurationPropertyName alias : getAliases().getAliases(name)) { - ConfigurationPropertyState aliasResult = this.source - .containsDescendantOf(alias); - if (aliasResult != ConfigurationPropertyState.ABSENT) { - return aliasResult; + Set aliasNames = this.aliases.getAllNames(); + for (ConfigurationPropertyName configurationPropertyName : aliasNames) { + boolean descendantPresentInAlias = this.aliases + .getAliases(configurationPropertyName).stream() + .filter(name::isAncestorOf).findFirst().isPresent(); + if (descendantPresentInAlias) { + ConfigurationProperty configurationProperty = this.getSource() + .getConfigurationProperty(configurationPropertyName); + if (configurationProperty != null) { + return ConfigurationPropertyState.PRESENT; + } } } return ConfigurationPropertyState.ABSENT; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java index f668bae35f..36b33f74e4 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; @@ -74,4 +75,8 @@ public final class ConfigurationPropertyNameAliases { .findFirst().orElse(null); } + public Set getAllNames() { + return this.aliases.keySet(); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java index ca9e4e34e5..fd1a7af93e 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java @@ -19,6 +19,8 @@ package org.springframework.boot.context.properties.source; import org.junit.Test; import org.mockito.Answers; +import org.springframework.boot.origin.Origin; + import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -105,8 +107,30 @@ public class AliasedConfigurationPropertySourceTests { .willReturn(ConfigurationPropertyState.ABSENT); given(source.containsDescendantOf(ConfigurationPropertyName.of("bar"))) .willReturn(ConfigurationPropertyState.PRESENT); + ConfigurationPropertyName barBar = ConfigurationPropertyName.of("bar.bar"); + given(source.getConfigurationProperty(barBar)).willReturn( + new ConfigurationProperty(barBar, "barBarValue", mock(Origin.class))); ConfigurationPropertySource aliased = source - .withAliases(new ConfigurationPropertyNameAliases("foo", "bar")); + .withAliases(new ConfigurationPropertyNameAliases("bar.bar", "foo.foo")); + assertThat(aliased.containsDescendantOf(name)) + .isEqualTo(ConfigurationPropertyState.PRESENT); + } + + @Test + public void containsDescendantOfWhenPresentInAliasShouldReturnPresent() { + ConfigurationPropertyName name = ConfigurationPropertyName.of("baz"); + ConfigurationPropertySource source = mock(ConfigurationPropertySource.class, + withSettings().defaultAnswer(Answers.CALLS_REAL_METHODS)); + given(source.containsDescendantOf(name)) + .willReturn(ConfigurationPropertyState.ABSENT); + + ConfigurationPropertyName barFoo = ConfigurationPropertyName.of("bar.foo"); + + given(source.getConfigurationProperty(barFoo)).willReturn( + new ConfigurationProperty(barFoo, "barFooValue", mock(Origin.class))); + + ConfigurationPropertySource aliased = source + .withAliases(new ConfigurationPropertyNameAliases("bar.foo", "baz.foo")); assertThat(aliased.containsDescendantOf(name)) .isEqualTo(ConfigurationPropertyState.PRESENT); } From 5603d61909637d3fd93307ce50e826eb4f8a3873 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2018 15:30:28 -0700 Subject: [PATCH 2/4] Polish "Consider aliases when checking descendants" See gh-14967 --- .../AliasedConfigurationPropertySource.java | 25 +++++++++--------- .../ConfigurationPropertyNameAliases.java | 10 ++++--- ...iasedConfigurationPropertySourceTests.java | 26 +++++-------------- 3 files changed, 26 insertions(+), 35 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java index 6b2eefa708..42beffcb55 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java @@ -16,8 +16,6 @@ package org.springframework.boot.context.properties.source; -import java.util.Set; - import org.springframework.util.Assert; /** @@ -60,16 +58,19 @@ class AliasedConfigurationPropertySource implements ConfigurationPropertySource if (result != ConfigurationPropertyState.ABSENT) { return result; } - Set aliasNames = this.aliases.getAllNames(); - for (ConfigurationPropertyName configurationPropertyName : aliasNames) { - boolean descendantPresentInAlias = this.aliases - .getAliases(configurationPropertyName).stream() - .filter(name::isAncestorOf).findFirst().isPresent(); - if (descendantPresentInAlias) { - ConfigurationProperty configurationProperty = this.getSource() - .getConfigurationProperty(configurationPropertyName); - if (configurationProperty != null) { - return ConfigurationPropertyState.PRESENT; + for (ConfigurationPropertyName alias : getAliases().getAliases(name)) { + ConfigurationPropertyState aliasResult = this.source + .containsDescendantOf(alias); + if (aliasResult != ConfigurationPropertyState.ABSENT) { + return aliasResult; + } + } + for (ConfigurationPropertyName from : getAliases()) { + for (ConfigurationPropertyName alias : getAliases().getAliases(from)) { + if (name.isAncestorOf(alias)) { + if (this.source.getConfigurationProperty(from) != null) { + return ConfigurationPropertyState.PRESENT; + } } } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java index 36b33f74e4..3ca9f6beb6 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java @@ -18,9 +18,9 @@ package org.springframework.boot.context.properties.source; import java.util.Arrays; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; @@ -34,7 +34,8 @@ import org.springframework.util.MultiValueMap; * @since 2.0.0 * @see ConfigurationPropertySource#withAliases(ConfigurationPropertyNameAliases) */ -public final class ConfigurationPropertyNameAliases { +public final class ConfigurationPropertyNameAliases + implements Iterable { private final MultiValueMap aliases = new LinkedMultiValueMap<>(); @@ -75,8 +76,9 @@ public final class ConfigurationPropertyNameAliases { .findFirst().orElse(null); } - public Set getAllNames() { - return this.aliases.keySet(); + @Override + public Iterator iterator() { + return this.aliases.keySet().iterator(); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java index fd1a7af93e..8d4d15a527 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySourceTests.java @@ -16,11 +16,11 @@ package org.springframework.boot.context.properties.source; +import java.util.Collections; + import org.junit.Test; import org.mockito.Answers; -import org.springframework.boot.origin.Origin; - import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -107,31 +107,19 @@ public class AliasedConfigurationPropertySourceTests { .willReturn(ConfigurationPropertyState.ABSENT); given(source.containsDescendantOf(ConfigurationPropertyName.of("bar"))) .willReturn(ConfigurationPropertyState.PRESENT); - ConfigurationPropertyName barBar = ConfigurationPropertyName.of("bar.bar"); - given(source.getConfigurationProperty(barBar)).willReturn( - new ConfigurationProperty(barBar, "barBarValue", mock(Origin.class))); ConfigurationPropertySource aliased = source - .withAliases(new ConfigurationPropertyNameAliases("bar.bar", "foo.foo")); + .withAliases(new ConfigurationPropertyNameAliases("foo", "bar")); assertThat(aliased.containsDescendantOf(name)) .isEqualTo(ConfigurationPropertyState.PRESENT); } @Test public void containsDescendantOfWhenPresentInAliasShouldReturnPresent() { - ConfigurationPropertyName name = ConfigurationPropertyName.of("baz"); - ConfigurationPropertySource source = mock(ConfigurationPropertySource.class, - withSettings().defaultAnswer(Answers.CALLS_REAL_METHODS)); - given(source.containsDescendantOf(name)) - .willReturn(ConfigurationPropertyState.ABSENT); - - ConfigurationPropertyName barFoo = ConfigurationPropertyName.of("bar.foo"); - - given(source.getConfigurationProperty(barFoo)).willReturn( - new ConfigurationProperty(barFoo, "barFooValue", mock(Origin.class))); - + ConfigurationPropertySource source = new MapConfigurationPropertySource( + Collections.singletonMap("foo.bar", "foobar")); ConfigurationPropertySource aliased = source - .withAliases(new ConfigurationPropertyNameAliases("bar.foo", "baz.foo")); - assertThat(aliased.containsDescendantOf(name)) + .withAliases(new ConfigurationPropertyNameAliases("foo.bar", "baz.foo")); + assertThat(aliased.containsDescendantOf(ConfigurationPropertyName.of("baz"))) .isEqualTo(ConfigurationPropertyState.PRESENT); } From 997de53f20e9b98538a367cc2c7061f979a47e89 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2018 16:35:00 -0700 Subject: [PATCH 3/4] Remove uncessary call in PropertiesMigrationReport Closes gh-14974 --- .../context/properties/migrator/PropertiesMigrationReporter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java b/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java index 813a0ac367..4c90120c42 100644 --- a/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java +++ b/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java @@ -174,7 +174,6 @@ class PropertiesMigrationReporter { private Map getPropertySourcesAsMap() { Map map = new LinkedHashMap<>(); - ConfigurationPropertySources.get(this.environment); for (ConfigurationPropertySource source : ConfigurationPropertySources .get(this.environment)) { map.put(determinePropertySourceName(source), source); From 33c7a74bee323bdc76139ff2eaa11a15a00c966a Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2018 16:35:28 -0700 Subject: [PATCH 4/4] Update copyright year for changed files --- .../liquibase/LiquibaseEndpointAutoConfigurationTests.java | 2 +- .../boot/actuate/security/AuthorizationAuditListenerTests.java | 2 +- .../springframework/boot/cli/command/CommandRunnerTests.java | 2 +- .../boot/devtools/restart/DefaultRestartInitializer.java | 2 +- .../boot/devtools/restart/DefaultRestartInitializerTests.java | 2 +- .../boot/context/properties/bind/PlaceholdersResolver.java | 2 +- .../properties/source/AliasedConfigurationPropertySource.java | 2 +- .../properties/source/ConfigurationPropertyNameAliases.java | 2 +- .../org/springframework/boot/web/codec/CodecCustomizer.java | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/liquibase/LiquibaseEndpointAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/liquibase/LiquibaseEndpointAutoConfigurationTests.java index b420015975..1e0f11edef 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/liquibase/LiquibaseEndpointAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/liquibase/LiquibaseEndpointAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/security/AuthorizationAuditListenerTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/security/AuthorizationAuditListenerTests.java index 43e28dca81..259a1b3967 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/security/AuthorizationAuditListenerTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/security/AuthorizationAuditListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/CommandRunnerTests.java b/spring-boot-project/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/CommandRunnerTests.java index 989db667e6..dedf370a7b 100644 --- a/spring-boot-project/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/CommandRunnerTests.java +++ b/spring-boot-project/spring-boot-cli/src/test/java/org/springframework/boot/cli/command/CommandRunnerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/DefaultRestartInitializer.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/DefaultRestartInitializer.java index 3526a02420..dd6acbbe14 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/DefaultRestartInitializer.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/DefaultRestartInitializer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/DefaultRestartInitializerTests.java b/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/DefaultRestartInitializerTests.java index ba8bb8e374..98411e99b0 100644 --- a/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/DefaultRestartInitializerTests.java +++ b/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/DefaultRestartInitializerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/PlaceholdersResolver.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/PlaceholdersResolver.java index 0df74b89d7..54d7275568 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/PlaceholdersResolver.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/PlaceholdersResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java index 42beffcb55..6afef15822 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java index 3ca9f6beb6..7536aa6ea6 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameAliases.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/codec/CodecCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/codec/CodecCustomizer.java index d0a5d73cac..182f3c254b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/codec/CodecCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/codec/CodecCustomizer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.