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

How to configure empty string #671

Open
tomas-langer opened this issue Jan 21, 2021 · 11 comments
Open

How to configure empty string #671

tomas-langer opened this issue Jan 21, 2021 · 11 comments

Comments

@tomas-langer
Copy link

Description

I need to be able to configure an empty string as a configuration value.

Two use cases I have encountered so far:

  1. Database password - in tests and development environments, it is quite common to use an empty password (not null, empty)
  2. Sometimes I want to inject a configuration property and have it resolve into an empty string if not defined - example below
    @Inject
    @ConfigProperty(name = "server.static.classpath.context", defaultValue = "")
    private String context;

I am only interested in a value if defined in configuration, otherwise I need an empty string. This used to work in previous version, now it fails validation.

What is the correct way to achieve this?

@Emily-Jiang
Copy link
Member

The way to handle this is to use programmatic API: config.getOptionalValue() and then supply empty value with an empty string.

@Emily-Jiang
Copy link
Member

Full empty conversation can be found here

@tomas-langer
Copy link
Author

The problem is that an empty string differs in meaning from Optional.empty(). These are not the same thing semantically, so I cannot do it the way you say

@m0mus
Copy link
Member

m0mus commented Jan 21, 2021

The use case provided by @tomas-langer looks legit. Using programmatic API is this case is a complicated workaround.
Can someone explain in favor of what the backwards compatibility was broken? What's the advantage of new approach? What are the use cases?

@Emily-Jiang
Copy link
Member

The use case provided by @tomas-langer looks legit. Using programmatic API is this case is a complicated workaround.
Can someone explain in favor of what the backwards compatibility was broken? What's the advantage of new approach? What are the use cases?

@dmlloyd @radcortez can you explain in more details? In the meanwhile this issue documents some use case.

@radcortez
Copy link
Contributor

The main motivation to move with these changes was to have a way to empty configuration values on higher ordinal sources and to have consistency between all types: #531 and #532.

Using Optional might be an option. Also, because this is done at the Converter level, a new Converter can be added with higher priority that just returns the value.

Finally, probably the easiest way is to use the new ConfigValue API. Because ConfigValue does not apply any conversions and gives you the raw lookup value, you will get the empty String.

Does this help?

@tomas-langer
Copy link
Author

No, not really @radcortez

I had the following:

@ConfigProperty
String password;

The password has three possible values - it does not exist (e.g. Optional.empty if I decided to use Optional), its value is an empty string, or its value is an actual non-zero-length string.

All of these options have an actual meaning

  • null means there is no password - and when connecting to external systems, such as database, I do not configure the password at all
  • empty means the password is empty - this is a valid password value and can be send to backend
  • non-empty - same thing, just send it

What you want me to do is to either

  • merge the first two options - and how am I to guess which one is right?
  • or use ConfigValue - which in my implementation returns null, not empty string - and obviously this is either undefined, or there is a TCK test missing; event if it returned empty string, I would still need to do a lot of logic to get there

So I added an expression ${EMPTY} to my implementation to explicitly specify empty string, but that is not portable...

@radcortez
Copy link
Contributor

Hey @tomas-langer just to pick from our conversation this morning:

In SR Config, I've implemented ConfigValue#getValue to return the actual value lookup (without any additional rules). Probably a TCK is missing for that, but it was not intended to return null. In the javadocs we did specify that way:

/**
* The value of the property lookup with transformations (expanded, etc).
*
* @return the value of the property lookup or {@code null} if the property could not be found
*/
String getValue();
.

Anyway, this is separate from the entire empty / clear value discussion.

@gkwan-ibm
Copy link

I set

 @Inject
 @ConfigProperty(name = "system.context.root", defaultValue = "")
 String SYSTEM_CONTEXT_ROOT;

When I start dev mode, I get

[INFO] [ERROR ] CWWKZ0004E: An exception occurred while starting the application guide-arquillian-managed. The exception message was: com.ibm.ws.container.service.state.StateChangeException: org.jboss.weld.exceptions.DeploymentException: SRCFG02001: Failed to Inject @ConfigProperty for key system.context.root into io.openliberty.guides.inventory.client.SystemClient.SYSTEM_CONTEXT_ROOT SRCFG00040: The config property system.context.root is defined as the empty String ("") which the following Converter considered to be null: io.smallrye.config.Converters$BuiltInConverter

@keanerickson-mc
Copy link

Migrating from microprofie 2 to microprofile 5, I began receiving the "the config property ____ is defined as the empty string" error. I was under the impression that using a ConfigValue instead of a String, and then getting its value manually, would prevent this error when the property is not provided. I have not found a way to have a blank default without errors on startup.

@Emily-Jiang
Copy link
Member

Empty value is not permitted unfortunately. See the lengthy conversation here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants