Skip to content

Conversation

@nonactress
Copy link

@nonactress nonactress commented Nov 26, 2025

안녕하세요! 남해윤 리뷰어님! 리뷰어-리뷰이로 만나는 것은 처음인 것 같습니다.
잘 부탁드립니다 :)

🔹 예약 생성 로직 개선 (멱등성 + 동시성 의도)

  • 클라이언트가 요청 시 멱등성 키(Idempotency-Key)를 헤더로 보내면 그대로 사용하고,
    보내지 않은 경우에는 Controller에서 UUID를 생성하여 요청을 식별하도록 했습니다.
  • 같은 멱등성 키로 들어온 반복 요청은 기존 예약을 그대로 반환하도록 하여, 중복 예약 생성을 막는 멱등성 보장을 구현했습니다.
  • 요청은 DTO로 받고, DTO → Entity 변환을 거쳐 Service → Repository → DB 저장 흐름을 따릅니다.
  • insert 이후 DB에서 자동 생성된 id 값을 가져오는 구조로 변경했습니다.

❓궁금한 점

  1. 동시 요청 처리

    • 만약 두 요청이 거의 동시에 들어와 각각 UUID를 새로 생성했다면, 멱등성 키만으로는 중복 예약이 생길 수 있는 가능성이 남아 있습니다.
  2. 멱등성 설계

    • 현재는 “헤더에 키 제공 또는 Controller가 UUID 생성” 흐름인데,
    • 클라이언트가 키를 직접 제공하게 하거나, 혹은 서버 쪽 멱등성 키 생성 로직을 보강하는 게 더 바람직한지 의견이 궁금합니다.

nonactress and others added 30 commits November 6, 2025 02:16
@nonactress nonactress changed the title Feature/hyeonjin3 [Spring Core] 그리디 서현진 8,9,10단계 제출합니다. Nov 26, 2025
@nonactress nonactress changed the title [Spring Core] 그리디 서현진 8,9,10단계 제출합니다. [Spring Core]서현진 8,9,10단계 제출합니다. Nov 26, 2025
@haeyoon1
Copy link

haeyoon1 commented Nov 28, 2025

안녕하세요 현진님! 반갑습니다🙌
저도 잘 부탁드려요~

🤔 PR에 대한 질문

본격적인 리뷰 전 먼저 pr에 작성해주신 부분에 대한 추가적인 설명을 해주실 수 있을까요?

클라이언트가 요청 시 멱등성 키(Idempotency-Key)를 헤더로 보내면 그대로 사용하고,
보내지 않은 경우에는 Controller에서 UUID를 생성하여 요청을 식별하도록 했습니다.

해당 부분은 createReservation 메서드에서의 로직에 대해 설명해주신걸까요?
말씀해주신 대로 예약을 생성하는 요청을 보낼 때, new-reservation.js에서 uuid를 생성하고 header에 담아서 보내주고 있더라구요.
image

현진님께서 Idempotency-Key를 생성하신 이유가 무엇인가요?🤔
해당 값이 어디서 사용되는지 잘 모르겠습니다ㅠㅠ

같은 멱등성 키로 들어온 반복 요청은 기존 예약을 그대로 반환하도록 하여, 중복 예약 생성을 막는 멱등성 보장을 구현했습니다.

만일 멱등성 보장을 위해 해당 key를 생성했다라고 한다면, 이는 모호한 것 같습니다!
설명을 위해 멱등성에대한 스터디 자료를 가져와보았습니다.

멱등성이란?

  • 같은 요청을 여러 번 보내더라도 서버에 미치는 결과(상태 변화)가 동일하게 유지되는 성질

  • 즉, 같은 요청을 여러 번 수행해도 서버의 상태나 응답이 변하지 않아야 한다.
    → 이는 REST API 설계의 핵심 원칙 중 하나
    GET, DELETE, PUT 요청에 대해 멱등성이 보장된다

제가 이해한대로라면 post 요청에서 멱등성 보장을 시도하신 것 같은데 이는 적절하지 않은 것 같아요!

또한 uuid는 요청을 보낼 때 마다 달라지기 때문에, 같은 멱등성 키(Idempotency-Key)로 들어온 반복 요청은 있을 수 없습니다!

