-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
[refactor] Facade 객체를 활용해 uploadPhoto 트랜잭션에서 이메일 전송 로직 분리 #52
Conversation
@@ -34,6 +35,7 @@ public class AuthServiceImpl implements AuthService { | |||
private final String ISS = "https://kauth.kakao.com"; | |||
|
|||
@Override | |||
@Transactional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
login 로직에서 트랜잭션 어노테이션이 누락되어 추가했습니다.
(이번 PR에서는 uploadPhoto 트랜잭션에서 sendEmail만 분리하는 작업을 했는데, 이후 다른 이슈에서 login에서 provider를 당장 분리할 수 없는 이유에 대해 서술하겠습니다.)
@@ -9,7 +9,7 @@ | |||
@NoArgsConstructor | |||
@AllArgsConstructor | |||
@Builder | |||
public class EmailInfo { | |||
public class EmailInfoDto { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EmailInfo를 볼 때마다 엔티티로 착각해서, 네이밍을 수정했습니다.
@Service | ||
@RequiredArgsConstructor | ||
@Slf4j | ||
public class PhotoServiceFacadeImpl implements PhotoService{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PhotoService 인터페이스의 구현체를 PhotoserviceImpl에서 PhotoServiceFacadeImpl로 변경했습니다. PhotoServiceImpl에서는 레포지토리와 가장 가까운 단에 위치하여 요청을 처리하고, Facade 객체는 인터페이스와 PhotoServiceImpl 사이에 존재합니다.
서브 시스템을 감추는 상위 수준의 인터페이스를 제공함으로써 시스템의 복잡도를 낮추는 역할을 합니다.
private final JavaMailSender mailSender; | ||
private final TemplateEngine templateEngine; | ||
|
||
private final String EMAIL_LINK = "https://www.google.com/"; // TODO : 프론트 분들 링크 관련 답변 오면 프레임 페이지 링크 관련 수정 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이후에도 변동 가능성이 큰 링크여서 변수로 분리했습니다.
@Override | ||
public void uploadPhoto(Long photoRequestId, String imageUrl) { | ||
sendEmail(photoServiceImpl.uploadPhoto(photoRequestId, imageUrl)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- photoController는 여전히 photoService 인터페이스의 uploadPhoto를 사용합니다. 다만 구현체가 Facade 객체로 바뀌었습니다.
photoServiceImpl.uploadPhoto(photoRequestId, imageUrl)
내부에서 PhotoRequest, PhotoResult Repository update/save를 수행하고 EmailInfoDto를 반환합니다.- sendEmail에서는 전달받은 EmailInfoDto로 이메일 전송을 수행합니다!
} | ||
|
||
// 유저에게 이메일 전송 | ||
private void sendEmail(EmailInfoDto emailInfoDto) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR overview의 작업 후 이미지를 보면 한눈에 확인할 수 있습니다. sendEmail 로직이 Facade 객체에 위치합니다.
|
||
@Service | ||
@Transactional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이제 PhotoServiceImpl에서 Repository 단에 가까이 위치하며 트랜잭션을 수행합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른 클래스와 달리 이 클래스에서만 클래스 단위의 @transactional을 붙이신 특별한 이유가 있으실까요??
그리고 getPhotoResult는 조회 로직인 만큼 (readOnly = true) 옵션이 필요해 보입니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리팩토링 하는 과정에서 옵션을 지웠던 것 같아요, 클래스 단위 트랜잭션 어노테이션 제거하고 나눠서 작성했습니다! 빠뜨린 부분 알려주셔서 감사합니다!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 처음에 EmailService로 따로 분리하는 걸 생각했었는데 구현체 Layer를 늘리는 형태로 구현하는 방법도 있었네요! 고생하셨습니다.
|
||
@Service | ||
@Transactional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른 클래스와 달리 이 클래스에서만 클래스 단위의 @transactional을 붙이신 특별한 이유가 있으실까요??
그리고 getPhotoResult는 조회 로직인 만큼 (readOnly = true) 옵션이 필요해 보입니다!
연관 이슈
작업 내용
uploadPhoto 테스트를 진행할 때마다 sendEmail 메서드가 트랜잭션에 포함되어 있어 병목이 발생합니다. @ExeTimer AOP로 실행 시간을 측정한 결과 약 4.5초가 소요됩니다. 이메일 전송을 할 수 없는 상황일 때 uploadPhoto가 아예 안되어버리는 엄청난 일이 발생할 수 있기 때문에 Facade 객체를 앞 단에 두어 트랜잭션에서 분리했습니다.
트랜잭션의 범위가 정확히 레포지토리로 한정되었고, 병목인 로직을 분리하여 한 쓰레드가 트랜잭션을 오래 붙잡지 않습니다! uploadPhoto 테스트도 1.2초 내로 완료됩니다.
작업 전
작업 후
참고
Real MySQL 5.1.2 트랜잭션 주의사항을 참고하였습니다.