[feat] 저장한 책 또는 참여 중 모임의 책 조회 api 구현 및 패키지 정리#166
Conversation
Walkthrough저장된 책과 사용자가 참여 중인 모임방의 책을 조회하는 신규 API가 도입되었습니다. 이를 위해 도메인, 어댑터, 포트, 서비스 계층에 관련 기능이 추가 및 리팩토링되었으며, 불필요해진 saved 관련 코드(도메인, 포트, 어댑터, 서비스 등)는 대거 삭제되었습니다. 테스트 코드와 패키지 구조도 이에 맞게 정비되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BookQueryController
participant BookSelectableListUseCase
participant BookQueryPort
participant BookQueryMapper
Client->>BookQueryController: GET /books/selectable-list?type=SAVED|JOINING
BookQueryController->>BookSelectableListUseCase: getSelectableBookList(type, userId)
BookSelectableListUseCase->>BookQueryPort: findSavedBooksByUserId(userId) / findJoiningRoomsBooksByUserId(userId)
BookQueryPort-->>BookSelectableListUseCase: List<Book>
BookSelectableListUseCase->>BookQueryMapper: toBookInfoList(List<Book>)
BookQueryMapper-->>BookSelectableListUseCase: List<BookSelectableResult>
BookSelectableListUseCase-->>BookQueryController: List<BookSelectableResult>
BookQueryController-->>Client: BaseResponse<BookSelectableListResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 3
🧹 Nitpick comments (9)
src/main/java/konkuk/thip/book/application/port/in/dto/BookInfo.java (1)
3-10: 필드 검증 및 null 안전성 고려 필요BookInfo record가 잘 설계되었지만 다음 사항들을 고려해보세요:
- ISBN 필드에 대한 형식 검증 (표준화된 식별자)
- null 가능한 필드들에 대한 문서화
- 필수 필드들에 대한 validation 어노테이션
다음과 같이 개선할 수 있습니다:
+import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; + +/** + * 책 정보를 담는 DTO + * @param bookId 책 ID (필수) + * @param bookTitle 책 제목 (필수) + * @param authorName 저자명 (필수) + * @param publisher 출판사 (선택) + * @param bookImageUrl 책 이미지 URL (선택) + * @param isbn ISBN (필수, 10자리 또는 13자리) + */ public record BookInfo( + @NotBlank Long bookId, + @NotBlank String bookTitle, + @NotBlank String authorName, String publisher, String bookImageUrl, + @Pattern(regexp = "^(?:ISBN(?:-1[03])?:? )?(?=[0-9X]{10}$|(?=(?:[0-9]+[- ]){3})[- 0-9X]{13}$|97[89][0-9]{10}$|(?=(?:[0-9]+[- ]){4})[- 0-9]{17}$)(?:97[89][- ]?)?[0-9]{1,5}[- ]?[0-9]+[- ]?[0-9]+[- ]?[0-9X]$") String isbn ) {src/main/java/konkuk/thip/book/adapter/out/persistence/repository/SavedBookJpaRepository.java (1)
9-15: JPQL 쿼리가 올바르게 작성되었지만 Spring Data JPA 메서드 명명 규칙을 고려해보세요.현재 구현은 정확하게 작동하지만, Spring Data JPA의 메서드 명명 규칙을 사용하면 더 간결하게 구현할 수 있습니다.
다음과 같이 단순화할 수 있습니다:
- @Query("SELECT CASE WHEN COUNT(s) > 0 THEN true ELSE false END FROM SavedBookJpaEntity s " + - "WHERE s.userJpaEntity.userId = :userId AND s.bookJpaEntity.bookId = :bookId") - boolean existsByUserIdAndBookId(Long userId, Long bookId); + boolean existsByUserJpaEntity_UserIdAndBookJpaEntity_BookId(Long userId, Long bookId); - @Modifying - @Query("DELETE FROM SavedBookJpaEntity s WHERE s.userJpaEntity.userId = :userId AND s.bookJpaEntity.bookId = :bookId") - void deleteByUserIdAndBookId(Long userId, Long bookId); + @Modifying + void deleteByUserJpaEntity_UserIdAndBookJpaEntity_BookId(Long userId, Long bookId);이렇게 하면 JPQL을 수동으로 작성할 필요 없이 Spring Data JPA가 자동으로 쿼리를 생성합니다.
src/main/java/konkuk/thip/book/application/service/BookMostSearchRankService.java (1)
55-55: AtomicInteger 사용이 적절한지 검토해보세요.
@Scheduled메서드는 기본적으로 단일 스레드에서 실행되므로,AtomicInteger대신 일반int를 사용해도 충분할 수 있습니다. 하지만 코드 의도를 명확하게 표현하는 측면에서는 현재 구현이 더 명확합니다.src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java (1)
6-6: 와일드카드 import 사용 검토 필요와일드카드 import는 네임스페이스 충돌 가능성과 코드 가독성 저하를 야기할 수 있습니다. 명시적인 import 사용을 고려해보세요.
-import konkuk.thip.feed.adapter.out.jpa.*; +import konkuk.thip.feed.adapter.out.jpa.ContentJpaEntity; +import konkuk.thip.feed.adapter.out.jpa.FeedJpaEntity; +import konkuk.thip.feed.adapter.out.jpa.FeedTagJpaEntity; +import konkuk.thip.feed.adapter.out.jpa.SavedFeedJpaEntity; +import konkuk.thip.feed.adapter.out.jpa.TagJpaEntity;src/test/java/konkuk/thip/book/adapter/in/web/BookGetSelectableListApiTest.java (1)
68-90: 엣지 케이스 테스트 추가 권장현재 테스트는 정상 케이스만 다루고 있습니다. 다음과 같은 추가 테스트 케이스를 고려해보세요:
- 저장된 책이나 참여 중인 모임이 없는 경우 (빈 리스트 반환)
- 잘못된 type 파라미터 처리
- 인증되지 않은 사용자 요청 처리
추가 테스트 케이스를 생성해드릴까요?
src/main/java/konkuk/thip/book/application/service/BookSavedService.java (1)
63-64: 불필요한 조회 연산 최적화 가능save 메서드 후 findById를 다시 호출하는 것은 불필요할 수 있습니다. save 메서드가 완전한 Book 엔티티를 반환하도록 수정하거나, ID만으로 Book 객체를 생성하는 것을 고려해보세요.
- Long savedBookId = bookCommandPort.save(newBook); - return bookCommandPort.findById(savedBookId); + // save 메서드가 저장된 Book 엔티티를 반환하도록 수정 고려 + Long savedBookId = bookCommandPort.save(newBook); + newBook.setId(savedBookId); + return newBook;src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java (2)
33-41: 불필요한 사용자 조회를 제거하세요사용자 존재 여부 확인을 위해 전체 UserJpaEntity를 조회하고 있지만, 실제로는 전달받은 userId를 그대로 사용하고 있습니다. 성능 최적화를 위해
existsById를 사용하거나, bookJpaRepository에서 직접 빈 결과를 처리하는 것을 고려해보세요.@Override public Set<Book> findSavedBooksByUserId(Long userId) { - UserJpaEntity user = userJpaRepository.findById(userId) - .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); + if (!userJpaRepository.existsById(userId)) { + throw new EntityNotFoundException(USER_NOT_FOUND); + } - return bookJpaRepository.findSavedBooksByUserId(user.getUserId()).stream() + return bookJpaRepository.findSavedBooksByUserId(userId).stream() .map(bookMapper::toDomainEntity) .collect(Collectors.toSet()); }
43-52: 동일한 성능 최적화 적용 필요
findSavedBooksByUserId메서드와 동일한 패턴으로 불필요한 사용자 엔티티 조회가 발생하고 있습니다. 일관성 있는 성능 최적화를 위해 동일한 개선사항을 적용하세요.@Override public Set<Book> findJoiningRoomsBooksByUserId(Long userId) { - UserJpaEntity user = userJpaRepository.findById(userId) - .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); + if (!userJpaRepository.existsById(userId)) { + throw new EntityNotFoundException(USER_NOT_FOUND); + } - return bookJpaRepository.findJoiningRoomsBooksByUserId(user.getUserId()) + return bookJpaRepository.findJoiningRoomsBooksByUserId(userId) .stream() .map(bookMapper::toDomainEntity) .collect(Collectors.toSet()); }src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java (1)
114-144: 사용자 검증 최적화 및 쿼리 성능 개선 필요
- BookQueryPersistenceAdapter와 동일하게 불필요한 사용자 엔티티 전체 조회가 발생하고 있습니다.
- 저장된 피드와 태그를 가져오기 위해 여러 쿼리가 실행되고 있어 성능 문제가 발생할 수 있습니다.
@Override public SavedFeeds findSavedFeedsByUserId(Long userId) { - UserJpaEntity user = userJpaRepository.findById(userId) - .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); + if (!userJpaRepository.existsById(userId)) { + throw new EntityNotFoundException(USER_NOT_FOUND); + } List<SavedFeedJpaEntity> savedFeedEntities = - savedFeedJpaRepository.findAllByUserId(user.getUserId()); + savedFeedJpaRepository.findAllByUserId(userId);추가로, 피드와 태그를 한 번의 조인 쿼리로 가져오는 최적화를 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (64)
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java(3 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/BookSelectableListResponse.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/jpa/SavedBookJpaEntity.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/repository/SavedBookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java(2 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSelectableListUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookInfo.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookSelectableType.java(1 hunks)src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java(1 hunks)src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookMostSearchRankService.java(2 hunks)src/main/java/konkuk/thip/book/application/service/BookSavedService.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSearchService.java(3 hunks)src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java(1 hunks)src/main/java/konkuk/thip/book/domain/SavedBooks.java(0 hunks)src/main/java/konkuk/thip/feed/adapter/out/jpa/SavedFeedJpaEntity.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java(3 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedTag/FeedTagJpaRepository.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/SavedFeedJpaRepository.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/out/FeedCommandPort.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java(2 hunks)src/main/java/konkuk/thip/feed/application/port/out/dto/FeedIdAndTagProjection.java(1 hunks)src/main/java/konkuk/thip/feed/application/service/BasicFeedShowAllService.java(1 hunks)src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java(1 hunks)src/main/java/konkuk/thip/feed/application/service/FeedSavedService.java(3 hunks)src/main/java/konkuk/thip/feed/application/service/FeedShowAllOfUserService.java(1 hunks)src/main/java/konkuk/thip/feed/application/service/FeedShowSingleService.java(3 hunks)src/main/java/konkuk/thip/feed/application/service/FollowingPriorityFeedShowAllService.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomCreateService.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/in/web/SavedBookCommandController.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/in/web/SavedBookQueryController.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/in/web/SavedFeedCommandController.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/in/web/SavedFeedQueryController.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/in/web/request/UserSignupRequest.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/in/web/response/DummyResponse.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/out/mapper/SavedBookMapper.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/out/mapper/SavedFeedMapper.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/repository/SavedBookJpaRepository.java(0 hunks)src/main/java/konkuk/thip/saved/application/port/in/DummyUseCase.java(0 hunks)src/main/java/konkuk/thip/saved/application/port/in/dto/DummyCommand.java(0 hunks)src/main/java/konkuk/thip/saved/application/port/in/dto/DummyQuery.java(0 hunks)src/main/java/konkuk/thip/saved/application/port/in/dto/DummyResult.java(0 hunks)src/main/java/konkuk/thip/saved/application/port/out/SavedCommandPort.java(0 hunks)src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java(0 hunks)src/main/java/konkuk/thip/saved/application/service/SavedBookService.java(0 hunks)src/main/java/konkuk/thip/saved/application/service/SavedFeedService.java(0 hunks)src/main/java/konkuk/thip/saved/domain/SavedBook.java(0 hunks)src/main/java/konkuk/thip/saved/domain/SavedFeed.java(0 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookChangeSavedControllerTest.java(4 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookGetSelectableListApiTest.java(1 hunks)src/test/java/konkuk/thip/common/util/TestEntityFactory.java(3 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/BasicFeedShowAllApiTest.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedChangeSavedAPITest.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowMineApiTest.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSingleApiTest.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSpecificUserApiTest.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FollowingPriorityFeedShowAllApiTest.java(1 hunks)
💤 Files with no reviewable changes (22)
- src/main/java/konkuk/thip/saved/adapter/in/web/SavedFeedQueryController.java
- src/main/java/konkuk/thip/saved/adapter/in/web/SavedBookCommandController.java
- src/main/java/konkuk/thip/saved/adapter/in/web/response/DummyResponse.java
- src/main/java/konkuk/thip/saved/application/service/SavedFeedService.java
- src/main/java/konkuk/thip/saved/adapter/in/web/request/UserSignupRequest.java
- src/main/java/konkuk/thip/saved/application/service/SavedBookService.java
- src/main/java/konkuk/thip/book/domain/SavedBooks.java
- src/main/java/konkuk/thip/saved/adapter/out/mapper/SavedFeedMapper.java
- src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java
- src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java
- src/main/java/konkuk/thip/saved/adapter/out/mapper/SavedBookMapper.java
- src/main/java/konkuk/thip/saved/application/port/out/SavedCommandPort.java
- src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java
- src/main/java/konkuk/thip/saved/application/port/in/DummyUseCase.java
- src/main/java/konkuk/thip/saved/adapter/in/web/SavedBookQueryController.java
- src/main/java/konkuk/thip/saved/domain/SavedFeed.java
- src/main/java/konkuk/thip/saved/application/port/in/dto/DummyCommand.java
- src/main/java/konkuk/thip/saved/adapter/in/web/SavedFeedCommandController.java
- src/main/java/konkuk/thip/saved/application/port/in/dto/DummyResult.java
- src/main/java/konkuk/thip/saved/domain/SavedBook.java
- src/main/java/konkuk/thip/saved/application/port/in/dto/DummyQuery.java
- src/main/java/konkuk/thip/saved/adapter/out/persistence/repository/SavedBookJpaRepository.java
🧰 Additional context used
🧠 Learnings (4)
📓 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.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
📚 Learning: thip 프로젝트에서는 cqrs port 분리 시 다음 컨벤션을 따름: commandport에는 findbyxxx를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, querypo...
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.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/feed/application/service/FeedShowAllOfUserService.javasrc/main/java/konkuk/thip/feed/application/service/FollowingPriorityFeedShowAllService.javasrc/main/java/konkuk/thip/feed/application/service/BasicFeedShowAllService.javasrc/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.javasrc/main/java/konkuk/thip/feed/application/port/out/FeedCommandPort.javasrc/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.javasrc/main/java/konkuk/thip/book/application/port/out/BookCommandPort.javasrc/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.javasrc/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.javasrc/main/java/konkuk/thip/feed/application/service/FeedCreateService.javasrc/main/java/konkuk/thip/book/application/port/out/BookQueryPort.javasrc/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.javasrc/main/java/konkuk/thip/book/application/service/BookSavedService.javasrc/main/java/konkuk/thip/feed/application/service/FeedShowSingleService.javasrc/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.javasrc/main/java/konkuk/thip/feed/application/service/FeedSavedService.javasrc/main/java/konkuk/thip/book/application/service/BookSearchService.java
📚 Learning: thip 프로젝트에서는 query api(조회 api)에 한해서는 application 계층에서 adapter.in.web.response 패키지의 response dto를 직접 ...
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#78
File: src/main/java/konkuk/thip/user/application/port/out/FollowingQueryPort.java:3-3
Timestamp: 2025-07-14T18:22:56.538Z
Learning: THIP 프로젝트에서는 Query API(조회 API)에 한해서는 application 계층에서 adapter.in.web.response 패키지의 response DTO를 직접 참조하는 것을 허용함. 이는 CQRS 아키텍처에서 읽기 전용 작업의 효율성을 위한 팀 컨벤션임.
Applied to files:
src/main/java/konkuk/thip/feed/application/service/FeedShowAllOfUserService.javasrc/main/java/konkuk/thip/book/application/service/BookSearchService.java
📚 Learning: spring data jpa에서 findby{fieldname} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, optional 반환 타입을 사용하는 것이 nu...
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 안전성을 위해 권장됩니다.
Applied to files:
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.javasrc/main/java/konkuk/thip/book/application/service/BookSearchService.java
🧬 Code Graph Analysis (2)
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/SavedBookJpaRepository.java (1)
src/main/java/konkuk/thip/saved/adapter/out/persistence/repository/SavedBookJpaRepository.java (1)
SavedBookJpaRepository(8-12)
src/test/java/konkuk/thip/book/adapter/in/web/BookGetSelectableListApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(34-324)
⏰ 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 (48)
src/main/java/konkuk/thip/feed/application/port/out/dto/FeedIdAndTagProjection.java (1)
1-1: 패키지 이동이 올바르게 적용되었습니다.피드 도메인과 관련된 projection을
saved패키지에서feed패키지로 이동한 것이 적절합니다. 도메인별 패키지 구조가 더 명확해졌습니다.src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSingleApiTest.java (1)
16-17: import 구문이 패키지 재구성에 맞게 올바르게 업데이트되었습니다.
SavedFeedJpaEntity와SavedFeedJpaRepository가saved패키지에서feed패키지로 이동한 것에 맞춰 import가 정확하게 수정되었습니다.src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedTag/FeedTagJpaRepository.java (1)
5-5: import 업데이트가 패키지 재구성과 일치합니다.
FeedIdAndTagProjection이feed패키지로 이동한 것에 맞춰 import가 올바르게 수정되었습니다.src/main/java/konkuk/thip/feed/adapter/out/jpa/SavedFeedJpaEntity.java (1)
1-1: 엔티티의 패키지 이동이 적절합니다.
SavedFeedJpaEntity를saved패키지에서feed패키지로 이동한 것이 도메인 구조상 더 적합합니다. 저장된 피드 기능이 피드 도메인에 속하는 것이 논리적으로 올바릅니다.src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (1)
16-17: 책 관련 저장 엔티티의 import가 올바르게 업데이트되었습니다.
SavedBookJpaEntity와SavedBookJpaRepository가saved패키지에서book패키지로 이동한 것에 맞춰 import가 정확하게 수정되었습니다. 도메인별 패키지 구조가 더 일관성 있게 되었습니다.src/main/java/konkuk/thip/book/adapter/out/jpa/SavedBookJpaEntity.java (1)
1-1: 패키지 구조 변경 및 import 경로 업데이트 완료SavedBookJpaEntity를
konkuk.thip.book.adapter.out.jpa로 이동한 후,
구 코드(konkuk.thip.saved.adapter.out.jpa)를 참조하는 import가 없고
모든 참조가 새로운 패키지 경로로 올바르게 업데이트된 것을 확인했습니다.
변경 사항을 승인합니다.src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowMineApiTest.java (1)
10-10: Import 경로 업데이트 확인SavedFeedJpaRepository의 패키지 이동에 맞춰 import 경로가 올바르게 업데이트되었습니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedChangeSavedAPITest.java (1)
14-15: Import 경로들이 적절히 업데이트됨SavedFeed 관련 클래스들의 패키지 이동에 맞춰 import 경로들이 올바르게 수정되었습니다. Feed 도메인과 관련된 클래스들을 feed 패키지에서 관리하는 것이 더 적절합니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/SavedFeedJpaRepository.java (1)
1-3: 도메인별 패키지 구조로 개선됨SavedFeed 관련 레포지토리를 feed 패키지로 이동한 것은 도메인 중심의 패키지 구조를 만드는 좋은 변경입니다. 이는 코드의 응집도를 높이고 유지보수성을 개선합니다.
src/main/java/konkuk/thip/feed/application/service/FollowingPriorityFeedShowAllService.java (1)
50-50: FeedQueryPort 메서드 선언 및 구현 확인 완료
findSavedFeedIdsByUserIdAndFeedIds메서드는FeedQueryPort인터페이스에 선언되어 있고, Persistence Adapter에서 정상적으로 구현되어 있어 의존성 통합 변경을 승인합니다.
- src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java:39 — 메서드 선언 확인
- src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java:147-148 — 메서드 구현 확인
src/main/java/konkuk/thip/feed/application/service/FeedShowAllOfUserService.java (1)
58-58: 리팩토링이 올바르게 적용되었습니다.SavedQueryPort에서 FeedQueryPort로의 의존성 변경이 적절합니다. 저장된 피드 조회 기능을 feed 도메인으로 통합하는 것은 아키텍처 일관성을 높이며, CQRS 컨벤션에도 부합합니다.
src/main/java/konkuk/thip/feed/application/service/FeedShowSingleService.java (1)
9-9: 의존성 리팩토링이 일관성 있게 적용되었습니다.SavedQueryPort에서 FeedQueryPort로의 전환이 올바르게 이루어졌습니다. 필드 선언부터 실제 사용까지 일관되게 변경되어 있어 안정적인 리팩토링입니다.
Also applies to: 27-27, 43-43
src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSpecificUserApiTest.java (1)
11-12: 패키지 리팩토링에 따른 import 수정이 올바릅니다.SavedFeedJpaEntity와 SavedFeedJpaRepository의 패키지 위치가
saved에서feed로 변경된 것을 반영한 적절한 수정입니다.src/test/java/konkuk/thip/feed/adapter/in/web/BasicFeedShowAllApiTest.java (1)
11-12: 일관성 있는 패키지 리팩토링 적용입니다.다른 테스트 클래스들과 동일하게 SavedFeed 관련 클래스들의 패키지 변경이 올바르게 반영되었습니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FollowingPriorityFeedShowAllApiTest.java (1)
11-12: 패키지 구조 변경이 체계적으로 적용되었습니다.모든 테스트 클래스에서 SavedFeed 관련 import들이 일관되게 수정되어 리팩토링이 완전히 적용되었습니다.
src/main/java/konkuk/thip/feed/application/service/BasicFeedShowAllService.java (1)
51-51: 리팩토링 변경사항 승인SavedQueryPort에서 FeedQueryPort로 변경된 것이 올바릅니다. saved 패키지를 제거하고 도메인별 포트로 통합하는 리팩토링 목표와 일치합니다.
src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java (1)
27-28: CQRS 컨벤션을 잘 따르는 메서드 추가BookCommandPort에 저장된 책 관련 커맨드 메서드들이 적절히 추가되었습니다. THIP 프로젝트의 CQRS 컨벤션에 따라 CommandPort에 도메인 엔티티 조작 메서드를 추가한 것이 올바릅니다.
src/main/java/konkuk/thip/book/application/port/in/BookSelectableListUseCase.java (1)
8-10: 잘 설계된 유스케이스 인터페이스단일 책임 원칙을 잘 따르고 있으며, 메서드명이 명확하고 파라미터 타입도 적절합니다. BookSelectableType enum을 사용한 타입 안전성도 좋습니다.
src/main/java/konkuk/thip/feed/application/port/out/FeedCommandPort.java (1)
20-21: 도메인별 포트 통합 리팩토링 승인FeedCommandPort에 저장된 피드 관련 커맨드 메서드가 BookCommandPort와 일관된 패턴으로 추가되었습니다. saved 패키지를 제거하고 도메인별 포트로 통합하는 리팩토링 목표에 부합합니다.
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (1)
71-73: Optional 기반 리팩토링이 잘 되었습니다.예외 기반 제어 흐름을 Optional 기반으로 변경하여 코드가 더 함수형 스타일로 개선되었고 가독성이 향상되었습니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookChangeSavedControllerTest.java (4)
10-11: 패키지 재구성 반영이 잘 되었습니다.SavedBook 관련 엔티티와 레포지토리가 saved 패키지에서 book 패키지로 적절히 이동되었습니다.
125-125: 메서드명 단순화가 좋습니다.기존의 복잡한 JPA 메서드명
existsByUserJpaEntity_UserIdAndBookJpaEntity_BookId에서 간결한existsByUserIdAndBookId로 개선되어 가독성이 향상되었습니다.
151-151: 일관성 있는 메서드명 사용.다른 테스트 케이스와 동일하게 간결한 메서드명을 사용하여 일관성을 유지하고 있습니다.
206-206: 삭제 검증에서도 일관된 메서드 사용.저장 검증과 동일한 패턴으로 간결한 메서드명을 사용하여 코드 일관성이 유지되고 있습니다.
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (3)
4-4: 패키지 재구성에 맞는 import 변경.SavedBookJpaEntity가 book 패키지로 이동한 것을 올바르게 반영했습니다.
19-19: Feed 관련 엔티티 import 업데이트.SavedFeedJpaEntity가 feed 패키지로 이동한 것을 적절히 반영했습니다.
318-323: 테스트용 SavedBook 팩토리 메서드 추가.기존 패턴을 따라 일관된 구조로 SavedBook 엔티티 생성 메서드를 추가했습니다. 테스트 코드에서 편리하게 사용할 수 있습니다.
src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java (3)
6-6: SavedFeeds 도메인 클래스 import 추가.저장된 피드 조회 기능을 위한 도메인 클래스 import가 적절히 추가되었습니다.
37-37: CQRS 패턴을 따르는 쿼리 메서드 추가.학습된 컨벤션에 따라 QueryPort에 조회 API 응답 데이터를 위한 메서드를 추가했습니다. 메서드명도 명확하고 적절합니다.
39-39: 효율적인 필터링 메서드 추가.특정 피드 ID 집합에서 사용자가 저장한 것만 필터링하는 메서드로, 성능상 이점이 있는 접근법입니다.
src/main/java/konkuk/thip/book/application/port/in/dto/BookSelectableType.java (2)
8-11: 잘 설계된 enum 구조.책 선택 타입을 명확하게 정의하고 각 타입의 의미를 주석으로 설명해 두어 가독성이 좋습니다.
19-26: 적절한 유효성 검증과 예외 처리.문자열을 enum으로 변환하는 로직이 올바르게 구현되었고, 유효하지 않은 입력에 대해 적절한 예외를 발생시킵니다. 한국어 오류 메시지도 사용자 친화적입니다.
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java (1)
53-60: 예외 기반 제어 흐름을 Optional 기반으로 개선한 좋은 리팩토링입니다.try-catch 블록을 사용한 예외 기반 제어 흐름을 Optional의 함수형 메서드(
map,orElseGet)를 활용한 방식으로 변경하여 코드 가독성과 유지보수성이 향상되었습니다. 정상적인 비즈니스 플로우에서 예외를 사용하지 않게 되어 더 적절한 패턴을 사용하게 되었습니다.src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java (1)
30-38: 도메인 객체를 DTO로 변환하는 매핑 메서드가 적절히 구현되었습니다.
Book도메인 객체를BookInfoDTO로 변환하는 매핑과Set<Book>을List<BookInfo>로 변환하는 배치 매핑이 올바르게 구현되었습니다. 명시적인@Mapping어노테이션을 통해 필드 매핑이 명확하게 정의되어 있고, CQRS 패턴에서 QueryPort의 응답 데이터 변환 역할을 잘 수행합니다.src/main/java/konkuk/thip/book/adapter/in/web/response/BookSelectableListResponse.java (1)
7-12: API 응답을 위한 record 클래스가 적절히 구현되었습니다.
BookSelectableListResponse레코드는 선택 가능한 책 목록을 감싸는 응답 DTO로서 역할을 잘 수행하고 있습니다. 정적 팩토리 메서드of를 통한 인스턴스 생성 방식도 일반적인 패턴을 따르고 있어 좋습니다.src/main/java/konkuk/thip/feed/application/service/FeedSavedService.java (1)
8-8: 저장 기능을 도메인별로 통합한 좋은 아키텍처 개선입니다.
SavedQueryPort와SavedCommandPort를FeedQueryPort와FeedCommandPort로 변경하여 저장된 피드 기능을 피드 도메인 내로 통합한 것이 적절합니다. 기존 로직과 검증은 그대로 유지하면서 의존성만 변경되어 안전한 리팩토링이 이루어졌습니다.Also applies to: 19-19, 29-29, 34-34, 38-38
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (2)
25-28: 의존성 주입이 적절하게 구현되었습니다.새로 추가된
UserJpaRepository와SavedBookJpaRepository의존성들이 저장된 책 기능을 위해 적절하게 주입되었습니다.
84-88: 삭제 메서드가 적절하게 구현되었습니다.존재하지 않는 데이터에 대한 삭제 요청도 안전하게 처리됩니다.
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java (1)
1-31: 깔끔하게 구현된 서비스 클래스입니다.
- Switch 표현식을 사용한 타입별 분기 처리가 명확합니다.
- Set을 사용하여 ISBN 기반 중복 제거가 자동으로 이뤄집니다.
- 매퍼를 통한 도메인-DTO 변환이 적절합니다.
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (2)
34-34: 새로운 의존성이 적절하게 추가되었습니다.BookSelectableListUseCase 의존성이 새로운 API 기능을 위해 올바르게 주입되었습니다.
90-102: 새로운 API 엔드포인트가 잘 설계되었습니다.
- RESTful한 URL 설계와 명확한 파라미터 구조
- Swagger 문서화가 충분히 제공됨
- 응답 형식이 기존 API들과 일관성 있게 구현됨
src/main/java/konkuk/thip/book/application/service/BookMostSearchRankService.java (1)
55-78: Optional 패턴으로의 리팩토링이 코드 품질을 향상시켰습니다.
- 예외 기반 제어 흐름에서 Optional 기반으로 변경하여 더 함수형 프로그래밍 스타일에 맞게 개선되었습니다.
ifPresentOrElse사용으로 조건부 로직이 명확해졌습니다.src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java (1)
13-17: 저장된 책 조회 쿼리가 적절하게 구현되었습니다.JPQL 조인과 정렬 조건이 올바르게 작성되어 사용자별 저장된 책을 최신순으로 조회할 수 있습니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java (1)
110-121: saveSavedFeed 메서드 구현이 적절합니다사용자와 피드의 존재 여부를 확인하고 적절한 예외를 던지는 방식이 견고합니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookGetSelectableListApiTest.java (1)
31-36: 통합 테스트 구성이 적절합니다Spring Boot 통합 테스트 어노테이션과 트랜잭션 롤백 설정이 올바르게 구성되어 있습니다.
src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java (1)
9-13: BookQueryPort 인터페이스 확장이 적절합니다CQRS 패턴에 따른 QueryPort 메서드 추가가 적절하며, 중복 방지를 위한 Set 반환 타입 선택이 좋습니다.
src/main/java/konkuk/thip/book/application/service/BookSearchService.java (1)
92-115: Optional 기반 리팩토링이 훌륭합니다예외 기반 제어 흐름을 Optional을 사용한 함수형 접근으로 변경한 것이 코드 가독성과 유지보수성을 향상시켰습니다. 책이 존재하지 않을 때의 기본값 처리도 적절합니다.
src/main/java/konkuk/thip/book/application/service/BookSavedService.java (1)
27-48: changeSavedBook 메서드 리팩토링이 우수합니다Optional 기반 처리와 검증 로직 분리가 코드의 가독성과 유지보수성을 크게 향상시켰습니다.
| // 사용자가 책을 저장 | ||
| @Override | ||
| public void saveSavedBook(Long userId, Long bookId) { | ||
| UserJpaEntity user = userJpaRepository.findById(userId) | ||
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | ||
| BookJpaEntity book = bookJpaRepository.findById(bookId) | ||
| .orElseThrow(() -> new EntityNotFoundException(BOOK_NOT_FOUND)); | ||
| SavedBookJpaEntity entity = SavedBookJpaEntity.builder() | ||
| .userJpaEntity(user) | ||
| .bookJpaEntity(book) | ||
| .build(); | ||
| savedBookJpaRepository.save(entity); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복 저장 처리 로직 추가를 고려해보세요.
현재 구현에서는 이미 저장된 책을 다시 저장할 때의 중복 처리가 없습니다. 동일한 사용자-책 조합이 중복으로 저장될 가능성이 있습니다.
@Override
public void saveSavedBook(Long userId, Long bookId) {
+ // 이미 저장된 책인지 확인
+ if (savedBookJpaRepository.existsByUserIdAndBookId(userId, bookId)) {
+ return; // 이미 저장되어 있으면 무시하거나 예외 처리
+ }
UserJpaEntity user = userJpaRepository.findById(userId)
.orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));
BookJpaEntity book = bookJpaRepository.findById(bookId)
.orElseThrow(() -> new EntityNotFoundException(BOOK_NOT_FOUND));📝 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 void saveSavedBook(Long userId, Long bookId) { | |
| UserJpaEntity user = userJpaRepository.findById(userId) | |
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | |
| BookJpaEntity book = bookJpaRepository.findById(bookId) | |
| .orElseThrow(() -> new EntityNotFoundException(BOOK_NOT_FOUND)); | |
| SavedBookJpaEntity entity = SavedBookJpaEntity.builder() | |
| .userJpaEntity(user) | |
| .bookJpaEntity(book) | |
| .build(); | |
| savedBookJpaRepository.save(entity); | |
| } | |
| // 사용자가 책을 저장 | |
| @Override | |
| public void saveSavedBook(Long userId, Long bookId) { | |
| // 이미 저장된 책인지 확인 | |
| if (savedBookJpaRepository.existsByUserIdAndBookId(userId, bookId)) { | |
| return; // 이미 저장되어 있으면 무시하거나 예외 처리 | |
| } | |
| UserJpaEntity user = userJpaRepository.findById(userId) | |
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | |
| BookJpaEntity book = bookJpaRepository.findById(bookId) | |
| .orElseThrow(() -> new EntityNotFoundException(BOOK_NOT_FOUND)); | |
| SavedBookJpaEntity entity = SavedBookJpaEntity.builder() | |
| .userJpaEntity(user) | |
| .bookJpaEntity(book) | |
| .build(); | |
| savedBookJpaRepository.save(entity); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java
around lines 70 to 82, the saveSavedBook method lacks handling for duplicate
user-book entries, which can cause redundant saves. To fix this, before creating
and saving a new SavedBookJpaEntity, check if an entry with the same userId and
bookId already exists in savedBookJpaRepository. If it exists, skip saving or
handle accordingly to prevent duplicates.
There was a problem hiding this comment.
🧩 Analysis chain
@seongjunnoh 맞습니다! Clean Architecture 관점에서 볼 때 어댑터는 순수하게 저장/조회 기능만 담당해야 하고, 비즈니스 로직(중복 검사 포함)은 서비스 레이어에서 처리하는 것이 올바른 접근입니다.
실제로 BookSavedService를 확인해보니 validateSaveBookAction 메서드에서 이미 중복 검사를 수행하고 있네요:
private void validateSaveBookAction(boolean isSaveRequest, boolean isAlreadySaved) {
if (isSaveRequest && isAlreadySaved) {
throw new BusinessException(BOOK_ALREADY_SAVED);
}
// ...
}어댑터는 단순히 데이터 저장 책임만 가져야 한다는 의견에 동의합니다. 제안을 철회하겠습니다.
🏁 Script executed:
#!/bin/bash
# BookSavedService에서 중복 체크 로직이 있는지 확인
cat src/main/java/konkuk/thip/book/application/service/BookSavedService.java
Length of output: 3010
@seongjunnoh 완전히 맞습니다! 실제 BookSavedService 코드를 확인해보니 서비스 레이어에서 이미 완벽하게 중복 처리를 하고 있네요:
boolean alreadySaved = bookQueryPort.existsSavedBookByUserIdAndBookId(userId, book.getId());
validateSaveBookAction(isSaveRequest, alreadySaved);
private void validateSaveBookAction(boolean isSaveRequest, boolean alreadySaved) {
if (isSaveRequest && alreadySaved) {
throw new BusinessException(BOOK_ALREADY_SAVED);
}
// ...
}서비스 레이어에서 비즈니스 로직(중복 검사, 유효성 검증)을 담당하고, 어댑터는 순수하게 저장/삭제 동작만 수행하는 Clean Architecture 구조가 잘 구현되어 있습니다.
어댑터에 비즈니스 로직을 추가하자는 제안은 부적절했습니다. 현재 구조가 올바른 레이어 분리를 보여주고 있네요.
✏️ Learnings added
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#166
File: src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java:70-82
Timestamp: 2025-08-07T18:19:55.856Z
Learning: seongjunnoh는 Clean Architecture 원칙을 중시하며, 어댑터는 순수하게 저장/조회 기능만 담당하고 비즈니스 로직은 서비스 레이어에서 처리하는 것을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#166
File: src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java:70-82
Timestamp: 2025-08-07T18:19:55.856Z
Learning: seongjunnoh는 Clean Architecture 원칙을 중시하며, 어댑터는 순수하게 저장/조회 기능만 담당하고 비즈니스 로직은 서비스 레이어에서 처리하는 것을 선호한다.
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.
🧠 Learnings used
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
| @Query("SELECT b FROM BookJpaEntity b " + | ||
| "JOIN RoomJpaEntity r ON r.bookJpaEntity.bookId = b.bookId " + | ||
| "JOIN RoomParticipantJpaEntity rp ON rp.roomJpaEntity.roomId = r.roomId " + | ||
| "WHERE rp.userJpaEntity.userId = :userId " + | ||
| "AND r.status = 'ACTIVE' " + | ||
| "AND r.startDate <= CURRENT_TIMESTAMP") // 진행 중인 방만 조회 (모집 중 / 만료된 방 x) | ||
| List<BookJpaEntity> findJoiningRoomsBooksByUserId(Long userId); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
참여 중인 모임의 책 조회 쿼리 로직을 개선해보세요.
쿼리 로직은 올바르지만 다음 사항들을 고려해보세요:
- 하드코딩된
'ACTIVE'문자열 대신 enum 상수 사용 - 복잡한 조인으로 인한 성능 이슈 가능성
- "AND r.status = 'ACTIVE' " +
+ "AND r.status = konkuk.thip.room.domain.RoomStatus.ACTIVE " +다음 스크립트로 인덱스 설정 상태를 확인해보세요:
🏁 Script executed:
#!/bin/bash
# RoomParticipantJpaEntity와 RoomJpaEntity의 인덱스 설정 확인
fd "RoomParticipantJpaEntity.java|RoomJpaEntity.java" --exec grep -A 5 -B 5 "@Index\|@Table"Length of output: 658
참여 중인 모임 책 조회 쿼리 최적화 및 인덱스 추가
현재 RoomJpaEntity와 RoomParticipantJpaEntity에 인덱스가 정의되어 있지 않아, 복잡한 조인 시 성능 저하가 발생할 수 있습니다. 다음과 같이 수정을 권장드립니다.
• BookJpaRepository (src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java)
- 하드코딩된
'ACTIVE'대신RoomStatusenum 파라미터 사용
@Query("SELECT b FROM BookJpaEntity b " +
- "JOIN RoomJpaEntity r ON r.bookJpaEntity.bookId = b.bookId " +
- "JOIN RoomParticipantJpaEntity rp ON rp.roomJpaEntity.roomId = r.roomId " +
- "WHERE rp.userJpaEntity.userId = :userId " +
- "AND r.status = 'ACTIVE' " +
- "AND r.startDate <= CURRENT_TIMESTAMP")
- List<BookJpaEntity> findJoiningRoomsBooksByUserId(Long userId);
+ "JOIN b.roomJpaEntity r " +
+ "JOIN r.roomParticipantJpaEntities rp " +
+ "WHERE rp.userJpaEntity.userId = :userId " +
+ "AND r.status = :status " +
+ "AND r.startDate <= CURRENT_TIMESTAMP")
+ List<BookJpaEntity> findJoiningRoomsBooksByUserId(
+ @Param("userId") Long userId,
+ @Param("status") RoomStatus status);- 호출 예:
repository.findJoiningRoomsBooksByUserId(userId, RoomStatus.ACTIVE);
• RoomJpaEntity (src/main/java/konkuk/thip/room/adapter/out/persistence/RoomJpaEntity.java)
@Entity
@Table(
name = "rooms",
indexes = {
@Index(name = "idx_rooms_status_startDate", columnList = "status,start_date"),
@Index(name = "idx_rooms_book_id", columnList = "book_id")
}
)
public class RoomJpaEntity extends BaseJpaEntity { … }• RoomParticipantJpaEntity (src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantJpaEntity.java)
@Entity
@Table(
name = "room_participants",
indexes = {
@Index(name = "idx_participants_room_user", columnList = "room_id,user_id")
}
)
public class RoomParticipantJpaEntity { … }위 변경으로 쿼리 가독성과 재사용성을 높이고, 조인 성능 또한 개선할 수 있습니다.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java
around lines 19 to 25, refactor the query method to accept a RoomStatus enum
parameter instead of hardcoding the 'ACTIVE' status string. Modify the method
signature to include RoomStatus as a parameter and update the JPQL query to use
this parameter for the room status condition. Additionally, add the recommended
indexes to RoomJpaEntity and RoomParticipantJpaEntity classes as specified to
improve join performance.
| @Override | ||
| public void deleteSavedFeed(Long userId, Long feedId) { | ||
| savedFeedJpaRepository.deleteByUserIdAndFeedId(userId, feedId); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
deleteSavedFeed 메서드의 삭제 결과 검증 필요
삭제 작업이 실제로 수행되었는지 확인하지 않고 있습니다. 존재하지 않는 엔티티 삭제 시 예외 처리가 필요할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# SavedFeedJpaRepository의 deleteByUserIdAndFeedId 메서드 구현 확인
ast-grep --pattern 'interface SavedFeedJpaRepository {
$$$
deleteByUserIdAndFeedId($$$) {
$$$
}
$$$
}'
# 또는 메서드 시그니처 검색
rg -A 3 "deleteByUserIdAndFeedId" --type javaLength of output: 1245
deleteSavedFeed 삭제 결과 검증 및 예외 처리 추가 필요
현재 void 반환으로 삭제 결과를 확인할 수 없어, 대상 엔티티가 없을 경우 예외 처리 로직을 추가해야 합니다.
SavedFeedJpaRepository.deleteByUserIdAndFeedId메서드 시그니처를long deleteByUserIdAndFeedId(...)로 변경@Modifying및@Transactional어노테이션 추가FeedCommandPersistenceAdapter.deleteSavedFeed에서 반환된 삭제 개수를 확인하고,0인 경우EntityNotFoundException(또는 도메인에 맞는 예외) 발생
예시 코드 제안:
// src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/SavedFeedJpaRepository.java
@Transactional
@Modifying
long deleteByUserIdAndFeedId(
@Param("userId") Long userId,
@Param("feedId") Long feedId
);
// src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java
@Override
public void deleteSavedFeed(Long userId, Long feedId) {
long deletedCount = savedFeedJpaRepository.deleteByUserIdAndFeedId(userId, feedId);
if (deletedCount == 0) {
throw new EntityNotFoundException(
String.format("유저(%d)의 저장된 피드(%d)를 찾을 수 없습니다.", userId, feedId)
);
}
}🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java
around lines 123 to 126, the deleteSavedFeed method currently does not verify if
the deletion actually occurred. Modify the
SavedFeedJpaRepository.deleteByUserIdAndFeedId method to return a long
indicating the number of deleted records, and annotate it with @Modifying and
@Transactional. Then, in deleteSavedFeed, check the returned count and throw an
EntityNotFoundException if it is zero to handle the case where no entity was
deleted.
Test Results367 tests 367 ✅ 30s ⏱️ Results for commit 1593313. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/ci-workflow.yml (3)
48-48: Gradle 캐시 활용으로 빌드 시간 단축 제안
./gradlew clean build --stacktrace로clean까지 수행하면 incremental build 이점이 사라집니다.
CI 환경에서 캐시된 종속성 및 빌드 아티팩트를 재사용하려면:- run: ./gradlew clean build --stacktrace + run: ./gradlew build --stacktrace또는
gradle-build-action사용을 검토해 주세요.
56-61: JUnit 주석 액션 중복 코멘트 방지 가이드두 액션 모두 실패 테스트를 PR 코멘트로 남깁니다. 동일한 정보가 중복 게시되지 않도록
action-junit-report의job_summary: false,check_name지정 등을 활용해 출력 채널을 분리해 주세요.
61-61: EOF 개행 문자 누락마지막 줄에 개행이 없어 YAMLLint 경고가 발생합니다. 파일 끝에 빈 줄을 추가해 주세요.
- token: ${{ github.token }} +\ token: ${{ github.token }}\n
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-workflow.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
🪛 YAMLlint (1.37.1)
.github/workflows/ci-workflow.yml
[error] 61-61: no new line character at the end of file
(new-line-at-end-of-file)
⏰ 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 (2)
.github/workflows/ci-workflow.yml (2)
11-12: 권한 확장 범위 재검토 필요
checks: write,pull-requests: write권한을 부여하면 워크플로가 PR 코멘트와 체크를 수정·삭제할 수 있습니다.
현재 이벤트는pull_request에 한정되어 있으므로 문제가 될 가능성은 낮지만, 최소 권한 원칙(Principle of Least Privilege)에 따라 꼭 필요한 단계(publish-unit-test-result-action,action-junit-report)에서만permissions를 스코프-레벨로 설정하거나,permissionsstep-level override를 고려해 주세요.
50-55: 테스트 결과 경로 확인 요청
files: '**/build/test-results/test/TEST-*.xml'
Gradle 7+ 기본 경로는build/test-results/test/**.xml이 맞지만, 멀티 모듈·커스텀 태스크가 있는 경우 하위 모듈 경로가 누락될 수 있습니다.
필요 시**/build/test-results/**/*.xml로 확장하거나report_paths와 동일한 glob 패턴을 맞춰 주세요.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci-workflow.yml (2)
48-49: Gradle 캐시 스텝 활성화를 검토하세요
./gradlew build --stacktrace로 빌드 시간을 단축하기 위해 이미 주석 처리된 Gradle 캐시 스텝을 다시 활성화하는 것이 좋습니다. CI 반복 실행 시 수 분 단축 효과가 있습니다.
61-61: 파일 끝에 개행 문자 추가
YAMLLint 경고(new-line-at-end-of-file)를 해결하려면 EOF 에 빈 줄을 추가하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-workflow.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
🪛 YAMLlint (1.37.1)
.github/workflows/ci-workflow.yml
[error] 61-61: no new line character at the end of file
(new-line-at-end-of-file)
⏰ 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)
.github/workflows/ci-workflow.yml (1)
11-12: 권한(write) 부여 범위가 과도하지 않은지 재확인 필요
checks: write와pull-requests: write권한은 워크플로에서 주석 작성 외에 다른 쓰기 작업까지 허용합니다. 최소 권한 원칙(Principle of Least Privilege)에 따라 실제 사용 범위를 다시 점검해 주세요. 테스트 결과 댓글만 필요하다면pull-requests: write만으로 충분할 수 있습니다.
| - name: ✉️ Post test results as a comment on the PR | ||
| uses: EnricoMi/publish-unit-test-result-action@v2 | ||
| if: always() | ||
| with: | ||
| files: '**/build/test-results/test/TEST-*.xml' | ||
|
|
||
| - name: 🙁 Annotate failed test cases on changed lines | ||
| uses: mikepenz/action-junit-report@v3 | ||
| if: always() | ||
| with: | ||
| report_paths: '**/build/test-results/test/TEST-*.xml' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
액션 버전을 커밋 SHA 로 고정하여 서플라이 체인 리스크를 줄이세요
서드파티 액션을 @v2 / @v3 처럼 태그로 지정하면, 태그 이동 시 예상치 못한 코드가 실행될 수 있습니다. GitHub 권고사항에 따라 커밋 SHA 로 고정하거나 actions/dependency-review-action 등으로 모니터링을 추가해 주세요.
예시 diff:
- uses: EnricoMi/publish-unit-test-result-action@v2
+ uses: EnricoMi/publish-unit-test-result-action@v2.15.0 # 0e9f0b1
...
- uses: mikepenz/action-junit-report@v3
+ uses: mikepenz/action-junit-report@v3.8.0 # a1b2c3d📝 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.
| - name: ✉️ Post test results as a comment on the PR | |
| uses: EnricoMi/publish-unit-test-result-action@v2 | |
| if: always() | |
| with: | |
| files: '**/build/test-results/test/TEST-*.xml' | |
| - name: 🙁 Annotate failed test cases on changed lines | |
| uses: mikepenz/action-junit-report@v3 | |
| if: always() | |
| with: | |
| report_paths: '**/build/test-results/test/TEST-*.xml' | |
| - name: ✉️ Post test results as a comment on the PR | |
| uses: EnricoMi/publish-unit-test-result-action@v2.15.0 # 0e9f0b1 | |
| if: always() | |
| with: | |
| files: '**/build/test-results/test/TEST-*.xml' | |
| - name: 🙁 Annotate failed test cases on changed lines | |
| uses: mikepenz/action-junit-report@v3.8.0 # a1b2c3d | |
| if: always() | |
| with: | |
| report_paths: '**/build/test-results/test/TEST-*.xml' |
🤖 Prompt for AI Agents
In .github/workflows/ci-workflow.yml around lines 50 to 60, the third-party
GitHub Actions are referenced using version tags (@v2, @v3), which can lead to
supply chain risks if those tags move unexpectedly. To fix this, replace the
version tags with specific commit SHAs for each action to pin the exact code
version used. Obtain the commit SHA from the action's repository and update the
uses field accordingly to ensure consistent and secure execution.
There was a problem hiding this comment.
패키지 구조 변경해주시고, 기존 try-catch 구문도 수정해주시고 감사합니다!!
현준님이 리뷰 요구사항에 언급하셨던 BookInfo 의 활용에 대한 개인적인 의견을 리뷰로 작성하였습니다!
또한 jpaRepository 관련하여 코멘트 남겼으니 확인 부탁드립니다!
그리고 궁금한 게 있는데, 현재 구현하신 api 가 모임방 생성 시 책 선택하는 화면에서 [유저가 저장한 책 목록, 유저가 참여중인 진행중인 방에서의 책 목록] 을 조회하는 api 인거 같은데, 이때 정렬이 필요한가요??
jpql 코드에서는 정렬을 거셨는데, 이를 어댑터에서 다시 set 으로 만드셔서 요구사항이 어떤지 궁금합니다!
| @Getter | ||
| public enum BookSelectableType { | ||
| SAVED("SAVED"), // 저장된 책 | ||
| JOINING("JOINING") // 참여 중인 모임 방의 책 | ||
| ; |
There was a problem hiding this comment.
api 의 request param을 검증하기 위한 enum 좋습니다 패키지 위치도 좋네요
| @Query("SELECT b FROM BookJpaEntity b " + | ||
| "JOIN SavedBookJpaEntity s ON s.bookJpaEntity.bookId = b.bookId " + | ||
| "WHERE s.userJpaEntity.userId = :userId " + | ||
| "ORDER BY s.createdAt DESC") |
There was a problem hiding this comment.
p3 : ordering 결과를 사용하지 않는 것 같은데, 최신순으로 정렬한 이유가 있으실까요?
There was a problem hiding this comment.
p2 : 추가로 유저가 저장한 책을 조회하는 메서드를 SavedBookJpaRepository 가 아니라 BookJpaRepository 에서 구현하신 이유가 있을까요??
Saved라는 도메인은 없는게 맞지만, 유저-책 의 관계테이블(= 유저가 저장한 책 테이블) 은 여전히 존재하고 SavedBookJpaEntity 또한 존재하니 굳이 BookJpaRepository 에서 join을 걸어서 조회하는 것 보다는 SavedBookJpaRepository 에서 바로 유저가 저장한 책을 조회하는게 어떨까 싶습니다!
There was a problem hiding this comment.
p2 : 추가로 유저가 저장한 책을 조회하는 메서드를 SavedBookJpaRepository 가 아니라 BookJpaRepository 에서 구현하신 이유가 있을까요?? Saved라는 도메인은 없는게 맞지만, 유저-책 의 관계테이블(= 유저가 저장한 책 테이블) 은 여전히 존재하고 SavedBookJpaEntity 또한 존재하니 굳이 BookJpaRepository 에서 join을 걸어서 조회하는 것 보다는 SavedBookJpaRepository 에서 바로 유저가 저장한 책을 조회하는게 어떨까 싶습니다!
우선 이 부분은 처음에 저도 SavedBookJpaRepository에서 조회하여 List를 반환받은 후 savedBook.getBookJpaEntity()를 호출하여 Book이라는 Domain으로 변환하시는 방식으로 구현했습니다. 물론 이렇게 하면 N+1 문제가 발생하니 SavedBookJpaRepository에서 SavedBookJpaEntity를 조회할때 EntityGraph를 통해 bookJpaEntity를 fetch join 해서 사용했습니다. 이렇게 구현하고 보니 굳이 fetch join을 통해 BookJpaEntity와 SavedBookJpaEntity를 모두 메모리에 올려두는 것이 맞을까? 라는 의문이 들었습니다. 따라서, 조회 로직을 bookJpaRepository로 옮기고 BookJpaEntity를 바로 반환받는 것으로 수정하였습니다!
There was a problem hiding this comment.
p3 : ordering 결과를 사용하지 않는 것 같은데, 최신순으로 정렬한 이유가 있으실까요?
엇 정렬 조건이 사용자가 책을 저장한 최신순이라고 생각해서 구현했는데 어떤 부분때문에 그러시죠?
There was a problem hiding this comment.
p2 : 추가로 유저가 저장한 책을 조회하는 메서드를 SavedBookJpaRepository 가 아니라 BookJpaRepository 에서 구현하신 이유가 있을까요?? Saved라는 도메인은 없는게 맞지만, 유저-책 의 관계테이블(= 유저가 저장한 책 테이블) 은 여전히 존재하고 SavedBookJpaEntity 또한 존재하니 굳이 BookJpaRepository 에서 join을 걸어서 조회하는 것 보다는 SavedBookJpaRepository 에서 바로 유저가 저장한 책을 조회하는게 어떨까 싶습니다!
우선 이 부분은 처음에 저도 SavedBookJpaRepository에서 조회하여 List를 반환받은 후 savedBook.getBookJpaEntity()를 호출하여 Book이라는 Domain으로 변환하시는 방식으로 구현했습니다. 물론 이렇게 하면 N+1 문제가 발생하니 SavedBookJpaRepository에서 SavedBookJpaEntity를 조회할때 EntityGraph를 통해 bookJpaEntity를 fetch join 해서 사용했습니다. 이렇게 구현하고 보니 굳이 fetch join을 통해 BookJpaEntity와 SavedBookJpaEntity를 모두 메모리에 올려두는 것이 맞을까? 라는 의문이 들었습니다. 따라서, 조회 로직을 bookJpaRepository로 옮기고 BookJpaEntity를 바로 반환받는 것으로 수정하였습니다!
아아 조회하는 대상이 SavedBook이 아니라 Book 이었군요! 제가 착각했네요 하하
이런 이유라면 현재처럼 BookJpaRepository 에 위치하는것도 좋아보입니다!
There was a problem hiding this comment.
p3 : ordering 결과를 사용하지 않는 것 같은데, 최신순으로 정렬한 이유가 있으실까요?
엇 정렬 조건이 사용자가 책을 저장한 최신순이라고 생각해서 구현했는데 어떤 부분때문에 그러시죠?
이 메서드를 반환받는 어댑터에서 set 으로 만들어서 서비스에 던지길래 이러면 정렬한 이유가 없어지지않나? 라는 생각에 리뷰 남겨보았습니다
정렬하는게 요구사항이라면 어댑터 코드를 수정해야하지 않을까요??
| "WHERE rp.userJpaEntity.userId = :userId " + | ||
| "AND r.status = 'ACTIVE' " + | ||
| "AND r.startDate <= CURRENT_TIMESTAMP " + // 진행 중인 방만 조회 (모집 중 / 만료된 방 x) | ||
| "ORDER BY r.roomPercentage DESC") // 방의 진행률이 높은 순서로 정렬 |
There was a problem hiding this comment.
p3 : 여기도 마찬가지로 ordering 결과를 사용하지 않는 것 같은데, 최신순으로 정렬한 이유가 있으실까요? 영속성 adapter 에서 이 메서드 호출하고 set으로 만드는 거 같아서 여쭤봅니다
There was a problem hiding this comment.
이 메서드는 "유저가 참가 중인 방 중, 진행중인 방에서의 책들을 조회" 하는 메서드인 것 같으니, BookJpaRepository 에 위치하는게 맞는 것 같습니다! 굳굳
There was a problem hiding this comment.
p3 : 여기도 마찬가지로 ordering 결과를 사용하지 않는 것 같은데, 최신순으로 정렬한 이유가 있으실까요? 영속성 adapter 에서 이 메서드 호출하고 set으로 만드는 거 같아서 여쭤봅니다
앗 생각해보니 Set으로 변환하는 것을 깜빡했네요! DISTINCT 키워드를 추가하고 List로 변환해야 될 것 같네요,,
.github/workflows/ci-workflow.yml
Outdated
|
|
||
| - name: 🐘 build with Gradle | ||
| run: ./gradlew build No newline at end of file | ||
| run: ./gradlew build --stacktrace |
| - name: ✉️ Post test results as a comment on the PR | ||
| uses: EnricoMi/publish-unit-test-result-action@v2 | ||
| if: always() | ||
| with: | ||
| files: '**/build/test-results/test/TEST-*.xml' | ||
|
|
||
| - name: 🙁 Annotate failed test cases on changed lines | ||
| uses: mikepenz/action-junit-report@v3 | ||
| if: always() | ||
| with: | ||
| report_paths: '**/build/test-results/test/TEST-*.xml' | ||
| token: ${{ github.token }} No newline at end of file |
| public record BookInfo( | ||
| Long bookId, | ||
| String bookTitle, | ||
| String authorName, | ||
| String publisher, | ||
| String bookImageUrl, | ||
| String isbn | ||
| ) { |
There was a problem hiding this comment.
p3 : 책 조회용 데이터를 의미하는 dto 인 것 같은데, 이 dto를 application/port/in/dto 에 위치시킨 이유가 무엇인가요??
어떤 용도로 이 dto 를 사용하는 것을 생각하신 걸까요??
여러 api의 response 에서 dto를 의존하는 것은 개인적으로 유지보수에 취약하지 않나 생각합니다
지금은 여러 api 가 동일한 데이터를 반환하니 dto를 통해 서로 결합도가 생겨도 문제없을 수 있지만, 언제 어떤 api에서 response 데이터가 변경될 지 모르는데, 이때마다 dto를 수정해야하나? 이러면 dto를 의존하는 다른 api는 어떡하지? 아니면 새로운 response 를 다시 정의해야하나? 라는 고민을 하는 것보다는 inner class 를 통해 response 끼리의 의존성을 없애는게 맞지않나 싶습니다
또한 처음 헥사고날 아키텍처를 도입하고 api 별로 유스케이스를 분리하는 명목에도 어긋난다고 생각합니다
request/response 를 제외한 dto는 현재 저희가 사용하는 queryDto 처럼 특정 도메인의 조회용 모델로써 이를 service 가 받아 각각의 response 로의 매핑과정을 거치는게 좋지 않나 생각합니다!!
There was a problem hiding this comment.
음 사실 dto 이름이 현재는 BookInfo이긴 하지만 저희가 Query를 담당하는 UseCase에서 반환하기로한 BookXXXResult dto가 갖는 성격을 띈다고 생각해서 패키지를 port.in.dto에 넣었습니다.
저도 하나의 api가 하나의 dto를 담당해야 한다는 성준님 의견에는 동의합니다. 다만, 현재 BookInfoResult의 경우 Book이라는 도메인을 그대로 가져다 놓은 사실상 Book의 응답 도메인이라고 봐도 무방하다고 생각했습니다. 이렇게 Book을 그대로 반환하는 dto를 선언해두고 Book 도메인을 그대로 반환해야 하는 api가 있을때만 사용하도록 재사용하도록 제한하면, 성준님이 생각하는 dto가 수정될 우려사항이 해결되지 않을까 싶은데 어떻게 생각하시나요..?
이렇게 하면, BookQueryMapper에서 매핑되는 메서드까지 재사용이 가능할 것 같습니다!
There was a problem hiding this comment.
현준님 말처럼 단일 책의 정보를 반환하는 api는 해당 dto 의존하도록 해도 그렇게 문제는 없을 것 같네요!
그리고 언급하신 것처럼 queryMapper 에서의 매번 구성은 같지만 다른 response 로의 매핑 메서드를 정의하지 않아도 되는 이점도 있어보이네요
좋습니다! 다만 저희가 response 에서 해당 dto 의 의존성을 가질 때 현재 api 가 BookInfo 를 의존하는 것이 적절한지를 생각해보는건 중요할 것 같습니다
There was a problem hiding this comment.
혼자 고민을 조금 해봤는데 성준님 말처럼 각각의 api가 서로 다른 dto를 사용하는게 더 낫다는 결론이 들었습니다!! 그래서 BookSelectableResult로 네이밍을 변경하였습니다. 다만, 저는 inner class는 지양할수록 좋다고 생각해서 페이징처리를 수반하지 않는 조회 api의 경우 지금처럼 UseCase에서 List 형태로 반환하고 controller에서 response에 List를 주입하는 방향으로 구현하는 것이 좋을 것 같습니다!
| .isbn(isbn) | ||
| .build()); | ||
| } | ||
| bookCommandPort.findByIsbn(isbn).ifPresentOrElse( |
| isSaved | ||
| ); | ||
| }) | ||
| .orElseGet(() -> |
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.data.bookList").isArray()) | ||
| .andExpect(jsonPath("$.data.bookList.length()").value(1)) | ||
| .andExpect(jsonPath("$.data.bookList.length()").value(2)) |
There was a problem hiding this comment.
오 쉿 로컬에서 보여주는 것처럼 로그 나오네요 굳굳
|
|
||
| - name: 🐘 build with Gradle | ||
| run: ./gradlew build --stacktrace | ||
| run: ./gradlew build --parallel --stacktrace |
There was a problem hiding this comment.
오 이러면 병렬적으로 빌드가 되나요?? 궁금
There was a problem hiding this comment.
저희 테스트 코드가 서로 의존성이 없어서 가능한 것 같네요..! 로컬에서 통과되어서 추가해봤습니다!
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci-workflow.yml (1)
50-61: 서드-파티 GitHub Action 버전을 커밋 SHA 로 고정하세요이전 리뷰에서도 동일 지적이 있었지만 여전히
@v2,@v3태그를 사용하고 있습니다.
태그는 이동 가능하므로 공급망 공격에 노출될 수 있습니다. SHA 고정이 조직 보안 가이드라인입니다.예시 diff:
- uses: EnricoMi/publish-unit-test-result-action@v2 + uses: EnricoMi/publish-unit-test-result-action@v2.15.0 # 0e9f0b1 - uses: mikepenz/action-junit-report@v3 + uses: mikepenz/action-junit-report@v3.8.0 # a1b2c3d또한 YAMLlint 경고처럼 파일 끝에 개행을 추가해 주세요.
token: ${{ github.token }} +
🧹 Nitpick comments (1)
.github/workflows/ci-workflow.yml (1)
11-12: 워크플로 수준의 write 권한 부여가 과도할 수 있습니다.
checks: write,pull-requests: write권한은 주입된 액션(step)에서만 필요합니다.
보안 원칙(최소 권한 부여, PoLP)에 따라 워크플로 전체가 아닌 해당 스텝 또는 잡 수준으로 이동하는 것을 고려해 주세요.
필요 이상으로 넓은 권한을 열어두면 액션 공급망 위험이 커집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-workflow.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
🪛 YAMLlint (1.37.1)
.github/workflows/ci-workflow.yml
[error] 61-61: no new line character at the end of file
(new-line-at-end-of-file)
⏰ 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)
.github/workflows/ci-workflow.yml (1)
48-49: Gradle 빌드 플래그 추가 👍
--parallel --stacktrace옵션 추가로 빌드 속도와 디버깅 편의가 모두 향상됩니다. 특별한 이슈 없어 보입니다.
| private final BookMapper bookMapper; | ||
|
|
||
| @Override | ||
| public boolean existsSavedBookByUserIdAndBookId(Long userId, Long bookId) { |
| // Top 5 상세정보 저장 추가 | ||
| List<BookMostSearchResult.BookRankInfo> bookRankDetails = new ArrayList<>(); | ||
| int rank = 1; | ||
| AtomicInteger rank = new AtomicInteger(1); |
| // 삭제 요청 시 저장되어 있지 않으면 예외 발생 | ||
| savedBooks.validateCanDelete(book); | ||
| savedCommandPort.deleteBook(userId, book.getId()); | ||
| private void validateSaveBookAction(boolean isSaveRequest, boolean alreadySaved) { |
| import static konkuk.thip.common.exception.code.ErrorCode.*; | ||
|
|
||
| @Getter | ||
| public class SavedBooks { |
| import konkuk.thip.feed.adapter.out.mapper.FeedMapper; | ||
| import konkuk.thip.feed.adapter.out.persistence.repository.FeedJpaRepository; | ||
| import konkuk.thip.feed.adapter.out.persistence.repository.FeedTag.FeedTagJpaRepository; | ||
| import konkuk.thip.feed.adapter.out.persistence.repository.SavedFeedJpaRepository; |
| } catch (EntityNotFoundException e) { | ||
| return saveNewBookWithFromExternalApi(isbn); | ||
| } | ||
| return bookCommandPort.findByIsbn(isbn) |
hd0rable
left a comment
hd0rable
left a comment
There was a problem hiding this comment.
아니 리퀘스트 체인지 잘못눌럿어염.. 수고하셨습니다!! 리펙 잘 해주셔서 피드 저장 로직 수정할때 참고하겠씁니닷!!
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java (1)
9-13: CQRS 컨벤션 정렬: QueryPort가 도메인 엔티티(Book) 대신 조회 응답용 DTO/Projection을 반환하도록 변경 제안팀 컨벤션(학습 컨텍스트)에 따르면 QueryPort는 “조회 API의 response에 해당하는 데이터”를 반환하는 포트입니다. 현재
findSavedBooksByUserId,findJoiningRoomsBooksByUserId가List<Book>(도메인)을 반환하고 있어 CommandPort 성격과 혼재됩니다. 성능(N+1 방지)과 레이어 책임 분리를 위해 Query DTO(예:BookInfo) 또는 JPA Projection 반환으로 전환을 권장합니다.가능한 인터페이스 시그니처 개선 예시(diff):
-import konkuk.thip.book.domain.Book; +import konkuk.thip.book.application.port.in.dto.BookInfo; - List<Book> findSavedBooksByUserId(Long userId); - List<Book> findJoiningRoomsBooksByUserId(Long userId); + List<BookInfo> findSavedBookInfosByUserId(Long userId); + List<BookInfo> findJoiningRoomBookInfosByUserId(Long userId);또는 최소한 현재 시그니처 유지 시, 메서드 JavaDoc로 “ISBN 기준 중복 제거 보장” 및 “모집중/만료 제외” 등 결과 불변식을 명시해 주세요.
src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java (4)
23-26: 필드 주입 구성 적절하지만 User 로딩은 불필요할 수 있음
UserJpaRepository는 존재성 확인만 필요하므로 엔티티 로딩 대신existsById로 대체하면 비용을 줄일 수 있습니다(아래 메서드 코멘트 참고).
28-31: exists 구현은 간단·명확함 (OK)
SavedBookJpaRepository.existsByUserIdAndBookId위임은 적절합니다. 네이밍 가독성 측면에서isBookSavedByUser(userId, bookId)같은 도메인 용어 중심 메서드명도 고려해볼 수 있습니다(선택).
34-41: QueryPort에서 도메인 매핑 대신 Projection/DTO로 직접 조회 권장현재 어댑터에서 JPA 엔티티 → 도메인(Book) → 상위 레이어에서 DTO(BookInfo)로 이중 매핑됩니다. 조회 전용 경로에서는 JPA Projection 또는 Native Projection으로 바로
BookInfo를 반환하면:
- N+1 및 불필요한 도메인 조립/검증 비용 감소
- 포트 책임 분리(CQRS 컨벤션 준수)
예시(개념적):
BookJpaRepository에List<BookInfoProjection> find...추가BookQueryPort/Adapter에서List<BookInfo>반환(Mapper로 Projection→DTO 변환 또는 인터페이스 기반 DTO 직반환)Also applies to: 44-52
38-41: Stream 수집 방식은 OK. JDK 16+ 사용 시.toList()고려(선택)프로젝트 JDK가 16+라면
collect(Collectors.toList())대신.toList()로 간결화 가능합니다. JDK8 호환 필요 시 현 방식 유지.Also applies to: 48-52
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java(2 hunks)src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java
- src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java
- src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.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.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
📚 Learning: 2025-07-03T03:05:05.031Z
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.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.javasrc/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.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 (2)
src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java (1)
3-6: 임포트 추가 문제 없음현 변경과 일관됩니다.
src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java (1)
5-5: 임포트 추가 적절사용처와 일치합니다.
Also applies to: 7-18
| public List<Book> findSavedBooksByUserId(Long userId) { | ||
| UserJpaEntity user = userJpaRepository.findById(userId) | ||
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | ||
|
|
||
| return bookJpaRepository.findSavedBooksByUserId(user.getUserId()).stream() | ||
| .map(bookMapper::toDomainEntity) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
User 존재 검증 최적화 및 불필요한 엔티티 로딩 제거 제안
현재 findById로 User 엔티티를 로딩한 뒤 user.getUserId()만 사용하고 있습니다. 존재성만 필요하므로 existsById로 대체하여 쿼리/매핑 비용을 줄이세요.
적용 예시(diff):
- UserJpaEntity user = userJpaRepository.findById(userId)
- .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));
-
- return bookJpaRepository.findSavedBooksByUserId(user.getUserId()).stream()
- .map(bookMapper::toDomainEntity)
- .collect(Collectors.toList());
+ if (!userJpaRepository.existsById(userId)) {
+ throw new EntityNotFoundException(USER_NOT_FOUND);
+ }
+ return bookJpaRepository.findSavedBooksByUserId(userId).stream()
+ .map(bookMapper::toDomainEntity)
+ .collect(Collectors.toList());추가로, 도메인 매핑 시 연관 로딩으로 인한 N+1 위험이 있습니다. JPA 쿼리에서 필요한 연관만 fetch join하거나 Projection을 사용해 주세요(아래 검증 스크립트 참고).
📝 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.
| public List<Book> findSavedBooksByUserId(Long userId) { | |
| UserJpaEntity user = userJpaRepository.findById(userId) | |
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | |
| return bookJpaRepository.findSavedBooksByUserId(user.getUserId()).stream() | |
| .map(bookMapper::toDomainEntity) | |
| .collect(Collectors.toList()); | |
| } | |
| public List<Book> findSavedBooksByUserId(Long userId) { | |
| if (!userJpaRepository.existsById(userId)) { | |
| throw new EntityNotFoundException(USER_NOT_FOUND); | |
| } | |
| return bookJpaRepository.findSavedBooksByUserId(userId).stream() | |
| .map(bookMapper::toDomainEntity) | |
| .collect(Collectors.toList()); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java
around lines 34 to 41, replace the User entity loading via findById with
existsById to check user existence without loading the full entity, reducing
query and mapping overhead. Then, modify the JPA query in bookJpaRepository to
use fetch join or Projection to load only necessary associations and avoid N+1
query issues during domain mapping.
| public List<Book> findJoiningRoomsBooksByUserId(Long userId) { | ||
| UserJpaEntity user = userJpaRepository.findById(userId) | ||
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | ||
|
|
||
| return bookJpaRepository.findJoiningRoomsBooksByUserId(user.getUserId()) | ||
| .stream() | ||
| .map(bookMapper::toDomainEntity) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Join 방 책 조회도 동일 최적화 및 쿼리 조건 보장 검증 필요
- User 검증은
existsById로 최적화 가능(위와 동일). - 비즈니스 요건(모집중/만료 제외, ISBN 기준 중복 제거)이 repository 쿼리에서 보장되는지 확인이 필요합니다.
다음 스크립트로 해당 메서드 쿼리 정의를 찾아 상태 필터와 DISTINCT(ISBN 기준) 적용 여부를 확인해 주세요:
🏁 Script executed:
#!/bin/bash
# repository 메서드 정의 탐색
rg -n -A5 -B3 $'findSavedBooksByUserId\\(|findJoiningRoomsBooksByUserId\\('
# JPQL/Query 어노테이션 유무와 where 조건, distinct/Group by(ISBN) 사용 확인
# Projection 또는 fetch join 여부 확인Length of output: 7053
Join 방 책 조회 – User 검증 최적화 제안 및 쿼리 검증 완료
BookQueryPersistenceAdapter.java (44–52):
- User 존재 여부 확인은 실제 엔티티 조회가 아닌
existsById를 사용해 최적화할 수 있습니다. BookJpaRepository.findJoiningRoomsBooksByUserId쿼리에는 이미
•DISTINCT b
•r.status = 'ACTIVE'
•r.startDate <= CURRENT_TIMESTAMP
조건이 적용돼 있어 중복 제거 및 모집중/만료 방 제외 요건을 충족합니다.
제안되는 코드 수정 예시:
public List<Book> findJoiningRoomsBooksByUserId(Long userId) {
- UserJpaEntity user = userJpaRepository.findById(userId)
- .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));
-
- return bookJpaRepository.findJoiningRoomsBooksByUserId(user.getUserId())
+ if (!userJpaRepository.existsById(userId)) {
+ throw new EntityNotFoundException(USER_NOT_FOUND);
+ }
+ return bookJpaRepository.findJoiningRoomsBooksByUserId(userId)
.stream()
.map(bookMapper::toDomainEntity)
.collect(Collectors.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.
| public List<Book> findJoiningRoomsBooksByUserId(Long userId) { | |
| UserJpaEntity user = userJpaRepository.findById(userId) | |
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | |
| return bookJpaRepository.findJoiningRoomsBooksByUserId(user.getUserId()) | |
| .stream() | |
| .map(bookMapper::toDomainEntity) | |
| .collect(Collectors.toList()); | |
| } | |
| public List<Book> findJoiningRoomsBooksByUserId(Long userId) { | |
| if (!userJpaRepository.existsById(userId)) { | |
| throw new EntityNotFoundException(USER_NOT_FOUND); | |
| } | |
| return bookJpaRepository.findJoiningRoomsBooksByUserId(userId) | |
| .stream() | |
| .map(bookMapper::toDomainEntity) | |
| .collect(Collectors.toList()); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java
lines 44 to 52, optimize user existence validation by replacing the full entity
fetch with a call to userJpaRepository.existsById(userId) to check if the user
exists. If the user does not exist, throw EntityNotFoundException as before.
This avoids unnecessary entity loading while maintaining the same validation
logic. The rest of the method can remain unchanged since the query already
handles distinct books and active room conditions.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/main/java/konkuk/thip/book/adapter/in/web/response/BookSelectableListResponse.java (3)
5-5: 불변·널 안전성 강화 제안 (방어적 복사 + null 체크)응답 DTO가 외부에 그대로 노출되므로, 리스트에 대해 null 방지와 불변 보장을 권장합니다.
다음과 같이 수정해 주세요:
package konkuk.thip.book.adapter.in.web.response; import konkuk.thip.book.application.port.in.dto.BookSelectableResult; import java.util.List; +import java.util.Objects; public record BookSelectableListResponse( List<BookSelectableResult> bookList ) { public static BookSelectableListResponse of(List<BookSelectableResult> bookSelectableResults) { - return new BookSelectableListResponse(bookSelectableResults); + Objects.requireNonNull(bookSelectableResults, "bookSelectableResults must not be null"); + return new BookSelectableListResponse(List.copyOf(bookSelectableResults)); } }Also applies to: 10-12
7-12: 웹 응답이 애플리케이션 DTO에 직접 의존현재 web 응답이 application 계층 DTO(BookSelectableResult)를 그대로 노출합니다. 외부 API 스키마를 애플리케이션 내부 모델 변화로부터 분리하려면, web 전용 Response DTO를 두고 매핑하는 방식을 고려해 주세요.
8-8: 필드명 컨벤션 확인필드명
bookList가 기존 API 응답 컨벤션(books,items등)과 일치하는지 확인 바랍니다. 변경 필요 시 지금 맞춰 두는 편이 추후 호환성 이점이 있습니다.src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java (2)
28-28: 매퍼 메서드 네이밍 일관성
toBookInfoList는 반환 타입이BookSelectableResult여서 혼동 소지가 있습니다.toSelectableResultList등 의미를 직접 드러내는 이름으로 변경을 검토해 주세요.
22-29: DB 부하 완화(선택): 캐싱 고려유저별/타입별로 동일 요청이 빈번하다면
@Cacheable(cacheNames="bookSelectableList", key="#bookSelectableType.name() + ':' + #userId")적용을 검토할 수 있습니다. 단, “저장/해제” 변화가 잦다면 캐시 무효화 전략을 반드시 함께 설계해야 합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/konkuk/thip/book/adapter/in/web/response/BookSelectableListResponse.java(1 hunks)src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java(2 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSelectableListUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookSelectableResult.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/konkuk/thip/book/application/port/in/dto/BookSelectableResult.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/book/application/port/in/BookSelectableListUseCase.java
- src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#166
File: src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java:70-82
Timestamp: 2025-08-07T18:19:55.856Z
Learning: seongjunnoh는 Clean Architecture 원칙을 중시하며, 어댑터는 순수하게 저장/조회 기능만 담당하고 비즈니스 로직은 서비스 레이어에서 처리하는 것을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#166
File: src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java:70-82
Timestamp: 2025-08-07T18:19:55.856Z
Learning: seongjunnoh는 Clean Architecture 원칙을 중시하며, 어댑터는 순수하게 저장/조회 기능만 담당하고 비즈니스 로직은 서비스 레이어에서 처리하는 것을 선호한다.
⏰ 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
| import org.springframework.stereotype.Service; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class BookSelectableListService implements BookSelectableListUseCase { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
읽기 전용 트랜잭션 적용
조회 전용 서비스이므로 readOnly 트랜잭션을 적용해 JPA 플러시 방지 및 성능/의도 명확화를 권장합니다(지연 로딩 시에도 안전).
다음과 같이 적용해 주세요:
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
@Service
@RequiredArgsConstructor
+@Transactional(readOnly = true)
public class BookSelectableListService implements BookSelectableListUseCase {📝 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.
| import org.springframework.stereotype.Service; | |
| import java.util.List; | |
| @Service | |
| @RequiredArgsConstructor | |
| public class BookSelectableListService implements BookSelectableListUseCase { | |
| import lombok.RequiredArgsConstructor; | |
| import org.springframework.stereotype.Service; | |
| import org.springframework.transaction.annotation.Transactional; | |
| import java.util.List; | |
| @Service | |
| @RequiredArgsConstructor | |
| @Transactional(readOnly = true) | |
| public class BookSelectableListService implements BookSelectableListUseCase { | |
| // ... existing implementation ... | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java
around lines 10 to 16, the service class lacks a read-only transaction
annotation. Add @Transactional(readOnly = true) above the class declaration to
apply a read-only transaction, which prevents JPA flushes and clarifies the
intent for this query-only service.
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import java.util.List; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
입력 파라미터 null 검증 추가
bookSelectableType, userId에 대한 사전 검증이 없으면 NPE/잘못된 호출이 런타임으로 넘어갑니다. 초기에 명확히 실패시키는 편이 좋습니다.
import java.util.List;
+import java.util.Objects;
@Override
public List<BookSelectableResult> getSelectableBookList(BookSelectableType bookSelectableType, Long userId) {
+ Objects.requireNonNull(bookSelectableType, "bookSelectableType must not be null");
+ Objects.requireNonNull(userId, "userId must not be null");
List<Book> bookList = switch(bookSelectableType) {
case SAVED -> bookQueryPort.findSavedBooksByUserId(userId);
case JOINING -> bookQueryPort.findJoiningRoomsBooksByUserId(userId);
};Also applies to: 22-23
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java
around lines 12 and 22-23, add null checks for the input parameters
bookSelectableType and userId at the beginning of the relevant methods. If
either parameter is null, throw an appropriate exception such as
IllegalArgumentException to fail fast and prevent potential
NullPointerExceptions or invalid calls later in runtime.
| public List<BookSelectableResult> getSelectableBookList(BookSelectableType bookSelectableType, Long userId) { | ||
| List<Book> bookList = switch(bookSelectableType) { | ||
| case SAVED -> bookQueryPort.findSavedBooksByUserId(userId); | ||
| case JOINING -> bookQueryPort.findJoiningRoomsBooksByUserId(userId); | ||
| }; | ||
|
|
||
| return bookQueryMapper.toBookInfoList(bookList); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
중복 제거와 상태 필터링이 실제로 보장되는지 확인 필요
현재 계층 어디에서 ISBN 중복 제거와 “JOINING → 만료/모집중 제외”가 최종 보장되는지 점검 바랍니다. 아래 스크립트로 관련 쿼리/메서드 흔적을 빠르게 확인할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# 레포 전체에서 관련 메서드/쿼리 탐색
set -e
echo "== Port/Adapter 메서드 정의/사용 위치 확인 =="
rg -n "findSavedBooksByUserId|findJoiningRoomsBooksByUserId"
echo
echo "== JPA 쿼리에서 DISTINCT/ISBN 기준 처리/상태 필터링 여부 탐색 =="
rg -n "DISTINCT|isbn|ISBN|status|recruit|expired|expire|Recruit|JOINING|SAVED"
echo
echo "== SavedBook / Book 관련 레포지토리 시그니처 재확인 =="
fd -t f -e java "SavedBook.*Repository|BookJpaRepository" | xargs -I{} sh -c 'echo "\n--- {} ---"; sed -n "1,200p" "{}"'Length of output: 163356
BookJpaRepository에서 중복 제거 및 상태 필터링 처리 확인 필요
findSavedBooksByUserId
• JPQL의SELECT DISTINCT b로 ISBN 기준 중복 제거가 보장됩니다.findJoiningRoomsBooksByUserId
• 동일하게SELECT DISTINCT b로 중복 제거
•WHERE r.status = 'ACTIVE' AND r.startDate <= CURRENT_TIMESTAMP로 모집 중(미시작) 방은 제외
• 그러나 만료된 방(endDate < 현재 시각)에 대한 필터링(r.endDate >= CURRENT_TIMESTAMP)이 누락되어 있어, 의도대로 “만료된 방 제외”를 완전히 보장하지 않습니다.
→ 비즈니스 요구사항상 만료된 방을 제외해야 한다면, 아래 조건을 쿼리에 추가해주세요.
AND r.endDate >= CURRENT_TIMESTAMP -- 만료된 방 제외🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java
around lines 22 to 29, the query method findJoiningRoomsBooksByUserId in
BookJpaRepository lacks filtering for expired rooms. To fix this, update the
JPQL query in findJoiningRoomsBooksByUserId to include the condition "AND
r.endDate >= CURRENT_TIMESTAMP" to exclude expired rooms as per business
requirements.
| List<Book> bookList = switch(bookSelectableType) { | ||
| case SAVED -> bookQueryPort.findSavedBooksByUserId(userId); | ||
| case JOINING -> bookQueryPort.findJoiningRoomsBooksByUserId(userId); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
비즈니스 규칙의 계층 배치: ISBN 중복 제거 및 JOINING 필터링은 서비스에서
PR 설명대로 “ISBN 기준 중복 제거”와 “JOINING 시 만료/모집중 제외”는 비즈니스 규칙입니다. 팀 러닝(어댑터는 순수 저장/조회 선호)에 맞춰, 중복 제거/필터링은 서비스에서 보장하고 어댑터는 단순 조회에 집중하도록 권장합니다.
다음과 같이 서비스에서 중복 제거를 한 번 더 안전망으로 적용할 수 있습니다(어댑터/쿼리 레벨에서 처리하더라도 서비스에서 최종 보장):
import konkuk.thip.book.domain.Book;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import java.util.List;
+import java.util.LinkedHashMap;
+import java.util.function.Function;
+import java.util.stream.Collectors;
...
List<Book> bookList = switch (bookSelectableType) {
case SAVED -> bookQueryPort.findSavedBooksByUserId(userId);
case JOINING -> bookQueryPort.findJoiningRoomsBooksByUserId(userId);
};
- return bookQueryMapper.toBookInfoList(bookList);
+ // ISBN 기준 안정적 중복 제거(첫 항목 유지)
+ List<Book> dedupedByIsbn = bookList.stream()
+ .filter(b -> b.getIsbn() != null)
+ .collect(Collectors.toMap(
+ Book::getIsbn,
+ Function.identity(),
+ (a, b) -> a, // 충돌 시 첫 항목 유지
+ LinkedHashMap::new
+ ))
+ .values()
+ .stream()
+ .toList();
+
+ return bookQueryMapper.toBookInfoList(dedupedByIsbn);참고: 만료/모집중 제외 로직이 현재 어댑터/쿼리에 있다면, 서비스에서도 한 번 더 검증하거나 도메인 정책 메서드로 의도를 드러내면 유지보수성이 올라갑니다.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java
around lines 23 to 26, the current code retrieves books directly from the
adapter with filtering and deduplication logic possibly embedded there. To align
with the team's architecture, move the ISBN deduplication and JOINING status
filtering logic into the service layer to enforce business rules clearly. After
fetching the list from the adapter, apply a distinct filter on ISBNs and exclude
expired or non-recruiting JOINING books within the service method before
returning the result. This ensures the service layer guarantees business rules
regardless of adapter implementation.
|
|
||
| @Override | ||
| public Set<Book> findSavedBooksByUserId(Long userId) { | ||
| public List<Book> findSavedBooksByUserId(Long userId) { |
| public static BookSelectableListResponse of(List<BookSelectableListResult> bookSelectableListResults) { | ||
| return new BookSelectableListResponse(bookSelectableListResults); |
There was a problem hiding this comment.
use case 에서 반환하는 result dto와 controller가 api 의 결과로 반환하는 response dto 를 분리하셨군요!
저희 처음 코드 보는거 같네요ㅎ 좋습니다!
| BookSelectableListResult toBookInfo(Book book); | ||
|
|
||
| List<BookInfo> toBookInfoList(List<Book> books); | ||
| List<BookSelectableListResult> toBookInfoList(List<Book> books); |
There was a problem hiding this comment.
p2 : 이제 BookSelectableListResult 를 공통 dto가 아니라 '선택할 수 있는 책 조회 api' 전용 result 로 정의하였으니, 메서드 네이밍을 수정하는 것은 어떻게 생각하시나요??
There was a problem hiding this comment.
맞네요! dto 이름은 바꿨는데 메서드는 깜빡한 것 같습니다!! 수정하겠습니다~
| private final BookQueryMapper bookQueryMapper; | ||
|
|
||
| @Override | ||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
조회 서비스에는 읽기 전용 트랜잭션을 다는 컨벤션에 따라 추가해주셨군요!
단순 도메인 조회할때는 성능이슈로 인해 트랜잭션을 안 거는게 좋다고 생각하지만, 저희 jpa entity 상에 연관관계가 많이 존재하니 이 부분은 포기해야 겟네요,,
다른 방법이 있을 수 있으니 한번 고민해 보면 좋을것같습니다!
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
저장한 책 또는 참여 중 모임의 책 조회 api 구현
책 저장 상태 변경 로직 수정
회의에서 말했던 것처럼 SavedBooks라는 일급 컬렉션을 제거하고 existsBy를 통해 저장 검증을 하도록 했습니다. 이때 validate는 도메인 로직보다 서비스 로직에 가까운 것 같아. 서비스 로직에 메서드를 정의했습니다. (Book이라는 도메인이 저장 여부 상태를 담고있는 것은 아니기 때문)
Optional 반환을 통한 try-catch 제거
Optional 반환을 하는 findByIsbn 메서드를 통해 기존 서비스 로직에서 try catch를 하는 로직들을 모두 리팩토링하였습니다.
Saved 패키지 제거
기존 SavedBook과 SavedFeed는 각각 Book과 Feed에 종속되는 엔티티라고 판단되어 Book과 Feed 패키지 하위로 이동시켰습니다. 이에 따라 관련 저장 및 조회 로직도 이동했습니다. (Feed의 adapter가 조금 무거워지는 느낌은 드는데 추후에 Content 테이블을 제거하고 리팩토링하면 쿼리가 더 간단해질 것 같다고 생각합니다!)
📸 스크린샷
💬 리뷰 요구사항
이번 api에서는 페이징 처리가 따로 필요 없어 Book도메인을 그대로 사용했습니다. 이에 따라서 서비스 로직에서는 port.dto에 정의되어 있는 BookInfo의 리스트를 반환하고 컨트롤러 계층에서 Response를 생성하는 책임을 갖도록 했습니다.
BookInfo는 책에 대한 정보를 담고 있는 dto이기 때문에 현재 BookSearchListResponse의 inner class로 사용되고 있는 BookDto를 대체할 수 있을 것 같습니다. (책 검색은 페이징 변수를 다 제거해도 될 것 같다고 생각,, <- 기획적으로)
추가적으로, 구현하면서 들었던 리팩토링에 대한 제안은 노션 -> 서버 리팩토링 회의 밑에 적어두었으니 한번씩 확인부탁드립니다!!
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
/books/selectable-listAPI가 추가되었습니다.버그 수정
리팩터 및 구조 개선
테스트
문서화