-
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
Changes from all commits
cbc631b
7139dba
2bd8c08
1fbeb41
6a43f87
dbf348e
8b0ca48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package org.yapp.apis.book.dto.request | ||
|
|
||
| import org.yapp.apis.book.dto.response.BookCreateResponse | ||
| import org.yapp.apis.book.dto.response.UserBookResponse | ||
| 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, | ||
| bookCreateResponse: BookCreateResponse, | ||
| status: BookStatus | ||
| ): UpsertUserBookRequest { | ||
| return UpsertUserBookRequest( | ||
| userId = userId, | ||
| bookIsbn = bookCreateResponse.isbn, | ||
| bookTitle = bookCreateResponse.title, | ||
| bookAuthor = bookCreateResponse.author, | ||
| bookPublisher = bookCreateResponse.publisher, | ||
| bookCoverImageUrl = bookCreateResponse.coverImageUrl, | ||
| status = status | ||
| ) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| 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 | ||
| ) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,35 @@ | ||
| package org.yapp.apis.book.service | ||
|
|
||
| import org.springframework.stereotype.Service | ||
| import org.yapp.apis.book.dto.request.UpsertUserBookRequest | ||
| import org.yapp.apis.book.dto.response.UserBookResponse | ||
| import org.yapp.domain.book.Book | ||
| import org.yapp.domain.userbook.UserBookDomainService | ||
| import org.yapp.domain.userbook.BookStatus | ||
| import org.yapp.domain.userbook.vo.UserBookVO | ||
| import java.util.* | ||
|
|
||
| @Service | ||
| class UserBookService( | ||
| private val userBookDomainService: UserBookDomainService | ||
| ) { | ||
| fun upsertUserBook(userId: UUID, book: Book, status: BookStatus) = | ||
| userBookDomainService.upsertUserBook(userId, book, status) | ||
| fun upsertUserBook(upsertUserBookRequest: UpsertUserBookRequest): UserBookResponse = | ||
| UserBookResponse.from( | ||
| userBookDomainService.upsertUserBook( | ||
| upsertUserBookRequest.userId, | ||
| upsertUserBookRequest.bookIsbn, | ||
| upsertUserBookRequest.bookPublisher, | ||
| upsertUserBookRequest.bookAuthor, | ||
| upsertUserBookRequest.bookTitle, | ||
| upsertUserBookRequest.bookCoverImageUrl, | ||
| upsertUserBookRequest.status | ||
| ) | ||
| ) | ||
|
|
||
| fun findAllUserBooks(userId: UUID) = | ||
| userBookDomainService.findAllUserBooks(userId) | ||
| fun findAllUserBooks(userId: UUID): List<UserBookResponse> { | ||
| val userBooks: List<UserBookVO> = userBookDomainService.findAllUserBooks(userId) | ||
| return userBooks.map { userBook: UserBookVO -> | ||
| UserBookResponse.from(userBook) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| 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? | ||
| ) { | ||
| 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 | ||
| ) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,40 @@ | ||
| 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, | ||
| status: BookStatus | ||
| ): UserBookVO { | ||
| val userBook = userBookRepository.findByUserIdAndBookIsbn(userId, bookIsbn) | ||
| ?.apply { updateStatus(status) } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 새로운 UserBook 생성 시 status 파라미터가 누락되었습니다.
다음과 같이 수정하세요: ?: UserBook.create(
userId = userId,
bookIsbn = bookIsbn,
title = bookTitle,
author = bookAuthor,
publisher = bookPublisher,
coverImageUrl = bookCoverImageUrl,
+ status = status
)🤖 Prompt for AI Agents |
||
|
|
||
| fun upsertUserBook(userId: UUID, book: Book, status: BookStatus): UserBook { | ||
| val existing = userBookRepository.findByUserIdAndBookIsbn(userId, book.isbn) | ||
| return if (existing != null) { | ||
| val updated = existing.updateStatus(status) | ||
| userBookRepository.save(updated) | ||
| } else { | ||
| val created = UserBook.create(userId, book, status) | ||
| userBookRepository.save(created) | ||
| } | ||
| val savedUserBook = userBookRepository.save(userBook) | ||
| return UserBookVO.newInstance(savedUserBook) | ||
| } | ||
|
|
||
| fun findAllUserBooks(userId: UUID): List<UserBook> { | ||
| fun findAllUserBooks(userId: UUID): List<UserBookVO> { | ||
| return userBookRepository.findAllByUserId(userId) | ||
| .map(UserBookVO::newInstance) | ||
| } | ||
| } | ||
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를 연동하게 되는 상황까지 고려해보고 있었습니다!