Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.jetbrains.kotlin:kotlin-reflect'
implementation 'tools.jackson.module:jackson-module-kotlin'
implementation 'io.github.oshai:kotlin-logging-jvm:7.0.0'
implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.8.6'
runtimeOnly 'com.mysql:mysql-connector-j'
Expand Down
4 changes: 4 additions & 0 deletions mysql/index.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# 사용처 : CreateBookService.execute, UpdateBookService.execute
# 활성 도서끼리만 ISBN 중복을 금지한다.
CREATE UNIQUE INDEX ux_books_active_isbn
ON books ((IF(deleted_at IS NULL, isbn, NULL)));
42 changes: 42 additions & 0 deletions src/main/kotlin/org/library/book/application/CreateBookService.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.library.book.application

import io.swagger.v3.oas.annotations.media.Schema
import jakarta.validation.constraints.NotBlank
import org.library.book.domain.Book
import org.library.book.domain.BookRepository
import org.library.book.domain.error.BookError
import org.library.book.dto.BookResponse
import org.library.core.application.Result
import org.library.core.application.err
import org.library.core.application.ok
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional

@Service
class CreateBookService(
private val bookRepository: BookRepository,
) {

@Transactional
fun execute(request: Request): Result<BookResponse, BookError> {
val isbn = Book.normalizeIsbn(request.isbn)
if (isbn != null && bookRepository.findByIsbnAndDeletedAtIsNull(isbn) != null) {
return BookError.DUPLICATE_ISBN.err()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] ISBN 중복 검사가 check-then-act라 동시 요청 시 레이스 컨디션 발생

Problem: findByIsbnAndDeletedAtIsNull 조회와 save 사이에 DB 락이나 유니크 제약이 없다. 두 요청이 같은 ISBN으로 거의 동시에 들어오면 둘 다 중복 검사를 통과한 뒤 각자 커밋되어, DUPLICATE_ISBN이 막으려는 불변식이 깨진 채로 활성 도서 2건이 동일 ISBN을 가지게 된다.

Evidence: Book.kt#L24isbn 컬럼에 unique = true가 없고, 레포지토리 전체를 확인해도 마이그레이션/스키마 파일이 없어 DB 레벨 유니크 제약이 전혀 없다. 같은 패턴이 UpdateBookService.kt#L25-L31에도 있다.

Fix direction: 소프트 삭제 특성상 단순 컬럼 유니크 제약은 부적합하므로(삭제된 도서의 ISBN은 재사용 가능해야 함), deleted_at IS NULL 조건의 부분 유니크 인덱스를 DB에 추가하고, 저장 시 발생하는 제약 위반 예외를 잡아 DUPLICATE_ISBN으로 변환하는 방식을 권장한다.

try {
    val book = bookRepository.save(
        Book(title = request.title, author = request.author, isbn = isbn),
    )
    return BookResponse.from(book).ok()
} catch (e: DataIntegrityViolationException) {
    return BookError.DUPLICATE_ISBN.err()
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정 완료

  • 인덱스
    deleted_at IS NULL 조건의 부분 인덱스는 PostgreSQL 기능으로 MySQL에서 사용이 불가능하여
    함수 기반 유니크 인덱스를 통해 인덱스를 추가하여 활성 도서끼리만 ISBN 중복을 금지

    CREATE UNIQUE INDEX ux_books_active_isbn
        ON books ((IF(deleted_at IS NULL, isbn, NULL)));
  • 예외 처리
    예외를 전파하여 핸들러에서 ConstraintKind.UNIQUE 만 409로 반환하고 있습니다.

}
val book = bookRepository.save(
Book(title = request.title, author = request.author, isbn = isbn),
)
return BookResponse.from(book).ok()
}

data class Request(
@field:NotBlank
@field:Schema(description = "도서 제목", example = "클린 아키텍처")
val title: String,
@field:NotBlank
@field:Schema(description = "저자", example = "로버트 마틴")
val author: String,
@field:Schema(description = "ISBN (미입력 가능)", example = "9788966262472", nullable = true)
val isbn: String? = null,
)
}
23 changes: 23 additions & 0 deletions src/main/kotlin/org/library/book/application/DeleteBookService.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.library.book.application

