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.
This commit is contained in:
Stypox 2026-02-09 12:28:38 +01:00
parent d561444836
commit 5b10a93577
No known key found for this signature in database
GPG Key ID: 4BDF1B40A49FDD23
11 changed files with 136 additions and 139 deletions

View File

@ -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)
}
}
}

View File

@ -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"

View File

@ -297,7 +297,7 @@ public abstract class BaseListFragment<I, N> extends BaseStateFragment<I>
openLongPressMenuInActivity(
requireActivity(),
LongPressable.fromChannelInfoItem(selectedItem),
LongPressAction.fromChannelInfoItem(selectedItem, null)
LongPressAction.fromChannelInfoItem(selectedItem, false)
);
}
});

View File

@ -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<List<PlaylistLocalItem>, Void>
implements DebounceSavable {
@ -323,25 +319,6 @@ public final class BookmarkFragment extends BaseLocalListFragment<List<PlaylistL
// Playlist Metadata Manipulation
//////////////////////////////////////////////////////////////////////////*/
private void changeLocalPlaylistName(final long id, final String name) {
if (localPlaylistManager == null) {
return;
}
if (DEBUG) {
Log.d(TAG, "Updating playlist id=[" + id + "] "
+ "with new name=[" + name + "] items");
}
final Disposable disposable = localPlaylistManager.renamePlaylist(id, name)
.observeOn(AndroidSchedulers.mainThread())
.subscribe(longs -> { /*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<List<PlaylistL
LongPressable.fromPlaylistRemoteEntity(item),
LongPressAction.fromPlaylistRemoteEntity(
item,
// TODO passing this parameter is bad and should be fixed when migrating the
// bookmark fragment to Compose, for more info see method javadoc
() -> 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<List<PlaylistL
.setNegativeButton(R.string.cancel, null)
.show();
}
private void unsetPermanentThumbnail(final PlaylistMetadataEntry item) {
final long thumbnailStreamId = localPlaylistManager
.getAutomaticPlaylistThumbnailStreamId(item.getUid());
localPlaylistManager
.changePlaylistThumbnail(item.getUid(), thumbnailStreamId, false)
.observeOn(AndroidSchedulers.mainThread())
.subscribe();
}
}

View File

@ -336,8 +336,8 @@ public class StatisticsPlaylistFragment
final List<LocalItem> infoItems = itemListAdapter.getItemsList();
final List<StreamInfoItem> 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);

View File

@ -451,7 +451,9 @@ public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistSt
final var streamStateEntity = streamStates.get(i);
final int indexInHistory = Collections.binarySearch(historyStreamIds,
playlistItem.getStreamId());
final long duration = playlistItem.toStreamInfoItem().getDuration();
final long duration = playlistItem.getStreamEntity()
.toStreamInfoItem()
.getDuration();
if (indexInHistory < 0 // stream is not in history
// stream is in history but the streamStateEntity is null
@ -582,9 +584,9 @@ public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistSt
disposables.add(disposable);
}
private void changeThumbnailStreamId(final long thumbnailStreamId, final boolean isPermanent) {
if (playlistManager == null || (!isPermanent && playlistManager
.getIsPlaylistThumbnailPermanent(playlistId))) {
private void changeThumbnailStreamIdIfNotPermanent(final long thumbnailStreamId) {
if (playlistManager == null
|| playlistManager.getIsPlaylistThumbnailPermanent(playlistId)) {
return;
}
@ -598,7 +600,7 @@ public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistSt
}
final Disposable disposable = playlistManager
.changePlaylistThumbnail(playlistId, thumbnailStreamId, isPermanent)
.changePlaylistThumbnail(playlistId, thumbnailStreamId, false)
.observeOn(AndroidSchedulers.mainThread())
.subscribe(ignore -> successToast.show(), throwable ->
showError(new ErrorInfo(throwable, UserAction.REQUESTED_BOOKMARK,
@ -620,7 +622,7 @@ public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistSt
thumbnailStreamId = PlaylistEntity.DEFAULT_THUMBNAIL_ID;
}
changeThumbnailStreamId(thumbnailStreamId, false);
changeThumbnailStreamIdIfNotPermanent(thumbnailStreamId);
}
private void openRemoveDuplicatesDialog() {
@ -796,8 +798,10 @@ public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistSt
LongPressAction.fromPlaylistStreamEntry(
item,
() -> 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<List<PlaylistSt
final List<LocalItem> infoItems = itemListAdapter.getItemsList();
final List<StreamInfoItem> 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);

View File

@ -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<Long> getAutomaticPlaylistThumbnailStreamId(final long playlistId) {
return playlistStreamTable.getAutomaticThumbnailStreamId(playlistId)
.map(streamId -> (streamId >= 0 ? streamId : PlaylistEntity.DEFAULT_THUMBNAIL_ID));
}
private Maybe<Integer> modifyPlaylist(final long playlistId,

View File

@ -336,18 +336,7 @@ class SubscriptionFragment : BaseStateFragment<SubscriptionState>() {
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)
)
}

View File

@ -127,7 +127,9 @@ class MediaBrowserPlaybackPreparer(
//region Building play queues from playlists and history
private fun extractLocalPlayQueue(playlistId: Long, index: Int): Single<PlayQueue> {
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<PlayQueue> {

View File

@ -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)

View File

@ -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<LongPressAction> {
return listOf(
@ -330,7 +337,7 @@ data class LongPressAction(
item: StreamStatisticsEntry,
queueFromHere: (() -> PlayQueue)?
): List<LongPressAction> {
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<LongPressAction> {
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<LongPressAction> {
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<String?> { 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<LongPressAction> {
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