[WTH-149] account 도메인 코틀린 마이그레이션#12
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAccount/Receipt 모듈을 Java에서 Kotlin으로 전면 마이그레이션하고 CQRS 기반으로 Use Case를 재구성했으며, 엔티티·DTO·매퍼·레포지토리·서비스·컨트롤러·테스트가 Java → Kotlin으로 교체되었습니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant AccountController
participant GetAccountQueryService
participant AccountRepository
participant ReceiptRepository
participant FileReader
participant ReceiptMapper
participant AccountMapper
Client->>AccountController: GET /api/v1/account/{cardinal}
AccountController->>GetAccountQueryService: findByCardinal(cardinal)
GetAccountQueryService->>AccountRepository: findByCardinal(cardinal)
AccountRepository-->>GetAccountQueryService: Account?
GetAccountQueryService->>ReceiptRepository: findAllByAccountIdOrderByCreatedAtDesc(accountId)
ReceiptRepository-->>GetAccountQueryService: List<Receipt>
GetAccountQueryService->>FileReader: findAll(receiptIds)
FileReader-->>GetAccountQueryService: Map<Long, List<FileResponse>>
GetAccountQueryService->>ReceiptMapper: toResponses(receipts, filesByReceiptId)
ReceiptMapper-->>GetAccountQueryService: List<ReceiptResponse>
GetAccountQueryService->>AccountMapper: toResponse(account, receipts)
AccountMapper-->>GetAccountQueryService: AccountResponse
GetAccountQueryService-->>AccountController: AccountResponse
AccountController-->>Client: CommonResponse<AccountResponse>
sequenceDiagram
actor Admin
participant ReceiptAdminController
participant ManageReceiptUseCase
participant CardinalGetService
participant AccountRepository
participant ReceiptRepository
participant FileRepository
participant FileMapper
Admin->>ReceiptAdminController: POST /api/v1/admin/receipts
ReceiptAdminController->>ManageReceiptUseCase: save(request)
ManageReceiptUseCase->>CardinalGetService: findByAdminSide(cardinal)
CardinalGetService-->>ManageReceiptUseCase: verified
ManageReceiptUseCase->>AccountRepository: findByCardinal(cardinal)
AccountRepository-->>ManageReceiptUseCase: Account
ManageReceiptUseCase->>ReceiptRepository: save(Receipt)
ReceiptRepository-->>ManageReceiptUseCase: saved Receipt
ManageReceiptUseCase->>FileMapper: toEntities(files)
FileMapper-->>ManageReceiptUseCase: List<File>
ManageReceiptUseCase->>FileRepository: saveAll(files)
FileRepository-->>ManageReceiptUseCase: saved files
ManageReceiptUseCase-->>ReceiptAdminController: void
ReceiptAdminController-->>Admin: CommonResponse<Void> (RECEIPT_SAVE_SUCCESS)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
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)
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 (3)
src/main/kotlin/com/weeth/domain/account/domain/entity/Receipt.kt (1)
24-25:amount필드가var로 선언되어 유효성 검사를 우회할 수 있습니다.
create()와update()메서드에서amount > 0검증을 수행하지만,var로 선언된 필드는 직접 접근으로 유효성 검사를 우회할 수 있습니다 (예:receipt.amount = -5).필드를
private set으로 변경하거나 custom setter에 검증 로직을 추가하는 것을 권장합니다.♻️ private set 적용 제안
`@Column`(nullable = false) -var amount: Int, +var amount: Int + private set,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/account/domain/entity/Receipt.kt` around lines 24 - 25, The amount field on Receipt is currently var and can be mutated to invalid values; change its declaration to "var amount: Int private set" or add a custom setter on amount that enforces amount > 0 and throws an IllegalArgumentException on invalid values, and ensure the existing create() and update() methods use the same validation (or rely on the new setter) so direct property assignment (e.g., receipt.amount = -5) cannot bypass validation.src/test/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCaseTest.kt (1)
34-45: 정상 저장 테스트에서cardinalGetService.findByAdminSide()호출 검증 추가를 권장합니다.현재 테스트는
accountRepository.save()호출만 검증합니다.cardinalGetService.findByAdminSide()가 호출되었는지도 검증하면 UseCase의 전체 흐름을 더 완전하게 테스트할 수 있습니다.♻️ 검증 추가 제안
useCase.save(dto) verify(exactly = 1) { accountRepository.save(any()) } + verify(exactly = 1) { cardinalGetService.findByAdminSide(40) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCaseTest.kt` around lines 34 - 45, Add verification that cardinalGetService.findByAdminSide(...) is invoked in the success save test: after calling useCase.save(dto) add a verify call to assert cardinalGetService.findByAdminSide(40) was called (e.g., exactly once). This ensures the ManageAccountUseCaseTest checks both repository save and the dependency call to cardinalGetService.findByAdminSide in the useCase.save path.src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt (1)
16-21:cardinalGetService.findByAdminSide()반환값이 사용되지 않습니다.Line 19에서
findByAdminSide()호출 결과가 무시됩니다. 이것이 유효하지 않은 cardinal에 대해 예외를 던지는 검증 목적이라면, 의도를 명확히 하기 위해 명시적으로 문서화하거나also { }블록 대신 검증 전용 메서드 사용을 권장합니다.♻️ 의도 명확화 제안
`@Transactional` fun save(request: AccountSaveRequest) { if (accountRepository.existsByCardinal(request.cardinal)) throw AccountExistsException() - cardinalGetService.findByAdminSide(request.cardinal) + cardinalGetService.findByAdminSide(request.cardinal) // 존재하지 않으면 예외 발생 accountRepository.save(Account.create(request.description, request.totalAmount, request.cardinal)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt` around lines 16 - 21, ManageAccountUseCase.save에서 cardinalGetService.findByAdminSide(request.cardinal)의 반환값을 사용하지 않고 있어(현재 호출만 함) 검증 의도가 불명확합니다; findByAdminSide의 반환 엔티티를 실제로 사용하려면 저장 로직에서 활용하거나, 검증 전용 의도를 명확히 하기 위해 cardinalGetService에 findByAdminSide를 대체할 validate/ensure 메서드(예: ensureExistsAdminSide 또는 validateAdminSideCardinal) 를 추가해 ManageAccountUseCase.save에서 해당 검증 메서드를 호출하도록 변경하세요; 또는 호출 결과를 val으로 받아 널 여부를 체크하여 예외를 던지도록(또는 주석/문서로 의도를 명시) 수정해 불필요한 무시를 제거하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCase.kt`:
- Around line 46-49: Ensure the receipt belongs to the same account before
adjusting balances: after calling
cardinalGetService.findByAdminSide(request.cardinal) and fetching account via
accountRepository.findByCardinal(request.cardinal) and receipt via
receiptRepository.findByIdOrNull(receiptId), compare receipt.account.id with
account.id; if they differ, throw an appropriate exception (e.g.,
UnauthorizedOperationException or a domain-specific exception) instead of
calling account.adjustSpend(Money.of(receipt.amount), Money.of(request.amount));
keep existing AccountNotFoundException and ReceiptNotFoundException behavior but
add this ownership check to protect against adjusting the wrong account.
- Around line 50-53: The update currently treats null and empty request.files
the same, so an explicit empty list (user intent to delete all files) is
ignored; in ManageReceiptUseCase adjust the branch that reads request.files so
that if request.files is null you skip any file changes, but if request.files is
an empty list you perform deletion via
fileRepository.deleteAll(fileReader.findAll(FileOwnerType.RECEIPT, receiptId,
null)) and do not call saveAll, and if non-empty call
fileRepository.saveAll(fileMapper.toFileList(request.files,
FileOwnerType.RECEIPT, receiptId)); also review the asymmetric behavior between
update() and save() paths and add/adjust tests to cover the empty-list deletion
case.
In `@src/main/kotlin/com/weeth/domain/account/domain/entity/Account.kt`:
- Around line 38-44: adjustSpend currently calls cancelSpend(oldAmount) then
spend(newAmount) which can leave currentAmount in an inconsistent partial state
if spend throws; instead compute the net result first and perform validation
before mutating state so the update is atomic. In practice, calculate net =
currentAmount - oldAmount + newAmount (or a helper like computeAdjustedAmount),
validate that net is allowed, then assign currentAmount = net (or call a single
mutator method that applies the delta once); ensure any checks
(limits/constraints) run before changing currentAmount and remove the two-step
cancelSpend/spend sequence to avoid partial updates.
In `@src/main/kotlin/com/weeth/domain/account/presentation/AccountController.kt`:
- Around line 23-27: The find endpoint is missing the authenticated user
injection; update AccountController.find to accept a userId parameter annotated
with `@Parameter`(hidden = true) and `@CurrentUser` (e.g., fun
find(`@Parameter`(hidden = true) `@CurrentUser` userId: Long, `@PathVariable`
cardinal: Int): CommonResponse<AccountResponse>), then forward that userId to
getAccountQueryService (adjust getAccountQueryService.findByCardinal to accept
userId as first arg) so the service can perform ownership/authorization checks
before returning the AccountResponse; ensure method signatures for
AccountController.find and getAccountQueryService.findByCardinal are updated
consistently and any callers/tests adjusted accordingly.
In
`@src/test/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCaseTest.kt`:
- Around line 45-47: beforeTest의 clearMocks 호출에 fileRepository가 빠져 있어 mock 호출
누적으로 인해 verify(exactly = 1) 등이 불안정해질 수 있습니다; beforeTest 블록에서
clearMocks(receiptRepository, accountRepository, fileReader, cardinalGetService,
fileMapper) 호출에 fileRepository를 추가하여 모든 관련 mock을 초기화하도록 수정하세요 (참조: beforeTest,
clearMocks, receiptRepository, accountRepository, fileReader,
cardinalGetService, fileMapper, fileRepository).
---
Duplicate comments:
In
`@src/main/kotlin/com/weeth/domain/account/presentation/ReceiptAdminController.kt`:
- Around line 30-55: The controller methods save, delete, and update in
ReceiptAdminController are missing the `@CurrentUser` injection (same issue noted
in AccountController); add a parameter annotated with `@CurrentUser` (e.g.,
currentUser: <AuthUserType>) to each of the functions save, delete, and update,
and forward that currentUser to the corresponding manageReceiptUseCase methods
(manageReceiptUseCase.save, .delete, .update) or use it where
authorization/audit is required so the controller receives the authenticated
user context like AccountController does.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCase.kt`:
- Around line 16-21: ManageAccountUseCase.save에서
cardinalGetService.findByAdminSide(request.cardinal)의 반환값을 사용하지 않고 있어(현재 호출만 함)
검증 의도가 불명확합니다; findByAdminSide의 반환 엔티티를 실제로 사용하려면 저장 로직에서 활용하거나, 검증 전용 의도를 명확히
하기 위해 cardinalGetService에 findByAdminSide를 대체할 validate/ensure 메서드(예:
ensureExistsAdminSide 또는 validateAdminSideCardinal) 를 추가해
ManageAccountUseCase.save에서 해당 검증 메서드를 호출하도록 변경하세요; 또는 호출 결과를 val으로 받아 널 여부를
체크하여 예외를 던지도록(또는 주석/문서로 의도를 명시) 수정해 불필요한 무시를 제거하세요.
In `@src/main/kotlin/com/weeth/domain/account/domain/entity/Receipt.kt`:
- Around line 24-25: The amount field on Receipt is currently var and can be
mutated to invalid values; change its declaration to "var amount: Int private
set" or add a custom setter on amount that enforces amount > 0 and throws an
IllegalArgumentException on invalid values, and ensure the existing create() and
update() methods use the same validation (or rely on the new setter) so direct
property assignment (e.g., receipt.amount = -5) cannot bypass validation.
In
`@src/test/kotlin/com/weeth/domain/account/application/usecase/command/ManageAccountUseCaseTest.kt`:
- Around line 34-45: Add verification that
cardinalGetService.findByAdminSide(...) is invoked in the success save test:
after calling useCase.save(dto) add a verify call to assert
cardinalGetService.findByAdminSide(40) was called (e.g., exactly once). This
ensures the ManageAccountUseCaseTest checks both repository save and the
dependency call to cardinalGetService.findByAdminSide in the useCase.save path.
src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCase.kt
Show resolved
Hide resolved
src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCase.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+38
to
+44
| fun adjustSpend( | ||
| oldAmount: Money, | ||
| newAmount: Money, | ||
| ) { | ||
| cancelSpend(oldAmount) | ||
| spend(newAmount) | ||
| } |
There was a problem hiding this comment.
예외 발생 시 상태가 불일치할 수 있습니다.
adjustSpend가 cancelSpend 후 spend를 호출하므로, spend에서 예외가 나면 currentAmount가 이미 증가한 상태로 남습니다(부분 변경). 변경 전 검증 후 한 번에 조정하도록 원자적으로 처리하는 편이 안전합니다.
🛠️ 안전한 원자적 조정 예시
fun adjustSpend(
oldAmount: Money,
newAmount: Money,
) {
- cancelSpend(oldAmount)
- spend(newAmount)
+ require(oldAmount.value > 0) { "기존 금액은 0보다 커야 합니다: ${oldAmount.value}" }
+ require(newAmount.value > 0) { "변경 금액은 0보다 커야 합니다: ${newAmount.value}" }
+ val delta = newAmount.value - oldAmount.value
+ when {
+ delta > 0 -> {
+ check(currentAmount >= delta) { "잔액이 부족합니다. 현재: $currentAmount, 요청: $delta" }
+ currentAmount -= delta
+ }
+ delta < 0 -> {
+ check(currentAmount - delta <= totalAmount) { "총액을 초과할 수 없습니다. 총액: $totalAmount" }
+ currentAmount -= delta
+ }
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/weeth/domain/account/domain/entity/Account.kt` around
lines 38 - 44, adjustSpend currently calls cancelSpend(oldAmount) then
spend(newAmount) which can leave currentAmount in an inconsistent partial state
if spend throws; instead compute the net result first and perform validation
before mutating state so the update is atomic. In practice, calculate net =
currentAmount - oldAmount + newAmount (or a helper like computeAdjustedAmount),
validate that net is allowed, then assign currentAmount = net (or call a single
mutator method that applies the delta once); ensure any checks
(limits/constraints) run before changing currentAmount and remove the two-step
cancelSpend/spend sequence to avoid partial updates.
Comment on lines
+23
to
+27
| @GetMapping("/{cardinal}") | ||
| @Operation(summary = "회비 내역 조회") | ||
| fun find( | ||
| @PathVariable cardinal: Int, | ||
| ): CommonResponse<AccountResponse> = CommonResponse.success(ACCOUNT_FIND_SUCCESS, getAccountQueryService.findByCardinal(cardinal)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# `@CurrentUser` 사용 패턴 및 AccountController 확인
echo "=== `@CurrentUser` 사용 현황 ==="
rg -n "@CurrentUser" src/main --type kotlin
echo -e "\n=== AccountController 전체 구조 ==="
rg -n "class AccountController" -A 50 src/main --type kotlin | head -80
echo -e "\n=== findByCardinal 메서드 정의 ==="
rg -n "fun findByCardinal" -B 2 -A 10 src/main --type kotlin
echo -e "\n=== 같은 패키지의 다른 controller 인증 패턴 ==="
fd "Controller.kt$" src/main/kotlin/com/weeth/domain --exec rg -l "@CurrentUser" {} \;Repository: Team-Weeth/weeth-server
Length of output: 4825
@currentuser 주입 필수 — 회비 내역 조회 엔드포인트에 인증 사용자 주입 추가
컨트롤러 가이드라인 및 코드베이스 패턴(PostCommentController, NoticeCommentController 참고)에 따라 인증된 사용자 ID를 주입해야 합니다. 현재 구현은 @CurrentUser 매개변수가 누락되어 있으며, 서비스 레이어도 사용자 기반 접근 제어 검증을 수행하지 않습니다.
fun find(
`@Parameter`(hidden = true) `@CurrentUser` userId: Long,
`@PathVariable` cardinal: Int,
): CommonResponse<AccountResponse>회비 내역은 특정 사용자의 개인 정보이므로 조회 권한을 검증하고, userId를 서비스에 전달하여 권한 체크(예: 해당 계정의 소유자 여부)를 수행하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/weeth/domain/account/presentation/AccountController.kt`
around lines 23 - 27, The find endpoint is missing the authenticated user
injection; update AccountController.find to accept a userId parameter annotated
with `@Parameter`(hidden = true) and `@CurrentUser` (e.g., fun
find(`@Parameter`(hidden = true) `@CurrentUser` userId: Long, `@PathVariable`
cardinal: Int): CommonResponse<AccountResponse>), then forward that userId to
getAccountQueryService (adjust getAccountQueryService.findByCardinal to accept
userId as first arg) so the service can perform ownership/authorization checks
before returning the AccountResponse; ensure method signatures for
AccountController.find and getAccountQueryService.findByCardinal are updated
consistently and any callers/tests adjusted accordingly.
...test/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCaseTest.kt
Show resolved
Hide resolved
hyxklee
reviewed
Feb 20, 2026
Contributor
hyxklee
left a comment
hyxklee
left a comment
There was a problem hiding this comment.
아 리뷰를 한 줄 았앗는데 pending으로 남아 있었네요..
고생하셨습니다! 스웨거 예시랑 파일 업데이트 관련해서만 한 번 확인 부탁드릴게요!
| @field:NotNull | ||
| @field:Positive | ||
| val totalAmount: Int, | ||
| @field:Schema(description = "기수", example = "40") |
| @field:Schema(description = "사용 날짜", example = "2024-09-01") | ||
| @field:NotNull | ||
| val date: LocalDate, | ||
| @field:Schema(description = "기수", example = "40") |
| @field:NotNull | ||
| val cardinal: Int, | ||
| @field:Valid | ||
| val files: List<@NotNull FileSaveRequest>?, |
Contributor
There was a problem hiding this comment.
"첨부 파일 변경 규약: null=변경 안 함, []=전체 삭제, 배열 전달=해당 목록으로 교체"
이 내용도 같이 들어가면 좋을 것 같아요!
| @field:Schema(description = "사용 날짜", example = "2024-09-01") | ||
| @field:NotNull | ||
| val date: LocalDate, | ||
| @field:Schema(description = "기수", example = "40") |
src/main/kotlin/com/weeth/domain/account/application/usecase/command/ManageReceiptUseCase.kt
Outdated
Show resolved
Hide resolved
Collaborator
Author
|
다 수정했습니다!! |
hyxklee
approved these changes
Feb 20, 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
2 participants
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.
📌 Summary
Account 도메인 Java → Kotlin 마이그레이션 및 아키텍처 구조 변경
📝 Changes
What
Why
How
ReceiptMapper.toResponses(receipts, filesByReceiptId)배치 매핑으로 N+1 제거📸 Screenshots / Logs
💡 Reviewer 참고사항
fileReader.findAll1회 호출 verify로 검증합니다.✅ Checklist
Summary by CodeRabbit
New Features
Refactor
Tests