Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| val loan = loanRepository.findById(loanId).orElse(null) | ||
| ?: return LoanError.LOAN_NOT_FOUND.err() | ||
|
|
||
| if (loan.status != LoanStatus.ON_LOAN) { | ||
| return LoanError.LOAN_NOT_ON_LOAN.err() | ||
| } | ||
|
|
||
| val bookItem = bookItemRepository.findById(loan.bookItemId).orElse(null) | ||
| ?: error("Loan(${loan.id})이 가리키는 소장본을 찾을 수 없습니다.") | ||
|
|
||
| loan.markReturned(request.returnedAt ?: LocalDate.now()) | ||
| bookItem.markAvailable() |
There was a problem hiding this comment.
[MEDIUM] 반납 처리에 동시성 보호가 없어 이중 반납 처리가 가능합니다
Problem: ReturnLoanService.execute()는 loanRepository.findById와 bookItemRepository.findById로 락 없이 엔티티를 조회한 뒤 status == ON_LOAN을 체크하고 상태를 변경합니다. 동일 loanId에 대한 두 번의 동시 요청(더블클릭, 클라이언트 재시도)이 모두 체크를 통과해 markReturned()/markAvailable()가 중복 실행될 수 있습니다.
Evidence: 같은 PR의 CreateLoanService는 동일한 동시성 문제를 막기 위해 findByManagementNumberForUpdate(PESSIMISTIC_WRITE)를 명시적으로 사용하는데, ReturnLoanService에는 그 보호가 빠져 있습니다. Loan/BookItem에 @Version 필드도 없습니다. 동일한 check-then-act 패턴이 과거 PR #6, #12에서도 [MEDIUM]으로 지적된 바 있습니다.
Fix direction: Loan/BookItem 조회에 비관적 락을 걸거나 @Version을 통한 낙관적 락을 도입하세요.
@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("select l from Loan l where l.id = :id")
fun findByIdForUpdate(@Param("id") id: Long): Loan?There was a problem hiding this comment.
수정 완료
LoanRepository에 findByIdForUpdate(@Lock(PESSIMISTIC_WRITE))를 추가하고 사용하도록 수정
| @Lock(LockModeType.PESSIMISTIC_WRITE) | ||
| @Query("select bi from BookItem bi where bi.managementNumber = :managementNumber") | ||
| fun findByManagementNumberForUpdate(@Param("managementNumber") managementNumber: String): BookItem? |
There was a problem hiding this comment.
[HIGH] 신규 조회 메서드에 소프트 삭제 조건(deletedAt is null)이 누락됐습니다
Problem: 신규 추가된 findByManagementNumberForUpdate 쿼리는 deletedAt is null 조건이 없습니다. BookItem은 BaseEntity를 상속해 deletedAt/softDelete()를 이미 갖고 있으므로, 소프트 삭제된 소장본도 이 메서드로 조회·락·대출 처리될 수 있습니다.
Evidence: 같은 PR의 CreateLoanService.kt:36은 Book 조회 시 findByIdAndDeletedAtIsNull을 사용해 소프트 삭제 규약을 지키는데, 새로 추가된 이 BookItem 조회 메서드만 그 규약을 따르지 않습니다. 현재는 BookItem 삭제 기능이 없어 당장 악용되지는 않지만, 추후 소장본 삭제 기능이 추가되는 즉시 삭제된 소장본이 대출 가능해지는 잠재 버그가 됩니다.
Fix direction: 쿼리에 deletedAt is null 조건을 추가하세요.
@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("select bi from BookItem bi where bi.managementNumber = :managementNumber and bi.deletedAt is null")
fun findByManagementNumberForUpdate(@Param("managementNumber") managementNumber: String): BookItem?There was a problem hiding this comment.
BookItem 삭제 기능이 실제로 추가되는 시점(현재 기획에 존재 X)에 관련 기능 리펙토링 (별도 이슈로 관리 예정)
| @field:NotNull | ||
| @field:Schema(description = "대여일", example = "2026-07-31") | ||
| val loanDate: LocalDate, | ||
| @field:Schema(description = "대출자 이메일 (미입력 시 관리자 이메일로 알림 발송)", example = "hong@example.com", nullable = true) |
There was a problem hiding this comment.
[MEDIUM] Swagger 문서가 구현되지 않은 관리자 이메일 알림 기능을 명시하고 있습니다
Problem: borrowerEmail 필드의 Swagger 설명이 "미입력 시 관리자 이메일로 알림 발송"이라고 명시하지만, 실제 구현에는 이메일 발송/알림 로직이 전혀 없습니다. API 소비자가 이 문서를 신뢰하고 알림이 실제로 발송된다고 오인할 수 있습니다.
Evidence: CreateLoanService.execute()는 borrowerEmail을 그대로 Loan 엔티티 생성자에 전달할 뿐이며, 코드베이스 전체에 mail/notification 관련 인프라가 존재하지 않습니다.
Fix direction: 알림 기능을 구현하거나, 구현 전이라면 설명에서 알림 관련 문구를 제거하세요.
@field:Schema(description = "대출자 이메일", example = "hong@example.com", nullable = true)
val borrowerEmail: String? = null,
1 participant
연관 이슈
작업 사항
feat(#15): 대출 등록 API 추가@Lock(PESSIMISTIC_WRITE)로 비관적 락을 걸어 동시에 같은 관리번호로 대출 요청이 들어와도 하나만 성공하도록 처리feat(#15): 반납 처리 API 추가feat(#15): 대출 내역 검색·목록 API 추가bookTitle/borrowerName/department는 모두 부분 일치(LIKE) + 전부 AND 조합loanDate desc기본 정렬테스트
주의 사항 및 참고사항
bookTitle은 현재 MySQL LIKE 기반으로 Book 검색이 추후OpenSearch로 전환되면 해당 부분도 리펙토링 필요