따라서 중복 예약을 막는 로직은 Idempotency-Key를 통해서가 아닌,
동일 날짜시간을 통해 repository에서 비교해서 판별하는 것이고, 현재 해당 방식으로 로직을 잘 작성해주셨는데 설명에 오류가 있는 것 같습니다 (혹시 제가 잘못 이해한 것이라면 알려주세요😂)

@nonactress
Copy link
Author

nonactress commented Nov 28, 2025

해윤님의 답변을 보고 제가 멱등성 키를 왜 사용하려 했는지 그 본질을 생각해보게 하는 질문이여서 생각을 정리해보았습니다!


리뷰어님의 답변을 두개의 질문으로 정리해 볼 수 있을 것 같습니다!

  1. POST는 원래 멱등하지 않은데 왜 멱등성 키를 쓰는가?
  2. UUID는 매번 바뀌는데 어떻게 중복을 막는가?

제가 상정했던 상황을 설명하며 코드 로직의 배경을 설명해 보겠습니다!!

1) 네트워크 연결 불안정 시나리오

만약 서버가 요청을 받아 db에 저장하고 나서 클라이언트에게 “성공했다”는 응답을 보내려는 찰나에 네트워크가 끊긴다면 클라이언트는 응답을 못받았다고 생각하고 다시 요청을 시도 할 것이고 이를 막아주는 것이 멱등성 키이다 라고 생각하고 코드 로직을 짜보았습니다.

2) uuid는 생성시마다 달라지는데…???

사용자의 요청을 받은 프론트엔드에서 생성된 uuid가 서버로 post 요청을 보냈을 때 네트워크 오류가 난다면 다시 프론트엔드에서 다시 서버로 전송하려고 할때 똑같은 멱등성 키를 사용하여 같은 요청에 대해 처리를 하기 위해서 위와 같은 코드 구성을 해보았습니다!

3) Repository 중복 체크 vs 멱등성 키

repository에서 Date/Time 비교 로직은 비즈니스 규칙을 위한 ( 같은 시간을 예약하는 것을 방지 하기 위함) 것 이라고 생각하고 멱등성 키는 네트워크 통신 오류를 보장하기 위한 장치라고 생각하여 다른 역할을 한다고 생각합니다!

Copy link

@haeyoon1 haeyoon1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용자의 요청을 받은 프론트엔드에서 생성된 uuid가 서버로 post 요청을 보냈을 때 네트워크 오류가 난다면 다시 프론트엔드에서 다시 서버로 전송하려고 할때 똑같은 멱등성 키를 사용하여 같은 요청에 대해 처리를 하기 위해서 위와 같은 코드 구성을 해보았습니다!

꼼꼼하게 여러 케이스들을 고민해주셨군요!
저도 해당 경우에 익숙하지 않아 질문을 하나 드리려합니다

사용자의 요청을 받은 프론트엔드에서 생성된 uuid가 서버로 post 요청을 보냈을 때 네트워크 오류가 난다면 다시 프론트엔드에서 다시 서버로 전송하려고 할때 똑같은 멱등성 키를 사용하여 같은 요청에 대해 처리를 하기 위해서 위와 같은 코드 구성을 해보았습니다!

똑같은 멱등성 키를 사용하여 같은 요청에 대해 처리를 하는 부분은 어디에있나요?!


그리고 추가적으로 칠단계 테스트 코드가 터지는 것 같은데 확인부탁드려요!
스크린샷 2025-11-30 오후 3 54 23

Comment on lines 22 to 30
@Autowired
public ReservationService(ReservationRepository reservationRepository,
IdempotencyRepository idempotencyRepository
IdempotencyRepository idempotencyRepository,
TimeRepository timeRepository
) {
this.reservationRepository = reservationRepository;
this.idempotencyRepository = idempotencyRepository;
this.timeRepository = timeRepository;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생성자가 하나뿐일 때 는 @Autowired 를 생략할 수 있습니다!

또한 final필드만 생성자를 만들어주는 어노테이션이 있는데요 학습해보셔도 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RequiredArgsConstructor : final 필드만 생성자를 만들어주는 어노테이션

장점
1)의존하는 Repository가 늘어나거나 줄어들 때마다 생성자를 수정할 필요가 없습니다. 필드만 추가/삭제하면 끝!
2) final 키워드를 강제하므로, 객체가 생성된 후에 의존성이 변경되는 것을 막아줍니다.

