Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensured retry-after is respected, even when feeds share the same host #292

Merged
merged 1 commit into from
Jun 2, 2024
Merged

Ensured retry-after is respected, even when feeds share the same host #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
787 changes: 787 additions & 0 deletions app/schemas/com.nononsenseapps.feeder.db.room.AppDatabase/36.json
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions app/src/androidTest/java/com/nononsenseapps/feeder/db/room/MigrationFromLegacy5ToLatest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ class MigrationFromLegacy5ToLatest {
)

roomDb.let { db ->
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)

assertEquals("Wrong number of feeds", 2, feeds.size)

Expand Down Expand Up @@ -238,7 +238,7 @@ class MigrationFromLegacy5ToLatest {
)

roomDb.let { db ->
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)

assertEquals("Wrong number of feeds", 2, feeds.size)

Expand Down Expand Up @@ -266,7 +266,7 @@ class MigrationFromLegacy5ToLatest {
)

roomDb.let { db ->
val feed = db.feedDao().loadFeeds()[0]
val feed = db.feedDao().loadFeeds(now = Instant.EPOCH)[0]
assertEquals("feedA", feed.title)
val items =
db.feedItemDao().loadFeedItemsInFeedDesc(feedId = feed.id)
Expand Down Expand Up @@ -302,7 +302,7 @@ class MigrationFromLegacy5ToLatest {
)

roomDb.let { db ->
val feed = db.feedDao().loadFeeds()[1]
val feed = db.feedDao().loadFeeds(now = Instant.EPOCH)[1]
assertEquals("feedB", feed.title)
val items =
db.feedItemDao().loadFeedItemsInFeedDesc(feedId = feed.id)
Expand Down
8 changes: 4 additions & 4 deletions app/src/androidTest/java/com/nononsenseapps/feeder/db/room/MigrationFromLegacy6ToLatest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ class MigrationFromLegacy6ToLatest {
)

roomDb.let { db ->
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)

assertEquals("Wrong number of feeds", 2, feeds.size)

Expand All @@ -213,7 +213,7 @@ class MigrationFromLegacy6ToLatest {
)

roomDb.let { db ->
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)

assertEquals("Wrong number of feeds", 2, feeds.size)

Expand All @@ -240,7 +240,7 @@ class MigrationFromLegacy6ToLatest {
)

roomDb.let { db ->
val feed = db.feedDao().loadFeeds()[0]
val feed = db.feedDao().loadFeeds(now = Instant.EPOCH)[0]
assertEquals("feedA", feed.title)
val items = db.feedItemDao().loadFeedItemsInFeedDesc(feedId = feed.id)

Expand Down Expand Up @@ -274,7 +274,7 @@ class MigrationFromLegacy6ToLatest {
)

roomDb.let { db ->
val feed = db.feedDao().loadFeeds()[1]
val feed = db.feedDao().loadFeeds(now = Instant.EPOCH)[1]
assertEquals("feedB", feed.title)
val items = db.feedItemDao().loadFeedItemsInFeedDesc(feedId = feed.id)

Expand Down
69 changes: 69 additions & 0 deletions app/src/androidTest/java/com/nononsenseapps/feeder/db/room/TestMigrationFrom35To36.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.nononsenseapps.feeder.db.room

import androidx.room.testing.MigrationTestHelper
import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
import androidx.test.platform.app.InstrumentationRegistry
import com.nononsenseapps.feeder.FeederApplication
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.kodein.di.DI
import org.kodein.di.DIAware
import org.kodein.di.android.closestDI
import kotlin.test.assertEquals

@RunWith(AndroidJUnit4::class)
@LargeTest
class TestMigrationFrom35To36 : DIAware {
private val dbName = "testDb"
private val feederApplication: FeederApplication = ApplicationProvider.getApplicationContext()
override val di: DI by closestDI(feederApplication)

@Rule
@JvmField
val testHelper: MigrationTestHelper =
MigrationTestHelper(
InstrumentationRegistry.getInstrumentation(),
AppDatabase::class.java,
emptyList(),
FrameworkSQLiteOpenHelperFactory(),
)

@Test
fun migrate() {
@Suppress("SimpleRedundantLet")
testHelper.createDatabase(dbName, FROM_VERSION).let { oldDB ->
oldDB.execSQL(
"""
INSERT INTO feeds(id, title, url, custom_title, tag, notify, last_sync, response_hash, fulltext_by_default, open_articles_with, alternate_id, currently_syncing, when_modified, site_fetched, skip_duplicates)
VALUES(1, 'feed', 'http://url', '', '', 0, 0, 666, 0, '', 0, 0, 0, 0, 0)
""".trimIndent(),
)
}
val db =
testHelper.runMigrationsAndValidate(
dbName,
TO_VERSION,
true,
MigrationFrom35To36(di),
)

db.query(
"""
select retry_after from feeds
""".trimIndent(),
).use {
assert(it.count == 1)
assert(it.moveToFirst())
assertEquals(0, it.getLong(0))
}
}

companion object {
private const val FROM_VERSION = 35
private const val TO_VERSION = 36
}
}
70 changes: 62 additions & 8 deletions app/src/androidTest/java/com/nononsenseapps/feeder/model/RssLocalSyncKtTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ class RssLocalSyncKtTest : DIAware {
bind<SyncRemoteDao>() with singleton { testDb.db.syncRemoteDao() }
bind<FeedStore>() with singleton { FeedStore(di) }
bind<FeedItemStore>() with singleton { FeedItemStore(di) }
bind<SettingsStore>() with singleton { SettingsStore(di) }
bind<SettingsStore>() with
singleton {
SettingsStore(di).also {
it.setAddedFeederNews(true)
}
}
bind<SessionStore>() with singleton { SessionStore() }
bind<SyncRemoteStore>() with singleton { SyncRemoteStore(di) }
bind<OkHttpClient>() with singleton { cachingHttpClient() }
Expand Down Expand Up @@ -276,7 +281,7 @@ class RssLocalSyncKtTest : DIAware {

runBlocking {
rssLocalSync.syncFeeds(feedId = cowboyJsonId, forceNetwork = true)
testDb.db.feedDao().loadFeed(cowboyJsonId)!!.let { feed ->
testDb.db.feedDao().loadFeed(cowboyJsonId, now = Instant.EPOCH)!!.let { feed ->
assertTrue("Feed should have been synced", feed.lastSync.toEpochMilli() > 0)
// assertTrue("Feed should have a valid response hash", feed.responseHash > 0)
// "Long time" ago, but not unset
Expand All @@ -290,7 +295,7 @@ class RssLocalSyncKtTest : DIAware {
assertNotEquals(
"Cached response should still have updated feed last sync",
999L,
testDb.db.feedDao().loadFeed(cowboyJsonId)!!.lastSync.toEpochMilli(),
testDb.db.feedDao().loadFeed(cowboyJsonId, now = Instant.EPOCH)!!.lastSync.toEpochMilli(),
)
}

Expand All @@ -307,7 +312,7 @@ class RssLocalSyncKtTest : DIAware {
val fourteenMinsAgo = Instant.now().minusMinutes(14)
runBlocking {
rssLocalSync.syncFeeds(feedId = cowboyJsonId, forceNetwork = true)
testDb.db.feedDao().loadFeed(cowboyJsonId)!!.let { feed ->
testDb.db.feedDao().loadFeed(cowboyJsonId, now = Instant.EPOCH)!!.let { feed ->
assertTrue("Feed should have been synced", feed.lastSync.toEpochMilli() > 0)
// assertTrue("Feed should have a valid response hash", feed.responseHash > 0)

Expand All @@ -329,10 +334,59 @@ class RssLocalSyncKtTest : DIAware {
assertEquals(
"Last sync should not have changed",
fourteenMinsAgo,
testDb.db.feedDao().loadFeed(cowboyJsonId)!!.lastSync,
testDb.db.feedDao().loadFeed(cowboyJsonId, now = Instant.EPOCH)!!.lastSync,
)
}

@Test
fun feedsGetRetryAfterSetAndWillNotSyncLater() =
runBlocking {
val cowboyJsonId =
testDb.db.feedDao().insertFeed(
Feed(
title = "feed1",
url = server.url("/feed.json").toUrl(),
tag = "",
),
)

val someOtherFeedId =
testDb.db.feedDao().insertFeed(
Feed(
title = "feed2",
url = server.url("/feed2.json").toUrl(),
tag = "",
),
)

val response =
MockResponse().also {
it.setResponseCode(429)
it.setHeader("Retry-After", "30")
}
server.enqueue(response)

rssLocalSync.syncFeeds(feedId = cowboyJsonId)
assertEquals("Request should have been sent ", 1, server.requestCount)

// Get the feed and check value
val feed = testDb.db.feedDao().getFeed(cowboyJsonId)!!
assertTrue("Feed should have a retry after value in the future") {
feed.retryAfter > Instant.now()
}

val feed2 = testDb.db.feedDao().getFeed(someOtherFeedId)!!
assertTrue("Feed should have a retry after value in the future") {
feed2.retryAfter > Instant.now()
}

rssLocalSync.syncFeeds(feedId = cowboyJsonId)
assertEquals("Request should not have been sent due to retry-after", 1, server.requestCount)

rssLocalSync.syncFeeds(feedId = someOtherFeedId)
assertEquals("Request should not have been sent due to retry-after", 1, server.requestCount)
}

@Test
fun feedsSyncedWithin15MinAreNotIgnoredWhenForcingNetwork() =
runBlocking {
Expand All @@ -346,7 +400,7 @@ class RssLocalSyncKtTest : DIAware {
val fourteenMinsAgo = Instant.now().minusMinutes(14)
runBlocking {
rssLocalSync.syncFeeds(feedId = cowboyJsonId, forceNetwork = true)
testDb.db.feedDao().loadFeed(cowboyJsonId)!!.let { feed ->
testDb.db.feedDao().loadFeed(cowboyJsonId, now = Instant.EPOCH)!!.let { feed ->
assertTrue("Feed should have been synced", feed.lastSync.toEpochMilli() > 0)
// assertTrue("Feed should have a valid response hash", feed.responseHash > 0)

Expand All @@ -364,7 +418,7 @@ class RssLocalSyncKtTest : DIAware {
assertNotEquals(
"Last sync should have changed",
fourteenMinsAgo,
testDb.db.feedDao().loadFeed(cowboyJsonId)!!.lastSync,
testDb.db.feedDao().loadFeed(cowboyJsonId, now = Instant.EPOCH)!!.lastSync,
)
}

Expand Down Expand Up @@ -395,7 +449,7 @@ class RssLocalSyncKtTest : DIAware {
assertNotEquals(
"Last sync should have been updated",
Instant.EPOCH,
testDb.db.feedDao().loadFeed(failingJsonId)!!.lastSync,
testDb.db.feedDao().loadFeed(failingJsonId, now = Instant.EPOCH)!!.lastSync,
)

// Assert the feed was retrieved
Expand Down
19 changes: 10 additions & 9 deletions app/src/androidTest/java/com/nononsenseapps/feeder/model/opml/OPMLTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import org.kodein.di.singleton
import java.io.File
import java.io.IOException
import java.net.URL
import java.time.Instant
import kotlin.random.Random

@RunWith(AndroidJUnit4::class)
Expand Down Expand Up @@ -108,7 +109,7 @@ class OPMLTest : DIAware {
blockedPatterns = BLOCKED_PATTERNS,
tags = getTags(),
) { tag ->
db.feedDao().loadFeeds(tag = tag)
db.feedDao().getFeedsByTitle(tag = tag)
}

// check contents of file
Expand Down Expand Up @@ -152,7 +153,7 @@ class OPMLTest : DIAware {

// Verify database is correct
val seen = ArrayList<Int>()
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)
assertFalse("No feeds in DB!", feeds.isEmpty())
for (feed in feeds) {
val i = Integer.parseInt(feed.title.replace("[custom \"]".toRegex(), ""))
Expand Down Expand Up @@ -225,7 +226,7 @@ class OPMLTest : DIAware {

// should not kill the existing stuff
val seen = ArrayList<Int>()
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)
assertFalse("No feeds in DB!", feeds.isEmpty())
for (feed in feeds) {
val i = Integer.parseInt(feed.title.replace("[custom \"]".toRegex(), ""))
Expand Down Expand Up @@ -315,7 +316,7 @@ class OPMLTest : DIAware {
val parser = OpmlPullParser(opmlParserHandler)
parser.parseFile(path!!.absolutePath)

val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)
assertTrue("Expected no feeds and no exception", feeds.isEmpty())
}

Expand All @@ -339,7 +340,7 @@ class OPMLTest : DIAware {
// Then delete all feeds again
db.runInTransaction {
runBlocking {
db.feedDao().loadFeeds().forEach {
db.feedDao().loadFeeds(now = Instant.EPOCH).forEach {
db.feedDao().deleteFeed(it)
}
}
Expand Down Expand Up @@ -394,7 +395,7 @@ class OPMLTest : DIAware {
parser.parseInputStreamWithFallback(opmlStream)

// then
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)
val tags = db.feedDao().loadTags()
assertEquals("Expecting 8 feeds", 8, feeds.size)
assertEquals("Expecting 1 tags (incl empty)", 1, tags.size)
Expand Down Expand Up @@ -451,7 +452,7 @@ class OPMLTest : DIAware {
parser.parseInputStreamWithFallback(opmlStream)

// then
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)
val tags = db.feedDao().loadTags()
assertEquals("Expecting 11 feeds", 11, feeds.size)
assertEquals("Expecting 4 tags (incl empty)", 4, tags.size)
Expand Down Expand Up @@ -541,7 +542,7 @@ class OPMLTest : DIAware {
parser.parseInputStreamWithFallback(opmlStream)

// then
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)
val tags = db.feedDao().loadTags()
assertEquals("Expecting 30 feeds", 30, feeds.size)
assertEquals("Expecting 6 tags (incl empty)", 6, tags.size)
Expand Down Expand Up @@ -662,7 +663,7 @@ class OPMLTest : DIAware {
parser.parseInputStreamWithFallback(opmlStream)

// then
val feeds = db.feedDao().loadFeeds()
val feeds = db.feedDao().loadFeeds(now = Instant.EPOCH)
val tags = db.feedDao().loadTags()
assertEquals("Expecting 30 feeds", 30, feeds.size)
assertEquals("Expecting 6 tags (incl empty)", 6, tags.size)
Expand Down
Loading
Loading