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

[Refactor] ViewModel에서 Flow를 사용할 수 있도록 수정 #343

Merged
merged 23 commits into from
Mar 30, 2024

Conversation

dongx0915
Copy link
Member

@dongx0915 dongx0915 commented Mar 27, 2024

📌 개요

✨ 작업 내용

  1. CallAdapter 추가
  2. BaseReseponse 제거 + ResponseInterceptor 추가
  3. BaseViewModel + exceptionHandler 추가
  4. ViewModel에서 flow 사용 가능하도록 변경

RetrofitV2를 만들어서 마라톤 코스 요청 메소드에만 예시로 적용 해두었습니다.
(CourseV2Service, DiscoverViewModel 참고)

✨ PR 포인트

  1. CallAdapter 추가

    • Retrofit에서 kotlin.Result 타입으로 응답을 반환합니다.
      • 최대한 기존 코드와 유사하도록 kotlin.Result를 이용하도록 작업 하였습니다.
    • 통신은 성공 했으나 data만 null인 경우 Failure로 반환하도록 처리하였습니다. (ViewModel에서 체크 불필요)
  2. BaseReseponse 제거 + ResponseInterceptor 추가

    • 서버에서 내려주는 형식 때문에 불필요하게 BaseResponse로 래핑이 필요하였습니다.
    • ResponseInterceptor로 data 속성만 추출하도록 수정하여 BaseResponse를 사용하지 않아도 됩니다.
    • 추가로 에러 응답은 러넥트 서버 에러 + Http 기본 에러 형식을 모두 받을 수 있도록 ErrorResponse를 추가 하였습니다.
      • ResultCall > parseErrorResponse 메소드 참고
  3. BaseViewModel + exceptionHandler 추가

    • exceptionHandler를 추가하였습니다.
    • launchWithHandler 내에서 발생하는 예외는 모두 exceptionHandler에서 처리 됩니다.
      • exceptionHandler가 아닌 ViewModel에서 처리하고 싶은 경우 flow의 catch 연산자를 사용하면 됩니다.

📸 스크린샷/동영상

* 서버에서 BaseResponse 형태로 수신되고 있어서 불필요한 래핑 발생
* data 외의 값은 쓰지 않고 있어서 Interceptor를 통해 data만 추출
* 에러가 발생했을 때 러넥트 서버 에러 + 기본 에러를 같이 받을 수 있도록 처리
@dongx0915 dongx0915 added REFACTOR 🧹 코드 리팩토링 동현 🐨 동현 담당 labels Mar 27, 2024
@dongx0915 dongx0915 self-assigned this Mar 27, 2024
@unam98 unam98 requested review from leeeha, unam98 and sxunea March 27, 2024 12:30
Copy link
Contributor

@unam98 unam98 left a comment

Choose a reason for hiding this comment

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

it's good~ 수고하셨습니다!

@Serializable
data class ErrorResponse<T>(
@SerialName("status")
val status: Int,
Copy link
Contributor

Choose a reason for hiding this comment

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

여기만 nullable하지 않은 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

@unam98 status가 null이 오는 경우는 없다고 생각하여 Non-Null로 지정하였습니다.

서버 에러, Http 에러 발생시에도 status 코드는 모두 포함 되어있고, status 코드도 안내려오는 상황이라면 Exception일 것이라고 생각하였습니다.

Copy link
Member

@leeeha leeeha left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!! 새로운 거 많이 배워갑니다 최고 👍

RunnectException(
code = response.code(),
message = ERROR_MSG_RESPONSE_IS_NULL
)
Copy link
Member

Choose a reason for hiding this comment

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

뷰모델에서 매번 반복되었던 로직을 여기서 처리해주니까 넘 좋네요! 👍

code = errorBody.status,
message = message
)
}.getOrElse {
Copy link
Member

@leeeha leeeha Mar 29, 2024

Choose a reason for hiding this comment

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

Result에서 제공하는 다양한 확장함수들 덕분에 에러 핸들링 코드가 간결해진 거 같아요!
이 글 참고해서 저도 더 유연하게 사용해봐야겠어요 :)

