Merge pull request #9755 from Jared234/9458_faulty_playlist_thumbnail_update

Fixed a bug that caused erroneous updates of the playlist thumbnails

Author: Stypox

app/schemas/org.schabi.newpipe.database.AppDatabase/7.json

@@ -101,6 +101,13 @@ class DatabaseMigrationTest {
             Migrations.MIGRATION_5_6
         )
 
+        testHelper.runMigrationsAndValidate(
+            AppDatabase.DATABASE_NAME,
+            Migrations.DB_VER_7,
+            true,
+            Migrations.MIGRATION_6_7
+        )
+
         val migratedDatabaseV3 = getMigratedDatabase()
         val listFromDB = migratedDatabaseV3.streamDAO().all.blockingFirst()
 

app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt

@@ -6,6 +6,7 @@
 import static org.schabi.newpipe.database.Migrations.MIGRATION_3_4;
 import static org.schabi.newpipe.database.Migrations.MIGRATION_4_5;
 import static org.schabi.newpipe.database.Migrations.MIGRATION_5_6;
+import static org.schabi.newpipe.database.Migrations.MIGRATION_6_7;
 
 import android.content.Context;
 import android.database.Cursor;
@@ -26,7 +27,7 @@ private static AppDatabase getDatabase(final Context context) {
         return Room
                 .databaseBuilder(context.getApplicationContext(), AppDatabase.class, DATABASE_NAME)
                 .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5,
-                        MIGRATION_5_6)
+                        MIGRATION_5_6, MIGRATION_6_7)
                 .build();
     }
 

app/src/main/java/org/schabi/newpipe/NewPipeDatabase.java

@@ -1,6 +1,6 @@
 package org.schabi.newpipe.database;
 
-import static org.schabi.newpipe.database.Migrations.DB_VER_6;
+import static org.schabi.newpipe.database.Migrations.DB_VER_7;
 
 import androidx.room.Database;
 import androidx.room.RoomDatabase;
@@ -38,7 +38,7 @@
                 FeedEntity.class, FeedGroupEntity.class, FeedGroupSubscriptionEntity.class,
                 FeedLastUpdatedEntity.class
         },
-        version = DB_VER_6
+        version = DB_VER_7
 )
 public abstract class AppDatabase extends RoomDatabase {
     public static final String DATABASE_NAME = "newpipe.db";

app/src/main/java/org/schabi/newpipe/database/AppDatabase.java

@@ -24,6 +24,7 @@ public final class Migrations {
     public static final int DB_VER_4 = 4;
     public static final int DB_VER_5 = 5;
     public static final int DB_VER_6 = 6;
+    public static final int DB_VER_7 = 7;
 
     private static final String TAG = Migrations.class.getName();
     public static final boolean DEBUG = MainActivity.DEBUG;
@@ -197,6 +198,43 @@ public void migrate(@NonNull final SupportSQLiteDatabase database) {
         }
     };
 
+    public static final Migration MIGRATION_6_7 = new Migration(DB_VER_6, DB_VER_7) {
+        @Override
+        public void migrate(@NonNull final SupportSQLiteDatabase database) {
+            // Create a new column thumbnail_stream_id
+            database.execSQL("ALTER TABLE `playlists` ADD COLUMN `thumbnail_stream_id` "
+                    + "INTEGER NOT NULL DEFAULT -1");
+
+            // Migrate the thumbnail_url to the thumbnail_stream_id
+            database.execSQL("UPDATE playlists SET thumbnail_stream_id = ("
+                    + " SELECT CASE WHEN COUNT(*) != 0 then stream_uid ELSE -1 END"
+                    + " FROM ("
+                    + " SELECT p.uid AS playlist_uid, s.uid AS stream_uid"
+                    + " FROM playlists p"
+                    + " LEFT JOIN playlist_stream_join ps ON p.uid = ps.playlist_id"
+                    + " LEFT JOIN streams s ON s.uid = ps.stream_id"
+                    + " WHERE s.thumbnail_url = p.thumbnail_url) AS temporary_table"
+                    + " WHERE playlist_uid = playlists.uid)");
+
+            // Remove the thumbnail_url field in the playlist table
+            database.execSQL("CREATE TABLE IF NOT EXISTS `playlists_new`"
+                    + "(uid INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "
+                    + "name TEXT, "
+                    + "is_thumbnail_permanent INTEGER NOT NULL, "
+                    + "thumbnail_stream_id INTEGER NOT NULL)");
+
+            database.execSQL("INSERT INTO playlists_new"
+                    + " SELECT uid, name, is_thumbnail_permanent, thumbnail_stream_id "
+                    + " FROM playlists");
+
+
+            database.execSQL("DROP TABLE playlists");
+            database.execSQL("ALTER TABLE playlists_new RENAME TO playlists");
+            database.execSQL("CREATE INDEX IF NOT EXISTS "
+                    + "`index_playlists_name` ON `playlists` (`name`)");
+        }
+    };
+
     private Migrations() {
     }
 }

