Public Access
fix(cache): one cache config that expires after the write
publicStats is evicted on registration and Steam link/unlink. The cached user no longer holds the password hash. Tests no longer run with a 1 s TTL.
This commit is contained in:
1 parent
cf21f4a654
commit
509280da14
9 files changed
+56
-62
No files matched your search
@@ -136,6 +136,8 @@ layers:
|
|||||||
may log in and is shown to others" uses `shared/account/ActiveUser`.
|
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
|
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.
|
display name and the settings. A new setting needs no new repository method.
|
||||||
|
Two Caffeine caches in `CacheConfig`: `users` (by userId) and `publicStats`. Every write evicts what it changes;
|
||||||
|
entries also expire 10 minutes after the write.
|
||||||
`profile` and `stats` only read data. They have no entities and use `JdbcTemplate` queries in a `repo` package.
|
`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.
|
`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
|
`account` (delete and nightly cleanup) sets the flag with `JdbcTemplate` in a `repo` package too, and calls Core
|
||||||
|
|||||||
@@ -1,37 +1,28 @@
|
|||||||
package auth_service.configuration;
|
package auth_service.configuration;
|
||||||
|
|
||||||
import com.github.benmanes.caffeine.cache.Caffeine;
|
import com.github.benmanes.caffeine.cache.Caffeine;
|
||||||
import lombok.extern.slf4j.Slf4j;
|
|
||||||
import org.springframework.cache.CacheManager;
|
import org.springframework.cache.CacheManager;
|
||||||
import org.springframework.cache.annotation.EnableCaching;
|
import org.springframework.cache.annotation.EnableCaching;
|
||||||
import org.springframework.cache.caffeine.CaffeineCacheManager;
|
import org.springframework.cache.caffeine.CaffeineCacheManager;
|
||||||
import org.springframework.context.annotation.Bean;
|
import org.springframework.context.annotation.Bean;
|
||||||
import org.springframework.context.annotation.Configuration;
|
import org.springframework.context.annotation.Configuration;
|
||||||
import org.springframework.context.annotation.Profile;
|
|
||||||
|
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.time.Duration;
|
||||||
|
|
||||||
/**
|
|
||||||
* Global configuration to enable Spring Boot caching (backed by Caffeine).
|
|
||||||
*/
|
|
||||||
@Configuration
|
@Configuration
|
||||||
@EnableCaching
|
@EnableCaching
|
||||||
@Profile("!dev")
|
|
||||||
@Slf4j
|
|
||||||
public class CacheConfig {
|
public class CacheConfig {
|
||||||
|
|
||||||
|
// After write, not after access: an entry another path changed without an evict still ages out.
|
||||||
|
static final Duration DEFAULT_TTL = Duration.ofMinutes(10);
|
||||||
|
|
||||||
@Bean
|
@Bean
|
||||||
public CacheManager cacheManager() {
|
public CacheManager cacheManager() {
|
||||||
log.info("########### Productive cache active ###########");
|
|
||||||
CaffeineCacheManager cacheManager = new CaffeineCacheManager("users", "publicStats");
|
CaffeineCacheManager cacheManager = new CaffeineCacheManager("users", "publicStats");
|
||||||
cacheManager.setCaffeine(caffeineCacheBuilder());
|
cacheManager.setCaffeine(Caffeine.newBuilder()
|
||||||
|
.maximumSize(500)
|
||||||
|
.expireAfterWrite(DEFAULT_TTL)
|
||||||
|
.recordStats());
|
||||||
return cacheManager;
|
return cacheManager;
|
||||||
}
|
}
|
||||||
|
|
||||||
Caffeine<Object, Object> caffeineCacheBuilder() {
|
|
||||||
return Caffeine.newBuilder()
|
|
||||||
.initialCapacity(100)
|
|
||||||
.maximumSize(500)
|
|
||||||
.expireAfterAccess(10, TimeUnit.MINUTES)
|
|
||||||
.recordStats();
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
@@ -1,37 +0,0 @@
|
|||||||
package auth_service.configuration;
|
|
||||||
|
|
||||||
import com.github.benmanes.caffeine.cache.Caffeine;
|
|
||||||
import lombok.extern.slf4j.Slf4j;
|
|
||||||
import org.springframework.cache.CacheManager;
|
|
||||||
import org.springframework.cache.annotation.EnableCaching;
|
|
||||||
import org.springframework.cache.caffeine.CaffeineCacheManager;
|
|
||||||
import org.springframework.context.annotation.Bean;
|
|
||||||
import org.springframework.context.annotation.Configuration;
|
|
||||||
import org.springframework.context.annotation.Profile;
|
|
||||||
|
|
||||||
import java.util.concurrent.TimeUnit;
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Global configuration to enable Spring Boot caching (backed by Caffeine).
|
|
||||||
*/
|
|
||||||
@Configuration
|
|
||||||
@EnableCaching
|
|
||||||
@Profile("dev")
|
|
||||||
@Slf4j
|
|
||||||
public class DevCacheConfig {
|
|
||||||
@Bean
|
|
||||||
public CacheManager cacheManager() {
|
|
||||||
log.error("########### DEVELOPMENT CACHE ACTIVE ###########");
|
|
||||||
CaffeineCacheManager cacheManager = new CaffeineCacheManager("users", "publicStats");
|
|
||||||
cacheManager.setCaffeine(caffeineCacheBuilder());
|
|
||||||
return cacheManager;
|
|
||||||
}
|
|
||||||
|
|
||||||
Caffeine<Object, Object> caffeineCacheBuilder() {
|
|
||||||
return Caffeine.newBuilder()
|
|
||||||
.initialCapacity(100)
|
|
||||||
.maximumSize(500)
|
|
||||||
.expireAfterAccess(1, TimeUnit.SECONDS)
|
|
||||||
.recordStats();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -7,6 +7,7 @@ import auth_service.modules.registration.model.Registration;
|
|||||||
import auth_service.modules.registration.persistence.RegisteredUserRepository;
|
import auth_service.modules.registration.persistence.RegisteredUserRepository;
|
||||||
import auth_service.shared.security.Public;
|
import auth_service.shared.security.Public;
|
||||||
import lombok.RequiredArgsConstructor;
|
import lombok.RequiredArgsConstructor;
|
||||||
|
import org.springframework.cache.annotation.CacheEvict;
|
||||||
import org.springframework.security.crypto.password.PasswordEncoder;
|
import org.springframework.security.crypto.password.PasswordEncoder;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
|
|
||||||
@@ -20,6 +21,7 @@ public class RegistrationService {
|
|||||||
private final PasswordEncoder passwordEncoder;
|
private final PasswordEncoder passwordEncoder;
|
||||||
private final RegistrationAttemptService registrationAttemptService;
|
private final RegistrationAttemptService registrationAttemptService;
|
||||||
|
|
||||||
|
@CacheEvict(value = "publicStats", allEntries = true)
|
||||||
public long register(Registration registration, String clientIp) {
|
public long register(Registration registration, String clientIp) {
|
||||||
registrationAttemptService.acquire(clientIp);
|
registrationAttemptService.acquire(clientIp);
|
||||||
validate(registration);
|
validate(registration);
|
||||||
|
|||||||
@@ -11,7 +11,6 @@ import java.time.LocalDateTime;
|
|||||||
public class User {
|
public class User {
|
||||||
private long id;
|
private long id;
|
||||||
private String email;
|
private String email;
|
||||||
private String encryptedPassword;
|
|
||||||
private LocalDateTime lastLogin;
|
private LocalDateTime lastLogin;
|
||||||
private LocalDateTime createdDate;
|
private LocalDateTime createdDate;
|
||||||
private Long steamId;
|
private Long steamId;
|
||||||
|
|||||||
@@ -40,7 +40,6 @@ public abstract class EntityMapper {
|
|||||||
return User.builder()
|
return User.builder()
|
||||||
.id(entity.getId())
|
.id(entity.getId())
|
||||||
.email(entity.getEmail())
|
.email(entity.getEmail())
|
||||||
.encryptedPassword(entity.getPassword())
|
|
||||||
.lastLogin(entity.getLastLoginDate())
|
.lastLogin(entity.getLastLoginDate())
|
||||||
.createdDate(entity.getRegistrationDate())
|
.createdDate(entity.getRegistrationDate())
|
||||||
.steamId(entity.getSteamId())
|
.steamId(entity.getSteamId())
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ import lombok.RequiredArgsConstructor;
|
|||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
import org.springframework.cache.annotation.CacheEvict;
|
import org.springframework.cache.annotation.CacheEvict;
|
||||||
import org.springframework.cache.annotation.Cacheable;
|
import org.springframework.cache.annotation.Cacheable;
|
||||||
|
import org.springframework.cache.annotation.Caching;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
import org.springframework.transaction.annotation.Transactional;
|
import org.springframework.transaction.annotation.Transactional;
|
||||||
|
|
||||||
@@ -63,7 +64,7 @@ public class UserService {
|
|||||||
*/
|
*/
|
||||||
@Transactional
|
@Transactional
|
||||||
// All: the user who had typed the same Steam ID in loses it.
|
// All: the user who had typed the same Steam ID in loses it.
|
||||||
@CacheEvict(value = "users", allEntries = true)
|
@CacheEvict(value = {"users", "publicStats"}, allEntries = true)
|
||||||
public SteamLinkOutcome linkVerifiedSteamId(long userId, long steamId) {
|
public SteamLinkOutcome linkVerifiedSteamId(long userId, long steamId) {
|
||||||
Optional<SteamIdHolder> holder = userRepository.findSteamIdHolder(steamId);
|
Optional<SteamIdHolder> holder = userRepository.findSteamIdHolder(steamId);
|
||||||
if (holder.isPresent() && holder.get().userId() != userId) {
|
if (holder.isPresent() && holder.get().userId() != userId) {
|
||||||
@@ -84,7 +85,8 @@ public class UserService {
|
|||||||
*/
|
*/
|
||||||
@RequiresRole(Role.USER)
|
@RequiresRole(Role.USER)
|
||||||
@Transactional
|
@Transactional
|
||||||
@CacheEvict(value = "users", key = "#userId")
|
@Caching(evict = {@CacheEvict(value = "users", key = "#userId"),
|
||||||
|
@CacheEvict(value = "publicStats", allEntries = true)})
|
||||||
public String unlinkSteamId(long userId, Role role) {
|
public String unlinkSteamId(long userId, Role role) {
|
||||||
userRepository.clearSteamId(userId);
|
userRepository.clearSteamId(userId);
|
||||||
log.info("Steam ID unlinked for user {}", userId);
|
log.info("Steam ID unlinked for user {}", userId);
|
||||||
|
|||||||
@@ -93,8 +93,26 @@ class DeletedAccountFlowTest extends AbstractIntegrationTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private long totalUsers() throws Exception {
|
private long totalUsers() throws Exception {
|
||||||
|
return publicStat("totalUsers");
|
||||||
|
}
|
||||||
|
|
||||||
|
private long publicStat(String name) throws Exception {
|
||||||
String body = mockMvc.perform(get("/api/stats/public")).andReturn().getResponse().getContentAsString();
|
String body = mockMvc.perform(get("/api/stats/public")).andReturn().getResponse().getContentAsString();
|
||||||
return JsonPath.<Number>read(body, "$.totalUsers").longValue();
|
return JsonPath.<Number>read(body, "$." + name).longValue();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void steamLinkAndUnlink_CountInThePublicStatsAtOnce() throws Exception {
|
||||||
|
long userId = jdbc.queryForObject("SELECT id FROM users WHERE email = ?", Long.class, EMAIL);
|
||||||
|
long before = publicStat("usersWithSteamLinked");
|
||||||
|
|
||||||
|
userService.linkVerifiedSteamId(userId, 76561198099990002L);
|
||||||
|
assertThat(publicStat("usersWithSteamLinked")).isEqualTo(before + 1);
|
||||||
|
|
||||||
|
mockMvc.perform(delete("/api/users/steam-link").header(ClientHeaderFilter.CLIENT_HEADER, "web")
|
||||||
|
.cookie(login().getCookie(AuthCookies.ACCESS_COOKIE)))
|
||||||
|
.andExpect(status().is2xxSuccessful());
|
||||||
|
assertThat(publicStat("usersWithSteamLinked")).isEqualTo(before);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -154,6 +172,26 @@ class DeletedAccountFlowTest extends AbstractIntegrationTest {
|
|||||||
assertThat(totalUsers()).isEqualTo(usersBefore - 1);
|
assertThat(totalUsers()).isEqualTo(usersBefore - 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void registration_CountsInThePublicStatsAtOnce() throws Exception {
|
||||||
|
String newEmail = "delete-flow-registered@example.com";
|
||||||
|
jdbc.update("DELETE FROM users WHERE email = ?", newEmail);
|
||||||
|
long before = totalUsers();
|
||||||
|
|
||||||
|
try {
|
||||||
|
mockMvc.perform(post("/api/registration/new")
|
||||||
|
.header(ClientHeaderFilter.CLIENT_HEADER, "web")
|
||||||
|
.contentType(MediaType.APPLICATION_JSON)
|
||||||
|
.content("{\"email\":\"" + newEmail + "\",\"password\":\"" + PASSWORD
|
||||||
|
+ "\",\"inviteCode\":\"test-invite-code\"}"))
|
||||||
|
.andExpect(status().is2xxSuccessful());
|
||||||
|
|
||||||
|
assertThat(totalUsers()).isEqualTo(before + 1);
|
||||||
|
} finally {
|
||||||
|
jdbc.update("DELETE FROM users WHERE email = ?", newEmail);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void registration_WithTheEmailOfAFlaggedUser_IsStillRefused() throws Exception {
|
void registration_WithTheEmailOfAFlaggedUser_IsStillRefused() throws Exception {
|
||||||
jdbc.update("UPDATE users SET deleted_at = NOW() WHERE email = ?", EMAIL);
|
jdbc.update("UPDATE users SET deleted_at = NOW() WHERE email = ?", EMAIL);
|
||||||
|
|||||||
@@ -4,7 +4,6 @@ import org.junit.jupiter.api.Tag;
|
|||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
import org.springframework.boot.test.context.SpringBootTest;
|
import org.springframework.boot.test.context.SpringBootTest;
|
||||||
import org.springframework.jdbc.core.JdbcTemplate;
|
import org.springframework.jdbc.core.JdbcTemplate;
|
||||||
import org.springframework.test.context.ActiveProfiles;
|
|
||||||
import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
|
import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
|
||||||
import org.springframework.test.context.DynamicPropertyRegistry;
|
import org.springframework.test.context.DynamicPropertyRegistry;
|
||||||
import org.springframework.test.context.DynamicPropertySource;
|
import org.springframework.test.context.DynamicPropertySource;
|
||||||
@@ -23,7 +22,6 @@ import java.util.Base64;
|
|||||||
// The account cleanup must not run by itself, and Core is never reachable in tests.
|
// The account cleanup must not run by itself, and Core is never reachable in tests.
|
||||||
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
|
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
|
||||||
properties = {"app.account-cleanup.cron=-", "app.core.internal-url=http://127.0.0.1:1"})
|
properties = {"app.account-cleanup.cron=-", "app.core.internal-url=http://127.0.0.1:1"})
|
||||||
@ActiveProfiles("dev")
|
|
||||||
public abstract class AbstractIntegrationTest {
|
public abstract class AbstractIntegrationTest {
|
||||||
|
|
||||||
// One container per JVM, not per class: Spring caches the context across test classes,
|
// One container per JVM, not per class: Spring caches the context across test classes,
|
||||||
|
|||||||
Reference in new issue
Block a user