-
Notifications
You must be signed in to change notification settings - Fork 0
feat(store): store 검색 #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "feature/#26_Store\uAC80\uC0C9"
Conversation
|
""" Walkthrough이번 변경에서는 매장 이름을 부분적으로 검색할 수 있는 새로운 GET 엔드포인트( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StoreController
participant StoreService
participant StoreRepository
Client->>StoreController: GET /stores/search?name=XXX
StoreController->>StoreService: searchStoresByName(name)
StoreService->>StoreRepository: findByNameContainingIgnoreCaseAndDeletedFalse(name)
StoreRepository-->>StoreService: List<Store>
StoreService-->>StoreController: List<StoreReadDto>
StoreController-->>Client: ApiUtils.success(List<StoreReadDto>)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/example/gtable/store/controller/StoreController.java(2 hunks)src/main/java/com/example/gtable/store/repository/StoreRepository.java(1 hunks)src/main/java/com/example/gtable/store/service/StoreService.java(2 hunks)src/main/java/com/example/gtable/store/service/StoreServiceImpl.java(1 hunks)src/main/java/com/example/gtable/user/service/UserService.java(0 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/example/gtable/user/service/UserService.java
🔇 Additional comments (1)
src/main/java/com/example/gtable/store/controller/StoreController.java (1)
12-13:@RequestParam임포트는 적절합니다
필요한 임포트가 잘 추가되었습니다. 별다른 이슈 없습니다.
| List<Store> searchStoresByName(String name); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
서비스 계층이 엔티티를 직접 노출하고 있습니다
다른 메서드는 모두 DTO(StoreReadDto, StoreReadResponse 등)를 반환하는데, 새로 추가된 searchStoresByName만 Store 엔티티를 그대로 반환하여 API 스키마가 불일치합니다. 민감 필드 노출 가능성과 계층 분리 훼손이 우려됩니다. DTO 리스트 혹은 별도 Response 클래스로 바꾸는 리팩터가 필요합니다.
- List<Store> searchStoresByName(String name);
+ List<StoreReadDto> searchStoresByName(String name);📝 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.
| List<Store> searchStoresByName(String name); | |
| List<StoreReadDto> searchStoresByName(String name); |
🤖 Prompt for AI Agents
In src/main/java/com/example/gtable/store/service/StoreService.java around lines
24 to 25, the method searchStoresByName currently returns a List of Store
entities directly, which breaks the API schema consistency and risks exposing
sensitive fields. Refactor this method to return a List of appropriate DTOs such
as StoreReadDto or a dedicated response class instead of the entity. Update the
method signature and implementation accordingly to map Store entities to the
DTOs before returning.
| @Override | ||
| public List<Store> searchStoresByName(String name) { | ||
| return storeRepository.findByNameContainingIgnoreCase(name); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
트랜잭션 옵션·삭제 필터·DTO 변환 누락
readOnly속성이 없어 불필요한 쓰기 락이 걸릴 수 있습니다.- 삭제된 매장까지 함께 조회됩니다.
- 엔티티를 그대로 반환하여 외부 노출 위험이 있습니다.
아래와 같이 한 번에 개선해 주세요.
- @Override
- public List<Store> searchStoresByName(String name) {
- return storeRepository.findByNameContainingIgnoreCase(name);
- }
+ @Override
+ @Transactional(readOnly = true)
+ public List<StoreReadDto> searchStoresByName(String name) {
+ return storeRepository.findByNameContainingIgnoreCaseAndDeletedFalse(name)
+ .stream()
+ .map(StoreReadDto::fromEntity)
+ .toList();
+ }📝 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.
| @Override | |
| public List<Store> searchStoresByName(String name) { | |
| return storeRepository.findByNameContainingIgnoreCase(name); | |
| } | |
| @Override | |
| @Transactional(readOnly = true) | |
| public List<StoreReadDto> searchStoresByName(String name) { | |
| return storeRepository | |
| .findByNameContainingIgnoreCaseAndDeletedFalse(name) | |
| .stream() | |
| .map(StoreReadDto::fromEntity) | |
| .toList(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/gtable/store/service/StoreServiceImpl.java around
lines 95 to 98, the searchStoresByName method lacks a readOnly transaction
annotation, does not filter out deleted stores, and returns entity objects
directly, risking external exposure. Add @Transactional(readOnly = true) to
optimize transaction handling, modify the repository query or add filtering to
exclude deleted stores, and convert the resulting entities to DTOs before
returning to prevent exposing internal entity details.
| @GetMapping("/search") | ||
| public ResponseEntity<?> searchStores(@RequestParam("name") String name) { | ||
| return ResponseEntity | ||
| .ok() | ||
| .body( | ||
| ApiUtils.success( | ||
| storeService.searchStoresByName(name) | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
엔티티 직접 반환으로 API 응답 일관성/보안 문제
서비스가 Store 엔티티를 그대로 반환하므로 컨트롤러도 그대로 노출하고 있습니다. 기존 엔드포인트는 모두 DTO를 사용하므로 API 스키마가 달라집니다. 또한 내부 필드(삭제 플래그 등) 노출 가능성이 있습니다. 서비스·DTO 구조가 정리된 뒤 응답 타입도 함께 수정해 주세요.
추가로 name 파라미터에 대해 @NotBlank 등 검증을 적용하면 부적절한 요청을 미연에 방지할 수 있습니다.
🤖 Prompt for AI Agents
In src/main/java/com/example/gtable/store/controller/StoreController.java around
lines 91 to 100, the searchStores method currently returns Store entities
directly, causing inconsistency with other endpoints that use DTOs and risking
exposure of internal fields. Refactor the method to convert the Store entities
returned by storeService.searchStoresByName(name) into appropriate DTOs before
returning them in the response. Also, add validation annotation such as
@NotBlank to the 'name' request parameter to prevent invalid requests early.
작업 요약
Issue Link
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit
신규 기능
스타일