Skip to content

Commit

Permalink
feat: Improve Handling Default Language Selection from UI - MEED-7144 -
Browse files Browse the repository at this point in the history
Meeds-io/meeds#2230 (#935)

Prior to this change, the language cache key was using the language
without the country specialization. This change will ensure to use the
language full representation string using `toLanguageTag` instead of
`getLanguage`.
At the same time, this will fix the `User Locale` retrieved from
`LocalePolicy` service on each Web Controller Handler plugin.
  • Loading branch information
boubaker authored Jul 9, 2024
1 parent 1e73e45 commit 1ab5411
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,9 @@ public interface BrandingService {
*/
String getLoginSubtitle(Locale locale);

/**
* @return default platform language
*/
String getDefaultLanguage();

}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static LocaleContextInfo buildLocaleContextInfo(HttpServletRequest reques
localeCtx.setSessionLocale(sessionLocale);
// continue setting localCtx with data fetched from request
localeCtx.setUserProfileLocale(getUserLocale(username));
localeCtx.setBrowserLocales(Collections.list(request.getLocales()));
localeCtx.setBrowserLocales(request.getLocales() == null ? null : Collections.list(request.getLocales()));
localeCtx.setCookieLocales(getCookieLocales(request));
localeCtx.setRemoteUser(username);
return localeCtx;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public HttpRequestLocaleWrapper(HttpServletRequest request) {
public Locale getLocale() {
if (locale == null) {
LocaleContextInfo localeCtx = LocaleContextInfoUtils.buildLocaleContextInfo((HttpServletRequest) getRequest());
locale = ExoContainerContext.getService(LocalePolicy.class).determineLocale(localeCtx);
LocalePolicy localePolicyService = ExoContainerContext.getService(LocalePolicy.class);
locale = localePolicyService == null ? getRequest().getLocale() : localePolicyService.determineLocale(localeCtx);
}
return locale;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@
import org.exoplatform.portal.branding.model.Branding;
import org.exoplatform.portal.branding.model.Favicon;
import org.exoplatform.portal.branding.model.Logo;
import org.exoplatform.services.log.ExoLogger;
import org.exoplatform.services.log.Log;
import org.exoplatform.services.rest.resource.ResourceContainer;

import io.swagger.v3.oas.annotations.Operation;
Expand All @@ -55,8 +53,6 @@
@Tag(name = "/v1/platform/branding", description = "Managing branding information")
public class BrandingRestResourcesV1 implements ResourceContainer {

private static final Log LOG = ExoLogger.getLogger(BrandingRestResourcesV1.class);

private static final String IMAGE_MIME_TYPE = "image/png";

// 1 year
Expand All @@ -75,37 +71,11 @@ public BrandingRestResourcesV1(BrandingService brandingService) {
@RolesAllowed("users")
@Operation(summary = "Get Branding information", description = "Get Branding information", method = "GET")
@ApiResponses(value = {
@ApiResponse(responseCode = "200", description = "Request fulfilled"),
@ApiResponse(responseCode = "304", description = "Resource not modified"),
@ApiResponse(responseCode = "500", description = "Server error when retrieving branding information")
@ApiResponse(responseCode = "200", description = "Request fulfilled"),
})
public Response getBrandingInformation(
@Context
Request request,
@Parameter(description = "Whether force refresh information from database or not")
@QueryParam("forceRefresh")
String forceRefresh) {
try {
boolean refresh = StringUtils.equals("true", forceRefresh);
long lastUpdated = brandingService.getLastUpdatedTime();
EntityTag eTag = refresh ? null : new EntityTag(String.valueOf(lastUpdated));

//
Response.ResponseBuilder builder = request.evaluatePreconditions(eTag);
if (builder == null) {
Branding brandingInformation = brandingService.getBrandingInformation(refresh);
builder = Response.ok(brandingInformation);
CacheControl cc = new CacheControl();
cc.setMustRevalidate(true);
builder.tag(eTag);
builder.lastModified(new Date(lastUpdated));
builder.cacheControl(cc);
}
return builder.build();
} catch (Exception e) {
LOG.error("Error when retrieving branding information", e);
return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
}
public Response getBrandingInformation() {
Branding brandingInformation = brandingService.getBrandingInformation(false);
return Response.ok(brandingInformation).build();
}

@PUT
Expand All @@ -115,16 +85,10 @@ public Response getBrandingInformation(
@Operation(summary = "Update Branding information", description = "Update Branding information", method = "POST")
@ApiResponses(value = {
@ApiResponse(responseCode = "204", description = "Branding information updated"),
@ApiResponse(responseCode = "500", description = "Server error when updating branding information")
})
public Response updateBrandingInformation(Branding branding) {
try {
brandingService.updateBrandingInformation(branding);
return Response.noContent().build();
} catch (Exception e) {
LOG.error("Error when updating branding information", e);
return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
}
brandingService.updateBrandingInformation(branding);
return Response.noContent().build();
}

@GET
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ public Branding getBrandingInformation() {
public Branding getBrandingInformation(boolean retrieveBinaries) {
Branding branding = new Branding();
branding.setDefaultLanguage(getDefaultLanguage());
branding.setSupportedLanguages(Collections.unmodifiableMap(supportedLanguages));
branding.setSupportedLanguages(loadLanguages());
branding.setCompanyName(getCompanyName());
branding.setCompanyLink(getCompanyLink());
branding.setSiteName(getSiteName());
Expand Down Expand Up @@ -771,6 +771,11 @@ public String getLoginSubtitle(Locale locale) {
}
}

@Override
public String getDefaultLanguage() {
return getDefaultLocale().toLanguageTag();
}

/**
* Load init params
*
Expand Down Expand Up @@ -839,19 +844,21 @@ private void loadInitParams(InitParams initParams) { // NOSONAR
}
}

private void loadLanguages() {
private Map<String, String> loadLanguages() {
Locale defaultLocale = getDefaultLocale();
this.supportedLanguages =
localeConfigService.getLocalConfigs() == null ? Collections.singletonMap(defaultLocale.getLanguage(),
getLocaleDisplayName(defaultLocale,
defaultLocale)) :
localeConfigService.getLocalConfigs()
.stream()
.filter(localeConfig -> !StringUtils.equals(localeConfig.getLocaleName(),
"ma"))
.collect(Collectors.toMap(LocaleConfig::getLocaleName,
localeConfig -> getLocaleDisplayName(defaultLocale,
localeConfig.getLocale())));
this.supportedLanguages = localeConfigService.getLocalConfigs()
== null ?
Collections.singletonMap(defaultLocale.getLanguage(),
getLocaleDisplayName(defaultLocale,
defaultLocale)) :
localeConfigService.getLocalConfigs()
.stream()
.filter(localeConfig -> !StringUtils.equals(localeConfig.getLocaleName(),
"ma"))
.collect(Collectors.toMap(LocaleConfig::getLocaleName,
localeConfig -> getLocaleDisplayName(defaultLocale,
localeConfig.getLocale())));
return this.supportedLanguages;
}

private Locale getDefaultLocale() {
Expand All @@ -860,10 +867,6 @@ private Locale getDefaultLocale() {
.getLocale();
}

private String getDefaultLanguage() {
return getDefaultLocale().toLanguageTag();
}

private String getLocaleDisplayName(Locale defaultLocale, Locale locale) {
return defaultLocale.equals(locale) ? defaultLocale.getDisplayName(defaultLocale) :
locale.getDisplayName(defaultLocale) + " / " + locale.getDisplayName(locale);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,9 @@ public void run(){
for (Iterator<LocaleConfig> iter = localeConfigs.iterator(); iter.hasNext();){
LocaleConfig localeConfig = iter.next();
//Skip loaded resources before container startup
if (localeList_.contains(localeConfig.getLocale().getLanguage())){
continue;
if (!localeList_.contains(localeConfig.getLocale().toLanguageTag())){
loadResourcesForLocale(localeConfig.getLocale());
}
loadResourcesForLocale(localeConfig.getLocale());
}
}
},"ResourceBundleThread").start();
Expand Down Expand Up @@ -364,7 +363,7 @@ protected void loadResourcesForLocale(Locale locale)
for (String resource : getInitResources_()) {
initResources(resource, locale, cl);
}
localeList_.add(locale.getLanguage());
localeList_.add(locale.toLanguageTag());
}

/**
Expand Down Expand Up @@ -542,7 +541,7 @@ public GetResourceBundleFromDbContext(String name, Locale locale) {
ResourceBundle get(String id) {
ResourceBundle res = null;
//local resources is not yet loaded
if (!localeList_.contains(locale.getLanguage())){
if (!localeList_.contains(locale.toLanguageTag())){
//force the load of local resources
loadResourcesForLocale(locale);
}
Expand Down Expand Up @@ -587,7 +586,7 @@ ResourceBundle get(String id) {
outputBundled = new MapResourceBundle(locale);
for (int i = 0; i < name.length; i++) {
//local resources is not yet loaded
if (!localeList_.contains(locale.getLanguage())){
if (!localeList_.contains(locale.toLanguageTag())){
//force the load of local resources
loadResourcesForLocale(locale);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@

import io.meeds.portal.security.constant.UserRegistrationType;
import io.meeds.portal.security.service.SecuritySettingService;
import io.meeds.spring.web.localization.HttpRequestLocaleWrapper;

public class LoginHandler extends JspBasedWebHandler {

Expand Down Expand Up @@ -154,7 +155,7 @@ public void onInit(WebAppController controller, ServletConfig servletConfig) {

@Override
public boolean execute(ControllerContext context) throws Exception { // NOSONAR
HttpServletRequest request = context.getRequest();
HttpServletRequest request = new HttpRequestLocaleWrapper(context.getRequest());
HttpServletResponse response = context.getResponse();
try {
// We set the character encoding now to UTF-8 before obtaining parameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import org.exoplatform.web.application.javascript.JavascriptConfigService;
import org.exoplatform.web.controller.QualifiedName;

import io.meeds.spring.web.localization.HttpRequestLocaleWrapper;

/**
* @author <a href="mailto:[email protected]">Tuyen Nguyen The</a>.
*/
Expand Down Expand Up @@ -118,7 +120,7 @@ public PasswordRecoveryHandler(PortalContainer container, // NOSONAR

@Override
public boolean execute(ControllerContext controllerContext) throws Exception {// NOSONAR
HttpServletRequest request = controllerContext.getRequest();
HttpServletRequest request = new HttpRequestLocaleWrapper(controllerContext.getRequest());
HttpServletResponse response = controllerContext.getResponse();

Locale locale = request.getLocale();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import org.exoplatform.web.security.security.RemindPasswordTokenService;

import io.meeds.portal.security.service.SecuritySettingService;
import io.meeds.spring.web.localization.HttpRequestLocaleWrapper;

import nl.captcha.Captcha;
import nl.captcha.servlet.CaptchaServletUtil;
Expand Down Expand Up @@ -203,7 +204,7 @@ public String getHandlerName() {

@Override
public boolean execute(ControllerContext controllerContext) throws Exception {// NOSONAR
HttpServletRequest request = controllerContext.getRequest();
HttpServletRequest request = new HttpRequestLocaleWrapper(controllerContext.getRequest());
HttpServletResponse response = controllerContext.getResponse();

Map<String, Object> parameters = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
import org.exoplatform.web.login.UIParamsExtension;
import org.exoplatform.web.login.recovery.PasswordRecoveryService;

import io.meeds.spring.web.localization.HttpRequestLocaleWrapper;

import nl.captcha.Captcha;
import nl.captcha.servlet.CaptchaServletUtil;
import nl.captcha.text.producer.DefaultTextProducer;
Expand Down Expand Up @@ -145,7 +147,7 @@ protected boolean getRequiresLifeCycle() {

@Override
public boolean execute(ControllerContext controllerContext) throws Exception {
HttpServletRequest request = controllerContext.getRequest();
HttpServletRequest request = new HttpRequestLocaleWrapper(controllerContext.getRequest());
HttpServletResponse response = controllerContext.getResponse();

// If user is already authenticated, no registration form is required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.junit.MockitoJUnitRunner;

import static org.mockito.ArgumentMatchers.any;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

import org.gatein.wci.security.Credentials;
import org.json.JSONObject;
import org.junit.After;
import org.junit.Before;
Expand Down Expand Up @@ -83,7 +82,7 @@
import org.exoplatform.web.controller.router.Router;
import org.exoplatform.web.security.security.CookieTokenService;

@RunWith(MockitoJUnitRunner.class)
@RunWith(MockitoJUnitRunner.Silent.class)
public class PasswordRecoveryHandlerTest {

private static final Locale REQUEST_LOCALE = Locale.ENGLISH;
Expand Down Expand Up @@ -293,7 +292,7 @@ public void testSendResetPasswordUrlWhenUserEnabledAndSendFailure() throws Excep
assertEquals(username, applicationParameters.get(USERNAME_PARAM));

verify(passwordRecoveryService, never()).changePass(any(), any(), any(), any());
verify(passwordRecoveryService, times(1)).sendRecoverPasswordEmail(user, REQUEST_LOCALE, request);
verify(passwordRecoveryService, times(1)).sendRecoverPasswordEmail(eq(user), eq(REQUEST_LOCALE), any(HttpServletRequest.class));

assertFalse(applicationParameters.containsKey(SUCCESS_MESSAGE_PARAM));
assertEquals("gatein.forgotPassword.emailSendFailure", applicationParameters.get(ERROR_MESSAGE_PARAM));
Expand Down Expand Up @@ -330,7 +329,7 @@ public void testSendResetPasswordUrlWhenUserEnabledAndSendSuccess() throws Excep
assertEquals(username, applicationParameters.get(USERNAME_PARAM));

verify(passwordRecoveryService, never()).changePass(any(), any(), any(), any());
verify(passwordRecoveryService, times(1)).sendRecoverPasswordEmail(user, REQUEST_LOCALE, request);
verify(passwordRecoveryService, times(1)).sendRecoverPasswordEmail(eq(user), eq(REQUEST_LOCALE), any(HttpServletRequest.class));

assertFalse(applicationParameters.containsKey(ERROR_MESSAGE_PARAM));
assertEquals("gatein.forgotPassword.emailSendSuccessful", applicationParameters.get(SUCCESS_MESSAGE_PARAM));
Expand Down

0 comments on commit 1ab5411

Please sign in to comment.