DI #514
Open
DI #514
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replaced `Parcelable` with `kotlinx.serialization.Serializable` across data models to improve serialization performance and maintainability. - Replaced the `kotlin-parcelize` plugin with `kotlin-serialization`. - Added `kotlinx-serialization-json` dependency. - Updated `IdentityAnnouncement`, `BitchatMessage`, `BitchatPacket`, `NoisePayload`, `PrivateMessagePacket`, and `ReadReceipt` to use `@Serializable`. - Introduced a new `Serializers.kt` file with custom serializers for `Date`, `ByteArray`, `UByte`, and `ULong` to ensure compatibility.
Introduces `JsonUtil`, a Kotlin object that serves as a wrapper around the `kotlinx.serialization` library. This utility provides methods for serializing objects to JSON and deserializing JSON strings to objects, including safe versions that return null on error. It is configured to be lenient and ignore unknown keys.
Replaced Gson with `kotlinx.serialization` for JSON handling across the application. This improves performance and ensures better integration with Kotlin. - Updated `NostrEvent`, `NostrFilter`, `BitchatMessage`, `IdentityAnnouncement`, `BitchatPacket`, and `SeenMessageStore` data classes to use `@Serializable`. - Replaced `@SerializedName` with `@SerialName`. - Created custom `ByteArraySerializer`, `UByteSerializer`, and `ULongSerializer` for handling specific types in `BitchatPacket` and `IdentityAnnouncement`. - Introduced a `JsonUtil` helper to centralize serialization logic. - Removed the Gson dependency and related imports. - Refactored `NostrFilter` to move helper functions and the `Builder` class into a companion object.
This commit replaces the Gson library with kotlinx.serialization for all JSON processing related to Nostr messages (requests, responses, and protocol operations). Key changes include: - Removing custom `JsonSerializer` implementations. - Updating `NostrRequest.toJson` to use `buildJsonArray` from kotlinx.serialization. - Modifying `NostrResponse.fromJsonArray` and other parsing logic to use kotlinx.serialization's `JsonElement` API instead of Gson's. - Removing the Gson instance from `NostrRelayManager` and `NostrProtocol`.
Replaced Gson with kotlinx.serialization for `NoisePayload` and `NoiseChannelEncryption`. This change introduces a custom `ByteArraySerializer` to handle the serialization of the `data` field in `NoisePayload` and updates the channel key packet processing to use the new JSON utility.
Replaced all instances of the Gson library with the `kotlinx.serialization` library for JSON handling. - Introduced a new `JsonUtil` helper class for serialization and deserialization. - Updated `GeohashBookmarksStore`, `LocationChannelManager`, `FavoritesPersistenceService`, and `DataManager` to use `JsonUtil`. - Added the `@Serializable` annotation to `FavoriteRelationshipData`. - Ensured safer JSON parsing by handling potential nulls and exceptions during deserialization.
The Gson JSON library is no longer in use and has been removed from the project's dependencies.
…linx.serialization
Replaced the custom `CloseButton` composable with a standard `TextButton` in the `AboutSheet` and `LocationChannelsSheet`. This change removes the `CloseButton` implementation and updates the top bar of both sheets to use a `TextButton` with the text "Close" for dismissing the view.
- Convert singletons to @singleton injectable classes (TorManager, NostrRelayManager, MessageRouter, etc.) - Implement Koin dependency injection with Koin Annotations (@module, @componentscan, @KoinViewModel) - Update ViewModels (ChatViewModel, GeohashViewModel) to use constructor injection and koinViewModel delegate - Resolve circular dependencies in BluetoothMeshService and MessageRouter - Remove obsolete LocationNotesInitializer - Update BitchatApplication and MainActivity for Koin initialization
- Refactored NostrEventDeduplicator to use @singleton and @Inject - Refactored SeenMessageStore to use @singleton and @Inject constructor(Context) - Refactored PeerFingerprintManager to use @singleton and @Inject - Refactored DataManager to use @singleton and @Inject constructor(Context) - Refactored LocationChannelManager to use @singleton and @Inject constructor(Context, DataManager) - Refactored NoiseEncryptionService to use @singleton and @Inject constructor(Context, PeerFingerprintManager) - Refactored EncryptionService to use @singleton and @Inject constructor(Context, NoiseEncryptionService) - Removed all getInstance() singleton patterns from refactored services - Updated all usages to use constructor injection via Koin - Updated Composables to use koinInject() for dependency injection - Updated Activities to use inject() delegate for dependency injection - Updated ViewModels to receive dependencies via constructor parameters - Injected PeerFingerprintManager into NoiseEncryptionService, EncryptionService, BluetoothMeshService, PeerManager, and PrivateChatManager - Injected DataManager into LocationChannelManager - Injected LocationChannelManager into NostrTransport, GeohashViewModel, and all Composables that previously used getInstance() - Injected SeenMessageStore into NostrDirectMessageHandler, GeohashViewModel, and ChatViewModel - Injected NostrEventDeduplicator into NostrRelayManager All changes maintain 100% backward compatibility and the build is successful.
…oin DI - Refactored GeohashBookmarksStore to use @singleton and @Inject constructor(Context) - Removed getInstance() singleton pattern from GeohashBookmarksStore - Updated all GeohashBookmarksStore usages to use Koin injection: - ChatViewModel: injected via constructor parameter - LocationChannelsSheet: uses koinInject() - ChatHeader: uses koinInject() - Refactored DebugSettingsManager to use @singleton and @Inject constructor() - Added getInstance() bridge method for backward compatibility with mesh layer - Bridge method delegates to Koin for singleton instance management All changes maintain backward compatibility and the build is successful.
- Injected DebugSettingsManager into BluetoothMeshService constructor - Injected DebugSettingsManager into BluetoothConnectionManager constructor - Injected DebugSettingsManager into PacketProcessor constructor - Injected DebugSettingsManager into PacketRelayManager constructor - Removed lazy getInstance() calls from these classes - Replaced all getInstance() usage with injected debugManager Remaining getInstance() calls in sub-managers (BluetoothGattServerManager, BluetoothGattClientManager, BluetoothConnectionTracker, BluetoothPacketBroadcaster) still use the bridge method for now. Build is successful and all core mesh layer classes now use proper DI.
- Removed DebugSettingsManager.getInstance() bridge method - Injected DebugSettingsManager into BluetoothConnectionTracker - Injected DebugSettingsManager into BluetoothGattServerManager - Injected DebugSettingsManager into BluetoothGattClientManager - Injected DebugSettingsManager into BluetoothPacketBroadcaster - Injected DebugSettingsManager into ChatViewModel - Used koinInject() in DebugSettingsSheet - Updated BluetoothConnectionManager to pass injected DebugSettingsManager to sub-managers - Verified build success
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
66
to
70
| // Initialize permission management | ||
| permissionManager = PermissionManager(this) | ||
| // Initialize core mesh service first | ||
| meshService = BluetoothMeshService(this) | ||
| // Initialize core mesh service first - retrieve from Koin | ||
| meshService = org.koin.java.KoinJavaComponent.getKoin().get() | ||
| bluetoothStatusManager = BluetoothStatusManager( |
There was a problem hiding this comment.
MainActivity now retrieves BluetoothMeshService from Koin (getKoin().get()), but the only module in the app is AppModule which just component-scans Koin annotations, and none of the services are annotated with Koin’s @Single/DSL bindings—only jakarta.inject.Inject/@Singleton. Koin’s codegen won’t register those constructors, so the bean is never defined and the app will crash at startup with No definition found for class: com.bitchat.android.mesh.BluetoothMeshService as soon as onCreate runs. A Koin definition (via @Single or DSL) is required for the mesh service and its dependencies.
Useful? React with 👍 / 👎.
Comment on lines
+17
to
19
| import kotlinx.serialization.json.* | ||
| import com.bitchat.android.util.JsonUtil | ||
| import com.google.gson.JsonSyntaxException |
There was a problem hiding this comment.
Gson was removed from libs.versions.toml and app/build.gradle.kts, yet LocationChannelManager still imports and catches com.google.gson.JsonSyntaxException. Without the Gson library on the classpath this import is unresolved and the module won’t compile. Replace the exception type or add the missing dependency.
Useful? React with 👍 / 👎.
- Annotate `PermissionManager` with `@Singleton` and `@Inject` to make it injectable via Koin. - Remove manual instantiation of `PermissionManager` in `MainActivity`. - Update `MainActivity` to retrieve the `PermissionManager` instance from the Koin graph using `by inject()`.
- Add `PeerFingerprintManager` dependency to the `PeerManagerTest` constructor. - Add a mocked `PeerFingerprintManager` to the `CommandProcessorTest` setup. - This aligns the test classes with recent constructor changes.
- Convert DebugPreferenceManager from object to @singleton class - Remove lateinit var prefs and init() method - Inject Context via constructor - Remove initialization from BitchatApplication - Inject into BluetoothMeshService - Convert PoWPreferenceManager from object to @singleton class - Remove lateinit var sharedPrefs and init() method - Inject Context via constructor - Update all usages to use injected instance: - AboutSheet, PoWStatusIndicator use koinInject() - GeohashViewModel, GeohashMessageHandler, NostrClient via constructor - NostrProtocol.createEphemeralGeohashEvent() takes as parameter - Inject DebugPreferenceManager into BluetoothMeshService - Replace static calls with instance methods - Use in GossipSyncManager configuration - Update ChatViewModel to inject LocationChannelManager and PoWPreferenceManager - Pass to GeohashViewModel instead of using Koin.get() Benefits: - No more static initialization or lateinit checks - Better testability with constructor injection - KMP ready - no Android-specific static contexts - Type-safe compile-time dependency resolution
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor: Migrate to Koin Annotations & Prepare for KMP
🚀 Summary
This PR executes a major architectural refactor to modernize the dependency injection strategy across the application. We have fully migrated from legacy
getInstance()singleton patterns to Koin Annotations, paving the way for a seamless transition to Kotlin Multiplatform (KMP) in the future.🔑 Key Changes
Koin Annotations Migration
getInstance(): Eliminated legacy static accessors in core services (e.g.,DebugSettingsManager,PeerFingerprintManager,SeenMessageStore).@Singleton&@Inject: All singletons are now managed by Koin using standard annotations, ensuring a single source of truth and lifecycle management.ChatViewModel) and Managers (e.g.,BluetoothConnectionManager) to receive dependencies via constructor injection instead of static lookups.🧪 Verification
🔮 Future Work
sharedKMP module.What is needed for approv?
Version and doc