fix: implemented rate limiting for all requests

app/src/main/java/com/nononsenseapps/feeder/FeederApplication.kt

@@ -14,7 +14,6 @@ import coil3.SingletonImageLoader
 import coil3.annotation.ExperimentalCoilApi
 import coil3.disk.DiskCache
 import coil3.memory.MemoryCache
-import coil3.network.cachecontrol.CacheControlCacheStrategy
 import coil3.network.okhttp.OkHttpNetworkFetcherFactory
 import coil3.request.crossfade
 import coil3.request.maxBitmapSize
@@ -33,8 +32,12 @@ import com.nononsenseapps.feeder.db.room.SyncRemoteDao
 import com.nononsenseapps.feeder.di.androidModule
 import com.nononsenseapps.feeder.di.archModelModule
 import com.nononsenseapps.feeder.di.networkModule
+import com.nononsenseapps.feeder.model.AlwaysUseCacheIfPossibleRequestsInterceptor
 import com.nononsenseapps.feeder.model.ForceCacheOnSomeFailuresInterceptor
+import com.nononsenseapps.feeder.model.OneImageRequestPerHostInterceptor
+import com.nononsenseapps.feeder.model.RateLimitedInterceptor
 import com.nononsenseapps.feeder.model.TTSStateHolder
+import com.nononsenseapps.feeder.model.TooManyRequestsInterceptor
 import com.nononsenseapps.feeder.model.UserAgentInterceptor
 import com.nononsenseapps.feeder.notifications.NotificationsWorker
 import com.nononsenseapps.feeder.util.FilePathProvider
