Skip to content

Commit 0ea9884

Browse files
mrobermrober
authored
Use the private setter pattern on SessionGenerator.currentSession (#4785)
* Use the private setter pattern on SessionGenerator.currentSession * Comments
1 parent 8acde15 commit 0ea9884

File tree

2 files changed

+107
-59
lines changed

2 files changed

+107
-59
lines changed

‎firebase-sessions/src/main/kotlin/com/google/firebase/sessions/SessionGenerator.kt

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,35 +36,29 @@ internal data class SessionDetails(
3636
*
3737
* @hide
3838
*/
39-
internal class SessionGenerator(private var collectEvents: Boolean) {
40-
private var firstSessionId = ""
41-
private var sessionIndex: Int = -1
39+
internal class SessionGenerator(
40+
private val collectEvents: Boolean,
41+
private val uuidGenerator: () -> UUID = UUID::randomUUID
42+
) {
43+
private val firstSessionId = generateSessionId()
44+
private var sessionIndex = -1
4245

43-
private var thisSession: SessionDetails =
44-
SessionDetails(
45-
sessionId = "",
46-
firstSessionId = "",
47-
collectEvents,
48-
sessionIndex,
49-
)
46+
/** The current generated session, must not be accessed before calling [generateNewSession]. */
47+
lateinit var currentSession: SessionDetails
48+
private set
5049

51-
// Generates a new Session ID. If there was already a generated Session ID
52-
// from the last session during the app's lifecycle, it will also set the last Session ID
50+
/** Generates a new session. The first session's sessionId will match firstSessionId. */
5351
fun generateNewSession(): SessionDetails {
54-
val newSessionId = UUID.randomUUID().toString().replace("-", "").lowercase()
55-
56-
// If firstSessionId is set, use it. Otherwise set it to the
57-
// first generated Session ID
58-
firstSessionId = firstSessionId.ifEmpty { newSessionId }
59-
60-
sessionIndex += 1
61-
62-
thisSession =
63-
SessionDetails(sessionId = newSessionId, firstSessionId, collectEvents, sessionIndex)
64-
65-
return thisSession
52+
sessionIndex++
53+
currentSession =
54+
SessionDetails(
55+
sessionId = if (sessionIndex == 0) firstSessionId else generateSessionId(),
56+
firstSessionId,
57+
collectEvents,
58+
sessionIndex,
59+
)
60+
return currentSession
6661
}
6762

68-
val currentSession: SessionDetails
69-
get() = thisSession
63+
private fun generateSessionId() = uuidGenerator().toString().replace("-", "").lowercase()
7064
}

‎firebase-sessions/src/test/kotlin/com/google/firebase/sessions/SessionGeneratorTest.kt

Lines changed: 87 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.firebase.sessions
1818

1919
import com.google.common.truth.Truth.assertThat
20+
import java.util.UUID
2021
import org.junit.Test
2122

2223
class SessionGeneratorTest {
@@ -34,66 +35,119 @@ class SessionGeneratorTest {
3435
}
3536

3637
// This test case isn't important behavior. Nothing should access
37-
// currentSession before generateNewSession has been called. This test just
38-
// ensures it has consistent behavior.
39-
@Test
40-
fun currentSession_beforeGenerateReturnsDefault() {
38+
// currentSession before generateNewSession has been called.
39+
@Test(expected = UninitializedPropertyAccessException::class)
40+
fun currentSession_beforeGenerate_throwsUninitialized() {
4141
val sessionGenerator = SessionGenerator(collectEvents = false)
4242

43-
assertThat(sessionGenerator.currentSession.sessionId).isEqualTo("")
44-
assertThat(sessionGenerator.currentSession.firstSessionId).isEqualTo("")
45-
assertThat(sessionGenerator.currentSession.collectEvents).isFalse()
46-
assertThat(sessionGenerator.currentSession.sessionIndex).isEqualTo(-1)
43+
sessionGenerator.currentSession
4744
}
4845

4946
@Test
50-
fun generateNewSessionID_generatesValidSessionDetails() {
51-
val sessionGenerator = SessionGenerator(collectEvents = true)
47+
fun generateNewSession_generatesValidSessionIds() {
48+
val sessionGenerator = SessionGenerator(collectEvents = true) // defaults to UUID::randomUUID
5249

5350
sessionGenerator.generateNewSession()
5451

5552
assertThat(isValidSessionId(sessionGenerator.currentSession.sessionId)).isTrue()
5653
assertThat(isValidSessionId(sessionGenerator.currentSession.firstSessionId)).isTrue()
57-
assertThat(sessionGenerator.currentSession.firstSessionId)
58-
.isEqualTo(sessionGenerator.currentSession.sessionId)
59-
assertThat(sessionGenerator.currentSession.collectEvents).isTrue()
60-
assertThat(sessionGenerator.currentSession.sessionIndex).isEqualTo(0)
54+
55+
// Validate several random session ids.
56+
repeat(16) {
57+
assertThat(isValidSessionId(sessionGenerator.generateNewSession().sessionId)).isTrue()
58+
}
6159
}
6260

63-
// Ensures that generating a Session ID multiple times results in the fist
64-
// Session ID being set in the firstSessionId field
6561
@Test
66-
fun generateNewSessionID_incrementsSessionIndex_keepsFirstSessionId() {
67-
val sessionGenerator = SessionGenerator(collectEvents = true)
62+
fun generateNewSession_generatesValidSessionDetails() {
63+
val sessionGenerator = SessionGenerator(collectEvents = true, uuidGenerator = UUIDs()::next)
6864

6965
sessionGenerator.generateNewSession()
7066

71-
val firstSessionDetails = sessionGenerator.currentSession
67+
assertThat(isValidSessionId(sessionGenerator.currentSession.sessionId)).isTrue()
68+
assertThat(isValidSessionId(sessionGenerator.currentSession.firstSessionId)).isTrue()
69+
70+
assertThat(sessionGenerator.currentSession)
71+
.isEqualTo(
72+
SessionDetails(
73+
sessionId = SESSION_ID_1,
74+
firstSessionId = SESSION_ID_1,
75+
collectEvents = true,
76+
sessionIndex = 0,
77+
)
78+
)
79+
}
80+
81+
// Ensures that generating a Session ID multiple times results in the fist
82+
// Session ID being set in the firstSessionId field
83+
@Test
84+
fun generateNewSession_incrementsSessionIndex_keepsFirstSessionId() {
85+
val sessionGenerator = SessionGenerator(collectEvents = true, uuidGenerator = UUIDs()::next)
86+
87+
val firstSessionDetails = sessionGenerator.generateNewSession()
7288

7389
assertThat(isValidSessionId(firstSessionDetails.sessionId)).isTrue()
7490
assertThat(isValidSessionId(firstSessionDetails.firstSessionId)).isTrue()
75-
assertThat(firstSessionDetails.firstSessionId).isEqualTo(firstSessionDetails.sessionId)
76-
assertThat(firstSessionDetails.sessionIndex).isEqualTo(0)
7791

78-
sessionGenerator.generateNewSession()
79-
val secondSessionDetails = sessionGenerator.currentSession
92+
assertThat(firstSessionDetails)
93+
.isEqualTo(
94+
SessionDetails(
95+
sessionId = SESSION_ID_1,
96+
firstSessionId = SESSION_ID_1,
97+
collectEvents = true,
98+
sessionIndex = 0,
99+
)
100+
)
101+
102+
val secondSessionDetails = sessionGenerator.generateNewSession()
80103

81104
assertThat(isValidSessionId(secondSessionDetails.sessionId)).isTrue()
82105
assertThat(isValidSessionId(secondSessionDetails.firstSessionId)).isTrue()
83-
// Ensure the new firstSessionId is equal to the first Session ID from earlier
84-
assertThat(secondSessionDetails.firstSessionId).isEqualTo(firstSessionDetails.sessionId)
85-
// Session Index should increase
86-
assertThat(secondSessionDetails.sessionIndex).isEqualTo(1)
106+
107+
// Ensure the new firstSessionId is equal to the first sessionId, and sessionIndex increased
108+
assertThat(secondSessionDetails)
109+
.isEqualTo(
110+
SessionDetails(
111+
sessionId = SESSION_ID_2,
112+
firstSessionId = SESSION_ID_1,
113+
collectEvents = true,
114+
sessionIndex = 1,
115+
)
116+
)
87117

88118
// Do a third round just in case
89-
sessionGenerator.generateNewSession()
90-
val thirdSessionDetails = sessionGenerator.currentSession
119+
val thirdSessionDetails = sessionGenerator.generateNewSession()
91120

92121
assertThat(isValidSessionId(thirdSessionDetails.sessionId)).isTrue()
93122
assertThat(isValidSessionId(thirdSessionDetails.firstSessionId)).isTrue()
94-
// Ensure the new firstSessionId is equal to the first Session ID from earlier
95-
assertThat(thirdSessionDetails.firstSessionId).isEqualTo(firstSessionDetails.sessionId)
96-
// Session Index should increase
97-
assertThat(thirdSessionDetails.sessionIndex).isEqualTo(2)
123+
124+
assertThat(thirdSessionDetails)
125+
.isEqualTo(
126+
SessionDetails(
127+
sessionId = SESSION_ID_3,
128+
firstSessionId = SESSION_ID_1,
129+
collectEvents = true,
130+
sessionIndex = 2,
131+
)
132+
)
133+
}
134+
135+
private class UUIDs(val names: List<String> = listOf(UUID_1, UUID_2, UUID_3)) {
136+
var index = -1
137+
138+
fun next(): UUID {
139+
index = (index + 1).coerceAtMost(names.size - 1)
140+
return UUID.fromString(names[index])
141+
}
142+
}
143+
144+
@Suppress("SpellCheckingInspection") // UUIDs are not words.
145+
companion object {
146+
const val UUID_1 = "11111111-1111-1111-1111-111111111111"
147+
const val SESSION_ID_1 = "11111111111111111111111111111111"
148+
const val UUID_2 = "22222222-2222-2222-2222-222222222222"
149+
const val SESSION_ID_2 = "22222222222222222222222222222222"
150+
const val UUID_3 = "CCCCCCCC-CCCC-CCCC-CCCC-CCCCCCCCCCCC"
151+
const val SESSION_ID_3 = "cccccccccccccccccccccccccccccccc"
98152
}
99153
}

0 commit comments

Comments
 (0)