refactor: 도서 도메인 리팩토링 #37
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| package org.yapp.apis.book.dto.request | ||
|
|
||
| import org.yapp.domain.userbook.BookStatus | ||
| import java.util.UUID | ||
|
|
||
|
|
||
| data class UpsertUserBookRequest private constructor( | ||
| val userId: UUID, | ||
| val bookIsbn: String, | ||
| val bookTitle: String, | ||
| val bookAuthor: String, | ||
| val bookPublisher: String, | ||
| val bookCoverImageUrl: String, | ||
| val status: BookStatus | ||
| ) { | ||
| companion object { | ||
|
|
||
| fun of( | ||
| userId: UUID, | ||
| bookIsbn: String, | ||
| bookTitle: String, | ||
| bookAuthor: String, | ||
| bookPublisher: String, | ||
| bookCoverImageUrl: String, | ||
| status: BookStatus | ||
| ): UpsertUserBookRequest { | ||
| return UpsertUserBookRequest( | ||
| userId = userId, | ||
| bookIsbn = bookIsbn, | ||
| bookTitle = bookTitle, | ||
| bookAuthor = bookAuthor, | ||
| bookPublisher = bookPublisher, | ||
| bookCoverImageUrl = bookCoverImageUrl, | ||
| status = status | ||
| ) | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package org.yapp.apis.book.dto.response | ||
|
|
||
| import org.yapp.domain.book.vo.BookVO | ||
|
|
||
| data class BookCreateResponse private constructor( | ||
| val isbn: String, | ||
| val title: String, | ||
| val author: String, | ||
| val publisher: String, | ||
| val coverImageUrl: String | ||
| ) { | ||
| companion object { | ||
|
|
||
|
||
| fun from(bookVO: BookVO): BookCreateResponse { | ||
| return BookCreateResponse( | ||
| isbn = bookVO.isbn, | ||
| title = bookVO.title, | ||
| author = bookVO.author, | ||
| publisher = bookVO.publisher, | ||
| coverImageUrl = bookVO.coverImageUrl | ||
| ) | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Member
There was a problem hiding this comment. 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) }
}
Member
Author
There was a problem hiding this comment. 엇 ㅋㅋ 이부분 확인 못했는데 확인 감사합니다. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import org.yapp.apis.book.dto.response.BookDetailResponse | |
| import org.yapp.apis.book.dto.response.BookSearchResponse | ||
| import org.yapp.apis.book.dto.response.UserBookResponse | ||
| import org.yapp.apis.book.constant.BookQueryServiceQualifier | ||
| import org.yapp.apis.book.dto.request.UpsertUserBookRequest | ||
| import org.yapp.apis.book.service.BookManagementService | ||
| import org.yapp.apis.book.service.BookQueryService | ||
| import org.yapp.apis.book.service.UserBookService | ||
|
|
@@ -20,10 +21,12 @@ import java.util.UUID | |
| @UseCase | ||
| @Transactional(readOnly = true) | ||
| class BookUseCase( | ||
|
|
||
| @Qualifier(BookQueryServiceQualifier.ALADIN) | ||
| private val bookQueryService: BookQueryService, | ||
|
Comment on lines
25
to
26
Member
There was a problem hiding this comment. sealed class가 아닌 Qualifier를 써서 주입을 하시게 된 이유가 궁금합니다!
Member
There was a problem hiding this comment. 보통 Qualifier는 스프링에서 같은 타입의 Bean이 여러 개 있을 때, 특정 구현체를 명시적으로 주입하기 위해 사용하는 방식으로 알고있습니다. 그런데, 지금 상황에서는 시스템은 요청에 따라 동적으로 구현체를 선택해야 하는 상황으로 보입니다.(즉, 런타임 시점에 조건에 따라 구현체가 바뀌어야 함) 따라서 sealed interface + 구현체 방식으로 가는게 적합하지 않을까 해서 의견드립니다!
Member
Author
There was a problem hiding this comment. 좋은 의견 감사합니다! 🙇🏻♂️ 말씀 주신 것처럼 보통 @qualifier는 동일한 타입의 Bean이 여러 개일 때 명시적으로 어떤 구현체를 주입할지를 지정할 때 사용되는 방식이고, sealed class/interface는 런타임 분기 처리에 적합하다는 점 공감합니다. 다만 현재 구조에서는 외부 API가 런타임에 따라 동적으로 바뀌어야 할 요구사항은 없는 것으로 알고 있어서, 단순하게 주입만 구분하면 되는 상황이라 @qualifier를 사용했습니다. 혹시 향후 외부 API를 요청 조건에 따라 다르게 선택해야 하는 요구사항이 생기거나, 또는 외부 API를 조합해 처리해야 하는 복잡한 흐름이 생긴다면 말씀해주신 sealed interface 기반 설계가 더 적절할 것 같습니다! 혹시 제가 놓친 시나리오가 있다면 말씀 부탁드릴게요 :)
Member
There was a problem hiding this comment. 좋은 말씀 감사합니다! 🙇🏻♂️ 말씀하신 내용에 공감합니다. 현재는 단일 API만 사용하는 구조이기 때문에 @qualifier 방식도 충분히 동작하긴 하지만, 저는 추후 다른 외부 API를 연동하게 되는 상황까지 고려해보고 있었습니다! |
||
|
|
||
| private val userAuthService: UserAuthService, | ||
| private val userBookService: UserBookService, | ||
| private val bookManagementService: BookManagementService | ||
| ) { | ||
| fun searchBooks(request: BookSearchRequest): BookSearchResponse { | ||
|
|
@@ -38,20 +41,27 @@ class BookUseCase( | |
| fun upsertBookToMyLibrary(userId: UUID, request: UserBookRegisterRequest): UserBookResponse { | ||
| userAuthService.validateUserExists(userId) | ||
|
|
||
| val bookDetailResponse = bookQueryService.getBookDetail(BookDetailRequest.of(request.validBookIsbn())) | ||
|
|
||
| val bookCreateResponse = bookManagementService.findOrCreateBook(BookCreateRequest.create(bookDetailResponse)) | ||
|
||
| val upsertUserBookRequest = UpsertUserBookRequest.of( | ||
| userId = userId, | ||
| bookIsbn = bookCreateResponse.isbn, | ||
| bookTitle = bookCreateResponse.title, | ||
| bookAuthor = bookCreateResponse.author, | ||
| bookPublisher = bookCreateResponse.publisher, | ||
| bookCoverImageUrl = bookCreateResponse.coverImageUrl, | ||
| status = request.bookStatus | ||
| ) | ||
|
Member
There was a problem hiding this comment. val upsertUserBookRequest = UpsertUserBookRequest.of(
userId = userId,
bookCreateResponse = bookCreateResponse,
status = request.bookStatus
)
|
||
| val userBookResponse = userBookService.upsertUserBook(upsertUserBookRequest) | ||
|
|
||
| return userBookResponse | ||
| } | ||
|
|
||
|
|
||
| fun getUserLibraryBooks(userId: UUID): List<UserBookResponse> { | ||
| userAuthService.validateUserExists(userId) | ||
|
|
||
| return userBookService.findAllUserBooks(userId) | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. 너무 깔끔한 것 같습니다!! BookVO를 반환함으로써, 외부에 도메인 엔티티를 직접 노출하지 않는 부분이 좋은 것 같아요! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Member
There was a problem hiding this comment. 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
)
}
}
}(추천)
Member
Author
There was a problem hiding this comment. 저도 이 부분을 고민했었는데요, 즉, 이미 한 번 검증된 값을 다시 감싸는 단계에서 예외가 발생할 가능성은 사실상 없다고 생각하긴 했는데요,
Member
There was a problem hiding this comment. 말씀하신 것처럼 현재 구조에서는 DTO 단계에서 이미 1차적인 유효성 검증을 거치고, 이후 저장된 데이터를 기반으로 VO를 생성하고 있어서, VO의 init 블록에서 동일한 검증을 반복하는 것이 과연 실제로 의미가 있을까? 라는 의문이 드는 것도 충분히 이해됩니다. 다만 저는, 해당 VO가 반드시 현재 흐름에서만 사용된다는 보장이 없다는 점에서 조금 다른 입장을 갖고 있습니다. 향후 다른 비즈니스 로직이나 외부 모듈에서 해당 VO가 직접 생성되거나 사용되는 과정에서 검증이 누락될 가능성도 있다고 생각합니다. 그렇기 때문에 VO 내부에서 한 번 더 유효성 검증을 수행하는 것은 도메인 객체로서의 무결성과 불변성을 지키기 위한 최종 방어선의 역할을 한다고 보고 있습니다 😊 특히, 검증 로직이 교차적으로 중복되어 있지 않다면, 오히려 VO의 init 검증이 없을 경우 유효하지 않은 데이터가 도메인 계층을 뚫고 들어오는 위험도 있다고 생각합니다!! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package org.yapp.domain.book.vo | ||
|
|
||
| import org.yapp.domain.book.Book | ||
|
|
||
| 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? | ||
| ) { | ||
| companion object { | ||
| /** | ||
| * ✨ BookVO를 생성하는 팩토리 메서드 | ||
| */ | ||
| fun newInstance( | ||
| book: Book | ||
| ): BookVO { | ||
| return BookVO( | ||
| book.isbn, | ||
| book.title, | ||
| book.author, | ||
| book.publisher, | ||
| book.coverImageUrl, | ||
| book.publicationYear, | ||
| book.description | ||
| ) | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,39 @@ | ||
| package org.yapp.domain.userbook | ||
|
|
||
| import org.yapp.domain.book.Book | ||
| import org.yapp.domain.userbook.vo.UserBookVO | ||
| import org.yapp.globalutils.annotation.DomainService | ||
| import java.util.UUID | ||
|
|
||
| @DomainService | ||
| class UserBookDomainService( | ||
| private val userBookRepository: UserBookRepository | ||
| ) { | ||
| fun upsertUserBook( | ||
| userId: UUID, | ||
| bookIsbn: String, | ||
| bookTitle: String, | ||
| bookAuthor: String, | ||
| bookPublisher: String, | ||
| bookCoverImageUrl: String, | ||
| ): UserBookVO { | ||
| val userBook = userBookRepository.findByUserIdAndBookIsbn(userId, bookIsbn) | ||
| ?.apply { updateStatus(status) } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Member
There was a problem hiding this comment. 현재 status를 매개변수로 받지 않고 있는데, status가 써있어서 코드레빗 리뷰처럼 코드 수정이 필요할 것 같습니다. |
||
| ?: UserBook.create( | ||
| userId = userId, | ||
| bookIsbn = bookIsbn, | ||
| title = bookTitle, | ||
| author = bookAuthor, | ||
| publisher = bookPublisher, | ||
| coverImageUrl = bookCoverImageUrl, | ||
| ) | ||
|
Comment on lines
+23
to
+30
There was a problem hiding this comment. 새로운 UserBook 생성 시 status 파라미터가 누락되었습니다.
다음과 같이 수정하세요: ?: UserBook.create(
userId = userId,
bookIsbn = bookIsbn,
title = bookTitle,
author = bookAuthor,
publisher = bookPublisher,
coverImageUrl = bookCoverImageUrl,
+ status = status
)🤖 Prompt for AI Agents |
||
|
|
||
| val savedUserBook = userBookRepository.save(userBook) | ||
| return UserBookVO.newInstance(savedUserBook) | ||
| } | ||
|
|
||
| fun findAllUserBooks(userId: UUID): List<UserBookVO> { | ||
| return userBookRepository.findAllByUserId(userId) | ||
| .map(UserBookVO::newInstance) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
공백 제거 부탁드립니다!