import org.library.book.domain.BookRepository
import org.library.book.domain.error.BookError
import org.library.core.application.Result
import org.library.core.application.err
import org.library.core.application.ok
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional

@Service
class DeleteBookService(
private val bookRepository: BookRepository,
) {

@Transactional
fun execute(id: Long): Result<Unit, BookError> {
val book = bookRepository.findByIdAndDeletedAtIsNull(id)
?: return BookError.NOT_FOUND.err()
book.softDelete()
return Unit.ok()
}
}
23 changes: 23 additions & 0 deletions src/main/kotlin/org/library/book/application/GetBookService.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.library.book.application

import org.library.book.domain.BookRepository
import org.library.book.domain.error.BookError
import org.library.book.dto.BookResponse
import org.library.core.application.Result
import org.library.core.application.err
import org.library.core.application.ok
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional

@Service
@Transactional(readOnly = true)
class GetBookService(
private val bookRepository: BookRepository,
) {

fun execute(id: Long): Result<BookResponse, BookError> {
val book = bookRepository.findByIdAndDeletedAtIsNull(id)
?: return BookError.NOT_FOUND.err()
return BookResponse.from(book).ok()
}
}
47 changes: 47 additions & 0 deletions src/main/kotlin/org/library/book/application/UpdateBookService.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package org.library.book.application

import io.swagger.v3.oas.annotations.media.Schema
import jakarta.validation.constraints.NotBlank
import org.library.book.domain.Book
import org.library.book.domain.BookRepository
import org.library.book.domain.error.BookError
import org.library.book.dto.BookResponse
import org.library.core.application.Result
import org.library.core.application.err
import org.library.core.application.ok
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional

