diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt index 6726b72e..4ec0056f 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt @@ -186,45 +186,20 @@ internal fun rediscoverAlbumsQuery( ), ) -// RoomRawQuery binds by position, so the named fragments are rewritten once here instead of -// every caller carrying an index for each repeated :sourceId. +// RoomRawQuery takes a positional bind function, but SQLite gives a repeated named parameter one +// index, so the SQL can keep its names and only the first-appearance order needs binding. internal fun rawQuery( sql: String, bindings: Map, ): RoomRawQuery { - val names = mutableListOf() - val positional = - buildString(sql.length) { - var index = 0 - var quote: Char? = null - while (index < sql.length) { - val char = sql[index] - if (quote != null) { - append(char) - if (char == quote) quote = null - index++ - continue - } - if (char == '\'' || char == '"') { - quote = char - append(char) - index++ - continue - } - if (char == ':' && sql.getOrNull(index + 1).let { it != null && (it.isLetter() || it == '_') }) { - var end = index + 1 - while (end < sql.length && (sql[end].isLetterOrDigit() || sql[end] == '_')) end++ - names += sql.substring(index + 1, end) - append('?') - index = end - continue - } - append(char) - index++ - } - } - val values = names.map(bindings::getValue) - return RoomRawQuery(positional) { statement -> + val values = + Regex(":(\\w+)") + .findAll(sql) + .map { it.groupValues[1] } + .distinct() + .map(bindings::getValue) + .toList() + return RoomRawQuery(sql) { statement -> values.forEachIndexed { index, value -> statement.bind(index + 1, value) } } } diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index d5cd52a8..9a59d3d2 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -195,7 +195,16 @@ class LibraryRepository( userPreferences.sortNameMode().flatMapLatest { mode -> pager(40, enablePlaceholders = true) { val dao = db.libraryDao() - val query = albumsPageQuery(sourceId, starred.ordinal, search, sort, descending, downloaded, mode) + val query = + albumsPageQuery( + sourceId = sourceId, + starredFilter = starred.ordinal, + search = search, + sort = sort, + descending = descending, + downloaded = downloaded, + sortNameMode = mode, + ) if (downloaded) dao.downloadedAlbumsPaging(query) else dao.albumsPaging(query) } } @@ -211,7 +220,16 @@ class LibraryRepository( userPreferences.sortNameMode().flatMapLatest { mode -> pager(60) { val dao = db.libraryDao() - val query = artistsPageQuery(sourceId, starred.ordinal, search, sort, descending, downloaded, mode) + val query = + artistsPageQuery( + sourceId = sourceId, + starredFilter = starred.ordinal, + search = search, + sort = sort, + descending = descending, + downloaded = downloaded, + sortNameMode = mode, + ) if (downloaded) dao.downloadedArtistsPaging(query) else dao.artistsPaging(query) } } @@ -225,7 +243,14 @@ class LibraryRepository( ): Flow> = pager(40) { val dao = db.libraryDao() - val query = playlistsPageQuery(sourceId, search, sort, descending, downloaded) + val query = + playlistsPageQuery( + sourceId = sourceId, + search = search, + sort = sort, + descending = descending, + downloaded = downloaded, + ) if (downloaded) dao.downloadedPlaylistsPaging(query) else dao.playlistsPaging(query) } diff --git a/app/src/test/kotlin/com/subtracks/data/db/LibraryPagingTestSupport.kt b/app/src/test/kotlin/com/subtracks/data/db/LibraryPagingTestSupport.kt index e8be9c56..eae667bf 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryPagingTestSupport.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryPagingTestSupport.kt @@ -19,9 +19,29 @@ internal fun LibraryDao.albumPages( sortNameMode: SortNameMode = SortNameMode.SortName, ): PagingSource = if (downloaded) { - downloadedAlbumsPaging(albumsPageQuery(sourceId, starredFilter, search, sort, descending, true, sortNameMode)) + downloadedAlbumsPaging( + albumsPageQuery( + sourceId = sourceId, + starredFilter = starredFilter, + search = search, + sort = sort, + descending = descending, + downloaded = true, + sortNameMode = sortNameMode, + ), + ) } else { - albumsPaging(albumsPageQuery(sourceId, starredFilter, search, sort, descending, false, sortNameMode)) + albumsPaging( + albumsPageQuery( + sourceId = sourceId, + starredFilter = starredFilter, + search = search, + sort = sort, + descending = descending, + downloaded = false, + sortNameMode = sortNameMode, + ), + ) } internal fun LibraryDao.artistPages( @@ -34,9 +54,29 @@ internal fun LibraryDao.artistPages( sortNameMode: SortNameMode = SortNameMode.SortName, ): PagingSource = if (downloaded) { - downloadedArtistsPaging(artistsPageQuery(sourceId, starredFilter, search, sort, descending, true, sortNameMode)) + downloadedArtistsPaging( + artistsPageQuery( + sourceId = sourceId, + starredFilter = starredFilter, + search = search, + sort = sort, + descending = descending, + downloaded = true, + sortNameMode = sortNameMode, + ), + ) } else { - artistsPaging(artistsPageQuery(sourceId, starredFilter, search, sort, descending, false, sortNameMode)) + artistsPaging( + artistsPageQuery( + sourceId = sourceId, + starredFilter = starredFilter, + search = search, + sort = sort, + descending = descending, + downloaded = false, + sortNameMode = sortNameMode, + ), + ) } internal fun LibraryDao.playlistPages( @@ -47,7 +87,23 @@ internal fun LibraryDao.playlistPages( downloaded: Boolean = false, ): PagingSource = if (downloaded) { - downloadedPlaylistsPaging(playlistsPageQuery(sourceId, search, sort, descending, true)) + downloadedPlaylistsPaging( + playlistsPageQuery( + sourceId = sourceId, + search = search, + sort = sort, + descending = descending, + downloaded = true, + ), + ) } else { - playlistsPaging(playlistsPageQuery(sourceId, search, sort, descending, false)) + playlistsPaging( + playlistsPageQuery( + sourceId = sourceId, + search = search, + sort = sort, + descending = descending, + downloaded = false, + ), + ) } diff --git a/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt b/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt index ac5b93eb..30370648 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt @@ -113,17 +113,21 @@ class QueryTimingMeasurementTest { private suspend fun measureReads() { val dao = db.libraryDao() - timed("albums first page (name)") { dao.albumPages(1, 0, "", sort = AlbumSort.Name, downloaded = true).firstPage() } - timed("albums first page (name, default)") { dao.albumPages(1, 0, "", sort = AlbumSort.Name).firstPage() } - timed("albums first page (artist)") { dao.albumPages(1, 0, "", sort = AlbumSort.Artist, downloaded = true).firstPage() } - timed("albums first page (year)") { dao.albumPages(1, 0, "", sort = AlbumSort.Year, downloaded = true).firstPage() } - timed("albums first page (added)") { dao.albumPages(1, 0, "", sort = AlbumSort.Added, downloaded = true).firstPage() } - timed("albums first page (starred)") { dao.albumPages(1, 0, "", sort = AlbumSort.Starred, downloaded = true).firstPage() } - timed("albums first page (frequent)") { dao.albumPages(1, 0, "", sort = AlbumSort.Frequent, downloaded = true).firstPage() } - timed("albums first page (recent)") { dao.albumPages(1, 0, "", sort = AlbumSort.Recent, downloaded = true).firstPage() } - timed("artists first page (name)") { dao.artistPages(1, 0, "", sort = ArtistSort.Name, downloaded = true).firstPage() } - timed("artists first page (name, default)") { dao.artistPages(1, 0, "", sort = ArtistSort.Name).firstPage() } - timed("artists first page (frequent)") { dao.artistPages(1, 0, "", sort = ArtistSort.Frequent, downloaded = true).firstPage() } + timed("albums first page (name)") { dao.albumPages(1, 0, "", sort = AlbumSort.Name).firstPage() } + timed("albums first page (name, downloaded)") { + dao.albumPages(1, 0, "", sort = AlbumSort.Name, downloaded = true).firstPage() + } + timed("albums first page (artist)") { dao.albumPages(1, 0, "", sort = AlbumSort.Artist).firstPage() } + timed("albums first page (year)") { dao.albumPages(1, 0, "", sort = AlbumSort.Year).firstPage() } + timed("albums first page (added)") { dao.albumPages(1, 0, "", sort = AlbumSort.Added).firstPage() } + timed("albums first page (starred)") { dao.albumPages(1, 0, "", sort = AlbumSort.Starred).firstPage() } + timed("albums first page (frequent)") { dao.albumPages(1, 0, "", sort = AlbumSort.Frequent).firstPage() } + timed("albums first page (recent)") { dao.albumPages(1, 0, "", sort = AlbumSort.Recent).firstPage() } + timed("artists first page (name)") { dao.artistPages(1, 0, "", sort = ArtistSort.Name).firstPage() } + timed("artists first page (name, downloaded)") { + dao.artistPages(1, 0, "", sort = ArtistSort.Name, downloaded = true).firstPage() + } + timed("artists first page (frequent)") { dao.artistPages(1, 0, "", sort = ArtistSort.Frequent).firstPage() } timed("artist albums") { dao.albumsForArtist(1, "ar0").first() } timed("artist name") { dao.artistName(1, "ar0").first() } timed("artist songs") { db.queueDao().artistSongs(1, "ar0", 0, 100) } @@ -133,11 +137,13 @@ class QueryTimingMeasurementTest { timed("artists downloaded filter") { dao.artistPages(1, 0, "", downloaded = true).firstPage() } timed("artist statuses") { db.downloadDao().artistStatuses(1).first() } timed("artist status") { db.downloadDao().artistStatus(1, "ar0").first() } - timed("playlists first page (name)") { dao.playlistPages(1, "", sort = PlaylistSort.Name, downloaded = true).firstPage() } - timed("playlists first page (name, default)") { dao.playlistPages(1, "", sort = PlaylistSort.Name).firstPage() } - timed("albums deep page") { dao.albumPages(1, 0, "", sort = AlbumSort.Name, downloaded = true).deepPage(1000) } - timed("albums search FTS") { dao.albumPages(1, 0, "Album 1", sort = AlbumSort.Name, downloaded = true).firstPage() } - timed("albums search short") { dao.albumPages(1, 0, "al", sort = AlbumSort.Name, downloaded = true).firstPage() } + timed("playlists first page (name)") { dao.playlistPages(1, "", sort = PlaylistSort.Name).firstPage() } + timed("playlists first page (name, downloaded)") { + dao.playlistPages(1, "", sort = PlaylistSort.Name, downloaded = true).firstPage() + } + timed("albums deep page") { dao.albumPages(1, 0, "", sort = AlbumSort.Name).deepPage(1000) } + timed("albums search FTS") { dao.albumPages(1, 0, "Album 1", sort = AlbumSort.Name).firstPage() } + timed("albums search short") { dao.albumPages(1, 0, "al", sort = AlbumSort.Name).firstPage() } timed("songs search FTS") { dao.searchSongs(1, "Song", 50).first() } timed("home recently played") { dao.recentlyPlayedAlbums(1, 20).first() } timed("home most played") { dao.mostPlayedAlbums(1, 20).first() } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt index 5e0615d7..4fe24b0e 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt @@ -208,6 +208,42 @@ class LibraryRepositoryTest { } } + @Test + fun theDownloadedAlbumGridOnlyPresentsDownloadedRows() = + runBlocking { + val sourceId = sourceRepository.addSource("nav", "http://localhost/", "u", "p", false) + db.libraryDao().upsertAlbums(listOf(seedAlbum(sourceId, 1), seedAlbum(sourceId, 2))) + db.libraryDao().upsertSongs( + listOf( + seedSong(sourceId).copy(id = "s1", albumId = "al-0001"), + seedSong(sourceId).copy(id = "s2", albumId = "al-0002"), + ), + ) + db.downloadDao().upsert(SongDownload(sourceId, "s1", DownloadStatus.Completed)) + + val presenterDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher() + val presenter = RecordingPresenter(presenterDispatcher) + val job = + launch { + repository + .albums( + sourceId = sourceId, + sort = AlbumSort.Name, + descending = false, + starred = StarredFilter.Any, + search = "", + downloaded = true, + ).collect { presenter.collectFrom(it) } + } + try { + awaitRefreshes(presenter, 1) + assertEquals(1, presented(presenter, presenterDispatcher)) + } finally { + job.cancelAndJoin() + presenterDispatcher.close() + } + } + @Test fun artistAlbumsSeparatesAlbumCreditsFromTrackAppearances() = runBlocking {