Skip to content

feat: 정산 리스트 조회시 총 정산 금액 응답에 포함하도록 수정#23

Merged
sudhdkso merged 3 commits intodevelopfrom
feature/get-settlement
Feb 22, 2026
Merged

feat: 정산 리스트 조회시 총 정산 금액 응답에 포함하도록 수정#23
sudhdkso merged 3 commits intodevelopfrom
feature/get-settlement

Conversation

@sudhdkso
Copy link
Contributor

@sudhdkso sudhdkso commented Feb 22, 2026

#️⃣연관된 이슈

x

🔀반영 브랜치

feature/get-settlement -> develop

🔧변경 사항

코드에 추가사항 및 변경사항을 작성해주세요.

💬리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 정산 목록 검색에 limit 파라미터 추가
  • 개선 사항

    • 정산 목록 응답에 총액(totalAmount) 필드 추가
    • API 응답에서 null 필드 제외 처리
    • 정산 및 인증 관련 문서의 참조 경로 수정
    • 검증 오류 메시지 처리 개선

@coderabbitai
Copy link

coderabbitai bot commented Feb 22, 2026

Walkthrough

정산 조회에 총액 계산 기능을 추가하고, JWT 토큰 클레임 추출 방식을 개선하며, 예외 처리 로직을 단순화합니다. 또한 문서 경로를 수정하고 여러 응답 레코드에 필드를 추가하는 변경 사항입니다.

Changes

Cohort / File(s) Summary
Documentation Path Corrections
src/docs/asciidoc/auth.adoc, src/docs/asciidoc/settlement.adoc
파일 경로 수정 (언더스코어 제거: check-auth_-successcheck-auth-success, get-share-link-list_-successget-share-link-list-success) 및 구두점 추가.
JWT Claims Extraction & Response Serialization
src/main/java/com/dnd/moddo/auth/presentation/AuthController.java, src/main/java/com/dnd/moddo/auth/presentation/response/AuthCheckResponse.java
JWT 클레임 직접 추출 방식으로 변경 (claims.get(AUTH_ID, Long.class) 사용) 및 @JsonInclude(NON_NULL) 어노테이션 추가로 null 필드 직렬화 제외.
Exception Handling & Default Sort Logic
src/main/java/com/dnd/moddo/common/exception/GlobalExceptionHandler.java, src/main/java/com/dnd/moddo/event/application/query/QuerySettlementService.java
유효성 검사 예외 메시지 처리 단순화 및 정산 검색에 기본 정렬 타입 폴백 추가.
Settlement Query Enhancement
src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java, src/main/java/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java, src/main/java/com/dnd/moddo/event/presentation/response/SettlementListResponse.java
JPQL을 이용한 정산 총액 계산 로직 추가, limit 필드 및 totalAmount 필드 추가.
Test Coverage & Updates
src/test/java/com/dnd/moddo/domain/.../MemberTest.java, src/test/java/com/dnd/moddo/domain/.../AuthControllerTest.java, src/test/java/com/dnd/moddo/domain/.../SettlementControllerTest.java, src/test/java/com/dnd/moddo/domain/.../QuerySettlementServiceTest.java, src/test/java/com/dnd/moddo/domain/.../SettlementReaderTest.java
사용자 할당 테스트 케이스 추가, 토큰 클레임 기반 검증 업데이트, 정산 목록 응답에 totalAmount 처리 추가, 테스트 메서드명 표준화, null 타임스탐프 처리.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • 진행중인 정산/ 완료 정산 api 추가 #21: 정산 목록/조회 코드(QuerySettlementService, SettlementQueryRepositoryImpl 및 관련 리더/컨트롤러/응답 타입)를 수정하며, 본 PR이 그 PR의 정산 쿼리를 경비 총액 처리 및 응답 필드로 확장합니다.

Poem

🐰 토큰의 비결을 드러내고,
정산액을 더하는 마법을 펼쳐,
테스트는 튼튼히 보강하며,
문서의 오류는 쓸어내니—
나의 발자국처럼 정확하게! 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목이 주요 변경사항을 명확하게 반영하고 있습니다. 정산 리스트 조회시 총 정산 금액을 응답에 포함하는 기능 추가라는 핵심 변경사항이 제목에 잘 나타나 있으며, 제시된 raw_summary의 대부분의 변경사항들(SettlementListResponse에 totalAmount 필드 추가, 쿼리 로직 개선 등)이 이 제목과 일치합니다.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/get-settlement

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.

@github-actions
Copy link

📝 테스트 커버리지 리포트입니다!

File Coverage [97.22%] 🍏
QuerySettlementService.java 97.22% 🍏
Total Project Coverage 65.95%

@sudhdkso sudhdkso merged commit 243dbaf into develop Feb 22, 2026
2 of 3 checks passed
Copy link

@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: 5

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/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java (1)

10-22: ⚠️ Potential issue | 🟠 Major

@NotNull 검증과 서비스 레이어의 null 기본값 처리가 모순됩니다.