app/src/main/java/org/schabi/newpipe/database/Migrations.java

@@ -9,6 +9,7 @@
 import org.schabi.newpipe.database.playlist.PlaylistDuplicatesEntry;
 import org.schabi.newpipe.database.playlist.PlaylistMetadataEntry;
 import org.schabi.newpipe.database.playlist.PlaylistStreamEntry;
+import org.schabi.newpipe.database.playlist.model.PlaylistEntity;
 import org.schabi.newpipe.database.playlist.model.PlaylistStreamEntity;
 
 import java.util.List;
@@ -17,9 +18,11 @@
 
 import static org.schabi.newpipe.database.playlist.PlaylistDuplicatesEntry.PLAYLIST_TIMES_STREAM_IS_CONTAINED;
 import static org.schabi.newpipe.database.playlist.PlaylistMetadataEntry.PLAYLIST_STREAM_COUNT;
+import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.DEFAULT_THUMBNAIL;
 import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_ID;
 import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_NAME;
 import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_TABLE;
+import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_THUMBNAIL_STREAM_ID;
 import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_THUMBNAIL_URL;
 import static org.schabi.newpipe.database.playlist.model.PlaylistStreamEntity.JOIN_INDEX;
 import static org.schabi.newpipe.database.playlist.model.PlaylistStreamEntity.JOIN_PLAYLIST_ID;
@@ -57,14 +60,15 @@ default Flowable<List<PlaylistStreamEntity>> listByService(final int serviceId)
             + " WHERE " + JOIN_PLAYLIST_ID + " = :playlistId")
     Flowable<Integer> getMaximumIndexOf(long playlistId);
 
