Skip to content

Conversation

@chemistryx
Copy link

@chemistryx chemistryx commented Jan 1, 2026

안녕하세요! 백경환 리뷰어님! 이번 Spring Data JPA 미션 리뷰이로 참여하게된 하수한이라고 합니다! 이번 기회를 통해 처음 리뷰를 부탁드리는 것 같은데, 잘 부탁드리겠습니다!

이번 단계에서는 기존 인증 구현에 이어서 DAO 기반 로직을 순수 JPA로 마이그레이션하고, 추가로 예약 목록 조회 및 예약 대기 기능을 구현했습니다.

프로젝트 구조는 다음과 같습니다:

auth/
- AdminRoute - 관리자 endpoint를 구분하기 위한 어노테이션
- AdminRouteInterceptor - AdminRoute 어노테이션 구현
- JwtTokenProvider - JWT 관련 유틸리티 메소드
- LoginMember - 쿠키로부터 사용자 객체를 불러오는 커스텀 ArgumentResolver 어노테이션
- LoginMemberArgumentResolver - LoginMember 어노테이션 구현
config/
- WebConfig - WebMvcConfigurer 구현체
controller/
- MemberController - 사용자 관련 endpoint 정의
- ReservationController - 예약 관련 endpoint 정의
- ThemeController - 테마 관련 endpoint 정의
- TimeController - 시간 관련 endpoint 정의
- ViewController - thymeleaf 템플릿 관련 endpoint 정의
- WaitingController - 예약 대기 관련 endpoint 정의
repository/
- MemberRepository - 사용자 관련 DB 로직 정의
- ReservationRepository - 예약 관련 DB 로직 정의
- ThemeRepository - 테마 관련 DB 로직 정의
- TimeRepository - 시간 관련 DB 로직 정의
- WaitingRepository - 예약 대기 관련 DB 로직 정의
dto/
- ...(계층 간 데이터 전송에 필요한 객체들 정의)
exception/
- BadRequestException - 잘못된 요청에 대한 커스텀 예외
- GlobalExceptionHandler - 공통 예외 처리 로직
- UnauthorizedException - 인증되지 않은 사용자에 대한 커스텀 예외
model/
- ...(DB 테이블 스키마 정의)
service/
- AuthService - 사용자 인증 관련 로직 정의
- MemberService - 사용자 관련 로직 정의
- ReservationService - 예약 관련 로직 정의
- TimeService - 시간 관련 로직 정의
- WaitingService - 예약 대기 관련 로직 정의
RoomescapeApplication - Main entrypoint

긴 글 읽어주셔서 감사드리며, 코드 관련해서 궁금하신 점이나 다른 사항이 있으시다면 언제든지 편하게 말씀해주세요!

@chemistryx chemistryx marked this pull request as draft January 5, 2026 07:10
@chemistryx chemistryx marked this pull request as ready for review January 5, 2026 14:20
Copy link

@dooboocookie dooboocookie left a comment

Choose a reason for hiding this comment

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

안녕하세요, 수한.
리뷰어 루카입니다.🐟

리뷰로 만난게 처음이군요. 다른 미션의 PR을 읽어서인지 저는 나름 친밀감이 있네요. ☺️
파이팅해서 의미있는 리뷰 주고 받을 수 있으면 좋겠습니다.🔥

🎯 1차 리뷰 내용

결론부터 말씀드리면, 이번 리뷰는 별다른 코멘트 없이 깔끔하게 Request Chnage 드리겠습니다.

