Apply review

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`) "
+                        + "(`uid`, `name`, `is_thumbnail_permanent`, `thumbnail_stream_id`, "
-                        + "SELECT `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`)");
 

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,

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<PlaylistLocalItem> merge(
-            final List<PlaylistMetadataEntry> localPlaylists,
-            final List<PlaylistRemoteEntity> 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<PlaylistLocalItem> result = new ArrayList<>(
-                localPlaylists.size() + remotePlaylists.size());
-        final List<PlaylistLocalItem> 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<PlaylistLocalItem> result, final PlaylistLocalItem item,
-                        final List<PlaylistLocalItem> 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<PlaylistLocalItem> result,
-                                      final List<PlaylistLocalItem> itemsWithSameIndex) {
-        if (itemsWithSameIndex.size() > 1) {
-            Collections.sort(itemsWithSameIndex,
-                    Comparator.comparing(PlaylistLocalItem::getOrderingName,
-                            Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)));
-        }
-        result.addAll(itemsWithSameIndex);
-    }
 }

app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistRemoteDAO.java

@@ -42,7 +42,7 @@ public interface PlaylistRemoteDAO extends BasicDAO<PlaylistRemoteEntity> {
 
     @Query("SELECT * FROM " + REMOTE_PLAYLIST_TABLE
      + " ORDER BY " + REMOTE_PLAYLIST_DISPLAY_INDEX)
-    Flowable<List<PlaylistRemoteEntity>> getDisplayIndexOrderedPlaylists();
+    Flowable<List<PlaylistRemoteEntity>> getPlaylists();
 
     @Query("SELECT " + REMOTE_PLAYLIST_ID + " FROM " + REMOTE_PLAYLIST_TABLE
             + " WHERE " + REMOTE_PLAYLIST_URL + " = :url "

app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java

@@ -92,26 +92,6 @@ public interface PlaylistStreamDAO extends BasicDAO<PlaylistStreamEntity> {
             + " ORDER BY " + JOIN_INDEX + " ASC")
     Flowable<List<PlaylistStreamEntry>> 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<List<PlaylistMetadataEntry>> getPlaylistMetadata();
-
     @Transaction
     @Query("SELECT " + PLAYLIST_ID + ", " + PLAYLIST_NAME + ", "
             + PLAYLIST_THUMBNAIL_PERMANENT + ", " + PLAYLIST_THUMBNAIL_STREAM_ID + ", "
@@ -130,7 +110,7 @@ public interface PlaylistStreamDAO extends BasicDAO<PlaylistStreamEntity> {
             + " ON " + PLAYLIST_TABLE + "." + PLAYLIST_ID + " = " + JOIN_PLAYLIST_ID
             + " GROUP BY " + PLAYLIST_ID
             + " ORDER BY " + PLAYLIST_DISPLAY_INDEX)
-    Flowable<List<PlaylistMetadataEntry>> getDisplayIndexOrderedPlaylistMetadata();
+    Flowable<List<PlaylistMetadataEntry>> getPlaylistMetadata();
 
     @RewriteQueriesToDropUnusedColumns
     @Transaction

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<List<PlaylistL
         }
         isLoadingComplete.set(false);
 
-        Flowable.combineLatest(localPlaylistManager.getDisplayIndexOrderedPlaylists(),
+        getMergedOrderedPlaylists(localPlaylistManager, remotePlaylistManager)
-                        remotePlaylistManager.getDisplayIndexOrderedPlaylists(),
-                        PlaylistLocalItem::merge)
                 .onBackpressureLatest()
                 .observeOn(AndroidSchedulers.mainThread())
                 .subscribe(getPlaylistsSubscriber());
@@ -400,16 +398,14 @@ public final class BookmarkFragment extends BaseLocalListFragment<List<PlaylistL
         for (int i = 0; i < items.size(); i++) {
             final LocalItem item = items.get(i);
 
-            if (item instanceof PlaylistMetadataEntry) {
+            if (item instanceof PlaylistMetadataEntry
-                if (((PlaylistMetadataEntry) item).getDisplayIndex() != i) {
+                    && ((PlaylistMetadataEntry) item).getDisplayIndex() != i) {
-                    ((PlaylistMetadataEntry) item).setDisplayIndex(i);
+                ((PlaylistMetadataEntry) item).setDisplayIndex(i);
-                    localItemsUpdate.add((PlaylistMetadataEntry) item);
+                localItemsUpdate.add((PlaylistMetadataEntry) item);
-                }
+            } else if (item instanceof PlaylistRemoteEntity
-            } else if (item instanceof PlaylistRemoteEntity) {
+                    && ((PlaylistRemoteEntity) item).getDisplayIndex() != i) {
-                if (((PlaylistRemoteEntity) item).getDisplayIndex() != i) {
+                ((PlaylistRemoteEntity) item).setDisplayIndex(i);
-                    ((PlaylistRemoteEntity) item).setDisplayIndex(i);
+                remoteItemsUpdate.add((PlaylistRemoteEntity) item);
-                    remoteItemsUpdate.add((PlaylistRemoteEntity) item);
-                }
             }
         }
 

app/src/main/java/org/schabi/newpipe/local/bookmark/MergedPlaylistManager.java

@@ -0,0 +1,95 @@
+package org.schabi.newpipe.local.bookmark;
+
+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.local.playlist.LocalPlaylistManager;
+import org.schabi.newpipe.local.playlist.RemotePlaylistManager;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+import io.reactivex.rxjava3.core.Flowable;
+
+/**
+ * Takes care of remote and local playlists at once, hence "merged".
+ */
+public final class MergedPlaylistManager {
+
+    private MergedPlaylistManager() {
+    }
+
+    public static Flowable<List<PlaylistLocalItem>> 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<PlaylistLocalItem> merge(
+            final List<PlaylistMetadataEntry> localPlaylists,
+            final List<PlaylistRemoteEntity> remotePlaylists) {
+
+        // This algorithm is similar to the merge operation in merge sort.
+        final List<PlaylistLocalItem> result = new ArrayList<>(
+                localPlaylists.size() + remotePlaylists.size());
+        final List<PlaylistLocalItem> 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<PlaylistLocalItem> result,
+                                final PlaylistLocalItem item,
+                                final List<PlaylistLocalItem> 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<PlaylistLocalItem> result,
+                                              final List<PlaylistLocalItem> itemsWithSameIndex) {
+        Collections.sort(itemsWithSameIndex,
+                Comparator.comparing(PlaylistLocalItem::getOrderingName,
+                        Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)));
+        result.addAll(itemsWithSameIndex);
+    }
+}

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<List<PlaylistMetadataEntry>> getPlaylists() {
-        return playlistStreamTable.getPlaylistMetadata().subscribeOn(Schedulers.io());
-    }
-
     public Flowable<List<PlaylistStreamEntry>> getDistinctPlaylistStreams(final long playlistId) {
         return playlistStreamTable
                 .getStreamsWithoutDuplicates(playlistId).subscribeOn(Schedulers.io());
@@ -129,20 +124,14 @@ public class LocalPlaylistManager {
                 .subscribeOn(Schedulers.io());
     }
 
-    public Flowable<List<PlaylistMetadataEntry>> getDisplayIndexOrderedPlaylists() {
+    public Flowable<List<PlaylistMetadataEntry>> getPlaylists() {
-        return playlistStreamTable.getDisplayIndexOrderedPlaylistMetadata()
+        return playlistStreamTable.getPlaylistMetadata().subscribeOn(Schedulers.io());
-                .subscribeOn(Schedulers.io());
     }
 
     public Flowable<List<PlaylistStreamEntry>> getPlaylistStreams(final long playlistId) {
         return playlistStreamTable.getOrderedStreamsOf(playlistId).subscribeOn(Schedulers.io());
     }
 
-    public Single<Integer> deletePlaylist(final long playlistId) {
-        return Single.fromCallable(() -> playlistTable.deletePlaylist(playlistId))
-                .subscribeOn(Schedulers.io());
-    }
-
     public Maybe<Integer> renamePlaylist(final long playlistId, final String name) {
         return modifyPlaylist(playlistId, name, THUMBNAIL_ID_LEAVE_UNCHANGED, false, -1);
     }

app/src/main/java/org/schabi/newpipe/local/playlist/RemotePlaylistManager.java

@@ -23,11 +23,7 @@ public class RemotePlaylistManager {
     }
 
     public Flowable<List<PlaylistRemoteEntity>> getPlaylists() {
-        return playlistRemoteTable.getAll().subscribeOn(Schedulers.io());
+        return playlistRemoteTable.getPlaylists().subscribeOn(Schedulers.io());
-    }
-
-    public Flowable<List<PlaylistRemoteEntity>> getDisplayIndexOrderedPlaylists() {
-        return playlistRemoteTable.getDisplayIndexOrderedPlaylists().subscribeOn(Schedulers.io());
     }
 
     public Flowable<List<PlaylistRemoteEntity>> getPlaylist(final PlaylistInfo info) {

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(),
+        disposable = getMergedOrderedPlaylists(localPlaylistManager, remotePlaylistManager)
-                remotePlaylistManager.getDisplayIndexOrderedPlaylists(), PlaylistLocalItem::merge)
                 .observeOn(AndroidSchedulers.mainThread())
                 .subscribe(this::displayPlaylists, this::onError);
     }