[refac] 로그인 로직 리팩토링 #252
[refac] 로그인 로직 리팩토링 #252
Conversation
| package org.hankki.hankkiserver.external.openfeign.oauth; | ||
|
|
||
| public interface OAuthProvider { | ||
|
|
||
| SocialInfoDto getUserInfo(String token, String name); | ||
|
|
||
| void requestRevoke(String code, String Id); | ||
| } |
There was a problem hiding this comment.
인터페이스 위치는 api의 서비스가 적절하지 않을까요?
There was a problem hiding this comment.
구현체인 Kakao/AppleOAuthProvider 클래스가 external 패키지에 위치하기 때문에 같은 패키지에 위치시키는 것이 맞다고 생각했습니다. 실제로 해당 인터페이스는 외부 서버와 통신할 때 사용하는 인터페이스이기 때문에 api보다는 external에 위치시키는것이 더 나을것 같다고 생각했어요
There was a problem hiding this comment.
저희 앞으로 구조를 바꿀텐데, 서비스에서의 의존성들을 최소화하고 구현체들을 갈아끼우는 형태로 바꾸려면 service에서 interface를 정의하는게 맞지 않을까요?
There was a problem hiding this comment.
잘 이해를 못했는데, 현재 서비스에서도 직접적으로 구현체에 의존하고 있지 않고 OAuthProvider라는 인터페이스에만 의존하고 있기 때문에 괜찮지 않나요? 무엇보다 저는 구현체인 클래스들과 그 상위 인터페이스를 다른 페키지에 위치시키는 것이 오히려 응집도가 떨어진다고 생각해요. 인터페이스와 구현체가 같은 맥락에서 관리되어야 유지보수와 가독성이 더 높아질 수 있다고 생각합니다.
There was a problem hiding this comment.
지금구조에서는 service 패키지가 다른 패키지들에 많은 의존성을 지니지만, 앞으로 service의 의존성들을 없애는 방향으로 개발을 한다면, 외부와 통신하는 인터페이스를 service에 두어 external이 반대로 service에 의존하게 한다는 의미에서 말씀드린겁니다.
물론 말씀하신 것처럼 지금도 구현체에 의존하는 것은 아니지만 어쨌든 service에서 external에 대한 의존성이 생기는 것은 맞으니까용
|
|
||
| private final KakaoOAuthProvider kakaoOAuthProvider; | ||
| private final AppleOAuthProvider appleOAuthProvider; | ||
|
|
There was a problem hiding this comment.
그냥 List나 map으로 oauthProvider로 바로 주입 받는 방식이 ocp에는 더 좋을 거 같아요
이렇게 하면 결국 새로운 구현체가 추가되었을 때 코드를 수정해야하니까요
| public OAuthProvider findProvider(final Platform platform) { | ||
| if (KAKAO == platform) { | ||
| return kakaoOAuthProvider; | ||
| } | ||
| return appleOAuthProvider; | ||
| } |
There was a problem hiding this comment.
위에 남겨주신 리뷰까지 모두 반영하여 수정했습니당
| @Service | ||
| @Transactional |
There was a problem hiding this comment.
아직 외부 통신 트랜잭션 분리는 안 하신거죠?
There was a problem hiding this comment.
넹 이건 따로 이슈 파서 진행할 예정입니다~
Related Issue 📌
close #247
Description ✔️
To Reviewers
변경 사항이 많은건 패키지를 바꿔서 그렇습니다.
중점적으로는 OAuthProvider 인터페이스 만든거랑 각 구현체인 Kakao/AppleOAuthProvider 클래스, AuthService, OAuthProviderFactory만 보면 될 것 같습니다.
고민이 있는데,
OAuthProvider라는 인터페이스를 만들었지만 각 구현체 별로 필요로 하는 파라미터 개수나 타입이 달라서 어떻게 해야할지 고민입니다.대표적인 예가 AuthService의 withdraw 로직에 주석으로 적혀있듯이
애플 로그인은 code만 필요하고 카카오 로그인은 후자만 필요한데, 인터페이스로 처리하려다 보니 불필요하게 파라미터를 둘 다 받아야하는 문제점이 생겨버림..
그래서 각 OAuth마다 필요로 하는 응답을 또 만들어주는 클래스를 만들까 고민도 했지만 그렇게 되면 응답으로 넘기는 데이터를 클래스로 감싸야하고 또 그 위에 인터페이스도 둬야하고.... 너무 과분리 같기도 하고 고민입니둥