Don't retrieve the whole config if the server returns HTTP 304 Not Modified. (#3091)

Co-authored-by: Mohamed <mohamed.metwalli@sap.com>

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigModule.kt

 package de.rki.coronawarnapp.appconfig
 
+import android.content.Context
 import dagger.Module
 import dagger.Provides
 import de.rki.coronawarnapp.appconfig.download.AppConfigApiV2
@@ -14,10 +15,13 @@ import de.rki.coronawarnapp.appconfig.mapping.PresenceTracingConfigMapper
 import de.rki.coronawarnapp.appconfig.mapping.SurveyConfigMapper
 import de.rki.coronawarnapp.environment.download.DownloadCDNHttpClient
 import de.rki.coronawarnapp.environment.download.DownloadCDNServerUrl
+import de.rki.coronawarnapp.util.di.AppContext
+import okhttp3.Cache
 import okhttp3.OkHttpClient
 import org.joda.time.Duration
 import retrofit2.Retrofit
 import retrofit2.converter.gson.GsonConverterFactory
+import java.io.File
 import java.util.concurrent.TimeUnit
 import javax.inject.Singleton
 
@@ -27,18 +31,14 @@ class AppConfigModule {
     @Singleton
     @Provides
     fun provideAppConfigApi(
+        @RemoteAppConfigCache cache: Cache,
         @DownloadCDNHttpClient client: OkHttpClient,
         @DownloadCDNServerUrl url: String,
         gsonConverterFactory: GsonConverterFactory
     ): AppConfigApiV2 {
 
         val configHttpClient = client.newBuilder().apply {
-            // We no longer use the retrofit cache, due to the complexity it adds when invalidating the cache.
-            // The our manual local storage offers more control and should replace it functionally.
-            // See **[de.rki.coronawarnapp.appconfig.sources.local.LocalAppConfigSource]**
-            // If we ever want to use it again, the previous cache path was:
-            // val cacheDir = File(context.cacheDir, "http_app-config")
-            // cache(cache)
+            cache(cache)
             connectTimeout(HTTP_TIMEOUT_APPCONFIG.millis, TimeUnit.MILLISECONDS)
             readTimeout(HTTP_TIMEOUT_APPCONFIG.millis, TimeUnit.MILLISECONDS)
             writeTimeout(HTTP_TIMEOUT_APPCONFIG.millis, TimeUnit.MILLISECONDS)
@@ -53,6 +53,14 @@ class AppConfigModule {
             .create(AppConfigApiV2::class.java)
     }
 
+    @RemoteAppConfigCache
+    @Provides
+    @Singleton
+    fun remoteAppConfigHttpCache(@AppContext context: Context): Cache {
+        val cacheDir = File(context.cacheDir, "http_app-config")
+        return Cache(cacheDir, DEFAULT_CACHE_SIZE)
+    }
+
     @Provides
     fun cwaMapper(mapper: CWAConfigMapper):
         CWAConfig.Mapper = mapper
@@ -90,5 +98,6 @@ class AppConfigModule {
 
     companion object {
         private val HTTP_TIMEOUT_APPCONFIG = Duration.standardSeconds(10)
+        private const val DEFAULT_CACHE_SIZE = 2 * 1024 * 1024L // 5MB
     }
 }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/RemoteAppConfigCache.kt

+package de.rki.coronawarnapp.appconfig
+
+import javax.inject.Qualifier
+
+@Qualifier
+@MustBeDocumented
+@Retention(AnnotationRetention.RUNTIME)
+annotation class RemoteAppConfigCache

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/internal/AppConfigSource.kt

@@ -81,6 +81,7 @@ class AppConfigSource @Inject constructor(
     suspend fun clear() {
         Timber.tag(TAG).d("clear()")
         localAppConfigSource.clear()
+        remoteAppConfigSource.clear()
     }
 
     companion object {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt

 package de.rki.coronawarnapp.appconfig.sources.remote
 
 import de.rki.coronawarnapp.appconfig.ConfigData
+import de.rki.coronawarnapp.appconfig.RemoteAppConfigCache
 import de.rki.coronawarnapp.appconfig.internal.ConfigDataContainer
 import de.rki.coronawarnapp.appconfig.mapping.ConfigParser
 import de.rki.coronawarnapp.appconfig.sources.local.AppConfigStorage
 import de.rki.coronawarnapp.util.coroutine.DispatcherProvider
 import kotlinx.coroutines.withContext
+import okhttp3.Cache
 import timber.log.Timber
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -13,6 +15,7 @@ import javax.inject.Singleton
 @Singleton
 class RemoteAppConfigSource @Inject constructor(
     private val server: AppConfigServer,
+    @RemoteAppConfigCache private val remoteCache: Cache,
     private val storage: AppConfigStorage,
     private val parser: ConfigParser,
     private val dispatcherProvider: DispatcherProvider
@@ -47,6 +50,11 @@ class RemoteAppConfigSource @Inject constructor(
         }
     }
 
+    fun clear() {
+        Timber.tag(TAG).d("clear()")
+        remoteCache.evictAll()
+    }
+
     companion object {
         private const val TAG = "AppConfigRetriever"
     }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/internal/AppConfigSourceTest.kt

@@ -11,6 +11,7 @@ import io.kotest.matchers.shouldBe
 import io.mockk.MockKAnnotations
 import io.mockk.Runs
 import io.mockk.coEvery
+import io.mockk.coVerify
 import io.mockk.coVerifyOrder
 import io.mockk.coVerifySequence
 import io.mockk.every
@@ -70,8 +71,14 @@ class AppConfigSourceTest : BaseTest() {
     fun setup() {
         MockKAnnotations.init(this)
 
-        coEvery { remoteSource.getConfigData() } returns remoteConfig
-        coEvery { localSource.getConfigData() } returns localConfig
+        remoteSource.apply {
+            coEvery { getConfigData() } returns remoteConfig
+            coEvery { clear() } just Runs
+        }
+        localSource.apply {
+            coEvery { getConfigData() } returns localConfig
+            coEvery { clear() } just Runs
+        }
         coEvery { defaultSource.getConfigData() } returns defaultConfig
 
         every { timeStamper.nowUTC } returns Instant.EPOCH.plus(Duration.standardHours(1))
@@ -244,4 +251,14 @@ class AppConfigSourceTest : BaseTest() {
             cwaSettings.lastDeviceTimeStateChangeState = ConfigData.DeviceTimeState.CORRECT
         }
     }
+
+    @Test
+    fun `clear calls subroutines`() = runBlockingTest {
+        createInstance().clear()
+
+        coVerify {
+            localSource.clear()
+            remoteSource.clear()
+        }
+    }
 }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigApiTest.kt

@@ -9,6 +9,7 @@ import io.kotest.matchers.shouldBe
 import io.mockk.MockKAnnotations
 import io.mockk.impl.annotations.MockK
 import kotlinx.coroutines.runBlocking
+import okhttp3.Cache
 import okhttp3.ConnectionSpec
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
@@ -25,6 +26,7 @@ class AppConfigApiTest : BaseIOTest() {
 
     private lateinit var webServer: MockWebServer
     private lateinit var serverAddress: String
+    private lateinit var cache: Cache
 
     private val testDir = File(IO_TEST_BASEDIR, this::class.java.simpleName)
 
@@ -32,6 +34,8 @@ class AppConfigApiTest : BaseIOTest() {
     fun setup() {
         MockKAnnotations.init(this)
 
+        cache = Cache(File(testDir, "cache"), 1024L)
+
         webServer = MockWebServer()
         webServer.start()
         serverAddress = "http://${webServer.hostName}:${webServer.port}"
@@ -57,7 +61,8 @@ class AppConfigApiTest : BaseIOTest() {
         return AppConfigModule().provideAppConfigApi(
             client = cdnHttpClient,
             url = serverAddress,
-            gsonConverterFactory = gsonConverterFactory
+            gsonConverterFactory = gsonConverterFactory,
+            cache = cache,
         )
     }
 

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSourceTest.kt

@@ -12,6 +12,8 @@ import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.impl.annotations.MockK
+import io.mockk.verify
+import okhttp3.Cache
 import okio.ByteString.Companion.decodeHex
 import org.joda.time.Duration
 import org.joda.time.Instant
@@ -31,6 +33,7 @@ class RemoteAppConfigSourceTest : BaseIOTest() {
     @MockK lateinit var configParser: ConfigParser
     @MockK lateinit var configData: ConfigData
     @MockK lateinit var timeStamper: TimeStamper
+    @MockK(relaxUnitFun = true) lateinit var remoteCache: Cache
 
     private val testDir = File(IO_TEST_BASEDIR, this::class.simpleName!!)
 
@@ -71,7 +74,8 @@ class RemoteAppConfigSourceTest : BaseIOTest() {
         server = configServer,
         storage = configStorage,
         parser = configParser,
-        dispatcherProvider = TestDispatcherProvider()
+        dispatcherProvider = TestDispatcherProvider(),
+        remoteCache = remoteCache
     )
 
     @Test
@@ -103,6 +107,13 @@ class RemoteAppConfigSourceTest : BaseIOTest() {
         coVerify(exactly = 0) { configStorage.setStoredConfig(any()) }
     }
 
+    @Test
+    fun `clear clears okhttp cache`() {
+        createInstance().clear()
+
+        verify { remoteCache.evictAll() }
+    }
+
     companion object {
         private val APPCONFIG_RAW = (
             "080b124d0a230a034c4f57180f221a68747470733a2f2f777777" +