spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
705 stars 701 forks source link

ConfigurationPropertiesRebinder does not rebind removed properties for ConfigurationProperty beans #1133

Open JordiMartinezVicent opened 2 years ago

JordiMartinezVicent commented 2 years ago

Spring boot version: 2.6.10 Spring Cloud version: 2021.0.2

Describe the bug Consider a ConfigurationProperty bean which has a default value defined at the code:

@ConfigurationProperties("org.jordi")
static class MyConfigProps {
  private Integer myProp = 0;

  public Integer getMyProp() {
    return this.myProp;
  }
  public void setMyProp(final Integer myProp) {
     this.myProp = myProp;
  }
}

The initial value of the bean MyConfigProps#myProp = 0, as it is defined at the code.

When the property changes at the environment org.jordi.my-prop=1 and the rebind is executed, the value of the bean MyConfigProps#myProp changes to 1. Which is great.

But when the same property is removed from the environment and the rebind is executed again, the value of the bean MyConfigProps#myProp remains to 1 instead of changing the value to the initial value.

Sample

See the behavior described at the following test:

@SpringBootTest(classes = ConfigurationPropertiesRebinderTestConfig.class)
@EnableAutoConfiguration
class ConfigurationPropertiesRebinderTest {

  @Autowired
  private ConfigurableEnvironment env;

  @Autowired
  private ConfigurationPropertiesRebinder rebinder;

  @Autowired
  private MyConfigProps myConfigProps;

  @Test
  void test() {

    // Value from code
    assertThat(this.myConfigProps.getMyProp()).isZero();

    this.addPropertyToEnvironment("org.jordi.my-prop", 1);
    this.rebinder.rebind();
    assertThat(this.myConfigProps.getMyProp()).isEqualTo(1);

    this.removePropertyFromEnvironment("org.jordi.my-prop");
    this.rebinder.rebind();

    // FAILS!!!
    // After removing the property from tne environment, myConfigProps.getMyProp() should take the initial value from the code but it  does not
    assertThat(this.myConfigProps.getMyProp()).isZero();

  }

  @Configuration(proxyBeanMethods = false)
  @EnableConfigurationProperties(MyConfigProps.class)
  static class ConfigurationPropertiesRebinderTestConfig {

  }

  @ConfigurationProperties("org.jordi")
  static class MyConfigProps {

    private Integer myProp = 0;

    public Integer getMyProp() {
      return this.myProp;
    }

    public void setMyProp(final Integer myProp) {
      this.myProp = myProp;
    }

  }

  private void addPropertyToEnvironment(final String name, final Object value) {
    final var myPropertySource = this.env.getPropertySources().get("MyEnvironment");

    if (myPropertySource == null) {

      final var propertiesMap = new HashMap<String, Object>();
      propertiesMap.put(name, value);

      this.env.getPropertySources().addFirst(new MapPropertySource("MyEnvironment", propertiesMap));
    } else {
      ((MapPropertySource) myPropertySource).getSource().put(name, value);
    }

  }

  private void removePropertyFromEnvironment(final String name) {
    final var myPropertySource = this.env.getPropertySources().get("MyEnvironment");
    if (myPropertySource != null) {
      ((MapPropertySource) myPropertySource).getSource().remove(name);
    }

  }
}
huifer commented 2 years ago

After removal, I think it should not be reset to the default value. After removal, it should be set to null

obourgain commented 1 year ago

Hello, this issue also affects Maps. A key removed from the properties is not removed from the map on properties rebind. This seems to come from MapBinder.merge() that doesn't check for removed keys.