status, sort, limit 모두 @NotNull로 선언되어 있지만, QuerySettlementService.search()에서는 각 필드가 null일 때 기본값을 할당하는 로직이 존재합니다. 또한 컨트롤러 테스트의 REST Docs에서도 해당 파라미터들을 .optional()로 문서화하고 있고, search_WhenLimitIsNull_ShouldUseDefaultLimit 테스트는 limitnull을 전달합니다.

두 가지 접근 중 하나를 선택해 일관성을 유지해야 합니다:

  1. @NotNull을 제거하고 서비스 레이어의 기본값 로직을 유지 (유연한 API)
  2. @NotNull을 유지하고 서비스 레이어의 null 체크를 제거 (엄격한 계약)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java`
around lines 10 - 22, The DTO SearchSettlementListRequest currently marks
status, sort and limit as `@NotNull` but QuerySettlementService.search() and
existing tests/docs expect nulls to be allowed; remove the `@NotNull` annotations
from the record fields (status, sort, limit) in SearchSettlementListRequest so
the service's null-defaulting logic remains valid, keep the `@Min/`@Max on limit
(they will only apply when a value is present), and ensure REST Docs/tests
(e.g., search_WhenLimitIsNull_ShouldUseDefaultLimit and the .optional()
parameter docs) remain consistent with this relaxed validation approach.
🧹 Nitpick comments (4)
src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java (1)

127-139: NullPointerException 대신 더 의도가 명확한 예외 타입 사용을 고려하세요.

현재 raw NullPointerException을 기대하는 방식은 구현 세부에 강하게 결합되어 있습니다. 만약 assignUser 내부가 Objects.requireNonNull을 사용한다면 의도적인 선택이지만, 추후 구현이 IllegalArgumentException 등으로 변경될 경우 테스트가 불필요하게 깨집니다. 또한 .hasMessageContaining(...) 없이 예외 타입만 검증하면 실수로 다른 코드 경로에서 발생한 NPE를 통과시킬 수 있습니다.

또한 다른 테스트 메서드와 달리 // given 주석 구조가 누락되어 있어 가독성이 떨어집니다.

♻️ 개선 제안
  `@DisplayName`("null 사용자를 할당하면 예외가 발생한다.")
  `@Test`
  void assignUser_throwException_whenUserIsNull() {
+     // given
      Member member = Member.builder()
          .name("기존이름")
          .settlement(mockSettlement)
          .role(ExpenseRole.PARTICIPANT)
          .isPaid(false)
          .build();
  
+     // when & then
      assertThatThrownBy(() -> member.assignUser(null))
-         .isInstanceOf(NullPointerException.class);
+         .isInstanceOf(NullPointerException.class)
+         .hasMessageContaining("user"); // Objects.requireNonNull(user, "user") 형태 권장
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java` around
lines 127 - 139, 테스트 MemberTest의 assignUser_throwException_whenUserIsNull에서 단순히
NullPointerException만 기대하는 대신 더 명확한 예외 타입과 메시지를 검증하도록 수정하세요: given 블록 주석을 추가해
테스트 구조를 맞추고, Member.builder(...)로 만든 객체에 대해 member.assignUser(null)을 호출할 때 발생하도록
되어 있는 예외를 assignUser 메서드의 요구사항에 맞는 타입(예: IllegalArgumentException 또는 도메인용 커스텀
예외)으로 변경하고
assertThatThrownBy(...).isInstanceOf(...).hasMessageContaining("user") 형태로 예외
메시지도 검증해 다른 경로의 NPE가 통과하지 않게 만드세요.
src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java (2)

92-96: groupBycreatedAt, completedAt 누락 — 데이터베이스 설정에 따라 오류 가능

settlement.createdAtsettlement.completedAtselect에 포함되어 있지만 groupBy에는 없습니다. settlement.id가 PK이므로 MySQL(ONLY_FULL_GROUP_BY + 함수 종속성 감지)과 PostgreSQL에서는 동작하지만, 일부 데이터베이스나 strict 모드 설정에서는 오류가 발생할 수 있습니다.

안전하게 groupBy에 추가하는 것을 권장합니다.

♻️ groupBy에 누락 필드 추가
 			.groupBy(
 				settlement.id,
 				settlement.code,
-				settlement.name
+				settlement.name,
+				settlement.createdAt,
+				settlement.completedAt
 			)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java`
around lines 92 - 96, The query in SettlementQueryRepositoryImpl uses select to
include settlement.createdAt and settlement.completedAt but the groupBy call
only lists settlement.id, settlement.code, settlement.name, which can fail under
strict SQL modes or other DBs; update the groupBy invocation in the method that
builds this query (the groupBy(...) call) to include settlement.createdAt and
settlement.completedAt alongside id, code, and name so all non-aggregated
selected columns are grouped.

129-133: default 분기가 LATEST와 동일하여 중복됩니다.

switch 표현식에서 LATESTdefault가 동일한 결과를 반환합니다. exhaustive switch를 사용하거나, default를 제거하고 모든 enum 값을 명시하면 향후 새 정렬 타입 추가 시 컴파일 타임에 누락을 감지할 수 있습니다.

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

In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java`
around lines 129 - 133, The switch in SettlementQueryRepositoryImpl that returns
sorting for sortType currently has a default branch duplicating LATEST; remove
the default branch and handle every enum constant explicitly (e.g., OLDEST,
LATEST, and any other SortType values) so the switch is exhaustive, or replace
the default with an explicit case or an IllegalArgumentException to ensure the
compiler/reporting catches any newly added enum values; update the switch
expression on sortType (the method returning settlement.createdAt.asc()/desc())
accordingly.
src/main/java/com/dnd/moddo/auth/presentation/response/AuthCheckResponse.java (1)

