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

Empty property in application.properties throw exception by injecting with @ConfigProperty eaven with defaultValue #2765

Closed
waldimiro opened this issue Jun 9, 2019 · 12 comments
Labels
area/config kind/bug Something isn't working

Comments

@waldimiro
Copy link

  1. Add an empty property in application.properties
    my.empty.property=

  2. Inject property
    @Inject @ConfigProperty(name = "my.empty.property") protected String myProperty = null;

  3. Exception
    javax.enterprise.inject.spi.DeploymentException: No config value of type [java.lang.String] exists for: my.empty.property

@waldimiro waldimiro added the kind/bug Something isn't working label Jun 9, 2019
@mb010865
Copy link

mb010865 commented Jun 9, 2019

I think this is and should be intended. If you want a possible empty Property you can make myProperty an Optional

@geoand
Copy link
Contributor

geoand commented Jun 9, 2019

I personally think that is should be allowed (and opened #2766) for it. Lets see what others think.

geoand added a commit to geoand/quarkus that referenced this issue Jun 9, 2019
geoand added a commit to geoand/quarkus that referenced this issue Jun 10, 2019
geoand added a commit to geoand/quarkus that referenced this issue Jun 13, 2019
@gsmet
Copy link
Member

gsmet commented Nov 13, 2019

@dmlloyd what's the status of the discussion in MP Config regarding this one?

@dmlloyd
Copy link
Member

dmlloyd commented Nov 14, 2019

Ongoing. I would say that 50%+ of the weekly meetings have been cancelled in the past few months so progress has been slow.

What I'm thinking is that the policy should be that primitives (other than char) would default to 0/false but would still be required like we do for @ConfigItem. Objects should not and would be considered required unless they're explicitly Optional. That means the case in this issue would never be supported without usage of Optional. Otherwise the property would always be initialized to an empty string even if the value was never specified, which makes it impossible to cause the property to be required.

@rsvoboda
Copy link
Member

java.util.NoSuchElementException: Required property my.empty.property not found is a bit confusing message.

Can SRye or Quarkus detect that the property is present, but empty ?

@dmlloyd
Copy link
Member

dmlloyd commented Jan 26, 2020

There is no functional difference between empty and missing-without-default. Perhaps a better message would be "Value not present for property x".

@rsvoboda
Copy link
Member

Perhaps a better message would be "Value not present for property x".

+1

@cpmoore
Copy link
Contributor

cpmoore commented Mar 5, 2020

Personally I think empty values should be allowed as it may be an intended value.

@dmlloyd
Copy link
Member

dmlloyd commented Mar 6, 2020

If you want to allow empty values, use Optional, which works for all types (not just strings or lists).

@pdolezal
Copy link

pdolezal commented Feb 5, 2021

I personally hate this behavior. There is IMO a big difference between an absent property and a string property with an empty string as its value. Optional is supposed to indicate possibly absent value, i.e., something which would be null otherwise. An empty string is not absent, it is just an empty string. What frustrated me even more was the moment when I specified @ConfigProperty(name = "foo", defaultValue = "") with no success.

Besides… although I understand that using Optional as the silver bullet here is appealing, I don't like it for other reasons as well. I think that Mark Stuart explained that quite clearly: Optional is intended for return types, using it elsewhere smells.

@dmlloyd
Copy link
Member

dmlloyd commented Feb 5, 2021

It's the best we can do within the limits of the Microprofile-based configuration framework. Within that framework, an empty value is used to delete a lower-priority configuration value. It's a tradeoff: either there's no good way to represent a present-but-empty string (like today), or there's no good way to explicitly delete a value. The former problem mostly affects extension developers and configuration consumers, whereas the latter problem mostly affects people writing and reading configurations. In the end, the latter group was considered to be more important.

That said, in retrospect, I think we should not have based our configuration system on Microprofile Config. That was a decision I made, but in hindsight I think it was a bad one. Had we based configuration on a more capable structured format, we probably wouldn't have had to make this particular tradeoff at all, while likely still being able to support users of Microprofile Config at an API level.

In the future maybe we'll get a chance to revisit this design.

@sven-geisenhainer
Copy link

Using Optional along with @ConfigProperty in a JAX-RS provider gives a warning:

Directly injecting a @ConfigProperty into a JAX-RS provider may lead to unexpected results. To ensure proper results, please change the type of the field to javax.enterprise.inject.Instance<java.util.Optional<java.lang.String>>.

But Instance<Optional<String>> is not an option as it would throw the DeploymentException again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants