refactor(settings): one load-change-save update for all settings

readme.md

@@ -134,6 +134,8 @@ layers:
 `user` owns the tables `users` and `user_settings` through the one JPA entity `UserEntity`. `login` and
 `registration` read and write `users` with `JdbcTemplate` (refresh tokens stay JPA). Every query for "a user who
 may log in and is shown to others" uses `shared/account/ActiveUser`.
+A settings change is one `UserRepository.update(userId, change)`: load the user, change the model, write back the
+display name and the settings. A new setting needs no new repository method.
 `profile` and `stats` only read data. They have no entities and use `JdbcTemplate` queries in a `repo` package.
 `version` has only an `api` package: `GET /api/version` answers the commit and build time of the running image.
 `account` (delete and nightly cleanup) sets the flag with `JdbcTemplate` in a `repo` package too, and calls Core

src/main/java/auth_service/modules/user/persistence/UserRepository.java

@@ -1,15 +1,11 @@
 package auth_service.modules.user.persistence;
 
-import auth_service.modules.user.model.Appearance;
-import auth_service.modules.user.model.ProfileStyle;
-import auth_service.modules.user.model.Tile;
 import auth_service.modules.user.model.SteamIdHolder;
 import auth_service.modules.user.model.User;
-import auth_service.modules.user.model.UserSettings;
 import jakarta.transaction.Transactional;
 
-import java.util.List;
 import java.util.Optional;
+import java.util.function.Consumer;
 
 /**
  * Repository for database operations regarding the User entity.
@@ -20,19 +16,11 @@ public interface UserRepository {
      */
     Optional<User> findById(long userId);
 
+    /**
+     * Stores only the display name and the settings of the changed user.
+     */
     @Transactional
-    void updateSettings(long userId, String displayName, UserSettings settings);
+    void update(long userId, Consumer<User> change);
-
-    @Transactional
-    void updateAppearance(long userId, Appearance appearance);
-
-    void updateDashboardTiles(long userId, List<Tile> tiles);
-
-    void updateProfileTiles(long userId, List<Tile> tiles);
-
-    void updateFavoritePlatform(long userId, String platform);
-
-    void updateProfileStyle(long userId, ProfileStyle style);
 
     Optional<SteamIdHolder> findSteamIdHolder(long steamId);
 

src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java

@@ -1,14 +1,9 @@
 package auth_service.modules.user.persistence;
 
 import auth_service.modules.user.exception.UserNotFoundException;
-import auth_service.modules.user.model.Appearance;
-import auth_service.modules.user.model.ProfileStyle;
-import auth_service.modules.user.model.Tile;
 import auth_service.modules.user.model.SteamIdHolder;
 import auth_service.modules.user.model.User;
-import auth_service.modules.user.model.UserSettings;
 import auth_service.modules.user.persistence.entities.UserEntity;
-import auth_service.modules.user.persistence.entities.UserSettingsEntity;
 import auth_service.modules.user.persistence.mapper.EntityMapper;
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.PersistenceContext;
@@ -16,8 +11,8 @@ import jakarta.persistence.TypedQuery;
 import jakarta.transaction.Transactional;
 import org.springframework.stereotype.Repository;
 
-import java.util.List;
 import java.util.Optional;