-    @Query("SELECT CASE WHEN COUNT(*) != 0 then " + STREAM_THUMBNAIL_URL + " ELSE :defaultUrl END"
+    @Query("SELECT CASE WHEN COUNT(*) != 0 then " + STREAM_ID
+            + " ELSE " + PlaylistEntity.DEFAULT_THUMBNAIL_ID + " END"
             + " FROM " + STREAM_TABLE
             + " LEFT JOIN " + PLAYLIST_STREAM_JOIN_TABLE
             + " ON " + STREAM_ID + " = " + JOIN_STREAM_ID
             + " WHERE " + JOIN_PLAYLIST_ID + " = :playlistId "
             + " LIMIT 1"
     )
-    Flowable<String> getAutomaticThumbnailUrl(long playlistId, String defaultUrl);
+    Flowable<Long> getAutomaticThumbnailStreamId(long playlistId);
 
     @RewriteQueriesToDropUnusedColumns
     @Transaction
@@ -87,20 +91,34 @@ default Flowable<List<PlaylistStreamEntity>> listByService(final int serviceId)
     Flowable<List<PlaylistStreamEntry>> getOrderedStreamsOf(long playlistId);
 
     @Transaction
-    @Query("SELECT " + PLAYLIST_ID + ", " + PLAYLIST_NAME + ", " + PLAYLIST_THUMBNAIL_URL + ", "
-            + "COALESCE(COUNT(" + JOIN_PLAYLIST_ID + "), 0) AS " + PLAYLIST_STREAM_COUNT
+    @Query("SELECT " + PLAYLIST_ID + ", " + PLAYLIST_NAME + ","
+
+            + " 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_ID + " = " + JOIN_PLAYLIST_ID
+            + " 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_TABLE + "." + PLAYLIST_ID + ", "
             + PLAYLIST_NAME + ", "
-            + PLAYLIST_TABLE + "." + PLAYLIST_THUMBNAIL_URL + ", "
+
+            + " 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 + ", "
             + "COALESCE(SUM(" + STREAM_URL + " = :streamUrl), 0) AS "
                 + PLAYLIST_TIMES_STREAM_IS_CONTAINED

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

@@ -8,14 +8,22 @@
 import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_NAME;
 import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_TABLE;
 
+import org.schabi.newpipe.R;
+
 @Entity(tableName = PLAYLIST_TABLE,
         indices = {@Index(value = {PLAYLIST_NAME})})
 public class PlaylistEntity {
+
+    public static final String DEFAULT_THUMBNAIL = "drawable://"
+            + R.drawable.placeholder_thumbnail_playlist;
+    public static final long DEFAULT_THUMBNAIL_ID = -1;
+
     public static final String PLAYLIST_TABLE = "playlists";
     public static final String PLAYLIST_ID = "uid";
     public static final String PLAYLIST_NAME = "name";
     public static final String PLAYLIST_THUMBNAIL_URL = "thumbnail_url";
     public static final String PLAYLIST_THUMBNAIL_PERMANENT = "is_thumbnail_permanent";
+    public static final String PLAYLIST_THUMBNAIL_STREAM_ID = "thumbnail_stream_id";
 
     @PrimaryKey(autoGenerate = true)
     @ColumnInfo(name = PLAYLIST_ID)
@@ -24,17 +32,17 @@ public class PlaylistEntity {
     @ColumnInfo(name = PLAYLIST_NAME)
     private String name;
 
-    @ColumnInfo(name = PLAYLIST_THUMBNAIL_URL)
-    private String thumbnailUrl;
-
     @ColumnInfo(name = PLAYLIST_THUMBNAIL_PERMANENT)
     private boolean isThumbnailPermanent;
 
-    public PlaylistEntity(final String name, final String thumbnailUrl,
-                          final boolean isThumbnailPermanent) {
+    @ColumnInfo(name = PLAYLIST_THUMBNAIL_STREAM_ID)
+    private long thumbnailStreamId;
+
+    public PlaylistEntity(final String name, final boolean isThumbnailPermanent,
+                          final long thumbnailStreamId) {
         this.name = name;
-        this.thumbnailUrl = thumbnailUrl;
         this.isThumbnailPermanent = isThumbnailPermanent;
+        this.thumbnailStreamId = thumbnailStreamId;
     }
 
     public long getUid() {
@@ -53,12 +61,12 @@ public void setName(final String name) {
         this.name = name;
     }
 
-    public String getThumbnailUrl() {
-        return thumbnailUrl;
+    public long getThumbnailStreamId() {
+        return thumbnailStreamId;
     }
 
-    public void setThumbnailUrl(final String thumbnailUrl) {
-        this.thumbnailUrl = thumbnailUrl;
+    public void setThumbnailStreamId(final long thumbnailStreamId) {
+        this.thumbnailStreamId = thumbnailStreamId;
     }
 
     public boolean getIsThumbnailPermanent() {

app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java

@@ -280,10 +280,10 @@ private void showLocalDialog(final PlaylistMetadataEntry selectedItem) {
                 showDeleteDialog(selectedItem.name,
                         localPlaylistManager.deletePlaylist(selectedItem.uid));
             } else if (isThumbnailPermanent && items.get(index).equals(unsetThumbnail)) {
-                final String thumbnailUrl = localPlaylistManager
-                        .getAutomaticPlaylistThumbnail(selectedItem.uid);
+                final long thumbnailStreamId = localPlaylistManager
+                        .getAutomaticPlaylistThumbnailStreamId(selectedItem.uid);
                 localPlaylistManager
-                        .changePlaylistThumbnail(selectedItem.uid, thumbnailUrl, false)
+                        .changePlaylistThumbnail(selectedItem.uid, thumbnailStreamId, false)
                         .observeOn(AndroidSchedulers.mainThread())
                         .subscribe();
             }

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

@@ -14,6 +14,7 @@
 
 import org.schabi.newpipe.NewPipeDatabase;
 import org.schabi.newpipe.R;
+import org.schabi.newpipe.database.playlist.model.PlaylistEntity;
 import org.schabi.newpipe.database.playlist.PlaylistDuplicatesEntry;
 import org.schabi.newpipe.database.stream.model.StreamEntity;
 import org.schabi.newpipe.local.LocalItemListAdapter;
@@ -154,17 +155,19 @@ private void onPlaylistSelected(@NonNull final LocalPlaylistManager manager,
 
         final Toast successToast = Toast.makeText(getContext(), toastText, Toast.LENGTH_SHORT);
 
-        if (playlist.thumbnailUrl
-                .equals("drawable://" + R.drawable.placeholder_thumbnail_playlist)) {
-            playlistDisposables.add(manager
-                    .changePlaylistThumbnail(playlist.uid, streams.get(0).getThumbnailUrl(), false)
-                    .observeOn(AndroidSchedulers.mainThread())
-                    .subscribe(ignored -> successToast.show()));
-        }
-
         playlistDisposables.add(manager.appendToPlaylist(playlist.uid, streams)
                 .observeOn(AndroidSchedulers.mainThread())
-                .subscribe(ignored -> successToast.show()));
+                .subscribe(ignored -> {
+                    successToast.show();
+
+                    if (playlist.thumbnailUrl.equals(PlaylistEntity.DEFAULT_THUMBNAIL)) {
+                        playlistDisposables.add(manager
+                                .changePlaylistThumbnail(playlist.uid, streams.get(0).getUid(),
+                                        false)
+                                .observeOn(AndroidSchedulers.mainThread())
+                                .subscribe(ignore -> successToast.show()));
+                    }
+                }));
 
         requireDialog().dismiss();
     }

app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java

@@ -35,6 +35,7 @@
 import org.schabi.newpipe.database.LocalItem;
 import org.schabi.newpipe.database.history.model.StreamHistoryEntry;
 import org.schabi.newpipe.database.playlist.PlaylistStreamEntry;
+import org.schabi.newpipe.database.playlist.model.PlaylistEntity;
 import org.schabi.newpipe.database.stream.model.StreamEntity;
 import org.schabi.newpipe.databinding.DialogEditTextBinding;
 import org.schabi.newpipe.databinding.LocalPlaylistHeaderBinding;
@@ -417,8 +418,8 @@ public void removeWatchedStreams(final boolean removePartiallyWatched) {
                             if (indexInHistory < 0) {
                                 itemsToKeep.add(playlistItem);
                             } else if (!isThumbnailPermanent && !thumbnailVideoRemoved
-                                    && playlistManager.getPlaylistThumbnail(playlistId)
-                                    .equals(playlistItem.getStreamEntity().getThumbnailUrl())) {
+                                    && playlistManager.getPlaylistThumbnailStreamId(playlistId)
+                                    == playlistItem.getStreamEntity().getUid()) {
                                 thumbnailVideoRemoved = true;
                             }
                         }
@@ -438,8 +439,8 @@ public void removeWatchedStreams(final boolean removePartiallyWatched) {
                                     && !streamStateEntity.isFinished(duration))) {
                                 itemsToKeep.add(playlistItem);
                             } else if (!isThumbnailPermanent && !thumbnailVideoRemoved
-                                    && playlistManager.getPlaylistThumbnail(playlistId)
-                                    .equals(playlistItem.getStreamEntity().getThumbnailUrl())) {
+                                    && playlistManager.getPlaylistThumbnailStreamId(playlistId)
+                                    == playlistItem.getStreamEntity().getUid()) {
                                 thumbnailVideoRemoved = true;
                             }
                         }
@@ -587,7 +588,7 @@ private void changePlaylistName(final String title) {
         disposables.add(disposable);
     }
 
-    private void changeThumbnailUrl(final String thumbnailUrl, final boolean isPermanent) {
+    private void changeThumbnailStreamId(final long thumbnailStreamId, final boolean isPermanent) {
         if (playlistManager == null || (!isPermanent && playlistManager
                 .getIsPlaylistThumbnailPermanent(playlistId))) {
             return;
@@ -599,11 +600,11 @@ private void changeThumbnailUrl(final String thumbnailUrl, final boolean isPerma
 
         if (DEBUG) {
             Log.d(TAG, "Updating playlist id=[" + playlistId + "] "
-                    + "with new thumbnail url=[" + thumbnailUrl + "]");
+                    + "with new thumbnail stream id=[" + thumbnailStreamId + "]");
         }
 
         final Disposable disposable = playlistManager
-                .changePlaylistThumbnail(playlistId, thumbnailUrl, isPermanent)
+                .changePlaylistThumbnail(playlistId, thumbnailStreamId, isPermanent)
                 .observeOn(AndroidSchedulers.mainThread())
                 .subscribe(ignore -> successToast.show(), throwable ->
                         showError(new ErrorInfo(throwable, UserAction.REQUESTED_BOOKMARK,
@@ -616,16 +617,16 @@ private void updateThumbnailUrl() {
             return;
         }
 
-        final String newThumbnailUrl;
+        final long thumbnailStreamId;
 
         if (!itemListAdapter.getItemsList().isEmpty()) {
-            newThumbnailUrl = ((PlaylistStreamEntry) itemListAdapter.getItemsList().get(0))
-                    .getStreamEntity().getThumbnailUrl();
+            thumbnailStreamId = ((PlaylistStreamEntry) itemListAdapter.getItemsList().get(0))
+                    .getStreamEntity().getUid();
         } else {
-            newThumbnailUrl = "drawable://" + R.drawable.placeholder_thumbnail_playlist;
+            thumbnailStreamId = PlaylistEntity.DEFAULT_THUMBNAIL_ID;
         }
 
-        changeThumbnailUrl(newThumbnailUrl, false);
+        changeThumbnailStreamId(thumbnailStreamId, false);
     }
 
     private void deleteItem(final PlaylistStreamEntry item) {
@@ -634,8 +635,7 @@ private void deleteItem(final PlaylistStreamEntry item) {
         }
 
         itemListAdapter.removeItem(item);
-        if (playlistManager.getPlaylistThumbnail(playlistId)
-                .equals(item.getStreamEntity().getThumbnailUrl())) {
+        if (playlistManager.getPlaylistThumbnailStreamId(playlistId) == item.getStreamId()) {
             updateThumbnailUrl();
         }
 
@@ -793,7 +793,7 @@ context, getPlayQueueStartingAt(item), true))
                     .setAction(
                             StreamDialogDefaultEntry.SET_AS_PLAYLIST_THUMBNAIL,
                             (f, i) ->
-                                    changeThumbnailUrl(item.getStreamEntity().getThumbnailUrl(),
+                                    changeThumbnailStreamId(item.getStreamEntity().getUid(),
                                             true))
                     .setAction(
                             StreamDialogDefaultEntry.DELETE,