Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

점주 회원가입 #12

Merged
merged 32 commits into from
Aug 13, 2024
Merged

점주 회원가입 #12

merged 32 commits into from
Aug 13, 2024

Conversation

Dr-KoKo
Copy link
Member

@Dr-KoKo Dr-KoKo commented Aug 10, 2024 •
edited
Loading

💡 다음 이슈를 해결했어요.

Issue Link - #6


💡 이슈를 처리하면서 추가된 코드가 있어요.

도메인(Vendor)

Vendor를 생성할 때 검증로직을 통과해야합니다. 실패하면 InvalidCreationException가 발생합니다.

기타 도메인 변경

  • Order 생성 테이블 이름 변경 (Order -> Orders)
  • 컨벤션을 준수하지 않은 공백 수정 (space -> tab)

서비스(SignUpVendorService)

Vendor를 생성 및 DB에 저장하는 서비스 입니다.
Vendor는 모두 PayAccount를 필수적으로 가지고 있어야하므로 하나의 트랜젝션 내에서 PayAccount 및 Vendor를 생성합니다.
만약 신규 Vendor 저장을 (중복된 이메일 등의 이유로) 실패하면 롤백 됩니다.

컨트롤러(VendorController)

서비스(또는 도메인)에서 발생한 예외를 받아서 사용자가 받을 수 있는 형태의 데이터로 가공합니다.

GlobalExceptionHandler

공통 예외를 RFC 7807에 부합하게 처리하기 위해 ResponseEntityExceptionHandler를 상속받았습니다.

ApiResponse

공통 응답을 위해 모델을 고안하였습니다. 구체적인 모델에 대해서는 추후 논의가 필요해보입니다. 따라서 Discussion탭에 논의사항으로 올리도록 하겠습니다. - #13

테스트

각각의 레이어(도메인,서비스,컨트롤러)에서 단위테스트를 진행하였습니다.


💡 이런 고민을 했어요.

  • 서비스에서 롤백이 수행되는지 테스트할 수 있나? 하는 것이 옳나?

💡 다음 자료를 참고하면 좋아요.


✅ 셀프 체크리스트

  • 내 코드를 스스로 검토했습니다.
  • 필요한 테스트를 추가했습니다.
  • 모든 테스트를 통과합니다.
  • 브랜치 전략에 맞는 브랜치에 PR을 올리고 있습니다.
  • 커밋 메세지를 컨벤션에 맞추었습니다.
  • wiki를 수정했습니다.

@Dr-KoKo
[feat] Vendor 생성자
99db493 
제약조건 설정 및 검증 로직 구현
@Dr-KoKo
[fix] Order 테이블 이름 변경
54ea7e9 
Order가 예약어이기 때문에 테이블 이름 변경
@Dr-KoKo
[style] 코딩 컨벤션 적용
e7bc581 
개행문자를 space에서 tab으로 변경
@Dr-KoKo
[feat] jpa 설정
82f364f 
`~` show-sql: 실행되는 sql 조회
`~` generate-ddl: 테스트시 테이블 자동 생성
`~` ddl-auto: 어플리케이션 생성시 생성, 종료시 삭제
`~` open-in-view: 의도하지 않은 조회 쿼리 발생 방지
@Dr-KoKo
[feat] SignUpVendorService
ed11df9 
`+` SignUpVendorCommand: 서비스 호출을 위한 command
`+` NoOpPasswordEncoder: 아무런 기능을 제공하지 않는 PasswordEncoder
`+` DuplicateException(및 DuplicateEmailException): unique 제약조건에 부합하지 않을 때 발생하는 예외
`+` InvalidCreationException: 기타 제약조건에 부합하지 않는 경우 발생하는 예외
@Dr-KoKo
[feat] Vendor 회원가입 전송 계층 구현
03dd63c 
`+` AuthController: "/auth" 컨텍스트 처리
`+` ApiResponse: 공통 응답처리를 위한 모델
`+` ErrorCode: 공통 예외처리를 위한 코드
`+` SignUpVendorRequest: POST /auth/vendor 요청을 받는 dto
@Dr-KoKo
[chore] .gitkeep 삭제
17a2834
@Dr-KoKo
[test] Vendor 도메인 테스트
6c2522b 
Vendor 생성시 검증로직 테스트 코드 작성
@Dr-KoKo
[test] Vendor 레포지토리 테스트
bcc20a3 
`+` 성공시 DB에 저장
`+` 동일한 이메일 존재시 예외 발생
@Dr-KoKo
[test] Vendor 서비스 테스트
a0a49f3 
`+` 성공시 vendorReposity/payAccountRepository의 save 호출
`+` 실패시 DuplicateEmailException 발생
@Dr-KoKo
[fix] Vendor 컨트롤러 변경
db202b2 
`~` /auth/vendors -> /vendors
`~` 예외처리 rfc7807에 부합하게 변경
@Dr-KoKo
[feat] Phone 검증 기능
dc51829 
전화번호를 검증하기 위한 어노테이션 및 커스텀 Validator 구현
@Dr-KoKo
[test] Vendor 컨트롤러 테스트
0900ed4 
`+` 판매자 회원가입 테스트
@Dr-KoKo Dr-KoKo added the ✨ Feature 기능 개발 label Aug 10, 2024
@Dr-KoKo Dr-KoKo added this to the 프로토타입 만들기 milestone Aug 10, 2024
@Dr-KoKo Dr-KoKo requested a review from a team August 10, 2024 14:35
@Dr-KoKo Dr-KoKo self-assigned this Aug 10, 2024
@Dr-KoKo Dr-KoKo linked an issue Aug 10, 2024 that may be closed by this pull request
[기능] 점주 회원가입 #6
Closed
@Dr-KoKo Dr-KoKo closed this Aug 10, 2024
@Dr-KoKo Dr-KoKo reopened this Aug 10, 2024
@Hyeon-Uk
Copy link
Contributor

