-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 내서재 동적검색 기능 구현 #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 Walkthrough""" Walkthrough서재에 등록된 도서를 동적으로 검색, 필터, 정렬, 페이징할 수 있도록 전체 API, 도메인, 인프라 계층에 걸쳐 확장되었습니다. QueryDSL 지원이 추가되고, 관련 엔티티의 타임스탬프 필드가 nullable로 변경되었으며, 저자/출판사 문자열 정제 유틸이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant UseCase
participant Service
participant DomainService
participant Repository
Client->>Controller: GET /my-library?status&sort&page
Controller->>UseCase: getUserLibraryBooks(userId, status, sort, pageable)
UseCase->>Service: findUserBooksByDynamicConditionWithStatusCounts(userId, status, sort, pageable)
Service->>DomainService: findUserBooksByDynamicCondition(userId, status, sort, pageable)
DomainService->>Repository: findUserBooksByDynamicCondition(userId, status, sort, pageable)
Repository-->>DomainService: Page<UserBook>
DomainService-->>Service: Page<UserBookInfoVO>
Service->>DomainService: getUserBookStatusCounts(userId)
DomainService->>Repository: countUserBooksByStatus(userId, status)
Repository-->>DomainService: Long
DomainService-->>Service: UserBookStatusCountsVO
Service-->>UseCase: UserBookPageResponse
UseCase-->>Controller: UserBookPageResponse
Controller-->>Client: 200 OK + UserBookPageResponse
Estimated code review effort3 (~45 minutes) Possibly related PRs
✨ 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (32)
apis/src/main/kotlin/org/yapp/apis/ApisApplication.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookPageResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookResponse.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/BookManagementService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt(1 hunks)build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/Dependencies.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/book/Book.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/user/User.kt(6 hunks)domain/src/main/kotlin/org/yapp/domain/user/UserDomainService.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt(4 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(3 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookInfoVO.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookStatusCountsVO.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt(1 hunks)infra/build.gradle.kts(2 hunks)infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/common/BaseTimeEntity.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/querydsl/QuerydslConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/entity/UserBookEntity.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookQuerydslRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (2)
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (1)
countUserBooksByStatus(49-51)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (1)
countUserBooksByStatus(25-25)
🪛 detekt (1.23.8)
infra/src/main/kotlin/org/yapp/infra/config/internal/querydsl/QuerydslConfig.kt
[warning] 10-11: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt
[warning] 19-20: The class or object JpaConfig is empty.
(detekt.empty-blocks.EmptyClassBlock)
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt
[warning] 34-34: In most cases using a spread operator causes a full copy of the array to be created before calling a method. This may result in a performance penalty.
(detekt.performance.SpreadOperator)
⏰ 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-validation
🔇 Additional comments (47)
buildSrc/src/main/kotlin/Dependencies.kt (2)
11-12: 코드 포맷팅 개선이 적절합니다.상수 선언을 여러 줄로 나누어 가독성을 향상시켰습니다.
56-59: QueryDSL 의존성 추가가 적절합니다.동적 검색 기능 구현을 위한 QueryDSL 5.0.0 버전과 jakarta 분류자 사용이 현대적인 Spring Boot 프로젝트에 적합합니다.
domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookStatusCountsVO.kt (2)
6-15: 값 객체 설계가 우수합니다.private 생성자와 init 블록의 유효성 검증을 통해 데이터 무결성을 보장하고 있습니다. 음수 카운트 방지는 비즈니스 로직에 적합합니다.
17-37: 팩토리 메서드들이 잘 설계되었습니다.명시적 매개변수 방식과 Map 기반 방식 두 가지 팩토리 메서드를 제공하여 유연성을 확보했습니다. Map에서 누락된 상태에 대한 기본값 처리(?: 0L)도 적절합니다.
infra/src/main/kotlin/org/yapp/infra/common/BaseTimeEntity.kt (1)
1-1: 패키지 구조 개선이 적절합니다.JPA 특화 어노테이션들(@MappedSuperclass, @EntityListeners)을 포함한 BaseTimeEntity를 infra 패키지로 이동하여 도메인과 인프라 관심사를 더 명확히 분리했습니다. 클린 아키텍처 원칙에 부합합니다.
apis/src/main/kotlin/org/yapp/apis/book/service/BookManagementService.kt (1)
17-17: 중요한 버그 수정입니다.이전에는
request.validAuthor()가 중복 호출되어 출판사 정보가 올바르게 전달되지 않았습니다.request.validPublisher()로 수정하여 책 메타데이터가 정확하게 처리되도록 했습니다.infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt (2)
7-7: QueryDSL 설정 클래스 import가 적절합니다.새로운 QueryDSL 설정을 위한 필요한 import 추가입니다.
15-16: 인프라 설정 그룹에 QueryDSL 추가가 적절합니다.기존 설정 그룹들과 동일한 패턴을 따르며, 동적 검색 기능을 위한 QueryDSL 설정을 인프라 레이어에 통합했습니다.
infra/build.gradle.kts (2)
3-5: QueryDSL 설정을 위한 KAPT 플러그인 추가가 적절합니다.QueryDSL 어노테이션 처리를 위해 필요한 KAPT 플러그인이 올바르게 추가되었습니다.
24-26: QueryDSL 버전 정의 확인 필요buildSrc 디렉터리 내에서 QueryDSL 버전 설정이 발견되지 않아 자동 검증이 불가합니다. 아래 사항을 직접 확인해주세요:
buildSrc/src/main/kotlin/Dependencies.kt(또는 유사 파일)에서 QueryDSL 버전 상수 위치 및 이름 확인- 확인된 버전이 현재 사용 중인 Spring Boot 버전과 호환되는지 검토
apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt (1)
13-15: QUERY_DSL 상수 정의 확인 완료 및 변경사항 승인InfraBaseConfigGroup enum에
QUERY_DSL(QuerydslConfig::class.java)상수가 정의되어 있음을 확인했습니다. 변경사항을 승인합니다.infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt (1)
7-7: 검증 완료: QueryDSL 리포지토리 확장
JpaUserBookQuerydslRepository 인터페이스가 정상적으로 정의되어 있으며, 동적 쿼리 기능이 올바르게 적용되었습니다. 변경 사항을 승인합니다.infra/src/main/kotlin/org/yapp/infra/userbook/entity/UserBookEntity.kt (2)
6-6: BaseTimeEntity import 경로 변경이 적절합니다.도메인 계층에서 인프라 계층으로 BaseTimeEntity 이동이 올바르게 반영되었습니다.
71-82: ✅ JPA Auditing 설정 확인 완료
- infra/src/main/kotlin/org/yapp/infra/common/BaseTimeEntity.kt
• @MappedSuperclass
• @CreatedDate, @LastModifiedDate 필드 적용- infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt
• @EnableJpaAuditing 활성화명시적 타임스탬프 할당 제거 후 BaseTimeEntity의 자동 처리 활용이 정상 동작함을 확인했습니다.
자동 타임스탬프 처리 변경을 승인합니다.infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt (2)
10-10: BaseTimeEntity import 경로 변경이 일관성 있게 적용되었습니다.UserBookEntity와 동일한 패턴으로 BaseTimeEntity가 인프라 계층으로 일관성 있게 이동되었습니다.
69-78: 타임스탬프 자동 처리 개선이 일관성 있게 적용되었습니다.다른 엔티티들과 동일하게 명시적 타임스탬프 할당을 제거하여 BaseTimeEntity의 JPA Auditing 기능을 활용하도록 개선되었습니다.
apis/src/main/kotlin/org/yapp/apis/ApisApplication.kt (1)
21-21: 페이지네이션 지원을 위한 적절한 설정
VIA_DTO모드로 설정한 것이 새로운 페이지네이션 기능과 잘 맞습니다. API 응답에서 페이지 메타데이터가 DTO 형태로 직렬화되어 일관된 응답 구조를 제공할 수 있습니다.infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt (2)
7-7: BaseTimeEntity 위치 이동 승인
BaseTimeEntity를 infra 패키지로 이동한 것은 적절한 아키텍처 개선입니다. 인프라 계층의 공통 기능이므로 infra 패키지에 위치하는 것이 맞습니다.
76-76: 타임스탬프 자동 관리로 개선
fromDomain메서드에서 타임스탬프 할당을 제거한 것은 JPA Auditing 적용으로 인한 좋은 변화입니다. 수동 관리에서 자동 관리로 전환하여 일관성과 안정성이 향상됩니다.infra/src/main/kotlin/org/yapp/infra/config/internal/querydsl/QuerydslConfig.kt (1)
1-17: QueryDSL 설정이 올바르게 구현됨JPAQueryFactory 빈 생성을 위한 표준적인 QueryDSL 설정입니다. @PersistenceContext를 통한 EntityManager 주입과 JPAQueryFactory 빈 생성이 적절하게 구현되어 있습니다.
정적 분석 도구의 lateinit 경고는 이 경우 거짓 양성입니다. Spring의 @PersistenceContext와 함께 사용하는 lateinit은 표준적이고 안전한 패턴입니다.
infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt (1)
14-20: JPA 감사 기능 활성화가 올바르게 구현되었습니다.
@EnableJpaAuditing어노테이션 추가로 JPA 엔티티의 생성/수정 시간 자동 관리가 활성화되었습니다. 빈 클래스 블록에 대한 정적 분석 경고는 이 경우 의도된 것으로, 설정 클래스의 역할이 어노테이션을 통한 기능 활성화이므로 문제가 없습니다.apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookPageResponse.kt (1)
6-36: 페이지네이션 응답 DTO가 잘 설계되었습니다.다음과 같은 좋은 설계 패턴을 적용했습니다:
- private 생성자와 companion object 팩토리 메서드로 객체 생성 제어
- Swagger 어노테이션으로 API 문서화
- 명확한 필드명으로 읽기 상태별 개수 표현
- Spring Data의 Page 타입 활용으로 표준 페이지네이션 지원
코드 구조와 명명이 직관적이고 유지보수하기 좋습니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookResponse.kt (1)
30-33: 데이터 정제 로직이 적절하게 적용되었습니다.
BookDataValidator를 사용하여 저자명과 출판사명에서 괄호와 내용을 제거하는 것은 API 응답의 데이터 품질을 향상시키는 좋은 개선사항입니다. 응답 DTO에서 일관되게 적용되어 사용자에게 깔끔한 정보를 제공할 수 있습니다.global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt (1)
5-5: 정규식 패턴이 올바르게 구현되었습니다.괄호와 내용을 제거하는 정규식
\\s*\\([^)]*\\)\\s*이 정확하게 작성되었습니다. 괄호 앞뒤의 공백까지 제거하여 깔끔한 결과를 제공합니다.domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (1)
18-26: 동적 검색을 위한 저장소 인터페이스가 잘 설계되었습니다.새로 추가된 메서드들이 Spring Data의 관례를 잘 따르고 있으며, 페이징과 정렬을 지원하는 적절한 시그니처를 가지고 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (2)
50-76: 동적 검색 서비스 메서드들이 잘 구현되었습니다.두 개의 새로운 메서드가 적절하게 도메인 서비스에 위임하고 있으며, 페이징과 상태별 카운트를 함께 제공하는 조합 메서드가 잘 설계되었습니다.
27-28: 파라미터 순서 일관성 확인 완료
Service 레이어의upsertUserBook호출부와 Domain 레이어 정의에서bookAuthor,bookPublisher순서가 일치합니다. 추가 검토나 수정은 필요 없습니다.domain/src/main/kotlin/org/yapp/domain/book/Book.kt (2)
50-51: reconstruct 메서드의 nullable 파라미터 변경이 일관성 있게 적용되었습니다.타임스탬프 필드의 nullable 변경이 factory 메서드에도 올바르게 반영되었습니다.
17-18: JPA Auditing 적용 및 nullable 타임스탬프 변경 안전 확인
- infra/src/main/kotlin/org/yapp/infra/common/BaseTimeEntity.kt
• @CreatedDate, @LastModifiedDate 어노테이션으로 timestamp 자동 관리- infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt
• @EnableJpaAuditing 설정 완료- infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt
• BaseTimeEntity 상속으로 createdAt/updatedAt이 pre-persist/pre-update 시점에 non-null 로 채워짐- 도메인 → VO/API 매핑
• toDomain() → Book.reconstruct(…, createdAt, updatedAt)
• UserBookInfoVO 및 UserBookResponse에서는 non-null 값(!!) 또는 VO(LocalDateTime) 호출(.format)만 사용위 검증 결과, 기존 코드가 non-null 타임스탬프를 기대하는 지점은 모두 JPA Auditing 이후 엔티티/VO 수준에서 처리되고 있으므로 추가 조치가 필요 없습니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookQuerydslRepository.kt (1)
9-21: QueryDSL 저장소 인터페이스가 잘 설계되었습니다.동적 쿼리를 위한 인터페이스가 명확하고 적절한 시그니처를 가지고 있습니다. Spring Data의 관례를 잘 따르고 있으며, 인프라 계층에 적합한 엔티티 타입을 사용하고 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (1)
63-71: API 엔드포인트가 동적 검색 기능을 잘 지원하도록 개선되었습니다.페이징, 필터링, 정렬을 지원하는 적절한 파라미터들이 추가되었고, 기본값 설정도 합리적입니다.
domain/src/main/kotlin/org/yapp/domain/user/UserDomainService.kt (1)
31-34: 메서드 파라미터 포맷팅 개선멀티라인 파라미터 포맷팅으로 가독성이 향상되었습니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (2)
39-47: 동적 검색 기능 구현 승인페이징과 필터링을 지원하는 동적 쿼리 메서드가 올바르게 구현되었습니다. JPA repository에 위임하고 도메인 객체로 변환하는 패턴이 일관성 있게 적용되었습니다.
49-51: 상태별 개수 조회 메서드 승인사용자 도서의 상태별 개수를 조회하는 메서드가 깔끔하게 구현되었습니다. 단순한 위임 패턴으로 책임이 명확히 분리되어 있습니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (2)
43-51: 동적 조건 검색 도메인 서비스 승인페이징과 필터링을 지원하는 도메인 서비스 메서드가 올바르게 구현되었습니다. Repository 계층에 적절히 위임하고 도메인 VO로 변환하는 패턴이 일관됩니다.
72-74: 헬퍼 메서드 구현 승인상태별 개수 조회를 위한 private 헬퍼 메서드가 적절하게 구현되었습니다. 재사용성과 가독성이 향상되었습니다.
domain/src/main/kotlin/org/yapp/domain/user/User.kt (3)
31-32: JPA Auditing 전환을 위한 타임스탬프 필드 변경 승인createdAt과 updatedAt 필드를 nullable로 변경하여 JPA Auditing에 의한 자동 관리를 지원하도록 개선되었습니다. 도메인 모델이 인프라 관심사에서 분리되어 더 깔끔해졌습니다.
39-39: restore 메서드 단순화 승인사용자 복원 시 updatedAt 필드를 명시적으로 설정하지 않고 JPA Auditing에 위임하도록 변경되었습니다. 비즈니스 로직이 더 명확해졌습니다.
49-49: factory 메서드 타임스탬프 파라미터 제거 승인create, createWithRole, reconstruct 메서드에서 타임스탬프 관련 파라미터가 적절히 제거되었습니다. 도메인 객체 생성이 더 간결해지고 JPA Auditing과의 책임 분리가 명확해졌습니다.
Also applies to: 58-58, 69-69, 78-78, 90-91
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (4)
17-18: JPA Auditing 적용에 따른 타임스탬프 필드 변경이 적절합니다.
createdAt과updatedAt필드를 nullable로 변경하고 기본값을 null로 설정한 것은 JPA Auditing 기능을 활용하기 위한 적절한 변경입니다. 이를 통해 인프라 계층에서 자동으로 타임스탬프를 관리할 수 있게 됩니다.
26-45: 팩토리 메서드의 status 매개변수 필수화가 적절합니다.
create메서드에서status매개변수의 기본값을 제거하여 명시적으로 상태를 지정하도록 한 것은 도메인 모델의 명확성을 높이는 좋은 변경입니다. 또한 타임스탬프 초기화를 제거하여 JPA Auditing에 위임한 것도 적절합니다.
47-73: reconstruct 메서드의 nullable 타임스탬프 처리가 일관성 있습니다.
reconstruct메서드에서 타임스탬프 매개변수들을 nullable로 변경하고 기본값을 null로 설정한 것은 JPA 엔티티에서 도메인 객체로 변환할 때 유연성을 제공합니다. 기존 데이터나 새로 생성된 데이터 모두를 적절히 처리할 수 있게 됩니다.
21-23: updatedAt 업데이트 처리 검토 필요
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt의updateStatus메서드에서updatedAt필드 업데이트 로직이 제거되었습니다.
JPA Auditing(@EnableJpaAuditing,@LastModifiedDate)가 적용되어 있으면 자동으로 처리되지만, 도메인 관점에서 상태 변경이 수정 시점을 의미하는지 반드시 확인해야 합니다.아래 사항을 검토해 주세요:
- 프로젝트 전반에 JPA Auditing이 활성화되어 있는지 확인
UserBook엔티티에@LastModifiedDate및@EntityListeners(AuditingEntityListener::class)가 적용되어 있는지 확인- 상태 변경이 실제로
updatedAt갱신을 의미하는지 비즈니스 로직 관점에서 재검토infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt (4)
16-21: QueryDSL 구현 클래스의 기본 구조가 적절합니다.생성자 주입을 통한
JPAQueryFactory의존성 주입과 Q클래스 인스턴스 초기화가 QueryDSL의 표준 패턴을 잘 따르고 있습니다.
51-63: 상태별 카운트 조회 메서드가 효율적으로 구현되었습니다.단순한 카운트 쿼리로 성능이 우수하며, null 안전성도 적절히 처리되었습니다. 상태별 통계 정보 제공에 적합한 구현입니다.
65-67: 조건부 필터링 유틸리티 메서드가 깔끔합니다.
let함수를 사용한 null-safe 조건 생성이 Kotlin다운 코드 스타일을 잘 보여줍니다.
22-49: 성능 최적화 검토: N+1, 카운트 쿼리 최적화 및 인덱스 확인동적 검색 구현은 잘 되어 있으나, 실제 운영환경에서 성능 저하를 예방하기 위해 아래 사항을 검토해주세요.
- N+1 문제 방지
연관 엔티티가 있을 경우, 필요에 따라 fetch join 또는@EntityGraph사용을 고려- 카운트 쿼리 최적화
복잡한 필터링·조인이 많아질 경우, 별도의 카운트 전용 쿼리로 분리하여 성능 저하 방지- 인덱스 활용
JPA 엔티티(@Table(indexes = …)) 또는 데이터베이스 마이그레이션 파일에서
user_book테이블의user_id,status,created_at컬럼에 복합 인덱스가 정의되어 있는지 확인필요 시 엔티티 설정 또는 마이그레이션 스크립트에 인덱스 구성을 추가/검증해주세요.
| val response = bookUseCase.getUserLibraryBooks(userId) | ||
| @AuthenticationPrincipal userId: UUID, | ||
| @RequestParam(required = false) status: BookStatus?, | ||
| @RequestParam(required = false) sort: String?, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
정렬 파라미터에 대한 검증 추가를 고려해보세요.
sort 파라미터가 String 타입으로 받아지고 있는데, 허용되는 정렬 필드에 대한 검증을 추가하는 것이 좋겠습니다. 이를 통해 잘못된 필드명으로 인한 오류를 방지할 수 있습니다.
다음과 같은 검증 로직을 추가할 수 있습니다:
@RequestParam(required = false) sort: String?,를
@RequestParam(required = false)
@Pattern(regexp = "^(createdAt|updatedAt|title)$", message = "정렬 필드는 createdAt, updatedAt, title 중 하나여야 합니다")
sort: String?,로 변경하거나, enum을 사용하는 방법도 고려해볼 수 있습니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt at line
65, the sort parameter lacks validation for allowed values. Add validation by
annotating the sort parameter with a pattern constraint that restricts input to
"createdAt", "updatedAt", or "title". Alternatively, replace the String type
with an enum representing these allowed fields to enforce validation at the type
level.
| fun getUserLibraryBooks( | ||
| @AuthenticationPrincipal userId: UUID, | ||
| @RequestParam(required = false) status: BookStatus?, | ||
| @RequestParam(required = false) sort: String?, | ||
| @PageableDefault(size = 10, sort = ["createdAt"], direction = Sort.Direction.DESC) | ||
| pageable: Pageable | ||
| ): ResponseEntity<UserBookPageResponse> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
동적 검색 API 인터페이스 개선 승인
사용자 서재 조회 API가 필터링, 정렬, 페이징을 지원하도록 확장되었습니다. 파라미터 설계와 기본값 설정이 적절합니다.
기본 페이지 크기(10)와 정렬 기준(createdAt DESC)이 사용자 경험에 적합한지 검토해보세요.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
around lines 140 to 146, the getUserLibraryBooks function has been updated to
support filtering, sorting, and paging with default page size 10 and sorting by
createdAt descending. Review and confirm that the default page size of 10 and
sorting by createdAt DESC are appropriate for the user experience; if needed,
adjust the @PageableDefault annotation parameters to better fit expected usage.
| fun getUserLibraryBooks( | ||
| userId: UUID, | ||
| status: BookStatus?, | ||
| sort: String?, | ||
| pageable: Pageable | ||
| ): UserBookPageResponse { | ||
| userAuthService.validateUserExists(userId) | ||
|
|
||
| return userBookService.findAllUserBooks(userId) | ||
| return userBookService.findUserBooksByDynamicConditionWithStatusCounts(userId, status, sort, pageable) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
동적 검색 기능이 올바르게 구현되었습니다.
다음과 같은 좋은 점들이 있습니다:
- 사용자 존재 검증 유지
- 선택적 필터링과 정렬 파라미터 지원
- 서비스 계층으로의 적절한 위임
- 페이지네이션 응답 타입 사용
다만 sort 파라미터에 대한 유효성 검증을 고려해보세요. 허용되지 않는 정렬 필드가 전달될 경우를 대비한 검증 로직이 있으면 더 안전할 것 같습니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt around lines
73 to 82, the getUserLibraryBooks function accepts a sort parameter without
validation. To fix this, add validation logic to check if the provided sort
value is among the allowed sorting fields before passing it to the service. If
the sort parameter is invalid, handle it appropriately by either defaulting to a
safe value or throwing a validation exception to ensure only valid sort criteria
are processed.
| tasks.withType<org.jetbrains.kotlin.gradle.tasks.KaptGenerateStubs>().configureEach { | ||
| doFirst { | ||
| delete(querydslDir) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
KAPT 태스크 설정 확인
KAPT 실행 전 디렉토리 정리 로직은 적절하지만, 너무 공격적일 수 있습니다. 증분 빌드 최적화를 고려하여 필요한 경우에만 정리하는 것을 검토해보세요.
🤖 Prompt for AI Agents
In build.gradle.kts around lines 94 to 98, the current KAPT task configuration
deletes the querydslDir directory unconditionally before execution, which can be
too aggressive and harm incremental build performance. Modify the doFirst block
to check if the directory exists and contains files that require deletion, or
implement a condition to delete only when necessary, thus preserving incremental
build optimizations.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt
Show resolved
Hide resolved
domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookInfoVO.kt
Outdated
Show resolved
Hide resolved
| fun removeParenthesesFromAuthor(author: String): String { | ||
| return author.replace(PARENTHESIS_REGEX, "") | ||
| } | ||
|
|
||
| fun removeParenthesesFromPublisher(publisher: String): String { | ||
| return publisher.replace(PARENTHESIS_REGEX, "") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
코드 중복 제거와 엣지 케이스 처리를 고려해보세요.
현재 두 메서드가 동일한 로직을 사용하고 있습니다. 다음과 같은 개선을 고려해보세요:
- 공통 private 메서드 추출
- 중첩된 괄호 처리 (예: "Author (Part 1 (Sub)) Name")
- 빈 문자열 또는 null 입력 처리
+private fun removeParentheses(input: String): String {
+ return input.replace(PARENTHESIS_REGEX, "").trim()
+}
fun removeParenthesesFromAuthor(author: String): String {
- return author.replace(PARENTHESIS_REGEX, "")
+ return removeParentheses(author)
}
fun removeParenthesesFromPublisher(publisher: String): String {
- return publisher.replace(PARENTHESIS_REGEX, "")
+ return removeParentheses(publisher)
}🤖 Prompt for AI Agents
In
global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt
between lines 7 and 13, both removeParenthesesFromAuthor and
removeParenthesesFromPublisher methods contain duplicated logic. Refactor by
extracting a single private method that handles the removal of parentheses from
any input string. Enhance this method to correctly handle nested parentheses
cases and add checks to safely handle null or empty string inputs by returning
them unchanged or appropriately processed.
infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt
Outdated
Show resolved
Hide resolved
...src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookInfoVO.kt (1)
42-43: 이전 리뷰 제안사항이 정확히 구현되었습니다.null 안전성을 위해 non-null assertion(
!!) 대신 Elvis 연산자를 사용하여 적절한 예외 처리를 구현한 점이 우수합니다. 런타임 크래시 위험이 제거되었습니다.build.gradle.kts (1)
78-95: QueryDSL 설정이 올바르게 구현되었으나 이전 제안사항 검토 필요
layout.buildDirectory.get()사용으로 Gradle 호환성이 개선되었습니다. 하지만 이전 리뷰에서 지적된 KAPT 태스크의 공격적인 디렉토리 삭제로 인한 증분 빌드 성능 영향은 여전히 검토가 필요합니다.domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
61-66: 이전 제안사항이 정확히 적용되었습니다.
BookStatus.entries.associateWith를 사용하여 enum 값들을 동적으로 처리하는 방식이 구현되었습니다. 이는 유지보수성을 크게 향상시킵니다.infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt (1)
64-72: 정렬 로직 성능 최적화가 우수합니다.이전 리뷰에서 제안한 성능 개선사항이 정확히 구현되었습니다. 단일
OrderSpecifier반환과.asc()/.desc()메서드 사용으로 코드가 더 깔끔하고 효율적이 되었습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
build.gradle.kts(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(3 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookInfoVO.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (2)
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (1)
countUserBooksByStatus(49-51)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (1)
countUserBooksByStatus(25-25)
⏰ 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-validation
🔇 Additional comments (2)
infra/src/main/kotlin/org/yapp/infra/config/internal/jpa/JpaConfig.kt (1)
10-16: JPA Auditing 설정이 적절하게 구현되었습니다.
@EnableJpaAuditing어노테이션 추가로 엔티티의 타임스탬프 필드 자동 관리가 활성화되었습니다. 설정 클래스 구조도 Spring Boot 모범 사례를 따르고 있습니다.domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
43-51: 동적 쿼리 기능이 적절하게 구현되었습니다.페이지네이션과 동적 필터링을 지원하는 새로운 메서드가 올바르게 구현되었습니다. 리포지토리 계층으로의 위임과 VO 매핑도 적절합니다.
...src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt
Show resolved
Hide resolved
move-hoon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무너무 고생 많으셨습니다 민우님!!
궁금한 부분에서 몇가지 코멘트 남겼습니다!
“서재 내 검색”의 경우, 아카이빙한 책의 수가 일반적으로 100권 이하일 것으로 예상돼서 offset 방식도 충분히 괜찮을 것 같습니다.
또한, 내 서재이기 때문에 다른 사람이 중간에 데이터를 추가할 일도 없기에 데이터 중복도 발생하지 않을 것 같아요!!
만약 추후 생각지 못한 성능 문제가 생긴다면 cursor 방식으로 마이그레이션 해봐도 좋을 것 같다는 생각이 들었습니다 ㅎㅎ
고생하셨습니다~~
| ], | ||
| exclude = [JpaRepositoriesAutoConfiguration::class] | ||
| ) | ||
| @EnableSpringDataWebSupport(pageSerializationMode = EnableSpringDataWebSupport.PageSerializationMode.VIA_DTO) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요거는 PageConfig 같은 파일을 만들어서 InfraConfig로 가져오면 좋을 것 같아요!!
| val completedCount: Long | ||
| ) { | ||
| companion object { | ||
| fun from( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인자를 여러 개 받고 있어서 of가 좋을 것 같습니다 ㅎㅎ
| request.validTitle(), | ||
| request.validAuthor(), | ||
| request.validAuthor(), | ||
| request.validPublisher(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿굿!
| val books = findUserBooksByDynamicCondition(userId, status, sort, pageable) | ||
| val statusCounts = userBookDomainService.getUserBookStatusCounts(userId) | ||
|
|
||
| return UserBookPageResponse.from( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 of로 바꾸면 좋을 것 같네용!
| fun findUserBooksByDynamicCondition( | ||
| userId: UUID, | ||
| status: BookStatus?, | ||
| sort: String?, | ||
| pageable: Pageable | ||
| ): Page<UserBookResponse> { | ||
| return userBookDomainService.findUserBooksByDynamicCondition(userId, status, sort, pageable) | ||
| .map { UserBookResponse.from(it) } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 메서드는 UserBookService에서만 호출되고 있는데 Private 접근자를 부여하고, 가장 아래에 둔다면 코드를 읽기 쉬울 것 같아요!
| sort: String?, | ||
| pageable: Pageable | ||
| ): UserBookPageResponse { | ||
| val books = findUserBooksByDynamicCondition(userId, status, sort, pageable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
page 객체에 맞게 변수 네이밍을 하면 좋을 것 같아요! 예를 들면 userBookResponsePage도 좋을 것 같아요!
| pageable: Pageable | ||
| ): UserBookPageResponse { | ||
| val books = findUserBooksByDynamicCondition(userId, status, sort, pageable) | ||
| val statusCounts = userBookDomainService.getUserBookStatusCounts(userId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 변수명에서 VO임을 명시적으로 드러내면 좋을 것 같아요!!
| createdAt = userBook.createdAt ?: throw IllegalStateException("createdAt은 null일 수 없습니다."), | ||
| updatedAt = userBook.updatedAt ?: throw IllegalStateException("updatedAt은 null일 수 없습니다.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 IllegalStateException 대신 저희 커스텀 예외인 CommonException으로 처리하면, ExceptionHandler에서 확실히 WARN 레벨로 로그를 남길 수 있을 것 같습니다.
또한, 모든 도메인에서 공통적으로 체크되어야 하는 부분이라 별도의 CommonException을 상속한 또 다른 서브 클래스가 아닌 CommonException 자체로 처리하는 것이 적절하다고 생각하는데 민우님 생각은 어떠신가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 의견 주셔서 감사합니다.
말씀해주신 것처럼 CommonException을 사용하면 ExceptionHandler에서 로깅 수준을 명확하게 제어할 수 있다는 점에서 운영 측면에서의 안정성이나 가시성 확보에 유리한 장점이 있다고 생각합니다. 특히, 도메인 전반에 걸쳐 공통적으로 체크되어야 하는 조건이라면, 예외 타입을 통일하여 처리 로직을 간결하게 유지하는 것도 의미 있는 접근이라고 보입니다.
다만, 이번 케이스의 상황이 정상적인 흐름에서 예외가 발생할 수 있는 비즈니스 오류라기보다는, 시스템 내부 상태가 잘못된 경우, 예컨대 개발자가 설정을 누락했거나 객체가 예상치 못하게 초기화되지 않은 상태 등과 같이 발생 자체가 비정상적인 흐름일 경우에는 IllegalStateException의 의미가 더 명확하게 전달된다고 생각합니다.
따라서 해당 상황이 도메인 정책 위반이나 사용자의 잘못된 입력에 의한 것이 아니라, 시스템 로직의 비정상적인 흐름이라면, CommonException보다는 IllegalStateException을 유지한 채, GlobalExceptionHandler에서 이를 WARN 레벨로 로깅하고 공통 응답 포맷으로 처리하는 방향도 충분히 합리적인 선택이라고 생각됩니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
민우님이 말씀해주신 부분에 저도 동의합니다 😊
IllegalStateException은 예상치 못한 예외 상황에 해당하기 때문에, GlobalExceptionHandler에서 warn 레벨로 로깅하기보다는 error 레벨로 로깅하는 것이 더 적절해 보입니다.
또한, 해당 예외가 Exception의 하위 클래스이기 때문에, 별도로 명시하지 않더라도 전역 처리기에서 가장 상위인 Exception 블록에서 처리될 것 같아요~~
| import jakarta.persistence.PersistenceContext | ||
| import org.springframework.context.annotation.Bean | ||
| import org.yapp.infra.InfraBaseConfig | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
설정 클래스인데 @Configuration가 없는 것 같아요!
...src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt
Show resolved
Hide resolved
| createdAt = userBook.createdAt ?: throw IllegalStateException("createdAt은 null일 수 없습니다."), | ||
| updatedAt = userBook.updatedAt ?: throw IllegalStateException("updatedAt은 null일 수 없습니다.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
민우님이 말씀해주신 부분에 저도 동의합니다 😊
IllegalStateException은 예상치 못한 예외 상황에 해당하기 때문에, GlobalExceptionHandler에서 warn 레벨로 로깅하기보다는 error 레벨로 로깅하는 것이 더 적절해 보입니다.
또한, 해당 예외가 Exception의 하위 클래스이기 때문에, 별도로 명시하지 않더라도 전역 처리기에서 가장 상위인 Exception 블록에서 처리될 것 같아요~~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apis/src/main/kotlin/org/yapp/apis/ApisApplication.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookPageResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/internal/page/PageConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt(1 hunks)
🧬 Code Graph Analysis (1)
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (3)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
findUserBooksByDynamicCondition(43-51)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (1)
findUserBooksByDynamicCondition(39-47)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (1)
findUserBooksByDynamicCondition(18-23)
🪛 detekt (1.23.8)
infra/src/main/kotlin/org/yapp/infra/config/internal/page/PageConfig.kt
[warning] 9-10: The class or object PageConfig is empty.
(detekt.empty-blocks.EmptyClassBlock)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (3)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
findUserBooksByDynamicCondition(43-51)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (1)
findUserBooksByDynamicCondition(39-47)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (1)
findUserBooksByDynamicCondition(18-23)
🪛 detekt (1.23.8)
infra/src/main/kotlin/org/yapp/infra/config/internal/page/PageConfig.kt
[warning] 9-10: The class or object PageConfig is empty.
(detekt.empty-blocks.EmptyClassBlock)
⏰ 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-validation
🔇 Additional comments (11)
apis/src/main/kotlin/org/yapp/apis/ApisApplication.kt (1)
17-17: 포맷팅 개선이 적절합니다.코드 스타일 개선을 위한 공백 제거가 정확히 적용되었습니다.
apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt (1)
14-15: LGTM! Querydsl과 페이지네이션 설정이 적절히 추가되었습니다.동적 검색 기능을 위한 Querydsl 설정과 페이지네이션 지원을 위한 설정이 올바르게 활성화되었습니다.
infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt (2)
7-8: 필요한 import문이 올바르게 추가되었습니다.새로운 설정 클래스들에 대한 import문이 적절히 추가되었습니다.
15-18: 새로운 config group들이 적절히 추가되었습니다.PAGE와 QUERY_DSL enum 항목들이 알파벳 순서로 올바르게 배치되었고, 각각의 설정 클래스와 정확히 매핑되었습니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookPageResponse.kt (1)
7-36: 잘 구성된 페이지네이션 응답 DTO입니다.private constructor와 companion object의 of 팩토리 메서드를 사용하여 객체 생성을 적절히 제어하고 있습니다. Swagger 문서화도 잘 되어 있고, 페이지네이션 데이터와 상태별 개수 정보를 효과적으로 조합하고 있습니다.
infra/src/main/kotlin/org/yapp/infra/config/internal/page/PageConfig.kt (1)
7-10: 적절한 페이지네이션 설정 구성입니다.Spring Data의 웹 지원을 활성화하고 DTO를 통한 페이지 직렬화 모드를 설정한 것이 API 응답에 적합합니다. detekt에서 빈 클래스로 감지하고 있지만, 이는 설정 클래스의 정상적인 패턴으로 false positive입니다.
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (3)
27-28: 매개변수 순서가 올바르게 수정되었습니다.bookAuthor와 bookPublisher의 순서가 정정되어 올바른 매개변수 매핑이 이루어졌습니다.
50-58: private 접근 제한자 사용이 적절합니다.내부에서만 사용되는 메서드에 private 접근 제한자를 적용하고 적절한 위치에 배치하여 코드 가독성을 높였습니다.
69-74: UserBookPageResponse 생성이 잘 구현되었습니다.of 팩토리 메서드를 사용하여 응답 객체를 생성하는 패턴이 과거 리뷰 피드백을 적절히 반영하고 있습니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt (2)
22-51: QueryDSL 동적 검색 구현이 우수합니다!과거 리뷰에서 지적된 deprecated
fetchCount()메서드 대신 별도의 count 쿼리를 사용하여 올바르게 구현되었습니다. 동적 필터링, 정렬, 페이징이 모두 적절하게 처리되고 있습니다.
71-79: 정렬 옵션 처리가 깔끔하게 개선되었습니다!과거 리뷰에서 제안된 대로 단일
OrderSpecifier를 반환하도록 변경되어 spread operator 사용을 피하고 성능이 개선되었습니다. QueryDSL의 내장 메서드를 활용한 구현이 간결하고 우수합니다.
| pageable: Pageable | ||
| ): UserBookPageResponse { | ||
| val userBookResponsePage = findUserBooksByDynamicCondition(userId, status, sort, pageable) | ||
| val userBookStatusCountsVO = userBookDomainService.getUserBookStatusCounts(userId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
변수명을 더 명확하게 개선할 수 있습니다.
과거 리뷰 의견을 반영하여 VO임을 명시적으로 드러내는 변수명을 사용하면 좋겠습니다.
다음과 같이 변수명을 개선해보세요:
- val userBookStatusCountsVO = userBookDomainService.getUserBookStatusCounts(userId)
+ val statusCountsVO = userBookDomainService.getUserBookStatusCounts(userId)또는 VO임을 더 명확히 표현하려면:
- val userBookStatusCountsVO = userBookDomainService.getUserBookStatusCounts(userId)
+ val bookStatusCountsVO = userBookDomainService.getUserBookStatusCounts(userId)🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt at line
67, the variable name userBookStatusCountsVO should be improved to more
explicitly indicate that it is a Value Object (VO). Rename the variable to
include "VO" clearly in its name, such as userBookStatusCountsVO or
userBookStatusCountsValueObject, to enhance code readability and maintain
consistency with past review suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~~
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
infra: Querydsl repository 구현 및 설정 적용infra: 내서재 동적검색 시 querydsl config 설정domain: 불필요한 괄호 제거용 validator 분리domain: 내서재 동적검색을 위한 domainService 및 VO 작성domain,infra: JPA Auditing 적용 (생성일/수정일 자동 처리)apis,buildSrc: Querydsl 관련 설정 적용apis: 내서재 동적검색 관련 usecase, service, dto, controller 작성🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트
Summary by CodeRabbit
신규 기능
버그 수정
개선 및 리팩터링
인프라 및 빌드
기타