[FEAT] 독서 기록 API 수정 & 로그인 리팩토링#264
Merged
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Walkthrough이 PR은 두 개의 독립적인 변경을 포함합니다: (1) 독서 기록 감정 집계를 사용자 전체에서 특정 책(bookId) 범위로 변경하고 DTO/저장소/서비스/컨트롤러/테스트/문서를 연쇄 변경. (2) 리프레시 토큰 기반 액세스 토큰 재발급 흐름(엔드포인트, 서비스, Redis 조회)과 JWT 클레임에서 이메일 대신 userId 사용으로 인증 필터/토큰 제공자 로직을 변경. Changes기록 감정별 개수 조회 범위 변경
토큰 재발급 인증 흐름
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller
participant QueryService
participant Repo
participant DB
Client->>Controller: GET /api/v1/records/emotions/{bookId}
Controller->>QueryService: getRecordEmotionCounts(user, bookId)
QueryService->>QueryService: validate book exists & ownership
QueryService->>Repo: countRecordsByEmotion(userId, bookId)
Repo->>DB: query grouped emotion counts for user+book
DB-->>Repo: grouped counts (emotion string, count)
Repo-->>QueryService: RecordEmotionCountResponse
QueryService->>QueryService: normalize (exclude EMPTY), prepend "ALL"
QueryService-->>Controller: ApiResponse(200, payload)
Controller-->>Client: 200 OK
sequenceDiagram
actor Client
participant AuthController
participant UserService
participant JwtProvider
participant Redis
participant UserRepo
Client->>AuthController: POST /api/v1/auth/reissue {refreshToken}
AuthController->>UserService: reissueAccessToken(refreshToken)
UserService->>JwtProvider: validateToken(refreshToken)
JwtProvider-->>UserService: valid/expired/invalid
alt expired
UserService-->>AuthController: throw TOKEN_EXPIRED
else valid
UserService->>Redis: findByRefreshToken(refreshToken)
Redis-->>UserService: TokenRedis{userId}
UserService->>UserRepo: findById(userId)
UserRepo-->>UserService: User
UserService->>JwtProvider: createAccessToken(user)
JwtProvider-->>UserService: accessToken
UserService->>Redis: save(new TokenRedis with tokens)
Redis-->>UserService: saved
UserService-->>AuthController: TokenReissueResponse(accessToken, refreshToken)
AuthController-->>Client: 200 OK
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 50 minutes and 9 seconds.Comment |
Contributor
Author
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/app/nook/user/dto/UserDTO.java (1)
12-22:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
refreshToken을 JSON 응답 본문에 노출하면 XSS 취약점에 노출될 수 있습니다
refreshToken은 수명이 3일로 길고, 새 액세스 토큰을 계속 발급받는 데 사용되므로accessToken보다 훨씬 민감한 자산입니다. 응답 본문(JSON)에 포함하면 브라우저의 JavaScript에서 접근 가능해져 XSS 공격으로 탈취될 위험이 있습니다.보안 권장 사항:
refreshToken은HttpOnly,Secure,SameSite=Strict속성을 갖춘 쿠키로 전달하세요.- 응답 JSON(
LoginResponse)에서는refreshToken필드를 제거하고, 서버에서 쿠키로 직접 설정해야 합니다./auth/reissue엔드포인트도 동일하게 요청 바디 대신 쿠키에서refreshToken을 읽어야 합니다.웹 클라이언트 외에 모바일 앱만 사용하는 경우라면 응답 본문 전달도 허용될 수 있으나, 이 경우에도 저장 위치(SecureStorage 등)에 대한 클라이언트 측 보안 가이드라인이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/user/dto/UserDTO.java` around lines 12 - 22, LoginResponse currently exposes the sensitive refreshToken in JSON; remove the refreshToken field from the LoginResponse DTO (class LoginResponse) and stop returning it in login responses, and instead set the refresh token as a HttpOnly, Secure, SameSite=Strict cookie on the server; also update the `/auth/reissue` flow to read the refresh token from the cookie (not request body) and adjust any methods that previously relied on LoginResponse.refreshToken to use the cookie-based token retrieval.src/main/java/app/nook/user/redis/TokenRedis.java (1)
15-25:⚠️ Potential issue | 🟠 MajorRedis 메모리 누수:
@Indexed필드의 세컨더리 인덱스가 정리되지 않음
@RedisHash(timeToLive = 60*60*24*3)로 설정된 기본 해시는 3일 후 만료되지만,@Indexed필드의 세컨더리 인덱스 셋(token:refreshToken:<value>,token:accessToken:<value>)은 TTL이 적용되지 않아 영구적으로 Redis에 잔류합니다.원인:
RedisConfig의@EnableRedisRepositories에서enableKeyspaceEvents가 활성화되지 않음 (현재 기본값)application.yml에spring.data.redis.notify-keyspace-events설정 없음- 코드베이스에 Redis 키 만료 이벤트 리스너 없음
영향:
refreshToken과accessToken모두@Indexed로 지정되어 있어 매 로그인마다 새로운 인덱스 항목 두 개씩 누적- 고빈도 로그인 환경에서 Redis 메모리 지속적 증가 → 메모리 한계 도달 가능
해결 방법:
@EnableRedisRepositories(enableKeyspaceEvents = "notify-keyspace-events")활성화 및 Redis 서버의 keyspace 알림 설정(notify-keyspace-events = KEA)과 함께 구성해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/user/redis/TokenRedis.java` around lines 15 - 25, The TokenRedis entity uses `@Indexed` on refreshToken and accessToken which creates secondary index keys that never expire because keyspace notifications are not enabled; update your Redis repository configuration (the class with `@EnableRedisRepositories`) to set enableKeyspaceEvents="notify-keyspace-events" and ensure your application.yml sets spring.data.redis.notify-keyspace-events=KEA on the Redis server, then add a Redis keyspace event listener/bean to react to EXPIRATION/DEL events and remove the corresponding secondary index entries (or alternatively stop indexing these fields if you prefer no secondary keys); reference TokenRedis, the `@EnableRedisRepositories` annotation in RedisConfig, and the application.yml property when making the changes.src/docs/asciidoc/record-query.adoc (1)
30-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
path-parameters.adoc누락 —bookId경로 파라미터가 문서에 표시되지 않음컨트롤러 테스트(
RecordControllerTest.javaLines 360-362)에서pathParameters로bookId를 문서화하고 있어 스니펫이 생성되지만,record-query.adoc에 해당 include가 없습니다. "특정 책 기록 감정 필터 조회" 섹션의 패턴을 참고하여 추가하세요.📄 제안 수정
include::{snippets}/record-controller-test/독서_기록_감정별_개수_조회_성공/http-request.adoc[opts=optional] include::{snippets}/record-controller-test/독서_기록_감정별_개수_조회_성공/request-headers.adoc[opts=optional] +include::{snippets}/record-controller-test/독서_기록_감정별_개수_조회_성공/path-parameters.adoc[opts=optional] include::{snippets}/record-controller-test/독서_기록_감정별_개수_조회_성공/http-response.adoc[opts=optional] include::{snippets}/record-controller-test/독서_기록_감정별_개수_조회_성공/response-fields.adoc[opts=optional]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/docs/asciidoc/record-query.adoc` around lines 30 - 33, The documentation is missing the path-parameters include for the "독서_기록_감정별_개수_조회_성공" snippets so the bookId path parameter documented in RecordControllerTest.java (the pathParameters snippet) doesn't appear; add an include line for the path-parameters snippet (include::{snippets}/record-controller-test/독서_기록_감정별_개수_조회_성공/path-parameters.adoc[opts=optional]) into record-query.adoc in the same place/pattern used by the "특정 책 기록 감정 필터 조회" section so the bookId path parameter is rendered.
🧹 Nitpick comments (3)
src/test/java/app/nook/user/service/UserServiceTest.java (1)
194-206: ⚡ Quick win
validateToken → false, isExpiredToken → false분기(순수 형식 오류 토큰)에 대한 테스트 누락
UserService.reissueAccessTokenLines 91-92의throw new CustomException(AuthErrorCode.INVALID_TOKEN)분기는 현재 테스트에서 검증되지 않습니다.레디스에없는토큰_예외테스트는validateToken → true경로에서의 INVALID_TOKEN이므로, JWT 자체가 유효하지 않지만(만료는 아닌) 서명 오류·형식 오류 등의 경우에 대한 별도 테스트가 필요합니다.✅ 누락된 테스트 케이스 추가 제안
`@Test` void reissueAccessToken_유효하지않은토큰_예외() { String refreshToken = "malformed-token"; given(jwtProvider.validateToken(refreshToken)).willReturn(false); given(jwtProvider.isExpiredToken(refreshToken)).willReturn(false); // 만료 아님, 형식 오류 CustomException ex = assertThrows( CustomException.class, () -> userService.reissueAccessToken(refreshToken) ); assertThat(ex.getErrorCode()).isEqualTo(AuthErrorCode.INVALID_TOKEN); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/app/nook/user/service/UserServiceTest.java` around lines 194 - 206, Add a unit test for the branch where the JWT is syntactically/signature-invalid but not expired: in UserServiceTest add a test (e.g., reissueAccessToken_유효하지않은토큰_예외) that stubs jwtProvider.validateToken(refreshToken) to return false and jwtProvider.isExpiredToken(refreshToken) to return false, then calls userService.reissueAccessToken(refreshToken) and asserts a CustomException with error code AuthErrorCode.INVALID_TOKEN; this ensures the throw new CustomException(AuthErrorCode.INVALID_TOKEN) path in UserService.reissueAccessToken is exercised.src/test/java/app/nook/user/filter/JwtFilterTest.java (1)
51-130: ⚡ Quick win
userId클레임 누락 케이스도 테스트로 고정해 두면 좋겠습니다.지금 스위트는
userId가 있는 정상 케이스만 검증합니다.src/main/java/app/nook/user/filter/JwtFilter.java의 Line 64~Line 67은 이 클레임이 없으면UNAUTHORIZED를 던지도록 바뀌었으니,validateToken()==true인데parseClaims()에userId가 없는 경우를 하나 추가해 두면 이번 계약 변경을 안정적으로 묶어둘 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/app/nook/user/filter/JwtFilterTest.java` around lines 51 - 130, Add a test in JwtFilterTest (e.g., method named 유효한_토큰이지만_userId_클레임이_없으면_UNAUTHORIZED를_낸다) that simulates jwtProvider.validateToken(accessToken) == true but jwtProvider.parseClaims(accessToken) returns Claims without "userId"; call jwtFilter.doFilter(...) and assert that SecurityContextHolder.getContext().getAuthentication() is null, that the response has the UNAUTHORIZED status (or JwtFilter's expected status), and that filterChain.doFilter(...) is not invoked; reference jwtProvider.validateToken, jwtProvider.parseClaims, JwtFilter.doFilter and SecurityContextHolder to locate the code to modify.src/test/java/app/nook/record/service/RecordQueryServiceTest.java (1)
232-241: ⚡ Quick win인덱스 기반 단언이 Emotion 열거형 선언 순서에 의존
get(2),get(5),get(6)방식의 단언은Emotion열거형의 선언 순서가 변경되거나 새 값이 추가될 경우 오탐 또는 잘못된 검증을 유발합니다. 감정 이름 기반으로 조회하는 방식을 권장합니다.♻️ 리팩터링 제안 (인덱스 대신 감정 이름으로 필터링)
-assertThat(result.emotionCounts().get(0).emotion()).isEqualTo("ALL"); -assertThat(result.emotionCounts().get(0).recordCount()).isEqualTo(8L); -assertThat(result.emotionCounts().get(1).emotion()).isEqualTo("FUN"); -assertThat(result.emotionCounts().get(1).recordCount()).isEqualTo(5L); -assertThat(result.emotionCounts().get(2).emotion()).isEqualTo("EMPATHIZING"); -assertThat(result.emotionCounts().get(2).recordCount()).isZero(); -assertThat(result.emotionCounts().get(5).emotion()).isEqualTo("SAD"); -assertThat(result.emotionCounts().get(5).recordCount()).isEqualTo(3L); -assertThat(result.emotionCounts().get(6).emotion()).isEqualTo("UNCOMFORTABLE"); -assertThat(result.emotionCounts().get(6).recordCount()).isZero(); +assertThat(result.emotionCounts().get(0).emotion()).isEqualTo("ALL"); +assertThat(result.emotionCounts().get(0).recordCount()).isEqualTo(8L); +assertThat(result.emotionCounts()) + .filteredOn(item -> item.emotion().equals("FUN")) + .singleElement() + .extracting(BookRecordDto.RecordEmotionDto::recordCount) + .isEqualTo(5L); +assertThat(result.emotionCounts()) + .filteredOn(item -> item.emotion().equals("SAD")) + .singleElement() + .extracting(BookRecordDto.RecordEmotionDto::recordCount) + .isEqualTo(3L); +assertThat(result.emotionCounts()) + .filteredOn(item -> item.emotion().equals("EMPATHIZING")) + .singleElement() + .extracting(BookRecordDto.RecordEmotionDto::recordCount) + .isZero();🤖 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/RecordQueryServiceTest.java` around lines 232 - 241, Replace index-based assertions on result.emotionCounts() (e.g., get(2), get(5), get(6)) with lookups that find entries by emotion name so tests no longer depend on Emotion enum declaration order; for example, locate the element whose emotion() equals "EMPATHIZING"/"SAD"/"UNCOMFORTABLE" and assert its recordCount() (and emotion()) accordingly. Update assertions around result.emotionCounts().get(0) and get(1) similarly to verify by emotion name ("ALL", "FUN") rather than by position. Ensure you use a clear helper or stream filter (e.g., filtering result.emotionCounts() by e -> e.emotion().equals(...)) to find the matching element and fail the test if not present.
🤖 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/domain/enums/Emotion.java`:
- Line 5: RecordQueryServiceTest의 위치 기반 어서션이 현재 Emotion enum 선언 순서와 불일치해 실패합니다;
수정 방법: 테스트 코드(RecordQueryServiceTest)에서 Emotion.values()를 기반으로 하거나 하드코딩된 기대값을 현재
enum 순서로 맞춰 변경하세요 — 즉 enum 선언은 FUN, EMPATHIZING, USEFUL, COMPLICATED, SAD,
UNCOMFORTABLE(EMPTY 제외) 이므로 test의 get(2), get(3), get(4), get(5) 등 위치 기반 기대 문자열을
각각 "EMPATHIZING", "USEFUL", "COMPLICATED", "SAD" 로 업데이트하거나 대신
Emotion.values()[index].name()를 사용하도록 변경해 위치 변화에 강하게 만드세요.
In `@src/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.java`:
- Around line 299-310: The query in RecordQueryRepositoryImpl
(countRecordsByEmotion / the fetch into BookRecordDto.RecordEmotionDto) can
return NULL for record.emotion, causing Emotion.valueOf(item.emotion()) in
RecordQueryService to throw NPE; fix by excluding NULL emotions at query time
(add record.emotion.isNotNull() to the .where(...) clause) so
RecordEmotionDto.emotion() is never null, or if you prefer a defensive approach
adjust RecordQueryService to skip/null-check items before calling
Emotion.valueOf; prefer the query-level change in countRecordsByEmotion().
In `@src/main/java/app/nook/record/service/RecordQueryService.java`:
- Around line 93-110: The code currently excludes Emotion.EMPTY from
normalizedEmotionCounts which makes ALL (response.totalCount()) larger than the
sum of individual emotions; to fix, include EMPTY in the per-emotion list so
sums match: in RecordQueryService change the normalizedEmotionCounts
construction (the stream filtering of Emotion.values()) to include Emotion.EMPTY
(i.e., remove the .filter(emotion -> emotion != Emotion.EMPTY) or explicitly add
a BookRecordDto.RecordEmotionDto for Emotion.EMPTY using
emotionCountMap.getOrDefault(Emotion.EMPTY, 0L)), then keep building
emotionCountsWithAll (with "ALL" using response.totalCount()) so totalCount
equals the sum of the per-emotion entries.
In `@src/main/java/app/nook/user/filter/JwtFilter.java`:
- Around line 43-46: JwtFilter currently only logs expired/invalid token cases
(jwtProvider.isExpiredToken(accessToken)) and continues the filter chain, which
prevents JwtExceptionFilter from returning JWT-specific errors; update JwtFilter
(the doFilter/doFilterInternal branch handling jwtProvider.isExpiredToken(...)
and the invalid token branch) to stop the chain and surface the failure by
either throwing the existing JwtException (or a specific
TokenExpiredException/InvalidTokenException used in the project) or by writing
the JWT error response directly to the HttpServletResponse and returning, so
JwtExceptionFilter can handle or the correct TOKEN_EXPIRED/INVALID_TOKEN
response is returned instead of falling through to generic 401/403 handling.
In `@src/main/java/app/nook/user/service/UserService.java`:
- Around line 101-111: The code stores and reuses the same refreshToken when
issuing a new access token; implement refresh-token rotation by generating a new
refresh token (e.g., via jwtProvider.createRefreshToken or similar) when
reissuing, replace the stored TokenRedis.refreshToken with the newly generated
token in tokenRedisRepository.save (update the existing TokenRedis entry built
by TokenRedis.builder for user.getId()), and return the new refresh token
alongside the new access token in UserDTO.TokenReissueResponse so the client
receives and persists the rotated token; also ensure any logic that checks for
reuse can detect and handle presentation of an old refresh token (invalidate or
log reuse) if present.
---
Outside diff comments:
In `@src/docs/asciidoc/record-query.adoc`:
- Around line 30-33: The documentation is missing the path-parameters include
for the "독서_기록_감정별_개수_조회_성공" snippets so the bookId path parameter documented in
RecordControllerTest.java (the pathParameters snippet) doesn't appear; add an
include line for the path-parameters snippet
(include::{snippets}/record-controller-test/독서_기록_감정별_개수_조회_성공/path-parameters.adoc[opts=optional])
into record-query.adoc in the same place/pattern used by the "특정 책 기록 감정 필터 조회"
section so the bookId path parameter is rendered.
In `@src/main/java/app/nook/user/dto/UserDTO.java`:
- Around line 12-22: LoginResponse currently exposes the sensitive refreshToken
in JSON; remove the refreshToken field from the LoginResponse DTO (class
LoginResponse) and stop returning it in login responses, and instead set the
refresh token as a HttpOnly, Secure, SameSite=Strict cookie on the server; also
update the `/auth/reissue` flow to read the refresh token from the cookie (not
request body) and adjust any methods that previously relied on
LoginResponse.refreshToken to use the cookie-based token retrieval.
In `@src/main/java/app/nook/user/redis/TokenRedis.java`:
- Around line 15-25: The TokenRedis entity uses `@Indexed` on refreshToken and
accessToken which creates secondary index keys that never expire because
keyspace notifications are not enabled; update your Redis repository
configuration (the class with `@EnableRedisRepositories`) to set
enableKeyspaceEvents="notify-keyspace-events" and ensure your application.yml
sets spring.data.redis.notify-keyspace-events=KEA on the Redis server, then add
a Redis keyspace event listener/bean to react to EXPIRATION/DEL events and
remove the corresponding secondary index entries (or alternatively stop indexing
these fields if you prefer no secondary keys); reference TokenRedis, the
`@EnableRedisRepositories` annotation in RedisConfig, and the application.yml
property when making the changes.
---
Nitpick comments:
In `@src/test/java/app/nook/record/service/RecordQueryServiceTest.java`:
- Around line 232-241: Replace index-based assertions on result.emotionCounts()
(e.g., get(2), get(5), get(6)) with lookups that find entries by emotion name so
tests no longer depend on Emotion enum declaration order; for example, locate
the element whose emotion() equals "EMPATHIZING"/"SAD"/"UNCOMFORTABLE" and
assert its recordCount() (and emotion()) accordingly. Update assertions around
result.emotionCounts().get(0) and get(1) similarly to verify by emotion name
("ALL", "FUN") rather than by position. Ensure you use a clear helper or stream
filter (e.g., filtering result.emotionCounts() by e -> e.emotion().equals(...))
to find the matching element and fail the test if not present.
In `@src/test/java/app/nook/user/filter/JwtFilterTest.java`:
- Around line 51-130: Add a test in JwtFilterTest (e.g., method named
유효한_토큰이지만_userId_클레임이_없으면_UNAUTHORIZED를_낸다) that simulates
jwtProvider.validateToken(accessToken) == true but
jwtProvider.parseClaims(accessToken) returns Claims without "userId"; call
jwtFilter.doFilter(...) and assert that
SecurityContextHolder.getContext().getAuthentication() is null, that the
response has the UNAUTHORIZED status (or JwtFilter's expected status), and that
filterChain.doFilter(...) is not invoked; reference jwtProvider.validateToken,
jwtProvider.parseClaims, JwtFilter.doFilter and SecurityContextHolder to locate
the code to modify.
In `@src/test/java/app/nook/user/service/UserServiceTest.java`:
- Around line 194-206: Add a unit test for the branch where the JWT is
syntactically/signature-invalid but not expired: in UserServiceTest add a test
(e.g., reissueAccessToken_유효하지않은토큰_예외) that stubs
jwtProvider.validateToken(refreshToken) to return false and
jwtProvider.isExpiredToken(refreshToken) to return false, then calls
userService.reissueAccessToken(refreshToken) and asserts a CustomException with
error code AuthErrorCode.INVALID_TOKEN; this ensures the throw new
CustomException(AuthErrorCode.INVALID_TOKEN) path in
UserService.reissueAccessToken is exercised.
🪄 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: 5902144b-b787-49e8-bb3d-3e4969cdb545
📒 Files selected for processing (24)
src/docs/asciidoc/record-query.adocsrc/main/java/app/nook/library/domain/Library.javasrc/main/java/app/nook/record/controller/RecordController.javasrc/main/java/app/nook/record/domain/enums/Emotion.javasrc/main/java/app/nook/record/dto/BookRecordDto.javasrc/main/java/app/nook/record/repository/RecordQueryRepository.javasrc/main/java/app/nook/record/repository/RecordQueryRepositoryImpl.javasrc/main/java/app/nook/record/service/RecordCommandService.javasrc/main/java/app/nook/record/service/RecordQueryService.javasrc/main/java/app/nook/user/controller/AuthController.javasrc/main/java/app/nook/user/dto/UserDTO.javasrc/main/java/app/nook/user/filter/JwtFilter.javasrc/main/java/app/nook/user/jwt/JwtProvider.javasrc/main/java/app/nook/user/oauth/OAuthService.javasrc/main/java/app/nook/user/redis/TokenRedis.javasrc/main/java/app/nook/user/redis/TokenRedisRepository.javasrc/main/java/app/nook/user/service/UserService.javasrc/test/java/app/nook/controller/record/RecordControllerTest.javasrc/test/java/app/nook/controller/user/AuthControllerTest.javasrc/test/java/app/nook/record/repository/RecordRepositoryTest.javasrc/test/java/app/nook/record/service/RecordQueryServiceTest.javasrc/test/java/app/nook/record/service/RecordServiceTest.javasrc/test/java/app/nook/user/filter/JwtFilterTest.javasrc/test/java/app/nook/user/service/UserServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/java/app/nook/user/jwt/JwtProvider.java
Contributor
Author
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/app/nook/controller/focus/FocusControllerTest.java (1)
107-129:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLine 129의
endFocus스텁에서eq(1L)로 하드코딩된 userId —@WithCustomUser기본값과의 암묵적 결합
focusService.endFocus(eq(1L), eq(request))로 스텁을 등록하고 있어,@WithCustomUser가 주입하는 userId가 정확히1L이어야만 스텁이 매칭됩니다. Line 75의startFocus스텁은any(User.class)를 사용해 User 객체 전체를 무시하고 매칭되므로, 같은 방식으로endFocus도 개선할 수 있습니다.현재
@WithCustomUser의 기본값이userId = 1L이므로 테스트가 통과하지만, 만약 기본값이 변경되면 matcher가 실패하고 stub은 null을 반환하게 됩니다.🔧 수정 제안
- given(focusService.endFocus(eq(1L), eq(request))).willReturn(response); + given(focusService.endFocus(anyLong(), eq(request))).willReturn(response);또는 명시적 상수로 참조:
+ // WithCustomUser의 기본 userId 값과 일치해야 함 given(focusService.endFocus(eq(1L), eq(request))).willReturn(response);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/app/nook/controller/focus/FocusControllerTest.java` around lines 107 - 129, The stub in FocusControllerTest hardcodes the userId with eq(1L) when stubbing focusService.endFocus, which couples the test to `@WithCustomUser`'s default; change the matcher to a non-specific one (e.g., anyLong() or any(Long.class)) so the stub doesn't rely on a specific userId. Update the given call that references focusService.endFocus(eq(1L), eq(request)) to use focusService.endFocus(anyLong(), eq(request)) (or anyLong() for primitive long / any(Long.class) for Long) to match the startFocus-style flexible matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/java/app/nook/controller/record/RecordControllerTest.java`:
- Around line 654-658: The response-field documentation for result.emotion (in
the test using fieldWithPath("result.emotion")) omits the possible value
"EMPTY"; update the description passed to fieldWithPath("result.emotion") to
list all allowed values including EMPTY (e.g., "기록 감정 값. EMPTY, FUN,
EMPATHIZING, USEFUL, COMPLICATED, SAD, UNCOMFORTABLE 중 하나") so the generated API
docs match the request-side notes and other tests that state "미입력 시 EMPTY로 저장".
- Around line 363-367: 문서의 응답 필드 설명을 명확히 하도록 수정하세요: ApiResponseSnippet 내
fieldWithPath("result.totalCount") 설명에 EMPTY 감정 레코드가 ALL(totalCount)에 포함되지만 개별
감정 버킷에는 포함되지 않는다는 문구를 추가하고, fieldWithPath("result.emotionCounts[].emotion") 설명에는
EMPTY 값이 별도의 집계 버킷으로 제공되지 않음을 명시(요청에서 emotion 미입력 시 EMPTY로 저장됨도 함께 언급). 해당 변경은
RecordControllerTest의 responseFields 블록에 있는 fieldWithPath
항목들("result.totalCount"와 "result.emotionCounts[].emotion")을 수정하여 반영하세요.
---
Outside diff comments:
In `@src/test/java/app/nook/controller/focus/FocusControllerTest.java`:
- Around line 107-129: The stub in FocusControllerTest hardcodes the userId with
eq(1L) when stubbing focusService.endFocus, which couples the test to
`@WithCustomUser`'s default; change the matcher to a non-specific one (e.g.,
anyLong() or any(Long.class)) so the stub doesn't rely on a specific userId.
Update the given call that references focusService.endFocus(eq(1L), eq(request))
to use focusService.endFocus(anyLong(), eq(request)) (or anyLong() for primitive
long / any(Long.class) for Long) to match the startFocus-style flexible
matching.
🪄 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: 2dc0433d-3f65-4e7d-86b1-93a90a2ad768
📒 Files selected for processing (14)
src/docs/asciidoc/record-query.adocsrc/main/java/app/nook/record/controller/RecordController.javasrc/main/java/app/nook/record/repository/RecordRepository.javasrc/main/java/app/nook/record/service/RecordQueryService.javasrc/main/java/app/nook/user/dto/UserDTO.javasrc/main/java/app/nook/user/filter/JwtFilter.javasrc/main/java/app/nook/user/service/UserService.javasrc/test/java/app/nook/controller/focus/FocusControllerTest.javasrc/test/java/app/nook/controller/focus/FocusThemeControllerTest.javasrc/test/java/app/nook/controller/library/LibraryControllerTest.javasrc/test/java/app/nook/controller/record/RecordControllerTest.javasrc/test/java/app/nook/controller/user/AuthControllerTest.javasrc/test/java/app/nook/user/filter/JwtFilterTest.javasrc/test/java/app/nook/user/service/UserServiceTest.java
✅ Files skipped from review due to trivial changes (2)
- src/test/java/app/nook/controller/library/LibraryControllerTest.java
- src/docs/asciidoc/record-query.adoc
🚧 Files skipped from review as they are similar to previous changes (5)
- src/test/java/app/nook/controller/user/AuthControllerTest.java
- src/main/java/app/nook/user/service/UserService.java
- src/main/java/app/nook/record/controller/RecordController.java
- src/test/java/app/nook/user/service/UserServiceTest.java
- src/main/java/app/nook/record/service/RecordQueryService.java
Contributor
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📄 작업 내용 요약
📎 Issue 번호
✅ 작업 목록
📝 기타 참고사항
Summary by CodeRabbit
New Features
Improvements
Documentation / Tests