music: refactor indexer responses

Make indexer responses use Result instead of an arbitrary response
type.

This is part of a more general rework to move the "No music found"
screen from the home view to the individual home lists.
This commit is contained in:
Alexander Capehart 2023-01-01 13:54:56 -07:00
parent 80e85bfffa
commit b103eb4749
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
4 changed files with 59 additions and 110 deletions

View file

@ -49,14 +49,7 @@ import org.oxycblt.auxio.home.list.GenreListFragment
import org.oxycblt.auxio.home.list.SongListFragment import org.oxycblt.auxio.home.list.SongListFragment
import org.oxycblt.auxio.home.tabs.AdaptiveTabStrategy import org.oxycblt.auxio.home.tabs.AdaptiveTabStrategy
import org.oxycblt.auxio.list.selection.SelectionFragment import org.oxycblt.auxio.list.selection.SelectionFragment
import org.oxycblt.auxio.music.Album import org.oxycblt.auxio.music.*
import org.oxycblt.auxio.music.Artist
import org.oxycblt.auxio.music.Genre
import org.oxycblt.auxio.music.Music
import org.oxycblt.auxio.music.MusicMode
import org.oxycblt.auxio.music.MusicViewModel
import org.oxycblt.auxio.music.Song
import org.oxycblt.auxio.music.Sort
import org.oxycblt.auxio.music.system.Indexer import org.oxycblt.auxio.music.system.Indexer
import org.oxycblt.auxio.ui.MainNavigationAction import org.oxycblt.auxio.ui.MainNavigationAction
import org.oxycblt.auxio.ui.NavigationViewModel import org.oxycblt.auxio.ui.NavigationViewModel
@ -322,7 +315,7 @@ class HomeFragment :
private fun updateIndexerState(state: Indexer.State?) { private fun updateIndexerState(state: Indexer.State?) {
val binding = requireBinding() val binding = requireBinding()
when (state) { when (state) {
is Indexer.State.Complete -> setupCompleteState(binding, state.response) is Indexer.State.Complete -> setupCompleteState(binding, state.result)
is Indexer.State.Indexing -> setupIndexingState(binding, state.indexing) is Indexer.State.Indexing -> setupIndexingState(binding, state.indexing)
null -> { null -> {
logD("Indexer is in indeterminate state") logD("Indexer is in indeterminate state")
@ -331,54 +324,43 @@ class HomeFragment :
} }
} }
private fun setupCompleteState(binding: FragmentHomeBinding, response: Indexer.Response) { private fun setupCompleteState(
if (response is Indexer.Response.Ok) { binding: FragmentHomeBinding,
result: Result<MusicStore.Library>
) {
if (result.isSuccess) {
logD("Received ok response") logD("Received ok response")
binding.homeFab.show() binding.homeFab.show()
binding.homeIndexingContainer.visibility = View.INVISIBLE binding.homeIndexingContainer.visibility = View.INVISIBLE
} else { } else {
logD("Received non-ok response") logD("Received non-ok response")
val context = requireContext() val context = requireContext()
val throwable = unlikelyToBeNull(result.exceptionOrNull())
binding.homeIndexingContainer.visibility = View.VISIBLE binding.homeIndexingContainer.visibility = View.VISIBLE
binding.homeIndexingProgress.visibility = View.INVISIBLE binding.homeIndexingProgress.visibility = View.INVISIBLE
when (response) { if (throwable is Indexer.NoPermissionException) {
is Indexer.Response.Err -> { logD("Updating UI to permission request state")
logD("Updating UI to Response.Err state") binding.homeIndexingStatus.text = context.getString(R.string.err_no_perms)
binding.homeIndexingStatus.text = context.getString(R.string.err_index_failed) // Configure the action to act as a permission launcher.
// Configure the action to act as a reload trigger. binding.homeIndexingAction.apply {
binding.homeIndexingAction.apply { visibility = View.VISIBLE
visibility = View.VISIBLE text = context.getString(R.string.lbl_grant)
text = context.getString(R.string.lbl_retry) setOnClickListener {
setOnClickListener { musicModel.refresh() } requireNotNull(storagePermissionLauncher) {
"Permission launcher was not available"
}
.launch(Indexer.PERMISSION_READ_AUDIO)
} }
} }
is Indexer.Response.NoMusic -> { } else {
// TODO: Move this state to the list fragments (quality of life) logD("Updating UI to error state")
logD("Updating UI to Response.NoMusic state") binding.homeIndexingStatus.text = context.getString(R.string.err_index_failed)
binding.homeIndexingStatus.text = context.getString(R.string.err_no_music) // Configure the action to act as a reload trigger.
// Configure the action to act as a reload trigger. binding.homeIndexingAction.apply {
binding.homeIndexingAction.apply { visibility = View.VISIBLE
visibility = View.VISIBLE text = context.getString(R.string.lbl_retry)
text = context.getString(R.string.lbl_retry) setOnClickListener { musicModel.refresh() }
setOnClickListener { musicModel.refresh() }
}
} }
is Indexer.Response.NoPerms -> {
logD("Updating UI to Response.NoPerms state")
binding.homeIndexingStatus.text = context.getString(R.string.err_no_perms)
// Configure the action to act as a permission launcher.
binding.homeIndexingAction.apply {
visibility = View.VISIBLE
text = context.getString(R.string.lbl_grant)
setOnClickListener {
requireNotNull(storagePermissionLauncher) {
"Permission launcher was not available"
}
.launch(Indexer.PERMISSION_READ_AUDIO)
}
}
}
else -> {}
} }
} }
} }

