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.

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

[Feat] #197 - 토큰 재발급 정상 작동하도록 수정 / 플그 미등록 비활동 유저 자동로그인 처리 / 스플래시 이미지 변경 #200

Merged
merged 4 commits into from
Apr 22, 2023

Conversation

L-j-h-c
Copy link
Contributor

@L-j-h-c L-j-h-c commented Apr 21, 2023

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

  • 토큰 재발급 정상 작동하도록 수정
  • 플그 미등록 비활동 유저 자동로그인 처리
  • 스플래시 이미지 변경

📌 참고 사항

토큰 재발급 로직

Interceptor가 정상적으로 작동하지 않고 있었네요. Alamofire로 refresh logic을 구현할 때 인지하고 있던 이슈인데 Moya로 넘어오면서 적용하지 못했네요..!

Known Issue

  1. Alamofire의 Interceptor에서 클래스 네임이 다른 라이브러리의 이름과 겹치는 경우 잘 작동하지 않는 이슈가 있었습니다. 코드를 보시면 Swift.Error와 같이 모듈의 이름을 명시하여 해결이 가능합니다.
  2. Almofire에서는 validate()를, Moya에서는 validationType을 설정해줘야만 interceptor가 작동합니다.

플그 미등록 비활동 유저 자동로그인 처리 & 메인뷰 알러트 처리 (cc. @lsj8706 )

  1. 해당 케이스도 로그인을 수행하기는 했기 때문에 액세스 토큰에 빈 스트링을 넣어서 자동로그인이 수행되도록 했습니다.
  2. �메인 뷰에서 토큰 재발급 로직이 실패한 경우 네트워크 알러트를 띄우지 않도록 했습니다. 이 경우 로그인 뷰로 이동하는 것이 필요합니다.
  3. 현재 witherror를 사용하는 방식이 확장성 및 응집도에 한계가 있어 보여서, Error를 재정의하고 해당 Error에 따라 action을 분기하는 것이 필요해 보입니다.

📮 관련 이슈

@L-j-h-c L-j-h-c requested a review from a team April 21, 2023 12:24
@L-j-h-c L-j-h-c self-assigned this Apr 21, 2023
@L-j-h-c L-j-h-c requested review from elesahich, 0inn, lsj8706 and devxsby and removed request for a team April 21, 2023 12:24
Copy link
Contributor

@0inn 0inn left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ~ !

Comment on lines +41 to +43
// NOTE: (@준호) 플그 미등록 + 비활동 유저의 경우 임시로 accessToken 빈 스트링 부여
// 자동로그인 시 활용하기 위함
UserDefaultKeyList.Auth.appAccessToken = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻이제 잘 동작하겠군요

Copy link
Member

@devxsby devxsby left a comment

Choose a reason for hiding this comment

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

감사합니다!!
다음 챕터회의때 에러처리 같이 논의해봐용 🐯

Comment on lines +31 to +40
.catch({ error in
var model: UserMainInfoModel?
if let error = error as? APIError,
case .tokenReissuanceFailed = error {
model = UserMainInfoModel(withError: false)
} else {
model = UserMainInfoModel(withError: true)
}
return Just(model)
})
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

Copy link
Member

@lsj8706 lsj8706 left a comment

Choose a reason for hiding this comment

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

감사합니다!!
적어주신 내용 확인했어요!

userService.getUserMainInfo()
.map { $0.toDomain() }
.catch({ error in
Copy link
Member

Choose a reason for hiding this comment

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

withError로 임시 처리를 했지만 결국 Error 타입을 제대로 정의해서 핸들링 하는 것이 맞다고 생각해요!
레포지토리에서 APIError 열거형을 사용하여 네트워크 에러가 발생한경우를 잡아내고 플그 미등록 유저인 경우도 이 열거형을 통해 구분하는 것이 맞을까요? 아니면 APIError가 아닌 새로운 열거형을 생성해서 에러 처리를 하는 것이 좋을까요?

@@ -11,6 +11,6 @@ import Core
import Combine

public protocol MainRepositoryInterface {
func getUserMainInfo() -> AnyPublisher<UserMainInfoModel?, Error>
func getUserMainInfo() -> AnyPublisher<UserMainInfoModel?, Never>
Copy link
Member

Choose a reason for hiding this comment

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

에러 핸들링을 하게 된다면 여기에 Never로 적힌 것도 새로 정의한 Error로 바꾸는게 맞을까요?

@L-j-h-c L-j-h-c merged commit 810e179 into sopt-makers:develop Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Chore] 플그 미등록 + 비활동 유저 토큰 관리, 리프레시 로직 수정, 크루팀 URL 변경
4 participants