Hyeon-Uk commented Aug 10, 2024 •
edited
Loading

전반적으로 CheckedException을 이용해서 구현하셨는데 구체적인 이유가 궁금합니다 :)

@Dr-KoKo
Copy link
Member Author

Dr-KoKo commented Aug 11, 2024 •
edited
Loading

전반적으로 CheckedException을 이용해서 구현하셨는데 구체적인 이유가 궁금합니다 :)

예외에는 1) 도메인에서 던지는 예외, 2) 서비스에서 던지는 예외, 3) 컨트롤러(전송계층)에서 던지는 예외 3가지로 분류할 수 있습니다. 이렇게 던져진 예외는 컨트롤러 또는 ExceptionHandler에서 받아서 수행하게 될 거예요.

그런데 서비스 또는 도메인에서 UncheckedException을 던지면, 1) (예외를 받는) 컨트롤러를 구현할 때 서비스 하단에서 발생하는 예외를 찾기 위해 내부의 코드를 뜯어봐야합니다. 2) 또 서비스의 로직이 바뀌었을 때 변경사항이 외부에 노출되지 않습니다. (메서드 시그니처가 변경되지 않습니다).

따라서 저는 서비스/도메인에서는 (특히 변경이 자주 일어나는 프로젝트에서) CheckedException을 던지는 편이고, 이를 컨트롤러에서 받아서 처리하는 편입니다. 컨트롤러에서 예외를 받을 때, 공통적으로 처리되어야 하는 예외가 있다면 UncheckedException으로 래핑해서 던지게 됩니다.

@Hyeon-Uk
Copy link
Contributor

그런데 서비스 또는 도메인에서 UncheckedException을 던지면, 1) (예외를 받는) 컨트롤러를 구현할 때 서비스 하단에서 발생하는 예외를 찾기 위해 내부의 코드를 뜯어봐야합니다. 2) 또 서비스의 로직이 바뀌었을 때 변경사항이 외부에 노출되지 않습니다. (메서드 시그니처가 변경되지 않습니다).

저는 이 부분에 대해서 살짝 다른 관점입니다!

먼저 저는 예외 상황에 대해 복구를 강제해야 하면 Checked Exception을 사용해야 하고, 복구를 강제할 필요가 없다면 Unchecked Exception을 사용하는 편입니다!

그래서 해당 관점에서 보게 된다면

  1. (예외를 받는) 컨트롤러를 구현할 때 서비스 하단에서 발생하는 예외를 찾기 위해 내부의 코드를 뜯어봐야 합니다.

    • 대부분의 컨트롤러는 서비스에서 던지는 Exception을 복구한다는 느낌보단 오류에 대한 format으로 감싸 클라이언트에게 보내주는 역할만 한다고 생각합니다.
    • 따라서 컨트롤러의 각 메서드들이 하나하나 알 필요가 있느냐는 의문이 들었습니다. 해당 관심사를 @RestControllerAdvice에서 공통으로 처리해 줘도 되지 않을까? 라는 생각을 했습니다.

  2. 서비스의 로직이 바뀌었을 때 변경사항이 외부에 노출되지 않습니다. (메서드 시그니처가 변경되지 않습니다).

    • 시그니처의 변경이 일어나게 된다면 개발자에게 처리를 강제할 수 있기 때문에 코드의 안정성이 향상됩니다.
    • 하지만 해당 메서드를 사용하는 모든 코드를 수정하고 처리해 줘야 하는 유지/보수의 문제가 생긴다고 생각합니다.

