Conversation
Walkthrough카페테리아별 상위 3개 메뉴를 반환하는 공개 엔드포인트와 Redis 기반 메뉴 랭킹 저장/갱신 로직이 추가되었습니다. DTO Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as MenuController
participant Service as MenuService
participant Redis as Redis
participant Repo as MenuRepository
Client->>Controller: GET /cafeteria/{id}/top-menus
Controller->>Service: getTop3MenusByCafeteriaId(id)
Service->>Redis: ZREVRANGE cafeteria:{id}:menu:rank 0 2
Redis-->>Service: [menuId1, menuId2, menuId3]
loop for each menuId
Service->>Repo: findById(menuId)
Repo-->>Service: Menu entity
end
Service-->>Controller: List<TopMenuResponse> (ranked)
Controller-->>Client: 200 OK + payload
sequenceDiagram
participant Client as Client
participant OrderSvc as OrderService
participant OrderRepo as OrderRepository
participant MenuRepo as MenuRepository
participant Redis as Redis
Client->>OrderSvc: POST /orders (order)
OrderSvc->>OrderRepo: save(order)
OrderSvc->>OrderSvc: afterCommit callback triggers
loop for each orderItem
OrderSvc->>MenuRepo: findById(menuId)
MenuRepo-->>OrderSvc: Menu -> Category -> CafeteriaId
OrderSvc->>Redis: ZINCRBY cafeteria:{cafeteriaId}:menu:rank quantity menuId
Redis-->>OrderSvc: updated score
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
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)
150-160: Redis에서 메뉴 삭제 누락 - 데이터 불일치 발생메뉴 삭제 시 Redis sorted set에서 해당 메뉴를 제거하지 않아 데이터 불일치가 발생합니다.
getTop3MenusByCafeteriaId에서 삭제된 메뉴 ID가 조회될 수 있으며, DB 조회 시 null이 됩니다.🐛 제안된 수정 사항
`@Transactional` 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); } else { + Long cafeteriaId = menu.get().getCategory().getCafeteria().getCafeteriaId(); + String key = "cafeteria:" + cafeteriaId + ":menu:rank"; + stringRedisTemplate.opsForZSet().remove(key, String.valueOf(menuId)); menuRepository.delete(menu.get()); } }
🤖 Fix all issues with AI agents
In `@src/main/java/com/campustable/be/domain/menu/service/MenuService.java`:
- Around line 63-71: The DB save followed by Redis ZSet update in MenuService
(menuRepository.save(...) then stringRedisTemplate.opsForZSet().add(...)) is not
atomic and Redis failures will leave DB/Redis inconsistent; modify the method to
either (A) wrap the Redis call in a try-catch (catch
DataAccessException/RedisConnectionFailureException or Exception), log the
savedMenu id, cafeteria key and error, and swallow the exception so the DB
commit stands, or (B) instead publish an event (use ApplicationEventPublisher)
after menuRepository.save(savedMenu) and implement an async listener that
performs stringRedisTemplate.opsForZSet().add(key,
String.valueOf(savedMenu.getId()), 0.0) with its own retry/error handling;
update callers to use MenuResponse.from(savedMenu) unchanged.
In `@src/main/java/com/campustable/be/domain/order/service/OrderService.java`:
- Around line 72-83: The Redis ZINCRBY calls currently run inside the
`@Transactional` createOrder() and can cause DB/Redis inconsistency if Redis
fails; change the logic so the loop that calls
stringRedisTemplate.opsForZSet().incrementScore(...) for each OrderItem (inside
createOrder()) is executed only after the DB transaction commits by registering
a TransactionSynchronization (e.g.,
TransactionSynchronizationManager.registerSynchronization or
TransactionSynchronization.afterCommit) and moving the key construction
("cafeteria:"+cafeteriaId+":menu:rank") and increment operations into the
afterCommit callback so Redis updates occur post-commit.
🧹 Nitpick comments (3)
src/main/java/com/campustable/be/domain/menu/service/MenuService.java (2)
66-69: Redis 키 패턴 중복 - 상수로 추출 권장
"cafeteria:" + cafeteriaId + ":menu:rank"패턴이createMenu와getTop3MenusByCafeteriaId에서 중복됩니다. 오타나 불일치를 방지하기 위해 상수 또는 메서드로 추출하세요.♻️ 제안된 리팩토링
+ private static final String MENU_RANK_KEY_FORMAT = "cafeteria:%d:menu:rank"; + + private String getMenuRankKey(Long cafeteriaId) { + return String.format(MENU_RANK_KEY_FORMAT, cafeteriaId); + } // createMenu 내에서: - String key = "cafeteria:" + cafeteriaId + ":menu:rank"; + String key = getMenuRankKey(cafeteriaId); // getTop3MenusByCafeteriaId 내에서: - String key = "cafeteria:" + cafeteriaId + ":menu:rank"; + String key = getMenuRankKey(cafeteriaId);
163-164: 읽기 전용 트랜잭션으로 변경 필요
getTop3MenusByCafeteriaId는 데이터를 읽기만 하므로@Transactional(readOnly = true)로 변경하여 성능을 최적화하세요.♻️ 제안된 수정 사항
- `@Transactional` + `@Transactional`(readOnly = true) public List<TopMenuResponse> getTop3MenusByCafeteriaId(Long cafeteriaId) {src/main/java/com/campustable/be/domain/order/service/OrderService.java (1)
72-83: Redis ZSET 다건 업데이트는 파이프라인으로 성능 개선 권장현재 코드는 각 orderItem마다
ZINCRBY명령을 개별 호출하므로 네트워크 왕복이 누적됩니다. Spring Data Redis 3.5.7에서 지원하는executePipelined()또는SessionCallback을 사용하면 여러 명령을 배치로 처리하여 네트워크 지연을 상당히 줄일 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/campustable/be/domain/menu/controller/MenuController.javasrc/main/java/com/campustable/be/domain/menu/controller/MenuControllerDocs.javasrc/main/java/com/campustable/be/domain/menu/dto/MenuRequest.javasrc/main/java/com/campustable/be/domain/menu/dto/MenuResponse.javasrc/main/java/com/campustable/be/domain/menu/dto/TopMenuResponse.javasrc/main/java/com/campustable/be/domain/menu/service/MenuService.javasrc/main/java/com/campustable/be/domain/order/service/OrderService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/campustable/be/domain/menu/service/MenuService.java (1)
src/main/java/com/campustable/be/domain/cafeteria/service/CafeteriaService.java (1)
Slf4j(17-84)
🔇 Additional comments (5)
src/main/java/com/campustable/be/domain/menu/dto/MenuRequest.java (1)
38-48: LGTM!빌더 체인 내 빈 줄 추가는 순수 포맷팅 변경으로, 기능에 영향이 없습니다.
src/main/java/com/campustable/be/domain/menu/dto/TopMenuResponse.java (1)
1-21: LGTM!DTO 구현이 깔끔합니다. Builder 패턴과 정적 팩토리 메서드 사용이 적절합니다.
src/main/java/com/campustable/be/domain/menu/controller/MenuController.java (1)
97-105: LGTM!새로운 엔드포인트가 올바르게 구현되었습니다. 서비스 레이어에 적절히 위임하고 있습니다.
src/main/java/com/campustable/be/domain/menu/controller/MenuControllerDocs.java (1)
127-142: LGTM!API 문서가 잘 작성되었습니다. 성공 및 에러 케이스에 대한 응답이 적절하게 문서화되어 있습니다.
src/main/java/com/campustable/be/domain/menu/dto/MenuResponse.java (1)
38-39: 원본 코드의 체이닝된 호출menu.getCategory().getCafeteria().getCafeteriaId()는 NPE 위험이 없습니다.Menu 엔티티의
category필드(라인 33-35)는@NotNull어노테이션과nullable = false제약 조건으로 보호되고, Category 엔티티의cafeteria필드(라인 36-38)도 동일한 제약 조건을 가지고 있습니다. 따라서 두 관계 모두 데이터베이스 및 JPA 레벨에서 null이 될 수 없음이 보장되므로, 체이닝된 호출은 안전합니다.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/main/java/com/campustable/be/domain/menu/service/MenuService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/campustable/be/domain/order/service/OrderService.java
Outdated
Show resolved
Hide resolved
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/menu/service/MenuService.java`:
- Around line 183-185: topMenuIds creation using
topMenus.stream().map(Long::parseLong).toList() can throw NumberFormatException
if Redis values are malformed; update MenuService to defensively parse: replace
the direct Long::parseLong mapping with a safe parse that catches
NumberFormatException (e.g., a helper like safeParseLong or an inline try/catch
in the stream), filter out or skip invalid entries, and log a warning (using the
existing logger in MenuService) when a value cannot be parsed before collecting
into topMenuIds.
🧹 Nitpick comments (2)
src/main/java/com/campustable/be/domain/menu/service/MenuService.java (1)
170-171: 읽기 전용 트랜잭션 사용 권장이 메서드는 데이터 조회만 수행하므로
@Transactional(readOnly = true)로 설정하면 Hibernate flush 모드 최적화와 DB 읽기 복제본 라우팅 이점을 얻을 수 있습니다.♻️ 수정 제안
- `@Transactional` + `@Transactional`(readOnly = true) public List<TopMenuResponse> getTop3MenusByCafeteriaId(Long cafeteriaId) {src/main/java/com/campustable/be/domain/order/service/OrderService.java (1)
84-101: 에러 핸들링이 적절합니다.각 OrderItem별로 try-catch를 적용하여 하나의 항목 실패가 전체 랭킹 업데이트를 중단시키지 않도록 구현되었습니다. 다만, 에러 로그에 메뉴 ID나 cafeteriaId 같은 추가 컨텍스트를 포함하면 디버깅에 도움이 됩니다.
♻️ 로깅 개선 제안
catch (Exception e) { - log.error("랭킹 점수 반영 실패: {}",e.getMessage()); + log.error("랭킹 점수 반영 실패 - menuId: {}, quantity: {}, 원인: {}", + orderItem.getMenu().getId(), orderItem.getQuantity(), e.getMessage()); }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/campustable/be/domain/menu/controller/MenuController.java`:
- Line 22: The class MenuController is missing the implementation of the
abstract method getMenuById declared in MenuControllerDocs; add a public method
in MenuController with the signature matching the interface (e.g., public
ResponseEntity<MenuResponse> getMenuById(Long menuId)) annotated with `@Override`,
`@GetMapping`("/menus/{menu_id}") and `@LogMonitoringInvocation`, take
`@PathVariable`(name="menu_id") Long menuId, and return
ResponseEntity.ok(menuService.getMenuById(menuId)) so the controller fulfills
the interface contract.
In `@src/main/java/com/campustable/be/domain/menu/service/MenuService.java`:
- Around line 61-78: The code in MenuService declares Menu menu and Menu
savedMenu twice; remove the duplicated declaration block so only one Menu menu =
request.toEntity(category) and one Menu savedMenu = menuRepository.save(menu)
remain (retain the try/catch that uses savedMenu). Locate the duplicate
declarations around the method that creates a menu (variables named menu and
savedMenu) and delete the extra repeated lines to avoid the compile error while
keeping the Redis registration logic intact.
🧹 Nitpick comments (1)
src/main/java/com/campustable/be/domain/menu/service/MenuService.java (1)
221-221: 읽기 전용 트랜잭션 권장이 메서드는 데이터를 조회만 하므로
@Transactional(readOnly = true)로 변경하면 성능 최적화와 의도 명확화에 도움이 됩니다.♻️ 수정 제안
- `@Transactional` + `@Transactional`(readOnly = true) public List<TopMenuResponse> getTop3MenusByCafeteriaId(Long cafeteriaId) {
src/main/java/com/campustable/be/domain/menu/controller/MenuController.java
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 변경 사항
✅ 테스트
Summary by CodeRabbit
새로운 기능
문서