@Service
class UpdateBookService(
private val bookRepository: BookRepository,
) {

@Transactional
fun execute(id: Long, request: Request): Result<BookResponse, BookError> {
val book = bookRepository.findByIdAndDeletedAtIsNull(id)
?: return BookError.NOT_FOUND.err()

val isbn = Book.normalizeIsbn(request.isbn)
if (isbn != null) {
val owner = bookRepository.findByIsbnAndDeletedAtIsNull(isbn)
if (owner != null && owner.id != id) {
return BookError.DUPLICATE_ISBN.err()
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] 동일한 ISBN 레이스 컨디션이 수정 API에도 존재

Problem: owner == null || owner.id == id 검사와 book.update(...) 사이에 락이나 DB 유니크 제약이 없다. 서로 다른 도서를 같은 새 ISBN으로 동시에 수정하면 둘 다 검사를 통과한 뒤 커밋되어 DUPLICATE_ISBN이 막으려는 불변식이 깨진다.

Evidence: 동일 패턴에 대한 근본 원인과 수정 방향은 CreateBookService.kt#L23 코멘트에 정리했다.

Fix direction: deleted_at IS NULL 부분 유니크 인덱스를 추가하고, book.update(...) 저장 시 발생하는 제약 위반 예외를 DUPLICATE_ISBN으로 변환하는 방식을 동일하게 적용한다.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정 완료 (상단 리뷰 참고)


book.update(title = request.title, author = request.author, isbn = isbn)
return BookResponse.from(book).ok()
}

data class Request(
@field:NotBlank
@field:Schema(description = "도서 제목", example = "클린 아키텍처")
val title: String,
@field:NotBlank
@field:Schema(description = "저자", example = "로버트 마틴")
val author: String,
@field:Schema(description = "ISBN (미입력 시 기존 값이 지워진다)", example = "9788966262472", nullable = true)
val isbn: String? = null,
)
}
81 changes: 81 additions & 0 deletions src/main/kotlin/org/library/book/controller/BookController.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package org.library.book.controller

import io.swagger.v3.oas.annotations.Operation
import io.swagger.v3.oas.annotations.Parameter
import io.swagger.v3.oas.annotations.media.Content
import io.swagger.v3.oas.annotations.media.Schema
import io.swagger.v3.oas.annotations.responses.ApiResponse
import io.swagger.v3.oas.annotations.tags.Tag
import jakarta.validation.Valid
import org.library.book.application.CreateBookService
import org.library.book.application.DeleteBookService
import org.library.book.application.GetBookService
import org.library.book.application.UpdateBookService
import org.library.book.domain.error.BookError
import org.library.book.dto.BookResponse
import org.library.core.application.getOrThrow
import org.library.core.swagger.ApiErrorCode
import org.springframework.http.HttpStatus
import org.springframework.http.ResponseEntity
import org.springframework.web.bind.annotation.DeleteMapping
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.PatchMapping
import org.springframework.web.bind.annotation.PathVariable
import org.springframework.web.bind.annotation.PostMapping
import org.springframework.web.bind.annotation.RequestBody
import org.springframework.web.bind.annotation.RequestMapping
import org.springframework.web.bind.annotation.RestController

@Tag(name = "Book", description = "도서 관리 API")
@RestController
@RequestMapping("/books")
class BookController(
private val createBookService: CreateBookService,
private val getBookService: GetBookService,
private val updateBookService: UpdateBookService,
private val deleteBookService: DeleteBookService,
) {

@Operation(
summary = "도서 등록",
description = "새 도서를 등록한다. ISBN은 미입력할 수 있고, 값이 있으면 중복될 수 없다.",
)
@ApiResponse(
responseCode = "201",
description = "도서 등록 성공",
content = [Content(schema = Schema(implementation = BookResponse::class))],
)
@ApiErrorCode(errorCodes = [BookError::class], only = ["DUPLICATE_ISBN"])
@PostMapping
fun create(@Valid @RequestBody request: CreateBookService.Request): ResponseEntity<BookResponse> {
val book = createBookService.execute(request).getOrThrow()
return ResponseEntity.status(HttpStatus.CREATED).body(book)
}

@Operation(summary = "도서 단건 조회", description = "ID로 도서 한 건을 조회한다.")
@ApiErrorCode(errorCodes = [BookError::class], only = ["NOT_FOUND"])
@GetMapping("/{id}")
fun get(@Parameter(description = "도서 ID") @PathVariable id: Long): BookResponse =
getBookService.execute(id).getOrThrow()

@Operation(
summary = "도서 수정",
description = "도서의 제목·저자·ISBN을 수정한다. 요청 본문이 곧 최종 상태이며, ISBN을 생략하면 기존 값이 지워진다.",
)
@ApiErrorCode(errorCodes = [BookError::class], only = ["NOT_FOUND", "DUPLICATE_ISBN"])
@PatchMapping("/{id}")
fun update(
@Parameter(description = "도서 ID") @PathVariable id: Long,
@Valid @RequestBody request: UpdateBookService.Request,
): BookResponse =
updateBookService.execute(id, request).getOrThrow()

@Operation(summary = "도서 삭제", description = "도서를 소프트 삭제한다.")
@ApiResponse(responseCode = "204", description = "도서 삭제 성공")
@ApiErrorCode(errorCodes = [BookError::class], only = ["NOT_FOUND"])
@DeleteMapping("/{id}")
fun delete(@Parameter(description = "도서 ID") @PathVariable id: Long): ResponseEntity<Unit> {
deleteBookService.execute(id).getOrThrow()
return ResponseEntity.noContent().build()
}
}
8 changes: 8 additions & 0 deletions src/main/kotlin/org/library/book/domain/Book.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ class Book(
require(author.isNotBlank()) { "저자는 비어 있을 수 없습니다." }
}

fun update(title: String, author: String, isbn: String?) {
require(title.isNotBlank()) { "제목은 비어 있을 수 없습니다." }
require(author.isNotBlank()) { "저자는 비어 있을 수 없습니다." }
this.title = title
this.author = author
this.isbn = normalizeIsbn(isbn)
}

companion object {

fun normalizeIsbn(isbn: String?): String? = isbn?.trim()?.takeIf { it.isNotBlank() }
Expand Down
4 changes: 4 additions & 0 deletions src/main/kotlin/org/library/book/domain/BookRepository.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@ package org.library.book.domain
import org.springframework.data.jpa.repository.JpaRepository

interface BookRepository : JpaRepository<Book, Long> {

fun findByIsbnAndDeletedAtIsNull(isbn: String): Book?

fun findByIdAndDeletedAtIsNull(id: Long): Book?
}
15 changes: 15 additions & 0 deletions src/main/kotlin/org/library/book/domain/error/BookError.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.library.book.domain.error

import org.library.core.exception.ErrorCode
import org.springframework.http.HttpStatus

enum class BookError(
override val status: HttpStatus,
override val message: String,
) : ErrorCode {
NOT_FOUND(HttpStatus.NOT_FOUND, "도서를 찾을 수 없습니다."),
DUPLICATE_ISBN(HttpStatus.CONFLICT, "이미 등록된 ISBN입니다."),
;

override val code: String get() = name
}
29 changes: 29 additions & 0 deletions src/main/kotlin/org/library/book/dto/BookResponse.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.library.book.dto

import io.swagger.v3.oas.annotations.media.Schema
import org.library.book.domain.Book
import java.time.LocalDateTime

@Schema(description = "도서 응답")
data class BookResponse(
@field:Schema(description = "도서 ID", example = "1")
val id: Long,
@field:Schema(description = "도서 제목", example = "클린 아키텍처")
val title: String,
@field:Schema(description = "저자", example = "로버트 마틴")
val author: String,
@field:Schema(description = "ISBN (미입력 가능)", example = "9788966262472", nullable = true)
val isbn: String?,
@field:Schema(description = "등록 일시")
val createdAt: LocalDateTime,
) {
companion object {
fun from(book: Book): BookResponse = BookResponse(
id = book.id,
title = book.title,
author = book.author,
isbn = book.isbn,
createdAt = book.createdAt,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ enum class CommonErrorCode(
) : ErrorCode {
INTERNAL_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "서버 내부 오류가 발생했습니다."),
INVALID_INPUT(HttpStatus.BAD_REQUEST, "요청 값이 올바르지 않습니다."),
DATA_CONFLICT(HttpStatus.CONFLICT, "이미 존재하는 데이터입니다."),
;

override val code: String get() = name
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package org.library.core.exception

import io.github.oshai.kotlinlogging.KotlinLogging
import org.hibernate.exception.ConstraintViolationException
import org.library.core.logging.TraceIdFilter
import org.slf4j.MDC
import org.springframework.dao.DataIntegrityViolationException
import org.springframework.http.HttpHeaders
import org.springframework.http.HttpStatus
import org.springframework.http.HttpStatusCode
Expand All @@ -23,6 +25,19 @@ class SystemExceptionHandler : ResponseEntityExceptionHandler() {
fun handleDomain(e: DomainException): ProblemDetail =
problem(e.errorCode.status, e.errorCode.code, e.errorCode.message)

@ExceptionHandler(DataIntegrityViolationException::class)
fun handleDataIntegrity(e: DataIntegrityViolationException): ProblemDetail {
val kind = (e.cause as? ConstraintViolationException)?.kind
if (kind != ConstraintViolationException.ConstraintKind.UNIQUE) return handleUnexpected(e)

log.warn(e) { "Unique constraint violation" }
return problem(
CommonErrorCode.DATA_CONFLICT.status,
CommonErrorCode.DATA_CONFLICT.code,
CommonErrorCode.DATA_CONFLICT.message,
)
}

@ExceptionHandler(Exception::class)
fun handleUnexpected(e: Exception): ProblemDetail {
log.error(e) { "Unhandled exception" }
Expand Down