From cf21f4a6543f5f1fec888473d26db6a951d35b54 Mon Sep 17 00:00:00 2001 From: lobo Date: Wed, 30 Sep 2026 13:41:56 +0200 Subject: [PATCH] refactor(settings): one load-change-save update for all settings --- readme.md | 2 + .../user/persistence/UserRepository.java | 22 ++---- .../user/persistence/UserRepositoryImpl.java | 67 ++----------------- .../user/persistence/mapper/EntityMapper.java | 25 +++++++ .../modules/user/service/UserService.java | 19 ++++-- .../user/persistence/UserRepositoryTest.java | 42 +++++++----- .../modules/user/service/UserServiceTest.java | 42 ++++++++++-- 7 files changed, 109 insertions(+), 110 deletions(-) diff --git a/readme.md b/readme.md index 255aa76..3935d77 100644 --- a/readme.md +++ b/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 diff --git a/src/main/java/auth_service/modules/user/persistence/UserRepository.java b/src/main/java/auth_service/modules/user/persistence/UserRepository.java index f7450eb..c58d29e 100644 --- a/src/main/java/auth_service/modules/user/persistence/UserRepository.java +++ b/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 findById(long userId); + /** + * Stores only the display name and the settings of the changed user. + */ @Transactional - void updateSettings(long userId, String displayName, UserSettings settings); - - @Transactional - void updateAppearance(long userId, Appearance appearance); - - void updateDashboardTiles(long userId, List tiles); - - void updateProfileTiles(long userId, List tiles); - - void updateFavoritePlatform(long userId, String platform); - - void updateProfileStyle(long userId, ProfileStyle style); + void update(long userId, Consumer change); Optional findSteamIdHolder(long steamId); diff --git a/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java b/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java index 4c13098..0586bf5 100644 --- a/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java +++ b/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) { - 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 tiles) { - settingsOf(userId).setDashboardTiles(tiles == null ? null : Tile.storedAll(tiles)); - } - - @Transactional - @Override - public void updateProfileTiles(long userId, List 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) { + public void update(long userId, Consumer change) { UserEntity userEntity = entityManager.find(UserEntity.class, userId); if (userEntity == null) { throw new UserNotFoundException("Could not find user with id " + userId); } - if (userEntity.getSettings() == null) { - throw new IllegalStateException("User " + userId + " has no settings row"); - } - return userEntity.getSettings(); + User user = EntityMapper.mapToModel(userEntity); + change.accept(user); + EntityMapper.apply(user, userEntity); } @Override diff --git a/src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java b/src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java index ba2131e..4511768 100644 --- a/src/main/java/auth_service/modules/user/persistence/mapper/EntityMapper.java +++ b/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()); + } } diff --git a/src/main/java/auth_service/modules/user/service/UserService.java b/src/main/java/auth_service/modules/user/service/UserService.java index ffd246e..fbca2cf 100644 --- a/src/main/java/auth_service/modules/user/service/UserService.java +++ b/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 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 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)); } } diff --git a/src/test/java/auth_service/modules/user/persistence/UserRepositoryTest.java b/src/test/java/auth_service/modules/user/persistence/UserRepositoryTest.java index 3b1e4f0..830cf98 100644 --- a/src/test/java/auth_service/modules/user/persistence/UserRepositoryTest.java +++ b/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 dashboard = List.of(new Tile("STATS_TREND", "L"), new Tile("PLAYING", "S")); List profile = List.of(new Tile("STATS_PILE", "M"), new Tile("SUMMARY", "S")); - userRepository.updateDashboardTiles(userEntity.getId(), dashboard); - userRepository.updateProfileTiles(userEntity.getId(), profile); + userRepository.update(userEntity.getId(), user -> user.getSettings().setDashboardTiles(dashboard)); + 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 - void updateAppearance_UnknownUser() { - assertThrows(UserNotFoundException.class, () -> userRepository.updateAppearance(-1L, Appearance.DEFAULT)); + private void updateSettings(long userId, String displayName, UserSettings settings) { + userRepository.update(userId, user -> { + user.setDisplayName(displayName); + user.getSettings().setLanguage(settings.getLanguage()); + user.getSettings().setTheme(settings.getTheme()); + user.getSettings().setPublicProfile(settings.isPublicProfile()); + }); } } diff --git a/src/test/java/auth_service/modules/user/service/UserServiceTest.java b/src/test/java/auth_service/modules/user/service/UserServiceTest.java index 544ad2e..aed151d 100644 --- a/src/test/java/auth_service/modules/user/service/UserServiceTest.java +++ b/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"))); - verify(userRepository).updateProfileTiles(1L, List.of(new Tile("SUMMARY", "M"))); + List changed = appliedChanges(2); + 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 appliedChanges(int count) { + ArgumentCaptor> 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(); + } }