Skip to content

Conversation

@snowykte0426
Copy link
Member

@snowykte0426 snowykte0426 commented Mar 8, 2025

💡 PR 요약

  • 예약 가능한 장소를 조회하는 API를 구현하였습니다(GET /booking/available)
  • 예약 정보를 조회하는 API를 구현하였습니다(GET /booking/information)
  • 예약을 추가,수정,삭제하는 API 3종을 구현하였습니다
    Resolves: [Feat] Booking 관련 기능 구현 #10

📋 작업 내용

먼저 구현된 인증/인가 로직을 이용하여 기능/API 명세서의 요구사항을 만족하는 API 5종을 구현 및 테스트하였습니다.이번 작업으로 애플리케이션의 핵심 기능을 구현하였습니다

🤝 리뷰 시 참고사항

몇몇 클래스(특히 CreateBookingUseCase,UpdateBookingUseCase)의 내부 로직이 너무 길어진 느낌이 있어 이를 클래스 내부에서 private 메서드로 분할하고 public 메서드에서 순서대로 호출하여 구현하는 방식으로 바꿔야 하나 고민했었는데 어떻게 해야할 지 정확히 모르겠습니다


8기 친구들과 꼼꼼히 프로젝트 구조를 정립하고 꾸준히 서로의 진행상황을 공유여야 했었는데 그러지 못하여서 추후 프로젝트 전반적인 리팩터링을 해야할 것 같습니다

✅ 체크리스트

  • 이 작업으로 인해 변경이 필요한 문서를 작성 또는 수정했나요? (e.g. README, .env.example)
  • 작업한 코드가 정상적으로 동작하는지 확인했나요?
  • 작업한 코드에 대한 테스트 코드를 작성하거나 수정했나요?
  • Merge 대상 브랜치를 올바르게 설정했나요?
  • 해당 PR과 관련 없는 작업이 포함되지는 않았나요?
  • PR의 올바른 라벨과 리뷰어를 설정했나요?

SQL 로그를 가독성 좋게 포맷팅 해주는 옵션을 설정하였습니다
Booking 관련 Application 계층의 출입구과 될 인터페이스와 Booking의 보조격인 Place 테이블의 Persistence 계층 인터페이스를 선언하였습니다
일부 메서드를 구현하고 미구현이더라도 필요한 메서드들을 오버라이드 했습니다
프로젝트 컨벤션에 맞게 Custom Repository 클래스,인터페이스 명에 Jpa라는 단어를 추가하였습니다
장소 관련 정보를 반환하는 API의 응답 형식을 강제하는 DTO를 정의하였습니다
예약 관련 정보를 쿼리하는 Repository를 선언하였습니다
장소 관련 정보를 요구에 맞게 쿼리하는 Repository를 구현하였습니다
해당하는 시간대에 예약이 없는 장소들을 반환하여 예약이 가능한 장소의 리스트를 알려주는 Use Case를 구현하였습니다
Place의 영속 계층에서 사용될 Adapter를 선언하였습니다
Entity와 변환 및 값 저장을 고려하여 추가적인 Record를 정의하고 이를 기존 필드에 사용하도록 하였습니다
ERD에 따라 충실히 구현하지 못했던 Entity 클래스들을 수정하여 실제 RDB와 일치하게 동작하도록 재구성하였습니다
ERD에 따라 충실히 구현하지 못했던 Entity 클래스들을 수정하여 실제 RDB와 일치하게 동작하도록 재구성하는 작업이 이뤄짐에 따라 Mapper 클래스 역시 수정하였고 Response DTO로 변환이 추가적으로 요구되어 메서드를 추가하였습니다
선언만 이뤄지고 실제 구현은 되지 않았었던 엔드포인트를 실제로 구현하였습니다
API 명세서와 다르게 설계되었던 엔드포인트를 문서에 맞도록 수정하였습니다
쿼리 파라미터를 정의하고 알맞은 메서드를 연결하였습니다
예약 관련 정보를 담는 객체 간의 변환을 수행하는 클래스에 도메인 객체를 응답 DTO로 변환하는 메서드를 추가하였습니다
개발과정에 사용되었던 로깅 어노테이션을 제거하였습니다
대표자 정보를 포함하는 필드를 요구 사항의 변화에 따라 추가하였습니다
날짜,시간대,장소에 따라 예약 정보를 반환하는 역할의 클래스를 선언하였지만 실제 구현은 하지 않았습니다
BookingJpaRepositoryCustomImpl에 날짜,시간,장소종류에 따라 예약 데이터들을 반환하는 메서드를 구현하였습니다
예약 정보의 영속 계층 진입점을 제공하는 Port와 Adapter를 구현하였습니다
실제로 미구현 되어있던 클래스를 구현하였습니다
요구사항의 변화에 따라 특정 파라미터가 필수가 아니도록 설정하였습니다
실제 테이블과 Entity의 칼럼명이 일치 하지 않던 문제를 수정하였습니다
예약을 생성하는 클래스를 구현하였습니다
올바르지 않은 시간대,장소로 예약이 요청되었거나 중복된 예약이 생성요청 되었을 경우 등 다양한 예외 상황을 정의하였습니다
예약 생성 API 실행 시 발행될 예외 클래스 3종을 정의하였습니다
JPA 기본 메서드인 save를 제공하도록 Port와 Adapter를 확장하였습니다
``POST /api/v1/booking``을 처리하는 메서드의 세부 내용을 구현하였습니다
@snowykte0426 snowykte0426 requested a review from se0hui March 8, 2025 14:09
Comment on lines 16 to 18
private final BookingPersistenceAdapter bookingPersistenceAdapter;
private final BookingMapper bookingMapper;

