피드백 - 정현우 튜터

[Nuung]

안녕하세요 다빈님!! 매번 새로운 도전과제가 생겨서 쉽지않음에도 잘 해주시고 계셔서 좋습니다!! 🔥🔥🔥

전체 과제 체크 사항

• API 명세서 없어요!! ERD 없어요!! 안돼요!! 다빈님 개인과제가,, 완성도가 점점 높아져야 하는데,, 아쉽습니다,, 정말 🥹🥹🥹
• 왜 배포를 안하셨어요!!! 고로,, 코드 기반 리뷰를 해봅시다!!
• 이번에 CI/CD 도 학습하셨으니, 이를 바탕으로 배포까지 쭉 해봅시다!!
• 더욱이 swagger 까지 붙이셔서, 배포한 swagger + API 명세라면 다빈님의 프로젝트가 더 완벽해지지 않을까요?!
• 보너스 기능에 집중하시느라 필수 기능에서 빠진 부분이 있는 것 같아요!! 하나씩 다시 체크해보자구요!

필수 기능 구현 리스트

• [x] 로그인 / 회원가입
• [x] 프로필 보기
• [x] 새 공연 등록
• [x] 공연 목록 보기
• [x] 공연 검색하기
• [x] 공연 상세보기
• [ ] 좌석을 지정하지 않고 공연 예매하기
• [x] 예매 확인하기

보너스 기능 구현 리스트

• [x] 공연의 좌석 예매 정보 확인하기
• [x] 좌석을 지정하여 예매하기
• [x] 동시성 처리하기
• [x] 예매 취소하기
• [ ] 테스트 코드 내용 채우기

---

세부 피드백

• 우선 전체적으로 서비스 계층에 DTO 를 풀어서 전달하고 계신데, DTO 는 "Data Transfer Object" 약자로, 주로 "계층간 데이터 공유" 에 활용됩니다.
• 지금 다빈님처럼 DTO 를 풀어버리면, DTO 가 바뀌면 controller 의 service 호출 부분, service 에서 파라미터 수정, service 내부 로직 수정까지 3단계가 수정이 필요합니다. 하지만 DTO 자체를 넘긴다면, 서비스 계층의 내부 서비스로직만 바뀌면 되는거죠.
• 즉, 서비스 계층에 바로 풀어 넘겨버리면 다빈님의 추후 유지보수에 볼륨이 커집니다! 이 부분 다시 고려해보시는게 좋을 것 같아요

user

• 사용자와 Role, Auth (jwt) 기본 강의 형태가 동일해서 세부 피드백은 넘어가겠습니다.
• 그리고 제가 예전 부터 피드백 드린 부분이 있는데, admin 으로 가입하는 API 은 기본 사용자와 아예 전혀 다른 보안 로직을 태우거나, 아예 없어야 합니다.
• 태초의 admin 이 딱 하나 수동으로 만들어지고, 이 admin 이 다른 user 를 admin ROLE 가질 수 있게 update 하는 방향이 훨씬 올바른 방향입니다.
• profile 에서 aggregation function 을 위한 createQueryBuilder 좋았습니다! 이런 aggregation 에 익숙해지실 필요가 있어요!

show

https://github.com/ratempty/online-reservation/blob/2d005f5e4caf65412628f21dcb6b3855868f74ed/src/shows/shows.controller.ts#L33-L39

• @Query('title') title: string 로 url query string 받는 것 좋았습니다. 그에 따라 다른 서비스 로직 호출하는 것도 좋았구요
• 해당 서비스로직에 다빈님은 한 단계 더 나아가 "페이지네이션" 을 만들어 보시는게 좋을 것 같아요. 또한 title 뿐 아니라 다양한 값을 추가해보시는 것도 좋으며 이에따라 https://tkssharma.com/nestjs-playing-with-query-param-dto/ 와 같이 query string 에 대한 DTO & validation pipe 를 만들어 보는 것은 어떨까요

https://github.com/ratempty/online-reservation/blob/2d005f5e4caf65412628f21dcb6b3855868f74ed/src/shows/shows.service.ts#L25-L47