val originalResponse = chain.proceed(chain.request())
if (!originalResponse.isSuccessful) return originalResponse

val bodyString = originalResponse.peekBody(Long.MAX_VALUE).string()
Copy link
Member

Choose a reason for hiding this comment

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

Long.MAX_VALUE 값을 인자로 넘겨주는 이유가 무엇인가요? 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

@leeeha 기존 응답에서 얼만큼을 읽어올 지 사이즈를 정해주는 값입니다!
저희는 모든 응답을 그대로 가져올 것이기 떄문에 Long.MAX_VALUE를 전달해주었습니다

override suspend fun getMarathonCourse(): Flow<Result<List<MarathonCourse>>> {
return remoteCourseDataSource.getMarathonCourse().mapToFlowResult {
it.toMarathonCourses()
}
Copy link
Member

Choose a reason for hiding this comment

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

오호 이렇게 사용하면 되는군요!

.baseUrl(baseUrl)
.client(client)
.addConverterFactory(GsonConverterFactory.create())
.addCallAdapterFactory(ResultCallAdapterFactory.create())
Copy link
Member

Choose a reason for hiding this comment

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

오호 여기서 주입해주는 거군요!

): OkHttpClient = OkHttpClient.Builder()
.addInterceptor(logger)
.addInterceptor(appInterceptor)
.addInterceptor(responseInterceptor)
Copy link
Member

Choose a reason for hiding this comment

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

ResponseInterceptor 적용하는 건 처음 봅니다! 인터셉터 활용을 이렇게나 다양하게 할 수 있다니 👍

import okhttp3.MultipartBody
import okhttp3.RequestBody
import retrofit2.Response

interface CourseRepository {
suspend fun getMarathonCourse(): Result<List<MarathonCourse>?>
suspend fun getMarathonCourse(): Flow<Result<List<MarathonCourse>>>
Copy link
Member

Choose a reason for hiding this comment

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

드디어 여기서 ? 가 없어졌다 ㅎㅎㅎ 😆

}
else -> {
Timber.e(throwable)
}
Copy link
Member

Choose a reason for hiding this comment

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

Timber.e(throwable) 를 똑같이 출력하는데 굳이 when문으로 구분하는 이유가 있을까요..?!

Copy link
Member Author

Choose a reason for hiding this comment

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

@leeeha 추후 BaseViewModel에 공통 StateFlow or SharedFlow를 선언해두고, 예외 타입에 따라 UiState를 방출 시킬 예정입니다.

아직 정확한 방향이 안정해져서 임시로 구분만 해놨습니다~!

block: suspend CoroutineScope.() -> Unit
) {
viewModelScope.launch(dispatcher + exceptionHandler, start, block)
}
Copy link
Member

Choose a reason for hiding this comment

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

우와 이제까지 launch로 코루틴 블록 만드는 정도밖에 안 해봤는데.. 이렇게 커스텀 할 수 있다는 게 넘 신기하네요!
코루틴에 대해서도 자세히 알아봐야겠어요 🙏

result.onSuccess {
_marathonCourseGetState.value = UiStateV2.Success(it)
}.onFailure {
_marathonCourseGetState.value = UiStateV2.Failure(it.toLog())
}
Copy link
Member

Choose a reason for hiding this comment

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

크으... 깔끔하다!!! flow에서 제공하는 다양한 연산자들 알아봐야겠어요! 혹시 자료 추천해주실 수 있을까요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

@leeeha 답변 남기는걸 까먹었었네요..ㅎ
flow 연산자에 대한 내용은 아니지만 아래 블로그 글 추천합니다.

flow가 어떻게 동작하는지 잘 보여주는 글인 것 같아요

@dongx0915 dongx0915 merged commit 6a5991d into develop Mar 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REFACTOR 🧹 코드 리팩토링 동현 🐨 동현 담당
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactor] ViewModel에서 Flow를 사용할 수 있도록 수정
3 participants