feat: 기록 수정, 삭제 기능 추가 #94
Conversation
Walkthrough독서기록 수정(PATCH) 및 삭제(DELETE) 엔드포인트와 요청 DTO, 서비스·유스케이스·도메인 메서드, 리포지토리 및 인프라 구현을 추가하고, 조회 시 soft-delete 필터(deletedAt IS NULL)를 적용함. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as ReadingRecordController
participant UseCase as ReadingRecordUseCase
participant AppSvc as ReadingRecordService
participant DomainSvc as ReadingRecordDomainService
participant Repo as Repositories
Client->>Controller: PATCH /reading-records/{id} (UpdateReadingRecordRequest)
Controller->>UseCase: updateReadingRecord(userId, id, request)
UseCase->>AppSvc: updateReadingRecord(id, request)
AppSvc->>DomainSvc: modifyReadingRecord(id, page, quote, review, tags)
DomainSvc->>Repo: validate & find ReadingRecord/UserBook
DomainSvc->>Repo: deleteAllByReadingRecordId(id) (if tags provided)
DomainSvc->>Repo: save updated ReadingRecord + tags
DomainSvc-->>AppSvc: ReadingRecordInfoVO
AppSvc-->>UseCase: ReadingRecordResponse
UseCase-->>Controller: ReadingRecordResponse
Controller-->>Client: 200 OK (ReadingRecordResponse)
sequenceDiagram
autonumber
actor Client
participant Controller as ReadingRecordController
participant UseCase as ReadingRecordUseCase
participant AppSvc as ReadingRecordService
participant DomainSvc as ReadingRecordDomainService
participant Repo as Repositories
Client->>Controller: DELETE /reading-records/{id}
Controller->>UseCase: deleteReadingRecord(userId, id)
UseCase->>AppSvc: deleteReadingRecord(id)
AppSvc->>DomainSvc: deleteReadingRecord(id)
DomainSvc->>Repo: validate & find ReadingRecord/UserBook
DomainSvc->>Repo: deleteAllByReadingRecordId(id)
DomainSvc->>Repo: deleteById(id)
DomainSvc->>Repo: save UserBook.decreaseReadingRecordCount()
UseCase-->>Controller: Unit
Controller-->>Client: 204 No Content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
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. ✨ 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: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecordtag/ReadingRecordTagRepository.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/JpaReadingRecordQuerydslRepositoryImpl.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/impl/ReadingRecordTagRepositoryImpl.kt(1 hunks)
⏰ 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)
- GitHub Check: build-validation
🔇 Additional comments (24)
domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt (1)
34-34: deleteById는 이미 소프트 삭제로 동작하므로 변경 불필요
프로젝트 전반에 적용된 Hibernate soft delete 패턴(@SQLDelete/@SQLRestriction)에 따라deleteById()호출 시 실제로는 UPDATE 쿼리가 실행되어 소프트 삭제가 수행됩니다. 따라서 하드 삭제 명세나 주석 추가 없이 현행대로 유지하시면 됩니다.Likely an incorrect or invalid review comment.
infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt (1)
56-58: deleteById는 이미 소프트 삭제로 동작하며, 트랜잭션 경계도 올바르게 적용되어 있습니다.
- ReadingRecordEntity에 @SQLDelete/@SQLRestriction가 적용되어 있어, jpaReadingRecordRepository.deleteById(id) 호출 시 실제로는 UPDATE 쿼리로 soft delete가 수행됩니다.
- 사용된 UseCase 계층(ReadingRecordUseCase.deleteReadingRecord)은 @transactional로 감싸여 있어, soft delete 및 UserBook 카운트 감소 로직이 동일 트랜잭션 내에서 실행됩니다.
- 물리 삭제나 FK 정합성 문제, EmptyResultDataAccessException 예외 매핑(도메인에서 findById 후 예외 처리) 등 하드 삭제 전제의 검증은 불필요합니다.
해당 리뷰 코멘트는 본 PR의 코드·설계 내용과 맞지 않으므로 무시해 주세요.
Likely an incorrect or invalid review comment.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (1)
31-33: LGTM — 음수 방지(coerceAtLeast)로 카운터 하한 보장 구현이 깔끔합니다도메인 불변성을 유지하며 명확하게 의도를 표현했습니다. 상위 로직에서 증가/감소의 균형만 보장되면 정합성 문제도 없겠습니다.
infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/JpaReadingRecordQuerydslRepositoryImpl.kt (1)
29-29: LGTM — 소프트 삭제 레코드 제외 조건 추가 적절
deletedAt.isNull()조건 추가로 조회/카운트 일관성이 확보됩니다. 변경 취지와 잘 맞습니다.infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt (1)
9-9: LGTM! JPA 파생 쿼리 메서드가 적절히 추가되었습니다.Spring Data JPA의 명명 규칙을 따르는
deleteAllByReadingRecordId메서드가 올바르게 구현되었습니다. 감정 태그 수정 시 기존 태그를 삭제하는 요구사항에 부합합니다.domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt (2)
61-78: LGTM! 독서 기록 수정 로직이 올바르게 구현되었습니다.
- nullable 파라미터에 대한 선택적 업데이트 로직이 적절합니다
updatedAt이 현재 시간으로 자동 설정됩니다- 기존 validation 로직이 value class를 통해 유지됩니다
createdAt과deletedAt이 보존되어 데이터 무결성이 유지됩니다
137-139: 소프트 삭제 구현이 적절합니다.
copy함수를 사용하여deletedAt만 현재 시간으로 설정하는 소프트 삭제 방식이 올바르게 구현되었습니다. 다른 필드들은 그대로 유지되어 데이터 무결성이 보장됩니다.infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/impl/ReadingRecordTagRepositoryImpl.kt (1)
27-29: LGTM! 리포지토리 구현이 적절합니다.JPA 리포지토리에 대한 단순한 위임 구조로 올바르게 구현되었습니다. 도메인 계층의 인터페이스를 인프라 계층에서 적절히 구현했습니다.
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt (3)
19-19: LGTM! 필요한 import가 추가되었습니다.새로운 수정 요청 DTO에 대한 import가 적절히 추가되었습니다.
136-165: LGTM! 독서 기록 수정 API 명세가 적절합니다.
- RESTful한 PATCH 메서드 사용이 적절합니다
- OpenAPI 문서화가 충분합니다
- 에러 응답 코드들(400, 404)이 적절히 정의되었습니다
- 요청 validation과 인증이 올바르게 설정되었습니다
167-188: LGTM! 독서 기록 삭제 API 명세가 적절합니다.
- RESTful한 DELETE 메서드 사용이 적절합니다
- 204 No Content 응답 코드가 삭제 성공에 적합합니다
- 404 에러 응답이 적절히 정의되었습니다
- 요청 body가 없는 삭제 API 설계가 올바릅니다
apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt (3)
8-8: Import 추가가 적절합니다.새로운 업데이트 기능을 위한 DTO import가 올바르게 추가되었습니다.
81-93: 독서 기록 수정 로직이 올바르게 구현되었습니다.사용자 검증 후 서비스 계층으로 적절히 위임하는 구조로 잘 구현되었습니다. 트랜잭션 어노테이션도 올바르게 적용되었습니다.
95-102: 독서 기록 삭제 로직이 올바르게 구현되었습니다.사용자 검증 후 서비스 계층으로 적절히 위임하는 구조로 잘 구현되었습니다. 트랜잭션 어노테이션도 올바르게 적용되었습니다.
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt (3)
6-6: Import 추가가 적절합니다.새로운 업데이트 기능을 위한 DTO import가 올바르게 추가되었습니다.
50-62: 독서 기록 수정 서비스 로직이 잘 구현되었습니다.기존 생성 로직과 일관된 패턴으로 구현되었으며, 요청 데이터 검증과 응답 매핑이 적절히 처리되었습니다.
64-68: 독서 기록 삭제 서비스 로직이 간결하고 적절합니다.도메인 서비스로의 위임이 깔끔하게 구현되었습니다.
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt (4)
10-10: 필요한 Spring 어노테이션들이 올바르게 import되었습니다.PATCH와 DELETE 매핑을 위한 어노테이션들이 적절히 추가되었습니다.
Also applies to: 14-14
20-20: 업데이트 요청 DTO import가 적절합니다.새로운 기능을 위한 DTO가 올바르게 import되었습니다.
87-99: 독서 기록 수정 API가 RESTful하게 잘 구현되었습니다.PATCH 메서드를 사용하고, 요청 검증(@Valid)과 응답 처리가 적절히 구현되었습니다. HTTP 200 OK 응답도 적절합니다.
101-108: 독서 기록 삭제 API가 RESTful하게 잘 구현되었습니다.DELETE 메서드를 사용하고, HTTP 204 No Content 응답이 삭제 작업에 적절합니다.
domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt (3)
105-107: 기존 조회 메서드 수정이 적절합니다.메서드 체이닝 방식으로 변경하여 가독성이 향상되었습니다.
109-147: 독서 기록 수정 로직이 포괄적이고 잘 구현되었습니다.
- 존재하지 않는 기록에 대한 예외 처리가 적절합니다
- 감정 태그 업데이트 시 기존 태그 삭제 후 재생성하는 로직이 올바릅니다
- 기존 생성 로직과 일관된 패턴으로 태그 생성/연결 처리가 구현되었습니다
- 중앙화된 VO 빌더 사용으로 일관성을 유지합니다
149-164: 독서 기록 삭제 로직이 잘 구현되었습니다.
- 독서 기록과 사용자 책 모두에 대한 존재 검증이 적절합니다
- 사용자 책의 독서 기록 카운트 감소 처리가 올바르게 구현되었습니다
- 적절한 예외 처리와 에러 코드 사용이 이루어졌습니다
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt
Outdated
Show resolved
Hide resolved
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt
Outdated
Show resolved
Hide resolved
| interface ReadingRecordTagRepository { | ||
| fun saveAll(readingRecordTags: List<ReadingRecordTag>): List<ReadingRecordTag> | ||
| fun findByReadingRecordId(readingRecordId: UUID): List<ReadingRecordTag> | ||
| fun deleteAllByReadingRecordId(readingRecordId: UUID) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
파생 삭제 쿼리 사용 시 영속성 컨텍스트 동기화 및 결과 피드백 개선 제안
- 인프라 JPA 레이어의
JpaReadingRecordTagRepository.deleteAllByReadingRecordId(...)는 파생 삭제 쿼리로 실행될 가능성이 큽니다. 동일 트랜잭션에서 직후 재조회/재등록을 수행한다면 영속성 컨텍스트 정합성을 위해@Modifying(clearAutomatically = true, flushAutomatically = true)적용을 검토해 주세요. - 운영 가시성을 위해 삭제 건수(Int/Long) 반환을 고려하면 장애 분석/메트릭 수집에 유용합니다.
- 태그 동기화는 “전체 삭제 후 재등록”이 단순하지만, 변경량이 적은 경우 set diff(추가/삭제 분리) 방식이 비용 절감 및 createdAt 보존 측면에서 유리할 수 있습니다. 추후 리팩터링 후보로 권장합니다.
🏁 Script executed:
#!/bin/bash
# Locate the repository interface file
fd ReadingRecordTagRepository.kt -x sed -n '1,200p'Length of output: 975
삭제 쿼리 동기화 및 삭제 건수 반환 적용 검토 요청
아래 개선안을 검토해 주세요.
• infra 레이어:
– 파일: infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt
– deleteAllByReadingRecordId 메서드에 @Modifying(clearAutomatically = true, flushAutomatically = true) 애노테이션 추가
– 반환 타입을 Int (또는 Long) 으로 변경해 삭제 건수 리턴
• 도메인 레이어:
– 파일: domain/src/main/kotlin/org/yapp/domain/readingrecordtag/ReadingRecordTagRepository.kt
– deleteAllByReadingRecordId 반환 타입을 Int (또는 Long) 으로 변경 검토
• 향후 리팩터링 제안:
– 전체 삭제 후 재등록 대신 변경량(추가/삭제) diff 처리로 비용 절감 및 createdAt 보존
예시 diff:
--- infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt
+++ infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt
@@
-import org.springframework.data.jpa.repository.JpaRepository
+import org.springframework.data.jpa.repository.JpaRepository
+import org.springframework.data.jpa.repository.Modifying
+
interface JpaReadingRecordTagRepository : JpaRepository<ReadingRecordTagEntity, UUID>, JpaReadingRecordTagQuerydslRepository {
- fun deleteAllByReadingRecordId(readingRecordId: UUID)
+ @Modifying(clearAutomatically = true, flushAutomatically = true)
+ fun deleteAllByReadingRecordId(readingRecordId: UUID): Int
}
--- domain/src/main/kotlin/org/yapp/domain/readingrecordtag/ReadingRecordTagRepository.kt
+++ domain/src/main/kotlin/org/yapp/domain/readingrecordtag/ReadingRecordTagRepository.kt
@@
- fun deleteAllByReadingRecordId(readingRecordId: UUID)
+ fun deleteAllByReadingRecordId(readingRecordId: UUID): Int| ): Page<ReadingRecordEntity> { | ||
|
|
||
| val whereCondition = readingRecord.userBookId.eq(userBookId) | ||
| .and(readingRecord.deletedAt.isNull()) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
대량 트래픽 대비 인덱스 설계 권장
where readingRecord.userBookId = ? and deletedAt is null 패턴이 빈번하다면 다음 인덱스를 고려해 주세요:
- (userBookId, deletedAt) 복합 인덱스
- 정렬 기본값이 createdAt DESC이므로, 커버리지 고려 시 (userBookId, deletedAt, createdAt)까지 검토
DB 벤더 특성에 따라 Partial Index/Filtered Index 지원 여부도 함께 검토하면 좋습니다.
🤖 Prompt for AI Agents
infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/JpaReadingRecordQuerydslRepositoryImpl.kt
around line 29: the query frequently filters by "userBookId = ? AND deletedAt IS
NULL", so add a DB index to support this pattern—create a composite index on
(user_book_id, deleted_at) and, if you want a covering index for the default
ORDER BY createdAt DESC, consider (user_book_id, deleted_at, created_at);
additionally evaluate using a partial/filtered index on deleted_at IS NULL if
the DB vendor supports it to reduce index size and improve performance.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt(2 hunks)
⏰ 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)
- GitHub Check: build-validation
🔇 Additional comments (4)
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt (3)
6-6: 필요한 DTO import 추가 적절Update 요청 DTO의 import가 명확합니다.
50-68: 서비스 계층의 위임과 응답 매핑 일관성 양호도메인 서비스 위임과 VO→Response 매핑이 기존 흐름과 일관적이며, 신규 API 요구사항에 부합합니다.
50-62: @Valid이 이미 적용되어 있습니다.ReadingRecordController.kt(91번 라인)와 ReadingRecordControllerApi.kt(164번 라인) 모두
@Valid @RequestBody request: UpdateReadingRecordRequest로 선언되어 있어, Bean Validation이 정상적으로 적용됩니다. 별도 추가 작업은 필요하지 않습니다.apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt (1)
26-33: 필드 검증 제약 지정 적절pageNumber/quote/review에 대한 길이/범위 제약과 @Schema 메타가 적절합니다. 업데이트 요청의 선택적 필드 특성(Int?/String?)과도 일관됩니다.
| { | ||
| "pageNumber": 50, | ||
| "quote": "수정된 기억에 남는 문장입니다.", | ||
| "review": "수정된 감상평입니다.", | ||
| "emotionTags": ["놀라움"] | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
OpenAPI 예시를 최대 3개 제약에 맞게 보완하세요
스키마 설명은 “최대 3개”이나 예시는 1개만 포함되어 있어 오해 소지가 있습니다. 2~3개의 예시를 넣어 제약을 명확히 전달하는 것을 권장합니다.
예시를 다음과 같이 조정:
- "emotionTags": ["놀라움"]
+ "emotionTags": ["놀라움", "영감"]📝 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.
| { | |
| "pageNumber": 50, | |
| "quote": "수정된 기억에 남는 문장입니다.", | |
| "review": "수정된 감상평입니다.", | |
| "emotionTags": ["놀라움"] | |
| } | |
| { | |
| "pageNumber": 50, | |
| "quote": "수정된 기억에 남는 문장입니다.", | |
| "review": "수정된 감상평입니다.", | |
| "emotionTags": ["놀라움", "영감"] | |
| } |
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt
Show resolved
Hide resolved
| @field:Size(max = 3, message = "감정 태그는 최대 3개까지 가능합니다.") | ||
| @field:Schema(description = "수정할 감정 태그 목록", example = """["놀라움"]""") | ||
| val emotionTags: List<@Size(max = 10, message = "감정 태그는 10자를 초과할 수 없습니다.") String>? | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
감정 태그 원소의 공백 문자열 방지 권장
리스트 크기/원소 길이만 제한되어 있어 공백(""), 탭 등 실질적인 빈 태그가 허용될 수 있습니다. 원소에 @notblank를 추가해 데이터 품질을 보강하는 것을 권장합니다. (null은 리스트 자체를 null로 전달하면 되므로, 원소는 공백 금지가 자연스럽습니다)
다음 변경을 제안합니다:
- val emotionTags: List<@Size(max = 10, message = "감정 태그는 10자를 초과할 수 없습니다.") String>?
+ val emotionTags: List<@NotBlank(message = "감정 태그는 비어 있을 수 없습니다.") @Size(max = 10, message = "감정 태그는 10자를 초과할 수 없습니다.") String>?추가로 필요한 import:
import jakarta.validation.constraints.NotBlank🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt
around lines 34 to 37, the element constraints allow blank strings in
emotionTags; add jakarta.validation.constraints.NotBlank to the List element
type so each tag cannot be empty/whitespace, and add the corresponding import
(import jakarta.validation.constraints.NotBlank) at the top of the file; keep
the existing @Size annotations so the element has both @NotBlank and @Size(max =
10) and the List retains @field:Size(max = 3).
| fun updateReadingRecord( | ||
| readingRecordId: UUID, | ||
| request: UpdateReadingRecordRequest | ||
| ): ReadingRecordResponse { | ||
| val readingRecordInfoVO = readingRecordDomainService.modifyReadingRecord( | ||
| readingRecordId = readingRecordId, | ||
| pageNumber = request.pageNumber, | ||
| quote = request.quote, | ||
| review = request.review, | ||
| emotionTags = request.emotionTags | ||
| ) | ||
| return ReadingRecordResponse.from(readingRecordInfoVO) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
부분 업데이트 시 null/empty 의미를 명확히 하고 도메인 구현과의 계약을 확인해주세요
- pageNumber/quote/review/emotionTags가 null일 때는 “미변경”, 빈 문자열("")/빈 리스트([])일 때는 “해당 값 초기화”로 해석되는지 도메인 서비스(modifyReadingRecord → 엔티티 update)가 일관되게 처리하는지 확인이 필요합니다.
- 특히 emotionTags는 null(유지) vs [](모두 삭제)의 의미가 다르므로, 현재 구현이 이 구분을 준수하는지 검증해 주세요. API 문서(Schema 설명)에도 이 동작을 명시하는 것을 권장합니다.
다음 스크립트로 도메인 계층의 처리 로직을 빠르게 확인할 수 있습니다:
🏁 Script executed:
#!/bin/bash
# modifyReadingRecord 선언/구현 조회
rg -n -A 15 -B 5 'modifyReadingRecord\(' --type kotlin
# ReadingRecord 엔티티의 update 메서드 후보 탐색
rg -n -A 20 -B 5 'class ReadingRecord|data class ReadingRecord' --type kotlin
rg -n -A 15 -B 3 'fun update\(' --type kotlin | sed -n '1,200p'Length of output: 58867
🏁 Script executed:
#!/bin/bash
# Locate Quote class implementation
rg -nR "class Quote" -A 20 -B 0 --type kotlin domain/src/main/kotlin/org/yapp/domain/readingrecordLength of output: 505
🏁 Script executed:
#!/bin/bash
# Show ReadingRecord.kt up to update method to inspect value object implementations
sed -n '1,100p' domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.ktLength of output: 3352
🏁 Script executed:
#!/bin/bash
# List domain/src/main/kotlin/org/yapp/domain/readingrecord contents
echo "Files in domain/src/main/kotlin/org/yapp/domain/readingrecord:"
ls domain/src/main/kotlin/org/yapp/domain/readingrecord
# Display Quote, Review, EmotionTag implementations if present
for f in domain/src/main/kotlin/org/yapp/domain/readingrecord/{Quote,Review,EmotionTag}.kt; do
if [ -f "$f" ]; then
echo ">>>> $f"
sed -n '1,200p' "$f"
fi
doneLength of output: 814
🏁 Script executed:
#!/bin/bash
# Display Quote, Review, EmotionTag value classes in ReadingRecord.kt
sed -n '100,200p' domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt
sed -n '200,300p' domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.ktLength of output: 1426
#!/bin/bash
# Locate UpdateReadingRecordRequest
rg -n 'class UpdateReadingRecordRequest' -A15 -B5 --type kotlin apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request🏁 Script executed:
#!/bin/bash
# Find and display UpdateReadingRecordRequest
rg -n 'data class UpdateReadingRecordRequest' -A10 -B2 --type kotlin apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/requestLength of output: 1861
부분 업데이트 null/empty 처리 명세 확인 및 검증 보강 필요
현재 도메인과 API의 동작은 다음과 같습니다.
-
ReadingRecord.update (domain)
- pageNumber: null → 기존 값 유지
- quote/review: null → 기존 값 유지
• 빈 문자열("") 전달 시 Quote.newInstance/Review.newInstance 내부require(value.isNotBlank())에서 예외 발생 - emotionTags: null → 기존 값 유지
빈 리스트([]) → 빈 리스트로 덮어쓰기(모두 삭제)
-
UpdateReadingRecordRequest (API)
@field:Size(max = 1000)만 적용되어 빈 문자열("") 허용- emotionTags:
List<String>?(null 허용, 리스트 내부 blank 검사 없음)
위에서 보듯 API는 quote/review에 빈 문자열을 허용하지만, 도메인은 빈 문자열을 모두 예외 처리합니다. 또한 emotionTags만이 null vs 빈 리스트를 구분해 “유지/초기화”를 명확히 지원하고 있습니다.
- API 스키마/문서(UpdateReadingRecordRequest)에 null vs 빈값 처리(미변경 vs 초기화) 동작을 명시
- quote/review 필드에 빈 문자열 허용 여부를 명확히 하고, 필요 시
@NotBlank검증 추가 또는 도메인 newInstance 로직 변경 - emotionTags 처리 방식(초기화 시 빈 리스트 전달)을 문서에 명시
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt
around lines 50-62, the API currently allows empty strings for quote/review and
lacks element-level validation for emotionTags, which conflicts with domain
behavior; update the UpdateReadingRecordRequest DTO to either (A) enforce
non-empty values by adding @field:NotBlank on quote and review and add
element-level validation on emotionTags (e.g., @field:Size / @field:NotBlank on
list items), or (B) if you want to support "empty -> clear" semantics, normalize
empty strings to null in the service before calling the domain (map "" -> null)
so domain.keep/clear behavior is consistent; also add validation to emotionTags
elements (disallow blank tags) and update the API schema/docs to explicitly
state null vs empty list semantics for emotionTags and the treatment of empty
quote/review.
move-hoon
left a comment
move-hoon
left a comment
There was a problem hiding this comment.
고생하셨습니다~~
몇 가지 코멘트 남겼습니다!
혹시 가능하시다면 수정에서는 updatedAt 시간이 변경되는지, 삭제에서는 hard delete 되지 않고 deleteAt 필드에 값이 들어오는지 체크해주시면 감사할 것 같습니다 ㅎㅎ
| override fun deleteReadingRecord( | ||
| @AuthenticationPrincipal userId: UUID, | ||
| @PathVariable readingRecordId: UUID | ||
| ): ResponseEntity<Void> { |
There was a problem hiding this comment.
Kotlin에서는 Unit이 Java의 void와 대응되기 때문에 수정하면 좋을 것 같아요!
| fun deleteReadingRecord( | ||
| @AuthenticationPrincipal @Parameter(description = "인증된 사용자 ID") userId: UUID, | ||
| @PathVariable @Parameter(description = "삭제할 독서 기록 ID") readingRecordId: UUID | ||
| ): ResponseEntity<Void> |
| "pageNumber": 50, | ||
| "quote": "수정된 기억에 남는 문장입니다.", | ||
| "review": "수정된 감상평입니다.", | ||
| "emotionTags": ["놀라움"] |
There was a problem hiding this comment.
저희 예시에 놀라움 대신, 진짜 있는 태그로 하면 좋을 것 같아요!
(따뜻함, 즐거움, 슬픔, 깨달음)
There was a problem hiding this comment.
해당 request에서는 Validation이 없는데 의도하신건 지 궁금합니다!!
만약 없다면 @Valid 어노테이션 + valid function 패턴을 하는게 좋을 것 같아서요!
| fun update( | ||
| pageNumber: Int?, | ||
| quote: String?, | ||
| review: String?, | ||
| emotionTags: List<String>? | ||
| ): ReadingRecord { | ||
| return ReadingRecord( | ||
| id = this.id, | ||
| userBookId = this.userBookId, | ||
| pageNumber = pageNumber?.let { PageNumber.newInstance(it) } ?: this.pageNumber, | ||
| quote = quote?.let { Quote.newInstance(it) } ?: this.quote, | ||
| review = review?.let { Review.newInstance(it) } ?: this.review, | ||
| emotionTags = emotionTags?.map { EmotionTag.newInstance(it) } ?: this.emotionTags, | ||
| createdAt = this.createdAt, | ||
| updatedAt = LocalDateTime.now(), | ||
| deletedAt = this.deletedAt | ||
| ) | ||
| } |
There was a problem hiding this comment.
수동으로 update 메서드를 만들 필요 없이 copy를 써도 좋을 것 같아요!!
ex)
fun update(
pageNumber: Int? = null,
quote: String? = null,
review: String? = null,
emotionTags: List<String>? = null
): ReadingRecord {
return this.copy(
pageNumber = pageNumber?.let { PageNumber.newInstance(it) } ?: this.pageNumber,
quote = quote?.let { Quote.newInstance(it) } ?: this.quote,
review = review?.let { Review.newInstance(it) } ?: this.review,
emotionTags = emotionTags?.map { EmotionTag.newInstance(it) } ?: this.emotionTags,
updatedAt = LocalDateTime.now()
)
}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt (1)
22-33: SecurityConfig.kt — 공개 엔드포인트를 GET으로 제한하세요 (수정 필요)검증 결과 WHITELIST_URLS가 authorizeHttpRequests에서 그대로 permitAll로 사용되고 있어("/gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt" — WHITELIST_URLS 정의 라인 22, it.requestMatchers(*WHITELIST_URLS).permitAll() 라인 52) 특정 경로의 모든 HTTP 메서드가 공개됩니다. 검색은 GET만 공개하는 게 안전하므로 아래처럼 수정해 주세요.
수정할 위치:
- gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt
- WHITELIST_URLS 정의: 대략 라인 22–33
- authorizeHttpRequests 블록: 대략 라인 51–54
권장 변경(핵심 diff):
- "/api/v1/books/search",authorizeHttpRequests 블록에 추가(참고):
.authorizeHttpRequests { // 검색은 GET만 공개 it.requestMatchers(HttpMethod.GET, "/api/v1/books/search").permitAll() // 나머지 화이트리스트 it.requestMatchers(*WHITELIST_URLS).permitAll() it.requestMatchers("/api/v1/admin/**").hasRole("ADMIN") it.anyRequest().authenticated() }CORS(OPTIONS) 주의: 브라우저에서 호출한다면 프리플라이트(OPTIONS)가 필요합니다. 전역 CORS 설정이 없다면 아래처럼 OPTIONS를 허용하거나 전역 CORS(CorsConfigurationSource 등)를 사용하세요.
it.requestMatchers(HttpMethod.OPTIONS, "/api/v1/books/search").permitAll()apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
105-117: userId null 가드 적절함. 추가로 isbn13s 비어있을 때도 단락 평가 고려(미래 대비용)현재 호출 경로에서는 빈 목록 호출이 발생하지 않지만, 함수가 재사용될 가능성을 고려하면 다음 가드를 추가하는 것도 미세한 최적화에 도움이 됩니다.
참고용 코드(범위 외):
if (isbn13s.isEmpty()) return emptyMap()
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(0 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(3 hunks)gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt(2 hunks)
💤 Files with no reviewable changes (1)
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
⏰ 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)
- GitHub Check: build-validation
🔇 Additional comments (2)
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
87-93: userId null/빈 목록 시 단락 평가: 적절합니다불필요한 조회를 방지하고 가독성도 좋습니다. 이대로 유지해도 충분합니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (1)
34-40: 공개 검색 엔드포인트: 인증된 사용자에 대한 개인화(내 서재 상태) 노출 여부 확인 필요현재 구현은 항상 비개인화 응답을 반환합니다. 인증된 사용자에게 "내 서재" 상태 등 개인화를 유지하려면 userId를 선택 파라미터로 받아 유스케이스에 전달하도록 변경하세요(게이트웨이 permitAll 유지).
수정이 필요한 위치(예시)
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (약 34–40행): searchBooks 서명 변경 및 useCase 호출 변경
- 관련: bookUseCase 인터페이스/구현부에서 searchBooks(request, userId) 시그니처 추가, 호출부/테스트/문서 검토
예시 코드:
@GetMapping("/search") override fun searchBooks( @AuthenticationPrincipal userId: UUID?, @Valid @ModelAttribute request: BookSearchRequest ): ResponseEntity<BookSearchResponse> { val response = bookUseCase.searchBooks(request, userId) return ResponseEntity.ok(response) }제품 요구사항이 "항상 비개인화"라면 현재 구현으로 충분합니다 — 의도 확인 부탁드립니다.
| fun searchBooks( | ||
| request: BookSearchRequest, | ||
| userId: UUID | ||
| request: BookSearchRequest | ||
| ): BookSearchResponse { | ||
| userService.validateUserExists(userId) | ||
|
|
||
| val searchResponse = bookQueryService.searchBooks(request) | ||
| val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, userId) | ||
| val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, null) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
검색 결과 개인화 유지 여부 확인 및 선택적 사용자 컨텍스트 허용 제안
검색을 공개로 전환한 의도는 타당합니다. 다만 사용자가 인증된 경우에 한해 개인화(내 서재 상태 표기)를 유지하고 싶다면, 파라미터를 선택적(기본 null)로 열어두는 방식이 가장 간단합니다. 시그니처 호환성도 그대로 유지됩니다.
해당 범위 내 적용 diff:
fun searchBooks(
- request: BookSearchRequest
+ request: BookSearchRequest,
+ userId: UUID? = null
): BookSearchResponse {
val searchResponse = bookQueryService.searchBooks(request)
- val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, null)
+ val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, userId)컨트롤러는 인증이 있으면 userId를 전달하고(없으면 null), 게이트웨이에서는 permitAll 유지로 비인증 접근도 허용하는 구조가 됩니다. 제품 요구사항이 “항상 비개인화 응답”이라면 현 상태로도 무방합니다. 요구사항을 한 번만 확인해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "------ Show BookUseCase.kt (first 300 lines) ------"
sed -n '1,300p' apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt || true
echo
echo "------ Search for mergeWithUserBookStatus occurrences ------"
rg "mergeWithUserBookStatus" -n || true
echo
echo "------ Search for function definition of mergeWithUserBookStatus ------"
rg "fun mergeWithUserBookStatus" -n || true
echo
echo "------ Search for any searchBooks function definitions/usages ------"
rg "fun searchBooks\(" -n || true
echo
echo "------ Search for BookUseCase references ------"
rg "BookUseCase" -n || true
echo
echo "------ Search for controllers referencing searchBooks or BookUseCase ------"
rg "searchBooks\(|BookUseCase" -n || trueLength of output: 8225
searchBooks에 선택적 userId 파라미터 추가 제안 — 인증 시 개인화 유지, 비인증 시 비개인화 유지
검증 결과: 현재 searchBooks는 mergeWithUserBookStatus(..., null)를 호출해 항상 비개인화 응답을 반환합니다. 인증된 사용자에 한해 개인화(내 서재 상태 표기)를 유지하려면 userId를 선택적 파라미터로 열어두고 컨트롤러에서 인증 시 전달하도록 변경하는 것이 안전합니다.
수정이 필요한 위치:
- apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt — searchBooks 시그니처 및 호출부 변경
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt — 인증 정보가 있으면 userId를 추출해 bookUseCase.searchBooks에 전달(없으면 null)
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt — 필요한 경우 시그니처/핸들러 확인 및 업데이트
적용 예시(diff):
fun searchBooks(
- request: BookSearchRequest
+ request: BookSearchRequest,
+ userId: UUID? = null
): BookSearchResponse {
val searchResponse = bookQueryService.searchBooks(request)
- val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, null)
+ val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, userId)운영 요구가 “항상 비개인화”라면 현재 상태 유지 가능하지만, 인증 시 개인화 허용이 필요하면 위 변경을 적용해 주세요.
📝 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 searchBooks( | |
| request: BookSearchRequest, | |
| userId: UUID | |
| request: BookSearchRequest | |
| ): BookSearchResponse { | |
| userService.validateUserExists(userId) | |
| val searchResponse = bookQueryService.searchBooks(request) | |
| val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, userId) | |
| val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, null) | |
| fun searchBooks( | |
| request: BookSearchRequest, | |
| userId: UUID? = null | |
| ): BookSearchResponse { | |
| val searchResponse = bookQueryService.searchBooks(request) | |
| val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, userId) |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt around lines
31–36, searchBooks currently always calls mergeWithUserBookStatus(..., null)
causing non-personalized results; change the searchBooks signature to accept an
optional userId (e.g. userId: Long? or String?), forward that userId into
mergeWithUserBookStatus instead of null, and update all internal calls
accordingly. Then, in
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt extract the
authenticated user id when present (from the security principal or request
context) and pass it to bookUseCase.searchBooks(userId = ...) or null if
unauthenticated. Finally, verify and update
apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt handler
signatures/types to accept/propagate the optional userId so controller and use
case signatures remain consistent.
| import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity | ||
| import org.springframework.security.config.http.SessionCreationPolicy | ||
| import org.springframework.core.convert.converter.Converter | ||
| import org.springframework.http.HttpMethod |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
HttpMethod import는 메서드 한정 매처 적용 시 활용됩니다
위 제안을 적용하면 본 import를 활용하게 됩니다. 그대로 진행하셔도 됩니다.
🤖 Prompt for AI Agents
In gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt around
line 9, the imported HttpMethod is intended to be used for method-specific
request matchers in the security configuration; update the security matcher
usages to employ HttpMethod (e.g., use method-specific matchers such as
requestMatchers(HttpMethod.GET, "/your-path") instead of only path-based
matchers) so the import is actually used and access rules apply per HTTP method.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt (3)
20-20: private constructor로 인해 JSON 바인딩이 실패합니다Spring/Jackson 역직렬화를 위해서는 공개 생성자가 필요합니다. private 생성자를 제거해 주세요.
-data class UpdateReadingRecordRequest private constructor( +data class UpdateReadingRecordRequest(
34-36: OpenAPI 예시가 제약(max=3)과 불일치합니다예시 배열에 4개의 태그가 포함되어 있습니다. 제약과 일치하도록 최대 3개로 수정해 주세요.
- @field:Schema(description = "수정할 감정 태그 목록", example = """["따뜻함","즐거움","슬픔","깨달음"]""") + @field:Schema(description = "수정할 감정 태그 목록", example = """["따뜻함","즐거움","슬픔"]""")
34-37: 감정 태그 원소 공백 방지: @notblank 추가 제안도메인에서 빈 문자열을 허용하지 않으므로 DTO 수준에서도 공백/빈 문자열을 차단하는 것이 일관적입니다.
@@ -import jakarta.validation.constraints.Size +import jakarta.validation.constraints.Size +import jakarta.validation.constraints.NotBlank @@ - val emotionTags: List<@Size(max = 10, message = "감정 태그는 10자를 초과할 수 없습니다.") String>? + val emotionTags: List< + @NotBlank(message = "감정 태그는 비어 있을 수 없습니다.") + @Size(max = 10, message = "감정 태그는 10자를 초과할 수 없습니다.") + String + >?apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt (1)
101-108: 삭제 응답 타입을 ResponseEntity로 변경 권장REST 관례와 상호운용성을 위해
ResponseEntity<Void>사용을 권장합니다. 이전 리뷰 의견과도 일치합니다.- ): ResponseEntity<Unit> { + ): ResponseEntity<Void> { readingRecordUseCase.deleteReadingRecord(userId, readingRecordId) return ResponseEntity.noContent().build() }
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-17T15:09:36.267Z
Learnt from: hoonyworld
PR: YAPP-Github/Reed-Server#39
File: apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt:45-45
Timestamp: 2025-07-17T15:09:36.267Z
Learning: Response DTO 클래스에서는 불변성을 지키기 위해 프로퍼티를 val로 선언하고, 업데이트가 필요한 경우 copy() 메서드를 사용해야 합니다.
Applied to files:
domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt
⏰ 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)
- GitHub Check: build-validation
🔇 Additional comments (1)
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt (1)
87-99: LGTM유스케이스 위임과 200 OK 반환이 명확합니다. 요청 DTO에 @Valid 적용되어 있어 컨트롤러에서의 1차 유효성 검증도 적절합니다.
| @PatchMapping("/{readingRecordId}") | ||
| fun updateReadingRecord( | ||
| @AuthenticationPrincipal @Parameter(description = "인증된 사용자 ID") userId: UUID, | ||
| @PathVariable @Parameter(description = "수정할 독서 기록 ID") readingRecordId: UUID, | ||
| @Valid @RequestBody @Parameter(description = "독서 기록 수정 요청 객체") request: UpdateReadingRecordRequest | ||
| ): ResponseEntity<ReadingRecordResponse> | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
API 문서에 401 Unauthorized 응답을 추가하세요
인증이 필요한 엔드포인트이므로 401 응답을 명시하는 것이 좋습니다.
@ApiResponses(
value = [
ApiResponse(
responseCode = "200",
description = "독서 기록 수정 성공",
content = [Content(schema = Schema(implementation = ReadingRecordResponse::class))]
),
+ ApiResponse(
+ responseCode = "401",
+ description = "인증 필요",
+ content = [Content(schema = Schema(implementation = ErrorResponse::class))]
+ ),
ApiResponse(
responseCode = "400",
description = "잘못된 요청",
content = [Content(schema = Schema(implementation = ErrorResponse::class))]
),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 160 to 166, the PATCH endpoint lacks an explicit 401 Unauthorized
response in the API documentation; add an OpenAPI/Swagger annotation (e.g.,
@ApiResponses with an @ApiResponse(responseCode = "401", description =
"Unauthorized") or include responseCode "401" in an existing
@Operation/@ApiResponses block) directly above the updateReadingRecord method so
the generated API docs clearly show authentication failures; ensure the
annotation imports are added if missing and that the 401 description matches
project i18n/style.
| @DeleteMapping("/{readingRecordId}") | ||
| fun deleteReadingRecord( | ||
| @AuthenticationPrincipal @Parameter(description = "인증된 사용자 ID") userId: UUID, | ||
| @PathVariable @Parameter(description = "삭제할 독서 기록 ID") readingRecordId: UUID | ||
| ): ResponseEntity<Unit> | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
삭제 응답 타입은 ResponseEntity가 관례적입니다
삭제는 본문이 없으므로 ResponseEntity<Void>가 더 명확하고, Java/Spring과의 상호운용성도 좋습니다. 인터페이스/구현 모두 동일하게 맞춰 주세요.
- ): ResponseEntity<Unit>
+ ): ResponseEntity<Void>구현 클래스 변경 예시(다른 파일):
// apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt
override fun deleteReadingRecord(
@AuthenticationPrincipal userId: UUID,
@PathVariable readingRecordId: UUID
): ResponseEntity<Void> {
readingRecordUseCase.deleteReadingRecord(userId, readingRecordId)
return ResponseEntity.noContent().build()
}🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt
around lines 184 to 189, change the delete endpoint return type from
ResponseEntity<Unit> to ResponseEntity<Void> to follow convention for no-body
DELETE responses; update the interface signature accordingly and then update the
implementing class (e.g., ReadingRecordController.kt) to return
ResponseEntity.noContent().build() after calling
readingRecordUseCase.deleteReadingRecord(userId, readingRecordId), and adjust
any imports/types to compile with ResponseEntity<Void>.
| fun update( | ||
| pageNumber: Int?, | ||
| quote: String?, | ||
| review: String?, | ||
| emotionTags: List<String>? | ||
| ): ReadingRecord { | ||
| return this.copy( | ||
| pageNumber = pageNumber?.let { PageNumber.newInstance(it) } ?: this.pageNumber, | ||
| quote = quote?.let { Quote.newInstance(it) } ?: this.quote, | ||
| review = review?.let { Review.newInstance(it) } ?: this.review, | ||
| emotionTags = emotionTags?.map { EmotionTag.newInstance(it) } ?: this.emotionTags, | ||
| updatedAt = LocalDateTime.now() | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
업데이트 입력값 정규화(트림/중복 제거) 및 no-op 최적화 제안
- 모든 인자가 null이면 변경사항이 없음에도 updatedAt만 변경됩니다. 무의미한 업데이트를 피하기 위해 early return을 권장합니다.
- quote/review는 trim 없이 그대로 검증하므로 앞뒤 공백이 의도치 않게 저장될 수 있습니다. trim 후 검증/적용을 권장합니다.
- emotionTags는 중복/공백 태그가 그대로 반영될 수 있습니다. trim → 공백 제거 → 중복 제거 → 래핑 순으로 정규화하는 편이 안전합니다.
아래처럼 개선을 제안합니다.
fun update(
pageNumber: Int?,
quote: String?,
review: String?,
emotionTags: List<String>?
): ReadingRecord {
- return this.copy(
- pageNumber = pageNumber?.let { PageNumber.newInstance(it) } ?: this.pageNumber,
- quote = quote?.let { Quote.newInstance(it) } ?: this.quote,
- review = review?.let { Review.newInstance(it) } ?: this.review,
- emotionTags = emotionTags?.map { EmotionTag.newInstance(it) } ?: this.emotionTags,
- updatedAt = LocalDateTime.now()
- )
+ if (pageNumber == null && quote == null && review == null && emotionTags == null) {
+ return this
+ }
+ return this.copy(
+ pageNumber = pageNumber?.let { PageNumber.newInstance(it) } ?: this.pageNumber,
+ quote = quote?.trim()?.let { Quote.newInstance(it) } ?: this.quote,
+ review = review?.trim()?.let { Review.newInstance(it) } ?: this.review,
+ emotionTags = emotionTags
+ ?.map { it.trim() }
+ ?.filter { it.isNotBlank() }
+ ?.distinct()
+ ?.map { EmotionTag.newInstance(it) }
+ ?: this.emotionTags,
+ updatedAt = LocalDateTime.now()
+ )
}📝 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 update( | |
| pageNumber: Int?, | |
| quote: String?, | |
| review: String?, | |
| emotionTags: List<String>? | |
| ): ReadingRecord { | |
| return this.copy( | |
| pageNumber = pageNumber?.let { PageNumber.newInstance(it) } ?: this.pageNumber, | |
| quote = quote?.let { Quote.newInstance(it) } ?: this.quote, | |
| review = review?.let { Review.newInstance(it) } ?: this.review, | |
| emotionTags = emotionTags?.map { EmotionTag.newInstance(it) } ?: this.emotionTags, | |
| updatedAt = LocalDateTime.now() | |
| ) | |
| } | |
| fun update( | |
| pageNumber: Int?, | |
| quote: String?, | |
| review: String?, | |
| emotionTags: List<String>? | |
| ): ReadingRecord { | |
| if (pageNumber == null && quote == null && review == null && emotionTags == null) { | |
| return this | |
| } | |
| return this.copy( | |
| pageNumber = pageNumber?.let { PageNumber.newInstance(it) } ?: this.pageNumber, | |
| quote = quote?.trim()?.let { Quote.newInstance(it) } ?: this.quote, | |
| review = review?.trim()?.let { Review.newInstance(it) } ?: this.review, | |
| emotionTags = emotionTags | |
| ?.map { it.trim() } | |
| ?.filter { it.isNotBlank() } | |
| ?.distinct() | |
| ?.map { EmotionTag.newInstance(it) } | |
| ?: this.emotionTags, | |
| updatedAt = LocalDateTime.now() | |
| ) | |
| } |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt around
lines 61 to 74, the update(...) method always sets updatedAt even when no actual
field changes and does not normalize inputs; change it to detect a no-op (all
incoming values either null or equal to current values) and return this early
without modifying updatedAt, trim quote and review before validation/wrapping
(use .trim() and treat empty as null if appropriate), and normalize emotionTags
by trimming each tag, filtering out blank entries, removing duplicates, then
wrapping to EmotionTag instances; only construct a new copy and set updatedAt
when at least one real change exists.
| quote = quote?.let { Quote.newInstance(it) } ?: this.quote, | ||
| review = review?.let { Review.newInstance(it) } ?: this.review, | ||
| emotionTags = emotionTags?.map { EmotionTag.newInstance(it) } ?: this.emotionTags, | ||
| updatedAt = LocalDateTime.now() |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
시간 의존성 주입 고려 (테스트 용이성/일관성)
LocalDateTime.now() 직접 호출 대신 java.time.Clock을 주입받아 LocalDateTime.now(clock) 형태로 사용하면 테스트 용이성과 시간대 일관성이 좋아집니다.
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt around
line 72, replace the direct call to LocalDateTime.now() with a Clock-based
approach: add a Clock dependency (inject via constructor or method parameter,
defaulting to Clock.systemDefaultZone() if needed) and use
LocalDateTime.now(clock) to set updatedAt; update any constructors/factory
methods and unit tests to supply a fixed Clock in tests for deterministic
timestamps.
| fun delete(): ReadingRecord { | ||
| return this.copy(deletedAt = LocalDateTime.now()) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
soft-delete 가드 및 updatedAt 동기화 권장 + 실제 삭제 로직과의 정합성 확인
- 이미 삭제된 엔티티에 대한 중복 삭제를 방지하는 가드가 없습니다.
- 삭제 시점에 updatedAt도 함께 갱신해주면 감사 관점에서 유용합니다.
- 만약 인프라 레이어에서 실제 삭제(deleteById)를 수행한다면, 도메인에서 설정한 deletedAt이 영속 전에 사라질 수 있습니다. soft-delete와 hard-delete가 동시에 쓰이지 않는지 확인이 필요합니다.
제안 변경:
- fun delete(): ReadingRecord {
- return this.copy(deletedAt = LocalDateTime.now())
- }
+ fun delete(): ReadingRecord {
+ require(deletedAt == null) { "이미 삭제된 독서 기록입니다." }
+ val now = LocalDateTime.now()
+ return this.copy(
+ deletedAt = now,
+ updatedAt = now
+ )
+ }다음 스크립트로 인프라/서비스 레이어에서 실제 삭제가 수행되는지, soft-delete와의 혼용 여부를 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# 삭제 관련 흐름 및 deletedAt 사용 검증
echo "== delete use-case/service 진입점 탐색 =="
rg -n -A 5 -B 3 "deleteReadingRecord\(" --type kotlin
echo
echo "== ReadingRecordRepository.deleteById 사용 여부 =="
rg -n -A 2 -B 2 "deleteById\(" --type kotlin
echo
echo "== QueryDSL에서 deletedAt.isNull() 필터 확인 =="
rg -n -A 3 -B 3 "deletedAt\.isNull\(\)" --type kotlin
echo
echo "== 삭제 시 태그 삭제 흐름 확인(태그 레포) =="
rg -n -A 3 -B 3 "deleteAllByReadingRecordId" --type kotlinLength of output: 14099
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
"domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt"
"domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt"
"domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt"
"infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt"
"infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/JpaReadingRecordQuerydslRepositoryImpl.kt"
"infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/JpaReadingRecordTagRepository.kt"
"infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/impl/ReadingRecordTagRepositoryImpl.kt"
)
for f in "${files[@]}"; do
echo "== FILE: $f =="
if [ -f "$f" ]; then
nl -ba -w3 -s': ' "$f"
else
echo "MISSING: $f"
fi
echo
doneLength of output: 1150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== show ReadingRecord.kt =="
sed -n '1,240p' domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt || true
echo
echo "== show ReadingRecordDomainService.kt =="
sed -n '1,260p' domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt || true
echo
echo "== show ReadingRecordRepository.kt =="
sed -n '1,200p' domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt || true
echo
echo "== show infra ReadingRecordRepositoryImpl.kt =="
sed -n '1,200p' infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt || true
echo
echo "== show JpaReadingRecordQuerydslRepositoryImpl.kt =="
sed -n '1,240p' infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/JpaReadingRecordQuerydslRepositoryImpl.kt || true
echo
echo "== show ReadingRecordTagRepositoryImpl.kt =="
sed -n '1,200p' infra/src/main/kotlin/org/yapp/infra/readingrecordtag/repository/impl/ReadingRecordTagRepositoryImpl.kt || true
echo
echo "== search for ReadingRecordTagEntity file(s) =="
rg -n "ReadingRecordTagEntity" || true
echo
echo "== show any ReadingRecordTagEntity file if found =="
for f in $(rg -n --files-with-matches "class .*ReadingRecordTagEntity" || true); do
echo "---- $f ----"
sed -n '1,240p' "$f" || true
doneLength of output: 23587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate ReadingRecordEntity files =="
rg -n --files-with-matches "class .*ReadingRecordEntity" || true
echo
echo "== show ReadingRecordEntity (if found) =="
for f in $(rg --files-with-matches "class .*ReadingRecordEntity" || true); do
echo "---- $f ----"
sed -n '1,320p' "$f" || true
done
echo
echo "== show JpaReadingRecordRepository files =="
rg -n --files-with-matches "interface .*JpaReadingRecordRepository" || true
for f in $(rg --files-with-matches "interface .*JpaReadingRecordRepository" || true); do
echo "---- $f ----"
sed -n '1,240p' "$f" || true
done
echo
echo "== search for @SQLDelete usages =="
rg -n "@SQLDelete" || true
echo
echo "== search for deleted_at / deletedAt column in infra entities =="
rg -n "deletedAt|deleted_at" infra || trueLength of output: 8925
soft-delete 흐름 정합성 확인 — infra(@SQLDelete) 기반 soft-delete 사용 중, 도메인 delete()만 수정하면 반영되지 않습니다
검증 요약: ReadingRecordDomainService.deleteReadingRecord는 readingRecordRepository.deleteById(...)를 호출하고, 엔티티(ReadingRecordEntity / ReadingRecordTagEntity 등)에 @SQLDelete가 있어 JPA 레벨에서 DB의 deleted_at을 업데이트(soft-delete)합니다. 따라서 도메인 내 delete() 변경만으로는 실제 DB의 updated_at 동기화가 되지 않으며, 연관 태그 처리가 누락되어 있을 수 있습니다.
조치 제안(우선순위 순):
-
ReadingRecord 삭제 시 관련 태그도 soft-delete 처리하도록 호출 추가
- 파일: domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt
- 제안 diff:
- readingRecordRepository.deleteById(readingRecordId) + // 연관 태그도 soft-delete 처리 + readingRecordTagRepository.deleteAllByReadingRecordId(readingRecordId) + readingRecordRepository.deleteById(readingRecordId)
-
DB 레벨 soft-delete 시 updated_at도 갱신해 감사 일관성 유지
- 파일: infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt
- 제안 diff:
-@SQLDelete(sql = "UPDATE reading_records SET deleted_at = NOW() WHERE id = ?") +@SQLDelete(sql = "UPDATE reading_records SET deleted_at = NOW(), updated_at = NOW() WHERE id = ?")
- 동일 변경을 ReadingRecordTagEntity 및 프로젝트 내 다른 @SQLDelete 사용 엔티티들(예: user_books, users, tags, books 등)에도 적용 권장:
- infra/src/main/kotlin/org/yapp/infra/readingrecordtag/entity/ReadingRecordTagEntity.kt
- infra/src/main/kotlin/org/yapp/infra/userbook/entity/UserBookEntity.kt
- infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt
- infra/src/main/kotlin/org/yapp/infra/tag/entity/TagEntity.kt
- infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt
-
도메인 delete()의 가드 및 updatedAt 동기화(선택)
- 파일: domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt
- 제안 diff (도메인 레벨로 삭제 로직을 사용하려면 적용; 현재는 서비스에서 deleteById를 사용하므로 선택적):
- fun delete(): ReadingRecord { - return this.copy(deletedAt = LocalDateTime.now()) - } + fun delete(): ReadingRecord {
-
require(deletedAt == null) { "이미 삭제된 독서 기록입니다." } -
val now = LocalDateTime.now() -
return this.copy(deletedAt = now, updatedAt = now) - }
권고: 삭제 흐름을 하나로 통일하세요.
- 도메인 주도 방식으로 통일하려면 DomainService에서 읽어온 엔티티에 대해 readingRecord.delete() 호출 후 readingRecordRepository.save(...)로 영속화.
- 현재 구조(서비스에서 deleteById → @SQLDelete로 DB 업데이트)를 유지하려면 위의 @SQLDelete 변경(및 태그 삭제 호출)을 적용해 감사/연관 데이터 정합성 보장.
참고: QueryDSL 필터(readingRecord.deletedAt.isNull())는 이미 적용되어 있어 변경 불필요합니다.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
fun delete(): ReadingRecord {
require(deletedAt == null) { "이미 삭제된 독서 기록입니다." }
val now = LocalDateTime.now()
return this.copy(deletedAt = now, updatedAt = now)
}
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt (around
lines 133-135), the domain delete() only sets deletedAt on the in-memory object
but the app uses repository.deleteById(...) with @SQLDelete at the infra layer,
so the domain-only change won't persist or handle related tags; update
ReadingRecordDomainService.deleteReadingRecord to explicitly soft-delete
associated ReadingRecordTag entities (either by loading related tag entities and
setting their deletedAt and saving, or by calling a repository method that
executes the soft-delete for tags) so tag relations are consistent, and modify
infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt
(and other entities using @SQLDelete such as ReadingRecordTagEntity,
UserBookEntity, UserEntity, TagEntity, BookEntity) to include updated_at = now()
in the @SQLDelete SQL so DB-level soft-deletes update updated_at for auditing;
optionally, if switching to domain-driven deletion, change DomainService to load
the aggregate, call readingRecord.delete() and save via repository to persist
domain-side updatedAt too.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt (5)
8-19: OpenAPI 예시 정합성 개선: 실제 지원 태그/최대 개수 제약 반영클래스 레벨 예시에서 "놀라움" 대신 실제 지원되는 태그를 사용하고(팀 코멘트 반영), 2~3개를 제시해 제약이 드러나도록 하는 것이 좋습니다.
적용 diff:
- "emotionTags": ["놀라움"] + "emotionTags": ["따뜻함", "즐거움"]
3-7: @notblank import 추가 필요위 코멘트의 제약 추가를 위해 NotBlank import를 추가하세요.
적용 diff:
import io.swagger.v3.oas.annotations.media.Schema import jakarta.validation.constraints.Max import jakarta.validation.constraints.Min import jakarta.validation.constraints.Size +import jakarta.validation.constraints.NotBlank
20-20: Jackson 역직렬화 실패 가능: private 생성자 제거 필요Spring/Jackson이 DTO를 역직렬화할 수 있도록 생성자를 public으로 열어주세요. 현재 상태에서는 PATCH 요청 바인딩이 400으로 실패할 수 있습니다.
적용 diff:
-data class UpdateReadingRecordRequest private constructor( +data class UpdateReadingRecordRequest(
38-42: NPE 위험 및 PATCH 의미 훼손: 불필요한 valid 메서드 제거*nullable 필드에 대해
!!로 강제 언랩하면, 부분 업데이트 시 전달하지 않은 필드에서 NPE가 발생합니다. PATCH의 "미전달 = 미변경" 의미를 깨뜨립니다. 이 메서드들은 단순 위임이므로 제거하고 프로퍼티에 직접 접근하도록 전환하세요. (서비스 레이어도 함께 수정 필요)적용 diff:
-) { - fun validPageNumber(): Int = pageNumber!! - fun validQuote(): String = quote!! - fun validReview(): String = review!! - fun validEmotionTags(): List<String> = emotionTags!! -} +)
34-36: emotionTags 원소 공백 금지 및 예시 4개 → 3개로 수정
- 원소 수준에서 공백 문자열을 차단하세요.
- 최대 3개 제약인데 예시가 4개입니다. 3개로 줄이고 null/[] 의미를 설명에 명시하는 것을 권장합니다.
적용 diff:
- @field:Schema(description = "수정할 감정 태그 목록", example = """["따뜻함","즐거움","슬픔","깨달음"]""") - val emotionTags: List<@Size(max = 10, message = "감정 태그는 10자를 초과할 수 없습니다.") String>? + @field:Schema( + description = "수정할 감정 태그 목록(null=미변경, []=전체 삭제)", + example = """["따뜻함","즐거움","깨달음"]""" + ) + val emotionTags: List<@NotBlank(message = "감정 태그는 비어 있을 수 없습니다.") @Size(max = 10, message = "감정 태그는 10자를 초과할 수 없습니다.") String>?
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt(2 hunks)
⏰ 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)
- GitHub Check: build-validation
| @field:Size(max = 1000, message = "기억에 남는 문장은 1000자를 초과할 수 없습니다.") | ||
| @field:Schema(description = "수정할 기억에 남는 문장", example = "수정된 기억에 남는 문장입니다.") | ||
| val quote: String?, | ||
|
|
||
| @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.") | ||
| @field:Schema(description = "수정할 감상평", example = "수정된 감상평입니다.") | ||
| val review: String?, | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
quote/review의 공백 문자열 차단 필요 (도메인 규칙과 정합성 확보)
도메인에서 공백을 허용하지 않는다면 DTO 단계에서 @notblank로 조기에 검증하는 것이 좋습니다. 필드는 nullable이라 전달된 경우에만 제약이 적용됩니다.
적용 diff:
@@
- @field:Size(max = 1000, message = "기억에 남는 문장은 1000자를 초과할 수 없습니다.")
+ @field:Size(max = 1000, message = "기억에 남는 문장은 1000자를 초과할 수 없습니다.")
+ @field:NotBlank(message = "기억에 남는 문장은 비어 있을 수 없습니다.")
@@
- @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.")
+ @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.")
+ @field:NotBlank(message = "감상평은 비어 있을 수 없습니다.")추가로 import가 필요합니다(아래 별도 코멘트 참조).
만약 "빈 문자열 전달 시 해당 값 초기화"가 의도라면, @notblank 대신 서비스 계층에서 "" → null로 정규화(map)하는 전략으로 일관되게 처리하도록 논의가 필요합니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Searching for validation annotations usage ----"
rg -n "@field:NotBlank|@NotBlank|@field:Size|@Size|jakarta.validation|javax.validation|@field:Pattern|@Pattern" || true
echo
echo "---- Showing target file ----"
sed -n '1,240p' apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt || trueLength of output: 15245
Update DTO는 nullable이므로 @notblank 대신 "전달된 경우만 공백 차단"하는 검증 사용 권고
UpdateReadingRecordRequest의 quote/review는 nullable(선택) 필드이므로 @notblank를 붙이면 null(미전달)도 거부되어 업데이트 용도와 맞지 않습니다. 전달된 경우에만 공백(공백 문자만 있는 값)을 차단하려면 @field:Pattern(regexp = ".\S.") 처럼 패턴 검증을 사용하세요.
수정 대상
- apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt — quote, review 필드
- 참고: 생성 DTO(apIs/src/.../CreateReadingRecordRequest.kt)는 생성 시 필수라 @notblank 사용됨(용도 차이 확인 필요)
권장 적용 diff:
@@
-import jakarta.validation.constraints.Size
+import jakarta.validation.constraints.Size
+import jakarta.validation.constraints.Pattern
@@
- @field:Size(max = 1000, message = "기억에 남는 문장은 1000자를 초과할 수 없습니다.")
- @field:Schema(description = "수정할 기억에 남는 문장", example = "수정된 기억에 남는 문장입니다.")
- val quote: String?,
+ @field:Size(max = 1000, message = "기억에 남는 문장은 1000자를 초과할 수 없습니다.")
+ @field:Pattern(regexp = ".*\\S.*", message = "기억에 남는 문장은 비어 있을 수 없습니다.")
+ @field:Schema(description = "수정할 기억에 남는 문장", example = "수정된 기억에 남는 문장입니다.")
+ val quote: String?,
@@
- @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.")
- @field:Schema(description = "수정할 감상평", example = "수정된 감상평입니다.")
- val review: String?,
+ @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.")
+ @field:Pattern(regexp = ".*\\S.*", message = "감상평은 비어 있을 수 없습니다.")
+ @field:Schema(description = "수정할 감상평", example = "수정된 감상평입니다.")
+ val review: String?,추가 유의사항: 만약 빈 문자열("")을 수신 시 자동으로 null로 정규화하려는 의도라면(@pattern 대신) 서비스 계층에서 "" → null로 변환하는 전략을 택하세요. 또한 현재 DTO의 validX() 메서드들이 !!를 사용하므로 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.
| @field:Size(max = 1000, message = "기억에 남는 문장은 1000자를 초과할 수 없습니다.") | |
| @field:Schema(description = "수정할 기억에 남는 문장", example = "수정된 기억에 남는 문장입니다.") | |
| val quote: String?, | |
| @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.") | |
| @field:Schema(description = "수정할 감상평", example = "수정된 감상평입니다.") | |
| val review: String?, | |
| @field:Size(max = 1000, message = "기억에 남는 문장은 1000자를 초과할 수 없습니다.") | |
| @field:Pattern(regexp = ".*\\S.*", message = "기억에 남는 문장은 비어 있을 수 없습니다.") | |
| @field:Schema(description = "수정할 기억에 남는 문장", example = "수정된 기억에 남는 문장입니다.") | |
| val quote: String?, | |
| @field:Size(max = 1000, message = "감상평은 1000자를 초과할 수 없습니다.") | |
| @field:Pattern(regexp = ".*\\S.*", message = "감상평은 비어 있을 수 없습니다.") | |
| @field:Schema(description = "수정할 감상평", example = "수정된 감상평입니다.") | |
| val review: String?, |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/request/UpdateReadingRecordRequest.kt
around lines 26–33, quote and review are nullable update fields but currently
use @NotBlank semantics (per review) — change their validation so null is
allowed but whitespace-only strings are rejected: keep @field:Size(max = 1000)
and replace any @NotBlank with a pattern validator such as @field:Pattern(regexp
= ".*\\S.*") to enforce "if provided, must contain a non-space character"; do
not reject nulls. Also ensure callers of the DTO’s validX() methods handle
nullable values (or normalize "" → null in service layer) to avoid unsafe !!
usages.
| fun updateReadingRecord( | ||
| readingRecordId: UUID, | ||
| request: UpdateReadingRecordRequest | ||
| ): ReadingRecordResponse { | ||
| val readingRecordInfoVO = readingRecordDomainService.modifyReadingRecord( | ||
| readingRecordId = readingRecordId, | ||
| pageNumber = request.validPageNumber(), | ||
| quote = request.validQuote(), | ||
| review = request.validReview(), | ||
| emotionTags = request.validEmotionTags() | ||
| ) | ||
| return ReadingRecordResponse.from(readingRecordInfoVO) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
PATCH 부분 업데이트가 NPE로 실패할 수 있음: DTO 프로퍼티를 그대로 전달
UpdateReadingRecordRequest의 valid*()가 !!로 강제 언랩하므로, 선택적 필드를 전달하지 않으면 NPE가 발생합니다. PATCH 의미(미전달=미변경)를 살리려면 nullable 프로퍼티를 그대로 도메인 서비스로 전달하세요. DTO의 불필요한 valid* 메서드도 제거하세요.
적용 diff:
val readingRecordInfoVO = readingRecordDomainService.modifyReadingRecord(
readingRecordId = readingRecordId,
- pageNumber = request.validPageNumber(),
- quote = request.validQuote(),
- review = request.validReview(),
- emotionTags = request.validEmotionTags()
+ pageNumber = request.pageNumber,
+ quote = request.quote,
+ review = request.review,
+ emotionTags = request.emotionTags
)참고: createReadingRecord 쪽도 동일 패턴을 사용 중이면, 동일하게 프로퍼티 접근으로 교체해 주세요(변경 범위 밖이라 예시만 첨부).
예시:
readingRecordDomainService.createReadingRecord(
userBookId = userBookId,
pageNumber = request.pageNumber,
quote = request.quote,
review = request.review,
emotionTags = request.emotionTags
)🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt
around lines 50-62, the updateReadingRecord call uses request.valid*() methods
that force-unwrap optional fields and cause NPE for PATCH semantics; change the
call to pass the DTO nullable properties directly (pageNumber, quote, review,
emotionTags) so omitted fields remain null and are treated as "no change" by the
domain service, and remove the unnecessary valid*() methods from the DTO; also
scan createReadingRecord for the same pattern and apply the same nullable
property passing if present.
| fun deleteReadingRecord( | ||
| readingRecordId: UUID | ||
| ) { | ||
| readingRecordDomainService.deleteReadingRecord(readingRecordId) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
삭제 정책 점검: 멱등성, soft-delete 일관성, 권한 검증 경계
- 멱등성: 이미 삭제된 리소스에 대한 재삭제 요청 시 응답(404 vs 204/200) 정책을 명확히 하고, 카운트 감소가 중복 반영되지 않도록 해주세요.
- soft-delete: PR 설명대로 deletedAt 기반 soft-delete를 사용하는지, 하드 삭제와의 혼용 여부를 확인해 주세요.
- 권한/소유 검증: 본 메서드는 userId 인자를 받지 않습니다(사용자 경계 검증을 상위 usecase/controller에서 처리한다면 OK). 체계적으로 어디에서 소유권 확인이 수행되는지 문서화/확인 바랍니다.
아래 스크립트로 관련 구현을 빠르게 확인할 수 있습니다(레포 루트에서 실행).
🏁 Script executed:
#!/bin/bash
# deleteReadingRecord 구현 및 삭제/soft-delete 흔적 탐색
rg -n -A 12 -B 6 'deleteReadingRecord\(' --type kotlin
rg -n -A 4 -B 4 'deletedAt' --type kotlin
rg -n -A 6 -B 2 'decreaseReadingRecordCount\(|readingRecordCount' --type kotlin
# 하드 삭제 호출 여부
rg -n -A 2 -B 2 'deleteById\(' --type kotlinLength of output: 49912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
"domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt"
"domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecord.kt"
"domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt"
"infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt"
"infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt"
"domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt"
"domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt"
"domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt"
)
for f in "${files[@]}"; do
if [ -f "$f" ]; then
printf "\n==== FILE: %s ====\n\n" "$f"
sed -n '1,240p' "$f" || true
else
printf "\n==== FILE NOT FOUND: %s ====\n\n" "$f"
fi
doneLength of output: 29026
조치 필요: 삭제(soft-delete) 흐름은 확인됐으나 — 멱등성·권한 검증 보강 필요
요약:
- soft-delete는 infra entity의 @SQLDelete/@SQLRestriction + BaseTimeEntity.deletedAt로 적용되어 있습니다.
- 현재 삭제 흐름은 findById → deleteById → userBook.decreaseReadingRecordCount() 로 동작하며, 이미 삭제된 리소트에 대한 재요청은 findById가 null을 반환해 ReadingRecordNotFoundException(→404)을 발생시킵니다. 멱등성(404 vs 204) 정책이 불명확합니다.
- 사용자 소유권 검증이 빠져 있습니다. 컨트롤러/유스케이스는 userId 존재만 확인하고 실제 reading record의 소유자(userBook.userId) 검증을 수행하지 않습니다.
주의가 필요한 위치:
- domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordDomainService.kt
- fun deleteReadingRecord(readingRecordId: UUID) { ... readingRecordRepository.deleteById(...) / userBookRepository.save(userBook.decreaseReadingRecordCount()) }
- infra/src/main/kotlin/org/yapp/infra/readingrecord/entity/ReadingRecordEntity.kt
- @SQLDelete, @SQLRestriction 및 deletedAt 필드 (soft-delete 구현)
- apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt
- fun deleteReadingRecord(userId: UUID, readingRecordId: UUID) — userService.validateUserExists(userId)만 호출, 소유권 검증 미실행
- apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt
- deleteReadingRecord(readingRecordId: UUID) — 도메인 서비스 위임
권장 조치(간단히)
- 멱등성 정책 결정·문서화
- 재요청에 204를 허용하려면 유스케이스/컨트롤러에서 NotFound를 무시하고 noContent 반환하거나, 도메인에 deleteIfExists 같은 반환값 있는 API 도입.
- 404로 유지할 경우 문서에 명시.
- 소유권(권한) 검증 추가
- 유스케이스(또는 도메인)에서 readingRecord → userBook을 조회해 userBook.userId == 요청자 userId 검증 또는 userBookRepository.existsByIdAndUserId(...) 호출 후 미일치 시 Forbidden 처리.
- 일관성/동시성 검토
- 동시 삭제 시 readingRecordCount가 중복 감소될 수 있음(경합 상황). 필요시 낙관적 잠금/DB에서의 조건부 감소 또는 트랜잭션/검증 로직 보강 검토.
간단한 참고 코드 위치(검토/수정 권장):
- domain/.../readingrecord/ReadingRecordDomainService.kt (delete flow)
- infra/.../readingrecord/entity/ReadingRecordEntity.kt (@SQLDelete/@SQLRestriction)
- apis/.../usecase/ReadingRecordUseCase.kt (권한 검사 추가 지점)
수정이 필요한 상태입니다.
📙 작업 내역
PATCH /reading-record/{readingRecordId}) 추가UpdateReadingRecordRequestDTO 생성 (페이지 번호, 인용문, 감상평, 감정 태그 수정 가능)ReadingRecord.update()메서드 구현DELETE /reading-record/{readingRecordId}) 추가UserBook.readingRecordCount감소 처리deletedAt처리 및 실제 DB 삭제 로직 구현deletedAt.isNull()추가하여 삭제된 기록 제외 조회🔗 관련 이슈
🧪 테스트 내역
404응답 확인Summary by CodeRabbit
New Features
Bug Fixes
Documentation