From 74c389ee1e183072017f7cb1dd3fcafc072a2b83 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:00:47 +0800 Subject: [PATCH] fix(review): scope concurrent write conflicts Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../exception/GlobalExceptionHandler.java | 6 +----- .../exception/GlobalExceptionHandlerTest.java | 19 +++++++------------ .../exception/DomainConflictException.java | 16 ++++++++++++++++ .../domain/social/SkillRatingService.java | 11 +++++++++-- .../domain/social/SkillRatingServiceTest.java | 17 +++++++++++++++++ 5 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java index e64abc56..ba93bb1d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java @@ -13,7 +13,6 @@ import jakarta.servlet.http.HttpServletRequest; import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -70,10 +69,7 @@ public class GlobalExceptionHandler { return renderLocalizedError(ex, HttpStatus.valueOf(ex.statusCode()), request); } - @ExceptionHandler({ - ObjectOptimisticLockingFailureException.class, - DataIntegrityViolationException.class - }) + @ExceptionHandler(ObjectOptimisticLockingFailureException.class) public ResponseEntity> handlePersistenceConflict( RuntimeException ex, HttpServletRequest request) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java index dd12fb51..82b2a3b6 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java @@ -30,7 +30,6 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.slf4j.LoggerFactory; import org.springframework.context.support.StaticMessageSource; -import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -154,19 +153,15 @@ class GlobalExceptionHandlerTest { attachAppender(); prepareClientErrorRequest("PUT", "/api/v1/skills/10/reviews/me"); - for (RuntimeException exception : List.of( - new ObjectOptimisticLockingFailureException(SkillRating.class, 7L), - new DataIntegrityViolationException("duplicate key value violates constraint user-secret"))) { - ResponseEntity> response = handler.handlePersistenceConflict(exception, request); + ResponseEntity> response = handler.handlePersistenceConflict( + new ObjectOptimisticLockingFailureException(SkillRating.class, 7L), request); - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); - assertThat(response.getBody()).isNotNull(); - assertThat(response.getBody().code()).isEqualTo(409); - assertThat(response.getBody().msg()).isEqualTo("Refresh and try again"); - } + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().code()).isEqualTo(409); + assertThat(response.getBody().msg()).isEqualTo("Refresh and try again"); assertThat(loggedMessages()).allSatisfy(message -> assertThat(message) - .doesNotContain("user-secret") - .doesNotContain("duplicate key")); + .doesNotContain("user-secret")); } @Test diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java new file mode 100644 index 00000000..085c593a --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java @@ -0,0 +1,16 @@ +package com.iflytek.skillhub.domain.shared.exception; + +/** + * Domain exception used when a concurrent request prevents a safe state change. + */ +public class DomainConflictException extends LocalizedDomainException { + + public DomainConflictException(String messageCode, Object... messageArgs) { + super(messageCode, messageArgs); + } + + @Override + public int statusCode() { + return 409; + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java index 4cf954df..879b7eac 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java @@ -1,10 +1,12 @@ package com.iflytek.skillhub.domain.social; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.social.event.SkillRatedEvent; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -50,8 +52,13 @@ public class SkillRatingService { SkillRating rating = ratingRepository.findBySkillIdAndUserId(skillId, userId) .orElseGet(() -> new SkillRating(skillId, userId, score)); rating.updateReview(score, reviewText); - SkillRating saved = ratingRepository.save(rating); - ratingRepository.flush(); + SkillRating saved; + try { + saved = ratingRepository.save(rating); + ratingRepository.flush(); + } catch (DataIntegrityViolationException exception) { + throw new DomainConflictException("error.request.conflict"); + } eventPublisher.publishEvent(new SkillRatedEvent(skillId, userId, score)); return saved; } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java index 07785ed4..45b98388 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.domain.social; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; @@ -11,6 +12,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.*; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import java.util.Optional; @@ -118,6 +120,21 @@ class SkillRatingServiceTest { .isInstanceOf(DomainBadRequestException.class); } + @Test + void upsertReview_mapsConcurrentFirstInsertToConflict() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.empty()); + when(ratingRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0)); + doThrow(new DataIntegrityViolationException("duplicate rating")) + .when(ratingRepository).flush(); + + assertThatThrownBy(() -> service.upsertReview(1L, "user-1", (short) 4, "Useful")) + .isInstanceOf(DomainConflictException.class) + .hasMessage("error.request.conflict"); + + verify(eventPublisher, never()).publishEvent(any(SkillRatedEvent.class)); + } + @Test void clearReview_keeps_rating_row_and_score() { when(skillRepository.findById(1L)).thenReturn(Optional.of(skill()));