Skip to content

Commit

Permalink
Fixed performance when many entries in blocklist
Browse files Browse the repository at this point in the history
  • Loading branch information
spacecowboy committed Apr 24, 2024
1 parent 313e91b commit 8a86acb
Show file tree
Hide file tree
Showing 13 changed files with 1,032 additions and 20 deletions.
781 changes: 781 additions & 0 deletions app/schemas/com.nononsenseapps.feeder.db.room.AppDatabase/35.json

Large diffs are not rendered by default.

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 TestMigrationFrom34To35 : 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,
MigrationFrom34To35(di),
)

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

companion object {
private const val FROM_VERSION = 34
private const val TO_VERSION = 35
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import androidx.paging.PagingData
import androidx.paging.map
import androidx.sqlite.db.SimpleSQLiteQuery
import com.nononsenseapps.feeder.db.room.AppDatabase
import com.nononsenseapps.feeder.db.room.BlocklistDao
import com.nononsenseapps.feeder.db.room.FeedItem
import com.nononsenseapps.feeder.db.room.FeedItemCursor
import com.nononsenseapps.feeder.db.room.FeedItemDao
Expand Down Expand Up @@ -38,8 +39,16 @@ import java.util.Locale

class FeedItemStore(override val di: DI) : DIAware {
private val dao: FeedItemDao by instance()
private val blocklistDao: BlocklistDao by instance()
private val appDatabase: AppDatabase by instance()

suspend fun setBlockStatusForNewInFeed(
feedId: Long,
blockTime: Instant,
) {
blocklistDao.setItemBlockStatusForNewInFeed(feedId, blockTime)
}

fun getFeedItemCountRaw(
feedId: Long,
tag: String,
Expand Down Expand Up @@ -109,7 +118,7 @@ class FeedItemStore(override val di: DI) : DIAware {
val onlySavedArticles = feedId == ID_SAVED_ARTICLES

// Always blocklist
append("NOT EXISTS (SELECT 1 FROM blocklist WHERE lower(feed_items.plain_title) GLOB blocklist.glob_pattern)\n")
append("block_time is null\n")
// List filter
if (!onlySavedArticles) {
append("AND (\n")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.nononsenseapps.feeder.db.room.SyncDevice
import com.nononsenseapps.feeder.db.room.SyncRemote
import com.nononsenseapps.feeder.model.FeedUnreadCount
import com.nononsenseapps.feeder.model.ThumbnailImage
import com.nononsenseapps.feeder.model.workmanager.BlockListWorker
import com.nononsenseapps.feeder.model.workmanager.SyncServiceSendReadWorker
import com.nononsenseapps.feeder.model.workmanager.requestFeedSync
import com.nononsenseapps.feeder.sync.DeviceListResponse
Expand Down Expand Up @@ -187,9 +188,22 @@ class Repository(override val di: DI) : DIAware {

val blockList: Flow<List<String>> = settingsStore.blockListPreference

suspend fun addBlocklistPattern(pattern: String) = settingsStore.addBlocklistPattern(pattern)
suspend fun addBlocklistPattern(pattern: String) {
settingsStore.addBlocklistPattern(pattern)
scheduleBlockListUpdate(0)
}

suspend fun removeBlocklistPattern(pattern: String) {
settingsStore.removeBlocklistPattern(pattern)
scheduleBlockListUpdate(0)
}

suspend fun removeBlocklistPattern(pattern: String) = settingsStore.removeBlocklistPattern(pattern)
suspend fun setBlockStatusForNewInFeed(
feedId: Long,
blockTime: Instant,
) {
feedItemStore.setBlockStatusForNewInFeed(feedId, blockTime)
}

val currentSorting: StateFlow<SortingOptions> = settingsStore.currentSorting

Expand Down Expand Up @@ -701,6 +715,26 @@ class Repository(override val di: DI) : DIAware {
)
}

fun scheduleBlockListUpdate(delaySeconds: Long) {
logDebug(LOG_TAG, "Scheduling work")

val constraints =
Constraints.Builder()

val workRequest =
OneTimeWorkRequestBuilder<BlockListWorker>()
.addTag("feeder")
.keepResultsForAtLeast(5, TimeUnit.MINUTES)
.setConstraints(constraints.build())
.setInitialDelay(delaySeconds, TimeUnit.SECONDS)

workManager.enqueueUniqueWork(
BlockListWorker.UNIQUE_BLOCKLIST_NAME,
ExistingWorkPolicy.REPLACE,
workRequest.build(),
)
}

suspend fun loadFeedIfStale(
feedId: Long,
staleTime: Long,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const val COL_SITE_FETCHED = "site_fetched"
const val COL_WORD_COUNT = "word_count"
const val COL_WORD_COUNT_FULL = "word_count_full"
const val COL_SKIP_DUPLICATES = "skip_duplicates"
const val COL_BLOCK_TIME = "block_time"

// year 5000
val FAR_FUTURE = Instant.ofEpochSecond(95635369646)
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import androidx.room.TypeConverters
import androidx.room.migration.Migration
import androidx.sqlite.db.SupportSQLiteDatabase
import com.nononsenseapps.feeder.FeederApplication
import com.nononsenseapps.feeder.archmodel.Repository
import com.nononsenseapps.feeder.blob.blobOutputStream
import com.nononsenseapps.feeder.crypto.AesCbcWithIntegrity
import com.nononsenseapps.feeder.util.FilePathProvider
Expand Down Expand Up @@ -53,7 +54,7 @@ private const val LOG_TAG = "FEEDER_APPDB"
views = [
FeedsWithItemsForNavDrawer::class,
],
version = 34,
version = 35,
)
@TypeConverters(Converters::class)
abstract class AppDatabase : RoomDatabase() {
Expand Down Expand Up @@ -132,12 +133,30 @@ fun getAllMigrations(di: DI) =
MigrationFrom31To32(di),
MigrationFrom32To33(di),
MigrationFrom33To34(di),
MigrationFrom34To35(di),
)

/*
* 6 represents legacy database
* 7 represents new Room database
*/
class MigrationFrom34To35(override val di: DI) : Migration(34, 35), DIAware {
private val repository: Repository by instance()

override fun migrate(database: SupportSQLiteDatabase) {
database.execSQL("drop view feeds_with_items_for_nav_drawer")

database.execSQL("alter table feed_items add column block_time integer default null")
database.execSQL("create index index_feed_items_block_time on feed_items (block_time)")

// Room schema is anal about whitespace
val sql = "CREATE VIEW `feeds_with_items_for_nav_drawer` AS select feeds.id as feed_id, item_id, case when custom_title is '' then title else custom_title end as display_title, tag, image_url, unread, bookmarked\n from feeds\n left join (\n select id as item_id, feed_id, read_time is null as unread, bookmarked\n from feed_items\n where block_time is null\n )\n ON feeds.id = feed_id"
database.execSQL(sql)

repository.scheduleBlockListUpdate(0)
}
}

class MigrationFrom33To34(override val di: DI) : Migration(33, 34), DIAware {
override fun migrate(database: SupportSQLiteDatabase) {
// Room schema is anal about whitespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.nononsenseapps.feeder.db.room
import androidx.room.Dao
import androidx.room.Query
import kotlinx.coroutines.flow.Flow
import java.time.Instant

@Dao
interface BlocklistDao {
Expand Down Expand Up @@ -30,4 +31,45 @@ interface BlocklistDao {
""",
)
fun getGlobPatterns(): Flow<List<String>>

@Query(
"""
update feed_items
set block_time = case
when exists(select 1 from blocklist where lower(feed_items.plain_title) glob blocklist.glob_pattern)
then coalesce(block_time, :blockTime)
else null
end
""",
)
suspend fun setItemBlockStatus(blockTime: Instant)

@Query(
"""
update feed_items
set block_time = case
when exists(select 1 from blocklist where lower(feed_items.plain_title) glob blocklist.glob_pattern)
then :blockTime
else null
end
where block_time is null
""",
)
suspend fun setItemBlockStatusWhereNull(blockTime: Instant)

@Query(
"""
update feed_items
set block_time = case
when exists(select 1 from blocklist where lower(feed_items.plain_title) glob blocklist.glob_pattern)
then :blockTime
else null
end
where feed_id = :feedId and block_time is null
""",
)
suspend fun setItemBlockStatusForNewInFeed(
feedId: Long,
blockTime: Instant,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import androidx.room.Ignore
import androidx.room.Index
import androidx.room.PrimaryKey
import com.nononsenseapps.feeder.db.COL_AUTHOR
import com.nononsenseapps.feeder.db.COL_BLOCK_TIME
import com.nononsenseapps.feeder.db.COL_BOOKMARKED
import com.nononsenseapps.feeder.db.COL_ENCLOSURELINK
import com.nononsenseapps.feeder.db.COL_ENCLOSURE_TYPE
Expand Down Expand Up @@ -47,6 +48,7 @@ private val patternWhitespace = "\\s+".toRegex()
indices = [
Index(value = [COL_GUID, COL_FEEDID], unique = true),
Index(value = [COL_FEEDID]),
Index(value = [COL_BLOCK_TIME]),
Index(
name = "idx_feed_items_cursor",
value = [COL_PRIMARYSORTTIME, COL_PUBDATE, COL_ID],
Expand Down Expand Up @@ -116,6 +118,7 @@ data class FeedItem
) var readTime: Instant? = null,
@ColumnInfo(name = COL_WORD_COUNT) var wordCount: Int = 0,
@ColumnInfo(name = COL_WORD_COUNT_FULL) var wordCountFull: Int = 0,
@ColumnInfo(name = COL_BLOCK_TIME) var blockTime: Instant? = null,
) : FeedItemForFetching, FeedItemCursor {
constructor() : this(id = ID_UNSET)

Expand Down
Loading

0 comments on commit 8a86acb

Please sign in to comment.