제가 앞서 수한이 리뷰요청을 해주신 디스코드 쓰레드에 다음과 같은 추가 요구사항을 드렸습니다.

  1. 이번 JPA 미션 범위에 대한 커밋으로만 PR을 남겨주세요. (지금 PR은 conflict도 발생하고 있네요)
  2. [NEXTSTEP JPA 학습테스트](https://edu.nextstep.camp/s/n5Ayu6QE/ls/DdIkrM09 ")를 진행해보지 않으셨다면 해보시길 바랍니다.

1번은 잘 해결하신 것 확인했습니다.
2번은 학습테스트를 잘 하셨을거라고 생각하겠습니다.

2번의 요구사항을 드린 이유는
Spring Data JPA를 활용한 Repository로 Entity의 영속화를 관리하도록 변경하길 바래서였습니다.

지금 Repository를 보면, Spring Data JPA 를 사용한 것으로 보이지는 않습니다.
제가 보기에는 Dao를 Repository라는 이름으로 변경한 내용만 읽히는데요.
미션 요구사항과 학습테스트를 통해서 미션에서 학습하고자 하는 목표를 명확히해주세요.
(혹시나 Dao라는 네이밍에서 Repository라는 네이밍으로 변경한 확실한 이유가 있거나, Spring Data를 사용하지 않은 확실한 이유가 있으면 말씀해주세요.)

이번 미션 제목이 JPA인만큼 미션에서 요구하는만큼은 적용해주세요.
그리고 본인만의 학습포인트를 잡아서 공유해주세요. (질문이나, 얻은 인사이트나, ...)

지금 PR내용으로는 제가 comment할 내용은 보이지 않습니다.

프로젝트 구조와 같은 내용은 PR내용으로 작성하지 않아도 제가 직접 코드를 보면 당연히 알게되는 내용이니, 리뷰에서 더 중점적으로 다뤄야할 맥락에 대해서 작성해주시면 좋겠어요.

🔥 Request Chnage

아무래도 이런 학습 방식이 익숙하지 않을 수 있지만, 리뷰를 통한 학습에서 제 생각보다 중요한 것은 수한이 직접 개발을하며 느낀 생각입니다.
꼭 고상한 질문이나 학습목표를 잡아야하는 것은 아닙니다.
예를들면 "이런 미션 왜하라고 하는지 모르겠어요." 이런 질문도 괜찮으니, 수한이 이 미션을 진행하며 드신 생각에 대해서 공유해주세요

@chemistryx
Copy link
Author

chemistryx commented Jan 6, 2026

@dooboocookie 루카님 안녕하세요! 코멘트 달아주신 내용에 대해서는 확인했습니다. 말씀해주신 두번째 요구사항에 관해서는 말씀드릴 내용이 있는데요, 제가 이 미션을 수행할 당시에는 기존 NEXTSTEP 내 존재하는 Spring Data JPA 미션을 2단계로 구분하여 1차(현재 PR)의 경우에는 순수 JPA만으로만 구현하도록 전달받았습니다.
따라서 현재 PR의 경우 Spring Data JPA를 사용하지 않고 순수 JPA만을 사용하여 구현한 것이 제가 의도한 바가 맞습니다..!

그리고 이번 미션의 경우 시간이 촉박하다 보니 단순 구현에만 집중한 부분이 없지않아 있었는데요, 코멘트에서 말씀해주신대로 이번 기회에 다시 제가 구현 당시에 떠올랐던 의문점이나 질문 사항들을 정리하여 다음과 같이 나타내보았습니다.

  1. 현재 예약 대기 기능의 경우 NEXTSTEP 내 힌트를 참고하여 별도의 엔티티(Waiting)를 통해 구현을 진행했습니다. 제가 생각해보았을 때 Reservation 모델의 내용(필드)이나 Waiting 모델의 내용이 유사하다고 생각되어 Reservation 모델 하나만을 가지고 예약 대기 기능을 구현해볼 수 있을 것 같다는 생각이 듭니다.(e.g., Reservation 내 status 필드 추가 -> ReservationStatus.CONFIRMED)
    이러한 방식으로 구현한다면 별도의 엔티티를 추가로 생성하지 않아도 되어 좀 더 단순한 구현이 가능할 것 같다는 생각이 드는데, 루카님께서는 이러한 방식에 대해서 어떻게 생각하시는지 궁금합니다.

  2. 현재 예약 중복 체크를 서비스 레이어에서 수행하고 있습니다. (WaitingService::create) 현재 구현의 경우 동시 요청이 들어오면 해당 검증 로직이 올바르게 작동하는지 여부를 확실하게 보장할 수 없다고 생각하는데, 이러한 케이스에 대한 테스트 및 검증은 어떠한 방식으로 보통 수행하게 되는지 궁금합니다!

관련해서 수정이나 요구 사항이 있으시다면 언제든지 말씀해주세요!

Copy link

@dooboocookie dooboocookie left a comment

Choose a reason for hiding this comment

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

안녕하세요, 수한.
제가 지난 코멘트 때 오해를 한 게 있었군요.
조금 더 잘 읽고, 잘 물어볼걸 그랬네요.

이미 2차 미션이 시작한 것으로 알고 있습니다.
여기서 추가적인 Request Change는 드리지 않고 몇가지 코멘트와 질문에 대한 답변으로 이번 리뷰는 마무리 해보도록 하겠습니다.


📬 질문에 대한 답변

  1. 현재 예약 대기 기능의 경우 NEXTSTEP 내 힌트를 참고하여 별도의 엔티티(Waiting)를 통해 구현을 진행했습니다. 제가 생각해보았을 때 Reservation 모델의 내용(필드)이나 Waiting 모델의 내용이 유사하다고 생각되어 Reservation 모델 하나만을 가지고 예약 대기 기능을 구현해볼 수 있을 것 같다는 생각이 듭니다.(e.g., Reservation 내 status 필드 추가 -> ReservationStatus.CONFIRMED) 이러한 방식으로 구현한다면 별도의 엔티티를 추가로 생성하지 않아도 되어 좀 더 단순한 구현이 가능할 것 같다는 생각이 드는데, 루카님께서는 이러한 방식에 대해서 어떻게 생각하시는지 궁금합니다.

상태만 다르게 한다는 것은 예약-예약대기를 같은 개념으로 본다는 것인데요.
만약 수한의 생각이 그렇다면 좋은 전략인 것 같습니다.

일단 질문주신 내용에 답변부터 드리자면, 저는 거의 높은 확률로 따로할 것 같아요.
이유는 '단순한' 구현을 위해서입니다. 테이블이 두개일 경우 복잡도나 관리포인트가 늘어나느것 아닐까? 라고 직관적으로 생각할 수 있습니다.
저는 선험적으로 파이브가이즈 햄버거 가게에 가서 매장 대기줄에 서있는 것과 매장에서 햄버거를 시키고 기다리고 있는 것이 매우 다르다는 것을 알고 있습니다.
Reservation과 Waiting 두 도메인이 확장됨에 따라 전혀 다른 속성들과 동작을 하게될 수 있습니다.
예를 들어 삭제를 한다고 했을 때 예약은 취소 위약금을 결제해야한다거나 말이죠. 이렇게 상태에 따라 다른 동작들을 해야될때는 같은 개념으로 뒀을 때 개발의 복잡도가 훨씬 올라갑니다. if문이나 switch문이 많이 생길 수 있겠죠.

반면, 모든 설계가 이렇게 현실세계와 밀접하게 연관이 있거나 명확한게 아닌데요. 그럴때는 저도 1개로 표현해보는 편이에요.
그래서 만약 상태에 따라 요구사항이 구체화되면 그때 그 복잡도를 나눠서 낮추는 방법을 선택할 것 같습니다.

  1. 현재 예약 중복 체크를 서비스 레이어에서 수행하고 있습니다. (WaitingService::create) 현재 구현의 경우 동시 요청이 들어오면 해당 검증 로직이 올바르게 작동하는지 여부를 확실하게 보장할 수 없다고 생각하는데, 이러한 케이스에 대한 테스트 및 검증은 어떠한 방식으로 보통 수행하게 되는지 궁금합니다!

백엔드 API를 개발하다 보면 이런 동시성 이슈에 대해서 고민할 경우가 많은데요.

동시성을 해결하는 방법은 정말 여러가지 방법이 있습니다.
왜냐하면
동시성 문제를 발생시키는 필드나 도메인의 성향이 많이 다르고,
얼마나 빈번하게 발생되는 문제이냐,
팀에서 얼마나 동시성을 핸들링하는 것이 중요한 문제냐
같은 이유들이 많이 다르기 때문입니다.

그래서 이런 문제를 바라볼 때는 수한도 '내가 어떤 동시성 이슈를 고민하고 있지?' 를 조금 더 구체화하시면 좋은 고민이 될것입니다.
동시에 waiting을 create하는 여러 요청이 들어왔을 때, 수한이 걱정하는 내용은 무엇인가요?

  • 동일한 회원이 waiting을 두개 등록해버릴 가능성?
  • 먼저 접근한 회원보다 나중에 접근한 회원이 빠른 순번을 받게될 가능성?
  • reservation이 없는 줄 알고, waiting을 생성했는데, reservation이 취소될 가능성?
  • ...
    같은 문제들이 있을 수 있겠죠.

어떤 내용이냐에 따라 아예 핸들링하는 방법이 달라질 수 있습니다.
동일한 회원이 waiting을 2개 등록하는 경우는
DB 제약조건을 추가해서 애초에 문제가 발생되지 않게할 수 있겠죠.

질문에서 남겨주신 '검증하고 싶은 올바른 동작'이 무엇인지는 모르겠으나, 그게 무엇이냐에 따라 조금씩 달라질 수 있겠습니다.
보통은 실제로 동시에 서비스 로직을 여러번 호출할 수 있도록(비동기적으로) 테스트짜보기도 하는것 같아요.
(반면, 여러 요청이 한번에 날라올때 정도의 엄청 넓은 범위의 테스트를 코드로 짜놔야하는 경우는 좀 드문것 같기도하네요.)


💪 마치며

제가 문해력 이슈로 코멘트 주고받는 과정을 너무 지연시켜버렸네요.
(다행히도) 이번 미션은 크게 주고받을만한 내용이라기 보다는, JPA에 대한 연습이 위주인 미션이라 이만 approve 하겠습니다.
2차 미션도 화이팅하시고 드린 내용에 대해서는 2차 때 한번 고민해보시고 적용부탁드릴게요.
질문있으시면 언제든지 연락주세요.

Comment on lines +25 to +31
@ManyToOne(fetch = FetchType.EAGER)
@JoinColumn(name = "time_id", nullable = false)
private Time time;

@ManyToOne(fetch = FetchType.EAGER)
@JoinColumn(name = "theme_id", nullable = false)
private Theme theme;

Choose a reason for hiding this comment

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

👀 Comment

이렇게 ManyToOne 연관관계 매핑에 패치타입을 즉시로딩으로 명시한 이유가 있으실까요??
일단 fetch type의 옵션은 LAZY, EAGER 이렇게 두가지가 있는데요.(ManyToOne은 기본값이 아마 EAGER일거입니다.)
특별히 EAGER로 두었다는 것은 LAZY가 아니였으면 했다는 의도가 보이는 것 같은데 그 이유를 말씀해주세요!

Copy link
Author

Choose a reason for hiding this comment

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

말씀하신대로 초기 구현 당시에는 Waiting 엔티티 내 Time, Theme 엔티티가 존재하고 있어 이를 항상 함께 사용하기 위해 EAGER로 설정했습니다. 다만 현재 코드와 쿼리들을 다시 살펴보니 실제로는 해당 연관 엔티티들의 식별자 위주로만 사용되고 있어 엔티티 자체를 즉시 로딩할 필요는 크지 않다고 판단했습니다.

따라서 이 부분의 경우 LAZY로 변경하는 것이 더 타당하기에 수정 진행하도록 하겠습니다!

Comment on lines +12 to +13
@Entity
public class Waiting {

Choose a reason for hiding this comment

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

🔥 Request Change

이제 테이블 정의는 Entity에서 준 설정을 통해서 진행되는데요.

waiting 테이블이 가지고 있어야하는 제약조건이 어떤 것이 있을까요?
(어쩌면 수한이 주신 질문에 약간의 힌트가 될 수도 있겠네요.)

Copy link
Author

Choose a reason for hiding this comment

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

Waiting 테이블의 경우 현재 애플리케이션 레이어에서 중복 대기를 차단하고 있는데, 해당 레벨에서의 검증만으로는 동시성 문제를 완전히 막을 수 없다고 생각됩니다, 따라서 DB 레벨에서 UNIQUE 제약 조건을 추가하여 데이터 무결성을 보장하는 것이 타당하다고 생각합니다!

Comment on lines +62 to +79
public List<WaitingWithRank> findWaitingsWithRankByMemberId(Long memberId) {
String jpql =
"SELECT new roomescape.dto.WaitingWithRank(" +
" w, " +
" (SELECT COUNT(w2) + 1 " +
" FROM Waiting w2 " +
" WHERE w2.theme = w.theme " +
" AND w2.date = w.date " +
" AND w2.time = w.time " +
" AND w2.id < w.id)" +
") " +
"FROM Waiting w " +
"WHERE w.member.id = :memberId";
TypedQuery<WaitingWithRank> query = entityManager.createQuery(jpql, WaitingWithRank.class);
query.setParameter("memberId", memberId);

return query.getResultList();
}

Choose a reason for hiding this comment

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

👀 Comment

제가 이해한게 맞다면 이 레파지토리 조회 메서드는,
순위를 아이디 순서에 의해서 결정하는 로직으로 보여집니다.

2가지 고민해볼 필요가 있을 것 같아요.

  1. 순서에 대한 유의미한 값을 DB에 저장할 필요는 없을지?
  2. 이 과정이 서비스에서 진행되어야할지? 레파지토리에서 진행되어야할지?

Copy link
Author

Choose a reason for hiding this comment

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

남겨주신 코멘트에 대해서 고민해본 결과, 1번의 경우 현재 ID 값은 단순 식별자임에도 불구하고 순서를 계산하는 데 사용되고 있습니다.
다만 현 단계에서 대기 순번은 일종의 PoC로써 사용자가 자신의 대기 순번을 대략적으로 인지하기 위한 참고용 정보라고 판단하였습니다.
물론 향후 대기 순번이 중요한 케이스로 확장될 경우에는 순서를 명시적으로 관리하는 컬럼 도입이나 동시성 제어와 같은 다른 방식으로의 개선이 필요하다고 생각합니다!

2번과 관련해서는, 현재 계산 방식 자체가 DB 레벨에서 가장 효율적으로 처리할 수 있는 연산이기에 서비스 레이어보다는 레포지토리 조회 로직에서 함께 처리하는 것이 더 적절하다고 생각합니다!

@boorownie boorownie merged commit 589609a into next-step:chemistryx Jan 8, 2026
@chemistryx
Copy link
Author

@dooboocookie 빠른 리뷰 감사합니다!
1번 질문에 관해서는 저는 오히려 테이블을 추가로 만들게되면 관리포인트가 늘어나는 것이 아닌가 라는 생각을 가지고 있었는데, 말씀해주신 것처럼 각 도메인(예약, 예약 대기)이 확장됨에 따라 서로 다른 규칙과 행위를 갖게 될 가능성이 높고, 이러한 경우 하나의 엔티티로 유지하는 것이 오히려 복잡도를 키울 수 있다는 관점을 새롭게 이해하게 되었습니다.

2번 동시성 관련 질문의 경우에도 막연히 동시에 요청이 들어오면 문제가 될 수 있지 않을까? 라는 생각만 가지고 있었는데, 어떤 동작으로부터 동시성 문제가 발생할 수 있는지 보다 구체적으로 판단하는 행위가 우선시되어야한다는 좋은 인사이트를 얻을 수 있었습니다!

해당 코멘트 이외에도 관련해서 달아주신 리뷰에 대해서 답변을 달아보았는데, Request Change와 같은 부분들의 경우 다음 미션(2차)에 이어서 반영하도록 하겠습니다! 감사합니다 ㅎㅎ 🙇 🙇

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.

3 participants