From 2c667b4b4032bc93c4f18bbfe50ec4227ad29600 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Tue, 18 Jul 2017 11:53:41 +0200 Subject: [PATCH 1/3] More flexible standalone parsing of Accept-Language there is now a function to replicate the default behaviour of a translation context (as returning null simply uses the first element in languages) + the cases that were advertised in the examples (like rewriting en_GB to en_US if only that is available) actually work now and are unittested. Validation for checking if there are ambigious languages in the allowed languages is still not implemented, but documented how it should be done to avoid issues. --- web/vibe/web/i18n.d | 113 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 88 insertions(+), 25 deletions(-) diff --git a/web/vibe/web/i18n.d b/web/vibe/web/i18n.d index 8393a6a8ed..70e26607a5 100644 --- a/web/vibe/web/i18n.d +++ b/web/vibe/web/i18n.d @@ -10,7 +10,7 @@ module vibe.web.i18n; import vibe.http.server : HTTPServerRequest; import std.algorithm : canFind, min, startsWith; - +import std.range.primitives : isForwardRange; /** Annotates an interface method or class with translation information. @@ -62,6 +62,7 @@ unittest { struct TranslationContext { import std.typetuple; + // A language can be in the form en_US, en-US or en. Put the languages you want to prioritize first. alias languages = TypeTuple!("en_US", "de_DE", "fr_FR"); //mixin translationModule!"app"; //mixin translationModule!"somelib"; @@ -70,7 +71,7 @@ unittest { // "Accept-Language" header static string determineLanguage(scope HTTPServerRequest req) { - if (!req.session) return null; // use default language + if (!req.session) return req.determineLanguageByHeader(languages); // default behaviour using "Accept-Language" header return req.session.get("language", ""); } } @@ -221,11 +222,93 @@ template tr(CTX, string LANG) } } -package string determineLanguage(alias METHOD)(scope HTTPServerRequest req) +/// Determines a language code from the value of a header string. +/// Returns: The best match from the Accept-Language header for a language. `null` if there is no supported language. +public string determineLanguageByHeader(T)(string acceptLanguage, T allowedLanguages) @safe pure @nogc + if (isForwardRange!T) { + import std.algorithm : splitter, countUntil; import std.string : indexOf; - import std.array; + // TODO: verify that allowedLanguages doesn't contain a mix of languages with and without extra specifier for the same lanaguage (but only if one without specifier comes before those with specifier) + // Implementing that feature should try to give a compile time warning and not change the behaviour of this function. + + if (!acceptLanguage.length) + return null; + + string okayMatch = null; + foreach (accept; acceptLanguage.splitter(",")) { + auto sidx = accept.indexOf(';'); + if (sidx >= 0) + accept = accept[0 .. sidx]; + + string acceptLang, acceptExtra; + auto acceptSeparator = accept.countUntil!(a => a == '_' || a == '-'); + if (acceptSeparator < 0) + acceptLang = accept; + else { + acceptLang = accept[0 .. acceptSeparator]; + acceptExtra = accept[acceptSeparator + 1 .. $]; + } + + foreach (lang; allowedLanguages) { + string langCode, langExtra; + auto separatorIndex = lang.countUntil!(a => a == '_' || a == '-'); + if (separatorIndex < 0) + langCode = lang; + else { + langCode = lang[0 .. separatorIndex]; + langExtra = lang[separatorIndex + 1 .. $]; + } + // request en_US == serve en_US + if (langCode == acceptLang && langExtra == acceptExtra) + return lang; + // request en_* == serve en + if (langCode == acceptLang && !langExtra.length) + return lang; + // request en* == serve en_* && be first occurence + if (langCode == acceptLang && langExtra.length && !okayMatch.length) + okayMatch = lang; + } + } + + return okayMatch; +} + +/// ditto +public string determineLanguageByHeader(Tuple...)(string acceptLanguage, Tuple allowedLanguages) @safe pure +{ + return determineLanguageByHeader(acceptLanguage, [allowedLanguages]); +} + +/// ditto +public string determineLanguageByHeader(T)(HTTPServerRequest req, T allowedLanguages) @safe pure + if (isForwardRange!T) +{ + return determineLanguageByHeader(req.headers.get("Accept-Language", null), allowedLanguages); +} + +/// ditto +public string determineLanguageByHeader(Tuple...)(HTTPServerRequest req, Tuple allowedLanguages) @safe pure +{ + return determineLanguageByHeader(req.headers.get("Accept-Language", null), [allowedLanguages]); +} + +@safe unittest { + assert(determineLanguageByHeader("de,de-DE;q=0.8,en;q=0.6,en-US;q=0.4", ["en-US", "de_DE", "de_CH"]) == "de_DE"); + assert(determineLanguageByHeader("de,de-CH;q=0.8,en;q=0.6,en-US;q=0.4", ["en_US", "de_DE", "de-CH"]) == "de-CH"); + assert(determineLanguageByHeader("en_CA,en_US", ["ja_JP", "en"]) == "en"); + assert(determineLanguageByHeader("en", ["ja_JP", "en"]) == "en"); + assert(determineLanguageByHeader("en", ["ja_JP", "en_US"]) == "en_US"); + assert(determineLanguageByHeader("en_US", ["ja-JP", "en"]) == "en"); + assert(determineLanguageByHeader("de,de-DE;q=0.8,en;q=0.6,en-US;q=0.4", ["ja_JP"]) is null); + assert(determineLanguageByHeader("de, de-DE ;q=0.8 , en ;q=0.6 , en-US;q=0.4", ["de-DE"]) == "de-DE"); + assert(determineLanguageByHeader("en_GB", ["en_US"]) == "en_US"); + assert(determineLanguageByHeader("de_DE", ["en_US"]) is null); +} + +package string determineLanguage(alias METHOD)(scope HTTPServerRequest req) +{ alias CTX = GetTranslationContext!METHOD; static if (!is(CTX == void)) { @@ -234,27 +317,7 @@ package string determineLanguage(alias METHOD)(scope HTTPServerRequest req) "determineLanguage in a translation context must return a language string."); return CTX.determineLanguage(req); } else { - auto accept_lang = req.headers.get("Accept-Language", null); - - size_t csidx = 0; - while (accept_lang.length) { - auto cidx = accept_lang[csidx .. $].indexOf(','); - if (cidx < 0) cidx = accept_lang.length; - auto entry = accept_lang[csidx .. csidx + cidx]; - auto sidx = entry.indexOf(';'); - if (sidx < 0) sidx = entry.length; - auto entrylang = entry[0 .. sidx]; - - foreach (lang; CTX.languages) { - if (entrylang == replace(lang, "_", "-")) return lang; - if (entrylang == split(lang, "_")[0]) return lang; // FIXME: ensure that only one single-lang entry exists! - } - - if (cidx >= accept_lang.length) break; - accept_lang = accept_lang[cidx+1 .. $]; - } - - return null; + return determineLanguageByHeader(req, CTX.languages); } } else return null; } From 15b7c65293d5a69574a5f7f576aa800bb4f8f2c1 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Tue, 18 Jul 2017 11:59:17 +0200 Subject: [PATCH 2/3] Adhere to codestyle --- web/vibe/web/i18n.d | 58 ++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/web/vibe/web/i18n.d b/web/vibe/web/i18n.d index 70e26607a5..405f54f2b6 100644 --- a/web/vibe/web/i18n.d +++ b/web/vibe/web/i18n.d @@ -224,74 +224,74 @@ template tr(CTX, string LANG) /// Determines a language code from the value of a header string. /// Returns: The best match from the Accept-Language header for a language. `null` if there is no supported language. -public string determineLanguageByHeader(T)(string acceptLanguage, T allowedLanguages) @safe pure @nogc +public string determineLanguageByHeader(T)(string accept_language, T allowed_languages) @safe pure @nogc if (isForwardRange!T) { import std.algorithm : splitter, countUntil; import std.string : indexOf; - // TODO: verify that allowedLanguages doesn't contain a mix of languages with and without extra specifier for the same lanaguage (but only if one without specifier comes before those with specifier) + // TODO: verify that allowed_languages doesn't contain a mix of languages with and without extra specifier for the same lanaguage (but only if one without specifier comes before those with specifier) // Implementing that feature should try to give a compile time warning and not change the behaviour of this function. - if (!acceptLanguage.length) + if (!accept_language.length) return null; - string okayMatch = null; - foreach (accept; acceptLanguage.splitter(",")) { + string fallback = null; + foreach (accept; accept_language.splitter(",")) { auto sidx = accept.indexOf(';'); if (sidx >= 0) accept = accept[0 .. sidx]; - string acceptLang, acceptExtra; - auto acceptSeparator = accept.countUntil!(a => a == '_' || a == '-'); - if (acceptSeparator < 0) - acceptLang = accept; + string alang, aextra; + auto asep = accept.countUntil!(a => a == '_' || a == '-'); + if (asep < 0) + alang = accept; else { - acceptLang = accept[0 .. acceptSeparator]; - acceptExtra = accept[acceptSeparator + 1 .. $]; + alang = accept[0 .. asep]; + aextra = accept[asep + 1 .. $]; } - foreach (lang; allowedLanguages) { - string langCode, langExtra; - auto separatorIndex = lang.countUntil!(a => a == '_' || a == '-'); - if (separatorIndex < 0) - langCode = lang; + foreach (lang; allowed_languages) { + string lcode, lextra; + sidx = lang.countUntil!(a => a == '_' || a == '-'); + if (sidx < 0) + lcode = lang; else { - langCode = lang[0 .. separatorIndex]; - langExtra = lang[separatorIndex + 1 .. $]; + lcode = lang[0 .. sidx]; + lextra = lang[sidx + 1 .. $]; } // request en_US == serve en_US - if (langCode == acceptLang && langExtra == acceptExtra) + if (lcode == alang && lextra == aextra) return lang; // request en_* == serve en - if (langCode == acceptLang && !langExtra.length) + if (lcode == alang && !lextra.length) return lang; // request en* == serve en_* && be first occurence - if (langCode == acceptLang && langExtra.length && !okayMatch.length) - okayMatch = lang; + if (lcode == alang && lextra.length && !fallback.length) + fallback = lang; } } - return okayMatch; + return fallback; } /// ditto -public string determineLanguageByHeader(Tuple...)(string acceptLanguage, Tuple allowedLanguages) @safe pure +public string determineLanguageByHeader(Tuple...)(string accept_language, Tuple allowed_languages) @safe pure { - return determineLanguageByHeader(acceptLanguage, [allowedLanguages]); + return determineLanguageByHeader(accept_language, [allowed_languages]); } /// ditto -public string determineLanguageByHeader(T)(HTTPServerRequest req, T allowedLanguages) @safe pure +public string determineLanguageByHeader(T)(HTTPServerRequest req, T allowed_languages) @safe pure if (isForwardRange!T) { - return determineLanguageByHeader(req.headers.get("Accept-Language", null), allowedLanguages); + return determineLanguageByHeader(req.headers.get("Accept-Language", null), allowed_languages); } /// ditto -public string determineLanguageByHeader(Tuple...)(HTTPServerRequest req, Tuple allowedLanguages) @safe pure +public string determineLanguageByHeader(Tuple...)(HTTPServerRequest req, Tuple allowed_languages) @safe pure { - return determineLanguageByHeader(req.headers.get("Accept-Language", null), [allowedLanguages]); + return determineLanguageByHeader(req.headers.get("Accept-Language", null), [allowed_languages]); } @safe unittest { From f3be49236b10d9295fcd78e101e303465de2525e Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Tue, 18 Jul 2017 12:37:45 +0200 Subject: [PATCH 3/3] Use std.range.only & more unittests --- web/vibe/web/i18n.d | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/web/vibe/web/i18n.d b/web/vibe/web/i18n.d index 405f54f2b6..953183bc03 100644 --- a/web/vibe/web/i18n.d +++ b/web/vibe/web/i18n.d @@ -11,6 +11,7 @@ import vibe.http.server : HTTPServerRequest; import std.algorithm : canFind, min, startsWith; import std.range.primitives : isForwardRange; +import std.range : only; /** Annotates an interface method or class with translation information. @@ -276,9 +277,9 @@ public string determineLanguageByHeader(T)(string accept_language, T allowed_lan } /// ditto -public string determineLanguageByHeader(Tuple...)(string accept_language, Tuple allowed_languages) @safe pure +public string determineLanguageByHeader(Tuple...)(string accept_language, Tuple allowed_languages) @safe pure @nogc { - return determineLanguageByHeader(accept_language, [allowed_languages]); + return determineLanguageByHeader(accept_language, only(allowed_languages)); } /// ditto @@ -291,7 +292,7 @@ public string determineLanguageByHeader(T)(HTTPServerRequest req, T allowed_lang /// ditto public string determineLanguageByHeader(Tuple...)(HTTPServerRequest req, Tuple allowed_languages) @safe pure { - return determineLanguageByHeader(req.headers.get("Accept-Language", null), [allowed_languages]); + return determineLanguageByHeader(req.headers.get("Accept-Language", null), only(allowed_languages)); } @safe unittest { @@ -305,6 +306,11 @@ public string determineLanguageByHeader(Tuple...)(HTTPServerRequest req, Tuple a assert(determineLanguageByHeader("de, de-DE ;q=0.8 , en ;q=0.6 , en-US;q=0.4", ["de-DE"]) == "de-DE"); assert(determineLanguageByHeader("en_GB", ["en_US"]) == "en_US"); assert(determineLanguageByHeader("de_DE", ["en_US"]) is null); + assert(determineLanguageByHeader("en_US,enCA", ["en_GB"]) == "en_GB"); + assert(determineLanguageByHeader("en_US,enCA", ["en_GB", "en"]) == "en"); + assert(determineLanguageByHeader("en_US,enCA", ["en", "en_GB"]) == "en"); + // TODO from above (should be invalid input having a more generic language first in the list!) + //assert(determineLanguageByHeader("en_US,enCA", ["en", "en_US"]) == "en_US"); } package string determineLanguage(alias METHOD)(scope HTTPServerRequest req)