Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e70f015
feat: select ... for update 쿼리를 이용한 송금 요청 정보 조회
donggi-lee-bit Apr 4, 2025
8467123
test: RemittanceRequest 상태별 수락 처리 테스트 추가
donggi-lee-bit Apr 4, 2025
aead5dd
refactor: 계좌 락 순서 보장 로직을 AccountLockingService로 분리
donggi-lee-bit Apr 4, 2025
97b2b9f
refactor: RemittanceRequest 상태 처리를 위한 RemittanceRequestProcessor 분리
donggi-lee-bit Apr 4, 2025
ad217c2
feat: 송금 요청 수락 기능
donggi-lee-bit Apr 4, 2025
d77b07f
refactor: 송금 요청 수락 통합 테스트 클래스 네이밍 변경
donggi-lee-bit Apr 4, 2025
290c6ce
feat: 송금 요청 수락 시 수신자 검증 로직 추가
donggi-lee-bit Apr 4, 2025
6cca070
feat: 송금 요청 수락 컨트롤러 추가
donggi-lee-bit Apr 4, 2025
2a2b503
feat: 송금 거절 기능
donggi-lee-bit Apr 4, 2025
05a9827
feat: 송금 요청 거절 컨트롤러
donggi-lee-bit Apr 4, 2025
ef8b7e0
refactor: 송금 요청 처리 로직 중복 제거 및 가독성 개선
donggi-lee-bit Apr 4, 2025
b0ffa3c
feat: 계좌 이체 및 금액 변경 후 업데이트 기능 추가
donggi-lee-bit Apr 4, 2025
8d29bfa
feat: InvalidRemittanceRequestStatusException 예외 핸들링
donggi-lee-bit Apr 4, 2025
a742064
chore: 테스트 검증부 예외 메세지 수정
donggi-lee-bit Apr 4, 2025
91b0959
chore: 아직 작성되지 않은 메서드 제거
donggi-lee-bit Apr 5, 2025
0ec5e1e
fix: 배타 락 획득 목적의 메서드에서 트랜잭션 readonly 옵션 제거
donggi-lee-bit Apr 5, 2025
93aa86d
refactor: 송금자, 수신자 계좌 구분 로직을 LockedAccounts로 캡슐화
donggi-lee-bit Apr 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package com.donggi.sendzy.account.application;

import com.donggi.sendzy.account.domain.Account;
import com.donggi.sendzy.account.domain.AccountRepository;
import com.donggi.sendzy.account.exception.AccountNotFoundException;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

@RequiredArgsConstructor
@Service
public class AccountLockingService {

private final AccountRepository accountRepository;

/**
* 두 개의 계좌를 계좌 ID 기준으로 오름차순 정렬하여 락을 획득합니다.
* 데드락을 방지하기 위해 항상 일정한 순서로 락을 획득합니다.
* @param accountId1 첫 번째 계좌 ID
* @param accountId2 두 번째 계좌 ID
* @return ID 오름차순으로 정렬된 두 계좌 목록
*/
@Transactional(readOnly = true)
public List<Account> getAccountsWithLockOrdered(final long accountId1, final long accountId2) {
List<Long> sortedIds = getSortedIds(accountId1, accountId2);
return sortedIds.stream()
.map(accountId -> accountRepository.findByMemberIdForUpdate(accountId)
.orElseThrow(() -> new AccountNotFoundException(accountId)))
.collect(Collectors.toList());
}

/**
* 회원 ID로 계좌를 조회하고 해당 계좌의 락을 획득합니다.
* @param senderId 송신자 ID
* @return 조회된 계좌
*/
@Transactional(readOnly = true)
public Account getByMemberIdForUpdate(final long senderId) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

X Lock의 획득을 위한 메서드인데 readOnly 옵션을 주는게 적절할까요?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

X Lock을 사용하는 쿼리는 데이터를 수정하기 위한 목적의 조회이므로 readOnly 옵션 제거하였습니다!

return accountRepository.findByMemberIdForUpdate(senderId)
.orElseThrow(() -> new AccountNotFoundException(senderId));
}

private List<Long> getSortedIds(final long senderId, final long receiverId) {
return Stream.of(senderId, receiverId)
.sorted()
.toList();
}
}
11 changes: 10 additions & 1 deletion src/main/java/com/donggi/sendzy/account/domain/Account.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,20 @@ public Account(final Long memberId) {
this.pendingAmount = 0L;
}

