Issue mozilla-mobile#11738: Add refreshTopSitesIfCacheExpired method …

…to ContileTopSitesProvider

components/service/contile/src/main/java/mozilla/components/service/contile/ContileTopSitesProvider.kt

@@ -29,7 +29,7 @@ internal const val MINUTE_IN_MS = 60 * 1000
 /**
  * Provides access to the Contile services API.
  *
- * @property context A reference to the application context.
+ * @param context A reference to the application context.
  * @property client [Client] used for interacting with the Contile HTTP API.
  * @property endPointURL The url of the endpoint to fetch from. Defaults to [CONTILE_ENDPOINT_URL].
  * @property maxCacheAgeInMinutes Maximum time (in minutes) the cache should remain valid
@@ -46,6 +46,11 @@ class ContileTopSitesProvider(
     private val logger = Logger("ContileTopSitesProvider")
     private val diskCacheLock = Any()
 
+    // The last modified time of the disk cache.
+    @VisibleForTesting
+    @Volatile
+    internal var diskCacheLastModified: Long? = null
+
     /**
      * Fetches from the top sites [endPointURL] to provide a list of provided top sites.
      * Returns a cached response if [allowCache] is true and the cache is not expired
@@ -76,6 +81,16 @@ class ContileTopSitesProvider(
         }
     }
 
+    /**
+     * Refreshes the cache with the latest top sites response from [endPointURL]
+     * if the cache is active (@see [maxCacheAgeInMinutes]) and expired.
+     */
+    suspend fun refreshTopSitesIfCacheExpired() {
+        if (maxCacheAgeInMinutes < 0 || !isCacheExpired()) return
+
+        getTopSites(allowCache = false)
+    }
+
     private fun fetchTopSites(): List<TopSite.Provided> {
         client.fetch(
             Request(url = endPointURL)
@@ -113,7 +128,10 @@ class ContileTopSitesProvider(
     @VisibleForTesting
     internal fun writeToDiskCache(responseBody: String) {
         synchronized(diskCacheLock) {
-            getCacheFile().writeString { responseBody }
+            getCacheFile().let {
+                it.writeString { responseBody }
+                diskCacheLastModified = System.currentTimeMillis()
+            }
         }
     }
 
@@ -123,8 +141,17 @@ class ContileTopSitesProvider(
 
     @VisibleForTesting
     internal fun getCacheLastModified(): Long {
+        diskCacheLastModified?.let { return it }
+
         val file = getBaseCacheFile()
-        return if (file.exists()) file.lastModified() else -1
+
+        return if (file.exists()) {
+            file.lastModified().also {
+                diskCacheLastModified = it
+            }
+        } else {
+            -1
+        }
     }
 
     private fun getCacheFile(): AtomicFile = AtomicFile(getBaseCacheFile())

components/service/contile/src/test/java/mozilla/components/service/contile/ContileTopSitesProviderTest.kt

@@ -15,6 +15,8 @@ import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.whenever
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -93,11 +95,16 @@ class ContileTopSitesProviderTest {
             )
         )
 
+        assertNull(provider.diskCacheLastModified)
+        assertNull(cachingProvider.diskCacheLastModified)
+
         provider.getTopSites()
         verify(provider, never()).writeToDiskCache(jsonResponse)
+        assertNull(provider.diskCacheLastModified)
 
         cachingProvider.getTopSites()
         verify(cachingProvider).writeToDiskCache(jsonResponse)
+        assertNotNull(cachingProvider.diskCacheLastModified)
     }
 
     @Test
@@ -146,6 +153,63 @@ class ContileTopSitesProviderTest {
         assertFalse(provider.isCacheExpired())
     }
 
+    @Test
+    fun `WHEN the cache last modified time is fetched THEN the returned value is cached`() {
+        val provider = spy(ContileTopSitesProvider(testContext, prepareClient()))
+        val file = File(testContext.filesDir, CACHE_FILE_NAME)
+
+        assertNull(provider.diskCacheLastModified)
+
+        file.createNewFile()
+
+        assertTrue(file.exists())
+
+        var cacheLastModified = provider.getCacheLastModified()
+
+        assertEquals(cacheLastModified, provider.diskCacheLastModified)
+        assertTrue(file.delete())
+
+        cacheLastModified = provider.getCacheLastModified()
+
+        assertEquals(cacheLastModified, provider.diskCacheLastModified)
+    }
+
+    @Test
+    fun `GIVEN cache is not expired WHEN top sites are refreshed THEN do nothing`() = runBlocking {
+        val provider = spy(
+            ContileTopSitesProvider(
+                testContext,
+                client = prepareClient(),
+                maxCacheAgeInMinutes = 10
+            )
+        )
+
+        whenever(provider.isCacheExpired()).thenReturn(false)
+        provider.refreshTopSitesIfCacheExpired()
+        verify(provider, never()).getTopSites(allowCache = false)
+
+        Unit
+    }
+
+    @Test
+    fun `GIVEN cache is expired WHEN top sites are refreshed THEN fetch and write new response to cache`() = runBlocking {
+        val jsonResponse = loadResourceAsString("/contile/contile.json")
+        val provider = spy(
+            ContileTopSitesProvider(
+                testContext,
+                client = prepareClient(jsonResponse),
+                maxCacheAgeInMinutes = 10
+            )
+        )
+
+        whenever(provider.isCacheExpired()).thenReturn(true)
+
+        provider.refreshTopSitesIfCacheExpired()
+
+        verify(provider).getTopSites(allowCache = false)
+        verify(provider).writeToDiskCache(jsonResponse)
+    }
+
     private fun prepareClient(
         jsonResponse: String = loadResourceAsString("/contile/contile.json"),
         status: Int = 200