+import java.util.function.Consumer;
 
 @Repository
 public class UserRepositoryImpl implements UserRepository {
@@ -44,66 +39,14 @@ public class UserRepositoryImpl implements UserRepository {
 
     @Transactional
     @Override
-    public void updateSettings(long userId, String displayName, UserSettings settings) {
+    public void update(long userId, Consumer<User> change) {
-        UserSettingsEntity settingsEntity = settingsOf(userId);
-        settingsEntity.setLanguage(settings.getLanguage());
-        settingsEntity.setTheme(settings.getTheme());
-        settingsEntity.setPublicProfile(settings.isPublicProfile());
-
-        // settingsOf loaded the user: find answers from the persistence context.
-        entityManager.find(UserEntity.class, userId).setDisplayName(displayName);
-    }
-
-    @Transactional
-    @Override
-    public void updateAppearance(long userId, Appearance appearance) {
-        UserSettingsEntity settingsEntity = settingsOf(userId);
-        settingsEntity.setAccent(appearance.accent());
-        settingsEntity.setCorners(appearance.corners());
-        settingsEntity.setWideLayout(appearance.wideLayout());
-        settingsEntity.setBackgroundAppid(appearance.backgroundAppid());
-        settingsEntity.setBannerAppid(appearance.bannerAppid());
-        settingsEntity.setShowBanner(appearance.showBanner());
-    }
-
-    @Transactional
-    @Override
-    public void updateDashboardTiles(long userId, List<Tile> tiles) {
-        settingsOf(userId).setDashboardTiles(tiles == null ? null : Tile.storedAll(tiles));
-    }
-
-    @Transactional
-    @Override
-    public void updateProfileTiles(long userId, List<Tile> tiles) {
-        settingsOf(userId).setProfileTiles(Tile.storedAll(tiles));
-    }
-
-    @Transactional
-    @Override
-    public void updateFavoritePlatform(long userId, String platform) {
-        settingsOf(userId).setFavoritePlatform(platform);
-    }
-
-    @Transactional
-    @Override
-    public void updateProfileStyle(long userId, ProfileStyle style) {
-        UserSettingsEntity settingsEntity = settingsOf(userId);
-        settingsEntity.setProfileAccent(style.accent());
-        settingsEntity.setProfileCorners(style.corners());
-        settingsEntity.setProfileBannerAppid(style.bannerAppid());
-        settingsEntity.setProfileBackgroundAppid(style.backgroundAppid());
-    }
-
-    // Every user has a settings row since V20; registration inserts it.
-    private UserSettingsEntity settingsOf(long userId) {
         UserEntity userEntity = entityManager.find(UserEntity.class, userId);
         if (userEntity == null) {
             throw new UserNotFoundException("Could not find user with id " + userId);
         }
-        if (userEntity.getSettings() == null) {
+        User user = EntityMapper.mapToModel(userEntity);
-            throw new IllegalStateException("User " + userId + " has no settings row");
+        change.accept(user);
-        }
+        EntityMapper.apply(user, userEntity);
-        return userEntity.getSettings();
     }
 
     @Override

src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java

@@ -50,4 +50,29 @@ public abstract class EntityMapper {
                 .settings(settingsModel)
                 .build();
     }
+
+    public static void apply(User user, UserEntity entity) {
+        entity.setDisplayName(user.getDisplayName());
+        UserSettings settings = user.getSettings();
+        UserSettingsEntity target = entity.getSettings();
+        target.setLanguage(settings.getLanguage());
+        target.setTheme(settings.getTheme());
+        target.setPublicProfile(settings.isPublicProfile());
+        Appearance appearance = settings.getAppearance();
+        target.setAccent(appearance.accent());
+        target.setCorners(appearance.corners());
+        target.setWideLayout(appearance.wideLayout());
+        target.setBackgroundAppid(appearance.backgroundAppid());
+        target.setBannerAppid(appearance.bannerAppid());
+        target.setShowBanner(appearance.showBanner());
+        ProfileStyle style = settings.getProfileStyle();
+        target.setProfileAccent(style.accent());
+        target.setProfileCorners(style.corners());
+        target.setProfileBannerAppid(style.bannerAppid());
+        target.setProfileBackgroundAppid(style.backgroundAppid());
+        target.setDashboardTiles(settings.getDashboardTiles() == null ? null
+                : Tile.storedAll(settings.getDashboardTiles()));
+        target.setProfileTiles(Tile.storedAll(settings.getProfileTiles()));
+        target.setFavoritePlatform(settings.getFavoritePlatform());
+    }
 }

src/main/java/auth_service/modules/user/service/UserService.java

@@ -47,7 +47,12 @@ public class UserService {
         if (displayName != null && userRepository.isDisplayNameUsedByOtherUser(userId, displayName)) {
             throw new DisplayNameAlreadyInUseException("The display name is already in use.");
         }
-        userRepository.updateSettings(userId, displayName, userSettings);
+        userRepository.update(userId, user -> {
+            user.setDisplayName(displayName);
+            user.getSettings().setLanguage(userSettings.getLanguage());
+            user.getSettings().setTheme(userSettings.getTheme());
+            user.getSettings().setPublicProfile(userSettings.isPublicProfile());
+        });
         log.info("User settings updated for user {}", userId);
     }
 
@@ -90,34 +95,36 @@ public class UserService {
     @Transactional
     @CacheEvict(value = "users", key = "#userId")
     public void updateAppearance(long userId, Appearance appearance) {
-        userRepository.updateAppearance(userId, appearance);
+        userRepository.update(userId, user -> user.getSettings().setAppearance(appearance));
     }
 
     @RequiresRole(Role.USER)
     @Transactional
     @CacheEvict(value = "users", key = "#userId")
     public void updateDashboardTiles(long userId, List<Tile> tiles) {
-        userRepository.updateDashboardTiles(userId, tiles == null ? null : Tile.distinct(tiles));
+        userRepository.update(userId,
+                user -> user.getSettings().setDashboardTiles(tiles == null ? null : Tile.distinct(tiles)));
     }
 
     @RequiresRole(Role.USER)
     @Transactional
     @CacheEvict(value = "users", key = "#userId")
     public void updateProfileTiles(long userId, List<Tile> tiles) {
-        userRepository.updateProfileTiles(userId, Tile.distinct(tiles));
+        userRepository.update(userId, user -> user.getSettings().setProfileTiles(Tile.distinct(tiles)));
     }
 
     @RequiresRole(Role.USER)
     @Transactional
     @CacheEvict(value = "users", key = "#userId")
     public void updateFavoritePlatform(long userId, String platform) {
-        userRepository.updateFavoritePlatform(userId, platform == null ? null : platform.strip());
+        userRepository.update(userId,
+                user -> user.getSettings().setFavoritePlatform(platform == null ? null : platform.strip()));
     }
 
     @RequiresRole(Role.USER)
     @Transactional
     @CacheEvict(value = "users", key = "#userId")
     public void updateProfileStyle(long userId, ProfileStyle style) {
-        userRepository.updateProfileStyle(userId, style);
+        userRepository.update(userId, user -> user.getSettings().setProfileStyle(style));
     }
 }

src/test/java/auth_service/modules/user/persistence/UserRepositoryTest.java

@@ -67,7 +67,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
         // The delete request commits after the settings request loaded the user.
         jdbc.update("UPDATE users SET deleted_at = NOW() WHERE id = ?", user.getId());
 
-        userRepository.updateSettings(user.getId(), "RaceName", UserSettings.builder().language("de").theme("dark").build());
+        updateSettings(user.getId(), "RaceName", UserSettings.builder().language("de").theme("dark").build());
         entityManager.flush();
 
         assertNotNull(jdbc.queryForObject("SELECT deleted_at FROM users WHERE id = ?", Object.class, user.getId()));
@@ -113,7 +113,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
                 .build();
 
         // Act
-        userRepository.updateSettings(userEntity.getId(), null, settings);
+        updateSettings(userEntity.getId(), null, settings);
 
         // Assert: the Steam ID changes only through the Steam link
         var updatedUser = entityManager.find(UserEntity.class, userEntity.getId());
@@ -197,7 +197,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
     void updateSettings_UnknownUser() {
         var settings = UserSettings.builder().language("en").theme("light").build();
 
-        assertThrows(UserNotFoundException.class, () -> userRepository.updateSettings(-1L, null, settings));
+        assertThrows(UserNotFoundException.class, () -> updateSettings(-1L, null, settings));
     }
 
     @Test
@@ -224,7 +224,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
         var settings = UserSettings.builder().language("en").theme("light").build();
 
         // Act
-        userRepository.updateSettings(owner.getId(), "Lobo", settings);
+        updateSettings(owner.getId(), "Lobo", settings);
         entityManager.flush();
 
         // Assert
@@ -243,7 +243,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
                 .registrationDate(LocalDateTime.now())
                 .build());
 
-        userRepository.updateSettings(userEntity.getId(), "Lobo",
+        updateSettings(userEntity.getId(), "Lobo",
                 UserSettings.builder().language("de").theme("dark").publicProfile(true).build());
         entityManager.flush();
         entityManager.clear();
@@ -259,10 +259,11 @@ class UserRepositoryTest extends AbstractPersistenceTest {
                 .enabled(true)
                 .registrationDate(LocalDateTime.now())
                 .build());
-        userRepository.updateAppearance(userEntity.getId(), new Appearance("pink", "sharp", false, 220L, null, true));
+        userRepository.update(userEntity.getId(),
+                user -> user.getSettings().setAppearance(new Appearance("pink", "sharp", false, 220L, null, true)));
         entityManager.flush();
 
-        userRepository.updateSettings(userEntity.getId(), null,
+        updateSettings(userEntity.getId(), null,
                 UserSettings.builder().language("de").theme("dark").build());
         entityManager.flush();
         entityManager.clear();
@@ -283,7 +284,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
                 .getSettings().getProfileStyle());
 
         ProfileStyle style = new ProfileStyle("pink", "sharp", 620L, 1086940L);
-        userRepository.updateProfileStyle(userEntity.getId(), style);
+        userRepository.update(userEntity.getId(), user -> user.getSettings().setProfileStyle(style));
         entityManager.flush();
         entityManager.clear();
 
@@ -304,10 +305,10 @@ class UserRepositoryTest extends AbstractPersistenceTest {
 
         List<Tile> dashboard = List.of(new Tile("STATS_TREND", "L"), new Tile("PLAYING", "S"));
         List<Tile> profile = List.of(new Tile("STATS_PILE", "M"), new Tile("SUMMARY", "S"));
-        userRepository.updateDashboardTiles(userEntity.getId(), dashboard);
+        userRepository.update(userEntity.getId(), user -> user.getSettings().setDashboardTiles(dashboard));
-        userRepository.updateProfileTiles(userEntity.getId(), profile);
+        userRepository.update(userEntity.getId(), user -> user.getSettings().setProfileTiles(profile));
         entityManager.flush();
-        userRepository.updateSettings(userEntity.getId(), null,
+        updateSettings(userEntity.getId(), null,
                 UserSettings.builder().language("de").theme("dark").build());
         entityManager.flush();
         entityManager.clear();
@@ -327,13 +328,13 @@ class UserRepositoryTest extends AbstractPersistenceTest {
                 .build());
         assertNull(findByEmail("favorite@example.com").orElseThrow().getSettings().getFavoritePlatform());
 
-        userRepository.updateFavoritePlatform(userEntity.getId(), "Switch");
+        userRepository.update(userEntity.getId(), user -> user.getSettings().setFavoritePlatform("Switch"));
         entityManager.flush();
         entityManager.clear();
         assertEquals("Switch", findByEmail("favorite@example.com").orElseThrow().getSettings()
                 .getFavoritePlatform());
 
-        userRepository.updateFavoritePlatform(userEntity.getId(), null);
+        userRepository.update(userEntity.getId(), user -> user.getSettings().setFavoritePlatform(null));
         entityManager.flush();
         entityManager.clear();
         assertNull(findByEmail("favorite@example.com").orElseThrow().getSettings().getFavoritePlatform());
@@ -347,18 +348,23 @@ class UserRepositoryTest extends AbstractPersistenceTest {
                 .enabled(true)
                 .registrationDate(LocalDateTime.now())
                 .build());
-        userRepository.updateDashboardTiles(userEntity.getId(), List.of(new Tile("PLAYING", "S")));
+        userRepository.update(userEntity.getId(),
+                user -> user.getSettings().setDashboardTiles(List.of(new Tile("PLAYING", "S"))));
         entityManager.flush();
 
-        userRepository.updateDashboardTiles(userEntity.getId(), null);
+        userRepository.update(userEntity.getId(), user -> user.getSettings().setDashboardTiles(null));
         entityManager.flush();
         entityManager.clear();
 
         assertNull(findByEmail("reset@example.com").orElseThrow().getSettings().getDashboardTiles());
     }
 
-    @Test
+    private void updateSettings(long userId, String displayName, UserSettings settings) {
-    void updateAppearance_UnknownUser() {
+        userRepository.update(userId, user -> {
-        assertThrows(UserNotFoundException.class, () -> userRepository.updateAppearance(-1L, Appearance.DEFAULT));
+            user.setDisplayName(displayName);
+            user.getSettings().setLanguage(settings.getLanguage());
+            user.getSettings().setTheme(settings.getTheme());
+            user.getSettings().setPublicProfile(settings.isPublicProfile());
+        });
     }
 }

src/test/java/auth_service/modules/user/service/UserServiceTest.java

@@ -13,17 +13,21 @@ import auth_service.modules.user.persistence.UserRepository;
 import auth_service.shared.security.TokenGenerator;
 import auth_service.shared.test.AbstractUnitTest;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 
 import java.util.List;
 import java.util.Optional;
+import java.util.function.Consumer;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.*;
 
 /**
@@ -60,15 +64,16 @@ class UserServiceTest extends AbstractUnitTest {
         userService.updateDashboardTiles(1L, List.of(new Tile("PLAYING", "S"), new Tile("PLAYING", "L")));
         userService.updateProfileTiles(1L, List.of(new Tile("SUMMARY", "M"), new Tile("SUMMARY", "S")));
 
-        verify(userRepository).updateDashboardTiles(1L, List.of(new Tile("PLAYING", "S")));
+        List<User> changed = appliedChanges(2);
-        verify(userRepository).updateProfileTiles(1L, List.of(new Tile("SUMMARY", "M")));
+        assertEquals(List.of(new Tile("PLAYING", "S")), changed.get(0).getSettings().getDashboardTiles());
+        assertEquals(List.of(new Tile("SUMMARY", "M")), changed.get(1).getSettings().getProfileTiles());
     }
 
     @Test
     void updateDashboardTiles_NullStaysNull() {
         userService.updateDashboardTiles(1L, null);
 
-        verify(userRepository).updateDashboardTiles(1L, null);
+        assertNull(appliedChanges(1).getFirst().getSettings().getDashboardTiles());
     }
 
     @Test
@@ -77,7 +82,7 @@ class UserServiceTest extends AbstractUnitTest {
 
         userService.updateUserSettings(1L, null, userSettings);
 
-        verify(userRepository).updateSettings(1L, null, userSettings);
+        appliedChanges(1);
         verify(userRepository, never()).clearSteamId(anyLong());
         verify(userRepository, never()).setVerifiedSteamId(anyLong(), anyLong());
         verifyNoInteractions(tokenGenerator);
@@ -92,7 +97,7 @@ class UserServiceTest extends AbstractUnitTest {
         // Act & Assert
         assertThrows(DisplayNameAlreadyInUseException.class,
                 () -> userService.updateUserSettings(1L, "Lobo", userSettings));
-        verify(userRepository, never()).updateSettings(anyLong(), any(), any());
+        verify(userRepository, never()).update(anyLong(), any());
     }
 
     @Test
@@ -105,7 +110,9 @@ class UserServiceTest extends AbstractUnitTest {
         userService.updateUserSettings(1L, "Lobo", userSettings);
 
         // Assert
-        verify(userRepository).updateSettings(1L, "Lobo", userSettings);
+        User changed = appliedChanges(1).getFirst();
+        assertEquals("Lobo", changed.getDisplayName());
+        assertEquals("en", changed.getSettings().getLanguage());
     }
 
     private static final long STEAM_ID = 76561198000000001L;
@@ -165,7 +172,28 @@ class UserServiceTest extends AbstractUnitTest {
 
         userService.updateAppearance(1L, appearance);
 
-        verify(userRepository).updateAppearance(1L, appearance);
+        assertEquals(appearance, appliedChanges(1).getFirst().getSettings().getAppearance());
         verifyNoInteractions(tokenGenerator);
     }
+
+    @Test
+    void updateFavoritePlatform_StoresItStripped() {
+        userService.updateFavoritePlatform(1L, "  Switch ");
+
+        assertEquals("Switch", appliedChanges(1).getFirst().getSettings().getFavoritePlatform());
+    }
+
+    /**
+     * Runs each change user 1 got on a fresh user with default settings.
+     */
+    @SuppressWarnings("unchecked")
+    private List<User> appliedChanges(int count) {
+        ArgumentCaptor<Consumer<User>> change = ArgumentCaptor.forClass(Consumer.class);
+        verify(userRepository, times(count)).update(eq(1L), change.capture());
+        return change.getAllValues().stream().map(each -> {
+            User user = User.builder().id(1L).settings(UserSettings.builder().build()).build();
+            each.accept(user);
+            return user;
+        }).toList();
+    }
 }