From 5b10a9357776c7b9a857b8d5d9bf88e8ced21303 Mon Sep 17 00:00:00 2001 From: Stypox Date: Mon, 9 Feb 2026 12:28:38 +0100 Subject: [PATCH] Embed actions impls inside LongPressAction when possible ... and add TODO comments for the `onDelete` implementations, which will be embeddable only after migrating the list fragments to Compose. Also remove incomplete and misleading toStreamInfoItem() implementations in StreamStatisticsEntry and PlaylistStreamEntry. Now one has to do statisticsEntry.streamEntry.toStreamInfoItem() instead, making it clear that the call only uses data from streamEntry. --- .../database/playlist/PlaylistStreamEntry.kt | 15 --- .../database/stream/StreamStatisticsEntry.kt | 15 --- .../fragments/list/BaseListFragment.java | 2 +- .../local/bookmark/BookmarkFragment.java | 61 +-------- .../history/StatisticsPlaylistFragment.java | 4 +- .../local/playlist/LocalPlaylistFragment.java | 24 ++-- .../local/playlist/LocalPlaylistManager.java | 10 +- .../subscription/SubscriptionFragment.kt | 13 +- .../MediaBrowserPlaybackPreparer.kt | 4 +- .../playqueue/LocalPlaylistPlayQueue.kt | 2 +- .../ui/components/menu/LongPressAction.kt | 125 +++++++++++++++--- 11 files changed, 136 insertions(+), 139 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistStreamEntry.kt b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistStreamEntry.kt index 90fdee2d3..9c868b3bc 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistStreamEntry.kt +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistStreamEntry.kt @@ -31,19 +31,4 @@ data class PlaylistStreamEntry( override val localItemType: LocalItem.LocalItemType get() = LocalItem.LocalItemType.PLAYLIST_STREAM_ITEM - - @Throws(IllegalArgumentException::class) - fun toStreamInfoItem(): StreamInfoItem { - return StreamInfoItem( - streamEntity.serviceId, - streamEntity.url, - streamEntity.title, - streamEntity.streamType - ).apply { - duration = streamEntity.duration - uploaderName = streamEntity.uploader - uploaderUrl = streamEntity.uploaderUrl - thumbnails = ImageStrategy.dbUrlToImageList(streamEntity.thumbnailUrl) - } - } } diff --git a/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt b/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt index ce74678ca..227d8816b 100644 --- a/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt +++ b/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt @@ -37,21 +37,6 @@ data class StreamStatisticsEntry( override val localItemType: LocalItem.LocalItemType get() = LocalItem.LocalItemType.STATISTIC_STREAM_ITEM - @Ignore - fun toStreamInfoItem(): StreamInfoItem { - return StreamInfoItem( - streamEntity.serviceId, - streamEntity.url, - streamEntity.title, - streamEntity.streamType - ).apply { - duration = streamEntity.duration - uploaderName = streamEntity.uploader - uploaderUrl = streamEntity.uploaderUrl - thumbnails = ImageStrategy.dbUrlToImageList(streamEntity.thumbnailUrl) - } - } - companion object { const val STREAM_LATEST_DATE = "latestAccess" const val STREAM_WATCH_COUNT = "watchCount" diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java index c9709d59f..371e10934 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java @@ -297,7 +297,7 @@ public abstract class BaseListFragment extends BaseStateFragment openLongPressMenuInActivity( requireActivity(), LongPressable.fromChannelInfoItem(selectedItem), - LongPressAction.fromChannelInfoItem(selectedItem, null) + LongPressAction.fromChannelInfoItem(selectedItem, false) ); } }); diff --git a/app/src/main/java/org/schabi/newpipe/local/bookmark/BookmarkFragment.java b/app/src/main/java/org/schabi/newpipe/local/bookmark/BookmarkFragment.java index 7f8a163a9..b7a3be6d4 100644 --- a/app/src/main/java/org/schabi/newpipe/local/bookmark/BookmarkFragment.java +++ b/app/src/main/java/org/schabi/newpipe/local/bookmark/BookmarkFragment.java @@ -6,8 +6,6 @@ import static org.schabi.newpipe.ui.components.menu.LongPressMenuKt.openLongPres import android.os.Bundle; import android.os.Parcelable; -import android.text.InputType; -import android.util.Log; import android.util.Pair; import android.view.LayoutInflater; import android.view.View; @@ -32,7 +30,6 @@ import org.schabi.newpipe.database.LocalItem; import org.schabi.newpipe.database.playlist.PlaylistLocalItem; import org.schabi.newpipe.database.playlist.PlaylistMetadataEntry; import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity; -import org.schabi.newpipe.databinding.DialogEditTextBinding; import org.schabi.newpipe.error.ErrorInfo; import org.schabi.newpipe.error.UserAction; import org.schabi.newpipe.local.BaseLocalListFragment; @@ -55,7 +52,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; import io.reactivex.rxjava3.disposables.CompositeDisposable; -import io.reactivex.rxjava3.disposables.Disposable; public final class BookmarkFragment extends BaseLocalListFragment, Void> implements DebounceSavable { @@ -323,25 +319,6 @@ public final class BookmarkFragment extends BaseLocalListFragment { /*Do nothing on success*/ }, throwable -> showError( - new ErrorInfo(throwable, - UserAction.REQUESTED_BOOKMARK, - "Changing playlist name"))); - disposables.add(disposable); - } - private void deleteItem(final PlaylistLocalItem item) { if (itemListAdapter == null) { return; @@ -501,44 +478,27 @@ public final class BookmarkFragment extends BaseLocalListFragment showDeleteDialog(item.getOrderingName(), item) ) ); } private void showLocalDialog(final PlaylistMetadataEntry selectedItem) { - final boolean isThumbnailPermanent = localPlaylistManager - .getIsPlaylistThumbnailPermanent(selectedItem.getUid()); - openLongPressMenuInActivity( requireActivity(), LongPressable.fromPlaylistMetadataEntry(selectedItem), LongPressAction.fromPlaylistMetadataEntry( selectedItem, - () -> showRenameDialog(selectedItem), - () -> showDeleteDialog(selectedItem.getOrderingName(), selectedItem), - isThumbnailPermanent ? () -> unsetPermanentThumbnail(selectedItem) : null + localPlaylistManager.getIsPlaylistThumbnailPermanent(selectedItem.getUid()), + // TODO passing this parameter is bad and should be fixed when migrating the + // bookmark fragment to Compose, for more info see method javadoc + () -> showDeleteDialog(selectedItem.getOrderingName(), selectedItem) ) ); } - private void showRenameDialog(final PlaylistMetadataEntry selectedItem) { - final DialogEditTextBinding dialogBinding = - DialogEditTextBinding.inflate(getLayoutInflater()); - dialogBinding.dialogEditText.setHint(R.string.name); - dialogBinding.dialogEditText.setInputType(InputType.TYPE_CLASS_TEXT); - dialogBinding.dialogEditText.setText(selectedItem.getOrderingName()); - - new AlertDialog.Builder(activity) - .setView(dialogBinding.getRoot()) - .setPositiveButton(R.string.rename_playlist, (dialog, which) -> - changeLocalPlaylistName( - selectedItem.getUid(), - dialogBinding.dialogEditText.getText().toString())) - .setNegativeButton(R.string.cancel, null) - .show(); - } - private void showDeleteDialog(final String name, final PlaylistLocalItem item) { if (activity == null || disposables == null) { return; @@ -552,13 +512,4 @@ public final class BookmarkFragment extends BaseLocalListFragment infoItems = itemListAdapter.getItemsList(); final List streamInfoItems = new ArrayList<>(infoItems.size()); for (final LocalItem item : infoItems) { - if (item instanceof StreamStatisticsEntry) { - streamInfoItems.add(((StreamStatisticsEntry) item).toStreamInfoItem()); + if (item instanceof StreamStatisticsEntry streamStatisticsEntry) { + streamInfoItems.add(streamStatisticsEntry.getStreamEntity().toStreamInfoItem()); } } return new SinglePlayQueue(streamInfoItems, index); diff --git a/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java b/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java index 19703d6e9..5a00a7f6a 100644 --- a/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java @@ -451,7 +451,9 @@ public class LocalPlaylistFragment extends BaseLocalListFragment successToast.show(), throwable -> showError(new ErrorInfo(throwable, UserAction.REQUESTED_BOOKMARK, @@ -620,7 +622,7 @@ public class LocalPlaylistFragment extends BaseLocalListFragment getPlayQueueStartingAt(item), - () -> deleteItem(item), - () -> changeThumbnailStreamId(item.getStreamEntity().getUid(), true) + playlistId, + // TODO passing this parameter is bad and should be fixed when migrating the + // local playlist fragment to Compose, for more info see method javadoc + () -> deleteItem(item) ) ); } @@ -838,8 +842,8 @@ public class LocalPlaylistFragment extends BaseLocalListFragment infoItems = itemListAdapter.getItemsList(); final List streamInfoItems = new ArrayList<>(infoItems.size()); for (final LocalItem item : infoItems) { - if (item instanceof PlaylistStreamEntry) { - streamInfoItems.add(((PlaylistStreamEntry) item).toStreamInfoItem()); + if (item instanceof PlaylistStreamEntry playlistStreamEntry) { + streamInfoItems.add(playlistStreamEntry.getStreamEntity().toStreamInfoItem()); } } return new SinglePlayQueue(streamInfoItems, index); diff --git a/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistManager.java b/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistManager.java index 1480735fb..608dc9c1a 100644 --- a/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistManager.java +++ b/app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistManager.java @@ -151,13 +151,9 @@ public class LocalPlaylistManager { .isThumbnailPermanent(); } - public long getAutomaticPlaylistThumbnailStreamId(final long playlistId) { - final long streamId = playlistStreamTable.getAutomaticThumbnailStreamId(playlistId) - .blockingFirst(); - if (streamId < 0) { - return PlaylistEntity.DEFAULT_THUMBNAIL_ID; - } - return streamId; + public Flowable getAutomaticPlaylistThumbnailStreamId(final long playlistId) { + return playlistStreamTable.getAutomaticThumbnailStreamId(playlistId) + .map(streamId -> (streamId >= 0 ? streamId : PlaylistEntity.DEFAULT_THUMBNAIL_ID)); } private Maybe modifyPlaylist(final long playlistId, diff --git a/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionFragment.kt b/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionFragment.kt index 2a6d7bfb6..90e3ecc0d 100644 --- a/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionFragment.kt +++ b/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionFragment.kt @@ -336,18 +336,7 @@ class SubscriptionFragment : BaseStateFragment() { openLongPressMenuInActivity( requireActivity(), LongPressable.fromChannelInfoItem(selectedItem), - LongPressAction.fromChannelInfoItem( - item = selectedItem, - onUnsubscribe = { deleteChannel(selectedItem) } - ) - ) - } - - private fun deleteChannel(selectedItem: ChannelInfoItem) { - disposables.add( - subscriptionManager.deleteSubscription(selectedItem.serviceId, selectedItem.url).subscribe { - Toast.makeText(requireContext(), getString(R.string.channel_unsubscribed), Toast.LENGTH_SHORT).show() - } + LongPressAction.fromChannelInfoItem(selectedItem, true) ) } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt b/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt index 5a852cd5b..731407548 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt +++ b/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt @@ -127,7 +127,9 @@ class MediaBrowserPlaybackPreparer( //region Building play queues from playlists and history private fun extractLocalPlayQueue(playlistId: Long, index: Int): Single { return LocalPlaylistManager(database).getPlaylistStreams(playlistId).firstOrError() - .map { items -> SinglePlayQueue(items.map { it.toStreamInfoItem() }, index) } + .map { items -> + SinglePlayQueue(items.map { it.streamEntity.toStreamInfoItem() }, index) + } } private fun extractRemotePlayQueue(playlistId: Long, index: Int): Single { diff --git a/app/src/main/java/org/schabi/newpipe/player/playqueue/LocalPlaylistPlayQueue.kt b/app/src/main/java/org/schabi/newpipe/player/playqueue/LocalPlaylistPlayQueue.kt index ab0be643f..7aa8c9f7d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playqueue/LocalPlaylistPlayQueue.kt +++ b/app/src/main/java/org/schabi/newpipe/player/playqueue/LocalPlaylistPlayQueue.kt @@ -28,7 +28,7 @@ class LocalPlaylistPlayQueue(info: PlaylistMetadataEntry) : PlayQueue(0, listOf( .observeOn(AndroidSchedulers.mainThread()) .subscribe( { streamEntries -> - append(streamEntries.map { PlayQueueItem(it.toStreamInfoItem()) }) + append(streamEntries.map { PlayQueueItem(it.streamEntity.toStreamInfoItem()) }) }, { e -> Log.e(TAG, "Error fetching local playlist", e) diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressAction.kt b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressAction.kt index c0dd3b7ce..e720c0492 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressAction.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressAction.kt @@ -1,9 +1,11 @@ package org.schabi.newpipe.ui.components.menu import android.content.Context +import android.text.InputType import android.widget.Toast import androidx.annotation.MainThread import androidx.annotation.StringRes +import androidx.appcompat.app.AlertDialog import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.PlaylistAdd import androidx.compose.material.icons.filled.AddToQueue @@ -24,7 +26,10 @@ import androidx.compose.material.icons.filled.QueuePlayNext import androidx.compose.material.icons.filled.Share import androidx.compose.ui.graphics.vector.ImageVector import androidx.core.net.toUri +import kotlin.coroutines.resume +import kotlin.coroutines.suspendCoroutine import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.reactive.awaitFirst import kotlinx.coroutines.rx3.await import kotlinx.coroutines.rx3.awaitSingle import kotlinx.coroutines.withContext @@ -35,6 +40,7 @@ import org.schabi.newpipe.database.playlist.PlaylistStreamEntry import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity import org.schabi.newpipe.database.stream.StreamStatisticsEntry import org.schabi.newpipe.database.stream.model.StreamEntity +import org.schabi.newpipe.databinding.DialogEditTextBinding import org.schabi.newpipe.download.DownloadDialog import org.schabi.newpipe.extractor.InfoItem import org.schabi.newpipe.extractor.channel.ChannelInfoItem @@ -45,6 +51,7 @@ import org.schabi.newpipe.local.dialog.PlaylistAppendDialog import org.schabi.newpipe.local.dialog.PlaylistDialog import org.schabi.newpipe.local.history.HistoryRecordManager import org.schabi.newpipe.local.playlist.LocalPlaylistManager +import org.schabi.newpipe.local.subscription.SubscriptionManager import org.schabi.newpipe.player.helper.PlayerHolder import org.schabi.newpipe.player.playqueue.ChannelTabPlayQueue import org.schabi.newpipe.player.playqueue.LocalPlaylistPlayQueue @@ -147,7 +154,7 @@ data class LongPressAction( * that was long-pressed, and take care of searching through the list to find the item * index, and finally take care of building the queue. It would deduplicate some code in * fragments, but it's probably not possible to do because of all the different types of - * the items involved. + * the items involved. But this should be reconsidered if the types will be unified. */ private fun buildPlayerFromHereActionList(queueFromHere: () -> PlayQueue): List { return listOf( @@ -330,7 +337,7 @@ data class LongPressAction( item: StreamStatisticsEntry, queueFromHere: (() -> PlayQueue)? ): List { - return fromStreamEntity(item.streamEntity, queueFromHere) + + return fromStreamInfoItem(item.streamEntity.toStreamInfoItem(), queueFromHere) + listOf( Type.Delete.buildAction { context -> withContext(Dispatchers.IO) { @@ -344,39 +351,100 @@ data class LongPressAction( ) } + /** + * TODO [onDelete] is still passed externally to allow the calling fragment to debounce + * many deletions into a single database transaction, improving performance. This is + * however a bad pattern (which has already led to many bugs in NewPipe). Once we migrate + * the playlist fragment to Compose, we should make the database updates immediately, and + * use `collectAsLazyPagingItems()` to load data in chunks and thus avoid slowdowns. + */ @JvmStatic fun fromPlaylistStreamEntry( item: PlaylistStreamEntry, queueFromHere: (() -> PlayQueue)?, - // TODO possibly embed these two actions here - onDelete: Runnable, - onSetAsPlaylistThumbnail: Runnable + playlistId: Long, + onDelete: Runnable ): List { - return fromStreamEntity(item.streamEntity, queueFromHere) + + return fromStreamInfoItem(item.streamEntity.toStreamInfoItem(), queueFromHere) + listOf( - Type.Delete.buildAction { onDelete.run() }, - Type.SetAsPlaylistThumbnail.buildAction { onSetAsPlaylistThumbnail.run() } + Type.SetAsPlaylistThumbnail.buildAction { context -> + withContext(Dispatchers.IO) { + LocalPlaylistManager(NewPipeDatabase.getInstance(context)) + .changePlaylistThumbnail(playlistId, item.streamEntity.uid, true) + .awaitSingle() + } + Toast.makeText( + context, + R.string.playlist_thumbnail_change_success, + Toast.LENGTH_SHORT + ).show() + }, + Type.Delete.buildAction { onDelete.run() } ) } + /** + * TODO see [fromPlaylistStreamEntry] for why [onDelete] is here and why it's bad + */ @JvmStatic fun fromPlaylistMetadataEntry( item: PlaylistMetadataEntry, - onRename: Runnable, - onDelete: Runnable, - unsetPlaylistThumbnail: Runnable? + isThumbnailPermanent: Boolean, + onDelete: Runnable ): List { return buildPlayerActionList { LocalPlaylistPlayQueue(item) } + buildPlayerShuffledActionList { LocalPlaylistPlayQueue(item) } + listOf( - Type.Rename.buildAction { onRename.run() }, - Type.Delete.buildAction { onDelete.run() }, + Type.Rename.buildAction { context -> + // open the dialog and wait for its completion in the coroutine + val newName = suspendCoroutine { continuation -> + val dialogBinding = DialogEditTextBinding.inflate( + context.findFragmentActivity().layoutInflater + ) + dialogBinding.dialogEditText.setHint(R.string.name) + dialogBinding.dialogEditText.setInputType(InputType.TYPE_CLASS_TEXT) + dialogBinding.dialogEditText.setText(item.orderingName) + AlertDialog.Builder(context) + .setView(dialogBinding.getRoot()) + .setPositiveButton(R.string.rename_playlist) { _, _ -> + continuation.resume(dialogBinding.dialogEditText.getText().toString()) + } + .setNegativeButton(R.string.cancel) { _, _ -> + continuation.resume(null) + } + .setOnCancelListener { + continuation.resume(null) + } + .show() + } ?: return@buildAction + + withContext(Dispatchers.IO) { + LocalPlaylistManager(NewPipeDatabase.getInstance(context)) + .renamePlaylist(item.uid, newName) + .awaitSingle() + } + }, Type.UnsetPlaylistThumbnail.buildAction( - enabled = { unsetPlaylistThumbnail != null } - ) { unsetPlaylistThumbnail?.run() } + enabled = { isThumbnailPermanent } + ) { context -> + withContext(Dispatchers.IO) { + val localPlaylistManager = + LocalPlaylistManager(NewPipeDatabase.getInstance(context)) + val thumbnailStreamId = localPlaylistManager + .getAutomaticPlaylistThumbnailStreamId(item.uid) + .awaitFirst() + localPlaylistManager + .changePlaylistThumbnail(item.uid, thumbnailStreamId, false) + .awaitSingle() + } + }, + Type.Delete.buildAction { onDelete.run() } ) } + /** + * TODO see [fromPlaylistStreamEntry] for why [onDelete] is here and why it's bad + */ @JvmStatic fun fromPlaylistRemoteEntity( item: PlaylistRemoteEntity, @@ -397,12 +465,12 @@ data class LongPressAction( @JvmStatic fun fromChannelInfoItem( item: ChannelInfoItem, - onUnsubscribe: Runnable? + showUnsubscribe: Boolean ): List { return buildPlayerActionList { ChannelTabPlayQueue(item.serviceId, item.url) } + buildPlayerShuffledActionList { ChannelTabPlayQueue(item.serviceId, item.url) } + buildShareActionList(item) + - listOfNotNull( + listOf( Type.ShowChannelDetails.buildAction { context -> NavigationHelper.openChannelFragmentUsingIntent( context, @@ -410,9 +478,26 @@ data class LongPressAction( item.url, item.name ) - }, - onUnsubscribe?.let { r -> Type.Unsubscribe.buildAction { r.run() } } - ) + } + ) + + if (showUnsubscribe) { + listOf( + Type.Unsubscribe.buildAction { context -> + withContext(Dispatchers.IO) { + SubscriptionManager(context) + .deleteSubscription(item.serviceId, item.url) + .await() + } + Toast.makeText( + context, + context.getString(R.string.channel_unsubscribed), + Toast.LENGTH_SHORT + ).show() + } + ) + } else { + listOf() + } } @JvmStatic