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

YamlPropertiesFactoryBean has empty string returned when empty array value is present in the YAML document #24879

Closed
dhivyaarumugam opened this issue Apr 8, 2020 · 9 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: invalid An issue that we don't feel is valid

Comments

@dhivyaarumugam
Copy link

dhivyaarumugam commented Apr 8, 2020

This is related to #21310.

I'm using the latest spring boot version 2.2.6 and latest spring cloud config version 2.2.2.

I use GitHub as source of truth for fetching my configurations from base YAML file.

Check the property cards: [""] or just cards: [] in line number 4.

Screen Shot 2020-04-08 at 1 18 57 PM

The response I get from my service for fetching the configuration is as below snapshot for a default profile API to spring cloud config server.

Please find below snapshot with response : "default.oihs.cards": ""

Screen Shot 2020-04-08 at 1 22 45 PM

Stack Overflow question: https://stackoverflow.com/questions/61096022/yamlpropertiesfactorybean-has-empty-string-returned-when-empty-array-value-is-pr

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 8, 2020
@sbrannen
Copy link
Member

sbrannen commented Apr 8, 2020

As per #21310, an empty array in a YAML file gets converted to an empty String when using YamlPropertiesFactoryBean.

That's be design, since values in a Properties object must be of type String.

As stated by @snicoll, conversion of an empty array to an empty string "corresponds to an empty comma-separated list of entries in the Properties format."

So what exactly does not work for you?

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) status: waiting-for-feedback We need additional information before we can continue labels Apr 8, 2020
@dhivyaarumugam
Copy link
Author

dhivyaarumugam commented Apr 9, 2020

@sbrannen : I'm retrieving the git yml properties, store it in cache and provide the configuration properties.
For spring cloud config version : 1.2.1.RELEASE, nothing was impacted since nothing was returned in the response for an empty array in gitHub.
Now with Latest spring cloud config version of usage, impact is there since I expect a empty array object but I get a String Object.

DataType mismatch is coming up.

Can the framework be fixed to return the object as -> cards: "[ ]" instead of cards: "" ?

For clarity to show empty String and empty Array get same o/p:

Git hub entries

Screen Shot 2020-04-09 at 5 41 44 PM

Response from spring cloud config profile API:

Screen Shot 2020-04-09 at 5 42 55 PM

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 9, 2020
@dhivyaarumugam
Copy link
Author

@sbrannen : more details on code:

e51330e - this is the commit which creates the [] -> “”
package org.springframework.beans.factory.config;
yamlProcessor Class

image

@snicoll
Copy link
Member

snicoll commented Apr 16, 2020

Can the framework be fixed to return the object as -> cards: "[ ]" instead of cards: "" ?

No. The properties format has no notion of an array.

DataType mismatch is coming up.

If you have a small sample (that does not involve spring cloud config server) that shows how a data type mismatch is thrown, we can have a look to your use case. Please share that as a small project we can run ourselves (a zip or a link to a github repo).

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Apr 16, 2020
@dhivyaarumugam
Copy link
Author

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 20, 2020
@dhivyaarumugam
Copy link
Author

@snicoll : A PR i have raised from my fork : dhivyaarumugam@3536a21

@snicoll
Copy link
Member

snicoll commented Apr 20, 2020

Thanks for the sample. As I've indicated previously, the properties format has no notion of an array so you can't expect that to be available in any shape or form in the returned Map.

Your sample is just raw checking the Map and it doesn't demonstrate the DataType mismatch you've described (all values are of type String). If you need to bind this to an Array, then whatever binder you have should account for the fact that an empty String bound to an Array leads to an empty array. The Spring Boot binder does that for you so perhaps you should be using that?

I've added this:

@Component
@ConfigurationProperties(prefix = "test")
public class TestProperties {

	private String[] emptyArray;

	public String[] getEmptyArray() {
		return this.emptyArray;
	}

	public void setEmptyArray(String[] emptyArray) {
		this.emptyArray = emptyArray;
	}
	
}

and the following config:

test:
  emptyArray: []

and when I inject TestProperties, getEmptyArray() returns an empty array. I am going to close this now as the sample works as designed.

@snicoll snicoll closed this as completed Apr 20, 2020
@snicoll snicoll added status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 20, 2020
@dhivyaarumugam
Copy link
Author

dhivyaarumugam commented Apr 20, 2020

@snicoll : In Spring cloud config, when propertySource is created it uses the map from yaml processor directly and doesn't have spring Binder involved in it.

Yes Spring binder can help in case of a POJO not in our case.

@snicoll
Copy link
Member

snicoll commented Apr 20, 2020

In Spring cloud config, when propertySource is created it uses the map from yaml processor directly

That makes total sense since the goal is to output a "properties view" of the configuration. There's nothing Spring Cloud Config should do about that.

Yes Spring binder can help in case of a POJO not in our case.

You haven't shared what your case is in your sample so I can't say anything about that. I think this issue has run its course in the core framework though and I hope the rationale I've shared explains why we're not going to change this.

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) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

4 participants