Skip to content

[FEAT] 독서 기록 조회 API 구현#241

Merged
JiwonLee42 merged 12 commits intodevelop-demofrom
feat/#219-record-api
Apr 12, 2026
Merged

[FEAT] 독서 기록 조회 API 구현#241
JiwonLee42 merged 12 commits intodevelop-demofrom
feat/#219-record-api

Conversation

@JiwonLee42
Copy link
Copy Markdown
Contributor

@JiwonLee42 JiwonLee42 commented Apr 5, 2026

📄 작업 내용 요약

독서 기록 조회 API를 구현했습니다.
정렬 조건이 많아서 QueryDSL 사용했습니다.
추후 캐싱 작업 하면 좋을 것 같긴 한데, 불러오는 내용이나 빈도수가 많지 않아서 지금은 스킵했습니다.

추가 구현 작업

  • 로그인 토큰 응답 수정
  • 삭제 API의 경우 204 응답으로 수정
  • 누락된 API adoc 파일에 추가

📎 Issue 번호


✅ 작업 목록

  • 기능 구현
  • 코드 리뷰 반영
  • 테스트 코드 작성
  • 문서 업데이트

📝 기타 참고사항

Summary by CodeRabbit

  • New Features

    • 독서 기록 조회 추가: 커서 기반 페이징, 정렬 옵션(최신/오래/기록수), 책별 기록 조회 및 이미지 presigned URL 포함
    • 감정별 기록 합계 조회 추가
  • Behavior Changes

    • 일부 삭제/빈결과 응답 코드가 200→204(빈 응답)로 변경 및 빈 페이지 처리 업데이트
  • Documentation

    • 관련 Asciidoc 문서 및 예시 스니펫 추가·갱신
  • Tests

    • REST Docs 및 컨트롤러/서비스 테스트 추가·확장

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 5, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

사용자 전체 기록 목록 조회, 특정 책 기록 조회(감정/정렬 필터 포함), 감정별 집계 조회용 커서 기반 읽기 API 3개와 이를 지원하는 DTO·쿼리·구현·서비스·컨버터·유틸·문서·테스트가 추가되었고, 글로벌 페이징 제네릭(CursoResponse<T, C>)과 라이브러리 연계 타입들이 Long 커서 타입으로 조정되었습니다.

Changes

Cohort / File(s) Summary
문서
src/docs/asciidoc/index.adoc, src/docs/asciidoc/record-query.adoc, src/docs/asciidoc/library.adoc
문서 인덱스에 record-query.adoc 포함 및 기록 조회 관련 신규 문서/스니펫 추가, 섹션 제목·빈 응답 표기 등 일부 문서 수정.
컨트롤러
src/main/java/app/nook/record/controller/RecordController.java, src/main/java/app/nook/library/controller/LibraryController.java
RecordController에 @Validated 및 커서 기반 GET 엔드포인트 3개 추가(파라미터 검증·cursor 디코딩·NO_CONTENT 처리 포함). LibraryController는 CursorResponse 제네릭/응답 코드·조건 일부 변경.
서비스 / 변환기
src/main/java/app/nook/record/service/RecordViewService.java, src/main/java/app/nook/record/converter/RecordConverter.java
읽기 전용 서비스 추가(레포지토리 호출·hasNext/nextCursor 계산·감정 파싱) 및 Record→RecordItemDto 변환기(이미지 정렬·키 필터링·PresignedUrl 변환).
쿼리 레이어 (인터페이스/구현)
src/main/java/app/nook/record/repository/RecordQueryRepository.java, src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java, src/main/java/app/nook/record/repository/RecordRepository.java
QueryDSL 기반 복합 조회 인터페이스·구현 추가(책 단위 그룹 페이징, 특정 책 레코드 페이징, 감정별 집계). RecordRepository가 새 인터페이스 상속.
DTO / 커서 유틸
src/main/java/app/nook/record/dto/BookRecordDto.java, src/main/java/app/nook/record/dto/RecordListCursor.java, src/main/java/app/nook/record/util/RecordListCursorCodec.java
책/레코드/감정 DTO 및 RecordListCursor(필드: lastCount, bookId, lastCreatedDate) 추가, Base64 인코딩/디코딩 유틸(검증·오류 매핑) 추가.
글로벌 페이징 타입 변경
src/main/java/app/nook/global/dto/CursorResponse.java
제네릭 확장: CursorResponse<T, C>로 변경하여 nextCursor 타입을 일반화(생성 팩토리 포함).
라이브러리 연계 변경
src/main/java/app/nook/library/..., src/test/java/app/nook/controller/library/LibraryControllerTest.java, src/main/java/app/nook/library/converter/LibraryConverter.java, src/main/java/app/nook/library/dto/LibraryViewDto.java, src/main/java/app/nook/library/service/LibraryService.java
CursorResponse 제네릭 변경에 맞춰 Controller/Service/Converter/DTO/테스트에서 cursor 타입을 Long으로 명시하도록 수정.
유틸·도메인·리포지토리 정리
src/main/java/app/nook/record/domain/enums/SortType.java, src/main/java/app/nook/library/repository/LibraryRepository.java, src/main/java/app/nook/global/config/WebSecurityConfig.java, src/main/java/app/nook/global/response/CommonErrorCode.java, src/main/java/app/nook/user/filter/JwtFilter.java
정렬 enum 추가, LibraryRepository Optional import 정리, CORS 허용 origin 교체, 공통 에러코드 항목 제거, JwtFilter에서 refresh-token 저장 로직 제거 및 입력 검증 강화.
테스트
src/test/java/app/nook/controller/record/RecordControllerTest.java, src/test/java/app/nook/record/service/RecordServiceTest.java
RecordController 테스트에 새로운 읽기 엔드포인트 REST Docs 추가/수정(파라미터·빈 응답 문서화) 및 RecordService 삭제 동작 단위 테스트 추가(이벤트 발행 검증).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller as RecordController
    participant Service as RecordViewService
    participant Repo as RecordRepository
    participant QueryImpl as RecordQueryRepositoryImpl
    participant DB as Database

    rect rgba(0, 100, 200, 0.5)
    Note over Client,DB: 사용자 전체 기록 목록 조회 (getUserRecords)
    Client->>Controller: GET /api/records?cursor=...&size=...&order=...
    Controller->>Service: getUserRecords(user, size, RecordListCursor, SortType)
    Service->>Repo: findRecordsByCursor(userId, cursor, sortType, size)
    Repo->>QueryImpl: QueryDSL 실행 (그룹/정렬/limit size+1)
    QueryImpl->>DB: SELECT ... GROUP BY book
    DB-->>QueryImpl: rows
    QueryImpl-->>Repo: BookRecordItemDto 리스트
    Repo-->>Service: 리스트 반환
    Service->>Service: hasNext 계산, nextCursor 생성 (RecordListCursorCodec.encode)
    Service-->>Controller: CursorResponse<BookRecordItemDto, String>
    Controller-->>Client: ApiResponse (OK 또는 NO_CONTENT)
    end
