mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 15:29:15 +01:00
search: require three characters and play songs in album context
Drop the library-wide instr fallback: below three characters the cross-type search now returns nothing instead of scanning every table, and the screen asks for more input. Lock in that a song result plays its album context rather than a search queue.
This commit is contained in:
@@ -128,33 +128,23 @@ internal const val PLAYLIST_ORDER_BY_ADDED_REVERSED = "created ASC, name COLLATE
|
|||||||
internal const val PLAYLIST_ORDER_BY_UPDATED = "changed DESC, name COLLATE NOCASE, id"
|
internal const val PLAYLIST_ORDER_BY_UPDATED = "changed DESC, name COLLATE NOCASE, id"
|
||||||
internal const val PLAYLIST_ORDER_BY_UPDATED_REVERSED = "changed ASC, name COLLATE NOCASE DESC, id DESC"
|
internal const val PLAYLIST_ORDER_BY_UPDATED_REVERSED = "changed ASC, name COLLATE NOCASE DESC, id DESC"
|
||||||
|
|
||||||
// Cross-type search: a three-character-or-longer query runs through the trigram index and anything
|
// Cross-type search only runs from three characters, since a shorter term cannot use the trigram
|
||||||
// shorter falls back to the instr scan, mirroring the per-tab filters above.
|
// index and scanning the whole library for it is not worth it.
|
||||||
internal const val ALBUM_SEARCH_FILTER =
|
internal const val ALBUM_SEARCH_FILTER =
|
||||||
"(:search <> '' AND (" +
|
|
||||||
"(length(:search) >= 3 AND rowid IN (SELECT rowid FROM album_search " +
|
"(length(:search) >= 3 AND rowid IN (SELECT rowid FROM album_search " +
|
||||||
"WHERE album_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"')) " +
|
"WHERE album_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"'))"
|
||||||
"OR (length(:search) < 3 AND (instr(lower(name), lower(:search)) > 0 " +
|
|
||||||
"OR instr(lower(albumArtist), lower(:search)) > 0))))"
|
|
||||||
|
|
||||||
internal const val ARTIST_SEARCH_FILTER =
|
internal const val ARTIST_SEARCH_FILTER =
|
||||||
"(:search <> '' AND (" +
|
|
||||||
"(length(:search) >= 3 AND rowid IN (SELECT rowid FROM artist_search " +
|
"(length(:search) >= 3 AND rowid IN (SELECT rowid FROM artist_search " +
|
||||||
"WHERE artist_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"')) " +
|
"WHERE artist_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"'))"
|
||||||
"OR (length(:search) < 3 AND instr(lower(name), lower(:search)) > 0)))"
|
|
||||||
|
|
||||||
internal const val PLAYLIST_SEARCH_FILTER =
|
internal const val PLAYLIST_SEARCH_FILTER =
|
||||||
"(:search <> '' AND (" +
|
|
||||||
"(length(:search) >= 3 AND rowid IN (SELECT rowid FROM playlist_search " +
|
"(length(:search) >= 3 AND rowid IN (SELECT rowid FROM playlist_search " +
|
||||||
"WHERE playlist_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"')) " +
|
"WHERE playlist_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"'))"
|
||||||
"OR (length(:search) < 3 AND instr(lower(name), lower(:search)) > 0)))"
|
|
||||||
|
|
||||||
internal const val SONG_SEARCH_FILTER =
|
internal const val SONG_SEARCH_FILTER =
|
||||||
"(:search <> '' AND (" +
|
|
||||||
"(length(:search) >= 3 AND songs.rowid IN (SELECT rowid FROM song_search " +
|
"(length(:search) >= 3 AND songs.rowid IN (SELECT rowid FROM song_search " +
|
||||||
"WHERE song_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"')) " +
|
"WHERE song_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"'))"
|
||||||
"OR (length(:search) < 3 AND (instr(lower(songs.title), lower(:search)) > 0 " +
|
|
||||||
"OR instr(lower(songs.artist), lower(:search)) > 0))))"
|
|
||||||
|
|
||||||
@Dao
|
@Dao
|
||||||
@DaoReturnTypeConverters(PagingSourceDaoReturnTypeConverter::class)
|
@DaoReturnTypeConverters(PagingSourceDaoReturnTypeConverter::class)
|
||||||
|
|||||||
@@ -510,4 +510,6 @@ const val REDISCOVER_WINDOW_MS = 60L * 24 * 60 * 60 * 1000
|
|||||||
|
|
||||||
const val SEARCH_RESULT_LIMIT = 50
|
const val SEARCH_RESULT_LIMIT = 50
|
||||||
|
|
||||||
|
const val SEARCH_MIN_LENGTH = 3
|
||||||
|
|
||||||
fun rediscoverCutoff(): Long = System.currentTimeMillis() - REDISCOVER_WINDOW_MS
|
fun rediscoverCutoff(): Long = System.currentTimeMillis() - REDISCOVER_WINDOW_MS
|
||||||
|
|||||||
@@ -37,6 +37,7 @@ import com.subtracks.data.model.Artist
|
|||||||
import com.subtracks.data.model.CoverArtRef
|
import com.subtracks.data.model.CoverArtRef
|
||||||
import com.subtracks.data.model.Playlist
|
import com.subtracks.data.model.Playlist
|
||||||
import com.subtracks.data.model.Song
|
import com.subtracks.data.model.Song
|
||||||
|
import com.subtracks.data.repo.SEARCH_MIN_LENGTH
|
||||||
import com.subtracks.data.source.StarType
|
import com.subtracks.data.source.StarType
|
||||||
import com.subtracks.playback.PlaybackController
|
import com.subtracks.playback.PlaybackController
|
||||||
import com.subtracks.ui.components.ContextMenuHost
|
import com.subtracks.ui.components.ContextMenuHost
|
||||||
@@ -144,9 +145,9 @@ fun SearchScreen(
|
|||||||
},
|
},
|
||||||
) { padding ->
|
) { padding ->
|
||||||
when {
|
when {
|
||||||
query.isBlank() -> {
|
query.length < SEARCH_MIN_LENGTH -> {
|
||||||
EmptyState(
|
EmptyState(
|
||||||
text = "Search for songs, albums, artists and playlists.",
|
text = "Type at least $SEARCH_MIN_LENGTH characters to search.",
|
||||||
modifier = Modifier.padding(padding),
|
modifier = Modifier.padding(padding),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import com.subtracks.data.model.CoverArtRef
|
|||||||
import com.subtracks.data.model.Playlist
|
import com.subtracks.data.model.Playlist
|
||||||
import com.subtracks.data.model.Song
|
import com.subtracks.data.model.Song
|
||||||
import com.subtracks.data.repo.LibraryRepository
|
import com.subtracks.data.repo.LibraryRepository
|
||||||
|
import com.subtracks.data.repo.SEARCH_MIN_LENGTH
|
||||||
import com.subtracks.data.repo.SourceRepository
|
import com.subtracks.data.repo.SourceRepository
|
||||||
import com.subtracks.playback.PlaybackController
|
import com.subtracks.playback.PlaybackController
|
||||||
import com.subtracks.ui.home.playSongInContext
|
import com.subtracks.ui.home.playSongInContext
|
||||||
@@ -45,7 +46,7 @@ class SearchViewModel(
|
|||||||
libraryRepository.activeSourceId
|
libraryRepository.activeSourceId
|
||||||
.flatMapLatest { sourceId ->
|
.flatMapLatest { sourceId ->
|
||||||
_query.flatMapLatest { text ->
|
_query.flatMapLatest { text ->
|
||||||
if (sourceId == null || text.isBlank()) {
|
if (sourceId == null || text.length < SEARCH_MIN_LENGTH) {
|
||||||
flowOf(SearchResults())
|
flowOf(SearchResults())
|
||||||
} else {
|
} else {
|
||||||
combine(
|
combine(
|
||||||
|
|||||||
@@ -777,15 +777,15 @@ class LibraryDaoTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun crossTypeSearchFallsBackToAScanBelowThreeCharacters() =
|
fun crossTypeSearchIgnoresQueriesShorterThanThreeCharacters() =
|
||||||
runTest {
|
runTest {
|
||||||
val sourceId = source()
|
val sourceId = source()
|
||||||
val dao = db.libraryDao()
|
val dao = db.libraryDao()
|
||||||
dao.upsertAlbums(listOf(album(sourceId, "al-1", "Gamma", year = null, starred = null)))
|
dao.upsertAlbums(listOf(album(sourceId, "al-1", "Gamma", year = null, starred = null)))
|
||||||
dao.upsertSongs(listOf(song(sourceId, "s1", "Gamma Ray", starred = null)))
|
dao.upsertSongs(listOf(song(sourceId, "s1", "Gamma Ray", starred = null)))
|
||||||
|
|
||||||
assertEquals(listOf("Gamma"), dao.searchAlbums(sourceId, "ga", 50).first().map { it.name })
|
assertTrue(dao.searchAlbums(sourceId, "ga", 50).first().isEmpty())
|
||||||
assertEquals(listOf("s1"), dao.searchSongs(sourceId, "ga", 50).first().map { it.song.id })
|
assertTrue(dao.searchSongs(sourceId, "ga", 50).first().isEmpty())
|
||||||
}
|
}
|
||||||
|
|
||||||
private suspend fun plan(sql: String): String =
|
private suspend fun plan(sql: String): String =
|
||||||
|
|||||||
@@ -62,14 +62,14 @@ class SearchScreenTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun anEmptyQueryPromptsAndAnEmptyResultSaysSo() {
|
fun aQueryShorterThanThreeCharactersPromptsForMoreInput() {
|
||||||
composeRule.setContent {
|
composeRule.setContent {
|
||||||
SubtracksTheme {
|
SubtracksTheme {
|
||||||
SearchScreen(query = "", onQueryChange = {}, results = SearchResults())
|
SearchScreen(query = "ro", onQueryChange = {}, results = SearchResults())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
composeRule.onNodeWithText("Search for songs, albums, artists and playlists.").assertIsDisplayed()
|
composeRule.onNodeWithText("Type at least 3 characters to search.").assertIsDisplayed()
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import androidx.room3.Room
|
|||||||
import androidx.sqlite.driver.bundled.BundledSQLiteDriver
|
import androidx.sqlite.driver.bundled.BundledSQLiteDriver
|
||||||
import androidx.test.core.app.ApplicationProvider
|
import androidx.test.core.app.ApplicationProvider
|
||||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||||
|
import com.subtracks.awaitUntil
|
||||||
import com.subtracks.cancelAndJoinBlocking
|
import com.subtracks.cancelAndJoinBlocking
|
||||||
import com.subtracks.data.db.SubtracksDatabase
|
import com.subtracks.data.db.SubtracksDatabase
|
||||||
import com.subtracks.data.download.ArtworkStore
|
import com.subtracks.data.download.ArtworkStore
|
||||||
@@ -12,6 +13,7 @@ import com.subtracks.data.download.FakeDownloadEngine
|
|||||||
import com.subtracks.data.model.Album
|
import com.subtracks.data.model.Album
|
||||||
import com.subtracks.data.model.Artist
|
import com.subtracks.data.model.Artist
|
||||||
import com.subtracks.data.model.Playlist
|
import com.subtracks.data.model.Playlist
|
||||||
|
import com.subtracks.data.model.QueueKind
|
||||||
import com.subtracks.data.model.Song
|
import com.subtracks.data.model.Song
|
||||||
import com.subtracks.data.prefs.fakeUserPreferences
|
import com.subtracks.data.prefs.fakeUserPreferences
|
||||||
import com.subtracks.data.repo.DownloadRepository
|
import com.subtracks.data.repo.DownloadRepository
|
||||||
@@ -116,12 +118,41 @@ class SearchViewModelTest {
|
|||||||
assertEquals(listOf("pl-1"), results.playlists.map { it.id })
|
assertEquals(listOf("pl-1"), results.playlists.map { it.id })
|
||||||
assertEquals(listOf("s1"), results.songs.map { it.song.id })
|
assertEquals(listOf("s1"), results.songs.map { it.song.id })
|
||||||
|
|
||||||
|
viewModel.setQuery("oa")
|
||||||
|
val short = runBlocking { withTimeout(5_000) { viewModel.results.first { it.isEmpty } } }
|
||||||
|
|
||||||
|
assertTrue(short.isEmpty)
|
||||||
|
|
||||||
viewModel.setQuery("")
|
viewModel.setQuery("")
|
||||||
val blank = runBlocking { withTimeout(5_000) { viewModel.results.first { it.isEmpty } } }
|
val blank = runBlocking { withTimeout(5_000) { viewModel.results.first { it.isEmpty } } }
|
||||||
|
|
||||||
assertTrue(blank.isEmpty)
|
assertTrue(blank.isEmpty)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun playingASearchResultPlaysItsAlbumInsteadOfASearchQueue() {
|
||||||
|
val sourceId =
|
||||||
|
runBlocking {
|
||||||
|
val id = sourceRepository.addSource("nav", "http://localhost/", "u", "p", false)
|
||||||
|
db.libraryDao().upsertAlbums(listOf(album(id, "al-1", "Road Album")))
|
||||||
|
db.libraryDao().upsertSongs(
|
||||||
|
listOf(
|
||||||
|
song(id, "s1", "Road Song"),
|
||||||
|
song(id, "s2", "Another").copy(track = 2),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
id
|
||||||
|
}
|
||||||
|
|
||||||
|
viewModel.play(song(sourceId, "s1", "Road Song"))
|
||||||
|
awaitUntil("the queue is built") { runBlocking { db.queueDao().entries().isNotEmpty() } }
|
||||||
|
|
||||||
|
val entries = runBlocking { db.queueDao().entries() }
|
||||||
|
assertEquals(1, entries.size)
|
||||||
|
assertEquals(QueueKind.Album, entries.first().kind)
|
||||||
|
assertEquals("al-1", entries.first().refId)
|
||||||
|
}
|
||||||
|
|
||||||
private fun album(
|
private fun album(
|
||||||
sourceId: Long,
|
sourceId: Long,
|
||||||
id: String,
|
id: String,
|
||||||
|
|||||||
Reference in New Issue
Block a user