Public Access
fix(library): keep backlog edits made during a sync, reject fractional ratings
UserGame now uses @DynamicUpdate, so the sync flush writes only the columns it changed and no longer overwrites a backlog update committed meanwhile.
This commit is contained in:
1 parent
c815a29993
commit
9ff00c1f8f
4 files changed
+57
No files matched your search
@@ -11,10 +11,13 @@ import lombok.AccessLevel;
|
|||||||
import lombok.Getter;
|
import lombok.Getter;
|
||||||
import lombok.NoArgsConstructor;
|
import lombok.NoArgsConstructor;
|
||||||
import lombok.Setter;
|
import lombok.Setter;
|
||||||
|
import org.hibernate.annotations.DynamicUpdate;
|
||||||
|
|
||||||
import java.time.Instant;
|
import java.time.Instant;
|
||||||
|
|
||||||
@Entity
|
@Entity
|
||||||
|
// The sync and the backlog update run in separate transactions on the same row; each must write only its own columns.
|
||||||
|
@DynamicUpdate
|
||||||
@Table(name = "user_game")
|
@Table(name = "user_game")
|
||||||
@IdClass(UserGameId.class)
|
@IdClass(UserGameId.class)
|
||||||
@Getter
|
@Getter
|
||||||
|
|||||||
@@ -3,6 +3,9 @@ spring:
|
|||||||
name: tbp-core-service
|
name: tbp-core-service
|
||||||
profiles:
|
profiles:
|
||||||
active: prod
|
active: prod
|
||||||
|
jackson:
|
||||||
|
deserialization:
|
||||||
|
accept-float-as-int: false
|
||||||
jpa:
|
jpa:
|
||||||
open-in-view: true
|
open-in-view: true
|
||||||
flyway:
|
flyway:
|
||||||
|
|||||||
@@ -100,6 +100,15 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
|||||||
assertEquals(GameStatus.BACKLOG, stored().getStatus());
|
assertEquals(GameStatus.BACKLOG, stored().getStatus());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void updateBacklog_FractionalRating_Is400() throws Exception {
|
||||||
|
mockMvc.perform(put("/api/library/games/220/backlog").contentType(MediaType.APPLICATION_JSON)
|
||||||
|
.content("{\"status\":\"PLAYING\",\"rating\":4.7}"))
|
||||||
|
.andExpect(status().isBadRequest());
|
||||||
|
|
||||||
|
assertNull(stored().getRating());
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void updateBacklog_NotesTooLong_Is400() throws Exception {
|
void updateBacklog_NotesTooLong_Is400() throws Exception {
|
||||||
String notes = "x".repeat(2001);
|
String notes = "x".repeat(2001);
|
||||||
|
|||||||
@@ -10,6 +10,11 @@ import org.junit.jupiter.api.Test;
|
|||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
import org.springframework.data.domain.PageRequest;
|
import org.springframework.data.domain.PageRequest;
|
||||||
import org.springframework.data.domain.Sort;
|
import org.springframework.data.domain.Sort;
|
||||||
|
import org.springframework.transaction.PlatformTransactionManager;
|
||||||
|
import org.springframework.transaction.TransactionDefinition;
|
||||||
|
import org.springframework.transaction.annotation.Propagation;
|
||||||
|
import org.springframework.transaction.annotation.Transactional;
|
||||||
|
import org.springframework.transaction.support.TransactionTemplate;
|
||||||
|
|
||||||
import java.time.Instant;
|
import java.time.Instant;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
@@ -28,6 +33,9 @@ class UserGameRepositoryTest extends AbstractPersistenceTest {
|
|||||||
@Autowired
|
@Autowired
|
||||||
private SteamGameRepository steamGameRepository;
|
private SteamGameRepository steamGameRepository;
|
||||||
|
|
||||||
|
@Autowired
|
||||||
|
private PlatformTransactionManager transactionManager;
|
||||||
|
|
||||||
@BeforeEach
|
@BeforeEach
|
||||||
void seed() {
|
void seed() {
|
||||||
steamGameRepository.saveAll(List.of(
|
steamGameRepository.saveAll(List.of(
|
||||||
@@ -89,4 +97,38 @@ class UserGameRepositoryTest extends AbstractPersistenceTest {
|
|||||||
assertNull(UserGameRepository.containsPattern(" "));
|
assertNull(UserGameRepository.containsPattern(" "));
|
||||||
assertNull(UserGameRepository.containsPattern(null));
|
assertNull(UserGameRepository.containsPattern(null));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@Transactional(propagation = Propagation.NOT_SUPPORTED)
|
||||||
|
void syncFlushKeepsBacklogEditCommittedMeanwhile() {
|
||||||
|
UserGameId id = new UserGameId(USER_ID, 220L);
|
||||||
|
TransactionTemplate syncTx = new TransactionTemplate(transactionManager);
|
||||||
|
TransactionTemplate userTx = new TransactionTemplate(transactionManager);
|
||||||
|
userTx.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
|
||||||
|
try {
|
||||||
|
syncTx.executeWithoutResult(sync -> {
|
||||||
|
// LibrarySyncWorker.upsertUserGame: entity loaded with status PLAYING (300 min playtime)
|
||||||
|
UserGame syncView = userGameRepository.findById(id).orElseThrow();
|
||||||
|
|
||||||
|
// BacklogService.update commits in its own transaction while the sync is still running
|
||||||
|
userTx.executeWithoutResult(user -> {
|
||||||
|
UserGame edit = userGameRepository.findById(id).orElseThrow();
|
||||||
|
edit.setStatus(GameStatus.FINISHED);
|
||||||
|
edit.setRating(5);
|
||||||
|
});
|
||||||
|
|
||||||
|
// the sync only touches playtime and lastSeenAt, then commits
|
||||||
|
syncView.setPlaytimeMinutes(400);
|
||||||
|
syncView.setLastSeenAt(Instant.now());
|
||||||
|
});
|
||||||
|
|
||||||
|
UserGame stored = userGameRepository.findById(id).orElseThrow();
|
||||||
|
assertEquals(400, stored.getPlaytimeMinutes());
|
||||||
|
assertEquals(GameStatus.FINISHED, stored.getStatus());
|
||||||
|
assertEquals(5, stored.getRating());
|
||||||
|
} finally {
|
||||||
|
userGameRepository.deleteAll();
|
||||||
|
steamGameRepository.deleteAll();
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user