[회원 가입] 회원가입 연관 비즈니스 로직 & API 구현(#8) #48
Conversation
보낼 화살 수를 담은 요청 DTO 정의
- 화살 송수신 내역 repository 정의 - 보낸 사용자, 받은 사용자를 통해 내역 존재 여부를 확인하는 로직 구현
보낸 사용자, 받은 사용자를 통해 송수신 내역 존재 여부를 확인하는 로직 구현
다음 상황들에 대한 예외 enum을 정의하였다. - 자기 자신에게 화살을 보내는 경우 - 이미 화살을 보낸 사용자인 경우 - 가진 화살 수가 부족해 화살을 보낼 수 없는 경우
감소하려는 화살 수가 사용자가 보유한 화살 수보다 클 경우 예외 발생
화살 보내기 비즈니스 로직은 다음 과정을 거친다. 1. 자기 자신에게 화살을 보내는 상황 검증 2. 화살을 보낸 사용자에게 또 보내는 상황 검증 3. 화살 내역 저장 4. 보내는 사용자 화살 감소, 화살이 부족할 경우 예외 발생(과정 3 롤백) 5. 받는 사용자 화살 증가
- query, command를 service, repository를 중개하는 별도의 계층을 통해 구분 - 이에 따라 기존 command repository 삭제 - 기존 command repository 사용 위치, 화살 command로 대체
# Conflicts: # be/src/main/java/yeonba/be/arrow/repository/ArrowCommand.java # be/src/main/java/yeonba/be/arrow/service/ArrowService.java # be/src/main/java/yeonba/be/exception/CommonException.java # be/src/main/java/yeonba/be/user/entity/User.java
서로 대치되는 작업을 수행하는 상황을 잘 표현할 수 있도록 메서드 이름 수정
- 이미 화살을 보낸 사용자 예외 - 화살이 부족하여 화살을 보낼 수 없는 예외
가독성 향상을 위해 간단한 이름을 수정
dev 브랜치 작업 내역 병합에 따른 코드 배치 수정
좀 더 명확하게 대치되는 의미를 나타낼 수 있도록 화살을 받는 사용자 ID 파라미터 이름을 receiverId로 수정
- 이름 칼럼 추가 - 테스트시 활용 가능한 생성자 추가
- 래퍼 타입 필드들 기본 타입으로 변경 - 프로필 사진 URL 리스트 필드 추가 - 사용자가 가진 총 화살 수 필드 추가 - 음주 성향, 흡연 성향 필드 추가 - 코드 정렬
- 레퍼런스 타입 not null 필드들, Column 어노테이션 사용 명시 - salt 필드 추가 - 생성 일시, 최종 수정 일시 필드 추가 - JpaAuditing 활용, 생성/최종 수정 일시 필드들 엔티티 저장시 자동 설정 - 새로운 생성자 구성
프로필을 조회하는 사용자 ID의 경우 상대방이 화살을 보넀던 사용자인지 확인하기 위해 파라미터로 받는다.
- 요구사항 변경에 따라 음주 성향, 흡연 성향 필드 삭제 - 성별 정보 제공 메서드 추가
- 음주 성향, 흡연 성향 필드 삭제 - 성별 필드 추가
- 음주 성향, 흡연 성향 필드 삭제 - 생성자 성별 필드 설정 로직 수정 - 코드 정렬
- 음주 성향, 흡연 성향 삭제 - 성별 정보 응답에 포함토록 수정 - 코드 정렬
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
간격 확인해서 다시 올려주시기 바랍니다. 이 PR은 close 하겠습니다. |
# Conflicts: # be/build.gradle # be/src/main/java/yeonba/be/config/SmsConfig.java # be/src/main/java/yeonba/be/exception/LoginException.java # be/src/main/java/yeonba/be/login/controller/LoginController.java # be/src/main/java/yeonba/be/login/dto/request/UserIdInquiryRequest.java # be/src/main/java/yeonba/be/login/dto/request/UserPhoneNumberVerifyRequest.java # be/src/main/java/yeonba/be/login/dto/response/UserEmailInquiryResponse.java # be/src/main/java/yeonba/be/login/service/LoginService.java # be/src/main/java/yeonba/be/mypage/service/MyPageService.java # be/src/main/java/yeonba/be/user/entity/User.java # be/src/main/java/yeonba/be/user/repository/UserQuery.java # be/src/main/java/yeonba/be/user/repository/UserRepository.java # be/src/main/java/yeonba/be/util/ServiceRegex.java # be/src/main/java/yeonba/be/util/SmsService.java # be/src/main/java/yeonba/be/util/VerificationCodeGenerator.java
dev 브랜치 내역 병합으로 인한 코드 수정
| example = "마른 체형") | ||
| example = "메른 체형") |
| private int photoSyncRate; | ||
| private double photoSyncRate; |
There was a problem hiding this comment.
싱크로율 데이터는 integer 타입으로 응답, 저장 등 관리되는데 double로 바꾸신 이유가 무엇인가요?
There was a problem hiding this comment.
업데이트하며 미처 수정되지 못한 부분 같습니다. 반영하겠습니다!
| @Getter | ||
| @RequiredArgsConstructor | ||
| public class ProfilePhotoDeletionEvent { | ||
|
|
||
| private final List<ProfilePhoto> profilePhotos; | ||
| } |
There was a problem hiding this comment.
해당 클래스에 대해서 설명 부탁드립니다.
- EOF? EOL? 지켜주시기 바랍니다.
|
|
||
| public void deleteProfilePhotos(List<ProfilePhoto> profilePhotos) { |
There was a problem hiding this comment.
profilePhoto를 delete하는 로직이 필요없어 추가하지 않는 것으로 결론이 났던 것같은데 추가하신 이유가 무엇인가요?
- 해당 로직 자체를 추가하면 안된다는 것은 아니지만 이유가 있더라도 한 번 결론이 났던 내용을 수정할 때는 논의 후에 수정하는 것이 좋을 것같은데 어떻게 생각하시나요?
There was a problem hiding this comment.
불필요한 이하 작업 내역들 전부 삭제하겠습니다!
- 프로필 사진 삭제 로직
- 프로필 사진 삭제 이벤트 & 이벤트 리스너
| private final Duration ACCESS_TOKEN_DURATION = Duration.of(8, ChronoUnit.HOURS); | ||
| private final Duration REFRESH_TOKEN_DURATION = Duration.of(10, ChronoUnit.DAYS); |
There was a problem hiding this comment.
제가 잘 몰라서 그러는데, ChronoUnit 클래스에 대해서 설명해주시면 감사하겠습니다.
There was a problem hiding this comment.
ChronoUnit은 java 8에서 추가된 time 패키지에 속한 표준 시간 단위를 표현하는 데 사용되는 열거형입니다. 특정 라이브러리, 프레임워크가 제공하는 시간 단위를 사용해야만 하는 상황이 아니면 모든 상황을 커버할 수 있는 타입으로 알고 있습니다. 서로 다른 일시 단위인 access token과 refresh token의 유효 기간을 일관성 있게 표현하기 위해 사용하였습니다.
| return Jwts.builder() | ||
| .setSubject(user.getEmail()) |
There was a problem hiding this comment.
jwt는 token을 알고 있다면 누구나 내용을 알 수 있는데 subject에 사용자 이메일을 넣은 이유가 무엇인가요?
제 생각에는 특별한 장점이 없고 굳이 사용자의 이메일 정보를 노출시킬 필요가 없다고 생각해서 여쭤봅니다.
There was a problem hiding this comment.
jwt의 sub 클레임은 토큰이 대상으로 하는 주체를 나타내는 용도로 사용한다고 알고 있습니다. 이에 관례적으로
이메일이 많이 쓰이는 것으로 알아 로직을 저런 형태로 구성하였습니다. 하지만, 주신 의견대로
현재 jwt를 이용한 인증/인가 방식에서는 별다른 이점이 없는 것 같습니다. 수정하겠습니다.
|
|
||
| private final ApplicationEventPublisher eventPublisher; |
There was a problem hiding this comment.
해당 클래스에 대한 설명 부탁드립니다.
| @Transactional | ||
| public void saveUserPreference(User user, UserJoinRequest request) { |
There was a problem hiding this comment.
해당 메서드는 JoinService의 @transactional이 붙어있는 join() 메서드에서 호출되는 메서드처럼 보이는데 @transactional이 있는 이유는 무엇인가요?
There was a problem hiding this comment.
- 스프링은 기존 트랜잭션 진행 중 다른 트랜잭션 로직이 실행되면 이 둘을 하나의 물리 트랜잭션(실제 DB 연결 트랜잭션)으로 다룹니다.
- 이런 최상위 물리 트랜잭션 하의 트랜잭션은 논리 트랜잭션으로 다뤄지며 기본적으로 하나의 논리 트랜잭션이라도 롤백될 경우 전체 물리 트랜잭션을 롤백합니다.
- 회원가입 과정에서 사용자 저장/프로필 사진 저장/선호 조건 저장은 한 트랜잭션 내에서 이뤄져야 하고 이들 중 한 작업이라도 롤백된다면 전체가 롤백되어야 하므로 스프링의 트랜잭션 관리를 이용하기 위해 하위 로직들에도
@Transactional어노테이션을 사용했습니다. - 한편, 해당 비즈니스 로직이 특정 상위 로직에 의해서만 이용되더라도 오해의 소지가 없도록 어떤 트랜잭션 하에서 실행되어야 하는 지 명시할 필요성이 있다고 생각합니다.
There was a problem hiding this comment.
어떤 상황인지는 이해되지만, 무슨 말인지 잘 이해가 안됩니다.
제가 이해하는 바로는 2번째 문장과 3번째 문장이 서로 상반된 말씀을 하고 계시는 것같습니다.
일단 제 말씀을 드리자면,
상위 메서드에 @Transactional이 붙어있다면 해당 메서드에서 호출되는 메서드에는 @Transactional이 없더라도 전체가 하나의 트랜잭션으로 동작하는 것으로 알고 있습니다. 즉, JoinService의 join() 메서드에 @Transactional이 있기때문에 호출되는메서드들 중 하나라도 Rollback되면 전체가 Rollback 되는 것으로 알고 있습니다.
추가적으로, 이 부분을 이해하고 작성하신 거라면, @Transactional 남용으로 인해 발생하는 오버헤드에 대해서도 생각해보신 건가요?
There was a problem hiding this comment.
제가 @Transactional의 사용에 대해 오인하고 있던 부분이 있었습니다. 수정토록 하겠습니다.
| @Transactional | ||
| public void saveProfilePhotos(User user, UserJoinRequest request) { |
There was a problem hiding this comment.
해당 메서드는 JoinService의 @transactional이 붙어있는 join() 메서드에서 호출되는 로직처럼 보이는데 @transactional이 있는 이유는 무엇인가요?
| if (!validateProfilePhotosExtensions(profilePhotos)) { | ||
| throw new IllegalArgumentException("jpg, jpeg, png 확장자 형식의 파일만 허용됩니다."); | ||
| } |
There was a problem hiding this comment.
기존과 다른 형식으로 예외처리를 한 이유가 무엇인가요?
There was a problem hiding this comment.
앞선 jwt 관련 예외를 IllegalArgumentException으로 처리한 것과 같은 이유입니다.
- 사진 싱크로율 필드 타입 int로 수정 - 채형 필드 명세 오타 수정
- 동일한 URL로 사진을 업로드하기에 이미 업로드된 사진 URL을 업데이트하는 방식으로 데이터 무결성 보장 가능 - 이에 따라 불필요한 로직들 제거
- issuedAt, 기존 서비스 명명 방식에 부합하고 더 친숙한 generatedAt으로 수정 - 불필요한 사용자 이메일 sub 설정 제거
- 유효하지 않은 JWT - 허용되지 않은 이미지 파일 확장자
# Conflicts: # be/src/main/java/yeonba/be/exception/JoinException.java # be/src/main/java/yeonba/be/exception/LoginException.java # be/src/main/java/yeonba/be/login/controller/LoginController.java # be/src/main/java/yeonba/be/login/service/LoginService.java
인가 과정이 필요 없는 리소스 경로들 설정
이미 사용하는 전화번호인지 확인하는 작업을 수행한다는 것을 잘 표현하기 위해 수정
- 이미 사용 중인 전화번호 검증 메서드명 변경에 따라 코드 수정 - dev 브랜치 작업 내역 병합에 따른 수정
dev 브랜치 작업내역 병합에 따른 코드 수정
| public boolean isAlreadyUsedPhoneNumber(String phoneNumber) { | ||
|
|
There was a problem hiding this comment.
메서드 이름으로 짓기에 애매한 것같은데 validateUsedPhoneNumber는 어떠신가요?
|
|
||
| return reason; | ||
| } | ||
| } |
| preferredAnimal); | ||
| userPreferenceCommand.save(userPreference); | ||
| } | ||
| } |
- 일단 사용하지 않는 jwt 검증 로직 삭제 - issuedAt 변수명, generatedAt으로 일괄 수정
사용 중인 데이터(이메일, 폰번호, 별명) 검증 로직 이름 일괄 수정
|
후속 PR에서 |
작업 대상
📄 작업 내용
🙋🏻 주의 사항
salt생성UserService에 엔티티를 생성하는 로직을 구현하고JoinService에서 해당 로직들을 하나의 트랜잭션으로 묶어 처리하도록 구성📎 관련 이슈
레퍼런스