Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add translation file request handler #18680
Changes from all commits
b9df16b
1f925fc
708934d
d57c40f
8921243
bee92a8
5cf1409
6637608
aa95f56
6f93215
0a35ceb
d6d80d1
08696ed
43fd67c
c2c0f9c
d6d232f
a3021dd
a1e201a
75e5810
4437b9f
1a53762
aeae7ca
1fdc530
049b232
3dfd498
f2ad135
71d2f8f
733d9f9
4ee0525
8ec3506
5bc27b1
fe28472
9cb6f08
6e10668
a4c6a70
c3a3cf2
314499e
5353f94
93090ef
a4e164f
d5c9edb
21235c4
bb424a8
6bc4f00
b39e6ea
4bed9de
52f26c0
fcfa250
6fc8aa1
2543e09
3be0cbe
c888d9b
f2f2cc2
0dfd516
bbb5560
aaf1f74
e012c19
ff459d9
543417f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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:
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
Locale.getDefault()
), it sends thattranslation.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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.