Skip to content

Commit

Permalink
fix!: change the format of a returned account locale to xx_YY (#30)
Browse files Browse the repository at this point in the history
When the carbonio-user-management returns the locale (zimbraLocalPref)
of a requested account, the string value was returned in upper case
(XX_YY) and this raised a problem because the format of this value was
unreadable by other systems.

Now the carbonio-user-management returns the locale in the xx_YY format
checking if the locale returned by the carbonio-mailbox is a valid one.
This check is necessary since the value of this type of pref can be set
manually by a sysadmin without any check from the carbonio-mailbox.

When the system receives an invalid locale it falls back to 'en'.

BREAKING CHANGE: The /users/myself API response is changed to follow the
different format used to represent the locale value. Now it is a string
(instead of an enumerator) and it can support all the values defined in
the standard.

refs: UM-25
  • Loading branch information
federicorispo committed Oct 30, 2023
1 parent 6496c4b commit c7065ca
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 43 deletions.
4 changes: 2 additions & 2 deletions boot/pom.xml
Expand Up @@ -25,14 +25,14 @@ SPDX-License-Identifier: AGPL-3.0-only
<parent>
<groupId>com.zextras.carbonio.user-management</groupId>
<artifactId>carbonio-user-management</artifactId>
<version>0.3.0-1</version>
<version>0.4.0-SNAPSHOT</version>
</parent>

<dependencies>
<dependency>
<groupId>com.zextras.carbonio.user-management</groupId>
<artifactId>carbonio-user-management-core</artifactId>
<version>0.3.0-1</version>
<version>0.4.0-SNAPSHOT</version>
</dependency>

<!-- Guice dependencies -->
Expand Down
2 changes: 1 addition & 1 deletion core/pom.xml
Expand Up @@ -23,7 +23,7 @@ SPDX-License-Identifier: AGPL-3.0-only
<parent>
<groupId>com.zextras.carbonio.user-management</groupId>
<artifactId>carbonio-user-management</artifactId>
<version>0.3.0-1</version>
<version>0.4.0-SNAPSHOT</version>
</parent>

<dependencies>
Expand Down
Expand Up @@ -10,23 +10,27 @@
import com.zextras.carbonio.user_management.cache.CacheManager;
import com.zextras.carbonio.user_management.entities.UserToken;
import com.zextras.carbonio.user_management.exceptions.ServiceException;
import com.zextras.carbonio.user_management.generated.model.Locale;
import com.zextras.carbonio.user_management.generated.model.UserId;
import com.zextras.carbonio.user_management.generated.model.UserInfo;
import com.zextras.carbonio.user_management.generated.model.UserMyself;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import org.apache.commons.lang3.LocaleUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import zimbraaccount.Attr;
import zimbraaccount.GetAccountInfoResponse;
import zimbraaccount.GetInfoResponse;
import zimbraaccount.Pref;

public class UserService {

private static final Logger logger = LoggerFactory.getLogger(UserService.class);

private final CacheManager cacheManager;

@Inject
Expand Down Expand Up @@ -152,14 +156,31 @@ public Optional<UserMyself> getMyselfByToken(String token) {
UserId userId = new UserId();
userId.setUserId(infoResponse.getId());

String locale = infoResponse
.getPrefs()
.getPref()
.stream()
.filter(perf -> perf.getName().equals("zimbraPrefLocale"))
.findFirst()
.map(Pref::getValue)
.orElse("en");
// This old style try/catch is necessary because:
// - the system cannot trust the user locale since it can be set manually by the sysadmin
// and there is no check if the value is a valid one. So the LocaleUtils#toLocale method
// can raise an exception if the Locale is malformed.
// - the project doesn't have the Vavr dependency containing the Try construct to handle the
// exception in a cleaner way and I don't want to add it now only for this.
Locale locale;
try {
locale = infoResponse
.getPrefs()
.getPref()
.stream()
.filter(perf -> perf.getName().equals("zimbraPrefLocale"))
.findFirst()
.map(pref -> LocaleUtils.toLocale(pref.getValue()))
.orElse(Locale.ENGLISH);
} catch (IllegalArgumentException exception) {
logger.error(
"The user id {} has a locale with an invalid format. The system falls back in '{}'",
userId.getUserId(),
Locale.ENGLISH
);

locale = Locale.ENGLISH;
}

String fullName = infoResponse
.getAttrs()
Expand All @@ -175,7 +196,7 @@ public Optional<UserMyself> getMyselfByToken(String token) {
userMyself.setEmail(infoResponse.getName());
userMyself.setDomain(infoResponse.getPublicURL());
userMyself.setFullName(fullName);
userMyself.setLocale(Locale.valueOf(locale.toUpperCase()));
userMyself.setLocale(locale.toString());

return Optional.of(userMyself);

Expand Down
Expand Up @@ -8,7 +8,6 @@
import com.zextras.carbonio.user_management.Simulator;
import com.zextras.carbonio.user_management.Simulator.SimulatorBuilder;
import com.zextras.carbonio.user_management.SoapHttpUtils;
import com.zextras.carbonio.user_management.generated.model.Locale;
import com.zextras.carbonio.user_management.generated.model.UserMyself;
import org.assertj.core.api.Assertions;
import org.eclipse.jetty.http.HttpHeader;
Expand Down Expand Up @@ -72,7 +71,7 @@ void givenAValidUserCookieTheGetUserMyselfApiShouldReturnTheRequesterFullInfo()
"fake@example.com",
"example.com",
"Fake Account",
"en"
"pt_BR"
)
));

Expand All @@ -96,7 +95,7 @@ void givenAValidUserCookieTheGetUserMyselfApiShouldReturnTheRequesterFullInfo()
Assertions.assertThat(userMyself.getEmail()).isEqualTo("fake@example.com");
Assertions.assertThat(userMyself.getDomain()).isEqualTo("example.com");
Assertions.assertThat(userMyself.getFullName()).isEqualTo("Fake Account");
Assertions.assertThat(userMyself.getLocale()).isEqualTo(Locale.EN);
Assertions.assertThat(userMyself.getLocale()).isEqualTo("pt_BR");
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/resources/logback-test.xml
Expand Up @@ -17,5 +17,5 @@ SPDX-License-Identifier: AGPL-3.0-only
<appender-ref ref="STDOUT"/>
</root>

<logger name="org.mockserver.log.MockServerEventLog" level="INFO"/>
<logger name="org.mockserver.log.MockServerEventLog" level="WARN"/>
</configuration>
2 changes: 1 addition & 1 deletion generated/pom.xml
Expand Up @@ -16,7 +16,7 @@ SPDX-License-Identifier: AGPL-3.0-only
<parent>
<groupId>com.zextras.carbonio.user-management</groupId>
<artifactId>carbonio-user-management</artifactId>
<version>0.3.0-1</version>
<version>0.4.0-SNAPSHOT</version>
</parent>

<dependencies>
Expand Down
4 changes: 2 additions & 2 deletions package/PKGBUILD
Expand Up @@ -7,8 +7,8 @@ targets=(
"centos"
)
pkgname="carbonio-user-management"
pkgver="0.3.0"
pkgrel="1"
pkgver="0.4.0"
pkgrel="SNAPSHOT"
pkgdesc="Carbonio User Management"
pkgdesclong=(
"Carbonio User Management"
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -21,7 +21,7 @@ SPDX-License-Identifier: AGPL-3.0-only

<groupId>com.zextras.carbonio.user-management</groupId>
<artifactId>carbonio-user-management</artifactId>
<version>0.3.0-1</version>
<version>0.4.0-SNAPSHOT</version>
<packaging>pom</packaging>

<name>carbonio-user-management</name>
Expand Down
22 changes: 1 addition & 21 deletions resources/user-management.yaml
Expand Up @@ -234,29 +234,9 @@ components:
domain:
type: string
locale:
$ref: '#/components/schemas/Locale'
type: string
UserId:
type: object
properties:
userId:
type: string
Locale:
type: string
enum:
- DE
- EN
- ES
- FR
- HI
- IT
- JA
- NL
- PL
- PT
- PT_BR
- RO
- RU
- TH
- TR
- VI
- ZH_CN

0 comments on commit c7065ca

Please sign in to comment.