diff --git a/user-manager-server/src/main/java/de/itvsh/kop/user/UserRepository.java b/user-manager-server/src/main/java/de/itvsh/kop/user/UserRepository.java index 90cea9fbbf9c08de72ccd4c8b835203aecce159b..633f3c7661f3c890587008c0a01c12f68aaa75d7 100644 --- a/user-manager-server/src/main/java/de/itvsh/kop/user/UserRepository.java +++ b/user-manager-server/src/main/java/de/itvsh/kop/user/UserRepository.java @@ -62,7 +62,10 @@ class UserRepository implements PanacheMongoRepository<User> { } public Optional<User> findById(String id) { - return findByIdOptional(new ObjectId(id)); + return Optional.ofNullable(id) + .filter(ObjectId::isValid) + .map(ObjectId::new) + .flatMap(this::findByIdOptional); } public Optional<User> findByEmail(String email) { diff --git a/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsRepository.java b/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsRepository.java index 9491fc4dbff13d2d7192130c462c36d7b93e1eee..b90a0114e73a52c2972623900f4d12f71ddb4a84 100644 --- a/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsRepository.java +++ b/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsRepository.java @@ -23,11 +23,13 @@ */ package de.itvsh.kop.user.settings; +import java.util.Optional; + import javax.enterprise.context.ApplicationScoped; -import de.itvsh.kop.common.logging.KopLogging; import org.bson.types.ObjectId; +import de.itvsh.kop.common.logging.KopLogging; import de.itvsh.kop.user.User; import io.quarkus.mongodb.panache.PanacheMongoRepository; @@ -35,10 +37,14 @@ import io.quarkus.mongodb.panache.PanacheMongoRepository; @KopLogging class UserSettingsRepository implements PanacheMongoRepository<User> { - public UserSettings updateByUserId(String userId, UserSettings userSettings) { - var savedUser = findById(new ObjectId(userId)); - savedUser.setUserSettings(userSettings); - update(savedUser); - return findById(savedUser.getId()).getUserSettings(); + public Optional<UserSettings> updateByUserId(String userId, UserSettings userSettings) { + return Optional.ofNullable(userId) + .filter(ObjectId::isValid) + .map(ObjectId::new) + .flatMap(id -> findByIdOptional(id).map(user -> { + user.setUserSettings(userSettings); + update(user); + return findById(user.getId()).getUserSettings(); + })); } } \ No newline at end of file diff --git a/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsResource.java b/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsResource.java index ec5d1ddd723cb0e8bf73b19f5fdf6af72c8c80fa..37cd684ddd72926e4e6410c84166827636243c52 100644 --- a/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsResource.java +++ b/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsResource.java @@ -85,9 +85,10 @@ public class UserSettingsResource { if (Objects.isNull(userSettings)) { throw new FunctionalException(() -> "Request Body missing."); } - var updatedSettings = userSettingsService.updateByUserId(userSettings, userId); - return resourceAssembler.toResource(updatedSettings, userId, userManagerUrl); + return userSettingsService.updateByUserId(userSettings, userId) + .map(updatedSettings -> resourceAssembler.toResource(updatedSettings, userId, userManagerUrl)) + .orElseThrow(() -> new FunctionalException(() -> "Invalid user id")); } void checkUserAccess(String userId) { diff --git a/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsService.java b/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsService.java index a22acb22fc7091f2425d6f72ef0029546c368293..986bf650740189eca48e31dbcaa0c69e13c2cb01 100644 --- a/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsService.java +++ b/user-manager-server/src/main/java/de/itvsh/kop/user/settings/UserSettingsService.java @@ -24,6 +24,7 @@ package de.itvsh.kop.user.settings; import java.util.Objects; +import java.util.Optional; import javax.enterprise.context.ApplicationScoped; import javax.inject.Inject; @@ -39,7 +40,7 @@ class UserSettingsService { @Inject UserSettingsRepository repository; - public UserSettings updateByUserId(UserSettings userSettings, String userId) { + public Optional<UserSettings> updateByUserId(UserSettings userSettings, String userId) { return repository.updateByUserId(userId, userSettings); } diff --git a/user-manager-server/src/test/java/de/itvsh/kop/user/UserRepositoryITCase.java b/user-manager-server/src/test/java/de/itvsh/kop/user/UserRepositoryITCase.java index f3accf3bf63a10d46e7fb2418f52394c73f32802..e469f0c9ce5444f1d8de8f3854e27c52cdef339c 100644 --- a/user-manager-server/src/test/java/de/itvsh/kop/user/UserRepositoryITCase.java +++ b/user-manager-server/src/test/java/de/itvsh/kop/user/UserRepositoryITCase.java @@ -297,6 +297,26 @@ class UserRepositoryITCase { assertThat(user).isNotNull(); assertThat(user.get()).usingRecursiveComparison().isEqualTo(UserTestFactory.create()); } + + @Test + void shouldNotFindIfIdNull() { + assertThat(repository.findById((String) null)).isEmpty(); + } + + @Test + void shouldNotFindIfEmptyId() { + assertThat(repository.findById("")).isEmpty(); + } + + @Test + void shouldNotFindIfBlankId() { + assertThat(repository.findById(" ")).isEmpty(); + } + + @Test + void shouldNotFindIfInvalidId() { + assertThat(repository.findById("wrong_id")).isEmpty(); + } } @DisplayName("Find by email") diff --git a/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsRepositoryITCase.java b/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsRepositoryITCase.java new file mode 100644 index 0000000000000000000000000000000000000000..98ba6e05eae2b41b8916532f998e7f8b3a2c56a8 --- /dev/null +++ b/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsRepositoryITCase.java @@ -0,0 +1,84 @@ +package de.itvsh.kop.user.settings; + +import static org.assertj.core.api.Assertions.*; + +import javax.inject.Inject; + +import org.bson.types.ObjectId; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import de.itvsh.kop.user.UserTestFactory; +import de.itvsh.kop.user.common.MongoDbTestProfile; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; + +@QuarkusTest +@TestProfile(MongoDbTestProfile.class) +class UserSettingsRepositoryITCase { + + @Inject + UserSettingsRepository repository; + + @Nested + class TestUpdateByUserId { + + @BeforeEach + void beforeEach() { + repository.deleteAll(); + } + + @Test + void shouldUpdateUserSettings() { + var user = UserTestFactory.create(); + repository.persist(user); + + var settings = repository.updateByUserId( + user.getId().toString(), + UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build()); + + assertThat(settings) + .isPresent() + .map(UserSettings::getNotificationsSendFor) + .hasValue(NotificationsSendFor.ALL); + } + + @Test + void shouldReturnEmptyIfUserNotExist() { + var user = UserTestFactory.create(); + repository.persist(user); + + var settings = repository.updateByUserId( + new ObjectId().toString(), + UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build()); + + assertThat(settings).isEmpty(); + } + + @Test + void shouldReturnEmptyIfInvalidUserId() { + var user = UserTestFactory.create(); + repository.persist(user); + + var settings = repository.updateByUserId( + "wrong_id", + UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build()); + + assertThat(settings).isEmpty(); + } + + @Test + void shouldReturnEmptyIfUserIdNull() { + var user = UserTestFactory.create(); + repository.persist(user); + + var settings = repository.updateByUserId( + null, + UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build()); + + assertThat(settings).isEmpty(); + } + } + +} \ No newline at end of file diff --git a/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsResourceTest.java b/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsResourceTest.java index f3e3f10ee3f2a1d586bbfd1dae1108e3ef9c1ae9..7e4cc5d13c96a7b6738346fe85e8ee1423ec87f3 100644 --- a/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsResourceTest.java +++ b/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsResourceTest.java @@ -27,6 +27,7 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; +import java.util.Optional; import java.util.UUID; import javax.ws.rs.core.UriInfo; @@ -49,6 +50,7 @@ import de.itvsh.kop.user.UserService; import de.itvsh.kop.user.UserTestFactory; import de.itvsh.kop.user.common.errorhandling.AccessForbiddenException; import de.itvsh.kop.user.common.errorhandling.FunctionalException; +import io.quarkus.hal.HalEntityWrapper; class UserSettingsResourceTest { @@ -133,6 +135,30 @@ class UserSettingsResourceTest { } } + @Nested + class TestUserIdNotExist { + @BeforeEach + void mockAccess() { + doNothing().when(resource).checkUserAccess(anyString()); + } + + @Test + void shouldThrowFunctionalExceptionIfInvalidUserId() { + var user = UserSettingsTestFactory.create(); + assertThatExceptionOfType(FunctionalException.class) + .isThrownBy(() -> resource.updateUserSettings("wrong_id", user)) + .withMessageStartingWith("Functional error: Invalid user id"); + } + + @Test + void shouldThrowFunctionalExceptionIfEmptyUserId() { + var user = UserSettingsTestFactory.create(); + assertThatExceptionOfType(FunctionalException.class) + .isThrownBy(() -> resource.updateUserSettings("", user)) + .withMessageStartingWith("Functional error: Invalid user id"); + } + } + @DisplayName("with filled body") @Nested class TestFilledBody { @@ -140,13 +166,14 @@ class UserSettingsResourceTest { @BeforeEach void mockAccess() { doNothing().when(resource).checkUserAccess(anyString()); - when(userSettingsService.updateByUserId(any(UserSettings.class), anyString())).thenReturn(updatedUserSettings); + when(userSettingsService.updateByUserId(any(UserSettings.class), anyString())).thenReturn(Optional.of(updatedUserSettings)); mockUserManagerUrl(); } @Test void shouldCheckAccess() { + when(resourceAssembler.toResource(any(), anyString(), anyString())).thenReturn(new HalEntityWrapper(null)); resource.updateUserSettings(USER_ID, userSettings); verify(resource).checkUserAccess(USER_ID); @@ -154,13 +181,16 @@ class UserSettingsResourceTest { @Test void shouldCallUserSettingsService() { + when(resourceAssembler.toResource(any(), anyString(), anyString())).thenReturn(new HalEntityWrapper(null)); resource.updateUserSettings(USER_ID, userSettings); verify(userSettingsService).updateByUserId(userSettings, USER_ID); } @Test - void shouldCallResourceAssembler() { + void + shouldCallResourceAssembler() { + when(resourceAssembler.toResource(any(), anyString(), anyString())).thenReturn(new HalEntityWrapper(null)); resource.updateUserSettings(USER_ID, userSettings); verify(resourceAssembler).toResource(updatedUserSettings, USER_ID, USER_MANAGER_URL); diff --git a/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsServiceITCase.java b/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsServiceITCase.java index d4a77869e5499e84b3230dc42e5e9a8d167c36e7..47114b6455a1ee15b92be7076e9587983c8f1b3a 100644 --- a/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsServiceITCase.java +++ b/user-manager-server/src/test/java/de/itvsh/kop/user/settings/UserSettingsServiceITCase.java @@ -68,7 +68,10 @@ class UserSettingsServiceITCase { var savedUserSettings = service.updateByUserId(userSettingsToSave, UserTestFactory.ID.toHexString()); - assertThat(savedUserSettings.getNotificationsSendFor()).isEqualTo(NotificationsSendFor.ALL); + assertThat(savedUserSettings) + .isPresent() + .map(UserSettings::getNotificationsSendFor) + .hasValue(NotificationsSendFor.ALL); } } }