feat: check connection availability before fetching sourceConfig #103

1abhishekpandey · 2025-01-14T21:51:12Z

Description

In this PR, we’ve added logic to check and wait for the connection to be available before attempting to fetch the sourceConfig. This ensures that the sourceConfig is not fetched until the network is available.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Code refactor/optimization

Implementation Details

SourceConfigManager

First, we will fetch the cached sourceConfig from storage, if available, and immediately notify the observers with the cached sourceConfig.
Then, we will refresh the sourceConfig by downloading the latest version, store it, and notify all observers.
Added test cases: Rewrote them from scratch.

Core Analytics

Refactored the setup portion of the core Analytics

LoggerAnalytics

Improved LoggerAnalytics by handling edge cases. I noticed that if it’s not initialized early in the lifecycle and a message is logged, it throws a fatal exception. Now, in such cases, we simply ignore any logging requests.

Checklist

I have added tests that prove my fix is effective or that my feature works.
I have added the necessary documentation (if appropriate).
I have ensured that my code follows the project's code style.
I have checked for potential performance impacts and optimized if necessary.
I have checked the code for security issues.
I have updated the changelog (if required).

How to test?

Run the Android app and toggle on the connection. The SC will be immediately fetched, as soon as the connection becomes available.
Run the Android app on different SDK API versions i.e., compatible SDK version - API 24 and above and legacy once API 21 - 23.
Run the core module, and an attempt will be made to fetch the sourceConfig immediately, as we assume (or the default behaviour is) that the network is always available in core SDKs.
Try to enable and disable connection multiple times, but sourceConfig will only be fetched once.

Log:

