From c270759decde1cf954707507d4f88621522dcc22 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Sat, 7 Dec 2024 17:19:30 -0700 Subject: [PATCH] musikr: improve music location creation --- .../org/oxycblt/auxio/music/MusicSettings.kt | 34 +++------------ .../music/locations/MusicSourcesDialog.kt | 30 ++++++------- .../org/oxycblt/musikr/fs/MusicLocation.kt | 42 +++++++++++++++---- 3 files changed, 55 insertions(+), 51 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt index dd94c51a8..1c2da46bb 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt @@ -24,8 +24,8 @@ import androidx.core.content.edit import dagger.hilt.android.qualifiers.ApplicationContext import javax.inject.Inject import org.oxycblt.auxio.R -import org.oxycblt.musikr.fs.path.DocumentPathFactory import org.oxycblt.auxio.settings.Settings +import org.oxycblt.musikr.fs.MusicLocation import timber.log.Timber as L /** @@ -35,7 +35,7 @@ import timber.log.Timber as L */ interface MusicSettings : Settings { /** The locations of music to load. */ - var musicLocations: List + var musicLocations: List /** Whether to exclude non-music audio files from the music library. */ val excludeNonMusic: Boolean /** Whether to be actively watching for changes in the music library. */ @@ -57,41 +57,19 @@ class MusicSettingsImpl @Inject constructor( @ApplicationContext context: Context, - private val documentPathFactory: DocumentPathFactory + private val musicLocationFactory: MusicLocation.Factory ) : Settings.Impl(context), MusicSettings { - // override var musicDirs: MusicDirectories - // get() { - // val dirs = - // (sharedPreferences.getStringSet(getString(R.string.set_key_music_dirs), null) - // ?: emptySet()) - // .mapNotNull(documentPathFactory::fromDocumentId) - // return MusicDirectories( - // dirs, - // sharedPreferences.getBoolean(getString(R.string.set_key_music_dirs_include), - // false)) - // } - // set(value) { - // sharedPreferences.edit { - // putStringSet( - // getString(R.string.set_key_music_dirs), - // value.dirs.map(documentPathFactory::toDocumentId).toSet()) - // putBoolean(getString(R.string.set_key_music_dirs_include), - // value.shouldInclude) - // apply() - // } - // } - - override var musicLocations: List + override var musicLocations: List get() { val dirs = sharedPreferences.getStringSet(getString(R.string.set_key_music_locations), null) ?: emptySet() - return dirs.map { Uri.parse(it) } + return dirs.mapNotNull { musicLocationFactory.existing(Uri.parse(it)) } } set(value) { sharedPreferences.edit { putStringSet( - getString(R.string.set_key_music_locations), value.map(Uri::toString).toSet()) + getString(R.string.set_key_music_locations), value.map { it.toString() }.toSet()) apply() } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/locations/MusicSourcesDialog.kt b/app/src/main/java/org/oxycblt/auxio/music/locations/MusicSourcesDialog.kt index abe5669c3..8f0489c8b 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/locations/MusicSourcesDialog.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/locations/MusicSourcesDialog.kt @@ -15,12 +15,11 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ - + package org.oxycblt.auxio.music.locations import android.content.ActivityNotFoundException import android.content.ContentResolver -import android.content.Context import android.content.Intent import android.net.Uri import android.os.Bundle @@ -51,8 +50,10 @@ class MusicSourcesDialog : ViewBindingMaterialDialogFragment(), LocationAdapter.Listener { private val locationAdapter = LocationAdapter(this) private var openDocumentTreeLauncher: ActivityResultLauncher? = null - @Inject lateinit var musicLocationFactory: MusicLocation.Factory - @Inject lateinit var musicSettings: MusicSettings + @Inject + lateinit var musicLocationFactory: MusicLocation.Factory + @Inject + lateinit var musicSettings: MusicSettings override fun onCreateBinding(inflater: LayoutInflater) = DialogMusicLocationsBinding.inflate(inflater) @@ -62,7 +63,7 @@ class MusicSourcesDialog : .setTitle(R.string.set_locations) .setNegativeButton(R.string.lbl_cancel, null) .setPositiveButton(R.string.lbl_save) { _, _ -> - val newDirs = locationAdapter.locations.map { it.uri } + val newDirs = locationAdapter.locations if (musicSettings.musicLocations != newDirs) { L.d("Committing changes") musicSettings.musicLocations = newDirs @@ -77,7 +78,8 @@ class MusicSourcesDialog : openDocumentTreeLauncher = registerForActivityResult( ActivityResultContracts.OpenDocumentTree(), - ::addDocumentTreeUriToDirs) + ::addDocumentTreeUriToDirs + ) binding.locationsAdd.apply { ViewCompat.setTooltipText(this, contentDescription) @@ -102,13 +104,11 @@ class MusicSourcesDialog : itemAnimator = null } - val locationUris = - savedInstanceState?.getStringArrayList(KEY_PENDING_LOCATIONS)?.map { Uri.parse(it) } - ?: musicSettings.musicLocations val locations = - locationUris.mapNotNull { - musicLocationFactory.create(it) + savedInstanceState?.getStringArrayList(KEY_PENDING_LOCATIONS)?.mapNotNull { + musicLocationFactory.existing(Uri.parse(it)) } + ?: musicSettings.musicLocations locationAdapter.addAll(locations) requireBinding().locationsEmpty.isVisible = locations.isEmpty() @@ -117,7 +117,8 @@ class MusicSourcesDialog : override fun onSaveInstanceState(outState: Bundle) { super.onSaveInstanceState(outState) outState.putStringArrayList( - KEY_PENDING_LOCATIONS, ArrayList(locationAdapter.locations.map { it.uri.toString() })) + KEY_PENDING_LOCATIONS, ArrayList(locationAdapter.locations.map { it.uri.toString() }) + ) } override fun onDestroyBinding(binding: DialogMusicLocationsBinding) { @@ -131,7 +132,8 @@ class MusicSourcesDialog : requireBinding().locationsEmpty.isVisible = locationAdapter.locations.isEmpty() } - @Inject lateinit var contentResolver: ContentResolver + @Inject + lateinit var contentResolver: ContentResolver /** * Add a Document Tree [Uri] chosen by the user to the current [MusicLocation]s. @@ -149,7 +151,7 @@ class MusicSourcesDialog : Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION contentResolver.takePersistableUriPermission(uri, takeFlags) - val location = musicLocationFactory.create(uri) + val location = musicLocationFactory.new(uri) if (location != null) { locationAdapter.add(location) diff --git a/app/src/main/java/org/oxycblt/musikr/fs/MusicLocation.kt b/app/src/main/java/org/oxycblt/musikr/fs/MusicLocation.kt index 90541d8f1..bf517179b 100644 --- a/app/src/main/java/org/oxycblt/musikr/fs/MusicLocation.kt +++ b/app/src/main/java/org/oxycblt/musikr/fs/MusicLocation.kt @@ -6,6 +6,7 @@ import android.net.Uri import android.provider.DocumentsContract import dagger.hilt.android.qualifiers.ApplicationContext import org.oxycblt.musikr.fs.path.DocumentPathFactory +import org.oxycblt.musikr.fs.path.VolumeManager import javax.inject.Inject class MusicLocation internal constructor(val uri: Uri, val path: Path) { @@ -13,10 +14,18 @@ class MusicLocation internal constructor(val uri: Uri, val path: Path) { other is MusicLocation && uri == other.uri && path == other.path override fun hashCode() = 31 * uri.hashCode() + path.hashCode() - override fun toString() = "src:$uri=$path" + override fun toString(): String { + val volumeId = when (path.volume) { + is Volume.Internal -> VOLUME_INTERNAL + is Volume.External -> path.volume.id + } + return "$uri=${volumeId}:${path.components.unixString}" + } interface Factory { - fun create(uri: Uri): MusicLocation? + fun new(uri: Uri): MusicLocation? + + fun existing(uri: Uri): MusicLocation? } } @@ -24,13 +33,28 @@ class MusicLocationFactoryImpl @Inject constructor( @ApplicationContext private val context: Context, private val documentPathFactory: DocumentPathFactory ) : MusicLocation.Factory { - override fun create(uri: Uri): MusicLocation? { - check(DocumentsContract.isTreeUri(uri)) { "URI $uri is not a document tree URI" } + override fun new(uri: Uri): MusicLocation? { + if (!DocumentsContract.isTreeUri(uri)) return null val path = documentPathFactory.unpackDocumentTreeUri(uri) ?: return null - context.contentResolverSafe.takePersistableUriPermission( - uri, - Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION - ) + val notPersisted = context.contentResolverSafe.persistedUriPermissions + .none { it.uri == uri && it.isReadPermission && it.isWritePermission } + if (notPersisted) { + context.contentResolverSafe.takePersistableUriPermission( + uri, + Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION + ) + } return MusicLocation(uri, path) } -} \ No newline at end of file + + override fun existing(uri: Uri): MusicLocation? { + if (!DocumentsContract.isTreeUri(uri)) return null + val notPersisted = context.contentResolverSafe.persistedUriPermissions + .none { it.uri == uri && it.isReadPermission && it.isWritePermission } + if (notPersisted) return null + val path = documentPathFactory.unpackDocumentTreeUri(uri) ?: return null + return MusicLocation(uri, path) + } +} + +private const val VOLUME_INTERNAL = "internal" \ No newline at end of file