feat: 내서제 도서 삭제 기능을 개발합니다. #95
Merged
Conversation
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
Walkthrough인증된 사용자 기준 내서재에서 UserBook을 삭제하는 DELETE 엔드포인트와 이를 수행하는 유스케이스·서비스·도메인·레포지토리·JPA 구현이 추가되어 UserBook 삭제 후 연관된 ReadingRecord를 일괄 삭제하도록 연결되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as BookController
participant UseCase as BookUseCase
participant UserSvc as UserService
participant UBkSvc as UserBookService
participant UBkDom as UserBookDomainService
participant UBkRepo as UserBookRepository
participant RRService as ReadingRecordService
participant RRDom as ReadingRecordDomainService
participant RRRepo as ReadingRecordRepository
Client->>Controller: DELETE /api/v1/books/my-library/{userBookId}
Controller->>UseCase: deleteBookFromMyLibrary(userId, userBookId)
UseCase->>UserSvc: validateUserExists(userId)
UseCase->>RRService: deleteAllByUserBookId(userBookId)
UseCase->>UBkSvc: deleteUserBook(userBookId, userId)
UBkSvc->>UBkSvc: validateUserBookExists(userBookId, userId)
UBkSvc->>UBkDom: deleteById(userBookId)
UBkDom->>UBkRepo: deleteById(userBookId)
RRService->>RRDom: deleteAllByUserBookId(userBookId)
RRDom->>RRRepo: deleteAllByUserBookId(userBookId)
UseCase-->>Controller: (void)
Controller-->>Client: 204 No Content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment — Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
⏰ 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)
🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.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(1 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt(1 hunks)
🔇 Additional comments (5)
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (1)
86-90: 삭제 흐름은 적절 — validateUserBookExists는 외부에서 사용 중(따라서 private 불가)
- 삭제 흐름(검증 후 도메인 서비스 위임)은 깔끔합니다.
- validateUserBookExists는 ReadingRecordUseCase 등 다른 클래스에서 호출되고 있어 private로 축소할 수 없습니다. 모듈 범위로만 숨기려면 Kotlin의
internal을 고려할 수 있습니다.- userBookService.deleteUserBook은 현재 BookUseCase.deleteBookFromMyLibrary에서만 호출되며 해당 메서드에
@Transactional이 있으므로 트랜잭션 경계도 적절합니다.주의할 위치:
- apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt — validateUserBookExists(), deleteUserBook()
- apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt — validateUserBookExists(...) 호출들
- apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt — deleteUserBook(...) 호출 (deleteBookFromMyLibrary에 @transactional)
Likely an incorrect or invalid review comment.
infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt (1)
24-26: 벌크 삭제 위임 LGTMJPA 레포지토리로의 단순 위임으로 불필요한 엔티티 로딩 없이 일괄 삭제가 기대됩니다. 상위 계층에서 트랜잭션이 보장되는 현재 구조와도 잘 맞습니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (1)
76-83: 확인: USER_BOOK_NOT_FOUND 예외가 404로 매핑됩니다GlobalExceptionHandler에서 CommonException을 처리하고 ErrorCode의 HttpStatus를 그대로 사용해 ResponseEntity를 반환하므로 UserBookException / UserBookNotFoundException(및 USER_BOOK_NOT_FOUND)은 HTTP 404로 응답됩니다. 따라서 현재 DELETE 핸들러(204 반환) 설계에 문제 없습니다.
수정 대상(참고):
- global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalExceptionHandler.kt — @ExceptionHandler(CommonException::class)에서 errorCode.getHttpStatus()로 응답 반환
- global-utils/src/main/kotlin/org/yapp/globalutils/exception/CommonException.kt — CommonException : ResponseStatusException(errorCode.getHttpStatus(), ...)
- apis/src/main/kotlin/org/yapp/apis/book/exception/UserBookException.kt — CommonException 상속
- apis/src/main/kotlin/org/yapp/apis/book/exception/UserBookErrorCode.kt — USER_BOOK_NOT_FOUND(HttpStatus.NOT_FOUND, ...)
- domain/src/main/kotlin/org/yapp/domain/userbook/exception/UserBookNotFoundException.kt 및 domain/.../UserBookErrorCode.kt — 동일한 404 매핑
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
90-97: 트랜잭션 경계 지정 적절합니다클래스 레벨 readOnly 트랜잭션을 메서드 레벨에서 명시적으로 오버라이드하여 쓰기 연산을 보장한 점이 좋습니다. 사용자 검증 → 삭제 서비스 호출의 순서도 합리적입니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
14-16: 연관 삭제를 위한 의존성 주입 구조 적절
ReadingRecordRepository를 의존성으로 받아 도메인 서비스에서 레코드 선삭제를 수행하는 설계는 FK 제약 조건을 고려한 안정적인 접근입니다.
Comment on lines
+145
to
+163
| @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> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
PathVariable 이름 명시 및 문서(401/403) 보완 제안
- 안정성을 위해
@PathVariable("userBookId")처럼 이름을 명시하는 것을 권장합니다. 빌드 옵션/리플렉션 설정에 따라 파라미터 이름 손실 시 매핑 이슈를 방지합니다. - OpenAPI 문서에 인증 실패/권한 부족(401/403) 응답 스키마를 추가하면 API 소비자에게 더 명확합니다.
적용 예시(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(...)로 연관 독서기록을 선삭제합니다.
수정 권고(적용 위치)
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
- @PathVariable 이름 명시(@PathVariable("userBookId"))
- ApiResponses에 401/403 응답 추가
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt
- 구현부에도 @PathVariable("userBookId") 명시 권장
참고(검증 근거)
- apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt :: validateUserBookExists(...) throws UserBookException(USER_BOOK_NOT_FOUND)
- domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt :: deleteById(...) calls deleteAllByUserBookId(...) then deleteById(...)
- infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt :: deleteAllByUserBookId(...) 존재
권장 적용 예시(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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @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> | |
| @Operation(summary = "내 서재에 저장한 도서 삭제", description = "내 서재에 저장한 도서를 삭제합니다.") | |
| @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") userBookId: UUID | |
| ): ResponseEntity<Unit> |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
around lines 145-163, the @PathVariable is not explicitly named and the OpenAPI
responses omit 401/403; update the signature to use @PathVariable("userBookId")
on the userBookId parameter and add ApiResponse entries for responseCode "401"
and "403" (with appropriate descriptions and ErrorResponse schema) to the
ApiResponses list; also apply the same @PathVariable("userBookId") annotation to
the corresponding method in
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt so
parameter binding is explicit and docs include authentication/authorization
responses.
|
|
||
| fun save(readingRecord: ReadingRecord): ReadingRecord | ||
|
|
||
| fun deleteAllByUserBookId(userBookId: UUID) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
반환값 설계(옵션): 삭제 건수 반환 고려
Line [12]의 deleteAllByUserBookId는 Unit을 반환합니다. 운영/검증(로깅, 모니터링, E2E 테스트) 관점에선 삭제된 행 수(Long)를 반환하면 유용합니다. 필수는 아니나, “정말로 연관 독서 기록이 모두 제거되었는지”를 상위에서 검증할 수 있습니다. 인프라 레이어(JPA)도 동일 시그니처로 맞추면 일관성이 좋아집니다.
🤖 Prompt for AI Agents
In
domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt
around line 12, change the deleteAllByUserBookId signature to return the
deleted-row count (Long) instead of Unit; update the repository interface method
to return Long, adjust any concrete implementations (including the
JPA/infrastructure repository method) to return and propagate the deletion
count, and update callers/tests to expect and, where useful, assert the returned
Long so callers can verify that related reading records were actually removed.
Comment on lines
82
to
86
| fun deleteById(userBookId: UUID) { | ||
| readingRecordRepository.deleteAllByUserBookId(userBookId) | ||
| userBookRepository.deleteById(userBookId) | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
부분 삭제 방지를 위한 트랜잭션 강제화 제안
현재는 UseCase에서 트랜잭션이 보장되지만, 도메인 계층의 변경 메서드가 트랜잭션 밖에서 호출되는 실수를 방지하려면 MANDATORY 전파를 적용해 트랜잭션 존재를 강제하는 것을 권장합니다.
다음과 같이 주입형 트랜잭션 강제화를 추가할 수 있습니다:
+ @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또한 대량 삭제 성능을 위해 reading_record.user_book_id(가칭) FK 컬럼에 인덱스 존재 여부를 확인해 주세요. 인덱스가 없으면 deleteAllByUserBookId 시 테이블 풀스캔이 발생할 수 있습니다.
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt
around lines 82 to 86, the domain deleteById method can be called outside a
transactional boundary by mistake; mark the method with
@Transactional(propagation = Propagation.MANDATORY) and add the required imports
(org.springframework.transaction.annotation.Propagation and
org.springframework.transaction.annotation.Transactional) at the top of the file
to enforce a transaction is present; additionally, verify that the
reading_record.user_book_id FK column has an index (add one if missing) to avoid
full table scans during deleteAllByUserBookId.
| 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.
💡 Verification agent
🧩 Analysis chain
deleteById 계약(존재하지 않는 ID 처리) 명확화 및 경쟁 상태 방지 제안
Line [15]의 deleteById 추가 자체는 타당합니다. 다만 SimpleJpaRepository의 deleteById는 대상이 없을 경우 EmptyResultDataAccessException을 던집니다. 상위 계층에서 404를 보장하려면 다음을 명확히 해두는 것이 안전합니다.
- 유스케이스 레벨에서 반드시
existsByIdAndUserId로 선검증 후 호출(현재 그렇게 사용 중인지 확인 필요). - 또는 도메인 계약을 “존재하지 않는 ID면 예외”로 문서화하여 일관성 유지.
- 추가 개선(선택): 레이스 컨디션을 줄이기 위해 한 번의 쿼리로 삭제를 보장하는
deleteByIdAndUserId(id, userId): Long같은 메서드를 제공하고, 영향 행 수를 기반으로 NotFound를 판단.
현재 PR 범위에서는 계약 문서화 + 상위 선검증 확인만으로 충분해 보입니다.
아래 스크립트로 상위 흐름에서 선검증을 수행하는지 확인해 주세요(예: existsByIdAndUserId 사용 및 404 변환).
🏁 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을 던지므로 계약을 명시하거나 레이스 컨디션 대비 처리를 권장합니다.
주의할 위치:
- apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (deleteBookFromMyLibrary → userBookService.deleteUserBook 호출)
- apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (deleteUserBook — validateUserBookExists 호출 후 userBookDomainService.deleteById 호출)
- domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (deleteById: readingRecordRepository.deleteAllByUserBookId → userBookRepository.deleteById)
- domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (현재 선언: fun deleteById(id: UUID))
- infra/.../JpaUserBookRepository.kt, impl/UserBookRepositoryImpl.kt (existsByIdAndUserId 구현/위임 존재)
권장 조치 (간단):
- 필수(권장 강하게): domain/.../UserBookRepository.kt의 deleteById에 KDoc로 동작 계약 명시(존재하지 않으면 예외 발생 또는 호출 전 상위에서 검증 필요).
- 선택(안전성 강화): 레이스 조건을 줄이려면 repository에 deleteByIdAndUserId(id, userId): Long 같은 영향행수 반환 메서드 추가하고 UserBookDomainService에서 해당 메서드로 교체하여 영향행수로 NotFound 판단(또는 delete 시 EmptyResultDataAccessException을 잡아 NotFound로 변환).
현재 PR 범위에서는 계약 문서화 + 상위 선검증 유지 확인으로 충분해 보입니다. 추가 안정화는 별도 개선으로 권장합니다.
🤖 Prompt for AI Agents
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt around
line 15: 현재 선언된 fun deleteById(id: UUID)에 대해 동작 계약을 명시하세요 — KDoc을 추가해 이 메서드가 대상이
없으면 SimpleJpaRepository#deleteById가 EmptyResultDataAccessException을 던진다는 점(혹은 호출
전 상위에서 존재/소유자 검증이 필수임)을 분명히 적어주고, 호출자들이 선검증을 수행해야 함을 문서화하세요; 추가로 안전성 강화를 원하면 별도
개선으로 repository에 deleteByIdAndUserId(id, userId): Long 같은 영향행수 반환 메서드 추가 및
UserBookDomainService에서 이를 사용해 영향행수 기준으로 NotFound 처리하도록 교체하는 옵션을 제안하세요.
Comment on lines
+14
to
+15
| fun deleteAllByUserBookId(userBookId: UUID) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
대량 삭제 시 영속성 컨텍스트 정합성 확보(@Modifying clearAutomatically 권장)
deleteAllByUserBookId는 파생 bulk delete 쿼리입니다. bulk 연산은 영속성 컨텍스트를 우회하므로 같은 트랜잭션에서 이미 로드된 엔티티가 있다면 스냅샷 불일치가 생길 수 있습니다. @Modifying(clearAutomatically = true) 추가를 권장합니다. 필요 시 메서드 수준 @Transactional도 명시해 주세요(관례적으로 서비스 레이어에서 트랜잭션을 갖고 있으면 생략 가능).
적용 예시(diff):
+ @org.springframework.data.jpa.repository.Modifying(clearAutomatically = true)
fun deleteAllByUserBookId(userBookId: UUID)파일 상단에 import 추가(예시):
import org.springframework.data.jpa.repository.Modifying또한, 삭제 성능을 위해 reading_record.user_book_id에 인덱스가 존재하는지 확인해 주세요(없다면 추가 권장).
인덱스 및 어노테이션 여부를 빠르게 점검:
🏁 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 인덱스 정의가 없습니다. 아래 수정 권장합니다.
수정 대상(예시)
- infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt
- fun deleteAllByUserBookId(userBookId: UUID) — @Modifying 미사용
- infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt
- infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt
- jpaReadingRecordRepository.deleteAllByUserBookId(userBookId) 호출 — 서비스 레이어에 트랜잭션이 없다면 메서드 수준 @transactional 고려
권장 적용 예시(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
In
infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt
around lines 14-15, the deleteAllByUserBookId method lacks
@Modifying(clearAutomatically = true) which can break persistence-context
consistency; annotate the repository method with @Modifying(clearAutomatically =
true) and ensure it's called within a @Transactional boundary (add
@Transactional at service/method level if not present). Also update
infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt
to declare an index on user_book_id via @Table(indexes = ...) to match DB
indexing, and add a DB migration that creates an index (e.g., ALTER TABLE
reading_records ADD INDEX idx_reading_records_user_book_id (user_book_id));
ensure ReadingRecordRepositoryImpl calls the repository within a transactional
context if the service layer is not already transactional.
Comment on lines
+41
to
+44
| override fun deleteById(id: UUID) { | ||
| jpaUserBookRepository.deleteById(id) | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
deleteById의 예외 전파/404 매핑을 명확히 해주세요
jpaUserBookRepository.deleteById(id)는 해당 ID가 없으면 EmptyResultDataAccessException을 던질 수 있습니다. 상위에서 존재/소유자 검증을 선행한다면 대부분 안전하지만, 경쟁 상태(동시 삭제) 시 예외가 발생할 여지가 있습니다.
권장:
- 글로벌 예외 처리에서 위 예외를 404로 매핑하거나,
- 서비스 레이어에서 예외를 Domain NotFound로 변환하여 일관된 응답을 보장.
현재 구현은 위임 자체는 적절하므로, 예외 처리 정책만 합의/문서화되면 됩니다.
deleteById 예외 전파/404 매핑 필요 — 수정 필요
간단 요약: jpaUserBookRepository.deleteById(id)는 ID가 없을 경우 EmptyResultDataAccessException을 던질 수 있습니다(경합 상황에서 발생 가능). 현재 구현은 예외를 처리하지 않고 위임하고 있어 예외 전파 정책을 명확히 해야 합니다.
권장(선택 중 하나):
- 글로벌 예외 핸들러에서 EmptyResultDataAccessException → 404로 매핑, 또는
- 서비스/유스케이스 레이어에서 EmptyResultDataAccessException을 도메인 NotFound 예외(또는 ResponseStatusException(HttpStatus.NOT_FOUND))로 변환
주의할 파일 위치:
- infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (라인 41–44)
예시 대체 코드(참고):
override fun deleteById(id: UUID) {
try {
jpaUserBookRepository.deleteById(id)
} catch (ex: EmptyResultDataAccessException) {
throw DomainNotFoundException("UserBook", id) // 또는 ResponseStatusException(HttpStatus.NOT_FOUND)
}
}
현재 위임 자체는 적절하므로, 예외 처리 정책(글로벌 매핑 또는 변환)을 합의하고 코드/문서에 반영해 주세요.
🤖 Prompt for AI Agents
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt
around lines 41-44: jpaUserBookRepository.deleteById(id) can throw
EmptyResultDataAccessException when the id does not exist; wrap the call in a
try/catch that catches EmptyResultDataAccessException and convert it to the
agreed-upon application-level response (either throw a
DomainNotFoundException("UserBook", id) or throw
ResponseStatusException(HttpStatus.NOT_FOUND)), or alternatively document and
ensure a global exception handler maps EmptyResultDataAccessException → 404;
implement the chosen policy here so the repository does not leak framework
exceptions directly.
move-hoon
reviewed
Aug 14, 2025
Member
move-hoon
left a comment
move-hoon
left a comment
There was a problem hiding this comment.
고생하셨습니다 민우님!!
몇가지 코멘트 남겼습니다 ㅎㅎ
해당 코드도 혹시 soft-delete가 정상적으로 되는 지 확인이 가능할까요??
| override fun deleteBookFromMyLibrary( | ||
| @AuthenticationPrincipal userId: UUID, | ||
| @PathVariable userBookId: UUID, | ||
| ): ResponseEntity<Unit> { |
Comment on lines
14
to
16
| private val userBookRepository: UserBookRepository, | ||
| private val readingRecordRepository: ReadingRecordRepository | ||
| ) { |
Member
There was a problem hiding this comment.
사실 이 방법이 간편하긴 한데, 다른 도메인의 코드도 삭제를 할 수 있어서 지양하는 방향이 개인적으로 좋아보여요!!
Usecase에 ReadingRecordService를 의존시키고 해당 비즈니스 메서드를 이용해 삭제하는 방향성은 어떨지 제안드려봅니다!
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (1)
81-83: 삭제 도메인 메서드에 트랜잭션(MANDATORY) 강제 필요 + PR 설명과 구현 불일치 검토
- 도메인 계층에서 삭제 메서드는 트랜잭션 경계 내에서만 실행되도록 강제하는 것이 안전합니다. 실수로 트랜잭션 밖에서 호출될 경우 부분 삭제/후속 처리 누락을 방지합니다.
- PR 설명에는 “도서 삭제 시 연관된 독서 기록도 함께 삭제 처리”라고 되어 있으나, 현재 도메인 메서드는 UserBook만 삭제합니다. 실제 연쇄 삭제는 UseCase에서 수행되고 있어 설명과 구현이 어긋납니다. 방향성(UseCase 오케스트레이션 vs 도메인 내부 연쇄 삭제)을 팀 합의대로 일원화해 주세요.
아래처럼 트랜잭션 강제를 추가하는 것을 제안합니다:
+import org.springframework.transaction.annotation.Propagation +import org.springframework.transaction.annotation.Transactional- fun deleteById(userBookId: UUID) { + @Transactional(propagation = Propagation.MANDATORY) + fun deleteById(userBookId: UUID) { userBookRepository.deleteById(userBookId) }검증 요청:
- 도메인에서 연쇄 삭제를 하지 않고 UseCase에서 오케스트레이션하는 현재 구조가 맞다면(PR 코멘트 상 선호 방향), PR 설명을 그에 맞게 수정하거나 도메인에서의 연쇄 삭제 표현을 제거해 주세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(3 hunks)
🔇 Additional comments (1)
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
30-32: 의존성 추가는 적절합니다UseCase 오케스트레이션을 위해 ReadingRecordService 주입을 추가한 방향이 일관적입니다.
Comment on lines
+49
to
+51
| fun deleteAllByUserBookId(userBookId: UUID) { | ||
| readingRecordDomainService.deleteAllByUserBookId(userBookId) | ||
| } |
There was a problem hiding this comment.
🛠️ 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
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt
around lines 49-51, the deleteAllByUserBookId method performs bulk deletions and
must be invoked only within an existing transaction; update the method to
require a transaction by annotating it with @Transactional(propagation =
Propagation.MANDATORY) and add the necessary Spring imports
(org.springframework.transaction.annotation.Transactional and
org.springframework.transaction.annotation.Propagation); ensure the method
visibility remains appropriate and that any callers (e.g.,
BookUseCase.deleteBookFromMyLibrary) already run inside a transaction.
Comment on lines
+125
to
+127
| fun deleteAllByUserBookId(userBookId: UUID) { | ||
| readingRecordRepository.deleteAllByUserBookId(userBookId) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
연관 엔티티(태그 매핑) 무결성 보장 및 트랜잭션 강제 필요
- 트랜잭션: 대량 삭제이므로 트랜잭션 경계 강제(MANDATORY)를 권장합니다.
- 무결성: reading_record_tag 같은 조인 테이블이 FK로 reading_record를 참조한다면, DB/JPA에서 ON DELETE CASCADE 또는 orphanRemoval/cascade 설정이 필요합니다. 설정이 없다면 현재 구현은 제약 위반으로 실패하거나 고아 레코드를 남길 수 있습니다.
제안:
+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)
}검증 항목:
- DB 마이그레이션 혹은 JPA 엔티티 설정에 다음 중 하나가 존재하는지 확인
- reading_record_tag(reading_record_id) → reading_record(id)에 ON DELETE CASCADE
- JPA에서 @onDelete(action = OnDeleteAction.CASCADE) 또는 cascade = REMOVE / orphanRemoval = true
필요하다면, 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가 고아 레코드로 남을 위험이 있습니다.
검증 근거(주요 파일)
- infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt — @SQLDelete 적용(soft-delete), 태그 연관 필드 없음
- infra/src/main/kotlin/org/yapp/infra/readingrecordtag/entity/ReadingRecordTagEntity.kt — readingRecordId: UUID 컬럼(연관 @manytoone 없음)
- infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt — fun deleteAllByUserBookId(userBookId: UUID)
- infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt — findByReadingRecordId만 존재(삭제 메서드 없음)
- 리포지토리/마이그레이션 검색 결과: ON DELETE CASCADE 관련 스크립트 없음
권장 수정 (안전한 삭제 흐름)
- 트랜잭션 보장: Domain 경계에서 명시적으로 트랜잭션 요구
- domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt 에서 deleteAllByUserBookId를 MANDATORY로 변경
- 태그 매핑 먼저 삭제 후 읽기 기록 삭제 (원자성 보장)
- 도메인/인프라에 “readingRecordId 목록 기반 삭제” API 추가(deleteAllByReadingRecordIdIn)하고, 이걸 먼저 호출한 뒤 readingRecord 삭제 실행
- 대안: DB 스키마에 FK(REFERENCES reading_records(id)) + ON DELETE CASCADE 추가(데이터 모델/soft-delete 정책과 충돌 가능성 검토 필요)
예시 패치(간단 참고)
추가 import:
- import org.springframework.transaction.annotation.Propagation
- import org.springframework.transaction.annotation.Transactional
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 패치와 테스트용 코드 패치를 제가 직접 제공해드리겠습니다.
Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+111
to
113
| lastRecordedAt = userBook.updatedAt | ||
| ?: throw IllegalStateException("UserBook의 updatedAt이 null입니다: ${userBook.id}"), | ||
| recordCount = 0 |
There was a problem hiding this comment.
🧹 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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| lastRecordedAt = userBook.updatedAt | |
| ?: throw IllegalStateException("UserBook의 updatedAt이 null입니다: ${userBook.id}"), | |
| recordCount = 0 | |
| lastRecordedAt = requireNotNull(userBook.updatedAt) { | |
| "UserBook의 updatedAt이 null입니다: ${userBook.id}" | |
| }, | |
| recordCount = 0 |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt
around lines 111 to 113, replace the explicit null-check with an idiomatic
requireNotNull call to make the intent clearer and reduce nesting: use
requireNotNull on userBook.updatedAt with the same error message so
lastRecordedAt is assigned the non-null value or throws the specified
IllegalStateException message if null.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
BookController및BookControllerApi에 내 서재 도서 삭제 API 추가BookUseCase에 사용자 검증 및 삭제 로직 추가UserBookService에 삭제 메서드(deleteUserBook) 구현UserBookDomainService에서 도서 삭제 시 연관된 독서 기록(ReadingRecord)도 함께 삭제 처리UserBookRepository,ReadingRecordRepository및 각 JPA 구현체에deleteById,deleteAllByUserBookId메서드 추가🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit