[test] 정기 모임 / 정산 / 결제 테스트 코드 추가 및 리팩토링#133
Conversation
…into refactor/settlement/gkdudans
Service 단위 테스트
Repository 단위 테스트
Repository 단위 테스트
UT-03-006~UT-03-006-7
UT-03-012
UT-03-012
UT-03-012
UT-03-015~UT-03-024
…into refactor/settlement/gkdudans
UT-03-025~UT-03-027
|
Caution Review failedThe pull request is closed. Walkthrough빌드/테스트 설정과 .gitignore를 확장하고, 스케줄·정산·결제·지갑·클럽·채팅 도메인의 엔티티·레포지토리·서비스·컨트롤러·DTO를 다수 변경·추가했습니다. 정산의 자동화·동시성 보호와 결제 실패 보고/지갑 보류 모델(postedBalance/pendingOut) 도입이 핵심입니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SC as ScheduleController
participant SS as ScheduleService
participant CR as ClubRepository
participant ST as SettlementRepository
participant WR as WalletRepository
U->>SC: POST /clubs/{clubId}/schedules (ScheduleRequestDto)
SC->>SS: createSchedule(clubId, dto)
SS->>CR: validate club & leader
SS->>SS: save Schedule
SS->>ST: create Settlement (initial)
SS-->>SC: ScheduleCreateResponseDto(scheduleId)
SC-->>U: 201 Created + body
sequenceDiagram
autonumber
actor L as Leader
participant C as SettlementController
participant S as SettlementService
participant SR as SettlementRepository
participant USR as UserSettlementRepository
participant WR as WalletRepository
L->>C: POST /clubs/{clubId}/schedules/{id}/settlements
C->>S: automaticSettlement(clubId, scheduleId)
S->>SR: markProcessing(settlementId) // guard to IN_PROGRESS
loop for each participant in HOLD_ACTIVE
S->>WR: holdBalanceIfEnough(userId, amount)
alt hold OK
S->>WR: captureHold(userId, amount)
S->>WR: creditByUserId(leaderId, amount)
S->>USR: updateStatusIfRequested(userSettlementId, COMPLETED)
else hold/capture fail
S->>SR: updateTotalStatus(FAILED)
S-->>C: throw error (rollback & log)
end
end
S->>SR: updateTotalStatus(COMPLETED) and close schedule
S-->>C: success (201)
sequenceDiagram
autonumber
actor U as User
participant P as PaymentService
participant TP as TossPaymentClient
participant WR as WalletRepository
participant PR as PaymentRepository
U->>P: confirm(req)
P->>TP: confirmPayment(...)
alt Toss DONE
P->>WR: creditByUserId(userId, amount)
P->>PR: save payment(Status.DONE, tossPaymentKey)
else Toss error/exception
P->>P: reportFail(req) // dedupe & create WalletTransaction FAILED
P-->>U: throw mapped exception
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/main/java/com/example/onlyone/domain/payment/entity/Payment.java (1)
47-52: Payment–WalletTransaction 양방향 연관관계 동기화 및 중복 로직 제거가 필요합니다.아래 파일과 메서드에서 연관관계 일관성이 보장되지 않아 영속성 컨텍스트와 DB 상태가 불일치할 위험이 있습니다. 또한 동일한 로직이 중복 구현되어 있어 단일 헬퍼 메서드로 통합하는 것을 권장드립니다.
– 대상 파일
• src/main/java/com/example/onlyone/domain/payment/entity/Payment.java– 문제 지점
• updateOnConfirm(String, Status, Method, WalletTransaction) (47–52행)
• updateWalletTransaction(WalletTransaction) (54–56행)– 제안된 수정(diff)
- public void updateOnConfirm(String paymentKey, Status status, Method method, WalletTransaction walletTransaction) { - this.tossPaymentKey = paymentKey; - this.status = status; - this.method = method; - this.walletTransaction = walletTransaction; - } + public void updateOnConfirm(String paymentKey, Status status, Method method, WalletTransaction walletTransaction) { + this.tossPaymentKey = paymentKey; + this.status = status; + this.method = method; + updateWalletTransaction(walletTransaction); + } - public void updateWalletTransaction(WalletTransaction walletTransaction) { - this.walletTransaction = walletTransaction; - } + public void updateWalletTransaction(WalletTransaction walletTransaction) { + if (this.walletTransaction == walletTransaction) return; + // 기존 연관관계 끊기 + if (this.walletTransaction != null) { + this.walletTransaction.updatePayment(null); + } + this.walletTransaction = walletTransaction; + // 소유측 동기화 + if (walletTransaction != null && walletTransaction.getPayment() != this) { + walletTransaction.updatePayment(this); + } + }– 추가 검토 사항
• updateOnConfirm, updateWalletTransaction 접근 제어자를 패키지(private) 등으로 제한해 외부에서 무분별한 상태 변경을 방지하세요.
• WalletTransaction.updatePayment(Payment) 메서드가 연관관계의 소유측을 올바르게 관리하는지 다시 한번 확인 바랍니다.이 리팩토링을 통해 양방향 관계의 일관성을 보장하고 중복된 필드 갱신 로직을 제거할 수 있습니다.
src/main/java/com/example/onlyone/global/exception/ErrorCode.java (1)
26-26: 유저 노출 메시지 오탈자/문장부호 수정 제안사용자-facing 문구여서 작은 오탈자도 고객 신뢰에 영향이 있습니다. 아래 정도는 이번 PR에서 함께 정리하는 걸 권장합니다.
적용 제안 diff:
- INVALID_CATEGORY(400, "INTEREST_400_1", "유효하지 않은 카데고리입니다."), + INVALID_CATEGORY(400, "INTEREST_400_1", "유효하지 않은 카테고리입니다."), - MEMBER_CANNOT_MODIFY_SCHEDULE(403, "SCHEDULE_403_1", "리더만 정기 모임을 수정할 수 있습니다,"), + MEMBER_CANNOT_MODIFY_SCHEDULE(403, "SCHEDULE_403_1", "리더만 정기 모임을 수정할 수 있습니다."), - MEMBER_CANNOT_DELETE_SCHEDULE(403, "SCHEDULE_403_2", "리더만 정기 모임을 삭제할 수 있습니다,"), + MEMBER_CANNOT_DELETE_SCHEDULE(403, "SCHEDULE_403_2", "리더만 정기 모임을 삭제할 수 있습니다."), - IMAGE_SIZE_EXCEEDED(413, "IMAGE_413_1", "이미지 크기가 허용된 최대 크기(5MB) 크기를 초과했니다."), + IMAGE_SIZE_EXCEEDED(413, "IMAGE_413_1", "이미지 크기가 허용된 최대 크기(5MB)을 초과했습니다."),Also applies to: 57-58, 129-129
src/main/java/com/example/onlyone/domain/wallet/service/WalletService.java (1)
129-157: FAILED 처리 시 상태 업데이트 결과 미검증으로 인한 중복/불일치 가능성updateStatusIfRequested는 조건부 업데이트입니다. 반환값(갱신 행 수)을 확인하지 않고 실패 거래와 Transfer를 생성하면, 이미 REQUESTED가 아닌 상태(예: IN_PROGRESS/COMPLETED 등)에서도 실패 레코드가 쌓일 수 있습니다. 이는 정산 이력 왜곡과 중복 기록을 유발합니다. 업데이트 결과가 0이면 조기 반환하도록 방어 로직을 추가해 주세요.
패치 제안:
public void createFailedWalletTransactions(Long walletId, Long leaderWalletId, int amount, Long userSettlementId, int walletBalance, int leaderWalletBalance) { @@ - walletTransactionRepository.save(failedOutgoing); - walletTransactionRepository.save(failedIncoming); - userSettlementRepository.updateStatusIfRequested(userSettlementId, SettlementStatus.FAILED); - createAndSaveTransfers(userSettlement, failedOutgoing, failedIncoming); + // 1) 상태 전이 시도 (REQUESTED -> FAILED) + int updated = userSettlementRepository.updateStatusIfRequested(userSettlementId, SettlementStatus.FAILED); + if (updated == 0) { + log.warn("createFailedWalletTransactions skipped. userSettlementId={} was not in REQUESTED.", userSettlementId); + return; // 상태 불일치 시 실패 트랜잭션/Transfer 생성 방지 + } + // 2) 상태 전이가 성공했을 때만 실패 트랜잭션 및 연결 생성 + walletTransactionRepository.save(failedOutgoing); + walletTransactionRepository.save(failedIncoming); + createAndSaveTransfers(userSettlement, failedOutgoing, failedIncoming);src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java (2)
82-122: 권한 검증을 엔티티 생성/저장 이전에 수행하세요.현재 비리더가 호출해도 Schedule이 먼저 저장된 뒤 예외가 발생합니다. 롤백이 되더라도 불필요한 INSERT가 발생하며, 예외 처리 방식에 따라 잔존 가능성도 있습니다. 권한 체크를 선행해 DB 부하와 리스크를 줄이세요.
적용 예시:
- Schedule schedule = requestDto.toEntity(club); - scheduleRepository.save(schedule); - User user = userService.getCurrentUser(); - UserClub userClub = userClubRepository.findByUserAndClub(user, club) + User user = userService.getCurrentUser(); + UserClub userClub = userClubRepository.findByUserAndClub(user, club) .orElseThrow(() -> new CustomException(ErrorCode.USER_CLUB_NOT_FOUND)); if (userClub.getClubRole() != ClubRole.LEADER) { throw new CustomException(ErrorCode.MEMBER_CANNOT_CREATE_SCHEDULE); } + Schedule schedule = requestDto.toEntity(club); + scheduleRepository.save(schedule);
317-339: 삭제 API가 Schedule 엔티티를 실제로 삭제하지 않습니다. 테스트/기대와 불일치합니다.현재는 Club 관계 해제와 ChatRoom 삭제만 수행하고 Schedule은 남아있습니다. 테스트는 삭제 후 조회가 비어있음을 기대합니다. 실제 삭제를 수행하거나, 소프트 삭제 정책이라면 테스트를 그에 맞게 수정해야 합니다.
두 가지 대안 중 하나를 선택하세요.
- 대안 A(하드 삭제):
ChatRoom chatRoom = chatRoomRepository.findByTypeAndScheduleId(Type.SCHEDULE, scheduleId) .orElseThrow(() -> new CustomException(ErrorCode.CHAT_ROOM_NOT_FOUND)); club.getSchedules().remove(schedule); chatRoomRepository.delete(chatRoom); + scheduleRepository.delete(schedule);
- 대안 B(소프트 삭제 유지): 테스트를 "스케줄 엔티티가 물리적으로 삭제되지는 않지만, ChatRoom 제거 및 관계 해제 확인"으로 수정.
🧹 Nitpick comments (84)
.gitignore (1)
49-49: 로그 파일 무시 패턴을 일반화단일 파일(
spy.log) 대신 일반 패턴을 사용하면 로그 산출물 전반을 포괄적으로 무시할 수 있습니다.- spy.log + # log files + *.log + logs/src/main/java/com/example/onlyone/domain/wallet/entity/WalletTransaction.java (1)
10-11: 사용되지 않는 import 정리
LocalDateTime를 import했지만 해당 클래스 내에서 사용되지 않습니다. 불필요한 import는 제거해 주세요.-import java.time.LocalDateTime;src/main/java/com/example/onlyone/global/feign/TossPaymentClient.java (1)
12-12: 프로퍼티 키 변경(하이픈 → 언더스코어)로 인한 환경설정 리스크 — 안전한 플레이스홀더로 완화 권장
payment.toss.base-url에서payment.toss.base_url로 바뀌면, profile 별application-*.yml/K8s ConfigMap/환경변수에서 키 미스매치가 발생할 수 있습니다. 양쪽 키를 모두 허용하는 fallback 플레이스홀더를 권장합니다. 또한 base URL에 슬래시 유무에 따라@PostMapping("/confirm")와의 조합에서 이중 슬래시가 생기지 않도록 값의 표준화를 확인해 주세요.- url = "${payment.toss.base_url}", + // base_url 우선, 없으면 기존 base-url을 사용 + url = "${payment.toss.base_url:${payment.toss.base-url}}",검증 체크리스트:
- application.yml 및 각 profile 파일에 두 키 중 최소 하나가 존재하는지 확인
- 운영/스테이징 환경변수에서
PAYMENT_TOSS_BASE_URL매핑 여부 확인- 값 끝에 슬래시가 없는 형태로 통일(예: https://api.example.com/v1/payments)
src/main/java/com/example/onlyone/domain/settlement/entity/SettlementStatus.java (1)
4-5: 새 상태(HOLD_ACTIVE/HOLD_RELEASED) 도입에 따른 영속성/전이 규칙 확인 및 문서화 제안
- Enum 값이 추가되었습니다. JPA 매핑이 전부
@Enumerated(EnumType.STRING)인지 다시 한번 확인해 주세요. ORDINAL 사용 중이면 데이터 불일치가 발생합니다.- 상태 의미가 중요하므로 간단한 주석을 추가하면 가독성과 유지보수성이 좋아집니다.
public enum SettlementStatus { - HOLD_ACTIVE, - HOLD_RELEASED, + /** 정산 대상 금액을 지갑에서 홀드한 상태 */ + HOLD_ACTIVE, + /** 홀드가 해제된 상태(정산 흐름에서 환원 또는 캡처 실패 등) */ + HOLD_RELEASED, REQUESTED, COMPLETED, PENDING, FAILED }추가 검증:
- UserSettlement/Settlement 전이 규칙에서 HOLD_* ↔ REQUESTED/COMPLETED/PENDING/FAILED 간 상태 전이가 일관적인지 테스트로 보장
- 비동기/재시도 시 중복 처리 방지를 위한 상태 전이 원자성 확인
src/main/java/com/example/onlyone/domain/schedule/dto/response/ScheduleCreateResponseDto.java (1)
1-12: 간단 불변 DTO는 Java 21 record로 단순화 가능단일 필드 응답이라면 Lombok + 클래스보다 record가 간결하고 직관적입니다. 직렬화에도 문제가 없습니다. 선호에 따라 유지해도 무방하나, record 전환을 권장합니다.
-package com.example.onlyone.domain.schedule.dto.response; - -import lombok.AllArgsConstructor; -import lombok.Builder; -import lombok.Getter; - -@Builder -@Getter -@AllArgsConstructor -public class ScheduleCreateResponseDto { - private Long scheduleId; -} +package com.example.onlyone.domain.schedule.dto.response; + +public record ScheduleCreateResponseDto(Long scheduleId) {}참고:
- OpenAPI 문서가 필요하면
@Schema를 record에 부착해 설명을 추가할 수 있습니다.build.gradle (1)
42-42: H2 범위 축소 권장 — 프로덕션 런타임 포함을 피하세요
runtimeOnly 'com.h2database:h2'는 프로덕션 런타임 클래스패스에도 포함됩니다. 테스트 전용 사용이라면testRuntimeOnly로 범위를 축소해 주세요.- runtimeOnly 'com.h2database:h2' + testRuntimeOnly 'com.h2database:h2'src/main/java/com/example/onlyone/domain/payment/entity/Payment.java (1)
23-24: tossPaymentKey의 nullable 전환: DB 제약/인덱스 동작을 명확화 필요
toss_payment_key를 nullable(+ unique)로 변경하셨는데, 대부분의 RDB(PostgreSQL/MySQL/H2)는 “unique + nullable”에서 NULL을 여러 건 허용합니다. 즉, 결제 확정 전까지tossPaymentKey = NULL인 Payment가 다수 존재할 수 있습니다. 의도한 모델이라면 OK입니다. 다만 “NULL은 1건만”을 기대한다면 DB 레벨에서 보장되지 않으므로(부분 인덱스 등 추가 필요) 요구사항을 재확인해 주세요.src/main/java/com/example/onlyone/domain/settlement/entity/TotalStatus.java (1)
4-8: TotalStatus enum 매핑에서 ORDINAL 미사용 확인
Settlement.java
- 39행:
@Enumerated(EnumType.STRING)적용 → ENUM 변경 시 ORDINAL 기반 데이터 훼손 위험 없음- 프로젝트 전반에서
@Enumerated(EnumType.ORDINAL)사용 사례가 없음 확인선택 사항: Settlement 상태 전이(REQUESTED→HOLDING→IN_PROGRESS→COMPLETED/FAILED)가 WalletTransactionStatus.PENDING의 동시성 잠금 역할을 대체·보완하려는 의도라면,
해당 로직의 상태 전이 원자성과 재시도/중복 처리(idempotency)가 보장되는지 검토하세요.src/test/java/com/example/onlyone/global/redis/TestRedisContainerConfig.java (3)
1-1: 파일 경로와 패키지 불일치(사소하나 혼란 유발 가능)파일 경로는
.../global/redis/...인데 패키지는com.example.onlyone.support입니다. 컴파일에는 문제가 없지만 IDE 탐색/패키지 정리 기준과 어긋납니다. 패키지를com.example.onlyone.global.redis로 맞추거나 파일 위치를 이동하는 정리가 좋습니다.
14-22: GenericContainer 대신 RedisContainer 사용 및 재사용 최적화 제안현재 구성은 동작에 문제 없으나, 다음 개선을 추천합니다.
- org.testcontainers:
RedisContainer사용으로 가독성과 기본 설정 이점.- 로컬 개발 속도 향상을 위해
.withReuse(true)옵션(개인 환경에서~/.testcontainers.properties설정 필요)을 고려.- private static final DockerImageName REDIS_IMAGE = - DockerImageName.parse("redis:7.2-alpine"); + private static final DockerImageName REDIS_IMAGE = + DockerImageName.parse("redis:7.2-alpine"); @Container @SuppressWarnings("resource") - public static final GenericContainer<?> REDIS = - new GenericContainer<>(REDIS_IMAGE) - .withExposedPorts(6379) - .waitingFor(Wait.forListeningPort()); + public static final org.testcontainers.containers.RedisContainer REDIS = + new org.testcontainers.containers.RedisContainer(REDIS_IMAGE) + .withExposedPorts(6379) + .waitingFor(Wait.forListeningPort()) + .withReuse(true);Spring Boot 3.1+를 사용 중이면
@ServiceConnection로 속성 오버라이드를 단순화할 수도 있습니다(현 방식도 충분히 명확합니다).
24-28: DynamicPropertySource 키/타입 적정성 확인
sprint.data.redis.host/port바인딩은 적절합니다. 포트는int로 제공되어 문제 없습니다. 테스트에서 DB index를 분리해야 하면spring.data.redis.database(기본 0)도 함께 주입하는 방안을 고려해 보세요.src/main/java/com/example/onlyone/domain/club/dto/request/ClubRequestDto.java (2)
15-17: primitive 타입에 대한 @Min/@max 사용은 적절 — @NotNull 생략 유지 확인
userLimit가int이므로@NotNull이 제거된 것은 자연스럽습니다. 검증 메시지가 ScheduleRequestDto와 일관되게 유지되는지도 함께 확인해 주세요.
19-20: description에 @notblank 추가로 API 스펙이 “필수”로 강화됨이 변경은 기존에 빈 문자열/공백 설명을 허용하던 클라이언트를 깨뜨릴 수 있습니다. 도메인 규칙상 필수라면 OK, 아니라면
@Size(max=50)만 유지하거나@NotBlank대신 선택 입력을 허용하는 방향을 재검토해 주세요.src/main/java/com/example/onlyone/domain/schedule/dto/request/ScheduleRequestDto.java (2)
18-19: location에 @notblank 추가는 합리적장소는 사용자 입력의 핵심 필드이므로 공백 금지 검증 추가가 타당합니다. 컨트롤러 단의 국제화 메시지 키와 일관성만 확인해 주세요.
21-26: primitive 필드에 대한 @NotNull 중복 — 제거 권장 또는 래퍼 타입으로 전환
cost,userLimit가int이므로@NotNull은 효과가 없습니다. 아래 둘 중 하나를 권장합니다.
- 단순화:
@NotNull제거- 의미 보강: 타입을
Integer로 바꾸고@NotNull유지(미입력과 0의 구분 필요 시)- @NotNull @Min(value = 0, message = "정기 모임 금액은 0원 이상이어야 합니다.") private int cost; - @NotNull @Min(value = 1, message = "정기 모임 정원은 1명 이상이어야 합니다.") @Max(value = 100, message = "정기 모임 정원은 100명 이하여야 합니다.") private int userLimit;src/main/java/com/example/onlyone/global/exception/ErrorCode.java (1)
66-66: 에러 메시지 일치성 수정 제안상수 BEFORE_SCHEDULE_END 는 ‘종료 이전(아직 종료되지 않음)’을 의미하는데, 현재 메시지
"아직 진행되지 않은 정기 모임입니다."는 ‘시작 전(아직 진행되지 않음)’으로 잘못 표기되어 혼선을 줄 수 있습니다.
rg -nP 'BEFORE_SCHEDULE_(START|END)'결과, 해당 상수는 서비스와 테스트에서 예외 코드 비교에만 사용되며(사용처 확인), 메시지 변경으로 동작에 영향이 없습니다.수정 대상
- 파일:
src/main/java/com/example/onlyone/global/exception/ErrorCode.java- 라인: 66
제안된 수정안
- BEFORE_SCHEDULE_END(409, "SCHEDULE_409_5", "아직 진행되지 않은 정기 모임입니다."), + BEFORE_SCHEDULE_END(409, "SCHEDULE_409_5", "아직 종료되지 않은 정기 모임입니다."),src/main/java/com/example/onlyone/domain/club/repository/ClubRepository.java (1)
114-114: 매개변수 타입 및 변수명 개선 제안
검증 결과,findByClubId(long l)호출부는 테스트 코드(ClubRepositoryTest.java303–306, 321–324)에서만 사용되고 있으며, 명시적으로null을 전달하는 패턴은 확인되지 않았습니다. 그럼에도 불구하고, 향후 서비스 계층이나 다른 호출부에서Long타입 변수가null일 경우 즉시 언박싱 NPE가 발생할 잠재적 위험이 남아 있습니다.• 호출부 검증
rg -nP '\bfindByClubId\s*\(' -C3→ 테스트에서만 호출 확인rg -nP '\bfindByClubId\s*\(\s*null\s*\)'→ null 리터럴 전달 패턴 없음권장 수정안:
- Club findByClubId(long l); + Club findByClubId(Long clubId);• 변수명도
l→clubId로 변경하여 가독성 개선 바랍니다.src/main/java/com/example/onlyone/domain/chat/entity/UserChatRoom.java (2)
8-8: 불필요한 import 제거
org.hibernate.annotations.Cascade를 사용하지 않습니다. 정리하여 경고를 없애주세요.권장 수정안:
-import org.hibernate.annotations.Cascade;
23-25: UserChatRoom.chatRoom 매핑에서 CascadeType.PERSIST 제거 권장 (불필요한 캐스케이드)현재 코드 흐름을 확인한 결과,
ChatRoom엔티티는 항상 같은 트랜잭션 컨텍스트 안에서
chatRoomRepository.save(chatRoom)또는chatRoomRepository.findBy…()를 통해 먼저 영속화/조회된 다음에
UserChatRoom.builder().chatRoom(chatRoom)…으로 주입되어 있습니다.
따라서CascadeType.PERSIST는 실제로 활용되지 않으며, 향후 Detached 상태의ChatRoom을 참조할 때 발생할 수 있는
detached entity passed to persist예외를 방지하기 위해 제거하는 것이 좋습니다.수정 제안:
- 파일:
src/main/java/com/example/onlyone/domain/chat/entity/UserChatRoom.java- 변경 전:
@ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) @JoinColumn(name = "chat_room_id", updatable = false) private ChatRoom chatRoom;- 변경 후:
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "chat_room_id", updatable = false) private ChatRoom chatRoom;src/main/java/com/example/onlyone/domain/settlement/entity/UserSettlement.java (1)
42-45: updateUserSettlement 메서드명 변경 영향 확인 및 도메인 검증 보강 제안리네임(updateSettlement → updateUserSettlement)은 호출부 전수(서비스·컨트롤러)에서 모두 일관되게 적용되어 컴파일/런타임 영향은 없습니다.
다만 도메인 무결성과 상태 전이 검증을 강화하면 좋겠습니다.검토할 코드 위치
- src/main/java/com/example/onlyone/domain/settlement/entity/UserSettlement.java:42–45
- (참고) src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java:162, 290
제안사항
- null 방어 추가
- settlementStatus에
@NotNull제약이 있으므로, null 인자가 들어올 경우 즉시 예외 처리해 flush 시점 에러를 예방하세요.- 상태 전이 규칙 검증
- 예:
COMPLETED상태로 전환 시completedTime이 null이 아니어야 함을 체크- Clock 주입 고려
- 도메인 로직 내에서 완료 시간을 자동 설정하려면
Clock을 주입해LocalDateTime.now(clock)을 사용하면 테스트 용이성과 일관성을 확보할 수 있습니다public void updateUserSettlement(SettlementStatus settlementStatus, LocalDateTime completedTime) { - this.settlementStatus = settlementStatus; - this.completedTime = completedTime; + if (settlementStatus == null) { + throw new IllegalArgumentException("settlementStatus must not be null"); + } + if (settlementStatus == SettlementStatus.COMPLETED && completedTime == null) { + throw new IllegalStateException("completedTime must be provided when marking as COMPLETED"); + } + this.settlementStatus = settlementStatus; + this.completedTime = completedTime; }src/main/java/com/example/onlyone/domain/schedule/controller/ScheduleController.java (1)
30-32: 201 Created 응답에 Location 헤더 추가 및 제네릭 명시 제안생성 성공 시 리소스 식별자를 포함한 Location 헤더를 제공하면 API 사용성이 높아집니다. 또한 ResponseEntity<?> 대신 구체 제네릭을 명시하면 문서화/정적분석 품질이 개선됩니다.
해당 범위 내 수정 예시:
- ScheduleCreateResponseDto responseDto = scheduleService.createSchedule(clubId, requestDto); - return ResponseEntity.status(HttpStatus.CREATED).body(CommonResponse.success(responseDto)); + ScheduleCreateResponseDto responseDto = scheduleService.createSchedule(clubId, requestDto); + java.net.URI location = org.springframework.web.servlet.support.ServletUriComponentsBuilder + .fromCurrentRequest() + .path("/{scheduleId}") + .buildAndExpand(responseDto.getScheduleId()) + .toUri(); + return ResponseEntity.created(location) + .body(CommonResponse.success(responseDto));추가로 시그니처를 다음처럼 구체화하는 것도 고려해 주세요:
ResponseEntity<CommonResponse>.src/test/java/com/example/onlyone/domain/schedule/repository/ScheduleRepositoryTest.java (3)
49-68: 저장 엔티티 핸들 보유 후 상태 검증까지 포함하도록 강화updateExpiredSchedules가 실제 대상 스케줄의 상태를 변경했는지까지 검증하면 테스트 신뢰도가 높아집니다. 저장 시 반환값을 변수로 받도록 변경을 제안합니다.
해당 범위 내 수정:
- scheduleRepository.save(Schedule.builder() + Schedule past = scheduleRepository.save(Schedule.builder() .club(club) .name("과거 스케줄") .location("장소") .cost(1000) .userLimit(10) .scheduleStatus(ScheduleStatus.READY) .scheduleTime(LocalDateTime.now().minusHours(1)) .build()); - scheduleRepository.save(Schedule.builder() + Schedule future = scheduleRepository.save(Schedule.builder() .club(club) .name("미래 스케줄") .location("장소") .cost(1000) .userLimit(10) .scheduleStatus(ScheduleStatus.READY) .scheduleTime(LocalDateTime.now().plusHours(1)) .build());
72-76: 단순 건수 외에 상태값까지 단언 추가건수만 검증하면 where 조건/파라미터 순서가 바뀌는 회귀를 잡기 어렵습니다. 갱신 후 past만 ENDED, future는 READY 유지 여부를 함께 단언해 주세요.
해당 범위 내 추가 단언 예시:
int updated = scheduleRepository.updateExpiredSchedules( ScheduleStatus.ENDED, ScheduleStatus.READY, LocalDateTime.now()); assertThat(updated).isEqualTo(1); + // 상태 재조회 단언 + Schedule reloadedPast = entityManager.find(Schedule.class, past.getScheduleId()); + Schedule reloadedFuture = entityManager.find(Schedule.class, future.getScheduleId()); + assertThat(reloadedPast.getScheduleStatus()).isEqualTo(ScheduleStatus.ENDED); + assertThat(reloadedFuture.getScheduleStatus()).isEqualTo(ScheduleStatus.READY);
26-33: 불필요한 의존 주입 정리 권장(UserRepository 미사용)UserRepository는 본 테스트에서 사용되지 않습니다. 의존을 최소화하면 테스트 가독성과 유지보수성이 좋아집니다.
src/main/java/com/example/onlyone/domain/settlement/controller/SettlementController.java (2)
27-29: 자동 정산 트리거의 멱등성/에러 매핑 확인automaticSettlement가 중복 호출에 대해 멱등성을 보장하는지(예: 이미 처리 중/완료 상태)와 예외 매핑을 일관되게 CommonResponse 에러 코드로 변환하는지 확인이 필요합니다. 필요 시 202 Accepted(비동기)로의 전환도 고려 가능합니다.
31-35: Deprecated 엔드포인트의 OpenAPI 표시도 함께 반영@deprecated만으로는 스웨거 문서에 확실히 반영되지 않을 수 있습니다. @operation(deprecated = true)로 명시하면 클라이언트에게 더 명확합니다.
해당 범위 내 수정:
- @Operation(summary = "스케줄 참여자 정산", description = "정기 모임의 참여자가 정산을 진행합니다.") + @Operation(summary = "스케줄 참여자 정산", description = "정기 모임의 참여자가 정산을 진행합니다.", deprecated = true) @PostMapping("/user") @Deprecatedsrc/main/java/com/example/onlyone/domain/wallet/service/WalletService.java (3)
51-53: 지갑 조회를 비잠금(read)으로 바꾼 의도 확인 + 읽기 트랜잭션 권장목록 조회 용도라면 비잠금 접근이 타당합니다. 다만 클래스 레벨 @transactional에 의해 기본이 쓰기 트랜잭션입니다. 불필요한 쓰기 트랜잭션과 플러시 비용을 피하려면 메서드에 readOnly=true를 명시해 주세요.
적용 예시:
- public WalletTransactionResponseDto getWalletTransactionList(Filter filter, Pageable pageable) { + @Transactional(readOnly = true) + public WalletTransactionResponseDto getWalletTransactionList(Filter filter, Pageable pageable) {
66-81: N+1 가능성: DTO 변환 시 연관 로딩 최적화 필요convertToDto에서 schedule → club 체인을 접근합니다. 트랜잭션 내 LAZY 로딩이 다수 발생하면 N+1이 나올 수 있습니다. 조회 쿼리 쪽에서 필요한 연관(transfer.userSettlement.settlement.schedule.club)을 fetch join 하거나, 전용 프로젝션/DTO 쿼리를 도입하는 것을 권장합니다.
19-21: 사용하지 않는 import 정리EntityManager, PersistenceContext를 사용하지 않습니다. 정리해 주세요.
-import jakarta.persistence.EntityManager; -import jakarta.persistence.PersistenceContext;src/test/java/com/example/onlyone/domain/user/service/UserServiceTest.java (1)
9-15: 불필요한 import 제거org.mockito.Mock는 사용되지 않습니다. 제거하여 노이즈를 줄여 주세요.
src/main/java/com/example/onlyone/domain/settlement/repository/SettlementRepository.java (1)
18-26: updated_at 등 감사 필드가 있다면 함께 갱신 고려Auditing을 쓰더라도 네이티브 UPDATE는 엔티티 리스너가 동작하지 않습니다. updated_at(또는 modified_at) 컬럼을 운영한다면 함께 갱신하는 것이 좋습니다.
예시:
- UPDATE settlement - SET total_status = 'IN_PROGRESS' + UPDATE settlement + SET total_status = 'IN_PROGRESS', + updated_at = NOW()DB 방언별 함수(NOW/CURRENT_TIMESTAMP)는 환경에 맞게 조정하세요.
src/main/java/com/example/onlyone/domain/user/service/UserService.java (2)
221-227: 웰컴 포인트 하드코딩 분리 권장100000 상수를 코드에 직접 하드코딩하기보다 설정값 또는 상수로 분리하면 테스트/운영 간 조정이 쉬워집니다.
간단한 리팩터링:
public class UserService { + private static final int WELCOME_POINTS = 100_000; @@ - Wallet wallet = Wallet.builder() + Wallet wallet = Wallet.builder() .user(user) - .postedBalance(100000) + .postedBalance(WELCOME_POINTS) .build();또는 application.yml에 user.welcome-points로 두고 @value 주입을 고려하세요.
167-174: Access/Refresh 토큰 subject 불일치 — 파싱 로직과의 정합성 확인Access는 kakaoId를 subject로, Refresh는 userId를 subject로 사용합니다. 토큰 검증/재발급 경로에서 subject를 동일한 의미로 기대한다면 오류가 발생할 수 있습니다. 파싱 로직이 각각 kakaoId/userId를 구분 처리하는지 확인하거나 subject를 통일하고 구분은 claim으로 두는 것을 권장합니다.
정렬안(둘 다 kakaoId로 통일하고 userId는 별도 claim):
- return Jwts.builder() - .subject(user.getUserId().toString()) + return Jwts.builder() + .subject(user.getKakaoId().toString()) + .claim("uid", user.getUserId()) .claim("type", "refresh")src/test/java/com/example/onlyone/domain/schedule/controller/ScheduleControllerTest.java (3)
3-6: 불필요한 Club 관련 import 제거해당 테스트 클래스에서는 ClubController/DTO가 사용되지 않습니다. import를 정리해 주세요.
55-71: 성공 케이스에서 Service 스텁 권장정상 생성 테스트는 현재 status만 검증합니다. Controller가 내부적으로 service 반환 DTO를 활용한다면 기본 mock(null 반환)에 의해 NPE 위험이 있습니다. 최소한 생성 경로에 대해 service 스텁을 추가해 주세요.
예시:
when(scheduleService.createSchedule(eq(1L), any(ScheduleRequestDto.class))) .thenReturn(new ScheduleCreateResponseDto(1L));
142-151: 단일 제약조건 검증을 위한 입력 정제userLimit 검증을 의도한 테스트들에서 scheduleTime을 과거로 설정하여 복수의 검증 에러가 동시에 발생할 수 있습니다. 테스트 안정성을 위해 해당 케이스들에서는 scheduleTime을 미래 시점으로 설정하세요.
예시 수정:
- LocalDateTime.now().minusDays(1) + LocalDateTime.now().plusDays(1)Also applies to: 169-174, 257-266, 284-289
src/test/java/com/example/onlyone/domain/wallet/service/WalletServiceTest.java (5)
36-36: 사용하지 않는 @MockBean import 제거 권장이 파일에서는 @MockitoBean을 사용하고 있어 @MockBean import는 불필요합니다. 테스트 슬라이스 전반에서 @MockitoBean으로 일관성을 유지하는 편이 좋습니다.
- import org.springframework.boot.test.mock.mockito.MockBean;
86-91: 시드 데이터(ID 1, 2, 지갑 존재)에 대한 강한 의존 — 테스트 취약
User(1L, 2L)와 해당Wallet이 사전 로드되어 있다는 가정에 의존하고 있어 환경 변경에 쉽게 깨질 수 있습니다. 테스트 픽스처를 테스트 내에서 생성하거나, 최소한 존재 검증/생성 fallback을 추가하는 형태로 자급자족하도록 리팩터링을 권장합니다.원하시면 테스트 픽스처 빌더(예: UserTestFactory, WalletTestFactory) 초안 코드를 제안해 드릴게요.
90-90: 불필요한 saveAndFlush 호출이미 조회된 엔티티에 변경이 없다면
saveAndFlush는 불필요합니다. 제거로 테스트 실행 시간을 조금이나마 줄일 수 있습니다.- walletRepository.saveAndFlush(wallet); + // no-op: 변경 사항이 없으므로 저장 불필요
38-39: 정렬 기준이 없는 상태에서 결과 순서를 가정 — 플래키 테스트 가능성현재
ALL케이스에서 첫 번째가 5000, 두 번째가 10000이라고 가정합니다. 서비스/DB의 정렬 기본값이 바뀌면 순서가 달라져 실패할 수 있습니다. PageRequest에 명시적 Sort를 주입해 순서를 고정하거나, 검증을 순서 비의존적으로 바꾸세요. 정렬 고정 예시는 아래와 같습니다.+ import org.springframework.data.domain.Sort; ... - Pageable pageable = PageRequest.of(0, 10); + Pageable pageable = PageRequest.of(0, 10, Sort.by(Sort.Direction.ASC, "createdAt"));또는 금액 리스트에 대해 순서 무관 검증으로 전환:
- assertThat(response.getUserWalletTransactionList().get(0).getAmount()).isEqualTo(5000); - assertThat(response.getUserWalletTransactionList().get(1).getAmount()).isEqualTo(10000); + assertThat(response.getUserWalletTransactionList()) + .extracting(it -> it.getAmount()) + .containsExactlyInAnyOrder(5000, 10000);Also applies to: 173-173, 187-187, 201-201
171-183: 검증 강도 보강 제안(선택)필터 동작을 보다 명확히 보증하려면 금액 외에 타입(
Type.CHARGE/Type.OUTGOING)이나 결제/정산 연계여부(예: payment/transfer 존재)도 함께 검증하는 것이 좋습니다. 현재 셋업이 이미 양쪽 연계를 구성하고 있으므로 DTO에 노출되는 필드 범위 내에서 추가 assertion을 권장합니다.필요하시면 DTO 스키마에 맞춘 추가 assertion 샘플을 드리겠습니다.
src/main/java/com/example/onlyone/domain/settlement/repository/UserSettlementRepository.java (3)
17-18: 불필요한 import(Map) 제거
java.util.Map은 사용되지 않습니다. 제거하여 경고를 없애세요.import java.util.List; -import java.util.Map;
120-122: 삭제 메서드의 영향도(영향 레코드 수) 반환 권장벌크 삭제의 결과를 호출 측에서 활용할 수 있도록 반환 타입을
int로 바꾸는 것을 권장합니다. 트랜잭션 경계 내에서 처리되는지(서비스 계층 @transactional)도 함께 확인해 주세요.- @Modifying(clearAutomatically = true, flushAutomatically = true) - @Query("delete from UserSettlement us where us.settlement.settlementId = :settlementId") - void deleteAllBySettlementId(@Param("settlementId") Long settlementId); + @Modifying(clearAutomatically = true, flushAutomatically = true) + @Query("delete from UserSettlement us where us.settlement.settlementId = :settlementId") + int deleteAllBySettlementId(@Param("settlementId") Long settlementId);추가로, 대량 삭제 빈도가 높다면
user_settlement.settlement_id에 인덱스가 있는지 점검해 주세요.
117-119: 파생 쿼리 네이밍은 적절 — 단, 호출 측의 상태전이 로직과 함께 사용 검토
findAllBySettlement_SettlementIdAndSettlementStatus는 적절한 파생 쿼리입니다. 다만 동일 세틀먼트에서 상태 전이가 빈번하다면 조회 직후 갱신 간 레이스(TOCTOU)가 발생할 수 있으므로, 필요 시 서비스 계층에서 상태 조건을 포함한 벌크 업데이트/삭제로 전환도 고려해 주세요.src/test/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepositoryTest.java (3)
192-192: 오탈자: 메서드명 ‘Optinal’ → ‘Optional’가독성을 위해 테스트 메서드명을 수정하세요.
- void 리더가_없으면_Optinal_empty를_반환한다() { + void 리더가_없으면_Optional_empty를_반환한다() {
46-58: 시드 데이터 의존(Interest 1L, User 1L) — 테스트 독립성 개선 권장고정 ID 레코드 전제는 데이터 초기화 방식 변경 시 쉽게 깨집니다. 픽스처를 자체 생성하거나, 최소한 존재하지 않을 때 생성하도록 보완해 주세요.
원하시면 간단한 테스트용 엔티티 팩토리/유틸을 제안하겠습니다.
75-86: 헬퍼에서 과도한 flush/clear 호출 — 테스트 성능·단순화 여지각 저장마다
flush/clear를 호출하면 불필요한 DB round-trip이 늘어납니다. 트랜잭션 롤백되는@DataJpaTest환경에서는 케이스 끝에서 한 번만 호출하거나 생략해도 됩니다.UserClub uc = userClubRepository.save( ... ); - entityManager.flush(); - entityManager.clear(); + // flush/clear는 테스트 말미에 1회 또는 필요 시에만 호출 return uc;UserSchedule us = userScheduleRepository.save( ... ); - entityManager.flush(); - entityManager.clear(); + // flush/clear는 테스트 말미에 1회 또는 필요 시에만 호출 return us;Also applies to: 88-99
src/test/java/com/example/onlyone/global/redis/TestRedisConfig.java (1)
12-37: 테스트용 Redis 템플릿 구성 적절 — 해시 직렬화 설정 추가 제안키/값 직렬화는 잘 설정되어 있습니다. 해시 연산(
opsForHash)을 사용하는 테스트까지 고려하면 해시 직렬화도 지정하는 것이 안전합니다.var str = new StringRedisSerializer(); t.setKeySerializer(str); -// t.setValueSerializer(str); t.setValueSerializer(new GenericJackson2JsonRedisSerializer()); + // Hash ops 직렬화 지정 + t.setHashKeySerializer(str); + t.setHashValueSerializer(new GenericJackson2JsonRedisSerializer());src/test/java/com/example/onlyone/domain/settlement/repository/SettlementRepositoryTest.java (2)
70-80: 변수/표기와 실제 스케줄 상태가 어긋납니다scheduleEnded/scheduleInProgress라는 변수명과 주석과 달리 scheduleStatus는 모두 READY로 저장됩니다. 가독성과 의도 전달을 위해 상태를 ENDED/IN_PROGRESS로 맞추거나 변수명을 중립적으로 변경하세요.
가능한 수정 예시:
- .scheduleStatus(ScheduleStatus.READY) + .scheduleStatus(ScheduleStatus.ENDED) ... - .scheduleStatus(ScheduleStatus.READY) + .scheduleStatus(ScheduleStatus.IN_PROGRESS)Also applies to: 82-92
121-126: 단일 원소 가정 대신 의도를 명시하는 단언으로 개선 제안현재 get(0) 접근은 리스트 크기를 1로 가정합니다. 크기 검증과 식별자 검증을 묶어 의도를 더 분명히 표현하면 가독성이 좋아집니다.
예시(AssertJ):
-assertThat(holding).hasSize(1); -assertThat(holding.get(0).getSchedule().getScheduleId()).isEqualTo(scheduleEnded.getScheduleId()); +assertThat(holding) + .singleElement() + .extracting(s -> s.getSchedule().getScheduleId()) + .isEqualTo(scheduleEnded.getScheduleId());src/test/java/com/example/onlyone/domain/club/controller/ClubControllerTest.java (6)
52-70: 성공 케이스에 대한 서비스 목 스텁 추가 권장성공 테스트에서
ClubService가@MockBean이므로 기본 반환값은 null입니다. 컨트롤러 구현이 반환 DTO를 참조한다면 NPE 가능성이 있습니다. 안전하게 스텁을 추가하세요.예시:
ClubRequestDto requestDto = new ClubRequestDto( ... ); + when(clubService.createClub(requestDto)).thenReturn(new ClubCreateResponseDto(1L));
84-86: 유효성 실패 케이스에서의 불필요한 스텁 제거입력값 검증 실패 시 서비스는 호출되지 않으므로 스텁이 불필요합니다. 테스트 노이즈를 줄이기 위해 제거하세요.
- ClubCreateResponseDto responseDto = new ClubCreateResponseDto(1L); - when(clubService.createClub(requestDto)).thenReturn(responseDto);
111-113: 유효성 실패 케이스에서의 불필요한 스텁 제거- ClubCreateResponseDto responseDto = new ClubCreateResponseDto(1L); - when(clubService.createClub(requestDto)).thenReturn(responseDto);
138-140: 유효성 실패 케이스에서의 불필요한 스텁 제거- ClubCreateResponseDto responseDto = new ClubCreateResponseDto(1L); - when(clubService.createClub(requestDto)).thenReturn(responseDto);
165-167: 유효성 실패 케이스에서의 불필요한 스텁 제거- ClubCreateResponseDto responseDto = new ClubCreateResponseDto(1L); - when(clubService.createClub(requestDto)).thenReturn(responseDto);
192-194: 유효성 실패 케이스에서의 불필요한 스텁 제거- ClubCreateResponseDto responseDto = new ClubCreateResponseDto(1L); - when(clubService.createClub(requestDto)).thenReturn(responseDto);src/main/java/com/example/onlyone/domain/settlement/entity/Settlement.java (3)
58-64: 중복된 상태 변경 메서드 정리 제안
update(TotalStatus, LocalDateTime)와updateTotalStatus(TotalStatus)가 역할이 겹칩니다. API 표면을 줄이기 위해 하나로 통합하거나 명확한 네이밍으로 구분하세요.예시:
- public void update(TotalStatus totalStatus, LocalDateTime completedTime) { - this.totalStatus = totalStatus; - this.completedTime = completedTime; - } + public void updateStatus(TotalStatus totalStatus) { + this.totalStatus = totalStatus; + } + public void completeAt(LocalDateTime completedTime) { + this.completedTime = completedTime; + }
33-36: 원시 타입에 대한 @NotNull은 효과 없습니다
int sum은 null이 될 수 없으므로 Bean Validation 측면에서 @NotNull은 의미가 없습니다. DB 제약(@column(nullable=false))만 유지하거나 검증이 필요하면Integer로 변경하세요.- @Column(name = "sum") - @NotNull - private int sum; + @Column(name = "sum", nullable = false) + private int sum;
50-52: 양방향 편의 메서드 추가 권장연관 컬렉션 일관성을 위해 편의 메서드를 제공하면 안전합니다.
예시:
public void addUserSettlement(UserSettlement us) { userSettlements.add(us); us.setSettlement(this); } public void removeUserSettlement(UserSettlement us) { userSettlements.remove(us); us.setSettlement(null); }src/test/java/com/example/onlyone/domain/wallet/repository/WalletRepositoryTest.java (3)
26-32: 테스트 픽스처를 명시적으로 보장하세요 (getReference 사용 시 엔티티 미존재 위험).User(1L), User(2L)가 테스트 DB에 항상 존재한다는 전제가 필요합니다.
getReference는 실제 조회를 지연하므로, 엔티티가 없으면 이후 접근 시점에EntityNotFoundException이 발생할 수 있습니다. 안전하게find/findById로 존재를 확인하거나, @Sql/데이터 시딩을 통해 픽스처를 보장해 주세요.적용 예시:
- user1 = entityManager.getReference(User.class, 1L); - user2 = entityManager.getReference(User.class, 2L); + user1 = entityManager.find(User.class, 1L); + user2 = entityManager.find(User.class, 2L); + assertThat(user1).as("테스트 사용자 1L 존재 확인").isNotNull(); + assertThat(user2).as("테스트 사용자 2L 존재 확인").isNotNull();
170-185: 크레딧 시나리오 검증은 명확하고 충분합니다.
postedBalance만 증가하고pendingOut은 불변임을 검증하는 포인트가 정확합니다. 여기서 추가로 idempotency(동일 호출 반복 시 누적 의도)를 확인하는 테스트를 한 케이스 더 두면 더욱 견고합니다.
56-71: Repository 신규 메서드 커버리지 보강 제안(getPendingOutByUserId).현재 테스트에서
getPendingOutByUserId직접 검증 케이스가 부족합니다. 홀드/해제/캡처 각 단계 후 해당 메서드 반환값과 엔티티 상태가 일치하는지 단정문을 추가하면 리그레션을 방지할 수 있습니다.원하시면 보강 테스트 메서드 초안을 드리겠습니다.
Also applies to: 73-87, 111-126, 153-168
src/main/java/com/example/onlyone/domain/wallet/entity/Wallet.java (1)
34-37: 주석 처리된 레거시 필드는 제거하세요.버전 관리 시스템이 이력을 보관하므로 주석 보관은 불필요합니다. 가독성을 위해 삭제를 권장합니다.
-// @Column(name = "balance") -// @NotNull -// private int balance;src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java (1)
137-170: 정모 비용 변경 시 다수 row UPDATE 루프의 트랜잭션/성능 고려.HOLD_ACTIVE 대상자마다 개별
holdBalanceIfEnough/releaseHoldBalance호출은 왕복이 많습니다. 배치 업데이트(IDs IN (...)) 또는 벌크 처리로 DB 라운드트립을 줄이는 방안을 고려해 주세요. 실패 시 전체 롤백되는 현재 구조는 타당합니다.필요 시 QueryDSL/JPA 벌크 업데이트 예시를 제공할 수 있습니다.
src/main/java/com/example/onlyone/domain/payment/service/PaymentService.java (1)
183-205: 실패 트랜잭션 중복 방지 로직은 적절하지만, 상태 갱신과 함께 묶으세요.
existingFailTx가 있다면 상태 정리는 이미 끝난 상태이므로 그대로 return 하는 현재 로직은 좋습니다. 다만 앞선 코멘트의 상태 전이(CANCELED) 반영이 선행되어야 전체 플로우가 일관됩니다.원하시면 실패 사유/타임스탬프를
Payment나WalletTransaction메타필드로 남기는 확장 예시를 드릴 수 있습니다.src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (2)
1048-1090: 테스트 이름과 검증 내용이 상충합니다.테스트명은 “이미 해제나 완료된 정모에 참여 취소하면 예외 발생”인데, 실제로는 예외를 기대하지 않고 멱등 처리(상태 변화 없음)를 검증합니다. 의도에 맞게 이름을 바꾸거나, 예외를 기대하도록 로직/검증을 정렬해 주세요.
적용 예시:
이미_해제나_완료된_정모는_참여_취소_요청을_무시한다
1334-1386: 정렬 테스트에서 Thread.sleep은 불필요합니다.정렬 기준이
scheduleTime DESC이므로 서로 다른scheduleTime만 주면 충분합니다. Sleep 없이도 안정적으로 통과합니다.- Thread.sleep(10); ... - Thread.sleep(10);src/main/java/com/example/onlyone/domain/schedule/repository/ScheduleRepository.java (4)
6-6: 불필요한 외부 라이브러리 import 제거 필요
com.google.api.gax.rpc.ServerStream는 본 리포지토리에서 사용되지 않으며(GCP GAX 의존 추가 가능성), 불필요한 의존 경로를 노출합니다. 삭제해주세요.-import com.google.api.gax.rpc.ServerStream;
17-18: 주석으로 비활성화된 메서드는 제거 또는 @deprecated로 명시주석 상태로 남기면 공개 API 의도를 흐립니다. 실제로 미사용이라면 삭제하거나, 호환성 유지 목적이면
@Deprecated주석으로 명확히 해주세요.-// List<Schedule> findByClubAndScheduleStatusNot(Club club, ScheduleStatus scheduleStatus); -// List<Schedule> findByScheduleStatus(ScheduleStatus scheduleStatus);
20-24: 벌크 업데이트 시 flush 전략 보완 제안
clearAutomatically = true설정은 1차 캐시 정합성에 유리합니다. 변경 전 엔티티 변경분이 남아 있을 수 있다면flushAutomatically = true도 함께 두는 것이 안전합니다.- @Modifying(clearAutomatically = true) + @Modifying(flushAutomatically = true, clearAutomatically = true)
26-30: 클럽별 조회에 대한 페이징/인덱스 고려
- 대규모 클럽의 스케줄이 많은 경우
List반환은 메모리 부담이 큽니다. 페이징(Page/Slice) 메서드 추가를 고려해 주세요.findByNameAndClub_ClubId는 중복 검증 용도로 보입니다. DB 레벨에서도(club_id, name)유니크 인덱스를 권장합니다. 케이스 인식이 문제라면 정규화/CI 인덱스도 검토.예시(추가 구현 제안):
Page<Schedule> findAllByClubOrderByScheduleTimeDesc(Club club, Pageable pageable);src/test/java/com/example/onlyone/domain/settlement/repository/UserSettlementRepositoryTest.java (1)
173-199: 테스트 메서드명 오타 및 경계 조건 보강 제안
- 메서드명 끝의 'O'는 오타로 보입니다. 명확성을 위해 제거 권장.
cutoff경계(= 정확히 24시간 전) 포함/제외 케이스를 분리 테스트로 추가하면 안정성이 올라갑니다(=>=vs>).-void 사용자의_최근완료_요청_실패_정산목록을_반환한다O() { +void 사용자의_최근완료_요청_실패_정산목록을_반환한다() {src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (5)
50-56: 미사용 import 정리
@Rollback,TransactionStatus는 사용되지 않습니다. 불필요한 import를 정리해주세요.-import org.springframework.test.annotation.Rollback; ... -import org.springframework.transaction.TransactionStatus;
169-170: 1차 캐시 영향 최소화를 위한 clear 재활성화 제안설정/전처리에서 상태 변경 후 바로 서비스 로직을 검증합니다.
entityManager.clear()를 활성화하면 테스트가 영속성 컨텍스트 상태에 덜 의존적이어서 실제 동작과의 괴리를 줄일 수 있습니다.-// entityManager.clear(); + entityManager.clear();
223-250: 테스트 명 오타와 시나리오-에러코드 불일치
- 메서드명에
ENDND오타가 있습니다.- 본 테스트는 사실상 "시작 전(READY) 정모에 대한 요청"을 검증하므로, 명칭도 해당 시나리오를 반영하면 가독성이 좋아집니다.
-void 상태가_ENDND가_아니고_시작_전인_정모에_정산_요청하면_예외가_발생한다() throws Exception { +void 시작_전_정모에_정산_요청하면_BEFORE_SCHEDULE_END_예외가_발생한다() throws Exception {
313-330: 테스트 설명과 실제 검증 불일치(네이밍 개선)테스트명은 "이미 진행 중/완료된 정산"을 언급하지만, 실제 예외는
BEFORE_SCHEDULE_END를 기대합니다. 시나리오를 반영해 명칭을 조정해 주세요.
437-508: 동시성/멱등성 검증 훌륭함 + 타임아웃 여유 증가 제안
- 정확히 1건 성공, 나머지
ALREADY_SETTLING_SCHEDULE검증이 명료합니다. 굿.- CI 환경 변동성을 고려해
await타임아웃을 10초→30초로 늘리는 것을 권장합니다.- doneGate.await(10, TimeUnit.SECONDS); + doneGate.await(30, TimeUnit.SECONDS);src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java (4)
169-176: Mock 인자 매칭을 더 유연하게특정
req인스턴스에만 매칭하면 테스트가 취약해질 수 있습니다. 인자 매처로 완화하는 편이 안전합니다.-when(tossPaymentClient.confirmPayment(req)).thenReturn(resp); +when(tossPaymentClient.confirmPayment(any(ConfirmTossPayRequest.class))).thenReturn(resp);
284-289: 에러 매핑 테스트에서도 인자 매처 사용 권장동일한 이유로
BadRequest매핑 테스트도 인자 매처를 사용하세요.-when(tossPaymentClient.confirmPayment(req)).thenThrow(ex); +when(tossPaymentClient.confirmPayment(any(ConfirmTossPayRequest.class))).thenThrow(ex);
335-377: 재시도 시나리오에서 1차 실패 상태 검증 추가 제안
reportFail호출 후 중간 상태가 기대대로(예:CANCELED/FAILED) 저장되었는지도 함께 단언하면 회귀 방지가 강화됩니다.예시 단언 추가:
Payment p0 = paymentRepository.findByTossOrderId(orderId).orElseThrow(); assertThat(p0.getStatus()).isEqualTo(Status.CANCELED);
445-449: 불필요한 sleep 제거 제안중복 실패 로그 방지 테스트에서
Thread.sleep(5)는 테스트를 느리게 하고 비결정성을 유발할 수 있습니다. ID/키 기반 멱등 제어라면 sleep 없이도 동일 검증이 가능합니다.src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java (3)
4-4: 이름 충돌 및 미사용 import 정리(+와일드카드 지양)
com.example.onlyone.domain.notification.entity.Type와com.example.onlyone.domain.wallet.entity.Type의 이름 충돌 가능성이 있습니다. 현재는 지시 import가 우선하지만, 가독성과 안전을 위해 지양해야 합니다.TransferRepository,WalletTransactionRepository,ApplicationEventPublisher는 사용되지 않습니다.wallet.entity.*와일드카드는 필요한 타입만 단건 import로 바꿔 주세요.-import com.example.onlyone.domain.notification.entity.Type; +import com.example.onlyone.domain.notification.entity.Type; // 사용됨 ... -import com.example.onlyone.domain.wallet.entity.*; +import com.example.onlyone.domain.wallet.entity.Wallet; +import com.example.onlyone.domain.wallet.entity.WalletTransaction; +import com.example.onlyone.domain.wallet.entity.WalletTransactionStatus; ... -import com.example.onlyone.domain.settlement.repository.TransferRepository; ... -import com.example.onlyone.domain.wallet.repository.WalletTransactionRepository; ... -import org.springframework.context.ApplicationEventPublisher;Also applies to: 20-23, 28-28
99-109: 비용 0원/참여자 1명 분기: 삭제 로직 간소화 및 변수 사용 일관성
findBySchedule(schedule)의 결과를ifPresent로 받으면서, 내부에서는 외부settlement변수를 참조하고 있습니다. NPE/논리 혼동 가능성이 있어 콜백 인자(s)를 사용해 일관성 있게 정리해 주세요.- schedule.removeSettlement(settlement); - settlementRepository.findBySchedule(schedule).ifPresent(s -> { - userSettlementRepository.deleteAllBySettlementId(settlement.getSettlementId()); - settlementRepository.deleteByScheduleId(schedule.getScheduleId()); - }); + schedule.removeSettlement(settlement); + settlementRepository.findBySchedule(schedule).ifPresent(s -> { + userSettlementRepository.deleteAllBySettlementId(s.getSettlementId()); + settlementRepository.deleteByScheduleId(schedule.getScheduleId()); + });
112-121: 선점 후 상태 재검증 로직의 시점 오류(구상태 사용)
markProcessing으로 DB 상태를 IN_PROGRESS로 바꾼 직후, 영속 객체settlement의 구상태(HOLDING/FAILED)로 다시 검증하고 있습니다. 이 검증은 의미가 없거나 오해의 소지가 있습니다. 선점은updated == 1여부만으로 충분하므로 2차 검증을 제거하거나, 필요 시 fresh read로 재검증하세요.- // 이미 정산 중인 스케줄 예외 처리 - if (!((settlement.getTotalStatus() == TotalStatus.HOLDING) || (settlement.getTotalStatus() == TotalStatus.FAILED))) { - throw new CustomException(ErrorCode.ALREADY_SETTLING_SCHEDULE); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (48)
.gitignore(1 hunks)build.gradle(2 hunks)src/main/java/com/example/onlyone/domain/chat/entity/UserChatRoom.java(2 hunks)src/main/java/com/example/onlyone/domain/club/dto/request/ClubRequestDto.java(1 hunks)src/main/java/com/example/onlyone/domain/club/entity/Club.java(2 hunks)src/main/java/com/example/onlyone/domain/club/repository/ClubRepository.java(1 hunks)src/main/java/com/example/onlyone/domain/club/repository/UserClubRepository.java(1 hunks)src/main/java/com/example/onlyone/domain/payment/entity/Payment.java(2 hunks)src/main/java/com/example/onlyone/domain/payment/service/PaymentService.java(4 hunks)src/main/java/com/example/onlyone/domain/schedule/controller/ScheduleController.java(2 hunks)src/main/java/com/example/onlyone/domain/schedule/dto/request/ScheduleRequestDto.java(1 hunks)src/main/java/com/example/onlyone/domain/schedule/dto/response/ScheduleCreateResponseDto.java(1 hunks)src/main/java/com/example/onlyone/domain/schedule/entity/Schedule.java(3 hunks)src/main/java/com/example/onlyone/domain/schedule/entity/ScheduleStatus.java(0 hunks)src/main/java/com/example/onlyone/domain/schedule/repository/ScheduleRepository.java(1 hunks)src/main/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepository.java(1 hunks)src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java(12 hunks)src/main/java/com/example/onlyone/domain/settlement/controller/SettlementController.java(1 hunks)src/main/java/com/example/onlyone/domain/settlement/entity/Settlement.java(3 hunks)src/main/java/com/example/onlyone/domain/settlement/entity/SettlementStatus.java(1 hunks)src/main/java/com/example/onlyone/domain/settlement/entity/TotalStatus.java(1 hunks)src/main/java/com/example/onlyone/domain/settlement/entity/UserSettlement.java(1 hunks)src/main/java/com/example/onlyone/domain/settlement/repository/SettlementRepository.java(1 hunks)src/main/java/com/example/onlyone/domain/settlement/repository/UserSettlementRepository.java(2 hunks)src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java(8 hunks)src/main/java/com/example/onlyone/domain/user/repository/UserRepository.java(0 hunks)src/main/java/com/example/onlyone/domain/user/service/UserService.java(2 hunks)src/main/java/com/example/onlyone/domain/wallet/entity/Wallet.java(1 hunks)src/main/java/com/example/onlyone/domain/wallet/entity/WalletTransaction.java(1 hunks)src/main/java/com/example/onlyone/domain/wallet/repository/WalletRepository.java(1 hunks)src/main/java/com/example/onlyone/domain/wallet/service/WalletService.java(3 hunks)src/main/java/com/example/onlyone/global/exception/ErrorCode.java(2 hunks)src/main/java/com/example/onlyone/global/feign/TossPaymentClient.java(1 hunks)src/test/java/com/example/onlyone/domain/club/controller/ClubControllerTest.java(1 hunks)src/test/java/com/example/onlyone/domain/club/service/ClubServiceTest.java(1 hunks)src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java(1 hunks)src/test/java/com/example/onlyone/domain/schedule/controller/ScheduleControllerTest.java(1 hunks)src/test/java/com/example/onlyone/domain/schedule/repository/ScheduleRepositoryTest.java(1 hunks)src/test/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepositoryTest.java(1 hunks)src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java(1 hunks)src/test/java/com/example/onlyone/domain/settlement/repository/SettlementRepositoryTest.java(1 hunks)src/test/java/com/example/onlyone/domain/settlement/repository/UserSettlementRepositoryTest.java(1 hunks)src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java(1 hunks)src/test/java/com/example/onlyone/domain/user/service/UserServiceTest.java(1 hunks)src/test/java/com/example/onlyone/domain/wallet/repository/WalletRepositoryTest.java(1 hunks)src/test/java/com/example/onlyone/domain/wallet/service/WalletServiceTest.java(1 hunks)src/test/java/com/example/onlyone/global/redis/TestRedisConfig.java(1 hunks)src/test/java/com/example/onlyone/global/redis/TestRedisContainerConfig.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/example/onlyone/domain/user/repository/UserRepository.java
- src/main/java/com/example/onlyone/domain/schedule/entity/ScheduleStatus.java
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-31T06:34:26.437Z
Learnt from: gkdudans
PR: GoormOnlyOne/OnlyOne-Back#53
File: src/main/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepository.java:22-23
Timestamp: 2025-07-31T06:34:26.437Z
Learning: In the OnlyOne application's schedule domain, every schedule always has exactly one leader and the leader always exists. The findLeaderByScheduleAndScheduleRole method in UserScheduleRepository returns a User (not Optional<User>) because the business logic guarantees that a leader will always be present. However, explicit null checking should be added in the service layer for safety.
Applied to files:
src/main/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepository.javasrc/test/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepositoryTest.javasrc/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java
📚 Learning: 2025-07-31T06:40:23.207Z
Learnt from: gkdudans
PR: GoormOnlyOne/OnlyOne-Back#53
File: src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java:52-62
Timestamp: 2025-07-31T06:40:23.207Z
Learning: In the OnlyOne application's ScheduleService, the user has refactored the updateScheduleStatus scheduled method to use JPQL batch update instead of iterating through individual entities for better performance. They are also considering future improvements with Spring Batch or QueryDSL update functionality for more complex batch processing scenarios.
Applied to files:
src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.javasrc/main/java/com/example/onlyone/domain/schedule/repository/ScheduleRepository.javasrc/main/java/com/example/onlyone/domain/schedule/entity/Schedule.javasrc/test/java/com/example/onlyone/domain/schedule/repository/ScheduleRepositoryTest.javasrc/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.javasrc/main/java/com/example/onlyone/domain/settlement/repository/SettlementRepository.java
📚 Learning: 2025-08-06T07:54:25.923Z
Learnt from: ghkddlscks19
PR: GoormOnlyOne/OnlyOne-Back#86
File: src/main/java/com/example/onlyone/domain/club/repository/ClubRepository.java:14-18
Timestamp: 2025-08-06T07:54:25.923Z
Learning: In the OnlyOne-Back project, the ClubRepository search methods use List<Object[]> return type instead of Page<Object[]> even with Pageable parameters. This is intentionally designed for infinite scrolling implementation on the frontend, where only the actual data is needed without pagination metadata.
Applied to files:
src/main/java/com/example/onlyone/domain/club/repository/ClubRepository.javasrc/main/java/com/example/onlyone/domain/schedule/repository/ScheduleRepository.java
📚 Learning: 2025-08-11T09:51:57.984Z
Learnt from: gkdudans
PR: GoormOnlyOne/OnlyOne-Back#110
File: src/main/java/com/example/onlyone/domain/wallet/entity/WalletTransactionStatus.java:5-5
Timestamp: 2025-08-11T09:51:57.984Z
Learning: In the OnlyOne project's wallet transaction system (WalletTransactionStatus enum in src/main/java/com/example/onlyone/domain/wallet/entity/WalletTransactionStatus.java), the PENDING status is used as a locking mechanism for concurrency control to prevent duplicate transaction processing, not for retry or timeout handling.
Applied to files:
src/main/java/com/example/onlyone/domain/settlement/entity/SettlementStatus.javasrc/main/java/com/example/onlyone/domain/settlement/entity/TotalStatus.java
🧬 Code graph analysis (13)
src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (3)
src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (1)
ActiveProfiles(57-1484)src/test/java/com/example/onlyone/domain/wallet/service/WalletServiceTest.java (1)
ActiveProfiles(49-212)src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java (1)
ActiveProfiles(50-471)
src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (2)
src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (1)
ActiveProfiles(73-510)src/test/java/com/example/onlyone/domain/club/service/ClubServiceTest.java (1)
ActiveProfiles(36-152)
src/test/java/com/example/onlyone/domain/schedule/controller/ScheduleControllerTest.java (1)
src/test/java/com/example/onlyone/domain/club/controller/ClubControllerTest.java (1)
ExtendWith(30-219)
src/test/java/com/example/onlyone/domain/wallet/service/WalletServiceTest.java (2)
src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (1)
ActiveProfiles(57-1484)src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (1)
ActiveProfiles(73-510)
src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java (2)
src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (1)
ActiveProfiles(57-1484)src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (1)
ActiveProfiles(73-510)
src/test/java/com/example/onlyone/domain/club/controller/ClubControllerTest.java (1)
src/test/java/com/example/onlyone/domain/schedule/controller/ScheduleControllerTest.java (1)
ExtendWith(31-304)
src/test/java/com/example/onlyone/domain/user/service/UserServiceTest.java (2)
src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (1)
ActiveProfiles(57-1484)src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java (1)
ActiveProfiles(50-471)
src/test/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepositoryTest.java (2)
src/test/java/com/example/onlyone/domain/settlement/repository/SettlementRepositoryTest.java (1)
ActiveProfiles(28-154)src/test/java/com/example/onlyone/domain/schedule/repository/ScheduleRepositoryTest.java (1)
ActiveProfiles(22-79)
src/test/java/com/example/onlyone/domain/settlement/repository/SettlementRepositoryTest.java (1)
src/test/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepositoryTest.java (1)
ActiveProfiles(30-208)
src/test/java/com/example/onlyone/domain/schedule/repository/ScheduleRepositoryTest.java (1)
src/test/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepositoryTest.java (1)
ActiveProfiles(30-208)
src/main/java/com/example/onlyone/domain/club/dto/request/ClubRequestDto.java (1)
src/main/java/com/example/onlyone/domain/schedule/dto/request/ScheduleRequestDto.java (1)
Getter(12-42)
src/main/java/com/example/onlyone/domain/schedule/dto/request/ScheduleRequestDto.java (1)
src/main/java/com/example/onlyone/domain/club/dto/request/ClubRequestDto.java (1)
Getter(9-40)
src/test/java/com/example/onlyone/domain/club/service/ClubServiceTest.java (2)
src/test/java/com/example/onlyone/domain/notification/repository/AppNotificationTypeRepositoryTest.java (1)
DataJpaTest(15-39)src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (1)
ActiveProfiles(57-1484)
src/main/java/com/example/onlyone/domain/club/dto/request/ClubRequestDto.java
Show resolved
Hide resolved
...test/java/com/example/onlyone/domain/settlement/repository/UserSettlementRepositoryTest.java
Show resolved
Hide resolved
src/test/java/com/example/onlyone/domain/user/service/UserServiceTest.java
Show resolved
Hide resolved
src/test/java/com/example/onlyone/domain/user/service/UserServiceTest.java
Show resolved
Hide resolved
src/test/java/com/example/onlyone/domain/wallet/repository/WalletRepositoryTest.java
Show resolved
Hide resolved
src/test/java/com/example/onlyone/domain/wallet/repository/WalletRepositoryTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java (1)
318-339: 삭제 시 재무/연관 정리 누락: 홀드 해제·참여자/정산 클린업 필요현재 ChatRoom 삭제·클럽 분리만 수행합니다. HOLD_ACTIVE 예약금 해제, UserSettlement/UserSchedule/유저-채팅 연관 정리 없으면 데이터/잔액 누수가 발생합니다.
다음과 같은 클린업을 추가해 주세요.
ChatRoom chatRoom = chatRoomRepository.findByTypeAndScheduleId(Type.SCHEDULE, scheduleId) .orElseThrow(() -> new CustomException(ErrorCode.CHAT_ROOM_NOT_FOUND)); - club.getSchedules().remove(schedule); - chatRoomRepository.delete(chatRoom); + // 1) 정산/홀드 정리 + settlementRepository.findBySchedule(schedule).ifPresent(s -> { + var targets = userSettlementRepository + .findAllBySettlement_SettlementIdAndSettlementStatus(s.getSettlementId(), SettlementStatus.HOLD_ACTIVE); + for (var us : targets) { + int ok = walletRepository.releaseHoldBalance(us.getUser().getUserId(), schedule.getCost()); + if (ok != 1) throw new CustomException(ErrorCode.WALLET_HOLD_STATE_CONFLICT); + } + userSettlementRepository.deleteAllBySettlementId(s.getSettlementId()); + settlementRepository.deleteByScheduleId(schedule.getScheduleId()); + }); + // 2) 참여자/채팅 연관 제거 + userScheduleRepository.findUsersBySchedule(schedule) + .forEach(usr -> userScheduleRepository.deleteById(usr.getUserId())); // 구현에 맞게 일괄 삭제 API 사용 권장 + // chatRoom ↔ users 매핑도 cascade 정책에 맞게 제거 필요 + club.getSchedules().remove(schedule); + chatRoomRepository.delete(chatRoom);src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java (1)
307-317: 컴파일 오류: STATUS_ROLLED_BACK 심볼 미해결정적 임포트가 없으므로 상수 참조를 정규화해야 합니다.
- if (status == STATUS_ROLLED_BACK) { + if (status == org.springframework.transaction.support.TransactionSynchronization.STATUS_ROLLED_BACK) {또는 상단에 정적 임포트를 추가하세요:
+import static org.springframework.transaction.support.TransactionSynchronization.STATUS_ROLLED_BACK;
♻️ Duplicate comments (2)
src/main/java/com/example/onlyone/domain/settlement/entity/Settlement.java (1)
50-53: @Builder.Default 추가 적절 — NPE 리스크 해소userSettlements 초기화가 보장되어 컬렉션 접근 시 안전합니다.
src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java (1)
183-189: 오류코드/Null 안전성/실패 표시 지속성 문제 (과거 지적 반복)
- 스케줄 미발견 시 SETTLEMENT_NOT_FOUND가 아니라 SCHEDULE_NOT_FOUND여야 합니다.
- failWallet가 null일 수 있어 NPE 위험.
- FAILED 업데이트는 같은 트랜잭션 롤백으로 사라질 수 있습니다(실패 상태 비지속).
다음 패치를 적용해 주세요.
- Schedule failedSchedule = scheduleRepository.findById(scheduleId) - .orElseThrow(() -> new CustomException(ErrorCode.SETTLEMENT_NOT_FOUND)); - Settlement faildSettlement = settlementRepository.findBySchedule(failedSchedule) + Schedule failedSchedule = scheduleRepository.findById(scheduleId) + .orElseThrow(() -> new CustomException(ErrorCode.SCHEDULE_NOT_FOUND)); + Settlement failedSettlement = settlementRepository.findBySchedule(failedSchedule) .orElseThrow(() -> new CustomException(ErrorCode.SETTLEMENT_NOT_FOUND)); - faildSettlement.updateTotalStatus(TotalStatus.FAILED); - registerFailureLogAfterRollback(failWallet.getWalletId(), leaderWallet.getWalletId(), schedule.getCost(), failUserSettlementId, failWallet.getPostedBalance(), leaderWallet.getPostedBalance()); + // 실패 상태는 afterCompletion(ROLLBACK)에서 별도 트랜잭션으로 마킹하도록 위임 권장 + long fwId = (failWallet != null) ? failWallet.getWalletId() : -1L; + int fwBal = (failWallet != null) ? failWallet.getPostedBalance() : 0; + registerFailureLogAfterRollback(fwId, leaderWallet.getWalletId(), schedule.getCost(), failUserSettlementId, fwBal, leaderWallet.getPostedBalance());또한 실패 상태를 커밋 이후에도 보존하려면 afterCompletion(STATUS_ROLLED_BACK)에서 REQUIRES_NEW로 FAILED 마킹을 수행하는 헬퍼 도입을 권장합니다(레포지토리 JPQL 벌크 또는 Tx-헬퍼 서비스). 필요 시 코드 제안 가능합니다.
🧹 Nitpick comments (16)
src/main/java/com/example/onlyone/domain/payment/dto/request/ConfirmTossPayRequest.java (2)
7-7: (선택) 빌더 기반 JSON 역직렬화까지 고려하면 @Jacksonized 추가 권장컨트롤러 바인딩에서 빌더 기반 역직렬화를 고려한다면 Lombok의 @Jacksonized를 함께 사용하면 안전합니다(@NoArgsConstructor가 있는 현재 구조도 동작 가능).
적용 예:
import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Getter; import lombok.NoArgsConstructor; +import lombok.extern.jackson.Jacksonized; @Getter @AllArgsConstructor @NoArgsConstructor @Builder +@Jacksonized public class ConfirmTossPayRequest {Also applies to: 14-15
24-26: (선택) 금액 검증 강화 (@positive 추가)음수/0 금액을 조기에 차단하려면 @positive를 병행하는 편이 안전합니다.
import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; @NotNull + @Positive @JsonProperty("amount") private Long amount;src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java (4)
78-86: (선택) 고정된 시드 데이터(사용자 ID=1) 의존성 제거테스트 초기화에서 findById(1L)·findByUser(user)에 의존하면 외부 시드 변화에 취약합니다. 테스트 내부에서 사용자·지갑을 명시 생성/저장하도록 전환을 권장합니다.
원하시면 현재 엔티티 스키마에 맞춘 픽스처 팩토리/DSL 템플릿을 제안하겠습니다.
167-171: (선택) DTO 모킹 대신 실제 객체 사용 권장요청 DTO를 Mockito.mock으로 스텁하면 직렬화/검증 어댑터와의 호환성 문제가 숨겨질 수 있습니다. ConfirmTossPayRequest는 빌더가 있으므로 실제 인스턴스를 사용하는 편이 테스트 신뢰도를 높입니다. SavePaymentRequestDto도 생성자/세터가 가능하면 실제 객체 사용을 권장합니다.
예시 헬퍼(테스트 클래스 내부 정적 메서드):
private static ConfirmTossPayRequest confirmReq(String orderId, long amount, String key) { return ConfirmTossPayRequest.builder() .orderId(orderId) .amount(amount) .paymentKey(key) .build(); }사용 예(일부 케이스 교체):
- var req = mock(ConfirmTossPayRequest.class); - when(req.getOrderId()).thenReturn(orderId); - when(req.getAmount()).thenReturn(amount); - when(req.getPaymentKey()).thenReturn(paymentKey); + var req = confirmReq(orderId, amount, paymentKey);Also applies to: 200-204, 240-244, 350-353, 373-376, 397-400, 416-420, 428-433, 106-109
219-341: 동시성·멱등 테스트 구성 우수
- 트랜잭션 분리(TransactionTemplate), 래치, 파라미터화로 재현성 확보
- PG 호출 1회 검증으로 선점 가드 보장 확인
사소한 제안:
- System.out.printf 대신 로거/테스트 리포터 사용을 권장합니다.
- System.out.printf("threads=%d, success=%d, already=%d, progress=%d, unexpected=%s%n", - threads, success.get(), already.get(), progress.get(), summary); + // log.debug(...) 혹은 테스트 보고용 출력 유틸 사용또한, 완료 대기 후에는 shutdown()이면 충분합니다.
- pool.shutdownNow(); + pool.shutdown();
82-84: updateBalance는 Setter로 구현되어 있어 초기화 목적에 부합합니다
Wallet.updateBalance(int balance)는 내부적으로 곧바로postedBalance를 덮어쓰는(set) 로직으로 구현되어 있으므로, 테스트에서updateBalance(0)를 호출해 잔액을 0으로 초기화하는 동작이 의도대로 수행됩니다.검증 결과
- src/main/java/com/example/onlyone/domain/wallet/entity/Wallet.java
→ balance를 누적(증감)하지 않고 직접 설정(set)함 확인public void updateBalance(int balance) { this.postedBalance = balance; }따라서 현재 테스트 코드(
PaymentServiceTest.java82–84번 라인)에서wallet.updateBalance(0)호출은 안전하며, 초기화 용도로 사용할 수 있습니다.권장 사항 (선택적 리팩토링)
- 메서드 이름의 모호성을 줄이기 위해
updateBalance를setBalance또는setPostedBalance로 변경하여 ‘설정’ 의도를 명확히 하는 것을 고려해 보세요.src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java (4)
141-169: 비용 변경 시 홀드 증감 처리의 경계조건 정교화 제안
- memberCount로 분기하기보다 targets.isEmpty() 기준으로 처리 여부를 결정하는 편이 정확합니다(HOLD_ACTIVE 참여자만 대상).
- 증액 실패 시 예외가 발생하면 앞선 성공분은 트랜잭션 롤백으로 되돌아가므로 OK. 다만 대상을 먼저 조회한 뒤 없으면 조기 반환하는 것이 불필요한 DB 왕복을 줄입니다.
다음과 같이 간결화 가능합니다.
- if (schedule.getCost() != requestDto.getCost()) { - int memberCount = userScheduleRepository.countBySchedule(schedule) - 1; - int delta = requestDto.getCost() - schedule.getCost(); - if (memberCount > 0 && delta != 0) { - List<UserSettlement> targets = - userSettlementRepository.findAllBySettlement_SettlementIdAndSettlementStatus( - schedule.getSettlement().getSettlementId(), SettlementStatus.HOLD_ACTIVE); + if (schedule.getCost() != requestDto.getCost()) { + int delta = requestDto.getCost() - schedule.getCost(); + if (delta != 0) { + List<UserSettlement> targets = userSettlementRepository + .findAllBySettlement_SettlementIdAndSettlementStatus( + schedule.getSettlement().getSettlementId(), SettlementStatus.HOLD_ACTIVE); + if (targets.isEmpty()) { + // 조정 대상 없음 + schedule.update(...); // 기존 업데이트 유지 + scheduleRepository.save(schedule); + return; + }
175-205: 참여 전 잔액 홀드 순서 적절함 + DB 유니크 제약 권장잔액 홀드를 먼저 수행 후 UserSchedule/채팅 등록하는 순서는 정합성에 유리합니다. 다만 동시 요청 방지를 위해 UserSchedule(user_id, schedule_id)와 UserSettlement(user_id, settlement_id)에 유니크 인덱스를 추천합니다.
221-227: UserSettlement 관계 역방향 연결 주석 해제 고려정산 상세 조회/지우기 시 컬렉션 일관성을 위해 Settlement.userSettlements에 추가하는 헬퍼를 두고 양방향 연계를 유지하는 편이 안전합니다.
-// settlement.updateUserSettlement(userSettlement); +// 헬퍼 메서드 예시: settlement.addUserSettlement(userSettlement);
241-263: FAILED 상태 처리의 멱등성 기준 재검토현재 FAILED도 해제 대상으로 포함되어 releaseHoldBalance 실패 시 WALLET_HOLD_STATE_CONFLICT를 던집니다. FAILED 의미가 “이미 홀드 없음”이라면 FAILED는 조용히 반환하는 편이 더 멱등적입니다.
- if (!(userSettlement.getSettlementStatus() == SettlementStatus.HOLD_ACTIVE - || userSettlement.getSettlementStatus() == SettlementStatus.FAILED)) { + if (userSettlement.getSettlementStatus() != SettlementStatus.HOLD_ACTIVE) { return; }src/main/java/com/example/onlyone/domain/settlement/controller/SettlementController.java (1)
31-37: 주석 보관된 Deprecated 엔드포인트는 제거 권장히스토리는 Git이 보존합니다. 주석 코드는 유지 보수성을 해칩니다.
src/main/java/com/example/onlyone/domain/settlement/entity/Settlement.java (1)
28-31: 연관관계 cascade 제거에 따른 운영 상 주의Schedule/receiver에서 cascade를 제거했으므로 수동 삭제/정리(예: 스케줄 삭제 시 정산/유저정산/홀드 해제)가 필요합니다. 상위 서비스 레이어에 동일 클린업 로직이 구현되어 있는지 확인하세요.
src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (3)
130-183: 픽스처 영속성 경계 명확화setUp 마지막의 entityManager.clear()가 주석 처리되어 있어 1차 캐시 상태가 테스트에 스며들 수 있습니다. flush+clear로 DB 상태 기반 검증을 일관되게 유지하세요.
- entityManager.flush(); -// entityManager.clear(); + entityManager.flush(); + entityManager.clear();
366-389: FAILED 상태 지속성 검증이 실제 커밋 후 보장되는지 확인 필요서비스 catch 블록의 FAILED 업데이트는 동일 트랜잭션에서 롤백될 수 있습니다. 커밋 경계를 끊고 재조회로 FAILED가 저장됐는지 검증하는 테스트를 추가하세요.
예시:
var tt = new TransactionTemplate(txManager); assertThrows(CustomException.class, () -> tt.execute(s -> { settlementService.automaticSettlement(...); return null; })); TestTransaction.start(); var reloaded = settlementRepository.findBySchedule(schedule).orElseThrow(); assertThat(reloaded.getTotalStatus()).isEqualTo(TotalStatus.FAILED); TestTransaction.end();
236-263: 테스트 이름 오탈자‘ENDND’ → ‘ENDED’.
- void 상태가_ENDND가_아니고_시작_전인_정모에_정산_요청하면_예외가_발생한다() throws Exception { + void 상태가_ENDED가_아니고_시작_전인_정모에_정산_요청하면_예외가_발생한다() throws Exception {src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java (1)
110-120: 중복 선점 검사 정리markProcessing가 1건 선점에 성공했다면, 직후의 in-memory 상태 검사(총상태가 HOLDING/FAILED인지 재확인)는 중복입니다. 제거하여 가독성을 높이세요.
- if (!((settlement.getTotalStatus() == TotalStatus.HOLDING) || (settlement.getTotalStatus() == TotalStatus.FAILED))) { - throw new CustomException(ErrorCode.ALREADY_SETTLING_SCHEDULE); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
build.gradle(5 hunks)src/main/java/com/example/onlyone/domain/payment/dto/request/ConfirmTossPayRequest.java(1 hunks)src/main/java/com/example/onlyone/domain/payment/service/PaymentService.java(4 hunks)src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java(12 hunks)src/main/java/com/example/onlyone/domain/settlement/controller/SettlementController.java(1 hunks)src/main/java/com/example/onlyone/domain/settlement/entity/Settlement.java(3 hunks)src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java(3 hunks)src/main/java/com/example/onlyone/domain/wallet/entity/WalletTransaction.java(2 hunks)src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java(1 hunks)src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java(1 hunks)src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/example/onlyone/domain/payment/service/PaymentService.java
- src/main/java/com/example/onlyone/domain/wallet/entity/WalletTransaction.java
- src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java
- build.gradle
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-31T06:40:23.207Z
Learnt from: gkdudans
PR: GoormOnlyOne/OnlyOne-Back#53
File: src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java:52-62
Timestamp: 2025-07-31T06:40:23.207Z
Learning: In the OnlyOne application's ScheduleService, the user has refactored the updateScheduleStatus scheduled method to use JPQL batch update instead of iterating through individual entities for better performance. They are also considering future improvements with Spring Batch or QueryDSL update functionality for more complex batch processing scenarios.
Applied to files:
src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java
📚 Learning: 2025-07-31T06:34:26.437Z
Learnt from: gkdudans
PR: GoormOnlyOne/OnlyOne-Back#53
File: src/main/java/com/example/onlyone/domain/schedule/repository/UserScheduleRepository.java:22-23
Timestamp: 2025-07-31T06:34:26.437Z
Learning: In the OnlyOne application's schedule domain, every schedule always has exactly one leader and the leader always exists. The findLeaderByScheduleAndScheduleRole method in UserScheduleRepository returns a User (not Optional<User>) because the business logic guarantees that a leader will always be present. However, explicit null checking should be added in the service layer for safety.
Applied to files:
src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java
🧬 Code graph analysis (2)
src/main/java/com/example/onlyone/domain/payment/dto/request/ConfirmTossPayRequest.java (2)
src/main/java/com/example/onlyone/domain/payment/dto/response/ConfirmTossPayResponse.java (2)
Getter(8-29)Getter(21-28)src/main/java/com/example/onlyone/domain/payment/dto/request/SavePaymentRequestDto.java (1)
Getter(7-14)
src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (3)
src/test/java/com/example/onlyone/domain/schedule/service/ScheduleServiceTest.java (1)
ActiveProfiles(58-1535)src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java (1)
ActiveProfiles(56-522)src/test/java/com/example/onlyone/domain/wallet/service/WalletServiceTest.java (1)
ActiveProfiles(49-212)
🔇 Additional comments (12)
src/main/java/com/example/onlyone/domain/payment/dto/request/ConfirmTossPayRequest.java (2)
14-15: 빌더 추가 적절테스트 코드와 사용 패턴(ConfirmTossPayRequest.builder() ...)에 맞게 DTO에 @builder를 도입한 선택이 타당합니다.
24-26: DTO 간 타입 일관성 확인(Long vs long)SavePaymentRequestDto.amount는 primitive long, 본 DTO는 Long입니다. 널 허용 여부 정책에 맞춰 통일하는지 확인 부탁드립니다.
src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java (4)
113-124: Redis TTL/삭제 검증 케이스 적절TTL>0, 삭제 후 getExpire=-2 검증은 명확하고 신뢰성 있습니다.
159-190: 승인 성공 플로우 검증 케이스 충실상태/수단/키/지갑잔액까지 일관 검증되어 회귀 방지에 유용합니다.
343-364: Feign 예외 매핑 테스트 명확400→INVALID_PAYMENT_INFO, 500→TOSS_PAYMENT_FAILED, 런타임→INTERNAL_SERVER_ERROR 매핑이 잘 드러납니다.
Also applies to: 366-388, 390-408
492-497: 중복 실패 로그 방지 케이스 적절동일 paymentKey 재시도 시 단일 WalletTransaction만 유지되는지 검증이 명확합니다.
src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java (2)
112-121: 정산 초기화 로직 자체는 적절합니다.sum=0, TotalStatus.HOLDING으로 초기 세팅 후 Schedule과의 연계를 저장하는 구성은 일관됩니다. 다만, 이후 실패 시 정산/스케줄 정합성 복구 경로(롤백/클린업)가 서비스 레벨에 존재하는지 확인이 필요합니다.
137-140: 상태 검증 OK.READY가 아닌 경우 수정 차단은 타당합니다.
src/main/java/com/example/onlyone/domain/settlement/controller/SettlementController.java (1)
27-29: automaticSettlement로의 위임 전환 LGTM엔드포인트 시그니처 유지하며 서비스 진입점을 통합한 방향이 명확합니다.
src/test/java/com/example/onlyone/domain/settlement/service/SettlementServiceTest.java (2)
395-424: afterCompletion 경계 활용 좋습니다실패 로그 콜백을 REQUIRES_NEW 트랜잭션 경계에서 검증하는 접근은 안정적입니다.
446-555: 동시성/멱등성 테스트 우수선점 1건/나머지 ALREADY_SETTLING_SCHEDULE 검증과 사후 멱등성 확인 흐름이 좋습니다. 성공/오류 요약 출력은 디버깅에도 유용합니다.
src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java (1)
155-163: 지갑 트랜잭션 기록 후 UserSettlement 저장 순서 OK멱등키 기반 기록 → 상태 변경 → 저장 순은 타당합니다.
| public ScheduleCreateResponseDto createSchedule(Long clubId, ScheduleRequestDto requestDto) { | ||
| Club club = clubRepository.findById(clubId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.CLUB_NOT_FOUND)); | ||
| Schedule schedule = requestDto.toEntity(club); | ||
| scheduleRepository.save(schedule); | ||
| User user = userService.getCurrentUser(); | ||
| UserClub userClub = userClubRepository.findByUserAndClub(user, club) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.USER_CLUB_NOT_FOUND)); | ||
| if (userClub.getClubRole() != ClubRole.LEADER) { | ||
| throw new CustomException(ErrorCode.MEMBER_CANNOT_CREATE_SCHEDULE); | ||
| } | ||
| UserSchedule userSchedule = UserSchedule.builder() |
There was a problem hiding this comment.
권한 검증이 스케줄 저장 이후 수행되어, 비인가 사용자가 스케줄을 생성할 수 있습니다.
리더 권한 확인을 스케줄/채팅/정산 엔티티 생성·저장 "이전"에 수행하세요. 현재 흐름은 권한 예외 발생 시 이미 생성된 Schedule이 남습니다.
다음과 같이 순서를 재배치해 주세요.
- Schedule schedule = requestDto.toEntity(club);
- scheduleRepository.save(schedule);
- User user = userService.getCurrentUser();
- UserClub userClub = userClubRepository.findByUserAndClub(user, club)
- .orElseThrow(() -> new CustomException(ErrorCode.USER_CLUB_NOT_FOUND));
- if (userClub.getClubRole() != ClubRole.LEADER) {
- throw new CustomException(ErrorCode.MEMBER_CANNOT_CREATE_SCHEDULE);
- }
+ User user = userService.getCurrentUser();
+ UserClub userClub = userClubRepository.findByUserAndClub(user, club)
+ .orElseThrow(() -> new CustomException(ErrorCode.USER_CLUB_NOT_FOUND));
+ if (userClub.getClubRole() != ClubRole.LEADER) {
+ throw new CustomException(ErrorCode.MEMBER_CANNOT_CREATE_SCHEDULE);
+ }
+ Schedule schedule = requestDto.toEntity(club);
+ scheduleRepository.save(schedule);📝 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.
| public ScheduleCreateResponseDto createSchedule(Long clubId, ScheduleRequestDto requestDto) { | |
| Club club = clubRepository.findById(clubId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.CLUB_NOT_FOUND)); | |
| Schedule schedule = requestDto.toEntity(club); | |
| scheduleRepository.save(schedule); | |
| User user = userService.getCurrentUser(); | |
| UserClub userClub = userClubRepository.findByUserAndClub(user, club) | |
| .orElseThrow(() -> new CustomException(ErrorCode.USER_CLUB_NOT_FOUND)); | |
| if (userClub.getClubRole() != ClubRole.LEADER) { | |
| throw new CustomException(ErrorCode.MEMBER_CANNOT_CREATE_SCHEDULE); | |
| } | |
| UserSchedule userSchedule = UserSchedule.builder() | |
| public ScheduleCreateResponseDto createSchedule(Long clubId, ScheduleRequestDto requestDto) { | |
| Club club = clubRepository.findById(clubId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.CLUB_NOT_FOUND)); | |
| User user = userService.getCurrentUser(); | |
| UserClub userClub = userClubRepository.findByUserAndClub(user, club) | |
| .orElseThrow(() -> new CustomException(ErrorCode.USER_CLUB_NOT_FOUND)); | |
| if (userClub.getClubRole() != ClubRole.LEADER) { | |
| throw new CustomException(ErrorCode.MEMBER_CANNOT_CREATE_SCHEDULE); | |
| } | |
| Schedule schedule = requestDto.toEntity(club); | |
| scheduleRepository.save(schedule); | |
| UserSchedule userSchedule = UserSchedule.builder() | |
| // … |
🤖 Prompt for AI Agents
In
src/main/java/com/example/onlyone/domain/schedule/service/ScheduleService.java
around lines 83 to 94, the method performs permission checks after persisting
the Schedule which allows unauthorized users to leave created Schedule rows when
the permission check fails; move the permission verification (fetch current
user, fetch UserClub, assert ClubRole.LEADER) to occur immediately after
resolving the Club and before constructing or saving any
Schedule/Chat/Settlement entities so no DB entities are written on authorization
failure; after the check passes, proceed to build and save the Schedule and
related entities and return the response.
| Schedule failedSchedule = scheduleRepository.findById(scheduleId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.SETTLEMENT_NOT_FOUND)); | ||
| Settlement faildSettlement = settlementRepository.findBySchedule(failedSchedule) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.SETTLEMENT_NOT_FOUND)); | ||
| faildSettlement.updateTotalStatus(TotalStatus.FAILED); | ||
| registerFailureLogAfterRollback(failWallet.getWalletId(), leaderWallet.getWalletId(), schedule.getCost(), failUserSettlementId, failWallet.getPostedBalance(), leaderWallet.getPostedBalance()); | ||
| throw new CustomException(ErrorCode.SETTLEMENT_PROCESS_FAILED); | ||
| } |
There was a problem hiding this comment.
일반 예외 처리 블록 동일 문제 반복
위와 동일하게 SCHEDULE_NOT_FOUND, NPE 가드, 실패 상태 영속 보장을 적용하세요.
- Schedule failedSchedule = scheduleRepository.findById(scheduleId)
- .orElseThrow(() -> new CustomException(ErrorCode.SETTLEMENT_NOT_FOUND));
- Settlement faildSettlement = settlementRepository.findBySchedule(failedSchedule)
+ Schedule failedSchedule = scheduleRepository.findById(scheduleId)
+ .orElseThrow(() -> new CustomException(ErrorCode.SCHEDULE_NOT_FOUND));
+ Settlement failedSettlement = settlementRepository.findBySchedule(failedSchedule)
.orElseThrow(() -> new CustomException(ErrorCode.SETTLEMENT_NOT_FOUND));
- faildSettlement.updateTotalStatus(TotalStatus.FAILED);
- registerFailureLogAfterRollback(failWallet.getWalletId(), leaderWallet.getWalletId(), schedule.getCost(), failUserSettlementId, failWallet.getPostedBalance(), leaderWallet.getPostedBalance());
+ long fwId = (failWallet != null) ? failWallet.getWalletId() : -1L;
+ int fwBal = (failWallet != null) ? failWallet.getPostedBalance() : 0;
+ registerFailureLogAfterRollback(fwId, leaderWallet.getWalletId(), schedule.getCost(), failUserSettlementId, fwBal, leaderWallet.getPostedBalance());Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/example/onlyone/domain/settlement/service/SettlementService.java
around lines 191 to 198, the exception handling should explicitly throw
SCHEDULE_NOT_FOUND, guard against possible NPEs for wallets/settlement, and
ensure the FAILED status is persisted; change the schedule lookup to throw
ErrorCode.SCHEDULE_NOT_FOUND if missing, null-check (or Optional-check) the
settlement and the involved wallet objects before using them, call
settlementRepository.save(failedSettlement) (or otherwise persist the status
change) after faildSettlement.updateTotalStatus(TotalStatus.FAILED), and only
call registerFailureLogAfterRollback once you have validated non-null wallet IDs
and balances (throw an appropriate CustomException if any required object is
missing).
|
|
||
| import static java.nio.charset.StandardCharsets.UTF_8; | ||
| import static org.assertj.core.api.Assertions.*; | ||
| import static org.junit.Assert.assertThrows; |
There was a problem hiding this comment.
JUnit4 Assert import로 인한 컴파일/실행 충돌 가능성
JUnit 5 기반(@test, @ParameterizedTest)에서 org.junit.Assert.assertThrows를 사용하고 있습니다. Jupiter의 Assertions.assertThrows로 교체하세요.
-import static org.junit.Assert.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertThrows;📝 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.
| import static org.junit.Assert.assertThrows; | |
| import static org.junit.jupiter.api.Assertions.assertThrows; |
🤖 Prompt for AI Agents
In
src/test/java/com/example/onlyone/domain/payment/service/PaymentServiceTest.java
around line 52, the test imports and uses JUnit4's org.junit.Assert.assertThrows
which can conflict with JUnit 5 (Jupiter) test runner; replace the JUnit4 import
and usages with org.junit.jupiter.api.Assertions.assertThrows (update the import
and any static import usages) so the tests use Jupiter's Assertions.assertThrows
consistently with @Test/@ParameterizedTest.
[test] 정기 모임 / 정산 / 결제 테스트 코드 추가 및 리팩토링
[test] 정기 모임 / 정산 / 결제 테스트 코드 추가 및 리팩토링
2 participants
#️⃣ Issue Number
📝 요약(Summary)
정기 모임 / 정산 / 결제 테스트 코드 추가 및 리팩토링
🛠️ PR 유형
어떤 변경 사항이 있나요?
📸스크린샷 (선택)
💬 공유사항 to 리뷰어
✅ PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.
Summary by CodeRabbit
New Features
Bug Fixes
Chores