View file

@ -48,9 +48,9 @@ class MusicViewModel : ViewModel(), Indexer.Listener {
override fun onIndexerStateChanged(state: Indexer.State?) { override fun onIndexerStateChanged(state: Indexer.State?) {
_indexerState.value = state _indexerState.value = state
if (state is Indexer.State.Complete && state.response is Indexer.Response.Ok) { if (state is Indexer.State.Complete) {
// New state is a completed library, update the statistics values. // New state is a completed library, update the statistics values.
val library = state.response.library val library = state.result.getOrNull() ?: return
_statistics.value = _statistics.value =
Statistics( Statistics(
library.songs.size, library.songs.size,

View file

@ -51,7 +51,7 @@ import org.oxycblt.auxio.util.logW
* @author Alexander Capehart (OxygenCobalt) * @author Alexander Capehart (OxygenCobalt)
*/ */
class Indexer private constructor() { class Indexer private constructor() {
private var lastResponse: Response? = null private var lastResponse: Result<MusicStore.Library>? = null
private var indexingState: Indexing? = null private var indexingState: Indexing? = null
private var controller: Controller? = null private var controller: Controller? = null
private var listener: Listener? = null private var listener: Listener? = null
@ -148,28 +148,14 @@ class Indexer private constructor() {
* be written, but no cache entries will be loaded into the new library. * be written, but no cache entries will be loaded into the new library.
*/ */
suspend fun index(context: Context, withCache: Boolean) { suspend fun index(context: Context, withCache: Boolean) {
if (ContextCompat.checkSelfPermission(context, PERMISSION_READ_AUDIO) == val result =
PackageManager.PERMISSION_DENIED) {
// No permissions, signal that we can't do anything.
emitCompletion(Response.NoPerms)
return
}
val response =
try { try {
val start = System.currentTimeMillis() val start = System.currentTimeMillis()
val library = indexImpl(context, withCache) val library = indexImpl(context, withCache)
if (library != null) { logD(
// Successfully loaded a library. "Music indexing completed successfully in " +
logD( "${System.currentTimeMillis() - start}ms")
"Music indexing completed successfully in " + Result.success(library)
"${System.currentTimeMillis() - start}ms")
Response.Ok(library)
} else {
// Loaded a library, but it contained no music.
logE("No music found")
Response.NoMusic
}
} catch (e: CancellationException) { } catch (e: CancellationException) {
// Got cancelled, propagate upwards to top-level co-routine. // Got cancelled, propagate upwards to top-level co-routine.
logD("Loading routine was cancelled") logD("Loading routine was cancelled")
@ -178,10 +164,9 @@ class Indexer private constructor() {
// Music loading process failed due to something we have not handled. // Music loading process failed due to something we have not handled.
logE("Music indexing failed") logE("Music indexing failed")
logE(e.stackTraceToString()) logE(e.stackTraceToString())
Response.Err(e) Result.failure(e)
} }
emitCompletion(result)
emitCompletion(response)
} }
/** /**
@ -212,9 +197,15 @@ class Indexer private constructor() {
* @param context [Context] required to load music. * @param context [Context] required to load music.
* @param withCache Whether to use the cache or not when loading. If false, the cache will still * @param withCache Whether to use the cache or not when loading. If false, the cache will still
* be written, but no cache entries will be loaded into the new library. * be written, but no cache entries will be loaded into the new library.
* @return A newly-loaded [MusicStore.Library], or null if nothing was loaded. * @return A newly-loaded [MusicStore.Library]. May be empty.
*/ */
private suspend fun indexImpl(context: Context, withCache: Boolean): MusicStore.Library? { private suspend fun indexImpl(context: Context, withCache: Boolean): MusicStore.Library {
if (ContextCompat.checkSelfPermission(context, PERMISSION_READ_AUDIO) ==
PackageManager.PERMISSION_DENIED) {
// No permissions, signal that we can't do anything.
throw NoPermissionException()
}
// Create the chain of extractors. Each extractor builds on the previous and // Create the chain of extractors. Each extractor builds on the previous and
// enables version-specific features in order to create the best possible music // enables version-specific features in order to create the best possible music
// experience. // experience.
@ -237,11 +228,6 @@ class Indexer private constructor() {
val metadataExtractor = MetadataExtractor(context, mediaStoreExtractor) val metadataExtractor = MetadataExtractor(context, mediaStoreExtractor)
val songs = buildSongs(metadataExtractor, Settings(context)) val songs = buildSongs(metadataExtractor, Settings(context))
if (songs.isEmpty()) {
// No songs, nothing else to do.
return null
}
// Build the rest of the music library from the song list. This is much more powerful // Build the rest of the music library from the song list. This is much more powerful
// and reliable compared to using MediaStore to obtain grouping information. // and reliable compared to using MediaStore to obtain grouping information.
val buildStart = System.currentTimeMillis() val buildStart = System.currentTimeMillis()
@ -249,7 +235,6 @@ class Indexer private constructor() {
val artists = buildArtists(songs, albums) val artists = buildArtists(songs, albums)
val genres = buildGenres(songs) val genres = buildGenres(songs)
logD("Successfully built library in ${System.currentTimeMillis() - buildStart}ms") logD("Successfully built library in ${System.currentTimeMillis() - buildStart}ms")
return MusicStore.Library(songs, albums, artists, genres) return MusicStore.Library(songs, albums, artists, genres)
} }
@ -395,10 +380,10 @@ class Indexer private constructor() {
* Emit a new [State.Complete] state. This can be used to signal the completion of the music * Emit a new [State.Complete] state. This can be used to signal the completion of the music
* loading process to external code. Will check if the callee has not been canceled and thus has * loading process to external code. Will check if the callee has not been canceled and thus has
* the ability to emit a new state * the ability to emit a new state
* @param response The new [Response] to emit, representing the outcome of the music loading * @param result The new [Response] to emit, representing the outcome of the music loading
* process. * process.
*/ */
private suspend fun emitCompletion(response: Response) { private suspend fun emitCompletion(result: Result<MusicStore.Library>) {
yield() yield()
// Swap to the Main thread so that downstream callbacks don't crash from being on // Swap to the Main thread so that downstream callbacks don't crash from being on
// a background thread. Does not occur in emitIndexing due to efficiency reasons. // a background thread. Does not occur in emitIndexing due to efficiency reasons.
@ -406,10 +391,10 @@ class Indexer private constructor() {
synchronized(this) { synchronized(this) {
// Do not check for redundancy here, as we actually need to notify a switch // Do not check for redundancy here, as we actually need to notify a switch
// from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete. // from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete.
lastResponse = response lastResponse = result
indexingState = null indexingState = null
// Signal that the music loading process has been completed. // Signal that the music loading process has been completed.
val state = State.Complete(response) val state = State.Complete(result)
controller?.onIndexerStateChanged(state) controller?.onIndexerStateChanged(state)
listener?.onIndexerStateChanged(state) listener?.onIndexerStateChanged(state)
} }
@ -427,10 +412,10 @@ class Indexer private constructor() {
/** /**
* Music loading has completed. * Music loading has completed.
* @param response The outcome of the music loading process. * @param result The outcome of the music loading process.
* @see Response * @see Response
*/ */
data class Complete(val response: Response) : State() data class Complete(val result: Result<MusicStore.Library>) : State()
} }
/** /**
@ -451,25 +436,10 @@ class Indexer private constructor() {
class Songs(val current: Int, val total: Int) : Indexing() class Songs(val current: Int, val total: Int) : Indexing()
} }
/** Represents the possible outcomes of the music loading process. */ /** Thrown when the required permissions to load the music library have not been granted yet. */
sealed class Response { class NoPermissionException : Exception() {
/** override val message: String
* Music load was successful and produced a [MusicStore.Library]. get() = "Not granted permissions to load music library"
* @param library The loaded [MusicStore.Library].
*/
data class Ok(val library: MusicStore.Library) : Response()
/**
* Music loading encountered an unexpected error.
* @param throwable The error thrown.
*/
data class Err(val throwable: Throwable) : Response()
/** Music loading occurred, but resulted in no music. */
object NoMusic : Response()
/** Music loading could not occur due to a lack of storage permissions. */
object NoPerms : Response()
} }
/** /**

View file

@ -129,11 +129,11 @@ class IndexerService :
override fun onIndexerStateChanged(state: Indexer.State?) { override fun onIndexerStateChanged(state: Indexer.State?) {
when (state) { when (state) {
is Indexer.State.Indexing -> updateActiveSession(state.indexing)
is Indexer.State.Complete -> { is Indexer.State.Complete -> {
if (state.response is Indexer.Response.Ok && val newLibrary = state.result.getOrNull()
state.response.library != musicStore.library) { if (newLibrary != null && newLibrary != musicStore.library) {
logD("Applying new library") logD("Applying new library")
val newLibrary = state.response.library
// We only care if the newly-loaded library is going to replace a previously // We only care if the newly-loaded library is going to replace a previously
// loaded library. // loaded library.
if (musicStore.library != null) { if (musicStore.library != null) {
@ -152,9 +152,6 @@ class IndexerService :
// handled right now. // handled right now.
updateIdleSession() updateIdleSession()
} }
is Indexer.State.Indexing -> {
updateActiveSession(state.indexing)
}
null -> { null -> {
// Null is the indeterminate state that occurs on app startup or after // Null is the indeterminate state that occurs on app startup or after
// the cancellation of a load, so in that case we want to stop foreground // the cancellation of a load, so in that case we want to stop foreground