[SDK - 4890] Network Info fetch and internet check in Android #903
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
Included Network Monitor for internet observeability and wifi connectivity checks.
Contributor
WalkthroughThe PR introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Factory as CleverTapFactory
participant NM as NetworkMonitor
participant CM as ConnectivityManager
participant EventQ as EventQueueManager
participant InApp as InAppController
Factory->>NM: new NetworkMonitor(context, config)
activate NM
NM->>CM: getActiveNetwork()
NM->>CM: registerNetworkCallback()
NM->>NM: computeInitialState()
deactivate NM
Factory->>EventQ: BaseEventQueueManager(networkMonitor)
Factory->>InApp: InAppController(networkMonitor)
Note over NM,CM: During Runtime
CM-->>NM: onAvailable / onLost
NM->>NM: updateNetworkState()
NM->>NM: emit to networkState Flow
EventQ->>NM: isNetworkOnline()
InApp->>NM: isNetworkOnline()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt (1)
644-644: Fix inconsistent reference to NetworkManager.Line 644 still references
NetworkManager.isNetworkOnline(any()), but this should beNetworkMonitor.isNetworkOnline(any())to match the other changes in this test file (lines 24, 62, 83-85). This inconsistency will likely cause compilation or test failures.Apply this diff:
- every { NetworkManager.isNetworkOnline(any()) } returns false + every { NetworkMonitor.isNetworkOnline(any()) } returns false
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
clevertap-core/src/main/AndroidManifest.xml(1 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.kt(5 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.kt(2 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/bitmap/BitmapDownloadRequestHandler.kt(2 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/events/EventQueueManager.java(7 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt(3 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkManager.kt(0 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkMonitor.kt(1 hunks)clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt(3 hunks)
💤 Files with no reviewable changes (1)
- clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkManager.kt
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 798
File: clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkManager.kt:0-0
Timestamp: 2025-07-01T13:00:26.100Z
Learning: In the CleverTap Android SDK's encryption system, the NetworkManager's behavior of silently falling back to unencrypted requests when encryption fails (EncryptionFailure) is intentional by design, not a security oversight. This fallback ensures request reliability and is planned to be documented.
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 866
File: clevertap-core/src/main/java/com/clevertap/android/sdk/ManifestInfo.java:266-268
Timestamp: 2025-10-07T14:10:07.979Z
Learning: In the CleverTap Android SDK, when reviewing visibility modifiers for getters in ManifestInfo.java, the team prefers to defer access control standardization to a future dedicated task rather than addressing individual getters incrementally during unrelated PRs.
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 830
File: clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.kt:223-240
Timestamp: 2025-06-27T10:11:22.171Z
Learning: In the CleverTap Android SDK, the asynchronous initialization of inAppFCManager in CleverTapFactory.kt is intentional behavior. It's acceptable for inAppFCManager to remain null initially after CoreState creation, as it will be created in another background thread later in the workflow if needed. This async pattern is designed to avoid blocking the main initialization flow.
📚 Learning: 2025-06-30T13:08:50.158Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 829
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java:71-81
Timestamp: 2025-06-30T13:08:50.158Z
Learning: In the CleverTap Android SDK InAppController class, the static `currentlyDisplayingInApp` field is intentionally designed to ensure only one in-app notification can be displayed on the UI at any given time across the entire application, regardless of how many InAppController instances exist. This prevents multiple in-app notifications from overlapping or competing for screen space.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt
📚 Learning: 2025-06-27T10:11:22.171Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 830
File: clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.kt:223-240
Timestamp: 2025-06-27T10:11:22.171Z
Learning: In the CleverTap Android SDK, the asynchronous initialization of inAppFCManager in CleverTapFactory.kt is intentional behavior. It's acceptable for inAppFCManager to remain null initially after CoreState creation, as it will be created in another background thread later in the workflow if needed. This async pattern is designed to avoid blocking the main initialization flow.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.ktclevertap-core/src/main/java/com/clevertap/android/sdk/events/EventQueueManager.java
📚 Learning: 2025-07-09T13:28:16.360Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 846
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.kt:412-417
Timestamp: 2025-07-09T13:28:16.360Z
Learning: In the CleverTap Android SDK CTInAppNotification class, CTLalit considers the media validation logic that mutates mediaUrl properties as legacy code and prefers not to refactor it, even when mutation could be avoided through filtering approaches.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.ktclevertap-core/src/main/AndroidManifest.xml
📚 Learning: 2025-07-09T13:28:25.364Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 846
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.kt:422-432
Timestamp: 2025-07-09T13:28:25.364Z
Learning: CTLalit considers the CTInAppNotification class in the CleverTap Android SDK as legacy code and prefers not to refactor it, even when the code has potential improvements like media validation logic that could be more efficient.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.ktclevertap-core/src/main/java/com/clevertap/android/sdk/events/EventQueueManager.javaclevertap-core/src/main/AndroidManifest.xml
📚 Learning: 2025-07-09T13:27:32.141Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 846
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppNativeHalfInterstitialImageFragment.kt:20-143
Timestamp: 2025-07-09T13:27:32.141Z
Learning: In the CleverTap Android SDK, CTLalit considers the CTInAppNativeHalfInterstitialImageFragment class as legacy code and prefers not to refactor it, even when the code has complex methods that could benefit from restructuring.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.ktclevertap-core/src/main/java/com/clevertap/android/sdk/events/EventQueueManager.java
📚 Learning: 2025-04-28T08:41:36.134Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 797
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:184-185
Timestamp: 2025-04-28T08:41:36.134Z
Learning: In the CleverTap Android SDK, parcels in the CTInAppNotification class exist only in memory and are not persisted, so backward compatibility concerns between different SDK versions are not applicable for these parcels.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.ktclevertap-core/src/main/AndroidManifest.xml
📚 Learning: 2025-05-08T08:25:14.692Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppBaseFullHtmlFragment.java:44-48
Timestamp: 2025-05-08T08:25:14.692Z
Learning: In the CleverTap Android SDK, fragments are managed using add and remove methods rather than replace, so resources are properly cleaned up in onDestroyView() without needing additional cleanup in onDestroy().
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt
📚 Learning: 2025-06-17T13:54:08.138Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 830
File: clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java:168-169
Timestamp: 2025-06-17T13:54:08.138Z
Learning: In the CleverTap Android SDK, the static mutable clevertapClock field in CleverTapAPI.java is acceptable because all instances consistently use Clock.SYSTEM, so the mutation doesn't cause practical multi-instance issues in their use case.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt
📚 Learning: 2025-06-27T10:13:58.579Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 830
File: clevertap-core/src/test/java/com/clevertap/android/sdk/MockCoreState.kt:37-38
Timestamp: 2025-06-27T10:13:58.579Z
Learning: In the CleverTap Android SDK tests, MockCTExecutors has a no-argument constructor that provides default mock implementations, so it doesn't require CleverTapInstanceConfig parameter like the production CTExecutors class does.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.ktclevertap-core/src/main/java/com/clevertap/android/sdk/events/EventQueueManager.java
📚 Learning: 2025-04-28T09:52:18.094Z
Learnt from: vasct
Repo: CleverTap/clevertap-android-sdk PR: 797
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inbox/CTInboxActivity.java:89-92
Timestamp: 2025-04-28T09:52:18.094Z
Learning: In the CleverTap Android SDK, if a CleverTapAPI instance is not null, then its getCoreState() method will never return null because CoreState is initialized in the constructor and set via setCoreState(). The CleverTapFactory.getCoreState() method throws a RuntimeException if it can't create a valid state.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.ktclevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.kt
📚 Learning: 2025-10-07T14:10:07.979Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 866
File: clevertap-core/src/main/java/com/clevertap/android/sdk/ManifestInfo.java:266-268
Timestamp: 2025-10-07T14:10:07.979Z
Learning: In the CleverTap Android SDK, when reviewing visibility modifiers for getters in ManifestInfo.java, the team prefers to defer access control standardization to a future dedicated task rather than addressing individual getters incrementally during unrelated PRs.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.ktclevertap-core/src/main/AndroidManifest.xml
📚 Learning: 2025-07-01T13:00:26.100Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 798
File: clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkManager.kt:0-0
Timestamp: 2025-07-01T13:00:26.100Z
Learning: In the CleverTap Android SDK's encryption system, the NetworkManager's behavior of silently falling back to unencrypted requests when encryption fails (EncryptionFailure) is intentional by design, not a security oversight. This fallback ensures request reliability and is planned to be documented.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.ktclevertap-core/src/main/java/com/clevertap/android/sdk/events/EventQueueManager.java
📚 Learning: 2025-07-01T12:49:30.819Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 798
File: clevertap-core/src/main/java/com/clevertap/android/sdk/network/api/EncryptedResponseBody.kt:10-16
Timestamp: 2025-07-01T12:49:30.819Z
Learning: In the CleverTap Android SDK's encryption system, exception handling for JSON parsing in EncryptedResponseBody.fromJsonString() is intentionally handled at the call site level (in NetworkEncryptionManager.decryptResponse) rather than within the parsing method itself, following a centralized exception handling pattern.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.kt
📚 Learning: 2025-06-24T05:54:46.949Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 798
File: clevertap-core/src/main/java/com/clevertap/android/sdk/network/IJRepo.kt:50-54
Timestamp: 2025-06-24T05:54:46.949Z
Learning: In CleverTap Android SDK's IJRepo class, the IJ SharedPreferences namespace is dedicated exclusively to storing the I and J values, so using editor.clear() in clearIJ() is appropriate and intended, rather than removing individual keys.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.kt
📚 Learning: 2025-06-06T10:41:32.111Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 798
File: clevertap-core/src/main/java/com/clevertap/android/sdk/cryption/CryptRepository.kt:64-66
Timestamp: 2025-06-06T10:41:32.111Z
Learning: In the CleverTap Android SDK's encryption system, the CryptRepository's plain SharedPreferences storage for encryption keys is intentionally designed as a fallback mechanism for Android API levels below 23, where Android Keystore is not available. The primary secure storage path uses Android Keystore for API 23+ devices, which represents the majority of modern client deployments. This is part of the backward compatibility strategy.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.kt
📚 Learning: 2025-09-22T04:00:43.536Z
Learnt from: Anush-Shand
Repo: CleverTap/clevertap-android-sdk PR: 874
File: clevertap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/content/ProductDisplayLinearBigContentView.kt:144-144
Timestamp: 2025-09-22T04:00:43.536Z
Learning: In ProductDisplayLinearBigContentView.kt, the `imageUrl!!` non-null assertion is safe because it's guarded by the `!fallback` condition. When imageUrl is null, loadImageURLIntoRemoteView returns true (fallback = true), so the line with `imageUrl!!` never executes.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/bitmap/BitmapDownloadRequestHandler.kt
📚 Learning: 2025-09-29T11:43:18.372Z
Learnt from: Anush-Shand
Repo: CleverTap/clevertap-android-sdk PR: 865
File: clevertap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/media/TemplateMediaManager.kt:15-18
Timestamp: 2025-09-29T11:43:18.372Z
Learning: For TemplateMediaManager in CleverTap Android SDK push templates: Simple mutable map caches are preferred over LRU cache implementations as the use case doesn't warrant the additional complexity. The cache clearing mechanism via clearCaches() and bounded nature of push template usage makes LRU unnecessary.
Applied to files:
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt
📚 Learning: 2025-04-28T08:57:50.096Z
Learnt from: vasct
Repo: CleverTap/clevertap-android-sdk PR: 797
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/customtemplates/TemplatesManager.kt:34-42
Timestamp: 2025-04-28T08:57:50.096Z
Learning: In the CleverTap Android SDK, the CustomTemplate class implements equals() and hashCode() to compare templates based on their name property, enabling collections to correctly identify duplicate templates without additional explicit name comparisons.
Applied to files:
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt
📚 Learning: 2025-09-24T12:32:37.589Z
Learnt from: Anush-Shand
Repo: CleverTap/clevertap-android-sdk PR: 885
File: clevertap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/validators/TimerTemplateValidator.kt:11-13
Timestamp: 2025-09-24T12:32:37.589Z
Learning: In CleverTap push templates validation system, the TimerTemplateValidator uses legacy patterns with non-null assertions (e.g., keys[PT_TIMER_DISMISS_AFTER]!!) which are acceptable because the validation system guarantees these keys exist in the keys map.
Applied to files:
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt
📚 Learning: 2025-09-22T04:44:06.841Z
Learnt from: Anush-Shand
Repo: CleverTap/clevertap-android-sdk PR: 874
File: clevertap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/handlers/CancelTemplateHandler.kt:10-21
Timestamp: 2025-09-22T04:44:06.841Z
Learning: The CancelTemplateHandler.renderCancelNotification method in the CleverTap Android SDK push templates is legacy code. When flagging potential safety issues like NumberFormatException handling or null service guards, the team prefers to defer such improvements rather than modify legacy code during feature-focused PRs.
Applied to files:
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt
📚 Learning: 2025-06-27T10:13:58.579Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 830
File: clevertap-core/src/test/java/com/clevertap/android/sdk/MockCoreState.kt:37-38
Timestamp: 2025-06-27T10:13:58.579Z
Learning: In the CleverTap Android SDK tests, MockCTExecutors has a JvmOverloads constructor with a default parameter (config: CleverTapInstanceConfig? = null), so it can be instantiated without arguments using MockCTExecutors() for testing purposes.
Applied to files:
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt
📚 Learning: 2025-09-24T12:08:40.196Z
Learnt from: Anush-Shand
Repo: CleverTap/clevertap-android-sdk PR: 872
File: clevertap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/styles/Style.kt:39-45
Timestamp: 2025-09-24T12:08:40.196Z
Learning: The Style.kt setNotificationBuilderBasics method in the CleverTap Android SDK push templates is legacy code. When flagging potential safety issues like NPE from Html.fromHtml(pt_title) with null values or Color.parseColor crashes with blank strings, the team prefers to defer such improvements rather than modify legacy code during feature-focused PRs.
Applied to files:
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/InAppControllerTest.kt
📚 Learning: 2025-06-05T11:55:02.777Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 798
File: clevertap-core/src/main/java/com/clevertap/android/sdk/network/http/NetworkRepo.kt:0-0
Timestamp: 2025-06-05T11:55:02.777Z
Learning: In CleverTap Android SDK, StorageHelper.getIntFromPrefs(context, config, key, default) and StorageHelper.putInt(context, StorageHelper.storageKeyWithSuffix(config.accountId, key), value) are consistent in their internal key handling despite appearing to use different patterns at the API level. The existing usage patterns in NetworkRepo.kt are correct and intentional.
Applied to files:
clevertap-core/src/main/java/com/clevertap/android/sdk/CoreState.kt
📚 Learning: 2025-05-08T08:31:50.340Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:180-182
Timestamp: 2025-05-08T08:31:50.340Z
Learning: The CTInAppNotification class in the CleverTap Android SDK adds new fields `aspectRatio` and `isRequestForPushPermission` to its Parcel implementation in version 7.4.0, but doesn't require parcel versioning as these parcels only exist in memory during a single app session.
Applied to files:
clevertap-core/src/main/AndroidManifest.xml
📚 Learning: 2025-01-24T11:56:39.526Z
Learnt from: CTLalit
Repo: CleverTap/clevertap-android-sdk PR: 741
File: clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.java:1143-1143
Timestamp: 2025-01-24T11:56:39.526Z
Learning: Privacy and security related changes in the CleverTap Android SDK, such as PII exposure in logs, should be discussed with the team before implementation to ensure proper handling of sensitive data.
Applied to files:
clevertap-core/src/main/AndroidManifest.xml
📚 Learning: 2025-05-28T11:10:51.166Z
Learnt from: Anush-Shand
Repo: CleverTap/clevertap-android-sdk PR: 734
File: clevertap-pushtemplates/src/main/res/layout-v31/timer.xml:58-60
Timestamp: 2025-05-28T11:10:51.166Z
Learning: For API 31+ (Android 12+) in CleverTap push templates, action buttons are handled through official Android notification APIs rather than custom layout includes. The API 31+ layout versions intentionally omit layout/action_buttons inclusions because the action buttons are managed by the Android system's notification builder APIs directly.
Applied to files:
clevertap-core/src/main/AndroidManifest.xml
🧬 Code graph analysis (1)
clevertap-core/src/main/java/com/clevertap/android/sdk/events/EventQueueManager.java (1)
clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkMonitor.kt (2)
isNetworkOnline(23-26)isNetworkOnline(204-204)
⏰ 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). (1)
- GitHub Check: lint-staticChecks-test-build
🔇 Additional comments (2)
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt (1)
74-74: LGTM! Clean dependency injection.The
networkMonitorparameter is properly added to the constructor and used for network checks at line 660. This follows dependency injection best practices and aligns with the PR's objective to introduce NetworkMonitor for connectivity management.clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.kt (1)
362-365: LGTM! NetworkMonitor properly instantiated.The NetworkMonitor is cleanly instantiated with necessary dependencies (context and config) and correctly wired into the dependency graph, being passed to EventQueueManager (line 384), InAppController (line 441), and exposed via CoreState (line 518).
Comment on lines
+96
to
+97
| private var _currentState: NetworkState = NetworkState.DISCONNECTED | ||
|
|
Contributor
There was a problem hiding this comment.
Mark _currentState as volatile (or use an atomic)
ConnectivityManager.NetworkCallback delivers updates on binder threads while consumers like EventQueueManager query isNetworkOnline() from other threads. Without a happens-before edge, _currentState updates can remain invisible, keeping callers stuck in the wrong connectivity mode. Please mark _currentState as @Volatile (or wrap it in an AtomicReference) so cross-thread readers always observe the latest state.
@Volatile
private var _currentState: NetworkState = NetworkState.DISCONNECTED
🤖 Prompt for AI Agents
In
clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkMonitor.kt
around lines 96-97, the private field _currentState is updated from
ConnectivityManager.NetworkCallback on binder threads but read from other
threads, so make the update visible across threads: mark _currentState as
@Volatile (or replace it with an AtomicReference<NetworkState>) and update the
usages accordingly so readers always observe the latest state.
Comment on lines
+155
to
+197
| private fun registerNetworkCallback() { | ||
| val callback = object : ConnectivityManager.NetworkCallback() { | ||
| override fun onAvailable(network: Network) { | ||
| updateInternalNetworkState() // Update internal state | ||
| logger.verbose(config.accountId, "Network became available: ${_currentState.networkType}") | ||
| } | ||
|
|
||
| override fun onLost(network: Network) { | ||
| updateInternalNetworkState() // Update internal state | ||
| logger.verbose(config.accountId, "Network lost") | ||
| } | ||
|
|
||
| override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) { | ||
| updateInternalNetworkState() // Update internal state | ||
| logger.verbose(config.accountId, "Network capabilities changed: ${_currentState.networkType}") | ||
| } | ||
|
|
||
| override fun onUnavailable() { | ||
| updateInternalNetworkState() // Update internal state | ||
| logger.verbose(config.accountId, "Network unavailable") | ||
| } | ||
| } | ||
|
|
||
| networkCallback = callback | ||
|
|
||
| try { | ||
| val networkRequest = NetworkRequest.Builder() | ||
| .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) | ||
| .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) | ||
| .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) | ||
| .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET) | ||
| .addTransportType(NetworkCapabilities.TRANSPORT_VPN) | ||
| .build() | ||
|
|
||
| connectivityManager?.registerNetworkCallback(networkRequest, callback) | ||
| logger.verbose(config.accountId, "Network callback registered successfully") | ||
|
|
||
| } catch (e: SecurityException) { | ||
| logger.debug(config.accountId, "Network callback registration failed: ${e.message}") | ||
| } catch (e: Exception) { | ||
| logger.debug(config.accountId, "Network callback registration failed: ${e.message}") | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Guard network callbacks for pre-Lollipop and keep state fresh
ConnectivityManager.registerNetworkCallback(...) only exists from API 21. Calling it on older devices throws NoSuchMethodError, and even if you guard the call later, _currentState would never refresh because callbacks aren’t supported there. Please feature-detect the API level before building the request, skip registration pre-L, and fall back to synchronous checks when networkCallback is null. Example:
- private fun registerNetworkCallback() {
- val callback = object : ConnectivityManager.NetworkCallback() {
+ private val supportsCallbacks = Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP
+
+ private fun registerNetworkCallback() {
+ if (!supportsCallbacks) {
+ logger.verbose(config.accountId, "Network callbacks unsupported pre-L; relying on synchronous checks")
+ return
+ }
+ val callback = object : ConnectivityManager.NetworkCallback() {
...
}
networkCallback = callback
-
- try {
- val networkRequest = NetworkRequest.Builder()
+ try {
+ val networkRequest = NetworkRequest.Builder()
...
connectivityManager?.registerNetworkCallback(networkRequest, callback)
logger.verbose(config.accountId, "Network callback registered successfully")And in the synchronous API surface do a quick recomputation when callbacks aren’t available:
- fun isNetworkOnline(): Boolean = _currentState.isAvailable
+ fun isNetworkOnline(): Boolean = if (supportsCallbacks) {
+ _currentState.isAvailable
+ } else {
+ isNetworkConnected(context).also { updateInternalNetworkState() }
+ }This prevents crashes on legacy devices and keeps behaviour aligned with the old per-call polling.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
clevertap-core/src/main/java/com/clevertap/android/sdk/network/NetworkMonitor.kt
around lines 155 to 197, guard usage of
ConnectivityManager.registerNetworkCallback with an API-level check (only call
and build NetworkRequest/register callback when API >= 21) and catch/handle
NoSuchMethodError; for pre-L devices skip registration (leave networkCallback
null), immediately call updateInternalNetworkState() so _currentState is
recomputed, and ensure all public synchronous network-check paths treat a null
networkCallback as “callbacks unsupported” and perform the synchronous
polling/recomputation flow instead; keep the existing logging for
success/failure but log and skip registration on older APIs to avoid crashes and
preserve state freshness.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Included Network Monitor for internet observeability and wifi connectivity checks.
Summary by CodeRabbit
Release Notes
New Features
Chores