Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to map null value from yaml [SPR-15425] #19986

Open
spring-projects-issues opened this issue Apr 10, 2017 · 14 comments
Open

Allow to map null value from yaml [SPR-15425] #19986

spring-projects-issues opened this issue Apr 10, 2017 · 14 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Apr 10, 2017

Simon Stratmann opened SPR-15425 and commented

The following issue occured to me while using Spring Boot but is caused by Spring code.

With Spring Boot I have a String property that is mapped to an application.yml by using @ConfigurationProperties. The value in the yml file is null (or ~). But the values are initialized with "" (empty String). According to YAML specification it does allow null values.

The culprit is org.springframework.beans.factory.config.YamlProcessor#buildFlattenedMap which executes result.put(key, (value != null ? value : "")).

I've extended the file and configured it to be used and this seems to work fine. But I have to take over a lot of final and private code which makes it a bit brittle. I think the class should support null values, explicitly remark that their converted to "" or make a change in behavior a bit more comfortable.

Code excerpts:

test:
    setting: ~
@ConfigurationProperties("test")
public class Test {

    String setting;

    public void setSetting(String setting) {
        this.setting = setting; //Will set "setting" to "" instead of null
    }
}

Thanks for your time.


Affects: 4.3.7

Attachments:

Issue Links:

4 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

Craig commented

#1798

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

I am not keen to support null values in the loader but I am ok to provide a hook point that allows you to do so if you want. 

We want things to be consistent in Spring Boot (and you can't set null in properties) so even if this was implemented as a default, we'd convert it back to empty string for consistency.

Can you review your PR to offer a hook point that allows you to define that strategy with an extension?

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

FWIW, I disagree with Stéphane here. Looking at things purely from the perspective of YAML processing, I think that the current behaviour of YamlProcessor is wrong. null is a first-class citizen in YAML and Java supports null values, but YamlProcessor currently coerces a null to an empty String. Given that Java is capable of representing null values, I think YamlProcessor should support them rather than the current lossy conversion.

Unfortunately, supporting null values may be easier said than done. YamlProcessor.MatchCallback and YamlProcessor.DocumentMatcher both use Properties in public API and Properties does not support null values. It looks like API changes will be necessary to fully support null values. It may be possible for those changes to be non-breaking by judicious use of default methods on the relevant interfaces but changing this probably isn't as easy as it first appears.

 

 

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Thanks Andy Wilkinson.

The purpose of my initial comment was largely targeted at the Spring Boot situation and I do agree that we should not push those considerations too restrictively. The proposed PR can't be merged as is anyway. 

Craig would you be willing to rework your PR with what has been considered here?

@spring-projects-issues
Copy link
Collaborator Author

Bruno Orsi Berton commented

I was working with this issue as well, I was trying to do the same solution as Craig and I came across the same problem of the Properties class not allowing null values, but my opinion is that a change in the code to replace the Properties class is not going to be feasible. Since this is one of my first issues in Spring, I am not familiar with the code, @Stéphane Nicoll, can you explain a little more your solution? I really wanna try to solve this issue, since I am new as a contributor.

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Hey Bruno, thanks for asking. I did ask Craig if he wanted to rework his PR but he didn't reply so I am going to decline it. I don't have any solution at the moment I am afraid, I need to look in the API and see if there is a way to do this in a backward compatible way.

@andrei-ivanov
Copy link

any chance for a fix? 😕

@andrei-ivanov
Copy link

@snicoll any suggestions on how this could be implemented?

@incodemode
Copy link

So was this dismissed or fixed? just wanting to know, in my case, the solution is simple for my particular project if(!null|!empty). but reading that spring is the new hot topic, I was wondering if this was fixed at some point

@sbrannen
Copy link
Member

@incodemode, as @wilkinsona hinted at in spring-projects/spring-boot#4877 (comment), this issue is still open and waiting for triage (i.e., waiting for further consideration/investigation).

The original PR for this issue (#1798) was declined as explained in #19986 (comment) above.

@sbrannen sbrannen added the type: enhancement A general enhancement label Feb 22, 2021
@snicoll
Copy link
Member

snicoll commented Feb 24, 2021

I think it is important to put this issue in context. While it is true that YamlProcessor is mapping null to an empty String too high in the hierarchy, we took that relaxed approach because Spring's Environment does not have the capability of storing that information at the moment anyway (null is not handled).

Fixing this bug, in the current state, would only help users that are using YamlProcessor directly. A user of, say Spring Boot, wouldn't see a single difference if we fixed it for the reason I've expressed above.

@szpak
Copy link

szpak commented May 16, 2023

We've encountered that problem trying to resolve an environment property or null if not provided (to fail the validation with the Jakarta JSR303 annotations). As SpEL is not evaluated in @ConfigurationProperties, something like ${FOO_BAR:#{null}} in YAML doesn't work.

Having an ability to pass a null value (also as a default value) in YAML would probably solve also our issue (without any other post processors).

@djchapm
Copy link

djchapm commented Jun 15, 2023

This would also be helpful if you're the client of a spring boot app and trying to override one of the properties delivered with the app using -Dspring.config.additional-location to null out just that one entry that is set with a default value when you need it to be null.

@Sroca3
Copy link

Sroca3 commented Apr 5, 2024

I'm a bit confused about the lack of ability to fix this. YamlProcessor is clearly dedicated to processing yaml files right? The implementation doesn't support parts of clearly defined yaml functionality. Whether or not that breaks additional callers of it is irrelevant, I would think. If Environment or Properties doesn't support null, then the extra data massaging should happen in a different place than the YamlProcessor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

9 participants