이 부분에서도 토론해 보면 좋을 거 같네요 :)

@Dr-KoKo
Copy link
Member Author

Dr-KoKo commented Aug 11, 2024

좋습니다. 더 자세한 논의는 #14 에서 진행하도록 하겠습니다.

kimhyun5u
kimhyun5u approved these changes Aug 11, 2024
View reviewed changes
Copy link
Member

@kimhyun5u kimhyun5u left a comment

Choose a reason for hiding this comment

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

👍

사담: 점주랑 구매자 회원가입이 거의 동일하게 구현되어서 동훤님 코드 많이 참고했습니다..ㅎㅎ
그리고 제가 동훤님 커밋을 가져와야할 부분이 많았어서 저 작은 단위로 커밋해주시면 좋을 것 같아요

PayAccount Repository 같은 같이 사용하는 부분은 그냥 동훤님 commit cherry pick 해서 가져올라 했거든여

그리고 예외 처리 방법에 대해서 논의 해보면 좋을 것 같네요!

src/main/java/camp/woowak/lab/web/dto/request/SignUpVendorRequest.java Outdated Show resolved Hide resolved
@Dr-KoKo
[fix] Vendor 회원가입 RequestDto 수정
7bab415 
파라미터가 오지 않은 경우 또는 비어있는 경우에 대한 검증 추가
@Dr-KoKo
[test] 파리미터가 비어있는 Vendor 회원가입 요청
8d52b59 
파라미터가 오지 않은 경우 또는 비어있는 경우에 대한 테스트 추가
@Dr-KoKo
[fix] open-in-view 기본 설정으로 복구
c5755f2
@Dr-KoKo Dr-KoKo closed this Aug 12, 2024
@Dr-KoKo Dr-KoKo reopened this Aug 12, 2024
@Dr-KoKo Dr-KoKo marked this pull request as draft August 12, 2024 14:54
@Dr-KoKo
Merge remote-tracking branch 'origin/main' into feature/6_Dr-KoKo_점주_…
1383d37 
…회원가입
@Dr-KoKo
[fix] 전역 ExceptionHandler 수정
f9073b5 
`-` 기존에 등록된 ExceptionHandler
`~` 새로운 ExceptionHandler에 ResponseEntityExceptionHandler 상속
@Dr-KoKo
[fix] DuplicateEmailException
49d8714 
HttpStatusException을 상속하는 구조로 변경
@Dr-KoKo
[feat] Vendor 전용 ErrorCode 생성
a967a2c
@Dr-KoKo
[fix] Vandor 도메인에서 검증 로직 분리
819b08d 
검증 로직은 VendorValidator에서 수행
@Dr-KoKo
[test] Vandor 도메인 테스트 수정
033b759 
Exception 계층 수정으로 인한 테스트 코드 수정
@Dr-KoKo
[fix] SignUpVendorService는 UncheckedException을 던진다
ffdb3a4 
서비스에서 UncheckedException을 던지게 수정
@Dr-KoKo
Merge remote-tracking branch 'origin/main' into feature/6_Dr-KoKo_점주_…
f1798b2 
…회원가입
@Dr-KoKo
[refactor] 코딩 컨벤션 적용
2f94a72
@Dr-KoKo
[fix] VendorApiController는 APIResponse를 응답
d6895c3 
예외는 VendorApiControllerAdvice에서 처리
@Dr-KoKo
[refactor] 코딩 컨벤션 적용
5fbc11a
@Dr-KoKo
[test] 중복 이메일은 ProblemDetail로 응답
5235f93
@Dr-KoKo Dr-KoKo marked this pull request as ready for review August 13, 2024 05:57
@Dr-KoKo
[fix] 기존의 CheckedException 제거
1c0386f 
HttpStatusException을 상속받은 RuntimeException으로 변경
@Dr-KoKo Dr-KoKo merged commit 2b44166 into main Aug 13, 2024
1 check passed
@kimhyun5u kimhyun5u deleted the feature/6_Dr-KoKo_점주_회원가입 branch August 22, 2024 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kimhyun5u kimhyun5u kimhyun5u approved these changes

@Hyeon-Uk Hyeon-Uk Hyeon-Uk approved these changes

@june-777 june-777 june-777 approved these changes

Assignees

@Dr-KoKo Dr-KoKo

Labels
✨ Feature 기능 개발
Projects
3K1K Project
Status: Done
Milestone
프로토타입 만들기
Development

Successfully merging this pull request may close these issues.

[기능] 점주 회원가입
4 participants
@Dr-KoKo @Hyeon-Uk @kimhyun5u @june-777