-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 도서 검색 결과에 사용자 읽음 상태 표시 기능 #39
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
|
Caution Review failedThe pull request is closed. 📝 Walkthrough""" Walkthrough도서 검색 결과에 사용자 읽음 상태를 표시하기 위해, 컨트롤러와 유스케이스, 서비스, 레포지토리 계층에 사용자 ID 기반의 도서 상태 조회 기능이 추가되었습니다. 검색 결과의 각 도서 요약 정보에 해당 사용자의 읽음 상태가 포함되어 반환되도록 전체 흐름이 확장되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BookController
participant BookUseCase
participant UserAuthService
participant BookQueryService
participant UserBookService
participant UserBookDomainService
participant UserBookRepository
Client->>BookController: searchBooks(userId, searchRequest)
BookController->>BookUseCase: searchBooks(searchRequest, userId)
BookUseCase->>UserAuthService: validateUser(userId)
UserAuthService-->>BookUseCase: (OK)
BookUseCase->>BookQueryService: searchBooks(searchRequest)
BookQueryService-->>BookUseCase: BookSearchResponse(books)
BookUseCase->>UserBookService: findAllByUserIdAndBookIsbnIn(userId, isbns)
UserBookService->>UserBookDomainService: findAllByUserIdAndBookIsbnIn(userId, isbns)
UserBookDomainService->>UserBookRepository: findAllByUserIdAndBookIsbnIn(userId, isbns)
UserBookRepository-->>UserBookDomainService: List<UserBook>
UserBookDomainService-->>UserBookService: List<UserBookInfoVO>
UserBookService-->>BookUseCase: List<UserBookResponse>
BookUseCase-->>BookController: BookSearchResponse(books + userBookStatus)
BookController-->>Client: ResponseEntity<BookSearchResponse>
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 없음) Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
✨ 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: 8
🔭 Outside diff range comments (1)
domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookVO.kt (1)
1-40: 클래스 문서화 추가 권장값 객체의 목적과 사용법에 대한 KDoc 주석을 추가하면 코드의 가독성과 유지보수성이 향상됩니다.
클래스 상단에 다음과 같은 문서화를 추가해보세요:
+/** + * 사용자 도서 정보를 표현하는 값 객체 + * + * 도메인 엔티티 UserBook으로부터 생성되며, + * API 응답에서 사용자별 도서 상태 정보를 전달하는 목적으로 사용됩니다. + * + * @property id 사용자 도서 고유 식별자 + * @property userId 사용자 식별자 + * @property bookIsbn 도서 ISBN + * @property status 사용자의 도서 읽음 상태 + */ data class UserBookVO private constructor(
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookResponse.kt(3 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(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookVO.kt(1 hunks)gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt(3 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/JpaUserBookRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserBookRepositoryImpl.kt(1 hunks)
⏰ 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 (15)
gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt (3)
10-12: CORS 설정을 위한 import문들이 올바르게 추가되었습니다.Spring Security의 CORS 설정에 필요한 표준 클래스들이 적절히 import되었습니다.
34-34: CORS 설정이 올바르게 활성화되었습니다.커스텀 CORS 설정 소스를 사용하여 CORS가 적절히 활성화되었습니다.
52-72: CORS 설정이 잘 구현되었습니다.인증된 사용자의 크로스 오리진 요청을 지원하기 위한 CORS 설정이 적절히 구현되었습니다.
allowCredentials = true설정으로 인증 토큰을 포함한 요청이 가능하며, 개발 및 프로덕션 환경을 모두 고려한 origin 설정이 포함되어 있습니다.domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (1)
14-14: 메서드 시그니처가 올바르게 정의되었습니다.새로 추가된
findAllByUserIdAndBookIsbnIn메서드는 명명 규칙을 잘 따르고 있으며, 파라미터 타입과 반환 타입이 적절합니다. 여러 ISBN으로 사용자 도서를 조회하는 기능에 적합한 인터페이스입니다.apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (1)
58-58: 기본값 설정이 적절합니다.
BookStatus.BEFORE_READING을 기본값으로 설정한 것은 적절한 선택입니다. 사용자가 로그인하지 않았거나 해당 도서에 대한 상태가 없을 때의 기본 상태를 잘 나타냅니다.apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (2)
4-4: Import 문 정리가 잘 되었습니다.와일드카드 import 대신 명시적으로 필요한 클래스들만 import하여 의존성을 명확하게 하였습니다.
Also applies to: 8-9
21-24: 새로운 메서드 구현이 적절합니다.도메인 서비스를 호출하고 DTO로 매핑하는 표준적인 서비스 레이어 패턴을 잘 따르고 있습니다. 단일 책임 원칙에 맞게 구현되었습니다.
infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserBookRepositoryImpl.kt (1)
28-34: Repository 구현이 일관성 있게 작성되었습니다.기존 메서드들과 동일한 패턴을 따라 JPA repository를 호출하고 도메인 객체로 매핑하는 구조가 잘 구현되었습니다. 여러 ISBN을 한 번에 조회하는 효율적인 방식입니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (2)
4-4: 필요한 import가 적절히 추가되었습니다.UserBookVO 사용을 위한 import가 추가되어 코드의 의존성이 명확해졌습니다.
28-34: 도메인 서비스 메서드가 효율적으로 구현되었습니다.빈 리스트에 대한 조기 반환으로 불필요한 데이터베이스 호출을 방지하고, repository 결과를 VO로 매핑하는 로직이 적절합니다. 도메인 계층의 책임을 잘 분리하였습니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (1)
28-32: 인증된 사용자 정보를 안전하게 주입하도록 수정되었습니다.@AuthenticationPrincipal 어노테이션을 사용하여 인증된 사용자의 UUID를 안전하게 가져오고, 이를 useCase에 전달하는 구조가 적절합니다. Spring Security의 표준 패턴을 잘 따르고 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt (2)
31-33: API 문서가 새로운 기능을 잘 설명하고 있습니다.알라딘 API 사용과 사용자 도서 상태 표시 기능에 대한 설명이 명확하게 추가되어 API 사용자가 기능을 이해하기 쉽게 되었습니다.
50-53: 인터페이스 시그니처가 적절히 업데이트되었습니다.@AuthenticationPrincipal 어노테이션을 사용한 userId 파라미터 추가로 인증된 사용자 정보를 안전하게 전달할 수 있도록 하였고, 컨트롤러 구현체와 일관성을 유지하고 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
29-29: 모든searchBooks호출부에userId인자가 포함된 것을 확인했습니다.컨트롤러(
BookController.kt) 등 UseCase 호출부에서 모두 새로운 시그니처(request, userId)로 업데이트되어 컴파일 오류가 발생하지 않습니다.
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt:
bookUseCase.searchBooks(request, userId)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookVO.kt (1)
8-19: 값 객체 설계가 잘 구현되었습니다private 생성자와 companion object 팩토리 메서드 패턴을 사용한 설계가 적절합니다. 불변성을 보장하고 제어된 인스턴스 생성을 통해 도메인 무결성을 유지할 수 있습니다.
| "https://qweasdzxc.shop" | ||
| ) | ||
| config.allowedMethods = listOf("GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH") | ||
| config.allowedHeaders = listOf("*") |
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)
보안 고려사항: 헤더 허용 범위를 제한하는 것을 검토해주세요.
allowedHeaders = listOf("*")는 모든 헤더를 허용하므로 보안상 위험할 수 있습니다. 필요한 헤더만 명시적으로 허용하는 것이 더 안전합니다.
다음과 같이 필요한 헤더만 명시적으로 허용하는 것을 고려해보세요:
- config.allowedHeaders = listOf("*")
+ config.allowedHeaders = listOf(
+ "Authorization",
+ "Content-Type",
+ "Accept",
+ "X-Requested-With",
+ "Cache-Control"
+ )📝 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.
| config.allowedHeaders = listOf("*") | |
| config.allowedHeaders = listOf( | |
| "Authorization", | |
| "Content-Type", | |
| "Accept", | |
| "X-Requested-With", | |
| "Cache-Control" | |
| ) |
🤖 Prompt for AI Agents
In gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt at line 64,
the allowedHeaders is set to allow all headers with "*", which poses a security
risk. Replace the wildcard with a list of only the specific headers your
application requires, such as "Authorization", "Content-Type", and any other
necessary headers, to restrict access and improve security.
| config.allowedOrigins = listOf( | ||
| "http://localhost:8080", | ||
| "http://localhost:3000", | ||
| "http://api.qweasdzxc.shop", | ||
| "https://api.qweasdzxc.shop", | ||
| "https://www.qweasdzxc.shop", | ||
| "https://qweasdzxc.shop" | ||
| ) |
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.
💡 Verification agent
🧩 Analysis chain
도메인명 불일치를 확인해주세요.
PR 목표에서 언급된 도메인(api.qwasdzxc.shop)과 코드의 도메인(api.qweasdzxc.shop) 사이에 철자 차이가 있습니다. 실제 프로덕션 도메인과 일치하는지 확인이 필요합니다.
다음 스크립트로 실제 도메인 사용량을 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# 설명: 프로젝트 전체에서 도메인 사용 패턴 확인
# qwasdzxc vs qweasdzxc 도메인 패턴 검색
echo "=== qwasdzxc 도메인 패턴 ==="
rg "qwasdzxc\.shop" --type-add 'config:*.{yml,yaml,properties,kt,java}' -t config
echo -e "\n=== qweasdzxc 도메인 패턴 ==="
rg "qweasdzxc\.shop" --type-add 'config:*.{yml,yaml,properties,kt,java}' -t config
echo -e "\n=== 전체 도메인 설정 파일들 ==="
fd -e yml -e yaml -e properties | xargs grep -l "shop"Length of output: 830
allowedOrigins 도메인 철자 오류 확인 및 수정 필요 (qweasdzxc → qwasdzxc)
코드에서 사용 중인 도메인이 qweasdzxc.shop으로, PR 목표에 적힌 실제 프로덕션 도메인 qwasdzxc.shop과 불일치합니다. 아래 파일에서 도메인 철자를 확인하고 수정해주세요.
- gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt (55–62행)
제안 수정(diff 예시):
- "http://api.qweasdzxc.shop",
- "https://api.qweasdzxc.shop",
- "https://www.qweasdzxc.shop",
- "https://qweasdzxc.shop"
+ "http://api.qwasdzxc.shop",
+ "https://api.qwasdzxc.shop",
+ "https://www.qwasdzxc.shop",
+ "https://qwasdzxc.shop"📝 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.
| config.allowedOrigins = listOf( | |
| "http://localhost:8080", | |
| "http://localhost:3000", | |
| "http://api.qweasdzxc.shop", | |
| "https://api.qweasdzxc.shop", | |
| "https://www.qweasdzxc.shop", | |
| "https://qweasdzxc.shop" | |
| ) | |
| config.allowedOrigins = listOf( | |
| "http://localhost:8080", | |
| "http://localhost:3000", | |
| "http://api.qwasdzxc.shop", | |
| "https://api.qwasdzxc.shop", | |
| "https://www.qwasdzxc.shop", | |
| "https://qwasdzxc.shop" | |
| ) |
🤖 Prompt for AI Agents
In gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt between
lines 55 and 62, the allowedOrigins list contains domain names with a typo:
"qweasdzxc.shop" should be corrected to "qwasdzxc.shop" to match the actual
production domain. Update all occurrences of "qweasdzxc.shop" in the
allowedOrigins list to "qwasdzxc.shop" to fix the domain spelling.
apis/src/main/kotlin/org/yapp/apis/book/service/BookManagementService.kt
Outdated
Show resolved
Hide resolved
| interface JpaUserBookRepository : JpaRepository<UserBookEntity, UUID> { | ||
| fun findByUserIdAndBookIsbn(userId: UUID, bookIsbn: String): UserBookEntity? | ||
| fun findAllByUserId(userId: UUID): List<UserBookEntity> | ||
| fun findAllByUserIdAndBookIsbnIn(userId: UUID, bookIsbnList: List<String>): List<UserBookEntity> |
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)
JPA 메서드 명명 규칙을 올바르게 따르고 있습니다.
Spring Data JPA의 In 키워드를 사용한 메서드 시그니처가 올바르게 정의되었습니다. 다만 도메인 레이어에서는 bookIsbns를 사용하는데 여기서는 bookIsbnList를 사용하여 파라미터 명명에 일관성이 없습니다. 가능하다면 통일하는 것을 고려해보세요.
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/user/repository/JpaUserBookRepository.kt
at line 10, the method parameter name bookIsbnList is inconsistent with the
domain layer's usage of bookIsbns. Rename the parameter from bookIsbnList to
bookIsbns to maintain naming consistency across layers.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
Outdated
Show resolved
Hide resolved
| fun from(vo: UserBookVO): UserBookResponse { | ||
| return UserBookResponse( | ||
| userBookId = vo.id, | ||
| userId = vo.userId, | ||
| bookIsbn = vo.bookIsbn, | ||
| bookTitle = vo.title, | ||
| bookAuthor = vo.author, | ||
| status = vo.status, | ||
| coverImageUrl = vo.coverImageUrl, | ||
| publisher = vo.publisher, | ||
| createdAt = vo.createdAt.format(DateTimeFormatter.ISO_LOCAL_DATE_TIME), | ||
| updatedAt = vo.updatedAt.format(DateTimeFormatter.ISO_LOCAL_DATE_TIME), | ||
| ) | ||
| } |
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)
팩터리 메서드가 올바르게 구현되었습니다.
새로 추가된 from(vo: UserBookVO) 메서드는 기존 from(userBook: UserBook) 메서드와 일관된 패턴을 따르고 있으며, 필드 매핑과 날짜 포맷팅이 적절히 구현되었습니다.
두 팩터리 메서드 간에 코드 중복이 있어 DRY 원칙을 위반하고 있지만, 메서드가 간단하므로 현재로서는 허용 가능한 수준입니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookResponse.kt
around lines 39 to 52, the from(vo: UserBookVO) factory method is correctly
implemented and consistent with the existing from(userBook: UserBook) method. No
changes are needed as the code is acceptable despite minor duplication, so
approve the code changes as is.
| val isbns = searchResponse.books.map { it.isbn } | ||
| val userBooks = userBookService.findAllByUserIdAndBookIsbnIn(userId, isbns) | ||
| val statusMap = userBooks.associateBy({ it.bookIsbn }, { it.status }) | ||
| searchResponse.books.forEach { bookSummary -> | ||
| statusMap[bookSummary.isbn]?.let { status -> | ||
| bookSummary.userBookStatus = status | ||
| } | ||
| } |
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)
매핑 로직 최적화 및 성능 개선 제안
현재 구현은 올바르게 작동하지만 다음과 같은 개선사항을 고려해볼 수 있습니다:
associateBy를 사용한 맵 생성과forEach를 통한 업데이트가 분리되어 있어 가독성이 떨어집니다.- 추가적인 데이터베이스 쿼리로 인한 성능 영향을 고려해야 합니다.
다음과 같이 리팩토링을 고려해보세요:
- val searchResponse = bookQueryService.searchBooks(request)
- val isbns = searchResponse.books.map { it.isbn }
- val userBooks = userBookService.findAllByUserIdAndBookIsbnIn(userId, isbns)
- val statusMap = userBooks.associateBy({ it.bookIsbn }, { it.status })
- searchResponse.books.forEach { bookSummary ->
- statusMap[bookSummary.isbn]?.let { status ->
- bookSummary.userBookStatus = status
- }
- }
- return searchResponse
+ val searchResponse = bookQueryService.searchBooks(request)
+
+ if (searchResponse.books.isNotEmpty()) {
+ val statusMap = userBookService.findAllByUserIdAndBookIsbnIn(
+ userId,
+ searchResponse.books.map { it.isbn }
+ ).associateBy(UserBookResponse::bookIsbn, UserBookResponse::status)
+
+ searchResponse.books.forEach { bookSummary ->
+ bookSummary.userBookStatus = statusMap[bookSummary.isbn]
+ }
+ }
+
+ return searchResponse🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt around lines
33 to 40, the current code separately creates a map of user book statuses and
then iterates over books to update their status, which reduces readability and
may impact performance. Refactor by combining the mapping and updating steps
into a single operation, such as using a map or associate function that directly
updates each book's userBookStatus while iterating, to improve clarity and
potentially reduce overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (2)
38-42: BookController.getBookDetail의 userId 사용 불일치
BookController#getBookDetail 메서드에서 @AuthenticationPrincipal로 주입받은userId: UUID를BookUseCase.getBookDetail(request)로 전달하지 않고 있습니다.유스케이스 레이어에서
userId가 필요하다면:
BookUseCase.getBookDetail시그니처에userId: UUID를 추가하고BookQueryService.getBookDetail호출부까지userId를 전달하도록 수정해주세요.컨트롤러에서
userId가 불필요하다면:
BookController#getBookDetail메서드 시그니처에서userId파라미터를 제거해주세요.점검 대상 파일:
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (getBookDetail 메서드)
- apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (47라인: getBookDetail 시그니처)
- apis/src/main/kotlin/org/yapp/apis/book/service/BookQueryService.kt (11라인: getBookDetail 시그니처)
37-43: BookController.getBookDetail의 userId 파라미터 불일치 확인 필요
현재BookController.getBookDetail메소드에는@AuthenticationPrincipal userId: UUID가 선언되어 있지만, 이를bookUseCase.getBookDetail호출에 전달하지 않고 있습니다.
한편,BookUseCase.getBookDetail시그니처는 다음과 같습니다:
- apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt:47
fun getBookDetail(bookDetailRequest: BookDetailRequest): BookDetailResponse따라서 아래 중 하나를 선택하여 수정이 필요합니다:
userId가 실제 로직에 필요하다면
•BookUseCase.getBookDetail및 하위 서비스(BookQueryService.getBookDetail) 시그니처에userId: UUID파라미터를 추가
• 호출부를bookUseCase.getBookDetail(request, userId)로 변경userId가 불필요하다면
• 컨트롤러에서@AuthenticationPrincipal userId: UUID파라미터 제거수정 대상 파일 위치:
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (37–43줄)
- apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (47줄)
위 방향에 따라 일관된 파라미터 전달 구조로 맞춰주세요.
♻️ Duplicate comments (1)
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
36-43: 매핑 로직 최적화 제안 (이전 리뷰 참조)이전 리뷰 코멘트에서 제안된 매핑 로직 최적화를 고려해보시기 바랍니다. 현재 구현은 올바르게 작동하지만, 가독성과 성능 개선을 위해 빈 리스트 체크와 단일 연산으로의 결합을 고려할 수 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.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/UserBookRepositoryImpl.kt(1 hunks)
⏰ 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 (13)
infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt (1)
10-10: JPA 리포지토리 메소드 추가 승인Spring Data JPA 네이밍 컨벤션을 올바르게 따르고 있으며, ISBN 목록으로 사용자 도서 정보를 조회하는 기능이 적절히 구현되었습니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (1)
28-34: 리포지토리 구현 승인도메인 인터페이스를 올바르게 구현하고 있으며, JPA 리포지토리 호출 후 도메인 객체로 매핑하는 기존 패턴을 일관되게 따르고 있습니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
40-46: 도메인 서비스 구현 승인빈 리스트에 대한 조기 반환 최적화가 잘 구현되어 있고, 리포지토리 호출 후 VO 객체로의 매핑이 적절히 처리되었습니다.
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (2)
5-5: Import 최적화 승인와일드카드 import 대신 명시적 UUID import를 사용하여 코드 가독성을 향상시켰습니다.
Also applies to: 8-8
35-38: 서비스 메소드 구현 승인도메인 서비스에 적절히 위임하고 응답 DTO로 매핑하는 로직이 기존 패턴과 일관되게 구현되었습니다.
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
32-34: 사용자 유효성 검사 승인사용자 ID 매개변수 추가와 함께 사용자 존재 여부 검증이 적절히 구현되어 보안성이 향상되었습니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (3)
29-29: 사용자 인증 파라미터 추가가 적절합니다.
@AuthenticationPrincipal어노테이션을 사용하여 인증된 사용자 ID를 받아오는 방식이 올바르게 구현되었습니다.
32-32: 사용자 ID가 유스케이스로 올바르게 전달됩니다.검색 기능에서 사용자별 도서 상태를 조회하기 위해 userId를 bookUseCase.searchBooks에 전달하는 로직이 적절합니다.
28-34: searchBooks 메소드 구현이 올바르게 되었습니다.사용자 ID를 받아서 use case에 전달하는 로직이 정확하게 구현되어 있습니다. 사용자별 도서 상태를 검색 결과에 포함시키는 기능이 제대로 연결되어 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt (4)
31-34: API 문서 개선이 우수합니다.
@Operation어노테이션에 알라딘 API 사용과 사용자 도서 상태 표시에 대한 설명이 추가되어 API 문서의 명확성이 향상되었습니다.
50-53: searchBooks 메서드 시그니처 업데이트가 적절합니다.사용자 인증 정보를 받기 위한
userId파라미터가 올바르게 추가되었고, 기존 파라미터 검증 어노테이션도 그대로 유지되었습니다.
31-34: searchBooks API 문서가 적절하게 업데이트되었습니다.알라딘 API를 통한 검색과 함께 사용자의 도서 상태가 포함된다는 설명이 명확하게 추가되어 있습니다. 기능의 목적을 잘 설명하고 있습니다.
50-53: searchBooks 메소드 시그니처가 올바르게 업데이트되었습니다.사용자 인증 정보를 받기 위한 userId 파라미터가 적절하게 추가되었습니다.
| fun getBookDetail( | ||
| @AuthenticationPrincipal userId: UUID, | ||
| @Valid @Parameter(description = "도서 상세 조회 요청 객체") request: BookDetailRequest | ||
| ): ResponseEntity<BookDetailResponse> |
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)
getBookDetail 메서드의 userId 파라미터 필요성 검토
인터페이스에서 userId 파라미터를 추가했지만, 구현체에서는 실제로 사용하지 않습니다. 도서 상세 조회에서 사용자별 상태 정보가 필요한지 검토하고, 필요하지 않다면 이 파라미터를 제거하는 것을 고려해보세요.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
around lines 76 to 79, the getBookDetail method includes a userId parameter that
is not used in the implementation. Review whether user-specific information is
needed for the book detail retrieval; if not, remove the userId parameter from
both the interface and its implementations to simplify the method signature.
💡 Verification agent
🧩 Analysis chain
getBookDetail 메소드의 userId 파라미터 필요성을 확인해주세요.
인터페이스에서 userId 파라미터가 추가되었지만, 실제 구현체(BookController.kt)에서는 이 파라미터를 사용하지 않고 있습니다. 만약 도서 상세 조회에서 사용자별 상태 정보가 필요하지 않다면, 인터페이스에서도 이 파라미터를 제거하는 것이 좋겠습니다.
일관성을 위해 다음 중 하나를 선택해야 합니다:
- 도서 상세 조회에서도 사용자 상태가 필요하다면 구현체에서 userId를 사용하도록 수정
- 필요하지 않다면 인터페이스에서 userId 파라미터 제거
다음 스크립트로 BookUseCase의 getBookDetail 메소드 구현을 확인해보세요:
🏁 Script executed:
#!/bin/bash
# BookUseCase의 getBookDetail 메소드 구현 확인
ast-grep --pattern $'fun getBookDetail($$$) {
$$$
}'Length of output: 55
BookUseCase 인터페이스 및 구현체에서 getBookDetail 메서드 정의를 전체적으로 찾아보려면 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
# BookUseCase 인터페이스와 구현체에서 getBookDetail 메소드 검색
rg "getBookDetail" -n .Length of output: 1131
getBookDetail에서 사용되지 않는 userId 파라미터 제거
현재 API 인터페이스와 컨트롤러 구현에만 존재하는 @AuthenticationPrincipal userId: UUID 파라미터는 실제 로직(BookUseCase → BookQueryService)에서 전혀 사용되지 않으므로, 일관성을 위해 제거해 주세요.
• apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt (76–79행)
• apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (37–42행)
예시 diff:
--- a/apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
+++ b/apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
@@ -76,4 +76,4 @@
- fun getBookDetail(
- @AuthenticationPrincipal userId: UUID,
+ fun getBookDetail(
+ @Valid @Parameter(description = "도서 상세 조회 요청 객체") request: BookDetailRequest
): ResponseEntity<BookDetailResponse>--- a/apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt
+++ b/apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt
@@ -37,5 +37,4 @@
- override fun getBookDetail(
- @AuthenticationPrincipal userId: UUID,
- request: BookDetailRequest
- ): ResponseEntity<BookDetailResponse> {
+ override fun getBookDetail(request: BookDetailRequest): ResponseEntity<BookDetailResponse> {
val response = bookUseCase.getBookDetail(request)
return ResponseEntity.ok(response)🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt lines
76 to 79 and
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt lines 37 to
42, remove the unused parameter @AuthenticationPrincipal userId: UUID from the
getBookDetail function signature in both the API interface and the controller
implementation to maintain consistency since it is not used in the business
logic.
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.
리뷰 완료했슴다!
|
|
||
| val searchResponse = bookQueryService.searchBooks(request) | ||
| val isbns = searchResponse.books.map { it.isbn } | ||
| val userBooks = userBookService.findAllByUserIdAndBookIsbnIn(userId, isbns) |
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.
요거 ResponseDTO라 userBooksReponse로 명명하면 vo가 아니라는걸 명확히 알 수 있을 것 같습니다!
| userAuthService.validateUserExists(userId) | ||
|
|
||
| val searchResponse = bookQueryService.searchBooks(request) | ||
| val isbns = searchResponse.books.map { it.isbn } |
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.
(사소) 요거 requestDTO로 포장해서 findAllByUserIdAndBookIsbnIn 메서드의 인자로 넘기면 좋을 것 같긴 합니다! (하지만 중요한 부분은 아닙니다 ㅎㅎ)
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.
LGTM~
|


🔗 관련 이슈
📘 작업 유형
📙 작업 내역
UserBookVO,UserBookResponse매핑 로직 추가🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신규 기능
기능 개선