[FEAT] api-user BFF 전환 #75
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 106 files out of 217 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/gemini review |
Summary of ChangesHello @unifolio0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
api-user 및 api-owner 모듈을 BFF 아키텍처로 성공적으로 전환한 훌륭한 PR입니다. 도메인 모듈을 독립적인 서비스로 분리하고, Feign 클라이언트를 도입하여 통신하며, 테스트 코드를 Mock 기반으로 리팩토링하는 등 마이크로서비스 전환의 핵심적인 부분들이 매우 잘 구현되었습니다. 특히 common-client 모듈을 통해 DTO를 공유하고, Flyway로 각 서비스의 DB 스키마를 관리하는 방식은 모범적입니다. 다만, BFF 서비스의 분산 트랜잭션 처리와 성능(N+1 문제) 관련하여 몇 가지 중요한 개선점이 필요해 보입니다. 자세한 내용은 아래 리뷰 코멘트를 참고해 주세요.
| public CreateReservationResponse reserve(String memberId, CreateReservationRequest request) { | ||
| // 1. Redis 분산 락 획득 | ||
| reservationRedisService.isReserving(memberId, request.getRestaurantId(), request.getAvailableDateId()); | ||
|
|
||
| // 2. 중복 예약 체크 (BFF에서 직접 처리) | ||
| List<ReservationDTO> memberReservations = reservationClient.getReservationsByMember(memberId); | ||
| boolean alreadyReserved = memberReservations.stream() | ||
| .anyMatch(r -> r.restaurantId().equals(request.getRestaurantId()) | ||
| && r.availableDateId().equals(request.getAvailableDateId()) | ||
| && r.status().equals(ReservationStatus.CONFIRMED)); | ||
| if (alreadyReserved) { | ||
| throw new IllegalStateException("이미 예약된 날짜입니다."); | ||
| } | ||
|
|
||
| // 3. Member, Restaurant, AvailableDate 조회 | ||
| MemberDTO member = memberClient.getMember(memberId); | ||
| RestaurantDTO restaurant = restaurantClient.getRestaurant(request.getRestaurantId()); | ||
| AvailableDateDTO availableDate = restaurantClient.getAvailableDate( | ||
| request.getRestaurantId(), request.getAvailableDateId() | ||
| ); | ||
|
|
||
| // 4. Capacity 감소 | ||
| availableDateClient.decreaseCapacity(new DecreaseCapacityRequest( | ||
| request.getAvailableDateId(), request.getPartySize())); | ||
|
|
||
| // 5. Reservation 생성 | ||
| CreateReservationDTO createRequest = new CreateReservationDTO( | ||
| request.getRestaurantId(), | ||
| request.getAvailableDateId(), | ||
| memberId, | ||
| request.getPartySize(), | ||
| request.getSpecialRequest() | ||
| ); | ||
| ReservationDTO savedReservation = reservationClient.createReservation(createRequest); | ||
|
|
||
| // 6. 이벤트 발행 | ||
| LocalDateTime dateTime = LocalDateTime.of(availableDate.date(), availableDate.time()); | ||
| ReservationCreatedEvent event = new ReservationCreatedEvent( | ||
| savedReservation, member.name(), restaurant.name(), dateTime); | ||
| eventPublishService.publishReservationCreatedEvent(event); | ||
|
|
||
| return new CreateReservationResponse(savedReservation, restaurant.name(), availableDate); | ||
| } |
There was a problem hiding this comment.
이 reserve 메소드는 분산 트랜잭션 처리에 심각한 문제가 있습니다. availableDateClient.decreaseCapacity 호출 후 reservationClient.createReservation 호출이 실패할 경우, decreaseCapacity에 대한 보상(compensation) 로직이 없어 데이터 불일치가 발생합니다. 재고는 줄어들었지만 예약은 생성되지 않은 상태가 됩니다. updateReservation, cancel 메소드도 비슷한 문제를 가지고 있습니다.
Saga 패턴(Orchestration 또는 Choreography)을 적용하거나, 최소한 try-catch 블록을 사용하여 실패 시 보상 트랜잭션을 명시적으로 호출해야 합니다.
예시:
try {
availableDateClient.decreaseCapacity(...);
reservationClient.createReservation(...);
} catch (Exception e) {
// 보상 트랜잭션 호출
availableDateClient.increaseCapacity(...);
throw e; // 예외 다시 던지기
}이 문제는 예약 시스템의 핵심 기능에 영향을 미치므로 반드시 수정이 필요합니다.
| public List<NearbyRestaurantResponse> findWithNearbyRestaurant(double latitude, double longitude) { | ||
| return restaurantClient.getAllRestaurants() | ||
| .stream() | ||
| .filter(restaurant -> { | ||
| double distance = DistanceCalculator.calculateDistance( | ||
| latitude, longitude, | ||
| restaurant.latitude(), restaurant.longitude() | ||
| ); | ||
| return distance <= SEARCH_RADIUS_M; | ||
| }) | ||
| .map(restaurant -> getNearbyRestaurantResponse(restaurant, latitude, longitude)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
restaurantClient.getAllRestaurants()를 호출하여 모든 레스토랑 정보를 가져온 후 메모리에서 거리를 계산하여 필터링하는 방식은 매우 비효율적입니다. 레스토랑 수가 많아지면 BFF에 심각한 성능 부하와 메모리 문제를 일으킬 수 있습니다.
domain-restaurant 서비스에 위도, 경도, 반경을 파라미터로 받아 데이터베이스 수준에서 필터링하여 결과를 반환하는 새로운 API 엔드포인트를 추가하는 것을 강력히 권장합니다.
예: GET /api/restaurants/nearby?latitude={lat}&longitude={lon}&radius={radius}
| public List<FavoriteRestaurantResponse> getFavoriteRestaurants(String memberId) { | ||
| List<FavoriteRestaurantDTO> favoriteRestaurants = favoriteRestaurantClient.getFavoritesByMemberId(memberId); | ||
| if (favoriteRestaurants.isEmpty()) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| List<String> restaurantIds = favoriteRestaurants.stream() | ||
| .map(FavoriteRestaurantDTO::restaurantId) | ||
| .toList(); | ||
|
|
||
| Map<String, RestaurantDTO> restaurantsById = restaurantClient | ||
| .getRestaurantsByIds(new RestaurantIdsRequest(restaurantIds)) | ||
| .stream() | ||
| .collect(Collectors.toMap(RestaurantDTO::id, Function.identity())); | ||
|
|
||
| return favoriteRestaurants.stream() | ||
| .map(favoriteRestaurant -> { | ||
| RestaurantDTO restaurant = restaurantsById.get(favoriteRestaurant.restaurantId()); | ||
| return getFavoriteRestaurantResponse(restaurant); | ||
| }) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
즐겨찾기 목록을 조회한 후, stream().map() 내부에서 각 레스토랑의 평균 평점을 restaurantClient.getAverageRating()으로 개별 호출하고 있습니다. 이는 N+1 문제를 유발하여 즐겨찾기 개수만큼 네트워크 호출이 발생합니다.
성능 개선을 위해 domain-restaurant 서비스에 여러 레스토랑 ID를 받아 평균 평점 맵을 반환하는 배치 API를 추가하는 것을 고려해 보세요.
예: POST /api/reviews/average-ratings/batch with List<String> restaurantIds in the body.
| public List<SummaryReservationResponse> getReservations(String memberId) { | ||
| List<ReservationDTO> reservations = reservationClient.getReservationsByMember(memberId); | ||
|
|
||
| if (reservations.isEmpty()) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| List<String> restaurantIds = reservations.stream() | ||
| .map(ReservationDTO::restaurantId) | ||
| .distinct() | ||
| .toList(); | ||
|
|
||
| // Restaurant 배치 조회 | ||
| Map<String, RestaurantDTO> restaurantsById = restaurantClient | ||
| .getRestaurantsByIds(new RestaurantIdsRequest(restaurantIds)) | ||
| .stream() | ||
| .collect(Collectors.toMap(RestaurantDTO::id, Function.identity())); | ||
|
|
||
| return reservations.stream() | ||
| .map(reservation -> { | ||
| RestaurantDTO restaurant = restaurantsById.get(reservation.restaurantId()); | ||
| // AvailableDate는 각 Restaurant에서 개별 조회 | ||
| AvailableDateDTO availableDate = restaurantClient.getAvailableDate( | ||
| reservation.restaurantId(), | ||
| reservation.availableDateId() | ||
| ); | ||
| return new SummaryReservationResponse( | ||
| reservation, | ||
| restaurant.name(), | ||
| availableDate | ||
| ); | ||
| }) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
getFavoriteRestaurants와 유사하게, stream().map() 내부에서 각 예약에 대한 AvailableDate 정보를 restaurantClient.getAvailableDate()로 개별 호출하고 있어 N+1 문제가 발생합니다.
domain-restaurant 서비스에 여러 availableDateId를 받아 AvailableDateDTO 맵을 반환하는 배치 API를 추가하여 성능을 개선할 수 있습니다.
예: POST /api/available-dates/batch with List<Long> availableDateIds in the body.
| List<ReservationDTO> memberReservations = reservationClient.getReservationsByMember(memberId); | ||
| boolean alreadyReserved = memberReservations.stream() | ||
| .anyMatch(r -> r.restaurantId().equals(request.getRestaurantId()) | ||
| && r.availableDateId().equals(request.getAvailableDateId()) | ||
| && r.status().equals(ReservationStatus.CONFIRMED)); | ||
| if (alreadyReserved) { | ||
| throw new IllegalStateException("이미 예약된 날짜입니다."); | ||
| } |
There was a problem hiding this comment.
중복 예약을 확인하기 위해 회원의 모든 예약을 가져와서 메모리에서 필터링하는 방식은 회원의 예약 내역이 많아질 경우 비효율적일 수 있습니다.
domain-reservation 서비스에 memberId, restaurantId, availableDateId를 파라미터로 받아 중복 여부를 확인하는 API 엔드포인트를 추가하는 것을 고려해 보세요. 이렇게 하면 데이터베이스에서 직접 조회하여 더 효율적입니다.
예: GET /api/reservation/exists?memberId=...&restaurantId=...&availableDateId=...
Summary of ChangesHello @unifolio0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 WellMeet-Backend 프로젝트의 마이크로서비스 아키텍처 전환 로드맵의 핵심 단계인 Phase 5, 즉 BFF 전환을 완료합니다. api-user 및 api-owner 모듈이 Feign Client를 통해 도메인 서비스와 통신하도록 재구성되었으며, 이를 통해 모듈 간의 결합도를 낮추고 각 서비스의 독립성을 강화했습니다. 또한, 테스트 전략을 Mock 기반으로 전환하여 개발 효율성을 높이고, 문서 구조를 개편하여 프로젝트 이해도를 증진시켰습니다. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Summary of ChangesHello @unifolio0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 마이크로서비스 전환의 중요한 이정표인 Phase 5를 완료하며, api-user 및 api-owner 모듈을 Backend For Frontend (BFF) 아키텍처로 완전히 전환했습니다. 이는 도메인 모듈에 대한 직접적인 의존성을 Feign Client를 통한 HTTP 통신으로 대체하고, DTO를 새로운 common-client 모듈로 중앙화하는 과정을 포함합니다. 또한, 테스트 전략을 Mock 기반으로 전환하여 테스트 실행 속도를 크게 개선했으며, 핵심 도메인 서비스들이 독립적인 애플리케이션으로 실행될 수 있도록 활성화하고 Flyway를 통합했습니다. 이로써 프로젝트의 모듈화와 확장성이 한층 강화되었습니다. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
api-user와 api-owner 모듈을 BFF 패턴으로 전환하는 대규모 리팩토링이 진행되었습니다. 도메인 모듈과의 직접 의존성을 제거하고 Feign 클라이언트를 통한 통신으로 변경한 점, 공유 DTO를 위한 common-client 모듈을 도입한 점, 테스트 코드를 Mock 기반으로 전환한 점 등 마이크로서비스 아키텍처로의 전환을 위한 올바른 방향의 변경들입니다. 문서 구조 또한 claudedocs로 분리하여 가독성과 유지보수성을 크게 향상시켰습니다.
다만, 몇 가지 성능 문제(N+1 쿼리, 전체 데이터 조회 후 필터링)와 중복된 Enum 사용으로 인한 잠재적 런타임 오류가 발견되었습니다. 특히 BFF 서비스 로직에서의 성능 최적화와 모듈 간 데이터 객체의 일관성 확보에 대한 검토가 필요합니다. 자세한 내용은 각 파일에 남긴 리뷰 코멘트를 참고해 주세요.
| package com.wellmeet.reservation.dto; | ||
|
|
||
| public enum ReservationStatus { | ||
|
|
||
| PENDING, | ||
| CONFIRMED, | ||
| CANCELED | ||
| } |
There was a problem hiding this comment.
중복되고 일관되지 않은 ReservationStatus enum이 정의되어 있습니다. common-client 모듈의 com.wellmeet.common.dto.ReservationStatus와 내용이 다릅니다. 특히 common-client에서는 CANCELLED로, 여기서는 CANCELED로 정의되어 있어 ReservationStatus.valueOf() 호출 시 IllegalArgumentException이 발생할 수 있습니다.
BFF 계층에서는 DTO를 공유하는 common-client 모듈의 enum을 직접 사용해야 합니다. 이 파일은 삭제하고, 관련 코드에서 com.wellmeet.common.dto.ReservationStatus를 import하여 사용하도록 수정해야 합니다. 이 문제는 api-owner 모듈에 있는 중복 enum에도 동일하게 적용됩니다.
| public List<NearbyRestaurantResponse> findWithNearbyRestaurant(double latitude, double longitude) { | ||
| return restaurantClient.getAllRestaurants() | ||
| .stream() | ||
| .filter(restaurant -> { | ||
| double distance = DistanceCalculator.calculateDistance( | ||
| latitude, longitude, | ||
| restaurant.latitude(), restaurant.longitude() | ||
| ); | ||
| return distance <= SEARCH_RADIUS_M; | ||
| }) | ||
| .map(restaurant -> getNearbyRestaurantResponse(restaurant, latitude, longitude)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
심각한 성능 저하가 우려됩니다. restaurantClient.getAllRestaurants()를 호출하여 모든 레스토랑 정보를 가져온 후 메모리에서 거리를 계산하고 필터링하고 있습니다. 레스토랑 수가 증가하면 BFF의 메모리 사용량이 급증하고 네트워크 트래픽이 과도하게 발생하여 시스템 전체에 심각한 부하를 줄 수 있습니다.
domain-restaurant 서비스에 위도, 경도, 반경을 파라미터로 받아 위치 기반으로 필터링된 레스토랑 목록을 반환하는 API를 추가하고, 이를 Feign 클라이언트를 통해 호출하도록 즉시 수정해야 합니다. 데이터베이스 수준에서 공간 쿼리(Spatial Query)를 활용하여 이 문제를 해결하는 것이 가장 효율적입니다.
| .map(reservation -> { | ||
| MemberDTO member = membersById.get(reservation.getMemberId()); | ||
| MemberDTO member = membersById.get(reservation.memberId()); | ||
| AvailableDateDTO availableDate = restaurantClient.getAvailableDate( | ||
| reservation.getRestaurantId(), reservation.getAvailableDateId()); | ||
| reservation.restaurantId(), reservation.availableDateId()); | ||
| return new ReservationResponse( | ||
| reservation, | ||
| availableDate, | ||
| member.getName(), | ||
| member.getPhone(), | ||
| member.getEmail(), | ||
| member.name(), | ||
| member.phone(), | ||
| member.email(), | ||
| member.isVip() | ||
| ); | ||
| }) |
There was a problem hiding this comment.
N+1 API 호출 문제가 발생할 수 있습니다. reservations 리스트를 순회하면서 각 예약마다 restaurantClient.getAvailableDate()를 호출하고 있습니다. 이로 인해 예약 수만큼 Feign 호출이 발생하여 성능 저하를 유발할 수 있습니다.
개선을 위해 레스토랑의 모든 예약 가능 날짜 정보를 한 번에 가져와 Map에 저장한 후 사용하는 것을 권장합니다. RestaurantFeignClient에 getAvailableDatesByRestaurant(String restaurantId)와 같은 배치 조회 메소드를 추가하고 활용하는 것을 고려해 보세요.
| this.open = dto.getOpen(); | ||
| this.close = dto.getClose(); | ||
| this.operating = dto.isOperating(); | ||
| this.dayOfWeek = DayOfWeek.valueOf(dto.dayOfWeek().name()); |
There was a problem hiding this comment.
중복된 DayOfWeek enum을 사용하고 있습니다. com.wellmeet.reservation.dto.DayOfWeek enum 대신 java.time.DayOfWeek을 직접 사용하는 것이 좋습니다. BusinessHourDTO의 dayOfWeek()는 이미 java.time.DayOfWeek를 반환하므로, 불필요한 변환 과정(valueOf 및 name())을 제거할 수 있습니다.
이 클래스 및 관련 DTO에서 com.wellmeet.reservation.dto.DayOfWeek에 대한 의존성을 제거하고 java.time.DayOfWeek를 사용하도록 리팩토링하는 것을 권장합니다.
| private FavoriteRestaurantResponse getFavoriteRestaurantResponse(RestaurantDTO restaurant) { | ||
| Double rating = restaurantClient.getAverageRating(restaurant.id()); | ||
| double ratingValue = (rating != null) ? rating : 0.0; | ||
| return new FavoriteRestaurantResponse(restaurant, ratingValue); | ||
| } |
There was a problem hiding this comment.
N+1 API 호출 문제가 발생할 수 있습니다. getFavoriteRestaurants 메소드 내에서 즐겨찾기 목록을 순회하며 각 레스토랑의 평균 평점을 개별적으로 조회(restaurantClient.getAverageRating)하고 있습니다. 즐겨찾기 한 레스토랑이 N개일 경우, N번의 Feign 호출이 발생하여 성능 저하를 유발할 수 있습니다.
개선을 위해 여러 레스토랑 ID를 받아 각 레스토랑의 평균 평점을 Map 형태로 반환하는 배치(batch) API를 domain-restaurant 서비스에 추가하고, RestaurantFeignClient를 통해 호출하는 것을 권장합니다.
| return reservations.stream() | ||
| .map(reservation -> { | ||
| RestaurantDTO restaurant = restaurantsById.get(reservation.restaurantId()); | ||
| // AvailableDate는 각 Restaurant에서 개별 조회 | ||
| AvailableDateDTO availableDate = restaurantClient.getAvailableDate( | ||
| reservation.restaurantId(), | ||
| reservation.availableDateId() | ||
| ); | ||
| return new SummaryReservationResponse( | ||
| reservation, | ||
| restaurant.name(), | ||
| availableDate | ||
| ); | ||
| }) |
There was a problem hiding this comment.
N+1 API 호출 문제가 발생할 수 있습니다. getReservations 메소드 내에서 예약 목록을 순회하며 각 예약마다 restaurantClient.getAvailableDate()를 호출하고 있습니다. 이로 인해 예약 수만큼 Feign 호출이 발생하여 성능 저하를 유발할 수 있습니다.
개선을 위해 다음과 같은 로직을 고려해볼 수 있습니다:
- 모든 예약에 대해
restaurantId와availableDateId를 수집합니다. restaurantId를 기준으로 그룹화합니다.- 각
restaurantId별로availableDateClient.getAvailableDatesByRestaurant(restaurantId)를 호출하여 해당 레스토랑의 모든 예약 가능 날짜를 한 번에 가져옵니다. - 가져온 정보를 Map에 저장하여 사용합니다.
| 3. [모듈별 테스트 전략](#모듈별-테스트-전략) | ||
| 4. [테스트 작성 규칙](#테스트-작성-규칙) | ||
| 5. [테스트 인프라](#테스트-인프라) | ||
| 2. [아키텍처 마이그레이션 로드맵](#-아키텍처-마이그레이션-로드맵) |
There was a problem hiding this comment.
문서의 목차 링크가 깨져있습니다. '아키텍처 마이그레이션 로드맵' 섹션으로 이동하는 링크가 작동하지 않습니다. 헤더의 이모지(🏗️) 때문에 앵커 생성이 예상과 다를 수 있습니다. 링크가 올바르게 동작하도록 앵커를 수정하는 것을 권장합니다.
| 2. [아키텍처 마이그레이션 로드맵](#-아키텍처-마이그레이션-로드맵) | |
| 2. [아키텍처 마이그레이션 로드맵](#️-아키텍처-마이그레이션-로드맵) |
There was a problem hiding this comment.
Code Review
api-user와 api-owner 모듈을 BFF 패턴으로 전환하는 대규모 리팩토링이네요. 도메인 모듈과의 직접 의존성을 제거하고 Feign Client를 도입하여 마이크로서비스 아키텍처로의 중요한 첫 걸음을 내딛으셨습니다. DTO를 common-client 모듈로 분리하고, 테스트 전략을 Mock 기반으로 전환하는 등 전체적인 구조 개선이 인상적입니다. 코드 변경 사항들은 대체로 훌륭하며, 몇 가지 성능 및 일관성 측면에서 개선할 수 있는 부분을 제안드립니다. 멋진 작업입니다!
| public List<NearbyRestaurantResponse> findWithNearbyRestaurant(double latitude, double longitude) { | ||
| return restaurantClient.getAllRestaurants() | ||
| .stream() | ||
| .filter(restaurant -> { | ||
| double distance = DistanceCalculator.calculateDistance( | ||
| latitude, longitude, | ||
| restaurant.latitude(), restaurant.longitude() | ||
| ); | ||
| return distance <= SEARCH_RADIUS_M; | ||
| }) | ||
| .map(restaurant -> getNearbyRestaurantResponse(restaurant, latitude, longitude)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
findWithNearbyRestaurant 메서드는 모든 레스토랑을 조회한 후 메모리에서 필터링하고 있습니다. 레스토랑 수가 많아지면 심각한 성능 저하를 유발할 수 있습니다. 데이터베이스 수준에서 위치 기반으로 필터링하는 것이 효율적입니다. domain-restaurant 서비스에 경도/위도 및 반경을 파라미터로 받아 필터링된 결과를 반환하는 API를 추가하고, BFF에서는 이 API를 호출하는 것을 권장합니다. 예를 들어, GET /api/restaurants/nearby?latitude={lat}&longitude={lon}&radius={radius}와 같은 엔드포인트를 domain-restaurant에 구현할 수 있습니다.
| package com.wellmeet.reservation.dto; | ||
|
|
||
| public enum ReservationStatus { | ||
|
|
||
| PENDING, | ||
| CONFIRMED, | ||
| CANCELED | ||
| } |
There was a problem hiding this comment.
common-client 모듈에 이미 com.wellmeet.common.dto.ReservationStatus enum이 정의되어 있습니다. 중복된 enum을 정의하면 혼란을 야기하고 유지보수를 어렵게 만듭니다. 이 enum을 삭제하고 common-client의 ReservationStatus를 직접 사용하는 것이 좋습니다. 이렇게 하면 ReservationResponse 및 CreateReservationResponse에서 ReservationStatus.valueOf(reservation.status().name())과 같은 변환 코드도 제거할 수 있습니다.
| @Getter | ||
| @Builder | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class CreateReservationDTO { | ||
| public class UpdateReservationDTO { | ||
|
|
||
| private String restaurantId; | ||
| private Long availableDateId; | ||
| private String memberId; | ||
| private int partySize; | ||
| private String specialRequest; | ||
| } |
There was a problem hiding this comment.
이 프로젝트의 다른 새로운 DTO들은 대부분 Java Record로 구현되어 있습니다. 일관성을 위해 UpdateReservationDTO도 record로 변환하는 것을 고려해 보세요. Record를 사용하면 보일러플레이트 코드를 줄이고 불변성을 보장할 수 있으며, 불필요한 Lombok 어노테이션과 import도 제거할 수 있습니다.
public record UpdateReservationDTO(
String restaurantId,
Long availableDateId,
int partySize,
String specialRequest
) {
}
There was a problem hiding this comment.
Code Review
이 Pull Request는 api-user와 api-owner 모듈을 BFF 패턴으로 전환하는 중요한 리팩토링을 수행합니다. Feign 클라이언트와 common-client 모듈을 도입하여 도메인 모듈과의 의존성을 성공적으로 분리하였고, 테스트 전략을 Mock 기반으로 개선한 점이 훌륭합니다. 하지만 분산 트랜잭션 처리에서 데이터 불일치를 유발할 수 있는 심각한 결함이 발견되었습니다. 또한, N+1 문제와 비효율적인 데이터 조회 등 성능 저하를 일으킬 수 있는 부분들이 있어 개선이 필요합니다.
| public CreateReservationResponse reserve(String memberId, CreateReservationRequest request) { | ||
| // 1. Redis 분산 락 획득 | ||
| reservationRedisService.isReserving(memberId, request.getRestaurantId(), request.getAvailableDateId()); | ||
|
|
||
| // 2. 중복 예약 체크 (BFF에서 직접 처리) | ||
| List<ReservationDTO> memberReservations = reservationClient.getReservationsByMember(memberId); | ||
| boolean alreadyReserved = memberReservations.stream() | ||
| .anyMatch(r -> r.restaurantId().equals(request.getRestaurantId()) | ||
| && r.availableDateId().equals(request.getAvailableDateId()) | ||
| && r.status().equals(ReservationStatus.CONFIRMED)); | ||
| if (alreadyReserved) { | ||
| throw new IllegalStateException("이미 예약된 날짜입니다."); | ||
| } | ||
|
|
||
| // 3. Member, Restaurant, AvailableDate 조회 | ||
| MemberDTO member = memberClient.getMember(memberId); | ||
| RestaurantDTO restaurant = restaurantClient.getRestaurant(request.getRestaurantId()); | ||
| AvailableDateDTO availableDate = restaurantClient.getAvailableDate( | ||
| request.getRestaurantId(), request.getAvailableDateId() | ||
| ); | ||
|
|
||
| // 4. Capacity 감소 | ||
| availableDateClient.decreaseCapacity(new DecreaseCapacityRequest( | ||
| request.getAvailableDateId(), request.getPartySize())); | ||
|
|
||
| // 5. Reservation 생성 | ||
| CreateReservationDTO createRequest = new CreateReservationDTO( | ||
| request.getRestaurantId(), | ||
| request.getAvailableDateId(), | ||
| memberId, | ||
| request.getPartySize(), | ||
| request.getSpecialRequest() | ||
| ); | ||
| ReservationDTO savedReservation = reservationClient.createReservation(createRequest); | ||
|
|
||
| // 6. 이벤트 발행 | ||
| LocalDateTime dateTime = LocalDateTime.of(availableDate.date(), availableDate.time()); | ||
| ReservationCreatedEvent event = new ReservationCreatedEvent( | ||
| savedReservation, member.name(), restaurant.name(), dateTime); | ||
| eventPublishService.publishReservationCreatedEvent(event); | ||
|
|
||
| return new CreateReservationResponse(savedReservation, restaurant.name(), availableDate); | ||
| } |
There was a problem hiding this comment.
reserve 메소드의 분산 트랜잭션 처리에 결함이 있어 데이터 불일치를 유발할 수 있습니다. availableDateClient.decreaseCapacity 호출이 성공한 후 reservationClient.createReservation 호출이 실패할 경우, 감소된 capacity가 복구되지 않아 재고가 맞지 않게 됩니다.
일련의 원격 호출 과정을 try-catch 블록으로 감싸고, 예외 발생 시 increaseCapacity를 호출하는 보상 트랜잭션을 실행하여 데이터 정합성을 보장해야 합니다. updateReservation 메소드에도 유사한 문제가 있으므로 동일한 패턴을 적용해야 합니다.
public CreateReservationResponse reserve(String memberId, CreateReservationRequest request) {
// 1. Redis 분산 락 획득
reservationRedisService.isReserving(memberId, request.getRestaurantId(), request.getAvailableDateId());
// 2. 중복 예약 체크 (BFF에서 직접 처리)
List<ReservationDTO> memberReservations = reservationClient.getReservationsByMember(memberId);
boolean alreadyReserved = memberReservations.stream()
.anyMatch(r -> r.restaurantId().equals(request.getRestaurantId())
&& r.availableDateId().equals(request.getAvailableDateId())
&& r.status().equals(ReservationStatus.CONFIRMED));
if (alreadyReserved) {
throw new IllegalStateException("이미 예약된 날짜입니다.");
}
// 3. Member, Restaurant, AvailableDate 조회
MemberDTO member = memberClient.getMember(memberId);
RestaurantDTO restaurant = restaurantClient.getRestaurant(request.getRestaurantId());
AvailableDateDTO availableDate = restaurantClient.getAvailableDate(
request.getRestaurantId(), request.getAvailableDateId()
);
// 4. Capacity 감소
availableDateClient.decreaseCapacity(new DecreaseCapacityRequest(
request.getAvailableDateId(), request.getPartySize()));
ReservationDTO savedReservation;
try {
// 5. Reservation 생성
CreateReservationDTO createRequest = new CreateReservationDTO(
request.getRestaurantId(),
request.getAvailableDateId(),
memberId,
request.getPartySize(),
request.getSpecialRequest()
);
savedReservation = reservationClient.createReservation(createRequest);
} catch (Exception e) {
// 보상 트랜잭션: 감소시킨 Capacity 복구
availableDateClient.increaseCapacity(new IncreaseCapacityRequest(
request.getAvailableDateId(), request.getPartySize()));
throw e; // 예외 다시 던지기
}
// 6. 이벤트 발행
LocalDateTime dateTime = LocalDateTime.of(availableDate.date(), availableDate.time());
ReservationCreatedEvent event = new ReservationCreatedEvent(
savedReservation, member.name(), restaurant.name(), dateTime);
eventPublishService.publishReservationCreatedEvent(event);
return new CreateReservationResponse(savedReservation, restaurant.name(), availableDate);
}| public List<NearbyRestaurantResponse> findWithNearbyRestaurant(double latitude, double longitude) { | ||
| return restaurantClient.getAllRestaurants() | ||
| .stream() | ||
| .filter(restaurant -> { | ||
| double distance = DistanceCalculator.calculateDistance( | ||
| latitude, longitude, | ||
| restaurant.latitude(), restaurant.longitude() | ||
| ); | ||
| return distance <= SEARCH_RADIUS_M; | ||
| }) | ||
| .map(restaurant -> getNearbyRestaurantResponse(restaurant, latitude, longitude)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
findWithNearbyRestaurant 메소드는 restaurantClient.getAllRestaurants()를 통해 모든 식당 정보를 가져온 후 BFF 서비스에서 거리를 계산하여 필터링하고 있습니다. 식당 데이터가 많아질 경우, 이 방식은 심각한 성능 저하와 높은 메모리 사용량을 유발할 수 있습니다.
domain-restaurant 서비스에 위도, 경도, 검색 반경을 파라미터로 받아 데이터베이스 수준에서 효율적으로 필터링(예: 공간 인덱스 활용)하는 API를 추가하고, BFF에서는 이 API를 호출하도록 수정하는 것을 강력히 권장합니다.
| return reservations.stream() | ||
| .map(reservation -> { | ||
| RestaurantDTO restaurant = restaurantsById.get(reservation.restaurantId()); | ||
| // AvailableDate는 각 Restaurant에서 개별 조회 | ||
| AvailableDateDTO availableDate = restaurantClient.getAvailableDate( | ||
| reservation.restaurantId(), | ||
| reservation.availableDateId() | ||
| ); | ||
| return new SummaryReservationResponse( | ||
| reservation, | ||
| restaurant.name(), | ||
| availableDate | ||
| ); | ||
| }) | ||
| .toList(); |
There was a problem hiding this comment.
getReservations 메소드에서 N+1 문제가 발생합니다. 예약 목록을 조회한 후, 각 예약을 순회하며 restaurantClient.getAvailableDate를 개별적으로 호출하고 있습니다. 예약 건수가 많아지면 네트워크 요청이 급증하여 성능이 저하될 것입니다.
이 문제를 해결하기 위해, domain-restaurant 서비스에 여러 availableDateId를 리스트로 받아 한 번에 조회하는 배치(batch) API를 추가하고, BFF에서는 이 API를 호출하여 필요한 모든 AvailableDate 정보를 한 번의 요청으로 가져오도록 수정해야 합니다.
| @Getter | ||
| @Builder | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class CreateReservationDTO { | ||
| public class UpdateReservationDTO { | ||
|
|
||
| private String restaurantId; | ||
| private Long availableDateId; | ||
| private String memberId; | ||
| private int partySize; | ||
| private String specialRequest; | ||
| } |
There was a problem hiding this comment.
대부분의 신규 DTO가 Java record로 구현된 점은 매우 좋습니다. 일관성을 위해 UpdateReservationDTO 또한 record로 전환하는 것을 권장합니다. 코드가 더 간결해지고 불변성이 보장되는 이점이 있습니다.
public record UpdateReservationDTO(
String restaurantId,
Long availableDateId,
int partySize,
String specialRequest
) {
}| this.open = dto.getOpen(); | ||
| this.close = dto.getClose(); | ||
| this.operating = dto.isOperating(); | ||
| this.dayOfWeek = DayOfWeek.valueOf(dto.dayOfWeek().name()); |
There was a problem hiding this comment.
DayHours 생성자에서 DayOfWeek.valueOf(dto.dayOfWeek().name())을 사용하여 enum을 변환하고 있습니다. 이 방식은 com.wellmeet.common.dto.DayOfWeek와 com.wellmeet.reservation.dto.DayOfWeek의 enum 멤버 이름이 항상 동일해야 한다는 강한 결합을 만듭니다. 만약 어느 한쪽의 enum이 변경되면 런타임 에러가 발생할 수 있습니다.
이러한 암묵적인 결합 대신, 두 enum 간의 변환을 명시적으로 처리하는 별도의 매핑 함수를 만들어 사용하는 것이 더 안전하고 유지보수하기 좋은 방법입니다.
1 participant
🚩 Issue
closed #71
🗣️ 리뷰 요구사항 (선택)