Skip to content
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.

Sign up for GitHub

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

Fix initialisation error on the Flagsmith SDK when enabling analytics - fixes #39 #40

Merged
merged 3 commits into from
Feb 14, 2024
Merged

Fix initialisation error on the Flagsmith SDK when enabling analytics - fixes #39 #40

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b671645
Update the tests to reproduce the initialisation error
gazreese Dec 13, 2023
b096549
Fix the initialisation error
gazreese Dec 13, 2023
a4954d0
Remove TODO comment
gazreese Jan 18, 2024
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
12 changes: 8 additions & 4 deletions FlagsmithClient/src/main/java/com/flagsmith/Flagsmith.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@ class Flagsmith constructor(
private lateinit var retrofit: FlagsmithRetrofitService
private var cache: Cache? = null
private var lastUsedIdentity: String? = null
private val analytics: FlagsmithAnalytics? =
if (!enableAnalytics) null
else if (context != null) FlagsmithAnalytics(context, retrofit, analyticsFlushPeriod)
else throw IllegalArgumentException("Flagsmith requires a context to use the analytics feature")
private var analytics: FlagsmithAnalytics? = null //TODO: Make this a lateinit var and instead initialise in init block
gazreese marked this conversation as resolved.
Show resolved Hide resolved

private val eventService: FlagsmithEventService? =
if (!enableRealtimeUpdates) null
Expand Down Expand Up @@ -93,6 +90,13 @@ class Flagsmith constructor(
writeTimeoutSeconds = writeTimeoutSeconds, timeTracker = this, klass = FlagsmithRetrofitService::class.java)
retrofit = pair.first
cache = pair.second

if (enableAnalytics) {
if (context == null || context.applicationContext == null) {
throw IllegalArgumentException("Flagsmith requires a context to use the analytics feature")
}
analytics = FlagsmithAnalytics(context, retrofit, analyticsFlushPeriod)
}
}

companion object {
Expand Down
3 changes: 2 additions & 1 deletion FlagsmithClient/src/test/java/com/flagsmith/FeatureFlagCachingTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class FeatureFlagCachingTests {
flagsmithWithCache = Flagsmith(
environmentKey = "",
baseUrl = "http://localhost:${mockServer.localPort}",
enableAnalytics = false,
enableAnalytics = true, // Mix up the analytics flag to test initialisation
context = mockApplicationContext,
defaultFlags = defaultFlags,
cacheConfig = FlagsmithCacheConfig(enableCache = true)
Expand Down Expand Up @@ -112,6 +112,7 @@ class FeatureFlagCachingTests {
`when`(mockContextResources.getBoolean(anyInt())).thenReturn(false)
`when`(mockContextResources.getDimension(anyInt())).thenReturn(100f)
`when`(mockContextResources.getIntArray(anyInt())).thenReturn(intArrayOf(1, 2, 3))
`when`(mockApplicationContext.applicationContext).thenReturn(mockApplicationContext)
}

@After
Expand Down