Skip to content

Add support for collection properties in ApplicationProperties#1759

Closed
rodmibielli wants to merge 4 commits intospring-io:mainfrom
rodmibielli:rodmibielli
Closed

Add support for collection properties in ApplicationProperties#1759
rodmibielli wants to merge 4 commits intospring-io:mainfrom
rodmibielli:rodmibielli

Conversation

@rodmibielli
Copy link
Contributor

@rodmibielli rodmibielli commented Feb 25, 2026

I have faced a real problem when I tried to merge two distinct values with same key at application.yaml (or application.properties).

I have got an assertion error denying adding the same property key with its value more than once.
So, I have modified the application initializr code in order to be more generic: it does not have the strict rule of adding application property key with its value only once anymore: if you add more than once, at application.properties it will join the distinct values with comma; at application.yaml, it will create an array structure for the property (check the tests code as example).

This solution really worked for me as I think it will also work for a lot of people.

@rodmibielli rodmibielli changed the title Rodmibielli Allowing merging distinct application properties values with the same key Feb 25, 2026
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 25, 2026
@wilkinsona
Copy link
Contributor

wilkinsona commented Feb 25, 2026

if you add more than once, at application.properties it will join the distinct values with comma; at application.yaml, it will create an array structure for the property (check the tests code as example).

For properties that can consume a list, that might be OK. However, for properties that should only have a single value, it sounds like a step in the wrong direction. Configuring twice a property with a scalar value will no longer fail. Instead, you'll end up with something like example.single-int-value=1,2.

What's the use case for configuring the same property in two different places in an additive manner? I think it would be more robust if the code configuring the property, which has more context and knows that it's a list, dealt with combining the values.

@rodmibielli
Copy link
Contributor Author

What's the use case for configuring the same property in two different places in an additive manner? I think it would be more robust if the code configuring the property, which has more context and knows that it's a list, dealt with combining the values.

Use case:

  • two custom jars (initializr-generator-spring-ai-ollama.jar and initializr-generator-qdrant.jar) - they don't know about the existence of each other;
  • both jars contain classes that inherit io.spring.initializr.generator.spring.properties.ApplicationPropertiesCustomizer (OllamaApplicationPropertiesCustomizer and QdrantApplicationPropertiesCustomizer);
  • Both classes need to contribute with spring.docker.compose.file property values (./ollama/docker-compose.yml and ./qdrant/docker-compose.yml).

Okay, it could be solved externally and it would be easier if ApplicationProperties had implemented methods such as contains(String key) and get(String key) (both of them). Why it does not have them?

@mhalbritter
Copy link
Contributor

mhalbritter commented Feb 26, 2026

I agree with @wilkinsona . The changes to write lists are fine, but I don't like the automatic merging for properties into a list.

Okay, it could be solved externally and it would be easier if ApplicationProperties had implemented methods such as contains(String key) and get(String key) (both of them). Why it does not have them?

Because we haven't had a need for them. But I'm happy to add them.

@rodmibielli
Copy link
Contributor Author

I agree with @wilkinsona . The changes to write lists are fine, but I don't like the automatic merging for properties into a list.

Okay, it could be solved externally and it would be easier if ApplicationProperties had implemented methods such as contains(String key) and get(String key) (both of them). Why it does not have them?

Because we haven't had a need for them. But I'm happy to add them.

This is great! Thank you!

@rodmibielli
Copy link
Contributor Author

I agree with @wilkinsona . The changes to write lists are fine, but I don't like the automatic merging for properties into a list.

Okay, it could be solved externally and it would be easier if ApplicationProperties had implemented methods such as contains(String key) and get(String key) (both of them). Why it does not have them?

Because we haven't had a need for them. But I'm happy to add them.

...I was thinking here... wouldn´t be possible to create a new method merge(ApplicationProperties other) at ApplicationProperties so it merges other application properties with this object and then the merging properties with same key would apply instead of using the add method? I better you that there are more people facing the same problem...just an opinion...

@mhalbritter
Copy link
Contributor

mhalbritter commented Feb 26, 2026

We can't just automatically merge all properties into a list of properties - some properties are lists, some are scalar values. Initializr doesn't know this, the caller using the ApplicationProperties should know that. Therefore I don't think any automatic creation of lists will cut it.

We could think about an API design which distinguishes between scalar values and list properties - the former doesn't allow duplicate values, while the latter would automatically append to the list IFF the property has been added as a list property in the first place.

However, I wouldn't do that in this PR. This PR should focus on writing properties of type List as YAML lists or comma-separated properties. Together with the contains and get this should at least work around the problem until we have a API design for list properties.

Do you want to remove the automatic merging part or should I do this when polishing this PR?

@rodmibielli
Copy link
Contributor Author

rodmibielli commented Feb 26, 2026

Do you want to remove the automatic merging part or should I do this when polishing this PR?

Ok, for now contains and get methods work for me.
Please, I also need a method in order to replace the old value, such as set(String key,Object value) or remove(String key) method.
Thank you.

@eunseo9311
Copy link
Contributor

Thanks for the discussion and clarification.

I'm happy to remove the automatic merging behavior and focus this PR on supporting list properties as suggested. The contains/get methods should be sufficient to handle the use case externally.

Please let me know if there are any additional changes you'd like me to make.

@rodmibielli
Copy link
Contributor Author

Thanks for the discussion and clarification.

I'm happy to remove the automatic merging behavior and focus this PR on supporting list properties as suggested. The contains/get methods should be sufficient to handle the use case externally.

Please let me know if there are any additional changes you'd like me to make.

Also either set or remove methods in order to replace or remove/add the old values.

Thank you.

@mhalbritter mhalbritter changed the title Allowing merging distinct application properties values with the same key Add support for List properties in ApplicationProperties Mar 3, 2026
@mhalbritter mhalbritter self-assigned this Mar 3, 2026
@mhalbritter mhalbritter added type: enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 3, 2026
@mhalbritter
Copy link
Contributor

mhalbritter commented Mar 3, 2026

@rodmibielli Something is off with the git commits. This PR includes the commit 4a8e70c, which has nothing to do with the PR. Additionally, the commit b178bf5, which implements the feature we want, is a merge commit, which doesn't seem right:

image

Could you please fix the PR? Thanks!

@mhalbritter
Copy link
Contributor

mhalbritter commented Mar 3, 2026

@rodmibielli never mind, sorry for the ping. I managed to transform the merge commit to a "normal" commit with some git fu.

@rodmibielli
Copy link
Contributor Author

rodmibielli commented Mar 3, 2026 via email

@mhalbritter
Copy link
Contributor

Sorry @rodmibielli , one more thing. The commit b178bf5 is missing the Signed-off-by trailer. Can you please add it so that we have a good DCO?

@rodmibielli
Copy link
Contributor Author

rodmibielli commented Mar 3, 2026 via email

mhalbritter pushed a commit that referenced this pull request Mar 3, 2026
Signed-off-by: rodmibielli <rodrigo.mibielli@gmail.com>

See gh-1759
@mhalbritter mhalbritter added this to the 0.24.0 milestone Mar 3, 2026
@mhalbritter
Copy link
Contributor

Thanks @rodmibielli !

@mhalbritter
Copy link
Contributor

I created #1760 for the read / remove methods.

@mhalbritter mhalbritter changed the title Add support for List properties in ApplicationProperties Add support for Collection properties in ApplicationProperties Mar 3, 2026
@mhalbritter mhalbritter changed the title Add support for Collection properties in ApplicationProperties Add support for collection properties in ApplicationProperties Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants