feat: sdk 1008 manual session #404

itsdebs · 2024-03-11T07:40:22Z

Fixes # (issue)

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)
This change requires a documentation update

Checklist:

Version upgraded (project, README, gradle, podspec etc)
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added unit tests for the code
I have made corresponding changes to the documentation

feat: removed json adapter out of configuration. feat: changed to lazy configuration initialization to help restoring it from storage. Signed-off-by: Debanjan Chatterjee <[email protected]>

if trackAutoSesssion is true, manual session will continue till timeout Signed-off-by: Debanjan Chatterjee <[email protected]>

sonarcloud · 2024-03-11T07:41:02Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

session is not dependent on trackLifeCycle If previous session(manual or automatic) is not timed out, no new session will be created

# Conflicts: # android/src/main/java/com/rudderstack/android/RudderAnalytics.kt # android/src/main/java/com/rudderstack/android/compat/RudderAnalyticsBuilderCompat.java # android/src/test/java/com/rudderstack/android/android/RudderAnalyticsTest.kt # core/src/main/java/com/rudderstack/core/Analytics.kt # core/src/main/java/com/rudderstack/core/compat/AnalyticsBuilderCompat.java # core/src/main/java/com/rudderstack/core/compat/ConfigurationBuilder.java # core/src/test/java/com/rudderstack/core/internal/plugins/EventSizeFilterPluginTest.kt # core/src/test/java/com/rudderstack/core/internal/plugins/GDPRPluginTest.kt # samples/sample-kotlin-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/RudderAnalyticsUtils.kt # samples/sample-kotlin-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/Values.kt

ChryssaAliferi

Great effort with the analytics test and TestAnalytics provider!! But lets not create more generateTestAnalytics methods.
In this patch file I have refactored both generateTestAnalytics methods to one, the fun provideAnalytics(...):
feat__refactors_provideAnalytics_test_provider.patch

Take a look and let me know! This is way more organised and adjustable on all test/business logic cases.

Also I need us to merge the branch with the current feat/android because there are conflicts that make me unsure on what to review. When we do this I will try to review once more!

ChryssaAliferi · 2024-04-29T17:24:41Z

.../sample-kotlin-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/Values.kt

+const val WRITE_KEY = "2CYw61Oy8Wsrkh0ZHf65QDLYpDJ"
+const val DATA_PLANE_URL = "https://rudderstaclpl.dataplane.dev.rudderlabs.com/v1/batch"
+const val CONTROL_PLANE_URL = "https://api.dev.rudderlabs.com"
+const val BASE_URL = "https://BASE_URL"


We shouldn't merge back this to feat/android, as this has already been changed logic by using rudderstack.properties.

ChryssaAliferi · 2024-05-01T21:31:05Z

core/src/test/java/com/rudderstack/core/internal/plugins/GDPRPluginTest.kt

