Navigation Menu

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

Document that the Map backing a MapPropertySource should not contain null values #25142

Closed
philwebb opened this issue May 27, 2020 · 8 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: documentation A documentation task
Milestone

Comments

@philwebb
Copy link
Member

philwebb commented May 27, 2020

See spring-projects/spring-boot#21542 for background.

Currently a PropertySource can legitimately contain null values. For example, a MapPropertySource can return true from containsProperty and null from getProperty if the backing Map has an entry with a null value.

The PropertySourcesPropertyResolver class currently treats null results as missing properties. This makes if impossible for a higher PropertySource to override a lower one with a null value.

The resolver could call containsProperty to check for a value and allow null results, however, this has some downsides:

  • It's less efficient since two calls need to be made for each matching value
  • It potentially breaks back-compatibility

I've raised this issue to discuss if the design is intentional or not.

@jhoeller
Copy link
Contributor

jhoeller commented May 27, 2020

This is intentional as far as I see, with PropertySource.getProperty documented as "Return the value associated with the given name, or null if not found". From that perspective, potential misbehavior is rather on MapPropertySource not handling containsProperty correctly.

@jhoeller
Copy link
Contributor

On review, I'd rather let MapPropertySource perform its efficient containsProperty call, just documenting that the given Map must not contain null values (either by using a corresponding implementation or by simply not populating it with null values) in order to get consistent getProperty vs containsProperty behavior.

@jhoeller
Copy link
Contributor

Reading through the Boot issue, maybe affected PropertySource implementations could explicitly turn user-declared null values into empty Strings?

@rgordeev
Copy link

rgordeev commented May 27, 2020

@jhoeller
But the problem concerns not only null values for strings, but empty values for lists or maps - it's impossible to override values from application.yaml with SPRING_APPLICATION_JSON, that contains empty values or nulls.
E.g.

application:
  a: some string
  b:
    - 1
    - 2

If I try to override b property with

SPRING_APPLICATION_JSON={"application": {"b": null}}

or

SPRING_APPLICATION_JSON={"application": {"b": {}}}

or

SPRING_APPLICATION_JSON={"application": {"b": []}}

nothing happens, application will get

application:
  a: some string
  b:
    - 1
    - 2

in every mentioned case.

@philwebb
Copy link
Member Author

@rgordeev I missed those from the original report. I've reopened the Spring Boot issue.

@philwebb
Copy link
Member Author

philwebb commented May 27, 2020

Thanks @jhoeller, I'll reopen the Boot issue and update this one as documentation for MapPropertySource.

@philwebb philwebb reopened this May 27, 2020
@philwebb philwebb changed the title PropertySourcesPropertyResolver.getProperty never considers null results. Document that the Map backing a MapPropertySource should not contain null values May 27, 2020
@sbrannen sbrannen added type: documentation A documentation task in: core Issues in core modules (aop, beans, core, context, expression) labels May 27, 2020
@jhoeller jhoeller removed the status: waiting-for-triage An issue we've not yet triaged or decided on label May 27, 2020
@jhoeller jhoeller added this to the 5.2.7 milestone May 27, 2020
@rgordeev
Copy link

Sometimes results of overriding are not obvious at all. You may see test cases Tests

Test case Defaults

application-test.yaml
application:
  a: "some string"
  b:
    - 1
    - 2
    - 3
  c:
    - "one"
    - "two"
    - "three"
  m:
    - one: 1
    - two: 2
Expecting                    Actual
 a == ""                     a == ""
 b == [1,2,3]                b == [1,2,3]
 c == ["one","two","three"]  c == ["one","two","three"]
 m == {"one":1, "two":2}     m == {"one":1, "two":2}

Test case With Empty Lists

application-test.yaml
application:
  a: "some string"
  b:
    - 1
    - 2
    - 3
  c:
    - "one"
    - "two"
    - "three"
  m:
    - one: 1
    - two: 2

We are trying to override

  • string property with null,
  • list properties with empty json array [],
  • map with empty json object {}
SPRING_APPLICATION_JSON = {
    "application": {
        "a": null,
        "b": [],
        "c": [],
        "m": {}
    }
}
Expecting                            Actual
 a == null                           a == "some string"
 b == null | empty array list        b == [1,2,3]
 c == null | empty array list        c == ["one","two","three"]
 m == null | empty map               m == {"one":1, "two":2}

Test case With Empty Objects

application-test.yaml
application:
  a: "some string"
  b:
    - 1
    - 2
    - 3
  c:
    - "one"
    - "two"
    - "three"
  m:
    - one: 1
    - two: 2

We are trying to override

  • string property with null,
  • list properties with empty object {},
  • map with empty json object {}
SPRING_APPLICATION_JSON = {
    "application": {
        "a": null,
        "b": {},
        "c": {},
        "m": {}
    }
}
Expecting                            Actual
 a == null                           a == "some string"
 b == null | empty array list        b == [1,2,3]
 c == null | empty array list        c == ["one","two","three"]
 m == null | empty map               m == {"one":1, "two":2}

Test case With Empty Strings

application-test.yaml
application:
  a: "some string"
  b:
    - 1
    - 2
    - 3
  c:
    - "one"
    - "two"
    - "three"
  m:
    - one: 1
    - two: 2

We are trying to override

  • string property with empty string "",
  • list properties with empty string "",
  • map properties with empty string ""
SPRING_APPLICATION_JSON = {
    "application": {
        "a": "",
        "b": "",
        "c": "",
        "m": ""
    }
}
Expecting                            Actual
 a == ""                             a == ""
 b == null | empty array list        b == []
 c == null | empty array list        c == []
 m == null | empty map               m == {}

Test case With Non null Values

application-test.yaml
application:
  a: "some string"
  b:
    - 1
    - 2
    - 3
  c:
    - "one"
    - "two"
    - "three"
  m:
    - one: 1
    - two: 2

We are trying to override

  • string property with empty string "",
  • list properties with non empty json arrays [9, 10] and ["ten", "nine"],
  • map properties with non empty json object {"nine": 9}
SPRING_APPLICATION_JSON = {
    "application": {
        "a": "",
        "b": [9,10],
        "c": ["ten", "nine"],
        "m": {"nine": 9}
    }
}
Expecting                    Actual
 a == ""                     a == ""
 b == [9,10]                 b == [9,10]
 c == ["ten","nine"]         c == ["ten","nine"]
 m == {"nine": 9}            m == {"one":1, "two":2, "nine":9}

Test case With Null values

application-test.yaml
application:
  a: "some string"
  b:
    - 1
    - 2
    - 3
  c:
    - "one"
    - "two"
    - "three"
  m:
    - one: 1
    - two: 2

We are trying to override

  • string property with null,
  • list properties with null,
  • map properties with null
SPRING_APPLICATION_JSON = {
    "application": {
        "a": null,
        "b": null,
        "c": null,
        "m": null
    }
}
Expecting                    Actual
 a == ""                     a == "some string"
 b == null | empty           b == [1,2,3]
 c == null | empty           c == ["one","two","three"]
 m == null | empty           m == {"one":1, "two":2}

@rgordeev
Copy link

E.g.

  • you may override every non empty property with empty string "" regardless the type of properties (ordinal or collection)
  • if you try to override map property with non empty values it adds value to existing ones, not overrides them
  • if you try to override property with null or [] or {} nothing happens

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: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

5 participants