• 이는 하나의 transaction 으로 묶이는게 좋습니다. 지금 설계한 모델 상으로는 seat 없는 show 는 아무 의미 없으며 (예약 불가능) show 없는 seat도 아무 의미가 없습니다. 그리고 이 둘은 CASCADE 제약조건 이 맞구요!
• 그리고 단일 save 대신 bulk create (bulk insert) 를 할 수 있습니다. 이는 지금과 같은 단일 save 보다 고효율이며 더 고차원적인 데이터 정합성을 보장할 수 도 있습니다.
• save 에 배열 자체를 던지는 방법과 queryBuilder 를 사용하는 방법이 있습니다!
• https://orkhan.gitbook.io/typeorm/docs/repository-api 에서 save 인자에 array가 들어가는 점
• https://orkhan.gitbook.io/typeorm/docs/insert-query-builder 에서 query-builder 사용하는 것 두 개 모두 확인해 보셔요!

reservations

• DTO 왜 없나요!!!!
• 좌석 지정 없이 예약할 수 있는 기능이 없어요! 지금 로직에서 바로 가능한 좌석을 찾아서 자동으로 할당하는 로직 충분히 가능할 것 같아요!
• create method에서 "실패하는 경우를 가장 위에서 먼저 하시는 것" 을 추천해요!
• 물론 지금도 방향은 좋았지만, 코드가 리펙토링 된다면, 가령 아래와 같이요 (전후 생략된 부분 고려)

    // 좌석 조회와 예약 가능 여부 확인
    const seats = await this.seatRepository.findBy({
      showId: showId,
      seatNum: seatNum.seats,
    });

    if (seats.length !== seatNum.seats.length || seats.some(seat => !seat.canBook)) {
      throw new Error('일부 좌석이 예약 불가능합니다.');
    }

    const show = await this.showRepository.findOneBy({ id: showId });
    if (!show) {
      throw new Error('공연을 찾을 수 없습니다.');
    }

    const currentPoint = await this.pointRepository
      .createQueryBuilder('point')
      .select('SUM(point.point)', 'sum')
      .where('point.userId = :userId', { userId })
      .getRawOne();

    if (currentPoint.sum < show.price * seats.length) {
      throw new Error('보유 포인트 부족!');
    }

• transaction 으로 묶어주시는 것도 좋았지만, 저와 얘기할 때 처럼 왜 저게 "동시성을 막아주는 가" 에 대한 고려를 해보시는게 아주 중요합니다.
• 그 이유도 README 에 업데이트 해봅시다!

---

마무리 첨언

• 다빈님이 아무래도 어려운 기능에 집중하시다가 시간이 좀 부족했던 것 같은데, 이럴때 일 수록 천천히, 차근이, 첫 단추부터 출발해 봅시다.
• 문서화는 프로젝트 완성도에 지대한 영향을 미치며 이는 다빈님이 차후 사용하실 포폴에도 아주 중요한 부분 중 하나가 됩니다!
• 하나씩 다시 쌓아 올려 보자구요!
• 그리고 다빈님은 transaction 말고 DBMS의 "격리 수준와 Lock" 애 대해 더 학습해 보시고, 이를 활용해 동시성을 처리하는 방법과
• 그리고 REDIS 와 같은 3rd party 를 활용한 동시성 제어도 경험해보시는게 좋아요! 🥹🥹🥹 할 수 있습니다, 다빈님 충분히!!

[Nuung]

[ 동시성 처리하기 ]

(1) 가장 쉽게 처리하는 방법은 query builder 를 호출하셔서 DBMS 에서 트랜잭션을 하는 것

(2) DBMS 의 Lock 을 활용하는 것

• https://orkhan.gitbook.io/typeorm/docs/select-query-builder#set-locking
• 이는 구현 난이도보다 이해의 난이도가 좀 더 있을 겁니다.

  (3) 3rd party (REDIS 등)를 활용해 외부의존성으로 동시성 해결하는 것