Ensure indexer output is deterministic and repeatable

[vpavic]

The output file that spring-context-indexer produces is currently written using Properties#store. This prevents the build of a project that uses indexer from being repeatable, due to current timestamp being included in comment inside output file, and also entries possibly not being ordered deterministically.

This PR ensures that indexer output is written in deterministic and repeatable manner by filtering out comments and ordering entries.

[vpavic]

Any feedback on this PR? The current behavior is preventing us from adopting spring-context-indexer as our builds must be repeatable.

[snicoll]

I had a chat with @wilkinsona as we have a similar requirement for Spring Boot. I am not a huge fan of the manual handling and assuming that values are not multi-lines. I hadn't enough cycles to try to improve it though but perhaps you do?

[snicoll]

Out of curiosity, have you actually noticed a significant improvement using the indexer? If so, can you share a bit more details?

[vpavic]

On application with over 1000 beans I've observed an improvement in startup time of roughly half a second. In relative terms that is around 3%, so quite measurable.

I'm not a fan of manual handling of writing Properties, but unfortunately the API leaves a lot to be desired. Will multi-line values ever be the case in indexer's output? Alternative to the current approach could be something involving a custom OutputStream that filters out undesirable values.

[vpavic]

With #23018 now resolved, I assumed this is unblocked and can be addressed in 5.2?

The only question is how to leverage the new SortedProperties, having in mind spring-context-indexer doesn't have dependencies to any other modules - is it OK to add dependency on spring-core?

[snicoll]

is it OK to add dependency on spring-core

I don't think it is so we'll have to decide what we want to do here and if we accept copy/pasting this.

[sbrannen]

I don't foresee any real issue with having a copy of SortedProperties used internally within the indexer. We'd just have to make sure they stay in sync in case a bug is ever fixed in one of the implementations.

The only dependency that would need to be worked around is the current use of StringUtils.tokenizeToStringArray(...), but that also should not be a problem.

[vpavic]

Assuming copy of SortedProperties within spring-context-indexer, what's the preferred way of handling StringUtils#tokenizeToStringArray dependency? Trimmed-down copy of StringUtils or something else?

[sbrannen]

Assuming copy of SortedProperties within spring-context-indexer, what's the preferred way of handling StringUtils#tokenizeToStringArray dependency? Trimmed-down copy of StringUtils or something else?

Maybe something as simple as String.split(...). We could even make the same change within SortedProperties to ensure consistency.

The reason I used StringUtils#tokenizeToStringArray was simply that that's how it's often done within core Spring, but I don't see that as a must.

[vpavic]

I've update the PR to take the approach with a copy of SortedProperties. I've also added a commit to update the original SortedProperties to use String#split instead of StringUtils as you suggested @sbrannen.

[sbrannen]

I've update the PR to take the approach with a copy of SortedProperties.

Looks fine to me.

I've also added a commit to update the original SortedProperties to use String#split instead of StringUtils as you suggested @sbrannen.

I actually pushed the same change to master just now in 53597f9. So that's already taken care of.