적용 커밋 : db68a06

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

에러 코드를 enum으로 관리할 때의 장점은 어떤 것들이 있을까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

장점

  1. 비즈니스 로직과 HTTP 상태의 분리
    서비스 계층(Service)와 웹 계층(Controller, HTTP Status) 분리를 하게 되면 서비스는 예약이 중복되었단 사실만 enum에서 받아올 수 있고 GlobalExceptionHandler에서 400,409 인지 결정하기 때문에 유지보수 관점에서도 좋습니다!

  2. 유지보수의 편의성
    이전 구조에선 에러 메세지를 수정할 때 모든 파일들을 찾아가면 에러를 수정해야했는데 이젠 ErrorCode만 확인하면 되니 편합니다!

  3. 하드코딩 방지 및 타입 안정성
    ErrorCode.RESERVATOIN_DUPLICATED (오타 발생 시) -> 컴파일 에러가 발생하여 실행 전에 즉시 수정 가능합니다!

)
{
Map<String, String> error = new HashMap<>();
error.put("message", "이미 예약이 존재하는 시간은 삭제할 수 없습니다.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 ErrorCode enum 내부에서 관리하도록 할 수 있을 것 같은데 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum에서 관리하는 것이 훨씬 좋은 것 같습니다. 적용해보겠습니다!
적용 커밋 : ecfb3eb

Comment on lines 44 to 46
if (reservationRepository.existsByDateAndTime(request.date(), time.getId())) {
throw new IllegalArgumentException("이미 예약된 시간입니다!");
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 경우의 에러 메세지도 enum으로 관리할 수 있을 것 같아보여요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

적용해보겠습니다!!
적용 커밋 : ecfb3eb

public Reservation get(ReservationCreateRequest request) {
return reservationRepository.get(request.name(), request.date(), request.time());
if (count == 0) {
throw new IllegalArgumentException("삭제할 예약을 찾을 수 없습니다.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 ..!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 수정해 보겠습니다!
적용 커밋 : ecfb3eb

Comment on lines +64 to +69
public boolean exitsById(Long id)
{
String sql= "SELECT count(*) FROM time WHERE id = ?";
Integer count = jdbcTemplate.queryForObject(sql, Integer.class, id);
return count != null && count > 0;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 메서드는 어디에서 사용되나요??

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 원래 TimeServicedeleteTime에서 존재 여부 확인 후 삭제 하려고 만들어놓은 코드였는데 적용을 확인을 못하고 넘어 간 것 같습니다. 적용해보겠습니다!!
적용 커밋 : b5c11d7

@nonactress
Copy link
Author

nonactress commented Dec 1, 2025

똑같은 멱등성 키를 사용하여 같은 요청에 대해 처리를 하는 부분은 어디에있나요?!


멱등성 처리 로직은 ReservationService.javaaddReservation에서 관리하고 있습니다!

  1. 키 존재 여부 확인: idempotencyRepository.exists(key)로 중복 요청을 확인합니다.
  2. 요청 데이터 검증 : 키가 존재할 경우, 무조건 저장된 값을 반환하지 않고 validateAndGetExistingReservation 메서드를 통해 기존 예약 정보와 현재 요청 정보(이름, 날짜, 시간)가 일치하는지 matches메소드로 2차 검증을 수행합니다.
  3. 검증된 객체 반환 : 검증 후 기존 예약 객체를 반환하여, 동일한 키로 다른 데이터를 보내는 예외를 처리 했습니다.

그리고 추가적으로 칠단계 테스트 코드가 터지는 것 같은데 확인부탁드려요!


기존 ReservationService 로직이 Time를 ID로 조회하도록 변경되었으나, 테스트 코드는 여전히 단순 문자열("10:00")을 전송하고 있어 DTO 불일치 및 데이터 누락으로 인한 테스트 실패가 발생했습니다. 그래서 테스트에서도 먼저 시간을 등록해서 ID를 받아온 뒤에, 그 ID로 예약을 생성하도록 흐름을 맞췄습니다. 그리고 마지막에 데이터가 제대로 지워지는지 확인하는 DELETE 요청도 빠져있어서 추가해두었습니다!

적용 커밋 : 77083a4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants