-
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 all 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -140,4 +140,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 |
|---|---|---|
|
|
@@ -47,6 +47,9 @@ class ReadingRecordService( | |
| return page.map { ReadingRecordResponse.from(it) } | ||
| } | ||
|
|
||
| fun deleteAllByUserBookId(userBookId: UUID) { | ||
| readingRecordDomainService.deleteAllByUserBookId(userBookId) | ||
| } | ||
|
Comment on lines
+50
to
+52
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. 🛠️ Refactor suggestion 읽기 기록 일괄 삭제는 트랜잭션 경계 내에서만 호출되도록 강제하세요 이 메서드는 대량 삭제를 수행하므로 상위 경계(UseCase)의 트랜잭션 안에서만 실행되도록 MANDATORY 전파를 권장합니다. 현재 BookUseCase.deleteBookFromMyLibrary가 @transactional로 감싸고 있어 호환됩니다. +import org.springframework.transaction.annotation.Propagation
+import org.springframework.transaction.annotation.Transactional- fun deleteAllByUserBookId(userBookId: UUID) {
+ @Transactional(propagation = Propagation.MANDATORY)
+ fun deleteAllByUserBookId(userBookId: UUID) {
readingRecordDomainService.deleteAllByUserBookId(userBookId)
}🤖 Prompt for AI Agents |
||
| fun updateReadingRecord( | ||
| readingRecordId: UUID, | ||
| request: UpdateReadingRecordRequest | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -146,6 +146,9 @@ class ReadingRecordDomainService( | |
| return buildReadingRecordInfoVO(savedReadingRecord) | ||
| } | ||
|
|
||
| fun deleteAllByUserBookId(userBookId: UUID) { | ||
| readingRecordRepository.deleteAllByUserBookId(userBookId) | ||
| } | ||
|
Comment on lines
+149
to
+151
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연관 엔티티(태그 매핑) 무결성 보장 및 트랜잭션 강제 필요
제안: +import org.springframework.transaction.annotation.Propagation
+import org.springframework.transaction.annotation.Transactional- fun deleteAllByUserBookId(userBookId: UUID) {
+ @Transactional(propagation = Propagation.MANDATORY)
+ fun deleteAllByUserBookId(userBookId: UUID) {
readingRecordRepository.deleteAllByUserBookId(userBookId)
}검증 항목:
필요하다면, readingRecordTagRepository에 “레코드 ID 목록 기반 삭제”를 추가한 뒤(예: deleteAllByReadingRecordIdIn), 태그 매핑 → 읽기 기록 순으로 삭제하는 안전한 경로도 제공합니다. 원하시면 해당 레포지토리/도메인 변경 패치도 함께 드리겠습니다. 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Searching for entities, repositories, migrations and cascade/orphan settings related to ReadingRecord..."
# Find Kotlin classes and usages
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "class\s+ReadingRecord\b" --type kotlin || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "class\s+ReadingRecordTag\b" --type kotlin || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "interface\s+ReadingRecordRepository\b" --type kotlin || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "deleteAllByUserBookId" || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "deleteAllByReadingRecordId" || true
# Search for JPA cascade/on-delete/orphan settings
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "orphanRemoval\s*=\s*true" || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "cascade\s*=\s*CascadeType" || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "@OnDelete" || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "OnDeleteAction" || true
# Search DB migration / SQL files for foreign keys referencing reading_record or reading_record_tag
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "reading_record_tag" || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "reading_record" || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "ON DELETE CASCADE" || true
rg -n -C2 --hidden --no-ignore-vcs --glob '!**/build/**' --glob '!**/.gradle/**' "REFERENCES .*reading_record" -P || true
echo "Done."Length of output: 10351 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== ReadingRecordEntity ==="
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt || true
echo
echo "=== ReadingRecordTagEntity ==="
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/readingrecordtag/entity/ReadingRecordTagEntity.kt || true
echo
echo "=== JpaReadingRecordRepository ==="
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt || true
echo
echo "=== ReadingRecordRepositoryImpl ==="
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt || true
echo
echo "=== JpaReadingRecordTagRepository (if any) ==="
rg -n --hidden --no-ignore-vcs --glob '!**/build/**' 'readingrecordtag/repository|ReadingRecordTagRepository' || true
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt || true
echo
echo "=== Domain service & API service locations ==="
sed -n '1,240p' domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt || true
sed -n '1,240p' apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt || true
sed -n '1,240p' apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt || true
echo
echo "=== Search for DB migrations (sql/flyway/liquibase) mentioning reading_records or reading_record_tags ==="
rg -n --hidden --no-ignore-vcs --glob '!**/build/**' '(reading_records|reading_record_tags|ON DELETE CASCADE|REFERENCES .*reading_records|REFERENCES .*reading_record_tags)' -S || trueLength of output: 24422 연관 엔티티(태그 매핑) 무결성 보장 필요 — 수정 필요 확인 결과: reading_records와 reading_record_tags는 JPA 연관 매핑이 없고 DB 마이그레이션에서도 ON DELETE CASCADE를 찾을 수 없습니다. 또한 JpaReadingRecordRepository의 deleteAllByUserBookId는 파생 삭제(벌크 DELETE)로 동작할 가능성이 높아 엔티티 라이프사이클(@SQLDelete)을 우회하고, 그 결과 reading_record_tags가 고아 레코드로 남을 위험이 있습니다. 검증 근거(주요 파일)
권장 수정 (안전한 삭제 흐름)
예시 패치(간단 참고) 추가 import:
domain/src/.../ReadingRecordDomainService.kt 변경 예: - fun deleteAllByUserBookId(userBookId: UUID) {
- readingRecordRepository.deleteAllByUserBookId(userBookId)
- }
+ @Transactional(propagation = Propagation.MANDATORY)
+ fun deleteAllByUserBookId(userBookId: UUID) {
+ val records = readingRecordRepository.findAllByUserBookId(userBookId)
+ val recordIds = records.map { it.id.value }
+ if (recordIds.isNotEmpty()) {
+ readingRecordTagRepository.deleteAllByReadingRecordIdIn(recordIds)
+ }
+ readingRecordRepository.deleteAllByUserBookId(userBookId)
+ }infra/src/.../readingrecordtag/repository/JpaReadingRecordTagRepository.kt 예(간단): interface JpaReadingRecordTagRepository : JpaRepository<ReadingRecordTagEntity, UUID>, JpaReadingRecordTagQuerydslRepository {
fun findByReadingRecordId(readingRecordId: UUID): List<ReadingRecordTagEntity>
+ fun deleteAllByReadingRecordIdIn(readingRecordIds: List<UUID>)
}요약: 현재 구현은 태그 매핑 고아화 또는 제약 위반 가능성이 있으므로, 위 중 하나(애플리케이션 레벨 선삭제 → 레코드 삭제 OR DB FK+ON DELETE CASCADE)를 적용하고, 도메인 서비스에 트랜잭션 경계(MANDATORY) 적용을 권장합니다. 필요한 경우 readingRecordTagRepository/impl 패치와 테스트용 코드 패치를 제가 직접 제공해드리겠습니다.
|
||
| fun deleteReadingRecord(readingRecordId: UUID) { | ||
| val readingRecord = readingRecordRepository.findById(readingRecordId) | ||
| ?: throw ReadingRecordNotFoundException( | ||
|
|
||
| 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,12 @@ 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, | ||||||||||||||||
| ) { | ||||||||||||||||
| fun upsertUserBook( | ||||||||||||||||
| userId: UUID, | ||||||||||||||||
|
|
@@ -77,6 +78,10 @@ class UserBookDomainService( | |||||||||||||||
| return userBookRepository.existsByIdAndUserId(userBookId, userId) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| fun deleteById(userBookId: UUID) { | ||||||||||||||||
| userBookRepository.deleteById(userBookId) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| fun findBooksWithRecordsOrderByLatest(userId: UUID): List<HomeBookVO> { | ||||||||||||||||
| val resultTriples = userBookRepository.findRecordedBooksSortedByRecency(userId) | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -103,7 +108,8 @@ class UserBookDomainService( | |||||||||||||||
| return userBooks.map { userBook -> | ||||||||||||||||
| HomeBookVO.newInstance( | ||||||||||||||||
| userBook = userBook, | ||||||||||||||||
| lastRecordedAt = userBook.updatedAt ?: throw IllegalStateException("UserBook의 updatedAt이 null입니다: ${userBook.id}"), | ||||||||||||||||
| lastRecordedAt = userBook.updatedAt | ||||||||||||||||
| ?: throw IllegalStateException("UserBook의 updatedAt이 null입니다: ${userBook.id}"), | ||||||||||||||||
| recordCount = 0 | ||||||||||||||||
|
Comment on lines
+111
to
113
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) NPE 방지 의도는 좋으나 requireNotNull이 더 간결하고 의도가 명확합니다 Kotlin 표준 함수로 의도를 명확히 드러내고 중괄호 중첩을 줄일 수 있습니다. - lastRecordedAt = userBook.updatedAt
- ?: throw IllegalStateException("UserBook의 updatedAt이 null입니다: ${userBook.id}"),
+ lastRecordedAt = requireNotNull(userBook.updatedAt) {
+ "UserBook의 updatedAt이 null입니다: ${userBook.id}"
+ },📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| ) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| 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.
굿굿!