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

feat: add translation file request handler #18680

Merged
merged 59 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
b9df16b
feat: add translation file request handler
ugur-vaadin Feb 12, 2024
1f925fc
refactor: remove static keyword for consistency
ugur-vaadin Feb 12, 2024
708934d
refactor: introduce translation file request enum and constant
ugur-vaadin Feb 12, 2024
d57c40f
refactor: use language tag instead of separate params
ugur-vaadin Feb 20, 2024
8921243
refactor: lower severity of missing bundle log
ugur-vaadin Feb 21, 2024
bee92a8
refactor: add retrieved locale to response
ugur-vaadin Feb 21, 2024
5cf1409
refactor: increase logging severity for missing resource bundle
ugur-vaadin Feb 21, 2024
6637608
chore: add missing default bundle to log message
ugur-vaadin Feb 21, 2024
aa95f56
test: add tests for system default locale and rename default bundle
ugur-vaadin Feb 22, 2024
6f93215
chore: run formatter
ugur-vaadin Feb 22, 2024
0a35ceb
refactor: only use the root locale as the fallback locale
ugur-vaadin Feb 22, 2024
d6d80d1
chore: do not use wildcard imports
ugur-vaadin Feb 26, 2024
08696ed
refactor: update retrieved locale header name
ugur-vaadin Feb 26, 2024
43fd67c
fix: handle translation file requests only for default i18n provider
ugur-vaadin Feb 26, 2024
c2c0f9c
test: mock service to fix tests
ugur-vaadin Feb 26, 2024
d6d232f
test: fix i18nprovider mocking
ugur-vaadin Feb 26, 2024
a3021dd
chore: remove unused imports
ugur-vaadin Feb 26, 2024
a1e201a
refactor: check i18n provider when initializing handlers
ugur-vaadin Feb 27, 2024
75e5810
refactor: make getbundle package private
ugur-vaadin Feb 27, 2024
4437b9f
refactor: set i18n provider to handler and handle custom provider
ugur-vaadin Feb 27, 2024
1a53762
test: add test for custom i18n provider
ugur-vaadin Feb 27, 2024
aeae7ca
test: rename tests
ugur-vaadin Feb 27, 2024
1fdc530
refactor: add prod case and update status codes for custom i18n provider
ugur-vaadin Feb 27, 2024
049b232
Update flow-server/src/main/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Feb 28, 2024
3dfd498
Update flow-server/src/main/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Feb 28, 2024
f2ad135
Update flow-server/src/test/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Feb 28, 2024
71d2f8f
Update flow-server/src/main/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Feb 28, 2024
733d9f9
Update flow-server/src/main/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Feb 28, 2024
4ee0525
refactor: extend synchronized request handler
ugur-vaadin Feb 28, 2024
8ec3506
Merge branch 'feat-add-translation-file-request-handler' of https://g…
ugur-vaadin Feb 28, 2024
5bc27b1
refactor: return not implemented for custom i18n provider when not in…
ugur-vaadin Feb 28, 2024
fe28472
Update flow-server/src/main/java/com/vaadin/flow/server/VaadinService…
ugur-vaadin Feb 28, 2024
9cb6f08
Merge branch 'feat-add-translation-file-request-handler' of https://g…
ugur-vaadin Feb 28, 2024
6e10668
Update flow-server/src/test/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Feb 28, 2024
a4c6a70
Merge branch 'feat-add-translation-file-request-handler' of https://g…
ugur-vaadin Feb 28, 2024
c3a3cf2
chore: do not use wildcard in imports
ugur-vaadin Feb 28, 2024
314499e
Update flow-server/src/test/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Feb 28, 2024
5353f94
Merge branch 'feat-add-translation-file-request-handler' of https://g…
ugur-vaadin Feb 28, 2024
93090ef
chore: run formatter
ugur-vaadin Feb 28, 2024
a4e164f
test: add tests for verifying response status codes
ugur-vaadin Feb 28, 2024
d5c9edb
test: add failure messages to response content assertions
ugur-vaadin Feb 28, 2024
21235c4
refactor: add underscore in language tag support
ugur-vaadin Feb 28, 2024
bb424a8
refactor: return bundle for partial locale match
ugur-vaadin Feb 28, 2024
6bc4f00
chore: run formatter
ugur-vaadin Feb 29, 2024
b39e6ea
refactor: fix and clarify missing locale logs
ugur-vaadin Feb 29, 2024
4bed9de
test: add tests for unavailable language tags with underscores
ugur-vaadin Feb 29, 2024
52f26c0
test: refactor initializing block
ugur-vaadin Feb 29, 2024
fcfa250
refactor: check for exact match in i18n provider when handling
ugur-vaadin Feb 29, 2024
6fc8aa1
docs: add javadoc for the handler
ugur-vaadin Feb 29, 2024
2543e09
docs: further clarify the parameter and response header
ugur-vaadin Feb 29, 2024
3be0cbe
Merge branch 'main' into feat-add-translation-file-request-handler
mcollovati Mar 1, 2024
c888d9b
refactor: check for i18n provider class and fallback bundle in constr…
ugur-vaadin Mar 1, 2024
f2f2cc2
test: refactor and cleanup tests
ugur-vaadin Mar 1, 2024
0dfd516
test: add tests for empty language tags
ugur-vaadin Mar 1, 2024
bbb5560
Update flow-server/src/main/java/com/vaadin/flow/i18n/TranslationFile…
ugur-vaadin Mar 1, 2024
aaf1f74
refactor: make fields private
ugur-vaadin Mar 1, 2024
e012c19
fix: separate logs for fallback bundles and best match bundles
ugur-vaadin Mar 1, 2024
ff459d9
refactor: add one more log case and use placeholders
ugur-vaadin Mar 1, 2024
543417f
chore: simplify logs and use default bundle to indicate the fallback …
ugur-vaadin Mar 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,23 @@ public String getTranslation(String key, Locale locale, Object... params) {

private ResourceBundle getBundle(Locale locale) {
try {
return ResourceBundle.getBundle(BUNDLE_PREFIX, locale,
I18NUtil.getClassLoader());
return getBundle(locale, null);
} catch (final MissingResourceException e) {
getLogger().warn("Missing resource bundle for " + BUNDLE_PREFIX
+ " and locale " + locale.getDisplayName(), e);
}
return null;
}

ResourceBundle getBundle(Locale locale, ResourceBundle.Control control) {
if (control == null) {
return ResourceBundle.getBundle(BUNDLE_PREFIX, locale,
I18NUtil.getClassLoader());
}
return ResourceBundle.getBundle(BUNDLE_PREFIX, locale,
I18NUtil.getClassLoader(), control);
}

static Logger getLogger() {
return LoggerFactory.getLogger(DefaultI18NProvider.class);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
/*
* Copyright 2000-2024 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package com.vaadin.flow.i18n;

import com.vaadin.flow.server.HandlerHelper;
import com.vaadin.flow.server.HttpStatusCode;
import com.vaadin.flow.server.SynchronizedRequestHandler;
import com.vaadin.flow.server.VaadinRequest;
import com.vaadin.flow.server.VaadinResponse;
import com.vaadin.flow.server.VaadinSession;
import elemental.json.Json;
import elemental.json.JsonObject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.Locale;
import java.util.MissingResourceException;
import java.util.Objects;
import java.util.Optional;
import java.util.ResourceBundle;
import java.util.Set;

/**
* Handles translation file requests. Translation file requests are internal
* requests sent by the client-side to retrieve the translation file for the
* specified language tag. The response contains the translations in JSON
* format. Also, the language tag of the retrieved translation file is included
* as a header with the name {@code X-Vaadin-Retrieved-Locale}. The language tag
* parameter {@code langtag} supports both dash and underscore as separators.
* <p>
* The translation file to return is determined by matching the requested locale
* to the available bundles with the following prioritization order:
* <ul>
* <li>Exact match</li>
* <li>Language and country match</li>
* <li>Language match</li>
* <li>Default bundle (root bundle)</li>
* </ul>
* <p>
* For internal use only. May be renamed or removed in a future release.
*
* @author Vaadin Ltd
* @since 24.4
*/
public class TranslationFileRequestHandler extends SynchronizedRequestHandler {
mcollovati marked this conversation as resolved.
Show resolved Hide resolved

static final String LANGUAGE_TAG_PARAMETER_NAME = "langtag";

static final String RETRIEVED_LOCALE_HEADER_NAME = "X-Vaadin-Retrieved-Locale";

private static final Locale FALLBACK_LOCALE = Locale.ROOT;

private final DefaultI18NProvider i18NProvider;

private final boolean hasFallbackBundle;

public TranslationFileRequestHandler(I18NProvider i18NProvider) {
boolean hasDefaultI18NProvider = i18NProvider != null
&& DefaultI18NProvider.class.equals(i18NProvider.getClass());
this.i18NProvider = hasDefaultI18NProvider
? (DefaultI18NProvider) i18NProvider
: null;
this.hasFallbackBundle = hasFallbackBundle();
}

@Override
public boolean synchronizedHandleRequest(VaadinSession session,
VaadinRequest request, VaadinResponse response) throws IOException {
if (i18NProvider == null) {
handleCustomI18NProvider(session, response);
return true;
}
Locale locale = getLocale(request);
ResourceBundle translationPropertyFile = getTranslationPropertyFile(
locale);
if (translationPropertyFile == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the perspective of the client we need to have one set of translations in order to for an app to display something to the user. Returning a not found is problematic:

  • The client would first make a request for the identified language
  • Gets a 404, and then does what?
  • Maybe tries to load a fallback language, which would result in an additional request. That would mean it takes even longer before the app can display something to the user. Also the fallback language configured in the client might not exist either.

I was thinking the server should return a "default" set of translations whenever it doesn't find a set of translations for the specified language. That means there would only ever be one request before the app can display something. In terms of Java resource bundles, that would probably mean we send the contents of the default / root properties file. The response would then also need to contain some information for which language the set of translations are actually for, so the client knows that it received the fallback instead of the thing it requested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the PR to include the language tag for the retrieved bundle. The handler returns the default file if it is available, and file not found response is only the case when there is no default to return. The language tag is added to the response with the header name Retrieved-locale.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it does return a default bundle now, though it might not be the one you expect. What seems to happen now is:

  • If there is a bundle for the server locale (Locale.getDefault()), it sends that
  • Otherwise it sends the root bundle (translation.properties)

Since Flow's DefaultI18nProvider uses the same method call, I would assume it works the same way, but it would be good to double check. Otherwise we might want to have a discussion what the fallback should be. Should it consider the server locale as fallback, or should it always use the root bundle? If we consider the server locale it could return different results in different environments. On the other hand I don't know what the best practice is for the root bundle. Would it be a safe assumption that the root bundle contains translations for the default language? An alternative could be to let developers explicitly specify a fallback language of which they know that a bundle exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests with mocked system default locale that confirm this behaviour. I will modify the tests based on the decisions on whether to introduce a specified fallback locale and the use of system default locale in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the PR to have only a single fallback locale. It is currently the root locale and not customizable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work well, also still handles fallbacks such as from de -> de_DE and vice versa.

handleNotFound(response);
} else {
handleFound(response, translationPropertyFile);
}
return true;
}

@Override
protected boolean canHandleRequest(VaadinRequest request) {
return HandlerHelper.isRequestType(request,
HandlerHelper.RequestType.TRANSLATION_FILE);
}

private void handleFound(VaadinResponse response,
ResourceBundle translationPropertyFile) throws IOException {
response.setStatus(HttpStatusCode.OK.getCode());
response.setHeader(RETRIEVED_LOCALE_HEADER_NAME,
translationPropertyFile.getLocale().toLanguageTag());
writeFileToResponse(response, translationPropertyFile);
}

private void handleNotFound(VaadinResponse response) {
response.setStatus(HttpStatusCode.NOT_FOUND.getCode());
}

private void handleCustomI18NProvider(VaadinSession session,
VaadinResponse response) throws IOException {
String errorMessage = "Loading translations is not supported when using a custom i18n provider.";
if (session.getService().getDeploymentConfiguration()
.isProductionMode()) {
response.setStatus(HttpStatusCode.NOT_FOUND.getCode());
} else {
response.sendError(HttpStatusCode.NOT_IMPLEMENTED.getCode(),
errorMessage);
}
getLogger().debug(errorMessage);
}

private void writeFileToResponse(VaadinResponse response,
ResourceBundle translationPropertyFile) throws IOException {
JsonObject json = Json.createObject();
translationPropertyFile.keySet().forEach(
key -> json.put(key, translationPropertyFile.getString(key)));
response.getWriter().write(json.toJson());
}

private Locale getLocale(VaadinRequest request) {
String languageTag = Objects.requireNonNullElse(
request.getParameter(LANGUAGE_TAG_PARAMETER_NAME), "");
if (languageTag.contains("_")) {
mcollovati marked this conversation as resolved.
Show resolved Hide resolved
String[] tokens = languageTag.split("_");
String language = tokens[0];
String country = tokens.length > 1 ? tokens[1] : "";
String variant = tokens.length > 2 ? tokens[2] : "";
return new Locale(language, country, variant);
}
return Locale.forLanguageTag(languageTag);
}

private ResourceBundle getTranslationPropertyFile(Locale locale) {
mcollovati marked this conversation as resolved.
Show resolved Hide resolved
Locale bestMatchLocale = getBestMatchLocale(locale);
if (bestMatchLocale == null) {
String fallbackString = FALLBACK_LOCALE.getDisplayName().isEmpty()
? ""
: (" " + FALLBACK_LOCALE.getDisplayName());
if (FALLBACK_LOCALE.equals(locale)) {
getLogger().debug(
"Missing the requested fallback resource bundle for {} and locale{}.",
DefaultI18NProvider.BUNDLE_PREFIX, fallbackString);
} else {
getLogger().debug(
"Missing resource bundles for {}, both the requested locale {} and the fallback locale{}.",
DefaultI18NProvider.BUNDLE_PREFIX,
locale.getDisplayName(), fallbackString);
}
return null;
}
if (!locale.equals(bestMatchLocale)) {
if (FALLBACK_LOCALE.equals(bestMatchLocale)) {
String fallbackString = FALLBACK_LOCALE.getDisplayName()
.isEmpty() ? ""
: (" " + FALLBACK_LOCALE.getDisplayName());
getLogger().debug(
"Missing resource bundle for {} and locale {}. Using the fallback locale{}.",
DefaultI18NProvider.BUNDLE_PREFIX,
locale.getDisplayName(), fallbackString);
} else {
getLogger().debug(
"Missing resource bundle for {} and locale {}. Using the best match locale {}.",
DefaultI18NProvider.BUNDLE_PREFIX,
locale.getDisplayName(),
bestMatchLocale.getDisplayName());
}
}
return i18NProvider.getBundle(bestMatchLocale,
ResourceBundle.Control.getNoFallbackControl(
ResourceBundle.Control.FORMAT_PROPERTIES));
}

private Locale getBestMatchLocale(Locale locale) {
Set<Locale> providedLocales = Set
.copyOf(i18NProvider.getProvidedLocales());
if (providedLocales.contains(locale)) {
return locale;
}
Optional<Locale> languageAndCountryMatch = providedLocales.stream()
.filter(providedLocale -> providedLocale.getLanguage()
.equals(locale.getLanguage())
&& providedLocale.getCountry()
.equals(locale.getCountry()))
.findAny();
if (languageAndCountryMatch.isPresent()) {
return languageAndCountryMatch.get();
}
Optional<Locale> languageMatch = providedLocales.stream()
.filter(providedLocale -> providedLocale.getLanguage()
.equals(locale.getLanguage()))
.findAny();
if (languageMatch.isPresent()) {
return languageMatch.get();
}
mcollovati marked this conversation as resolved.
Show resolved Hide resolved
if (hasFallbackBundle) {
return FALLBACK_LOCALE;
}
return null;
}

private boolean hasFallbackBundle() {
if (this.i18NProvider != null) {
try {
this.i18NProvider.getBundle(FALLBACK_LOCALE,
ResourceBundle.Control.getNoFallbackControl(
ResourceBundle.Control.FORMAT_PROPERTIES));
return true;
} catch (MissingResourceException e) {
// NO-OP
}
}
return false;
}

private Logger getLogger() {
return LoggerFactory.getLogger(TranslationFileRequestHandler.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,12 @@ public enum RequestType {
/**
* Page showing that the browser is unsupported.
*/
BROWSER_TOO_OLD("oldbrowser");
BROWSER_TOO_OLD("oldbrowser"),

/**
* Translation properties file requests.
*/
TRANSLATION_FILE(ApplicationConstants.REQUEST_TYPE_TRANSLATION_FILE);

private final String identifier;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.vaadin.flow.di.Lookup;
import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.i18n.I18NProvider;
import com.vaadin.flow.i18n.TranslationFileRequestHandler;
import com.vaadin.flow.internal.CurrentInstance;
import com.vaadin.flow.internal.LocaleUtil;
import com.vaadin.flow.internal.UsageStatistics;
Expand Down Expand Up @@ -333,6 +334,8 @@ protected List<RequestHandler> createRequestHandlers()
handlers.add(new UnsupportedBrowserHandler());
handlers.add(new StreamRequestHandler());
handlers.add(new PwaHandler(() -> getPwaRegistry()));
handlers.add(new TranslationFileRequestHandler(
getInstantiator().getI18NProvider()));

handlers.add(new WebComponentBootstrapHandler());
handlers.add(new WebComponentProvider());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ public class ApplicationConstants implements Serializable {
*/
public static final String REQUEST_TYPE_WEBCOMPONENT_RESYNC = "webcomponent-resync";

/**
* Request type parameter value indicating a translation properties file
* request.
*/
public static final String REQUEST_TYPE_TRANSLATION_FILE = "i18n";

/**
* Attribute name for marking internal router link anchors.
*/
Expand Down
Loading
Loading