-
Notifications
You must be signed in to change notification settings - Fork 308
Prevent/Recover from DB corruption in runtime #6014
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
base: develop
Are you sure you want to change the base?
Prevent/Recover from DB corruption in runtime #6014
Conversation
SDK Size Comparison 📏
|
|
This pull request has been automatically marked as stale because it has been inactive for 14 days. It will be closed in 7 days if no further activity occurs. |
|
This pull request has been automatically marked as stale because it has been inactive for 14 days. It will be closed in 7 days if no further activity occurs. |
WalkthroughThe PR introduces database recovery and write-operation synchronization mechanisms. It adds Recoverable*Dao wrappers that lazily delegate to the current ChatDatabase instance, enabling runtime database replacement if corruption occurs. It also adds mutex-based locking to all database write operations and implements SQLite open-helper corruption callbacks for automatic database recreation. Changes
Sequence DiagramssequenceDiagram
participant Client
participant DAO as RecoverableDAO
participant Factory as DAO Factory
participant DB as ChatDatabase
participant Helper as SQLiteOpenHelper
Client->>DAO: insert/select/delete
DAO->>DAO: resolve getDatabase()
alt Database Healthy
DAO->>DB: access current DAO
DB->>Helper: execute query
Helper-->>DB: result
DB-->>DAO: return data
DAO-->>Client: return result
else Database Corrupted
Helper->>Helper: detect corruption
Helper->>Factory: onCorrupted()
Factory->>DB: recreate ChatDatabase
DB-->>Factory: new instance
DAO->>DB: retry with new instance
DB->>Helper: execute on new DB
Helper-->>DB: result
DB-->>DAO: return data
DAO-->>Client: return result
end
sequenceDiagram
participant App
participant Repo as DatabaseRepository
participant Scope as CoroutineScope
participant Mutex
participant DAO
App->>Repo: insertData()
Repo->>Repo: scope.launchWithMutex(mutex)
Repo->>Mutex: acquire lock
alt Lock Acquired
Mutex-->>Repo: locked
Repo->>DAO: insert/update/delete
DAO-->>Repo: complete
Repo->>Mutex: release lock
Mutex-->>Repo: unlocked
Repo-->>App: success
else Lock Contention
Mutex->>Mutex: queue operation
Mutex->>Mutex: wait for lock
Mutex-->>Repo: acquire
Repo->>DAO: insert (serialized)
DAO-->>Repo: complete
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/DatabaseThreadsRepository.kt (1)
69-79: Cache updated before fire-and-forget DB write could cause inconsistency.The cache is updated synchronously (lines 71-73) while the DB write is launched asynchronously via
scope.launchWithMutex. If the DB write fails (e.g., during corruption or recovery), the cache will contain data that isn't persisted. Additionally, if the coroutine scope is cancelled, pending writes may be lost while the cache retains the data.Consider either:
- Updating cache inside the mutex-protected block after successful DB write
- Documenting that cache may be ahead of DB during transient failures
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.kt (1)
63-73: Unsynchronized repository caching can cause duplicate instances in concurrent access.The
repositoriesCacheis accessed without synchronization across allcreate*Repository()methods. When multiple coroutines call these methods concurrently, they may both readnulland create separate repository instances, with only the last write persisting. Since repositories maintain internal state (as seen in other repositories usingCollections.synchronizedMapandMutex), duplicates can lead to inconsistent caching behavior.Use a thread-safe map (e.g.,
ConcurrentHashMap) or add synchronization to the cache writes across all factory methods.
🧹 Nitpick comments (5)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/DatabaseChannelConfigRepository.kt (3)
58-60: Consider error handling for DB writes on corrupted database.The mutex prevents concurrent write corruption, but write operations may still fail if the database is corrupted or closed. Consider wrapping DB operations in try-catch to gracefully handle such failures and potentially trigger recovery.
🔎 Example error handling pattern
mutex.withLock { - channelConfigDao.insert(configs.map(ChannelConfig::toEntity)) + try { + channelConfigDao.insert(configs.map(ChannelConfig::toEntity)) + } catch (e: Exception) { + // Log and potentially trigger DB recovery + } }
68-70: Consider error handling for DB writes on corrupted database.Similar to insertChannelConfigs, this write operation should handle potential database corruption or closure gracefully.
74-76: Consider error handling for DB writes on corrupted database.The deleteAll operation should also handle database errors gracefully to prevent crashes during corruption scenarios.
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.kt (1)
1-2: Minor: Copyright year inconsistency.This new file uses copyright year 2022 while other new files in this PR use 2023 or 2024. Consider updating to reflect the actual creation year.
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.kt (1)
148-175: Redundant nestedsynchronizedblock increateDb.Since
createDbis only called from withingetDatabase's synchronized block (and fromonCorruptedwhich also acquires the same lock), the innersynchronized(this)at line 149 is redundant. While JVM intrinsic locks are reentrant so this won't deadlock, it adds confusion and unnecessary overhead.🔎 Proposed fix
private fun createDb(context: Context, userId: String): ChatDatabase { - synchronized(this) { - val db = Room.databaseBuilder( - context.applicationContext, - ChatDatabase::class.java, - "stream_chat_database_$userId", - ) - .fallbackToDestructiveMigration() - .addCallback( - object : Callback() { - override fun onOpen(db: SupportSQLiteDatabase) { - db.execSQL("PRAGMA synchronous = 1") - } - }, - ) - .openHelperFactory( - StreamSQLiteOpenHelperFactory(onCorrupted = { - synchronized(this) { - // Re-instantiate the DB if corrupted - INSTANCES.remove(userId) - INSTANCES[userId] = createDb(context, userId) - } - }), - ) - .build() - return db - } + val db = Room.databaseBuilder( + context.applicationContext, + ChatDatabase::class.java, + "stream_chat_database_$userId", + ) + .fallbackToDestructiveMigration() + .addCallback( + object : Callback() { + override fun onOpen(db: SupportSQLiteDatabase) { + db.execSQL("PRAGMA synchronous = 1") + } + }, + ) + .openHelperFactory( + StreamSQLiteOpenHelperFactory(onCorrupted = { + synchronized(this@Companion) { + // Re-instantiate the DB if corrupted + INSTANCES.remove(userId) + INSTANCES[userId] = createDb(context, userId) + } + }), + ) + .build() + return db }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/extensions/CoroutineScopeExtensions.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/plugin/factory/StreamOfflinePluginFactory.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/DatabaseChannelConfigRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/MessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/ReplyMessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/DatabaseQueryChannelsRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/DatabaseReactionRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/DatabaseThreadsRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseDomainTest2.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseRepositoryFacadeIntegrationTest.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/MessageRepositoryTests.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/QueryChannelsImplRepositoryTest.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/ReactionRepositoryTest.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/utils/MockDatabase.kt
💤 Files with no reviewable changes (1)
- stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/utils/MockDatabase.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/MessageRepositoryTests.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/QueryChannelsImplRepositoryTest.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/DatabaseQueryChannelsRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/MessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/extensions/CoroutineScopeExtensions.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/DatabaseThreadsRepository.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseDomainTest2.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/DatabaseChannelConfigRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/ReplyMessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/ReactionRepositoryTest.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseRepositoryFacadeIntegrationTest.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/DatabaseReactionRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/plugin/factory/StreamOfflinePluginFactory.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented
Document public APIs with KDoc, including thread expectations and state notes
Files:
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/MessageRepositoryTests.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/QueryChannelsImplRepositoryTest.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/DatabaseQueryChannelsRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/MessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/extensions/CoroutineScopeExtensions.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/DatabaseThreadsRepository.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseDomainTest2.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/DatabaseChannelConfigRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/ReplyMessageDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/ReactionRepositoryTest.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseRepositoryFacadeIntegrationTest.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/DatabaseReactionRepository.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/plugin/factory/StreamOfflinePluginFactory.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.ktstream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.kt
**/src/test/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/src/test/**/*.kt: Use backtick test names (for example:funmessage list filters muted channels()) for readability
Use deterministic tests withrunTest+ virtual time for concurrency-sensitive logic (uploads, sync, message state)
Keep helper extensions private/internal in test files
Files:
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/MessageRepositoryTests.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/QueryChannelsImplRepositoryTest.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseDomainTest2.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/ReactionRepositoryTest.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseRepositoryFacadeIntegrationTest.kt
🧠 Learnings (2)
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use deterministic tests with `runTest` + virtual time for concurrency-sensitive logic (uploads, sync, message state)
Applied to files:
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/MessageRepositoryTests.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/QueryChannelsImplRepositoryTest.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/ReactionRepositoryTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-ui-components/**/*Test.kt : Record Shot baselines when behaviour changes in XML kit UI tests
Applied to files:
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseDomainTest2.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/ReactionRepositoryTest.ktstream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseRepositoryFacadeIntegrationTest.kt
🧬 Code graph analysis (8)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (9)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.kt (1)
getDatabase(30-82)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.kt (1)
getDatabase(28-68)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.kt (1)
getDatabase(29-58)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.kt (1)
getDatabase(30-78)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
getDatabase(28-56)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.kt (1)
getDatabase(28-67)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.kt (7)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.kt (1)
getDatabase(138-146)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.kt (1)
getDatabase(32-199)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
getDatabase(28-56)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.kt (1)
getDatabase(28-67)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.kt (11)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.kt (1)
getDatabase(138-146)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.kt (1)
getDatabase(30-82)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.kt (1)
getDatabase(28-68)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.kt (1)
getDatabase(32-199)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.kt (1)
getDatabase(29-58)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
getDatabase(28-56)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.kt (1)
getDatabase(28-67)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.kt (8)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.kt (1)
getDatabase(30-82)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.kt (1)
getDatabase(28-68)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.kt (1)
getDatabase(32-199)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.kt (1)
getDatabase(29-58)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.kt (1)
getDatabase(30-78)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
getDatabase(28-56)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (1)
getDatabase(28-44)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/extensions/CoroutineScopeExtensions.kt (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt (1)
launch(310-312)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.kt (11)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.kt (1)
getDatabase(138-146)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.kt (1)
getDatabase(30-82)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.kt (1)
getDatabase(28-68)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.kt (1)
getDatabase(32-199)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.kt (1)
getDatabase(29-58)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.kt (1)
getDatabase(30-78)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
getDatabase(28-56)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.kt (1)
getDatabase(28-67)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.kt (9)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.kt (1)
getDatabase(30-82)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.kt (1)
getDatabase(28-68)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.kt (1)
getDatabase(29-58)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.kt (1)
getDatabase(30-78)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
getDatabase(28-56)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.kt (1)
getDatabase(28-67)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.kt (6)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.kt (1)
getDatabase(32-199)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.kt (1)
getDatabase(28-44)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
getDatabase(28-56)stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (1)
getDatabase(28-44)
⏰ 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). (6)
- GitHub Check: base-android-ci / Run unit tests
- GitHub Check: base-android-ci / Run static checks
- GitHub Check: base-android-ci / Build
- GitHub Check: Detekt
- GitHub Check: compare-sdk-sizes / Compare SDK sizes
- GitHub Check: Build / compose apks
🔇 Additional comments (32)
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/ReplyMessageDao.kt (1)
46-48: LGTM!The deprecation annotation is appropriate for marking this unused method for future removal. The message clearly communicates the intent.
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/MessageRepositoryTests.kt (1)
107-107: LGTM! Aligns with deterministic testing guidelines.Passing the explicit testDispatcher to runTest ensures deterministic coroutine execution, consistent with the project's testing guidelines for concurrency-sensitive logic.
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/QueryChannelsImplRepositoryTest.kt (1)
55-55: LGTM! Correctly passes test scope to repository.The updated constructor properly provides testCoroutines.scope, ensuring the repository's coroutine operations execute in the controlled test environment.
stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/repository/ReactionRepositoryTest.kt (1)
57-57: LGTM! Test updated for new repository constructor.The repository instantiation correctly provides the test scope, DAO, and currentUser lambda, maintaining test coverage with the updated constructor signature.
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverablePollDao.kt (1)
28-43: LGTM! Recoverable DAO pattern correctly implemented.The lazy delegate property ensures all PollDao operations use the current ChatDatabase instance, enabling runtime database replacement after corruption recovery. The pattern is consistent with other Recoverable*Dao implementations across the codebase.
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/syncState/internal/RecoverableSyncStateDao.kt (1)
28-43: LGTM! Recoverable DAO pattern correctly implemented.The lazy delegate pattern ensures SyncStateDao operations always target the current ChatDatabase instance, supporting runtime database recovery. Implementation is consistent with the broader recoverable DAO architecture.
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/MessageDao.kt (2)
63-70: Method can be safely removed - no external callers found.The deprecated
upsertMessageInnerEntitymethod has no external usages. It is only called withinRecoverableMessageDaoas a delegation, and the batch replacementupsertMessageInnerEntitiesis already actively used. The method definition can be removed from bothMessageDaoandRecoverableMessageDaowithout any migration concerns.
50-52: Update the deprecation plan: the method exists only in the DAO layer.The deprecated
insert(MessageEntity)method is called within RecoverableMessageDao's wrapper delegation (line 52), which is expected since RecoverableMessageDao must implement all MessageDao interface methods. No external call sites exist outside the DAO layer. The method can be safely removed by updating both MessageDao and RecoverableMessageDao together, but no additional fixes are needed elsewhere in the codebase.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/plugin/factory/StreamOfflinePluginFactory.kt (1)
86-89: LGTM! Lazy database initialization enables runtime recovery.The lambda-based database provider allows the repository factory to fetch the current ChatDatabase instance on-demand. Thread safety is properly handled via
@Volatileand synchronized blocks with double-checked locking. Per-user database isolation is correctly implemented usinguser.idas both the map key and part of the database filename. Database corruption is transparently recovered through theStreamSQLiteCallbackmechanism, which detects corruption viaonCorruption()and automatically re-instantiates the database instance.stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseRepositoryFacadeIntegrationTest.kt (1)
51-51: LGTM!The test correctly adapts to the new
DatabaseRepositoryFactorysignature by passing a lambda{ chatDatabase }instead of the direct instance. The lambda captures thechatDatabasereference correctly since it's initialized insetup()before the factory is used.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/DatabaseReactionRepository.kt (2)
55-57: Writes are fire-and-forget, which may not match caller expectations.
launchWithMutexlaunches a coroutine and returns immediately without waiting for completion. Callers ofinsertReaction,updateReactionsForMessageByDeletedDate, anddeleteReactionmay expect these suspend functions to complete the write before returning.This is inconsistent with
clear()(lines 156-158), which usesmutex.withLockand suspends until completion.If fire-and-forget is intentional for resilience (allowing the caller to continue even if the DB write is pending), consider documenting this behavior. Otherwise, consider awaiting the launched job:
scope.launchWithMutex(mutex) { ... }.join()
32-43: KDoc is clear and the mutex pattern is consistent with the PR's approach.The documentation properly explains the
scopeparameter and the repository's purpose. The mutex initialization follows the pattern used across other repositories in this PR.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableReplyMessageDao.kt (1)
29-57: LGTM!The implementation correctly follows the recoverable DAO pattern established across the codebase. The lazy
delegateproperty ensures each call retrieves the DAO from the currentChatDatabaseinstance, enabling seamless recovery after database recreation. The deprecateddeletemethod properly preserves its annotation.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadOrderDao.kt (1)
28-43: LGTM!The implementation correctly follows the established recoverable DAO pattern, matching the structure of
RecoverableThreadDaoand other recoverable DAOs in the codebase. AllThreadOrderDaointerface methods are properly delegated.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/extensions/CoroutineScopeExtensions.kt (1)
42-42: Non-suspending function returns aJobbut callers cannot await writes.Changing from
suspend funtofunmeans callers receive aJobbut the operations complete asynchronously. This is intentional per the PR design, but callers expecting synchronous write semantics should be aware.The returned
Jobcan be used with.join()if awaiting is needed.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.kt (3)
130-136: LGTM!Idiomatic Kotlin style improvement -
messageCache[it.id]is equivalent tomessageCache.get(it.id)but more concise.
143-152: Good improvement: Poll insertion now within mutex-protected block.Moving poll insertion inside
scope.launchWithMutex(dbMutex)ensures all related DB writes (polls, reply messages, messages) are serialized together, maintaining consistency.
164-175: Draft message operations now use fire-and-forget pattern.Same pattern as other write operations -
insertDraftMessageanddeleteDraftMessagelaunch coroutines and return immediately. Consistent with the PR's approach but worth noting that callers won't know when writes complete.stream-chat-android-offline/src/test/java/io/getstream/chat/android/offline/integration/BaseDomainTest2.kt (1)
181-181: LGTM!The test correctly adapts to the new
DatabaseRepositoryFactorysignature by wrapping the database instance in a lambda. Thedbvariable is initialized insetup()beforecreateChatDomain()is called, ensuring the captured reference is valid.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/reaction/internal/RecoverableReactionDao.kt (1)
30-77: LGTM!The implementation correctly follows the established recoverable DAO pattern. All
ReactionDaointerface methods are properly delegated to the lazily-resolved delegate, enabling seamless database recovery after corruption.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.kt (2)
179-200: LGTM!The
StreamSQLiteOpenHelperFactorycorrectly wraps the delegate factory and injects the custom callback. The configuration builder properly preserves all original settings while swapping in the corruption-aware callback.
208-244: LGTM!The
StreamSQLiteCallbackproperly delegates all lifecycle methods to the original callback and invokes theonCorruptedhandler after the delegate'sonCorruptioncompletes. The logging is appropriately placed for debugging.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/user/internal/RecoverableUserDao.kt (1)
28-66: LGTM!The
RecoverableUserDaocorrectly implements the lazy delegation pattern. The computeddelegateproperty ensures each operation uses the current database instance, enabling seamless recovery after DB recreation. The KDoc clearly explains the purpose and thread-safety expectations.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/RecoverableQueryChannelsDao.kt (1)
28-43: LGTM!The
RecoverableQueryChannelsDaofollows the established recoverable DAO pattern consistently. The implementation correctly delegates all operations to the current database instance.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/DatabaseQueryChannelsRepository.kt (1)
63-67: LGTM!The
clear()method correctly usesmutex.withLockto ensure the delete operation completes before returning, and the lock prevents concurrent modifications.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/RecoverableChannelConfigDao.kt (1)
28-67: LGTM!The
RecoverableChannelConfigDaoconsistently follows the established recoverable DAO pattern. All methods correctly delegate to the current database instance's DAO.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.kt (2)
55-61: LGTM!The factory constructor correctly accepts a lazy database provider
() -> ChatDatabase. This enables runtime database replacement without requiring repository recreation, as the Recoverable*Dao wrappers will always obtain the current database instance.
142-161: LGTM!The
createMessageRepositorycorrectly wires multiple Recoverable*Dao instances (MessageDao, ReplyMessageDao, PollDao), ensuring all message-related DAOs support database recovery.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/DatabaseThreadsRepository.kt (1)
164-170: LGTM!The
clear()method correctly evicts the cache first, then usesmutex.withLockto ensure both DAO delete operations complete before returning. This guarantees a consistent cleared state.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/threads/internal/RecoverableThreadDao.kt (1)
28-55: LGTM!The
RecoverableThreadDaofollows the established recoverable DAO pattern consistently, correctly delegating all thread operations to the current database instance.stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channel/internal/RecoverableChannelDao.kt (1)
23-82: LGTM! Clean implementation of the recoverable DAO pattern.The lazy delegate resolution via the getter property ensures that each operation uses the current
ChatDatabaseinstance, enabling seamless recovery after database recreation. The implementation is consistent with other recoverable DAO wrappers in this PR (e.g.,RecoverablePollDao,RecoverableSyncStateDao,RecoverableThreadDao).stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/RecoverableMessageDao.kt (1)
32-199: LGTM! Comprehensive delegation with correct deprecation handling.The implementation correctly forwards all
MessageDaooperations to the lazily-resolved delegate. The@Suppress("TooManyFunctions")annotation is appropriate given the large interface surface area. Deprecated methods properly carry forward their annotations.Note: The non-suspend methods (
deleteAttachments,deleteAttachmentsChunked,updateMessageInnerEntity) are inherited from theMessageDaointerface. If these are called from a non-IO context, they could block the calling thread — but that concern is orthogonal to this wrapper and should be addressed at the interface level if needed.
| * Any [Exception] thrown during execution is caught and logged. We assume such exceptions | ||
| * can happen only in the edge-case where the database is corrupted/closed, so we can safely ignore them. | ||
| * | ||
| * @param mutex The [Mutex] to acquire before executing the block. | ||
| * @param block The suspend function to execute within the mutex lock. | ||
| * @return A [kotlinx.coroutines.Job] representing the launched coroutine. | ||
| */ | ||
| internal fun CoroutineScope.launchWithMutex(mutex: Mutex, block: suspend () -> Unit) = launch { | ||
| try { | ||
| mutex.withLock { block() } | ||
| } catch (e: IllegalStateException) { | ||
| logger.e(e) { "Exception in launchWithMutex: ${e.message}" } | ||
| } |
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.
Documentation states "Any Exception" but code only catches IllegalStateException.
The KDoc at line 35-36 states "Any [Exception] thrown during execution is caught and logged", but the implementation at line 45 only catches IllegalStateException. Other exceptions (e.g., SQLiteException, SQLiteDatabaseCorruptException) would propagate uncaught.
If the intent is to catch all DB-related exceptions, consider broadening the catch or updating the documentation to match the actual behavior:
Option 1: Catch broader exception types
try {
mutex.withLock { block() }
- } catch (e: IllegalStateException) {
+ } catch (e: Exception) {
+ if (e is kotlinx.coroutines.CancellationException) throw e // Don't suppress cancellation
logger.e(e) { "Exception in launchWithMutex: ${e.message}" }
}Option 2: Update documentation to match implementation
- * Any [Exception] thrown during execution is caught and logged. We assume such exceptions
+ * Any [IllegalStateException] thrown during execution is caught and logged. We assume such exceptions📝 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.
| * Any [Exception] thrown during execution is caught and logged. We assume such exceptions | |
| * can happen only in the edge-case where the database is corrupted/closed, so we can safely ignore them. | |
| * | |
| * @param mutex The [Mutex] to acquire before executing the block. | |
| * @param block The suspend function to execute within the mutex lock. | |
| * @return A [kotlinx.coroutines.Job] representing the launched coroutine. | |
| */ | |
| internal fun CoroutineScope.launchWithMutex(mutex: Mutex, block: suspend () -> Unit) = launch { | |
| try { | |
| mutex.withLock { block() } | |
| } catch (e: IllegalStateException) { | |
| logger.e(e) { "Exception in launchWithMutex: ${e.message}" } | |
| } | |
| * Any [Exception] thrown during execution is caught and logged. We assume such exceptions | |
| * can happen only in the edge-case where the database is corrupted/closed, so we can safely ignore them. | |
| * | |
| * @param mutex The [Mutex] to acquire before executing the block. | |
| * @param block The suspend function to execute within the mutex lock. | |
| * @return A [kotlinx.coroutines.Job] representing the launched coroutine. | |
| */ | |
| internal fun CoroutineScope.launchWithMutex(mutex: Mutex, block: suspend () -> Unit) = launch { | |
| try { | |
| mutex.withLock { block() } | |
| } catch (e: Exception) { | |
| if (e is kotlinx.coroutines.CancellationException) throw e // Don't suppress cancellation | |
| logger.e(e) { "Exception in launchWithMutex: ${e.message}" } | |
| } |
| * Any [Exception] thrown during execution is caught and logged. We assume such exceptions | |
| * can happen only in the edge-case where the database is corrupted/closed, so we can safely ignore them. | |
| * | |
| * @param mutex The [Mutex] to acquire before executing the block. | |
| * @param block The suspend function to execute within the mutex lock. | |
| * @return A [kotlinx.coroutines.Job] representing the launched coroutine. | |
| */ | |
| internal fun CoroutineScope.launchWithMutex(mutex: Mutex, block: suspend () -> Unit) = launch { | |
| try { | |
| mutex.withLock { block() } | |
| } catch (e: IllegalStateException) { | |
| logger.e(e) { "Exception in launchWithMutex: ${e.message}" } | |
| } | |
| * Any [IllegalStateException] thrown during execution is caught and logged. We assume such exceptions | |
| * can happen only in the edge-case where the database is corrupted/closed, so we can safely ignore them. | |
| * | |
| * @param mutex The [Mutex] to acquire before executing the block. | |
| * @param block The suspend function to execute within the mutex lock. | |
| * @return A [kotlinx.coroutines.Job] representing the launched coroutine. | |
| */ | |
| internal fun CoroutineScope.launchWithMutex(mutex: Mutex, block: suspend () -> Unit) = launch { | |
| try { | |
| mutex.withLock { block() } | |
| } catch (e: IllegalStateException) { | |
| logger.e(e) { "Exception in launchWithMutex: ${e.message}" } | |
| } |
| fun getDatabase(context: Context, userId: String): ChatDatabase { | ||
| if (!INSTANCES.containsKey(userId)) { | ||
| synchronized(this) { | ||
| val db = Room.databaseBuilder( | ||
| context.applicationContext, | ||
| ChatDatabase::class.java, | ||
| "stream_chat_database_$userId", | ||
| ).fallbackToDestructiveMigration() | ||
| .addCallback( | ||
| object : Callback() { | ||
| override fun onOpen(db: SupportSQLiteDatabase) { | ||
| db.execSQL("PRAGMA synchronous = 1") | ||
| } | ||
| }, | ||
| ) | ||
| .build() | ||
| val db = createDb(context, userId) | ||
| INSTANCES[userId] = db | ||
| } | ||
| } | ||
| return INSTANCES[userId] ?: error("DB not created") | ||
| } |
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.
Race condition in getDatabase() due to check-then-act pattern.
The containsKey check at line 139 is outside the synchronized block, so two threads can both pass the check and enter the synchronized block sequentially, potentially creating duplicate database instances (the second one overwriting the first).
🔎 Proposed fix
fun getDatabase(context: Context, userId: String): ChatDatabase {
- if (!INSTANCES.containsKey(userId)) {
- synchronized(this) {
+ synchronized(this) {
+ if (!INSTANCES.containsKey(userId)) {
val db = createDb(context, userId)
INSTANCES[userId] = db
}
}
return INSTANCES[userId] ?: error("DB not created")
}🤖 Prompt for AI Agents
In
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.kt
around lines 138-146, the current check-then-act using
INSTANCES.containsKey(userId) happens outside the synchronized block which
allows a race where two threads both pass the check and create duplicate DB
instances; move the existence check inside the synchronized(this) block so the
check-and-create are atomic (or replace the pattern with a thread-safe atomic
operation such as using INSTANCES.putIfAbsent / getOrPut on a
ConcurrentHashMap), i.e. enter synchronized first, then if the instance for
userId is missing create it and store it, and finally return the instance.
| private fun Message.filterReactions(): Message = copy( | ||
| ownReactions = ownReactions | ||
| .filter { it.deletedAt == null } | ||
| .filter { currentUser == null || it.userId == currentUser.id }, | ||
| .filter { it.userId == currentUser.id }, | ||
| latestReactions = latestReactions.filter { it.deletedAt == null }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find User class definition to check id field constraints
fd -e kt -e kts | xargs rg "data class User|class User" -A 20 | head -100Repository: GetStream/stream-chat-android
Length of output: 16033
🏁 Script executed:
#!/bin/bash
# Check the DatabaseMessageRepository constructor and currentUser parameter
rg "class DatabaseMessageRepository" -A 10 stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.ktRepository: GetStream/stream-chat-android
Length of output: 559
🏁 Script executed:
#!/bin/bash
# Find how DatabaseMessageRepository is instantiated
fd -e kt -e kts | xargs rg "DatabaseMessageRepository\(" -B 2 -A 5 | head -150Repository: GetStream/stream-chat-android
Length of output: 8728
🏁 Script executed:
#!/bin/bash
# Search for the User domain class definition
fd -e kt -e kts | xargs rg "^data class User|^class User" -B 2 -A 30 | grep -A 30 "^[^-]*:.*data class User\|^[^-]*:.*class User" | head -150Repository: GetStream/stream-chat-android
Length of output: 55
🏁 Script executed:
#!/bin/bash
# More specific search for User class in common locations
fd "User.kt" -e kt | xargs cat | head -100Repository: GetStream/stream-chat-android
Length of output: 3844
🏁 Script executed:
#!/bin/bash
# Search for User class references specifically in the models directory
find . -path "*/model*" -name "*.kt" | xargs rg "data class User\|class User" -A 20 | head -150Repository: GetStream/stream-chat-android
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Search for User class in models package
find . -name "*.kt" | xargs rg "package io\.getstream\.chat\.android\.models" -A 50 | grep -B 5 -A 40 "class User\|data class User" | head -200Repository: GetStream/stream-chat-android
Length of output: 16625
🏁 Script executed:
#!/bin/bash
# Try a different approach - look for User.kt file
find . -name "User.kt" -type f | head -5Repository: GetStream/stream-chat-android
Length of output: 572
🏁 Script executed:
#!/bin/bash
# Search for where User is defined
rg "class User\(" -t kt -A 25 | head -150Repository: GetStream/stream-chat-android
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Get the complete User class definition
cat stream-chat-android-core/src/main/java/io/getstream/chat/android/models/User.kt | head -80Repository: GetStream/stream-chat-android
Length of output: 3608
🏁 Script executed:
#!/bin/bash
# Now let's look at the filterReactions function more carefully
cat -n stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.kt | sed -n '330,350p'Repository: GetStream/stream-chat-android
Length of output: 679
🏁 Script executed:
#!/bin/bash
# Check if there's any prior defensive null-checking for currentUser.id
rg "currentUser\\.id|currentUser\\?" stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.kt -nRepository: GetStream/stream-chat-android
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Check the DatabaseRepositoryFactory to see how currentUser is provided
cat -n stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/factory/internal/DatabaseRepositoryFactory.kt | sed -n '1,100p'Repository: GetStream/stream-chat-android
Length of output: 6462
🏁 Script executed:
#!/bin/bash
# Look for any validation of currentUser.id in the codebase
rg "currentUser.*id.*empty|currentUser.*id.*blank|User.*id.*validation" -t kt | head -20Repository: GetStream/stream-chat-android
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check if there's any validation when a User is created or connected
rg "require.*id|check.*id" stream-chat-android-core/src/main/java/io/getstream/chat/android/models/User.kt -A 2 -B 2Repository: GetStream/stream-chat-android
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Find where DatabaseRepositoryFactory is created/instantiated
rg "DatabaseRepositoryFactory\(" -A 5 -B 2 --type-list | head -50Repository: GetStream/stream-chat-android
Length of output: 462
🏁 Script executed:
#!/bin/bash
# Search for any validation or checks on currentUser before it's used
rg "User\(.*id.*=" stream-chat-android-core/src/main/java/io/getstream/chat/android/models/User.kt | head -5Repository: GetStream/stream-chat-android
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Check if there are any tests that verify empty id behavior
rg "User\(id = \"\"" --type-list | head -20Repository: GetStream/stream-chat-android
Length of output: 462
🏁 Script executed:
#!/bin/bash
# Look for tests related to filterReactions
rg "filterReactions" -A 10 -B 2 | head -100Repository: GetStream/stream-chat-android
Length of output: 23733
Ensure currentUser.id is validated as non-empty before filtering reactions.
The User class allows id to default to "" (empty string) with no validation constraint. The new implementation at line 336 assumes currentUser.id is always valid by directly comparing it.userId == currentUser.id. If currentUser has an empty id in production, reactions with userId = "" would incorrectly match the filter. Either validate that currentUser.id is non-empty when the repository is initialized, or add a defensive check to skip filtering if currentUser.id is empty/blank.
🤖 Prompt for AI Agents
In
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/message/internal/DatabaseMessageRepository.kt
around lines 333 to 337, the filtering uses currentUser.id directly which can be
empty; update the logic to first check currentUser.id is not blank before
applying the userId equality filter and otherwise skip the userId check (i.e.,
filter only by deletedAt when currentUser.id is blank), or alternatively add
validation at repository initialization to ensure currentUser.id is non-empty
and throw/log if invalid; implement the defensive check so reactions with userId
== "" are not accidentally treated as the current user.
| override suspend fun insertQueryChannels(queryChannelsSpec: QueryChannelsSpec) { | ||
| queryChannelsDao.insert(toEntity(queryChannelsSpec)) | ||
| scope.launchWithMutex(mutex) { | ||
| queryChannelsDao.insert(toEntity(queryChannelsSpec)) | ||
| } | ||
| } |
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.
insertQueryChannels is fire-and-forget but declared as suspend fun.
Using scope.launchWithMutex launches a coroutine that runs asynchronously—the function returns immediately without waiting for the insert to complete. This is inconsistent with the suspend modifier which typically implies the operation completes before returning. Callers may expect the data to be persisted after this function returns.
If fire-and-forget is intentional for performance, consider documenting this behavior. Otherwise, you should await the result.
🔎 Proposed fix (if awaiting is desired)
override suspend fun insertQueryChannels(queryChannelsSpec: QueryChannelsSpec) {
- scope.launchWithMutex(mutex) {
- queryChannelsDao.insert(toEntity(queryChannelsSpec))
- }
+ mutex.withLock {
+ queryChannelsDao.insert(toEntity(queryChannelsSpec))
+ }
}📝 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 suspend fun insertQueryChannels(queryChannelsSpec: QueryChannelsSpec) { | |
| queryChannelsDao.insert(toEntity(queryChannelsSpec)) | |
| scope.launchWithMutex(mutex) { | |
| queryChannelsDao.insert(toEntity(queryChannelsSpec)) | |
| } | |
| } | |
| override suspend fun insertQueryChannels(queryChannelsSpec: QueryChannelsSpec) { | |
| mutex.withLock { | |
| queryChannelsDao.insert(toEntity(queryChannelsSpec)) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/queryChannels/internal/DatabaseQueryChannelsRepository.kt
around lines 47–51, the suspend function insertQueryChannels currently launches
a fire-and-forget coroutine (scope.launchWithMutex) so it returns before the
insert completes; instead, make the function actually suspend until the insert
finishes by acquiring the mutex and performing the DAO insert directly (e.g.,
use mutex.withLock { queryChannelsDao.insert(toEntity(queryChannelsSpec)) } or
run the insert inside an appropriate withContext(Dispatchers.IO) block while
holding the mutex), so callers can rely on persistence when the suspend function
returns.
|


🎯 Goal
We have reports of crashes originating from the DB caused by performing write operations after the DB has been corrupted. With this PR we are adding several guards and optimistic improvements that would:
🛠 Implementation details
Mutexlocks for write operations on the same table (follows the pattern established in theDatabaseChannelRepository/DatabaseMessageRepository->scope.launchWithMutext(mutex) { writeOperation() }, which runs the write in a new coroutine, locked on the providedMutex)Recoverable<Entity>Daoclasses, which are wrappers around thelazygetDatabase().getDao()which always retrieves a Dao from the current DB instance, instead of keeping a reference to the initially retrieved DAO.🎨 UI Changes
NA
🧪 Testing
Corrupt DBbutton, which will attempt to simulate a DB corruption, and insert a dummy user afterwards.Corrupt DBbutton - sometimes multiple attempts are required in order for the DB to become corruptedChat:StreamSQLiteCallback-onCorruption called for DBit means that a corruption happenedProvide the patch summary here
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.