Conversation
20260113 #66 rank 기능 구현
|
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. Walkthrough메뉴 관련 엔드포인트(단건 조회, 구내별 목록, 구내별 상위 3개, 이미지 업로드)와 TopMenuResponse DTO가 추가되었고, 메뉴 생성 및 상위 조회를 위한 Redis ZSet(ranking) 연동 및 관련 서비스 로직이 도입되었습니다. MenuResponse에 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as MenuController
participant Service as MenuService
participant Redis
participant DB as Database
Client->>Controller: GET /cafeteria/{id}/top-menus
Controller->>Service: getTop3MenusByCafeteriaId(cafeteriaId)
Service->>DB: verify cafeteria exists
DB-->>Service: exists
Service->>Redis: ZREVRANGE cafeteria:{id}:menu:rank 0 2
Redis-->>Service: list of menuIds
Service->>DB: fetch menus by IDs
DB-->>Service: Menu entities
Service->>Service: convert Menu -> MenuResponse -> TopMenuResponse
Service-->>Controller: List<TopMenuResponse>
Controller-->>Client: 200 OK
sequenceDiagram
participant Client
participant OrderSvc as OrderService
participant DB as Database
participant TxSync as TransactionSynchronization
participant MenuSvc as MenuService
participant Redis
Client->>OrderSvc: create order request
OrderSvc->>DB: save order (transaction)
OrderSvc->>TxSync: register afterCommit callback (ranking payload)
DB-->>OrderSvc: commit
TxSync->>MenuSvc: updateMenuRanking(rankingDtos)
MenuSvc->>Redis: ZINCRBY cafeteria:{id}:menu:rank quantity menuId
Redis-->>MenuSvc: score updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/campustable/be/domain/menu/service/MenuService.java (1)
197-213:⚠️ Potential issue | 🟠 Major메뉴 삭제 시 Redis 랭킹 데이터 미정리
deleteMenu에서 DB의 메뉴는 삭제하지만 Redis sorted set(cafeteria:<id>:menu:rank)의 해당 멤버는 제거하지 않습니다. 삭제된 메뉴가 상위 랭킹을 차지하고 있으면,getTop3MenusByCafeteriaId에서null체크로 건너뛰게 되어 3개 미만의 결과가 반환될 수 있습니다.🛠️ 수정 제안
public void deleteMenu(Long menuId) { Optional<Menu> menu = menuRepository.findById(menuId); if (menu.isEmpty()) { log.error("menuId not found {}", menuId); throw new CustomException(ErrorCode.MENU_NOT_FOUND); } + // Redis 랭킹에서 제거 + try { + Long cafeteriaId = menu.get().getCategory().getCafeteria().getCafeteriaId(); + String key = "cafeteria:" + cafeteriaId + ":menu:rank"; + stringRedisTemplate.opsForZSet().remove(key, String.valueOf(menuId)); + } catch (Exception e) { + log.warn("deleteMenu: Redis 랭킹 제거 실패. menuId={}", menuId, e); + } if (menu.get().getMenuUrl() != null && !menu.get().getMenuUrl().isBlank()) {
🤖 Fix all issues with AI agents
In `@src/main/java/com/campustable/be/domain/order/service/OrderService.java`:
- Around line 74-79: The afterCommit callback registered via
TransactionSynchronizationManager.registerSynchronization calls
updateMenuRanking and accesses lazy associations
(menu.getCategory().getCafeteria()) outside the transaction causing
LazyInitializationException; fix by collecting the needed primitives inside the
transaction (e.g., cafeteriaId, menuId, quantity) while still in the
transactional method in OrderService (before
TransactionSynchronizationManager.registerSynchronization) and pass that
collection (List/DTO of ids and counts) into the
TransactionSynchronization.afterCommit so updateMenuRanking consumes only those
IDs/quantities instead of dereferencing menu.getCategory()/Category.cafeteria
after the session is closed.
🧹 Nitpick comments (3)
src/main/java/com/campustable/be/domain/menu/service/MenuService.java (2)
216-217:@Transactional(readOnly = true)로 변경 권장이 메서드는 DB에 쓰기 작업이 없으므로
readOnly = true를 설정하여 Hibernate 플러시 모드 최적화 및 읽기 전용 커넥션 라우팅 혜택을 받을 수 있습니다.♻️ 수정 제안
- `@Transactional` + `@Transactional`(readOnly = true) public List<TopMenuResponse> getTop3MenusByCafeteriaId(Long cafeteriaId) {
223-239:reverseRange의 반환 타입Set<String>은 순서 보장이 명시적이지 않음
reverseRange는 내부적으로LinkedHashSet을 반환하여 순서가 유지되지만, API 시그니처상Set<String>이므로 이 동작에 의존하는 것은 다소 취약합니다.reverseRangeWithScores를 사용하면 순서와 점수를 동시에 얻을 수 있어 더 안전하고, 실제 점수 기반 랭킹도 가능합니다.src/main/java/com/campustable/be/domain/menu/controller/MenuController.java (1)
60-67: REST 경로가 리소스 의미와 불일치
GET /api/cafeteria/{cafeteria-id}는List<MenuResponse>를 반환하지만, URL만 보면 식당 상세 정보를 반환할 것처럼 보입니다. 리소스를 명확히 나타내기 위해/cafeteria/{cafeteria-id}/menus로 변경하는 것을 권장합니다.♻️ 수정 제안
- `@GetMapping`("/cafeteria/{cafeteria-id}") + `@GetMapping`("/cafeteria/{cafeteria-id}/menus")
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/campustable/be/domain/order/service/OrderService.java`:
- Line 40: OrderService currently has an unused dependency field
stringRedisTemplate injected via `@RequiredArgsConstructor`; remove the private
final StringRedisTemplate stringRedisTemplate field (and any related
commented-out usages) so Spring won't require a StringRedisTemplate bean at
startup, and reintroduce the field only when the commented functionality is
re-enabled; ensure the class-level `@RequiredArgsConstructor` remains correct
after removing the field.
🧹 Nitpick comments (1)
src/main/java/com/campustable/be/domain/order/service/OrderService.java (1)
42-104: 주석 처리된 코드 블록 제거 권장
createOrder,updateMenuRanking,RankingDto등 약 60줄의 코드가 주석 처리되어 있습니다. 이 코드는 Git 히스토리에서 추적할 수 있으므로, 소스 파일에 남겨둘 필요가 없습니다. 주석 처리된 코드는 가독성을 떨어뜨리고, 사용하지 않는 import/필드 의존성을 유발합니다.기능이 준비되지 않았다면 feature branch에서 개발 후 병합하거나, 필요한 경우 TODO/이슈로 추적하는 것이 좋습니다.
| } | ||
|
|
||
| public void updateCategoryToReady(Long orderId,Long categoryId) { | ||
| private final StringRedisTemplate stringRedisTemplate; |
There was a problem hiding this comment.
사용되지 않는 stringRedisTemplate 의존성이 생성자 주입됨
@RequiredArgsConstructor로 인해 stringRedisTemplate이 생성자 매개변수로 주입됩니다. 현재 이 필드를 사용하는 코드가 모두 주석 처리되어 있으므로, Spring 컨텍스트에 StringRedisTemplate 빈이 반드시 존재해야 하는 불필요한 의존성이 생깁니다. 빈이 없으면 애플리케이션 시작 시 실패하고, 있더라도 사용되지 않는 의존성을 강제합니다.
주석 처리된 코드와 함께 이 필드도 제거하고, 기능 활성화 시 다시 추가하는 것을 권장합니다.
🔧 수정 제안
- private final StringRedisTemplate stringRedisTemplate;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private final StringRedisTemplate stringRedisTemplate; |
🤖 Prompt for AI Agents
In `@src/main/java/com/campustable/be/domain/order/service/OrderService.java` at
line 40, OrderService currently has an unused dependency field
stringRedisTemplate injected via `@RequiredArgsConstructor`; remove the private
final StringRedisTemplate stringRedisTemplate field (and any related
commented-out usages) so Spring won't require a StringRedisTemplate bean at
startup, and reintroduce the field only when the commented functionality is
re-enabled; ensure the class-level `@RequiredArgsConstructor` remains correct
after removing the field.
1 participant
✨ 변경 사항
✅ 테스트
Summary by CodeRabbit
새로운 기능
문서
기타