Conversation
There was a problem hiding this comment.
Code Review
이번 PR은 키워드 검색 및 페이지네이션을 지원하는 도서 목록 조회 API(Controller, Service, Repository)를 새롭게 추가합니다. 코드 리뷰에서는 검색 키워드의 길이 제한 검증 추가, 상속된 페이지네이션 필드에 대한 유효성 검증 어노테이션 재정의, API 응답 시 물리적 상태 대신 논리적 상태 반환, 그리고 Querydsl 조건절 생성 시 Kotlin의 listOfNotNull을 활용한 안전한 Null 처리 등 안정성과 유지보수성을 높이기 위한 개선 사항들이 제안되었습니다.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| data class ReadBookListRequest( | ||
| @Schema(description = "검색 키워드 (제목/저자 부분 검색)", example = "릴케") | ||
| val keyword: String? = null, |
There was a problem hiding this comment.
검색 키워드(keyword)에 대한 길이 제한 검증이 누락되어 있습니다. 사용자가 악의적으로 매우 긴 문자열을 입력할 경우 데이터베이스 쿼리 성능 저하나 예기치 못한 오류가 발생할 수 있으므로, @field:Size(max = 100) 등을 통해 적절한 길이 제한 검증을 추가하는 것을 권장합니다.
| data class ReadBookListRequest( | |
| @Schema(description = "검색 키워드 (제목/저자 부분 검색)", example = "릴케") | |
| val keyword: String? = null, | |
| data class ReadBookListRequest( | |
| @Schema(description = "검색 키워드 (제목/저자 부분 검색)", example = "릴케") | |
| @field:Size(max = 100, message = "검색어는 최대 100자까지 입력 가능합니다.") | |
| val keyword: String? = null, |
References
- 각 아키텍처 레이어에서 유효성 검증을 수행해야 합니다. 컨트롤러 레벨에서는 @Valid와 같은 프레임워크 기능을 활용하여 검증을 구현합니다.
| @Schema(description = "페이지 번호 (1부터 시작, 기본값: 1)", example = "1") | ||
| override val page: Int = 1, | ||
| @Schema(description = "페이지 크기 (기본값: 10, 최대: 100)", example = "10") | ||
| override val size: Int = 10, | ||
| ) : PaginationRequest(page, size) |
There was a problem hiding this comment.
요청 객체에서 부모 클래스의 page 및 size 프로퍼티를 재정의할 때는, 올바른 API 문서화를 위해 재정의된 프로퍼티에도 @Schema 어노테이션을 명시적으로 적용해야 합니다. 또한, 재정의 시 부모 클래스의 Bean Validation 어노테이션이 상속되지 않으므로 @field:Min 등의 검증 어노테이션도 함께 추가해주어야 합니다.
@Schema(description = "페이지 번호 (1부터 시작, 기본값: 1)", example = "1")
@field:Min(1)
override val page: Int = 1,
@Schema(description = "페이지 크기 (기본값: 10, 최대: 100)", example = "10")
@field:Min(1) @field:Max(100)
override val size: Int = 10,
) : PaginationRequest(page, size)References
- 요청 객체에서 부모 클래스의 프로퍼티(예: page, size)를 재정의할 때는 올바른 API 문서화를 위해 재정의된 프로퍼티에도 @Schema 어노테이션을 명시적으로 적용해야 합니다.
| data class BookItemDetail( | ||
| @Schema(description = "청구기호", example = "A-001") | ||
| val callNumber: String, | ||
| @Schema(description = "대출 상태", example = "AVAILABLE") | ||
| val status: BookItem.Status, | ||
| ) |
There was a problem hiding this comment.
특정 도메인 엔티티를 나타내는 API 엔드포인트에서는 연관된 엔티티의 물리적 상태(예: BookItem.Status) 대신 해당 도메인 엔티티의 논리적 상태를 응답으로 반환하여 의미적 명확성을 확보해야 합니다. 현재 BookItemDetail에서 BookItem.Status를 직접 반환하는 대신, 도메인의 논리적 상태를 반환하도록 변경하는 것을 권장합니다.
References
- 특정 도메인 엔티티를 나타내는 API 엔드포인트에서는 연관된 엔티티의 물리적 상태 대신 해당 도메인 엔티티의 논리적 상태를 반환하여 의미적 명확성을 확보해야 합니다.
| val predicates = | ||
| arrayOf( | ||
| book.deleted.isFalse, | ||
| keywordContains(request.keyword), | ||
| ) |
There was a problem hiding this comment.
현재 predicates 배열에 keywordContains(request.keyword)의 결과로 null이 포함될 수 있습니다. Querydsl의 where 메서드는 null 파라미터를 무시하도록 설계되어 있지만, 명시적으로 null을 배제한 논리 조건 배열을 전달하는 것이 더 안전하고 직관적입니다.
Kotlin의 listOfNotNull을 사용하면 null이 아닌 조건들만 깔끔하게 필터링하여 안전하게 전달할 수 있습니다.
| val predicates = | |
| arrayOf( | |
| book.deleted.isFalse, | |
| keywordContains(request.keyword), | |
| ) | |
| val predicates = | |
| listOfNotNull( | |
| book.deleted.isFalse, | |
| keywordContains(request.keyword), | |
| ).toTypedArray() |
References
- Kotlin의 Null Safety를 적절히 활용하고 있는지 확인하고, Java 스타일의 처리 대신 Kotlin의 장점을 살린 코드를 제안합니다. (link)
관련 이슈
작업 내용
GET /books) 구현 (키워드(제목/저자) 부분 일치 검색 + 페이지네이션 지원)bookIds로 소장본을 별도 조회 (bookitemjoin에 의한 N+1 방지)테스트
주의 사항
부하 테스트 시 성능 저하 확인되어 FULLTEXT INDEX 기반으로 리팩터링 예정