[NDGL-17] 네트워크 구조 설정 #10
Conversation
WalkthroughFirebase(Google Services, Crashlytics) 플러그인 및 BuildConfig 활성화, Timber 로깅 초기화, 토큰 기반 인증(Interceptor, Authenticator, TokenManager)과 에러 디코딩(CallAdapter), DataStore 기반 로컬 인증 저장소 및 관련 DI/모델이 추가/연결되었습니다. CI에 google‑services.json 설치 단계가 추가되었습니다. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Interceptor as NDGLInterceptor
participant OkHttp as OkHttpClient
participant Server as API Server
participant Authenticator as NDGLAuthenticator
participant TokenMgr as TokenManager
participant LocalStore as LocalAuthDataSource
Client->>Interceptor: HTTP Request
Interceptor->>TokenMgr: getAccessToken()
TokenMgr->>LocalStore: getAccessToken()
LocalStore-->>TokenMgr: token
TokenMgr-->>Interceptor: token
Interceptor->>OkHttp: Request + Authorization Header
OkHttp->>Server: HTTP Request
Server-->>OkHttp: 401 Unauthorized
OkHttp->>Authenticator: authenticate()
Authenticator->>TokenMgr: refreshToken()
TokenMgr->>Server: login(uuid)
Server-->>TokenMgr: AuthResponse
TokenMgr->>LocalStore: setAccessToken(newToken)
LocalStore-->>TokenMgr: ✓
TokenMgr-->>Authenticator: newAccessToken
Authenticator->>OkHttp: Retry Request + New Authorization
OkHttp->>Server: HTTP Request (Retry)
Server-->>OkHttp: 200 OK
OkHttp-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 7
🤖 Fix all issues with AI agents
In `@data/core/build.gradle.kts`:
- Around line 12-22: defaultConfig currently reads local.properties unguarded
and casts localProperties["NDGL_BASE_URL"] to String causing a crash when the
file or key is missing; modify the block around localProperties and the
buildConfigField call to (1) check rootProject.file("local.properties").exists()
before loading, (2) use localProperties.getProperty("NDGL_BASE_URL") or
localProperties["NDGL_BASE_URL"] as? String and fall back to
System.getenv("NDGL_BASE_URL") or a sensible default, and (3) if neither exists
throw a clear GradleException with a helpful message; update the
buildConfigField("String", "NDGL_BASE_URL", ...) to use the safe/validated
value.
In
`@data/core/src/main/java/com/yapp/data/core/adapter/NDGLCallAdapterFactory.kt`:
- Around line 27-30: The code in NDGLCallAdapterFactory performs a raw cast
(type as ParameterizedType) which will throw ClassCastException for raw Call
types; before casting, ensure you check that 'type' is an instance of
ParameterizedType and return null if not, then safely call
getParameterUpperBound(0, type as ParameterizedType) to obtain 'wrapperType' and
continue validating that getRawType(wrapperType) == BaseResponse::class.java;
update the logic around getRawType(type), the ParameterizedType check, and the
wrapperType extraction to avoid the unsafe cast.
- Around line 85-86: execute() currently throws NotImplementedError and must be
implemented to mirror enqueue()'s synchronous behavior: delegate the call via
the same internal delegate used in enqueue()/NDGLCall, obtain the retrofit
Response, if response.isSuccessful return it, otherwise read
response.errorBody(), parse it to ErrorResponse using the same json parser, log
with Timber, and throw HttpResponseException populated with the parsed
ErrorResponse and response details; use the same helper types (delegate, json,
ErrorResponse, HttpResponseException, Timber) and error-parsing logic as
enqueue() so the sync path matches the async path.
In
`@data/core/src/main/java/com/yapp/data/core/authenticator/NDGLAuthenticator.kt`:
- Around line 43-56: The token-refresh code in NDGLAuthenticator uses
localAuthDataSource.getUuid() and the subsequent setAccessToken/setUuid calls
outside the mutex and doesn't handle exceptions from ndglApi.get().login(...) or
authResponse.getData(), causing race conditions and crashes; fix by moving the
getUuid() call inside the same mutex.withLock block that performs
ndglApi.get().login(LoginRequest(uuid)) and the localAuthDataSource.apply {
setAccessToken(...); setUuid(...) } updates so the UUID read and token write are
atomic, and wrap the network call plus getData() in a try/catch to handle and
log exceptions (return a controlled failure or rethrow a specific exception)
rather than letting exceptions crash the app.
In `@data/core/src/main/java/com/yapp/data/core/di/NetworkModule.kt`:
- Around line 39-43: The debug logging currently logs headers at BODY level
exposing the Authorization token; update the NetworkModule.kt block where
HttpLoggingInterceptor is created (inside the BuildConfig.DEBUG branch) to call
loggingInterceptor.redactHeader("Authorization") before
builder.addInterceptor(loggingInterceptor) so the Authorization header is masked
in logs; ensure you call redactHeader on the same HttpLoggingInterceptor
instance used by builder.addInterceptor.
In `@data/core/src/main/java/com/yapp/data/core/interceptor/NDGLInterceptor.kt`:
- Around line 17-21: The code currently in NDGLInterceptor calls runBlocking {
localAuthDataSource.getAccessToken() } inline and unconditionally adds
"Authorization: Bearer " even when token is empty; change the flow to first
retrieve the token into a local val via runBlocking {
localAuthDataSource.getAccessToken() }, check token.isNotBlank(), and only then
call requestBuilder.header("Authorization", "Bearer $token") (use header()
instead of addHeader() to guarantee a single Authorization header) when
isAccessTokenUsed(originRequest) is true.
In `@data/core/src/main/java/com/yapp/data/core/model/BaseResponse.kt`:
- Around line 12-14: The current BaseResponse.getData extension uses an unsafe
cast ("Unit as T") that can throw ClassCastException; replace this by returning
the actual nullable data instead of forcing a cast—change BaseResponse.getData
to return T? and simply return the data field (or, alternatively, throw a clear
exception when data is null if that semantics is required). Update any call
sites that assume non-null to handle the nullable return or use an explicit
non-null assertion where appropriate; reference: BaseResponse.getData and the
data property.
🧹 Nitpick comments (5)
data/core/src/main/java/com/yapp/data/core/local/util/DataStoreUtil.kt (1)
9-16: IOException 발생 시 로깅 추가 권장I/O 오류 시 빈 Preferences를 반환하는 것은 적절한 폴백이지만, 예외를 완전히 무시하면 디버깅이 어려워질 수 있습니다. Timber를 활용해 예외를 로깅하는 것을 권장합니다.
♻️ 로깅 추가 제안
+import timber.log.Timber + internal fun Flow<Preferences>.handleException(): Flow<Preferences> = this.catch { exception -> if (exception is IOException) { + Timber.w(exception, "DataStore IOException occurred, returning empty preferences") emit(emptyPreferences()) } else { throw exception } }data/core/build.gradle.kts (1)
2-2: 불필요한 import
kotlin.apply는 Kotlin 표준 라이브러리에서 기본적으로 사용 가능하므로 명시적 import가 불필요합니다.♻️ 수정 제안
import java.util.Properties -import kotlin.applydata/core/src/main/java/com/yapp/data/core/interceptor/NDGLInterceptor.kt (1)
17-21: 요청마다 DataStore I/O를 동기적으로 수행하는 구조는 병목이 될 수 있습니다.
runBlocking으로 매 요청마다 토큰을 읽으면 지연이 누적될 수 있으니, 메모리 캐시(StateFlow)나 토큰 공급자 주입으로 접근을 최소화하는 방안을 검토해 주세요.data/core/src/main/java/com/yapp/data/core/local/datasource/LocalAuthDataSource.kt (1)
18-49: 토큰을 평문 DataStore에 저장하면 보안 리스크가 큽니다.
Line 18-49는 Preferences DataStore에 그대로 저장합니다. 보안 요구사항이 있다면 EncryptedDataStore/Jetpack Security로 보호하는 방안을 검토해주세요.data/core/src/main/java/com/yapp/data/core/authenticator/NDGLAuthenticator.kt (1)
23-64: 다수의runBlocking호출을 하나로 통합하는 것을 권장합니다.현재 4개의 개별
runBlocking호출이 있어 코드 가독성과 효율성이 떨어집니다. 하나의runBlocking블록으로 통합하면 더 깔끔한 코드가 됩니다.
| defaultConfig { | ||
| val localProperties = Properties().apply { | ||
| load(rootProject.file("local.properties").bufferedReader()) | ||
| } | ||
|
|
||
| buildConfigField( | ||
| "String", | ||
| "NDGL_BASE_URL", | ||
| localProperties["NDGL_BASE_URL"] as String, | ||
| ) | ||
| } |
There was a problem hiding this comment.
파이프라인 실패: local.properties에서 NDGL_BASE_URL이 없을 때 빌드 실패
CI 환경이나 local.properties 파일에 NDGL_BASE_URL 키가 없는 경우 null cannot be cast to non-null type kotlin.String 오류가 발생합니다. 파일 존재 여부와 프로퍼티 null 체크가 필요합니다.
🐛 null 안전 처리를 위한 수정 제안
defaultConfig {
- val localProperties = Properties().apply {
- load(rootProject.file("local.properties").bufferedReader())
+ val localPropertiesFile = rootProject.file("local.properties")
+ val localProperties = Properties().apply {
+ if (localPropertiesFile.exists()) {
+ load(localPropertiesFile.bufferedReader())
+ }
}
+ val baseUrl = localProperties["NDGL_BASE_URL"] as? String ?: "\"\""
buildConfigField(
"String",
"NDGL_BASE_URL",
- localProperties["NDGL_BASE_URL"] as String,
+ baseUrl,
)
}추가로 CI 환경에서는 GitHub Secrets를 통해 환경 변수를 주입하는 방식을 고려해 보세요.
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 20-20: Command './gradlew ktlintCheck detekt' failed. Build failed: null cannot be cast to non-null type kotlin.String (data/core/build.gradle.kts:20).
🤖 Prompt for AI Agents
In `@data/core/build.gradle.kts` around lines 12 - 22, defaultConfig currently
reads local.properties unguarded and casts localProperties["NDGL_BASE_URL"] to
String causing a crash when the file or key is missing; modify the block around
localProperties and the buildConfigField call to (1) check
rootProject.file("local.properties").exists() before loading, (2) use
localProperties.getProperty("NDGL_BASE_URL") or localProperties["NDGL_BASE_URL"]
as? String and fall back to System.getenv("NDGL_BASE_URL") or a sensible
default, and (3) if neither exists throw a clear GradleException with a helpful
message; update the buildConfigField("String", "NDGL_BASE_URL", ...) to use the
safe/validated value.
| if (getRawType(type) != Call::class.java) return null | ||
|
|
||
| val wrapperType = getParameterUpperBound(0, type as ParameterizedType) | ||
| if (getRawType(wrapperType) != BaseResponse::class.java) return null |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n data/core/src/main/java/com/yapp/data/core/adapter/NDGLCallAdapterFactory.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 4085
raw Call 사용 시 ClassCastException 위험이 있습니다.
라인 27-30에서 type as ParameterizedType는 타입 매개변수 없이 raw Call을 선언했을 때 크래시를 유발합니다. 첫 번째 체크는 raw 타입이 Call인지만 확인하고, ParameterizedType 여부는 검증하지 않기 때문입니다. 타입 체크 후 반환하도록 보완하세요.
🛠️ 수정 제안
- if (getRawType(type) != Call::class.java) return null
-
- val wrapperType = getParameterUpperBound(0, type as ParameterizedType)
+ if (getRawType(type) != Call::class.java) return null
+ if (type !is ParameterizedType) return null
+
+ val wrapperType = getParameterUpperBound(0, type)📝 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.
| if (getRawType(type) != Call::class.java) return null | |
| val wrapperType = getParameterUpperBound(0, type as ParameterizedType) | |
| if (getRawType(wrapperType) != BaseResponse::class.java) return null | |
| if (getRawType(type) != Call::class.java) return null | |
| if (type !is ParameterizedType) return null | |
| val wrapperType = getParameterUpperBound(0, type) | |
| if (getRawType(wrapperType) != BaseResponse::class.java) return null |
🤖 Prompt for AI Agents
In `@data/core/src/main/java/com/yapp/data/core/adapter/NDGLCallAdapterFactory.kt`
around lines 27 - 30, The code in NDGLCallAdapterFactory performs a raw cast
(type as ParameterizedType) which will throw ClassCastException for raw Call
types; before casting, ensure you check that 'type' is an instance of
ParameterizedType and return null if not, then safely call
getParameterUpperBound(0, type as ParameterizedType) to obtain 'wrapperType' and
continue validating that getRawType(wrapperType) == BaseResponse::class.java;
update the logic around getRawType(type), the ParameterizedType check, and the
wrapperType extraction to avoid the unsafe cast.
| override fun execute(): Response<T> = | ||
| throw NotImplementedError("NDGLCall doesn't support execute()") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "NDGLCallAdapterFactory.kt" | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 157
🏁 Script executed:
cat -n ./data/core/src/main/java/com/yapp/data/core/adapter/NDGLCallAdapterFactory.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 4085
execute() 메서드를 동기 경로에 맞게 구현하세요.
현재 구현에서 execute()는 NotImplementedError를 던져 런타임 크래시를 발생시킵니다. 비동기 경로인 enqueue()와 동일한 에러 처리 로직으로 구현해야 합니다:
- 성공 응답은 그대로 반환
- 실패 응답은 errorBody를
ErrorResponse로 파싱하여HttpResponseException발생
모든 필요한 타입(delegate, json, ErrorResponse, HttpResponseException, Timber)과 로직이 이미 enqueue() 구현에 존재하므로, 그 패턴을 동기 경로에도 적용하면 됩니다.
🛠️ 수정 제안
- override fun execute(): Response<T> =
- throw NotImplementedError("NDGLCall doesn't support execute()")
+ override fun execute(): Response<T> {
+ val response = delegate.execute()
+ val body = response.body()
+
+ if (response.isSuccessful && body != null) return response
+
+ val errorBody = response.errorBody()?.string() ?: ""
+ val errorResponse = try {
+ json.decodeFromString<ErrorResponse>(errorBody)
+ } catch (e: Exception) {
+ Timber.e(e, "Failed to parse error response")
+ null
+ }
+
+ throw HttpResponseException(
+ code = errorResponse?.code ?: response.code().toString(),
+ errorMessage = errorResponse?.message ?: errorBody,
+ fieldErrors = errorResponse?.errors,
+ )
+ }📝 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.
| override fun execute(): Response<T> = | |
| throw NotImplementedError("NDGLCall doesn't support execute()") | |
| override fun execute(): Response<T> { | |
| val response = delegate.execute() | |
| val body = response.body() | |
| if (response.isSuccessful && body != null) return response | |
| val errorBody = response.errorBody()?.string() ?: "" | |
| val errorResponse = try { | |
| json.decodeFromString<ErrorResponse>(errorBody) | |
| } catch (e: Exception) { | |
| Timber.e(e, "Failed to parse error response") | |
| null | |
| } | |
| throw HttpResponseException( | |
| code = errorResponse?.code ?: response.code().toString(), | |
| errorMessage = errorResponse?.message ?: errorBody, | |
| fieldErrors = errorResponse?.errors, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In `@data/core/src/main/java/com/yapp/data/core/adapter/NDGLCallAdapterFactory.kt`
around lines 85 - 86, execute() currently throws NotImplementedError and must be
implemented to mirror enqueue()'s synchronous behavior: delegate the call via
the same internal delegate used in enqueue()/NDGLCall, obtain the retrofit
Response, if response.isSuccessful return it, otherwise read
response.errorBody(), parse it to ErrorResponse using the same json parser, log
with Timber, and throw HttpResponseException populated with the parsed
ErrorResponse and response details; use the same helper types (delegate, json,
ErrorResponse, HttpResponseException, Timber) and error-parsing logic as
enqueue() so the sync path matches the async path.
There was a problem hiding this comment.
@mj010504, 동기 호출을 의도적으로 방지하는 설계 방식을 이해했습니다. Android에서 메인 스레드 차단을 막기 위한 좋은 접근입니다.
선택적으로, 에러 메시지를 더 명확하게 개선할 수 있습니다:
override fun execute(): Response<T> =
throw NotImplementedError("Synchronous calls are not supported. Use enqueue() instead.")이렇게 하면 개발자가 실수로 execute()를 호출했을 때 의도를 더 명확하게 전달할 수 있습니다.
✏️ Learnings added
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 10
File: data/core/src/main/java/com/yapp/data/core/adapter/NDGLCallAdapterFactory.kt:85-86
Timestamp: 2026-01-25T08:13:20.069Z
Learning: In the NDGL Android project, the NDGLCallAdapterFactory intentionally prevents synchronous network calls by throwing NotImplementedError in the execute() method to enforce async-only API usage via enqueue().
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
data/core/src/main/java/com/yapp/data/core/authenticator/NDGLAuthenticator.kt
Outdated
Show resolved
Hide resolved
| if (BuildConfig.DEBUG) { | ||
| val loggingInterceptor = HttpLoggingInterceptor() | ||
| loggingInterceptor.level = HttpLoggingInterceptor.Level.BODY | ||
| builder.addInterceptor(loggingInterceptor) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "NetworkModule.kt" -type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 143
🏁 Script executed:
rg "HttpLoggingInterceptor" --type kt -B 2 -A 5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 105
🏁 Script executed:
rg "redactHeader" --type ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 105
🏁 Script executed:
cat -n ./data/core/src/main/java/com/yapp/data/core/di/NetworkModule.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2413
🏁 Script executed:
rg "HttpLoggingInterceptor" -B 3 -A 8Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2315
🏁 Script executed:
rg "redactHeader" -iRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
find . -name "build.gradle*" -o -name "pom.xml" | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 494
🏁 Script executed:
rg "okhttp3|logging" -i | grep -i "version\|dependency\|implementation\|okhttp"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1527
🌐 Web query:
HttpLoggingInterceptor redactHeader method OkHttp documentation
💡 Result:
HttpLoggingInterceptor.redactHeader(name) tells OkHttp’s logging interceptor to mask the value of any header with that (case-insensitive) name in its logs (useful for headers like Authorization / Cookie). The method exists in both OkHttp 3.x and 5.x docs. [1][2]
Example:
val logging = HttpLoggingInterceptor().apply {
level = HttpLoggingInterceptor.Level.HEADERS
redactHeader("Authorization")
redactHeader("Cookie")
}Docs: OkHttp 5.x HttpLoggingInterceptor (includes redactHeader). [1] OkHttp 3.x HttpLoggingInterceptor (includes redactHeader(String name)). [2]
Sources
[1] https://square.github.io/okhttp/5.x/logging-interceptor/okhttp3.logging/-http-logging-interceptor/index.html
[2] https://square.github.io/okhttp/3.x/logging-interceptor/okhttp3/logging/HttpLoggingInterceptor.html
🏁 Script executed:
cat gradle/libs.versions.toml | grep -A 2 "okhttp"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 352
디버그 로깅에서 Authorization 헤더를 마스킹하세요.
BODY 로깅 레벨은 모든 HTTP 헤더를 로그에 남기므로 Authorization 토큰이 노출됩니다. HttpLoggingInterceptor의 redactHeader() 메서드로 민감한 헤더를 마스킹해야 합니다.
수정 제안
if (BuildConfig.DEBUG) {
- val loggingInterceptor = HttpLoggingInterceptor()
- loggingInterceptor.level = HttpLoggingInterceptor.Level.BODY
+ val loggingInterceptor = HttpLoggingInterceptor().apply {
+ level = HttpLoggingInterceptor.Level.BODY
+ redactHeader("Authorization")
+ }
builder.addInterceptor(loggingInterceptor)
}📝 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.
| if (BuildConfig.DEBUG) { | |
| val loggingInterceptor = HttpLoggingInterceptor() | |
| loggingInterceptor.level = HttpLoggingInterceptor.Level.BODY | |
| builder.addInterceptor(loggingInterceptor) | |
| } | |
| if (BuildConfig.DEBUG) { | |
| val loggingInterceptor = HttpLoggingInterceptor().apply { | |
| level = HttpLoggingInterceptor.Level.BODY | |
| redactHeader("Authorization") | |
| } | |
| builder.addInterceptor(loggingInterceptor) | |
| } |
🤖 Prompt for AI Agents
In `@data/core/src/main/java/com/yapp/data/core/di/NetworkModule.kt` around lines
39 - 43, The debug logging currently logs headers at BODY level exposing the
Authorization token; update the NetworkModule.kt block where
HttpLoggingInterceptor is created (inside the BuildConfig.DEBUG branch) to call
loggingInterceptor.redactHeader("Authorization") before
builder.addInterceptor(loggingInterceptor) so the Authorization header is masked
in logs; ensure you call redactHeader on the same HttpLoggingInterceptor
instance used by builder.addInterceptor.
| if (isAccessTokenUsed(originRequest)) { | ||
| requestBuilder.addHeader( | ||
| "Authorization", | ||
| "Bearer ${runBlocking { localAuthDataSource.getAccessToken() }}", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "NDGLInterceptor.kt" -type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 154
🏁 Script executed:
cd . && find . -name "*.kt" | grep -i interceptor | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 154
🏁 Script executed:
find . -path "*/data/core/src/main/java/com/yapp/data/core/interceptor/*" -type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 154
🏁 Script executed:
cat -n ./data/core/src/main/java/com/yapp/data/core/interceptor/NDGLInterceptor.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1368
🏁 Script executed:
rg "Authorization" --type kotlin -B 2 -A 2Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1810
🏁 Script executed:
cat -n ./data/core/src/main/java/com/yapp/data/core/local/datasource/LocalAuthDataSource.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2159
🏁 Script executed:
web_search
OkHttp3 addHeader vs header method differenceRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 175
🌐 Web query:
OkHttp3 addHeader vs header method difference
💡 Result:
In OkHttp3 Request.Builder (and similarly Response.Builder):
header(name, value): sets/replaces the header. If headers with the samenamealready exist, they’re all removed and replaced with this single value. [1][2]addHeader(name, value): adds another header entry with the samenamewithout removing existing ones (useful for multi-valued headers likeCookie/Set-Cookie). [1][2][5]
Notes:
- OkHttp may override certain headers (e.g.,
Content-Length,Content-Encoding) based on the request body, even if you set/add them. [1][2]
Example
new Request.Builder()
.header("Authorization", "Bearer A") // ensures only one Authorization
.addHeader("Cookie", "a=1") // keep/add multiple Cookie lines if needed
.addHeader("Cookie", "b=2");Sources: [1] [2] [5]
빈 토큰 전송을 방지하고 안전한 헤더 설정을 사용하세요.
getAccessToken()이 토큰이 없으면 빈 문자열("")을 반환하므로, 현재 코드는 Bearer 를 전송합니다. 토큰을 먼저 가져온 후 isNotBlank() 확인을 추가하고, addHeader() 대신 header()를 사용하여 단일 Authorization 헤더를 보장하는 것이 안전합니다.
제안된 수정사항
if (isAccessTokenUsed(originRequest)) {
+ val token = runBlocking { localAuthDataSource.getAccessToken() }
+ if (token.isNotBlank()) {
- requestBuilder.addHeader(
+ requestBuilder.header(
"Authorization",
- "Bearer ${runBlocking { localAuthDataSource.getAccessToken() }}",
+ "Bearer $token",
)
+ }
}📝 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.
| if (isAccessTokenUsed(originRequest)) { | |
| requestBuilder.addHeader( | |
| "Authorization", | |
| "Bearer ${runBlocking { localAuthDataSource.getAccessToken() }}", | |
| ) | |
| if (isAccessTokenUsed(originRequest)) { | |
| val token = runBlocking { localAuthDataSource.getAccessToken() } | |
| if (token.isNotBlank()) { | |
| requestBuilder.header( | |
| "Authorization", | |
| "Bearer $token", | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@data/core/src/main/java/com/yapp/data/core/interceptor/NDGLInterceptor.kt`
around lines 17 - 21, The code currently in NDGLInterceptor calls runBlocking {
localAuthDataSource.getAccessToken() } inline and unconditionally adds
"Authorization: Bearer " even when token is empty; change the flow to first
retrieve the token into a local val via runBlocking {
localAuthDataSource.getAccessToken() }, check token.isNotBlank(), and only then
call requestBuilder.header("Authorization", "Bearer $token") (use header()
instead of addHeader() to guarantee a single Authorization header) when
isAccessTokenUsed(originRequest) is true.
mj010504
force-pushed
the
feature/NDGL-17
branch
from
d0e91c2 to
2a57ad0
Compare
jihee-dev
left a comment
jihee-dev
left a comment
There was a problem hiding this comment.
코드 계속 보는 중인데 답변이 자꾸 늦어지는 것 같아서 보이는 대로 틈틈히 코멘트 남기겠습니다!
mj010504
force-pushed
the
feature/NDGL-17
branch
from
d4369cd to
1446ac7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@data/core/build.gradle.kts`:
- Line 10: The namespace value "com.yapp.ngdl.data.core" appears to have a typo;
update the namespace assignment to use "ndgl" (e.g., "com.yapp.ndgl.data.core")
to match other identifiers like the plugin ID `ndgl.data` and constants such as
`NDGL_BASE_URL`; locate the namespace assignment (namespace =
"com.yapp.ngdl.data.core") in build.gradle.kts and change it to the corrected
string, then sync the build/IDE to ensure no other references need adjusting.
🧹 Nitpick comments (1)
data/core/build.gradle.kts (1)
2-2: 불필요한 import 제거
kotlin.apply는 Kotlin 표준 라이브러리의 스코프 함수로 명시적 import가 필요하지 않습니다.♻️ 수정 제안
import java.util.Properties -import kotlin.apply
mj010504
force-pushed
the
feature/NDGL-17
branch
from
1446ac7 to
732a52c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@gradle/libs.versions.toml`:
- Around line 138-139: The repo is missing the google-services.json required by
the com.google.gms.google-services plugin referenced by google-services in
gradle/libs.versions.toml; add a valid google-services.json (from your Firebase
project) to the app module's root (app/google-services.json) so the plugin
"com.google.gms.google-services" can find it during build, and ensure the file
is the exact JSON downloaded from Firebase Console and committed to the
repository (or add instructions to securely provide it if you cannot commit
secrets).
🧹 Nitpick comments (2)
data/core/src/main/java/com/yapp/ngdl/data/core/adapter/NDGLCallAdapterFactory.kt (1)
43-46: Json 인스턴스를 매번 생성하는 것은 비효율적입니다.
NDGLCall인스턴스가 생성될 때마다 새로운Json인스턴스가 생성됩니다. Json 객체는 생성 비용이 있으므로, companion object나 생성자 파라미터로 공유하는 것이 좋습니다.수정 제안
private class NDGLCall<T : Any>( private val delegate: Call<T>, + private val json: Json, ) : Call<T> { - private val json = Json { ignoreUnknownKeys = true }그리고
NDGLCallAdapter에서 Json 인스턴스를 전달하도록 수정:private class NDGLCallAdapter( private val resultType: Type, private val json: Json, ) : CallAdapter<Any, Call<Any>> { override fun responseType(): Type = resultType override fun adapt(call: Call<Any>): Call<Any> = NDGLCall(call, json) }data/core/src/main/java/com/yapp/ngdl/data/core/authenticator/NDGLAuthenticator.kt (1)
47-50:isNullOrEmpty()대신isEmpty()사용을 권장합니다.
LocalAuthDataSource.getUuid()는String을 반환하므로 null이 될 수 없습니다.isNullOrEmpty()대신isEmpty()를 사용하는 것이 더 명확합니다.수정 제안
val uuid = localAuthDataSource.getUuid() - if (uuid.isNullOrEmpty()) { + if (uuid.isEmpty()) { return@withLock null }
- null 반환 시 Unit 캐스팅 대신 명확한 에러 메시지 출력 - code와 message 정보 포함하여 디버깅 용이성 향상
- uuid null/empty 체크 추가 - mutex 내에서 토큰 갱신 및 저장을 원자적으로 수행 - 예외 발생 시 Timber 로깅 추가 - 에러 발생 시 null 반환으로 재시도 중단
mj010504
force-pushed
the
feature/NDGL-17
branch
from
732a52c to
e0edb7c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@gradle/libs.versions.toml`:
- Around line 33-35: Update the DataStore version entry named "datastore" in
gradle/libs.versions.toml from "1.1.1" to a newer stable release; either set it
to "1.1.7" (minimal recommended) or to "1.2.0" (latest) and if you choose 1.2.0
verify the Kotlin Gradle Plugin version is >= 2.0.0 before bumping.
🧹 Nitpick comments (2)
data/core/src/main/java/com/yapp/ndgl/data/core/local/datasource/LocalAuthDataSource.kt (1)
53-56: 보안 고려사항: 토큰 저장 시 암호화 검토현재 Access Token이 평문으로 DataStore에 저장됩니다. 민감한 인증 데이터의 경우
EncryptedSharedPreferences또는 Android Keystore 기반 암호화를 고려해 보세요.프로젝트의 보안 요구사항에 따라 선택적으로 적용할 수 있습니다.
data/core/src/main/java/com/yapp/ndgl/data/core/adapter/NDGLCallAdapterFactory.kt (1)
43-46: Json 인스턴스를 재사용하도록 개선현재 매
NDGLCall인스턴스마다 새로운Json객체가 생성됩니다. companion object로 이동하여 재사용하면 메모리 효율성이 향상됩니다.♻️ 수정 제안
private class NDGLCall<T : Any>( private val delegate: Call<T>, ) : Call<T> { - private val json = Json { ignoreUnknownKeys = true } + // ... existing code ... + + private companion object { + private val json = Json { ignoreUnknownKeys = true } + }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@data/auth/src/main/java/com/yapp/ndgl/data/auth/token/TokenManagerImpl.kt`:
- Around line 33-42: refreshToken() can race and lacks error handling; add a
Mutex (e.g., a private val refreshMutex = Mutex()) and wrap the login flow
inside refreshMutex.withLock { ... } to ensure only one concurrent request from
suspend fun refreshToken(), and use a double-check if needed (re-read current
access token or uuid inside the lock) to avoid duplicate network calls. Surround
the authApi.login(LoginRequest(uuid)).getData() call with a try/catch that
catches network/HTTP exceptions and converts them to a controlled error (e.g.,
throw a TokenRefreshException or return a clear failure value) and log the cause
before rethrowing so the Authenticator can handle it without crashing; keep
calls to getUuid(), setAccessToken(response.accessToken) and
setUuid(response.uuid) inside the locked/try block.
🧹 Nitpick comments (3)
data/auth/src/main/java/com/yapp/ndgl/data/auth/local/util/DataStoreUtil.kt (1)
9-16: IOException 발생 시 로깅 추가를 고려해보세요.
emptyPreferences()로 대체하는 것은 적절한 폴백 전략입니다. 다만 디버깅을 위해 예외 발생 시 Timber로 로깅하는 것을 권장합니다.💡 로깅 추가 제안
internal fun Flow<Preferences>.handleException(): Flow<Preferences> = this.catch { exception -> if (exception is IOException) { + Timber.w(exception, "DataStore IOException occurred, using empty preferences") emit(emptyPreferences()) } else { throw exception } }Timber import 추가 필요:
import timber.log.Timberbuild-logic/src/main/kotlin/NDGLDataPlugin.kt (1)
26-31: 의존성 스코프 검토모든 네트워킹 라이브러리가
implementation스코프로 추가되어 있습니다. 만약 data 모듈의 public API에서 Retrofit 타입이나 OkHttp 타입이 노출된다면api스코프 사용을 고려해보세요.data/auth/src/main/java/com/yapp/ndgl/data/auth/token/TokenManagerImpl.kt (1)
35-35:check()대신 커스텀 예외 사용을 권장합니다.
check()는IllegalStateException을 던지는데, 이는 UUID가 비어있는 상황(사용자 미인증 상태)을 명확히 표현하지 못합니다. 인증 관련 커스텀 예외를 사용하면 상위 레이어에서 더 적절하게 처리할 수 있습니다.
data/auth/src/main/java/com/yapp/ndgl/data/auth/token/TokenManagerImpl.kt
Show resolved
Hide resolved
data/core/src/main/java/com/yapp/ndgl/data/core/authenticator/NDGLAuthenticator.kt
Outdated
Show resolved
Hide resolved
mj010504
force-pushed
the
feature/NDGL-17
branch
from
cbc5db4 to
daaecf1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@data/auth/src/main/java/com/yapp/ndgl/data/auth/local/LocalAuthDataSource.kt`:
- Around line 34-43: setAccessToken and setUuid currently write plaintext into
DataStore via dataStore.edit (using ACCESS_TOKEN_KEY and UUID_KEY); change these
to encrypt values with a key managed by Android Keystore (use an AES-GCM key
material via a secure library such as Google Tink) before writing and decrypt on
read. Implement helper functions encrypt(String):String and
decrypt(String):String that use the Keystore-backed key, call encrypt(...)
inside setAccessToken and setUuid before storing, and update the corresponding
getter(s) that read ACCESS_TOKEN_KEY/UUID_KEY to call decrypt(...) after
reading. Also ensure tokens are scoped to minimal lifetime (store expiry
metadata) and handle decryption errors by clearing invalid entries in the same
LocalAuthDataSource.
data/auth/src/main/java/com/yapp/ndgl/data/auth/local/LocalAuthDataSource.kt
Show resolved
Hide resolved
jihee-dev
left a comment
jihee-dev
left a comment
There was a problem hiding this comment.
고생하셨습니다~!
CodeRabbit 리뷰 반영된 거나 무시해도 될 만한 코멘트는 resolve 해 주시면 좋을 것 같습니다!
개요
변경사항
테스트 체크 리스트
Summary by CodeRabbit
새로운 기능
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.