feat(Store): 주점 정렬 로직 구현#148
Hidden character warning
Conversation
|
Caution Review failedThe pull request is closed. """ WalkthroughSwagger 문서의 일부 설명이 수정되었으며, 대기 인원 기준으로 매장 목록을 정렬해 반환하는 새로운 GET API가 StoreController에 추가되었습니다. 이를 위해 StoreWaitingInfo DTO, StoreService 인터페이스 및 구현체에 관련 메서드가 추가되고, Redis를 활용한 대기 인원 집계 로직이 구현되었습니다. 또한, WaitingUserResponse DTO가 예약 ID와 유저 ID를 명확히 구분하도록 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StoreController
participant StoreService
participant StoreServiceImpl
participant Redis
participant Database
Client->>StoreController: GET /waiting-list?order={order}
StoreController->>StoreService: getStoresByWaitingCount(desc)
StoreService->>StoreServiceImpl: getStoresByWaitingCount(desc)
StoreServiceImpl->>Redis: Find keys matching waiting:*
Redis-->>StoreServiceImpl: List of keys
loop for each valid key
StoreServiceImpl->>Redis: ZCARD (get waiting count)
StoreServiceImpl->>Database: Find storeName by storeId
end
StoreServiceImpl-->>StoreService: List<StoreWaitingInfo>
StoreService-->>StoreController: List<StoreWaitingInfo>
StoreController-->>Client: API 응답 반환
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/controller/StoreController.java (2)
102-104: API 문서화가 개선 가능합니다.Operation 설명에서 "예약 많은순/적은순"이라고 했는데, 실제로는 "대기 많은순/적은순"입니다. 또한 파라미터 설명이 더 구체적이면 좋겠습니다.
다음과 같이 수정을 제안합니다:
-@Operation(summary = "예약 많은순/적은순 주점 리스트 조회", description = "desc(대기 많은순) , asc(대기 적은순)") +@Operation(summary = "대기 인원 기준 주점 리스트 조회", description = "대기 인원 수를 기준으로 주점을 정렬합니다. order 파라미터: desc(대기 많은순, 기본값), asc(대기 적은순)")
105-107: 파라미터 처리 로직을 개선할 수 있습니다.현재 로직은 올바르게 작동하지만, 더 명확하게 표현할 수 있습니다.
다음과 같이 개선을 제안합니다:
public ResponseEntity<?> getStoreWaitingList( - @RequestParam(defaultValue = "desc") String order) { - boolean desc = !"asc".equalsIgnoreCase(order); // 기본: 대기 많은 순 + @RequestParam(defaultValue = "desc") String order) { + boolean desc = "desc".equalsIgnoreCase(order) || (!"asc".equalsIgnoreCase(order) && !"desc".equalsIgnoreCase(order));또는 enum을 사용하여 더 안전하게 처리할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/controller/ReservationController.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/controller/StoreController.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/dto/StoreWaitingInfo.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreService.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreServiceImpl.java(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/controller/StoreController.java (1)
nowait-app-user-api/src/main/java/com/nowait/applicationuser/reservation/controller/ReservationController.java (1)
ReservationController(29-113)
🔇 Additional comments (7)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/controller/ReservationController.java (2)
36-36: Swagger 문서 업데이트가 적절합니다.대기 상태와 호출 상태를 명확히 구분하여 문서화한 것이 좋습니다.
44-44: 문서 설명이 개선되었습니다.취소/완료 상태를 명확히 표현한 업데이트가 적절합니다.
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreService.java (1)
11-11: 새로운 서비스 메서드 시그니처가 적절합니다.대기 인원수 기준 정렬 기능을 위한 메서드 추가가 적절하며, boolean 파라미터로 정렬 방향을 제어하는 것이 직관적입니다.
Also applies to: 25-25
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/dto/StoreWaitingInfo.java (1)
7-19: DTO 클래스 구조가 적절합니다.Lombok 어노테이션과 Swagger 문서화가 잘 되어 있고, 필드 구성도 적절합니다.
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/controller/StoreController.java (1)
109-116: 응답 구조가 일관성 있게 구현되었습니다.다른 엔드포인트와 동일한 패턴으로 ApiUtils.success()를 사용한 것이 좋습니다.
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreServiceImpl.java (2)
10-10: Redis Template 의존성 추가가 적절합니다.Redis 데이터 접근을 위한 의존성 추가가 올바르게 구현되었습니다.
Also applies to: 38-38
203-212: 필터링 로직과 매핑 로직이 적절합니다.waiting:party: 키를 제외하고 zset 타입만 처리하는 로직이 잘 구현되었습니다.
| @Schema(description = "주점 대기 현황 정보 DTO") | ||
| public class StoreWaitingInfo { | ||
| @Schema(description = "주점 ID", example = "1") | ||
| private String storeId; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
storeId 타입 일관성을 확인하세요.
다른 부분에서 storeId가 Long 타입으로 사용되는데, 여기서는 String으로 정의되어 있습니다. 타입 일관성을 위해 Long 타입 사용을 고려해보세요.
다음 스크립트로 storeId 타입 사용 패턴을 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# storeId 필드의 타입 사용 패턴 확인
rg -A 2 -B 2 "storeId.*Long|Long.*storeId" --type java
rg -A 2 -B 2 "storeId.*String|String.*storeId" --type javaLength of output: 143956
🏁 Script executed:
#!/bin/bash
cat nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/dto/StoreWaitingInfo.javaLength of output: 586
StoreWaitingInfo DTO의 storeId를 Long 타입으로 변경하세요.
일관된 타입 사용을 위해 아래를 수정해주세요:
• nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/dto/StoreWaitingInfo.java
- @Schema(description = "주점 ID", example = "1")
- private String storeId;
+ @Schema(description = "주점 ID", example = "1")
+ private Long storeId;• 서비스 구현부(nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreServiceImpl.java)
• key에서 추출한 storeId를 String이 아닌 Long으로 파싱
• new StoreWaitingInfo(storeId, …) 호출부의 첫 번째 인자도 Long으로 변경
이렇게 하면 전체 코드베이스에서 storeId가 Long으로 통일되어 의도치 않은 형 변환 오류를 방지할 수 있습니다.
📝 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 String storeId; | |
| @Schema(description = "주점 ID", example = "1") | |
| private Long storeId; |
🤖 Prompt for AI Agents
In
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/dto/StoreWaitingInfo.java
at line 12, change the type of the storeId field from String to Long. Then, in
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreServiceImpl.java,
update the code to parse the storeId key as a Long instead of a String and
modify all calls to the StoreWaitingInfo constructor to pass a Long storeId.
This ensures consistent use of Long for storeId across the codebase and prevents
type conversion errors.
| // 주점 대기 리스트 반환 (많은 순/적은 순) | ||
| @Transactional(readOnly = true) | ||
| public List<StoreWaitingInfo> getStoresByWaitingCount(boolean desc) { | ||
| // 1. 모든 waiting:{storeId} key 조회 (패턴 탐색) | ||
| Set<String> keys = redisTemplate.keys("waiting:*"); | ||
| if (keys == null) return List.of(); | ||
|
|
||
| List<StoreWaitingInfo> result = keys.stream() | ||
| .filter(key -> key.startsWith("waiting:") && !key.startsWith("waiting:party:")) | ||
| .filter(key -> "zset".equals(redisTemplate.type(key).code())) // 타입 확인 확실히! | ||
| .map(key -> { | ||
| Long count = redisTemplate.opsForZSet().zCard(key); | ||
| String storeId = key.replace("waiting:", ""); | ||
| String storeName = storeRepository.findById(Long.valueOf(storeId)) | ||
| .map(Store::getName) | ||
| .orElse("UNKNOWN"); | ||
| return new StoreWaitingInfo(storeId, storeName, count != null ? count : 0); | ||
| }) | ||
| .sorted((a, b) -> b.getWaitingCount().compareTo(a.getWaitingCount())) | ||
| .toList(); | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
전반적인 메서드 구조 개선이 필요합니다.
성능, 예외 처리, 파라미터 활용 등 여러 개선점이 있지만, 기본적인 비즈니스 로직은 올바르게 구현되었습니다.
Redis 성능 최적화, 예외 처리 강화, 정렬 로직 수정을 포함한 전체적인 리팩토링을 권장합니다.
🤖 Prompt for AI Agents
In
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreServiceImpl.java
around lines 196 to 218, refactor the getStoresByWaitingCount method to improve
Redis performance by minimizing calls inside streams, add exception handling to
manage potential Redis or database errors gracefully, and utilize the boolean
parameter 'desc' to control sorting order dynamically instead of hardcoding
descending order. Also, optimize key filtering and consider batch fetching store
names to reduce database calls.
| @Transactional(readOnly = true) | ||
| public List<StoreWaitingInfo> getStoresByWaitingCount(boolean desc) { | ||
| // 1. 모든 waiting:{storeId} key 조회 (패턴 탐색) | ||
| Set<String> keys = redisTemplate.keys("waiting:*"); |
There was a problem hiding this comment.
Redis KEYS 명령어 사용에 대한 성능 우려사항이 있습니다.
Redis KEYS 명령어는 모든 키를 스캔하므로 운영 환경에서 성능 문제를 일으킬 수 있습니다.
SCAN 명령어를 사용하거나, 별도의 인덱스 구조를 고려해보세요:
-Set<String> keys = redisTemplate.keys("waiting:*");
+// SCAN을 사용한 개선 방안
+Set<String> keys = new HashSet<>();
+ScanOptions options = ScanOptions.scanOptions().match("waiting:*").build();
+try (Cursor<String> cursor = redisTemplate.scan(options)) {
+ cursor.forEachRemaining(keys::add);
+}📝 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.
| Set<String> keys = redisTemplate.keys("waiting:*"); | |
| // SCAN을 사용한 개선 방안 | |
| Set<String> keys = new HashSet<>(); | |
| ScanOptions options = ScanOptions.scanOptions().match("waiting:*").build(); | |
| try (Cursor<String> cursor = redisTemplate.scan(options)) { | |
| cursor.forEachRemaining(keys::add); | |
| } |
🤖 Prompt for AI Agents
In
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreServiceImpl.java
at line 200, replace the use of redisTemplate.keys("waiting:*") with a Redis
SCAN command implementation to avoid performance issues caused by the KEYS
command scanning all keys. Implement a cursor-based iteration using SCAN to
retrieve matching keys incrementally, or consider maintaining a separate index
structure for these keys to improve efficiency in production environments.
| .map(key -> { | ||
| Long count = redisTemplate.opsForZSet().zCard(key); | ||
| String storeId = key.replace("waiting:", ""); | ||
| String storeName = storeRepository.findById(Long.valueOf(storeId)) |
There was a problem hiding this comment.
Long 파싱에서 예외 처리가 누락되었습니다.
storeId를 Long으로 파싱할 때 NumberFormatException이 발생할 수 있습니다.
예외 처리를 추가하세요:
-String storeName = storeRepository.findById(Long.valueOf(storeId))
+String storeName;
+try {
+ storeName = storeRepository.findById(Long.valueOf(storeId))
+ .map(Store::getName)
+ .orElse("UNKNOWN");
+} catch (NumberFormatException e) {
+ storeName = "INVALID_STORE_ID";
+}📝 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.
| String storeName = storeRepository.findById(Long.valueOf(storeId)) | |
| String storeName; | |
| try { | |
| storeName = storeRepository.findById(Long.valueOf(storeId)) | |
| .map(Store::getName) | |
| .orElse("UNKNOWN"); | |
| } catch (NumberFormatException e) { | |
| storeName = "INVALID_STORE_ID"; | |
| } |
🤖 Prompt for AI Agents
In
nowait-app-user-api/src/main/java/com/nowait/applicationuser/store/service/StoreServiceImpl.java
at line 209, the code parses storeId to Long without handling potential
NumberFormatException. Add a try-catch block around the Long.valueOf(storeId)
parsing to catch NumberFormatException and handle it appropriately, such as
logging the error and returning a meaningful response or throwing a custom
exception.
1 participant
작업 요약
Issue Link
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit
신규 기능
문서화
기능 개선