5-29: UserInfo 중첩 레코드에도 @JsonInclude(NON_NULL) 추가 고려

@JsonInclude(NON_NULL)은 선언된 AuthCheckResponse의 최상위 필드(user, reason)에만 적용되며, 중첩 레코드 UserInfo의 필드(id, role)에는 적용되지 않습니다. AuthController에서 클레임 추출 결과가 null인 경우 user.id 또는 user.role이 null로 직렬화됩니다. 방어적으로 UserInfo에도 어노테이션을 추가하는 것을 고려할 수 있습니다.

♻️ 리팩토링 제안
+import com.fasterxml.jackson.annotation.JsonInclude;
+
 `@JsonInclude`(JsonInclude.Include.NON_NULL)
 public record AuthCheckResponse(...) {
     ...
+    `@JsonInclude`(JsonInclude.Include.NON_NULL)
     public record UserInfo(
         Long id,
         String role
     ) {
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/dnd/moddo/auth/presentation/response/AuthCheckResponse.java`
around lines 5 - 29, The nested UserInfo record's fields aren't covered by the
top-level `@JsonInclude` on AuthCheckResponse, so null id/role will still be
serialized; add `@JsonInclude`(JsonInclude.Include.NON_NULL) to the UserInfo
record declaration (the nested record named UserInfo inside AuthCheckResponse)
so its null fields are omitted during JSON serialization.
🤖 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/auth.adoc`:
- Around line 110-116: 현재 토큰 상태 확인 섹션에서 '=== Example'과 '==== 요청'의 스니펫이 역전되어 포함되어
있으니, auth.adoc의 해당 블록에서
include::{snippets}/auth-controller-test/check-auth-success/http-request.adoc[]를
'==== 요청' 아래로 옮기고
include::{snippets}/auth-controller-test/check-auth-success/curl-request.adoc[]를
'=== Example' 아래로 교체하여 다른 API 섹션과 동일한 규칙(=== Example → curl-request.adoc, ====
요청 → http-request.adoc)을 따르도록 수정하세요.

In `@src/main/java/com/dnd/moddo/auth/presentation/AuthController.java`:
- Around line 105-106: In AuthController, guard the two claims.get(...) calls by
retrieving the id claim as a Number (or Object) first and converting it to Long
(e.g. Number idNum = claims.get(AUTH_ID.getMessage(), Number.class); Long userId
= idNum == null ? null : idNum.longValue()) and by checking the role claim for
null (String role = claims.get(ROLE.getMessage(), String.class)); if either is
null, return/throw an appropriate unauthorized/bad-token response instead of
allowing nulls into the UserInfo record (or provide a safe default), so replace
the existing direct claims.get(...) usage with null checks and Number→Long
conversion in the method that builds the UserInfo in AuthController.

In `@src/main/java/com/dnd/moddo/common/exception/GlobalExceptionHandler.java`:
- Line 75: The handler currently uses e.getMessage() in GlobalExceptionHandler
for MethodArgumentNotValidException which leaks internal Spring details; change
the handling in the catch for MethodArgumentNotValidException (the variable e)
to extract user-friendly messages from the BindingResult: collect
FieldError.getDefaultMessage() for all field errors (or at minimum use the first
FieldError.getDefaultMessage()), and return those messages in the API response
instead of e.getMessage(); locate the MethodArgumentNotValidException handling
branch in GlobalExceptionHandler and replace the direct e.getMessage() usage
with logic that calls e.getBindingResult().getFieldErrors() -> map to
FieldError::getDefaultMessage (or pick the first) and use that string/list in
the response.

In `@src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java`:
- Around line 113-124: The test contains an unused stub: remove the unnecessary
when(secondUser.getName()).thenReturn("두번째") in MemberTest since
assignUser(Member member) throws IllegalStateException before calling
secondUser.getName(); keep creating the secondUser mock but delete the getName()
stubbing so the test only verifies that member.assignUser(secondUser) throws the
expected exception.

In
`@src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java`:
- Line 192: Update the REST Docs description for the totalAmount field in
SettlementControllerTest by fixing the typo: locate the
fieldWithPath("[].totalAmount") call in the test and change its description
string from "총 정산 금" to "총 정산 금액" so the documentation shows the correct label.

---

Outside diff comments:
In
`@src/main/java/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java`:
- Around line 10-22: The DTO SearchSettlementListRequest currently marks status,
sort and limit as `@NotNull` but QuerySettlementService.search() and existing
tests/docs expect nulls to be allowed; remove the `@NotNull` annotations from the
record fields (status, sort, limit) in SearchSettlementListRequest so the
service's null-defaulting logic remains valid, keep the `@Min/`@Max on limit (they
will only apply when a value is present), and ensure REST Docs/tests (e.g.,
search_WhenLimitIsNull_ShouldUseDefaultLimit and the .optional() parameter docs)
remain consistent with this relaxed validation approach.

---

Duplicate comments:
In
`@src/test/java/com/dnd/moddo/domain/settlement/service/QuerySettlementServiceTest.java`:
- Around line 293-322: The test search_WhenLimitIsNull_ShouldUseDefaultLimit
contradicts the `@NotNull` constraint on SearchSettlementListRequest.limit; update
the test to reflect validation rules by removing the null-limit scenario and
replacing it with a valid request (e.g., provide an explicit limit or assert
validation failure), or adjust the request class if you intend null->default
behavior; specifically modify QuerySettlementServiceTest (method
search_WhenLimitIsNull_ShouldUseDefaultLimit) to either pass a non-null limit to
SearchSettlementListRequest or change the expected outcome to a validation error
when invoking querySettlementService.search, and ensure mocks/verify calls
reference settlementReader.findListByUserIdAndStatus with the submitted non-null
limit or that validation is asserted.

---

Nitpick comments:
In
`@src/main/java/com/dnd/moddo/auth/presentation/response/AuthCheckResponse.java`:
- Around line 5-29: The nested UserInfo record's fields aren't covered by the
top-level `@JsonInclude` on AuthCheckResponse, so null id/role will still be
serialized; add `@JsonInclude`(JsonInclude.Include.NON_NULL) to the UserInfo
record declaration (the nested record named UserInfo inside AuthCheckResponse)
so its null fields are omitted during JSON serialization.

In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java`:
- Around line 92-96: The query in SettlementQueryRepositoryImpl uses select to
include settlement.createdAt and settlement.completedAt but the groupBy call
only lists settlement.id, settlement.code, settlement.name, which can fail under
strict SQL modes or other DBs; update the groupBy invocation in the method that
builds this query (the groupBy(...) call) to include settlement.createdAt and
settlement.completedAt alongside id, code, and name so all non-aggregated
selected columns are grouped.
- Around line 129-133: The switch in SettlementQueryRepositoryImpl that returns
sorting for sortType currently has a default branch duplicating LATEST; remove
the default branch and handle every enum constant explicitly (e.g., OLDEST,
LATEST, and any other SortType values) so the switch is exhaustive, or replace
the default with an explicit case or an IllegalArgumentException to ensure the
compiler/reporting catches any newly added enum values; update the switch
expression on sortType (the method returning settlement.createdAt.asc()/desc())
accordingly.

In `@src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java`:
- Around line 127-139: 테스트 MemberTest의
assignUser_throwException_whenUserIsNull에서 단순히 NullPointerException만 기대하는 대신 더
명확한 예외 타입과 메시지를 검증하도록 수정하세요: given 블록 주석을 추가해 테스트 구조를 맞추고, Member.builder(...)로
만든 객체에 대해 member.assignUser(null)을 호출할 때 발생하도록 되어 있는 예외를 assignUser 메서드의 요구사항에
맞는 타입(예: IllegalArgumentException 또는 도메인용 커스텀 예외)으로 변경하고
assertThatThrownBy(...).isInstanceOf(...).hasMessageContaining("user") 형태로 예외
메시지도 검증해 다른 경로의 NPE가 통과하지 않게 만드세요.

Comment on lines +110 to +116
include::{snippets}/auth-controller-test/check-auth-success/http-request.adoc[]

=== HTTP

==== 요청

include::{snippets}/auth-controller-test/check-auth_-success/curl-request.adoc[]
include::{snippets}/auth-controller-test/check-auth-success/curl-request.adoc[]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

=== Example==== 요청 섹션의 스니펫 파일이 다른 섹션과 반대로 포함되어 있습니다

다른 API 섹션(게스트 토큰 발급, 액세스 토큰 재발급 등)은 모두 아래 규칙을 따릅니다:

  • === Examplecurl-request.adoc
  • ==== 요청http-request.adoc

그런데 토큰 상태 확인 섹션(110, 116번 라인)은 반대로 포함되어 있습니다.

📝 수정 제안
 === Example

-include::{snippets}/auth-controller-test/check-auth-success/http-request.adoc[]
+include::{snippets}/auth-controller-test/check-auth-success/curl-request.adoc[]

 === HTTP

 ==== 요청

-include::{snippets}/auth-controller-test/check-auth-success/curl-request.adoc[]
+include::{snippets}/auth-controller-test/check-auth-success/http-request.adoc[]
📝 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.

Suggested change
include::{snippets}/auth-controller-test/check-auth-success/http-request.adoc[]
=== HTTP
==== 요청
include::{snippets}/auth-controller-test/check-auth_-success/curl-request.adoc[]
include::{snippets}/auth-controller-test/check-auth-success/curl-request.adoc[]
include::{snippets}/auth-controller-test/check-auth-success/curl-request.adoc[]
=== HTTP
==== 요청
include::{snippets}/auth-controller-test/check-auth-success/http-request.adoc[]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/docs/asciidoc/auth.adoc` around lines 110 - 116, 현재 토큰 상태 확인 섹션에서 '===
Example'과 '==== 요청'의 스니펫이 역전되어 포함되어 있으니, auth.adoc의 해당 블록에서
include::{snippets}/auth-controller-test/check-auth-success/http-request.adoc[]를
'==== 요청' 아래로 옮기고
include::{snippets}/auth-controller-test/check-auth-success/curl-request.adoc[]를
'=== Example' 아래로 교체하여 다른 API 섹션과 동일한 규칙(=== Example → curl-request.adoc, ====
요청 → http-request.adoc)을 따르도록 수정하세요.

Comment on lines +105 to +106
Long userId = claims.get(AUTH_ID.getMessage(), Long.class);
String role = claims.get(ROLE.getMessage(), String.class);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find build configuration and JJWT version
find . -type f \( -name "*.gradle" -o -name "*.gradle.kts" -o -name "pom.xml" \) | head -20

Repository: moddo-kr/moddo-backend

Length of output: 98


🏁 Script executed:

# Search for JwtProvider class
rg -l "class JwtProvider" --type java

Repository: moddo-kr/moddo-backend

Length of output: 139


🏁 Script executed:

# Search for AuthCheckResponse class
rg -l "class AuthCheckResponse" --type java

Repository: moddo-kr/moddo-backend

Length of output: 48


🏁 Script executed:

# Find the target file to confirm lines 105-106
rg -n "Long userId = claims.get" --type java -A 2 -B 2

Repository: moddo-kr/moddo-backend

Length of output: 593


🏁 Script executed:

# Read build.gradle to find JJWT version
cat build.gradle | head -100

Repository: moddo-kr/moddo-backend

Length of output: 3050


🏁 Script executed:

# Find and read JwtProvider class
cat -n src/main/java/com/dnd/moddo/auth/infrastructure/security/JwtProvider.java

Repository: moddo-kr/moddo-backend

Length of output: 4937


🏁 Script executed:

# Search for AuthCheckResponse
rg -n "class AuthCheckResponse" --type java -A 30

Repository: moddo-kr/moddo-backend

Length of output: 48


🏁 Script executed:

# Search for UserInfo class
rg -n "class UserInfo" --type java -A 20

Repository: moddo-kr/moddo-backend

Length of output: 48


🏁 Script executed:

# Check what happens after lines 105-106 in AuthController
sed -n '105,130p' src/main/java/com/dnd/moddo/auth/presentation/AuthController.java

Repository: moddo-kr/moddo-backend

Length of output: 875


🏁 Script executed:

# Search more broadly for AuthCheckResponse and UserInfo
rg "AuthCheckResponse\|UserInfo" --type java -l

Repository: moddo-kr/moddo-backend

Length of output: 48


🏁 Script executed:

# Look at the full AuthController file around lines 105
sed -n '80,130p' src/main/java/com/dnd/moddo/auth/presentation/AuthController.java

Repository: moddo-kr/moddo-backend

Length of output: 1623


🏁 Script executed:

# Search for response classes
fd -e java -path "*/response/*" | xargs grep -l "AuthCheckResponse\|UserInfo" 2>/dev/null

Repository: moddo-kr/moddo-backend

Length of output: 236


🏁 Script executed:

# Find response classes in the presentation/response directory
find src/main/java/com/dnd/moddo -path "*/response/*" -name "*.java" -type f

Repository: moddo-kr/moddo-backend

Length of output: 2406


🏁 Script executed:

# Search for AuthCheckResponse definition
rg -n "class AuthCheckResponse" --type java -A 50

Repository: moddo-kr/moddo-backend

Length of output: 48


🏁 Script executed:

# Search for UserInfo definition
rg -n "class UserInfo" --type java -A 30

Repository: moddo-kr/moddo-backend

Length of output: 48


🏁 Script executed:

# Read the AuthCheckResponse file directly
cat -n src/main/java/com/dnd/moddo/auth/presentation/response/AuthCheckResponse.java

Repository: moddo-kr/moddo-backend

Length of output: 935


🏁 Script executed:

# Search for UserInfo more broadly
rg "class UserInfo" --type java -B 2 -A 20

Repository: moddo-kr/moddo-backend

Length of output: 48


🏁 Script executed:

# Search for `@JsonInclude` annotations
rg "@JsonInclude" --type java -B 2 -A 5

Repository: moddo-kr/moddo-backend

Length of output: 886


🏁 Script executed:

# Check if there are any null checks or validation in the auth check endpoint
sed -n '90,140p' src/main/java/com/dnd/moddo/auth/presentation/AuthController.java

Repository: moddo-kr/moddo-backend

Length of output: 1496


🏁 Script executed:

# Verify JJWT 0.11.5 behavior with null claims
web_search
JJWT 0.11.5 Claims.get() null handling Integer Long type coercion

Repository: moddo-kr/moddo-backend

Length of output: 267


🌐 Web query:

JJWT 0.11.5 Claims.get() null handling when claim is missing

💡 Result:

In JJWT 0.11.5, Claims is a Map<String,Object>, so claims.get("foo") returns null when the claim is missing (and it will also return null if the claim exists but its value is null, so you can’t distinguish those two cases with get alone). This is consistent with the library’s API contract that claim access returns null “if not present”. [1]

How to handle it:

  • If you need to distinguish “missing” vs “present but null”:

    • Use claims.containsKey("foo") (standard Map), and then claims.get("foo"). (Claims extends Map.) [1]
    • If you’re using the Claims-style API that exposes it, you can also use contains(claimName) which returns false when the claim does not exist. [2]
  • If you want a typed read:
    Use claims.get("foo", MyType.class), which also returns null if not present (and throws if present but of the wrong type). [1]

  • If you want “missing claim” to be an error instead of null:
    Use the parser “require…” methods so parsing fails with MissingClaimException / IncorrectClaimException when expectations aren’t met. [3]

Also note: when building tokens in 0.11.5, setting a claim to null removes it (so “missing” is the normal representation, not “present with null”). [4]

Sources
[1] JJWT Claims Javadoc (0.11.x): typed get(claimName, requiredType) returns null if not present; Claims extends Map https://javadoc.io/static/io.jsonwebtoken/jjwt-api/0.11.2/io/jsonwebtoken/Claims.html
[2] “Returns null if the claim not exist…” / contains(claimName) contract (Claims interface description) https://micronaut-projects.github.io/micronaut-security/2.0.0.RC1/api/io/micronaut/security/oauth2/endpoint/token/response/JWTOpenIdClaims.html
[3] JJWT GitHub docs: using require* methods and missing/incorrect claim exceptions https://github.com/jwtk/jjwt
[4] JwtBuilder.claim(name, value): “A null value will remove the property from the Claims.” https://javadoc.io/static/io.jsonwebtoken/jjwt-api/0.11.5/io/jsonwebtoken/JwtBuilder.html


🌐 Web query:

JJWT Integer to Long type coercion issue JSON parsing

💡 Result:

This happens because JWT claims are JSON, and when JJWT uses Jackson to deserialize the claims into an untyped Map, Jackson uses “natural” number mapping: small integral values become Integer, and only larger ones become Long. Then claims.get("x", Long.class) can fail (often as a RequiredTypeException / ClassCastException) because the stored value is actually an Integer, not a Long. [3], [2]

Fix options

1) Read as Number and convert (most robust)

Number n = claims.get("accountId", Number.class);
long accountId = n.longValue();

2) Configure JJWT’s Jackson ObjectMapper to force longs for ints
Enable Jackson’s USE_LONG_FOR_INTS, and plug that mapper into JJWT’s JacksonDeserializer:

ObjectMapper om = new ObjectMapper()
    .enable(DeserializationFeature.USE_LONG_FOR_INTS);

JwtParser parser = Jwts.parser()
    .json(new JacksonDeserializer(om))
    .build();

JJWT explicitly supports supplying your own ObjectMapper via JacksonDeserializer(ObjectMapper). [1], [2]

3) Store the claim as a string (if you control token creation and want to avoid numeric ambiguity)

claims.put("accountId", Long.toString(accountId));

Sources: JJWT JSON/Jackson customization docs [1], Jackson USE_LONG_FOR_INTS behavior [2], explanation of Integer vs Long claim deserialization causing typed-get failures [3].

References: [1] (github.com) [2] (fasterxml.github.io) [3] (stackoverflow.com)

Citations:


claims.get() 결과에 대한 null 처리 필수

JJWT 0.11.5에서 Claims.get(claimName, requiredType)은 클레임이 존재하지 않으면 null을 반환합니다. 현재 코드는 이 null 값에 대한 처리가 없어, userId나 role이 누락된 경우 다음과 같은 문제가 발생합니다:

  1. 응답에 null 필드 포함: UserInfo 레코드는 @JsonInclude 애너테이션이 없으므로, null 값이 그대로 JSON으로 직렬화됩니다. 결과적으로 {"authenticated": true, "user": {"id": null, "role": null}}이 클라이언트에 전송됩니다.
  2. Integer → Long 변환 문제: Jackson이 JSON의 작은 숫자를 Integer로 역직렬화하면, claims.get(AUTH_ID.getMessage(), Long.class)가 예외를 발생시키거나 null을 반환할 수 있습니다.

null 체크를 추가하세요:

수정 안내
 Claims claims = jwtProvider.parseClaims(token);

 Long userId = claims.get(AUTH_ID.getMessage(), Long.class);
 String role = claims.get(ROLE.getMessage(), String.class);

+if (userId == null || role == null) {
+    return ResponseEntity.ok(
+        AuthCheckResponse.fail(AuthCheckResponse.AuthFailReason.INVALID_TOKEN)
+    );
+}
📝 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.

Suggested change
Long userId = claims.get(AUTH_ID.getMessage(), Long.class);
String role = claims.get(ROLE.getMessage(), String.class);
Long userId = claims.get(AUTH_ID.getMessage(), Long.class);
String role = claims.get(ROLE.getMessage(), String.class);
if (userId == null || role == null) {
return ResponseEntity.ok(
AuthCheckResponse.fail(AuthCheckResponse.AuthFailReason.INVALID_TOKEN)
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/dnd/moddo/auth/presentation/AuthController.java` around
lines 105 - 106, In AuthController, guard the two claims.get(...) calls by
retrieving the id claim as a Number (or Object) first and converting it to Long
(e.g. Number idNum = claims.get(AUTH_ID.getMessage(), Number.class); Long userId
= idNum == null ? null : idNum.longValue()) and by checking the role claim for
null (String role = claims.get(ROLE.getMessage(), String.class)); if either is
null, return/throw an appropriate unauthorized/bad-token response instead of
allowing nulls into the UserInfo record (or provide a safe default), so replace
the existing direct claims.get(...) usage with null checks and Number→Long
conversion in the method that builds the UserInfo in AuthController.

.orElse("잘못된 요청입니다.");
LoggingUtils.warn(e);

String message = e.getMessage();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

e.getMessage()는 Spring 내부 구현 상세를 API 클라이언트에 그대로 노출합니다.

MethodArgumentNotValidException.getMessage()는 다음과 같이 Spring 내부의 장황한 문자열을 반환합니다:

Validation failed for argument [0] in public void ...com.example.Dto with 2 errors:
[Field error in object 'dto' on field 'name': rejected value [null];
codes [NotBlank.dto.name,...]; arguments [...]; default message [이름을 입력해주세요.]] ...

이 문자열에는 완전한 클래스명, 필드명, 제약 코드, 거부된 값 등이 포함되어 API 응답으로 반환되면 내부 모델 구조가 외부에 노출됩니다(정보 누출). 또한 이전 구현에서 사용자 친화적인 메시지(FieldError.getDefaultMessage())를 반환하던 것에서 회귀된 변경입니다.

모든 필드 오류 메시지를 수집하거나, 단일 필드 오류 메시지를 반환하는 방식으로 수정하는 것을 권장합니다.

🛡️ 수정 제안
 	`@ExceptionHandler`(MethodArgumentNotValidException.class)
 	public ResponseEntity<ErrorResponse> handleValidation(
 		MethodArgumentNotValidException e
 	) {
 		LoggingUtils.warn(e);
 
-		String message = e.getMessage();
+		String message = e.getBindingResult().getFieldErrors().stream()
+			.map(fieldError -> fieldError.getField() + ": " + fieldError.getDefaultMessage())
+			.collect(java.util.stream.Collectors.joining(", "));
 
 		return ResponseEntity.badRequest()
 			.body(new ErrorResponse(400, message));
 	}

여러 필드 오류를 모두 반환하기 어렵다면, 첫 번째 필드 오류만 추출하는 방식도 가능합니다:

-		String message = e.getMessage();
+		String message = e.getBindingResult().getFieldErrors().stream()
+			.findFirst()
+			.map(org.springframework.validation.FieldError::getDefaultMessage)
+			.orElse("잘못된 요청입니다.");
📝 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.

Suggested change
String message = e.getMessage();
`@ExceptionHandler`(MethodArgumentNotValidException.class)
public ResponseEntity<ErrorResponse> handleValidation(
MethodArgumentNotValidException e
) {
LoggingUtils.warn(e);
String message = e.getBindingResult().getFieldErrors().stream()
.map(fieldError -> fieldError.getField() + ": " + fieldError.getDefaultMessage())
.collect(java.util.stream.Collectors.joining(", "));
return ResponseEntity.badRequest()
.body(new ErrorResponse(400, message));
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/dnd/moddo/common/exception/GlobalExceptionHandler.java` at
line 75, The handler currently uses e.getMessage() in GlobalExceptionHandler for
MethodArgumentNotValidException which leaks internal Spring details; change the
handling in the catch for MethodArgumentNotValidException (the variable e) to
extract user-friendly messages from the BindingResult: collect
FieldError.getDefaultMessage() for all field errors (or at minimum use the first
FieldError.getDefaultMessage()), and return those messages in the API response
instead of e.getMessage(); locate the MethodArgumentNotValidException handling
branch in GlobalExceptionHandler and replace the direct e.getMessage() usage
with logic that calls e.getBindingResult().getFieldErrors() -> map to
FieldError::getDefaultMessage (or pick the first) and use that string/list in
the response.

Comment on lines +113 to +124
User firstUser = mock(User.class);
User secondUser = mock(User.class);

when(firstUser.getName()).thenReturn("첫번째");
when(secondUser.getName()).thenReturn("두번째");

member.assignUser(firstUser);

// when & then
assertThatThrownBy(() -> member.assignUser(secondUser))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("이미 사용자와 연결된 멤버입니다.");
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find test configuration and MockitoExtension usage
rg -n --type=java "MockitoExtension|STRICT_STUBS|MockitoSettings" -C2

Repository: moddo-kr/moddo-backend

Length of output: 41559


🏁 Script executed:

#!/bin/bash
# Read the test file to verify the code snippet
find . -name "MemberTest.java" -type f | head -5

Repository: moddo-kr/moddo-backend

Length of output: 132


🏁 Script executed:

#!/bin/bash
# Check for test setup configuration files
find . -name "*.properties" -o -name "*.yml" -o -name "*.yaml" | grep -E "(test|config)" | head -20

Repository: moddo-kr/moddo-backend

Length of output: 102


🏁 Script executed:

#!/bin/bash
# Read the MemberTest.java file to verify the code
cat -n ./src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java | sed -n '100,135p'

Repository: moddo-kr/moddo-backend

Length of output: 1218


🏁 Script executed:

#!/bin/bash
# Find the Member entity class
find . -name "Member.java" -type f | grep -E "entity|domain" | grep -v Test

Repository: moddo-kr/moddo-backend

Length of output: 127


🏁 Script executed:

#!/bin/bash
# Check test configuration for Mockito strict stubbing
cat ./src/test/resources/application.yml | head -30

Repository: moddo-kr/moddo-backend

Length of output: 686


🏁 Script executed:

#!/bin/bash
# Read the Member.java file to see assignUser method
cat -n ./src/main/java/com/dnd/moddo/event/domain/member/Member.java | grep -A 20 "assignUser"

Repository: moddo-kr/moddo-backend

Length of output: 695


🏁 Script executed:

#!/bin/bash
# Read MemberTest.java class header to check MockitoExtension
cat -n ./src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java | head -30

Repository: moddo-kr/moddo-backend

Length of output: 1079


🏁 Script executed:

#!/bin/bash
# Search for MockitoSettings or Strictness configuration in the test class
rg -n "MockitoSettings|Strictness" ./src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java

Repository: moddo-kr/moddo-backend

Length of output: 48


불필요한 스텁을 제거하세요.

Line 117의 when(secondUser.getName()).thenReturn("두번째")는 실제로 사용되지 않습니다. assignUser() 메서드는 사용자가 이미 연결되어 있는지 먼저 확인한 후 예외를 던지므로, secondUsergetName()이 호출되기 전에 IllegalStateException이 발생합니다. 해당 스텁은 dead code입니다.

제거 제안
  User firstUser = mock(User.class);
  User secondUser = mock(User.class);
  
  when(firstUser.getName()).thenReturn("첫번째");
- when(secondUser.getName()).thenReturn("두번째");
  
  member.assignUser(firstUser);
📝 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.

Suggested change
User firstUser = mock(User.class);
User secondUser = mock(User.class);
when(firstUser.getName()).thenReturn("첫번째");
when(secondUser.getName()).thenReturn("두번째");
member.assignUser(firstUser);
// when & then
assertThatThrownBy(() -> member.assignUser(secondUser))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("이미 사용자와 연결된 멤버입니다.");
User firstUser = mock(User.class);
User secondUser = mock(User.class);
when(firstUser.getName()).thenReturn("첫번째");
member.assignUser(firstUser);
// when & then
assertThatThrownBy(() -> member.assignUser(secondUser))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("이미 사용자와 연결된 멤버입니다.");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/com/dnd/moddo/domain/Member/entity/MemberTest.java` around
lines 113 - 124, The test contains an unused stub: remove the unnecessary
when(secondUser.getName()).thenReturn("두번째") in MemberTest since
assignUser(Member member) throws IllegalStateException before calling
secondUser.getName(); keep creating the secondUser mock but delete the getName()
stubbing so the test only verifies that member.assignUser(secondUser) throws the
expected exception.

fieldWithPath("[].groupId").description("정산 ID"),
fieldWithPath("[].groupCode").description("정산 코드"),
fieldWithPath("[].name").description("정산 이름"),
fieldWithPath("[].totalAmount").description("총 정산 금"),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

REST Docs 설명 오타: "총 정산 금" → "총 정산 금액"

"총 정산 금"은 "액"이 누락된 것으로 보입니다.

📝 수정 제안
-					fieldWithPath("[].totalAmount").description("총 정산 금"),
+					fieldWithPath("[].totalAmount").description("총 정산 금액"),
📝 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.

Suggested change
fieldWithPath("[].totalAmount").description("총 정산 "),
fieldWithPath("[].totalAmount").description("총 정산 금액"),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java`
at line 192, Update the REST Docs description for the totalAmount field in
SettlementControllerTest by fixing the typo: locate the
fieldWithPath("[].totalAmount") call in the test and change its description
string from "총 정산 금" to "총 정산 금액" so the documentation shows the correct label.

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.

1 participant