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

Value from Java enum are not correct #198

Closed
angelozerr opened this issue Jan 23, 2020 · 3 comments · Fixed by #201
Closed

Value from Java enum are not correct #198

angelozerr opened this issue Jan 23, 2020 · 3 comments · Fixed by #201
Assignees
Labels
bug Something isn't working properties Features supported in application.properties
Milestone

Comments

@angelozerr
Copy link
Contributor

angelozerr commented Jan 23, 2020

Today value from Java enum are used in the application.properties file

For instance we have:

quarkus.log.console.async.overflow = BLOCK
where BLOCK is the Java enum name.

But we should have (see discussion at https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/What.20is.20the.20case.20sensitive.20rule.20for.20value.20property.20with.20enum)

quarkus.log.console.async.overflow = block
We must study the rule of convert of Java enum value (see https://quarkus.io/guides/writing-extensions#enhanced-conversion)

It seems that BLOCK and block are correct for quarkus.log.console.async.overflow

We have the same problem with quarkus.datasource.transaction-isolation-level. Our allowed value are for instance REPEATABLE_READ although documentation
See https://quarkus.io/guides/all-config#quarkus-agroal_quarkus.datasource.transaction-isolation-level says repeatable-read

@angelozerr angelozerr added bug Something isn't working properties Features supported in application.properties labels Jan 23, 2020
@fbricon
Copy link
Collaborator

fbricon commented Jan 23, 2020

Same problem with quarkus.hibernate-search.elasticsearch.index-defaults.lifecycle.required-status
Proposed values are GREEN, YELLOW, RED instead of green, yellow, red.

@fbricon fbricon added this to the v0.0.5 milestone Jan 23, 2020
@angelozerr
Copy link
Contributor Author

angelozerr commented Jan 23, 2020

After reading Quarkus documentation https://quarkus.io/guides/writing-extensions#enhanced-conversion the Quarkus Java enumeration use hyphenate converter by default:

Java enum Canonical equivalent
DISCARD discard
READ_UNCOMMITTED read-uncommitted

But this behavior can be changed if you set a converter with @ConvertWith annotation. It can be complex to manage this usecase.

IMHO I think we should manage the default Quarkus behavior, in other words, I think we should convert enum values with hyphenate (like we have done for property name according the @configitem value). We could do this hyphenate conversion only for Java enum which is used by Quarkus properties.

@fbricon have you agree with that?

After the question is what about MicroProfile properties? For the moment I have not seen use-case with MicroProfile and Java enum.

@jclingan what do you think if we convert Java enum with hyphenate rule for MicroProfile properties?

Perhaps you don't like this idea, so perhaps the solution is to check if the project is a Quarkus project (in this case hyphenate rule is used), otherwise if it's a just a MicroProfile project without Quarkus, there is no conversion to do for enum.

@jclingan @fbricon what do you think about those ideas?

@angelozerr angelozerr self-assigned this Jan 24, 2020
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 24, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
@angelozerr
Copy link
Contributor Author

After playing with Java enum with Quarkus and MicroProfile, here the conclusion (for default behaviour):

  • MicroProfile with (ConfigProperty annotation) use the real Java enumeration name.
  • Quarkus (with ConfigIRoot annotation) uses kebab-case (hyphenate with '-'), but accept too real java enumeration name.

It means that this kebab-case must be done on MicroProfile LS side and not on JDT LS side because a given Java enumeration can be used on MicroProfile ConfigProperty and Quarkus ConfigRoot both.

angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 27, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 27, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 27, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 27, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 27, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 27, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 27, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 28, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 28, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 28, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
angelozerr added a commit to angelozerr/quarkus-ls that referenced this issue Jan 28, 2020
Fixes redhat-developer#198

Signed-off-by: azerr <azerr@redhat.com>
fbricon pushed a commit that referenced this issue Jan 28, 2020
Fixes #198

Signed-off-by: azerr <azerr@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working properties Features supported in application.properties
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants