ConfigurationPropertyName equals method fails when uppercase values start the same

[gzurowski]

After upgrading to Spring Boot 2.1 I've noticed some configuration property map structures are not properly mapped to the corresponding Java representations as given in the following example. The same code and properties are working without issues with Spring Boot 2.0 and 1.5.

application.yml:

example:
  mymap:
    MY_KEY: value1
    MY_KEY_WITH_OTHER_VALUE: value2

Java:

@Configuration
@Data
@ConfigurationProperties("example")
public class ExampleConfig {
  private Map<String, String> mymap;
}

When running the above code, mymap will contain the following data:

key: MY_KEY => value: value1
key: MY_KEY_WITH_OTHER_VALUE => value: value1

Note that the expected value for key MY_KEY_WITH_OTHER_VALUE would be value2.

[wilkinsona]

Here's a minimal example that reproduces the problem:

package com.example.demo;

import java.util.Map;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.ConfigurableApplicationContext;

import com.example.demo.Gh15152Application.ExampleConfig;

@SpringBootApplication
@EnableConfigurationProperties(ExampleConfig.class)
public class Gh15152Application {

	public static void main(String[] args) {
		ConfigurableApplicationContext context = SpringApplication.run(Gh15152Application.class,
				"--example.mymap.MY_KEY=value1",
				"--example.mymap.MY_KEY_WITH_OTHER_VALUE=value2");
		ExampleConfig bean = context.getBean(ExampleConfig.class);
		System.out.println(bean.getMymap());
	}

	@ConfigurationProperties("example")
	public static class ExampleConfig {

		private Map<String, String> mymap;

		public Map<String, String> getMymap() {
			return mymap;
		}

		public void setMymap(Map<String, String> mymap) {
			this.mymap = mymap;
		}

	}

}

Output with 2.0.6:

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::        (v2.0.6.RELEASE)

2018-11-12 10:33:11.281  INFO 82059 --- [           main] com.example.demo.Gh15152Application      : Starting Gh15152Application on Andys-MacBook-Pro.local with PID 82059 (/Users/awilkinson/dev/workspaces/spring-projects/spring-boot/master/gh-15152/target/classes started by awilkinson in /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/master/gh-15152)
2018-11-12 10:33:11.283  INFO 82059 --- [           main] com.example.demo.Gh15152Application      : No active profile set, falling back to default profiles: default
2018-11-12 10:33:11.316  INFO 82059 --- [           main] s.c.a.AnnotationConfigApplicationContext : Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@79ad8b2f: startup date [Mon Nov 12 10:33:11 GMT 2018]; root of context hierarchy
2018-11-12 10:33:11.651  INFO 82059 --- [           main] o.s.j.e.a.AnnotationMBeanExporter        : Registering beans for JMX exposure on startup
2018-11-12 10:33:11.662  INFO 82059 --- [           main] com.example.demo.Gh15152Application      : Started Gh15152Application in 0.593 seconds (JVM running for 0.855)
{MY_KEY=value1, MY_KEY_WITH_OTHER_VALUE=value2}

And with 2.1.0:

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::        (v2.1.0.RELEASE)

2018-11-12 10:34:15.948  INFO 82062 --- [           main] com.example.demo.Gh15152Application      : Starting Gh15152Application on Andys-MacBook-Pro.local with PID 82062 (/Users/awilkinson/dev/workspaces/spring-projects/spring-boot/master/gh-15152/target/classes started by awilkinson in /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/master/gh-15152)
2018-11-12 10:34:15.951  INFO 82062 --- [           main] com.example.demo.Gh15152Application      : No active profile set, falling back to default profiles: default
2018-11-12 10:34:16.319  INFO 82062 --- [           main] com.example.demo.Gh15152Application      : Started Gh15152Application in 0.599 seconds (JVM running for 0.866)
{MY_KEY=value1, MY_KEY_WITH_OTHER_VALUE=value1}

[wilkinsona]

@gzurowski Unrelated to this problem, but a @ConfigurationProperties class should not be annotated with @Configuration. You should either annotate it what @Component or use @EnableConfigurationProperties(EnableConfig.class) on a @Configuration class to enable it.

[wilkinsona]

@gzurowski The keys that you have used do not meet the documented requirements:

When binding to Map properties, if the key contains anything other than lowercase alpha-numeric characters or -, you need to use the bracket notation so that the original value is preserved.

The quote above is from the 2.0.x documentation and the requirements remain the same in 2.1. I have yet to pinpoint why the behaviour has changed in 2.1, but your example was only accidentally working with 2.0.x so I would recommend updating the keys to enclose them in [].

[wilkinsona]

It looks like the change in behaviour is due to the changes made for #13414. When a configuration property name has been adapted, equality is not computed correctly. As a result, the following test fails:

@Test
public void equalsWhenStartsWithOfAdaptedName() {
	ConfigurationPropertyName n1 = ConfigurationPropertyName
			.adapt("example.mymap.ALPHA", '.');
	ConfigurationPropertyName n2 = ConfigurationPropertyName
			.adapt("example.mymap.ALPHA_BRAVO", '.');
	assertThat(n1).isNotEqualTo(n2);
}

Wrapping the name in [] affects how the name is adapted. As a result, the following test passes:

@Test
public void equalsWhenStartsWithOfAdaptedNameUsingBracketNotation() {
	ConfigurationPropertyName n1 = ConfigurationPropertyName
			.adapt("example.mymap.[ALPHA]", '.');
	ConfigurationPropertyName n2 = ConfigurationPropertyName
			.adapt("example.mymap.[ALPHA_BRAVO]", '.');
	assertThat(n1).isNotEqualTo(n2);
}

[wilkinsona]

The first test passes with the following change:

diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java
index f4b31f8259..f493f2b15a 100644
--- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java
+++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java
@@ -843,7 +843,7 @@ public final class ConfigurationPropertyName
                }
 
                private static boolean isAlpha(char ch) {
-                       return ch >= 'a' && ch <= 'z';
+                       return (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z');
                }
 
                private static boolean isNumeric(char ch) {

However, I'm not sure that we should make it. On the one hand, I suspect it may only be in support of properties that don't match the documented requirements. On the other hand, the end result reported here is certainly confusing as the wrong value gets bound into the map.

[philwebb]

I'm not sure if we should make the suggested change or not, but I do think we need to fix this in some way. I'm guessing that we might have the same problem if we we have names like example.myapp.!@#. I think ideally those should fail rather than be considered equal.

[mbhave]

Something similar was fixed in #14665. But that probably didn't apply to map keys that didn't match the documented requirements. +1 for @philwebb's point about failing than being treated as equal.