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

Add respecting Accept-Language header #413

Closed
wants to merge 1 commit into from

Conversation

Azbesciak
Copy link
Contributor

Will be applied as a fallback in case of missing lang query param, only if supported languages intersects languages in the header with header priority spec maintained.

…g lang query param

Will be applied only if supported languages intersects languages in the header with header priority
@Azbesciak
Copy link
Contributor Author

Any feedback?

@lonvia
Copy link
Collaborator

lonvia commented Jun 11, 2019

Sorry for the late review. So, I think this is useful in general. I have three comments about the implementation though:

  1. It needs to be implemented for the ReverseRequestFactory as well.
  2. What about the language strings like 'en-US', which are common in Accept-Language headers?
  3. I find wrapping the LanguageChecker class a little bit overkill. I would just merge the new RequestLanguageResolver and LanguageChecker in one.

@otbutz
Copy link
Contributor

otbutz commented Jun 12, 2019

2. What about the language strings like 'en-US', which are common in Accept-Language headers?

Works flawlessly using Locale.lookupTag()

Minimal working example:

List<String> supportedLangs = Arrays.asList("de", "en", "es", "fr");
String ranges = "fr-CH, fr-FR;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5";
List<Locale.LanguageRange> languageRanges = Locale.LanguageRange.parse(ranges);
String tag = Locale.lookupTag(languageRanges, supportedLangs);
System.out.println(tag); // returns "fr"

@lonvia
Copy link
Collaborator

lonvia commented Jun 14, 2019

Ah, so we only need a test for it. :)

@lonvia lonvia mentioned this pull request Jun 7, 2020
@lonvia
Copy link
Collaborator

lonvia commented Jun 7, 2020

Closing in favour of #467.

@lonvia lonvia closed this Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants