Conversation
WalkthroughQueryDSL 자카르타 의존성 및 코드 생성 설정을 추가하고 JPAQueryFactory 빈을 등록했습니다. 사용자별·상태별 정산 조회 기능이 QueryDSL 기반 리포지토리(인터페이스 + impl), Reader, 서비스, 컨트롤러 및 응답 DTO로 구현되었고, 인증(JWT) 검사 엔드포인트와 예외·로깅 핸들러도 확장되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client"
participant Controller as "SettlementController"
participant Service as "QuerySettlementService"
participant Reader as "SettlementReader"
participant RepoImpl as "SettlementQueryRepositoryImpl"
participant JPAQueryFactory as "JPAQueryFactory (QueryDSL)"
participant DB as "Database"
Client->>Controller: GET /api/v1/group/list?status=IN_PROGRESS
Controller->>Service: search(userId, request)
Service->>Service: status == null ? ALL : status
Service->>Reader: findListByUserIdAndStatus(userId, status, sort, limit)
Reader->>RepoImpl: findByUserAndStatus(userId, status, sortType, limit)
RepoImpl->>JPAQueryFactory: build and execute QueryDSL query (joins, aggregates, group by, order by, limit)
JPAQueryFactory->>DB: SQL 실행
DB-->>JPAQueryFactory: 결과 반환
JPAQueryFactory-->>RepoImpl: List<SettlementListResponse>
RepoImpl-->>Reader: 반환된 목록
Reader-->>Service: 반환된 목록
Service-->>Controller: 반환된 목록
Controller-->>Client: HTTP 200 (JSON)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
src/main/java/com/dnd/moddo/event/domain/member/Member.java (1)
45-47:nullable = true는@JoinColumn의 기본값으로 생략 가능
@JoinColumn의nullable기본값은true이므로 명시할 필요가 없습니다.♻️ 제안하는 수정 사항
`@ManyToOne`(fetch = FetchType.LAZY) -@JoinColumn(name = "user_id", nullable = true) +@JoinColumn(name = "user_id") private User user;🤖 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/domain/member/Member.java` around lines 45 - 47, Member 클래스의 user 필드에서 `@JoinColumn`(name = "user_id", nullable = true)처럼 명시된 nullable = true는 기본값이므로 제거하세요: Member 클래스의 private User user 필드 선언부에서 `@JoinColumn` 어노테이션을 `@JoinColumn`(name = "user_id")로 변경해 불필요한 속성을 제거합니다.src/main/java/com/dnd/moddo/event/application/impl/MemberCreator.java (1)
22-38:isPaid초기화 방식 일관성 개선 제안빌더에서
isPaid를 별도로 설정하지 않아 기본값false로 초기화된 후, 곧바로updatePaymentStatus(true)로 덮어쓰고 있습니다. INSERT 경로이므로 동작상 문제는 없지만, 의도를 명확히 하기 위해 빌더에서 직접isPaid(true)를 설정하거나,updatePaymentStatus호출만 유지하는 방향으로 일관성을 맞추는 것이 더 읽기 좋습니다.🤖 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/application/impl/MemberCreator.java` around lines 22 - 38, The createManagerForSettlement method in MemberCreator currently relies on updating isPaid after building the Member (Member.builder(...).build() followed by member.updatePaymentStatus(true)), which is inconsistent; either set isPaid(true) in the builder when constructing the Member (via Member.builder().isPaid(true)...) or remove the builder setting and only call updatePaymentStatus(true) before saving; update the Member construction in createManagerForSettlement to set isPaid to true in the builder (or conversely remove the post-build call) so the intent is explicit, then call memberRepository.save(member).src/test/java/com/dnd/moddo/domain/Member/service/CommandMemberServiceTest.java (1)
103-110:isPaid(true)중복 호출 제거라인 106과 109에서
.isPaid(true)가 중복으로 호출되고 있습니다. Lombok 빌더는 마지막에 설정한 값이 적용되므로 동작 상 문제는 없지만, 불필요한 중복 코드입니다.♻️ 제안하는 수정 사항
Member paidMember = Member.builder() .name("김반숙") .settlement(mockSettlement) .isPaid(true) .profileId(1) .role(ExpenseRole.PARTICIPANT) - .isPaid(true) .build();🤖 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/service/CommandMemberServiceTest.java` around lines 103 - 110, The Member builder for variable paidMember calls .isPaid(true) twice (in the Member.builder() chain); remove the redundant .isPaid(true) so that .isPaid(true) appears only once in the builder chain for paidMember (locate the paidMember declaration using Member.builder() in CommandMemberServiceTest).build.gradle (1)
147-148:$buildDir는 Gradle 8.x에서 deprecated —layout.buildDirectory로 교체 권장.
Project.buildDir속성은 deprecated되었으며, eager API를 사용하고 값이 늦게 변경될 경우 순서 문제가 생길 수 있습니다.Project.layout.buildDirectory로 교체하는 것이 공식 권장 방법입니다.🔧 수정 예시
-def querydslDir = "$buildDir/generated/querydsl" +def querydslDir = layout.buildDirectory.dir("generated/querydsl").get().asFile그리고
sourceSets블록에서도 일관성을 위해:sourceSets { main { java { - srcDirs += querydslDir + srcDirs += [querydslDir] } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle` around lines 147 - 148, The use of "$buildDir" in the QueryDSL declaration (querydslDir) is deprecated; replace it with the lazy API via Project.layout.buildDirectory and use its file/path value to construct the generated QueryDSL directory, and update any matching uses inside the sourceSets block to consistently reference layout.buildDirectory (or its asFile/asPath) instead of buildDir so the value is evaluated lazily and avoids Gradle 8.x deprecation issues.src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java (1)
52-57: 원시 SQL 템플릿 대신 타입 안전한CaseBuilder사용을 권장합니다.
Expressions.numberTemplate에 raw SQL을 문자열로 삽입하면 컴파일 타임 검증이 불가능하고 DB 방언 의존성이 생깁니다. QueryDSL 5.xNumberExpression은sum()메서드를 제공합니다.CaseBuilder를 활용하면 동일한 결과를 타입 안전하게 표현할 수 있습니다.♻️ CaseBuilder로 리팩토링 예시
-import com.querydsl.core.types.dsl.Expressions; +import com.querydsl.core.types.dsl.CaseBuilder; - NumberExpression<Long> completedCount = - Expressions.numberTemplate( - Long.class, - "sum(case when {0} = true then 1 else 0 end)", - member.isPaid - ); + NumberExpression<Long> completedCount = + new CaseBuilder() + .when(member.isPaid.isTrue()) + .then(1L) + .otherwise(0L) + .sum();🤖 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 52 - 57, The current completedCount uses Expressions.numberTemplate with raw SQL; replace it with a type-safe QueryDSL CaseBuilder expression and QueryDSL's sum to avoid dialect-specific templates. Locate the completedCount declaration in SettlementQueryRepositoryImpl (the NumberExpression<Long> completedCount using Expressions.numberTemplate and member.isPaid) and build a CaseBuilder that yields 1 when member.isPaid.isTrue() and 0 otherwise, then call .sum() (or wrap with NumberExpression.sum) to produce the aggregated Long expression; update any references that expect completedCount accordingly.src/main/java/com/dnd/moddo/event/domain/settlement/type/SettlementStatus.java (1)
3-7: LGTM. 다만ALL이 정산 상태값이 아닌 "필터 없음"을 의미하는 센티넬 값임이 코드에서 명확하지 않습니다. Javadoc이나 주석으로 의도를 표시해 두면 향후 혼동을 줄일 수 있습니다. 또한 Spring MVC의 기본 enum 바인딩은 대소문자를 구분하므로, 클라이언트가 소문자(in_progress)로 요청을 보내면 400 오류가 발생합니다.@JsonCreator또는Converter를 추가하는 것을 고려하세요.🤖 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/domain/settlement/type/SettlementStatus.java` around lines 3 - 7, The enum SettlementStatus currently includes ALL as a sentinel "no filter" value but lacks documentation and is vulnerable to client binding errors due to Spring/JSON case-sensitive enum binding; add a Javadoc comment above the SettlementStatus enum explaining that ALL is a sentinel meaning "no filter" (not an actual settlement state) and implement robust deserialization/binding for the enum by adding a `@JsonCreator` static factory on SettlementStatus that accepts a String (normalizes case and common formats like snake_case/kebab_case) and maps to the correct constant, or alternatively add a Spring Converter< String, SettlementStatus> to handle client inputs such as "in_progress" -> IN_PROGRESS; ensure the factory/converter throws a clear IllegalArgumentException for unknown values.src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java (1)
128-174:status파라미터 미전달(null) 및 잘못된 값 케이스에 대한 테스트가 없습니다.현재
IN_PROGRESS정상 경로만 검증되어 있습니다. REST Docs 문서에는status가 optional로 명시되어 있으므로, 아래 시나리오도 추가하는 것을 권장합니다:
status파라미터 생략 시 (?status없음) → 컨트롤러가null을 서비스에 전달하여ALL로 처리되는지 확인- 유효하지 않은 enum 값 (
?status=INVALID) → 400 응답 반환 여부 확인♻️ 추가 테스트 케이스 예시
`@Test` `@DisplayName`("status 파라미터 없이 요청하면 전체 정산을 조회한다.") void searchSettlementList_WhenNoStatus_ShouldReturnAll() throws Exception { given(loginUserArgumentResolver.supportsParameter(any())).willReturn(true); given(loginUserArgumentResolver.resolveArgument(any(), any(), any(), any())) .willReturn(new LoginUserInfo(1L, "USER")); given(querySettlementService.search(1L, null)) .willReturn(List.of()); mockMvc.perform(get("/api/v1/group/list")) .andExpect(status().isOk()); verify(querySettlementService).search(1L, null); } `@Test` `@DisplayName`("유효하지 않은 status 값을 전달하면 400을 반환한다.") void searchSettlementList_WhenInvalidStatus_ShouldReturn400() throws Exception { mockMvc.perform(get("/api/v1/group/list") .param("status", "INVALID_VALUE")) .andExpect(status().isBadRequest()); }🤖 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` around lines 128 - 174, Add two tests in SettlementControllerTest: one that omits the status param and asserts the controller calls querySettlementService.search(1L, null) and returns 200 (e.g., new test method searchSettlementList_WhenNoStatus_ShouldReturnAll), and one that sends an invalid status value and expects 400 (e.g., searchSettlementList_WhenInvalidStatus_ShouldReturn400). Reuse the loginUserArgumentResolver setup used in searchSettlementList for the first test, stub given(querySettlementService.search(1L, null)).willReturn(List.of()), and in the invalid-value test simply perform get("/api/v1/group/list").param("status","INVALID_VALUE").andExpect(status().isBadRequest()); also verify(querySettlementService).search(1L, null) in the no-status test.src/main/java/com/dnd/moddo/event/application/impl/SettlementReader.java (1)
28-48:@Transactional(readOnly = true)적용 범위가 일관되지 않습니다.이번 PR에서
findIdByGroupCode에@Transactional(readOnly = true)이 추가되었으나, 동일한 읽기 전용 메서드인read(),findBySettlement(),findByHeader()에는 적용되지 않았습니다. 특히findByHeader()는 두 레포지토리를 순차 호출하므로, 트랜잭션 없이는 두 호출 사이에 데이터 변경이 발생할 경우 불일관된 응답을 반환할 수 있습니다.클래스 레벨에
@Transactional(readOnly = true)를 선언하거나, 최소한 모든 읽기 메서드에 일관되게 적용하는 것이 좋습니다.♻️ 일관된 트랜잭션 적용 제안
`@Service` `@RequiredArgsConstructor` +@Transactional(readOnly = true) public class SettlementReader { public Settlement read(Long settlementId) { ... } public List<Member> findBySettlement(Long settlementId) { ... } public SettlementHeaderResponse findByHeader(Long settlementId) { ... } - `@Transactional`(readOnly = true) public Long findIdByGroupCode(String code) { ... } - `@Transactional`(readOnly = true) public List<SettlementListResponse> findListByUserIdAndStatus(Long userId, SettlementStatus status) { ... } }🤖 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/application/impl/SettlementReader.java` around lines 28 - 48, The read-only transaction annotation is applied only to findIdByGroupCode but not to other read methods, causing potential inconsistency between repository calls in read(), findBySettlement(), and findByHeader(); update the class-level annotation by adding `@Transactional`(readOnly = true) to the SettlementReader class (or, if you prefer method-level, add `@Transactional`(readOnly = true) to read, findBySettlement, and findByHeader) so all read-only repository interactions (settlementRepository, memberRepository, expenseRepository calls used in read, findBySettlement, findByHeader) run under a consistent read-only transaction; ensure any write methods (if present) explicitly declare `@Transactional` without readOnly to avoid affecting them.src/main/java/com/dnd/moddo/event/presentation/SettlementController.java (1)
91-91:ResponseEntity<?>대신 명시적 타입을 사용하세요.와일드카드 타입은 컴파일 타임 타입 안전성을 잃게 하고, API 의도를 모호하게 만듭니다.
♻️ 타입 명시 제안
- public ResponseEntity<?> search( + public ResponseEntity<List<SettlementListResponse>> search(🤖 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/SettlementController.java` at line 91, The method signature in SettlementController.search currently returns a raw ResponseEntity<?>; replace the wildcard with the concrete DTO/collection type that the endpoint actually returns (e.g. ResponseEntity<SettlementResponse> or ResponseEntity<List<SettlementResponse>> / a wrapper DTO), update the method signature in search and any related return statements to use that explicit type, add or adjust the necessary imports, and ensure controllers/tests/clients expecting the endpoint are updated to the explicit response type to restore compile-time type safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle`:
- Around line 107-113: The Jacoco filtering currently uses fileTree(dir: it)
inside afterEvaluate (where it is the Project), causing the project root to be
scanned; change the Jacoco classDirectories filtering to operate on the compiled
class outputs instead of using fileTree(dir: it) — e.g. update the task that
references classDirectories so you call
classDirectories.setFrom(classDirectories.files.collect { fileTree(dir:
it).matching { include '**/application/**'; include '**/domain/**'; exclude
'**/domain/**/exception/**'; exclude '**/domain/**/type/**' } }) (i.e., filter
the actual class output dirs rather than the Project root). Also replace
deprecated $buildDir usage with the Gradle 8+ API (use
layout.buildDirectory.dir("generated/querydsl").get().asFile) where the code
references buildDir to ensure compatibility with Gradle 8/9.
In `@src/main/java/com/dnd/moddo/event/domain/member/Member.java`:
- Around line 72-78: The assignUser method on Member is unused and incorrectly
attempts to mutate the name field (this.name = user.getName()) which won't be
persisted because name is mapped with updatable = false; remove the this.name
assignment from Member.assignUser and either keep assignUser only to set
this.user (with the existing null-check and exception) or consider deleting the
entire assignUser method if it truly has no callers across the codebase; update
Member.assignUser accordingly to avoid attempting to modify the non-updatable
name field.
In
`@src/main/java/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java`:
- Around line 3-6: SearchSettlementListRequest is an empty record and should be
removed or populated; either delete the unused record class
SearchSettlementListRequest if the controller reads SettlementStatus directly
from query parameters, or add a field such as a SettlementStatus status (and
related constructor/getter is implicit in the record) to this record so it
actually carries the request parameter expected by the controller; update any
usages of SearchSettlementListRequest (e.g., controller method signatures) to
match the chosen approach.
In
`@src/main/java/com/dnd/moddo/event/presentation/response/SettlementListResponse.java`:
- Around line 1-10: SettlementListResponse is placed in presentation.response
but is used as an SQL projection type by SettlementQueryRepository and
SettlementQueryRepositoryImpl, creating an infrastructure→presentation
dependency; move the record SettlementListResponse into an application-level DTO
package (e.g. com.dnd.moddo.event.application.dto) and update
SettlementQueryRepository and SettlementQueryRepositoryImpl to reference the new
type, then perform mapping from that application DTO to the presentation
response in the service or controller layer (create a mapper in the
service/controller to convert the application DTO to the presentation response).
In `@src/main/java/com/dnd/moddo/event/presentation/SettlementController.java`:
- Line 93: The controller accepts a SettlementStatus query param
(SettlementController -> `@RequestParam` SettlementStatus status) but invalid enum
values cause Spring to throw MethodArgumentTypeMismatchException which isn't
handled; add an `@ExceptionHandler`(MethodArgumentTypeMismatchException.class)
method in GlobalExceptionHandler that catches this exception, returns a 400 Bad
Request with a clear error message indicating the invalid enum value and
expected SettlementStatus values, and ensure you import
org.springframework.web.method.annotation.MethodArgumentTypeMismatchException
and construct the response payload consistent with existing Moddo error
responses.
---
Duplicate comments:
In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepository.java`:
- Around line 5-6: SettlementQueryRepository currently imports
SettlementListResponse from the presentation.response package, which violates
layered architecture; move the DTO SettlementListResponse into the application
layer (e.g., application.response or application.dto) and update
SettlementListResponse's package declaration, then update all references and
imports in SettlementQueryRepository to the new package so the repository
depends only on domain/application types (ensure any mapper or converter used by
methods in SettlementQueryRepository that referenced SettlementListResponse is
updated to construct the relocated DTO).
---
Nitpick comments:
In `@build.gradle`:
- Around line 147-148: The use of "$buildDir" in the QueryDSL declaration
(querydslDir) is deprecated; replace it with the lazy API via
Project.layout.buildDirectory and use its file/path value to construct the
generated QueryDSL directory, and update any matching uses inside the sourceSets
block to consistently reference layout.buildDirectory (or its asFile/asPath)
instead of buildDir so the value is evaluated lazily and avoids Gradle 8.x
deprecation issues.
In `@src/main/java/com/dnd/moddo/event/application/impl/MemberCreator.java`:
- Around line 22-38: The createManagerForSettlement method in MemberCreator
currently relies on updating isPaid after building the Member
(Member.builder(...).build() followed by member.updatePaymentStatus(true)),
which is inconsistent; either set isPaid(true) in the builder when constructing
the Member (via Member.builder().isPaid(true)...) or remove the builder setting
and only call updatePaymentStatus(true) before saving; update the Member
construction in createManagerForSettlement to set isPaid to true in the builder
(or conversely remove the post-build call) so the intent is explicit, then call
memberRepository.save(member).
In `@src/main/java/com/dnd/moddo/event/application/impl/SettlementReader.java`:
- Around line 28-48: The read-only transaction annotation is applied only to
findIdByGroupCode but not to other read methods, causing potential inconsistency
between repository calls in read(), findBySettlement(), and findByHeader();
update the class-level annotation by adding `@Transactional`(readOnly = true) to
the SettlementReader class (or, if you prefer method-level, add
`@Transactional`(readOnly = true) to read, findBySettlement, and findByHeader) so
all read-only repository interactions (settlementRepository, memberRepository,
expenseRepository calls used in read, findBySettlement, findByHeader) run under
a consistent read-only transaction; ensure any write methods (if present)
explicitly declare `@Transactional` without readOnly to avoid affecting them.
In `@src/main/java/com/dnd/moddo/event/domain/member/Member.java`:
- Around line 45-47: Member 클래스의 user 필드에서 `@JoinColumn`(name = "user_id",
nullable = true)처럼 명시된 nullable = true는 기본값이므로 제거하세요: Member 클래스의 private User
user 필드 선언부에서 `@JoinColumn` 어노테이션을 `@JoinColumn`(name = "user_id")로 변경해 불필요한 속성을
제거합니다.
In
`@src/main/java/com/dnd/moddo/event/domain/settlement/type/SettlementStatus.java`:
- Around line 3-7: The enum SettlementStatus currently includes ALL as a
sentinel "no filter" value but lacks documentation and is vulnerable to client
binding errors due to Spring/JSON case-sensitive enum binding; add a Javadoc
comment above the SettlementStatus enum explaining that ALL is a sentinel
meaning "no filter" (not an actual settlement state) and implement robust
deserialization/binding for the enum by adding a `@JsonCreator` static factory on
SettlementStatus that accepts a String (normalizes case and common formats like
snake_case/kebab_case) and maps to the correct constant, or alternatively add a
Spring Converter< String, SettlementStatus> to handle client inputs such as
"in_progress" -> IN_PROGRESS; ensure the factory/converter throws a clear
IllegalArgumentException for unknown values.
In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java`:
- Around line 52-57: The current completedCount uses Expressions.numberTemplate
with raw SQL; replace it with a type-safe QueryDSL CaseBuilder expression and
QueryDSL's sum to avoid dialect-specific templates. Locate the completedCount
declaration in SettlementQueryRepositoryImpl (the NumberExpression<Long>
completedCount using Expressions.numberTemplate and member.isPaid) and build a
CaseBuilder that yields 1 when member.isPaid.isTrue() and 0 otherwise, then call
.sum() (or wrap with NumberExpression.sum) to produce the aggregated Long
expression; update any references that expect completedCount accordingly.
In `@src/main/java/com/dnd/moddo/event/presentation/SettlementController.java`:
- Line 91: The method signature in SettlementController.search currently returns
a raw ResponseEntity<?>; replace the wildcard with the concrete DTO/collection
type that the endpoint actually returns (e.g. ResponseEntity<SettlementResponse>
or ResponseEntity<List<SettlementResponse>> / a wrapper DTO), update the method
signature in search and any related return statements to use that explicit type,
add or adjust the necessary imports, and ensure controllers/tests/clients
expecting the endpoint are updated to the explicit response type to restore
compile-time type safety.
In
`@src/test/java/com/dnd/moddo/domain/Member/service/CommandMemberServiceTest.java`:
- Around line 103-110: The Member builder for variable paidMember calls
.isPaid(true) twice (in the Member.builder() chain); remove the redundant
.isPaid(true) so that .isPaid(true) appears only once in the builder chain for
paidMember (locate the paidMember declaration using Member.builder() in
CommandMemberServiceTest).
In
`@src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java`:
- Around line 128-174: Add two tests in SettlementControllerTest: one that omits
the status param and asserts the controller calls
querySettlementService.search(1L, null) and returns 200 (e.g., new test method
searchSettlementList_WhenNoStatus_ShouldReturnAll), and one that sends an
invalid status value and expects 400 (e.g.,
searchSettlementList_WhenInvalidStatus_ShouldReturn400). Reuse the
loginUserArgumentResolver setup used in searchSettlementList for the first test,
stub given(querySettlementService.search(1L, null)).willReturn(List.of()), and
in the invalid-value test simply perform
get("/api/v1/group/list").param("status","INVALID_VALUE").andExpect(status().isBadRequest());
also verify(querySettlementService).search(1L, null) in the no-status test.
src/main/java/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java
Outdated
Show resolved
Hide resolved
Comment on lines
+1
to
+10
| package com.dnd.moddo.event.presentation.response; | ||
|
|
||
| public record SettlementListResponse( | ||
| Long groupId, | ||
| String groupCode, | ||
| String name, | ||
| Long totalMemberCount, | ||
| Long completedMemberCount | ||
|
|
||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
레이어 아키텍처 위반 — DTO를 적절한 패키지로 이동해야 합니다.
SettlementListResponse가 presentation.response 패키지에 위치하지만, SettlementQueryRepository(infrastructure)와 SettlementQueryRepositoryImpl에서 SQL 프로젝션 결과 타입으로 직접 사용됩니다. 이는 infrastructure → presentation 방향의 의존성을 만들어 레이어 규칙을 위반합니다.
이 타입을 application 또는 domain 계층의 패키지(예: com.dnd.moddo.event.application.dto)로 이동하고, 필요하다면 프레젠테이션 응답으로의 매핑을 서비스나 컨트롤러 계층에서 처리하는 것을 권장합니다.
🤖 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/response/SettlementListResponse.java`
around lines 1 - 10, SettlementListResponse is placed in presentation.response
but is used as an SQL projection type by SettlementQueryRepository and
SettlementQueryRepositoryImpl, creating an infrastructure→presentation
dependency; move the record SettlementListResponse into an application-level DTO
package (e.g. com.dnd.moddo.event.application.dto) and update
SettlementQueryRepository and SettlementQueryRepositoryImpl to reference the new
type, then perform mapping from that application DTO to the presentation
response in the service or controller layer (create a mapper in the
service/controller to convert the application DTO to the presentation response).
src/main/java/com/dnd/moddo/event/presentation/SettlementController.java
Outdated
Show resolved
Hide resolved
📝 테스트 커버리지 리포트입니다!
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
build.gradle (1)
148-150:tasks.withType(JavaCompile)범위가 불필요하게 넓음 —compileJava로 한정현재 설정은
compileTestJava등 모든JavaCompile태스크에generatedSourceOutputDirectory를 적용합니다. QueryDSL Q타입 생성은 메인 소스 컴파일(compileJava) 시에만 필요하므로 범위를 한정하는 것이 명확합니다.♻️ 수정 제안
-tasks.withType(JavaCompile).configureEach { - options.generatedSourceOutputDirectory = file(querydslDir) -} +tasks.named('compileJava') { + options.generatedSourceOutputDirectory = file(querydslDir) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle` around lines 148 - 150, The current tasks.withType(JavaCompile).configureEach { options.generatedSourceOutputDirectory = file(querydslDir) } is too broad; restrict this configuration to the main compilation task by applying the generatedSourceOutputDirectory only to the compileJava task. Locate the tasks.withType(JavaCompile) usage and replace it so the compileJava task (reference: compileJava, generatedSourceOutputDirectory, querydslDir, and JavaCompile) is configured directly (e.g., configure the named/typed compileJava task) to set options.generatedSourceOutputDirectory = file(querydslDir).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle`:
- Around line 82-86: Update the QueryDSL dependency and stop using the
deprecated $buildDir: replace the dependency string
'com.querydsl:querydsl-jpa:5.0.0:jakarta' (and matching
'com.querydsl:querydsl-apt:5.0.0:jakarta' annotationProcessor) with a supported
version (e.g., 'com.querydsl:querydsl-jpa:5.1.0' /
'com.querydsl:querydsl-apt:5.1.0') or switch to the OpenFeign fork
'io.github.openfeign.querydsl:querydsl-jpa:7.1' as suggested; and locate any
uses of the deprecated '$buildDir' and replace them with Gradle 8-compatible API
such as layout.buildDirectory.get().asFile or the buildDir property where
appropriate to avoid deprecation warnings.
---
Duplicate comments:
In `@build.gradle`:
- Line 146: Replace the deprecated "$buildDir/generated/querydsl" usage in the
querydslDir definition with the Gradle 8+ API: use
layout.buildDirectory.dir("generated/querydsl").get().asFile (or
.get().asFile.path if a string is required) so change def querydslDir =
"$buildDir/generated/querydsl" to def querydslDir =
layout.buildDirectory.dir("generated/querydsl").get().asFile (or .path) to avoid
the deprecated $buildDir and future breakage.
---
Nitpick comments:
In `@build.gradle`:
- Around line 148-150: The current tasks.withType(JavaCompile).configureEach {
options.generatedSourceOutputDirectory = file(querydslDir) } is too broad;
restrict this configuration to the main compilation task by applying the
generatedSourceOutputDirectory only to the compileJava task. Locate the
tasks.withType(JavaCompile) usage and replace it so the compileJava task
(reference: compileJava, generatedSourceOutputDirectory, querydslDir, and
JavaCompile) is configured directly (e.g., configure the named/typed compileJava
task) to set options.generatedSourceOutputDirectory = file(querydslDir).
📝 테스트 커버리지 리포트입니다!
|
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (11)
src/main/java/com/dnd/moddo/common/logging/LoggingUtils.java (1)
12-25:warn메서드의 중복 코드 — 제네릭 메서드로 통합을 고려해 주세요.
ModdoException,MethodArgumentNotValidException,MethodArgumentTypeMismatchException세warn오버로드가 모두 동일한 로직(getExceptionMessage+log.warn)을 수행합니다. 새 예외 타입이 추가될 때마다 복사-붙여넣기가 반복되므로 하나의 메서드로 통합할 수 있습니다.♻️ 통합 제안
- public static void warn(ModdoException exception) { - String message = getExceptionMessage(exception.getMessage()); - log.warn(message + "\n \t {}", exception); - } - - public static void warn(MethodArgumentNotValidException exception) { - String message = getExceptionMessage(exception.getMessage()); - log.warn(message + "\n \t {}", exception); - } - - public static void warn(MethodArgumentTypeMismatchException exception) { - String message = getExceptionMessage(exception.getMessage()); - log.warn(message + "\n \t {}", exception); - } + public static void warn(Exception exception) { + String message = getExceptionMessage(exception.getMessage()); + log.warn(message + "\n \t {}", exception); + }🤖 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/logging/LoggingUtils.java` around lines 12 - 25, The three overloaded warn methods (warn(ModdoException), warn(MethodArgumentNotValidException), warn(MethodArgumentTypeMismatchException)) duplicate the same logic; replace them with a single generic warn method (e.g., warn(Throwable exception) or warn(Exception exception)) that calls getExceptionMessage(exception.getMessage()) and then log.warn(message + "\n \t {}", exception), and remove the three specific overloads; update any callers if they rely on the specific signatures to use the generic warn and keep getExceptionMessage and log.warn usages intact.src/main/java/com/dnd/moddo/auth/infrastructure/security/JwtProvider.java (2)
100-112:validate()메서드가 현재 사용되지 않고 있습니다.
AuthController.checkAuth()에서parseClaims()를 직접 호출하고 있어validate()메서드가 사용되지 않습니다. 의도적으로 남겨둔 것이라면, 컨트롤러에서validate()를 사용하도록 통일하거나, 사용 계획이 없다면 제거를 검토해 주세요.🤖 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/infrastructure/security/JwtProvider.java` around lines 100 - 112, The JwtProvider.validate(String token) method is unused because AuthController.checkAuth() calls parseClaims() directly; either replace the direct parseClaims() call in AuthController.checkAuth() with a call to JwtProvider.validate(token) to centralize validation logic (preserving the existing exception mapping in validate), or delete JwtProvider.validate(String token) if you do not plan to use it; update imports/tests accordingly and ensure AuthController continues to handle exceptions the same way when switching to validate.
117-128:getUserId()와getRole()호출 시 토큰이 매번 재파싱됩니다.
getUserId()와getRole()이 각각 내부적으로parseClaims()를 호출하므로, 두 값을 모두 필요로 하는 호출자(예:AuthController.checkAuth)는 토큰을 불필요하게 여러 번 파싱하게 됩니다.Claims객체를 인자로 받는 오버로드를 제공하면 중복 파싱을 방지할 수 있습니다.♻️ Claims를 인자로 받는 메서드 추가 제안
public Long getUserId(String token) { - Claims claims = parseClaims(token); - return claims.get(AUTH_ID.getMessage(), Long.class); + return getUserId(parseClaims(token)); + } + + public Long getUserId(Claims claims) { + return claims.get(AUTH_ID.getMessage(), Long.class); } public String getRole(String token) { - Claims claims = parseClaims(token); - return claims.get(ROLE.getMessage(), String.class); + return getRole(parseClaims(token)); + } + + public String getRole(Claims claims) { + return claims.get(ROLE.getMessage(), String.class); }🤖 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/infrastructure/security/JwtProvider.java` around lines 117 - 128, Add overloads for getUserId(Claims claims) and getRole(Claims claims) and update callers to parse the token only once: call parseClaims(token) in the caller (e.g., AuthController.checkAuth), then pass that Claims instance to the new getUserId(Claims) and getRole(Claims) methods instead of calling the token-based getters; keep existing getUserId(String) and getRole(String) for backward compatibility by delegating them to the new overloads (i.e., parseClaims(token) -> getUserId(claims)/getRole(claims)).src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepository.java (1)
18-18:findBySettlementList메서드 이름이 의도를 명확히 전달하지 못합니다."by settlement list로 조회"로 읽혀 혼동을 줍니다.
SettlementReader의 위임 메서드 이름(findShareListByUserId)과 일치시키는 것이 좋습니다.✏️ 이름 변경 제안
- List<SettlementShareResponse> findBySettlementList(Long userId); + List<SettlementShareResponse> findShareListByUserId(Long userId);🤖 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/SettlementQueryRepository.java` at line 18, Rename the repository method List<SettlementShareResponse> findBySettlementList(Long userId) to match the delegation name used in SettlementReader: findShareListByUserId(Long userId); update the method declaration in SettlementQueryRepository and any implementing classes and all callers to use findShareListByUserId so names are consistent with SettlementReader::findShareListByUserId and avoid confusion.src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java (2)
112-123:getOrderSpecifier의memberCount,completedCount파라미터가 사용되지 않습니다.두 파라미터가 메서드 시그니처에 선언되어 있으나 본문에서 전혀 참조되지 않는 데드 코드입니다. 제거하여 호출부도 함께 정리하는 것이 좋습니다.
♻️ 리팩터링 제안
private OrderSpecifier<?> getOrderSpecifier( SettlementSortType sortType, - QSettlement settlement, - NumberExpression<Long> memberCount, - NumberExpression<Long> completedCount + QSettlement settlement ) { // 호출부 - getOrderSpecifier(sortType, settlement, memberCount, completedCount); + getOrderSpecifier(sortType, settlement);그리고
switch의default브랜치는OLDEST/LATEST두 케이스가 이미 열거형 값을 모두 커버하므로 실행 불가 코드입니다.default를 제거하면 향후 새 열거형 값 추가 시 컴파일 에러로 누락을 방지할 수 있습니다.♻️ switch 수정 제안
return switch (sortType) { case OLDEST -> settlement.createdAt.asc(); case LATEST -> settlement.createdAt.desc(); - default -> settlement.createdAt.desc(); };🤖 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 112 - 123, Remove the unused parameters memberCount and completedCount from the getOrderSpecifier method signature and all its call sites (the method is currently declared as getOrderSpecifier(SettlementSortType sortType, QSettlement settlement, NumberExpression<Long> memberCount, NumberExpression<Long> completedCount)); then simplify the switch over SettlementSortType to only handle the explicit enum cases (OLDEST -> settlement.createdAt.asc(), LATEST -> settlement.createdAt.desc()) and remove the unreachable default branch so the compiler will force handling of any new enum values in the future.
57-62:completedCount계산에 DB 방언 의존적인 네이티브 SQL 템플릿이 사용되고 있습니다.
Expressions.numberTemplate에"sum(case when {0} = true then 1 else 0 end)"문자열을 직접 사용하면 데이터베이스 방언에 따라 동작이 달라질 수 있습니다. QueryDSL의CaseBuilder를 사용하면 타입 안전하고 방언 독립적으로 작성할 수 있습니다.♻️ 리팩터링 제안
- NumberExpression<Long> completedCount = - Expressions.numberTemplate( - Long.class, - "sum(case when {0} = true then 1 else 0 end)", - member.isPaid - ); + NumberExpression<Long> completedCount = + new com.querydsl.core.types.dsl.CaseBuilder() + .when(member.isPaid.isTrue()) + .then(1L) + .otherwise(0L) + .sum();🤖 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 57 - 62, The current completedCount uses a DB-dialect-dependent native template; replace it with a QueryDSL CaseBuilder to be dialect-independent: create a NumberExpression (e.g., paidFlag) via new CaseBuilder().when(member.isPaid).then(1).otherwise(0), then use paidFlag.sum() (and cast to Long if needed) to assign to completedCount in SettlementQueryRepositoryImpl, replacing the Expressions.numberTemplate usage.src/main/java/com/dnd/moddo/event/application/impl/SettlementReader.java (1)
47-62:@Transactional(readOnly = true)적용이 일관되지 않습니다.이번 PR에서
findIdByGroupCode및 신규 메서드 3개에는@Transactional(readOnly = true)가 추가됐지만, 기존의read,findBySettlement,findByHeader는 여전히 어노테이션이 없습니다. 특히findByHeader는 두 개의 리포지토리를 순차 호출하므로 동일한 트랜잭션 컨텍스트가 보장되지 않습니다.클래스 레벨에
@Transactional(readOnly = true)를 선언하고, 쓰기 작업이 있는 메서드에만 오버라이드하는 방식을 권장합니다.♻️ 리팩터링 제안
`@Service` `@RequiredArgsConstructor` +@Transactional(readOnly = true) public class SettlementReader {그 후 각 메서드의 개별
@Transactional(readOnly = true)어노테이션은 제거합니다.🤖 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/application/impl/SettlementReader.java` around lines 47 - 62, The class-level transactional behavior is inconsistent: add `@Transactional`(readOnly = true) at the top of SettlementReader so all read methods (including read, findBySettlement, findByHeader, findIdByGroupCode, findListByUserIdAndStatus, findShareListByUserId) run in the same read-only transaction context, remove the individual `@Transactional`(readOnly = true) annotations from those methods, and only annotate methods that perform writes with a non-read-only `@Transactional` to override the class-level setting; target the SettlementReader class and its methods findIdByGroupCode, findListByUserIdAndStatus, findShareListByUserId, read, findBySettlement, and findByHeader when making these changes.src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java (1)
198-200: 테스트 메서드 이름이 비정상적인 스니펫 디렉토리명을 생성합니다.
getShareLinkList_Success라는 메서드명은 Spring REST Docs에서get-share-link-list_-success라는 디렉토리명으로 변환됩니다._-조합이 포함되어 가독성이 떨어지며,settlement.adoc에서도 이 비정상적인 경로를 참조하고 있습니다. 메서드명을getShareLinkListSuccess나getShareLinkList로 변경하면 더 깔끔한 스니펫 경로가 생성됩니다.♻️ 수정 제안
- void getShareLinkList_Success() throws Exception { + void getShareLinkListSuccess() throws Exception {이 경우
settlement.adoc의 참조도get-share-link-list-success로 함께 수정해야 합니다.🤖 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` around lines 198 - 200, Rename the test method getShareLinkList_Success in SettlementControllerTest (the JUnit test method) to remove the underscore (e.g., getShareLinkListSuccess or getShareLinkList) so Spring REST Docs generates a clean snippet directory name; then update the corresponding snippet reference inside settlement.adoc to the new kebab-case path (e.g., get-share-link-list-success) so the doc points to the renamed test output.src/main/java/com/dnd/moddo/event/presentation/SettlementController.java (1)
93-108:ResponseEntity<?>대신 구체적인 타입을 사용하세요.기존 엔드포인트들은
ResponseEntity<SettlementSaveResponse>,ResponseEntity<SettlementDetailResponse>등 구체적인 제네릭 타입을 사용하고 있습니다. 새 엔드포인트들도 동일한 패턴을 따르는 것이 일관성과 타입 안전성을 위해 바람직합니다.♻️ 수정 제안
`@GetMapping`("/list") - public ResponseEntity<?> search( + public ResponseEntity<List<SettlementListResponse>> search( `@LoginUser` LoginUserInfo user, `@Valid` `@ModelAttribute` SearchSettlementListRequest request ) { List<SettlementListResponse> response = querySettlementService.search(user.userId(), request); return ResponseEntity.ok(response); } `@GetMapping`("/share") - public ResponseEntity<?> getShareLinkList( + public ResponseEntity<List<SettlementShareResponse>> getShareLinkList( `@LoginUser` LoginUserInfo user ) { List<SettlementShareResponse> response = querySettlementService.findSettlementShareList(user.userId()); return ResponseEntity.ok(response); }🤖 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/SettlementController.java` around lines 93 - 108, Change the controller methods to return concrete ResponseEntity types instead of ResponseEntity<?>: update the search(...) method to return ResponseEntity<List<SettlementListResponse>> and the getShareLinkList(...) method to return ResponseEntity<List<SettlementShareResponse>>; adjust the method signatures for search and getShareLinkList and their return statements that wrap the results from querySettlementService.search(...) and querySettlementService.findSettlementShareList(...) so the generic types match the List element types (SettlementListResponse and SettlementShareResponse) for stronger typing and consistency with other endpoints.src/main/java/com/dnd/moddo/event/application/query/QuerySettlementService.java (1)
54-54: 매직 넘버10을 상수로 추출하는 것을 권장합니다.기본 limit 값
10이 하드코딩되어 있습니다. 이 값은 향후 변경될 수 있으므로private static final int DEFAULT_LIMIT = 10;과 같이 클래스 상수로 추출하면 가독성과 유지보수성이 향상됩니다.🤖 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/application/query/QuerySettlementService.java` at line 54, Extract the hardcoded default limit into a class constant and use it when request.limit() is null: add a private static final int DEFAULT_LIMIT = 10 in QuerySettlementService and replace the inline 10 in the limit assignment (the variable named limit and the expression request.limit()) to reference DEFAULT_LIMIT to improve readability and maintainability.src/docs/asciidoc/settlement.adoc (1)
149-165: 스니펫 경로에_-패턴이 포함되어 있습니다.
get-share-link-list_-success라는 스니펫 경로는 테스트 메서드명getShareLinkList_Success에서 자동 생성된 것으로, 비정상적인_-조합이 포함됩니다. 테스트 메서드명 변경 시 이 경로들도 함께 업데이트해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/docs/asciidoc/settlement.adoc` around lines 149 - 165, The snippets include paths contain a malformed "_-" sequence (e.g. get-share-link-list_-success) which was generated from the test method name getShareLinkList_Success; fix by removing the underscore in the test name or explicitly setting the snippet names so directory names do not contain "_-": rename the test method getShareLinkList_Success to getShareLinkListSuccess (or annotate/adjust the snippet generation to emit get-share-link-list-success) and then update the Asciidoc includes (the include::{snippets}/... entries) to match the new snippet directory names so the include paths and test-generated snippet folders remain consistent.
🤖 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`:
- Line 106: The description sentence "쿠키에 담긴 토큰의 유효성을 확인합니다" in the auth.adoc
document is missing a period; update that sentence to end with a full stop (.)
so it reads "쿠키에 담긴 토큰의 유효성을 확인합니다." to match the punctuation style of other
section descriptions.
- Around line 110-116: The "토큰 상태 확인" section has the two snippet includes
swapped: ensure the `=== Example` section includes the curl snippet and the
`==== 요청` (HTTP subheading) includes the http-request snippet; specifically swap
the includes so
`include::{snippets}/auth-controller-test/check-auth_-success/curl-request.adoc[]`
is under `=== Example` and
`include::{snippets}/auth-controller-test/check-auth_-success/http-request.adoc[]`
is under `==== 요청` to match the project's established pattern.
- Around line 110-132: Update the snippet include paths in auth.adoc to match
Spring REST Docs' underscore-to-hyphen conversion: change occurrences of
check-auth_-success, check-auth_-expired, and check-auth_-invalid-token to
check-auth-success, check-auth-expired, and check-auth-invalid-token
respectively (i.e., update the include::{snippets}/auth-controller-test/...
paths), and add an include for the check-auth-no-token snippet to reflect the
existing test method checkAuth_NoToken().
In `@src/main/java/com/dnd/moddo/auth/presentation/AuthController.java`:
- Around line 100-108: The code parses the JWT three times by calling
jwtProvider.parseClaims(token) but never uses the stored claims variable; fix by
reusing the parsed Claims: call Claims claims = jwtProvider.parseClaims(token)
once in AuthController (the variable currently named claims) and extract userId
and role from that Claims instance instead of calling
jwtProvider.getUserId(token) and jwtProvider.getRole(token); alternatively add
and use jwtProvider.getUserId(Claims) and jwtProvider.getRole(Claims) overloads
if preferred, ensuring parseClaims, getUserId, and getRole references are
updated to consume the single Claims object to remove redundant parsing.
In
`@src/main/java/com/dnd/moddo/auth/presentation/response/AuthCheckResponse.java`:
- Around line 3-13: The record AuthCheckResponse currently serializes null
fields which breaks the test expecting missing JSON keys; annotate the
AuthCheckResponse record with Jackson's
`@JsonInclude`(JsonInclude.Include.NON_NULL) so that when success() returns
reason=null or fail() returns user=null those fields are omitted from JSON;
update imports to include com.fasterxml.jackson.annotation.JsonInclude and apply
the annotation on the AuthCheckResponse declaration (affecting the success() and
fail() factory methods' serialization behavior).
In `@src/main/java/com/dnd/moddo/common/exception/GlobalExceptionHandler.java`:
- Around line 68-82: The handleValidation method is missing a logging call; add
a LoggingUtils.warn(...) invocation similar to handleTypeMismatch and
handleDefineException to record the validation failure. Inside handleValidation
(MethodArgumentNotValidException handler) log a concise message that includes
the composed message variable and the exception (e) or its stack/summary so
monitoring captures details, then return the existing ResponseEntity with new
ErrorResponse(400, message).
- Around line 84-91: In GlobalExceptionHandler.update the
handleConstraintViolation(ConstraintViolationException e) to log the exception
via LoggingUtils.warn(e) (or the new generic warn(Exception) overload) and avoid
returning e.getMessage() directly; instead build a user-friendly ErrorResponse
message (e.g., "Invalid request data" or join only the violation messages
without exposing property paths) and return that with 400, so clients don’t see
internal field paths while the full exception is recorded in logs.
In
`@src/main/java/com/dnd/moddo/event/application/query/QuerySettlementService.java`:
- Around line 47-57: search currently forwards request.sort() without
null-checking; update QuerySettlementService.search to handle null sort the same
way as status/limit by computing an effectiveSort (e.g. request.sort() == null ?
<defaultSort> : request.sort()) and pass that into
settlementReader.findListByUserIdAndStatus instead of request.sort(); use
SearchSettlementListRequest and settlementReader.findListByUserIdAndStatus to
locate where to apply the change and choose an appropriate default sort constant
for your domain (e.g. CREATED_DESC or a defined DEFAULT_SORT).
In
`@src/main/java/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java`:
- Around line 18-20: The limit field in SearchSettlementListRequest is an
Integer without `@NotNull`, causing potential NPE when unboxed for
SettlementReader.findListByUserIdAndStatus; add `@NotNull` to the limit field (and
import the appropriate javax.validation.constraints.NotNull or
jakarta.validation.constraints.NotNull) so validation rejects missing values, or
alternatively change the field to primitive int with a sensible default,
ensuring callers and the controller binding align with the chosen approach.
In
`@src/main/java/com/dnd/moddo/event/presentation/response/SettlementShareResponse.java`:
- Around line 1-12: SettlementShareResponse is placed in presentation.response
but is used as a SQL projection type by infrastructure classes
(SettlementQueryRepository, SettlementQueryRepositoryImpl), causing an
architecture-layer dependency violation; move SettlementShareResponse (and
SettlementListResponse) into the application.dto package so infrastructure
depends only on application DTOs, and update usages in SettlementQueryRepository
and SettlementQueryRepositoryImpl to reference the new application.dto types;
ensure presentation-layer controllers/services perform any conversion from these
application DTOs to response models if a different response shape is required.
In
`@src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java`:
- Line 236: In SettlementControllerTest replace the incorrect
.andExpect(jsonPath("$[0].completedAt").isEmpty()) with a null-value check using
Hamcrest: call .andExpect(jsonPath("$[0].completedAt", Matchers.nullValue()))
(ensure org.hamcrest.Matchers is imported) so the test validates completedAt is
null the same way as in the searchSettlementList test.
In
`@src/test/java/com/dnd/moddo/domain/settlement/service/implementation/SettlementReaderTest.java`:
- Around line 147-166: The test fixture creates mockList of
SettlementListResponse instances with completedAt set to LocalDateTime.now(),
but IN_PROGRESS rows must have completedAt == null; update the
SettlementListResponse construction in SettlementReaderTest (the mockList
variable) so that any entry representing IN_PROGRESS uses a null completedAt
(leave other fields unchanged) to match the production filter logic that checks
settlement.completedAt.isNull(); ensure you only set completedAt to null for the
IN_PROGRESS entries.
---
Duplicate comments:
In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepository.java`:
- Around line 7-8: SettlementQueryRepository currently imports presentation DTOs
(SettlementListResponse, SettlementShareResponse), violating layer boundaries;
change the repository to return domain entities or dedicated
infrastructure-level projection interfaces (e.g., Settlement,
SettlementProjection) instead of presentation.response types and move any
mapping to SettlementListResponse/SettlementShareResponse into the service or
presentation layer (or provide a mapper class used by the service). Update the
method signatures in SettlementQueryRepository to use domain types or
projections and adjust callers to perform the conversion to
SettlementListResponse/SettlementShareResponse outside the infrastructure layer.
In
`@src/main/java/com/dnd/moddo/event/presentation/response/SettlementListResponse.java`:
- Around line 1-15: The added fields createdAt and completedAt in
SettlementListResponse (and the same pattern in SettlementShareResponse)
reintroduce an infrastructure→presentation dependency; remove direct references
to infrastructure/domain types by changing these fields to presentation-safe
types (e.g., String or a presentation-layer DateTime DTO) and update the mapper
used to populate SettlementListResponse and SettlementShareResponse to convert
domain/infrastructure date objects into that presentation-safe format; ensure
only presentation-layer classes and primitives are imported in
SettlementListResponse and SettlementShareResponse and perform the conversion
inside the mapper or service that constructs these records.
---
Nitpick comments:
In `@src/docs/asciidoc/settlement.adoc`:
- Around line 149-165: The snippets include paths contain a malformed "_-"
sequence (e.g. get-share-link-list_-success) which was generated from the test
method name getShareLinkList_Success; fix by removing the underscore in the test
name or explicitly setting the snippet names so directory names do not contain
"_-": rename the test method getShareLinkList_Success to getShareLinkListSuccess
(or annotate/adjust the snippet generation to emit get-share-link-list-success)
and then update the Asciidoc includes (the include::{snippets}/... entries) to
match the new snippet directory names so the include paths and test-generated
snippet folders remain consistent.
In `@src/main/java/com/dnd/moddo/auth/infrastructure/security/JwtProvider.java`:
- Around line 100-112: The JwtProvider.validate(String token) method is unused
because AuthController.checkAuth() calls parseClaims() directly; either replace
the direct parseClaims() call in AuthController.checkAuth() with a call to
JwtProvider.validate(token) to centralize validation logic (preserving the
existing exception mapping in validate), or delete JwtProvider.validate(String
token) if you do not plan to use it; update imports/tests accordingly and ensure
AuthController continues to handle exceptions the same way when switching to
validate.
- Around line 117-128: Add overloads for getUserId(Claims claims) and
getRole(Claims claims) and update callers to parse the token only once: call
parseClaims(token) in the caller (e.g., AuthController.checkAuth), then pass
that Claims instance to the new getUserId(Claims) and getRole(Claims) methods
instead of calling the token-based getters; keep existing getUserId(String) and
getRole(String) for backward compatibility by delegating them to the new
overloads (i.e., parseClaims(token) -> getUserId(claims)/getRole(claims)).
In `@src/main/java/com/dnd/moddo/common/logging/LoggingUtils.java`:
- Around line 12-25: The three overloaded warn methods (warn(ModdoException),
warn(MethodArgumentNotValidException),
warn(MethodArgumentTypeMismatchException)) duplicate the same logic; replace
them with a single generic warn method (e.g., warn(Throwable exception) or
warn(Exception exception)) that calls
getExceptionMessage(exception.getMessage()) and then log.warn(message + "\n \t
{}", exception), and remove the three specific overloads; update any callers if
they rely on the specific signatures to use the generic warn and keep
getExceptionMessage and log.warn usages intact.
In `@src/main/java/com/dnd/moddo/event/application/impl/SettlementReader.java`:
- Around line 47-62: The class-level transactional behavior is inconsistent: add
`@Transactional`(readOnly = true) at the top of SettlementReader so all read
methods (including read, findBySettlement, findByHeader, findIdByGroupCode,
findListByUserIdAndStatus, findShareListByUserId) run in the same read-only
transaction context, remove the individual `@Transactional`(readOnly = true)
annotations from those methods, and only annotate methods that perform writes
with a non-read-only `@Transactional` to override the class-level setting; target
the SettlementReader class and its methods findIdByGroupCode,
findListByUserIdAndStatus, findShareListByUserId, read, findBySettlement, and
findByHeader when making these changes.
In
`@src/main/java/com/dnd/moddo/event/application/query/QuerySettlementService.java`:
- Line 54: Extract the hardcoded default limit into a class constant and use it
when request.limit() is null: add a private static final int DEFAULT_LIMIT = 10
in QuerySettlementService and replace the inline 10 in the limit assignment (the
variable named limit and the expression request.limit()) to reference
DEFAULT_LIMIT to improve readability and maintainability.
In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepository.java`:
- Line 18: Rename the repository method List<SettlementShareResponse>
findBySettlementList(Long userId) to match the delegation name used in
SettlementReader: findShareListByUserId(Long userId); update the method
declaration in SettlementQueryRepository and any implementing classes and all
callers to use findShareListByUserId so names are consistent with
SettlementReader::findShareListByUserId and avoid confusion.
In
`@src/main/java/com/dnd/moddo/event/infrastructure/SettlementQueryRepositoryImpl.java`:
- Around line 112-123: Remove the unused parameters memberCount and
completedCount from the getOrderSpecifier method signature and all its call
sites (the method is currently declared as getOrderSpecifier(SettlementSortType
sortType, QSettlement settlement, NumberExpression<Long> memberCount,
NumberExpression<Long> completedCount)); then simplify the switch over
SettlementSortType to only handle the explicit enum cases (OLDEST ->
settlement.createdAt.asc(), LATEST -> settlement.createdAt.desc()) and remove
the unreachable default branch so the compiler will force handling of any new
enum values in the future.
- Around line 57-62: The current completedCount uses a DB-dialect-dependent
native template; replace it with a QueryDSL CaseBuilder to be
dialect-independent: create a NumberExpression (e.g., paidFlag) via new
CaseBuilder().when(member.isPaid).then(1).otherwise(0), then use paidFlag.sum()
(and cast to Long if needed) to assign to completedCount in
SettlementQueryRepositoryImpl, replacing the Expressions.numberTemplate usage.
In `@src/main/java/com/dnd/moddo/event/presentation/SettlementController.java`:
- Around line 93-108: Change the controller methods to return concrete
ResponseEntity types instead of ResponseEntity<?>: update the search(...) method
to return ResponseEntity<List<SettlementListResponse>> and the
getShareLinkList(...) method to return
ResponseEntity<List<SettlementShareResponse>>; adjust the method signatures for
search and getShareLinkList and their return statements that wrap the results
from querySettlementService.search(...) and
querySettlementService.findSettlementShareList(...) so the generic types match
the List element types (SettlementListResponse and SettlementShareResponse) for
stronger typing and consistency with other endpoints.
In
`@src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java`:
- Around line 198-200: Rename the test method getShareLinkList_Success in
SettlementControllerTest (the JUnit test method) to remove the underscore (e.g.,
getShareLinkListSuccess or getShareLinkList) so Spring REST Docs generates a
clean snippet directory name; then update the corresponding snippet reference
inside settlement.adoc to the new kebab-case path (e.g.,
get-share-link-list-success) so the doc points to the renamed test output.
|
|
||
| == 토큰 상태 확인 | ||
|
|
||
| 쿠키에 담긴 토큰의 유효성을 확인합니다 |
There was a problem hiding this comment.
설명 문장 끝에 마침표가 누락되었습니다.
기존 모든 섹션의 설명문은 마침표로 끝나지만, 이 섹션만 누락되어 있습니다.
📄 수정 제안
-쿠키에 담긴 토큰의 유효성을 확인합니다
+쿠키에 담긴 토큰의 유효성을 확인합니다.📝 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
| 쿠키에 담긴 토큰의 유효성을 확인합니다 | |
| 쿠키에 담긴 토큰의 유효성을 확인합니다. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/docs/asciidoc/auth.adoc` at line 106, The description sentence "쿠키에 담긴
토큰의 유효성을 확인합니다" in the auth.adoc document is missing a period; update that
sentence to end with a full stop (.) so it reads "쿠키에 담긴 토큰의 유효성을 확인합니다." to
match the punctuation style of other section descriptions.
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[] |
There was a problem hiding this comment.
=== Example과 ==== 요청 섹션에서 스니펫 파일이 서로 뒤바뀌었습니다.
파일 내 모든 기존 섹션은 아래 패턴을 따릅니다.
=== Example→curl-request.adoc==== 요청(HTTP 하위) →http-request.adoc
그러나 새로 추가된 "토큰 상태 확인" 섹션은 두 파일이 반대로 포함되어 있어, 렌더링된 문서에 잘못된 내용이 표시됩니다.
📄 수정 제안
=== 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[] | |
| === 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, The "토큰 상태 확인" section
has the two snippet includes swapped: ensure the `=== Example` section includes
the curl snippet and the `==== 요청` (HTTP subheading) includes the http-request
snippet; specifically swap the includes so
`include::{snippets}/auth-controller-test/check-auth_-success/curl-request.adoc[]`
is under `=== Example` and
`include::{snippets}/auth-controller-test/check-auth_-success/http-request.adoc[]`
is under `==== 요청` to match the project's established pattern.
Comment on lines
+110
to
+132
| 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/http-response.adoc[] | ||
|
|
||
| === Body | ||
|
|
||
| ==== 응답 (valid) | ||
|
|
||
| include::{snippets}/auth-controller-test/check-auth_-success/response-body.adoc[] | ||
|
|
||
| ==== 응답 (invalid) | ||
|
|
||
| include::{snippets}/auth-controller-test/check-auth_-expired/response-body.adoc[] | ||
|
|
||
| include::{snippets}/auth-controller-test/check-auth_-invalid-token/response-body.adoc[] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 실제 생성된 스니펫 디렉터리 이름을 확인하고, 테스트 메서드명을 조회합니다.
# 생성된 스니펫 디렉터리 확인
echo "=== Snippet directories with 'check-auth' ==="
fd -t d "check-auth" --full-path
# 테스트 클래스에서 check-auth 관련 메서드명 조회
echo -e "\n=== Test methods related to check-auth ==="
rg -n "checkAuth|check_auth|check-auth" --type=java -A 2Repository: moddo-kr/moddo-backend
Length of output: 1813
🏁 Script executed:
#!/bin/bash
# Description: REST Docs 스니펫 출력 위치 확인 및 실제 디렉터리명 검색
echo "=== 일반적인 REST Docs 출력 디렉터리 검색 ==="
# 일반적으로 build/, target/, build-snippets/ 등에 생성됨
find . -type d -name "*snippet*" -o -type d -name "*rest-docs*" 2>/dev/null | head -20
echo -e "\n=== 'check' 키워드로 관련 디렉터리 검색 ==="
fd -t d "check" --max-depth 5 | grep -v node_modules | head -20
echo -e "\n=== build 디렉터리 내 check-auth 관련 파일/디렉터리 ==="
find build -type d -name "*check-auth*" -o -type d -name "*checkAuth*" 2>/dev/nullRepository: moddo-kr/moddo-backend
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Description: AuthControllerTest.java 전체 내용 조회 - checkAuth 관련 메서드들
cat -n src/test/java/com/dnd/moddo/domain/auth/controller/AuthControllerTest.java | grep -A 1 "void checkAuth"Repository: moddo-kr/moddo-backend
Length of output: 349
스니펫 디렉터리 경로의 _- 패턴을 수정하세요.
Spring REST Docs는 테스트 메서드명을 디렉터리명으로 변환할 때 언더스코어를 하이픈으로 변환합니다. 현재 문서의 경로들은 올바르지 않습니다:
check-auth_-success→check-auth-success수정 필요check-auth_-expired→check-auth-expired수정 필요check-auth_-invalid-token→check-auth-invalid-token수정 필요
또한 checkAuth_NoToken() 테스트 메서드도 존재하므로 해당 스니펫 경로(check-auth-no-token)도 문서에 추가하세요.
🤖 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 - 132, Update the snippet
include paths in auth.adoc to match Spring REST Docs' underscore-to-hyphen
conversion: change occurrences of check-auth_-success, check-auth_-expired, and
check-auth_-invalid-token to check-auth-success, check-auth-expired, and
check-auth-invalid-token respectively (i.e., update the
include::{snippets}/auth-controller-test/... paths), and add an include for the
check-auth-no-token snippet to reflect the existing test method
checkAuth_NoToken().
src/main/java/com/dnd/moddo/auth/presentation/response/AuthCheckResponse.java
Show resolved
Hide resolved
src/main/java/com/dnd/moddo/event/application/query/QuerySettlementService.java
Show resolved
Hide resolved
src/main/java/com/dnd/moddo/event/presentation/request/SearchSettlementListRequest.java
Show resolved
Hide resolved
Comment on lines
+1
to
+12
| package com.dnd.moddo.event.presentation.response; | ||
|
|
||
| import java.time.LocalDateTime; | ||
|
|
||
| public record SettlementShareResponse( | ||
| Long settlementId, | ||
| String name, | ||
| String groupCode, | ||
| LocalDateTime createdAt, | ||
| LocalDateTime completedAt | ||
| ) { | ||
| } No newline at end of file |
There was a problem hiding this comment.
레이어 아키텍처 위반 — DTO를 적절한 패키지로 이동해야 합니다.
SettlementShareResponse가 presentation.response 패키지에 위치하지만, SettlementQueryRepository(infrastructure)와 SettlementQueryRepositoryImpl에서 SQL 프로젝션 결과 타입으로 직접 사용됩니다. 이는 SettlementListResponse에 대한 기존 리뷰 코멘트와 동일한 infrastructure → presentation 방향의 의존성 위반입니다.
두 DTO 모두 application.dto 계층으로 이동하고, 프레젠테이션 레이어 응답으로의 변환은 서비스 또는 컨트롤러 계층에서 처리하도록 개선하는 것을 권장합니다.
🤖 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/response/SettlementShareResponse.java`
around lines 1 - 12, SettlementShareResponse is placed in presentation.response
but is used as a SQL projection type by infrastructure classes
(SettlementQueryRepository, SettlementQueryRepositoryImpl), causing an
architecture-layer dependency violation; move SettlementShareResponse (and
SettlementListResponse) into the application.dto package so infrastructure
depends only on application DTOs, and update usages in SettlementQueryRepository
and SettlementQueryRepositoryImpl to reference the new application.dto types;
ensure presentation-layer controllers/services perform any conversion from these
application DTOs to response models if a different response shape is required.
src/test/java/com/dnd/moddo/domain/settlement/controller/SettlementControllerTest.java
Show resolved
Hide resolved
src/test/java/com/dnd/moddo/domain/settlement/service/implementation/SettlementReaderTest.java
Show resolved
Hide resolved
This was referenced Mar 8, 2026
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.
#️⃣연관된 이슈
🔀반영 브랜치
feature/get-settlement -> develop
🔧변경 사항
💬리뷰 요구사항(선택)
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
Bug Fixes