Loading
sequenceDiagram
    participant Client
    participant Controller as RecordController
    participant Service as RecordViewService
    participant Repo as RecordRepository
    participant QueryImpl as RecordQueryRepositoryImpl
    participant DB as Database
    participant Converter as RecordConverter

    rect rgba(200, 0, 100, 0.5)
    Note over Client,DB: 특정 책의 기록 조회 (getBookRecords)
    Client->>Controller: GET /api/records/books/{bookId}?cursor=...&emotion=...
    Controller->>Service: getBookRecords(user, bookId, size, cursor, emotion)
    Service->>Repo: findBookRecordsByCursor(userId, bookId, cursor, emotion, size)
    Repo->>QueryImpl: id 목록 조회 (limit size+1)
    QueryImpl->>DB: SELECT record ids
    DB-->>QueryImpl: ids
    QueryImpl-->>Repo: Record 엔티티(페치조인 포함)
    Repo-->>Service: Record 리스트
    loop for each record
      Service->>Converter: toRecordItemDto(userId, record)
      Converter->>Service: RecordItemDto (imgUrls via PresignedUrlService)
    end
    Service->>Service: hasNext 계산, nextCursor 설정
    Service-->>Controller: CursorResponse<RecordItemDto, Long>
    Controller-->>Client: ApiResponse (OK 또는 NO_CONTENT)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related PRs

Poem

🐰 폴짝, 커서 따라 흘러가는 기록들,
책마다 감정이 반짝이는 작은 별,
이미지에 URL 날개를 달아주고,
다음 페이지는 마지막 항목이 알려주네,
토끼는 기뻐서 당근춤을 춘다 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning PR은 주요 목표인 조회 API 구현 외에 추가 변경사항을 포함합니다: 토큰 응답 수정(JwtFilter), 삭제 API 응답 코드 변경(204), CORS 설정 변경, 에러 코드 정리 등이 포함되어 있습니다. 범위 밖의 변경사항(토큰 응답, CORS, 에러 코드)을 별도 PR로 분리하거나, 이번 PR의 목표와 이러한 변경사항의 관계를 명확히 문서화하세요.
Docstring Coverage ⚠️ Warning Docstring coverage is 32.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive PR 요약에서 주요 변경사항이 명시되었으나, 범위 밖의 변경사항(로그인 토큰 응답, CORS 설정, 에러 코드 정리)에 대한 상세 설명이 부족합니다. PR 본문에서 각 변경사항의 목적과 정당성을 상세히 설명하세요. 특히 핵심 목표와 무관한 변경사항들의 필요성을 명확히 기술하세요.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 PR의 주요 변경사항을 명확하게 설명합니다. 독서 기록 조회 API 구현이 핵심 변경사항이며 코드 요약에서도 이를 확인할 수 있습니다.
Linked Issues check ✅ Passed PR은 이슈 #219의 여러 요구사항을 충족합니다: 독서 기록 목록 조회 API(GET /records), 독서 기록 감상별 개수 조회(GET /records/emotions), 특정 책 기록 조회(GET /records/books/{bookId})가 구현되었습니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/#219-record-api

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
src/main/java/app/nook/record/converter/RecordConverter.java (1)

21-30: 이미지 URL 변환은 부분 실패 허용 정책을 고려해 주세요.

현재는 이미지 key 하나만 깨져 있어도 전체 기록 조회가 실패할 수 있습니다. 조회 API 특성상 실패한 이미지 항목만 제외하고 나머지 데이터를 반환하도록 완화하면 UX/안정성이 좋아집니다.

예시 방향
-                .map(key -> presignedUrlService.getImageUrl(userId, key))
+                .map(key -> resolveImageUrlSafely(userId, key))
+                .filter(Objects::nonNull)
                 .toList();
