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.

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

Fixed performance when many entries in blocklist #272

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading