-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 내서제 도서 삭제 기능을 개발합니다. #95
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 3 commits
3f7f49c
2c19e30
c9ff101
6f1331a
833e050
c985ba2
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -141,4 +141,24 @@ interface BookControllerApi { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PageableDefault(size = 10, sort = ["createdAt"], direction = Sort.Direction.DESC) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pageable: Pageable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): ResponseEntity<UserBookPageResponse> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Operation(summary = "내 서재에 저장한 도서 삭제", description = "내 서재에 저장한 도서를 삭제합니다.") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @ApiResponses( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ApiResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseCode = "204", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description = "성공적으로 도서를 삭제했습니다." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ApiResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseCode = "404", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description = "해당하는 도서를 찾을 수 없습니다.", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| content = [Content(schema = Schema(implementation = ErrorResponse::class))] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @DeleteMapping("/my-library/{userBookId}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fun deleteBookFromMyLibrary( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @AuthenticationPrincipal userId: UUID, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Parameter(description = "삭제할 도서 ID") @PathVariable userBookId: UUID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): ResponseEntity<Unit> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+144
to
+162
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. 💡 Verification agent 🧩 Analysis chainPathVariable 이름 명시 및 문서(401/403) 보완 제안
적용 예시(diff): - fun deleteBookFromMyLibrary(
- @AuthenticationPrincipal userId: UUID,
- @Parameter(description = "삭제할 도서 ID") @PathVariable userBookId: UUID
- ): ResponseEntity<Unit>
+ fun deleteBookFromMyLibrary(
+ @AuthenticationPrincipal userId: UUID,
+ @Parameter(description = "삭제할 도서 ID") @PathVariable("userBookId") userBookId: UUID
+ ): ResponseEntity<Unit>문서(선택, 동일 블록 내 추가): @ApiResponses(
value = [
ApiResponse(
responseCode = "204",
description = "성공적으로 도서를 삭제했습니다."
),
+ ApiResponse(
+ responseCode = "401",
+ description = "인증 실패",
+ content = [Content(schema = Schema(implementation = ErrorResponse::class))]
+ ),
+ ApiResponse(
+ responseCode = "403",
+ description = "권한 없음",
+ content = [Content(schema = Schema(implementation = ErrorResponse::class))]
+ ),
ApiResponse(
responseCode = "404",
description = "해당하는 도서를 찾을 수 없습니다.",
content = [Content(schema = Schema(implementation = ErrorResponse::class))]
)
]
)삭제 흐름에서 존재하지 않는 도서/타인 소유 도서에 대한 404 처리가 실제로 보장되는지 아래 스크립트로 확인해 주세요. 🏁 Script executed: #!/bin/bash
set -euo pipefail
# 컨트롤러/유스케이스 삭제 진입점
rg -n -A 15 -B 5 $'deleteBookFromMyLibrary\\('
# 서비스 레이어 삭제 구현
rg -n -A 20 -B 5 $'deleteUserBook\\('
# 소유자/존재 검증
rg -n -A 3 -B 3 $'existsByIdAndUserId\\('
# 연관 독서 기록 선삭제 호출
rg -n -A 5 -B 3 $'deleteAllByUserBookId\\('Length of output: 16419 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- find validateUserBookExists occurrences ----"
rg -n "validateUserBookExists" || true
echo
echo "---- show UserBookService.kt ----"
sed -n '1,240p' apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt || true
echo
echo "---- show UserBookDomainService.kt ----"
sed -n '1,240p' domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt || true
echo
echo "---- show UserBookRepository.kt ----"
sed -n '1,240p' domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt || true
echo
echo "---- show infra UserBookRepositoryImpl.kt ----"
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt || true
echo
echo "---- search for NotFound/NotFoundException/EntityNotFound/ResourceNotFound ----"
rg -n "NotFound|NotFoundException|EntityNotFound|ResourceNotFound" || true
echo
echo "---- search for exception handler (@ControllerAdvice) ----"
rg -n "@ControllerAdvice" || true
rg -n "ExceptionHandler\\(" || trueLength of output: 19149 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- locate files ----"
rg -n "class UserBookException" || true
rg -n "enum class UserBookErrorCode" || true
rg -n "class CommonException" || true
rg -n "class ErrorResponse" || true
rg -n "GlobalExceptionHandler.kt" || true
echo
echo "---- show apis UserBookException.kt ----"
sed -n '1,240p' apis/src/main/kotlin/org/yapp/apis/book/exception/UserBookException.kt || true
echo
echo "---- show apis UserBookErrorCode.kt ----"
sed -n '1,240p' apis/src/main/kotlin/org/yapp/apis/book/exception/UserBookErrorCode.kt || true
echo
echo "---- show global CommonException (if exists) ----"
sed -n '1,240p' global-utils/src/main/kotlin/org/yapp/globalutils/exception/CommonException.kt || true
sed -n '1,240p' global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalException.kt || true || true
echo
echo "---- show GlobalExceptionHandler.kt ----"
sed -n '1,240p' global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalExceptionHandler.kt || true
echo
echo "---- show ErrorResponse.kt ----"
sed -n '1,240p' global-utils/src/main/kotlin/org/yapp/globalutils/response/ErrorResponse.kt || trueLength of output: 9752 @PathVariable 이름 명시 + OpenAPI(401/403) 문서 추가 권고 — 삭제(404) 처리와 연관 삭제는 이미 구현되어 있습니다 검증 결과: 존재하지 않거나 타인 소유 도서에 대해 404는 UserBookService.validateUserBookExists → UserBookException(USER_BOOK_NOT_FOUND)로 보장되며, UserBookDomainService.deleteById에서 readingRecordRepository.deleteAllByUserBookId(...) 후 userBookRepository.deleteById(...)로 연관 독서기록을 선삭제합니다. 수정 권고(적용 위치)
참고(검증 근거)
권장 적용 예시(diff) --- a/apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
@@
@ApiResponses(
value = [
ApiResponse(
responseCode = "204",
description = "성공적으로 도서를 삭제했습니다."
),
+ ApiResponse(
+ responseCode = "401",
+ description = "인증 실패",
+ content = [Content(schema = Schema(implementation = ErrorResponse::class))]
+ ),
+ ApiResponse(
+ responseCode = "403",
+ description = "권한 없음",
+ content = [Content(schema = Schema(implementation = ErrorResponse::class))]
+ ),
ApiResponse(
responseCode = "404",
description = "해당하는 도서를 찾을 수 없습니다.",
content = [Content(schema = Schema(implementation = ErrorResponse::class))]
)
]
)
@DeleteMapping("/my-library/{userBookId}")
fun deleteBookFromMyLibrary(
@AuthenticationPrincipal userId: UUID,
- @Parameter(description = "삭제할 도서 ID") @PathVariable userBookId: UUID
+ @Parameter(description = "삭제할 도서 ID") @PathVariable("userBookId") userBookId: UUID
): ResponseEntity<Unit>--- a/apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt
@@
@DeleteMapping("/my-library/{userBookId}")
override fun deleteBookFromMyLibrary(
@AuthenticationPrincipal userId: UUID,
- @PathVariable userBookId: UUID,
+ @PathVariable("userBookId") userBookId: UUID,
): ResponseEntity<Unit> {
bookUseCase.deleteBookFromMyLibrary(userId, userBookId)
return ResponseEntity.noContent().build()
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ interface ReadingRecordRepository { | |
|
|
||
| fun save(readingRecord: ReadingRecord): ReadingRecord | ||
|
|
||
| fun deleteAllByUserBookId(userBookId: UUID) | ||
|
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. 🧹 Nitpick (assertive) 반환값 설계(옵션): 삭제 건수 반환 고려 Line [12]의 🤖 Prompt for AI Agents |
||
|
|
||
| fun findById(id: UUID): ReadingRecord? | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,11 +6,13 @@ import org.yapp.domain.userbook.vo.HomeBookVO | |
| import org.yapp.domain.userbook.vo.UserBookInfoVO | ||
| import org.yapp.domain.userbook.vo.UserBookStatusCountsVO | ||
| import org.yapp.globalutils.annotation.DomainService | ||
| import org.yapp.domain.readingrecord.ReadingRecordRepository | ||
| import java.util.* | ||
|
|
||
| @DomainService | ||
| class UserBookDomainService( | ||
| private val userBookRepository: UserBookRepository | ||
| private val userBookRepository: UserBookRepository, | ||
| private val readingRecordRepository: ReadingRecordRepository | ||
| ) { | ||
|
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. 사실 이 방법이 간편하긴 한데, 다른 도메인의 코드도 삭제를 할 수 있어서 지양하는 방향이 개인적으로 좋아보여요!! |
||
| fun upsertUserBook( | ||
| userId: UUID, | ||
|
|
@@ -77,6 +79,11 @@ class UserBookDomainService( | |
| return userBookRepository.existsByIdAndUserId(userBookId, userId) | ||
| } | ||
|
|
||
| fun deleteById(userBookId: UUID) { | ||
| readingRecordRepository.deleteAllByUserBookId(userBookId) | ||
| userBookRepository.deleteById(userBookId) | ||
| } | ||
|
|
||
|
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. 🧹 Nitpick (assertive) 부분 삭제 방지를 위한 트랜잭션 강제화 제안 현재는 UseCase에서 트랜잭션이 보장되지만, 도메인 계층의 변경 메서드가 트랜잭션 밖에서 호출되는 실수를 방지하려면 다음과 같이 주입형 트랜잭션 강제화를 추가할 수 있습니다: + @Transactional(propagation = Propagation.MANDATORY)
fun deleteById(userBookId: UUID) {
readingRecordRepository.deleteAllByUserBookId(userBookId)
userBookRepository.deleteById(userBookId)
}해당 어노테이션을 사용하려면 파일 상단에 아래 import가 필요합니다: import org.springframework.transaction.annotation.Propagation
import org.springframework.transaction.annotation.Transactional또한 대량 삭제 성능을 위해 🤖 Prompt for AI Agents |
||
| fun findBooksWithRecordsOrderByLatest(userId: UUID): List<HomeBookVO> { | ||
| val resultTriples = userBookRepository.findRecordedBooksSortedByRecency(userId) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ interface UserBookRepository { | |
| fun existsByIdAndUserId(id: UUID, userId: UUID): Boolean | ||
| fun findById(id: UUID): UserBook? | ||
| fun save(userBook: UserBook): UserBook | ||
| fun deleteById(id: UUID) | ||
|
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. 💡 Verification agent 🧩 Analysis chaindeleteById 계약(존재하지 않는 ID 처리) 명확화 및 경쟁 상태 방지 제안 Line [15]의
현재 PR 범위에서는 계약 문서화 + 상위 선검증 확인만으로 충분해 보입니다. 아래 스크립트로 상위 흐름에서 선검증을 수행하는지 확인해 주세요(예: 🏁 Script executed: #!/bin/bash
set -euo pipefail
# delete 유스케이스/컨트롤러 구현 확인
rg -n -A 15 -B 5 $'fun\\s+deleteBookFromMyLibrary\\('
# 서비스 계층의 삭제 진입점 확인
rg -n -A 20 -B 5 $'deleteUserBook\\('
# 선검증 여부(존재/소유자 확인) 검색
rg -n -A 3 -B 3 $'existsByIdAndUserId\\('
# 도메인에서 연관 레코드 선삭제 흐름 확인
rg -n -A 5 -B 3 $'deleteAllByUserBookId\\('Length of output: 16425 deleteById 계약 확인 — 상위선검증 존재, 문서화 또는 레이스 대비 권고 검증 결과 삭제 흐름에서 호출 전 존재/소유자 선검증이 수행되고 있음을 확인했습니다. 다만 SimpleJpaRepository#deleteById는 대상이 없을 경우 EmptyResultDataAccessException을 던지므로 계약을 명시하거나 레이스 컨디션 대비 처리를 권장합니다. 주의할 위치:
권장 조치 (간단):
현재 PR 범위에서는 계약 문서화 + 상위 선검증 유지 확인으로 충분해 보입니다. 추가 안정화는 별도 개선으로 권장합니다. 🤖 Prompt for AI Agents |
||
| fun findAllByUserId(userId: UUID): List<UserBook> | ||
| fun findAllByUserIdAndBookIsbn13In(userId: UUID, bookIsbn13s: List<String>): List<UserBook> | ||
| fun findUserBooksByDynamicCondition( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,8 @@ interface JpaReadingRecordRepository : JpaRepository<ReadingRecordEntity, UUID>, | |
|
|
||
| fun findAllByUserBookId(userBookId: UUID): List<ReadingRecordEntity> | ||
|
|
||
| fun deleteAllByUserBookId(userBookId: UUID) | ||
|
|
||
|
Comment on lines
+14
to
+15
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. 💡 Verification agent 🧩 Analysis chain대량 삭제 시 영속성 컨텍스트 정합성 확보(@Modifying clearAutomatically 권장)
적용 예시(diff): + @org.springframework.data.jpa.repository.Modifying(clearAutomatically = true)
fun deleteAllByUserBookId(userBookId: UUID)파일 상단에 import 추가(예시): import org.springframework.data.jpa.repository.Modifying또한, 삭제 성능을 위해 인덱스 및 어노테이션 여부를 빠르게 점검: 🏁 Script executed: #!/bin/bash
set -euo pipefail
# @Modifying 사용 여부
rg -n $'@Modifying\\(' infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt || true
# 엔티티에 userBookId 컬럼 및 인덱스 힌트 존재 여부(엔티티/DDL 힌트 탐색)
rg -n -A 3 -B 3 $'userBookId' infra | sed -n '1,200p'
rg -n -A 5 -B 2 $'@Table\\(|@Index\\(' infra | sed -n '1,200p' || trueLength of output: 25821 영속성 컨텍스트 정합성 확보: @Modifying(clearAutomatically = true) 추가 필요 검증 결과: JpaReadingRecordRepository의 deleteAllByUserBookId에 @Modifying이 없고, ReadingRecordEntity(reading_records)에는 user_book_id 인덱스 정의가 없습니다. 아래 수정 권장합니다. 수정 대상(예시)
권장 적용 예시(diff): +import org.springframework.data.jpa.repository.Modifying
+import org.springframework.transaction.annotation.Transactional // 선택사항
interface JpaReadingRecordRepository : JpaRepository<ReadingRecordEntity, UUID>, JpaReadingRecordQuerydslRepository {
fun findAllByUserBookId(userBookId: UUID): List<ReadingRecordEntity>
+ @Modifying(clearAutomatically = true)
+ // @Transactional // 서비스에서 트랜잭션을 보장하면 생략 가능
fun deleteAllByUserBookId(userBookId: UUID)인덱스 권장(마이그레이션으로 추가): ALTER TABLE reading_records
ADD INDEX idx_reading_records_user_book_id (user_book_id);요약: @Modifying(clearAutomatically = true) 추가 및 필요 시 메서드 수준 @transactional 적용, 그리고 reading_records.user_book_id에 DB 인덱스 추가를 통해 성능·정합성 확보하세요. 🤖 Prompt for AI Agents |
||
| fun findAllByUserBookId(userBookId: UUID, pageable: Pageable): Page<ReadingRecordEntity> | ||
|
|
||
| fun findAllByUserBookIdIn(userBookIds: List<UUID>): List<ReadingRecordEntity> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,10 @@ class UserBookRepositoryImpl( | |
| return savedEntity.toDomain() | ||
| } | ||
|
|
||
| override fun deleteById(id: UUID) { | ||
| jpaUserBookRepository.deleteById(id) | ||
| } | ||
|
|
||
|
Comment on lines
+41
to
+44
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. 💡 Verification agent ❓ Verification inconclusivedeleteById의 예외 전파/404 매핑을 명확히 해주세요
권장:
현재 구현은 위임 자체는 적절하므로, 예외 처리 정책만 합의/문서화되면 됩니다. deleteById 예외 전파/404 매핑 필요 — 수정 필요 간단 요약: jpaUserBookRepository.deleteById(id)는 ID가 없을 경우 EmptyResultDataAccessException을 던질 수 있습니다(경합 상황에서 발생 가능). 현재 구현은 예외를 처리하지 않고 위임하고 있어 예외 전파 정책을 명확히 해야 합니다. 권장(선택 중 하나):
주의할 파일 위치:
예시 대체 코드(참고): 현재 위임 자체는 적절하므로, 예외 처리 정책(글로벌 매핑 또는 변환)을 합의하고 코드/문서에 반영해 주세요. 🤖 Prompt for AI Agents |
||
| override fun findAllByUserId(userId: UUID): List<UserBook> { | ||
| return jpaUserBookRepository.findAllByUserId(userId).map { it.toDomain() } | ||
| } | ||
|
|
||
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.
굿굿!