[FEAT] 사장님 - 계좌 CRUD 기능 구현#5
Conversation
- 패키지 구조 수정
WalkthroughUser 도메인 패키지 구조가 domain.user → domain.user.domain.model로 변경되며 참조 전반이 업데이트됨. 오너(사장) 계좌 CRUD 기능이 추가됨: 엔티티/레포지토리/서비스/밸리데이터/컨트롤러/DTO/에러코드 도입. Member/Owner 빈 클래스는 새 패키지로 이동 또는 삭제. UserRepository 추가. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Owner as Owner(사장)
participant OC as OwnerController
participant OS as OwnerService
participant OV as OwnerValidator
participant UR as UserRepository
participant BAS as BankAccountService
participant BAR as BankAccountRepository
rect rgba(200,230,255,0.25)
note over Owner,OC: 등록(POST) / 조회(GET) / 수정(PATCH) / 삭제(DELETE)
end
Owner->>OC: 요청 (등록/조회/수정/삭제)
OC->>OS: delegate(ownerId, payload)
rect rgba(230,255,230,0.25)
OS->>OV: validateAndGetOwner(ownerId)
OV->>UR: findByUserIdAndRoleAndStatus(OWNER, ACTIVE)
UR-->>OV: User or empty
OV-->>OS: User or throw USER_NOT_FOUND
end
alt 등록
OS->>BAS: registerBankAccount(request, owner)
BAS->>BAR: existsByOwner_UserId(ownerId)
BAR-->>BAS: boolean
BAS->>BAR: existsByAccountNumber(accountNumber)
BAR-->>BAS: boolean
BAS->>BAR: save(BankAccount)
BAR-->>BAS: BankAccount
BAS-->>OS: void
else 조회
OS->>BAS: getBankAccount(ownerId)
BAS->>BAR: findByOwnerId(ownerId)
BAR-->>BAS: Optional<BankAccount>
BAS-->>OS: BankAccountResponse or null
else 수정
OS->>BAS: updateBankAccount(ownerId, bankAccountId, request)
BAS->>BAR: findById(bankAccountId)
BAR-->>BAS: Optional<BankAccount>
BAS->>BAS: verifyOwner(ownerId) or throw FORBIDDEN
BAS->>BAR: existsByAccountNumber(newNumber) [if changed]
BAR-->>BAS: boolean
BAS->>BAS: update(fields)
BAS-->>OS: void
else 삭제
OS->>BAS: deleteBankAccount(ownerId, bankAccountId)
BAS->>BAR: findById(bankAccountId)
BAR-->>BAS: Optional<BankAccount>
BAS->>BAS: verifyOwner(ownerId)
BAS->>BAR: delete(entity)
BAR-->>BAS: done
BAS-->>OS: void
end
OS-->>OC: 응답 DTO/BaseResponse
OC-->>Owner: 결과
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/konkuk/chacall/domain/foodtruck/domain/Rating.java (1)
26-32: 회원-푸드트럭 중복 평점 방지 제약 필요.
한 사용자가 한 푸드트럭에 여러 번 평점을 남길 수 없도록 DB 유니크 제약을 권장합니다. 조회 성능을 위한 인덱스도 함께 추가하세요.적용 예:
-@Table(name = "ratings") +@Table( + name = "ratings", + uniqueConstraints = @UniqueConstraint( + name = "uk_ratings_user_foodtruck", + columnNames = {"user_id", "food_truck_id"} + ), + indexes = { + @Index(name = "idx_ratings_user_id", columnList = "user_id"), + @Index(name = "idx_ratings_food_truck_id", columnList = "food_truck_id") + } +)src/main/java/konkuk/chacall/domain/member/domain/SavedFoodTruck.java (1)
20-26: 중복 찜 방지 및 조회 성능을 위한 제약/인덱스 추가 권장.
한 사용자가 같은 푸드트럭을 중복 저장하지 못하게 DB 유니크 제약이 필요합니다.적용 예:
-@Table(name = "saved_food_trucks") +@Table( + name = "saved_food_trucks", + uniqueConstraints = @UniqueConstraint( + name = "uk_saved_user_foodtruck", + columnNames = {"user_id", "food_truck_id"} + ), + indexes = { + @Index(name = "idx_saved_user_id", columnList = "user_id"), + @Index(name = "idx_saved_food_truck_id", columnList = "food_truck_id") + } +)src/main/java/konkuk/chacall/domain/owner/domain/model/ChatTemplate.java (1)
1-24: BankAccount 1:1 제약을 DB 레벨에서 확정해 주세요.
PR 요약대로User↔BankAccount를 1:1로 쓰려면 DB 유니크 제약이 필수입니다. 현재 엔티티(참고 스니펫)에선 그 보장이 보이지 않습니다.적용 예(참조:
src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java):// class-level @Table( name = "bank_accounts", uniqueConstraints = { @UniqueConstraint(name = "uk_bank_accounts_user_id", columnNames = "user_id"), @UniqueConstraint(name = "uk_bank_accounts_bank_acc", columnNames = {"bank_name", "account_number"}) }, indexes = { @Index(name = "idx_bank_accounts_user_id", columnList = "user_id"), @Index(name = "idx_bank_accounts_account_number", columnList = "account_number") } ) public class BankAccount { @OneToOne(fetch = FetchType.LAZY) @JoinColumn(name = "user_id", nullable = false, unique = true) private User owner; }
- 다른 사장님이 동일 계좌번호를 쓰지 못하게
(bank_name, account_number)유니크 권장(은행별 번호 체계 고려).- 서비스 레이어 검증은 유지하되, 최종 보장은 DB 유니크로 이중화하세요.
🧹 Nitpick comments (20)
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateBankAccountRequest.java (2)
5-14: DTO 네이밍 컨벤션 제안(리뷰 요청 1번 응답)
- Request: 동사+리소스+Request(예: RegisterBankAccountRequest, UpdateBankAccountRequest) 유지 권장.
- Response: 리소스+Response(예: BankAccountResponse). 목록은 BankAccountListResponse 또는 PageResponse로 명확화.
- 삭제는 PathVar만 받으면 별도 Request 생략.
1-14: 오너 검증 헬퍼의 패키지 위치 정리 제안(리뷰 요청 2번 응답)
owner/application/util대신 의미 있는 네임스페이스로 이동해 주세요. 예)
owner/application/validator/OwnerValidator또는- 횡단 관심사라면
owner/application/support혹은global/auth/authorization에 Guard/Authorizer로 명명.
유틸 명칭은 역할이 모호해지는 경향이 있어 지양을 권장합니다.src/main/java/konkuk/chacall/domain/member/application/MemberService.java (1)
1-4: 빈 서비스 클래스 — @service 추가 또는 파일 제거 권장rg 검색 결과, 프로젝트 내에서 MemberService는 본 파일 외에 참조되지 않습니다. 파일을 유지할 예정이면 @service를 추가하고, 아니라면 파일을 삭제하세요.
파일: src/main/java/konkuk/chacall/domain/member/application/MemberService.java
package konkuk.chacall.domain.member.application; +import org.springframework.stereotype.Service; -public class MemberService { +@Service +public class MemberService { }src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
1-39: PR 요청사항 답변: DTO 네이밍/헬퍼 위치 제안
- DTO 네이밍 규칙(제안):
- 요청: Create/Update/Delete/… + Resource + Request (예: CreateBankAccountRequest, UpdateBankAccountRequest)
- 응답: Resource + Response (예: BankAccountResponse, BankAccountSummaryResponse)
- 패키지:
owner/bankaccount/dto/request,owner/bankaccount/dto/response로 분리.- 약어(Req/Res) 미사용, 복수 응답은 컬렉션으로 감싸되 단수형 DTO 유지.
- 계좌 미등록 조회 응답:
- API 계약을 명시하세요. 200(Body에
bankAccount: null) 또는 204(No Content) 중 하나로 일관되게.- 헬퍼 클래스 위치:
owner/application/util대신owner/application/validator또는owner/application/support로 이동을 권장. 이름도OwnerValidator보다는 역할이 드러나게OwnerGuard/OwnerAccessValidator등.- 반복 검증 로직은 AOP/어노테이션(
@PreAuthorize, 커스텀 Aspect)로 횡단 관심사화 고려.src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java (1)
68-70: FK 컬럼명과 인덱스 정리 제안(가독성/쿼리 성능).
- 컬럼명을
owner_id로 명시하면 스키마 가독성이 좋아집니다(다른 곳ChatRoom은owner_id/member_id를 사용).- 소유자 기준 조회가 잦다면 인덱스를 추가하세요.
마이그레이션 동반 변경 예:
- @JoinColumn(name = "user_id", nullable = false) + @JoinColumn(name = "owner_id", nullable = false, referencedColumnName = "user_id") private User owner;
@Table인덱스 추가:-@Table(name = "food_trucks") +@Table( + name = "food_trucks", + indexes = @Index(name = "idx_food_trucks_owner_id", columnList = "owner_id") +)DB에선 다음으로 인덱스 유무를 확인해 주세요.
src/main/java/konkuk/chacall/domain/foodtruck/domain/Rating.java (1)
21-24: 불리언 필드 네이밍 소폭 정리 제안.
isRated대신rated로 두면 Lombok/자바빈 게터 혼동 여지를 줄입니다. 영향이 크지 않으므로 선택사항입니다.src/main/java/konkuk/chacall/domain/owner/domain/model/ChatTemplate.java (1)
21-23: 소유자 기준 조회 최적화를 위한 인덱스 제안.
owner기반 조회/정렬이 있다면 인덱스를 권장합니다.예:
-@Table(name = "chat_templates") +@Table( + name = "chat_templates", + indexes = @Index(name = "idx_chat_templates_user_id", columnList = "user_id") +)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterBankAccountRequest.java (1)
5-14: DTO 네이밍 컨벤션 일관화 제안 (Register → Create 권장)CRUD 중심 컨벤션이면
CreateBankAccountRequest로 통일하는 편이 읽기 좋습니다. Update DTO와의 용어 혼재(등록/수정 vs 생성/수정)를 피합시다.src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
35-40: 소유주 불일치 전용 에러코드 추가 고려소유주 검증 실패를 403으로 명확히 구분하면 핸들링이 쉬워집니다.
적용 diff:
- BANK_ACCOUNT_ALREADY_EXISTS_FOR_USER(HttpStatus.CONFLICT, 70003, "해당 유저에게 이미 등록된 계좌가 존재합니다.") + BANK_ACCOUNT_ALREADY_EXISTS_FOR_USER(HttpStatus.CONFLICT, 70003, "해당 유저에게 이미 등록된 계좌가 존재합니다."), + BANK_ACCOUNT_OWNER_MISMATCH(HttpStatus.FORBIDDEN, 70004, "계좌 소유주가 일치하지 않습니다.")src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (1)
18-24: 조회 성능을 위한 인덱스 추가 제안(member_id/owner_id)채팅방 목록/조회가 많다면 FK 컬럼 인덱스가 필요합니다.
적용 diff:
-@Table(name = "chat_rooms") +@Table(name = "chat_rooms", indexes = { + @Index(name = "idx_chat_rooms_member_id", columnList = "member_id"), + @Index(name = "idx_chat_rooms_owner_id", columnList = "owner_id") +})src/main/java/konkuk/chacall/domain/owner/application/util/OwnerValidator.java (2)
1-1: 패키지 위치 변경 제안: util → validator/support비즈니스 의미가 있는 컴포넌트이므로
...application.validator또는...application.support가 적합합니다. util 패키지는 의미가 약합니다.
18-21: 예외 세분화/정확화 고려 (404 vs 403/409)현재는 미존재·권한·비활성 모두 404로 귀결됩니다. 정책에 따라 403(권한 없음)/409(상태 충돌) 등을 분리하면 클라이언트 처리와 모니터링이 좋아집니다. (예: OWNER_NOT_ACTIVE, OWNER_ROLE_REQUIRED 등)
src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/BankAccountResponse.java (1)
5-10: 미등록 시 응답 스펙: null(200) 대신 204 No Content 권장리소스 부재는 204가 의미적으로 명확합니다. 컨트롤러/서비스 응답 정책 합의 후 일괄 적용을 권합니다.
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (2)
3-4: Validator의 패키지 위치 재고: util 보다는 validation/guard 패키지가 명확
owner.application.util.OwnerValidator보다는 의미가 드러나는owner.application.validation또는owner.application.guard등이 유지보수에 유리합니다. util 네임스페이스는 기능이 비대해지기 쉽습니다.Also applies to: 16-18
5-7: DTO 네이밍 통일 제안 — Register vs Update 불일치RegisterBankAccountRequest와 UpdateBankAccountRequest가 혼용되어 일관성·가독성 저하. 아래 중 하나로 통일을 권장합니다:
- Create/Update 페어: CreateBankAccountRequest / UpdateBankAccountRequest
- ResourceAction 형태: BankAccountCreateRequest / BankAccountUpdateRequest
수정 필요 파일:
- src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java
- src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java
- src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateBankAccountRequest.java
- src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java
- src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterBankAccountRequest.java
선호안 알려주시면 전체 참조 일괄 변경 스크립트 제공하겠습니다.
src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (1)
32-36: 엔티티 내 입력 정규화(트림/구분기호 제거) 적용 권장공백/하이픈 유무로 동일 계좌가 중복 저장될 수 있습니다. 업데이트 시 정규화하면 일관성이 좋아집니다.
아래처럼 간단히 정규화를 적용할 수 있습니다.
- public void update(String bankName, String accountHolderName, String accountNumber) { - this.bankName = bankName; - this.accountHolderName = accountHolderName; - this.accountNumber = accountNumber; - } + public void update(String bankName, String accountHolderName, String accountNumber) { + this.bankName = bankName.trim(); + this.accountHolderName = accountHolderName.trim(); + this.accountNumber = normalizeAccountNumber(accountNumber); + }추가 메서드(클래스 내부 임의 위치)에 아래를 포함하세요.
private String normalizeAccountNumber(String raw) { return raw == null ? null : raw.replaceAll("\\s|-", ""); }src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (3)
46-53: 삭제 응답 204(No Content) 고려삭제 성공 시 보디 없는 204가 관례적입니다. 팀 컨벤션에 맞추어
@ResponseStatus(NO_CONTENT)또는 래퍼 내 status 필드로 표현 고려 바랍니다.
25-26: 하드코딩한 ownerId(1L) 제거 계획토큰 연동 시
@AuthenticationPrincipal혹은 커스텀HandlerMethodArgumentResolver로 ownerId 주입을 권장합니다. 적용 시점 공유 부탁드립니다. 필요하면 샘플 제공 가능합니다.Also applies to: 34-35, 41-42, 50-51
13-17: 엔드포인트/DTO 네이밍 컨벤션 일치화 논의컨트롤러 레벨에서 동사-명사 규칙을 명확히 하시면 문서화/클라이언트 연동이 수월합니다. 예:
POST /owners/me/bank-accounts↔CreateBankAccountRequest,PATCH ...↔UpdateBankAccountRequest.src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (1)
64-70: 계좌번호 변경 중복 체크는 합리적이나 정규화 병행 권장사전 체크 전에 DTO/엔티티에서 계좌번호 정규화(공백/하이픈 제거)를 적용하면 false negative/positive를 줄일 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/Rating.java(1 hunks)src/main/java/konkuk/chacall/domain/member/application/MemberService.java(1 hunks)src/main/java/konkuk/chacall/domain/member/domain/SavedFoodTruck.java(1 hunks)src/main/java/konkuk/chacall/domain/member/presentation/MemberController.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/application/util/OwnerValidator.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java(2 hunks)src/main/java/konkuk/chacall/domain/owner/domain/model/ChatTemplate.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterBankAccountRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateBankAccountRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/BankAccountResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/user/domain/model/Gender.java(1 hunks)src/main/java/konkuk/chacall/domain/user/domain/model/Role.java(1 hunks)src/main/java/konkuk/chacall/domain/user/domain/model/User.java(1 hunks)src/main/java/konkuk/chacall/domain/user/domain/repository/UserRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/user/member/application/MemberService.java(0 hunks)src/main/java/konkuk/chacall/domain/user/member/presentation/MemberController.java(0 hunks)src/main/java/konkuk/chacall/domain/user/owner/application/OwnerService.java(0 hunks)src/main/java/konkuk/chacall/domain/user/owner/presentation/OwnerController.java(0 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)
💤 Files with no reviewable changes (4)
- src/main/java/konkuk/chacall/domain/user/owner/application/OwnerService.java
- src/main/java/konkuk/chacall/domain/user/member/application/MemberService.java
- src/main/java/konkuk/chacall/domain/user/member/presentation/MemberController.java
- src/main/java/konkuk/chacall/domain/user/owner/presentation/OwnerController.java
🧰 Additional context used
🧬 Code graph analysis (21)
src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/BankAccountResponse.java (1)
src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)
src/main/java/konkuk/chacall/domain/member/presentation/MemberController.java (2)
src/main/java/konkuk/chacall/domain/user/member/presentation/MemberController.java (1)
MemberController(3-4)src/main/java/konkuk/chacall/domain/user/member/application/MemberService.java (1)
MemberService(3-4)
src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java (2)
src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)
src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java (2)
src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)src/main/java/konkuk/chacall/domain/user/owner/domain/ChatTemplate.java (1)
Entity(9-24)
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (2)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(12-52)src/main/java/konkuk/chacall/global/common/exception/handler/GlobalExceptionHandler.java (1)
Slf4j(29-188)
src/main/java/konkuk/chacall/domain/member/application/MemberService.java (1)
src/main/java/konkuk/chacall/domain/user/member/application/MemberService.java (1)
MemberService(3-4)
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateBankAccountRequest.java (2)
src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)src/main/java/konkuk/chacall/domain/test/TestController.java (1)
Getter(86-98)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (2)
src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (1)
Service(18-93)src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)
src/main/java/konkuk/chacall/domain/user/domain/repository/UserRepository.java (2)
src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)src/main/java/konkuk/chacall/global/common/domain/BaseEntity.java (1)
Getter(11-27)
src/main/java/konkuk/chacall/domain/user/domain/model/Gender.java (1)
src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)
src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (3)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(12-52)src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)
src/main/java/konkuk/chacall/domain/owner/application/util/OwnerValidator.java (5)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(12-52)src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)src/main/java/konkuk/chacall/global/common/domain/BaseEntity.java (1)
Getter(11-27)src/main/java/konkuk/chacall/domain/user/Role.java (1)
Getter(5-14)
src/main/java/konkuk/chacall/domain/user/domain/model/Role.java (2)
src/main/java/konkuk/chacall/domain/user/Role.java (2)
Getter(5-14)Role(11-13)src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterBankAccountRequest.java (1)
src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (4)
src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)src/main/java/konkuk/chacall/global/common/exception/BusinessException.java (1)
Getter(6-19)src/main/java/konkuk/chacall/global/common/exception/AuthException.java (1)
Getter(6-18)src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)
src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java (3)
src/main/java/konkuk/chacall/domain/user/member/domain/SavedFoodTruck.java (1)
Entity(10-28)src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)
src/main/java/konkuk/chacall/domain/foodtruck/domain/Rating.java (1)
src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)
src/main/java/konkuk/chacall/domain/user/domain/model/User.java (8)
src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (1)
Entity(7-37)src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java (1)
Entity(12-43)src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (1)
Entity(8-25)src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java (1)
Entity(13-72)src/main/java/konkuk/chacall/domain/foodtruck/domain/Rating.java (1)
Entity(10-33)src/main/java/konkuk/chacall/domain/member/domain/SavedFoodTruck.java (1)
Entity(10-28)src/main/java/konkuk/chacall/domain/owner/domain/model/ChatTemplate.java (1)
Entity(9-24)src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)
src/main/java/konkuk/chacall/domain/owner/domain/model/ChatTemplate.java (2)
src/main/java/konkuk/chacall/domain/user/owner/domain/ChatTemplate.java (1)
Entity(9-24)src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (2)
src/main/java/konkuk/chacall/domain/user/User.java (1)
Entity(8-37)src/main/java/konkuk/chacall/domain/user/owner/domain/ChatTemplate.java (1)
Entity(9-24)
src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (2)
src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
Entity(9-39)src/main/java/konkuk/chacall/domain/user/owner/domain/BankAccount.java (1)
Entity(8-29)
🔇 Additional comments (20)
src/main/java/konkuk/chacall/domain/user/domain/model/Gender.java (1)
1-14: 패키지 정리 LGTM패키지 경로 정리는 일관성 측면에서 좋습니다. 추가 액션은 없어 보입니다.
src/main/java/konkuk/chacall/domain/user/domain/model/Role.java (1)
1-14: 패키지 이동 점검 — 기존 import(konkuk.chacall.domain.user.Role) 잔존 여부 수동 확인 필요
rg 검색 출력 없음으로 자동 검증 불가. 저장소 전체에서 해당 import 잔존 여부 재확인(권장 명령: rg -n "import\s+konkuk.chacall.domain.user.Role" || git grep -n "konkuk.chacall.domain.user.Role").src/main/java/konkuk/chacall/domain/member/presentation/MemberController.java (1)
1-4: 컨트롤러 스켈레톤 — 사용 예정이면 @RestController/@RequestMapping 추가, 아니면 파일 제거하세요.검증: 실행한 rg 결과 '@RequestMapping("/api/v1/members")' 패턴은 검색되지 않았고, 해당 파일에는 빈 클래스(public class MemberController {})만 존재합니다.
package konkuk.chacall.domain.member.presentation; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; -public class MemberController { +@RestController +@RequestMapping("/api/v1/members") +public class MemberController { }src/main/java/konkuk/chacall/domain/user/domain/model/User.java (3)
6-11: @Getter 추가 LGTM.
엔티티 접근을 게터로만 제한하는 현재 스타일과도 일치합니다.
32-38: Enum 컬럼 길이 재검토 필요 — DB에 값 잘림/에러 발생 가능
- 파일: src/main/java/konkuk/chacall/domain/user/domain/model/User.java (약 32–38행): @Enumerated(EnumType.STRING)인 gender에 @column(length = 1)은 MALE/FEMALE 등으로 잘릴 위험. Role도 enum 상수 최대 길이에 맞춰 여유를 주세요.
적용 예:
- @Column(length = 1) + @Column(length = 10) // 혹은 enum 최대 길이에 맞춘 값 private Gender gender; - @Column(length = 10, nullable = false) + @Column(length = 20, nullable = false) private Role role;enum 선언·길이 확인(실제 enum 최대 이름 길이에 따라 위 값 조정):
rg -nP '\benum\s+Gender\b' -S -C2 rg -nP '\benum\s+Role\b' -S -C2
1-1: 패키지 이동 확인 — 레거시 import(konkuk.chacall.domain.user.User) 없음
git ls-files + rg(--hidden -uu) 전역 검색 결과 해당 FQCN('konkuk.chacall.domain.user.User')을 참조하는 파일이 없고, 모든 참조는 'konkuk.chacall.domain.user.domain.model.User'로 업데이트되어 있습니다.src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java (1)
5-5: User 경로 변경 적용 확인했습니다.
컴파일 타임 의존만 변경이며 동작 영향은 없습니다.src/main/java/konkuk/chacall/domain/foodtruck/domain/Rating.java (1)
4-4: User 경로 변경 LGTM.
의존성만 변경되었습니다.src/main/java/konkuk/chacall/domain/member/domain/SavedFoodTruck.java (1)
1-1: 패키지/임포트 경로 변경 LGTM.
모듈 구조 정리에 일관성이 있습니다.Also applies to: 5-5
src/main/java/konkuk/chacall/domain/owner/domain/model/ChatTemplate.java (1)
1-1: 패키지/임포트 경로 변경 LGTM.
도메인 패키징 방향성과 일치합니다.Also applies to: 4-4
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
35-40: LGTM — 은행계좌 도메인 에러코드 정의 적절HTTP 상태/메시지/코드 범위 사용이 타당합니다.
src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java (1)
5-5: LGTM — User import 경로 변경 정상패키지 리팩터링과 일치합니다.
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (1)
4-4: LGTM — User import 경로 변경 정상src/main/java/konkuk/chacall/domain/user/domain/repository/UserRepository.java (1)
12-15: LGTM — 파생쿼리 시그니처 적절Role/Status 조건을 리포지토리에서 캡슐화한 점 좋습니다.
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
12-19: 파사드-도메인 책임 분리 잘 됨컨트롤러→파사드(OwnerService)→도메인 서비스(BankAccountService)로 흐름을 단순화하고, 소유주 검증을 파사드로 끌어올린 구조 괜찮습니다.
src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (2)
7-13: Lombok 조합은 적절함Builder/Getter/AllArgsConstructor 조합은 사용성 측면에서 좋습니다. 기본 생성자 보호 수준도 적절합니다.
1-1: DDL 반영 여부 확인 필요src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java는 table="bank_accounts"로 매핑되어 있으며 엔티티에 unique 제약 선언(@Table.uniqueConstraints 또는 @column(unique=true))은 없습니다. src/main/resources 내에 bank_accounts 관련 Flyway/Liquibase 마이그레이션 스크립트(또는 SQL)도 존재하지 않습니다. 유니크 제약을 도입하거나 엔티티를 변경할 계획이면 마이그레이션 스크립트를 추가하거나 DB 스키마 관리 방식을 명확히 해 주세요.
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (1)
37-44: PATCH 엔드포인트는 바인딩/흐름 적절
@RequestBody @Valid적용과 파사드 위임 흐름은 깔끔합니다.src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (2)
49-56: 미등록 계좌는 null 반환하는 정책 반영 OK요구사항에 맞는 동작이며, 컨트롤러에서
BaseResponse.ok(null)로 일관됩니다.
88-91: 소유주 검증 쿼리 방식 적절
findByBankAccountIdAndOwner_UserId로 소유권 검증을 일원화한 점 좋습니다.
| @Column(length = 30, nullable = false) | ||
| private String accountNumber; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
계좌번호 유니크 + 정합성 강화를 위한 DB 제약 권장
애플리케이션 레벨 중복 체크만으로는 동시성 경쟁 상황에서 중복 계좌가 생성될 수 있습니다. 위 코멘트의 테이블 유니크 제약으로 보완하시길 권장합니다.
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java
around lines 25-26, the accountNumber field lacks a database-level uniqueness
constraint so concurrent requests could create duplicate accounts; add a
uniqueness constraint by declaring the column as unique (or add a class-level
@Table(uniqueConstraints=...)) so the DB enforces uniqueness, and update the
schema/migration to add the unique index on account_number to ensure atomic
protection against race conditions.
| @OneToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "user_id", nullable = false) | ||
| private User owner; |
There was a problem hiding this comment.
OneToOne 제약 미설정: user_id에 유니크 제약 필요
@OneToOne만으로는 DB 레벨에서 1:1이 보장되지 않습니다. @JoinColumn에 unique = true 혹은 @Table(uniqueConstraints=...)로 유니크 제약을 명시해야 다중 계좌가 동일 사용자에 매달리는 것을 방지할 수 있습니다.
아래처럼 유니크 제약과 인덱스를 추가해 주세요(계좌번호도 함께 유니크 권장).
-@Table(name = "bank_accounts")
+@Table(
+ name = "bank_accounts",
+ uniqueConstraints = {
+ @UniqueConstraint(name = "uk_bank_accounts_user_id", columnNames = "user_id"),
+ @UniqueConstraint(name = "uk_bank_accounts_account_number", columnNames = "account_number")
+ },
+ indexes = {
+ @Index(name = "ix_bank_accounts_user_id", columnList = "user_id")
+ }
+)
@@
- @Column(length = 30, nullable = false)
+ @Column(name = "account_number", length = 30, nullable = false)
private String accountNumber;
@@
- @OneToOne(fetch = FetchType.LAZY)
- @JoinColumn(name = "user_id", nullable = false)
+ @OneToOne(fetch = FetchType.LAZY)
+ @JoinColumn(name = "user_id", nullable = false, unique = true)
private User owner;📝 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.
| @OneToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "user_id", nullable = false) | |
| private User owner; | |
| @OneToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "user_id", nullable = false, unique = true) | |
| private User owner; |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java
around lines 28-30, the @OneToOne mapping for owner does not enforce a DB-level
uniqueness on user_id so multiple BankAccount rows could reference the same
User; add a uniqueness constraint by setting unique = true on the @JoinColumn
(or alternatively add a @Table(uniqueConstraints = ...) including user_id) to
enforce 1:1 at the database level, and also add a unique constraint/index for
the account number column to prevent duplicate account entries.
| // 계좌 번호가 DB 상에 이미 존재하는지 여부 조회 | ||
| boolean existsByAccountNumber(String accountNumber); | ||
|
|
||
| // 사장님 ID 기반 계좌 등록 여부 조회 | ||
| boolean existsByOwner_UserId(Long userId); | ||
|
|
||
| // 사장님 ID 기반 계좌 조회 | ||
| Optional<BankAccount> findByOwner_UserId(Long userId); | ||
|
|
||
| // 계좌 ID와 사장님 ID로 계좌 조회 | ||
| Optional<BankAccount> findByBankAccountIdAndOwner_UserId(Long bankAccountId, Long ownerId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
업데이트 시 자기 자신 제외 중복검사 메서드 추가
수정에서 계좌번호가 변하지 않아도 existsByAccountNumber만 쓰면 409로 오판될 수 있습니다.
적용 diff:
// 계좌 번호가 DB 상에 이미 존재하는지 여부 조회
boolean existsByAccountNumber(String accountNumber);
+ // 업데이트 시 자기 자신을 제외한 중복 여부 조회
+ boolean existsByAccountNumberAndBankAccountIdNot(String accountNumber, Long bankAccountId);📝 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.
| // 계좌 번호가 DB 상에 이미 존재하는지 여부 조회 | |
| boolean existsByAccountNumber(String accountNumber); | |
| // 사장님 ID 기반 계좌 등록 여부 조회 | |
| boolean existsByOwner_UserId(Long userId); | |
| // 사장님 ID 기반 계좌 조회 | |
| Optional<BankAccount> findByOwner_UserId(Long userId); | |
| // 계좌 ID와 사장님 ID로 계좌 조회 | |
| Optional<BankAccount> findByBankAccountIdAndOwner_UserId(Long bankAccountId, Long ownerId); | |
| } | |
| // 계좌 번호가 DB 상에 이미 존재하는지 여부 조회 | |
| boolean existsByAccountNumber(String accountNumber); | |
| // 업데이트 시 자기 자신을 제외한 중복 여부 조회 | |
| boolean existsByAccountNumberAndBankAccountIdNot(String accountNumber, Long bankAccountId); | |
| // 사장님 ID 기반 계좌 등록 여부 조회 | |
| boolean existsByOwner_UserId(Long userId); | |
| // 사장님 ID 기반 계좌 조회 | |
| Optional<BankAccount> findByOwner_UserId(Long userId); | |
| // 계좌 ID와 사장님 ID로 계좌 조회 | |
| Optional<BankAccount> findByBankAccountIdAndOwner_UserId(Long bankAccountId, Long ownerId); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java
around lines 10-21, add an existence-check method that excludes a given
bankAccountId so updates that keep the same account number don't falsely trigger
a duplicate; specifically declare a method such as
existsByAccountNumberAndBankAccountIdNot(String accountNumber, Long
bankAccountId) in the repository interface and use this method in the update
flow (instead of existsByAccountNumber) to verify uniqueness excluding the
current record.
💡 Verification agent
🧩 Analysis chain
OneToOne 제약 DB 보장 및 엔티티 매핑 일치 여부 확인 필요
PR 설명은 User↔BankAccount를 1:1이라 했으나, 일부 코드/스니펫에는 @manytoone 흔적이 있습니다. DB 유니크 제약(owner_id unique, account_number unique) 없으면 경쟁조건으로 중복이 저장될 수 있습니다.
다음 스크립트로 매핑/DDL을 점검해 주세요:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== BankAccount 매핑 확인 =="
rg -nP -C3 '(?i)class\s+BankAccount\b|@(ManyToOne|OneToOne)|@JoinColumn\(' --type=java
echo "== owner_id 유니크 제약 확인(@JoinColumn(unique=true) 또는 @Table(uniqueConstraints)) =="
rg -nP -C2 '@JoinColumn\([^)]*name\s*=\s*"user_id"[^)]*\)' --type=java
rg -nP -C2 '@Table\([^)]*uniqueConstraints' --type=java
echo "== 마이그레이션에서 bank_accounts 제약 확인 =="
rg -n "bank_accounts" -g "**/db/migration/**" -n -C2 || true
echo "== 서비스에서 중복검사 사용 위치 확인 =="
rg -nP --type=java -C3 'updateBankAccount\s*\('
rg -nP --type=java -C2 'existsByAccountNumber\s*\('Length of output: 13356
DB 유니크 제약(user_id, account_number)을 마이그레이션으로 반드시 추가하세요.
- 확인: src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java는 @OnetoOne(fetch = LAZY) + @joincolumn(name = "user_id", nullable = false)로 매핑되어 있음.
- 문제: @joincolumn에 unique=true 또는 @table(uniqueConstraints)가 없고, db/migration에서 bank_accounts 관련 유니크 제약도 발견되지 않음 — DB 제약 없이는 경쟁조건으로 중복이 저장될 수 있음.
- 조치(우선순위): DB 마이그레이션으로 bank_accounts.user_id UNIQUE 및 bank_accounts.account_number UNIQUE 추가. 서비스는 existsByAccountNumber 검사 외에 DB 제약 위반 예외(중복 처리)를 반드시 처리하도록 보완. (선택: JPA에 @joincolumn(unique=true)/@column(unique=true) 또는 @table(uniqueConstraints) 병행 적용)
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java
around lines 10 to 21, the review notes missing DB-level UNIQUE constraints for
(user_id, account_number) which risks duplicate rows under concurrency; add a
migration that creates UNIQUE constraint(s) on bank_accounts.user_id and
bank_accounts.account_number (or a composite unique index if intended), and
update service code to catch and handle DB constraint-violation exceptions
(e.g., translate SQL/constraint violation to a meaningful duplicate-account
response); optionally also annotate the JPA entity (BankAccount) with
@Column(unique=true) or @Table(uniqueConstraints=...) to reflect the uniqueness
in the model.
| package konkuk.chacall.domain.owner.presentation.dto.request; | ||
|
|
||
| import jakarta.validation.constraints.NotBlank; | ||
|
|
||
| public record UpdateBankAccountRequest( | ||
| @NotBlank(message = "은행명은 비어있을 수 없습니다.") | ||
| String bankName, | ||
|
|
||
| @NotBlank(message = "예금주명은 비어있을 수 없습니다.") | ||
| String accountHolderName, | ||
|
|
||
| @NotBlank(message = "계좌번호는 비어있을 수 없습니다.") | ||
| String accountNumber | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복 계좌 등록 경쟁 조건 방지: DB 유니크 제약 추가
서비스 레벨 검증만으로는 동시성 레이스를 막기 어렵습니다. DB 차원에서
- 오너당 1계좌 보장: FK 컬럼에 UNIQUE,
- 계좌 중복 방지: (bank_name, account_number) 복합 UNIQUE(또는 account_number 단일 UNIQUE, 정책에 따름)
을 권장합니다. 예시:
// BankAccount.java (예시)
@Entity
@Table(name = "bank_accounts",
uniqueConstraints = {
@UniqueConstraint(name = "uk_bank_account_owner", columnNames = "owner_id"),
@UniqueConstraint(name = "uk_bank_account_banknum", columnNames = {"bank_name","account_number"})
})
public class BankAccount {
@OneToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "owner_id", nullable = false, unique = true)
private User owner;
@Column(length = 30, nullable = false)
private String accountNumber;
@Column(length = 20, nullable = false)
private String bankName;
}예외 매핑: DataIntegrityViolationException -> ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS(_FOR_USER)로 변환 처리.
<details>
<summary>🤖 Prompt for AI Agents</summary>
In
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateBankAccountRequest.java
around lines 1 to 14, the reviewer requests DB-level uniqueness to prevent
race-condition duplicate bank accounts; update the BankAccount entity to add
UNIQUE constraints (one-to-one owner_id unique to enforce one account per owner
and a unique constraint for bank_name+account_number or account_number alone per
your policy) and mark the owner FK unique, then add global exception handling to
map DataIntegrityViolationException (thrown on constraint violation) to a domain
error like ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS (and optionally
BANK_ACCOUNT_ALREADY_EXISTS_FOR_USER) so service-level race conditions are
caught and translated into proper API errors.
</details>
<!-- fingerprinting:phantom:medusa:chinchilla -->
<!-- This is an auto-generated comment by CodeRabbit -->
- BankAccount <-> User 연관관계에 unique 제약 조건 추가 - 계좌번호 컬럼에 unique 제약 조건 추가 - 누락된 @RequestBody 설정 추가
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
수고하셨습니다! 파사드 패턴을 처음 적용해보는데 전반적으로 코드가 깔끔해서 보기 편했습니다. 다른 api 구현시에 많이 참고할게욥~ 리뷰 몇가지 남겼는데 확인 부탁드려요
| private final BankAccountService bankAccountService; | ||
| // 파사드에서 사장님 검증을 거침으로써 실제 계좌 관련 서비스 로직에서는 사장님 검증에 신경쓰지 않도록 책임 분리 | ||
| private final OwnerValidator ownerValidator; | ||
|
|
||
| public void registerBankAccount(RegisterBankAccountRequest request, Long ownerId) { | ||
| // 사장님인지 먼저 검증 | ||
| User owner = ownerValidator.validateAndGetOwner(ownerId); | ||
|
|
||
| // 검증된 유저 정보를 넘겨 계좌 등록 로직 호출 | ||
| bankAccountService.registerBankAccount(request, owner); | ||
| } |
There was a problem hiding this comment.
오호 전체 애그리거트를 파사드 패턴으로 또다시 하위 도메인으로 나누는 느낌이네요!! 굿굿 너무 좋습니다
| public BankAccountResponse getBankAccount(Long ownerId) { | ||
| // 사장님인지 먼저 검증 | ||
| ownerValidator.validateAndGetOwner(ownerId); | ||
|
|
||
| // 계좌 조회 로직 호출 | ||
| return bankAccountService.getBankAccount(ownerId); | ||
| } | ||
|
|
||
| public void updateBankAccount(Long ownerId, Long bankAccountId, UpdateBankAccountRequest request) { | ||
| // 사장님인지 먼저 검증 | ||
| ownerValidator.validateAndGetOwner(ownerId); | ||
|
|
||
| // 계좌 수정 로직 호출 | ||
| bankAccountService.updateBankAccount(ownerId, bankAccountId, request); | ||
| } | ||
|
|
||
| public void deleteBankAccount(Long ownerId, Long bankAccountId) { | ||
| // 사장님인지 먼저 검증 | ||
| ownerValidator.validateAndGetOwner(ownerId); | ||
|
|
||
| // 계좌 삭제 로직 호출 | ||
| bankAccountService.deleteBankAccount(ownerId, bankAccountId); | ||
| } |
There was a problem hiding this comment.
음 밑에 같은 줄의 중복 코드가 발생하는건 지금 상황에선 어쩔 수 없을 것 같네요,,우선 기능 개발 마치고 추후에 aop로 중복 코드를 제거하는 식으로 리팩해보는 것도 좋을 것 같슴다
| // 해당 유저의 계좌가 이미 있는지 확인 | ||
| if (bankAccountRepository.existsByOwner_UserId(owner.getUserId())) { | ||
| throw new DomainRuleException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS_FOR_USER); | ||
| } | ||
|
|
||
| // 중복 계좌 검증 | ||
| if (bankAccountRepository.existsByAccountNumber(request.accountNumber())) { | ||
| throw new DomainRuleException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS); | ||
| } |
There was a problem hiding this comment.
p2: 엇 이거 제가 예외 클래스에 대해서 정확히 전달을 못 드렸던 것 같네요,, 죄송함다 🥲 그 BusinessException과 DomainRuleException의 차이는 예외가 발생하는 계층으로 두었습니다! 서비스 계층에서 발생하는 예외는 BusinessException, 도메인 계층에서 발생하는 예외는 DomainRuleException으로 통일하는 것을 의도해서 정의하긴 했는데 상균님 생각은 혹시 어떠신가욥??
There was a problem hiding this comment.
아하 예외가 발생하는 계층의 차이를 통한 예외 구분이군여 좋습니다 해당 방식으로 바꿔볼게용
| BankAccount bankAccount = BankAccount.builder() | ||
| .bankName(request.bankName()) | ||
| .accountHolderName(request.accountHolderName()) | ||
| .accountNumber(request.accountNumber()) | ||
| .owner(owner) | ||
| .build(); |
There was a problem hiding this comment.
p3: 엔티티 생성에 대한 것은 엔티티 내부에서 담당하도록 하는 것이 어떨까요..? 내부에서 정적 팩토리 메서드를 통해 생성하도록 하면 추후에 엔티티 생성시에 도메인 규칙이 추가되었을 때 조금더 유연하게 대응할 수 있을 것 같아욥!
There was a problem hiding this comment.
좋습니다~ 기존에 양방향 관계를 생성할 때는 항상 내부에 팩토리 메서드, 혹은 양방향 관계를 설정해주기 위한 팩토리 메서드를 만들었었는데, 양방향 관계가 사라지다보니 이 부분을 고려하지 못한 것 같습니다. 적용해보겠슴다
|
|
||
| public BankAccountResponse getBankAccount(Long ownerId) { | ||
| // 계좌 조회 | ||
| Optional<BankAccount> bankAccountOptional = bankAccountRepository.findByOwner_UserId(ownerId); |
There was a problem hiding this comment.
현재 '_'를 포함한 JPA 쿼리 함수에서 자동으로 join이 발생하는 것 같은데, 혹시 쿼리가 단순해서 성능 부담보다는 가독성을 우선하신걸로 이해하면 될까요?
There was a problem hiding this comment.
이게 변수명이 Owner 로 되어있다보니 메서드네이밍 기반으로 쿼리문을 작성할 때 좀 헷갈리더라구요. join 발생안하도록 네이밍 변경해보겠습니당
| private BankAccount findBankAccountAndVerifyOwner(Long ownerId, Long bankAccountId) { | ||
| return bankAccountRepository.findByBankAccountIdAndOwner_UserId(bankAccountId, ownerId) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.BANK_ACCOUNT_NOT_FOUND)); | ||
| } |
There was a problem hiding this comment.
p3: 현재 메서드 네이밍에 verify가 명시되어 있어서 검증한다로 해석되는데 실제 동작은 조회만 하고 있어서 약간 헷갈리는 것 같아요! 따라서 다음 두가지 방향으로 제안드려요!
- 네이밍 수정 :
findBankAccountByIdAndOwner - BankAccountId로만 조회후 조회한 엔티티의 fk와 ownerId를 비교
private BankAccount findBankAccountAndVerifyOwner(Long ownerId, Long bankAccountId) {
BankAccount bankAccount = bankAccountRepository.findById(bankAccountId)
.orElseThrow(() -> new EntityNotFoundException(ErrorCode.BANK_ACCOUNT_NOT_FOUND));
// 요청자가 실제 소유주인지 검증
bankAccount.verifyOwner(ownerId);
return bankAccount;
}BankAccount 내부
public void verifyOwner(Long ownerId) {
if (!this.owner.getUserId().equals(ownerId)) {
throw new DomainRuleException(ErrorCode.BANK_ACCOUNT_FORBIDDEN);
}
}There was a problem hiding this comment.
오호 개인적으로 2번째 방식이 더 좋은 것 같네요.
DomainRuleException 을 저런 식으로 활용하는 군요 좋은 피드백인 것 같습니다!
변경해볼게용
| package konkuk.chacall.domain.owner.application.util; | ||
|
|
||
| import konkuk.chacall.domain.user.domain.model.Role; | ||
| import konkuk.chacall.domain.user.domain.model.User; | ||
| import konkuk.chacall.domain.user.domain.repository.UserRepository; | ||
| import konkuk.chacall.global.common.domain.BaseStatus; | ||
| import konkuk.chacall.global.common.exception.EntityNotFoundException; | ||
| import konkuk.chacall.global.common.exception.code.ErrorCode; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class OwnerValidator { | ||
|
|
||
| private final UserRepository userRepository; | ||
|
|
||
| public User validateAndGetOwner(Long ownerId) { | ||
| return userRepository.findByUserIdAndRoleAndStatus(ownerId, Role.OWNER, BaseStatus.ACTIVE) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.USER_NOT_FOUND)); | ||
| } | ||
| } |
There was a problem hiding this comment.
굿굿 헬퍼 서비스 좋은 것 같습니다! pr에 남겨주신 것처럼 util이 살짝 애매한데 그냥 validator라는 패키지로 구분하고 @Component 어노테이션 또한 @HelperService 라는 커스텀 어노테이션을 정의해서 사용하는거 어떨까요??
import org.springframework.core.annotation.AliasFor;
import org.springframework.stereotype.Service;
import java.lang.annotation.*;
@Target({ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
@Service
public @interface HelperService {
@AliasFor(annotation = Service.class)
String value() default "";
}There was a problem hiding this comment.
좋은 것 같습니다! 역할을 확실하게 구분할 수 있을 것 같네용
| public BaseResponse<Void> registerBankAccount( | ||
| @RequestBody @Valid RegisterBankAccountRequest registerBankAccountRequest | ||
| ) { | ||
| // todo 추후에 토큰 추가될 시 id 값은 토큰에서 추출하여 전달 |
| public BankAccountResponse getBankAccount(Long ownerId) { | ||
| // 계좌 조회 | ||
| Optional<BankAccount> bankAccountOptional = bankAccountRepository.findByOwner_UserId(ownerId); | ||
|
|
||
| return bankAccountOptional | ||
| .map(BankAccountResponse::from) // 계좌가 존재하면 DTO 로 변환 | ||
| .orElse(null); // 계좌가 존재하지 않으면 null 반환 | ||
| } |
There was a problem hiding this comment.
Response Dto는 파사드 하위 서비스 클래스에서 변환해서 파사드 클래스로 넘어가는 거군요! 다른 api 구현할때 유의하겠습니다
| if (!bankAccount.getAccountNumber().equals(request.accountNumber())) { | ||
| if (bankAccountRepository.existsByAccountNumber(request.accountNumber())) { | ||
| throw new DomainRuleException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS); | ||
| } | ||
| } |
There was a problem hiding this comment.
p2: 이 부분 코드를 보면서 두 가지 의문점이 들어서 남겨봅니다!
1. 서로 다른 사장님들이 동일한 계좌를 사용하는 경우는 요구사항상 허용되지 않는 건가요? <- pm님께 여쭤본 결과 맞다고 하네요! 한 계좌 = 한 사장
2. 계좌 번호는 동일하지만 은행이 다른 경우도 발생할 수 있지 않을까요?
There was a problem hiding this comment.
2번 같은 경우 확인해봤을 때, 은행마다 계좌번호 자릿수가 다 다르고, 계좌번호 체계가 다 달라서 중복될 일은 없다는 것 같긴 합니다!
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (4)
84-93: 헬퍼 네이밍/책임 분리 합의 재확인메서드명
findBankAccountAndVerifyOwner는 동작을 정확히 드러냅니다. 다만 이전 코멘트에서 제안된 대로verify라는 단어의 모호성 지적이 있어, 필요 시loadAndCheckOwnership같은 네이밍으로의 변경도 고려해 보세요.
60-65: 업데이트 중복 검사 간소화자기 자신을 제외한 중복검사 메서드를 쓰면 분기 없이 한 줄로 정리됩니다.
- if (!bankAccount.getAccountNumber().equals(request.accountNumber())) { - if (bankAccountRepository.existsByAccountNumber(request.accountNumber())) { - throw new BusinessException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS); - } - } + if (bankAccountRepository.existsByAccountNumberAndBankAccountIdNot( + request.accountNumber(), bankAccountId)) { + throw new BusinessException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS); + }
67-73: flush 시점 제약 위반을 도메인 예외로 변환update 후 flush 시 유니크 제약이 터지면 500이 날 수 있습니다. 조기 flush + 매핑을 권장합니다.
bankAccount.update( request.bankName(), request.accountHolderName(), request.accountNumber() ); + try { + bankAccountRepository.flush(); + } catch (DataIntegrityViolationException e) { + String m = String.valueOf(e.getMostSpecificCause() != null ? e.getMostSpecificCause().getMessage() : e.getMessage()); + if (m.contains("uk_bank_accounts_account_number")) { + throw new BusinessException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS); + } + throw new BusinessException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS_FOR_USER); + }
26-42: 등록 시 경쟁 조건 및 ‘detached User’ 대응
- exists 체크 ↔ save 사이 경쟁으로 DB 유니크 위반 시 500이 날 수 있습니다. 제약 위반을 BusinessException으로 변환해 주세요.
- Owner는 다른 트랜잭션 컨텍스트에서 로드된 detached일 가능성이 있어 getReference(...)로 현재 트랜잭션에 맞게 참조를 획득하는 것이 안전합니다.
import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.dao.DataIntegrityViolationException; +import jakarta.persistence.EntityManager; +import jakarta.persistence.PersistenceContext; @@ public class BankAccountService { private final BankAccountRepository bankAccountRepository; + @PersistenceContext + private EntityManager em; @@ public void registerBankAccount(RegisterBankAccountRequest request, User owner) { @@ - BankAccount bankAccount = BankAccount.of(request.bankName(), request.accountHolderName(), request.accountNumber(), owner); - - bankAccountRepository.save(bankAccount); + User ownerRef = em.getReference(User.class, owner.getUserId()); + BankAccount bankAccount = BankAccount.of( + request.bankName(), + request.accountHolderName(), + request.accountNumber(), + ownerRef + ); + try { + bankAccountRepository.save(bankAccount); + } catch (DataIntegrityViolationException e) { + String m = String.valueOf(e.getMostSpecificCause() != null ? e.getMostSpecificCause().getMessage() : e.getMessage()); + if (m.contains("uk_bank_accounts_account_number")) { + throw new BusinessException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS); + } + if (m.contains("uk_bank_accounts_user_id")) { + throw new BusinessException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS_FOR_USER); + } + throw new BusinessException(ErrorCode.BANK_ACCOUNT_ALREADY_EXISTS); + } }src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java (1)
12-16: 업데이트 중복 검사용 exists 메서드 추가수정 시 자기 자신을 제외한 중복 검증을 위한 메서드를 추가해 두면 안전하고 단순해집니다.
// 계좌 번호가 DB 상에 이미 존재하는지 여부 조회 boolean existsByAccountNumber(String accountNumber); + // 업데이트 시 자기 자신 제외 중복 여부 조회 + boolean existsByAccountNumberAndBankAccountIdNot(String accountNumber, Long bankAccountId); // 사장님 ID 기반 계좌 등록 여부 조회 boolean existsByOwner_UserId(Long userId);src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (1)
10-16: 1:1 및 유니크 제약의 명시적 네이밍 권장(@table 수준)운영 DB에서 제약 위반 매핑을 안정적으로 하려면 제약명(uk_...)을 고정하는 편이 좋습니다. 컬럼 단위 unique=true 대신 @Table.uniqueConstraints로 명시해 주세요.
-@Table(name = "bank_accounts") +@Table( + name = "bank_accounts", + uniqueConstraints = { + @UniqueConstraint(name = "uk_bank_accounts_user_id", columnNames = "user_id"), + @UniqueConstraint(name = "uk_bank_accounts_account_number", columnNames = "account_number") + }, + indexes = { + @Index(name = "ix_bank_accounts_user_id", columnList = "user_id") + } +)
🧹 Nitpick comments (6)
src/main/java/konkuk/chacall/global/common/annotation/HelperService.java (1)
10-14: 커스텀 stereotype 보강: @documented 추가 권장Javadoc/IDE에서 메타정보가 노출되도록 @documented를 추가해 주세요. 기능 영향은 없고 가독성만 개선됩니다.
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.lang.annotation.Documented; + @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @Service +@Documented public @interface HelperService { }src/main/java/konkuk/chacall/domain/owner/application/validator/OwnerValidator.java (1)
15-21: 메서드 네이밍 경량 조정 제안의도는 “활성 OWNER를 조회하거나 없으면 예외”이므로
getActiveOwnerOrThrow같이 동작이 드러나는 네이밍을 고려해 보세요. 변경은 선택 사항입니다.src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (2)
28-33: 제약 정의 중복 제거위 @table로 제약을 일원화하는 경우, 필드/조인 컬럼의 unique=true는 제거해 주세요(중복 생성 방지).
- @Column(length = 30, nullable = false, unique = true) + @Column(name = "account_number", length = 30, nullable = false) private String accountNumber; - @OneToOne(fetch = FetchType.LAZY) - @JoinColumn(name = "user_id", nullable = false, unique = true) + @OneToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "user_id", nullable = false) private User owner;
22-26: 길이 한도 재검토(도메인 제약 확인)bankName/accountHolderName 각각 20자는 짧을 수 있습니다(은행명 복합 표기, 긴 예금주). 여유 있게 30~40자로 상향을 제안합니다.
- @Column(length = 20, nullable = false) + @Column(length = 40, nullable = false) private String bankName; - @Column(length = 20, nullable = false) + @Column(length = 40, nullable = false) private String accountHolderName;src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (2)
45-52: null 반환 정책 주석 또는 어노테이션으로 명시요구사항상 미등록 시 null 반환이 맞다면, @nullable 또는 자바독으로 명시해 두면 이후 오용을 줄일 수 있습니다.
- public BankAccountResponse getBankAccount(Long ownerId) { + @org.springframework.lang.Nullable + public BankAccountResponse getBankAccount(Long ownerId) {
5-7: DTO 네이밍 컨벤션 통일Create/Update의 동사 계열을 맞추면 탐색성이 좋아집니다.
RegisterBankAccountRequest→CreateBankAccountRequest등의 통일을 제안드립니다. 응답은BankAccountResponse로 충분히 일관적입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/application/validator/OwnerValidator.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java(1 hunks)src/main/java/konkuk/chacall/global/common/annotation/HelperService.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java
- src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/konkuk/chacall/domain/owner/application/validator/OwnerValidator.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(12-52)
src/main/java/konkuk/chacall/global/common/annotation/HelperService.java (2)
src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (1)
Service(19-95)src/main/java/konkuk/chacall/domain/owner/application/validator/OwnerValidator.java (1)
HelperService(13-23)
src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(12-52)
src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (1)
src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
Entity(9-39)
🔇 Additional comments (4)
src/main/java/konkuk/chacall/global/common/annotation/HelperService.java (1)
10-14: 패키지/역할 정합성 OK도메인에 종속되지 않는 범용 헬퍼 stereotype을 global.common.annotation에 두고, @service 메타‑애노테이션으로 빈 등록을 위임한 판단 좋아요. 현재 용도(Validator)와도 합치합니다.
src/main/java/konkuk/chacall/domain/owner/domain/model/BankAccount.java (1)
50-54: 소유주 검증 로직 LGTM도메인 규칙 위반을 DomainRuleException으로 승격하는 경계 설정이 명확합니다.
src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java (1)
10-20: 인덱스/제약 조건 확인 — owner_id 및 account_number에 UNIQUE + INDEX 적용 여부 확인 필요rg 실행 결과 db 마이그레이션 파일을 찾지 못해 자동 검증 불가. Flyway(src/main/resources/db/migration 또는 db/migration) 또는 Liquibase(src/main/resources/db/changelog) 마이그레이션에서 owner_id와 account_number에 대해 UNIQUE 제약과 적절한 인덱스가 반영되어 있는지 확인하고, 누락 시 관련 마이그레이션을 추가하세요.
src/main/java/konkuk/chacall/domain/owner/application/bankAccount/BankAccountService.java (1)
26-42: 마이그레이션에 DB 유니크 제약(uk_bank_accounts_user_id / uk_bank_accounts_account_number) 존재 여부 확인 필요실행한 스크립트가 "No files were searched"를 반환해 확인이 불가합니다. 프로젝트 루트에서 아래 스크립트를 실행해 마이그레이션에 해당 제약명이 존재하는지 출력 결과를 공유하세요. 제약이 없으면 마이그레이션에 UNIQUE 제약을 추가하세요.
#!/bin/bash set -euo pipefail echo "== Find bank_accounts unique constraints in migrations ==" # repo-wide search (포함: 숨김파일, 제외: 종료 코드 오류 방지) rg -nP '(?i)uk_bank_accounts_(user_id|account_number)|bank_accounts|unique\s*\(.*(?:user_id|account_number)' --hidden --no-ignore -S -C2 || true echo "== Likely migration directories ==" find . -type d -path '*/db/migration*' -print || true
| // 사장님 ID 기반 계좌 조회 | ||
| @Query("SELECT ba FROM BankAccount ba WHERE ba.owner.userId = :ownerId") | ||
| Optional<BankAccount> findByOwnerId(@Param("ownerId") Long ownerId); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메서드 네이밍 정합성 및 쿼리 제거
메서드명이 findByOwnerId인데 실제로는 owner.userId로 조회합니다. 혼동을 줄이기 위해 파생쿼리로 대체하며 이름을 findByOwner_UserId로 맞추는 것을 권장합니다.
- // 사장님 ID 기반 계좌 조회
- @Query("SELECT ba FROM BankAccount ba WHERE ba.owner.userId = :ownerId")
- Optional<BankAccount> findByOwnerId(@Param("ownerId") Long ownerId);
+ // 사장님 ID 기반 계좌 조회
+ Optional<BankAccount> findByOwner_UserId(Long ownerId);서비스 호출부도 함께 변경해 주세요.
-Optional<BankAccount> bankAccountOptional = bankAccountRepository.findByOwnerId(ownerId);
+Optional<BankAccount> bankAccountOptional = bankAccountRepository.findByOwner_UserId(ownerId);📝 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.
| // 사장님 ID 기반 계좌 조회 | |
| @Query("SELECT ba FROM BankAccount ba WHERE ba.owner.userId = :ownerId") | |
| Optional<BankAccount> findByOwnerId(@Param("ownerId") Long ownerId); | |
| // 사장님 ID 기반 계좌 조회 | |
| Optional<BankAccount> findByOwner_UserId(Long ownerId); |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/owner/domain/repository/BankAccountRepository.java
around lines 18 to 20, the method is named findByOwnerId but uses a JPQL @Query
that compares owner.userId which is inconsistent; replace the custom @Query with
a Spring Data derived query method signature named findByOwner_UserId that
returns Optional<BankAccount> and remove the @Query/@Param annotations, then
update all service/controller call sites to invoke
repository.findByOwner_UserId(ownerId) instead of findByOwnerId so the naming
matches the property path and avoids the manual query.
| @Getter | ||
| @Builder | ||
| public class BankAccount { | ||
| public class BankAccount extends BaseEntity { |
2 participants
#️⃣연관된 이슈
📝작업 내용
계좌 관련 CRUD 기능을 구현하였습니다.
계좌 등록
피그마를 확인해보니, 사장님 한 분당 하나의 계좌만 등록할 수 있도록 요구사항이 변경된 것을 확인할 수 있었습니다.
따라서 User <-> BankAccount 간의 연관관계를 OneToOne 으로 변경하고, 외래키는 여전히 BankAccount 쪽에서 가지고있도록 구현을 변경하였습니다.
계좌 등록시 검증해주어야할 사항이 몇가지 존재했고, 그 항목들은 다음과 같습니다.
계좌 등록을 요청한 사람이 DB 상에 존재하는지 && 사장님인지 && status = ACTIVE 인지이미 등록된 계좌가 존재하지는 않는지이미 등록된 계좌가 없다고 하더라도, 등록하고자 하는 계좌를 다른 어떤 사장님이 사용하고있지는 않은지계좌 조회
계좌 조회에서 유의해야했던 점은, 계좌가 등록되어있지 않은 상황을 에러로 판단하지 않는다는 점이었습니다.
최초로 가입한 후, 혹은 등록해뒀던 계좌를 삭제한 후에는 특정 사장님에 대해서 계좌가 존재하지 않는 경우가 존재할 수 있었습니다.
또한 계좌 등록 여부에 따라 결제 관리 뷰에서 '수정하기' 버튼을 비활성화 하느냐 마느냐를 결정해야했기 때문에, 계좌가 등록되어있지 않다면 계좌 조회시 null 응답을 반환하도록 하고, 계좌가 등록되어있다면 데이터가 정상적으로 반환되도록 구현하였습니다.
계좌 수정
계좌 수정은 사실상 계좌 등록과 동일했습니다. 계좌 등록 때처럼 은행명, 예금주명, 계좌 번호를 모두 입력받도록 구현하였고, 그렇게 받은 값들을 기반으로 JPA 변경 감지 기능을 활용하여 수정되도록 구현하였습니다.
계좌 수정 및 삭제 시에는 동일한 케이스를 검증하게 되는데, 수정 혹은 삭제하고자 하는 계좌가 본인의 명의가 맞는지, 즉 소유주가 본인이 맞는지를 검증하게됩니다.
계좌 삭제
위에 언급한 것처럼 삭제하고자하는 계좌가 본인의 명의인지를 검증 후 삭제를 진행합니다.
공통 처리 로직
사장님 관련 로직들을 작성할 때는 해당 요청을 보낸 사용자가 DB에 존재하는지, 사장님인지, status = ACTIVE 인지를 매번 검증해주도록 하였습니다. 일반유저 ROLE 을 가지고 사장님 관련 API 로 요청을 보내는 케이스를 방지하기 위함입니다.
다만 각각의 기능마다 사장님인지를 검증하는 로직이 들어가야하다보니 모든 메서드에 중복된 로직이 들어가야했고, 계좌 관련 서비스 로직에서 불필요하게 유저 검증 로직을 함께 담당하는 문제점이 존재했습니다.
따라서 owner/application 하위에 util 패키지를 만들고, 해당 패키지 내부에 사장님 여부를 검증해주는 별도의 헬퍼클래스를 구현하여 중복된 로직을 처리해줄 수 있도록 하였습니다.
스크린샷 (선택)
💬리뷰 요구사항(선택)
현재 요청 DTO = ~~~Request / 응답 DTO = ~~~Response 와 같이 네이밍을 해두었는데, 이처럼 요청 DTO / 응답 DTO 클래스에 대한 네이밍을 통일시키면 좋을 것 같습니다.
헬퍼클래스를 구현할 위치를 어디로 지정하면 좋을지 논의해보면 좋을 것 같습니다.
지금은 일단 실제 서비스 로직들을 담당하는 application 패키지의 하위에 util 이라는 패키지를 만든 후 그 내부에 헬퍼 클래스를 구현하였습니다.
다만 약간 마음에 안드는 점이, 현재 저는
특정 도메인/application하위에 해당 도메인에 대한 파사드 서비스를 하나 만들어둔 상태입니다.또한 파사드 내부에서 조립하여 쓸, 각각의 특화 기능 (ex) 계좌 / 자주쓰는 채팅 / 나의 푸드트럭 ) 들을 위한 패키지(ex) bankAccount) 를 별도로 만들어주었습니다.
그러다보니 실제 서비스 로직을 담당하는 패키지들과 util 패키지가 뒤섞여버려서 이 부분이 좀 마음에 들지 않더라구요. 그래서 이 부분에 대해 이야기해보고 싶습니다.
Summary by CodeRabbit