moves all non‑UI logic #515
Closed
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
- 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
This commit centralizes UI-related business logic from various composables into `ChatViewModel` to create a single source of truth and simplify the UI layer.
- **Introduce `ChatViewModel` to UI Sheets:**
- `AboutSheet`, `LocationChannelsSheet`, `LocationNotesSheet`, and related presenters now accept a `ChatViewModel` instance.
- Direct dependency injection (`koinInject`) is removed from these composables.
- **Delegate UI Actions to ViewModel:**
- User actions (e.g., changing theme, Tor mode, PoW settings, managing location channels, sending notes) are now handled by methods in `ChatViewModel`.
- UI components call `viewModel` methods instead of interacting directly with managers (`LocationChannelManager`, `LocationNotesManager`, `TorManager`, etc.).
- **Expose State via ViewModel:**
- LiveData and StateFlows from various managers are now exposed directly from `ChatViewModel`, providing a unified state management approach.
- This includes location channel state, location notes, geohash bookmarks, Tor status, and theme/PoW preferences.
- **Simplify Composable Logic:**
- Removed redundant manager instantiations and state observations from individual composable files, making them more declarative and easier to maintain.
- Move geohash teleport logic from `MessageComponents.kt` into a new `teleportToGeohash` method in `ChatViewModel`. - Add an `onGeohashClick` callback to `MessageList` and `MessageBubble` to trigger the new ViewModel method. - Refactor `PoWStatusIndicator` to receive PoW state (enabled, difficulty, mining status) from `ChatViewModel` instead of injecting `PoWPreferenceManager` directly. - Update `ChatHeader` and `PoWStatusIndicator` to pass the `ChatViewModel` instance.
![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
+6
to
+8
| @Module | ||
| @ComponentScan("com.bitchat.android") | ||
| class AppModule |
There was a problem hiding this comment.
MainActivity now relies on Koin (by inject() for PermissionManager/BluetoothMeshService and by viewModel() for ChatViewModel), but the only module registered at startup is AppModule, which just component-scans com.bitchat.android without any Koin @Single/@Factory bindings. All the managers were annotated with jakarta.inject.Singleton instead of Koin’s annotations, so the generated module is effectively empty and Koin cannot provide the injected dependencies. At runtime the app will throw NoBeanDefFoundException as soon as MainActivity is created, blocking startup. Consider registering the bindings explicitly or switching the classes to Koin annotations so the component scan produces definitions.
Useful? React with 👍 / 👎.
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.
Overview
This PR moves all non‑UI logic (data initialization, domain concepts, and manager interactions) from UI components into the central
ChatViewModel. The UI now only observes state and triggers actions via the ViewModel, achieving a clean MVVM architecture.Why This Change?
ChatViewModel.Key Changes
ChatViewModel.ktLocationNotesManager,TorManager,GeohashBookmarksStore. Exposed LiveData/StateFlow for location notes, channels, bookmarks, Tor status, theme, PoW. Added methods for teleporting, toggling bookmarks, enabling/disabling services, and settings updates.ChatScreen.ktonGeohashClickcallback that callsviewModel.teleportToGeohash. Updated usage ofMessagesList.ChatHeader.ktTorManager&GeohashBookmarksStoreinjections. Now usesviewModelfor Tor status, bookmarks, and PoW indicator.LocationNotesButton.ktviewModel.AboutSheet.ktChatViewModeland uses it for theme, PoW, and Tor settings.LocationNotesSheet.ktChatViewModelfor notes state and actions.LocationNotesSheetPresenter.ktChatViewModelfor channel data and location actions.LocationChannelsSheet.ktChatViewModelfor channel selection, bookmarks, and location services.PoWStatusIndicator.ktChatViewModeland observes PoW state (powEnabled,powDifficulty,isMining).MessageComponents.ktLocationChannelManagerinjection. AddedonGeohashClickcallback toMessagesList,MessageItem, andMessageTextWithClickableNicknames. Cleaned duplicate parameters.Benefits
ChatViewModel.Whats need