[Refactor] API 서비스 패키지 구조 개편 #123 #127
Conversation
-TokenRefreshService.kt -VerificationService.kt
- SettingService 내의 logout()을 AuthService로 이동 - UserRepositoryImpl에서 logout 호출 시 AuthService 사용하도록 수정 - SettingService는 member 패키지로 이동
WalkthroughRetrofit 서비스들을 도메인별 패키지로 재구성하고, 인증·검증 API들을 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant App as App
participant API as Backend
participant Auth as AuthService
participant Store as TokenStore
note over App,API: 보호된 API 요청 흐름 (리프레시 포함)
App->>API: 요청(Access Token)
API-->>App: 401 Unauthorized
App->>Auth: refreshToken(refresh)
Auth-->>App: 200 + new tokens / 4xx
alt success
App->>Store: saveTokens(new)
App->>API: 재시도(새 Access Token)
API-->>App: 200 OK
else failure
App->>Store: clearTokens()
App-->>Client: 인증 필요 (sign-out)
end
sequenceDiagram
autonumber
actor User
participant Repo as UserRepository
participant Auth as AuthService
participant Store as TokenStore
User->>Repo: logout()
Repo->>Store: getRefresh()
Repo->>Auth: logout(Authorization: Bearer <refresh>)
Auth-->>Repo: 200/err
Repo->>Store: clearTokens()
Repo-->>User: 완료/오류
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersInfoRepositoryImpl.kt (1)
41-62: 치명적: 함수 반환 타입 불일치로 컴파일/런타임 오류 위험.
updateElder는Result<Unit>을 반환하도록 선언됐지만, 성공 시response.body()(EldersInfoResponseDto)를 그대로 반환하고 있어 제네릭이 어긋납니다. 성공 시Unit을 반환하도록 수정하고 예외 타입도 일관되게HttpException으로 맞추는 것을 권장합니다.아래처럼 정리해 주세요:
- ): Result<Unit> = runCatching { + ): Result<Unit> = runCatching { val response = eldersInfoService.updateElder( id, ElderRegisterRequestDto( request.name, birthDate = request.birthDate.formatAsDate(), gender = if (request.gender) GenderType.MALE else GenderType.FEMALE, phone = request.phoneNumber, - relationship = RelationshipType.entries.find { it.displayName == request.relationship }!!, - residenceType = ElderResidenceType.entries.find { it.displayName == request.livingType }!!, + relationship = requireNotNull( + RelationshipType.entries.find { it.displayName == request.relationship } + ) { "Unknown relationship: ${request.relationship}" }, + residenceType = requireNotNull( + ElderResidenceType.entries.find { it.displayName == request.livingType } + ) { "Unknown residenceType: ${request.livingType}" }, ), ) - if (response.isSuccessful) { - response.body() ?: throw IllegalStateException("Response body is null") - } else { - val errorBody = response.errorBody()?.string() ?: "Unknown error" - throw Exception(errorBody) - } + if (response.isSuccessful) { + Unit + } else { + throw HttpException(response) + } }app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt (1)
18-26: errorBody를 두 번.string()호출 — 두 번째는 항상 빈 값.
ResponseBody.string()은 1회성입니다. 한 번만 읽고 재사용하세요. 그렇지 않으면 로그/예외 메시지가 비게 됩니다.- if (!response.isSuccessful) { - Log.e("SetCallRepository", "HTTP ${response.code()} ${response.message()}") - Log.e("SetCallRepository", "ErrorBody=${response.errorBody()?.string()}") - throw Exception( - "Error saving care call times: ${ - response.errorBody()?.string() - } / SetCallRepository.kt", - ) - } + if (!response.isSuccessful) { + val err = response.errorBody()?.string() + Log.e("SetCallRepository", "HTTP ${response.code()} ${response.message()}") + Log.e("SetCallRepository", "ErrorBody=$err") + throw Exception("Error saving care call times: $err / SetCallRepository.kt") + }app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.kt (1)
19-24: 에러 바디를 읽고 버리고 있어요(소비 부작용 가능)
errorBody()?.string()은 스트림을 소비합니다. 현재 미사용이므로 제거하거나 로깅에만 사용하세요.적용 예:
- } else { - val errorBody = response.errorBody()?.string() ?: "Unknown error" - throw HttpException(response) - } + } else { + // TODO: 필요 시 여기서 errorBody를 파싱해 사용자 친화적 에러로 매핑 + throw HttpException(response) + }app/src/main/java/com/konkuk/medicarecall/data/di/NetworkModule.kt (3)
44-48: 로그 인터셉터가 토큰을 노출할 수 있습니다(Release 포함 BODY 레벨)
Authorization헤더를 redaction하고, Release 빌드에서는 LEVEL을 낮추세요.fun provideHttpLoggingInterceptor(): HttpLoggingInterceptor { - return HttpLoggingInterceptor().apply { - level = HttpLoggingInterceptor.Level.BODY - } + return HttpLoggingInterceptor().apply { + // 민감 헤더 마스킹 + redactHeader("Authorization") + level = if (BuildConfig.DEBUG) + HttpLoggingInterceptor.Level.BODY + else + HttpLoggingInterceptor.Level.BASIC + } }
33-40: AuthService 직접 주입은 DI 순환을 유발할 수 있습니다OkHttpClient → Authenticator → AuthService(Retrofit) → OkHttpClient 순환이 발생하기 쉽습니다. Lazy/Provider 주입 또는 리프레시 전용 Retrofit(Authenticator 미적용 클라이언트)로 분리하는 걸 권장합니다.
정석 옵션 중 하나:
- AuthAuthenticator 생성자:
private val authService: Lazy<AuthService>로 변경- NetworkModule:
provideAuthAuthenticator(..., authService: Lazy<AuthService>)- Authenticator 내부 호출 시
authService.get().refreshToken(...)
52-61: OkHttpClient에 AuthAuthenticator 미등록 — 401 자동 갱신 실패 위험
provideOkHttpClient에AuthAuthenticator의존성을 추가하고.authenticator(authAuthenticator)호출을 등록해 주세요. (Lines 52–61)app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt (2)
31-38: 새 accessToken이 null/blank인 경우 "Bearer null" 헤더가 붙을 수 있습니다재시도 분기에서
accessTokennull/blank를 체크하고, 없으면 null 반환이 안전합니다.- if (oldAccessToken != null && oldAccessToken != accessToken) { + if (oldAccessToken != null && oldAccessToken != accessToken) { + if (accessToken.isNullOrBlank()) return null return response.request.newBuilder() .header("Authorization", "Bearer $accessToken") .build() }
40-49: 토큰 존재 체크는 null 뿐 아니라 blank도 함께 보세요빈 문자열이 저장된 경우 무한 재시도 루프에 가까운 동작이 나올 수 있습니다.
- if (refreshToken == null) { + if (refreshToken.isNullOrBlank()) { // 여기서 로그인 화면으로 보내는 로직을 추가할 수 있습니다. (예: EventBus, SharedFlow 등) return null } // 6. 토큰 갱신 API 호출 (runBlocking 사용) - val refreshResponse = runBlocking { - authService.refreshToken(TokenRefreshRequestDto(refreshToken)) - } + val refreshResponse = try { + runBlocking { + authService.refreshToken(TokenRefreshRequestDto(refreshToken)) + } + } catch (t: Throwable) { + Log.e("AuthAuthenticator", "Token refresh call failed", t) + return null + }
🧹 Nitpick comments (14)
app/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.kt (2)
22-26: 업데이트에 POST 사용 확인 필요 (메서드명/HTTP verb 정합성).
updateElder가@POST("elders/{elderId}")를 사용합니다. 서버 스펙이 POST‑update로 정의돼 있다면 그대로 OK이지만, 일반적으로는 PUT/PATCH를 쓰거나 메서드명을upsertElder처럼 명확히 하는 편이 헷갈림을 줄입니다. 백엔드 스펙 기준으로 한 번만 확인해 주세요.
39-42: 케어콜 API는 SetCallService로 모아 cohesion 높이기 제안.
GET elders/{elderId}/care-call-setting이 여기 있고, 저장은SetCallService에 있습니다. 조회/저장을 동일 서비스(SetCallService또는CareCallService)로 묶으면 탐색성과 변경 파급이 줄어듭니다. 이동 가능하면 옮기는 걸 추천합니다.app/src/main/java/com/konkuk/medicarecall/data/api/elders/SetCallService.kt (1)
10-15: 메서드명/HTTP verb 확인 한 번만 부탁!
saveCareCallTimes가 POST입니다. 서버가 멱등 업데이트를 요구하면 PUT이 더 적합하고, POST 유지 시엔 이름을updateCareCallTimes로 바꾸는 것도 선택지입니다. 스펙대로면 그대로 진행해도 무방합니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersInfoRepositoryImpl.kt (4)
20-28: 에러 처리 일관성 + 불필요 변수 제거.
getElders()에서errorBody를 구해놓고HttpException(response)를 던져 실제로는 사용하지 않습니다. 불필요 변수 제거하거나, 커스텀 예외 메시지로 쓸 거면Exception(errorBody)로 통일하는 등 한 가지 방식으로 맞추는 게 좋습니다. 현재 파일 내 메서드들 간에도HttpException/Exception혼재되어 있습니다.
64-71: DELETE 성공 처리 단순화 제안.
Response<Unit>는 본문이 의미 없으니 성공 시 본문 접근 없이Unit만 반환하는 게 간결하고 안전합니다(본문 소비에 의존하지 않음).- if (response.isSuccessful) { - response.body() ?: throw IllegalStateException("Response body is null") - } else { + if (response.isSuccessful) { + Unit + } else { val errorBody = response.errorBody()?.string() ?: "Unknown error" throw HttpException(response) }
49-54: 널 단정(!!)
RelationshipType/ElderResidenceType매핑에!!을 사용하면 런타임 크래시 납니다. 위 수정안처럼requireNotNull로 바꾸고 원인 파악 쉬운 메시지를 남기는 쪽이 안전합니다.매핑 기준이 UI 표시 문자열(
displayName)이라 레이아웃/로캘 변경에 취약할 수 있습니다. 서버 표준 코드 기반으로 맵핑할 수 있는지 확인 부탁드립니다.
13-13: 레이어링: data 레이어가 ui 유틸에 의존.
formatAsDate가ui.common.util에 있어 data→ui 의존이 생깁니다. 공통 모듈(core/util 등)로 옮기고 두 레이어가 그걸 참조하도록 구조를 정리하는 걸 추천합니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt (1)
37-46: 시간 변환 입력값 검증 추가 제안.
amPm/h12/m범위를 보장하지 않으면 이상치가 그대로 서버로 전달됩니다. 간단한require검증을 추가해 조기 실패하도록 하죠.private fun Triple<Int, Int, Int>.toHHmm(): String { val (amPm, h12, m) = this + require(amPm in 0..1) { "amPm must be 0(AM) or 1(PM): $amPm" } + require(h12 in 1..12) { "hour(12h) must be 1..12: $h12" } + require(m in 0..59) { "minute must be 0..59: $m" } val h24 = when { amPm == 0 && h12 == 12 -> 0 amPm == 1 && h12 < 12 -> h12 + 12 else -> h12 % 24 } Log.d("SetCallRepository", "Converting time: $this to 24-hour format: $h24:$m") return "%02d:%02d".format(h24, m) }app/src/main/java/com/konkuk/medicarecall/data/api/elders/GlucoseService.kt (1)
13-18: 문자열 대신 타입 세이프 enum 사용 제안.
type파라미터는 도메인 상BEFORE_MEAL/AFTER_MEAL두 값뿐이라 enum으로 제한하면 안전합니다. 서버가 대문자 값을 기대한다면 enum 이름과 정확히 일치시켜 주세요.- @Query("type") type: String // BEFORE_MEAL or AFTER_MEAL + @Query("type") type: GlucoseType ): Response<GlucoseResponseDto> } + +enum class GlucoseType { BEFORE_MEAL, AFTER_MEAL }app/src/main/java/com/konkuk/medicarecall/data/api/elders/HomeService.kt (1)
18-21: care-call 엔드포인트 분리 제안
패키지를 prefix 기준으로 재정리한 흐름이라면/care-call/immediate엔드포인트는 별도 서비스(예:carecall전용 서비스)로 떼어내는 편이 정리에 더 잘 맞을 것 같아요. 지금처럼 elders 패키지 안에 두면 이후 유지보수 시 혼선이 생길 여지가 있어 보여요.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.kt (1)
12-16: 파라미터type은 enum/sealed로 제한하는 것을 고려하세요자유 문자열 대신 enum(예: Day/Week/Month 등)으로 모델링하면 오입력 방지와 호출부 가독성이 좋아집니다.
app/src/main/java/com/konkuk/medicarecall/data/api/member/SettingService.kt (1)
15-18: 요청/응답 DTO를 분리해 주세요
updateMyInfo가 요청 바디로MyInfoResponseDto를 사용합니다. 요청 전용 DTO(예:MyInfoUpdateRequestDto)로 분리하면 직렬화 안정성과 스키마 진화를 안전하게 처리할 수 있습니다.app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt (1)
64-73: 실패 시 토큰 정리는 access/refresh 모두 일관되게 처리하세요현재 refreshToken만
""로 저장합니다. 이후 체크를isNullOrBlank()로 바꿨으니 최소한 둘 다 비우거나 별도 clear 메서드를 쓰는 편이 안전합니다.- runBlocking { dataStoreRepository.saveRefreshToken("") } + runBlocking { + dataStoreRepository.saveAccessToken("") + dataStoreRepository.saveRefreshToken("") + }추가로 DataStore 층에
clearTokens()같은 API가 있다면 그걸 사용하세요.app/src/main/java/com/konkuk/medicarecall/data/api/elders/HealthService.kt (1)
8-14: 서비스 반환 타입 스타일 통일 필요
HealthService는 DTO(HealthResponseDto) 직접 반환하는 반면, NoticeService·SettingService·SubscribeService·EldersInfoService 등에서는 Response를 반환하고 있어요. 팀 가이드에 맞춰 전부 DTO 직접 반환 또는 전부 Response 중 한 가지 방식으로 통일해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
app/build.gradle.kts(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/TokenRefreshService.kt(0 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/VerificationService.kt(0 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/auth/AuthService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/ElderRegisterService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/EldersInfoService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/GlucoseService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/HealthService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/HomeService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/MealService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/MedicineService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/MentalService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/SetCallService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/SleepService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/StatisticsService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/elders/SubscribeService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/member/MemberRegisterService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/member/SettingService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/notice/NoticeService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/api/payments/NaverPayService.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/di/ApiModule.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/di/NetworkModule.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt(4 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ElderRegisterRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersHealthInfoRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersInfoRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/GlucoseRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HealthRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MealRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MedicineRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MemberRegisterRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/NaverPayRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/NoticeRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SleepRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/StatisticsRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SubscribeRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/UpdateElderInfoRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/UserRepositoryImpl.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/VerificationRepositoryImpl.kt(2 hunks)
💤 Files with no reviewable changes (2)
- app/src/main/java/com/konkuk/medicarecall/data/api/TokenRefreshService.kt
- app/src/main/java/com/konkuk/medicarecall/data/api/VerificationService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (23)
app/build.gradle.kts (1)
123-125: Detekt JVM 타깃 정렬 좋아요.
Kotlin/Java 컴파일 설정과 동일하게 맞춰 통합 빌드 환경을 유지할 수 있네요.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersInfoRepositoryImpl.kt (1)
1-3: 마이그레이션 잔재 없음 확인
제공한 스크립트 실행 결과, 구 패키지(API 패키지, TokenRefreshService/VerificationService) 및 로그아웃 경로(SettingService.logout, AuthService.logout) 참조가 전부 제거된 것을 확인했습니다.app/src/main/java/com/konkuk/medicarecall/data/api/elders/MentalService.kt (1)
1-1: 패키지 이동 확인
elders도메인에 맞게 패키지 경로를 정리한 덕분에 Retrofit 엔드포인트 구조와 한눈에 매칭됩니다. 추가 수정 필요 없어 보입니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersHealthInfoRepositoryImpl.kt (1)
4-5: 도메인별 서비스 경로 정리 👍Elder 관련 서비스 의존성을
data.api.elders로 옮기면서 DI 구성과 일관성이 좋아졌습니다. 다른 레이어 영향도 없네요.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SubscribeRepositoryImpl.kt (1)
4-4: elders 전용 서비스로 정리 완료구독 API도 elders 패키지로 이동해 네이밍 체계가 통일되었습니다. 문제 없이 잘 맞물립니다.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/ElderRegisterRepositoryImpl.kt (1)
4-4: Import 경로 정리 잘 됐어요Elder 등록 흐름이 elders 패키지로 모여서 추후 유지보수에도 도움이 될 듯합니다. 다른 로직 영향 없습니다.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MedicineRepositoryImpl.kt (1)
3-3: 의약품 API도 elders 묶음으로 이동MedicineService 의존성도 elders 네임스페이스로 맞춰져 서비스 분리가 자연스럽습니다. 이 상태로 괜찮아요.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/NoticeRepositoryImpl.kt (1)
4-4: 패키지 경로 업데이트 확인 완료NoticeService를 notice 패키지로 옮긴 흐름이 잘 반영됐네요. 기존 로직에 영향 없이 구조만 더 깔끔해진 것 같아요.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/UpdateElderInfoRepositoryImpl.kt (1)
4-4: elders 패키지로의 이동 적합EldersInfoService가 elders 패키지로 묶이면서 DI/레포 구조에 맞춰 잘 정리됐어요. 다른 의존성 영향도 없어 보입니다.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/SleepService.kt (1)
1-1: 패키지 네임스페이스 이동 OKSleepService도 elders 네임스페이스로 맞춰진 덕에 서비스 그룹이 더 명확해졌네요. 인터페이스 자체는 그대로라 안심입니다.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MentalRepositoryImpl.kt (1)
4-4: MentalService import 정리 👍MentalService를 elders 패키지로 이동시킨 변경이 잘 반영됐습니다. 레포 로직은 그대로 유지돼서 리스크 없어요.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt (1)
4-4: HomeService도 elders 쪽으로 합류 완료HomeService까지 elders 패키지로 묶이면서 서비스 구조가 한눈에 들어오네요. 기존 기능 흐름은 변함없어 보입니다.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MemberRegisterRepositoryImpl.kt (1)
3-3: 패키지 리팩터링 반영 깔끔합니다.새로 분리된
member패키지 경로로 의존성 정리된 것 확인됐어요. 다른 영향 범위도 없어서 그대로 가면 될 듯합니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/VerificationRepositoryImpl.kt (1)
15-16: 비정상 응답 처리 확인 필요
authService.requestCertificationCode(...)호출 결과가Response<*>형태라면,confirmPhoneNumber처럼isSuccessful체크 없이 바로 반환하면 4xx/5xx가 성공 케이스로 들어와버려요. 기존VerificationService시절과 동일하게 예외가 던져지는지, 아니면 명시적으로response.isSuccessful분기 추가가 필요한지 한 번만 더 확인 부탁드립니다.app/src/main/java/com/konkuk/medicarecall/data/api/elders/StatisticsService.kt (1)
1-15: 패키지 이동 깔끔합니다
패키지 네임스페이스만 정리된 거라 추가 리스크 없어 보입니다. 👍app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/NaverPayRepositoryImpl.kt (1)
3-3: 의존성 경로 업데이트 문제 없습니다
새 패키지 구조에 맞춰 import만 바뀐 거라 동작 영향은 없고, 정리도 잘 된 것 같아요.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SleepRepositoryImpl.kt (1)
3-3: 패키지 리팩토링 반영 잘 됐어요새 elders 네임스페이스에 맞춰 import 업데이트된 거 확인했습니다. 다른 레포지토리들도 같은 구조로 정리되어서 일관성이 좋아졌네요.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/StatisticsRepositoryImpl.kt (1)
3-3: elders 서비스 경로로 정리된 부분 좋습니다StatisticsService도 elders 패키지로 옮겨진 흐름과 맞춰져서 의존성 관계가 한눈에 들어와요. 👍
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HealthRepositoryImpl.kt (1)
3-3: HealthService import 교체 깔끔해요리팩토링된 elders 패키지 구조에 맞춰 HealthService 경로도 잘 조정됐습니다. 추가 수정 없이 기존 로직과 잘 이어지네요.
app/src/main/java/com/konkuk/medicarecall/data/api/member/MemberRegisterService.kt (1)
1-16: member 패키지로 이동한 구성 좋아요파일 위치와 패키지 선언이 일치하도록 정리돼서 구조적 가독성이 확실히 좋아졌습니다. 인터페이스 시그니처도 그대로라 영향도 없겠네요.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/MealRepositoryImpl.kt (1)
3-3: MealService import 동기화 완료 확인했습니다elders 패키지로 서비스가 정리된 흐름에 따라 import만 교체해 주신 부분 문제없어요.
app/src/main/java/com/konkuk/medicarecall/data/api/elders/HealthService.kt (1)
1-1: 패키지 이동 LGTMelders 도메인으로의 이동 방향성 좋습니다. 네이밍/경로 일관성도 OK.
app/src/main/java/com/konkuk/medicarecall/data/api/member/SettingService.kt (1)
9-20: SettingService.logout 참조 정리 및 AuthService.logout 전환 확인
SettingService 인터페이스에는 더 이상 logout 메서드가 없으며, ViewModel( MyDataViewModel )과 Repository( UserRepositoryImpl )에서 UserRepository.logout → AuthService.logout 호출로 전환된 것을 확인했습니다.
app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt (2)
19-26: 리프레시 엔드포인트 자기호출(무한 재귀) 차단 가드 필요
authenticate내에서 토큰 리프레시 요청도 401이 나면 다시Authenticator가 호출돼 재귀·루프가 발생할 수 있습니다. 리프레시/인증 계열 경로는 즉시null을 반환하도록 가드를 추가해 주세요.override fun authenticate(route: Route?, response: Response): Request? { // 1. 401 에러가 아니면 null 반환 (다른 에러는 이 Authenticator가 처리하지 않음) if (response.code != 401) { return null } + // 1.5 리프레시/인증 경로에서는 Authenticator 비활성화하여 재귀 방지 + val path = response.request.url.encodedPath + if (path.startsWith("/auth/") || path.startsWith("/verification/")) { + return null + }
41-44: 빈 문자열 토큰 처리 불일치로 인한 반복 리프레시 시도현재는 실패 시
refreshToken을""로 저장(Line 70)하지만, 존재 여부 체크는null만 검사(Line 41)합니다. 그 결과 빈 문자열이 저장된 상태에서도 매 401마다 리프레시 호출이 재시도됩니다.isNullOrBlank()로 통일하고 실패 시 액세스/리프레시 토큰을 모두 빈값으로 정리하세요.- if (refreshToken == null) { + if (refreshToken.isNullOrBlank()) { // 여기서 로그인 화면으로 보내는 로직을 추가할 수 있습니다. (예: EventBus, SharedFlow 등) return null } ... - runBlocking { dataStoreRepository.saveRefreshToken("") } + runBlocking { + dataStoreRepository.saveAccessToken("") + dataStoreRepository.saveRefreshToken("") + }Also applies to: 70-71
app/src/main/java/com/konkuk/medicarecall/data/di/NetworkModule.kt (1)
44-47: 로그 민감정보 노출 위험: BODY 레벨/토큰 미마스킹
HttpLoggingInterceptor.Level.BODY를 상시 사용하면 본문·헤더(Authorization 포함)가 로그에 남습니다. 릴리스에서는 비활성화하고 민감 헤더는 마스킹하세요.fun provideHttpLoggingInterceptor(): HttpLoggingInterceptor { - return HttpLoggingInterceptor().apply { - level = HttpLoggingInterceptor.Level.BODY - } + return HttpLoggingInterceptor().apply { + level = if (BuildConfig.DEBUG) + HttpLoggingInterceptor.Level.BODY + else + HttpLoggingInterceptor.Level.NONE + redactHeader("Authorization") + redactHeader("Cookie") + }
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/data/di/NetworkModule.kt (1)
57-63: 인터셉터 순서/타임아웃 정비 제안
- 로깅은 마지막에 두어 최종 요청/응답을 관찰하도록 권장.
connectTimeout/writeTimeout도 명시해 일관성 확보.return OkHttpClient.Builder() - .readTimeout(20, TimeUnit.SECONDS) + .readTimeout(20, TimeUnit.SECONDS) + .connectTimeout(20, TimeUnit.SECONDS) + .writeTimeout(20, TimeUnit.SECONDS) .addInterceptor(authInterceptor) - .addInterceptor(loggingInterceptor) - .authenticator(authAuthenticator) + .authenticator(authAuthenticator) + .addInterceptor(loggingInterceptor) // 마지막에 .build()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/konkuk/medicarecall/data/api/auth/AuthService.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/di/NetworkModule.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/konkuk/medicarecall/data/api/auth/AuthService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (3)
app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt (1)
16-16: Lazy 주입으로 순환참조 리스크 해소
dagger.Lazy<AuthService>로 전환한 점 좋습니다. 과거 코멘트에서 제안된 순환 참조 방지 가이드를 충실히 반영했습니다.app/src/main/java/com/konkuk/medicarecall/data/di/NetworkModule.kt (2)
35-40: AuthAuthenticator에 Lazy 주입 배선 OKProvider 형태로 주입해 순환 참조 가능성을 낮춘 구성 좋습니다.
52-56: 리프레시 전용 클라이언트 분리 확인 필요
현재 제공된 OkHttpClient(provideOkHttpClient)에는.authenticator(authAuthenticator)가 장착되어 있어, 같은 인스턴스를AuthService.refreshToken()호출에 재사용하면 무한 재귀가 발생할 위험이 있습니다. 별도 인증로직이 배제된 Retrofit/OkHttpClient(@nAmed("noAuth") 등)를 구성했는지 확인 바랍니다.
| val refreshResponse = runBlocking { | ||
| tokenRefreshService.get().refreshToken(TokenRefreshRequestDto(refreshToken)) | ||
| authService.get().refreshToken(TokenRefreshRequestDto(refreshToken)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
리프레시 호출이 동일 OkHttp(인터셉터/Authentictor 포함)를 타는지 확인 필요
authService.get().refreshToken(...)가 현재 클라이언트의 AuthInterceptor/AuthAuthenticator를 그대로 타면, 401 시 재귀가 발생합니다. 리프레시 전용 Retrofit(인터셉터·Authenticator 미적용) 또는 인터셉터에서 리프레시 경로를 명시적으로 스킵하는지 확인 부탁드립니다. 필요 시 @Named 클라이언트 분리 권장.
🏁 Script executed:
#!/bin/bash
# AuthService 제공자/클라이언트 배선 확인
rg -nP --type=kt -C3 'provideAuthService|Retrofit\\s*\\.\\s*create\\(\\s*AuthService::class' app
# AuthInterceptor가 리프레시/인증 경로를 스킵하는지 확인
rg -nP --type=kt -C3 'class\\s+AuthInterceptor\\b|intercept\\(|encodedPath|auth/refresh|verification' app
# 별도 no-auth/refresh 클라이언트 존재 여부
rg -nP --type=kt -C2 '@Named\\(|@Qualifier|noAuth|unauth|refresh' appLength of output: 218
스크립트 패턴의 괄호 이스케이프가 빠져 에러가 발생한 것 같습니다. 아래 수정된 스크립트를 실행해 리프레시 전용 클라이언트 분리 여부와 인터셉터/Authenticator 스킵 로직을 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# AuthService 제공자/클라이언트 배선 확인
rg -nP --type=kt -C3 'provideAuthService' app
rg -nP --type=kt -C3 'Retrofit\s*\.create\(\s*AuthService::class\s*\)' app
# AuthInterceptor가 리프레시/인증 경로를 스킵하는지 확인
rg -nP --type=kt -C3 'class\s+AuthInterceptor' app
rg -nP --type=kt -C3 'fun\s+intercept' app
rg -nP --type=kt -C3 'encodedPath.*refresh' app
# 별도 no-auth/refresh 클라이언트 존재 여부
rg -nP --type=kt -C2 '@Named.*(noAuth|unauth|refresh)' appLength of output: 161
ripgrep에서 --type=kt를 인식하지 못하는 것 같습니다. 글로브 필터로 .kt 파일을 지정해 다시 실행해 보세요.
🏁 Script executed:
#!/bin/bash
# AuthService 제공자/클라이언트 배선 확인
rg -g '*.kt' -n -C3 'provideAuthService' app
rg -g '*.kt' -n -C3 'Retrofit\s*\.create\(\s*AuthService::class\s*\)' app
# AuthInterceptor가 리프레시/인증 경로를 스킵하는지 확인
rg -g '*.kt' -n -C3 'class\s+AuthInterceptor\b' app
rg -g '*.kt' -n -C3 'fun\s+intercept\b' app
rg -g '*.kt' -n -C3 'encodedPath.*refresh' app
# 별도 no-auth/refresh 클라이언트 존재 여부
rg -g '*.kt' -n -C2 '@Named.*(noAuth|unauth|refresh)' appLength of output: 2204
리프레시 전용 Retrofit 클라이언트 분리 필요
현재 authService.get().refreshToken(...)가 동일한 OkHttp 클라이언트(인터셉터/Authenticator 포함)를 사용해 호출되어 401 응답 시 무한 재귀 위험이 있습니다.
리프레시 전용 Retrofit 클라이언트를 @Named 또는 @Qualifier로 분리하거나, AuthInterceptor에서 refresh 경로를 명시적으로 스킵하도록 구현하세요.
🤖 Prompt for AI Agents
In app/src/main/java/com/konkuk/medicarecall/data/network/AuthAuthenticator.kt
around lines 47-49, the code calls authService.get().refreshToken(...) using the
same OkHttp client (with interceptors/Authenticator), which risks infinite
recursion on 401 during token refresh; create and inject a dedicated
Retrofit/OkHttp client for refresh calls (annotated with @Named or a custom
@Qualifier) that has no Authenticator or request interceptor that triggers token
refresh, or modify the interceptor to explicitly skip the refresh endpoint
(e.g., check request.url.path and return without adding auth/refresh logic);
update DI to provide both the regular authService and a refresh-only authService
and use the refresh-only instance inside runBlocking so refreshToken requests do
not pass through the Authenticator/interceptors.
|
|
||
| tasks.withType<io.gitlab.arturbosch.detekt.Detekt>().configureEach { | ||
| jvmTarget = "11" | ||
| } |
There was a problem hiding this comment.
Detekt 실행 시 빌드 실패가 발생해서 추가한 코드입니다!
| fun provideAuthAuthenticator( | ||
| dataStoreRepository: DataStoreRepository, | ||
| tokenRefreshService: dagger.Lazy<TokenRefreshService>, | ||
| authService: dagger.Lazy<AuthService> | ||
| ): AuthAuthenticator { | ||
| return AuthAuthenticator(dataStoreRepository, tokenRefreshService) | ||
| return AuthAuthenticator(dataStoreRepository, authService) |
There was a problem hiding this comment.
여기 코드에서 순환 의존성 때문에 Lazy 로 선언하신 것 같습니다.
- AuthService → Retrofit → OkHttpClient → AuthAuthenticator → AuthService
현재 위와 같이 순환 의존성이 있어서, 맨 뒤 AuthService 를 Lazy 로 선언해서 빌드 시에 순환 참조가 되지 않도록 하는 셈인데,
위와 같은 상황을 방지하게 위해 지연 생성을 하기 보단, tokenRepository를 사용해서 하면 좋을 것 같습니다!
현재 PR 에 대한 내용은 아니니 추후 참고용으로 달아놓을게요!
There was a problem hiding this comment.
피드백 감사합니다! 말씀 주신 부분 추후 리팩토링 시 참고하면 좋을 것 같아요
librawish808
changed the title
|
LGTM |
🔗 관련 이슈
📙 작업 설명
API 서비스 패키지를 엔드포인트 prefix 기준으로 재구성했습니다.
인증 관련 API 통합
ApiModule 수정
UserRepositoryImpl 수정
AuthAuthenticator
build.gradle.kts
💬 추가 설명 or 리뷰 포인트 (선택)
HomeService에서 prefix가 elders와 care-call로 같이 있는데 분리하는게 좋을까요?
HomeService.kt는 현재 elders패키지에 있습니다
작업 중 AuthAuthenticator.kt에서 토큰 갱신 로직에 잠재적인 무한 루프 위험이 있는 것을 발견하여, 이를 해결하기 위한 리팩토링을 포함했습니다.
이 부분을 어떻게 고치면 좋을지 의견이 궁금합니다!
Summary by CodeRabbit