-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] Home/HomeDetail Ktorfit 전환 및 Retrofit 의존 제거 #216 #220
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRetrofit에서 Ktorfit으로 전환했습니다. 빌드 설정에 Ktor/Ktorfit 의존성 추가 및 버전 업데이트, Elders API 인터페이스의 어노테이션/Response 타입 교체, Ktorfit 기반 DI 모듈 추가, 리포지토리의 네트워크 응답 처리 흐름을 response.isSuccessful/response.body() 검사 기반으로 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor UI
participant Repo as Repository
participant Service as KtorfitService
participant Remote as RemoteAPI
UI->>Repo: (예: getDailySleep 요청)
Repo->>Service: Service.getDailySleep(elderId, date)
Service->>Remote: HTTP 요청 전송
Remote-->>Service: HTTP 응답 (status + body)
Service-->>Repo: Response<T>
Repo->>Repo: response.isSuccessful? -> body() 검증
alt 성공 & 유효한 body
Repo-->>UI: 도메인/UiState 반환
else 404
Repo-->>UI: EMPTY 상태 반환
else 실패
Repo-->>UI: error(응답 코드) throw
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/konkuk/medicarecall/data/api/elders/MedicineService.kt:
- Around line 4-13: MedicineService currently returns MedicineResponseDto
directly but other repositories expect HTTP errors to be represented via
Response<T> and handled with .isSuccessful; verify Ktorfit is configured with an
HttpResponseValidator or a ResponseConverter that throws on non-2xx responses
and, if not, either revert the MedicineService signature to
Response<MedicineResponseDto> or add the validator so
medicineService.getDailyMedication(...) will actually throw HTTP errors for
runCatching to work; also locate the duplicate providers in ApiModule and
KtorApiModule and ensure only the intended MedicineService provider is
bound/used at runtime.
In @app/src/main/java/com/konkuk/medicarecall/data/di/KtorApiModule.kt:
- Around line 17-38: The module is requesting a Ktorfit instance but none is
provided and duplicates of the same Retrofit-provided services exist; add a
single provider for Ktorfit (e.g., a @Single function that builds Ktorfit via
Ktorfit.Builder and any required HttpClient/config) so homeService,
glucoseService, mealService, medicineService, mentalService, and sleepService
can receive it, and resolve the binding conflict with ApiModule by either
removing the duplicate Retrofit-based service providers or using distinct
qualifiers for the Retrofit vs Ktorfit variants (or consolidating all service
creation to one module such as NetworkModule) to avoid Koin binding collisions.
🧹 Nitpick comments (3)
app/build.gradle.kts (1)
54-56: jvmTarget 설정이 deprecated되었습니다.파이프라인에서
jvmTarget: String is deprecated경고가 발생하고 있습니다.compilerOptionsDSL로 마이그레이션하는 것을 권장합니다.♻️ compilerOptions DSL로 마이그레이션
-kotlinOptions { - jvmTarget = "11" -} +kotlin { + compilerOptions { + jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_11) + } +}app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt (1)
22-35: 에러 처리 일관성 체크해보면 좋을 것 같아요
getHomeSummary는 DTO를 직접 반환하는 반면,requestImmediateCareCall은Result<Unit>으로 감싸고 있어 에러 처리 방식이 다릅니다.리포지토리 인터페이스 설계에 따른 의도된 차이일 수 있지만, 혹시
getHomeSummary도Result<HomeResponseDto>를 반환하도록 통일하는 것을 고려해보면 호출부에서 에러를 일관되게 처리할 수 있을 것 같습니다.app/src/main/java/com/konkuk/medicarecall/data/api/elders/HomeService.kt (1)
17-19: 반환 타입 Unit은 생략 가능합니다.Kotlin에서
Unit반환 타입은 생략할 수 있습니다. 명시적으로 선언하는 것도 문제는 없지만, 코틀린 관례상 생략하는 것이 더 간결합니다.♻️ 간소화 제안
@POST("care-call/immediate") suspend fun requestImmediateCareCall( @Body request: ImmediateCallRequestDto, - ) + ): Unit또는 완전히 생략:
@POST("care-call/immediate") - suspend fun requestImmediateCareCall( - @Body request: ImmediateCallRequestDto, - ) + suspend fun requestImmediateCareCall(@Body request: ImmediateCallRequestDto)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
app/build.gradle.ktsapp/src/main/java/com/konkuk/medicarecall/data/api/elders/GlucoseService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/HomeService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/MealService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/MedicineService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/MentalService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/SleepService.ktapp/src/main/java/com/konkuk/medicarecall/data/di/KtorApiModule.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HealthRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MedicineRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SleepRepositoryImpl.ktgradle/libs.versions.toml
🧰 Additional context used
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HealthRepositoryImpl.kt
[error] 20-20: Detekt: Unexpected indentation (20) (should be 16) [Indentation].
app/build.gradle.kts
[warning] 55-55: jvmTarget: String is deprecated. Please migrate to the compilerOptions DSL.
🔇 Additional comments (18)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HealthRepositoryImpl.kt (1)
19-20: 로직 변경 확인 필요
isRecorded계산 로직이 변경됐어요. 기존에는symptomList만 체크했는데, 이제analysisComment도 함께 확인하고 있네요.기존:
symptomList가 비어있지 않으면 true
변경:symptomListORanalysisComment중 하나라도 데이터가 있으면 true이제 증상이 없어도 분석 코멘트만 있으면 기록된 것으로 처리되는데, 이게 의도된 변경인지 한번 확인해주세요.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/MealService.kt (1)
4-6: Retrofit에서 Ktorfit으로 깔끔하게 마이그레이션되었습니다.어노테이션 import만 변경되고 메서드 시그니처는 그대로 유지되어 안전한 전환입니다.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/MentalService.kt (1)
4-6: Ktorfit으로 마이그레이션 완료.다른 서비스들과 동일한 패턴으로 일관성 있게 전환되었습니다.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.kt (1)
18-42: 에러 핸들링이 함수형 패턴으로 개선되었습니다.
runCatching/fold패턴으로 전환하여 더 간결하고 명확해졌고,orEmpty()로 null 안전성도 강화되었습니다. 실패 시 EMPTY 상태로 폴백하는 전략도 적절합니다.gradle/libs.versions.toml (3)
70-74: Ktor 클라이언트 의존성이 적절하게 추가되었습니다.core, okhttp, content-negotiation, kotlinx-json 등 필수 모듈들이 포함되어 있고 모두 동일한 버전을 참조하여 일관성이 유지됩니다.
77-78: Ktorfit 의존성 설정이 올바릅니다.런타임용
ktorfit-lib와 코드 생성용ktorfit-ksp가 모두 포함되어 있습니다.
3-3: 호환성 걱정 안 해도 괜찮습니다.KSP 2.3.2는 KSP2 라인의 핫픽스 릴리스로, 특정 Kotlin 컴파일러 버전에 묶여 있지 않습니다. 오히려 Kotlin 버전 의존성에서 벗어나도록 설계되었기 때문에 Kotlin 2.2.20과 호환됩니다.
Likely an incorrect or invalid review comment.
app/build.gradle.kts (3)
8-8: Serialization 플러그인 버전이 Kotlin 버전과 일치합니다.2.2.20으로 통일되어 호환성 문제가 없습니다.
106-114: Ktor와 Ktorfit 의존성이 올바르게 추가되었습니다.필요한 모듈들이 모두 포함되어 있고 KSP 프로세서도 설정되었습니다.
99-104: Retrofit 의존성은 여전히 필수적으로 사용 중입니다.빌드 파일에 남아있는 Retrofit 의존성들을 확인한 결과, API 서비스 정의, 에러 처리, 네트워크 설정 등 전체 데이터 계층에서 광범위하게 사용 중입니다:
- API 서비스 인터페이스들 (AuthService, RefreshService, EldersInfoService 등)이
retrofit2.Response<T>반환 타입 사용- 저장소 구현체들에서
retrofit2.HttpException활용한 에러 처리- 뷰 모델들에서 HttpException catch 처리
- NetworkModule, ApiModule에서 Retrofit 인스턴스 구성
현재 상태로는 Retrofit를 제거할 수 없습니다. PR 목표가 Retrofit 제거라면, 먼저 모든 API 통신 계층을 다른 라이브러리로 마이그레이션해야 합니다.
Likely an incorrect or invalid review comment.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/SleepService.kt (1)
4-13: Ktorfit 마이그레이션 완료, 문제없어 보입니다!Retrofit에서 Ktorfit으로 어노테이션만 깔끔하게 교체되었고, 메서드 시그니처는 그대로 유지되어 안정적인 전환입니다.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt (1)
14-20: 에러 처리 간소화, 깔끔합니다!
runCatching으로 감싸서Result<Unit>을 반환하는 방식으로 전환되어, Retrofit의isSuccessful체크와HttpException처리 로직이 제거되었습니다. Ktorfit의 예외 기반 에러 처리와 잘 맞는 구조입니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.kt (1)
12-23: 리팩토링 잘 되었습니다!
HttpException처리 로직을 제거하고runCatching으로 깔끔하게 전환했네요. 특히 named parameter를 사용해서 가독성도 좋아졌습니다.Result<GlucoseResponseDto>반환 타입도 에러 처리 패턴과 일관성 있게 유지되고 있습니다.app/src/main/java/com/konkuk/medicarecall/data/api/elders/GlucoseService.kt (1)
4-15: Ktorfit 전환 완료!Retrofit 어노테이션에서 Ktorfit 어노테이션으로 교체되고,
Response<GlucoseResponseDto>래퍼가 제거되어 DTO를 직접 반환하도록 변경되었습니다. 다른 서비스 파일들과 일관된 패턴으로 마이그레이션되었네요.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SleepRepositoryImpl.kt (2)
35-72: 에러 처리 리팩토링이 깔끔하게 적용되었습니다.
runCatching().fold()패턴으로 전환하면서 코드가 더 함수형이고 읽기 쉬워졌습니다. isValid 검증 로직을 통해 null-safe 처리도 강화되었네요.
52-70: date 필드 처리는 일관되고 적절합니다성공 시
response.date를 사용하고, 실패 시date.toString()을 사용하는 것은 일관된 로직입니다. API 성공 시 서버의 날짜를 우선하고, 실패 시 요청한 날짜로 폴백하는 합리적인 패턴입니다. 두 경우 모두 String 타입으로 일관되게 처리되므로 문제가 없습니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MedicineRepositoryImpl.kt (1)
69-118: Ktorfit 전환에 맞춰 데이터 매핑 로직이 잘 개선되었습니다.
runCatching().fold()패턴을 적용하고, DTO를 직접 처리하도록 수정되었습니다. 정렬 로직(correctOrder.indexOf())과 시간대별 매핑 로직도 명확하게 구현되어 있습니다.app/src/main/java/com/konkuk/medicarecall/data/di/KtorApiModule.kt (1)
13-39: Ktorfit 기반 DI 모듈이 깔끔하게 구성되었습니다.6개의 서비스를 일관된 패턴으로 제공하고 있으며,
@Single스코프도 적절합니다.ktorfit.create()사용법도 정확합니다.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/MedicineService.kt
Outdated
Show resolved
Hide resolved
| fun homeService(ktorfit: Ktorfit): HomeService = | ||
| ktorfit.create() | ||
|
|
||
| @Single | ||
| fun glucoseService(ktorfit: Ktorfit): GlucoseService = | ||
| ktorfit.create() | ||
|
|
||
| @Single | ||
| fun mealService(ktorfit: Ktorfit): MealService = | ||
| ktorfit.create() | ||
|
|
||
| @Single | ||
| fun medicineService(ktorfit: Ktorfit): MedicineService = | ||
| ktorfit.create() | ||
|
|
||
| @Single | ||
| fun mentalService(ktorfit: Ktorfit): MentalService = | ||
| ktorfit.create() | ||
|
|
||
| @Single | ||
| fun sleepService(ktorfit: Ktorfit): SleepService = | ||
| ktorfit.create() |
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.
🧩 Analysis chain
🏁 Script executed:
# Ktorfit 인스턴스를 제공하는 함수 찾기
rg -n "@Single|@Provides|fun.*Ktorfit" app/src/main/java/com/konkuk/medicarecall/data/di/ --type=kt -B2 -A5Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# KtorApiModule 파일 전체 구조 확인
find app/src/main/java/com/konkuk/medicarecall/data/di/ -name "*.kt" | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 275
🏁 Script executed:
# Koin 설정에서 Ktorfit 관련 부분 찾기
rg -n "Ktorfit|HttpClient" app/src/main/java/com/konkuk/medicarecall/ --type=kt -B2 -A2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# KtorApiModule.kt 전체 내용 확인
cat -n app/src/main/java/com/konkuk/medicarecall/data/di/KtorApiModule.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 1475
🏁 Script executed:
# NetworkModule.kt 확인 - Ktorfit 제공자가 있을 가능성
cat -n app/src/main/java/com/konkuk/medicarecall/data/di/NetworkModule.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 3229
🏁 Script executed:
# ApiModule.kt 확인
cat -n app/src/main/java/com/konkuk/medicarecall/data/di/ApiModule.ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 5321
🏁 Script executed:
# Search entire codebase for Ktorfit provider
grep -r "fun.*Ktorfit" app/src/ --include="*.kt" | head -30Repository: Medicare-Call/Medicare-Call-Android
Length of output: 818
🏁 Script executed:
# Search for Ktorfit.Builder to see if it's configured anywhere
grep -r "Ktorfit\.Builder\|Ktorfit(" app/src/ --include="*.kt" -B2 -A5Repository: Medicare-Call/Medicare-Call-Android
Length of output: 61
Ktorfit 인스턴스가 DI에 등록되어 있지 않습니다.
모든 provider 메서드가 Ktorfit 파라미터를 요구하지만, DI 컨테이너에서 Ktorfit을 제공하는 곳이 없습니다. NetworkModule.kt에서는 Retrofit만 제공하고 있으며, Ktorfit.Builder를 사용하는 코드도 찾을 수 없습니다.
추가로, 같은 서비스들(HomeService, GlucoseService, MealService, MedicineService, MentalService, SleepService)이 ApiModule.kt에서 이미 Retrofit으로 제공되고 있어서, 두 모듈이 동시에 존재하면 Koin 바인딩 충돌이 발생할 것입니다.
🤖 Prompt for AI Agents
In @app/src/main/java/com/konkuk/medicarecall/data/di/KtorApiModule.kt around
lines 17 - 38, The module is requesting a Ktorfit instance but none is provided
and duplicates of the same Retrofit-provided services exist; add a single
provider for Ktorfit (e.g., a @Single function that builds Ktorfit via
Ktorfit.Builder and any required HttpClient/config) so homeService,
glucoseService, mealService, medicineService, mentalService, and sleepService
can receive it, and resolve the binding conflict with ApiModule by either
removing the duplicate Retrofit-based service providers or using distinct
qualifiers for the Retrofit vs Ktorfit variants (or consolidating all service
creation to one module such as NetworkModule) to avoid Koin binding collisions.
| @Path("elderId") elderId: Int, | ||
| @Query("date") date: String, | ||
| ): Response<MedicineResponseDto> | ||
| ): MedicineResponseDto |
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.
이 부분에서 Response를 없앤 이유가 따로 있을까요 ??
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.
Ktorfit에서 HTTP 실패를 예외로 처리한다고 해서
Response 없이 DTO 직접 반환하도록했습니다!
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt:
- Line 23: The log string in HomeRepositoryImpl currently interpolates the type
name `$Int` instead of the variable; replace `$Int` with the actual elder id
variable (e.g., `$elderId` or `${elderId}`) in the message `"전화 걸림, 어르신: $Int,
시간: $careCallOption"` so it prints the elder's id correctly and keep
`$careCallOption` as-is.
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MealRepositoryImpl.kt:
- Around line 16-50: The code incorrectly references response.code (which
Ktorfit Response doesn't expose); update the non-success branch in
MealRepositoryImpl to check the HTTP status via the Ktorfit-compatible property
(e.g., response.raw?.status?.value or response.status?.value) instead of
response.code, and fall back to treating unknown/non-available status as a
generic error; keep using response.isSuccessful and response.body() as before
and only return the 404 default MealUiState list when the resolved status value
equals 404.
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.kt:
- Line 18: Remove the unnecessary blank line at the start of the affected method
and delete the extra consecutive blank lines at the end of the file in
MentalRepositoryImpl (class MentalRepositoryImpl / the reported method around
line 18 and trailing lines around 40); ensure the method begins immediately with
its first statement and the file ends with a single newline only to satisfy lint
rules.
🧹 Nitpick comments (4)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MealRepositoryImpl.kt (2)
17-18: 에러 처리 방식 개선을 고려해보세요.현재
error()함수를 사용하여 에러를 던지고 있는데, 이는IllegalStateException을 발생시켜 앱이 크래시될 수 있습니다.더 안전한 방식으로 처리하는 것을 추천합니다:
Result<T>타입을 반환하여 성공/실패를 명시적으로 표현- 도메인 특화 예외를 정의하여 상위 레이어에서 처리
- sealed class로 상태를 표현 (Success, Error, etc.)
호출하는 쪽에서 try-catch로 처리하고 있을 수도 있지만, 더 명시적인 에러 처리가 유지보수에 도움이 될 것 같습니다.
Also applies to: 47-49
20-46: 반복되는 문자열 리터럴을 상수로 추출하면 좋을 것 같아요."식사 기록 전이에요." 문자열이 여러 곳에서 반복되고 있네요. 나중에 메시지를 변경하거나 다국어 지원을 추가할 때를 대비해서 companion object에 상수로 추출하는 것을 고려해보세요.
예시:
companion object { private const val NO_MEAL_RECORD_MESSAGE = "식사 기록 전이에요." }그리고 각 meal의
isRecorded체크 로직이 깔끔하게 구현되어 있고, 404 응답에 대한 별도 처리도 잘 되어 있습니다! 👍app/build.gradle.kts (2)
8-8: 버전 카탈로그 사용을 권장합니다.Kotlin serialization 플러그인 버전이 하드코딩되어 있습니다. 다른 플러그인들과의 일관성을 위해
gradle/libs.versions.toml에 버전을 정의하고alias(libs.plugins.kotlin.serialization)형태로 사용하는 것을 고려해보세요.♻️ 제안하는 리팩토링
gradle/libs.versions.toml에 다음을 추가:[versions] kotlin = "2.2.20" [plugins] kotlin-serialization = { id = "org.jetbrains.kotlin.plugin.serialization", version.ref = "kotlin" }그런 다음 이 파일에서:
- kotlin("plugin.serialization") version "2.2.20" + alias(libs.plugins.kotlin.serialization)
112-115: Ktorfit 의존성 버전 관리를 일관되게 유지하세요.Ktorfit 라이브러리와 KSP는 버전 카탈로그를 사용하지만, line 115의
ktorfit-converters-response는 버전이 하드코딩되어 있습니다. 일관성과 유지보수를 위해 모든 Ktorfit 관련 의존성을 버전 카탈로그로 관리하는 것을 추천합니다.♻️ 제안하는 리팩토링
gradle/libs.versions.toml에 다음을 추가:[versions] ktorfit = "2.7.1" [libraries] ktorfit-converters-response = { module = "de.jensklingenberg.ktorfit:ktorfit-converters-response", version.ref = "ktorfit" }그런 다음:
- implementation("de.jensklingenberg.ktorfit:ktorfit-converters-response:2.7.1") + implementation(libs.ktorfit.converters.response)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/build.gradle.ktsapp/src/main/java/com/konkuk/medicarecall/data/api/elders/GlucoseService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/HomeService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/MealService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/MedicineService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/MentalService.ktapp/src/main/java/com/konkuk/medicarecall/data/api/elders/SleepService.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MealRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.ktapp/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SleepRepositoryImpl.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- app/src/main/java/com/konkuk/medicarecall/data/api/elders/GlucoseService.kt
- app/src/main/java/com/konkuk/medicarecall/data/api/elders/MealService.kt
- app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.kt
- app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SleepRepositoryImpl.kt
- app/src/main/java/com/konkuk/medicarecall/data/api/elders/HomeService.kt
- app/src/main/java/com/konkuk/medicarecall/data/api/elders/MedicineService.kt
🧰 Additional context used
🪛 GitHub Actions: Android CI
app/build.gradle.kts
[warning] 55-55: 'jvmTarget: String' is deprecated. Please migrate to the compilerOptions DSL. More details: https://kotl.in/u1r8ln
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.kt
[error] 41-41: Detekt: Needless blank line(s) [NoConsecutiveBlankLines]
[error] 18-18: Detekt: First line in a method block should not be empty [NoEmptyFirstLineInMethodBlock]
🔇 Additional comments (10)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt (2)
28-30: Ktorfit 응답 처리 방식으로 잘 전환되었습니다
HttpException대신error()를 사용하고, 응답 코드를 포함한 명확한 에러 메시지를 제공하고 있어 좋습니다.
37-44: Response 래퍼 처리 로직이 안전하게 구현되었습니다
isSuccessful체크와 null-safety 검증이 모두 포함되어 있어 안전한 응답 처리가 가능합니다. 에러 메시지도 명확하게 작성되어 디버깅에 도움이 될 것 같습니다.app/src/main/java/com/konkuk/medicarecall/data/api/elders/SleepService.kt (2)
4-7: Retrofit에서 Ktorfit으로 import 마이그레이션 완료!import 변경이 깔끔하게 처리되었습니다. Ktorfit의 동등한 어노테이션들로 잘 전환되었네요.
14-14: Response 래퍼 처리 잘 구현되어 있네요SleepRepositoryImpl에서 Response 타입을
isSuccessful체크와body()언래핑으로 깔끔하게 처리 중입니다. 404 에러도 별도로 핸들링하고 null 바디에 대한 방어 로직도 있어서 좋습니다.app/src/main/java/com/konkuk/medicarecall/data/api/elders/MentalService.kt (2)
4-7: Ktorfit import로 깔끔하게 전환되었습니다.Retrofit의 http 어노테이션을 Ktorfit의 동일한 어노테이션으로 교체했네요. 다른 마이그레이션된 서비스들(GlucoseService, MealService 등)과 일관성 있게 진행되었습니다.
14-14: Ktorfit Response 래퍼 적용이 검증되었습니다.Ktorfit Response API가
isSuccessful,code,body()메서드를 제공하는 것이 확인되었으며, MentalRepositoryImpl에서 모두 정확하게 사용되고 있습니다. 전체 30개 이상의 서비스와 RepositoryImpl 구현체에서 일관되게 적용되고 있어 마이그레이션이 올바르게 진행되었습니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.kt (2)
19-30: Response 기반 성공 처리가 잘 구현되었습니다.
response.body()?.commentList.orEmpty()로 null-safe하게 처리하고,isRecorded를 댓글 리스트가 비어있지 않은지로 판단하는 로직이 명확합니다. Retrofit의 예외 처리 방식에서 Ktorfit의 Response 기반 처리로 깔끔하게 전환되었네요.
31-37: 에러 처리 로직이 적절합니다.404는 데이터 미존재를 의미하므로
MentalUiState.EMPTY를 반환하고, 그 외 에러는error()로 예외를 던져 상위 레이어에서 처리하도록 한 것이 합리적입니다. 에러 메시지에 response code를 포함시켜 디버깅도 용이하네요.app/build.gradle.kts (2)
100-104: Retrofit 의존성은 유지해야 합니다.코드베이스 전반에서 Retrofit이 광범위하게 사용되고 있어 현재 의존성 제거가 불가능합니다. 인증, 노인정보 관리, 알림, 결제, 통계 등 핵심 모듈의 다수 서비스(AuthService, EldersInfoService, NotificationService, SettingService, NaverPayService 등)와 저장소 구현체에서
retrofit2.Response, Retrofit 어노테이션(@get, @post 등),retrofit2.HttpException을 활발히 사용 중입니다.PR 제목과 달리 이 마이그레이션은 일부 모듈(Home/HomeDetail)만 단계적으로 진행 중이며, Retrofit 완전 제거는 향후 작업으로 미루는 것이 맞습니다.
106-110: Ktor 클라이언트 의존성 추가가 좋아요.Kotlin 2.2.20과 함께 쓰기에도 딱 좋고, Ktorfit 2.7.1 이상이랑도 완벽하게 호환돼요. 버전 카탈로그로 관리하고 있으니 나중에 업데이트하기도 편하겠네요. 필요한 라이브러리들 - 코어, OkHttp 엔진, 컨텐츠 협상, JSON 직렬화 - 다 잘 갖춰져 있습니다.
| if (response.isSuccessful) { | ||
| Log.d( | ||
| "httplog", | ||
| "전화 걸림, 어르신: $Int, 시간: $careCallOption", |
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.
로그 메시지의 문자열 보간 오류
$Int는 변수가 아니라 타입 이름이므로, 문자열에 그대로 "Int"로 출력됩니다. $elderId로 수정이 필요합니다.
🐛 수정 제안
- "전화 걸림, 어르신: $Int, 시간: $careCallOption",
+ "전화 걸림, 어르신: $elderId, 시간: $careCallOption",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "전화 걸림, 어르신: $Int, 시간: $careCallOption", | |
| "전화 걸림, 어르신: $elderId, 시간: $careCallOption", |
🤖 Prompt for AI Agents
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt
at line 23, The log string in HomeRepositoryImpl currently interpolates the type
name `$Int` instead of the variable; replace `$Int` with the actual elder id
variable (e.g., `$elderId` or `${elderId}`) in the message `"전화 걸림, 어르신: $Int,
시간: $careCallOption"` so it prints the elder's id correctly and keep
`$careCallOption` as-is.
| return if (response.isSuccessful) { | ||
| val body = response.body() | ||
| ?: error("Meal response body is null") | ||
|
|
||
| listOf( | ||
| MealUiState( | ||
| mealTime = "아침", | ||
| description = body.meals.breakfast ?: "식사 기록 전이에요.", | ||
| isRecorded = body.meals.breakfast != null, | ||
| isEaten = null, | ||
| ), | ||
| MealUiState( | ||
| mealTime = "점심", | ||
| description = body.meals.lunch ?: "식사 기록 전이에요.", | ||
| isRecorded = body.meals.lunch != null, | ||
| isEaten = null, | ||
| ), | ||
| MealUiState( | ||
| mealTime = "저녁", | ||
| description = body.meals.dinner ?: "식사 기록 전이에요.", | ||
| isRecorded = body.meals.dinner != null, | ||
| isEaten = null, | ||
| ), | ||
| ) | ||
| } else { | ||
| if (response.code == 404) { | ||
| listOf( | ||
| MealUiState("아침", "식사 기록 전이에요.", false, null), | ||
| MealUiState("점심", "식사 기록 전이에요.", false, null), | ||
| MealUiState("저녁", "식사 기록 전이에요.", false, null), | ||
| ) | ||
| } else { | ||
| error("Failed to fetch meal data: ${response.code}") | ||
| } | ||
| } |
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.
🧩 Analysis chain
🌐 Web query:
Ktorfit Response type API documentation isSuccessful code properties
💡 Result:
Short answer — Ktorfit includes a built-in Response wrapper (added in 1.4.0) with the usual convenience members similar to Retrofit’s Response:
- isSuccessful: Boolean — true for successful responses (2xx). [1]
- body(): returns the deserialized success body (when isSuccessful == true). [1][2]
- errorBody(): returns the error body for unsuccessful responses. [1][2]
- (Underlying HTTP status/code available via the Ktor HttpResponse used by converters — e.g. HttpResponse.status / status.value). [3]
References:
- Ktorfit changelog (Response wrapper, example using isSuccessful/body/errorBody). [1]
- Ktorfit Response/ResponseConverter docs (how the wrapper is produced by converters). [2]
- Ktor HttpResponse docs (status / status.value access). [3]
🏁 Script executed:
# Find and read the MealRepositoryImpl.kt file to check imports and full context
fd "MealRepositoryImpl.kt" -type f | head -1 | xargs cat -nRepository: Medicare-Call/Medicare-Call-Android
Length of output: 249
🏁 Script executed:
# Find and read the MealRepositoryImpl.kt file
fd "MealRepositoryImpl.kt" | head -1 | xargs cat -nRepository: Medicare-Call/Medicare-Call-Android
Length of output: 2383
🏁 Script executed:
# Also search for Response imports in the entire codebase to understand which Response type is being used
rg "import.*Response" --type kotlin | head -20Repository: Medicare-Call/Medicare-Call-Android
Length of output: 2840
🏁 Script executed:
# Find MealService to check the return type annotation
fd "MealService.kt" | xargs cat -nRepository: Medicare-Call/Medicare-Call-Android
Length of output: 673
🏁 Script executed:
# Search for response.code usage to see if this is a pattern in the codebase (indicating Retrofit still in use)
rg "response\.code" --type kotlin | head -10Repository: Medicare-Call/Medicare-Call-Android
Length of output: 1525
🏁 Script executed:
# Check if Ktorfit is in the dependencies
rg "de\.jensklingenberg\.ktorfit" build.gradle.kts app/build.gradle.kts 2>/dev/null | head -5Repository: Medicare-Call/Medicare-Call-Android
Length of output: 182
Ktorfit Response에 code 속성이 없습니다. 런타임 오류가 발생할 예정입니다.
Ktorfit의 Response 래퍼는 isSuccessful과 body()는 제공하지만, HTTP 상태 코드는 code 속성으로 직접 접근할 수 없습니다. 현재 코드의 41번과 48번 줄에서 response.code를 사용하고 있는데, 이는 Ktorfit Response 타입에 존재하지 않는 속성입니다.
response.isSuccessful✓ (사용 가능)response.body()✓ (사용 가능)response.code✗ (존재하지 않음)
Ktorfit에서 HTTP 상태 코드에 접근하려면 응답 객체의 다른 속성을 확인하거나, 404 등의 특정 상황을 다르게 처리해야 합니다. 현재 형태로는 404 상태 확인 시 런타임 오류가 발생합니다.
🤖 Prompt for AI Agents
In
@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MealRepositoryImpl.kt
around lines 16 - 50, The code incorrectly references response.code (which
Ktorfit Response doesn't expose); update the non-success branch in
MealRepositoryImpl to check the HTTP status via the Ktorfit-compatible property
(e.g., response.raw?.status?.value or response.status?.value) instead of
response.code, and fall back to treating unknown/non-available status as a
generic error; keep using response.isSuccessful and response.body() as before
and only return the 404 default MealUiState list when the resolved status value
equals 404.
🔗 관련 이슈
📙 작업 설명
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리스 노트
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.