SourceConfig is fetched successfully: SourceConfig(source=RudderServerConfigSource(sourceId=......

Breaking Changes

Maintainers Checklist

The code has been reviewed.
CI tests have passed.
All necessary documentation has been updated.

Screenshots (if applicable)

Additional Context

This is to ensure that we don't need to do un-necessary casting of application object

For better readability.

There are few refactors to improve the readability and maintainability of the code.

…eration begins

ChryssaAliferi

Great effort!! 🚀

vgupta98 · 2025-01-17T04:37:52Z

core/src/main/kotlin/com/rudderstack/sdk/kotlin/core/internals/models/SourceConfig.kt

@@ -15,6 +15,7 @@ import org.jetbrains.annotations.VisibleForTesting
 *
 * @property source The configuration details of a RudderStack source.
 */
+@Suppress("MaximumLineLength")


NIT: We don't need to suppress MaximumLineLength rule here. We can have updatedSourceConfig of NotifyObserversAction on a new line. For some reason, detekt fails to autocorrect it.

vgupta98 · 2025-01-17T04:38:29Z

core/src/main/kotlin/com/rudderstack/sdk/kotlin/core/internals/utils/ConnectivityUtils.kt

+fun Analytics.notifyOnlyOnceOnConnectionAvailable(block: suspend () -> Unit) {
+    this.analyticsScope.launch {
+        [email protected]
+            .filter { it }
+            .take(ONLY_ONE_ELEMENT)
+            .collect { block() }
+    }


vgupta98 · 2025-01-17T04:45:20Z

core/src/test/kotlin/com/rudderstack/sdk/kotlin/core/SourceConfigManagerTest.kt

    @Before
    fun setUp() {
-        Dispatchers.setMain(testDispatcher)


Remove the Dispatchers.resetMain() from teardown as well.

vgupta98 · 2025-01-17T04:49:44Z

core/src/main/kotlin/com/rudderstack/sdk/kotlin/core/internals/models/SourceConfig.kt

@@ -34,7 +35,7 @@ internal data class SourceConfig(
        )
    }

-    class UpdateAction(@VisibleForTesting internal val updatedSourceConfig: SourceConfig) : FlowAction<SourceConfig> {
+    class NotifyObserversAction(@VisibleForTesting internal val updatedSourceConfig: SourceConfig) : FlowAction<SourceConfig> {


Suggestion: IMO we should not call it NotifyObserversAction because notifying the observers is the inherent property of a MutableStateFlow and it is not the responsibility of this specific action. This action's responsibility is to just update (or set) the sourceConfig state. Thoughts?

Agree. Thanks for the clarification.

vgupta98 · 2025-01-17T04:51:37Z

core/src/main/kotlin/com/rudderstack/sdk/kotlin/core/Analytics.kt

+    private fun runForBaseTypeOnly() {
+        if (this::class == Analytics::class) {
+            setLogger(logger = KotlinLogger())
+            connectivityState.dispatch(ConnectivityState.SetDefaultStateAction())
+            setupSourceConfig()
+        }


Great job in detecting this subtle error about base and parent class!

…oved-implement-of-source-config-logic

As it is not needed.

1abhishekpandey added 29 commits January 14, 2025 18:39

chore: add ExceptionHandler.kt

edbe0d8

feat: add ConnectivityState.kt

a6c5959

feat: add AnalyticsConfiguration.connectivityState

ae789c5

feat(core): setup default connectivity state

57293de

chore(android): add AndroidUtils.kt

c813386

feat(android): add AndroidConnectivityObserverPlugin.kt

065c8e8

feat(android): setup AndroidConnectivityObserverPlugin in Analytics

2251d3a

chore: refactor AndroidConnectivityObserverPlugin.kt

c471908

refactor(android): create and use application extension function

91600af

This is to ensure that we don't need to do un-necessary casting of application object

chore(android): provide reason for suppression

2680cc7

test: add ExceptionHandlingTest.kt

3edd96a

refactor: move ConnectivityState inside models directory

34dc896

test: add ConnectivityStateTest.kt

62701e4

test: add AndroidUtilsTest.kt

4227d4b

test: improve AndroidUtilsTest.kt

29374c0

test: improve AndroidUtilsTest.kt

86174a6

refactor: separate ToggleStateAction into two explicit actions

8c478ce

For better readability.

refactor: make the AndroidConnectivityObserverPlugin unit testable

77e56fc

test: add AndroidConnectivityObserverTest.kt

5d4b1d1

test: fix AndroidConnectivityObserverTest.kt

34df954

refactor: change the file name to ExceptionHandlerUtils.kt

10897a7

chore: add ConnectivityUtils.kt

f76dad2

chore: add default safeExecute()

8d18d4a

fix: handle edge case in LoggerAnalytics.kt

e2995ac

refactor: rename SourceConfig.UpdateAction to NotifyObserversAction

bec8916

feat: check connection availability while fetching sourceConfig

3cd964b

There are few refactors to improve the readability and maintainability of the code.

test: refactor SourceConfigTest.kt

e21919e

test: re write test cases in SourceConfigManagerTest.kt

9ca3f41

refactor(core): sdk setup

c7f6527

1abhishekpandey self-assigned this Jan 14, 2025

1abhishekpandey marked this pull request as ready for review January 14, 2025 22:11

1abhishekpandey added the ready for review label Jan 14, 2025

1abhishekpandey requested review from ChryssaAliferi and vgupta98 January 14, 2025 22:11

refactor: ensure logger is always available before sourceConfig is op…

ea04a27

…eration begins

ChryssaAliferi approved these changes Jan 15, 2025

View reviewed changes

vgupta98 reviewed Jan 17, 2025

View reviewed changes

Base automatically changed from feat/sdk-2850-implementation-network-observer-using-plugin to develop January 17, 2025 05:49

1abhishekpandey added 4 commits January 17, 2025 11:23

Merge remote-tracking branch 'origin/develop' into feat/sdk-2849-impr…

1ab6c69

…oved-implement-of-source-config-logic

refactor: remove suppression of MaximumLineLength

4fdd189

test: remove teardown logic from SourceConfigManagerTest

52b5967

As it is not needed.

refactor: revert back to the UpdateAction

6b1c39c

1abhishekpandey requested a review from vgupta98 January 17, 2025 06:04

vgupta98 approved these changes Jan 17, 2025

View reviewed changes

1abhishekpandey merged commit fdbe891 into develop Jan 17, 2025
2 checks passed

1abhishekpandey deleted the feat/sdk-2849-improved-implement-of-source-config-logic branch January 17, 2025 09:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: check connection availability before fetching sourceConfig #103

feat: check connection availability before fetching sourceConfig #103

1abhishekpandey commented Jan 14, 2025 •

edited

Loading

ChryssaAliferi left a comment

vgupta98 Jan 17, 2025

vgupta98 Jan 17, 2025

vgupta98 Jan 17, 2025

vgupta98 Jan 17, 2025

1abhishekpandey Jan 17, 2025

vgupta98 Jan 17, 2025

feat: check connection availability before fetching sourceConfig #103

feat: check connection availability before fetching sourceConfig #103

Conversation

1abhishekpandey commented Jan 14, 2025 • edited Loading

Description

Type of change

Implementation Details

SourceConfigManager

Core Analytics

LoggerAnalytics

Checklist

How to test?

Breaking Changes

Maintainers Checklist

Screenshots (if applicable)

Additional Context

ChryssaAliferi left a comment

Choose a reason for hiding this comment

vgupta98 Jan 17, 2025

Choose a reason for hiding this comment

vgupta98 Jan 17, 2025

Choose a reason for hiding this comment

vgupta98 Jan 17, 2025

Choose a reason for hiding this comment

vgupta98 Jan 17, 2025

Choose a reason for hiding this comment

1abhishekpandey Jan 17, 2025

Choose a reason for hiding this comment

vgupta98 Jan 17, 2025

Choose a reason for hiding this comment

1abhishekpandey commented Jan 14, 2025 •

edited

Loading