Vojtech Szocs has posted comments on this change.
Change subject: userportal,webadmin: Fix behavior unsupported locale
......................................................................
Patch Set 4: (6 inline comments)
Sorry for posting comments on older patch set, some of them might be resolved
already, please take a look.
....................................................
File
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/LocaleFilter.java
Line 64: * @return The determined {@code Locale}
Line 65: */
Line 66: private Locale determineLocale(final HttpServletRequest request) {
Line 67: // Step 1.
Line 68: Locale locale =
LocaleUtils.getLocaleFromString(request.getParameter(LOCALE));
Please ensure that LocaleUtils.getLocaleFromString(String,boolean) uses
LocaleFilter.DEFAULT_LOCALE instead of Locale.US directly.
Line 69: // Step 2.
Line 70: if (locale == null) { // No locale parameter.
Line 71: locale = getLocaleFromCookies(request.getCookies());
Line 72: }
Line 75: locale = request.getLocale();
Line 76: }
Line 77: // Step 4.
Line 78: if (locale == null) { // No accept headers.
Line 79: locale = Locale.US;
Please use LocaleFilter.DEFAULT_LOCALE instead of Locale.US directly.
Line 80: }
Line 81: Locale resolvedLocale = lookupSupportedLocale(locale,
getLocaleKeys());
Line 82: log.debug(String.format("Incoming locale: %s. Filter
determined locale to be: %s",
Line 83: locale.toLanguageTag(),
resolvedLocale.toLanguageTag()));
Line 106: * Get a list of available locales.
Line 107: * @return A {@code List} of Locale strings.
Line 108: */
Line 109: public static List<String> getLocaleKeys() {
Line 110: ResourceBundle bundle =
ResourceBundle.getBundle(LocaleFilter.class.getPackage().getName() + LANGUAGES,
I'd prefer using Properties instead of ResourceBundle, since
languages.properties file isn't meant to be localized anyway.
Line 111: Locale.US);
Line 112: List<String> keys = Collections.list(bundle.getKeys());
Line 113: // Can't just return the unsorted list, it won't match the
GWT output.
Line 114: // Apparently GWT does a sort before showing the list as
well, which
Line 124: * @return The supported locale based on the passed in
filteredLocale and list of supported locales.
Line 125: */
Line 126: private Locale lookupSupportedLocale(final Locale filteredLocale,
final List<String> supportedLocales) {
Line 127: for (String localeString: supportedLocales) {
Line 128: Locale locale =
LocaleUtils.getLocaleFromString(localeString);
Small thing: I'd prefer renaming this variable to "supportedLocale"
Line 129: @SuppressWarnings("unchecked")
Line 130: List<Locale> localeLookupList =
org.apache.commons.lang.LocaleUtils.localeLookupList(locale);
Line 131: for (Locale lookupLocale: localeLookupList) {
Line 132: if (lookupLocale.equals(filteredLocale)) {
Line 126: private Locale lookupSupportedLocale(final Locale filteredLocale,
final List<String> supportedLocales) {
Line 127: for (String localeString: supportedLocales) {
Line 128: Locale locale =
LocaleUtils.getLocaleFromString(localeString);
Line 129: @SuppressWarnings("unchecked")
Line 130: List<Locale> localeLookupList =
org.apache.commons.lang.LocaleUtils.localeLookupList(locale);
+1 using commons LocaleUtils.localeLookupList
Line 131: for (Locale lookupLocale: localeLookupList) {
Line 132: if (lookupLocale.equals(filteredLocale)) {
Line 133: // Matched on one of the locales. Find the
supported one in the list.
Line 134: return locale;
Line 135: }
Line 136: }
Line 137: }
Line 138: // TODO: When branding patch is merged update me to say
LocaleFilter.defaultLocale
Line 139: return Locale.US;
Please use LocaleFilter.DEFAULT_LOCALE instead of Locale.US directly.
Line 140: }
Line 141:
Line 142: @Override
Line 143: public void init(final FilterConfig filterConfig) throws
ServletException {
--
To view, visit http://gerrit.ovirt.org/15520
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5beceaf7f4ce2333e83cd72c3fa8721103cf805
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
