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

Need a way to specify empty list/array in microprofile-config.properties #397

Closed
njr-11 opened this issue Jan 16, 2019 · 11 comments · Fixed by #598
Closed

Need a way to specify empty list/array in microprofile-config.properties #397

njr-11 opened this issue Jan 16, 2019 · 11 comments · Fixed by #598
Labels
use case 💡 An issue which illustrates a desired use case for the specification

Comments

@njr-11
Copy link

njr-11 commented Jan 16, 2019

The MicroProfile Config spec currently specifies that Array Converters are built in, and provides examples of how to configure values that are multiple array elements (delimit by ,). However, it does not appear to document how to configure a value that is an empty array.

On one implementation, I tried out specifying an empty value, like this,

test.property.name=

but it ended up with a value that is an array of size 1 containing the empty string.

I also tried out specifying the value as the value of the org.eclipse.microprofile.config.inject.ConfigProperty.UNCONFIGURED_VALUE constant (lacks documentation, so not sure if this is proper usage. Also, it seems to have two different values depending on which version of the ConfigProperty class is used),

test.property.name.1=org.eclipse.microprofile.config.configproperty.unconfigureddvalue
test.property.name.2=org.eclipse.microprofile.config.configproperty.unconfiguredvalue

but this resulted in a size 1 array containing the specified constant value.

It's likely the above behavior is to be expected, but the requirement to be able to specify an empty list also seems valid. In another MicroProfile spec, we are utilizing MicroProfile Config to allow the user to supply configuration property values, some of which are lists/arrays. It ought to be possible for MicroProfile Config to have a standard way to set these list/array values to empty.

@Emily-Jiang
Copy link
Member

test.property.name= is the way to specify an empty string.
test.property.name.1=org.eclipse.microprofile.config.configproperty.unconfigureddvalue
test.property.name.2=org.eclipse.microprofile.config.configproperty.unconfiguredvalue
The above 2 lines are to specify two strings.

@hutchig
Copy link
Member

hutchig commented Jan 17, 2019

An Optional which stores a Collection which it indicates is not null but that has no elements. Using converters.

@njr-11
Copy link
Author

njr-11 commented Jan 17, 2019

@hutchig I understand that a component can write its own Converter to interpret a value as an empty array/list/collection, although that doesn't really address the problem of how the end user should expect to disambiguate between String[] { "" } and String[] {} in microprofile-config.properties. I was expecting that MicroProfile Config would standardize some sort of syntax for this. It looks like you have an escape character, \, which can be used when list values include commas. Could the escape character be used somehow to represent that an empty list is wanted? Maybe \0 to indicate list of size 0 ?

@njr-11
Copy link
Author

njr-11 commented Jan 17, 2019

Alternatively, the MicroProfile spec that I'm working on could document that the way for users to specify empty array is to specify empty string in MP Config. If that's the solution, I'm fine with it. I just want to first be sure that I'm truly following the guidance of MP Config rather than going off on my own and defining something non-standard that will lack consistency with other MicroProfile specifications.

@OndroMih
Copy link
Contributor

OndroMih commented Jan 18, 2019

It's true that the spec doesn't say how an empty value is converted to an array. It should be clarified and also assured by a test.

I suggest that an empty value is converted to an empty array, i.e. an array with 0 elements. Additionally, we can also specify an edge case for an array with a single empty string which would be created if the value is just the escape character ( \).

This should be in line with how other converters work. A string converter represents an empty value as an empty string. An array then should represent it as an empty array.

For example:
test.property.name= is converted to an empty string ""
test.property.name= is converted to an empty array {}
test.property.name=\ is converted to an array with a single empty string element {""}
test.property.name=\\ is converted to an array with a single backslash string element {"\"}
test.property.name=, is converted to an array with two empty string elements {"", ""}

We could also do it the other way around and specify that an empty value results in an array with a single empty string element, while to specify an empty array people would have to use \. This would be useful if the configuration is generated programmatically. But for editing manually, I think that interpreting an empty value as an empty array makes much more sense to real people.

@OndroMih
Copy link
Contributor

By the way, I think that the spec is wrong in the example:

myPets=dog,cat,dog\\,cat should end up in {"dog", "cat", "dog\", "cat"}
myPets=dog,cat,dog\,cat should end up in {"dog", "cat", "dog,cat"}

@jmesnil
Copy link
Contributor

jmesnil commented Jan 18, 2019

@OndroMih iirc you can't write myPets=dog,cat,dog\,cat as \, is not a valid Java character so you have to double the \ to be able to read it properly

@jmesnil
Copy link
Contributor

jmesnil commented Feb 15, 2019

I've opened #407 to specify how the Config API handles config property with empty value as well as a TCK test to verify these assertions.

Tl;DR: a config property with an empty value results in an empty "thing" (e.g. an empty string or an empty collection)

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Feb 25, 2019

By the way, I think that the spec is wrong in the example:

myPets=dog,cat,dog\\,cat should end up in {"dog", "cat", "dog\", "cat"}
myPets=dog,cat,dog\,cat should end up in {"dog", "cat", "dog,cat"}

@OndroMih Can you elaborate a bit more on why you think it is wrong.

@Emily-Jiang
Copy link
Member

We need to discuss this in this week's call to agree on the behavior. Please make sure to attend the call @jmesnil @OndroMih !

@dmlloyd
Copy link
Contributor

dmlloyd commented Mar 6, 2020

Related to #531.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
use case 💡 An issue which illustrates a desired use case for the specification
Projects
None yet
6 participants