-
Notifications
You must be signed in to change notification settings - Fork 0
[feat/#24] 게시물 리스트 조회 API 연동 #29
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
WalkthroughAdds DataStore-backed token persistence and an AuthInterceptor; introduces paging for community lists (PagingSource, ViewModel, repository); reorganizes DTO packages; parameterizes Kakao OAuth scheme; updates DI/NetworkModule, build properties, and CI secrets; adjusts UI and navigation to consume paging and new signatures. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as CommunityScreen
participant VM as CommunityViewModel
participant Repo as CommunityRepository
participant Paging as CommunityPagingSource
participant Service as CommunityPostService
participant Interceptor as AuthInterceptor
participant Store as TokenDataStore
UI->>VM: selectTab(tab)
VM->>Repo: request paging Flow for tab
Repo->>Paging: construct PagingSource(type)
Paging->>Service: getCommunity(type, page)
Service->>Interceptor: HTTP request
Interceptor->>Store: accessToken.first()
Store-->>Interceptor: token?
alt token present
Interceptor->>Service: request with Authorization header
end
Service-->>Paging: CommunityListResponseDto
Paging->>Repo: Page<CommunityPost>
Repo-->>VM: Flow<PagingData<CommunityPost>>
VM-->>UI: LazyPagingItems stream
UI->>UI: render posts / handle LoadState
sequenceDiagram
participant Client as OkHttpClient
participant Auth as AuthInterceptor
participant Store as TokenDataStore
participant Net as Server
Client->>Auth: intercept(chain)
Auth->>Store: accessToken.first() (blocking)
Store-->>Auth: token or null
alt token exists
Auth->>Client: add Authorization: Bearer {token}
end
Auth->>Net: proceed(request)
Net-->>Auth: response
Auth-->>Client: return response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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: 9
🧹 Nitpick comments (3)
app/src/main/java/com/hsLink/hslink/presentation/community/component/main/CommunityCardItem.kt (1)
82-127: Consider lighter typography for separator dots.The separator dots (lines 96-98, 112-114) use
title_16Strongwhich appears too heavy for a separator. Consider usingcaption_12Normalor a lighter style to match the surrounding text weight.Apply this diff for more visually balanced separators:
Text( text = "·", color = HsLinkTheme.colors.Grey700, - style = HsLinkTheme.typography.title_16Strong, + style = HsLinkTheme.typography.caption_12Normal, maxLines = 1, overflow = TextOverflow.Ellipsis )(Apply the same change to the second separator at lines 112-114)
app/src/main/java/com/hsLink/hslink/data/dto/response/community/CommunityListResponseDto.kt (1)
18-32: Consider nullable fields for optional backend data.All fields are currently non-nullable, which will cause deserialization to fail if the backend sends null for any field. Consider whether fields like
summaryorauthorStatusshould be nullable with default values if they might be optional in the API response.app/src/main/java/com/hsLink/hslink/data/repositoryimpl/CommunityRepositoryImpl.kt (1)
28-33: Consider making page size configurable.The page size is hardcoded to 20. Consider making this configurable via a constant or configuration class to allow easier tuning without code changes.
Example:
override fun getCommunityPosts(type: String): Flow<PagingData<CommunityPost>> { return Pager( config = PagingConfig( pageSize = COMMUNITY_PAGE_SIZE, prefetchDistance = COMMUNITY_PREFETCH_DISTANCE ), pagingSourceFactory = { CommunityPagingSource(communityPostService, type) } ).flow } companion object { private const val COMMUNITY_PAGE_SIZE = 20 private const val COMMUNITY_PREFETCH_DISTANCE = 20 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
.idea/caches/deviceStreaming.xml(4 hunks)app/build.gradle.kts(2 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/com/hsLink/hslink/data/di/DataStoreModule.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/auth/SocialLoginRequestDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/community/PostRequestDto.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/auth/SocialLoginResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/community/CommunityListResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/community/CommunityPostResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/home/PostResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/mypage/UserProfileResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/search/SearchResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/local/TokenDataStore.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/paging/CommunityPagingSource.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/AuthInterceptor.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasource/CommunityPostDataSource.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasource/PostDataSource.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasource/SearchDataSource.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/CommunityPostDataSourceImpl.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/PostDataSourceImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/SearchDataSourceImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/AuthRepositoryImpl.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/CommunityRepositoryImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/home/PostRepositoryImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/service/commuunity/CommunityPostService.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/service/home/PostService.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/service/login/AuthService.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/service/search/SearchService.kt(1 hunks)app/src/main/java/com/hsLink/hslink/domain/model/community/CommunityPost.kt(1 hunks)app/src/main/java/com/hsLink/hslink/domain/repository/AuthRepository.kt(1 hunks)app/src/main/java/com/hsLink/hslink/domain/repository/community/CommunityRepository.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/component/main/CommunityCardItem.kt(3 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/navigation/main/Communitynavigation.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/screen/main/CommunityScreen.kt(3 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/screen/write/CommunityWritingScreen.kt(7 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/viewmodel/CommunityViewModel.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/hsLink/hslink/presentation/community/screen/main/CommunityScreen.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/community/component/main/CommunityCardItem.kt (1)
CommunityCardItem(40-130)
app/src/main/java/com/hsLink/hslink/presentation/community/screen/write/CommunityWritingScreen.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/community/viewmodel/CommunityViewModel.kt (1)
createPost(36-51)
app/src/main/java/com/hsLink/hslink/presentation/community/navigation/main/Communitynavigation.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/community/screen/main/CommunityScreen.kt (1)
CommunityRoute(58-76)
🪛 GitHub Actions: Android CI/CD
app/build.gradle.kts
[error] 34-34: Gradle build failed due to type cast error: 'null cannot be cast to non-null type kotlin.String' in app/build.gradle.kts:34. Command: './gradlew assembleDebug'.
🔇 Additional comments (30)
.idea/caches/deviceStreaming.xml (1)
756-1289: IDE cache file changes unrelated to PR objectives.This file is IDE-specific device configuration typically auto-managed by Android Studio. The device additions (OPPO, OnePlus, Samsung, vivo, Xiaomi devices) don't relate to the post-list API integration objective and likely represent local development environment changes.
Verify that
.idea/caches/is not in.gitignoreand whether tracking IDE cache files is intentional. Consider committing only application source code changes and excluding IDE-specific files.app/src/main/java/com/hsLink/hslink/data/service/login/AuthService.kt (1)
3-4: LGTM! Clean package reorganization.The imports have been updated to reflect the feature-scoped package structure (
auth), which improves code organization and maintainability. The interface signature remains unchanged, ensuring no behavioral impact.app/src/main/java/com/hsLink/hslink/data/dto/response/search/SearchResponseDto.kt (1)
1-1: Refactoring verified—all imports successfully updated.The package reorganization is complete. Verification confirms that all three DTOs (
MentorListResponseDto,MentorListResultDto,MentorItemDto) are being imported from the newcom.hsLink.hslink.data.dto.response.searchpackage in all consuming files:
SearchService.ktSearchDataSource.ktSearchDataSourceImpl.ktNo stale references to the old package path remain. This is part of a sound feature-based DTO organization strategy (search, auth, community, home, mypage packages).
app/src/main/java/com/hsLink/hslink/data/repositoryimpl/home/PostRepositoryImpl.kt (1)
3-3: LGTM! Package reorganization aligns with feature-scoped structure.The import path update reflects the broader DTO package restructuring to organize response types by feature (home, auth, community, etc.).
app/build.gradle.kts (1)
95-100: Good additions for token persistence and paging.DataStore enables secure token storage, and Paging3 dependencies support the new community post list pagination architecture mentioned in the PR objectives.
app/src/main/java/com/hsLink/hslink/data/dto/response/mypage/UserProfileResponseDto.kt (1)
1-1: LGTM! Package relocation improves organization.Moving to the
mypagesubpackage aligns with the feature-scoped DTO organization across the codebase.app/src/main/AndroidManifest.xml (1)
39-39: Good parameterization, but depends on fixing build.gradle.kts.Externalizing the Kakao app key via manifest placeholder is the correct approach. However, this relies on the
manifestPlaceholders["kakaoAppKey"]assignment inapp/build.gradle.ktsline 34, which currently has a critical null-cast bug that must be fixed first.app/src/main/java/com/hsLink/hslink/data/dto/request/auth/SocialLoginRequestDto.kt (1)
1-1: LGTM! Consistent auth namespace organization.Moving both request and response DTOs to the
authsubpackage creates a cohesive feature boundary.app/src/main/java/com/hsLink/hslink/data/remote/datasource/SearchDataSource.kt (1)
3-4: LGTM! Import updates align with DTO reorganization.The updated import paths correctly reference the relocated DTOs in their new feature-scoped packages (
searchandmypage).app/src/main/java/com/hsLink/hslink/data/dto/response/auth/SocialLoginResponseDto.kt (1)
1-1: LGTM! Auth response DTO properly scoped.Relocating to
response.authcreates a clear auth feature boundary alongside the corresponding request DTO.app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/SearchDataSourceImpl.kt (1)
3-4: LGTM! Import paths correctly updated.The implementation's imports align with the interface changes and the relocated DTO packages.
app/src/main/java/com/hsLink/hslink/presentation/community/screen/write/CommunityWritingScreen.kt (1)
56-63: LGTM! Clean ViewModel integration.The Hilt-based dependency injection pattern is well-implemented, with the ViewModel providing the
createPostfunction to the composable.app/src/main/java/com/hsLink/hslink/domain/repository/community/CommunityRepository.kt (1)
12-12: LGTM! Proper paging integration with domain model.The new
getCommunityPostsmethod correctly uses the domain modelCommunityPostand integrates well with Paging 3 library viaFlow<PagingData<>>. This is the right architectural pattern.app/src/main/java/com/hsLink/hslink/data/dto/response/community/CommunityPostResponseDto.kt (1)
1-18: LGTM! Package reorganization is clean.The DTO has been successfully moved to the community-specific package, improving code organization. The serialization annotations and entity conversion are properly implemented.
app/src/main/java/com/hsLink/hslink/data/di/DataStoreModule.kt (1)
12-21: LGTM! Standard DI module implementation.The module correctly provides TokenDataStore as a singleton with proper context injection. The setup follows Hilt best practices.
app/src/main/java/com/hsLink/hslink/data/repositoryimpl/AuthRepositoryImpl.kt (2)
10-13: LGTM! Token persistence properly integrated.The addition of TokenDataStore to the constructor and its usage for persisting tokens after successful authentication is well-implemented.
24-26: LGTM! Token storage correctly placed in success path.Tokens are saved only after successful login validation, which is the correct placement. The error handling for empty responses remains intact.
app/src/main/java/com/hsLink/hslink/domain/repository/AuthRepository.kt (1)
1-10: LGTM! Import path updated for DTO reorganization.The interface signature remains unchanged; only the import reflects the new package structure.
app/src/main/java/com/hsLink/hslink/data/remote/datasource/CommunityPostDataSource.kt (1)
1-11: LGTM! Import paths updated consistently.The interface remains functionally unchanged; imports now reflect the reorganized DTO package structure.
app/src/main/java/com/hsLink/hslink/domain/model/community/CommunityPost.kt (1)
5-23: LGTM! Clean domain model with proper conversion.The CommunityPost domain model is well-structured, and the extension function provides a clear conversion from DTO to domain entity.
app/src/main/java/com/hsLink/hslink/presentation/community/component/main/CommunityCardItem.kt (1)
41-49: LGTM! Parameters properly added.The new userId and author parameters are correctly integrated into the component signature.
app/src/main/java/com/hsLink/hslink/data/dto/response/community/CommunityListResponseDto.kt (1)
6-16: LGTM: Clean DTO structure for paginated response.The pagination metadata (currentPage, hasNext, totalElements) and post list structure are well-designed for the paging use case.
app/src/main/java/com/hsLink/hslink/data/service/commuunity/CommunityPostService.kt (3)
4-6: LGTM: Package restructuring for better organization.The DTO imports have been properly updated to feature-scoped packages under the community namespace.
13-16: LGTM: Interceptor-based authentication is the correct pattern.The removal of the header parameter indicates authentication is now handled by an interceptor, which is the recommended approach for centralized token management.
18-22: LGTM: Clean paging API design.The query parameters for type filtering and page number are appropriate for the paging use case.
app/src/main/java/com/hsLink/hslink/data/repositoryimpl/CommunityRepositoryImpl.kt (1)
15-17: LGTM: Simplified dependency injection.Injecting the service directly instead of the data source implementation reduces unnecessary abstraction layers and simplifies the architecture.
app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/CommunityPostDataSourceImpl.kt (1)
4-5: LGTM: Package updates align with restructuring.The DTO imports have been properly updated to the new community-scoped packages.
app/src/main/java/com/hsLink/hslink/data/paging/CommunityPagingSource.kt (1)
31-35: LGTM: Standard refresh key implementation.The refresh key calculation follows the recommended pattern for
PagingSource.app/src/main/java/com/hsLink/hslink/presentation/community/navigation/main/Communitynavigation.kt (1)
16-28: LGTM: Improved type safety with Int post ID.Changing
navigateToPostparameter from(String) -> Unitto(Int) -> Unitimproves type safety by using the actual domain type for post IDs. The use of named parameters also enhances code readability.app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt (1)
51-55: Incorrect review comment—converting Int to String for navigation is required architecture.In Jetpack Compose navigation, you can only pass string values to destinations. The conversion of
postIdtoStringon line 54 is not an unnecessary type conversion but rather a requirement of the navigation system. By default all arguments are parsed as strings in route-based navigation, so converting anInttoStringwhen calling a navigation function that acceptsStringparameters is the correct and expected pattern. This reflects sound architecture where the domain layer uses native types while the navigation layer handles string-based routing.Likely an incorrect or invalid review comment.
| return Retrofit.Builder() | ||
| .baseUrl(BuildConfig.BASE_URL) | ||
| .addConverterFactory(converterFactory) | ||
| .client(client) | ||
| .client(okHttpClient) | ||
| .addConverterFactory(Json.asConverterFactory("application/json".toMediaType())) | ||
| .build() |
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.
Restore permissive Json configuration
Json.asConverterFactory uses the strict default settings, so any new/unknown fields from the backend will now trigger SerializationException. This is a regression compared with the typical Json { ignoreUnknownKeys = true } setup and will break the app as soon as the API adds an extra field. Please reinstate the relaxed configuration (and consider preserving coerceInputValues if we had it before) so responses keep deserializing safely.
- .addConverterFactory(Json.asConverterFactory("application/json".toMediaType()))
+ .addConverterFactory(
+ Json {
+ ignoreUnknownKeys = true
+ coerceInputValues = true
+ }.asConverterFactory("application/json".toMediaType())
+ )📝 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.
| return Retrofit.Builder() | |
| .baseUrl(BuildConfig.BASE_URL) | |
| .addConverterFactory(converterFactory) | |
| .client(client) | |
| .client(okHttpClient) | |
| .addConverterFactory(Json.asConverterFactory("application/json".toMediaType())) | |
| .build() | |
| return Retrofit.Builder() | |
| .baseUrl(BuildConfig.BASE_URL) | |
| .client(okHttpClient) | |
| .addConverterFactory( | |
| Json { | |
| ignoreUnknownKeys = true | |
| coerceInputValues = true | |
| }.asConverterFactory("application/json".toMediaType()) | |
| ) | |
| .build() |
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt around lines 44
to 48, the Retrofit converter is created with Json.asConverterFactory using the
default strict Json instance; restore a permissive Json instance by creating
Json { ignoreUnknownKeys = true; coerceInputValues = true } (or at least
ignoreUnknownKeys = true if coerceInputValues was not previously used) and pass
that Json instance to asConverterFactory so unknown backend fields (and coerced
values) won't throw SerializationException.
| class TokenDataStore @Inject constructor(private val context: Context) { | ||
|
|
||
| private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "hsu_connect_tokens") | ||
|
|
||
| private val accessTokenKey = stringPreferencesKey("access_token") | ||
| private val refreshTokenKey = stringPreferencesKey("refresh_token") | ||
|
|
||
| val accessToken: Flow<String?> = context.dataStore.data | ||
| .map { preferences -> | ||
| preferences[accessTokenKey] | ||
| } | ||
|
|
||
| val refreshToken: Flow<String?> = context.dataStore.data | ||
| .map { preferences -> | ||
| preferences[refreshTokenKey] | ||
| } | ||
|
|
||
| suspend fun saveTokens(accessToken: String, refreshToken: String) { | ||
| context.dataStore.edit { | ||
| it[accessTokenKey] = accessToken | ||
| it[refreshTokenKey] = refreshToken | ||
| } | ||
| } | ||
|
|
||
| suspend fun clearTokens() { | ||
| context.dataStore.edit { | ||
| it.clear() | ||
| } | ||
| } |
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
Consider encrypting tokens at rest.
DataStore Preferences stores data unencrypted on disk. Access and refresh tokens are sensitive credentials that, if compromised, could allow unauthorized access to user accounts. While Android's app sandboxing provides some protection, consider using EncryptedSharedPreferences or implementing encryption at the application layer for additional security.
This is particularly important if:
- Tokens have long expiry times
- The app handles sensitive user data
- Compliance requirements mandate encryption at rest
If token encryption is required, would you like me to provide an implementation example using EncryptedSharedPreferences or a custom encryption wrapper?
I'll help you verify this review comment. Let me gather information about DataStore encryption practices and examine the codebase context.
🏁 Script executed:
#!/bin/bash
# Search for build.gradle or gradle dependencies to understand project setup
fd -e gradle -e toml | head -20Length of output: 25
🏁 Script executed:
#!/bin/bash
# Search for token usage patterns in the codebase to understand sensitivity
rg -n "Token|token" --type=kt -C 3 | head -100Length of output: 26
🏁 Script executed:
#!/bin/bash
# Check if EncryptedSharedPreferences is already used or imported anywhere
rg "EncryptedSharedPreferences|encryption|Encrypted" --type=kt -nLength of output: 26
🌐 Web query:
Android DataStore Preferences encryption default behavior
💡 Result:
Short answer: Preferences DataStore is not encrypted by default — data is stored on disk in your app's private storage but DataStore doesn't perform encryption for you. Use Jetpack Security (EncryptedSharedPreferences / EncryptedFile), Android Keystore-based encryption, or encrypt before saving if you need confidentiality. [1][2][3]
Sources:
- Android DataStore docs (Preferences DataStore docs/codelab). [1]
- Android DataStore overview / architecture. [2]
- Community/confirmation (Jetpack DataStore does not offer built-in encryption). [3]
🌐 Web query:
Kotlin DataStore encryption options EncryptedSharedPreferences alternative
💡 Result:
Short answer: EncryptedSharedPreferences has been deprecated, and the recommended alternative is to keep using Jetpack DataStore for storage and add your own encryption layer — typically by protecting an AES key with Android Keystore (or using Google Tink) and encrypting values before writing to DataStore. Other options: use a full-database solution with SQLCipher, or a maintained community library (e.g., SafeBox) that replaces EncryptedSharedPreferences. [1][2][3][4][5]
Practical options with references
- Stop using EncryptedSharedPreferences (deprecated). [1]
- DataStore + app-managed encryption: DataStore itself is not encrypted by default; encrypt/decrypt values in your app and store ciphertext in Preferences or Proto DataStore. Protect keys with Android Keystore (wrap/unwrap) or use an envelope approach. [2][3]
- Use Google Tink (AndroidKeysetManager) to manage keys and perform AEAD encryption before storing in DataStore — Tink is recommended for safe primitives and key management. [4]
- Use SQLCipher if you prefer an encrypted SQLite-backed store (suitable for more complex structured data). (community/common guidance). [3][4]
- Consider maintained community replacements (e.g., SafeBox) if you need a drop‑in EncryptedSharedPreferences alternative — evaluate auditability and maintenance first. [5]
If you want, I can:
- show a minimal Kotlin example of DataStore + Tink or DataStore + Keystore-wrapped AES, or
- recommend dependencies and a migration plan from EncryptedSharedPreferences to encrypted DataStore.
Sources:
[1] EncryptedSharedPreferences API docs (androidx.security:security-crypto) — deprecation notice.
[2] Jetpack DataStore docs / release notes (DataStore is the modern replacement; it is not encrypted by default).
[3] StackOverflow / community guidance: “DataStore does not offer encryption; encrypt in-app or use SQLCipher.”
[4] Tink/key-management docs — recommended library for safe encryption and key management on Android.
[5] SafeBox GitHub (drop‑in replacement examples / benchmarks).
🏁 Script executed:
#!/bin/bash
# Check available dependencies
cat gradle/libs.versions.tomlLength of output: 6336
🏁 Script executed:
#!/bin/bash
# Search for token usage and authentication patterns (retry without type filter)
rg -n "Token|token" -C 3 | head -150Length of output: 15601
🏁 Script executed:
#!/bin/bash
# Check for security, encryption, or crypto dependencies
rg -n "security|crypto|tink|keystore|cipher" -i -C 2Length of output: 2325
🏁 Script executed:
#!/bin/bash
# Look for any authentication or compliance-related context
rg -n "auth|AUTH|compliance|COMPLIANCE|sensitive|SENSITIVE" -C 2 | head -80Length of output: 4999
Encrypt tokens before storage in DataStore.
The security concern is valid—tokens are sensitive credentials that grant account access. DataStore Preferences is not encrypted by default, and your app handles authentication tokens for user accounts.
However, EncryptedSharedPreferences is deprecated. The recommended approach is to add an encryption layer within DataStore itself. Your project already includes androidx-security:security-crypto-ktx (v1.1.0-alpha06); use this to:
- Encrypt token values before writing to DataStore and decrypt on read, OR
- Consider DataStore + app-managed encryption using Android Keystore or Google Tink for key management
Implement encryption at the application layer in TokenDataStore to ensure tokens are protected at rest.
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/data/local/TokenDataStore.kt around lines
13-41, the TokenDataStore currently writes plain tokens to DataStore; update it
to encrypt tokens before writing and decrypt on read. Create or obtain a master
key via androidx.security.crypto (MasterKey/AndroidKeyStore), then use a
symmetric cipher (e.g., AES/GCM) or the security-crypto helper to encrypt the
access and refresh token strings before saving them into DataStore and decrypt
them when mapping the flows; update saveTokens to encrypt both tokens before
edit, update accessToken and refreshToken flows to map and return decrypted
values (handle decryption failures by returning null and logging), and ensure
all crypto operations run off the main thread (use withContext(Dispatchers.IO)
or make encryption functions suspend).
| override suspend fun load(params: LoadParams<Int>): LoadResult<Int, CommunityPost> { | ||
| val page = params.key ?: 0 | ||
|
|
||
| return try { | ||
| val response = communityPostService.getCommunity(type = type, page = page) | ||
| val posts = response.result.posts.map { it.toEntity() } | ||
|
|
||
| LoadResult.Page( | ||
| data = posts, | ||
| prevKey = if (page == 0) null else page - 1, | ||
| nextKey = if (response.result.hasNext) page + 1 else null | ||
| ) | ||
| } catch (e: Exception) { | ||
| LoadResult.Error(e) | ||
| } | ||
| } |
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.
Add null safety check for API response.
Line 19 accesses response.result.posts without checking if result is null or if response.isSuccess is true. If the API returns an error response, result might be null, causing a NullPointerException.
Apply this diff to add proper null safety:
override suspend fun load(params: LoadParams<Int>): LoadResult<Int, CommunityPost> {
val page = params.key ?: 0
return try {
val response = communityPostService.getCommunity(type = type, page = page)
- val posts = response.result.posts.map { it.toEntity() }
+
+ if (!response.isSuccess || response.result == null) {
+ return LoadResult.Error(Exception(response.message ?: "Unknown error"))
+ }
+
+ val posts = response.result.posts.map { it.toEntity() }
LoadResult.Page(
data = posts,
prevKey = if (page == 0) null else page - 1,
nextKey = if (response.result.hasNext) page + 1 else null
)
} catch (e: Exception) {
LoadResult.Error(e)
}
}| override fun intercept(chain: Interceptor.Chain): Response { | ||
| val accessToken = runBlocking { | ||
| tokenDataStore.accessToken.first() | ||
| } | ||
|
|
||
| val requestBuilder = chain.request().newBuilder() | ||
|
|
||
| if (!accessToken.isNullOrBlank()) { | ||
| requestBuilder.addHeader("Authorization", "Bearer $accessToken") | ||
| } | ||
|
|
||
| return chain.proceed(requestBuilder.build()) | ||
| } |
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.
Avoid runBlocking in OkHttp interceptor.
Using runBlocking in the intercept() method blocks OkHttp's limited thread pool, which can cause performance degradation, thread starvation, and potential ANRs under load.
Consider one of these alternatives:
Option 1 (Recommended): Cache token in memory
class AuthInterceptor @Inject constructor(
private val tokenDataStore: TokenDataStore
) : Interceptor {
@Volatile
private var cachedToken: String? = null
init {
// Update cache when token changes
CoroutineScope(Dispatchers.IO).launch {
tokenDataStore.accessToken.collect { token ->
cachedToken = token
}
}
}
override fun intercept(chain: Interceptor.Chain): Response {
val requestBuilder = chain.request().newBuilder()
cachedToken?.takeIf { it.isNotBlank() }?.let {
requestBuilder.addHeader("Authorization", "Bearer $it")
}
return chain.proceed(requestBuilder.build())
}
}Option 2: Use OkHttp Authenticator for token refresh scenarios
If you need token refresh capabilities, implement okhttp3.Authenticator instead, which is designed for auth scenarios.
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/data/remote/AuthInterceptor.kt around
lines 14-26, the interceptor currently uses runBlocking to read the token which
can block OkHttp threads; replace this by removing runBlocking and instead
maintain an in-memory volatile cachedToken that is updated asynchronously from
tokenDataStore (launch a background coroutine on IO in init to collect
accessToken and set cachedToken), then in intercept read the cachedToken (check
not blank) and add the Authorization header if present; if you need automatic
token refresh on 401, implement an OkHttp Authenticator rather than blocking in
the interceptor.
| override suspend fun createCommunityPost( | ||
| request : PostRequestDto | ||
| ) : BaseResponse<CommunityPostResponseDto> { | ||
| // TODO: Replace dummy token with real access token | ||
| return communityPostService.postCommunity(token = "", requestBody = request) | ||
| return communityPostService.postCommunity(requestBody = request) | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Consider removing this data source layer.
Since CommunityRepositoryImpl now injects and calls CommunityPostService directly, this data source implementation may be redundant. If it's no longer used, consider removing it to reduce unnecessary abstraction layers.
Run the following script to check if this data source is still referenced elsewhere:
🏁 Script executed:
#!/bin/bash
# Description: Find usages of CommunityPostDataSourceImpl
rg -n "CommunityPostDataSourceImpl" --type kotlin -C 3Length of output: 2508
I need to verify whether CommunityPostDataSource (the interface) is actually used by CommunityRepositoryImpl or if it's truly unused. Let me search for the interface usage and the repository implementation.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check where CommunityPostDataSource interface is used
rg -n "CommunityPostDataSource" --type kotlin -C 3
Length of output: 4628
🏁 Script executed:
#!/bin/bash
# Check CommunityRepositoryImpl to see what it injects and uses
rg -n "class CommunityRepositoryImpl" --type kotlin -A 20Length of output: 2512
🏁 Script executed:
#!/bin/bash
# Check if CommunityPostService is directly injected anywhere
rg -n "CommunityPostService" --type kotlin -C 2Length of output: 5649
Remove unused data source layer: CommunityPostDataSource and CommunityPostDataSourceImpl.
Verification confirms the data source is not referenced anywhere in the codebase. CommunityRepositoryImpl directly injects CommunityPostService (line 16), and the CommunityPostDataSource interface is never injected or used despite the Dagger binding existing in DataSourceModule.kt.
Remove:
app/src/main/java/com/hsLink/hslink/data/remote/datasource/CommunityPostDataSource.kt(interface)app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/CommunityPostDataSourceImpl.kt(implementation)- The
@Bindsbinding method inDataSourceModule.kt(lines 22–25)
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/CommunityPostDataSourceImpl.kt
(around lines 13–17) and related locations: the CommunityPostDataSource
interface and its implementation are unused; delete the interface file
app/src/main/java/com/hsLink/hslink/data/remote/datasource/CommunityPostDataSource.kt
and the implementation file
app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/CommunityPostDataSourceImpl.kt,
and remove the corresponding @Binds binding method in DataSourceModule.kt
(remove lines 22–25) so the dead data source layer and its Dagger binding are
fully eliminated; also update imports/usages if any leftover references remain
(compile to verify).
| import com.hsLink.hslink.data.dto.request.community.PostRequestDto | ||
| import com.hsLink.hslink.data.dto.response.community.CommunityPostResponseDto |
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.
🛠️ Refactor suggestion | 🟠 Major
Domain layer should not depend on data layer DTOs.
The repository interface imports and uses DTOs (PostRequestDto, CommunityPostResponseDto) from the data layer, which violates clean architecture principles. The domain layer should define its own models, and the data layer should handle mapping between DTOs and domain models.
Consider:
- Creating domain models (e.g.,
PostRequest,CommunityPost) in the domain layer - Having
createCommunityPostaccept and return domain models - Performing DTO ↔ domain model mapping in the repository implementation (data layer)
This improves testability and decouples the domain from data layer implementation details.
Also applies to: 10-10
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/domain/repository/community/CommunityRepository.kt
around lines 4-5, the repository interface currently imports and uses data-layer
DTOs (PostRequestDto, CommunityPostResponseDto); change the interface to depend
only on domain models by defining domain-level classes (e.g., PostRequest,
CommunityPost) in the domain module and update the repository method signatures
to accept/return those domain models instead of DTOs; remove the DTO imports
from this file and move all DTO ↔ domain mapping into the data-layer repository
implementation so the domain layer no longer depends on data-layer types.
| onConfirm = { | ||
| selectedBoardType?.let { boardType -> | ||
| createPost(boardType.name, title, content) | ||
| } | ||
| showUploadDialog = false | ||
| navigateToCommunity() | ||
| }, |
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
Add error handling and user feedback before navigation.
The post creation call is fire-and-forget—navigation happens immediately without waiting for the API response or handling errors. Users won't receive feedback if the post creation fails.
Consider:
- Showing a loading state while the post is being created
- Waiting for the API response before navigating
- Displaying an error message if creation fails
- Only navigating on successful creation
Check how createPost in CommunityViewModel handles results:
🏁 Script executed:
#!/bin/bash
# Description: Check if createPost in CommunityViewModel exposes success/failure state
ast-grep --pattern $'fun createPost($$$) {
$$$
}'Length of output: 1993
I need to check if there are observable state variables in the ViewModel that track post creation results:
Now let me check if CommunityViewModel exposes any observable state variables for post creation:
Now I need to check the actual CommunityViewModel to see if it exposes any state variables for post creation status:
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Check all public properties and state in CommunityViewModel
cat app/src/main/java/com/hsLink/hslink/presentation/community/viewmodel/CommunityViewModel.kt
Length of output: 1752
Now let me verify the exact navigation behavior in CommunityWritingScreen:
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Check the complete onConfirm lambda and surrounding context in CommunityWritingScreen
sed -n '260,280p' app/src/main/java/com/hsLink/hslink/presentation/community/screen/write/CommunityWritingScreen.kt
Length of output: 571
Add error handling and user feedback before navigation.
The post creation call is fire-and-forget—navigateToCommunity() executes immediately after calling createPost() without waiting for the API response. The ViewModel's createPost() launches a coroutine in viewModelScope but exposes no observable state tracking success or failure. Users receive no feedback if the post creation fails.
Implement proper async state handling:
- Expose a
postCreationState(LiveData or StateFlow) from the ViewModel tracking loading/success/error states - Update success/failure handlers in
CommunityViewModel.createPost()to mutate this state instead of leaving them empty - Observe this state in the screen before navigating
- Show a loading indicator while the API call is in progress
- Display error messages on failure and only navigate on successful creation
| val communityPosts: Flow<PagingData<CommunityPost>> = _selectedTab | ||
| .flatMapLatest { tab -> | ||
| repository.getCommunityPosts(tab.name.lowercase()) | ||
| } | ||
| .cachedIn(viewModelScope) | ||
|
|
||
| fun selectTab(tab: CommunityTab) { | ||
| _selectedTab.value = tab | ||
| } |
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.
Use a locale-invariant lowercase
tab.name.lowercase() relies on the device locale; in Turkish (and a few others) this mangles I/İ, producing invalid query params and 404s from the backend. Switch to Locale.ROOT (or Locale.ENGLISH) so we always hit the correct endpoint regardless of user locale.
-import kotlinx.coroutines.flow.flatMapLatest
+import kotlinx.coroutines.flow.flatMapLatest
+import java.util.Locale
...
- repository.getCommunityPosts(tab.name.lowercase())
+ repository.getCommunityPosts(tab.name.lowercase(Locale.ROOT))📝 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.
| val communityPosts: Flow<PagingData<CommunityPost>> = _selectedTab | |
| .flatMapLatest { tab -> | |
| repository.getCommunityPosts(tab.name.lowercase()) | |
| } | |
| .cachedIn(viewModelScope) | |
| fun selectTab(tab: CommunityTab) { | |
| _selectedTab.value = tab | |
| } | |
| val communityPosts: Flow<PagingData<CommunityPost>> = _selectedTab | |
| .flatMapLatest { tab -> | |
| repository.getCommunityPosts(tab.name.lowercase(Locale.ROOT)) | |
| } | |
| .cachedIn(viewModelScope) | |
| fun selectTab(tab: CommunityTab) { | |
| _selectedTab.value = tab | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/community/viewmodel/CommunityViewModel.kt
around lines 26 to 34, the call to tab.name.lowercase() is locale-sensitive (can
break I/İ in Turkish); change it to use a locale-invariant lowercase such as
tab.name.lowercase(Locale.ROOT) (or Locale.ENGLISH) and add the corresponding
import (java.util.Locale) so the query param is stable across device locales.
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: 0
🧹 Nitpick comments (1)
.github/workflows/ci-cd.yml (1)
94-96: Add documentation for Fastlane environment variables.While the environment variable injection mirrors the Build step (good for consistency), add an inline comment explaining what these secrets are for and optionally reference the Fastlane configuration file that consumes them.
- name: Run Fastlane distribute run: bundle exec fastlane distribute env: + # BASE_URL and KAKAO_NATIVE_APP_KEY required by Fastlane configuration BASE_URL: ${{ secrets.BASE_URL }} KAKAO_NATIVE_APP_KEY: ${{ secrets.KAKAO_NATIVE_APP_KEY }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-cd.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/ci-cd.yml (1)
38-40: No issues found—environment variable resolution is correctly implemented.Verification confirms that:
getRequiredProperty()properly readsBASE_URLandKAKAO_NATIVE_APP_KEYfrom environment variables (with naming conversion: dot→underscore, uppercase)- Fallback to
local.propertiesensures local development works- Clear error handling throws
GradleExceptionif neither source exists- Workflow injection pattern at lines 38-40 and 94-96 aligns with the property resolution chain
- Both Build and Fastlane steps consistently inject the same environment variables
The implementation is sound.
ISSUE
❗ WORK DESCRIPTION
📸 SCREENSHOT
-.Clipchamp.7.mp4