Add support for Spring Boot multi-document properties files

[philwebb]

Spring Boot 2.4 will add support for multi-document .properties files. These behave in a similar way to multi-document YAML files and allow the user to split a physical .properties file into multiple independent logical documents. With properties #--- is used as the separator. The reference documentation has an updated section that covers the new syntax.

The new format may cause a few problem with the IDE editor. Specifically, you can get errors about duplicate keys. It would be nice if the IDE could treat each logical section as a different document and not present those warnings. If you're interested in some Java code that parses the new format, you can look at OriginTrackedPropertiesLoader.java from the Spring Boot codebase.

[kdvolder]

Is it an option to use a different extension for this 'new' format?

I think it is a bad idea to redefine the existing format. A cleaner solution would be to choose a different extension for the new format (e.g. .mproperties or whatever). This will avoid issues with existing tooling (not just Eclipse I expect, but any editor that tries to validate the .properties file because the file extension makes the tool assume it is in fact a .properties file.

[martinlippert]

@philwebb First of all, thanks a lot for raising this issue here and making us aware of these updates in Spring Boot. That is very very much appreciated.

As far as I can see, the YAML editing support deals with this multi-document files already and looks good so far. If you experience any issues within YAML files, please let us know.

The editing experience for this in the properties editor causes indeed duplicate key errors to show up. We should fix that. In addition to that the content-assist should probably also add an entry to actually insert the #--- document separator.

@philwebb Do you have any other tricks in mind that could make editing those files easier or more convenient? If so, please let us know.

@kdvolder I disagree with you on this. The properties file is still a valid properties file. We should adapt our duplicate keys validation (which is specific to our tooling) to deal with this new multi-document semantics. And since this is a backwards compatible change for Boot users, I would not introduce a new file type/extension for this.

(@kdvolder In addition to that I would strongly prefer not to assess new, updated or changed features of any project here as bad, wrong, or anything like that.)

[kdvolder]

We should adapt our duplicate keys validation

I think it comes from Eclipse.

[kdvolder]

I think it comes from Eclipse

Actually, quickly checked this and no it doesn't. The error comes from our language server. That's good news as we can fix it relatively easily.

(@kdvolder In addition to that I would strongly prefer not to assess new, updated or changed features of any project here as bad, wrong, or anything like that.)

I would strongly prefer to be able to speak my mind and say what I think in an open discussion. I have no problems with it if other people disagree with me, especially if they can present good technical reasons for their own opinions. If my suggestions for considering changing the extension are rejected for whatever reason, that is fine.

However I do not think we should just avoid discussion about such matters altogether for fear of hurting each others feelings.

I still do not think it is a good idea to redefine an existing format and I have good reasons for this which I have stated and which I stand by. Anybody should feel free to disagree and present your own arguments. In doing so, they can rest assured, this will not offend me, as I think they in turn should also not feel offended by my own position.

Even an argument like 'we already made this decision a while ago and it is now too late to change it, so we do not wish to discuss it further'... is fine by me.

What is not fine by me is telling people they should simply keep their opinions to themselves and not raise potentially valid concerns/questions beforehand.

Anyhow, since the error about duplicate keys is under our control (at least in STS without other plugins installed that may validate the file as well) it isn't terribly difficult for us to turn this warning of. And that is good news.

[philwebb]

I think it is a bad idea to redefine the existing format. A cleaner solution would be to choose a different extension for the new format (e.g. .mproperties or whatever)

We did think about this, but it unfortunately comes with a number of downsides. We need to check quite a few locations for application.[yml|properties] files and didn't want to add another extension that would cause more locations to be checked and increase the startup time. We also wanted users to be able to quickly take an existing application.properties files and add new documents to it.

We landed on using #--- as the separator so that we could continue to use .properties files. For simple property file parsers the #--- is just treated as a comment and things work as before. We didn't realize that it would trip up the more advanced editors that have duplicate key detection.

As far as I can see, the YAML editing support deals with this multi-document files already and looks good so far. If you experience any issues within YAML files, please let us know.

I've not seen any problems with the YAML editor. The YAML spec has always supported multiple documents so you probably already deal with them.

Do you have any other tricks in mind that could make editing those files easier or more convenient? If so, please let us know.

Yeah, there's also support for importing configs using spring.config.import. Out-of-the-box you can reference other files and there's a pluggable system that allows any kind of config data to be imported (Spring Cloud will use it for consul, zookeeper etc). It might be possible to have content-assist to help you pick directories or files.

It might also be nice to have something in the outline view that shows the documents and allows you to jump to them quickly. There's some spring.config.active.on-.... properties that are used to determine when a document is used. Those could be helpful for people as well. I'm not sure who they could be presented. Perhaps in the gutter?

I would strongly prefer to be able to speak my mind and say what I think in an open discussion. I have no problems with it if other people disagree with me, especially if they can present good technical reasons for their own opinions

@kdvolder I appreciate the feedback and wasn't bothered by it. We hoped when doing the design that by using the # prefix we'd get away it since it's still a valid .properties file. All things considered, my current opinion is that the pros of using the same format outweigh the cons of having a different extension (but I admit that tooling does complicate things somewhat).

[martinlippert]

I would strongly prefer to be able to speak my mind and say what I think in an open discussion. I have no problems with it if other people disagree with me, especially if they can present good technical reasons for their own opinions. If my suggestions for considering changing the extension are rejected for whatever reason, that is fine.
However I do not think we should just avoid discussion about such matters altogether for fear of hurting each others feelings.

I am not talking about whether we should discuss things or not. I am purely talking about "how" we discuss things. If you have questions about that @kdvolder, we should talk about that offline.

[kdvolder]

I've not seen any problems with the YAML editor. The YAML spec has always supported multiple documents so you probably already deal with them.

Correct.

Yeah, there's also support for importing configs using spring.config.import. ...

Sounds interesting. I have to admit I didn't fully understand what you are suggesting / have in mind. But it sounds like if we discussed a bit... we could come up with something interesting to implement :-). It would be great if you create a ticket for these kinds of enhancements.

Also thanks Phil for explaining some of the reasoning behind the decision. I understand it is a tradeof, naturally I was more concerned about the tooling impact and not as in tune with other considerations. All in all I think the tooling impact won't be as bad. So I agree you probably made the right decision. My initial misgivings were overblown since I verified already that neither vanilla Eclipse nor vscode produces warnings about duplicate keys. So it's just coming from STS and therefore it is under our control.

Step 1 would be a simple fix to just disable those warnings when we detect the document marker is present. That would be a trivial fix. But it wouldn't be ideal.

Step 2 would be to make the editor truly aware of it's multi-document stucture (i.e as in the yaml editor). This is likely a bit more involved. I haven't a good idea yet how much work it would/will be and how important it is, but it should be feasible.

[philwebb]

It would be great if you create a ticket for these kinds of enhancements

Done in #536

[kdvolder]

@philwebb A question and possibly a suggestion...

How tolerant should we be of variations in the document marker that may look the same or similar to a user? E.g. should we tolerate spaces before/after the marker on the same line?

From the looks of the code you linked it doesn't seem to even tolerate extra spaces after the marker

https://github.com/spring-projects/spring-boot/blob/4e5161d5dcaa2f362a774a24774c94aeb3df4421/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedPropertiesLoader.java#L157

I think this might be confusing to users, since depending on how the editor displays whitespace... it might not be even possible to see the difference at all.

Anyhow... so the question is... what should STS do? Should STS recognize the marker only when it has no leading / trailing spaces?

And the suggestion is... if spring boot doesn't already tolerate trainling spaces, then perhaps it might be a good idea to tolerate them.

WDYT?

[philwebb]

The leading whitespace should be noticeable, but I agree that trailing whitespace will be hard to spot. I've opened spring-projects/spring-boot#23399 to see if we can address that on our side.

[kdvolder]

Okay great! Thanks for the quick response. So I will assume that trailing spaces are to be tolerated but leading ones are not.

[philwebb]

Yeah, I think that's safest. The leading ones should be easier to notice.

[spring-projects-issues]

(comment in Pivotal Tracker added by Nieraj Singh:)

Multi-doc support in properties file seems to work. See attached screenshot