Remove cache eviction on init and add a method to allow consumers to clear the cache (#37)

* Remove the cache clearing at initialisation, add method to manually clear the cache, and add unit tests to cover the expected cache behaviour.

* Improvements to the negative caching test

Author: gazreese

Diff for: FlagsmithClient/src/main/java/com/flagsmith/Flagsmith.kt

@@ -150,6 +150,10 @@ class Flagsmith constructor(
         retrofit.getIdentityFlagsAndTraits(identity).enqueueWithResult(defaults = null, result = result)
             .also { lastUsedIdentity = identity }
 
+    fun clearCache() {
+        cache?.evictAll()
+    }
+
     private fun getFeatureFlag(
         featureId: String,
         identity: String?,

Diff for: FlagsmithClient/src/main/java/com/flagsmith/internal/FlagsmithRetrofitService.kt

@@ -96,9 +96,6 @@ interface FlagsmithRetrofitService {
                 .cache(cache)
                 .build()
 
-            // Make sure that we start with a clean cache
-            client.cache?.evictAll()
-
             val retrofit = Retrofit.Builder()
                 .baseUrl(baseUrl)
                 .addConverterFactory(GsonConverterFactory.create())

Diff for: FlagsmithClient/src/test/java/com/flagsmith/FeatureFlagCachingTests.kt

@@ -13,6 +13,7 @@ import com.flagsmith.mockResponses.mockResponseFor
 import org.awaitility.Awaitility
 import org.awaitility.kotlin.await
 import org.awaitility.kotlin.untilNotNull
+import org.awaitility.kotlin.untilTrue
 import org.junit.After
 import org.junit.Assert
 import org.junit.Before
@@ -25,6 +26,7 @@ import org.mockito.MockitoAnnotations
 import org.mockserver.integration.ClientAndServer
 import java.io.File
 import java.time.Duration
+import java.util.concurrent.atomic.AtomicBoolean
 
 
 class FeatureFlagCachingTests {
@@ -291,4 +293,90 @@ class FeatureFlagCachingTests {
         Assert.assertNotNull(foundFromServer)
         Assert.assertEquals(7.0, foundFromServer?.featureStateValue)
     }
+
+    @Test
+    fun testGetFeatureFlagsWithNewCachedFlagsmithGetsCachedValue() {
+        mockServer.mockResponseFor(MockEndpoint.GET_IDENTITIES)
+        mockServer.mockFailureFor(MockEndpoint.GET_IDENTITIES)
+
+        // First time around we should be successful and cache the response
+        var foundFromServer: Flag? = null
+        flagsmithWithCache.clearCache()
+        flagsmithWithCache.getFeatureFlags(identity = "person") { result ->
+            Assert.assertTrue(result.isSuccess)
+
+            foundFromServer =
+                result.getOrThrow().find { flag -> flag.feature.name == "with-value" }
+        }
+
+        await untilNotNull { foundFromServer }
+        Assert.assertNotNull(foundFromServer)
+        Assert.assertEquals(756.0, foundFromServer?.featureStateValue)
+
+        // Now get a new Flagsmith instance with the same cache and expect the cached response to be returned
+        val newFlagsmithWithCache = Flagsmith(
+            environmentKey = "",
+            baseUrl = "http://localhost:${mockServer.localPort}",
+            enableAnalytics = false,
+            context = mockApplicationContext,
+            cacheConfig = FlagsmithCacheConfig(enableCache = true)
+        )
+
+        // Now we mock the failure and expect the cached response to be returned from the new flagsmith instance
+        var foundFromCache: Flag? = null
+        newFlagsmithWithCache.getFeatureFlags(identity = "person") { result ->
+            Assert.assertTrue("The request will fail but we should be successful as we fall back on the cache", result.isSuccess)
+
+            foundFromCache =
+                result.getOrThrow().find { flag -> flag.feature.name == "with-value" }
+        }
+
+        await untilNotNull { foundFromCache }
+        Assert.assertNotNull(foundFromCache)
+        Assert.assertEquals(756.0, foundFromCache?.featureStateValue)
+    }
+
+    @Test
+    fun testGetFeatureFlagsWithNewCachedFlagsmithDoesntGetCachedValueWhenWeClearTheCache() {
+        mockServer.mockResponseFor(MockEndpoint.GET_IDENTITIES)
+        mockServer.mockFailureFor(MockEndpoint.GET_IDENTITIES)
+
+        // First time around we should be successful and cache the response
+        var foundFromServer: Flag? = null
+        flagsmithWithCache.clearCache()
+        flagsmithWithCache.getFeatureFlags(identity = "person") { result ->
+            Assert.assertTrue(result.isSuccess)
+
+            foundFromServer =
+                result.getOrThrow().find { flag -> flag.feature.name == "with-value" }
+        }
+
+        await untilNotNull { foundFromServer }
+        Assert.assertNotNull(foundFromServer)
+        Assert.assertEquals(756.0, foundFromServer?.featureStateValue)
+
+        // Now get a new Flagsmith instance with the same cache and evict the cache straight away
+        val newFlagsmithWithClearedCache = Flagsmith(
+            environmentKey = "",
+            baseUrl = "http://localhost:${mockServer.localPort}",
+            enableAnalytics = false,
+            context = mockApplicationContext,
+            cacheConfig = FlagsmithCacheConfig(enableCache = true)
+        )
+        newFlagsmithWithClearedCache.clearCache()
+
+        // Now we mock the failure and expect the get to fail as we don't have the cache to fall back on
+        var foundFromCache: Flag? = null
+        val hasFinishedGetRequest = AtomicBoolean(false)
+        newFlagsmithWithClearedCache.getFeatureFlags(identity = "person") { result ->
+            Assert.assertFalse("This un-cached response should fail", result.isSuccess)
+
+            foundFromCache =
+                result.getOrNull()?.find { flag -> flag.feature.name == "with-value" }
+            hasFinishedGetRequest.set(true)
+        }
+
+        await untilTrue(hasFinishedGetRequest)
+        Assert.assertNull("Shouldn't get any data back as we don't have a cache", foundFromCache)
+    }
 }