@@ -52,6 +52,8 @@ class GDPRPluginTest {
        val optOutTestChain = CentralPluginChain(message, listOf(gdprPlugin, testPluginForOptOut)
            , originalMessage = message)
        //opted out
+//        gdprPlugin.updateConfiguration(Configuration(
+//            isOptOut = true))


If this code is not needed, please let's remove.

ChryssaAliferi · 2024-05-01T21:32:49Z

core/src/test/java/com/rudderstack/core/internal/plugins/EventSizeFilterPluginTest.kt


 class EventSizeFilterPluginTest {
-
+    @get:Rule
+    val mockkRule = MockKRule(this)


As far as I remember we agreed as a team not to use Mockk yet. What did change? In my opinion Mockk should be the only way to move forward, and there are not many counter arguments, but using Mockk now seems very unaccountable and passes wrong messages to the team members. Let's change, or let's seriously evaluate this tools usage across the project.

This was implemented prior to the discussion. Thanks for pointing.

We can remove these in separate PR

ChryssaAliferi · 2024-05-01T21:55:34Z

core/src/main/java/com/rudderstack/core/internal/plugins/EventSizeFilterPlugin.kt

@@ -14,17 +16,24 @@ import java.util.concurrent.atomic.AtomicReference
 class EventSizeFilterPlugin : Plugin {

    private val currentConfigurationAtomic = AtomicReference<Configuration?>()
+    private var _analyticsRef = WeakReference<Analytics>(null)


private val currentConfigurationAtomic = AtomicReference<Configuration?>() private val currentConfiguration get() = currentConfigurationAtomic.get()

These are two class members that do the same thing. It is advised to avoid having class members that consume memory without any particular reason. A simplification of the class like below would be preferable:

package com.rudderstack.core.internal.plugins import com.rudderstack.core.Analytics import com.rudderstack.core.Configuration import com.rudderstack.core.Plugin import com.rudderstack.core.RudderUtils.MAX_EVENT_SIZE import com.rudderstack.core.RudderUtils.getUTF8Length import com.rudderstack.models.Message import com.rudderstack.rudderjsonadapter.RudderTypeAdapter import java.lang.ref.WeakReference import java.util.concurrent.atomic.AtomicReference /** * A plugin to filter out events that exceed the maximum size limit. */ class EventSizeFilterPlugin : Plugin { private val currentConfiguration = AtomicReference<Configuration?>(null) private var _analyticsRef = WeakReference<Analytics>(null) override fun setup(analytics: Analytics) { _analyticsRef = WeakReference(analytics) } override fun updateConfiguration(configuration: Configuration) { currentConfiguration.set(configuration) } override fun intercept(chain: Plugin.Chain): Message { currentConfiguration.get()?.let { config -> val messageJSON = analyticsRef.get()?.jsonAdapter?.writeToJson(chain.message(), object : RudderTypeAdapter<Message>() {}) val messageSize = messageJSON?.toString()?.getUTF8Length() ?: 0 if (messageSize > MAX_EVENT_SIZE) { config.logger.error(log = "Event size exceeds the maximum size of $MAX_EVENT_SIZE bytes. Dropping the event.") return chain.message() } } return chain.proceed(chain.message()) } override fun onShutDown() { _analyticsRef.clear() } }

Thank you for pointing

I have not been through this plugin, this plugin was part of the event filtering task, can we deal with it in a separate PR?
You can create a linear ticket for improvements.

ChryssaAliferi · 2024-05-01T22:09:42Z

android/src/main/java/com/rudderstack/android/utilities/SessionUtils.kt

+    ((currentConfigurationAndroid?.trackAutoSession != true)
+            && androidStorage.trackAutoSession)
+
+private fun Analytics.updateSessionLastActiveTimestamp() {


You can simplify and use directly the userSessionsState value, like that:

private fun Analytics.updateSessionLastActiveTimestamp() { userSessionState?.value?.let { it.copy(lastActiveTimestamp = defaultLastActiveTimestamp) } }

ChryssaAliferi · 2024-05-01T22:16:53Z

core/src/main/java/com/rudderstack/core/internal/ConfigDownloadServiceImpl.kt

@@ -31,7 +32,8 @@ import java.util.concurrent.atomic.AtomicReference

 internal class ConfigDownloadServiceImpl @JvmOverloads constructor(
    private val writeKey: String,
-    webService: WebService? = null,
+    private val jsonAdapter: JsonAdapter,
+    webService: WebService? = null


private val webService: WebService? = null

ChryssaAliferi · 2024-05-01T22:50:00Z

...-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/RudderAnalyticsUtils.kt

+//                dataPlaneUrl = DATA_PLANE_URL,
+//                controlPlaneUrl = CONTROL_PLANE_URL,
+//                recordScreenViews = true,
+                copy(


Please would you like to explain more about this copy method and that callback added here? Frankly, it does not look much user-friendly.

# Conflicts: # android/src/main/java/com/rudderstack/android/RudderAnalytics.kt # android/src/main/java/com/rudderstack/android/storage/RudderEntityFactory.kt # android/src/test/java/com/rudderstack/android/internal/plugins/ReinstatePluginTest.kt # core/src/main/java/com/rudderstack/core/internal/AnalyticsDelegate.kt # core/src/main/java/com/rudderstack/core/internal/DataUploadServiceImpl.kt # core/src/test/java/com/rudderstack/core/internal/ConfigDownloadServiceImplTest.kt # samples/sample-kotlin-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/RudderAnalyticsUtils.kt

sonarcloud · 2024-05-13T10:58:50Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

# Conflicts: # android/src/main/java/com/rudderstack/android/ConfigurationAndroid.kt # android/src/main/java/com/rudderstack/android/RudderAnalytics.kt # android/src/main/java/com/rudderstack/android/compat/ConfigurationAndroidBuilder.java # android/src/main/java/com/rudderstack/android/compat/RudderAnalyticsBuilderCompat.java # android/src/main/java/com/rudderstack/android/internal/plugins/PlatformInputsPlugin.kt # android/src/main/java/com/rudderstack/android/utilities/SessionUtils.kt # android/src/test/java/com/rudderstack/android/RudderAnalyticsTest.kt # android/src/test/java/com/rudderstack/android/internal/infrastructure/ReinstatePluginTest.kt # android/src/test/java/com/rudderstack/android/utilities/SessionUtilsTest.kt # core/src/main/java/com/rudderstack/core/Analytics.kt # core/src/main/java/com/rudderstack/core/Configuration.kt # core/src/main/java/com/rudderstack/core/compat/ConfigurationBuilder.java # core/src/main/java/com/rudderstack/core/internal/DataUploadServiceImpl.kt # core/src/test/java/com/rudderstack/core/AnalyticsTest.kt # core/src/test/java/com/rudderstack/core/compat/ConfigurationBuilderTest.java # samples/sample-kotlin-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/RudderAnalyticsUtils.kt # samples/sample-kotlin-android/src/main/kotlin/com/rudderstack/android/sampleapp/mainview/MainViewModel.kt

# Conflicts: # android/src/main/java/com/rudderstack/android/ConfigurationAndroid.kt # android/src/main/java/com/rudderstack/android/compat/ConfigurationAndroidBuilder.java # android/src/main/java/com/rudderstack/android/storage/AndroidStorageImpl.kt # android/src/test/java/com/rudderstack/android/AndroidStorageTest.kt # android/src/test/java/com/rudderstack/android/RudderAnalyticsTest.kt # android/src/test/java/com/rudderstack/android/internal/infrastructure/ReinstatePluginTest.kt # android/src/test/java/com/rudderstack/android/plugins/PlatformInputsPluginTest.kt # android/src/test/java/com/rudderstack/android/utilities/SessionUtilsTest.kt # core/src/main/java/com/rudderstack/core/internal/AnalyticsDelegate.kt # core/src/test/java/com/rudderstack/core/compat/ConfigurationBuilderTest.java # samples/sample-kotlin-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/RudderAnalyticsUtils.kt

chore: fix tests

1abhishekpandey · 2024-06-25T15:37:19Z

...-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/RudderAnalyticsUtils.kt

+            configurationScope = {
+                dataPlaneUrl = DATA_PLANE_URL
+                controlPlaneUrl = CONTROL_PLANE_URL
+                trackLifecycleEvents = true
+                recordScreenViews = true
+                isPeriodicFlushEnabled = true
+                autoCollectAdvertisingId = true
+                trackAutoSession = true
+                logLevel = RudderLogger.LogLevel.DEBUG
+            }


Can you please revert this change for now?

I tried to disable the lifecycle flag to false and after that also I can see the lifecycle events in our dashboard.

feat: reset will now refresh the session id if it exists fix: manual session continuation

sonarcloud · 2024-06-28T09:03:24Z

Quality Gate passed

Issues
24 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

itsdebs added 2 commits March 7, 2024 19:57

feat: Removed application out of ConfigurationAndroid.

f327ebc

feat: removed json adapter out of configuration. feat: changed to lazy configuration initialization to help restoring it from storage. Signed-off-by: Debanjan Chatterjee <[email protected]>

fix: manual session to continue if track auto session is false

0c823c7

if trackAutoSesssion is true, manual session will continue till timeout Signed-off-by: Debanjan Chatterjee <[email protected]>

itsdebs requested review from ChryssaAliferi and desusai7 March 11, 2024 07:40

fix: according to spec changes

a33221d

session is not dependent on trackLifeCycle If previous session(manual or automatic) is not timed out, no new session will be created

ChryssaAliferi marked this pull request as ready for review April 29, 2024 07:54

ChryssaAliferi requested changes Apr 29, 2024

View reviewed changes

fix: fix merging. fix test cases

a4ab265

ChryssaAliferi reviewed May 1, 2024

View reviewed changes

itsdebs added 5 commits May 9, 2024 12:55

feat: merge commit

941808b

feat: change applyConfig signature

2cb3c79

fix: rudder analytics utils config initializer

00a5452

fix: save auto track session

7f68b89

itsdebs added the v2 label Jun 18, 2024

itsdebs added 5 commits June 19, 2024 14:51

chore: merge changes

a10bbeb

chore: fix tests

fix: issue with configuration scope

9acc606

fix: change session id to long instead of string

1084ca2

1abhishekpandey requested review from 1abhishekpandey and removed request for desusai7 June 25, 2024 15:05

1abhishekpandey requested changes Jun 25, 2024

View reviewed changes

itsdebs added 3 commits June 26, 2024 16:03

fix: trackLifecycle field fix

fcec638

fix: dependency on trackLifecycle removed.

5b7c28a

feat: Session won't expire in a single run.

3ac0537

feat: reset will now refresh the session id if it exists fix: manual session continuation

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: sdk 1008 manual session #404

feat: sdk 1008 manual session #404

itsdebs commented Mar 11, 2024

sonarcloud bot commented Mar 11, 2024

ChryssaAliferi left a comment

ChryssaAliferi Apr 29, 2024

ChryssaAliferi May 1, 2024

ChryssaAliferi May 1, 2024

itsdebs May 2, 2024 •

edited

Loading

itsdebs May 8, 2024

ChryssaAliferi May 1, 2024

itsdebs May 2, 2024

itsdebs May 8, 2024

ChryssaAliferi May 1, 2024

ChryssaAliferi May 1, 2024

ChryssaAliferi May 1, 2024

sonarcloud bot commented May 13, 2024

1abhishekpandey Jun 25, 2024

sonarcloud bot commented Jun 28, 2024

feat: sdk 1008 manual session #404

Are you sure you want to change the base?

feat: sdk 1008 manual session #404

Conversation

itsdebs commented Mar 11, 2024

Type of change

Checklist:

sonarcloud bot commented Mar 11, 2024

Quality Gate passed

ChryssaAliferi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsdebs May 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented May 13, 2024

Quality Gate passed

Choose a reason for hiding this comment

sonarcloud bot commented Jun 28, 2024

Quality Gate passed

itsdebs May 2, 2024 •

edited

Loading