Affects: 5.2.5

I have the following application.yml:

foo: 
  bar: 
    - 
      id: baz
      item: value
    // ...

Then I want to overwrite item value in tests using @DynamicPropertySource:

    @DynamicPropertySource
    @JvmStatic
    @Suppress("unused")
    fun setupProperties(registry: DynamicPropertyRegistry) {
        registry.add("foo.bar[0].item") { "new value" }
    }

Unfortunately, the above code: 1) removes all elements from the array 2) adds one element with item field set to new value

I created a sample java project highlighting this issue: https://github.com/kkocel/dynamicproperties

Comment From: sbrannen

Thanks for providing the example application.

For what it's worth, you will experience the same behavior using @TestPropertySource(properties = "foo.bar[0].item = other val") instead of @DynamicPropertySource.

Thus, this behavior is not specific to @DynamicPropertySource.

Comment From: sbrannen

Interestingly enough, no elements are removed from the set of registered property sources.

A little debugging reveals the following are present.

>>>>> Dynamic Test Properties :: {foo.bar[0].item=com.example.dynamicproperties.FailingOverwritingTests$$Lambda$453/0x000000080032e040@7f3ca64a}
>>>>> configurationProperties :: org.springframework.boot.context.properties.source.SpringConfigurationPropertySources@4d464510
>>>>> Inlined Test Properties :: {spring.jmx.enabled=false, org.springframework.boot.test.context.SpringBootTestContextBootstrapper=true, server.port=-1}
>>>>> systemProperties :: ............
>>>>> systemEnvironment ::  ............
>>>>> random :: java.util.Random@64e7d698
>>>>> applicationConfig: [classpath:/application.yml] :: {foo.bar[0].id=baz, foo.bar[0].item=val1, foo.bar[0].url=http://example1.com, foo.bar[1].id=boo, foo.bar[1].item=val2, foo.bar[1].url=http://example2.com}

Your dynamic property is present in Dynamic Test Properties, and the original properties are present in applicationConfig: [classpath:/application.yml].

Thus, something must be happening with the lookup within the Spring Environment.

FYI: this is how I debugged that.

@BeforeEach
void debugEnvironment(@Autowired ConfigurableEnvironment environment) {
     environment.getPropertySources().forEach(propertySource -> {
         System.err.println(">>>>> " + propertySource.getName() + " :: " + propertySource.getSource());
     });
}

Comment From: snicoll

This is the expected behaviour. When you override a list, the whole list is overridden, not individual elements, see the documentation for more details.

Comment From: kkocel

I would say that from @DynamicPropertySource user perspective it's very unexpected behavior.

I suggest:

1) Mention such overriding behavior in @DynamicPropertySource documentation: https://docs.spring.io/spring-framework/docs/5.2.5.RELEASE/spring-framework-reference/testing.html#testcontext-ctx-management-dynamic-property-sources

or

2) Reconsider implementing a smarter merging strategy - update one element value instead whole list - @snicoll as you can see it surprising even for other core Spring developers ;)

Comment From: snicoll

I would say that from @DynamicPropertySource user perspective it's very unexpected behavior.

This is an argument we've heard of before but IMO you are mixing two different things. The behaviour for overriding lists is documented and consistent.

Reconsider implementing a smarter merging strategy

The team has spent a significant amount of time looking at the pros and cons of each approach and I don't think that's an option. What you're calling smarter is a recipe for disaster when you want to set values for a list using profiles for instance.

as you can see it surprising even for other core Spring developers ;)

I didn't see that at all. What I've seen is a report from you that was described in such a way that it felt specific to a core framework issue. It wasn't and that's the confusing part IMO.

Comment From: kkocel

This is an argument we've heard of before but IMO you are mixing two different things. The behaviour for overriding lists is documented and consistent.

I agree that such change can lead to unexpected behavior in other parts ( as you mentioned profiles)

But then, there should be a possibility to provide the whole item in the list using @DynamicPropertySource.

Having that - it would be possible to overwrite part of properties without changing the merging algorithm.

Comment From: kkocel

After a bit of debugging, I found out that when @DynamicPropertySource provides all the necessary parameters for the list item. Then IndexedElementsBinder is able to successfully populate Configuration property.

        @DynamicPropertySource
        fun setupProperties(registry: DynamicPropertyRegistry) {
            registry.add("foo.bar[0].id") { "id" }
            registry.add("foo.bar[0].value") { "new value" }
            // all other necessary ...

@snicoll Sorry for the confusion!