[22기_변호영] 동시성 & 결제 연동 미션 제출합니다. #29

Hoyoung027 · 2025-10-25T14:39:28Z

gilbert09031

과제 수고하셨습니다 🔥

gilbert09031 · 2025-10-27T03:55:47Z

src/main/java/com/ceos22/cgv/module/movie/dto/MovieLikeResponse.java

+        boolean isLiked
+) {
+
+    public static MovieLikeResponse fromMovieAndUser(


from, of만 쓰는 것 보다 이렇게 네이밍 하는 것이 더 읽기 편하네요!
저도 수정해보겠습니다-!

저도 읽기 불편해서 지난번에 싹 갈아엎어버렸습니다! 좋은거같아용

gilbert09031 · 2025-10-27T03:58:19Z

src/main/java/com/ceos22/cgv/module/movie/dto/MovieResponse.java


-    public static MovieResponse of(Movie movie){
-        return new MovieResponse(movie.getId(), movie.getTitle(), null, null, null, null, null, null, null);
+    public static MovieResponse fromMovieToMovieSummary(Movie movie){


MovieSummaryResponse를 따로 만드는 것은 어떨까요 ?!

gilbert09031 · 2025-10-27T04:05:33Z

src/main/java/com/ceos22/cgv/module/movie/service/MovieService.java

                .map(s -> {
                    int totalSeat = s.getTheater().getRow() * s.getTheater().getColumn();
-                    int reserved = reservationSeatRepository.countByScheduleId(s.getId());
+                    long reserved = reservationSeatRepository.countByScheduleId(s.getId());


취소된 상태의 좌석은 제외하는 로직은 다른 부분에 포함되어 있을까요 ?

저는 reservation이라는 예약 테이블이 존재하고 예약된 좌석은 reservationSeat 테이블에 예약id, 상영일정id, 좌석 열, 좌석 행 형태로 unique 조건을 걸어 중복 예약을 방지하고 있습니다.

reservation을 취소하는 api에 아래 처럼 코드를 추가하여 reservationSeat의 데이터를 삭제하는 방법으로 취소된 좌석을 DB에서 삭제하고 있습니다!

public class ReservationService{ @Transactional public ReservationResponse cancel(Long reservationId, User authenticatedUser) { ... reservation.cancel(); // 예약 취소 reservation.getReservationSeats().clear(); // reservationSeat에서 예약된 좌석 제거 ... } }

gilbert09031 · 2025-10-27T04:08:15Z

src/main/java/com/ceos22/cgv/module/movie/service/MovieService.java

+        User user = userRepository.findById(authenticatedUser.getId())
+                .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "해당 유저가 존재하지 않습니다."));


authenticatedUser가 만약 이미 인증된 사용자라면, 다시 DB조회를 통해 확인하지 않아도 괜찮지 않을까요?

정확하십니다. 지난번 리펙토링 할 때 고려했던 내용인데 미처 발견하지 못한거같아요.
혁씨 눈썰미 기가 막혀~

gilbert09031 · 2025-10-27T04:11:01Z

src/main/java/com/ceos22/cgv/module/movie/service/MovieService.java

    @Transactional(readOnly = true)
    public MovieListResponse findMovies(MovieRequest request) {
-        return MovieListResponse.fromEntities(movieRepository.findAll());
+        return MovieListResponse.fromMovies(movieRepository.findAll());
    }


파라미터로 받는 request은 어디서 사용되는지 궁금합니다 -!

사실 이게 사용하지 않는 메소드인데요, 그렇다보니 제가 오류가 없는지 검증도 안하고 있었던 것 같습니다. 제거하도록 하겠습니당! 감사해요

gilbert09031 · 2025-10-27T04:23:55Z

src/main/java/com/ceos22/cgv/module/payment/service/PaymentService.java

+        // 매점 결제 확정 및 재고 차감
+        // 재고 부족 및 lock 획득 실패에 대한 예외처리
+        if(!isReservation){


처리하지 않은 오류에 대해서는 결제만 되고, 오류가 있어도 취소가 안되는 상황이 생길 것 같아요!
모든 오류에 대해서도 결제 취소를 보장하는 방법이 필요할 것 같아요 !

gilbert09031 · 2025-10-27T04:32:07Z

src/main/java/com/ceos22/cgv/module/payment/service/PaymentTransactionService.java

+    public String createPaymentId(){
+        long count = paymentRepository.count();
+        long next = count + 1;
+        String seq = String.format("%04d", next);
+        return STORE_ID + "_" + seq;
+    }


여러 스레드가 동시에 이 메서드에 접근할 때 중복 Id가 생길 수 있지 않을까요?

맞는 말씀이십니다. 여러 스레드가 동시에 결제를 시도하는 상황에 대해서는 분명 중복된 paymentId가 생성될 수 있을 것 같아요. 다만, 결제 시스템의 외부 API 연동 과정에서 paymentId가 중복되면 결제가 이루어지지 않는 것으로 이해하여 결제 성공은 1건만 발생할 것 같습니다.

구체적인 예시를 들여다보면

결제를 원하는 스레드 A, B가 있다고 가정 + paymentLog는 1개로 가정

A가 결제 API 호출하고 그 직후 B가 결제 API 호출

A가 paymentId "0002"을 획득한 직후 B도 "0002"을 획득

외부 API 호출로 A의 결제가 수행 -> paymentLog 1개 증가 (paymentLog 데이터 2개로 업데이트)

외부 API 호출로 B의 결제가 수행 -> 중복 paymentId 이므로 외부 API 서버에서 결제 취소

결론적으로 빠르게 수행되는 스레드가 결제에 성공하고 다른 것은 결제를 실패 (외부 API 결제 실패로 내부에 로그도 저장 X)

이 상황은 paymentId의 네이밍에 payment의 개수만 반영한다는 한계가 있고 그걸 계산하기 위해 paymentRepository.count();를 사용하고 있기 때문입니다. 이 규칙을 변형하여 userId를 추가로 paymentId에 도입하는 방식으로 제시한 문제를 피해갈 수 있을 것 같네요.

허나 만약 이 규칙을 고수한다면, 메서드의 동시성 제어 뿐만 아니라 앞선 결제가 성공하고 서비스의 DB에 해당 결제 내역이 반영될 때까지 본 API에 대한 모든 접근을 차단해야할 필요가 있을 것 같은데 이는 서비스 사용자 입장에서 굉장한 불편함이 될 것 같습니다. 어떻게 해결하는게 좋을지 혁님은 제안해주실 방법이 있을까요?

저도 고민해보겠습니다....!

gilbert09031 · 2025-10-27T04:39:19Z

src/main/java/com/ceos22/cgv/module/payment/service/PaymentService.java

+    }
+
+    @Transactional
+    public PaymentResponse cancelPayment(Long paymentLogId) {


결제시스템에서 취소 요청이 정상적으로 처리된 이후에 내부에서 예약/주문 취소를 하는 것이 안전하지 않을까요 ?

이 부분은 저도 고민을 좀 해보다가, 의도적으로 외부 결제 시스템에서의 취소를 가장 후순위로 미뤘습니다. 이유는 다음과 같아요. (약간의 제 고집이 섞여있습니다...)

우리 서비스의 손실을 최소화해야하지 않을까?

저는 재고에 대한 동시성 문제를 PessimisticLock으로 구현하였습니다.
만약 외부 결제 취소가 이루어진 상황에서 lcok에 의해 내부 수량을 주문 이전으로 복원 하지 못한다면, 재고가 실제로 있음에도 불구하고 주문을 받지 못하는 상황에 처할 수 있을 것 같아요. 이때 돈은 이미 환불되었으므로 서비스 입장에서는 손해가 될 것입니다. 그래서 순서를 뒤집었고 lock에 의한 취소가 최대한 발생하지 않도록 lock 획득에 대한 재시도 로직을 추가하여 사용자가 주문 취소에서도 다시 요청을 보내야 하는 상황을 최소화하고자 했습니다.
물론, 현재 구현된 코드로는 내부 로그에서 결제 취소가 되었다고 기록되어도 외부 결제 시스템에 문제가 생기면 문제가 생길 수 있을 것 같아요. 이걸 해결하기 위해선 결제 취소 실패시 재시도 로직을 작성해야 할까요? 어떻게 하는게 좋을지 고민해보겠습니다.

외부 결제가 실패할 가능성이 있는가?

외부 결제는 10% 확률로 실패하나, 결제 취소는 이런 조건이 없어 100% 성공한다고 가정했습니다. 그래서 1번에서 제시한 이유를 바탕으로 순서를 뒤집기로 결론 내렸어요.

gilbert09031 · 2025-10-27T04:42:19Z

src/main/java/com/ceos22/cgv/module/payment/service/PaymentTransactionService.java

+        reservation.cancel();
+        reservation.getReservationSeats().clear();
+        reservationRepository.save(reservation);


연관된 reservationSeats를 clear하는 과정을 reservationl.cancel에 포함시키는 것도 좋을 것 같아요 !

좋은데요? 당장 도입하겠습니다

Immmii

과제 수고하셨습니다!!
많이 배워갑니다

Immmii · 2025-10-27T08:08:26Z

src/main/java/com/ceos22/cgv/module/payment/service/PaymentService.java

+
+    @Transactional(readOnly = true)
+    public PaymentResponse getPayment(Long paymentLogId) {
+        String paymentId = paymentRepository.findById(paymentLogId)


서비스 레이어에서 paymentId 찾아다 줄 Reader 객체를 따로 분리해봐도 좋을 것 같아요
그러면 PaymentService에서 PaymentRepository 의존 안해도 돼요

사실 처음 수아님의 리뷰를 봤을 때는 Saver, Reader 객체까지 분리하는 것이 너무 과도한 책임 분리는 아닐까? 하는 생각이 들었습니다. 허나 조금 찾아보고 고민도 해보니 서비스에서 여러 Repository에 접근해야하거나 DB 교체 등이 일어날 때 분명한 장점이 있을 것 같아요.

제 코드에 어떻게 반영해볼까를 고민했는데요, 저는 현재 DDD style로 코드를 구현하고 있습니다. 하나의 기능을 수행하는 묶음이 폴더로 존재하고 그 안에 모든 controller, repository, service 코드가 존재하도록 만들었습니다. 따라서 payment를 제외하고는 다른 도메인의 repository에 접근하지 않도록 구현했기 때문에 saver와 reader 객체를 중간 레이어로 추가 도입할 필요성은 없을 것 같아요.

허나 결제 관련한 도메인에서는 order, reservation과 엮여있는 부분이 있다보니 paymentService에서 paymentLogRepostory 외에 다른 레포지토리에 접근하는 경우가 있는데요, 여기에서는 saver, reader 도입을 고민해볼 만한할 것 같다고 생각합니다.

좋은 제안 감사드립니다!

Immmii · 2025-10-27T08:12:31Z

src/main/java/com/ceos22/cgv/module/payment/service/PaymentTransactionService.java

+    public void cancelPaymentLog(PaymentLog paymentLog) {
+        paymentLog.cancel();
+        paymentRepository.save(paymentLog);
+    }


여기서도 Repository를 여러개 참조하게 되는데
Service레이어에 Saver 객체 따로 분리해서 Repository에paymentLog save 해줄 메서드 하나만 구현해두면
Repository 의존 없이 Saver만 갖다쓰면 돼요

Immmii · 2025-10-27T08:16:37Z

src/main/java/com/ceos22/cgv/module/payment/service/PaymentTransactionService.java

+            menu.increaseQuantity(item.getQuantity());
+        }
+    }
+


배워갑니다!!

Hoyoung027 added 6 commits October 15, 2025 13:23

fix : 코드리뷰 반영 1차

7229c94

fix : 코드리뷰 반영 2차

f351e18

feat : swagger 도입

47aeae2

feat : 영화 예약 동시성 문제 해결

21a10fd

feat : 결제 시스템 연동 및 매점 재고 동시성 제어

bbb7f46

docs : README 업데이트

82aa23a

gilbert09031 reviewed Oct 27, 2025

View reviewed changes

Immmii reviewed Oct 27, 2025

View reviewed changes

Gothax merged commit 0b30264 into CEOS-Developers:Hoyoung027 Oct 30, 2025

		User user = userRepository.findById(authenticatedUser.getId())
		.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "해당 유저가 존재하지 않습니다."));

[22기_변호영] 동시성 & 결제 연동 미션 제출합니다. #29

[22기_변호영] 동시성 & 결제 연동 미션 제출합니다. #29

Uh oh!

Conversation

Hoyoung027 commented Oct 25, 2025

Uh oh!

gilbert09031 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Immmii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants