Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthrough사용자가 소유한 매장 목록을 반환하는 새 API가 추가되었습니다. 매장별 활성 예약 수를 일괄 집계하고, 개점 여부와 메인 이미지 URL을 판단해 새로운 DTO로 응답합니다. 관련 리포지토리 쿼리, 서비스 및 컨트롤러 엔드포인트, DTO/변환기와 상태 코드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as "StoreController\n(rgba(52,152,219,0.5))"
participant Service as "StoreQueryService\n(rgba(46,204,113,0.5))"
participant UserRepo as "UserRepository\n(rgba(155,89,182,0.5))"
participant StoreRepo as "StoreRepository\n(rgba(241,196,15,0.5))"
participant BookingRepo as "BookingRepository\n(rgba(231,76,60,0.5))"
participant S3 as "S3Service\n(rgba(52,73,94,0.5))"
participant Converter as "StoreConverter\n(rgba(127,140,141,0.5))"
User->>Controller: GET /api/v1/stores/my
Controller->>Service: getMyStores(username)
Service->>UserRepo: findByEmail(username)
UserRepo-->>Service: User
Service->>StoreRepo: findAllByOwner(user)
StoreRepo-->>Service: List<Store>
Service->>BookingRepo: countActiveBookingsByStores(stores)
BookingRepo-->>Service: List(storeId, count)
Service->>S3: getMainImageUrl(store) (loop per store)
S3-->>Service: url
Service->>Converter: toMyStoreDto(...) (loop per store)
Converter-->>Service: MyStoreDto
Service->>Converter: toMyStoreListDto(dtos)
Converter-->>Service: MyStoreListDto
Service-->>Controller: MyStoreListDto
Controller-->>User: ApiResponse<MyStoreListDto>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`:
- Around line 83-88: Rename the method countActiveBookings in BookingRepository
to a name matching intent (e.g., countValidBookings or countTotalBookings) and
update any call sites; adjust the JPQL to reflect the intended semantics—either
remove COMPLETED from the IN list if completed bookings should not be counted,
or switch to a NOT IN pattern excluding only CANCELLED (and any other
non-counted statuses) using BookingStatus enum constants—to avoid future enum
additions breaking this logic; ensure the repository method signature and any
usages (countActiveBookings -> new name) are updated accordingly.
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/controller/StoreController.java`:
- Around line 111-121: The response status used in getMyStores currently
references StoreSuccessStatus._STORE_FOUND which is semantically for single-item
fetches; add a new enum constant (e.g., _MY_STORE_LIST_FOUND or
_STORE_LIST_FOUND) to StoreSuccessStatus and update StoreController.getMyStores
to return ApiResponse.of(StoreSuccessStatus._MY_STORE_LIST_FOUND,
storeQueryService.getMyStores(user.getUsername())); also update any related
documentation/messages for the new enum constant so the list-response message is
clear.
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/repository/StoreRepository.java`:
- Around line 24-25: The findAllByOwner method in StoreRepository can cause an
N+1 when StoreQueryServiceImpl.getMyStores calls Store.isOpenNow() which
accesses store.findBusinessHoursByDay() (the businessHours collection); update
StoreRepository.findAllByOwner to eagerly load businessHours (either add an
`@EntityGraph`(attributePaths = "businessHours") on findAllByOwner or replace it
with a custom JPQL/Spring Data query that uses a fetch join for businessHours)
so that businessHours are fetched in the same query and avoids per-Store
additional selects.
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java`:
- Around line 120-122: The parameter name username in
StoreQueryService.getMyStores and its implementation
StoreQueryServiceImpl.getMyStores is misleading because the code calls
userRepository.findByEmail(username); rename the parameter to email in both the
interface and implementation (and update all callers) so the signature reads
getMyStores(String email), or if renaming is risky add a clarifying comment
above StoreQueryServiceImpl.getMyStores stating that the passed-in username is
an email used with userRepository.findByEmail; make sure to update any
references to the method and keep userRepository.findByEmail(email) and the
existing exception handling unchanged.
- Around line 119-137: getMyStores performs N+1 DB calls because it calls
bookingRepository.countActiveBookings(store) inside the stream for each Store
returned by storeRepository.findAllByOwner; replace per-store counting with a
single bulk query method (e.g., add
BookingRepository.countActiveBookingsByStores(List<Store> stores) that returns
storeId→count) and in getMyStores call that once, convert the result to a
Map<Long,Long>, then use that map when building each StoreConverter.toMyStoreDto
(retain s3Service.toUrl and isOpenNow usage) so booking counts are fetched in
one query instead of N calls.
src/main/java/com/eatsfine/eatsfine/domain/store/controller/StoreController.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/store/repository/StoreRepository.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`:
- Around line 83-96: Both methods countActiveBookings and
countActiveBookingsByStores duplicate the same status filter and use a different
enum reference style than other queries; change both queries to accept a single
parameter (e.g., :statuses) of type
List<com.eatsfine.eatsfine.domain.booking.enums.BookingStatus> and use "b.status
in :statuses" to centralize the filter, and update other queries in the file to
use the fully-qualified BookingStatus enum
(com.eatsfine.eatsfine.domain.booking.enums.BookingStatus) for consistency so
adding a new status only requires updating the call site that builds the
statuses list rather than modifying multiple repository queries.
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java`:
- Around line 129-134: Replace the raw List<Object[]> projection from
bookingRepository.countActiveBookingsByStores with a type-safe DTO projection
(e.g., a record or interface like StoreBookingCount) returned by the repository
method, update the repository method signature (countActiveBookingsByStores) to
return List<StoreBookingCount> and construct the projection in the JPQL/Query,
then update the service mapping that builds bookingCountMap to collect from
StoreBookingCount using its accessors (StoreBookingCount::storeId,
StoreBookingCount::count) instead of casting Object[] elements.
- Around line 126-134: When myStores (from storeRepository.findAllByOwner(user))
is empty, avoid calling bookingRepository.countActiveBookingsByStores(myStores);
add a guard: if myStores.isEmpty(), set bookingCounts to an empty list (or skip
the DB call) and initialize bookingCountMap as an empty map, otherwise call
countActiveBookingsByStores(myStores) and build the map as existing code does;
this change should be applied around the myStores/bookingCounts logic in
StoreQueryServiceImpl (referencing myStores,
bookingRepository.countActiveBookingsByStores, and bookingCountMap).
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/status/StoreSuccessStatus.java`:
- Around line 23-25: The two enum constants _STORE_MAIN_IMAGE_UPLOAD_SUCCESS and
_STORE_MAIN_IMAGE_GET_SUCCESS in StoreSuccessStatus both use the duplicate code
"STORE2005"; update one to a distinct code (e.g., change
_STORE_MAIN_IMAGE_GET_SUCCESS to "STORE2007" or another unused code) so each
response is unique, ensuring you modify the code string passed to the enum
constructor for _STORE_MAIN_IMAGE_GET_SUCCESS and run any tests or usages that
reference that code to verify no other collisions occur.
| @Query("select count(b) from Booking b " + | ||
| "where b.store = :store " + | ||
| "and b.status in (com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.PENDING, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.COMPLETED)") | ||
| Long countActiveBookings(@Param("store") Store store); | ||
|
|
||
| @Query("select b.store.id, count(b) from Booking b " + | ||
| "where b.store in :stores " + | ||
| "and b.status in (com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.PENDING, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.COMPLETED) " + | ||
| "group by b.store.id") | ||
| List<Object[]> countActiveBookingsByStores(@Param("stores") List<Store> stores); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
상태 필터 조건의 중복 및 쿼리 스타일 불일치
두 가지 개선 포인트가 있습니다:
-
상태 필터 중복:
CONFIRMED, PENDING, COMPLETED조건이 두 메서드에서 동일하게 반복됩니다. 새 상태 추가 시 양쪽 모두 수정해야 하므로 유지보수 위험이 있습니다. -
쿼리 스타일 불일치: 이 파일 내 기존 쿼리(Lines 29, 36, 44 등)는 문자열 리터럴(
'CONFIRMED')을, 새 쿼리는 FQN enum 참조를 사용합니다. FQN 방식이 더 타입 안전하지만, 파일 내 일관성을 위해 통일하는 것이 좋습니다. 가능하다면 기존 쿼리도 FQN 방식으로 통일하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`
around lines 83 - 96, Both methods countActiveBookings and
countActiveBookingsByStores duplicate the same status filter and use a different
enum reference style than other queries; change both queries to accept a single
parameter (e.g., :statuses) of type
List<com.eatsfine.eatsfine.domain.booking.enums.BookingStatus> and use "b.status
in :statuses" to centralize the filter, and update other queries in the file to
use the fully-qualified BookingStatus enum
(com.eatsfine.eatsfine.domain.booking.enums.BookingStatus) for consistency so
adding a new status only requires updating the call site that builds the
statuses list rather than modifying multiple repository queries.
src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java
Show resolved
Hide resolved
| List<Object[]> bookingCounts = bookingRepository.countActiveBookingsByStores(myStores); | ||
| Map<Long, Long> bookingCountMap = bookingCounts.stream() | ||
| .collect(Collectors.toMap( | ||
| row -> (Long) row[0], | ||
| row -> (Long) row[1] | ||
| )); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
List<Object[]> 반환 타입의 타입 안전성 부족
bookingCounts의 각 원소를 (Long)으로 캐스팅하고 있는데, 이는 런타임에서만 검증됩니다. JPQL count() 결과가 Long을 보장하지만, 쿼리 변경 시 ClassCastException 위험이 있습니다. 타입 안전한 DTO projection(record 또는 인터페이스) 사용을 권장합니다.
♻️ 타입 안전한 projection 예시
BookingRepository에서:
public record StoreBookingCount(Long storeId, Long count) {}
`@Query`("select new com.eatsfine.eatsfine.domain.booking.repository.BookingRepository.StoreBookingCount(" +
"b.store.id, count(b)) from Booking b " +
"where b.store in :stores " +
"and b.status in (...) " +
"group by b.store.id")
List<StoreBookingCount> countActiveBookingsByStores(`@Param`("stores") List<Store> stores);서비스에서:
Map<Long, Long> bookingCountMap = bookingCounts.stream()
.collect(Collectors.toMap(StoreBookingCount::storeId, StoreBookingCount::count));🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java`
around lines 129 - 134, Replace the raw List<Object[]> projection from
bookingRepository.countActiveBookingsByStores with a type-safe DTO projection
(e.g., a record or interface like StoreBookingCount) returned by the repository
method, update the repository method signature (countActiveBookingsByStores) to
return List<StoreBookingCount> and construct the projection in the JPQL/Query,
then update the service mapping that builds bookingCountMap to collect from
StoreBookingCount using its accessors (StoreBookingCount::storeId,
StoreBookingCount::count) instead of casting Object[] elements.
src/main/java/com/eatsfine/eatsfine/domain/store/status/StoreSuccessStatus.java
Outdated
Show resolved
Hide resolved
b1f3e35 to
1b383ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`:
- Around line 113-119: The repository method countActiveBookingsByStores
currently returns List<Object[]> which is error-prone; replace it with a
type-safe projection (either an interface or a record) such as StoreBookingCount
(with methods/getters getStoreId() and getBookingCount()) and change the
BookingRepository method signature to return List<StoreBookingCount>; update the
`@Query` to select the projected fields (store id and count) matching the
projection and keep the same parameter `@Param`("stores") List<Store> stores so
callers receive a typed result instead of Object[].
- Around line 106-111: Remove the unused single-entity repository method
countActiveBookings from BookingRepository: delete the entire method declaration
(the `@Query` and Long countActiveBookings(`@Param`("store") Store store);) since
StoreQueryServiceImpl only uses the bulk method countActiveBookingsByStores;
ensure no other callers reference countActiveBookings and run tests/compile to
verify removal.
| @Query("select count(b) from Booking b " + | ||
| "where b.store = :store " + | ||
| "and b.status in (com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.PENDING, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.COMPLETED)") | ||
| Long countActiveBookings(@Param("store") Store store); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
rg -nP '\bcountActiveBookings\b' --type=java -C2Repository: Eatsfine/BE
Length of output: 784
🏁 Script executed:
rg -nP '\bcountActiveBookingsByStores\b' --type=java -C2Repository: Eatsfine/BE
Length of output: 1312
countActiveBookings 메서드는 사용되지 않으므로 제거해주세요.
코드베이스에서 이 메서드가 호출되는 곳이 없으며, StoreQueryServiceImpl에서는 countActiveBookingsByStores bulk 쿼리만 사용 중입니다. N+1 문제를 해결하기 위한 bulk 쿼리만 필요하므로 단건 메서드는 삭제하는 것이 좋습니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`
around lines 106 - 111, Remove the unused single-entity repository method
countActiveBookings from BookingRepository: delete the entire method declaration
(the `@Query` and Long countActiveBookings(`@Param`("store") Store store);) since
StoreQueryServiceImpl only uses the bulk method countActiveBookingsByStores;
ensure no other callers reference countActiveBookings and run tests/compile to
verify removal.
| @Query("select b.store.id, count(b) from Booking b " + | ||
| "where b.store in :stores " + | ||
| "and b.status in (com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.PENDING, " + | ||
| "com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.COMPLETED) " + | ||
| "group by b.store.id") | ||
| List<Object[]> countActiveBookingsByStores(@Param("stores") List<Store> stores); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
List<Object[]> 대신 타입 안전한 반환 타입 사용을 권장합니다.
List<Object[]>는 인덱스 기반 접근이 필요하여 호출부에서 캐스팅 실수가 발생하기 쉽습니다. DTO 프로젝션이나 record를 사용하면 타입 안전성과 가독성이 향상됩니다.
♻️ 인터페이스 프로젝션 예시
public interface StoreBookingCount {
Long getStoreId();
Long getBookingCount();
}- `@Query`("select b.store.id, count(b) from Booking b " +
+ `@Query`("select b.store.id as storeId, count(b) as bookingCount from Booking b " +
"where b.store in :stores " +
"and b.status in (com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED, " +
"com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.PENDING, " +
"com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.COMPLETED) " +
"group by b.store.id")
- List<Object[]> countActiveBookingsByStores(`@Param`("stores") List<Store> stores);
+ List<StoreBookingCount> countActiveBookingsByStores(`@Param`("stores") List<Store> stores);🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`
around lines 113 - 119, The repository method countActiveBookingsByStores
currently returns List<Object[]> which is error-prone; replace it with a
type-safe projection (either an interface or a record) such as StoreBookingCount
(with methods/getters getStoreId() and getBookingCount()) and change the
BookingRepository method signature to return List<StoreBookingCount>; update the
`@Query` to select the projected fields (store id and count) matching the
projection and keep the same parameter `@Param`("stores") List<Store> stores so
callers receive a typed result instead of Object[].
0fac292 to
1451dda
Compare
💡 작업 개요
✅ 작업 내용
🧪 테스트 내용
📝 기타 참고 사항
Summary by CodeRabbit