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

Memory fixes #259

Merged
merged 4 commits into from
Apr 9, 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
766 changes: 766 additions & 0 deletions app/schemas/com.nononsenseapps.feeder.db.room.AppDatabase/34.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 TestMigrationFrom33To34 : 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,
MigrationFrom33To34(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 = 33
private const val TO_VERSION = 34
}
}
77 changes: 15 additions & 62 deletions app/src/main/java/com/nononsenseapps/feeder/archmodel/FeedStore.kt
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
package com.nononsenseapps.feeder.archmodel

import android.database.sqlite.SQLiteConstraintException
import androidx.paging.Pager
import androidx.paging.PagingConfig
import androidx.paging.PagingData
import com.nononsenseapps.feeder.db.room.Feed
import com.nononsenseapps.feeder.db.room.FeedDao
import com.nononsenseapps.feeder.db.room.FeedForSettings
import com.nononsenseapps.feeder.db.room.FeedTitle
import com.nononsenseapps.feeder.db.room.ID_UNSET
import com.nononsenseapps.feeder.model.FeedUnreadCount
import com.nononsenseapps.feeder.ui.compose.navdrawer.DrawerFeed
import com.nononsenseapps.feeder.ui.compose.navdrawer.DrawerItemWithUnreadCount
import com.nononsenseapps.feeder.ui.compose.navdrawer.DrawerTag
import com.nononsenseapps.feeder.ui.compose.navdrawer.DrawerTop
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.mapLatest
import org.kodein.di.DI
import org.kodein.di.DIAware
import org.kodein.di.instance
Expand Down Expand Up @@ -64,59 +61,19 @@ class FeedStore(override val di: DI) : DIAware {

val feedForSettings: Flow<List<FeedForSettings>> = feedDao.loadFlowOfFeedsForSettings()

@OptIn(ExperimentalCoroutinesApi::class)
val drawerItemsWithUnreadCounts: Flow<List<DrawerItemWithUnreadCount>> =
feedDao.loadFlowOfFeedsWithUnreadCounts()
.mapLatest { feeds ->
// TODO would like to have a throttle here. Emit first immediately
// then at most every X ms (including latest item
// Must emit first immediately or the feed list will have a delay
mapFeedsToSortedDrawerItems(feeds)
}

private fun mapFeedsToSortedDrawerItems(feeds: List<FeedUnreadCount>): List<DrawerItemWithUnreadCount> {
var topTag = DrawerTop(unreadCount = 0, totalChildren = 0)
val tags: MutableMap<String, DrawerTag> = mutableMapOf()
val data: MutableList<DrawerItemWithUnreadCount> = mutableListOf()

for (feedDbo in feeds) {
val feed =
DrawerFeed(
unreadCount = feedDbo.unreadCount,
tag = feedDbo.tag,
id = feedDbo.id,
displayTitle = feedDbo.displayTitle,
imageUrl = feedDbo.imageUrl,
)

data.add(feed)
topTag =
topTag.copy(
unreadCount = topTag.unreadCount + feed.unreadCount,
totalChildren = topTag.totalChildren + 1,
)

if (feed.tag.isNotEmpty()) {
val tag =
tags[feed.tag] ?: DrawerTag(
tag = feed.tag,
unreadCount = 0,
uiId = getTagUiId(feed.tag),
totalChildren = 0,
)
tags[feed.tag] =
tag.copy(
unreadCount = tag.unreadCount + feed.unreadCount,
totalChildren = tag.totalChildren + 1,
)
}
fun getPagedNavDrawerItems(expandedTags: Set<String>): Flow<PagingData<FeedUnreadCount>> =
Pager(
config =
PagingConfig(
pageSize = 10,
initialLoadSize = 50,
prefetchDistance = 50,
jumpThreshold = 50,
),
) {
feedDao.getPagedNavDrawerItems(expandedTags)
}

data.add(topTag)
data.addAll(tags.values)

return data.sorted()
}
.flow

fun getFeedTitles(
feedId: Long,
Expand Down Expand Up @@ -165,10 +122,6 @@ class FeedStore(override val di: DI) : DIAware {
return feedDao.getFeedsOrderedByUrl()
}

fun getFlowOfFeedsOrderedByUrl(): Flow<List<Feed>> {
return feedDao.getFlowOfFeedsOrderedByUrl()
}

suspend fun deleteFeed(url: URL) {
feedDao.deleteFeedWithUrl(url)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.nononsenseapps.feeder.db.room.ID_UNSET
import com.nononsenseapps.feeder.db.room.RemoteFeed
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.SyncServiceSendReadWorker
import com.nononsenseapps.feeder.model.workmanager.requestFeedSync
Expand All @@ -33,7 +34,6 @@ import com.nononsenseapps.feeder.sync.SyncRestClient
import com.nononsenseapps.feeder.ui.compose.feed.FeedListItem
import com.nononsenseapps.feeder.ui.compose.feedarticle.FeedListFilter
import com.nononsenseapps.feeder.ui.compose.feedarticle.emptyFeedListFilter
import com.nononsenseapps.feeder.ui.compose.navdrawer.DrawerItemWithUnreadCount
import com.nononsenseapps.feeder.util.Either
import com.nononsenseapps.feeder.util.addDynamicShortcutToFeed
import com.nononsenseapps.feeder.util.logDebug
Expand Down Expand Up @@ -495,8 +495,10 @@ class Repository(override val di: DI) : DIAware {

val allTags: Flow<List<String>> = feedStore.allTags

val drawerItemsWithUnreadCounts: Flow<List<DrawerItemWithUnreadCount>> =
feedStore.drawerItemsWithUnreadCounts
fun getPagedNavDrawerItems(): Flow<PagingData<FeedUnreadCount>> =
expandedTags.flatMapLatest {
feedStore.getPagedNavDrawerItems(it)
}

val getUnreadBookmarksCount
get() =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ private const val LOG_TAG = "FEEDER_APPDB"
RemoteFeed::class,
SyncDevice::class,
],
version = 33,
views = [
FeedsWithItemsForNavDrawer::class,
],
version = 34,
)
@TypeConverters(Converters::class)
abstract class AppDatabase : RoomDatabase() {
Expand Down Expand Up @@ -128,12 +131,21 @@ fun getAllMigrations(di: DI) =
MigrationFrom30To31(di),
MigrationFrom31To32(di),
MigrationFrom32To33(di),
MigrationFrom33To34(di),
)

/*
* 6 represents legacy database
* 7 represents new Room database
*/
class MigrationFrom33To34(override val di: DI) : Migration(33, 34), DIAware {
override fun migrate(database: SupportSQLiteDatabase) {
// 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 not exists(select 1 from blocklist where lower(feed_items.plain_title) glob blocklist.glob_pattern)\n )\n ON feeds.id = feed_id"
database.execSQL(sql)
}
}

class MigrationFrom32To33(override val di: DI) : Migration(32, 33), DIAware {
override fun migrate(database: SupportSQLiteDatabase) {
database.execSQL(
Expand Down
43 changes: 30 additions & 13 deletions app/src/main/java/com/nononsenseapps/feeder/db/room/FeedDao.kt
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.nononsenseapps.feeder.db.room

import android.database.Cursor
import androidx.paging.PagingSource
import androidx.room.Dao
import androidx.room.Delete
import androidx.room.Insert
import androidx.room.OnConflictStrategy
import androidx.room.Query
import androidx.room.RoomWarnings
import androidx.room.Update
import androidx.room.Upsert
import com.nononsenseapps.feeder.db.COL_CUSTOM_TITLE
Expand Down Expand Up @@ -77,7 +79,7 @@ interface FeedDao {
@Query("SELECT * FROM feeds WHERE tag IS :tag")
suspend fun loadFeeds(tag: String): List<Feed>

@Query("SELECT * FROM feeds WHERE tag IS :tag AND last_sync < :staleTime")
@Query("SELECT * FROM feeds WHERE tag IS :tag AND last_sync < :staleTime ORDER BY last_sync")
suspend fun loadFeedsIfStale(
tag: String,
staleTime: Long,
Expand All @@ -95,7 +97,7 @@ interface FeedDao {
)
fun loadFeedsForContentProvider(): Cursor

@Query("SELECT * FROM feeds WHERE last_sync < :staleTime")
@Query("SELECT * FROM feeds WHERE last_sync < :staleTime ORDER BY last_sync")
suspend fun loadFeedsIfStale(staleTime: Long): List<Feed>

@Query("SELECT * FROM feeds WHERE url IS :url")
Expand All @@ -104,20 +106,35 @@ interface FeedDao {
@Query("SELECT id FROM feeds WHERE notify IS 1")
suspend fun loadFeedIdsToNotify(): List<Long>

// Suppressing sort fields
@SuppressWarnings(RoomWarnings.CURSOR_MISMATCH)
@Query(
"""
SELECT id, title, url, tag, custom_title, notify, currently_syncing, image_url, unread_count
FROM feeds
LEFT JOIN (SELECT COUNT(1) AS unread_count, feed_id
FROM feed_items
WHERE read_time is null
AND NOT EXISTS (SELECT 1 FROM blocklist WHERE lower(feed_items.plain_title) GLOB blocklist.glob_pattern)
GROUP BY feed_id
)
ON feeds.id = feed_id
""",
-- all items
select $ID_ALL_FEEDS as id, '' as display_title, '' as tag, '' as image_url, sum(unread) as unread_count, 0 as expanded, 0 as sort_section, 0 as sort_tag_or_feed
from feeds_with_items_for_nav_drawer
-- starred
union
select $ID_SAVED_ARTICLES as id, '' as display_title, '' as tag, '' as image_url, sum(unread) as unread_count, 0 as expanded, 1 as sort_section, 0 as sort_tag_or_feed
from feeds_with_items_for_nav_drawer
where bookmarked
-- tags
union
select $ID_UNSET as id, tag as display_title, tag, '' as image_url, sum(unread) as unread_count, tag in (:expandedTags) as expanded, 2 as sort_section, 0 as sort_tag_or_feed
from feeds_with_items_for_nav_drawer
where tag is not ''
group by tag
-- feeds
union
select feed_id as id, display_title, tag, image_url, sum(unread) as unread_count, 0 as expanded, case when tag is '' then 3 else 2 end as sort_section, 1 as sort_tag_or_feed
from feeds_with_items_for_nav_drawer
where tag is '' or tag in (:expandedTags)
group by feed_id
-- sort them
order by sort_section, tag, sort_tag_or_feed, display_title
""",
)
fun loadFlowOfFeedsWithUnreadCounts(): Flow<List<FeedUnreadCount>>
fun getPagedNavDrawerItems(expandedTags: Set<String>): PagingSource<Int, FeedUnreadCount>

@Query("UPDATE feeds SET notify = :notify WHERE id IS :id")
suspend fun setNotify(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.nononsenseapps.feeder.db.room

import androidx.room.ColumnInfo
import androidx.room.DatabaseView

@DatabaseView(
value = """
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
from feeds
left join (
select id as item_id, feed_id, read_time is null as unread, bookmarked
from feed_items
where not exists(select 1 from blocklist where lower(feed_items.plain_title) glob blocklist.glob_pattern)
)
ON feeds.id = feed_id
""",
viewName = "feeds_with_items_for_nav_drawer",
)
data class FeedsWithItemsForNavDrawer(
@ColumnInfo(name = "feed_id")
val feedId: Long,
val tag: String,
@ColumnInfo(name = "display_title")
val displayTitle: String,
@ColumnInfo(name = "image_url")
val imageUrl: String?,
val unread: Boolean,
@ColumnInfo(name = "item_id")
val itemId: Long?,
val bookmarked: Boolean,
)
Loading
Loading