-
Notifications
You must be signed in to change notification settings - Fork 4
[FEAT] 키워드 검색 API 추가 #369
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
[FEAT] 키워드 검색 API 추가 #369
Conversation
|
Warning Rate limit exceeded@kmw10693 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 14 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (2)
Walkthrough공지사항 검색 시 입력 키워드를 최근검색으로 저장하는 기능을 추가했습니다. 새로운 Changes
Sequence Diagram(s)sequenceDiagram
actor User as 사용자
participant NoticeController as NoticeController
participant NoticeService as NoticeService
participant RecentSearchService as RecentSearchService
participant NoticeRepo as NoticeRepository
participant RecentSearchRepo as RecentSearchRepository
participant UserService as UserService
participant DB as Database
User->>NoticeController: GET /api/v1/notices/search?keyword=test
NoticeController->>NoticeService: searchByKeyword("test")
NoticeService->>RecentSearchService: save("test")
RecentSearchService->>UserService: getCurrentUser()
UserService-->>RecentSearchService: User(id)
RecentSearchService->>RecentSearchRepo: findByUserIdAndKeyword(userId,"test")
RecentSearchRepo->>DB: SELECT recent_search WHERE user_id=? AND keyword=?
DB-->>RecentSearchRepo: existing?/none
alt existing
RecentSearchService->>RecentSearchRepo: save(touchedRecentSearch)
else new
RecentSearchService->>RecentSearchRepo: save(newRecentSearch)
end
RecentSearchService->>RecentSearchRepo: (if >20) delete oldest
NoticeService->>NoticeRepo: findByKeyword("test")
NoticeRepo->>DB: SELECT notice WHERE keyword LIKE ?
DB-->>NoticeRepo: notice list
NoticeService-->>NoticeController: results
NoticeController-->>User: BaseResponse(200, results)
sequenceDiagram
actor User as 사용자
participant RecentSearchController as RecentSearchController
participant RecentSearchService as RecentSearchService
participant UserService as UserService
participant RecentSearchRepo as RecentSearchRepository
participant DB as Database
User->>RecentSearchController: GET /api/v1/notices/searches/recent?limit=20
RecentSearchController->>RecentSearchService: list(20)
RecentSearchService->>UserService: getCurrentUser()
UserService-->>RecentSearchService: User(id)
RecentSearchService->>RecentSearchRepo: findByUserIdOrderByUpdatedAtDesc(userId, pageable)
RecentSearchRepo->>DB: SELECT ... ORDER BY updated_at DESC LIMIT ?
DB-->>RecentSearchRepo: recent searches
RecentSearchService-->>RecentSearchController: List<RecentSearch>
RecentSearchController-->>User: BaseResponse(200, 목록)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/java/ku_rum/backend/domain/search/domain/repository/RecentSearchRepository.java (1)
16-16: 삭제 메서드의 반환값 활용 고려
deleteByIdAndUserId와deleteByUserId메서드가 삭제된 레코드 수를 반환하지만, 서비스에서 이 값을 사용하지 않고 있습니다. 삭제 성공 여부를 확인하려면 반환값을 검증하는 것을 고려해보세요.Also applies to: 18-18
src/main/java/ku_rum/backend/domain/search/domain/RecentSearch.java (1)
29-30: User 엔티티 관계 설계 확인
userId를Long타입으로 저장하고 있습니다.@ManyToOne으로User엔티티와 직접 관계를 맺지 않는 설계인데, 이는 구조를 단순하게 유지하는 장점이 있지만 다음 사항을 고려해야 합니다:
- 데이터베이스 레벨의 참조 무결성(FK 제약조건) 부재
- 사용자 삭제 시 고아(orphaned) 레코드 발생 가능성
현재 설계가 의도된 것이라면 문제없지만, 향후 사용자 삭제 시 관련 검색 기록도 함께 정리하는 로직이 필요할 수 있습니다.
src/main/java/ku_rum/backend/domain/search/application/RecentSearchService.java (2)
40-49: 성능 최적화 고려: 매 저장마다 조회 및 삭제 수행검색이 발생할 때마다
MAX_RECENT_SEARCH + 1개의 레코드를 조회하고 초과분을 삭제하는 로직이 실행됩니다. 검색 빈도가 높은 경우 성능에 영향을 줄 수 있습니다.다음과 같은 최적화 방안을 고려해보세요:
- 조건부 정리: 새 레코드가 추가될 때만 정리 수행
- 배치 정리: 별도의 스케줄러로 주기적으로 정리
- 카운트 확인: 먼저 count 쿼리로 초과 여부 확인 후 필요시에만 삭제
🔎 조건부 정리 예시
+ boolean isNewRecord = repository.findByUserIdAndKeyword(user.getId(), normalized).isEmpty(); + repository.findByUserIdAndKeyword(user.getId(), normalized) .ifPresentOrElse( rs -> rs.touch(now), () -> repository.save( new RecentSearch(user.getId(), normalized, now) ) ); - // 최대 개수 초과 시 오래된 것 삭제 - List<RecentSearch> list = - repository.findByUserIdOrderByUpdatedAtDesc( - user.getId(), - PageRequest.of(0, MAX_RECENT_SEARCH + 1) - ); - - if (list.size() > MAX_RECENT_SEARCH) { - repository.deleteAll(list.subList(MAX_RECENT_SEARCH, list.size())); - } + // 새 레코드가 추가된 경우에만 정리 + if (isNewRecord) { + List<RecentSearch> list = + repository.findByUserIdOrderByUpdatedAtDesc( + user.getId(), + PageRequest.of(0, MAX_RECENT_SEARCH + 1) + ); + + if (list.size() > MAX_RECENT_SEARCH) { + repository.deleteAll(list.subList(MAX_RECENT_SEARCH, list.size())); + } + }
61-71: 삭제 작업 결과 확인삭제 메서드들이 실제로 레코드를 삭제했는지 확인하지 않습니다. 리포지토리 메서드의 반환값(삭제된 레코드 수)을 확인하면 사용자에게 더 명확한 피드백을 제공할 수 있습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.javasrc/main/java/ku_rum/backend/domain/notice/presentation/NoticeController.javasrc/main/java/ku_rum/backend/domain/search/application/RecentSearchService.javasrc/main/java/ku_rum/backend/domain/search/domain/RecentSearch.javasrc/main/java/ku_rum/backend/domain/search/domain/repository/RecentSearchRepository.javasrc/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.javasrc/test/java/ku_rum/backend/domain/search/presentation/RecentSearchControllerTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/ku_rum/backend/domain/search/application/RecentSearchService.java (1)
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.java (1)
Service(29-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/java/ku_rum/backend/domain/notice/presentation/NoticeController.java (1)
56-57: 코드 변경사항 확인 완료공백 라인 추가만 있는 포맷팅 변경으로 기능에 영향이 없습니다.
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.java (1)
12-12: 최근 검색어 저장 기능 통합 확인
RecentSearchService를 통해 검색 키워드를 저장하는 로직이 올바르게 추가되었습니다.save메서드가 자체 트랜잭션을 가지고 있어 트랜잭션 경계도 적절합니다.Also applies to: 40-40, 89-90
src/test/java/ku_rum/backend/domain/search/presentation/RecentSearchControllerTest.java (1)
50-149: 테스트 커버리지 우수모든 컨트롤러 엔드포인트에 대한 테스트가 잘 작성되었습니다. REST Docs 통합과 적절한 모킹이 적용되어 있습니다.
src/main/java/ku_rum/backend/domain/search/application/RecentSearchService.java
Outdated
Show resolved
Hide resolved
src/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.java
Show resolved
Hide resolved
Test Results 38 files 38 suites 12s ⏱️ Results for commit 3760223. ♻️ This comment has been updated with latest results. |
|
|
||
| @Transactional | ||
| public void save(String keyword) { | ||
| String normalized = keyword == null ? "" : keyword.trim(); |
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.
? 연산자 지양해봐요
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.
private으로 뺐습니다!
| import java.util.List; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api/v1/recent-searches") |
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.
엔드포인트 prefix가 적합하지 않아보입니다
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.
/api/v1/notices/searches/recent 로 수정했습니다.
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: 0
♻️ Duplicate comments (2)
src/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.java (2)
19-20:limit파라미터 검증 미반영이전 리뷰에서 지적된
limit파라미터의 입력 검증이 아직 추가되지 않았습니다. 음수, 0, 또는 매우 큰 값에 대한 검증을 추가해주세요.
11-14: 입력 검증을 위한@Validated어노테이션 추가 필요컨트롤러에서 요청 파라미터 검증을 수행하려면 클래스 레벨에
@Validated어노테이션이 필요합니다. 이전 리뷰에서 제안된limit파라미터 검증과 아래에서 제안하는id파라미터 검증이 동작하려면 반드시 추가되어야 합니다.🔎 제안하는 수정사항
+import org.springframework.validation.annotation.Validated; + @RestController @RequestMapping("/api/v1/notices/searches/recent") @RequiredArgsConstructor +@Validated public class RecentSearchController {
🧹 Nitpick comments (2)
src/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.java (2)
24-28:id파라미터 검증 추가 권장
id파라미터에 대한 검증이 없습니다. 음수 또는 0과 같은 유효하지 않은 값이 전달될 수 있습니다. 컨트롤러 레벨에서 입력 검증을 추가하는 것이 좋습니다.🔎 제안하는 수정사항
+import jakarta.validation.constraints.Positive; + @DeleteMapping("/{id}") - public BaseResponse<Void> delete(@PathVariable Long id) { + public BaseResponse<Void> delete(@PathVariable @Positive Long id) { recentSearchService.delete(id); return BaseResponse.ok(null); }
12-12: 엔드포인트 경로 재검토이전 리뷰에서 david-parkk님이 지적하신 엔드포인트 prefix 관련 의견이 반영되지 않았습니다. 현재 경로
/api/v1/notices/searches/recent가 적합한지 팀 내에서 논의 후 결정해주세요.대안 예시:
/api/v1/notices/recent-searches/api/v1/recent-searches(공지사항 특화가 아닌 경우)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ku_rum/backend/domain/search/application/RecentSearchService.javasrc/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ku_rum/backend/domain/search/application/RecentSearchService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.java (1)
30-34:deleteAll엔드포인트는 안전하게 구현되어 있습니다
repository.deleteByUserId(user.getId())를 통해 현재 사용자의 최근 검색 기록만 삭제하며, SecurityConfig에서 전역적으로 인증을 필수로 요구하므로 사용자별 격리가 제대로 처리됩니다. 관리자 권한 체크는 사용자 개별 데이터 삭제 작업이므로 불필요합니다.
📝작업 내용
Summary by CodeRabbit
새로운 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.