Skip to content

Commit e6f6502

Browse files
[Feature] add pessimistic locking on the Competition aggregate root (#558)
* feat: add findByIdForUpdate() to CompetitionRepository with pessimistic lock * feat: set timeout for locking in application.yaml * feat: add lockCompetitionForUpdate() to HierarchyValidator * feat: update changeStatus() in CompetitionServiceImpl so that it uses lockCompetitionForUpdate() from HierarchyValidator * feat: add handlePessimisticLockingFailure() handler to GlobalExceptionHandler * chore: fix formatter * feat: update unit tests in CompetitionServiceTest * feat: update unit tests in CompetitionControllerTest & HierarchyValidatorTest * chore: fix formatter
1 parent 1020cd1 commit e6f6502

8 files changed

Lines changed: 141 additions & 30 deletions

File tree

src/main/java/com/itasocialacademy/oitassist/competition/dao/repository/CompetitionRepository.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,28 @@
22

33
import com.itasocialacademy.oitassist.competition.dao.enums.CompetitionStatus;
44
import com.itasocialacademy.oitassist.competition.dao.model.Competition;
5+
import jakarta.persistence.LockModeType;
6+
import java.util.Optional;
57
import org.springframework.data.domain.Page;
68
import org.springframework.data.domain.Pageable;
79
import org.springframework.data.jpa.repository.JpaRepository;
810
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
11+
import org.springframework.data.jpa.repository.Lock;
12+
import org.springframework.data.jpa.repository.Query;
13+
import org.springframework.data.repository.query.Param;
914
import org.springframework.stereotype.Repository;
1015

1116
@Repository
1217
public interface CompetitionRepository extends JpaRepository<Competition, Long>, JpaSpecificationExecutor<Competition> {
1318
Page<Competition> findAllByCompetitionStatus(CompetitionStatus status, Pageable pageable);
19+
20+
/**
21+
* Fetches a Competition with a pessimistic write lock (SELECT ... FOR UPDATE),
22+
* used as the entry point for any structural hierarchy mutation or status
23+
* transition, to serialize concurrent changes on the same competition and close
24+
* the write-skew window between publish/finish and delete of a Stage/Tour.
25+
*/
26+
@Lock(LockModeType.PESSIMISTIC_WRITE)
27+
@Query("SELECT c from Competition c where c.id = :id")
28+
Optional<Competition> findByIdForUpdate(@Param("id") Long id);
1429
}

src/main/java/com/itasocialacademy/oitassist/competition/service/CompetitionServiceImpl.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ public Page<CompetitionResponse> getArchived(CompetitionSearchFilter filter, Pag
9696
@Override
9797
@Transactional
9898
public CompetitionResponse changeStatus(Long competitionId, ChangeCompetitionStatusRequest request) {
99-
Competition competition = competitionRepository.findById(competitionId)
100-
.orElseThrow(() -> new CompetitionNotFoundException(competitionId));
99+
Competition competition = validator.lockCompetitionForUpdate(competitionId);
101100

102101
validator.validateEntityVersion(request.version(), competition.getVersion(), Competition.class, competitionId);
103102
CompetitionStatus currentStatus = competition.getCompetitionStatus();

src/main/java/com/itasocialacademy/oitassist/competition/validation/HierarchyValidator.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,13 @@ public void checkIfStageInProgress(Long stageId, ExecutionStatus targetTourStatu
8989

9090
/**
9191
* Checks whether it is allowed to change the hierarchy (add/remove stages and
92-
* tours).
92+
* tours). Locks the Competition row for the duration of the transaction.
9393
*
9494
* @param competitionId ID of a competition
9595
*/
96-
@Transactional(readOnly = true)
96+
@Transactional
9797
public void validateImmutabilityByCompetitionId(Long competitionId) {
98-
Competition competition = competitionRepository.findById(competitionId)
99-
.orElseThrow(() -> new CompetitionNotFoundException(competitionId));
98+
Competition competition = lockCompetitionForUpdate(competitionId);
10099

101100
if (competition.getCompetitionStatus() == CompetitionStatus.ARCHIVED) {
102101
throw new CompetitionHierarchyValidationException(
@@ -360,4 +359,19 @@ public void validateEntityVersion(Long expectedVersion, Long actualVersion, Clas
360359
throw new StaleEntityVersionException(entityClass, entityId);
361360
}
362361
}
362+
363+
/**
364+
* Fetches and locks the Competition row (SELECT ... FOR UPDATE) for the
365+
* duration of the current transaction. Must be called before any structural
366+
* hierarchy mutation or lifecycle status transition, to serialize concurrent
367+
* changes on the same competition.
368+
*
369+
* @param competitionId Competition ID
370+
* @return the locked Competition entity
371+
*/
372+
@Transactional
373+
public Competition lockCompetitionForUpdate(Long competitionId) {
374+
return competitionRepository.findByIdForUpdate(competitionId)
375+
.orElseThrow(() -> new CompetitionNotFoundException(competitionId));
376+
}
363377
}

src/main/java/com/itasocialacademy/oitassist/core/web/GlobalExceptionHandler.java

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
package com.itasocialacademy.oitassist.core.web;
22

33
import com.itasocialacademy.oitassist.core.enums.ErrorCode;
4-
import com.itasocialacademy.oitassist.core.exceptions.*;
4+
import com.itasocialacademy.oitassist.core.exceptions.AuthenticationException;
5+
import com.itasocialacademy.oitassist.core.exceptions.BusinessException;
56
import com.itasocialacademy.oitassist.core.exceptions.SecurityException;
67
import com.itasocialacademy.oitassist.core.exceptions.TechnicalException;
78
import jakarta.servlet.http.HttpServletRequest;
9+
import java.time.Instant;
10+
import java.util.Map;
11+
import java.util.Objects;
12+
import java.util.stream.Collectors;
813
import lombok.RequiredArgsConstructor;
914
import lombok.extern.slf4j.Slf4j;
1015
import org.slf4j.MDC;
1116
import org.springframework.dao.OptimisticLockingFailureException;
17+
import org.springframework.dao.PessimisticLockingFailureException;
1218
import org.springframework.http.HttpStatus;
1319
import org.springframework.http.ResponseEntity;
1420
import org.springframework.http.converter.HttpMessageNotReadableException;
@@ -20,10 +26,6 @@
2026
import org.springframework.web.bind.MissingServletRequestParameterException;
2127
import org.springframework.web.bind.annotation.ControllerAdvice;
2228
import org.springframework.web.bind.annotation.ExceptionHandler;
23-
import java.time.Instant;
24-
import java.util.Map;
25-
import java.util.Objects;
26-
import java.util.stream.Collectors;
2729
import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException;
2830
import org.springframework.web.multipart.support.MissingServletRequestPartException;
2931

@@ -240,6 +242,19 @@ public ResponseEntity<ErrorResponse> handleOptimisticLockingFailure(
240242
null));
241243
}
242244

245+
@ExceptionHandler(PessimisticLockingFailureException.class)
246+
public ResponseEntity<ErrorResponse> handlePessimisticLockingFailure(
247+
PessimisticLockingFailureException ex, HttpServletRequest request) {
248+
log.warn("Pessimistic locking conflict: traceId={}", MDC.get(TRACE_ID_MDC));
249+
return ResponseEntity.status(HttpStatus.CONFLICT)
250+
.body(buildResponse(
251+
request,
252+
ErrorCode.COMMON_CONFLICT,
253+
"This competition is currently being edited; please try again.",
254+
HttpStatus.CONFLICT.value(),
255+
null));
256+
}
257+
243258
/**
244259
* Handles {@link MissingServletRequestPartException} by generating an
245260
* appropriate error response. This exception is thrown when a required part of

src/main/resources/application.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ spring:
1919
show-sql: ${JPA_SHOW_SQL}
2020
open-in-view: false
2121
properties:
22+
jakarta:
23+
persistence:
24+
lock:
25+
timeout: 3000
2226
hibernate.default_batch_fetch_size: 50
2327
hibernate:
2428
ddl-auto: ${JPA_HIBERNATE_DDL_AUTO:validate}

src/test/java/com/itasocialacademy/oitassist/competition/controller/CompetitionControllerTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.mockito.ArgumentCaptor;
3232
import org.mockito.InjectMocks;
3333
import org.mockito.Mock;
34+
import org.springframework.dao.PessimisticLockingFailureException;
3435
import org.springframework.data.domain.Page;
3536
import org.springframework.data.domain.PageImpl;
3637
import org.springframework.data.domain.Pageable;
@@ -259,6 +260,19 @@ void changeStatus_staleVersion_shouldReturn409() throws Exception {
259260
.andExpect(status().isConflict());
260261
}
261262

263+
@Test
264+
void changeStatus_pessimisticLockConflict_shouldReturn409() throws Exception {
265+
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.PUBLISHED, 1L);
266+
267+
when(competitionService.changeStatus(eq(1L), eq(request)))
268+
.thenThrow(new PessimisticLockingFailureException("Lock wait timeout exceeded"));
269+
270+
mockMvc.perform(patch("/api/v1/competitions/{id}/status", 1L)
271+
.contentType(MediaType.APPLICATION_JSON)
272+
.content(objectMapper.writeValueAsString(request)))
273+
.andExpect(status().isConflict());
274+
}
275+
262276
@Test
263277
void changeStatus_invalidRequest_nullVersion_shouldReturn400() throws Exception {
264278
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.PUBLISHED, null);

src/test/java/com/itasocialacademy/oitassist/competition/service/CompetitionServiceTest.java

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ void setUp() {
9898
@Test
9999
void changeStatus_draftToEnrollment_shouldSucceed() {
100100
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.ENROLLMENT, 1L);
101-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
101+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
102102

103103
when(stageRepository.existsByCompetitionId(1L)).thenReturn(true);
104104
when(stageRepository.countStagesWithoutTours(1L)).thenReturn(0L);
@@ -115,7 +115,7 @@ void changeStatus_draftToEnrollment_shouldSucceed() {
115115
@Test
116116
void changeStatus_draftToPublished_directly_shouldThrowInvalidTransition() {
117117
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.PUBLISHED, 1L);
118-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
118+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
119119

120120
doThrow(new CompetitionHierarchyValidationException("Invalid status transition from DRAFT to PUBLISHED"))
121121
.when(validator).validateCompetitionStatusTransition(CompetitionStatus.DRAFT, CompetitionStatus.PUBLISHED);
@@ -133,7 +133,8 @@ void changeStatus_enrollmentToPublished_withValidHierarchy_shouldSucceed() {
133133
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.PUBLISHED, 1L);
134134

135135
competition.setCompetitionStatus(CompetitionStatus.ENROLLMENT);
136-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
136+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
137+
137138
when(stageRepository.existsByCompetitionId(1L)).thenReturn(true);
138139
when(stageRepository.countStagesWithoutTours(1L)).thenReturn(0L);
139140
when(competitionRepository.save(any(Competition.class))).thenReturn(competition);
@@ -150,7 +151,7 @@ void changeStatus_enrollmentToPublished_withNoStages_shouldThrowException() {
150151
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.PUBLISHED, 1L);
151152

152153
competition.setCompetitionStatus(CompetitionStatus.ENROLLMENT);
153-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
154+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
154155
when(stageRepository.existsByCompetitionId(1L)).thenReturn(false);
155156

156157
CompetitionHierarchyValidationException exception = assertThrows(
@@ -166,7 +167,7 @@ void changeStatus_enrollmentToPublished_withEmptyStages_shouldThrowException() {
166167
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.PUBLISHED, 1L);
167168

168169
competition.setCompetitionStatus(CompetitionStatus.ENROLLMENT);
169-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
170+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
170171
when(stageRepository.existsByCompetitionId(1L)).thenReturn(true);
171172
when(stageRepository.countStagesWithoutTours(1L)).thenReturn(2L);
172173

@@ -183,7 +184,7 @@ void changeStatus_enrollmentToDraft_shouldThrowInvalidTransition() {
183184
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.DRAFT, 1L);
184185

185186
competition.setCompetitionStatus(CompetitionStatus.ENROLLMENT);
186-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
187+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
187188

188189
doThrow(new CompetitionHierarchyValidationException("Invalid status transition from ENROLLMENT to DRAFT"))
189190
.when(validator).validateCompetitionStatusTransition(CompetitionStatus.ENROLLMENT, CompetitionStatus.DRAFT);
@@ -200,7 +201,7 @@ void changeStatus_invalidTransition_publishedToDraft_shouldThrowException() {
200201
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.DRAFT, 1L);
201202

202203
competition.setCompetitionStatus(CompetitionStatus.PUBLISHED);
203-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
204+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
204205

205206
doThrow(new CompetitionHierarchyValidationException("Invalid status transition from PUBLISHED to DRAFT"))
206207
.when(validator).validateCompetitionStatusTransition(CompetitionStatus.PUBLISHED, CompetitionStatus.DRAFT);
@@ -219,7 +220,7 @@ void changeStatus_publishedToFinished_withAllStagesCompleted_shouldSucceed() {
219220
CompetitionStatus.FINISHED, 1L);
220221

221222
competition.setCompetitionStatus(CompetitionStatus.PUBLISHED);
222-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
223+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
223224
when(competitionRepository.save(any(Competition.class))).thenReturn(competition);
224225
when(mapper.toResponse(any(Competition.class))).thenReturn(getCompetitionResponse());
225226

@@ -236,7 +237,7 @@ void changeStatus_publishedToFinished_withIncompleteStage_shouldThrowException()
236237
CompetitionStatus.FINISHED, 1L);
237238

238239
competition.setCompetitionStatus(CompetitionStatus.PUBLISHED);
239-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
240+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
240241

241242
doThrow(new CompetitionHierarchyValidationException(
242243
"Cannot finish competition: Not all stages are completed. "
@@ -283,7 +284,7 @@ void create_validRequest_shouldSetDraftStatusAndSave() {
283284
@Test
284285
void changeStatus_versionMismatch_shouldThrowStaleEntityVersionException() {
285286
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.ENROLLMENT, 5L);
286-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
287+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
287288

288289
doThrow(new StaleEntityVersionException(Competition.class, 1L))
289290
.when(validator).validateEntityVersion(anyLong(), anyLong(), any(), anyLong());
@@ -297,7 +298,7 @@ void changeStatus_versionMismatch_shouldThrowStaleEntityVersionException() {
297298
@Test
298299
void changeStatus_versionMatches_shouldProceedToTransitionValidation() {
299300
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.ENROLLMENT, 1L);
300-
when(competitionRepository.findById(1L)).thenReturn(Optional.of(competition));
301+
when(validator.lockCompetitionForUpdate(1L)).thenReturn(competition);
301302
when(stageRepository.existsByCompetitionId(1L)).thenReturn(true);
302303
when(stageRepository.countStagesWithoutTours(1L)).thenReturn(0L);
303304
when(competitionRepository.save(any(Competition.class))).thenReturn(competition);
@@ -308,6 +309,16 @@ void changeStatus_versionMatches_shouldProceedToTransitionValidation() {
308309
verify(validator).validateCompetitionStatusTransition(CompetitionStatus.DRAFT, CompetitionStatus.ENROLLMENT);
309310
}
310311

312+
@Test
313+
void changeStatus_competitionNotFound_shouldPropagateFromValidator() {
314+
ChangeCompetitionStatusRequest request = new ChangeCompetitionStatusRequest(CompetitionStatus.ENROLLMENT, 1L);
315+
when(validator.lockCompetitionForUpdate(99L)).thenThrow(new CompetitionNotFoundException(99L));
316+
317+
assertThrows(CompetitionNotFoundException.class, () -> competitionService.changeStatus(99L, request));
318+
319+
verify(competitionRepository, never()).save(any());
320+
}
321+
311322
// ---- getVisibleById ----
312323

313324
@Test

0 commit comments

Comments
 (0)