-
Notifications
You must be signed in to change notification settings - Fork 3
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: OAuth API 호출을 트랜잭션 범위에서 분리 #471
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
1380942
refactor: AuthServiceFacade 적용
iamjooon2 a0781f1
test: AuthServiceFacade를 모킹으로 분리
iamjooon2 6c0ca3c
test: 외부 인프라 응답 생성 메서드명 통일
iamjooon2 f669c8f
refactor: 서드파티 성공 응답 fixture 생성
iamjooon2 c055b7e
chore: resolve conflict
iamjooon2 7f63f20
refactor: AuthControllerTest로 네이밍 변경
iamjooon2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
29 changes: 29 additions & 0 deletions
29
backend/src/main/java/zipgo/auth/application/AuthServiceFacade.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package zipgo.auth.application; | ||
|
||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
import zipgo.auth.application.dto.OAuthMemberResponse; | ||
import zipgo.auth.dto.TokenDto; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class AuthServiceFacade { | ||
|
||
private final AuthService authService; | ||
private final OAuthClient oAuthClient; | ||
|
||
public TokenDto login(String authCode) { | ||
String accessToken = oAuthClient.getAccessToken(authCode); | ||
OAuthMemberResponse oAuthMemberResponse = oAuthClient.getMember(accessToken); | ||
return authService.login(oAuthMemberResponse); | ||
} | ||
|
||
public String renewAccessTokenBy(String refreshToken) { | ||
return authService.renewAccessTokenBy(refreshToken); | ||
} | ||
|
||
public void logout(Long memberId) { | ||
authService.logout(memberId); | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
121 changes: 121 additions & 0 deletions
121
backend/src/test/java/zipgo/auth/application/AuthServiceFacadeTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
package zipgo.auth.application; | ||
|
||
import org.junit.jupiter.api.DisplayNameGeneration; | ||
import org.junit.jupiter.api.DisplayNameGenerator; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.ExtendWith; | ||
import org.mockito.InjectMocks; | ||
import org.mockito.Mock; | ||
import org.mockito.junit.jupiter.MockitoExtension; | ||
import zipgo.auth.application.dto.OAuthMemberResponse; | ||
import zipgo.auth.application.fixture.MemberResponseSuccessFixture; | ||
import zipgo.auth.dto.TokenDto; | ||
import zipgo.auth.exception.OAuthResourceNotBringException; | ||
import zipgo.auth.exception.OAuthTokenNotBringException; | ||
import zipgo.auth.exception.TokenExpiredException; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
import static org.junit.jupiter.api.Assertions.assertAll; | ||
import static org.mockito.Mockito.times; | ||
import static org.mockito.Mockito.verify; | ||
import static org.mockito.Mockito.when; | ||
|
||
@ExtendWith(MockitoExtension.class) | ||
@SuppressWarnings("NonAsciiCharacters") | ||
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) | ||
class AuthServiceFacadeTest { | ||
|
||
@Mock | ||
private OAuthClient oAuthClient; | ||
|
||
@Mock | ||
private AuthService authService; | ||
|
||
@InjectMocks | ||
private AuthServiceFacade authServiceFacade; | ||
|
||
@Test | ||
void 로그인에_성공하면_토큰을_발급한다() { | ||
// given | ||
when(oAuthClient.getAccessToken("인가 코드")) | ||
.thenReturn("엑세스 토큰"); | ||
OAuthMemberResponse 서드파티_사용자_응답 = new MemberResponseSuccessFixture(); | ||
when(oAuthClient.getMember("엑세스 토큰")) | ||
.thenReturn(서드파티_사용자_응답); | ||
when(authService.login(서드파티_사용자_응답)) | ||
.thenReturn(TokenDto.of("생성된 엑세스 토큰", "생성된 리프레시 토큰")); | ||
|
||
// when | ||
TokenDto 토큰 = authServiceFacade.login("인가 코드"); | ||
|
||
// then | ||
assertAll( | ||
() -> assertThat(토큰.accessToken()).isEqualTo("생성된 엑세스 토큰"), | ||
() -> assertThat(토큰.refreshToken()).isEqualTo("생성된 리프레시 토큰") | ||
); | ||
} | ||
|
||
@Test | ||
void 엑세스_토큰을_가져오지_못하면_예외가_발생한다() { | ||
// given | ||
when(oAuthClient.getAccessToken("인가 코드")) | ||
.thenThrow(new OAuthTokenNotBringException()); | ||
|
||
// expect | ||
assertThatThrownBy(() -> authServiceFacade.login("인가 코드")) | ||
.isInstanceOf(OAuthTokenNotBringException.class) | ||
.hasMessageContaining("서드파티 서비스에서 토큰을 받아오지 못했습니다. 잠시후 다시 시도해주세요."); | ||
} | ||
|
||
@Test | ||
void 사용자_정보를_가져오지_못하면_예외가_발생한다() { | ||
// given | ||
when(oAuthClient.getAccessToken("인가 코드")) | ||
.thenReturn("엑세스 토큰"); | ||
when(oAuthClient.getMember("엑세스 토큰")) | ||
.thenThrow(new OAuthResourceNotBringException()); | ||
|
||
// expect | ||
assertThatThrownBy(() -> authServiceFacade.login("인가 코드")) | ||
.isInstanceOf(OAuthResourceNotBringException.class) | ||
.hasMessageContaining("서드파티 서비스에서 정보를 받아오지 못했습니다. 잠시후 다시 시도해주세요"); | ||
} | ||
|
||
|
||
@Test | ||
void 엑세스_토큰을_갱신할_수_있다() { | ||
// given | ||
when(authService.renewAccessTokenBy("리프레시 토큰")) | ||
.thenReturn("갱신된 엑세스 토큰"); | ||
// when | ||
String 리프레시_토큰 = authServiceFacade.renewAccessTokenBy("리프레시 토큰"); | ||
|
||
// then | ||
assertThat(리프레시_토큰).isNotEmpty(); | ||
} | ||
|
||
@Test | ||
void 만료된_엑세스_토큰은_갱신시_예외가_발생한다() { | ||
// given | ||
when(authService.renewAccessTokenBy("리프레시 토큰")) | ||
.thenThrow(new TokenExpiredException()); | ||
// expect | ||
assertThatThrownBy(() -> authServiceFacade.renewAccessTokenBy("리프레시 토큰")) | ||
.isInstanceOf(TokenExpiredException.class) | ||
.hasMessageContaining("만료된 토큰입니다. 올바른 토큰으로 다시 시도해주세요."); | ||
} | ||
|
||
@Test | ||
void 로그아웃_할_수_있다() { | ||
// given | ||
Long memberId = 123L; | ||
|
||
// when | ||
authServiceFacade.logout(memberId); | ||
|
||
// then | ||
verify(authService, times(1)).logout(memberId); | ||
} | ||
|
||
} |
105 changes: 0 additions & 105 deletions
105
backend/src/test/java/zipgo/auth/application/AuthServiceMockTest.java
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
파마산 패턴 🧀 맛있는 방법이네요 👍🏻👍🏻👍🏻
보면서 단순한 궁금증이 생겼습니다.
이걸 쓰면 전체 메서드를 다 복사해줘야되나여?
oauthclient를 쓰는 건 login쪽만인디 나머지도 다 단순 복사해줘야 되서 메서드가 엄청 많아지면 귀찮아 보이기도 하네여 테스트도 추가해줘야되고
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.
컨트롤러에서 사용하는 서비스의 인터페이스가 서비스파마산이니 복사해주긴 해야합니다
어찌보면 HTTP 요청이라고 생각하고 컨트롤러에서 OAuthClient 의존하는게 더 단순했을 것 같긴 합니다만
일종의 트레이드오프라고 생각합니다
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.
오... 퍼사드 멋있네요.. 단순 의견 남기자면 @parkmuhyeun 퍼사드 의도 자체가 복잡한 메서드를 간단하게 만들어주는 용도로 사용해서 사용하는 입장에서는 더 가독성이 좋을듯요(테스트 코드랑 이것저것 추가해야 한다고 해도) 물론 복잡하지 않은데에 퍼사드 쓰는건 오남용이라고 생각합니다!
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.
아하 그렇게도 생각할 수 있긴하네요! 단순 트랜잭션을 분리하기 위해서는 더 좋은 방법이 있을 수도 있지만 그래도 가비가 하나의 방법으로 이런 디자인 패턴을 찾아보고 도입한건 고맙게 생각합니다.. 🙇♂️🙇♂️