private String resolveImageUrlSafely(Long userId, String key) {
    try {
        return presignedUrlService.getImageUrl(userId, key);
    } catch (RuntimeException ex) {
        // 정책적으로 로깅 후 null 반환(또는 특정 예외만 선별 처리)
        return null;
    }
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/converter/RecordConverter.java` around lines 21
- 30, The image URL mapping in RecordConverter currently calls
presignedUrlService.getImageUrl directly which can propagate exceptions and fail
the whole conversion; modify the mapping to use a safe resolver (e.g., add a
private helper method like resolveImageUrlSafely(Long userId, String key) that
catches runtime exceptions from presignedUrlService.getImageUrl, logs the error
and returns null) then change the stream mapping from .map(key ->
presignedUrlService.getImageUrl(userId, key)) to .map(key ->
resolveImageUrlSafely(userId, key)) and filter out nulls so only failed image
entries are dropped while the rest of the record is returned.
src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java (1)

30-31: Q-type 인스턴스를 인스턴스 필드로 선언하는 것은 불필요

QRecordQRecordImage가 인스턴스 필드로 선언되어 있습니다. 이 Q-type 클래스들은 stateless이므로 문제가 되지는 않지만, 메서드 내 지역 변수로 사용하거나 QRecord.record 정적 참조를 직접 사용하는 것이 더 일반적인 패턴입니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`
around lines 30 - 31, Remove the instance fields QRecord record and QRecordImage
recordImage from RecordQueryRepositoryImpl and instead use the stateless Q-type
references locally where needed (either by using QRecord.record and
QRecordImage.recordImage inline or by declaring local variables inside methods).
Update all method bodies that currently reference the instance fields to use the
local/static references (e.g., replace usages of record -> QRecord.record and
recordImage -> QRecordImage.recordImage or declare method-local QRecord record =
QRecord.record). This keeps Q-types stateless and follows the common pattern for
QueryDSL Q-type usage.
src/test/java/app/nook/record/service/RecordServiceTest.java (1)

66-72: 빈 테스트 클래스 제거 또는 구현 필요

Save nested 클래스가 비어 있습니다. 기록 등록 테스트를 구현하거나, 아직 구현 예정이라면 TODO 주석을 추가하는 것이 좋습니다.

💡 제안: TODO 주석 추가 또는 클래스 제거
 `@Nested`
 `@DisplayName`("기록 등록")
 class Save {
-
-
-
+    // TODO: 기록 등록 테스트 추가 예정
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/app/nook/record/service/RecordServiceTest.java` around lines 66
- 72, The nested test class Save inside RecordServiceTest is empty; either
remove the empty class or implement the intended tests (e.g., add test methods
for saveRecord behavior) or mark it explicitly as pending with a TODO comment.
Locate the nested class declaration "class Save" in RecordServiceTest and either
delete that class block, add one or more `@Test` methods covering the save
functionality (using existing test helpers/mocks in RecordServiceTest), or add a
clear TODO/JUnit `@Disabled` annotation with a comment explaining why it's
pending.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/app/nook/record/controller/RecordController.java`:
- Around line 43-48: The success responses currently ignore
SuccessCode.httpStatus because RecordController returns ApiResponse objects
directly; update the controller to return ResponseEntity with the proper HTTP
status (use SuccessCode from the branch where you return ApiResponse.onSuccess)
or add an httpStatus field to ApiResponse and have RecordController use it when
building a ResponseEntity; specifically, modify methods in RecordController that
call ApiResponse.onSuccess (e.g., the branch returning SuccessCode.NO_CONTENT
and SuccessCode.OK) to wrap the ApiResponse payload in
ResponseEntity.status(successCode.getHttpStatus()).body(apiResponse) (or alter
ApiResponse to expose getHttpStatus and return a ResponseEntity there) so the
SuccessCode HTTP statuses (NO_CONTENT, CREATED, etc.) are actually applied.

In `@src/main/java/app/nook/record/repository/RecordQueryRepository.java`:
- Around line 11-13: The current findRecordsByCursor(Long cursor, ...) signature
cannot represent a stable multi-key cursor for RECORD_COUNT_ASC/DESC; change the
API to accept a composite cursor object (e.g., RecordListCursor with lastCount,
lastRecordId, lastCreatedDate) and update repository logic that implements
findRecordsByCursor to use lastCount + lastRecordId as the tie-breaker for
count-based sorting and lastCreatedDate for date-based sorting, ensuring the
WHERE clause/criteria uses (count, id) comparisons or equivalent to produce
stable next-page results; update all callers and DTO conversions to construct
and consume RecordListCursor instead of a single Long.

In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`:
- Around line 91-109: cursorCondition currently compares the cursor against
record.id and uses record.createdDate directly, which breaks cursor pagination
for book-grouped results; update the comparisons to use book.id (e.g., compare
book.id to the cursor value provided by findRecordsByCursor) and, for
RECENT_RECORDED/OLDEST_RECORDED, use the aggregated date (max(createdDate) or
min(createdDate) as appropriate) instead of the raw record.createdDate, and
apply that date-based cursor filter in the HAVING clause of the grouped query
(move the date condition out of WHERE and into the having logic used for
group-by book) so grouping by book yields correct cursor pagination.
- Around line 63-78: The query repeats filters across two .where() calls;
consolidate all predicates into a single .where(...) to remove duplicate
emotionCondition(record.emotion, emotion) and library.user.id.eq(userId) checks
and include cursorCondition(cursor, sortType) and orderByCondition(sortType)
remains as-is; locate the builder chain in RecordQueryRepositoryImpl
(methods/symbols: emotionCondition(...), cursorCondition(...),
orderByCondition(...), record.library.book.*) and replace the two .where(...)
invocations with one .where(library.user.id.eq(userId),
emotionCondition(record.emotion, emotion), cursorCondition(cursor, sortType))
keeping .groupBy(...) and .orderBy(orderByCondition(sortType)) unchanged.

---

Nitpick comments:
In `@src/main/java/app/nook/record/converter/RecordConverter.java`:
- Around line 21-30: The image URL mapping in RecordConverter currently calls
presignedUrlService.getImageUrl directly which can propagate exceptions and fail
the whole conversion; modify the mapping to use a safe resolver (e.g., add a
private helper method like resolveImageUrlSafely(Long userId, String key) that
catches runtime exceptions from presignedUrlService.getImageUrl, logs the error
and returns null) then change the stream mapping from .map(key ->
presignedUrlService.getImageUrl(userId, key)) to .map(key ->
resolveImageUrlSafely(userId, key)) and filter out nulls so only failed image
entries are dropped while the rest of the record is returned.

In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`:
- Around line 30-31: Remove the instance fields QRecord record and QRecordImage
recordImage from RecordQueryRepositoryImpl and instead use the stateless Q-type
references locally where needed (either by using QRecord.record and
QRecordImage.recordImage inline or by declaring local variables inside methods).
Update all method bodies that currently reference the instance fields to use the
local/static references (e.g., replace usages of record -> QRecord.record and
recordImage -> QRecordImage.recordImage or declare method-local QRecord record =
QRecord.record). This keeps Q-types stateless and follows the common pattern for
QueryDSL Q-type usage.

In `@src/test/java/app/nook/record/service/RecordServiceTest.java`:
- Around line 66-72: The nested test class Save inside RecordServiceTest is
empty; either remove the empty class or implement the intended tests (e.g., add
test methods for saveRecord behavior) or mark it explicitly as pending with a
TODO comment. Locate the nested class declaration "class Save" in
RecordServiceTest and either delete that class block, add one or more `@Test`
methods covering the save functionality (using existing test helpers/mocks in
RecordServiceTest), or add a clear TODO/JUnit `@Disabled` annotation with a
comment explaining why it's pending.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 22f9b062-b423-4d5d-920e-8ec30360cba9

📥 Commits

Reviewing files that changed from the base of the PR and between 2c73c61 and d031e97.

📒 Files selected for processing (14)
  • src/docs/asciidoc/index.adoc
  • src/docs/asciidoc/record-query.adoc
  • src/main/java/app/nook/library/repository/LibraryRepository.java
  • src/main/java/app/nook/record/controller/RecordController.java
  • src/main/java/app/nook/record/converter/RecordConverter.java
  • src/main/java/app/nook/record/domain/enums/SortType.java
  • src/main/java/app/nook/record/dto/BookRecordDto.java
  • src/main/java/app/nook/record/repository/RecordQueryRepository.java
  • src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java
  • src/main/java/app/nook/record/repository/RecordRepository.java
  • src/main/java/app/nook/record/service/RecordService.java
  • src/main/java/app/nook/record/service/RecordViewService.java
  • src/test/java/app/nook/controller/record/RecordControllerTest.java
  • src/test/java/app/nook/record/service/RecordServiceTest.java
💤 Files with no reviewable changes (1)
  • src/main/java/app/nook/record/service/RecordService.java

Comment on lines +43 to +48
if (response.getItems().isEmpty()) {
return ApiResponse.onSuccess(null, SuccessCode.NO_CONTENT);
}

return ApiResponse.onSuccess(response, SuccessCode.OK);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check how other endpoints handle empty/no-content responses

rg -n "NO_CONTENT" --type java -C 3 src/main/java

Repository: UMC-NOOK/Server

Length of output: 3104


🏁 Script executed:

#!/bin/bash
# Check ApiResponse implementation and how it handles HTTP 204
rg -n "class ApiResponse" --type java -A 20 src/main/java

Repository: UMC-NOOK/Server

Length of output: 1905


🏁 Script executed:

#!/bin/bash
# Check for any response converters or interceptors that might filter 204 responses
rg -n "NO_CONTENT|204" --type java src/main/java/app/nook/global -C 5

Repository: UMC-NOOK/Server

Length of output: 1113


🏁 Script executed:

#!/bin/bash
# Look for any configuration related to response handling or HTTP status codes
fd -t f "\.java$" src/main/java/app/nook/global | head -20

Repository: UMC-NOOK/Server

Length of output: 1016


🏁 Script executed:

#!/bin/bash
# Check how responses are returned - look for ResponseEntity or response entity builders
rg -n "ResponseEntity|@ResponseStatus" --type java src/main/java/app/nook | head -30

Repository: UMC-NOOK/Server

Length of output: 2137


🏁 Script executed:

#!/bin/bash
# Check ExceptionAdvice and any GlobalExceptionHandler to see response handling
cat -n src/main/java/app/nook/global/exception/ExceptionAdvice.java | head -50

Repository: UMC-NOOK/Server

Length of output: 2594


🏁 Script executed:

#!/bin/bash
# Check WebConfig for any response converters or interceptors
cat -n src/main/java/app/nook/global/config/WebConfig.java

Repository: UMC-NOOK/Server

Length of output: 2394


🏁 Script executed:

#!/bin/bash
# Search for where httpStatus from SuccessCode is actually used
rg -n "httpStatus|getHttpStatus" --type java src/main/java

Repository: UMC-NOOK/Server

Length of output: 2306


🏁 Script executed:

#!/bin/bash
# Check if ApiResponse is wrapped in ResponseEntity anywhere or if status is set elsewhere
rg -n "ApiResponse.onSuccess" --type java -B 2 -A 2 src/main/java/app/nook/record/controller | head -40

Repository: UMC-NOOK/Server

Length of output: 3478


🏁 Script executed:

#!/bin/bash
# Search for ResponseBodyAdvice or HandlerMethodReturnValueHandler
rg -n "ResponseBodyAdvice|HandlerMethodReturnValueHandler" --type java src/main/java

Repository: UMC-NOOK/Server

Length of output: 41


🏁 Script executed:

#!/bin/bash
# Check entire SuccessCode implementation to confirm httpStatus field is defined but unused
cat -n src/main/java/app/nook/global/response/SuccessCode.java

Repository: UMC-NOOK/Server

Length of output: 762


🏁 Script executed:

#!/bin/bash
# Check entire ApiResponse to see if it uses httpStatus anywhere
cat -n src/main/java/app/nook/global/response/ApiResponse.java

Repository: UMC-NOOK/Server

Length of output: 1569


🏁 Script executed:

#!/bin/bash
# Final check: confirm that httpStatus getter is available but unused
rg -n "getHttpStatus" --type java src/main/java/app/nook/global/response

Repository: UMC-NOOK/Server

Length of output: 145


성공 응답의 HTTP 상태 코드가 적용되지 않는 문제

SuccessCode에 정의된 httpStatus 필드가 실제로 사용되지 않고 있습니다. ApiResponse 클래스는 httpStatus를 캡처하지 않으며, 성공 응답은 ResponseEntity 감싸지 않고 반환되기 때문에 모든 성공 응답이 기본 HTTP 200으로 반환됩니다.

이는 SuccessCode.NO_CONTENT (204), SuccessCode.CREATED (201), SuccessCode.ACCEPTED (202) 등이 정의되어 있음에도 실제로는 적용되지 않음을 의미합니다. 에러 응답은 ExceptionAdvice에서 getHttpStatus()를 명시적으로 호출하여 상태 코드를 설정하는 반면, 성공 응답은 이러한 메커니즘이 없습니다.

ApiResponsehttpStatus 필드를 추가하고 상태 코드를 적용하도록 수정하거나, 성공 응답을 ResponseEntity로 감싸서 상태 코드를 설정해야 합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/controller/RecordController.java` around lines
43 - 48, The success responses currently ignore SuccessCode.httpStatus because
RecordController returns ApiResponse objects directly; update the controller to
return ResponseEntity with the proper HTTP status (use SuccessCode from the
branch where you return ApiResponse.onSuccess) or add an httpStatus field to
ApiResponse and have RecordController use it when building a ResponseEntity;
specifically, modify methods in RecordController that call ApiResponse.onSuccess
(e.g., the branch returning SuccessCode.NO_CONTENT and SuccessCode.OK) to wrap
the ApiResponse payload in
ResponseEntity.status(successCode.getHttpStatus()).body(apiResponse) (or alter
ApiResponse to expose getHttpStatus and return a ResponseEntity there) so the
SuccessCode HTTP statuses (NO_CONTENT, CREATED, etc.) are actually applied.

Comment thread src/main/java/app/nook/record/repository/RecordQueryRepository.java
Comment thread src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java Outdated
Comment thread src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/app/nook/library/dto/LibraryViewDto.java`:
- Line 40: The generic-parameter mismatch in LibraryViewDto (field using
CursorResponse<? extends UserStatusBookItem, Long>) must be fixed: either update
the CursorResponse class declaration to accept two type parameters (e.g., change
class CursorResponse<T> to CursorResponse<T, C> and adjust its usages and
internal nextCursor/related fields/methods) or change the LibraryViewDto field
to match the existing single-parameter signature (use CursorResponse<? extends
UserStatusBookItem> and ensure any code assuming a Long nextCursor is adapted).
Locate references to CursorResponse, UserStatusBookItem and the field in
LibraryViewDto and apply the chosen consistent change across model and
consumers.

In `@src/main/java/app/nook/record/controller/RecordController.java`:
- Around line 37-48: The controller currently only accepts a LocalDateTime and
always constructs new RecordListCursor(null, cursor), which drops the
record-count cursor used for RECORD_COUNT_* ordering; update
RecordController.getUserRecords so the request accepts the same cursor shape as
the response (either accept a RecordListCursor request parameter or two params
matching RecordListCursor fields, e.g., recordCount and recordedAt) and pass
that RecordListCursor directly into recordViewService.getUserRecords instead of
always creating new RecordListCursor(null, cursor), ensuring RecordListCursor is
parsed from the incoming request and forwarded unchanged to
RecordViewService.getUserRecords.

In `@src/main/java/app/nook/record/dto/RecordListCursor.java`:
- Around line 7-14: RecordListCursor currently only holds lastCount and
lastCreatedDate so it drops the secondary sort key (bookId) and causes
missing/duplicate records at page boundaries; update the RecordListCursor record
to include a Long bookId (and adjust isEmpty accordingly), then update the code
that constructs the cursor in RecordViewService (the nextCursor creation around
RecordViewService.nextCursor) to populate bookId, and finally update the
repository comparison logic in RecordQueryRepositoryImpl (the query/sort
predicates around lines where it compares (lastCreatedDate, book.id) or
(recordCount, book.id)) to use the same composite key ordering/compare
expressions (i.e., compare by (lastCreatedDate, bookId) or (lastCount, bookId))
so paging uses a consistent composite cursor.

In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`:
- Around line 206-210: 현재 bookRecordCursorCondition(Long cursor) 등이 단일 id 비교만
사용해 정렬키(createdDate, id)와 커서키가 달라 키셋 페이지네이션이 깨집니다; 수정 방법은 두 가지 중 하나로 선택하세요: (1)
커서를 복합 키로 바꿔 (createdDate, id) 쌍을 전달·비교하도록 bookRecordCursorCondition과 다른 커서
헬퍼들(218-229 영역)을 변경하거나, (2) 현재 방식 유지 시 커서 id로 기준 레코드의 createdDate를 먼저 조회한 뒤 해당
createdDate와 id에 대해 복합 비교식(예: createdDate < cursorCreatedDate OR (createdDate =
cursorCreatedDate AND id < cursorId))으로 필터링하도록 구현을 변경하세요; 대상 함수
이름(bookRecordCursorCondition 및 동일 패턴의 다른 cursor 조건 헬퍼)을 찾아 위 로직 중 하나로 교체하세요.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 44a3b7dc-d141-46f6-8bab-186e7f494ebf

📥 Commits

Reviewing files that changed from the base of the PR and between d031e97 and f3442c2.

📒 Files selected for processing (11)
  • src/main/java/app/nook/library/controller/LibraryController.java
  • src/main/java/app/nook/library/converter/LibraryConverter.java
  • src/main/java/app/nook/library/dto/LibraryViewDto.java
  • src/main/java/app/nook/library/service/LibraryService.java
  • src/main/java/app/nook/record/controller/RecordController.java
  • src/main/java/app/nook/record/dto/RecordListCursor.java
  • src/main/java/app/nook/record/repository/RecordQueryRepository.java
  • src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java
  • src/main/java/app/nook/record/service/RecordViewService.java
  • src/test/java/app/nook/controller/library/LibraryControllerTest.java
  • src/test/java/app/nook/controller/record/RecordControllerTest.java
✅ Files skipped from review due to trivial changes (1)
  • src/main/java/app/nook/record/service/RecordViewService.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/test/java/app/nook/controller/record/RecordControllerTest.java
  • src/main/java/app/nook/record/repository/RecordQueryRepository.java

Comment thread src/main/java/app/nook/library/dto/LibraryViewDto.java
Comment thread src/main/java/app/nook/record/controller/RecordController.java Outdated
Comment thread src/main/java/app/nook/record/dto/RecordListCursor.java
Comment thread src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/main/java/app/nook/record/service/RecordViewService.java (1)

106-119: toNextCursor에서 추가 쿼리 발생 - 최적화 가능

RECENT_RECORDEDOLDEST_RECORDED 정렬 시 findBookBoundaryCreatedDate를 호출하여 매 페이지 요청마다 추가 DB 쿼리가 발생합니다.

findRecordsByCursor 쿼리에서 이미 max(createdDate) 또는 min(createdDate)를 계산하므로, BookRecordItemDto에 해당 값을 포함시키면 추가 쿼리를 제거할 수 있습니다.

♻️ 최적화 제안

BookRecordItemDtolastCreatedDate 필드를 추가하고, findRecordsByCursor 쿼리에서 함께 조회하면 toNextCursor에서 추가 쿼리 없이 커서를 생성할 수 있습니다.

// BookRecordDto.java
public record BookRecordItemDto(
        Long bookId,
        String title,
        String author,
        String recordContent,
        String coverImageUrl,
        Long recordCount,
        LocalDateTime lastCreatedDate // 추가
){}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/service/RecordViewService.java` around lines
106 - 119, toNextCursor currently calls
recordRepository.findBookBoundaryCreatedDate for
RECENT_RECORDED/OLDEST_RECORDED, causing an extra DB query per page; modify the
pipeline to include the boundary createdDate in the query that builds
BookRecordItemDto (update findRecordsByCursor to select max/min(createdDate) as
lastCreatedDate and add a lastCreatedDate field to BookRecordItemDto) and then
change toNextCursor to use item.lastCreatedDate() instead of invoking
findBookBoundaryCreatedDate so no additional query is needed.
src/test/java/app/nook/controller/record/RecordControllerTest.java (1)

78-174: 테스트 커버리지 개선을 권장합니다.

독서 기록 목록 조회 테스트가 성공 케이스와 빈 데이터 케이스를 잘 다루고 있습니다. 다만 다음 경계 케이스에 대한 테스트 추가를 권장합니다:

  • 잘못된 커서 문자열 (Base64 디코딩 실패 또는 파싱 실패) → INVALID_REQUEST 응답 확인
  • 잘못된 order 값 → 400 Bad Request 확인
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/app/nook/controller/record/RecordControllerTest.java` around
lines 78 - 174, Add two new tests in RecordControllerTest under the
GetUserRecords nested class: one for an invalid cursor string and one for an
invalid order value. For the invalid cursor test (e.g., name it
독서_기록_목록_조회_잘못된_커서), call
mockMvc.perform(get("/api/records").header(AUTH_HEADER,
AUTH_TOKEN).param("cursor","invalid-base64")), expect the controller to return
the invalid-request error JSON (e.g., isSuccess=false / code "INVALID_REQUEST"
or matching your global handler) and assert that
recordViewService.getUserRecords(...) is never invoked (use
verify(recordViewService, never(), ...)). For the invalid order test (e.g.,
독서_기록_목록_조회_잘못된_정렬값), call
mockMvc.perform(get("/api/records").header(AUTH_HEADER,
AUTH_TOKEN).param("order","BAD_ORDER")), expect 400 Bad Request (and/or
appropriate error JSON), and similarly verify recordViewService.getUserRecords
is not called; ensure both tests use `@WithCustomUser` like the others.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/main/java/app/nook/record/service/RecordViewService.java`:
- Around line 106-119: toNextCursor currently calls
recordRepository.findBookBoundaryCreatedDate for
RECENT_RECORDED/OLDEST_RECORDED, causing an extra DB query per page; modify the
pipeline to include the boundary createdDate in the query that builds
BookRecordItemDto (update findRecordsByCursor to select max/min(createdDate) as
lastCreatedDate and add a lastCreatedDate field to BookRecordItemDto) and then
change toNextCursor to use item.lastCreatedDate() instead of invoking
findBookBoundaryCreatedDate so no additional query is needed.

In `@src/test/java/app/nook/controller/record/RecordControllerTest.java`:
- Around line 78-174: Add two new tests in RecordControllerTest under the
GetUserRecords nested class: one for an invalid cursor string and one for an
invalid order value. For the invalid cursor test (e.g., name it
독서_기록_목록_조회_잘못된_커서), call
mockMvc.perform(get("/api/records").header(AUTH_HEADER,
AUTH_TOKEN).param("cursor","invalid-base64")), expect the controller to return
the invalid-request error JSON (e.g., isSuccess=false / code "INVALID_REQUEST"
or matching your global handler) and assert that
recordViewService.getUserRecords(...) is never invoked (use
verify(recordViewService, never(), ...)). For the invalid order test (e.g.,
독서_기록_목록_조회_잘못된_정렬값), call
mockMvc.perform(get("/api/records").header(AUTH_HEADER,
AUTH_TOKEN).param("order","BAD_ORDER")), expect 400 Bad Request (and/or
appropriate error JSON), and similarly verify recordViewService.getUserRecords
is not called; ensure both tests use `@WithCustomUser` like the others.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b81405d-12f0-449f-b0ed-d7b3a68637f6

📥 Commits

Reviewing files that changed from the base of the PR and between f3442c2 and 441e362.

📒 Files selected for processing (7)
  • src/main/java/app/nook/global/dto/CursorResponse.java
  • src/main/java/app/nook/record/controller/RecordController.java
  • src/main/java/app/nook/record/dto/RecordListCursor.java
  • src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java
  • src/main/java/app/nook/record/service/RecordViewService.java
  • src/main/java/app/nook/record/util/RecordListCursorCodec.java
  • src/test/java/app/nook/controller/record/RecordControllerTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/app/nook/record/dto/RecordListCursor.java

@JiwonLee42
Copy link
Copy Markdown
Contributor Author

Overall Project 58.27% -9.15%
Files changed 13.59%

File Coverage
LibraryController.java 100% 🍏
RecordController.java 100% 🍏
SortType.java 100% 🍏
CommonErrorCode.java 100% 🍏
LibraryService.java 87.82% 🍏
LibraryConverter.java 67.89% 🍏
RecordListCursorCodec.java 38.46% -61.54%
JwtFilter.java 26.14% -8.52%
RecordService.java 14.65% 🍏
RecordQueryRepositoryImpl.java 0%
RecordConverter.java 0%
RecordViewService.java 0%

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/app/nook/user/filter/JwtFilter.java (1)

106-108: ⚠️ Potential issue | 🟠 Major

액세스 토큰 원문 로깅은 제거가 필요합니다.

Line 107에서 newAccessToken을 그대로 로그에 남기면 토큰 유출 위험이 있습니다. 민감정보는 마스킹하거나 성공 이벤트만 기록하세요.

🔐 제안 수정안
-                log.info("[TOKEN] 토큰 만료, 새로운 토큰 발급 ={}", newAccessToken);
+                log.info("[TOKEN] 토큰 만료로 access token 재발급 완료. userId={}", userId);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/user/filter/JwtFilter.java` around lines 106 - 108,
The current JwtFilter logs the raw newAccessToken (log.info at JwtFilter around
response.setHeader and setAuthentication), which risks token leakage; remove the
plaintext token from logs and instead log only a non-sensitive success message
or a masked token (e.g., last 4 chars) if you must trace, and ensure
response.setHeader(HttpHeaders.AUTHORIZATION, JwtProvider.BEARER_PREFIX +
newAccessToken) and setAuthentication(newAccessToken) remain unchanged; update
the log.info call to exclude newAccessToken (or replace with a masked
representation) and keep logging limited to event-level info only.
♻️ Duplicate comments (1)
src/main/java/app/nook/record/controller/RecordController.java (1)

52-56: ⚠️ Potential issue | 🟠 Major

SuccessCode의 HTTP 상태가 실제 응답에 적용되지 않습니다.

Line 53, Line 71, Line 113처럼 ApiResponse.onSuccess(..., SuccessCode.NO_CONTENT)를 그대로 반환하면 Spring은 바디만 직렬화하고 HTTP status는 200으로 보냅니다. 같은 패턴이라 기존 CREATED도 실제로는 200이므로, ResponseEntity로 상태를 설정하거나 204를 유지할 거면 바디 없이 반환하는 쪽으로 정리해야 합니다.

Also applies to: 70-74, 113-113

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/controller/RecordController.java` around lines
52 - 56, The controller is returning ApiResponse objects but not setting HTTP
status, so Spring always returns 200; update RecordController to return
ResponseEntity and set status explicitly when using ApiResponse.onSuccess (e.g.,
wrap ApiResponse in
ResponseEntity.status(SuccessCode.NO_CONTENT.getHttpStatus()).body(...)) or, for
NO_CONTENT, return ResponseEntity.noContent().build() with no body; apply this
change where ApiResponse.onSuccess(..., SuccessCode.NO_CONTENT) and where
CREATED is intended (use
ResponseEntity.status(SuccessCode.CREATED.getHttpStatus()).body(...)). Ensure
you update method return types accordingly and adjust all occurrences of
ApiResponse.onSuccess with SuccessCode.NO_CONTENT and SuccessCode.CREATED in
RecordController.
🧹 Nitpick comments (1)
src/test/java/app/nook/controller/record/RecordControllerTest.java (1)

72-74: 핵심 pagination 로직은 아직 이 테스트들로 검증되지 않습니다.

Line 72-73에서 RecordViewService를 mock 처리해서, 이번 PR의 위험 구간인 QueryDSL keyset 조건과 next cursor 생성이 전혀 실행되지 않습니다. 특히 동일 createdDate/동일 recordCount tie-break 케이스는 RecordQueryRepositoryImpl 또는 RecordViewService 통합 테스트를 한 벌 더 두는 편이 안전합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/app/nook/controller/record/RecordControllerTest.java` around
lines 72 - 74, The tests in RecordControllerTest currently mock
RecordViewService, so the critical pagination logic (QueryDSL keyset conditions
and next-cursor generation, including tie-break on createdDate/recordCount)
never executes; remove or replace the `@MockitoBean` mock for RecordViewService in
RecordControllerTest and either (a) wire the real RecordViewService with an
embedded/test database so controller tests exercise RecordQueryRepositoryImpl's
keyset pagination and nextCursor logic, or (b) add a dedicated integration test
targeting RecordQueryRepositoryImpl (or a non-mocked RecordViewService) that
inserts tie-breaker cases (identical createdDate and recordCount) and asserts
the generated nextCursor and returned page ordering are correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/docs/asciidoc/library.adoc`:
- Around line 58-61: The include paths for the "empty response (200)" case are
pointing to the `_204` test artifact names (e.g., snippets including
최근_포커스_도서_조회_데이터없음_204/http-request.adoc), causing the wrong snippets to be
used; update each include to reference the correct `_200` artifacts (replace
`_204` with `_200` in the four include lines for http-request.adoc,
request-headers.adoc, http-response.adoc, and response-fields.adoc) so the
document pulls the matching "빈 응답(200)" test outputs.

In `@src/main/java/app/nook/record/controller/RecordController.java`:
- Line 63: The cursor parameter in RecordController's endpoint is annotated with
`@Min`(0) but should forbid 0 because cursor represents a previous response's
recordId and 0 is never a valid next-page value; update the annotation on the
`@RequestParam` Long cursor from `@Min`(0) to `@Min`(1) (in the method in class
RecordController) so requests with cursor=0 fail validation and do not return an
empty page.

In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`:
- Around line 296-304: 현재 RecordQueryRepositoryImpl의 emotionCounts 쿼리는
groupBy(record.emotion)만 있어 반환 순서가 불확정하므로 .orderBy(...)를 명시해 고정하세요; 예를 들어 감정 빈도
순으로 정렬하려면
queryFactory.select(...).from(record).where(record.library.user.id.eq(userId)).groupBy(record.emotion).orderBy(record.count().desc()).fetch();
또는 enum 정의 순으로 정렬하려면 .orderBy(record.emotion.asc())를 사용해 일관된 정렬을 보장하도록 수정하세요.
- Around line 226-245: The cursor subquery in bookRecordCursorCondition(Long
cursor) reads createdDate by cursor id only, allowing cursors from other
users/books to skew pagination; change the method to accept the current userId
and bookId (e.g., bookRecordCursorCondition(Long cursor, Long userId, Long
bookId)) and restrict the subquery to the same scope by adding
cursorRecord.userId.eq(userId) and cursorRecord.bookId.eq(bookId) to the
JPAExpressions WHERE clause; additionally, if no matching cursorRecord exists
(i.e., the subquery returns null) throw an explicit validation exception (e.g.,
IllegalArgumentException or map to INVALID_REQUEST) rather than silently using
the timestamp; apply the same fix to the analogous logic referenced around lines
248-260.

---

Outside diff comments:
In `@src/main/java/app/nook/user/filter/JwtFilter.java`:
- Around line 106-108: The current JwtFilter logs the raw newAccessToken
(log.info at JwtFilter around response.setHeader and setAuthentication), which
risks token leakage; remove the plaintext token from logs and instead log only a
non-sensitive success message or a masked token (e.g., last 4 chars) if you must
trace, and ensure response.setHeader(HttpHeaders.AUTHORIZATION,
JwtProvider.BEARER_PREFIX + newAccessToken) and
setAuthentication(newAccessToken) remain unchanged; update the log.info call to
exclude newAccessToken (or replace with a masked representation) and keep
logging limited to event-level info only.

---

Duplicate comments:
In `@src/main/java/app/nook/record/controller/RecordController.java`:
- Around line 52-56: The controller is returning ApiResponse objects but not
setting HTTP status, so Spring always returns 200; update RecordController to
return ResponseEntity and set status explicitly when using ApiResponse.onSuccess
(e.g., wrap ApiResponse in
ResponseEntity.status(SuccessCode.NO_CONTENT.getHttpStatus()).body(...)) or, for
NO_CONTENT, return ResponseEntity.noContent().build() with no body; apply this
change where ApiResponse.onSuccess(..., SuccessCode.NO_CONTENT) and where
CREATED is intended (use
ResponseEntity.status(SuccessCode.CREATED.getHttpStatus()).body(...)). Ensure
you update method return types accordingly and adjust all occurrences of
ApiResponse.onSuccess with SuccessCode.NO_CONTENT and SuccessCode.CREATED in
RecordController.

---

Nitpick comments:
In `@src/test/java/app/nook/controller/record/RecordControllerTest.java`:
- Around line 72-74: The tests in RecordControllerTest currently mock
RecordViewService, so the critical pagination logic (QueryDSL keyset conditions
and next-cursor generation, including tie-break on createdDate/recordCount)
never executes; remove or replace the `@MockitoBean` mock for RecordViewService in
RecordControllerTest and either (a) wire the real RecordViewService with an
embedded/test database so controller tests exercise RecordQueryRepositoryImpl's
keyset pagination and nextCursor logic, or (b) add a dedicated integration test
targeting RecordQueryRepositoryImpl (or a non-mocked RecordViewService) that
inserts tie-breaker cases (identical createdDate and recordCount) and asserts
the generated nextCursor and returned page ordering are correct.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a94d57fc-4915-4d68-89e7-61f51b4240cf

📥 Commits

Reviewing files that changed from the base of the PR and between 441e362 and 2b76487.

📒 Files selected for processing (16)
  • src/docs/asciidoc/index.adoc
  • src/docs/asciidoc/library.adoc
  • src/docs/asciidoc/record-query.adoc
  • src/main/java/app/nook/global/config/WebSecurityConfig.java
  • src/main/java/app/nook/global/response/CommonErrorCode.java
  • src/main/java/app/nook/library/controller/LibraryController.java
  • src/main/java/app/nook/record/controller/RecordController.java
  • src/main/java/app/nook/record/dto/BookRecordDto.java
  • src/main/java/app/nook/record/dto/RecordListCursor.java
  • src/main/java/app/nook/record/repository/RecordQueryRepository.java
  • src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java
  • src/main/java/app/nook/record/service/RecordViewService.java
  • src/main/java/app/nook/record/util/RecordListCursorCodec.java
  • src/main/java/app/nook/user/filter/JwtFilter.java
  • src/test/java/app/nook/controller/library/LibraryControllerTest.java
  • src/test/java/app/nook/controller/record/RecordControllerTest.java
✅ Files skipped from review due to trivial changes (5)
  • src/docs/asciidoc/index.adoc
  • src/main/java/app/nook/global/config/WebSecurityConfig.java
  • src/main/java/app/nook/record/dto/RecordListCursor.java
  • src/docs/asciidoc/record-query.adoc
  • src/main/java/app/nook/record/repository/RecordQueryRepository.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/test/java/app/nook/controller/library/LibraryControllerTest.java
  • src/main/java/app/nook/record/util/RecordListCursorCodec.java
  • src/main/java/app/nook/library/controller/LibraryController.java

Comment thread src/docs/asciidoc/library.adoc Outdated
Comment thread src/main/java/app/nook/record/controller/RecordController.java Outdated
Comment on lines +226 to +245
private BooleanExpression bookRecordCursorCondition(Long cursor) {
if (cursor == null) {
return null;
}

QRecord cursorRecord = new QRecord("cursorRecord");
return record.createdDate.lt(
JPAExpressions
.select(cursorRecord.createdDate)
.from(cursorRecord)
.where(cursorRecord.id.eq(cursor))
)
.or(record.createdDate.eq(
JPAExpressions
.select(cursorRecord.createdDate)
.from(cursorRecord)
.where(cursorRecord.id.eq(cursor))
)
.and(record.id.lt(cursor)));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

커서 기준 레코드를 현재 사용자/책 범위로 제한하세요.

Line 231-243의 서브쿼리는 cursorRecord.id만으로 기준 createdDate를 읽고, Line 260은 그 값을 그대로 페이지 경계로 사용합니다. 그래서 다른 책이나 다른 사용자의 recordId를 넘겨도 그 시각이 커서 기준값으로 적용되어 페이지가 틀어질 수 있습니다. 커서 조회에도 userIdbookId를 같이 걸거나, 범위를 벗어난 커서는 INVALID_REQUEST로 거절하는 편이 안전합니다.

Also applies to: 248-260

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`
around lines 226 - 245, The cursor subquery in bookRecordCursorCondition(Long
cursor) reads createdDate by cursor id only, allowing cursors from other
users/books to skew pagination; change the method to accept the current userId
and bookId (e.g., bookRecordCursorCondition(Long cursor, Long userId, Long
bookId)) and restrict the subquery to the same scope by adding
cursorRecord.userId.eq(userId) and cursorRecord.bookId.eq(bookId) to the
JPAExpressions WHERE clause; additionally, if no matching cursorRecord exists
(i.e., the subquery returns null) throw an explicit validation exception (e.g.,
IllegalArgumentException or map to INVALID_REQUEST) rather than silently using
the timestamp; apply the same fix to the analogous logic referenced around lines
248-260.

Comment on lines +296 to +304
List<BookRecordDto.RecordEmotionDto> emotionCounts = queryFactory
.select(Projections.constructor(BookRecordDto.RecordEmotionDto.class,
record.emotion,
record.count()
))
.from(record)
.where(record.library.user.id.eq(userId))
.groupBy(record.emotion)
.fetch();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

감정 집계 리스트의 정렬을 고정하세요.

Line 296-304는 groupBy(record.emotion)만 있고 orderBy(...)가 없습니다. 지금 응답 타입이 List<RecordEmotionDto>라서 DB/실행계획에 따라 감정 순서가 바뀔 수 있으니, enum 순서든 count 기준이든 한 가지 기준으로 명시적으로 정렬해 두는 편이 안전합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`
around lines 296 - 304, 현재 RecordQueryRepositoryImpl의 emotionCounts 쿼리는
groupBy(record.emotion)만 있어 반환 순서가 불확정하므로 .orderBy(...)를 명시해 고정하세요; 예를 들어 감정 빈도
순으로 정렬하려면
queryFactory.select(...).from(record).where(record.library.user.id.eq(userId)).groupBy(record.emotion).orderBy(record.count().desc()).fetch();
또는 enum 정의 순으로 정렬하려면 .orderBy(record.emotion.asc())를 사용해 일관된 정렬을 보장하도록 수정하세요.

@JiwonLee42
Copy link
Copy Markdown
Contributor Author

Overall Project 58.25% -9.15%
Files changed 13.37%

File Coverage
LibraryController.java 100% 🍏
RecordController.java 100% 🍏
SortType.java 100% 🍏
CommonErrorCode.java 100% 🍏
LibraryService.java 87.82% 🍏
LibraryConverter.java 67.89% 🍏
RecordListCursorCodec.java 38.46% -61.54%
JwtFilter.java 26.14% -8.52%
RecordService.java 14.65% 🍏
RecordQueryRepositoryImpl.java 0%
RecordConverter.java 0%
RecordViewService.java 0%

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
src/main/java/app/nook/record/controller/RecordController.java (1)

35-55: ⚠️ Potential issue | 🟠 Major

SuccessCode의 HTTP 상태가 전송 레벨에 반영되지 않을 가능성이 큽니다

Line 51, Line 69, Line 92, Line 111에서 SuccessCode.NO_CONTENT/CREATED를 사용하지만, 컨트롤러가 ApiResponse를 직접 반환하고 있어 실제 HTTP status가 200으로 고정될 수 있습니다(이전 리뷰 지적과 동일 이슈).

수정 예시 (ResponseEntity로 상태 코드 명시)
+import org.springframework.http.ResponseEntity;
...
-    public ApiResponse<CursorResponse<BookRecordDto.BookRecordItemDto, String>> getUserRecords(
+    public ResponseEntity<ApiResponse<CursorResponse<BookRecordDto.BookRecordItemDto, String>>> getUserRecords(
...
-            return ApiResponse.onSuccess(null, SuccessCode.NO_CONTENT);
+            return ResponseEntity.status(SuccessCode.NO_CONTENT.getHttpStatus())
+                    .body(ApiResponse.onSuccess(null, SuccessCode.NO_CONTENT));
...
-        return ApiResponse.onSuccess(response, SuccessCode.OK);
+        return ResponseEntity.status(SuccessCode.OK.getHttpStatus())
+                .body(ApiResponse.onSuccess(response, SuccessCode.OK));
     }
...
-    public ApiResponse<Void> saveRecord(
+    public ResponseEntity<ApiResponse<Void>> saveRecord(
...
-        return ApiResponse.onSuccess(null, SuccessCode.CREATED);
+        return ResponseEntity.status(SuccessCode.CREATED.getHttpStatus())
+                .body(ApiResponse.onSuccess(null, SuccessCode.CREATED));
     }
...
-    public ApiResponse<Void> deleteRecord(
+    public ResponseEntity<ApiResponse<Void>> deleteRecord(
...
-        return ApiResponse.onSuccess(null, SuccessCode.NO_CONTENT);
+        return ResponseEntity.status(SuccessCode.NO_CONTENT.getHttpStatus())
+                .body(ApiResponse.onSuccess(null, SuccessCode.NO_CONTENT));
     }

Also applies to: 57-73, 85-93, 105-112

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/app/nook/record/controller/RecordController.java` around lines
35 - 55, The controller is returning ApiResponse objects directly which risks
always sending HTTP 200 despite using SuccessCode (e.g., SuccessCode.NO_CONTENT,
CREATED); update the controller methods (e.g., RecordController.getUserRecords
and the other methods that return ApiResponse) to return
ResponseEntity<ApiResponse<?>> instead and map the SuccessCode to the
appropriate HttpStatus (e.g., NO_CONTENT -> HttpStatus.NO_CONTENT, CREATED ->
HttpStatus.CREATED) when building the ResponseEntity so the actual HTTP status
header reflects the SuccessCode; ensure you wrap the existing ApiResponse
payload in ResponseEntity.status(mappedStatus).body(apiResponse) for all
affected methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/main/java/app/nook/record/controller/RecordController.java`:
- Around line 35-55: The controller is returning ApiResponse objects directly
which risks always sending HTTP 200 despite using SuccessCode (e.g.,
SuccessCode.NO_CONTENT, CREATED); update the controller methods (e.g.,
RecordController.getUserRecords and the other methods that return ApiResponse)
to return ResponseEntity<ApiResponse<?>> instead and map the SuccessCode to the
appropriate HttpStatus (e.g., NO_CONTENT -> HttpStatus.NO_CONTENT, CREATED ->
HttpStatus.CREATED) when building the ResponseEntity so the actual HTTP status
header reflects the SuccessCode; ensure you wrap the existing ApiResponse
payload in ResponseEntity.status(mappedStatus).body(apiResponse) for all
affected methods.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fea7ca4d-adad-4f3f-9a0b-870cc546ceb0

📥 Commits

Reviewing files that changed from the base of the PR and between 2b76487 and f74ea11.

📒 Files selected for processing (2)
  • src/docs/asciidoc/library.adoc
  • src/main/java/app/nook/record/controller/RecordController.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/docs/asciidoc/library.adoc

@JiwonLee42 JiwonLee42 merged commit 75d985c into develop-demo Apr 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] 독서기록 API 구현

1 participant