Add SessionGenerator to generate the Session ID #4747
Conversation
Size Report 1Affected Products
Test Logs |
samedson
changed the title
firebase-sessions/src/main/kotlin/com/google/firebase/sessions/FirebaseSessions.kt
Outdated
Show resolved
Hide resolved
firebase-sessions/src/main/kotlin/com/google/firebase/sessions/SessionGenerator.kt
Outdated
Show resolved
Hide resolved
samedson
force-pushed
the
session-generator
branch
from
f260518 to
ff94345
Compare
firebase-sessions/src/main/kotlin/com/google/firebase/sessions/FirebaseSessions.kt
Show resolved
Hide resolved
| SessionInfo( | ||
| sessionId = "", | ||
| firstSessionId = "", | ||
| collectEvents = collectEvents, |
There was a problem hiding this comment.
Does SessionInfo need the collectEvents state?
There was a problem hiding this comment.
Yeah we'll use this later in the SDK. This class is almost identical to the iOS one.
| * | ||
| * @hide | ||
| */ | ||
| internal data class SessionInfo( |
There was a problem hiding this comment.
Is this fine to have a separate class defined inside another class? I see a need for this sessionInfo to be accessible outside of this context - should we move this to something more open for other classes to use?
There was a problem hiding this comment.
internal is like swift - it's internal to the compilation unit, so other classes in the SDK can use it.
|
|
||
| class SessionGeneratorTest { | ||
| fun isValidSessionId(sessionId: String): Boolean { | ||
| if (sessionId.length != 32) { |
There was a problem hiding this comment.
Is this a valid assumption? Why a specific need to be exactly 32 characters? I would rather recommend validating alpha numeric value of the sessionID.
There was a problem hiding this comment.
I believe this is a valid assumption of UUIDs - we have this in the iOS SDK too
| internal class SessionGenerator(collectEvents: Boolean) { | ||
| private var firstSessionId = "" | ||
| private var sessionIndex: Int = -1 | ||
| private var collectEvents = collectEvents |
There was a problem hiding this comment.
Do we plan to have a dynamic toggle of collectEvents state? Eg: SessionGenerator starts with the collectEvents state to be false and somewhere between turns to be true. Is this a usecase we want to support?
There was a problem hiding this comment.
Not anymore - we decided that the collectEvents state is consistent for the run of the app
Adds a SessionGenerator that rotates the Session ID and index.
#no-changelog