14조 코드리뷰 1회차 #14
14조 코드리뷰 1회차 #14
Conversation
이슈 템플릿 재작성을 위한 파일 삭제
PR 템플릿 잘못 작성
이슈 템플릿 적용되지 않는 부분 수정
템플릿으로 적용되지 않던 부분 수정
템플릿 파일들은 master 브랜치에 있으면 되기 때문에 삭제
.properties -> .yml
| @@ -0,0 +1,66 @@ | |||
| plugins { | |||
There was a problem hiding this comment.
다른 조에도 비슷한 리뷰를 드리긴했는데요~ version을 변수로 관리하면 이점을 얻을때가 있어서
저는 아래와 같이 사용하기도 해서 코멘트드려요~!
buildscript {
ext {
spring_boot_version = '3.3.3'
spring_dependency_management = '1.1.6'
}
repositories {
mavenCentral()
}
}
plugins {
id 'java'
id 'org.springframework.boot' version "${spring_boot_version}"
id 'io.spring.dependency-management' version "${spring_dependency_management}"
}| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @ToString | ||
| @Getter | ||
| public abstract class BaseEntity { |
There was a problem hiding this comment.
감사용 필드를 저장하기 위한 BaseEntity네요~
나중에는 "누가" 생성했는지, 수정했는지에 대한것도 추가되면 좋을것 같아요~
| private Long sellerId; // 판매자 식별자 | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| private Product productId; |
There was a problem hiding this comment.
오타일수도 있을것 같은데요~
Type이 Product이니, productId 보다는 product가 되는게 좋지 않을까요?
혹은 Type을 Long 혹은 String으로 변경해주시면 좋을것 같아요.
There was a problem hiding this comment.
어떤 비즈니스인지 제가 정확히 잘 몰라서 코멘트 드리기는 어렵지만
productId를 통해서 참조하게 되면, 상품명이 변경되면 기존 결제내역에 대한 상품명도 전부 변경될것같아요
이런 부분을 막기위해서는 결제 당시의 productName도 결제내역에 가지고 있어야 할수 있어요.(스냅샷)
이런 부분은 괜찮으실지, 팀내에서 검토되면 좋을것 같아요.
There was a problem hiding this comment.
현재는 결제가 말 그대로 "돈을 쓰는것"에 대해서만 표현되어있는것 같아요. 취소나 부분취소 같은 기능도 필요하실까요?
There was a problem hiding this comment.
어떤 비즈니스인지 제가 정확히 잘 몰라서 코멘트 드리기는 어렵지만 productId를 통해서 참조하게 되면, 상품명이 변경되면 기존 결제내역에 대한 상품명도 전부 변경될것같아요 이런 부분을 막기위해서는 결제 당시의 productName도 결제내역에 가지고 있어야 할수 있어요.(스냅샷) 이런 부분은 괜찮으실지, 팀내에서 검토되면 좋을것 같아요.
이 부분은 생각하지 못했었습니다. 좋은 방법인 것 같아요 반영할 수 있도록 하겠습니다!
현재는 결제가 말 그대로 "돈을 쓰는것"에 대해서만 표현되어있는것 같아요. 취소나 부분취소 같은 기능도 필요하실까요?
저희 서비스는 포인트라는 가상화폐를 통해서만 거래가 이루어지는데요. 결국 실질적인 결제가 발생하는 것은 포인트를 구매하는 것이 전부입니다. 그리고 환전기능을 추가하여 포인트 수량만큼 계좌로 송금받는 방식을 생각하고 있습니다. 결제 취소를 원하면 구매한 포인트를 환전 신청하도록 운영될 것 같습니다! 그래서 아직은 취소 기능의 필요성은 못 느끼고 있습니다
그런데 환전을 위해 사업자등록이 되지 않은 개인에게 송금하는 것을 어떻게 구현해야할지 잘 모르겠습니다. 현재 결제시스템은 PG사로 토스페이먼츠를 이용하고 있는데 토스페이먼츠에서 제공하는 https://docs.tosspayments.com/reference#지급대행 해당 기능은 유사해보이나 저희 서비스에서 원하는 기능과 약간 다른것 같네요 ㅜㅜ
There was a problem hiding this comment.
@nove1080
아하 그렇군요~ 포인트를 구매할때는 카드결제, 혹은 무통장 입금을 지원하나요?
이런 부분을 구현해야한다! 그런건 아니고, 뭔가 제가 학생이였을때는 고민해보지 못했던 그런 문제들에 대해서 말씀드리자면
카드결제시(여기서는 포인트 충전시?), 카드사 혹은 PG사에 결제금액에 대한 일정 수수료를 지급해야할 수 있습니다.
환전신청시(여기서는 결제취소시?) 통장 입금을 할때도 일정 금액에 대한 수수료를 지급해야할 수 있어요.
말씀주신것처럼 토스페이먼츠도 PG사중 하나인데요, 잘 연동되면 정말 큰 공부가 되겠네요ㅎㅎ.. 화이팅입니다!
| public class PaymentEvent extends BaseEntity { | ||
|
|
||
| @Column(nullable = false) | ||
| private Long buyerId; // 구매자 식별자 |
There was a problem hiding this comment.
명시적으로 객체참조를 끊어주시려고 Id만 매핑하는거라면 너무 좋네요~ 👍👍
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| @Getter | ||
| public class Spot { |
There was a problem hiding this comment.
도메인만 보고는 어떤 서비스인지 잘 파악하기 어렵네요 ㅋㅋ..
나중에 혹시 멘토링을 주실일이 있다면, 그때 서비스관련 설명을 해주시면 코드리뷰를 할때 더 도움될것 같아요!
There was a problem hiding this comment.
넵 다음 PR에서는 더 자세하게 작성해놓겠습니다!
| } | ||
|
|
||
| public void deleteSpot(Long id) { | ||
| spotRepository.deleteById(id); |
There was a problem hiding this comment.
제 경험상 서비스를 구현할때 데이터를 정말 삭제하는일은 많지 않은것 같아요.
Spot 자체에 Status값을 두고, REMOVED로 Soft Delete 하는것도 괜찮은 방법입니다~
|
|
||
| @Repository | ||
| public interface SpotRepository extends JpaRepository<Spot, Long> { | ||
| List<Spot> findByLatAndLng(BigDecimal lat, BigDecimal lng); |
There was a problem hiding this comment.
lat과 lng에 Database Index가 필요하겠네요.
There was a problem hiding this comment.
넵! 공부 더 해서 반영하겠습니다!
| package com.ordertogether.team14_be.payment.domain; | ||
|
|
||
| /** 결제 상태 */ | ||
| public enum PaymentOrderStatus { |
There was a problem hiding this comment.
안녕하세요 김상엽 멘토님! 저는 14조 쿠키 나제법입니다. 멘토님께서 결제 도메인에 대한 경험도 있으셔서 정말 많이 배울 수 있을 것 같아 기대됩니다! 앞으로 잘 부탁드립니다!😃
결제는 비동기로 진행되나요?
처음에는 비동기를 고려하지 않고 결제 시스템을 구상해보았습니다.
위 시퀀스 다이어그램은 초기에 구상했던 비동기를 고려하지 않은 결제 처리 방식을 나타내보았습니다.
참고로
Point는 저희 서비스에서 사용될 가상화폐(Point)를 의미하고 저희 결제 시스템은 이것을 구매하는 용도로만 사용됩니다.
동기 방식이 아닌 비동기를 선택한 이유
결제가 알 수 없는 예외로 인해 실패하게 되는 경우 사용자 경험을 위해 재시도를 통해서 결제를 최대한 성공시켜주는 것이 좋지 않을까? 라는 생각이 들었습니다.
또한, 선착순으로 진행되는 타임세일 등의 상황 등을 고려하면 먼저 결제를 시도했음에도 네트워크 지연등의 알 수 없는 이유로 결제 실패로 처리하는 것은 조금은 불합리 해보이기도 했습니다.
따라서 최대한 결제 요청을 성공시켜주고자 재시도를 통해 실패를 복구하는 과정을 추가하였습니다.
그런데 재시도를 통한 PSP로의 요청은 시간이 오래 걸리기 때문에 동기 방식에서의 수행은 바람직하지 못하다고 생각되었습니다. (보통 재시도에 딜레이를 주는 방식으로 많이 구현하더라구요)
두번째로는, 결제가 이루어진 다음 재고를 차감하는 등의 비즈니스 로직이 하나 둘씩 추가되면 오래 걸리지 않을까? 라는 생각을 해보았습니다.
위와 같은 이유로 비동기로 처리하는 것이 좀 더 괜찮지 않을까? 라고 생각했습니다.
아니면 비교적 구현이 수월한 동기방식으로 먼저 개발하고 이후에 테스트를 통해 비동기 처리가 필요한 부분이 생기면 해당 부분만 비동기로 전환하는 것은 어떤가요?(이 개발과정이 좀 더 재밌을 것 같아보입니다) 그리고 이에 대해 멘토님의 생각이 궁금합니다!
There was a problem hiding this comment.
@nove1080
말씀주신사항들이 되게 합리적이라고 생각하고, 사용자경험을 위해서 재시도를 통해 결제를 성공시켜주는것도 좋은것 같아요.
토스 페이먼츠 결제관련 문서를 읽고왔는데요, 결제승인 이전 사용자가 결제를 마치고 리다이렉트 해주는것을 확인하였습니다.
이 과정에서, 결제승인이 비동기로 동작하면 사용자는 결제가 완료되었다고 판단할 가능성이 높고,
다만 실제로 비동기로 동작하다보니 순간적으로 사용자가 확인했을때 카드결제도 되지 않았고 포인트도 충전되지 않은 상황이 벌어질 수 있을것 같아요.
여러 기타 간편결제 시스템을 사용하다보면 결제시에, 원형 로딩 애니메이션이 뜨면서 결제가 완료될때까지 기다리는 화면을 보신적이 있으실까요? 아마 제 생각에 그 부분이 결제승인 과정과 유사한 과정이라고 생각하는데요, 일정간격을 가지고 리트라이를 하되, 오히려 결제가 완료된 화면을 보는게 조금더 사용자 경험에서 좋지 않을까? 싶은 생각도 드네요!...
일단은 너무 앞서나가는 내용인것 같아서~ 팀내에서 천천히 고민해보시고 기획개발진행하시면 좋을것 같습니다.
|
질문 주신것에 대한 제 생각입니다 ㅎㅎ..
일반적인 스프링 계층구조, Controller -> Service -> Repository의 흐름에서 저는 DTO가 2가지 타입이 있다고 생각하는데요, 2.프로젝트 구조를 도메인별로 나눈 후 그 안에서 자신이 구현해야할 기능을 계층형으로 구현하기로 했습니다. 만약 각기 다른 도메인의 엔티티를 참조해야하는 엔티티(N:M 관계로 인해 생성되는 양 엔티티의 PK를 참조하는 엔티티)는 어느 도메인에 두는 것이 좋나요? 따로 패키지를 만들어서 관리하는 것이 좋나요? 말씀하신 엔티티가 비즈니스적으로 의미를 가진다면 별도 패키지를 만들어서 관리하는게 좋을것 같습니다. 특정 패키지에 두기에는 어려울것 같아요.
예외사항이 어떤것들이 있을지는 잘 모르겠지만, 결국 이미지서버가 어디까지 예외를 수용해야할까? 에 대한 역할과 책임문제인것 같네요. 이런 부분에 대해서는 사람마다 각기 의견이 달라서 정답은 없겠지만, 저는 이미지 서버에서 되도록이면 많은 예외를 처리하였으면 해요. 그리고 이미지 처리가 불가능 할 경우, 500에러를 주는방식으로 개발할것 같습니다. |
📌 관련 이슈
결제 시스템 데이터 모델링 #7
Spot API 구현 #2
카카오 로그인 기능 구현 #1
✨ PR 내용
✨ 작업 내용
https://quickest-asterisk-75d.notion.site/API-2be0fa351c72490ba10edff6bfab1205
위 링크는 저희 조 API 문서 링크입니다!
✏ 리뷰받고 싶은 내용
위 링크는 이미지 처리를 위해 fastapi로 만든 서버를 올려둔 레포지토리입니다. 이 서버에서 처리되어야 하는 예외사항들은 Spring의 Service Layer에서 하는 것이 나은지, 아니면 이미지 처리 서버에서 처리 후 반환하는 것이 좋은지 궁금합니다.