Skip to content

Add SessionGenerator to generate the Session ID #4747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import com.google.firebase.ktx.Firebase
import com.google.firebase.ktx.app

class FirebaseSessions internal constructor(firebaseApp: FirebaseApp) {

private val sessionGenerator = SessionGenerator(collectEvents = true)

init {
val sessionInitiator = SessionInitiator(WallClock::elapsedRealtime, this::initiateSessionStart)
val context = firebaseApp.applicationContext.applicationContext
Expand All @@ -40,6 +43,8 @@ class FirebaseSessions internal constructor(firebaseApp: FirebaseApp) {
private fun initiateSessionStart() {
// TODO(mrober): Generate a session
Log.i(TAG, "Initiate session start")

sessionGenerator.generateNewSession()
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.firebase.sessions

import java.util.UUID

/**
* [SessionInfo] is a data class responsible for storing information about the current Session.
*
* @hide
*/
internal data class SessionInfo(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal is like swift - it's internal to the compilation unit, so other classes in the SDK can use it.

val sessionId: String,
val firstSessionId: String,
val collectEvents: Boolean,
val sessionIndex: Int,
)

/**
* The [SessionGenerator] is responsible for generating the Session ID, and keeping the
* [SessionInfo] up to date with the latest values.
*
* @hide
*/
internal class SessionGenerator(collectEvents: Boolean) {
private var firstSessionId = ""
private var sessionIndex: Int = -1
private var collectEvents = collectEvents
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not anymore - we decided that the collectEvents state is consistent for the run of the app


private var thisSession: SessionInfo =
SessionInfo(
sessionId = "",
firstSessionId = "",
collectEvents = collectEvents,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does SessionInfo need the collectEvents state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we'll use this later in the SDK. This class is almost identical to the iOS one.

sessionIndex = sessionIndex
)

// Generates a new Session ID. If there was already a generated Session ID
// from the last session during the app's lifecycle, it will also set the last Session ID
fun generateNewSession() {
val newSessionId = UUID.randomUUID().toString().replace("-", "").lowercase()

// If firstSessionId is set, use it. Otherwise set it to the
// first generated Session ID
firstSessionId = if (firstSessionId.isEmpty()) newSessionId else firstSessionId

sessionIndex += 1

thisSession =
SessionInfo(
sessionId = newSessionId,
firstSessionId = firstSessionId,
collectEvents = collectEvents,
sessionIndex = sessionIndex
)
}

val currentSession: SessionInfo
get() = thisSession
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.firebase.sessions

import com.google.common.truth.Truth.assertThat
import org.junit.Test

class SessionGeneratorTest {
fun isValidSessionId(sessionId: String): Boolean {
if (sessionId.length != 32) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is a valid assumption of UUIDs - we have this in the iOS SDK too

return false
}
if (sessionId.contains("-")) {
return false
}
if (sessionId.lowercase() != sessionId) {
return false
}
return true
}

// This test case isn't important behavior. Nothing should access
// currentSession before generateNewSession has been called. This test just
// ensures it has consistent behavior.
@Test
fun currentSession_beforeGenerateReturnsDefault() {
val sessionGenerator = SessionGenerator(collectEvents = false)

assertThat(sessionGenerator.currentSession.sessionId).isEqualTo("")
assertThat(sessionGenerator.currentSession.firstSessionId).isEqualTo("")
assertThat(sessionGenerator.currentSession.collectEvents).isEqualTo(false)
assertThat(sessionGenerator.currentSession.sessionIndex).isEqualTo(-1)
}

@Test
fun generateNewSessionID_generatesValidSessionInfo() {
val sessionGenerator = SessionGenerator(collectEvents = true)

sessionGenerator.generateNewSession()

assertThat(isValidSessionId(sessionGenerator.currentSession.sessionId)).isEqualTo(true)
assertThat(isValidSessionId(sessionGenerator.currentSession.firstSessionId)).isEqualTo(true)
assertThat(sessionGenerator.currentSession.firstSessionId)
.isEqualTo(sessionGenerator.currentSession.sessionId)
assertThat(sessionGenerator.currentSession.collectEvents).isEqualTo(true)
assertThat(sessionGenerator.currentSession.sessionIndex).isEqualTo(0)
}

// Ensures that generating a Session ID multiple times results in the fist
// Session ID being set in the firstSessionId field
@Test
fun generateNewSessionID_incrementsSessionIndex_keepsFirstSessionId() {
val sessionGenerator = SessionGenerator(collectEvents = true)

sessionGenerator.generateNewSession()

val firstSessionInfo = sessionGenerator.currentSession

assertThat(isValidSessionId(firstSessionInfo.sessionId)).isEqualTo(true)
assertThat(isValidSessionId(firstSessionInfo.firstSessionId)).isEqualTo(true)
assertThat(firstSessionInfo.firstSessionId).isEqualTo(firstSessionInfo.sessionId)
assertThat(firstSessionInfo.sessionIndex).isEqualTo(0)

sessionGenerator.generateNewSession()
val secondSessionInfo = sessionGenerator.currentSession

assertThat(isValidSessionId(secondSessionInfo.sessionId)).isEqualTo(true)
assertThat(isValidSessionId(secondSessionInfo.firstSessionId)).isEqualTo(true)
// Ensure the new firstSessionId is equal to the first Session ID from earlier
assertThat(secondSessionInfo.firstSessionId).isEqualTo(firstSessionInfo.sessionId)
// Session Index should increase
assertThat(secondSessionInfo.sessionIndex).isEqualTo(1)

// Do a third round just in case
sessionGenerator.generateNewSession()
val thirdSessionInfo = sessionGenerator.currentSession

assertThat(isValidSessionId(thirdSessionInfo.sessionId)).isEqualTo(true)
assertThat(isValidSessionId(thirdSessionInfo.firstSessionId)).isEqualTo(true)
// Ensure the new firstSessionId is equal to the first Session ID from earlier
assertThat(thirdSessionInfo.firstSessionId).isEqualTo(firstSessionInfo.sessionId)
// Session Index should increase
assertThat(thirdSessionInfo.sessionIndex).isEqualTo(2)
}
}