-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: 도서 도메인 리팩토링 #37
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""" WalkthroughBook 및 UserBook 도메인 전반에 걸쳐 VO(Value Object)와 DTO(Data Transfer Object) 패턴이 도입 및 적용되었습니다. 서비스 계층에서 도메인 엔티티 대신 VO/DTO를 반환하도록 리팩토링되었으며, 관련 메서드 시그니처와 생성 로직이 일관되게 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant BookManagementService
participant BookDomainService
participant BookVO
participant BookCreateResponse
Controller->>BookManagementService: findOrCreateBook(request)
BookManagementService->>BookDomainService: findByIsbn(isbn)
alt Book exists
BookDomainService-->>BookManagementService: BookVO
else Book not exists
BookDomainService->>BookDomainService: save(...)
BookDomainService-->>BookManagementService: BookVO
end
BookManagementService->>BookCreateResponse: from(BookVO)
BookManagementService-->>Controller: BookCreateResponse
sequenceDiagram
participant Controller
participant UserBookService
participant UserBookDomainService
participant UserBookVO
participant UserBookResponse
Controller->>UserBookService: upsertUserBook(UpsertUserBookRequest)
UserBookService->>UserBookDomainService: upsertUserBook(userId, isbn, ...)
UserBookDomainService-->>UserBookService: UserBookVO
UserBookService->>UserBookResponse: from(UserBookVO)
UserBookService-->>Controller: UserBookResponse
Assessment against linked issues
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: 1
🔭 Outside diff range comments (2)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (1)
28-51: 도메인 엔티티 의존성 제거가 성공적으로 구현되었습니다.Book 엔티티 대신 개별 파라미터를 받도록 변경하여 의존성을 줄인 것은 좋은 설계입니다. 하지만 파라미터 순서가 논리적이지 않습니다.
가독성과 일관성을 위해 파라미터 순서를 다음과 같이 개선하는 것을 권장합니다:
fun create( userId: UUID, - coverImageUrl: String, bookIsbn: String, + title: String, + author: String, publisher: String, - title: String, - author: String, + coverImageUrl: String, initialStatus: BookStatus = BookStatus.BEFORE_READING ): UserBook {책의 기본 정보(ISBN, title, author)를 먼저 배치하고, 부가 정보(publisher, coverImageUrl)를 나중에 배치하는 것이 더 논리적입니다.
domain/src/main/kotlin/org/yapp/domain/book/BookDomainService.kt (1)
22-35: 불필요한 저장 작업을 피하도록 개선 고려현재 구현에서는 기존 책이 이미 존재하는 경우에도 변경사항 없이 다시 저장하고 있습니다. 이는 불필요한 데이터베이스 작업을 발생시킬 수 있습니다.
다음과 같이 개선할 수 있습니다:
): BookVO { - val book = bookRepository.findByIsbn(isbn) ?: Book.create( + val existingBook = bookRepository.findByIsbn(isbn) + if (existingBook != null) { + return BookVO.newInstance(existingBook) + } + + val newBook = Book.create( isbn = isbn, title = title, author = author, publisher = publisher, coverImageUrl = coverImageUrl, publicationYear = publicationYear, description = description ) - val savedBook = bookRepository.save(book) + val savedBook = bookRepository.save(newBook) return BookVO.newInstance(savedBook) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/UpsertUserBookRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookCreateResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.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(2 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(3 hunks)domain/src/main/kotlin/org/yapp/domain/book/BookDomainService.kt(3 hunks)domain/src/main/kotlin/org/yapp/domain/book/vo/BookVO.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookVO.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (1)
updateStatus(22-24)
⏰ 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 (22)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt (1)
32-32: 코드 포맷팅 개선 승인companion object 내부의 빈 줄 추가는 코드 가독성을 개선하는 좋은 변경입니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookResponse.kt (3)
4-4: VO 패턴 적용 승인도메인 엔티티
UserBook대신UserBookVO를 사용하도록 변경한 것은 도메인 계층의 캡슐화를 강화하는 훌륭한 개선입니다.
6-6: 구체적인 import 사용 개선
java.util.*대신java.util.UUID를 직접 import하는 것은 명확성과 성능 측면에서 좋은 개선입니다.
23-23: VO 기반 팩토리 메서드 승인
UserBookVO를 매개변수로 받도록 변경된from메서드는 계층 간 의존성을 줄이는 올바른 접근입니다.apis/src/main/kotlin/org/yapp/apis/book/dto/request/UpsertUserBookRequest.kt (2)
7-15: 잘 설계된 요청 DTOprivate constructor를 사용한 불변 데이터 클래스 설계가 훌륭합니다. 모든 필요한 필드가 적절히 포함되어 있습니다.
18-36: 팩토리 메서드 패턴 적용 우수companion object의
of팩토리 메서드를 통한 제어된 인스턴스 생성은 객체 생성을 캡슐화하는 좋은 패턴입니다.apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookCreateResponse.kt (2)
5-11: 응답 DTO 설계 우수private constructor를 사용한 불변 응답 DTO 설계가 훌륭하며, 필요한 도서 정보가 모두 포함되어 있습니다.
14-22: VO에서 DTO로의 변환 로직 승인
BookVO에서BookCreateResponse로의 매핑이 명확하고 정확합니다. 도메인 계층과 API 계층 간의 적절한 분리를 구현했습니다.apis/src/main/kotlin/org/yapp/apis/book/service/BookManagementService.kt (3)
12-12: 서비스 계층 반환 타입 개선 승인메서드 반환 타입을 도메인 엔티티
Book에서 DTOBookCreateResponse로 변경한 것은 계층 간 의존성을 줄이는 훌륭한 개선입니다.
15-24: VO 사용 및 도메인 서비스 호출 적절
bookDomainService에서 반환된BookVO를 사용하는 로직이 올바르게 구현되었습니다. 도메인 계층의 캡슐화가 잘 유지되고 있습니다.
25-25: DTO 변환 로직 승인
BookVO에서BookCreateResponse로의 변환이 명확하고 적절합니다. 서비스 계층에서 적절한 추상화 수준을 유지하고 있습니다.domain/src/main/kotlin/org/yapp/domain/book/vo/BookVO.kt (2)
5-13: Value Object 구조가 올바르게 설계되었습니다.private constructor를 사용하여 객체 생성을 제어하고, data class를 통해 불변성을 보장하는 구조가 잘 구현되어 있습니다. 모든 필드가 불변으로 선언되어 VO의 특성을 만족합니다.
14-31: 팩토리 메서드 패턴이 적절히 적용되었습니다.companion object를 통한 팩토리 메서드로 Book 엔티티로부터 BookVO를 생성하는 방식이 적절합니다. 도메인 엔티티와 VO 간의 변환 로직이 명확하게 분리되어 있습니다.
domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookVO.kt (2)
8-19: Value Object 구조가 일관성 있게 구현되었습니다.BookVO와 동일한 패턴으로 구현되어 일관성이 있습니다. deletedAt 필드가 제외된 것은 적절한 선택으로 보입니다 - VO는 활성 상태의 데이터만 표현하는 것이 일반적입니다.
21-38: 팩토리 메서드가 올바르게 구현되었습니다.UserBook 엔티티로부터 UserBookVO를 생성하는 매핑 로직이 명확하고 완전합니다. 모든 필수 필드가 적절히 매핑되어 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (3)
41-59: DTO 기반 리팩토링이 성공적으로 적용되었습니다.UpsertUserBookRequest DTO를 사용하여 파라미터를 캡슐화한 것은 좋은 개선입니다. 코드가 더 읽기 쉬워지고 타입 안전성이 향상되었습니다.
62-66: 메서드 단순화가 적절히 수행되었습니다.불필요한 매핑 로직이 제거되어 메서드가 더 간결해졌습니다. 서비스 계층에서 이미 적절한 DTO를 반환하므로 UseCase에서 추가 변환이 불필요합니다.
23-31: 생성자 파라미터 그룹화 일관 확인 완료AuthUseCase와 BookUseCase 모두
• 외부 API 호출 서비스 → 인증/유저 서비스 → 도메인 관리 서비스 순으로
역할별 그룹화를 따르고 있습니다.
BookUseCase의 파라미터 재배치는 이 패턴에 맞춘 의도된 변경이므로, 별도 수정은 불필요합니다.apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (2)
15-25: DTO 기반 인터페이스로 개선되었습니다.UpsertUserBookRequest를 사용하여 메서드 시그니처가 더 명확해졌습니다. 파라미터 추출 로직도 간결하고 이해하기 쉽습니다.
27-28: 매핑 로직이 일관성 있게 적용되었습니다.UserBookVO에서 UserBookResponse로의 변환이 일관된 패턴으로 구현되어 있습니다. 코드가 간결하고 함수형 스타일로 작성되어 읽기 좋습니다.
domain/src/main/kotlin/org/yapp/domain/book/BookDomainService.kt (1)
10-12: 잘 구현되었습니다!도메인 엔티티 대신 VO를 반환하도록 적절히 변경되었습니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
35-38: 적절한 VO 변환 구현도메인 엔티티를 VO로 변환하여 반환하도록 올바르게 구현되었습니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.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.
리팩토링 하시느라 너무 고생많으셨습니다!!
| @Qualifier(BookQueryServiceQualifier.ALADIN) | ||
| private val bookQueryService: BookQueryService, |
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.
sealed class가 아닌 Qualifier를 써서 주입을 하시게 된 이유가 궁금합니다!
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.
보통 Qualifier는 스프링에서 같은 타입의 Bean이 여러 개 있을 때, 특정 구현체를 명시적으로 주입하기 위해 사용하는 방식으로 알고있습니다.
그런데, 지금 상황에서는 시스템은 요청에 따라 동적으로 구현체를 선택해야 하는 상황으로 보입니다.(즉, 런타임 시점에 조건에 따라 구현체가 바뀌어야 함)
따라서 sealed interface + 구현체 방식으로 가는게 적합하지 않을까 해서 의견드립니다!
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.
좋은 의견 감사합니다! 🙇🏻♂️
말씀 주신 것처럼 보통 @qualifier는 동일한 타입의 Bean이 여러 개일 때 명시적으로 어떤 구현체를 주입할지를 지정할 때 사용되는 방식이고, sealed class/interface는 런타임 분기 처리에 적합하다는 점 공감합니다.
다만 현재 구조에서는 외부 API가 런타임에 따라 동적으로 바뀌어야 할 요구사항은 없는 것으로 알고 있어서, 단순하게 주입만 구분하면 되는 상황이라 @qualifier를 사용했습니다.
혹시 향후 외부 API를 요청 조건에 따라 다르게 선택해야 하는 요구사항이 생기거나, 또는 외부 API를 조합해 처리해야 하는 복잡한 흐름이 생긴다면 말씀해주신 sealed interface 기반 설계가 더 적절할 것 같습니다!
혹시 제가 놓친 시나리오가 있다면 말씀 부탁드릴게요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 말씀 감사합니다! 🙇🏻♂️
말씀하신 내용에 공감합니다. 현재는 단일 API만 사용하는 구조이기 때문에 @qualifier 방식도 충분히 동작하긴 하지만, 저는 추후 다른 외부 API를 연동하게 되는 상황까지 고려해보고 있었습니다!
| val status: BookStatus | ||
| ) { | ||
| companion object { | ||
|
|
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 coverImageUrl: String | ||
| ) { | ||
| companion object { | ||
|
|
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 upsertUserBookRequest = UpsertUserBookRequest.of( | ||
| userId = userId, | ||
| bookIsbn = bookCreateResponse.isbn, | ||
| bookTitle = bookCreateResponse.title, | ||
| bookAuthor = bookCreateResponse.author, | ||
| bookPublisher = bookCreateResponse.publisher, | ||
| bookCoverImageUrl = bookCreateResponse.coverImageUrl, | ||
| status = request.bookStatus | ||
| ) |
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 upsertUserBookRequest = UpsertUserBookRequest.of(
userId = userId,
bookCreateResponse = bookCreateResponse,
status = request.bookStatus
)- 해당 부분도 다른 dto와 마찬가지로 인자를 객체로 받아서 내부에서 getter를 호출하도록 리팩토링 하는게 어떨까요?
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 bookDetailResponse = bookQueryService.getBookDetail(BookDetailRequest.of(request.validBookIsbn())) | ||
|
|
||
| val userBook = userBookService.upsertUserBook(userId, book, request.bookStatus) | ||
| val bookCreateResponse = bookManagementService.findOrCreateBook(BookCreateRequest.create(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.
추가적으로 메서드 네이밍을 통일해보면 좋을 것 같아요!!
저는 개인적으로 다른 dto의 정적 팩토리 메서드에서는 of or from 네이밍을 사용중이어서, 해당 부분의 경우 from으로 가면 더 통일성이 있을 것 같아요!
(create라는 네이밍은 아직 제게 도메인 객체 생성용 메서드라는 인상이 강한 것 같아요.)
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.
아아 좋아요 dto는 from of 도메인 객체 생성용 메서드는 create로 하는게 통일성 있어보이네요!
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.
너무 깔끔한 것 같습니다!!
BookVO를 반환함으로써, 외부에 도메인 엔티티를 직접 노출하지 않는 부분이 좋은 것 같아요!
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.
data class BookVO private constructor(
val isbn: String,
val title: String,
val author: String,
val publisher: String,
val coverImageUrl: String,
val publicationYear: Int?,
val description: String?
) {
init {
// 생성 시점에 데이터의 유효성을 검증합니다.
require(isbn.isNotBlank()) { "ISBN은 비어 있을 수 없습니다." }
require(title.isNotBlank()) { "제목은 비어 있을 수 없습니다." }
require(author.isNotBlank()) { "저자는 비어 있을 수 없습니다." }
publicationYear?.let { require(it > 0) { "출판 연도는 0보다 커야 합니다." } }
}
companion object {
fun newInstance(book: Book): BookVO {
return BookVO(
book.isbn,
book.title,
book.author,
book.publisher,
book.coverImageUrl,
book.publicationYear,
book.description
)
}
}
}(추천)
- 사실 지금 도 너무 좋은 코드인 것 같긴 합니다 ㅎㅎ.
- 생성 시점에 검증 로직을 추가하면 스스로의 유효성을 보장하는 진짜 값 객체(Value Object)로 사용할 수 있을 것 같다는 생각이 듭니다!!
- ex. '제목이 비어있는 BookVO'는 원천적으로 존재할 수 없게 된다
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.
저도 이 부분을 고민했었는데요,
현재 저희 구조상 DB에 저장되기 전에 이미 DTO 단에서 1차 검증을 거치고, 이후 저장된 데이터를 다시 VO로 감싸서 사용하는 방식이다 보니, VO 생성 시에 또 init 블록에서 검증을 반복하는 게 과연 실제로 의미가 있을까? 라는 의문도 들었습니다.
즉, 이미 한 번 검증된 값을 다시 감싸는 단계에서 예외가 발생할 가능성은 사실상 없다고 생각하긴 했는데요,
혹시 이런 경우에도 init 유효성 체크가 불변성과 도메인 안전성을 보장하는 측면에서 필요한가?에 대해 다른 의견이나 경험 있으시면 공유 부탁드리겠습니다!
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.
말씀하신 것처럼 현재 구조에서는 DTO 단계에서 이미 1차적인 유효성 검증을 거치고, 이후 저장된 데이터를 기반으로 VO를 생성하고 있어서, VO의 init 블록에서 동일한 검증을 반복하는 것이 과연 실제로 의미가 있을까? 라는 의문이 드는 것도 충분히 이해됩니다.
다만 저는, 해당 VO가 반드시 현재 흐름에서만 사용된다는 보장이 없다는 점에서 조금 다른 입장을 갖고 있습니다.
향후 다른 비즈니스 로직이나 외부 모듈에서 해당 VO가 직접 생성되거나 사용되는 과정에서 검증이 누락될 가능성도 있다고 생각합니다.
그렇기 때문에 VO 내부에서 한 번 더 유효성 검증을 수행하는 것은 도메인 객체로서의 무결성과 불변성을 지키기 위한 최종 방어선의 역할을 한다고 보고 있습니다 😊
특히, 검증 로직이 교차적으로 중복되어 있지 않다면, 오히려 VO의 init 검증이 없을 경우 유효하지 않은 데이터가 도메인 계층을 뚫고 들어오는 위험도 있다고 생각합니다!!
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.
fun upsertUserBook(upsertUserBookRequest: UpsertUserBookRequest): UserBookResponse {
val userBook = userBookDomainService.upsertUserBook(
userId = upsertUserBookRequest.userId,
bookIsbn = upsertUserBookRequest.bookIsbn,
bookPublisher = upsertUserBookRequest.bookPublisher,
bookAuthor = upsertUserBookRequest.bookAuthor,
bookTitle = upsertUserBookRequest.bookTitle,
bookCoverImageUrl = upsertUserBookRequest.bookCoverImageUrl
)
return UserBookResponse.from(userBook)
}
fun findAllUserBooks(userId: UUID): List<UserBookResponse> {
return userBookDomainService
.findAllUserBooks(userId)
.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.
엇 ㅋㅋ 이부분 확인 못했는데 확인 감사합니다.
| bookCoverImageUrl: String, | ||
| ): UserBookVO { | ||
| val userBook = userBookRepository.findByUserIdAndBookIsbn(userId, bookIsbn) | ||
| ?.apply { updateStatus(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.
현재 status를 매개변수로 받지 않고 있는데, 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.
(사소) 해당 부분도 init 블럭으로 VO의 유효성 검증을 추가하면 좋지 않을까 생각했습니다 ㅎㅎ
minwoo1999
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.
GOOD 감사합니다.
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 upsertUserBookRequest = UpsertUserBookRequest.of( | ||
| userId = userId, | ||
| bookIsbn = bookCreateResponse.isbn, | ||
| bookTitle = bookCreateResponse.title, | ||
| bookAuthor = bookCreateResponse.author, | ||
| bookPublisher = bookCreateResponse.publisher, | ||
| bookCoverImageUrl = bookCreateResponse.coverImageUrl, | ||
| status = request.bookStatus | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조아요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (1)
16-27: DTO 기반 메서드 시그니처로 개선되었습니다.단일
UpsertUserBookRequestDTO 파라미터를 사용하여 메서드 시그니처가 더 명확해지고 유지보수성이 향상되었습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/request/UpsertUserBookRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookCreateResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(3 hunks)domain/src/main/kotlin/org/yapp/domain/book/vo/BookVO.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookVO.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (1)
updateStatus(22-24)
⏰ 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)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt (1)
34-34: 팩토리 메서드명 변경이 적절합니다.
create에서from으로 변경하여 다른 객체로부터 변환한다는 의미가 더 명확해졌습니다.apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookCreateResponse.kt (1)
5-23: DTO 구현이 깔끔하고 적절합니다.BookVO에서 API 응답에 필요한 필드만 선별하여 포함하고, private constructor와 팩토리 메서드 패턴을 적절히 사용했습니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/request/UpsertUserBookRequest.kt (1)
9-35: 사용자 도서 업서트 요청 DTO가 잘 구성되었습니다.BookCreateResponse와 사용자 정보를 조합하여 업서트에 필요한 모든 정보를 포함하고 있으며, 팩토리 메서드
of의 네이밍도 적절합니다.domain/src/main/kotlin/org/yapp/domain/book/vo/BookVO.kt (1)
14-19: Value Object 유효성 검증 구현이 우수합니다.이전 리뷰 논의에서 결정된 대로 init 블록에서 유효성 검증을 수행하여 도메인 객체의 무결성을 보장하고 있습니다. 모든 필수 필드에 대한 검증과 적절한 오류 메시지를 제공하고 있어 Value Object로서의 역할을 잘 수행하고 있습니다.
domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookVO.kt (1)
20-29: 포괄적인 유효성 검증이 잘 구현되었습니다.모든 필수 필드에 대한 검증뿐만 아니라 타임스탬프 검증(
createdAt <= updatedAt)까지 포함하여 도메인 제약사항을 완전히 반영하고 있습니다. 특히 생성일과 수정일 간의 논리적 관계를 검증하는 부분이 인상적입니다.apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (4)
14-14: 새로운 DTO 임포트 추가가 적절합니다.
UpsertUserBookRequestDTO의 추가는 도메인 엔티티 대신 DTO를 사용하는 리팩토링 목표와 일치합니다.
24-30: 생성자 파라미터 순서 변경이 논리적입니다.
bookQueryService를 첫 번째로 배치한 것은 메서드 실행 순서와 일치하여 코드 가독성을 향상시킵니다.
44-55: DTO 사용으로 데이터 흐름이 개선되었습니다.
UpsertUserBookRequestDTO를 통한 중간 객체 생성과 직접 반환은 계층 간 데이터 전달을 명확하게 하고 캡슐화를 향상시킵니다.
61-61: DTO 직접 반환으로 코드가 간소화되었습니다.서비스 계층에서 이미 적절한 DTO를 반환하므로 추가 매핑이 불필요해져 코드가 더 간결해졌습니다.
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (2)
4-5: 새로운 DTO 및 VO 임포트가 적절합니다.
UpsertUserBookRequest,UserBookResponse,UserBookVO의 추가는 계층 간 데이터 전달을 위한 적절한 객체들을 도입합니다.Also applies to: 9-9
29-34: 명시적 반환 타입 선언이 추가되었습니다.이전 리뷰 피드백을 반영하여 명시적 반환 타입 선언을 추가한 것이 코드 가독성을 향상시킵니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (4)
4-4: UserBookVO 임포트가 적절합니다.도메인 서비스에서 Value Object를 반환하는 패턴이 캡슐화를 개선합니다.
12-20: 메서드 시그니처 개선이 우수합니다.개별 책 정보 파라미터를 받도록 변경하여 도메인 엔티티 의존성을 줄이고 캡슐화를 향상시켰습니다.
32-33: UserBookVO 반환으로 캡슐화가 개선되었습니다.도메인 엔티티 대신 Value Object를 반환하여 상위 계층에서 도메인 엔티티에 직접 접근하는 것을 방지합니다.
36-39: 일관된 VO 반환 패턴이 적용되었습니다.
List<UserBookVO>반환으로 모든 도메인 서비스 메서드에서 일관된 Value Object 사용 패턴을 유지합니다.
| ?: UserBook.create( | ||
| userId = userId, | ||
| bookIsbn = bookIsbn, | ||
| title = bookTitle, | ||
| author = bookAuthor, | ||
| publisher = bookPublisher, | ||
| coverImageUrl = bookCoverImageUrl, | ||
| ) |
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.
새로운 UserBook 생성 시 status 파라미터가 누락되었습니다.
UserBook.create 호출에서 status 파라미터가 전달되지 않아 새로 생성되는 UserBook의 상태가 올바르게 설정되지 않습니다.
다음과 같이 수정하세요:
?: UserBook.create(
userId = userId,
bookIsbn = bookIsbn,
title = bookTitle,
author = bookAuthor,
publisher = bookPublisher,
coverImageUrl = bookCoverImageUrl,
+ status = status
)🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt
around lines 23 to 30, the call to UserBook.create is missing the status
parameter, causing the new UserBook's status to be unset or incorrect. Add the
status argument to the UserBook.create call with the appropriate value to ensure
the UserBook's status is properly initialized when created.
|
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.
어푸 완료!


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