[FEAT/#1658] 통합 서버 공통 응답 포맷 대응 - #1659
seungjunGong wants to merge 9 commits into
Conversation
앱/운영/인증 서버가 단일 도메인으로 통합됨에 따라 base URL 설정을 하나로 묶는다. - devBaseUrl / prodBaseUrl 프로퍼티 추가 → BuildConfig.DEV_BASE_URL / PROD_BASE_URL - NetModule의 Retrofit 4개 모두 통합 base URL 사용 - legacy 키(devApi, devOperationApi, devAuthApi 등)는 develop 브랜치 호환을 위해 유지, 전환 완료 후 제거 예정 - 인증/출석 서비스는 "/api/v1/..." 절대경로라 base의 /api/v2/ 를 무시하고 동작함 prodBaseUrl은 통합 prod 도메인 확정 후 교체 필요 (현재 legacy 앱 서버 placeholder)
Hyobeen-Park
left a comment
There was a problem hiding this comment.
우와!! 멋진 작업이 진행중이네요ㅎㅎ 코드 보다가 궁금한게 생겨서 질문 남겨놨어요! 수고하셨습니당🚀
There was a problem hiding this comment.
기존 서버별 Qualifier를 그대로 유지하신 이유가 무엇인지 궁금합니다!
서버 통합으로 모든 Retrofit의 baseUrl이 동일해져서 auth 관련 retrofit을 제외한 다른 retrofit들은 동일한 객체가 생성되고 있는 것 같아서요! client, interceptor 등에 별도의 차이가 없다면 하나로 통합하는 방향이 더 낫지 않을까 싶은 생각이 들어서 한 번 여쭤봅니다ㅎㅎ
There was a problem hiding this comment.
좋은 리뷰 감사합니다! BaseResponse 통일만 빠르게 작업하다 보니 Qualifier 쪽은 생각을 못 했네요..
해당 pr 에 통합해두겠습니다!
| @Serializable | ||
| data class BaseResponse<T>( | ||
| @SerialName("success") | ||
| val success: Boolean, | ||
| @SerialName("message") | ||
| val message: String = "", | ||
| @SerialName("data") | ||
| val data: T | ||
| ) |
There was a problem hiding this comment.
주석에 [data]가 비어 있을 수 있다고 되어있는데
- json에 data:null 자체가 명시적으로 오는 경우
- "data" 키 자체가 응답에서 아예 빠지는 경우 ( 그럴리는 없겠으나..)
2가지 경우가 있는데 명세를 확인해야겠으나.. 현재는 1번 케이스는 <?> 형태로 해결가능하나 2번 케이스는 막지 못하는데 우선은 이런게 있을 수도 있다 정도만 알고 추후 명세 나오고 적용해도 좋을 것 같아요
There was a problem hiding this comment.
의도는 1번(data: null)이었는데 제가 헷갈리게 작성했네요..
확인해보니 통합서버 dev에서 실제로 null 로 옵니다!
2번 키 누락은 지금 발생하지 않아서 말씀하신 대로 이 PR에서는 두고 필요하다면 Json에 explicitNulls = false 한 줄로 대응 가능할수도 있을거 같습니다. KDoc 표현은 고쳐둘게요!
| suspend fun createCode( | ||
| @Body request: CreateCodeRequest, | ||
| ): NullableBaseAuthResponse<Unit> | ||
| ): BaseResponse<Unit?> |
There was a problem hiding this comment.
이렇게 unit에 ?을 붙이는 방식이 그러면 궁금해지는데
백엔드에서 실제로 data:null이 오는지 아니면 data:{} 나 의미없는 값이 오는지 궁금하네요
만약 null이 아니라 빈 객체로 온다면 BaseResponse<Unit?>보다 BaseResponse(non-null)으로 선언하는 게 나을 것 같습니다
어차피 이 응답에서 data를 참조하는 곳이 없다면 굳이 nullable로 열어둘 이유가 없고 non-null로 유지해야 이후에 실수로 다른 곳에서 null 체크 분기가 생기는 걸 막을 수 있을 거라고 생각해요
반대로 백엔드가 정말 null을 내려준다면 해당 응답에 data가 원래 null로 온다는 것을 주석으로 남겨주시면 좋을 것 같습니다!
There was a problem hiding this comment.
확인해본바로는 null 을주는거 같은데 자세한건 명세가 나와야 알것 같아요
사실 해당 응답같은경우는 응답값 활용을 하지 않아서.. 아예 반환값을 없애는 방법도 있을거 같기는 합니다!
| interface PokeService { | ||
| @GET("poke/new") | ||
| suspend fun checkNewInPoke(): Response<CheckNewInPokeResult> | ||
| suspend fun checkNewInPoke(): Response<BaseResponse<CheckNewInPokeResult>> |
There was a problem hiding this comment.
여기 부분 Response로 BaseRespnse를 묶어서 사용해야할 이유가 있을까요? Retrofit 관련 데이터 상태를 확인해야하난요?
There was a problem hiding this comment.
이 부분은 일단 data만 BaseResponse로 갈아끼우다 보니 기존 구조를 그대로 가져오게 되었는데요!
확인해보니 실제로 Retrofit.Response로 http code를 받아서 ApiError로 분기하긴 하는데 UI에서는 ApiError랑 Failure 둘 다 같은 에러 토스트를 띄우고 있어서 동작 차이는 없네요.. 이 부분 Response를 제거하고 다른 모듈처럼 suspendRunCatching 기반으로 정리하는 방향으로 수정해보겠습니다!
There was a problem hiding this comment.
음 하다보니까 수정이 좀 많아졌는데 확인해주시면 감사하겠습니다!
- 성공 시 `data`가 비어 있는 경우 대신 `null`로 내려오는 경우로 설명을 구체화함.
* AuthRetrofit 및 OperationRetrofit 제거 후 AppRetrofit으로 단일화 - `Qualifiers.kt`에서 사용하지 않는 `@OperationRetrofit` 및 `@AuthRetrofit` 어노테이션 정의를 제거합니다. - `NetModule` 내에서 중복되던 `provideOperationRetrofit` 및 `provideAuthRetrofit` 생성 로직을 삭제합니다. - `ApiModule`, `AttendanceBindsModule`, `AuthModule` 등 의존성 주입부에서 기존 한정자 대신 `@AppRetrofit` 및 인증이 불필요한 경우 `@AppRetrofit(false)`를 사용하도록 수정합니다.
* Poke 도메인 내의 `ApiResult` 및 `BaseResponse` 관련 클래스를 제거하고 Kotlin 표준 `Result`를 사용하도록 변경합니다. * `PokeRepository` 및 UseCase의 반환 타입을 `Result<T>`로 변경하여 통일된 에러 처리를 지원합니다. * `UiState`에서 `ApiError` 상태를 제거하고 `Failure`로 통합하여 UI 대응 로직을 간소화합니다. * `PokeRemoteDataSource` 및 `PokeService`에서 Retrofit `Response` 래퍼를 제거하고 데이터를 직접 반환하도록 수정합니다. * `PokeMainViewModel`에 `PokeFriendEmptyException`을 추가하여 친구 목록이 비어 있는 특정 상황을 `Result.failure`로 처리합니다. * `NewHomeViewModel` 및 `SoptLogViewModel` 등에서 `ApiResult` 분기 로직을 `Result.map`을 사용한 간결한 로직으로 리팩터링합니다.
- `UiState` 제네릭의 상한 제한(`: Any`)을 제거하여 Nullable 데이터 타입을 지원하도록 수정합니다. - `PokeFriendEmptyException`을 제거하고, 친구 데이터가 없는 경우 `UiState.Failure` 대신 `UiState.Success(null)`을 반환하도록 변경합니다. - `PokeMainActivity` 및 `PokeScreen`에서 `UiState.Success` 데이터의 null 여부에 따라 뷰 가시성을 제어하도록 로직을 수정합니다. - `PokeMainViewModel`에서 친구 목록이 비어있을 때의 상태 발행 로직을 간소화합니다.
Related issue 🛠
Work Description ✏️
Screenshot 📸
N/A
Uncompleted Tasks 😅
To Reviewers 📢
BaseResponse를 core:network가 아니라 core:common에 둔 이유는 모든 모듈이 이미 common을 물고 있고 @AppRetrofit도 common에 있어서 gradle 수정 없이적용했습니다.
dev에서 플레이그라운드 웹뷰 SSO는 웹 쪽이 아직 옛 인증 서버를 봐서 안 됩니다. 이 PR 이슈 아님
브랜치 병합 전략은 해당 링크 참고해주세요