-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: 예약 취소 내 멱등성 구현 및 웨이팅 등록 유닛테스트 추가 #359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2ef3017
c90e3c7
a3a03d4
08c7bfd
b594ca5
9adb0c7
b4c0c4b
64ea4c6
8996da7
3388353
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.nowait.applicationuser.waiting.dto; | ||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Getter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class WaitingCancelIdempotencyValue { | ||
| private String state; | ||
| private CancelWaitingResponse response; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,6 @@ | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import java.time.LocalDateTime; | ||||||||||||||||||||||||||
| import java.time.format.DateTimeFormatter; | ||||||||||||||||||||||||||
| import java.util.Optional; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import org.springframework.context.ApplicationEventPublisher; | ||||||||||||||||||||||||||
| import org.springframework.stereotype.Service; | ||||||||||||||||||||||||||
|
|
@@ -13,6 +12,7 @@ | |||||||||||||||||||||||||
| import com.nowait.applicationuser.waiting.dto.GetWaitingSizeResponse; | ||||||||||||||||||||||||||
| import com.nowait.applicationuser.waiting.dto.RegisterWaitingRequest; | ||||||||||||||||||||||||||
| import com.nowait.applicationuser.waiting.dto.RegisterWaitingResponse; | ||||||||||||||||||||||||||
| import com.nowait.applicationuser.waiting.dto.WaitingCancelIdempotencyValue; | ||||||||||||||||||||||||||
| import com.nowait.applicationuser.waiting.dto.WaitingIdempotencyValue; | ||||||||||||||||||||||||||
| import com.nowait.applicationuser.waiting.event.AddWaitingRegisterEvent; | ||||||||||||||||||||||||||
| import com.nowait.applicationuser.waiting.redis.WaitingIdempotencyRepository; | ||||||||||||||||||||||||||
|
|
@@ -59,13 +59,11 @@ public class WaitingService { | |||||||||||||||||||||||||
| @Transactional | ||||||||||||||||||||||||||
| public RegisterWaitingResponse registerWaiting(CustomOAuth2User oAuth2User, String publicCode, RegisterWaitingRequest waitingRequest, HttpServletRequest httpServletRequest) { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| String idempotentKey = httpServletRequest.getHeader("Idempotency-Key"); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // TODO 멱등성 검증 로직 점검 필요 | ||||||||||||||||||||||||||
| Optional<WaitingIdempotencyValue> existingIdempotencyValue = waitingIdempotencyRepository.findByKey(idempotentKey); | ||||||||||||||||||||||||||
| if (existingIdempotencyValue.isPresent()) { | ||||||||||||||||||||||||||
| log.info("Existing idempotency key found: {}", idempotentKey); | ||||||||||||||||||||||||||
| return existingIdempotencyValue.get().getResponse(); | ||||||||||||||||||||||||||
| // TODO 멱등키 동시성 처리 로직 고려 필요 (분산락 등) | ||||||||||||||||||||||||||
| RegisterWaitingResponse registerWaitingResponse = validateIdempotency(httpServletRequest); | ||||||||||||||||||||||||||
| if (registerWaitingResponse != null) { | ||||||||||||||||||||||||||
| log.info("Idempotent request detected. Returning existing response."); | ||||||||||||||||||||||||||
| return registerWaitingResponse; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // TODO 유저 및 주점 존재 검증은 공통으로 많이 쓰이니 AOP로 빼는게 좋을 듯 | ||||||||||||||||||||||||||
|
|
@@ -75,19 +73,15 @@ public RegisterWaitingResponse registerWaiting(CustomOAuth2User oAuth2User, Stri | |||||||||||||||||||||||||
| User user = userRepository.findById(oAuth2User.getUserId()) | ||||||||||||||||||||||||||
| .orElseThrow(UserNotFoundException::new); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 일일 가능 웨이팅 최대 개수 초과 검증 | ||||||||||||||||||||||||||
| // TODO race condition 발생 가능성 점검 필요, DB 저장 로직 실패 시 롤백 처리 필요 | ||||||||||||||||||||||||||
| waitingRedisRepository.incrementAndCheckWaitingLimit(user.getId(), 3L); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 웨이팅 고유 번호 생성 - YYYYMMDD-storeId-sequence number 일련 번호 | ||||||||||||||||||||||||||
| Long storeId = store.getStoreId(); | ||||||||||||||||||||||||||
| LocalDateTime timestamp = LocalDateTime.now(); | ||||||||||||||||||||||||||
| String waitingNumber = generateWaitingNumber(storeId, timestamp); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 멱등키 검증 - 이미 동일한 멱등키로 등록된 웨이팅이 있는지 확인 | ||||||||||||||||||||||||||
| // waitingRedisRepository.idempotentKeyKeyExists(idempotentKey, ReservationStatus.WAITING.name()); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 일일 가능 웨이팅 최대 개수 초과 검증 | ||||||||||||||||||||||||||
| // TODO race condition 발생 가능성 점검 필요 | ||||||||||||||||||||||||||
| waitingRedisRepository.incrementAndCheckWaitingLimit(user.getId(), 3L); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // DB에 상태 값 저장 | ||||||||||||||||||||||||||
| Reservation reservation = Reservation.builder() | ||||||||||||||||||||||||||
| .reservationNumber(waitingNumber) | ||||||||||||||||||||||||||
|
|
@@ -115,25 +109,27 @@ public RegisterWaitingResponse registerWaiting(CustomOAuth2User oAuth2User, Stri | |||||||||||||||||||||||||
| .partySize(waitingRequest.getPartySize()) | ||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 멱등키가 있다면 멱등 응답 저장 | ||||||||||||||||||||||||||
| waitingIdempotencyRepository.saveIdempotencyValue(idempotentKey, response); | ||||||||||||||||||||||||||
| // TODO 멱등키 응답 실패 시 어떻게 처리할 지 점검 필요 | ||||||||||||||||||||||||||
| saveIdempotencyResponse(httpServletRequest.getHeader("Idempotency-Key"), response); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| @Transactional | ||||||||||||||||||||||||||
| public CancelWaitingResponse cancelWaiting(CustomOAuth2User oAuth2User, String publicCode, CancelWaitingRequest request) { | ||||||||||||||||||||||||||
| public CancelWaitingResponse cancelWaiting(CustomOAuth2User oAuth2User, String publicCode, CancelWaitingRequest request, HttpServletRequest httpServletRequest) { | ||||||||||||||||||||||||||
| // TODO 멱등키 동시성 처리 로직 고려 필요 (분산락 등) | ||||||||||||||||||||||||||
| CancelWaitingResponse cancelWaitingResponse = validateCancelIdempotency(httpServletRequest); | ||||||||||||||||||||||||||
| if (cancelWaitingResponse != null) { | ||||||||||||||||||||||||||
| log.info("Idempotent request detected. Returning existing response."); | ||||||||||||||||||||||||||
| return cancelWaitingResponse; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Store store = storeRepository.findByPublicCodeAndDeletedFalse(publicCode).orElseThrow(StoreNotFoundException::new); | ||||||||||||||||||||||||||
| Long storeId = store.getStoreId(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| User user = userRepository.findById(oAuth2User.getUserId()) | ||||||||||||||||||||||||||
| .orElseThrow(UserNotFoundException::new); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // TODO 멱등키 검증 로직 점검 필요 | ||||||||||||||||||||||||||
| // String idempotentKey = generateIdempotentKey(storeId, user.getId()); | ||||||||||||||||||||||||||
| // waitingRedisRepository.idempotentKeyKeyExists(idempotentKey, ReservationStatus.CANCELLED.name()); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // DB 웨이팅 상태 취소 처리 | ||||||||||||||||||||||||||
| Reservation reservation = reservationRepository.findReservationByReservationNumber(request.getWaitingNumber()) | ||||||||||||||||||||||||||
| .orElseThrow(ReservationNotFoundException::new); | ||||||||||||||||||||||||||
|
|
@@ -143,28 +139,62 @@ public CancelWaitingResponse cancelWaiting(CustomOAuth2User oAuth2User, String p | |||||||||||||||||||||||||
| // Redis 대기열 취소 이벤트 발행 | ||||||||||||||||||||||||||
| waitingRedisRepository.removeWaiting(storeId, user.getId()); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return CancelWaitingResponse.builder() | ||||||||||||||||||||||||||
| CancelWaitingResponse response = CancelWaitingResponse.builder() | ||||||||||||||||||||||||||
| .waitingNumber(reservation.getReservationNumber()) | ||||||||||||||||||||||||||
| .storeId(storeId) | ||||||||||||||||||||||||||
| .reservationStatus(reservation.getStatus()) | ||||||||||||||||||||||||||
| .canceledAt(reservation.getUpdatedAt()) | ||||||||||||||||||||||||||
| .message("대기 취소가 완료되었습니다.") | ||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 멱등키가 있다면 멱등 응답 저장 | ||||||||||||||||||||||||||
| waitingIdempotencyRepository.saveCancelIdempotencyValue(httpServletRequest.getHeader("Idempotency-Key"), response); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
|
Comment on lines
+149
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 취소 경로에서 멱등키 null/blank 검사 없이
멱등키 없이 취소 요청이 들어오면 null 키로 Redis에 저장을 시도하여 예외가 발생합니다. 🐛 수정 제안- // 멱등키가 있다면 멱등 응답 저장
- waitingIdempotencyRepository.saveCancelIdempotencyValue(httpServletRequest.getHeader("Idempotency-Key"), response);
+ // 멱등키가 있다면 멱등 응답 저장
+ String idempotentKey = httpServletRequest.getHeader("Idempotency-Key");
+ if (idempotentKey != null && !idempotentKey.isBlank()) {
+ waitingIdempotencyRepository.saveCancelIdempotencyValue(idempotentKey, response);
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 멱등키 검증 메서드 | ||||||||||||||||||||||||||
| private RegisterWaitingResponse validateIdempotency(HttpServletRequest httpServletRequest) { | ||||||||||||||||||||||||||
| String idempotentKey = httpServletRequest.getHeader("Idempotency-Key"); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 멱등키 검증 - 이미 동일한 멱등키로 등록된 웨이팅이 있는지 확인 | ||||||||||||||||||||||||||
| // TODO 멱등성 검증 로직 점검 필요 | ||||||||||||||||||||||||||
| return waitingIdempotencyRepository.findByKey(idempotentKey) | ||||||||||||||||||||||||||
| .map(WaitingIdempotencyValue::getResponse) | ||||||||||||||||||||||||||
| .orElse(null); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private CancelWaitingResponse validateCancelIdempotency(HttpServletRequest httpServletRequest) { | ||||||||||||||||||||||||||
| String idempotentKey = httpServletRequest.getHeader("Idempotency-Key"); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 멱등키 검증 - 이미 동일한 멱등키로 등록된 웨이팅이 있는지 확인 | ||||||||||||||||||||||||||
| // TODO 멱등성 검증 로직 점검 필요 | ||||||||||||||||||||||||||
| return waitingIdempotencyRepository.findByCancelKey(idempotentKey) | ||||||||||||||||||||||||||
| .map(WaitingCancelIdempotencyValue::getResponse) | ||||||||||||||||||||||||||
| .orElse(null); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+156
to
175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
멱등키가 선택적(optional)이라면, null/blank 체크 후 바로 🐛 수정 제안 private RegisterWaitingResponse validateIdempotency(HttpServletRequest httpServletRequest) {
String idempotentKey = httpServletRequest.getHeader("Idempotency-Key");
+ if (idempotentKey == null || idempotentKey.isBlank()) {
+ return null;
+ }
return waitingIdempotencyRepository.findByKey(idempotentKey)
.map(WaitingIdempotencyValue::getResponse)
.orElse(null);
}
private CancelWaitingResponse validateCancelIdempotency(HttpServletRequest httpServletRequest) {
String idempotentKey = httpServletRequest.getHeader("Idempotency-Key");
+ if (idempotentKey == null || idempotentKey.isBlank()) {
+ return null;
+ }
return waitingIdempotencyRepository.findByCancelKey(idempotentKey)
.map(WaitingCancelIdempotencyValue::getResponse)
.orElse(null);
}🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 멱등키 응답 저장 메서드 | ||||||||||||||||||||||||||
| private void saveIdempotencyResponse(String idempotentKey, RegisterWaitingResponse response) { | ||||||||||||||||||||||||||
| if (idempotentKey != null && !idempotentKey.isBlank()) { | ||||||||||||||||||||||||||
| waitingIdempotencyRepository.saveIdempotencyValue(idempotentKey, response); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // 현재 대기 인원 수 조회 | ||||||||||||||||||||||||||
| public GetWaitingSizeResponse getWaitingCount(CustomOAuth2User oAuth2User, String publicCode) { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Store store = storeRepository.findByPublicCodeAndDeletedFalse(publicCode) | ||||||||||||||||||||||||||
| .orElseThrow(StoreNotFoundException::new); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Long storeId = store.getStoreId(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| User user = userRepository.findById(oAuth2User.getUserId()) | ||||||||||||||||||||||||||
| .orElseThrow(UserNotFoundException::new); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Department department = departmentRepository.findById(store.getDepartmentId()) | ||||||||||||||||||||||||||
| .orElseThrow(DepartmentNotFoundException::new); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Long storeId = store.getStoreId(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Long waitingCount = waitingRedisRepository.getWaitingCount(storeId); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return GetWaitingSizeResponse.builder() | ||||||||||||||||||||||||||
|
|
@@ -190,8 +220,4 @@ private String generateWaitingNumber(Long storeId, LocalDateTime timestamp) { | |||||||||||||||||||||||||
| // 4) 최종 ID 조합 | ||||||||||||||||||||||||||
| return today + "-" + storeId + "-" + seqStr; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private String generateIdempotentKey(Long storeId, Long userId) { | ||||||||||||||||||||||||||
| return "idempotentKey" + ":" + storeId + ":" + userId; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
멱등성 검증과 저장 사이의 경합 조건(Race Condition)에 대해.
TODO 주석(Line 62)에 언급된 대로, 멱등키 검증(
validateIdempotency)과 응답 저장(saveIdempotencyValue) 사이에 경합 조건이 존재합니다. 동일한 멱등키를 가진 두 요청이 동시에 도달하면 둘 다 검증을 통과하여 중복 처리가 발생할 수 있습니다.일반적인 해결 방안:
SETNX(SET if Not eXists)를 활용하여 멱등키를 "PROCESSING" 상태로 먼저 선점한 후 작업 수행현재 TODO로 인지하고 계신 것으로 보이지만, 프로덕션 배포 전에 반드시 해결이 필요한 사항입니다.
🤖 Prompt for AI Agents