@@ -119,7 +122,9 @@ class FeederApplication :
                     cacheDirectory = (filePathProvider.httpCacheDir),
                 ) {
                     addNetworkInterceptor(UserAgentInterceptor)
+                    addNetworkInterceptor(RateLimitedInterceptor)
                     addNetworkInterceptor(ForceCacheOnSomeFailuresInterceptor)
+                    addNetworkInterceptor(TooManyRequestsInterceptor)
                     if (BuildConfig.DEBUG) {
                         addInterceptor { chain ->
                             val request = chain.request()
@@ -145,6 +150,7 @@ class FeederApplication :
                 val okHttpClient =
                     instance<OkHttpClient>()
                         .newBuilder()
+                        .addInterceptor(AlwaysUseCacheIfPossibleRequestsInterceptor)
                         .addInterceptor { chain ->
                             chain.proceed(
                                 when (!repository.loadImageOnlyOnWifi.value || currentlyUnmetered(this@FeederApplication)) {
@@ -184,15 +190,13 @@ class FeederApplication :
                             .maxSizeBytes(50 * 1024 * 1024)
                             .build()
                     }.components {
+                        add(OneImageRequestPerHostInterceptor)
                         add(IcoDecoder.Factory(this@FeederApplication))
                         add(
                             OkHttpNetworkFetcherFactory(
                                 callFactory = {
                                     okHttpClient
                                 },
-                                cacheStrategy = {
-                                    CacheControlCacheStrategy()
-                                },
                             ),
                         )
                     }.build()

app/src/main/java/com/nononsenseapps/feeder/model/AlwaysUseCacheIfPossibleRequestsInterceptor.kt

@@ -0,0 +1,25 @@
+package com.nononsenseapps.feeder.model
+
+import okhttp3.CacheControl
+import okhttp3.Interceptor
+import okhttp3.Response
+import java.util.concurrent.TimeUnit
+
+/**
+ * Interceptor that tries to use the cache if possible
+ */
+object AlwaysUseCacheIfPossibleRequestsInterceptor : Interceptor {
+    override fun intercept(chain: Interceptor.Chain): Response =
+        chain.proceed(
+            chain
+                .request()
+                .newBuilder()
+                .cacheControl(
+                    CacheControl
+                        .Builder()
+                        .maxStale(30, TimeUnit.DAYS)
+                        .maxAge(7, TimeUnit.DAYS)
+                        .build(),
+                ).build(),
+        )
+}

app/src/main/java/com/nononsenseapps/feeder/model/OneImageRequestPerHostInterceptor.kt

@@ -0,0 +1,55 @@
+package com.nononsenseapps.feeder.model
+
+import android.net.Uri
+import android.util.Log
+import coil3.intercept.Interceptor
+import coil3.request.ImageResult
+import com.nononsenseapps.feeder.util.logDebug
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
+import java.util.concurrent.ConcurrentHashMap
+
+/**
+ * Interceptor that ensures only one image request is made per host at a time.
+ *
+ * If a request is already in progress for a host, the new request suspends until the first one is done.
+ *
+ * This ensures that we don't flood a server with requests for images.
+ *
+ * And that cached responses are used when possible.
+ */
+object OneImageRequestPerHostInterceptor : Interceptor {
+    private const val LOG_TAG = "FEEDER_IMAGE"
+
+    // Uses Hash to bin hosts into separate locks so we don't accumulate too many locks
+    private val inProgressHosts = ConcurrentHashMap<Int, Mutex>()
+    private const val MAX_LOCKS = 8
+
+    override suspend fun intercept(chain: Interceptor.Chain): ImageResult {
+        if (chain.request.data !is String) {
+            return chain.proceed()
+        }
+
+        val url = chain.request.data as String
+
+        // It can be a data url
+        if (!url.startsWith("http")) {
+            return chain.proceed()
+        }
+
+        val uri =
+            try {
+                Uri.parse(url)
+            } catch (e: Exception) {
+                Log.e(LOG_TAG, "Failed to parse url: $url", e)
+                return chain.proceed()
+            }
+        val host = uri.host ?: return chain.proceed()
+
+        // If we are already fetching an image from this host, wait for it to finish
+        return inProgressHosts.computeIfAbsent(host.hashCode() % MAX_LOCKS) { Mutex() }.withLock {
+            logDebug(LOG_TAG, "Loading image [$host] $url")
+            chain.proceed()
+        }
+    }
+}

app/src/main/java/com/nononsenseapps/feeder/model/RateLimitedInterceptor.kt

@@ -0,0 +1,21 @@
+package com.nononsenseapps.feeder.model
+
+import okhttp3.Interceptor
+import okhttp3.Response
+
+/**
+ * Interceptor that handles Too Many Requests (429) responses for Coil.
+ *
+ * It intercepts requests, checking if a 429 timeout was received. If so, it cancels the request.
+ */
+object RateLimitedInterceptor : Interceptor {
+    private const val LOG_TAG = "FEEDER_RATEINTER"
+
+    override fun intercept(chain: Interceptor.Chain): Response {
+        val request = chain.request()
+
+        return RateLimiter.blockingRateLimited(request.url.host) {
+            chain.proceed(request)
+        }
+    }
+}

app/src/main/java/com/nononsenseapps/feeder/model/RateLimiter.kt

@@ -0,0 +1,73 @@
+package com.nononsenseapps.feeder.model
+
+import com.nononsenseapps.feeder.util.logDebug
+import kotlinx.coroutines.delay
+import java.util.concurrent.ConcurrentHashMap
+import kotlin.random.Random
+
+/**
+ * Limits arbitrary operations to a certain rate.
+ */
+object RateLimiter {
+    private const val LOG_TAG = "FEEDER_RATELIMIT"
+    private const val MAX_PER_SECOND = 3
+    private const val DELAY_MS = 1000L / MAX_PER_SECOND
+    private val lastTime = ConcurrentHashMap<String, Long>()
+
+    /**
+     * Will sleep if the rate limit is exceeded.
+     */
+    fun <T> blockingRateLimited(
+        key: String,
+        block: () -> T,
+    ): T {
+        while (true) {
+            val last = lastTime[key] ?: 0
+            val now = System.currentTimeMillis()
+            val diff = now - last
+            if (diff < DELAY_MS) {
+                val remainingTime = DELAY_MS - diff
+                val sleepTime = Random.nextLong(remainingTime, remainingTime * 2)
+                logDebug(LOG_TAG, "Rate limit hit [$key]. Sleeping for $sleepTime ms")
+                Thread.sleep(sleepTime)
+            } else {
+                return block().also {
+                    lastTime[key] = System.currentTimeMillis()
+                    cleanOldEntries()
+                }
+            }
+        }
+    }
+
+    /**
+     * Will suspend if the rate limit is exceeded.
+     */
+    suspend fun <T> suspendingRateLimited(
+        key: String,
+        block: suspend () -> T,
+    ): T {
+        while (true) {
+            val last = lastTime[key] ?: 0
+            val now = System.currentTimeMillis()
+            val diff = now - last
+            if (diff < DELAY_MS) {
+                val remainingTime = DELAY_MS - diff
+                val sleepTime = Random.nextLong(remainingTime, remainingTime * 2)
+                logDebug(LOG_TAG, "Delaying for $sleepTime ms")
+                delay(sleepTime)
+            } else {
+                return block().also {
+                    lastTime[key] = System.currentTimeMillis()
+                    cleanOldEntries()
+                }
+            }
+        }
+    }
+
+    fun cleanOldEntries() {
+        val now = System.currentTimeMillis()
+        lastTime.entries.removeIf {
+            it.value < now - 60
+        }
+    }
+}

app/src/main/java/com/nononsenseapps/feeder/model/TooManyRequestsInterceptor.kt

@@ -0,0 +1,56 @@
+package com.nononsenseapps.feeder.model
+
+import android.util.Log
+import okhttp3.CacheControl
+import okhttp3.Interceptor
+import okhttp3.Response
+import java.time.Instant
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.TimeUnit
+
+/**
+ * Interceptor that handles Too Many Requests (429) responses for Coil.
+ *
+ * It intercepts requests, checking if a 429 timeout was received. If so, it cancels the request.
+ */
+object TooManyRequestsInterceptor : Interceptor {
+    private val tooManyResponses = ConcurrentHashMap<String, Instant>()
+    private const val LOG_TAG = "FEEDER_TOOMANY"
+
+    override fun intercept(chain: Interceptor.Chain): Response {
+        val request = chain.request()
+        val now = Instant.now()
+
+        if (tooManyResponses[request.url.host]?.isAfter(now) == true) {
+            Log.i(LOG_TAG, "Too many requests for ${request.url.host}, blocked until ${tooManyResponses[request.url.host]}")
+            return chain.proceed(
+                request
+                    .newBuilder()
+                    .cacheControl(
+                        CacheControl
+                            .Builder()
+                            .onlyIfCached()
+                            .maxStale(Int.MAX_VALUE, TimeUnit.SECONDS)
+                            .maxAge(Int.MAX_VALUE, TimeUnit.SECONDS)
+                            .build(),
+                    ).build(),
+            )
+        }
+
+        // Drop the host
+        tooManyResponses.remove(request.url.host)
+        val response = chain.proceed(request)
+
+        if (response.code == 429) {
+            tooManyResponses.computeIfAbsent(request.url.host) {
+                val retryAfter = response.header("Retry-After")
+                val retryAfterSeconds = retryAfter?.toIntOrNull() ?: 60
+                Log.i(LOG_TAG, "Too many requests for ${request.url.host}, will intercept for $retryAfterSeconds seconds")
+
+                now.plusSeconds(retryAfterSeconds.toLong() + 1)
+            }
+        }
+
+        return response
+    }
+}

gradle/libs.versions.toml

@@ -97,7 +97,6 @@ coil-gif = { module = "io.coil-kt.coil3:coil-gif", version.ref = "coil" }
 coil-svg = { module = "io.coil-kt.coil3:coil-svg", version.ref = "coil" }
 coil-compose = { module = "io.coil-kt.coil3:coil-compose", version.ref = "coil" }
 coil-okhttp = { module = "io.coil-kt.coil3:coil-network-okhttp", version.ref = "coil" }
-coil-cache-control = { module = "io.coil-kt.coil3:coil-network-cache-control", version.ref = "coil" }
 kotlin-stdlib = { module = "org.jetbrains.kotlin:kotlin-stdlib", version.ref = "kotlin" }
 kotlin-stdlib-common = { module = "org.jetbrains.kotlin:kotlin-stdlib-common", version.ref = "kotlin" }
 kotlin-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlinxSerialization" }
@@ -144,7 +143,7 @@ ktlint-gradle = { id = "org.jlleitschuh.gradle.ktlint", version.ref = "ktlint-gr
 okhttp-android = ["okhttp", "okio", "conscrypt-android"]
 kotlin = ["kotlin-stdlib", "kotlin-stdlib-common", "kotlin-coroutines-core", "kotlin-serialization-json"]
 jvm = ["jsoup", "tagsoup", "readability4j", "retrofit", "retrofit-converter-moshi", "moshi", "moshi-kotlin", "moshi-adapters", "qrgen", "gofeed-android"]
-android = ["lifecycle-runtime-ktx", "lifecycle-viewmodel-ktx", "lifecycle-viewmodel-savedstate", "paging-runtime-ktx", "room-ktx", "room-paging", "core-ktx", "androidx-appcompat", "androidx-preference", "coil-gif", "coil-svg", "coil-okhttp", "coil-cache-control", "kotlin-coroutines-android", "kodein-androidx", "androidx-browser", "emoji2", "emoji2-view-helper"]
+android = ["lifecycle-runtime-ktx", "lifecycle-viewmodel-ktx", "lifecycle-viewmodel-savedstate", "paging-runtime-ktx", "room-ktx", "room-paging", "core-ktx", "androidx-appcompat", "androidx-preference", "coil-gif", "coil-svg", "coil-okhttp", "kotlin-coroutines-android", "kodein-androidx", "androidx-browser", "emoji2", "emoji2-view-helper"]
 compose = ["activity-compose", "ui", "foundation", "foundation-layout", "compose-material3", "compose-material", "compose-material-icons-extended", "runtime", "ui-tooling", "navigation-compose", "paging-compose", "window", "android-material", "accompanist-permissions", "accompanist-adaptive", "compose-material3-windowsizeclass", "lifecycle-runtime-compose", "coil-compose", "lazycolumnscrollbar"]
 test = ["kotlin-test-junit", "kotlin-coroutines-test", "junit", "mockk", "mockwebserver"]
 android-test = ["kotlin-test-junit", "kotlin-coroutines-test", "mockk-android", "junit", "mockwebserver", "androidx-test-core", "androidx-test-core-ktx", "androidx-test-runner", "androidx-test-junit-ktx", "room-testing", "espresso-core", "compose-ui-test-junit4"]