refactor: 기본정렬과 동적정렬 UPDATE시간으로도 가능하도록 수정 , 감상평 Optional #126
Conversation
Walkthrough기본 정렬 필드를 Changes
Sequence Diagram해당 변경들은 기본 정렬 필드와 nullable 전환을 위한 리팩토링으로 새로운 제어 흐름이나 비동기 상호작용을 도입하지 않으므로 시퀀스 다이어그램을 생략합니다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 분 검토 시 주의할 파일/영역:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt (1)
161-170: UPDATED_DATE 정렬 추가 및 기본값 변경, 방향성 OK — 안정성·성능 2가지 확인 권고
- 구현은 타당합니다. null 시 updatedAt.desc() 기본값도 의도에 부합합니다.
- 동일 updatedAt 행들 간 안정적 페이지네이션을 위해 2차 정렬(id DESC 등) 추가를 고려하세요.
- updatedAt 기준 정렬이 빈번하므로 user_book.updated_at 인덱스 존재 여부를 확인하세요.
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt (2)
34-36: 문서 예시가 제약과 불일치(@SiZe(max=3) vs 예시 4개)예시는 3개 이하로 맞추세요.
- @field:Schema(description = "수정할 감정 태그 목록", example = """["따뜻함","즐거움","슬픔","깨달음"]""") + @field:Schema(description = "수정할 감정 태그 목록", example = """["따뜻함","즐거움","슬픔"]""")
30-33:validReview()메서드 제거로 인한 컴파일 오류 - 두 곳에서 호출 실패
UpdateReadingRecordRequest에서validReview()메서드가 제거되었으나, 서비스 계층에서 여전히 호출하고 있어 빌드가 실패합니다:
ReadingRecordService.kt:28-request.validReview()호출ReadingRecordService.kt:67-request.validReview()호출
review필드가 선택사항(String?)이므로, 다음 중 하나로 수정하세요:
UpdateReadingRecordRequest에fun validReview(): String? = review메서드 추가- 또는 서비스에서
request.review(nullable 처리)로 변경apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt (1)
157-166:sort파라미터 이름 충돌(Spring Pageable vs Enum) — 400 바인딩 실패 위험메서드에
@RequestParam sort: UserBookSortType?와Pageable(기본sort파라미터)을 동시에 둔 상태에서 예시?sort=updatedAt,desc를 사용하면, enum 바인딩에서 실패(400)할 수 있습니다. 파라미터 명 분리 또는 노출 정책 정리가 필요합니다.권장 수정안(도메인 정렬 파라미터를
sortType으로 변경, 예시도 일치화):- @RequestParam(required = false) @Parameter(description = "정렬 방식") sort: UserBookSortType?, + @RequestParam(name = "sortType", required = false) + @Parameter( + description = "정렬 방식 (TITLE_ASC|TITLE_DESC|CREATED_DATE_ASC|CREATED_DATE_DESC|UPDATED_DATE_ASC|UPDATED_DATE_DESC)", + example = "UPDATED_DATE_DESC" + ) + sortType: UserBookSortType?, @@ - @Parameter( - description = "페이징 정보 (기본값: 10개, 최신 수정일 순)", - example = "?page=0&size=10&sort=updatedAt,desc" - ) + @Parameter( + description = "페이징 정보 (기본값: 10개, 최신 수정일 순)", + example = "?page=0&size=10" + )또는 Pageable의 sort 파라미터를 숨기고(문서) 도메인 정렬만 사용하도록 일원화하는 방안도 고려하세요.
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt (1)
85-88: 문서 문구 최신화 필요(“등록순”→“수정일 순” 포함)기본/권장 정렬이 updatedAt 기준으로 바뀌었으므로 설명을 최신화해 주세요.
- description = "사용자의 책에 대한 독서 기록을 페이징하여 조회합니다. 정렬은 페이지 번호 또는 최신 등록순으로 가능합니다." + description = "사용자의 책에 대한 독서 기록을 페이징하여 조회합니다. 정렬은 페이지 번호 또는 최신 수정일 순 등으로 가능합니다."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/CreateReadingRecordRequest.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookSortType.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/JpaUserBookQuerydslRepositoryImpl.kt(1 hunks)
🔇 Additional comments (1)
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookSortType.kt (1)
8-10: 새 정렬 타입 추가 LGTM — 노출·호환성 확인도메인 확장은 적절합니다. 컨트롤러의 쿼리 파라미터/문서 및 클라이언트 사용처도 함께 반영됐는지 확인 부탁드립니다.
| @AuthenticationPrincipal @Parameter(description = "인증된 사용자 ID") userId: UUID, | ||
| @PathVariable @Parameter(description = "독서 기록을 조회할 사용자 책 ID") userBookId: UUID, | ||
| @RequestParam(required = false) @Parameter(description = "정렬 방식 (PAGE_NUMBER_ASC, PAGE_NUMBER_DESC, CREATED_DATE_ASC, CREATED_DATE_DESC)") sort: ReadingRecordSortType?, | ||
| @PageableDefault(size = 10, sort = ["createdAt"], direction = Sort.Direction.DESC) | ||
| @Parameter(description = "페이지네이션 정보 (페이지 번호, 페이지 크기, 정렬 방식)") pageable: Pageable | ||
| @PageableDefault(size = 10, sort = ["updatedAt"], direction = Sort.Direction.DESC) | ||
| @Parameter(description = "페이지네이션 정보 (기본값: 10개, 최신 수정일 순)") pageable: Pageable | ||
| ): ResponseEntity<ReadingRecordPageResponse> |
There was a problem hiding this comment.
sort 파라미터 충돌 위험 — enum 정렬 vs Pageable 정렬 분리 필요
여기도 @RequestParam sort: ReadingRecordSortType?와 Pageable의 sort가 충돌합니다. ?sort=updatedAt,desc 사용 시 enum 바인딩 실패 가능.
권장 수정:
- @RequestParam(required = false) @Parameter(description = "정렬 방식 (PAGE_NUMBER_ASC, PAGE_NUMBER_DESC, CREATED_DATE_ASC, CREATED_DATE_DESC)") sort: ReadingRecordSortType?,
+ @RequestParam(name = "sortType", required = false)
+ @Parameter(description = "정렬 방식 (PAGE_NUMBER_ASC|PAGE_NUMBER_DESC|CREATED_DATE_ASC|CREATED_DATE_DESC|UPDATED_DATE_ASC|UPDATED_DATE_DESC)", example = "UPDATED_DATE_DESC")
+ sortType: ReadingRecordSortType?,
- @PageableDefault(size = 10, sort = ["updatedAt"], direction = Sort.Direction.DESC)
- @Parameter(description = "페이지네이션 정보 (기본값: 10개, 최신 수정일 순)") pageable: Pageable
+ @PageableDefault(size = 10, sort = ["updatedAt"], direction = Sort.Direction.DESC)
+ @Parameter(description = "페이지네이션 정보 (기본값: 10개, 최신 수정일 순)") pageable: Pageable서비스/레포에서 sortType 반영되도록 함께 조정이 필요합니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt
around lines 105 to 110, the request parameter name `sort` collides with Spring
Pageable's `sort` query parameter causing enum binding failures; rename the enum
param (e.g., `sortType`), update the annotation to @RequestParam(name =
"sortType", required = false) and the method signature accordingly, then ensure
the controller converts that sortType into a Spring Sort (or passes it to the
service/repository) and explicitly ignore or reconcile Pageable.sort so the
service applies the enum-based ordering consistently.
| @RequestParam(required = false) @Parameter(description = "정렬 방식 (PAGE_NUMBER_ASC, PAGE_NUMBER_DESC, CREATED_DATE_ASC, CREATED_DATE_DESC)") sort: ReadingRecordSortType?, | ||
| @PageableDefault(size = 10, sort = ["createdAt"], direction = Sort.Direction.DESC) | ||
| @Parameter(description = "페이지네이션 정보 (페이지 번호, 페이지 크기, 정렬 방식)") pageable: Pageable | ||
| @PageableDefault(size = 10, sort = ["updatedAt"], direction = Sort.Direction.DESC) | ||
| @Parameter(description = "페이지네이션 정보 (기본값: 10개, 최신 수정일 순)") pageable: Pageable | ||
| ): ResponseEntity<ReadingRecordPageResponse> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
OpenAPI 예시/문서에 UPDATED_DATE_ 추가 고려*
도메인 정렬 타입에 UPDATED_DATE가 추가되었으므로 문서에도 반영하세요.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt
around lines 107 to 110, the @Parameter description for the sort RequestParam
omits the newly added UPDATED_DATE sort values; update the description to
include UPDATED_DATE_ASC and UPDATED_DATE_DESC (e.g. list all supported enums
including PAGE_NUMBER_ASC, PAGE_NUMBER_DESC, CREATED_DATE_ASC,
CREATED_DATE_DESC, UPDATED_DATE_ASC, UPDATED_DATE_DESC) so the OpenAPI docs
reflect the domain change.
| @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.") | ||
| @field:Schema(description = "감상평", example = "이 책은 매우 인상적이었습니다.", required = true) | ||
| @field:Schema(description = "감상평", example = "이 책은 매우 인상적이었습니다.", required = false) | ||
| val review: String? = null, |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! 감상평 필드가 선택사항으로 올바르게 변경되었습니다.
@NotBlank 제약 조건이 제거되고 required = false로 설정되어 감상평을 선택적으로 입력할 수 있게 되었습니다. 기존의 @Size 검증은 유지되어 값이 제공될 경우 최대 1000자로 제한됩니다.
다음 스크립트를 실행하여 백엔드 로직이 nullable review를 올바르게 처리하는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: CreateReadingRecordRequest를 사용하는 서비스 계층이나 컨트롤러에서
# review 필드를 null로 처리하는지 확인
# Test: CreateReadingRecordRequest를 사용하는 모든 위치 검색
rg -n --type=kt -C 5 "CreateReadingRecordRequest" Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for CreateReadingRecordRequest usages without specifying file type
rg -n "CreateReadingRecordRequest" -A 5 -B 5
# Search for any calls to validReview method
echo "=== Searching for validReview() calls ==="
rg -n "validReview\s*\(" -A 3 -B 3
# Check the current state of the CreateReadingRecordRequest file
echo "=== Current CreateReadingRecordRequest.kt file ==="
fd "CreateReadingRecordRequest.kt" -x cat {}Length of output: 18142
긴급: validReview() 메서드가 제거되었으나 서비스 계층에서 여전히 호출되고 있습니다.
CreateReadingRecordRequest.kt에서 validReview() 메서드가 제거되었지만, ReadingRecordService.kt의 다음 위치에서 여전히 호출되고 있어 컴파일 실패가 발생합니다:
- 라인 28:
review = request.validReview(), - 라인 67:
review = request.validReview(),
수정 방법:
CreateReadingRecordRequest.kt에validReview()메서드를 추가하여 nullable review를 반환하거나,ReadingRecordService.kt에서request.validReview()호출을request.review로 변경해야 합니다.
review 필드가 이제 선택사항이므로 force unwrap(!!)을 사용할 수 없습니다.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/CreateReadingRecordRequest.kt
around lines 33-35 and in
apis/src/main/kotlin/org/yapp/apis/readingrecord/ReadingRecordService.kt at
lines 28 and 67, the request.validReview() method was removed causing
compilation failures; fix by either (A) reintroducing a simple validReview():
String? method on CreateReadingRecordRequest that returns the nullable review
as-is, or (B) update ReadingRecordService.kt at the two call sites to use
request.review (nullable) instead of request.validReview(); do not use !!,
preserve nullability.
| fun validPageNumber(): Int = pageNumber!! | ||
| fun validQuote(): String = quote!! | ||
| fun validReview(): String = review!! | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! validReview() 헬퍼 메서드 제거가 적절합니다.
감상평이 선택사항이 되면서 validReview() 메서드(이전의 review!!)가 제거되었습니다. 필수 필드인 pageNumber와 quote는 여전히 검증 헬퍼를 유지하고 있어 일관성 있는 구조를 유지하고 있습니다.
제거된 validReview() 메서드를 호출하는 코드가 있는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: validReview() 메서드 사용처 검색.
# 예상 결과: 사용처가 없어야 함 (있다면 컴파일 에러 발생)
# Test: validReview() 호출 검색
rg -n --type=kt "\.validReview\(\)"Length of output: 26
🏁 Script executed:
#!/bin/bash
# Description: validReview() 메서드 사용처 검색 (수정된 버전)
# 방법: glob 패턴으로 .kt 파일만 검색
rg -n --glob="*.kt" "\.validReview\(\)"Length of output: 255
🏁 Script executed:
#!/bin/bash
# Description: ReadingRecordService.kt의 validReview() 호출 부분 확인
# 라인 28 주변 코드 보기 (context: 10줄)
echo "=== Line 28 context ==="
rg -n --glob="*.kt" -B5 -A5 "validReview\(\)" apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt | head -20
echo ""
echo "=== Full view of problematic method ==="
# ReadingRecordService.kt 전체 구조 파악
cat apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.ktLength of output: 3998
🏁 Script executed:
#!/bin/bash
# Description: ReadingRecordDomainService의 review 파라미터 타입 확인
# ReadingRecordDomainService 찾기
find . -name "ReadingRecordDomainService.kt" -type f 2>/dev/null | head -5
# createReadingRecord, modifyReadingRecord 메서드 시그니처 확인
rg -n --glob="*.kt" -A10 "fun createReadingRecord|fun modifyReadingRecord" | grep -A10 "ReadingRecordDomainService" | head -30Length of output: 3575
🏁 Script executed:
#!/bin/bash
# Description: 요청 DTO 파일들에서 review 필드 확인
echo "=== CreateReadingRecordRequest.kt ==="
cat apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/CreateReadingRecordRequest.kt
echo ""
echo "=== UpdateReadingRecordRequest.kt ==="
cat apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.ktLength of output: 3252
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt에서 컴파일 에러 수정 필요
validReview() 메서드가 제거되었지만, 서비스 레이어에서 여전히 호출되고 있습니다.
- 라인 28:
review = request.validReview()→review = request.review로 변경 - 라인 67:
review = request.validReview()→review = request.review로 변경
추가로, CreateReadingRecordRequest의 review는 String? (nullable)이지만 ReadingRecordDomainService.createReadingRecord()는 review: String (non-nullable)을 기대합니다. 도메인 서비스의 파라미터 타입도 함께 검토 필요합니다.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt
around lines 28 and 67, replace the removed request.validReview() calls with
request.review and handle the nullable mismatch with the domain API: either (A)
update ReadingRecordDomainService.createReadingRecord(...) signature to accept
review: String? so it accepts the DTO's nullable review, or (B) add an explicit
null-check in the service before calling the domain (e.g., provide a default,
skip passing review, or throw a controlled validation exception) so you no
longer pass a nullable String into a parameter typed as non-nullable; also
ensure CreateReadingRecordRequest
(apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/CreateReadingRecordRequest.kt
line ~43) remains String? if you choose option A, or adjust DTO/validation if
you choose to keep domain non-nullable.
| fun validPageNumber(): Int = pageNumber!! | ||
| fun validQuote(): String = quote!! | ||
| fun validReview(): String = review!! | ||
|
|
||
| fun validEmotionTags(): List<String> = emotionTags!! | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
"!!" 사용으로 500(NPE) 위험 — 명시적 검증으로 대체 권장
PATCH DTO 특성상 필드가 null일 수 있는데, 현재 메서드들이 !!로 강제 해제합니다. 클라이언트 오류가 서버 500으로 번질 수 있습니다. requireNotNull 등으로 400 계열로 매핑되도록 바꾸세요.
다음과 같이 최소 수정 제안드립니다:
- fun validPageNumber(): Int = pageNumber!!
- fun validQuote(): String = quote!!
+ fun validPageNumber(): Int =
+ requireNotNull(pageNumber) { "pageNumber는 null일 수 없습니다." }
+ fun validQuote(): String =
+ requireNotNull(quote) { "quote는 null일 수 없습니다." }
- fun validEmotionTags(): List<String> = emotionTags!!
+ fun validEmotionTags(): List<String> =
+ requireNotNull(emotionTags) { "emotionTags는 null일 수 없습니다." }또는, 서비스/핸들러에서 null 여부를 분기 처리하고 이 유틸 메서드들은 제거하는 방안도 고려하세요.
📝 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.
| fun validPageNumber(): Int = pageNumber!! | |
| fun validQuote(): String = quote!! | |
| fun validReview(): String = review!! | |
| fun validEmotionTags(): List<String> = emotionTags!! | |
| } | |
| fun validPageNumber(): Int = | |
| requireNotNull(pageNumber) { "pageNumber는 null일 수 없습니다." } | |
| fun validQuote(): String = | |
| requireNotNull(quote) { "quote는 null일 수 없습니다." } | |
| fun validEmotionTags(): List<String> = | |
| requireNotNull(emotionTags) { "emotionTags는 null일 수 없습니다." } | |
| } |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt
around lines 38 to 42, the methods use the Kotlin non-null assertion operator
(!!) which will throw an NPE and surface as a 500; replace these with explicit
validation using requireNotNull(...) with a clear message (e.g., "pageNumber
must not be null") so they throw an IllegalArgumentException (400-level)
instead, or alternatively remove these helper methods and perform null checks
higher in the handler/service; update the three methods to call requireNotNull
with descriptive messages.
|
CI 실패라서 체크 한번만 부탁드려요~~ |
|
🔗 관련 이슈
Close [BOOK-377/refactor] 기본정렬을 UPDATE_AT으로 변경, 감상평은 Optional로 수정합니다. #125
📘 작업 유형
✨ Feature (기능 추가)
🐞 Bugfix (버그 수정)
🔧 Refactor (코드 리팩토링)
⚙️ Chore (환경 설정)
📝 Docs (문서 작성 및 수정)
✅ Test (기능 테스트)
🎨 style (코드 스타일 수정)
📙 작업 내역
BookControllerApi기본 정렬 기준을
createdAt→updatedAt으로 변경@Parameter예시 및 설명 추가 (페이징 예시 구체화)ReadingRecordControllerApi페이지네이션 기본 정렬 기준을
createdAt→updatedAt으로 변경Swagger 문서 설명(
@Parameter) 개선CreateReadingRecordRequestreview필드의@NotBlank제거 → 선택 입력 가능하도록 변경Schema(required = false)로 수정validReview()메서드 제거 (null 허용 반영)UpdateReadingRecordRequestvalidReview()제거validEmotionTags()의 non-null 보장 로직 수정UserBookSortTypeUPDATED_DATE_ASC,UPDATED_DATE_DESC정렬 타입 추가JpaUserBookQuerydslRepositoryImplUserBookSortType에 따른 정렬 로직 추가 (updatedAt기준)기본 정렬을
updatedAt.desc()로 변경🧪 테스트 내역
API 문서(Swagger)에서 정렬 기준 변경 확인
페이징 파라미터 정상 작동 확인
review필드 nullable 처리 후 생성 요청 성공 확인기존 기능 영향 없음
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
커밋 메시지가 명확합니다
PR 제목이 컨벤션에 맞습니다
관련 이슈 번호를 작성했습니다
기능이 정상적으로 작동합니다
불필요한 코드를 제거했습니다
💬 추가 설명 or 리뷰 포인트
review필드의 nullable 처리에 따라 백엔드 유효성 검증 로직 확인 부탁드립니다.updatedAt기준 정렬 변경으로 기존 정렬 결과가 달라질 수 있습니다.향후 정렬 타입 확장 시(
RATING_DESC등)UserBookSortTypeenum만 확장하면 대응 가능하도록 설계했습니다.Summary by CodeRabbit
새로운 기능
개선 사항
작업(마이그레이션)