public void withdraw(final Long amount) {
public void reserveWithdraw(final Long amount) {
validateWithdraw(amount);
this.pendingAmount += amount;
}

public void commitWithdraw(final long amount) {
this.balance -= amount;
this.pendingAmount -= amount;
}

public void cancelWithdraw(final long amount) {
this.pendingAmount -= amount;
}

public void deposit(final Long amount) {
final var fieldName = "amount";
Validator.notNull(amount, fieldName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,18 @@ public interface AccountRepository {
* @param memberId 회원 ID
* @return 조회된 계좌
*/
Optional<Account> findByMemberId(final Long memberId);
Optional<Account> findByMemberId(final long memberId);

/**
* 회원 ID로 계좌를 조회하고, 조회된 계좌에 배타적 잠금(Exclusive Lock)을 겁니다.
* @param memberId 회원 ID
* @return 잠금이 설정된 계좌(Optional)
*/
Optional<Account> findByIdForUpdate(final Long memberId);
Optional<Account> findByMemberIdForUpdate(final long memberId);

/**
* 계좌의 대기 중인 금액을 업데이트합니다.
* @param id 계좌 ID
* @param pendingAmount 대기 중인 금액
* 계좌를 업데이트합니다.
* @param account 업데이트할 계좌
*/
void updatePendingAmount(final Long id, final Long pendingAmount);

/**
* 계좌의 잔액을 업데이트합니다.
* @param id 계좌 ID
* @param balance 잔액
*/
void updateBalance(final Long id, final Long balance);
void update(final Account account);
}
21 changes: 14 additions & 7 deletions src/main/java/com/donggi/sendzy/account/domain/AccountService.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,26 @@ public Account getByMemberId(final long memberId) {

@Transactional
public void withdraw(final Account account, final long amount) {
account.withdraw(amount);
accountRepository.updatePendingAmount(account.getId(), account.getPendingAmount());
account.reserveWithdraw(amount);
accountRepository.update(account);
}

@Transactional
public void deposit(final Account account, final long amount) {
account.deposit(amount);
accountRepository.updateBalance(account.getId(), account.getBalance());
accountRepository.update(account);
}

@Transactional(readOnly = true)
public Account getByIdForUpdate(final long memberId) {
return accountRepository.findByIdForUpdate(memberId)
.orElseThrow(() -> new AccountNotFoundException(memberId));
@Transactional
public void transfer(final Account sender, final Account receiver, final long amount) {
sender.commitWithdraw(amount);
receiver.deposit(amount);
accountRepository.update(sender);
accountRepository.update(receiver);
}

@Transactional
public void update(final Account account) {
accountRepository.update(account);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,18 @@
import com.donggi.sendzy.account.domain.AccountRepository;
import com.donggi.sendzy.account.domain.TestAccountRepository;
import org.apache.ibatis.annotations.Mapper;
import org.apache.ibatis.annotations.Param;

import java.util.Optional;

@Mapper
public interface AccountMapper extends AccountRepository, TestAccountRepository {
Long create(final Account account);

Optional<Account> findByMemberId(final Long memberId);

void deleteAll();

void updatePendingAmount(@Param("id") final Long id, @Param("pendingAmount") final Long pendingAmount);
void update(final Account account);

Optional<Account> findByMemberId(final long memberId);

void updateBalance(@Param("id") final Long id, @Param("balance") final Long balance);
Optional<Account> findByMemberIdForUpdate(final long memberId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.donggi.sendzy.account.exception.InvalidWithdrawalException;
import com.donggi.sendzy.member.exception.EmailDuplicatedException;
import com.donggi.sendzy.member.exception.InvalidPasswordException;
import com.donggi.sendzy.member.exception.MemberNotFoundException;
import com.donggi.sendzy.remittance.exception.InvalidRemittanceRequestStatusException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ProblemDetail;
import org.springframework.security.access.AccessDeniedException;
Expand Down Expand Up @@ -85,4 +85,12 @@ public ProblemDetail handleInvalidWithdrawalException(final InvalidWithdrawalExc
public ProblemDetail handleBadRequestException(final BadRequestException e) {
return ProblemDetail.forStatusAndDetail(HttpStatus.BAD_REQUEST, e.getMessage());
}

/**
* 송금 요청이 수락 또는 거절 가능한 상태가 아닌 경우
*/
@ExceptionHandler(InvalidRemittanceRequestStatusException.class)
public ProblemDetail handleInvalidRemittanceRequestStatusException(final InvalidRemittanceRequestStatusException e) {
return ProblemDetail.forStatusAndDetail(HttpStatus.CONFLICT, e.getMessage());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.donggi.sendzy.remittance.application;

import com.donggi.sendzy.account.application.AccountLockingService;
import com.donggi.sendzy.account.domain.Account;
import com.donggi.sendzy.account.domain.AccountService;
import com.donggi.sendzy.common.exception.BadRequestException;
Expand All @@ -16,14 +17,12 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.stream.Stream;

@Service
@RequiredArgsConstructor
public class RemittanceRequestApplicationService {

private final AccountService accountService;
private final AccountLockingService accountLockingService;
private final RemittanceHistoryService remittanceHistoryService;
private final RemittanceRequestService remittanceRequestService;
private final RemittanceStatusHistoryService remittanceStatusHistoryService;
Expand All @@ -40,9 +39,9 @@ public long createRemittanceRequest(final Long senderId, final Long receiverId,
validateSenderAndReceiver(senderId, receiverId);

// 계좌 ID를 오름차순으로 정렬하여 일관된 순서로 Lock 확보
List<Long> sortedIds = getSortedIds(senderId, receiverId);
final Account firstAccount = accountService.getByIdForUpdate(sortedIds.get(0));
final Account secondAccount = accountService.getByIdForUpdate(sortedIds.get(1));
final var accounts = accountLockingService.getAccountsWithLockOrdered(senderId, receiverId);
final Account firstAccount = accounts.get(0);
final Account secondAccount = accounts.get(1);

final Account senderAccount = senderId.equals(firstAccount.getMemberId()) ? firstAccount : secondAccount;
final Account receiverAccount = receiverId.equals(firstAccount.getMemberId()) ? firstAccount : secondAccount;
Expand Down Expand Up @@ -112,10 +111,4 @@ private void validateSenderAndReceiver(final Long senderId, final Long receiverI
throw new BadRequestException("송금자와 수신자가 동일합니다.");
}
}

private List<Long> getSortedIds(final Long senderId, final Long receiverId) {
return Stream.of(senderId, receiverId)
.sorted()
.toList();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package com.donggi.sendzy.remittance.application;

import com.donggi.sendzy.account.application.AccountLockingService;
import com.donggi.sendzy.account.domain.AccountService;
import com.donggi.sendzy.remittance.domain.RemittanceRequest;
import com.donggi.sendzy.remittance.domain.RemittanceRequestStatus;
import com.donggi.sendzy.remittance.domain.RemittanceStatusHistory;
import com.donggi.sendzy.remittance.domain.service.RemittanceRequestService;
import com.donggi.sendzy.remittance.domain.service.RemittanceStatusHistoryService;
import com.donggi.sendzy.remittance.exception.InvalidRemittanceRequestStatusException;
import lombok.RequiredArgsConstructor;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@RequiredArgsConstructor
@Service
public class RemittanceRequestProcessor {

private final RemittanceRequestService remittanceRequestService;
private final AccountLockingService accountLockingService;
private final RemittanceStatusHistoryService remittanceStatusHistoryService;
private final AccountService accountService;

@Transactional
public void handleAcceptance(final long requestId, final long receiverId) {
// 송금 요청 조회 및 상태 확인 (PENDING 여부)
final var remittanceRequest = remittanceRequestService.getByIdForUpdate(requestId);
validateReceiverAuthorityAndStatus(remittanceRequest, receiverId);

// 송금자/수신자 계좌 락 + 조회 (ID 오름차순 → 데드락 방지)
final var accounts = accountLockingService.getAccountsWithLockOrdered(remittanceRequest.getSenderId(), remittanceRequest.getReceiverId());
final var senderAccount = remittanceRequest.getSenderId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1);
final var receiverAccount = remittanceRequest.getReceiverId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이 코드는 좀 더 객체지향적인 방식으로 변경하면 안정성있는 코드로 변경할 수 있습니다.
현재는 lock ordering을 위해 일단 파라미터를 넘기면 리스트형태의 순서를 모르는 값이 리턴되는 형태이고, 거기서 수신자와 송금자를 찾는 방식입니다. 이 방식은 일단 명확하지 않죠. 리턴된값중에 혹시나 다른 값이 섞인다면 엉뚱한 계좌 값을 송금자라고 인식할 수 있으니까요.

이런 경우 컬랙션을 직접 다루지 말고 클래스로 다루는것이 좀 더 안전한 코드 작성 방식입니다.

Suggested change
final var accounts = accountLockingService.getAccountsWithLockOrdered(remittanceRequest.getSenderId(), remittanceRequest.getReceiverId());
final var senderAccount = remittanceRequest.getSenderId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1);
final var receiverAccount = remittanceRequest.getReceiverId().equals(accounts.get(0).getMemberId()) ? accounts.get(0) : accounts.get(1);
final LockedAccounts accounts = accountLockingService.getAccountsWithLockOrdered(remittanceRequest.getSenderId(), remittanceRequest.getReceiverId());
final var senderAccount = lockedAccount.getSenderAccount();
final var receiverAccount = lockedAccount.getReceiverAccount();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

여러 개의 잠금 계좌 객체를 다뤄야 하는 상황에서, LockedAccounts 내에서 송금자와 수신자 계좌를 찾는 로직을 캡슐화할 수 있어 좋은 구조인 것 같습니다. 좋은 인사이트 감사합니다 😊


// 이체 처리
accountService.transfer(senderAccount, receiverAccount, remittanceRequest.getAmount());

// 송금 요청 상태 변경 → ACCEPTED
remittanceRequestService.accept(remittanceRequest);

// 상태 변경 히스토리 저장
recordStatus(remittanceRequest, RemittanceRequestStatus.ACCEPTED);
}

@Transactional
public void handleRejection(final long requestId, final long receiverId) {
// 송금 요청 조회 (락 획득)
final var remittanceRequest = remittanceRequestService.getByIdForUpdate(requestId);

// 수신자 권한 확인
validateReceiverAuthorityAndStatus(remittanceRequest, receiverId);

// 송금자 계좌 롤백 처리
final var senderAccount = accountLockingService.getByMemberIdForUpdate(remittanceRequest.getSenderId());
senderAccount.cancelWithdraw(remittanceRequest.getAmount());
accountService.update(senderAccount);

// 송금 요청 상태 변경 → REJECTED
remittanceRequestService.reject(remittanceRequest);

// 상태 변경 히스토리 저장
recordStatus(remittanceRequest, RemittanceRequestStatus.REJECTED);
}

private void validateReceiverAuthorityAndStatus(final RemittanceRequest remittanceRequest, final long receiverId) {
if (!remittanceRequest.getReceiverId().equals(receiverId)) {
throw new AccessDeniedException("해당 송금 요청의 수신자만 처리할 수 있습니다.");
}

if (!remittanceRequest.isPending()) {
throw new InvalidRemittanceRequestStatusException(remittanceRequest.getStatus());
}
}

private void recordStatus(RemittanceRequest request, RemittanceRequestStatus status) {
remittanceStatusHistoryService.recordStatusHistory(
new RemittanceStatusHistory(
request.getId(),
request.getSenderId(),
request.getReceiverId(),
request.getAmount(),
status
)
);
}

public void handleExpiration() {
// 요청 만료
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

아직 완성되지 않은 메서드는 제거해주세요. 완성된 이후에 올려주시길 부탁드립니다~

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

제거하였습니다!

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.donggi.sendzy.remittance.controller;

import com.donggi.sendzy.common.security.CustomUserDetails;
import com.donggi.sendzy.remittance.application.RemittanceRequestProcessor;
import lombok.RequiredArgsConstructor;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RequiredArgsConstructor
@RestController
@RequestMapping("/v1/remittance")
public class RemittanceRequestRestController {

private final RemittanceRequestProcessor remittanceRequestProcessor;

/**
* 송금 요청 수락
* @param requestId 송금 요청 ID
* @param userDetails 로그인 정보
*/
@PostMapping("/{requestId}/accept")
public void accept(@PathVariable("requestId") final long requestId, @AuthenticationPrincipal final CustomUserDetails userDetails) {
final var receiverId = userDetails.getMemberId();
remittanceRequestProcessor.handleAcceptance(requestId, receiverId);
}

/**
* 송금 요청 거절
* @param requestId 송금 요청 ID
* @param userDetails 로그인 정보
*/
@PostMapping("/{requestId}/reject")
public void reject(@PathVariable("requestId") final long requestId, @AuthenticationPrincipal final CustomUserDetails userDetails) {
final var receiverId = userDetails.getMemberId();
remittanceRequestProcessor.handleRejection(requestId, receiverId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,16 @@ public RemittanceRequest(
this.amount = amount;
this.createdAt = LocalDateTime.now();
}

public boolean isPending() {
return this.status == RemittanceRequestStatus.PENDING;
}

public void accept() {
status = this.status.accept();
}

public void reject() {
status = this.status.reject();
}
}
Loading
Loading