spring-cloud-commons icon indicating copy to clipboard operation
spring-cloud-commons copied to clipboard

ConfigurationPropertiesRebinder does not rebind removed properties for ConfigurationProperty beans

Open JordiMartinezVicent opened this issue 1 year ago • 2 comments

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

  }
}

JordiMartinezVicent avatar Jul 25 '22 10:07 JordiMartinezVicent

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

huifer avatar Aug 30 '22 09:08 huifer

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.

obourgain avatar Oct 12 '22 14:10 obourgain