fix avatar initials for usernames with whitespace (#971)

closes #970

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<String> avatar(@RequestParam("name") String name, Locale locale) {
 
         final Map<String, Object> model = Map.of("initials", getInitials(name));
@@ -35,11 +33,14 @@ public ResponseEntity<String> 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();
     }
 }

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<SecurityRole> 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);
 

src/main/resources/db/changelog/changelog-2.14.0-remove-leading-trailing-whitespaces-username.xml

@@ -0,0 +1,33 @@
+<?xml version="1.1" encoding="UTF-8" standalone="no"?>
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.5.xsd">
+
+  <changeSet author="seber" id="remove-leading-trailing-whitespaces-given-name">
+    <preConditions>
+      <tableExists tableName="tenant_user"/>
+      <columnExists tableName="tenant_user" columnName="given_name"/>
+    </preConditions>
+    <update tableName="tenant_user">
+      <column name="given_name" value="TRIM(given_name)" />
+      <where>
+        given_name LIKE ' %' OR given_name LIKE '% '
+      </where>
+    </update>
+  </changeSet>
+
+  <changeSet author="seber" id="remove-leading-trailing-whitespaces-family-name">
+    <preConditions>
+      <tableExists tableName="tenant_user"/>
+      <columnExists tableName="tenant_user" columnName="family_name"/>
+    </preConditions>
+    <update tableName="tenant_user">
+      <column name="family_name" value="TRIM(family_name)" />
+      <where>
+        family_name LIKE ' %' OR family_name LIKE '% '
+      </where>
+    </update>
+  </changeSet>
+
+</databaseChangeLog>
+

src/main/resources/db/changelog/db.changelog-main.xml

@@ -29,5 +29,6 @@
   <include relativeToChangelogFile="true" file="changelog-2.5.0-delete-tenant-user-cascade.xml"/>
   <include relativeToChangelogFile="true" file="changelog-2.7.0-add-user-status.xml"/>
   <include relativeToChangelogFile="true" file="changelog-2.6.4-enable-row-level-security-on-absence.xml"/>
+  <include relativeToChangelogFile="true" file="changelog-2.14.0-remove-leading-trailing-whitespaces-username.xml"/>
 
 </databaseChangeLog>

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 {
 

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("[email protected]"), 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", "[email protected]", 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("[email protected]"), 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", "[email protected]", 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("[email protected]");
+
+                when(repository.save(any(TenantUserEntity.class))).thenAnswer(returnsFirstArg());
+
+                final TenantUser result = sut.createNewUser(uuid, givenName, familyName, eMailAddress, Set.of());
+
+                final ArgumentCaptor<TenantUserEntity> 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<TenantUserEntity> 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  ", "[email protected]");
+
+                when(repository.findById(any())).thenReturn(Optional.of(currentEntity));
+                when(repository.save(any(TenantUserEntity.class))).thenAnswer(returnsFirstArg());
+
+                final TenantUser result = sut.updateUser(update);
+
+                final ArgumentCaptor<TenantUserEntity> 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("[email protected]"), 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", "[email protected]", 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("[email protected]"), 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", "[email protected]", Set.of(), now, now, null, null, UserStatus.ACTIVE);
     }
 }