Copy link

Choose a reason for hiding this comment

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

port가 아닌 adapter를 받고있음.

Copy link
Member Author

Choose a reason for hiding this comment

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

521775c

문제 수정하였습니다

Comment on lines 33 to 36
@Override
public Boolean ExistsBookingByDateAndTimeAndPlace(LocalDate date, String time, String place) {
return bookingJpaRepository.ExistsBookingByDateAndTimeAndPlace(date, time, place);
}
Copy link

Choose a reason for hiding this comment

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

camel case로

Copy link
Member Author

Choose a reason for hiding this comment

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

4ec7711

카멜케이스에 알맞게 수정하였습니다
리뷰 감사합니다

Comment on lines 15 to 20
@Repository
@RequiredArgsConstructor
public class BookingJpaRepositoryCustomImpl implements BookingJpaRepositoryCustom {

private final JPAQueryFactory queryFactory;

Copy link

Choose a reason for hiding this comment

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

persistence adapter가 존재한다면 repository custom을 만들고 그 내부에서 query dsl 쿼리를 짜도 괜찮겠지만 adapter에서 query factory를 di받고 짜도 괜찮은 것이 아닌지?

db layer의 컴포넌트를 정의한 outbound port가 존재하니 구현부는 adapter 내부에만 있는것이 적절해보임

Copy link
Member Author

Choose a reason for hiding this comment

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

1164ef3
db80837

리팩터링하여 반영하였습니다
중요한 코드리뷰 감사합니다

Comment on lines 43 to 52
public Boolean ExistsBookingByDateAndTimeAndPlace(LocalDate date, String time, String place) {
Integer result = queryFactory
.selectOne()
.from(bookingJpaEntity)
.where(bookingJpaEntity.bookingDate.eq(date)
.and(bookingJpaEntity.timeSlot.id.timeLabel.eq(time))
.and(bookingJpaEntity.timeSlot.place.placeName.eq(place))
)
.fetchFirst();
return result != null;
Copy link

Choose a reason for hiding this comment

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

method name must be camel

Copy link
Member Author

Choose a reason for hiding this comment

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

4ec7711

카멜케이스로 변경하였습니다,이런 실수 없도록 주의하겠습니다
리뷰 감사합니다

Comment on lines 52 to 58
@Override
public MemberJpaEntity findMemberByEmail(String email) {
return queryFactory
.selectFrom(memberJpaEntity)
.where(memberJpaEntity.email.eq(email))
.fetchOne();
}
Copy link

Choose a reason for hiding this comment

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

Optional로 npe 방지하기

Copy link
Member Author

Choose a reason for hiding this comment

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

d5dcdd8

Optional 적용하여 orElseThrow로 예외까지 설정하였습니다

코드리뷰 감사합니다

@Value("${spring.jwt.token.refresh-expiration}")
private long refreshTokenExpiration;

public JwtToken signIn(String email, String password) {
Copy link

Choose a reason for hiding this comment

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

로그인이라는 메서드 내에서 로직이 너무 많은 책임을 가지고있음. 비밀번호, 유저 접근 권한, 토큰 생성등 다양한 기능들은 코드만 보고는 읽기가 어려워보임. private으로 몇군데 분리를 시켜놓고 도메인 행위가 잘 나타났으면 좋겠음

Copy link
Member Author

Choose a reason for hiding this comment

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

65383e2

@se0hui서희와 논의한 후 리팩터링 하였습니다
좋은 리뷰 감사합니다!

runtimeOnly("com.h2database:h2:2.3.232")

/** Cache */
implementation("com.github.ben-manes.caffeine:caffeine:3.2.0")
Copy link

Choose a reason for hiding this comment

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

캐시 버저닝에 관한 고민을 해보고 어떻게 버저닝을 할건지에 대한 고민을 나중에라도 해보면 좋겠음.

Copy link
Member Author

Choose a reason for hiding this comment

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

현재는 단순히 TTL로 2시간만 설정되어 있는데
더 공부해보고 고민해보도록 하겠습니다
감사합니다❤️

Copy link

Choose a reason for hiding this comment

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

  1. 캐싱한 데이터가 두시간이 채 지나기 전에 수정이 된다면 어떻게 처리할 것인지.
  2. 서버를 여러대 띄우고 로드밸런싱하고있는데 서로다른 인스턴스들의 로컬캐시 버전이 일치하지 않을 수 있는 경우도 생각해보기
  3. 단순 디비 수정이 일어났을때 로컬캐시 동기화를 어떻게 해줄 것인지
  4. remote 캐시를 함께 사용하면서 캐시 저장소를 복제해서 여러대 두고있는데 master, slave 간의 버전 관리는 어떻게 해볼 수 있는지.
  5. master가 다운되어서 slave가 failvoer되었는데 slave의 최신 버전과 일치하지 않음. 그냥 둘건지?

이런거 생각해보세요

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 조언 감사합니다

private final BookingPersistencePort bookingPersistencePort;
private final TimeSlotPersistencePort timeSlotPersistencePort;

public void execute(String time, String place, List<Long> participants) {
Copy link

Choose a reason for hiding this comment

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

time과 place에 대한 index를 생성해 타게하면 성능 향상

Copy link
Member Author

Choose a reason for hiding this comment

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

f5be9c2

DB에서 인덱스 생성하고 Entity에도 적용하였습니다

코드 리뷰 감사합니다!!

Comment on lines 38 to 45
bookingPersistencePort.findBookingByDateAndTimeAndPlace(LocalDate.now(), selectedTimeSlot.getTimeSlotId().timeLabel(), place)
.stream()
.findAny()
.ifPresent(booking -> {
throw new DuplicateBookingException();
});
List<Member> participantEntities = memberPersistencePort.findMembersByIds(participants);
String email = SecurityContextHolder.getContext().getAuthentication().getPrincipal().toString();
Copy link

Choose a reason for hiding this comment

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

booking에 대한 xlock으로 (for update) 락을 잡아 동시성 문제를 피할 필요가 있어보임

Copy link
Member Author

@snowykte0426 snowykte0426 Mar 9, 2025

Choose a reason for hiding this comment

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

514d708
aa7dbff

CreateBookingUseCase에도 락 적용하였습니다
리뷰 감사합니다

동시성 문제를 인지하고 이를 방지하기 위하여 비관적 락을 적용하였습니다
비관적 락이 적용됨에 따라 발생한 메서드 변경을 테스트 코드에서도 반영하였습니다
NPE 방지를 위하여 Optional을 이용하여 Null 가능성을 검증하였습니다
카멜 케이스에 맞게 메서드 명을 수정하였습니다
…tenceAdapter``를 ``BookingPersistencePort``로 변경

Port가 아니라 Adapter를 상속 받던 것을 수정하였습니다
DB 계층이 이원화 되는 현상를 수정하고 불필요한 인터페이스 추가를 막기위하여 관련 로직을 Adapter로 이전하였습니다
DB 계층이 이원화 되는 현상를 수정하고 불필요한 인터페이스 추가를 막기위하여 관련 로직을 Adapter로 이전하였습니다
timeLabel과 place에 대한 복합 인덱스를 적용하여 성능 향상을 꾀하였습니다
예약 정보를 반환할 때 비밀번호가 노출이 되어 클라이언트로 반환되던 치명적인 결함을 수정하였습니다
쿼리문을 수정하여 예약이 가능한 장소 목록에 예약이 처음부터 불가능 하여 해당 시간대에 예약이 존재하지 않는 장소를 반환하던 치명적인 결함을 수정하였습니다
예외 클래스를 담고 있던 패키지명이 잘못되어 있던 것을 수정하였습니다
과도하게 하나의 메서드 내부에서 로직이 모여있던 것을 분리하여 그 기능과 흐름을 명확하게 재설계하였습니다
반환 DTO에서 id를 추가로 반환하도록 하였습니다
2일 전보다 오래된 Booking 데이터를 삭제하는 스케쥴러와 쿼리 메서드를 구현하였습니다
2가지 시나리오 상황에 대한 간단한 테스트 코드를 추가하였습니다
형식에 맞지 않는 클래스명의 내용을 변경하였습니다
@snowykte0426 snowykte0426 merged commit 8cfe6d2 into develop Mar 23, 2025
2 checks passed
@snowykte0426 snowykte0426 deleted the feature/booking-api branch March 23, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ Type: Feature 기능 추가

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] Booking 관련 기능 구현

3 participants