fix(settings): give every user a settings row

V20 adds the row for old users, registration inserts one. The mapper fallback (en, system) disagreed with the column defaults.
This commit is contained in:
lobo committed 2026-09-30 13:41:56 +02:00
1 parent d082f3befd
commit fb51f836a8
16 files changed
+97 -171

No files matched your search

@@ -39,6 +39,7 @@ public class RegisteredUserRepositoryImpl implements RegisteredUserRepository {
if (id == null) {
throw new IllegalStateException("No id for the new user");
}
jdbc.update("INSERT INTO user_settings (user_id) VALUES (?)", id);
return id;
}
}
@@ -1,6 +1,5 @@
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;
@@ -8,7 +7,6 @@ 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 jakarta.validation.constraints.NotNull;
import java.util.List;
import java.util.Optional;
@@ -22,9 +20,6 @@ public interface UserRepository {
*/
Optional<User> findById(long userId);
@NotNull
UserSettings findSettingsByUserId(String userId);
@Transactional
void updateSettings(long userId, String displayName, UserSettings settings);
@@ -14,12 +14,10 @@ import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext;
import jakarta.persistence.TypedQuery;
import jakarta.transaction.Transactional;
import jakarta.validation.constraints.NotNull;
import org.springframework.stereotype.Repository;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
@Repository
public class UserRepositoryImpl implements UserRepository {
@@ -33,7 +31,6 @@ public class UserRepositoryImpl implements UserRepository {
var userEntity = userEntityCriteriaQuery.from(UserEntity.class);
// Fetch the settings eagerly using a LEFT JOIN to prevent N+1 queries.
// A LEFT JOIN ensures the user is returned even if no settings exist yet.
userEntity.fetch("settings", jakarta.persistence.criteria.JoinType.LEFT);
userEntityCriteriaQuery.select(userEntity).where(criteriaBuilder.equal(userEntity.get("id"), userId));
@@ -45,44 +42,16 @@ public class UserRepositoryImpl implements UserRepository {
.map(EntityMapper::mapToModel);
}
@Override
public @NotNull UserSettings findSettingsByUserId(String userId) {
UserSettingsEntity entity = entityManager.find(UserSettingsEntity.class, UUID.fromString(userId));
if (entity == null) {
// Return defaults if none exist yet
return UserSettings.builder()
.language("")
.theme("light")
.build();
}
return UserSettings.builder()
.language(entity.getLanguage())
.theme(entity.getTheme())
.build();
}
@Transactional
@Override
public void updateSettings(long userId, String displayName, UserSettings settings) {
var userEntity = entityManager.find(UserEntity.class, userId);
if (userEntity == null) {
throw new UserNotFoundException("Could not find user with id " + userId);
}
var settingsEntity = userEntity.getSettings();
if (settingsEntity == null) {
settingsEntity = new UserSettingsEntity();
settingsEntity.setUserId(userId);
userEntity.setSettings(settingsEntity);
}
UserSettingsEntity settingsEntity = settingsOf(userId);
settingsEntity.setLanguage(settings.getLanguage());
settingsEntity.setTheme(settings.getTheme());
settingsEntity.setPublicProfile(settings.isPublicProfile());
userEntity.setDisplayName(displayName);
// settingsOf loaded the user: find answers from the persistence context.
entityManager.find(UserEntity.class, userId).setDisplayName(displayName);
}
@Transactional
@@ -125,23 +94,16 @@ public class UserRepositoryImpl implements UserRepository {
settingsEntity.setProfileBackgroundAppid(style.backgroundAppid());
}
/**
* The settings row of the user, created with the defaults of the user_settings columns when there is none.
*/
// 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);
}
UserSettingsEntity settingsEntity = userEntity.getSettings();
if (settingsEntity == null) {
settingsEntity = new UserSettingsEntity();
settingsEntity.setUserId(userId);
settingsEntity.setLanguage("");
settingsEntity.setTheme("light");
userEntity.setSettings(settingsEntity);
if (userEntity.getSettings() == null) {
throw new IllegalStateException("User " + userId + " has no settings row");
}
return settingsEntity;
return userEntity.getSettings();
}
@Override
@@ -1,9 +1,5 @@
package auth_service.modules.user.persistence.entities;
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.UserSettings;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
@@ -35,12 +31,11 @@ public class UserSettingsEntity {
@Column(name = "public_profile", nullable = false)
private boolean publicProfile;
// Java defaults as well: a row created here, not by the migration, must not insert null.
@Column(nullable = false)
private String accent = Appearance.DEFAULT.accent();
private String accent;
@Column(nullable = false)
private String corners = Appearance.DEFAULT.corners();
private String corners;
@Column(name = "wide_layout", nullable = false)
private boolean wideLayout;
@@ -52,13 +47,13 @@ public class UserSettingsEntity {
private Long bannerAppid;
@Column(name = "show_banner", nullable = false)
private boolean showBanner = Appearance.DEFAULT.showBanner();
private boolean showBanner;
@Column(name = "profile_accent", nullable = false)
private String profileAccent = ProfileStyle.DEFAULT.accent();
private String profileAccent;
@Column(name = "profile_corners", nullable = false)
private String profileCorners = ProfileStyle.DEFAULT.corners();
private String profileCorners;
@Column(name = "profile_banner_appid")
private Long profileBannerAppid;
@@ -76,5 +71,5 @@ public class UserSettingsEntity {
@JdbcTypeCode(SqlTypes.ARRAY)
@Column(name = "profile_tiles", nullable = false)
private List<String> profileTiles = Tile.storedAll(UserSettings.DEFAULT_PROFILE_TILES);
private List<String> profileTiles;
}
@@ -9,8 +9,6 @@ import auth_service.modules.user.persistence.entities.UserEntity;
import auth_service.modules.user.persistence.entities.UserSettingsEntity;
import auth_service.shared.security.Role;
import java.util.List;
public abstract class EntityMapper {
private EntityMapper() {
@@ -21,33 +19,23 @@ public abstract class EntityMapper {
return null;
}
// 1. Map the settings if they exist, otherwise use defaults
UserSettings settingsModel;
if (entity.getSettings() != null) {
settingsModel = UserSettings.builder()
.language(entity.getSettings().getLanguage())
.theme(entity.getSettings().getTheme())
.publicProfile(entity.getSettings().isPublicProfile())
.appearance(new Appearance(entity.getSettings().getAccent(), entity.getSettings().getCorners(),
entity.getSettings().isWideLayout(), entity.getSettings().getBackgroundAppid(),
entity.getSettings().getBannerAppid(), entity.getSettings().isShowBanner()))
.profileStyle(new ProfileStyle(entity.getSettings().getProfileAccent(),
entity.getSettings().getProfileCorners(), entity.getSettings().getProfileBannerAppid(),
entity.getSettings().getProfileBackgroundAppid()))
.dashboardTiles(entity.getSettings().getDashboardTiles() == null ? null
: entity.getSettings().getDashboardTiles().stream().map(Tile::parse).toList())
.profileTiles(entity.getSettings().getProfileTiles().stream().map(Tile::parse).toList())
.favoritePlatform(entity.getSettings().getFavoritePlatform())
.build();
} else {
// No settings row. "" means no language chosen, as in a new row; the frontend keeps its own.
settingsModel = UserSettings.builder()
.language("")
.theme("system")
.build();
UserSettingsEntity settings = entity.getSettings();
if (settings == null) {
throw new IllegalStateException("User " + entity.getId() + " has no settings row");
}
// 2. Map the main user fields and attach the settings
UserSettings settingsModel = UserSettings.builder()
.language(settings.getLanguage())
.theme(settings.getTheme())
.publicProfile(settings.isPublicProfile())
.appearance(new Appearance(settings.getAccent(), settings.getCorners(), settings.isWideLayout(),
settings.getBackgroundAppid(), settings.getBannerAppid(), settings.isShowBanner()))
.profileStyle(new ProfileStyle(settings.getProfileAccent(), settings.getProfileCorners(),
settings.getProfileBannerAppid(), settings.getProfileBackgroundAppid()))
.dashboardTiles(settings.getDashboardTiles() == null ? null
: settings.getDashboardTiles().stream().map(Tile::parse).toList())
.profileTiles(settings.getProfileTiles().stream().map(Tile::parse).toList())
.favoritePlatform(settings.getFavoritePlatform())
.build();
return User.builder()
.id(entity.getId())
@@ -62,13 +50,4 @@ public abstract class EntityMapper {
.settings(settingsModel)
.build();
}
public static UserSettingsEntity toPersistence(long userId, UserSettings settings) {
UserSettingsEntity entity = new UserSettingsEntity();
entity.setUserId(userId);
entity.setLanguage(settings.getLanguage());
entity.setTheme(settings.getTheme());
entity.setPublicProfile(settings.isPublicProfile());
return entity;
}
}
}
@@ -0,0 +1,4 @@
-- Registration made no settings row, so the code had fallback defaults that disagreed with the column defaults.
-- Every user gets the row now, with the column defaults ('' = no language chosen, 'light' as decided in item 25).
INSERT INTO user_settings (user_id)
SELECT u.id FROM users u WHERE NOT EXISTS (SELECT 1 FROM user_settings s WHERE s.user_id = u.id);
@@ -42,6 +42,7 @@ class ExpiredAccessTokenTest extends AbstractIntegrationTest {
jdbc.update("DELETE FROM users WHERE email = ?", EMAIL);
jdbc.update("INSERT INTO users (email, password, enabled, steam_id) VALUES (?, ?, true, NULL)",
EMAIL, passwordEncoder.encode(PASSWORD));
addSettingsRows();
}
private String expiredAccessCookie() throws Exception {
@@ -63,6 +63,7 @@ class SessionFlowTest extends AbstractIntegrationTest {
jdbc.update("DELETE FROM users WHERE email = ?", EMAIL);
jdbc.update("INSERT INTO users (email, password, enabled, steam_id) VALUES (?, ?, true, NULL)",
EMAIL, passwordEncoder.encode(PASSWORD));
addSettingsRows();
}
private MockHttpServletResponse login() throws Exception {
@@ -61,6 +61,7 @@ class SteamLinkFlowTest extends AbstractIntegrationTest {
jdbc.update("INSERT INTO users (email, password, enabled) VALUES (?, ?, true)", EMAIL, passwordEncoder.encode(PASSWORD));
// Someone who typed the same ID in before Steam linking existed.
jdbc.update("INSERT INTO users (email, password, enabled, steam_id) VALUES (?, 'x', true, ?)", TYPIST, STEAM_ID);
addSettingsRows();
when(steamOpenIdService.loginUrl(anyString(), anyString())).thenReturn("https://steamcommunity.com/openid/login?test");
}
@@ -59,6 +59,7 @@ class DeletedAccountFlowTest extends AbstractIntegrationTest {
deleteAccount(access, PASSWORD).andExpect(status().isNoContent());
Long newUser = jdbc.queryForObject("INSERT INTO users (email, password, enabled) VALUES (?, 'x', true) RETURNING id",
Long.class, newEmail);
addSettingsRows();
try {
assertThat(userService.linkVerifiedSteamId(newUser, steamId)).isEqualTo(SteamLinkOutcome.LINKED);
@@ -48,8 +48,10 @@ class AdminFlowTest extends AbstractIntegrationTest {
private long insertUser(String email, String role) {
jdbc.update("DELETE FROM users WHERE email = ?", email);
return jdbc.queryForObject("INSERT INTO users (email, password, enabled, role, display_name) VALUES (?, ?, true, ?, ?) "
Long id = jdbc.queryForObject("INSERT INTO users (email, password, enabled, role, display_name) VALUES (?, ?, true, ?, ?) "
+ "RETURNING id", Long.class, email, passwordEncoder.encode(PASSWORD), role, email.substring(0, 16));
addSettingsRows();
return id;
}
@BeforeEach
@@ -45,6 +45,7 @@ class HintFlowTest extends AbstractIntegrationTest {
jdbc.update("DELETE FROM users WHERE email = ?", email);
jdbc.update("INSERT INTO users (email, password, enabled, role, display_name) VALUES (?, ?, true, ?, ?)",
email, passwordEncoder.encode(PASSWORD), role, email.substring(0, 16));
addSettingsRows();
}
@BeforeEach
@@ -56,6 +56,16 @@ class RegisteredUserRepositoryTest extends AbstractPersistenceTest {
assertEquals("USER", saved.get("role"));
}
@Test
void save_CreatesTheSettingsRowWithTheColumnDefaults() {
long id = registeredUserRepository.save(registration("row@example.com", null));
Map<String, Object> settings = jdbc.queryForMap("SELECT * FROM user_settings WHERE user_id = ?", id);
assertEquals("", settings.get("language"));
assertEquals("light", settings.get("theme"));
assertEquals(false, settings.get("public_profile"));
}
// V19: a second account under the same email in other case is refused by the database too.
@Test
void emailsThatDifferOnlyInCase_AreOneAccount() {
@@ -43,9 +43,26 @@ class UserRepositoryTest extends AbstractPersistenceTest {
.flatMap(userRepository::findById);
}
// Registration inserts the settings row in production; the entity alone does not.
private UserEntity persisted(UserEntity user) {
entityManager.persist(user);
jdbc.update("INSERT INTO user_settings (user_id) VALUES (?)", user.getId());
entityManager.clear();
return user;
}
@Test
void findById_SettingsRowMissing_Throws() {
UserEntity user = entityManager.persistFlushFind(UserEntity.builder().email("norow@example.com")
.password("password").enabled(true).registrationDate(LocalDateTime.now()).build());
entityManager.clear();
assertThrows(IllegalStateException.class, () -> userRepository.findById(user.getId()));
}
@Test
void updateSettings_KeepsADeleteThatCommittedAfterTheLoad() {
var user = entityManager.persistFlushFind(UserEntity.builder().email("race@example.com").password("password")
var user = persisted(UserEntity.builder().email("race@example.com").password("password")
.enabled(true).registrationDate(LocalDateTime.now()).build());
// The delete request commits after the settings request loaded the user.
jdbc.update("UPDATE users SET deleted_at = NOW() WHERE id = ?", user.getId());
@@ -67,8 +84,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
.steamId(123456L)
.registrationDate(LocalDateTime.now())
.build();
entityManager.persist(userEntity);
entityManager.flush();
persisted(userEntity);
// Act
var result = findByEmail(email);
@@ -89,8 +105,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
.steamId(123456L)
.registrationDate(LocalDateTime.now())
.build();
userEntity = entityManager.persist(userEntity);
entityManager.flush();
userEntity = persisted(userEntity);
var settings = UserSettings.builder()
.language("de")
@@ -112,13 +127,12 @@ class UserRepositoryTest extends AbstractPersistenceTest {
void findByEmail_NoSteamId() {
// Arrange
var email = "nosteam@example.com";
entityManager.persist(UserEntity.builder()
persisted(UserEntity.builder()
.email(email)
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
// Act
var result = findByEmail(email);
@@ -129,7 +143,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
}
private UserEntity user(String email, Long steamId, boolean verified) {
UserEntity user = entityManager.persist(UserEntity.builder()
UserEntity user = persisted(UserEntity.builder()
.email(email)
.password("password")
.enabled(true)
@@ -137,7 +151,6 @@ class UserRepositoryTest extends AbstractPersistenceTest {
.steamIdVerified(verified)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
return user;
}
@@ -173,7 +186,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
@Test
void setVerifiedSteamId_DeletedUser_IsRefused() {
var deleted = entityManager.persistFlushFind(UserEntity.builder().email("deleted@example.com").password("password")
var deleted = persisted(UserEntity.builder().email("deleted@example.com").password("password")
.enabled(true).registrationDate(LocalDateTime.now()).build());
jdbc.update("UPDATE users SET deleted_at = NOW() WHERE id = ?", deleted.getId());
@@ -196,19 +209,18 @@ class UserRepositoryTest extends AbstractPersistenceTest {
@Test
void displayName_StoredAndCheckedCaseInsensitive() {
// Arrange
var owner = entityManager.persist(UserEntity.builder()
var owner = persisted(UserEntity.builder()
.email("owner2@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
var other = entityManager.persist(UserEntity.builder()
var other = persisted(UserEntity.builder()
.email("other2@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
var settings = UserSettings.builder().language("en").theme("light").build();
// Act
@@ -224,13 +236,12 @@ class UserRepositoryTest extends AbstractPersistenceTest {
@Test
void updateSettings_StoresPublicProfile_AndFindByEmailReadsIt() {
var userEntity = entityManager.persist(UserEntity.builder()
var userEntity = persisted(UserEntity.builder()
.email("public@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
userRepository.updateSettings(userEntity.getId(), "Lobo",
UserSettings.builder().language("de").theme("dark").publicProfile(true).build());
@@ -240,33 +251,14 @@ class UserRepositoryTest extends AbstractPersistenceTest {
assertTrue(findByEmail("public@example.com").orElseThrow().getSettings().isPublicProfile());
}
@Test
void updateAppearance_WithoutSettingsRow_CreatesItAndFindByEmailReadsIt() {
var userEntity = entityManager.persist(UserEntity.builder()
.email("appearance@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
userRepository.updateAppearance(userEntity.getId(), new Appearance("orange", "round", true, 620L, 1086940L, false));
entityManager.flush();
entityManager.clear();
var settings = findByEmail("appearance@example.com").orElseThrow().getSettings();
assertEquals(new Appearance("orange", "round", true, 620L, 1086940L, false), settings.getAppearance());
}
@Test
void updateSettings_KeepsTheAppearance() {
var userEntity = entityManager.persist(UserEntity.builder()
var userEntity = persisted(UserEntity.builder()
.email("keep@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
userRepository.updateAppearance(userEntity.getId(), new Appearance("pink", "sharp", false, 220L, null, true));
entityManager.flush();
@@ -279,29 +271,14 @@ class UserRepositoryTest extends AbstractPersistenceTest {
assertEquals(new Appearance("pink", "sharp", false, 220L, null, true), settings.getAppearance());
}
@Test
void findByEmail_WithoutSettingsRow_HasTheDefaultAppearance() {
entityManager.persist(UserEntity.builder()
.email("defaults@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
assertEquals(Appearance.DEFAULT, findByEmail("defaults@example.com").orElseThrow()
.getSettings().getAppearance());
}
@Test
void updateProfileStyle_IsReadBackAndStartsWithTheDefaults() {
UserEntity userEntity = entityManager.persist(UserEntity.builder()
UserEntity userEntity = persisted(UserEntity.builder()
.email("style@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
assertEquals(ProfileStyle.DEFAULT, findByEmail("style@example.com").orElseThrow()
.getSettings().getProfileStyle());
@@ -315,13 +292,12 @@ class UserRepositoryTest extends AbstractPersistenceTest {
@Test
void updateTiles_AreReadBackAndKeptByUpdateSettings() {
UserEntity userEntity = entityManager.persist(UserEntity.builder()
UserEntity userEntity = persisted(UserEntity.builder()
.email("tiles@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
entityManager.flush();
UserSettings fresh = findByEmail("tiles@example.com").orElseThrow().getSettings();
assertNull(fresh.getDashboardTiles());
assertEquals(UserSettings.DEFAULT_PROFILE_TILES, fresh.getProfileTiles());
@@ -343,7 +319,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
@Test
void updateFavoritePlatform_StoresAndClears() {
UserEntity userEntity = entityManager.persist(UserEntity.builder()
UserEntity userEntity = persisted(UserEntity.builder()
.email("favorite@example.com")
.password("password")
.enabled(true)
@@ -365,7 +341,7 @@ class UserRepositoryTest extends AbstractPersistenceTest {
@Test
void updateDashboardTiles_NullResetsToTheDefault() {
UserEntity userEntity = entityManager.persist(UserEntity.builder()
UserEntity userEntity = persisted(UserEntity.builder()
.email("reset@example.com")
.password("password")
.enabled(true)
@@ -1,16 +0,0 @@
package auth_service.modules.user.persistence.mapper;
import auth_service.modules.user.persistence.entities.UserEntity;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
class EntityMapperTest {
@Test
void mapToModel_UserWithoutSettingsRow_HasNoLanguage() {
UserEntity entity = UserEntity.builder().id(1L).email("a@b.c").role("USER").build();
assertThat(EntityMapper.mapToModel(entity).getSettings().getLanguage()).isEmpty();
}
}
@@ -1,7 +1,9 @@
package auth_service.shared.test;
import org.junit.jupiter.api.Tag;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
import org.springframework.test.context.DynamicPropertyRegistry;
@@ -33,6 +35,17 @@ public abstract class AbstractIntegrationTest {
postgres.start();
}
@Autowired
private JdbcTemplate baseJdbc;
/**
* For users a test inserts by SQL. Registration inserts the row itself.
*/
protected void addSettingsRows() {
baseJdbc.update("INSERT INTO user_settings (user_id) SELECT u.id FROM users u "
+ "WHERE NOT EXISTS (SELECT 1 FROM user_settings s WHERE s.user_id = u.id)");
}
// No key is committed: a fresh one per JVM.
private static final KeyPair KEYS = generateKeys();