vaadin / quarkus

An extension to Quarkus to support Vaadin Flow
Apache License 2.0
28 stars 3 forks source link

Vaadin i18n custom provider is not recognized (unless...) #145

Closed YassinHajaj closed 5 months ago

YassinHajaj commented 5 months ago

My code is straightforward, can be found here as a full reproducer.

Here is what my provider looks like and it is not found unless I listen to a startup event to force its initialization (see method dumbWorkAround)

@VaadinServiceEnabled
public class I18n implements I18NProvider {

    private static final Locale DUTCH = forLanguageTag("nl");

    private static final Logger LOGGER = Logger.getLogger(I18n.class.getName());

    @Override
    public List<Locale> getProvidedLocales() {
        return List.of(FRANCE, ENGLISH, DUTCH);
    }

    @Override
    public String getTranslation(String key, Locale locale, Object... params) {
        try {
            var bundle = ResourceBundle.getBundle("vaadin-i18n/translations", locale);
            return bundle.getString(key);
        } catch (MissingResourceException missingResourceException) {
            LOGGER.warning("Missing resource bundle for locale " + locale + ". Falling back to English.");
            var bundle = ResourceBundle.getBundle("vaadin-i18n/translations", ENGLISH);
            return bundle.getString(key);
        }
    }

    // comment this method out to reproduce the issue
    public void dumbWorkAround(@Observes StartupEvent startupEvent) {
        // ugly way to force the initialization of this bean
    }
}

From what I've seen, even if the CDI bean is not yet available, it should fallback to classloading. But the Quarkus' classloader seems to exclude all classes from the project from being discovered.

mcollovati commented 5 months ago

@VaadinServiceEnabled is just a qualifier, not a bean defining annotation.

On the server log you should see this message Can't find any @VaadinServiceScoped bean implementing 'I18NProvider'. Cannot use CDI beans for I18N, falling back to the default behavior.

Your class needs to be marked with a bean defining annotation to be discovered by CDI, like @ApplicationScoped or @VaadinServiceScoped.

You also need to add @Unremovable, to prevent the bean from being removed during build.

BTW, since Vaadin 24.3, translations from vaadin-i18/translations.properties are automatically picked up, so you do not need a custom I18Provider bean.

YassinHajaj commented 5 months ago

@VaadinServiceEnabled is a qualifier rather than a bean-defining annotation. You'll likely encounter this server log message: Can't find any @VaadinServiceScoped bean implementing 'I18NProvider'. Cannot use CDI beans for I18N, falling back to the default behavior. To ensure your class is recognized by CDI, it should be adorned with a bean-defining annotation such as @ApplicationScoped or @VaadinServiceScoped. Additionally, include @Unremovable to prevent the bean from being eliminated during the build process.

Indeed, your suggestion was effective – thank you for the prompt response! Maybe the documentation could offer some Quarkus-specific guidance. What are your thoughts on this?

By the way, as of Vaadin 24.3, translations from vaadin-i18n/translations.properties are automatically integrated. Therefore, a custom I18Provider bean is no longer necessary.

It seems I was slightly behind, believing I was up-to-date with the version mentioned in the documentation. This is actually positive, indicating that Vaadin is advancing faster than its users can keep up!

Do you think this could be specified somehow in the documentation too to avoid any misunderstanding for other users ?

YassinHajaj commented 5 months ago

To ensure your class is recognized by CDI, it should be adorned with a bean-defining annotation such as @ApplicationScoped or @VaadinServiceScoped.

By the way, this is not entirely true as including a listener on StartupEvent makes it discoverable. But that might be some Quarkus magic at work. I had tried everything except for the @Unremovable annotation.

mcollovati commented 5 months ago

Maybe the documentation could offer some Quarkus-specific guidance. What are your thoughts on this?

Actually, for CDI stuff, the Quarkus integration documentation refers to the Vaadin CDI section. In particular, for Vaadin interface implementations (like I18NProvider) it points to Vaadin Service Interfaces as CDI Beans.

However, a note about @Unremovable annotation could be added on the Quarkus integration docs.

Do you think this could be specified somehow in the documentation too to avoid any misunderstanding for other users?

If you mean documentation for I18NProvider, you can find the information in the Localization.

YassinHajaj commented 5 months ago

Ok thanks @mcollovati, it could indeed include more details about @Unremovable, that would be great.

If you mean documentation for I18NProvider, you can find the information in the Localization.

Indeed, the documentation is there but I meant about the fact that the "autodetection" is only available from 24.3. Because I was on 24.1but it wasn't working, making me wonder if it was linked to Quarkus.

By the way, if you point me to the right place for editing the documentation, I can submit a PR.

What do you think ?

mcollovati commented 5 months ago

Here's the link to the documentation repository: https://github.com/vaadin/docs

Indeed, the documentation is there but I meant about the fact that the "autodetection" is only available from 24.3. Because I was on 24.1but it wasn't working, making me wonder if it was linked to Quarkus.

Makes sense. I'll forward this comment internally.

YassinHajaj commented 5 months ago

Thanks @mcollovati I'll submit a PR ASAP.

I see it has been "corrected" but this is even more misleading, as Localization is not new at all, and certainly not since 24.3.

211345F9-E39A-4037-A895-9B52512D4C5D_4_5005_c

mcollovati commented 5 months ago

You're right, the annotation is in the wrong place. We will fix it

mshabarov commented 5 months ago

Closed, as the documentation has been updated