diff --git a/src/main/java/de/focusshift/zeiterfassung/avatar/AvatarController.java b/src/main/java/de/focusshift/zeiterfassung/avatar/AvatarController.java index 3a23c93be..e8d83ebc6 100644 --- a/src/main/java/de/focusshift/zeiterfassung/avatar/AvatarController.java +++ b/src/main/java/de/focusshift/zeiterfassung/avatar/AvatarController.java @@ -3,16 +3,15 @@ import org.springframework.http.CacheControl; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; -import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.RestController; import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; -@Controller +@RestController public class AvatarController { private final SvgService svgService; @@ -22,7 +21,6 @@ public class AvatarController { } @GetMapping(value = "/avatar", produces = "image/svg+xml") - @ResponseBody public ResponseEntity avatar(@RequestParam("name") String name, Locale locale) { final Map model = Map.of("initials", getInitials(name)); @@ -35,11 +33,14 @@ public ResponseEntity avatar(@RequestParam("name") String name, Locale l } private static String getInitials(String niceName) { - final int idxLastWhitespace = niceName.lastIndexOf(' '); + + final String normalizedNiceName = niceName.strip(); + + final int idxLastWhitespace = normalizedNiceName.lastIndexOf(' '); if (idxLastWhitespace == -1) { - return niceName.substring(0, 1).toUpperCase(); + return normalizedNiceName.substring(0, 1).toUpperCase(); } - return (niceName.charAt(0) + niceName.substring(idxLastWhitespace + 1, idxLastWhitespace + 2)).toUpperCase(); + return (normalizedNiceName.charAt(0) + normalizedNiceName.substring(idxLastWhitespace + 1, idxLastWhitespace + 2)).toUpperCase(); } } diff --git a/src/main/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImpl.java b/src/main/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImpl.java index 5653546a4..e72a38c81 100644 --- a/src/main/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImpl.java +++ b/src/main/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImpl.java @@ -39,7 +39,7 @@ public TenantUser createNewUser(String uuid, String givenName, String familyName final Instant now = clock.instant(); final TenantUserEntity tenantUserEntity = - new TenantUserEntity(null, uuid, now, now, givenName, familyName, eMailAddress.value(), distinct(authorities), now, now, null, null, UserStatus.ACTIVE); + new TenantUserEntity(null, uuid, now, now, givenName.strip(), familyName.strip(), eMailAddress.value(), distinct(authorities), now, now, null, null, UserStatus.ACTIVE); final TenantUserEntity persisted = tenantUserRepository.save(tenantUserEntity); @@ -56,8 +56,13 @@ public TenantUser updateUser(TenantUser user) { final TenantUserEntity current = getTenantUserOrThrow(user.localId()); + final String givenName = user.givenName().strip(); + final String familyName = user.familyName().strip(); + final String email = user.eMail().value(); + final Set authorities = distinct(user.authorities()); + final TenantUserEntity next = - new TenantUserEntity(current.getId(), current.getUuid(), current.getFirstLoginAt(), now, user.givenName(), user.familyName(), user.eMail().value(), distinct(user.authorities()), current.getCreatedAt(), now, current.getDeactivatedAt(), current.getDeletedAt(), current.getStatus()); + new TenantUserEntity(current.getId(), current.getUuid(), current.getFirstLoginAt(), now, givenName, familyName, email, authorities, current.getCreatedAt(), now, current.getDeactivatedAt(), current.getDeletedAt(), current.getStatus()); final TenantUserEntity persisted = tenantUserRepository.save(next); diff --git a/src/main/resources/db/changelog/changelog-2.14.0-remove-leading-trailing-whitespaces-username.xml b/src/main/resources/db/changelog/changelog-2.14.0-remove-leading-trailing-whitespaces-username.xml new file mode 100644 index 000000000..0565dd0c3 --- /dev/null +++ b/src/main/resources/db/changelog/changelog-2.14.0-remove-leading-trailing-whitespaces-username.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + given_name LIKE ' %' OR given_name LIKE '% ' + + + + + + + + + + + + + family_name LIKE ' %' OR family_name LIKE '% ' + + + + + + diff --git a/src/main/resources/db/changelog/db.changelog-main.xml b/src/main/resources/db/changelog/db.changelog-main.xml index 5598ba768..f51c3e0d4 100644 --- a/src/main/resources/db/changelog/db.changelog-main.xml +++ b/src/main/resources/db/changelog/db.changelog-main.xml @@ -29,5 +29,6 @@ + diff --git a/src/test/java/de/focusshift/zeiterfassung/avatar/AvatarControllerTest.java b/src/test/java/de/focusshift/zeiterfassung/avatar/AvatarControllerTest.java index ced8b4e71..82e278ec0 100644 --- a/src/test/java/de/focusshift/zeiterfassung/avatar/AvatarControllerTest.java +++ b/src/test/java/de/focusshift/zeiterfassung/avatar/AvatarControllerTest.java @@ -37,6 +37,7 @@ void setUp() { "The Batman,TB", "Batman,B", "The ultimate Batman,TB", + "' normal one ',NO", }) void ensureGeneratesAvatarWithInitials(String name, String expectedInitials) throws Exception { diff --git a/src/test/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImplTest.java b/src/test/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImplTest.java index 90763507c..46762b1c9 100644 --- a/src/test/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImplTest.java +++ b/src/test/java/de/focusshift/zeiterfassung/tenancy/user/TenantUserServiceImplTest.java @@ -30,28 +30,14 @@ class TenantUserServiceImplTest { private final Clock clock = Clock.fixed(Instant.now(), ZoneId.systemDefault()); + private TenantUserServiceImpl sut; + @Mock private TenantUserRepository repository; @Mock private ApplicationEventPublisher publisher; - private static TenantUser activeTenantUserOne(Instant now) { - return new TenantUser("my-external-id-1", 1L, "batman", "batman", new EMailAddress("batman@batman.com"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE); - } - - private static TenantUserEntity activeUserEntityOne(Instant now) { - return new TenantUserEntity(1L, "my-external-id-1", now, now, "batman", "batman", "batman@batman.com", Set.of(), now, now, null, null, UserStatus.ACTIVE); - } - - private static TenantUser activeTenantUserTwo(Instant now) { - return new TenantUser("my-external-id-2", 2L, "petra", "panter", new EMailAddress("petra@panter.com"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE); - } - - private static TenantUserEntity activeUserEntityTwo(Instant now) { - return new TenantUserEntity(2L, "my-external-id-2", now, now, "petra", "panter", "petra@panter.com", Set.of(), now, now, null, null, UserStatus.ACTIVE); - } - @BeforeEach void setUp() { sut = new TenantUserServiceImpl(repository, publisher, clock); @@ -237,6 +223,30 @@ void newUserIsCreatedSuccessfully() { verify(publisher).publishEvent(any(TenantUserCreatedEvent.class)); } + + @Test + void ensureGivenNameAndFamilyNameIsStripped() { + + final String uuid = "my-external-id"; + final String givenName = " Hans Joachim "; + final String familyName = " Sonnencreme "; + final EMailAddress eMailAddress = new EMailAddress("hans@example.com"); + + when(repository.save(any(TenantUserEntity.class))).thenAnswer(returnsFirstArg()); + + final TenantUser result = sut.createNewUser(uuid, givenName, familyName, eMailAddress, Set.of()); + + final ArgumentCaptor entityArgumentCaptor = ArgumentCaptor.forClass(TenantUserEntity.class); + verify(repository).save(entityArgumentCaptor.capture()); + + assertThat(entityArgumentCaptor.getValue()).satisfies(entity -> { + assertThat(entity.getGivenName()).isEqualTo("Hans Joachim"); + assertThat(entity.getFamilyName()).isEqualTo("Sonnencreme"); + }); + + assertThat(result.givenName()).isEqualTo("Hans Joachim"); + assertThat(result.familyName()).isEqualTo("Sonnencreme"); + } } @Nested @@ -258,7 +268,6 @@ void updateUserSuccessfully() { TenantUser result = sut.updateUser(update); - ArgumentCaptor entityArgumentCaptor = ArgumentCaptor.forClass(TenantUserEntity.class); verify(repository).save(entityArgumentCaptor.capture()); @@ -288,6 +297,31 @@ void updateUserSuccessfully() { }); } + @Test + void ensureGivenNameAndFamilyNameIsStripped() { + + final Instant now = clock.instant(); + + final TenantUserEntity currentEntity = activeUserEntityOne(now); + final TenantUser update = updateFromEntity(currentEntity, " Hans Joachim ", " Sonnencreme ", "hans@example.com"); + + when(repository.findById(any())).thenReturn(Optional.of(currentEntity)); + when(repository.save(any(TenantUserEntity.class))).thenAnswer(returnsFirstArg()); + + final TenantUser result = sut.updateUser(update); + + final ArgumentCaptor entityArgumentCaptor = ArgumentCaptor.forClass(TenantUserEntity.class); + verify(repository).save(entityArgumentCaptor.capture()); + + assertThat(entityArgumentCaptor.getValue()).satisfies(entity -> { + assertThat(entity.getGivenName()).isEqualTo("Hans Joachim"); + assertThat(entity.getFamilyName()).isEqualTo("Sonnencreme"); + }); + + assertThat(result.givenName()).isEqualTo("Hans Joachim"); + assertThat(result.familyName()).isEqualTo("Sonnencreme"); + } + @Test void updateUserThrowsExceptionWhenUserNotFound() { TenantUser user = activeTenantUserOne(Instant.now()); @@ -413,8 +447,22 @@ void deactivateUserThrowsExceptionWhenUserNotFound() { verify(repository).findById(userId); } - } + } + private static TenantUser activeTenantUserOne(Instant now) { + return new TenantUser("my-external-id-1", 1L, "batman", "batman", new EMailAddress("batman@batman.com"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE); + } + + private static TenantUserEntity activeUserEntityOne(Instant now) { + return new TenantUserEntity(1L, "my-external-id-1", now, now, "batman", "batman", "batman@batman.com", Set.of(), now, now, null, null, UserStatus.ACTIVE); + } + + private static TenantUser activeTenantUserTwo(Instant now) { + return new TenantUser("my-external-id-2", 2L, "petra", "panter", new EMailAddress("petra@panter.com"), now, Set.of(), now, now, null, null, UserStatus.ACTIVE); + } + + private static TenantUserEntity activeUserEntityTwo(Instant now) { + return new TenantUserEntity(2L, "my-external-id-2", now, now, "petra", "panter", "petra@panter.com", Set.of(), now, now, null, null, UserStatus.ACTIVE); } }