Issue mozilla-mobile#11529: Add an updater and caching layer for the …

…Contile Top Sites

components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt

@@ -100,7 +100,7 @@ class DefaultTopSitesStorage(
         ) {
             try {
                 val providerTopSites = topSitesProvider
-                    .getTopSites()
+                    .getTopSites(allowCache = true)
                     .take(numSitesRequired)
                 topSites.addAll(providerTopSites)
                 numSitesRequired -= providerTopSites.size

components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesProvider.kt

@@ -11,6 +11,10 @@ interface TopSitesProvider {
 
     /**
      * Provides a list of top sites.
+     *
+     * @param allowCache Whether or not the result may be provided from a previously
+     * cached response.
+     * @return a list of top sites from the provider.
      */
-    suspend fun getTopSites(): List<TopSite>
+    suspend fun getTopSites(allowCache: Boolean = true): List<TopSite>
 }

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

@@ -4,32 +4,70 @@
 
 package mozilla.components.service.contile
 
+import android.content.Context
+import android.util.AtomicFile
+import androidx.annotation.VisibleForTesting
 import mozilla.components.concept.fetch.Client
 import mozilla.components.concept.fetch.Request
 import mozilla.components.concept.fetch.isSuccess
 import mozilla.components.feature.top.sites.TopSite
 import mozilla.components.feature.top.sites.TopSitesProvider
 import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.ktx.android.org.json.asSequence
+import mozilla.components.support.ktx.util.readAndDeserialize
+import mozilla.components.support.ktx.util.writeString
 import org.json.JSONException
 import org.json.JSONObject
+import java.io.File
 import java.io.IOException
+import java.util.Date
 
 internal const val CONTILE_ENDPOINT_URL = "https://contile.services.mozilla.com/v1/tiles"
+internal const val CACHE_FILE_NAME = "mozilla_components_service_contile.json"
+internal const val MINUTE_IN_MS = 60 * 1000
 
 /**
  * Provide access to the Contile services API.
  *
+ * @property 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
+ * before a refresh is attempted. Defaults to -1, meaning no cache is being used by default.
  */
 class ContileTopSitesProvider(
-    private val client: Client
+    private val context: Context,
+    private val client: Client,
+    private val endPointURL: String = CONTILE_ENDPOINT_URL,
+    private val maxCacheAgeInMinutes: Long = -1
 ) : TopSitesProvider {
 
     private val logger = Logger("ContileTopSitesProvider")
 
+    private val diskCacheLock = Any()
+
+    /**
+     * 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
+     * (@see [maxCacheAgeInMinutes]).
+     *
+     * @param allowCache Whether or not the result may be provided from a previously cached
+     * response. Note that a [maxCacheAgeInMinutes] must be provided in order for the cache to be
+     * active.
+     * @throws IOException if the request failed to fetch any top sites.
+     */
     @Throws(IOException::class)
-    override suspend fun getTopSites(): List<TopSite.Provided> {
+    override suspend fun getTopSites(allowCache: Boolean): List<TopSite.Provided> {
+        val cachedTopSites = if (allowCache && !isCacheExpired()) {
+            readFromDiskCache()
+        } else {
+            null
+        }
+
+        if (!cachedTopSites.isNullOrEmpty()) {
+            return cachedTopSites
+        }
+
         return try {
             fetchTopSites()
         } catch (e: IOException) {
@@ -40,13 +78,17 @@ class ContileTopSitesProvider(
 
     private fun fetchTopSites(): List<TopSite.Provided> {
         client.fetch(
-            Request(url = CONTILE_ENDPOINT_URL)
+            Request(url = endPointURL)
         ).use { response ->
             if (response.isSuccess) {
                 val responseBody = response.body.string(Charsets.UTF_8)
 
                 return try {
-                    JSONObject(responseBody).getTopSites()
+                    JSONObject(responseBody).getTopSites().also {
+                        if (maxCacheAgeInMinutes > 0) {
+                            writeToDiskCache(responseBody)
+                        }
+                    }
                 } catch (e: JSONException) {
                     throw IOException(e)
                 }
@@ -58,6 +100,37 @@ class ContileTopSitesProvider(
             }
         }
     }
+
+    @VisibleForTesting
+    internal fun readFromDiskCache(): List<TopSite.Provided>? {
+        synchronized(diskCacheLock) {
+            return getCacheFile().readAndDeserialize {
+                JSONObject(it).getTopSites()
+            }
+        }
+    }
+
+    @VisibleForTesting
+    internal fun writeToDiskCache(responseBody: String) {
+        synchronized(diskCacheLock) {
+            getCacheFile().writeString { responseBody }
+        }
+    }
+
+    @VisibleForTesting
+    internal fun isCacheExpired() =
+        getCacheLastModified() < Date().time - maxCacheAgeInMinutes * MINUTE_IN_MS
+
+    @VisibleForTesting
+    internal fun getCacheLastModified(): Long {
+        val file = getBaseCacheFile()
+        return if (file.exists()) file.lastModified() else -1
+    }
+
+    private fun getCacheFile(): AtomicFile = AtomicFile(getBaseCacheFile())
+
+    @VisibleForTesting
+    internal fun getBaseCacheFile(): File = File(context.filesDir, CACHE_FILE_NAME)
 }
 
 internal fun JSONObject.getTopSites(): List<TopSite.Provided> =

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

@@ -11,24 +11,31 @@ import mozilla.components.concept.fetch.Response
 import mozilla.components.support.test.any
 import mozilla.components.support.test.file.loadResourceAsString
 import mozilla.components.support.test.mock
+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.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mockito.never
+import org.mockito.Mockito.spy
+import org.mockito.Mockito.verify
+import java.io.File
 import java.io.IOException
+import java.util.Date
 
 @RunWith(AndroidJUnit4::class)
 class ContileTopSitesProviderTest {
 
     @Test
-    fun `GIVEN a successful status response WHEN getTopSites is called THEN response should contain top sites`() = runBlocking {
+    fun `GIVEN a successful status response WHEN top sites are fetched THEN response should contain top sites`() = runBlocking {
         val client = prepareClient()
-        val provider = ContileTopSitesProvider(client)
+        val provider = ContileTopSitesProvider(testContext, client)
         val topSites = provider.getTopSites()
         var topSite = topSites.first()
 
         assertEquals(2, topSites.size)
-
         assertEquals(1L, topSite.id)
         assertEquals("Firefox", topSite.title)
         assertEquals("https://firefox.com", topSite.url)
@@ -47,13 +54,98 @@ class ContileTopSitesProviderTest {
     }
 
     @Test(expected = IOException::class)
-    fun `GIVEN a 500 status response WHEN getTopSites is called THEN throw an exception`() = runBlocking {
+    fun `GIVEN a 500 status response WHEN top sites are fetched THEN throw an exception`() = runBlocking {
         val client = prepareClient(status = 500)
-        val provider = ContileTopSitesProvider(client)
+        val provider = ContileTopSitesProvider(testContext, client)
         provider.getTopSites()
         Unit
     }
 
+    @Test
+    fun `GIVEN a cache configuration is allowed and not expired WHEN top sites are fetched THEN read from the disk cache`() = runBlocking {
+        val client = prepareClient()
+        val provider = spy(ContileTopSitesProvider(testContext, client))
+
+        provider.getTopSites(allowCache = false)
+        verify(provider, never()).readFromDiskCache()
+
+        whenever(provider.isCacheExpired()).thenReturn(true)
+        provider.getTopSites(allowCache = true)
+        verify(provider, never()).readFromDiskCache()
+
+        whenever(provider.isCacheExpired()).thenReturn(false)
+        provider.getTopSites(allowCache = true)
+        verify(provider).readFromDiskCache()
+
+        Unit
+    }
+
+    @Test
+    fun `GIVEN a cache configuration is allowed WHEN top sites are fetched THEN write response to cache`() = runBlocking {
+        val jsonResponse = loadResourceAsString("/contile/contile.json")
+        val client = prepareClient(jsonResponse)
+        val provider = spy(ContileTopSitesProvider(testContext, client))
+        val cachingProvider = spy(
+            ContileTopSitesProvider(
+                context = testContext,
+                client = client,
+                maxCacheAgeInMinutes = 1L
+            )
+        )
+
+        provider.getTopSites()
+        verify(provider, never()).writeToDiskCache(jsonResponse)
+
+        cachingProvider.getTopSites()
+        verify(cachingProvider).writeToDiskCache(jsonResponse)
+    }
+
+    @Test
+    fun `WHEN the base cache file getter is called THEN return existing base cache file`() {
+        val client = prepareClient()
+        val provider = spy(ContileTopSitesProvider(testContext, client))
+        val file = File(testContext.filesDir, CACHE_FILE_NAME)
+
+        file.createNewFile()
+
+        assertTrue(file.exists())
+
+        val cacheFile = provider.getBaseCacheFile()
+
+        assertTrue(cacheFile.exists())
+        assertEquals(file.name, cacheFile.name)
+
+        assertTrue(file.delete())
+        assertFalse(cacheFile.exists())
+    }
+
+    @Test
+    fun `GIVEN a max cache age WHEN the cache expiration is checked THEN return whether the cache is expired`() {
+        var provider =
+            spy(ContileTopSitesProvider(testContext, client = mock(), maxCacheAgeInMinutes = -1))
+
+        whenever(provider.getCacheLastModified()).thenReturn(Date().time)
+        assertTrue(provider.isCacheExpired())
+
+        whenever(provider.getCacheLastModified()).thenReturn(-1)
+        assertTrue(provider.isCacheExpired())
+
+        provider =
+            spy(ContileTopSitesProvider(testContext, client = mock(), maxCacheAgeInMinutes = 10))
+
+        whenever(provider.getCacheLastModified()).thenReturn(-1)
+        assertTrue(provider.isCacheExpired())
+
+        whenever(provider.getCacheLastModified()).thenReturn(Date().time - 60 * MINUTE_IN_MS)
+        assertTrue(provider.isCacheExpired())
+
+        whenever(provider.getCacheLastModified()).thenReturn(Date().time)
+        assertFalse(provider.isCacheExpired())
+
+        whenever(provider.getCacheLastModified()).thenReturn(Date().time + 60 * MINUTE_IN_MS)
+        assertFalse(provider.isCacheExpired())
+    }
+
     private fun prepareClient(
         jsonResponse: String = loadResourceAsString("/contile/contile.json"),
         status: Int = 200