From 4591c096378eb7254092e0692cf76852ceaed766 Mon Sep 17 00:00:00 2001 From: Stypox Date: Fri, 29 Mar 2024 17:59:28 +0100 Subject: [PATCH] Apply review --- .../schabi/newpipe/database/Migrations.java | 19 ++-- .../playlist/PlaylistDuplicatesEntry.java | 1 + .../database/playlist/PlaylistLocalItem.java | 74 --------------- .../playlist/dao/PlaylistRemoteDAO.java | 2 +- .../playlist/dao/PlaylistStreamDAO.java | 22 +---- .../local/bookmark/BookmarkFragment.java | 24 ++--- .../local/bookmark/MergedPlaylistManager.java | 95 +++++++++++++++++++ .../local/playlist/LocalPlaylistManager.java | 15 +-- .../local/playlist/RemotePlaylistManager.java | 6 +- .../settings/SelectPlaylistFragment.java | 6 +- 10 files changed, 123 insertions(+), 141 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/local/bookmark/MergedPlaylistManager.java diff --git a/app/src/main/java/org/schabi/newpipe/database/Migrations.java b/app/src/main/java/org/schabi/newpipe/database/Migrations.java index fa470c2f2..9d641965d 100644 --- a/app/src/main/java/org/schabi/newpipe/database/Migrations.java +++ b/app/src/main/java/org/schabi/newpipe/database/Migrations.java @@ -258,19 +258,19 @@ public final class Migrations { + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " + "`name` TEXT, `is_thumbnail_permanent` INTEGER NOT NULL, " + "`thumbnail_stream_id` INTEGER NOT NULL, " - + "`display_index` INTEGER NOT NULL DEFAULT 0)"); + + "`display_index` INTEGER NOT NULL)"); database.execSQL("INSERT INTO `playlists_tmp` " - + "(`uid`, `name`, `is_thumbnail_permanent`, `thumbnail_stream_id`) " - + "SELECT `uid`, `name`, `is_thumbnail_permanent`, `thumbnail_stream_id` " + + "(`uid`, `name`, `is_thumbnail_permanent`, `thumbnail_stream_id`, " + + "`display_index`) " + + "SELECT `uid`, `name`, `is_thumbnail_permanent`, `thumbnail_stream_id`, " + + "-1 " + "FROM `playlists`"); - // Replace the old table. + // Replace the old table, note that this also removes the index on the name which + // we don't need anymore. database.execSQL("DROP TABLE `playlists`"); database.execSQL("ALTER TABLE `playlists_tmp` RENAME TO `playlists`"); - // Create index on the new table. - database.execSQL("CREATE INDEX `index_playlists_name` ON `playlists` (`name`)"); - // Update remote_playlists. // Create a temp table to initialize display_index. @@ -285,13 +285,12 @@ public final class Migrations { + "SELECT `uid`, `service_id`, `name`, `url`, `thumbnail_url`, `uploader`, " + "`stream_count` FROM `remote_playlists`"); - // Replace the old table. + // Replace the old table, note that this also removes the index on the name which + // we don't need anymore. database.execSQL("DROP TABLE `remote_playlists`"); database.execSQL("ALTER TABLE `remote_playlists_tmp` RENAME TO `remote_playlists`"); // Create index on the new table. - database.execSQL("CREATE INDEX `index_remote_playlists_name` " - + "ON `remote_playlists` (`name`)"); database.execSQL("CREATE UNIQUE INDEX `index_remote_playlists_service_id_url` " + "ON `remote_playlists` (`service_id`, `url`)"); diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistDuplicatesEntry.java b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistDuplicatesEntry.java index dcd3b2b6c..3be85e6e1 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistDuplicatesEntry.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistDuplicatesEntry.java @@ -13,6 +13,7 @@ public class PlaylistDuplicatesEntry extends PlaylistMetadataEntry { @ColumnInfo(name = PLAYLIST_TIMES_STREAM_IS_CONTAINED) public final long timesStreamIsContained; + @SuppressWarnings("checkstyle:ParameterNumber") public PlaylistDuplicatesEntry(final long uid, final String name, final String thumbnailUrl, diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistLocalItem.java b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistLocalItem.java index efd7120d3..072c49e2c 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistLocalItem.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistLocalItem.java @@ -1,12 +1,6 @@ package org.schabi.newpipe.database.playlist; import org.schabi.newpipe.database.LocalItem; -import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; public interface PlaylistLocalItem extends LocalItem { String getOrderingName(); @@ -16,72 +10,4 @@ public interface PlaylistLocalItem extends LocalItem { long getUid(); void setDisplayIndex(long displayIndex); - - /** - * Merge localPlaylists and remotePlaylists by the display index. - * If two items have the same display index, sort them in {@code CASE_INSENSITIVE_ORDER}. - * - * @param localPlaylists local playlists - * @param remotePlaylists remote playlists - * @return merged playlists - */ - static List merge( - final List localPlaylists, - final List remotePlaylists) { - Collections.sort(localPlaylists, - Comparator.comparingLong(PlaylistMetadataEntry::getDisplayIndex)); - Collections.sort(remotePlaylists, - Comparator.comparingLong(PlaylistRemoteEntity::getDisplayIndex)); - - // This algorithm is similar to the merge operation in merge sort. - - final List result = new ArrayList<>( - localPlaylists.size() + remotePlaylists.size()); - final List itemsWithSameIndex = new ArrayList<>(); - - int i = 0; - int j = 0; - while (i < localPlaylists.size()) { - while (j < remotePlaylists.size()) { - if (remotePlaylists.get(j).getDisplayIndex() - <= localPlaylists.get(i).getDisplayIndex()) { - addItem(result, remotePlaylists.get(j), itemsWithSameIndex); - j++; - } else { - break; - } - } - addItem(result, localPlaylists.get(i), itemsWithSameIndex); - i++; - } - while (j < remotePlaylists.size()) { - addItem(result, remotePlaylists.get(j), itemsWithSameIndex); - j++; - } - addItemsWithSameIndex(result, itemsWithSameIndex); - - return result; - } - - static void addItem(final List result, final PlaylistLocalItem item, - final List itemsWithSameIndex) { - if (!itemsWithSameIndex.isEmpty() - && itemsWithSameIndex.get(0).getDisplayIndex() != item.getDisplayIndex()) { - // The new item has a different display index, add previous items with same - // index to the result. - addItemsWithSameIndex(result, itemsWithSameIndex); - itemsWithSameIndex.clear(); - } - itemsWithSameIndex.add(item); - } - - static void addItemsWithSameIndex(final List result, - final List itemsWithSameIndex) { - if (itemsWithSameIndex.size() > 1) { - Collections.sort(itemsWithSameIndex, - Comparator.comparing(PlaylistLocalItem::getOrderingName, - Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER))); - } - result.addAll(itemsWithSameIndex); - } } diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistRemoteDAO.java b/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistRemoteDAO.java index 8118bc40f..8ab8a2afd 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistRemoteDAO.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistRemoteDAO.java @@ -42,7 +42,7 @@ public interface PlaylistRemoteDAO extends BasicDAO { @Query("SELECT * FROM " + REMOTE_PLAYLIST_TABLE + " ORDER BY " + REMOTE_PLAYLIST_DISPLAY_INDEX) - Flowable> getDisplayIndexOrderedPlaylists(); + Flowable> getPlaylists(); @Query("SELECT " + REMOTE_PLAYLIST_ID + " FROM " + REMOTE_PLAYLIST_TABLE + " WHERE " + REMOTE_PLAYLIST_URL + " = :url " diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java b/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java index d795e6ea7..4e1c163a4 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java @@ -92,26 +92,6 @@ public interface PlaylistStreamDAO extends BasicDAO { + " ORDER BY " + JOIN_INDEX + " ASC") Flowable> getOrderedStreamsOf(long playlistId); - @Transaction - @Query("SELECT " + PLAYLIST_ID + ", " + PLAYLIST_NAME + ", " - + PLAYLIST_THUMBNAIL_PERMANENT + ", " + PLAYLIST_THUMBNAIL_STREAM_ID + ", " - + PLAYLIST_DISPLAY_INDEX + ", " - - + " CASE WHEN " + PLAYLIST_THUMBNAIL_STREAM_ID + " = " - + PlaylistEntity.DEFAULT_THUMBNAIL_ID + " THEN " + "'" + DEFAULT_THUMBNAIL + "'" - + " ELSE (SELECT " + STREAM_THUMBNAIL_URL - + " FROM " + STREAM_TABLE - + " WHERE " + STREAM_TABLE + "." + STREAM_ID + " = " + PLAYLIST_THUMBNAIL_STREAM_ID - + " ) END AS " + PLAYLIST_THUMBNAIL_URL + ", " - - + "COALESCE(COUNT(" + JOIN_PLAYLIST_ID + "), 0) AS " + PLAYLIST_STREAM_COUNT - + " FROM " + PLAYLIST_TABLE - + " LEFT JOIN " + PLAYLIST_STREAM_JOIN_TABLE - + " ON " + PLAYLIST_TABLE + "." + PLAYLIST_ID + " = " + JOIN_PLAYLIST_ID - + " GROUP BY " + PLAYLIST_ID - + " ORDER BY " + PLAYLIST_NAME + " COLLATE NOCASE ASC") - Flowable> getPlaylistMetadata(); - @Transaction @Query("SELECT " + PLAYLIST_ID + ", " + PLAYLIST_NAME + ", " + PLAYLIST_THUMBNAIL_PERMANENT + ", " + PLAYLIST_THUMBNAIL_STREAM_ID + ", " @@ -130,7 +110,7 @@ public interface PlaylistStreamDAO extends BasicDAO { + " ON " + PLAYLIST_TABLE + "." + PLAYLIST_ID + " = " + JOIN_PLAYLIST_ID + " GROUP BY " + PLAYLIST_ID + " ORDER BY " + PLAYLIST_DISPLAY_INDEX) - Flowable> getDisplayIndexOrderedPlaylistMetadata(); + Flowable> getPlaylistMetadata(); @RewriteQueriesToDropUnusedColumns @Transaction 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 41acd2615..59e2582ff 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 @@ -1,5 +1,6 @@ package org.schabi.newpipe.local.bookmark; +import static org.schabi.newpipe.local.bookmark.MergedPlaylistManager.getMergedOrderedPlaylists; import static org.schabi.newpipe.util.ThemeHelper.shouldUseGridLayout; import android.content.DialogInterface; @@ -47,7 +48,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import icepick.State; import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; -import io.reactivex.rxjava3.core.Flowable; import io.reactivex.rxjava3.disposables.CompositeDisposable; import io.reactivex.rxjava3.disposables.Disposable; @@ -184,9 +184,7 @@ public final class BookmarkFragment extends BaseLocalListFragment> getMergedOrderedPlaylists( + final LocalPlaylistManager localPlaylistManager, + final RemotePlaylistManager remotePlaylistManager) { + return Flowable.combineLatest( + localPlaylistManager.getPlaylists(), + remotePlaylistManager.getPlaylists(), + MergedPlaylistManager::merge + ); + } + + /** + * Merge localPlaylists and remotePlaylists by the display index. + * If two items have the same display index, sort them in {@code CASE_INSENSITIVE_ORDER}. + * + * @param localPlaylists local playlists, already sorted by display index + * @param remotePlaylists remote playlists, already sorted by display index + * @return merged playlists + */ + private static List merge( + final List localPlaylists, + final List remotePlaylists) { + + // This algorithm is similar to the merge operation in merge sort. + final List result = new ArrayList<>( + localPlaylists.size() + remotePlaylists.size()); + final List itemsWithSameIndex = new ArrayList<>(); + + int i = 0; + int j = 0; + while (i < localPlaylists.size()) { + while (j < remotePlaylists.size()) { + if (remotePlaylists.get(j).getDisplayIndex() + <= localPlaylists.get(i).getDisplayIndex()) { + addItem(result, remotePlaylists.get(j), itemsWithSameIndex); + j++; + } else { + break; + } + } + addItem(result, localPlaylists.get(i), itemsWithSameIndex); + i++; + } + while (j < remotePlaylists.size()) { + addItem(result, remotePlaylists.get(j), itemsWithSameIndex); + j++; + } + addItemsWithSameIndex(result, itemsWithSameIndex); + + return result; + } + + private static void addItem(final List result, + final PlaylistLocalItem item, + final List itemsWithSameIndex) { + if (!itemsWithSameIndex.isEmpty() + && itemsWithSameIndex.get(0).getDisplayIndex() != item.getDisplayIndex()) { + // The new item has a different display index, add previous items with same + // index to the result. + addItemsWithSameIndex(result, itemsWithSameIndex); + itemsWithSameIndex.clear(); + } + itemsWithSameIndex.add(item); + } + + private static void addItemsWithSameIndex(final List result, + final List itemsWithSameIndex) { + Collections.sort(itemsWithSameIndex, + Comparator.comparing(PlaylistLocalItem::getOrderingName, + Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER))); + result.addAll(itemsWithSameIndex); + } +} 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 e153f0a10..461ac2d0a 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 @@ -19,7 +19,6 @@ import java.util.List; import io.reactivex.rxjava3.core.Completable; import io.reactivex.rxjava3.core.Flowable; import io.reactivex.rxjava3.core.Maybe; -import io.reactivex.rxjava3.core.Single; import io.reactivex.rxjava3.schedulers.Schedulers; public class LocalPlaylistManager { @@ -108,10 +107,6 @@ public class LocalPlaylistManager { })).subscribeOn(Schedulers.io()); } - public Flowable> getPlaylists() { - return playlistStreamTable.getPlaylistMetadata().subscribeOn(Schedulers.io()); - } - public Flowable> getDistinctPlaylistStreams(final long playlistId) { return playlistStreamTable .getStreamsWithoutDuplicates(playlistId).subscribeOn(Schedulers.io()); @@ -129,20 +124,14 @@ public class LocalPlaylistManager { .subscribeOn(Schedulers.io()); } - public Flowable> getDisplayIndexOrderedPlaylists() { - return playlistStreamTable.getDisplayIndexOrderedPlaylistMetadata() - .subscribeOn(Schedulers.io()); + public Flowable> getPlaylists() { + return playlistStreamTable.getPlaylistMetadata().subscribeOn(Schedulers.io()); } public Flowable> getPlaylistStreams(final long playlistId) { return playlistStreamTable.getOrderedStreamsOf(playlistId).subscribeOn(Schedulers.io()); } - public Single deletePlaylist(final long playlistId) { - return Single.fromCallable(() -> playlistTable.deletePlaylist(playlistId)) - .subscribeOn(Schedulers.io()); - } - public Maybe renamePlaylist(final long playlistId, final String name) { return modifyPlaylist(playlistId, name, THUMBNAIL_ID_LEAVE_UNCHANGED, false, -1); } diff --git a/app/src/main/java/org/schabi/newpipe/local/playlist/RemotePlaylistManager.java b/app/src/main/java/org/schabi/newpipe/local/playlist/RemotePlaylistManager.java index 45d4ef644..4cc51f752 100644 --- a/app/src/main/java/org/schabi/newpipe/local/playlist/RemotePlaylistManager.java +++ b/app/src/main/java/org/schabi/newpipe/local/playlist/RemotePlaylistManager.java @@ -23,11 +23,7 @@ public class RemotePlaylistManager { } public Flowable> getPlaylists() { - return playlistRemoteTable.getAll().subscribeOn(Schedulers.io()); - } - - public Flowable> getDisplayIndexOrderedPlaylists() { - return playlistRemoteTable.getDisplayIndexOrderedPlaylists().subscribeOn(Schedulers.io()); + return playlistRemoteTable.getPlaylists().subscribeOn(Schedulers.io()); } public Flowable> getPlaylist(final PlaylistInfo info) { diff --git a/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java b/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java index 3e97d42e6..36abef9e5 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java @@ -1,5 +1,7 @@ package org.schabi.newpipe.settings; +import static org.schabi.newpipe.local.bookmark.MergedPlaylistManager.getMergedOrderedPlaylists; + import android.os.Bundle; import android.view.LayoutInflater; import android.view.View; @@ -31,7 +33,6 @@ import java.util.List; import java.util.Vector; import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; -import io.reactivex.rxjava3.core.Flowable; import io.reactivex.rxjava3.disposables.Disposable; public class SelectPlaylistFragment extends DialogFragment { @@ -90,8 +91,7 @@ public class SelectPlaylistFragment extends DialogFragment { final LocalPlaylistManager localPlaylistManager = new LocalPlaylistManager(database); final RemotePlaylistManager remotePlaylistManager = new RemotePlaylistManager(database); - disposable = Flowable.combineLatest(localPlaylistManager.getDisplayIndexOrderedPlaylists(), - remotePlaylistManager.getDisplayIndexOrderedPlaylists(), PlaylistLocalItem::merge) + disposable = getMergedOrderedPlaylists(localPlaylistManager, remotePlaylistManager) .observeOn(AndroidSchedulers.mainThread()) .subscribe(this::displayPlaylists, this::onError); }