Conversation
|
""" Walkthrough책 검색 API에 최근검색어 저장 기능이 추가되었습니다. 컨트롤러와 서비스, 포트, 어댑터 계층이 모두 확장되어 검색 시 사용자 ID와 검색어, 검색 타입이 저장됩니다. 또한 상세 책 검색 API가 신설되었고, 관련 테스트 코드가 보강되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller as BookQueryController
participant Service as BookSearchService
participant RecentSearchPort as RecentSearchCommandPort
participant UserRepo as UserJpaRepository
User->>Controller: GET /books?keyword=... (Authorization)
Controller->>Service: searchBooks(keyword, page, userId)
Service->>UserRepo: findById(userId)
UserRepo-->>Service: UserJpaEntity
Service->>RecentSearchPort: save(userId, keyword, SearchType.BOOK)
Service-->>Controller: NaverBookParseResult
Controller-->>User: 검색 결과 반환
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (13)
src/main/java/konkuk/thip/common/security/util/JwtUtil.java (1)
26-26: 환경 변수 설정을 빠르게 적용해야 합니다.TODO 주석에 명시된 대로 토큰 만료 시간을 환경 변수로 설정하여 환경별로 다른 값을 사용할 수 있도록 개선이 필요합니다.
환경 변수 설정 구현을 도와드릴까요?
src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java (1)
18-32: 중복 제거 로직 최적화를 고려해보세요.QueryDSL의 distinct()와 HashSet 생성자를 동시에 사용하고 있습니다. 성능 최적화를 위해 다음 중 하나를 선택하는 것을 권장합니다:
- QueryDSL distinct()만 사용하고 List로 반환받기
- HashSet을 사용할 경우 distinct() 제거하기
옵션 1: QueryDSL distinct()만 사용
-return new HashSet<>( - jpaQueryFactory - .select(userRoom.userJpaEntity.userId) - .distinct() - .from(userRoom) - .join(userRoom.roomJpaEntity, room) - .where(room.bookJpaEntity.bookId.eq(bookId)) - .fetch() -); +List<Long> userIds = jpaQueryFactory + .select(userRoom.userJpaEntity.userId) + .distinct() + .from(userRoom) + .join(userRoom.roomJpaEntity, room) + .where(room.bookJpaEntity.bookId.eq(bookId)) + .fetch(); +return new HashSet<>(userIds);옵션 2: HashSet으로 중복 제거
-return new HashSet<>( - jpaQueryFactory - .select(userRoom.userJpaEntity.userId) - .distinct() - .from(userRoom) - .join(userRoom.roomJpaEntity, room) - .where(room.bookJpaEntity.bookId.eq(bookId)) - .fetch() -); +return new HashSet<>( + jpaQueryFactory + .select(userRoom.userJpaEntity.userId) + .from(userRoom) + .join(userRoom.roomJpaEntity, room) + .where(room.bookJpaEntity.bookId.eq(bookId)) + .fetch() +);src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepositoryImpl.java (1)
17-29: QueryDSL을 사용한 구현이 올바르게 작성되었습니다.메서드 로직이 명확하고
distinct()를 사용하여 중복 제거가 적절히 처리되었습니다. 다만bookId파라미터에 대한 null 체크를 고려해보세요.null 안전성을 위한 개선안:
@Override public Set<Long> findUserIdsByBookId(Long bookId) { + if (bookId == null) { + return new HashSet<>(); + } QFeedJpaEntity feed = QFeedJpaEntity.feedJpaEntity; Set<Long> userIds = new HashSet<>( jpaQueryFactory .select(feed.userJpaEntity.userId) .distinct() .from(feed) .where(feed.bookJpaEntity.bookId.eq(bookId)) .fetch() ); return userIds; }src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java (1)
24-37: 입력 파라미터 검증을 추가하는 것을 고려해보세요.
keyword가 null이거나 빈 문자열인지,searchType이 null인지 검증하는 로직을 추가하면 더 견고한 코드가 될 것입니다.@Override public void save(Long userId, String keyword, SearchType searchType) { + if (userId == null || keyword == null || keyword.trim().isEmpty() || searchType == null) { + throw new BusinessException(INVALID_INPUT_VALUE); + } UserJpaEntity userJpaEntity = userJpaRepository.findById(userId) .orElseThrow(() -> new EntityNotFoundException(ALIAS_NOT_FOUND));src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)
115-141: 제약 조건 위반 예외 처리가 적절히 구현되었습니다.
@Size와@Pattern어노테이션에 대한 구체적인 처리와 로깅이 잘 구현되어 있습니다. 다만 여러 위반 사항이 있을 때 첫 번째만 처리하는 점을 개선해볼 수 있습니다.여러 위반 사항을 처리하는 개선안:
@ExceptionHandler(ConstraintViolationException.class) public ResponseEntity<ErrorResponse> handleConstraintViolationException(ConstraintViolationException e) { - // 여러 위반 중 첫 번째만 처리 ConstraintViolation<?> violation = e.getConstraintViolations().stream() .findFirst() .orElse(null); if (violation != null) { Class<?> annotationType = violation.getConstraintDescriptor().getAnnotation().annotationType(); log.error("[ConstraintViolationException] {}", e.getMessage()); + String violationMessage = violation.getMessage(); if (annotationType == Size.class) { return ResponseEntity .status(API_INVALID_SIZE.getHttpStatus()) - .body(ErrorResponse.of(API_INVALID_SIZE)); + .body(ErrorResponse.of(API_INVALID_SIZE, violationMessage)); } else if (annotationType == Pattern.class) { return ResponseEntity .status(API_INVALID_PATTERN.getHttpStatus()) - .body(ErrorResponse.of(API_INVALID_PATTERN)); + .body(ErrorResponse.of(API_INVALID_PATTERN, violationMessage)); } } return ResponseEntity .status(API_REQUEST_INVALID.getHttpStatus()) .body(ErrorResponse.of(API_REQUEST_INVALID)); }src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
54-54: 에러 코드 번호를 확인해주세요.
BOOK_NOT_FOUND의 에러 코드가800010으로 되어있는데, 다른 BOOK 관련 에러들과의 일관성을 고려하면80010이 맞는 것 같습니다.- BOOK_NOT_FOUND(HttpStatus.BAD_REQUEST, 800010, "존재하지 않는 BOOK 입니다."), + BOOK_NOT_FOUND(HttpStatus.BAD_REQUEST, 80010, "존재하지 않는 BOOK 입니다."),src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (1)
62-139: 테스트 데이터 설정이 복잡합니다.테스트마다 많은 엔티티를 생성하고 있어 테스트 가독성과 유지보수성이 떨어질 수 있습니다. 테스트 픽스처를 별도 클래스로 분리하거나 테스트 빌더 패턴을 고려해보세요.
// 예시: 테스트 데이터 빌더 클래스 분리 public class BookTestDataBuilder { public static BookDetailTestData createDefaultTestData() { // 테스트 데이터 생성 로직 } }src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (2)
31-31: ISBN 검증 패턴을 검토해주세요.현재 정규식
\\d{13}은 13자리 숫자만 허용하지만, 실제 ISBN-13은 하이픈(-)을 포함할 수 있습니다. 사용자 경험을 고려해 하이픈을 허용하거나, 입력 전처리에서 하이픈을 제거하는 것을 고려해보세요.- @Pattern(regexp = "\\d{13}") final String isbn, + @Pattern(regexp = "\\d{13}|\\d{3}-\\d{1}-\\d{3}-\\d{5}-\\d{1}") final String isbn,또는 컨트롤러에서 하이픈 제거 후 검증:
@Pattern(regexp = "\\d{13}") final String isbn = isbnRaw.replaceAll("-", "")
34-36: 불필요한 빈 줄을 제거해주세요.메서드 중간에 빈 줄이 여러 개 있어 코드 가독성이 떨어집니다.
- - - return BaseResponse.ok(GetBookDetailSearchResponse.of(bookSearchUseCase.searchDetailBooks(isbn,userId))); + return BaseResponse.ok(GetBookDetailSearchResponse.of(bookSearchUseCase.searchDetailBooks(isbn, userId)));src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (2)
37-43: null 파라미터 처리 개선 필요
detailSearchBook메서드에서start파라미터로 null을 전달하는 방식은 메서드 시그니처를 모호하게 만듭니다. 더 명확한 설계를 위해 다음 중 하나를 고려해보세요:
- 메서드 오버로딩을 통해 파라미터가 다른 별도의 메서드 생성
Optional<Integer>를 사용하여 null 가능성을 명시
53-59: URL 생성 방식 개선 고려현재 문자열 연결을 통한 URL 생성 방식은 동작하지만, 더 안전하고 유지보수가 쉬운 방법을 고려해보세요:
-private String buildSearchApiUrl(String query,Integer start) { - return bookSearchUrl+query+"&display="+PAGE_SIZE+"&start="+start; -} +private String buildSearchApiUrl(String query, Integer start) { + return String.format("%s%s&display=%d&start=%d", bookSearchUrl, query, PAGE_SIZE, start); +} -private String buildDetailSearchApiUrl(String query) { - return bookDetailSearchUrl+query; -} +private String buildDetailSearchApiUrl(String query) { + return String.format("%s%s", bookDetailSearchUrl, query); +}src/main/java/konkuk/thip/book/application/service/BookSearchService.java (2)
45-68: 트랜잭션 범위 재검토 필요현재 구조에서 최근 검색어 저장이 실패하면 전체 트랜잭션이 롤백되어 API 호출 결과도 손실될 수 있습니다. 최근 검색어 저장은 부가 기능이므로 실패해도 검색 결과는 반환되어야 합니다.
다음 방법을 고려해보세요:
- 최근 검색어 저장을 별도의 트랜잭션으로 분리
- 예외 발생 시 로그만 남기고 진행
- 이벤트 기반 비동기 처리
104-119: 읽기 참여 카운트 로직 구현 우수Set을 사용하여 중복 사용자를 제거하는 로직이 잘 구현되었습니다.
가독성을 위해 Stream API 사용을 고려해보세요:
Set<Long> combinedUsers = Stream.concat( roomParticipantUserIds.stream(), feedAuthorUserIds.stream() ).collect(Collectors.toSet());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/DummyResponse.java(0 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookDetailSearchResponse.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java(0 hunks)src/main/java/konkuk/thip/book/adapter/out/api/BookApiAdapter.java(0 hunks)src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java(3 hunks)src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverDetailBookParseResult.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java(0 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSearchUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookDetailSearchResult.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/DummyResult.java(0 hunks)src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java(1 hunks)src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSearchService.java(2 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(3 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(2 hunks)src/main/java/konkuk/thip/common/security/util/JwtUtil.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedJpaRepository.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepository.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java(1 hunks)src/main/java/konkuk/thip/recentSearch/adapter/out/jpa/SearchType.java(2 hunks)src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomJpaRepository.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomQueryPort.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookRepository.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedFeedJpaRepository.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserRoomJpaRepository.java(1 hunks)src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java(1 hunks)src/main/java/konkuk/thip/user/application/port/out/UserQueryPort.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java(1 hunks)
💤 Files with no reviewable changes (6)
- src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java
- src/main/java/konkuk/thip/book/adapter/out/api/BookApiAdapter.java
- src/main/java/konkuk/thip/book/application/port/in/dto/DummyResult.java
- src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java
- src/main/java/konkuk/thip/book/adapter/in/web/response/DummyResponse.java
- src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookRepository.java
🧰 Additional context used
🧠 Learnings (3)
src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
src/main/java/konkuk/thip/book/adapter/out/persistence/BookJpaRepository.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
🧬 Code Graph Analysis (3)
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (1)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java (1)
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java (1)
Repository(8-15)
src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java (2)
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
RequiredArgsConstructor(18-123)src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (1)
NaverBookXmlParser(16-124)
🔇 Additional comments (45)
src/main/java/konkuk/thip/user/adapter/out/persistence/UserRoomJpaRepository.java (1)
1-7: 깔끔한 JPA 리포지토리 구현입니다.표준 Spring Data JPA 패턴을 잘 따르고 있으며, 헥사고날 아키텍처의 패키지 구조도 적절합니다.
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedFeedJpaRepository.java (1)
6-6: 네이밍 일관성 개선이 좋습니다.다른 JPA 리포지토리들과 일관된 네이밍을 위해 "Jpa" 접미사를 추가한 것이 적절합니다.
src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java (2)
3-3: 필요한 import 추가가 적절합니다.Set 인터페이스 import가 새로운 메서드 반환 타입을 위해 적절히 추가되었습니다.
7-7: 메서드 시그니처가 명확하고 적절합니다.
findUserIdsByBookId메서드명이 기능을 명확히 표현하고,Set<Long>반환 타입이 중복 없는 사용자 ID 집합을 나타내기에 적합합니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepository.java (2)
3-3: Set import 추가가 적절합니다.새로운 메서드의 반환 타입을 위한 Set import가 올바르게 추가되었습니다.
5-7: 일관된 메서드 시그니처 설계입니다.다른 계층의 유사한 메서드들과 일관된 네이밍과 반환 타입을 사용하여 코드베이스의 일관성을 유지하고 있습니다.
src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java (1)
4-4: 도메인 import 추가가 적절합니다.Book 도메인 객체 import가 새로운 메서드 반환 타입을 위해 올바르게 추가되었습니다.
src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java (1)
8-8: 메서드 추가가 적절합니다.
findById메서드 추가가 표준 리포지토리 패턴을 따르고 있으며, 도메인 계층에서 사용자 조회 기능을 제공하기 위한 적절한 구현입니다.src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java (1)
4-4: 존재 여부 확인 메서드가 적절히 구현되었습니다.
existsByUserIdAndBookId메서드는 사용자가 특정 도서를 저장했는지 확인하는 기능을 제공하며, 메서드명과 반환 타입이 명확하고 적절합니다.src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java (1)
13-14: 리포지토리 네이밍 일관성 개선이 적절합니다.리포지토리 필드명에 "Jpa"를 포함하도록 변경하여 명명 일관성을 개선했습니다. 이는 코드의 명확성과 일관성을 높이는 좋은 리팩토링입니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepository.java (1)
1-7: 깔끔한 인터페이스 정의입니다.메서드 시그니처가 명확하고 반환 타입으로
Set<Long>을 사용하여 중복된 사용자 ID를 자동으로 제거하는 것이 좋습니다.src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java (1)
17-20: 메서드 구현이 올바릅니다.
findUserIdsByBookId메서드가 적절하게 repository에 위임하고 있으며, 메서드 시그니처가 인터페이스와 일치합니다.src/main/java/konkuk/thip/book/adapter/out/persistence/BookJpaRepository.java (1)
6-10: Spring Data JPA 명명 규칙을 잘 따른 구현입니다.
findByIsbn메서드가 Spring Data JPA의 명명 규칙을 따르고 있으며,Optional<BookJpaEntity>반환 타입을 사용하여 null 안전성을 확보한 것이 좋습니다. 학습된 내용에 따르면 이러한 패턴이 권장되는 방식입니다.src/main/java/konkuk/thip/recentSearch/adapter/out/jpa/SearchType.java (2)
4-8: 적절한 import 문 추가입니다.
BusinessException과ErrorCode를 import하여 명시적인 예외 처리를 위한 준비가 잘 되어 있습니다.
21-28: 예외 처리 개선이 훌륭합니다.이전에
null을 반환하던 방식에서BusinessException을 던지는 방식으로 변경하여 더 명확하고 안전한 에러 처리가 되었습니다.INVALID_SEARCH_TYPE에러 코드를 사용하여 구체적인 오류 정보를 제공하는 것도 좋습니다.src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java (1)
4-8: 명확한 포트 인터페이스 정의입니다.
save메서드의 파라미터가 명확하고 적절하며, 최근 검색어 저장 기능을 위한 포트 인터페이스로서 역할을 잘 정의하고 있습니다.SearchTypeenum을 사용하여 타입 안전성도 확보했습니다.src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java (1)
51-53: RoomJpaEntity 관계 변경 영향 검토 완료빌더 및 매퍼(RoomMapper, TagMapper), 테스트(TestEntityFactory, BookDetailSearchControllerTest) 등에서
categoryJpaEntity주입이 일관되게 반영되었습니다.추가로 확인이 필요한 사항:
- 데이터베이스 마이그레이션 스크립트(DDL) 적용 여부
- 비즈니스 로직에서 여러 Room이 동일 Category를 참조하는 시나리오 검증
src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java (1)
34-36: JPA 관계 변경 확인:aliasForUserJpaEntity는@ManyToOne이 의도된 설계입니다.
- Alias는 사전 정의된 옵션으로 여러 사용자가 동일한 Alias를 공유할 수 있도록 설계되어 있습니다.
- 테스트(TestEntityFactory·각종 Controller·Repository)와 비즈니스 로직 전반에서 Alias를 공유하는 패턴을 사용하고 있으며, 변경 내용이 일관됩니다.
별도 수정이 필요 없습니다.
src/main/java/konkuk/thip/user/application/port/out/UserQueryPort.java (1)
3-7: 새로운 쿼리 메서드 추가가 적절합니다.책 ID를 기반으로 관련 방에 참여한 사용자 ID를 조회하는 메서드가 잘 설계되었습니다. 메서드명이 명확하고 반환 타입(
Set<Long>)도 적절합니다.src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedJpaRepository.java (1)
6-6: FeedQueryRepository 구현체 확인 완료 및 승인FeedJpaRepository가 JpaRepository와 FeedQueryRepository를 올바르게 확장하고 있으며,
FeedQueryRepository 인터페이스와 그 구현체(FeedQueryRepositoryImpl)의findUserIdsByBookId메서드가 정상적으로 정의되어 있습니다.
추가 작업 없이 승인합니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryPersistenceAdapter.java (1)
8-8: 필요한 import가 적절히 추가되었습니다.새로운 메서드의 반환 타입을 위한
Setimport가 적절히 추가되었습니다.src/main/java/konkuk/thip/room/adapter/out/persistence/RoomJpaRepository.java (1)
9-9: 메서드 이름이 길지만 Spring Data JPA 규칙을 올바르게 따르고 있습니다.
countByBookJpaEntity_BookIdAndStartDateAfter메서드는 Spring Data JPA의 쿼리 메서드 명명 규칙을 정확히 따르고 있으며, 중첩된 프로퍼티 접근을 통해 책 ID와 시작 날짜 조건으로 방 개수를 카운트하는 기능을 제공합니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java (1)
33-39: 일관된 패턴으로 잘 구현된 메서드입니다.
findById메서드는 기존save메서드와 동일한 패턴을 따르고 있으며, 예외 처리와 엔티티 매핑이 일관되게 구현되어 있습니다.USER_NOT_FOUND오류 코드를 사용한 예외 처리도 적절합니다.src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (2)
17-18: 필드명 변경으로 코드 가독성이 향상되었습니다.
jpaRepository를bookJpaRepository로,userMapper를bookMapper로 변경하여 각 필드의 역할이 더 명확해졌습니다.
20-28: 일관된 패턴으로 구현된 ISBN 조회 메서드입니다.
findByIsbn메서드는UserCommandPersistenceAdapter의findById메서드와 동일한 패턴을 따르고 있으며, 예외 처리와 도메인 엔티티 매핑이 올바르게 구현되어 있습니다.src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverDetailBookParseResult.java (1)
6-15: 상세 도서 정보를 위한 적절한 레코드 클래스입니다.
@Builder어노테이션과 함께 도서의 상세 정보를 담는 레코드 클래스로 적절하게 설계되었습니다. 필드들은 도서의 핵심 메타데이터를 잘 표현하고 있습니다.src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java (2)
6-6: 인터페이스명 변경으로 목적이 더 명확해졌습니다.
SearchBookQueryPort에서BookApiQueryPort로 변경하여 도서 API 쿼리 포트라는 목적이 더 명확하게 표현되었습니다.
8-8: 일관된 명명 패턴을 따르는 새로운 메서드입니다.
findDetailBookByKeyword메서드는 기존findBooksByKeyword메서드와 일관된 명명 패턴을 따르고 있으며, ISBN을 통한 상세 도서 검색 기능을 제공합니다.src/main/java/konkuk/thip/room/application/port/out/RoomQueryPort.java (1)
3-6: 메서드 추가가 적절합니다.새로운 쿼리 메서드가 명확한 네이밍과 적절한 매개변수로 잘 정의되었습니다. LocalDate import도 함께 추가되어 일관성이 유지되었습니다.
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookJpaRepository.java (1)
1-8: JPA 리포지토리 인터페이스가 올바르게 구현되었습니다.Spring Data JPA의 쿼리 유도 메커니즘을 활용한 메서드명이 적절하며, 중첩된 엔티티 속성 접근도 정확합니다.
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java (2)
13-14: 리포지토리 필드명 변경이 적절합니다.새로운 JPA 리포지토리 인터페이스와 일치하도록 필드명이 올바르게 업데이트되었습니다.
18-21: 새로운 메서드 구현이 올바릅니다.existsByUserIdAndBookId 메서드가 적절하게 구현되었으며, 리포지토리 메서드로의 위임이 명확합니다.
src/main/java/konkuk/thip/book/application/port/in/BookSearchUseCase.java (2)
9-9: 새로운 상세 검색 메서드가 적절히 추가되었습니다.searchDetailBooks 메서드가 명확한 매개변수와 반환 타입으로 잘 정의되었습니다.
8-8: searchBooks 호출부 시그니처 업데이트 완료 확인
BookSearchService와 BookQueryController에서 모두userId매개변수를 포함해 호출부가 새로운 시그니처와 일치함을 확인했습니다. 별도 추가 수정은 필요 없습니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java (1)
3-6: 필요한 import와 애노테이션이 적절히 추가되었습니다.QueryDSL 사용을 위한 import들과 생성자 주입을 위한 @requiredargsconstructor가 올바르게 추가되었습니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java (2)
14-15: 필드명 변경이 코드 가독성을 향상시킵니다.
jpaRepository를roomJpaRepository로,userMapper를roomMapper로 변경한 것이 코드의 명확성을 높여줍니다.
17-20: 메서드 구현이 올바르게 작성되었습니다.새로 추가된
countRecruitingRoomsByBookAndStartDateAfter메서드가 포트 인터페이스를 올바르게 구현하고 있으며, 리포지토리로의 위임이 적절합니다.src/main/java/konkuk/thip/book/application/port/in/dto/BookDetailSearchResult.java (1)
6-24: Record 클래스 구현이 적절합니다.불변 데이터 구조에 record를 사용한 것이 적절하며, 정적 팩토리 메서드가 편의성을 제공합니다. 다만 팩토리 메서드가 단순히 생성자를 호출하는 것 외에 추가 로직이 없어 약간 중복적이지만, 향후 유연성을 위해서는 유용할 수 있습니다.
src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java (1)
11-27: 네이버 API 어댑터 구현이 잘 되어있습니다.포트-어댑터 패턴을 올바르게 따르고 있으며, 관심사의 분리가 잘 되어있습니다. 일반 검색과 상세 검색 기능을 명확히 구분하여 구현했고, 실제 API 호출과 파싱 로직은 적절히 유틸리티 클래스에 위임하고 있습니다.
src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookDetailSearchResponse.java (1)
6-32: 응답 DTO 구현이 깔끔합니다.Java 레코드를 활용한 불변 데이터 구조로 잘 설계되었고, 정적 팩토리 메서드를 통해 도메인 DTO에서 응답 DTO로의 변환이 명확합니다. 필드명도 직관적이고 필요한 모든 상세 정보를 포함하고 있습니다.
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
18-20: 새로운 에러 코드 추가가 적절합니다.API 유효성 검증, 사용자, 도서, 최근 검색 관련 에러 코드들이 체계적으로 추가되었고, 일관된 네이밍 패턴과 적절한 HTTP 상태 코드를 사용하고 있습니다.
Also applies to: 39-39, 52-54, 61-61
src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (1)
141-210: 테스트 시나리오 커버리지가 우수합니다.다양한 엣지 케이스들(모집 중인 방 없음, 피드/참여자 없음, 저장되지 않은 책)을 체계적으로 테스트하고 있어 품질이 높습니다.
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (1)
13-13: 컨트롤러 개선사항이 잘 적용되었습니다.유효성 검증 애노테이션 추가, 사용자 컨텍스트 통합, 새로운 상세 검색 엔드포인트 추가가 적절히 구현되었습니다. API 설계도 RESTful하게 잘 되어있습니다.
Also applies to: 24-26, 29-37
src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (2)
91-95: XML 보안 설정 적용 우수XXE(XML External Entity) 공격을 방지하기 위한 보안 설정이 적절히 구현되었습니다.
90-114: 헬퍼 메서드 추출 리팩토링 우수공통 로직을 별도 메서드로 추출하여 코드 재사용성과 가독성이 향상되었습니다.
| public interface BookCommandPort { | ||
|
|
||
| } | ||
| Book findByIsbn(String isbn); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
아키텍처 관점에서 검토가 필요합니다.
findByIsbn 메서드는 조회 기능이지만 BookCommandPort에 위치하고 있습니다. CQRS 원칙을 따르고 있다면, 이 메서드는 BookQueryPort에 있는 것이 더 적절할 수 있습니다.
Command Port는 일반적으로 쓰기 작업을, Query Port는 읽기 작업을 담당합니다.
- BookCommandPort에서 제거하고 BookQueryPort로 이동을 고려해보세요:// BookQueryPort.java에 추가
Book findByIsbn(String isbn);🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java at
line 8, the findByIsbn method is a read operation but is currently placed in the
Command Port interface. To follow CQRS principles, remove the findByIsbn method
from BookCommandPort and add it to the BookQueryPort interface instead, ensuring
that Command Port handles only write operations and Query Port handles read
operations.
There was a problem hiding this comment.
🛠️ Refactor suggestion
아키텍처 관점에서 검토가 필요합니다.
findByIsbn메서드는 조회 기능이지만BookCommandPort에 위치하고 있습니다. CQRS 원칙을 따르고 있다면, 이 메서드는BookQueryPort에 있는 것이 더 적절할 수 있습니다.Command Port는 일반적으로 쓰기 작업을, Query Port는 읽기 작업을 담당합니다.
- BookCommandPort에서 제거하고 BookQueryPort로 이동을 고려해보세요:// BookQueryPort.java에 추가 Book findByIsbn(String isbn);🤖 Prompt for AI Agents
팀 내부적으로 findByXXX 를 통해 도메인 엔티티를 찾아오는 코드는 commandPort 하위에 메서드로 추가하고, 조회 api의 response 에 해당하는 데이터들을 DB로부터 조회하는 코드는 queryPort 하위에 메서드로 추가하기로 정했어.
이런 구조는 어때?
There was a problem hiding this comment.
@seongjunnoh 팀의 아키텍처 결정을 이해했습니다!
이런 구조는 매우 합리적입니다. 전통적인 CQRS의 Command/Query 분리와는 다르지만, 여러분의 컨텍스트에서는 더 실용적인 접근 방식이네요:
- CommandPort: 비즈니스 로직에서 사용할 도메인 엔티티를 조회하는 메서드들
- QueryPort: API 응답을 위한 데이터를 조회하는 메서드들
이렇게 구분하면 도메인 엔티티 조회와 API 응답 데이터 조회의 책임이 명확하게 분리되어 좋은 설계입니다. findByIsbn이 Book 도메인 엔티티를 반환하므로 BookCommandPort에 위치하는 것이 팀의 컨벤션에 맞습니다.
앞으로 리뷰할 때 이 컨벤션을 고려하겠습니다!
✏️ Learnings added
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.000Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| public interface UserJpaRepository extends JpaRepository<UserJpaEntity, Long>, UserQueryRepository { | ||
| Optional<UserJpaEntity> findByOauth2Id(String oauth2Id); | ||
| boolean existsByNickname(String nickname); | ||
| Optional<UserJpaEntity> findById(Long userId); |
There was a problem hiding this comment.
중복 메서드 선언을 제거해야 합니다.
JpaRepository는 이미 findById(ID id) 메서드를 제공하므로, 동일한 시그니처의 메서드를 다시 선언할 필요가 없습니다. 이는 중복 선언이며 혼란을 야기할 수 있습니다.
다음 변경사항을 적용하여 중복 메서드를 제거하세요:
- Optional<UserJpaEntity> findById(Long userId);🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java
at line 11, remove the redundant declaration of the findById(Long userId) method
because JpaRepository already provides this method. Simply delete this line to
avoid duplicate method declarations and potential confusion.
| private final long tokenExpiredMs = 2592000000L; // 30일 | ||
| private final long signupTokenExpiredMs = 2592000000L; // 30일 |
There was a problem hiding this comment.
JWT 토큰 만료 시간 설정에 보안 문제가 있습니다.
액세스 토큰과 가입 토큰 모두 30일로 설정하는 것은 심각한 보안 위험을 초래합니다:
- 액세스 토큰 30일: 토큰이 탈취될 경우 30일간 악용 가능
- 가입 토큰 30일: 가입 토큰은 짧은 유효기간을 가져야 함
- 보안 모범 사례 위반: 일반적으로 액세스 토큰은 15분~1시간, 가입 토큰은 5-10분 권장
다음과 같은 개선사항을 권장합니다:
- private final long tokenExpiredMs = 2592000000L; // 30일
- private final long signupTokenExpiredMs = 2592000000L; // 30일
+ private final long tokenExpiredMs = 3600000L; // 1시간
+ private final long signupTokenExpiredMs = 300000L; // 5분또한 리프레시 토큰 메커니즘을 구현하여 사용자 경험을 유지하면서 보안을 강화하시기 바랍니다.
📝 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 long tokenExpiredMs = 2592000000L; // 30일 | |
| private final long signupTokenExpiredMs = 2592000000L; // 30일 | |
| private final long tokenExpiredMs = 3600000L; // 1시간 | |
| private final long signupTokenExpiredMs = 300000L; // 5분 |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/util/JwtUtil.java around lines 27
to 28, the token expiration times for both access and signup tokens are set to
30 days, which is a security risk. Change the access token expiration to a short
duration like 15 minutes to 1 hour, and set the signup token expiration to a
very short period such as 5 to 10 minutes. Additionally, implement a refresh
token mechanism to maintain user experience while enhancing security.
| private final FeedJpaRepository feedJpaRepository; | ||
| private final FeedMapper feedMapper; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
필드명 변경에 대한 일관성을 확인해 주세요.
이 클래스에서 jpaRepository를 feedJpaRepository로, userMapper를 feedMapper로 변경했지만, 관련 코드 스니펫을 보면 FeedCommandPersistenceAdapter에서는 여전히 jpaRepository와 userMapper를 사용하고 있습니다. 코드베이스 전체에서 일관된 네이밍을 유지하는 것이 좋겠습니다.
다음 스크립트로 Feed 관련 어댑터들의 필드명 일관성을 확인해 주세요:
🏁 Script executed:
#!/bin/bash
# Description: Feed 관련 어댑터들의 필드명 일관성 확인
# Expected: 모든 Feed 어댑터에서 일관된 필드명 사용
# Feed 관련 어댑터들의 필드 선언 패턴 검색
rg -A 5 "class.*Feed.*Adapter" --type java
rg "private final.*jpaRepository|private final.*feedJpaRepository" src/main/java/konkuk/thip/feed/
rg "private final.*userMapper|private final.*feedMapper" src/main/java/konkuk/thip/feed/Length of output: 2199
필드명 일관성 유지: Feed 어댑터 전반에서 동일한 네이밍 적용 필요
현재 두 어댑터의 필드명이 서로 다르게 사용되고 있습니다.
- FeedCommandPersistenceAdapter.java
- private final FeedJpaRepository jpaRepository;
- private final FeedMapper userMapper;
- FeedQueryPersistenceAdapter.java
- private final FeedJpaRepository feedJpaRepository;
- private final FeedMapper feedMapper;
모두 feedJpaRepository와 feedMapper로 통일하여 네이밍 일관성을 유지해주세요.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java
around lines 14 to 15, ensure the field names for FeedJpaRepository and
FeedMapper are consistent with the rest of the Feed adapters by renaming them to
feedJpaRepository and feedMapper respectively. This maintains naming uniformity
across the codebase.
| @Override | ||
| public Set<Long> findUserIdsParticipatedInRoomsByBookId(Long bookId) { | ||
| return userJpaRepository.findUserIdsByBookId(bookId); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
새로운 메서드 구현이 적절합니다.
UserQueryPort 인터페이스의 새로운 메서드를 적절히 구현했습니다. Repository에 위임하는 단순한 구조로 잘 설계되었습니다.
Repository의 findUserIdsByBookId 메서드가 제대로 구현되었는지 확인해주세요:
🏁 Script executed:
#!/bin/bash
# Description: UserJpaRepository의 findUserIdsByBookId 메서드 확인
# Expected: 해당 메서드가 UserJpaRepository 또는 관련 클래스에 구현되어 있는지 확인
rg -A 10 -B 5 "findUserIdsByBookId" --type javaLength of output: 9746
🏁 Script executed:
#!/bin/bash
# UserJpaRepository 정의 위치 확인
rg -n "interface UserJpaRepository" --type java
rg -n "class UserJpaRepository" --type java
rg -n "UserJpaRepository" --type javaLength of output: 3299
UserQueryRepositoryImpl의 쿼리 실행 누락 수정 필요
UserQueryPort 인터페이스와 PersistenceAdapter의 메서드 위임 구조는 적절하지만, UserQueryRepositoryImpl.findUserIdsByBookId 구현부를 확인한 결과 .where(...) 이후에 .fetch() 호출이 빠져 있어 실제로 쿼리가 실행되지 않습니다. 아래와 같이 수정 부탁드립니다.
수정 위치:
- src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java
- @Override
- public Set<Long> findUserIdsByBookId(Long bookId) {
- return new HashSet<>(
- jpaQueryFactory
- .select(userRoom.userJpaEntity.userId)
- .distinct()
- .from(userRoom)
- .join(userRoom.roomJpaEntity, room)
- .where(room.bookJpaEntity.bookId.eq(bookId))
- );
- }
+ @Override
+ public Set<Long> findUserIdsByBookId(Long bookId) {
+ return new HashSet<>(
+ jpaQueryFactory
+ .select(userRoom.userJpaEntity.userId)
+ .distinct()
+ .from(userRoom)
+ .join(userRoom.roomJpaEntity, room)
+ .where(room.bookJpaEntity.bookId.eq(bookId))
+ .fetch()
+ );
+ }위와 같이 .fetch()를 호출하여 쿼리 결과를 가져오도록 수정한 뒤 재검토 부탁드립니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java
around lines where findUserIdsByBookId is implemented, the query builder chain
is missing a .fetch() call after the .where(...) clause, so the query is not
executed and no results are returned. Fix this by adding .fetch() at the end of
the query chain to properly execute the query and return the results.
.../konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java
Outdated
Show resolved
Hide resolved
| @SpringBootTest | ||
| @AutoConfigureMockMvc(addFilters = false) | ||
| @ActiveProfiles("test") | ||
| class BookDetailSearchControllerTest { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트가 컨트롤러가 아닌 서비스 계층을 직접 테스트하고 있습니다.
클래스명이 BookDetailSearchControllerTest이지만 실제로는 BookSearchService를 직접 호출하고 있습니다. 컨트롤러 테스트라면 MockMvc를 사용해서 HTTP 엔드포인트를 테스트하는 것이 적절합니다.
다음과 같이 실제 HTTP 엔드포인트를 테스트하도록 수정을 고려해보세요:
- var result = bookSearchService.searchDetailBooks(isbn, user.getUserId());
+ mockMvc.perform(get("/books/{isbn}", isbn)
+ .header("Authorization", "Bearer " + generateValidToken(user.getUserId())))
+ .andExpect(status().isOk())
+ .andExpect(jsonPath("$.data.recruitingRoomCount").value(1));Also applies to: 147-147, 181-181, 195-195, 207-207
🤖 Prompt for AI Agents
In
src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java
at lines 34, 147, 181, 195, and 207, the test class named
BookDetailSearchControllerTest is directly testing the service layer instead of
the controller. To fix this, refactor the tests to use MockMvc to perform HTTP
requests against the controller endpoints, verifying the controller's behavior
and response. Remove direct calls to the service layer in these tests and
replace them with HTTP request simulations to properly test the controller
layer.
src/main/java/konkuk/thip/book/application/service/BookSearchService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/konkuk/thip/book/adapter/in/web/BookQueryControllerTest.java (2)
51-75: 테스트 데이터 설정 로직 개선 필요현재 모든 테스트에서 최근 검색어 엔티티를 생성하고 있으나, 실제로 필요한 테스트는
searchBooks_savesRecentSearch하나뿐입니다.@BeforeEach void setUp() { AliasJpaEntity alias = aliasJpaRepository.save(AliasJpaEntity.builder() .value("책벌레") .color("blue") .imageUrl("http://image.url") .build()); UserJpaEntity user = userJpaRepository.save(UserJpaEntity.builder() .oauth2Id("kakao_432708231") .nickname("User1") .imageUrl("https://avatar1.jpg") .role(UserRole.USER) .aliasForUserJpaEntity(alias) .build()); - RecentSearchJpaEntity recentSearch = recentSearchJpaRepository.save(RecentSearchJpaEntity.builder() - .searchTerm("테스트검색어") - .type(SearchType.BOOK_SEARCH) - .userJpaEntity(user) - .build()); testToken = jwtUtil.createAccessToken(user.getUserId()); }최근 검색어 데이터는 해당 기능을 테스트하는 메서드에서만 생성하는 것이 더 적절합니다.
165-194: 최근 검색어 저장 테스트 로직 개선 제안테스트 로직이 작동하지만 몇 가지 개선사항이 있습니다:
findAll().get(0)사용은 위험할 수 있습니다- 필터링 로직을 단순화할 수 있습니다
@Test @DisplayName("책 검색 성공 시 최근검색어 저장") void searchBooks_savesRecentSearch() throws Exception { // given String keyword = "테스트"; int page = 1; // when mockMvc.perform(get("/books") .header("Authorization", "Bearer " + testToken) .param("keyword", keyword) .param("page", String.valueOf(page)) .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()); // then - UserJpaEntity user = userJpaRepository.findAll().get(0); - RecentSearchJpaEntity recentSearch = recentSearchJpaRepository.findAll().stream() - .filter(rs -> rs.getUserJpaEntity().getUserId().equals(user.getUserId())) - .filter(rs -> rs.getSearchTerm().equals(keyword)) - .findFirst() - .orElse(null); + UserJpaEntity user = userJpaRepository.findByOauth2Id("kakao_432708231") + .orElseThrow(() -> new AssertionError("테스트 사용자를 찾을 수 없습니다")); + + List<RecentSearchJpaEntity> recentSearches = recentSearchJpaRepository + .findByUserJpaEntityAndSearchTermAndType(user, keyword, SearchType.BOOK_SEARCH); - assertThat(recentSearch).isNotNull(); - assertThat(recentSearch.getSearchTerm()).isEqualTo(keyword); - assertThat(recentSearch.getType()).isEqualTo(SearchType.BOOK_SEARCH); - assertThat(recentSearch.getUserJpaEntity().getUserId()).isEqualTo(user.getUserId()); + assertThat(recentSearches).isNotEmpty(); + RecentSearchJpaEntity recentSearch = recentSearches.get(0); + assertThat(recentSearch.getSearchTerm()).isEqualTo(keyword); + assertThat(recentSearch.getType()).isEqualTo(SearchType.BOOK_SEARCH); }참고: 이 개선안은
RecentSearchJpaRepository에 적절한 쿼리 메서드가 있다고 가정합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/konkuk/thip/book/adapter/in/web/BookQueryControllerTest.java(5 hunks)
🔇 Additional comments (4)
src/test/java/konkuk/thip/book/adapter/in/web/BookQueryControllerTest.java (4)
30-30: 보안 필터 활성화 확인됨
@AutoConfigureMockMvc에서 기본 필터가 활성화되어 인증 테스트가 가능해졌습니다.
37-48: 테스트 의존성 주입이 적절합니다사용자 관리와 최근 검색어 기능 테스트를 위한 리포지토리와 JWT 유틸리티가 올바르게 주입되었습니다.
77-82: 데이터 정리 순서 확인 필요외래 키 제약 조건으로 인해 데이터 삭제 순서가 중요할 수 있습니다.
@AfterEach void tearDown() { recentSearchJpaRepository.deleteAll(); userJpaRepository.deleteAll(); aliasJpaRepository.deleteAll(); }현재 순서가 올바른지 확인하세요. 일반적으로 자식 엔티티(RecentSearch) → 부모 엔티티(User) → 참조 엔티티(Alias) 순으로 삭제해야 합니다.
87-96: 인증 헤더 추가가 올바르게 적용됨모든 기존 테스트에 Authorization 헤더가 일관성 있게 추가되어 인증이 필요한 API를 올바르게 테스트하고 있습니다.
Also applies to: 108-117, 129-138, 148-157
…-search-recent-search-add
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/book/application/service/BookSearchService.java (1)
84-94: Optional을 활용한 안전한 Book 엔티티 처리입니다.과거 리뷰 코멘트에서 지적된 Book 엔티티 존재 검증 이슈가
Optional을 사용하여 안전하게 해결되었습니다. Book이 존재하지 않을 경우 기본값을 반환하는 처리가 적절합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java(2 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSearchService.java(2 hunks)src/main/java/konkuk/thip/common/exception/ExternalApiException.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(3 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(3 hunks)src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationResult.java(1 hunks)src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationStrategy.java(1 hunks)src/main/java/konkuk/thip/common/exception/validation/PatternViolationStrategy.java(1 hunks)src/main/java/konkuk/thip/common/exception/validation/SizeViolationStrategy.java(1 hunks)src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationResult.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java
- src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java
🧰 Additional context used
🧠 Learnings (1)
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
🧬 Code Graph Analysis (2)
src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java (2)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchQueryPersistenceAdapter.java (1)
Repository(8-15)
src/main/java/konkuk/thip/common/exception/validation/SizeViolationStrategy.java (1)
src/main/java/konkuk/thip/common/exception/validation/PatternViolationStrategy.java (1)
Component(13-30)
🔇 Additional comments (11)
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (2)
15-16: 필드명 변경으로 코드 가독성이 향상되었습니다.
jpaRepository에서bookJpaRepository로,userMapper에서bookMapper로 변경하여 각 필드의 역할이 명확해졌습니다.
19-22: Optional 반환 타입을 사용한 안전한 구현입니다.
findByIsbn메서드가Optional<Book>을 반환하여 null 안전성을 보장하고, Spring Data JPA 패턴을 올바르게 따르고 있습니다. 도메인 엔티티로의 매핑도 적절합니다.src/main/java/konkuk/thip/common/exception/ExternalApiException.java (1)
5-17: 외부 API 예외 처리를 위한 적절한 구현입니다.
RuntimeException을 상속받아 외부 API 오류를 처리하는 전용 예외 클래스를 생성했습니다. 두 개의 생성자를 제공하여 예외 체이닝을 지원하고,ErrorCode를 통해 일관된 오류 코드 관리가 가능합니다.src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationStrategy.java (1)
7-10: 검증 제약 조건 처리를 위한 전략 패턴 구현이 우수합니다.
ConstraintViolationStrategy인터페이스가 전략 패턴을 적절히 구현하여 다양한 검증 어노테이션 타입별로 확장 가능한 처리를 가능하게 합니다. SOLID 원칙을 잘 따르고 있습니다.src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java (1)
24-37: 최근 검색어 저장 로직이 올바르게 구현되었습니다.사용자 존재 여부를 확인하고 적절한
USER_NOT_FOUND오류 코드를 사용하여 예외 처리하고 있습니다.RecentSearchJpaEntity를 빌더 패턴으로 생성하여 저장하는 로직이 깔끔합니다. 이전 리뷰에서 지적된 오류 코드 문제도 해결되었습니다.src/main/java/konkuk/thip/common/exception/validation/SizeViolationStrategy.java (1)
15-32: @SiZe 검증 제약 조건 처리 전략이 일관성 있게 구현되었습니다.
ConstraintViolationStrategy인터페이스를 올바르게 구현하여@Size어노테이션 위반에 대한 처리를 담당합니다. 한국어 오류 메시지와 적절한 HTTP 상태 코드를 사용하며, 다른 전략 클래스들과 일관된 패턴을 따르고 있습니다.src/main/java/konkuk/thip/common/exception/validation/PatternViolationStrategy.java (1)
1-30: 잘 구현된 검증 전략 클래스입니다.Pattern 제약 조건 위반을 처리하는 전략 클래스가 명확하고 간결하게 구현되어 있습니다. 전략 패턴을 올바르게 활용하여 확장 가능한 검증 오류 처리 메커니즘을 제공합니다.
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)
122-150: 전략 패턴을 활용한 우수한 예외 처리 구현입니다.ConstraintViolationException을 처리하는 새로운 핸들러가 전략 패턴을 활용하여 확장 가능하고 유지보수성이 높게 구현되었습니다. 적절한 폴백 처리도 포함되어 있어 안정성을 보장합니다.
src/main/java/konkuk/thip/book/application/service/BookSearchService.java (3)
46-47: @transactional 어노테이션 적용이 적절합니다.최근 검색어 저장 기능이 추가되면서
@Transactional어노테이션을 적용한 것이 적절합니다. 데이터 일관성을 보장할 수 있습니다.
67-70: 최근 검색어 저장 로직이 잘 구현되었습니다.사용자 검증 후 최근 검색어를 저장하는 로직이 명확하고 안전하게 구현되었습니다.
118-134: 중복 사용자 제거 로직이 효율적으로 구현되었습니다.
HashSet을 활용하여 방 참여자와 피드 작성자의 중복을 제거하는 로직이 명확하고 효율적입니다. 합집합 연산을 통해 정확한 사용자 수를 계산할 수 있습니다.
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
리뷰 하나 남겼습니다! 확인 부탁드립니다!
| //최근검색어 추가 | ||
| User user = userCommandPort.findById(userId); | ||
| recentSearchCommandPort.save(user.getId(),keyword,BOOK_SEARCH); |
There was a problem hiding this comment.
p2 : 최근 검색어를 추가할때 새로운(= id값이 null인) RecentSearch 도메인 엔티티를 생성한 후, 도메인 엔티티를 영속성 어댑터로 넘겨주어 jpa 엔티티로 매핑 후, DB에 save 하는 플로우는 어떤가요??
현재 코드처럼 원시값들을 바로 영속성 어댑터로 넘긴 후, 이 값들을 조합하여 바로 Jpa 엔티티를 생성하면
- 서비스의 책임 증가 (원시값들에 대한 의존성이 생기므로)
- 도메인 개념 약화 (최근 검색어 도메인 자체를 저장하는게 도메인 관점으로 맞는 것 같습니다!)
라는 문제점이 있을 것 같습니다!
추가로 port.save() 메서드의 파라미터를 id 값이 null인 도메인 엔티티로 통일하면 전체적인 코드의 가독성 또한 높아질 것 같습니다!
There was a problem hiding this comment.
앗 제가 이부분을 놓치고 구현한거 같네요 ㅎㅎ,, 수정하겠습니당
| @Test | ||
| @DisplayName("책 검색 성공 시 최근검색어 저장") | ||
| void searchBooks_savesRecentSearch() throws Exception { |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/konkuk/thip/book/application/service/BookSearchService.java(5 hunks)src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/book/application/service/BookSearchService.java
- src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.000Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.000Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java (1)
4-4: 도메인 객체 import 추가 확인
RecentSearch도메인 객체를 import하여 새로운 메서드 시그니처에서 사용할 수 있도록 했습니다.
| public interface RecentSearchCommandPort { | ||
| void save(Long userId, RecentSearch recentSearch); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
메서드 시그니처 개선으로 도메인 객체 활용
이전의 개별 파라미터들(String keyword, SearchType searchType)을 RecentSearch 도메인 객체로 캡슐화한 것은 좋은 설계 개선입니다. 이는 다음과 같은 이점을 제공합니다:
- 관련 데이터의 응집성 향상
- 인터페이스의 유지보수성 개선
- 도메인 주도 설계 원칙 준수
다만, userId 파라미터가 RecentSearch 객체와 함께 전달되는 설계가 적절한지 검토해보시기 바랍니다.
다음 스크립트로 RecentSearch 도메인 객체에 사용자 정보가 포함되어 있는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# RecentSearch 도메인 객체의 구조 확인
ast-grep --pattern $'class RecentSearch {
$$$
}'Length of output: 51
🏁 Script executed:
#!/bin/bash
# RecentSearch 도메인 객체 정의 위치 및 구조 확인
file=$(find . -type f -name "RecentSearch.java" | head -n 1)
if [ -z "$file" ]; then
echo "RecentSearch.java 파일을 찾을 수 없습니다."
else
echo "==> $file <=="
sed -n '1,200p' "$file"
fiLength of output: 785
메서드 시그니처 간소화 제안
RecentSearch 도메인 객체에 이미 userId 필드가 정의되어 있으므로, save 메서드에서 별도의 userId 파라미터 전달은 중복입니다. 다음과 같이 수정을 권장드립니다:
- RecentSearchCommandPort 인터페이스
public interface RecentSearchCommandPort { - void save(Long userId, RecentSearch recentSearch);
- void save(RecentSearch recentSearch);
}
- 호출부에서 RecentSearch.builder() 등을 통해 userId를 설정 후 전달
- 인터페이스 응집성·가독성 향상 및 중복 인자 제거
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
public interface RecentSearchCommandPort {
void save(RecentSearch recentSearch);
}
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java
at lines 6-7, simplify the save method signature by removing the separate userId
parameter since RecentSearch already contains userId. Modify the save method to
accept only a RecentSearch object, and ensure callers set the userId within
RecentSearch before calling save. This improves interface cohesion and
eliminates redundant parameters.
| //컨트롤러 어드바이스 추가하고 예외처리 | ||
| //throw new GlobalException(INVALID_SEARCH_TYPE); | ||
| return null; | ||
| throw new BusinessException(INVALID_SEARCH_TYPE); |
| //최근검색어 추가 | ||
| User user = userCommandPort.findById(userId); | ||
| RecentSearch recentSearch = RecentSearch.builder() | ||
| .searchTerm(keyword) | ||
| .type(BOOK_SEARCH.getSearchType()) | ||
| .userId(user.getId()) | ||
| .build(); | ||
|
|
||
|
|
||
| recentSearchCommandPort.save(user.getId(),recentSearch); | ||
|
|
There was a problem hiding this comment.
Adapter에서 이미 User 엔티티를 id로 조회하여 매핑하고 있어서 서비스 로직에서는 User 엔티티를 조회할 필요는 없을 것 같습니다~~
| void setUp() { | ||
|
|
||
| AliasJpaEntity alias = aliasJpaRepository.save(AliasJpaEntity.builder() | ||
| .value("책벌레") |
|
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
RecentSearchCommandPort를 이용해 영속성 엔티티에 접근하고 최근검색어를 저장합니다.📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
테스트