quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

Recent `LocalesBuildTimeConfig.defaultLocale` changes break API #44312

Open zakkak opened 1 week ago

zakkak commented 1 week ago

Describe the bug

The locales adaptation in https://github.com/quarkusio/quarkus/pull/43448 in addition to the breaking default behavior change also breaks code depending on LocalesBuildTimeConfig.defaultLocale in two ways:

  1. The type of the field has changed from Locale to Optional<Locale>
  2. Although in Quarkus we default to the system's default locale when defaultLocale is not set (as described in the defaultValueDocumentation), there is no way for 3rd party users to get that default value without doing localesBuildTimeConfig.defaultLocale.orElse(Locale.getDefault()) themselves.

Issue first reported in https://quarkusio.zulipchat.com/#narrow/channel/187030-users/topic/Upgrading.20Quarkus.20from.203.2E16.2E1.20to.20999-SNAPSHOT.20breaks.20Locale

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

https://github.com/quarkusio/quarkus/commit/c814b75e2aac1fa33c38619fbf3fab32bb178eb1

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

Unfortunately I can't figure a way to implement the behavior described in https://github.com/quarkusio/quarkus/pull/43448#issue-2543016511 without having defaultLocale be an Optional, or initialized to null or the ROOT locale (which would fix issue 1, but not 2).

The options I see are:

  1. We allow Quarkus native images to be built with the build systems default locale if defaultLocale is not explicit set (we essentially revert to the current as in 3.16 behavior)
  2. Extend the migration guide to let people know they should use localesBuildTimeConfig.defaultLocale.orElse(Locale.getDefault()) when accessing defaultLocale. Or even better don't expose defaultLocale itself and expose a getter instead.

Ideas are very much welcome.

quarkus-bot[bot] commented 1 week ago

You added a link to a Zulip discussion, please make sure the description of the issue is comprehensive and doesn't require accessing Zulip

This message is automatically generated by a bot.

quarkus-bot[bot] commented 1 week ago

/cc @radcortez (config)

zakkak commented 1 week ago

cc @gsmet @maxandersen @cescoffier @geoand

gsmet commented 1 week ago

My personal opinion is that we need to have something on top of config to propagate what we want to propagate.

Config shouldn't be considered API from a developer POV: it's an API from the configuration file perspective.

We have the same issue with the HTTP config, I know @radcortez has a PR around and we should probably start to make progress on a pattern we can reuse in different parts of the code.

geoand commented 1 week ago

Config shouldn't be considered API from a developer POV: it's an API from the configuration file perspective.

Right, we've been bumping up against this more and more lately and we should make it clear

maxandersen commented 1 week ago

So you say if we could go back in time we would have another layer of accessing the resultant config ?

Or that when we see there will be a problem we should have introduced an api - lets call it "LocaleDefaults" which would have been introduced and mark the config api deprecated (or some other way point them to "LocaleDefaults") and then have that in some releases before breaking the before config api?

otherwise all config would have to be considered non public api for extension writers (and users?)

radcortez commented 1 week ago

It probably is a non-issue for 99% of the configuration, but there are a few cases in which users/extensions shouldn't rely on the config, especially cases where the configuration may not represent the final value, such as a random HTTP port. This requires us to mutate the configuration, which causes a couple of other issues.

Ideally, we should offer APIs for things that we know that the provided configuration value is not the value that we end up using.