From 96d6b309ec394dafb98ab944c842fda905af649e Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Wed, 13 Apr 2022 19:41:07 +0800 Subject: [PATCH 01/24] Migrate database --- .../6.json | 737 ++++++++++++++++++ .../newpipe/database/DatabaseMigrationTest.kt | 5 + .../org/schabi/newpipe/NewPipeDatabase.java | 4 +- .../schabi/newpipe/database/AppDatabase.java | 4 +- .../schabi/newpipe/database/Migrations.java | 42 +- .../playlist/model/PlaylistEntity.java | 12 + 6 files changed, 800 insertions(+), 4 deletions(-) create mode 100644 app/schemas/org.schabi.newpipe.database.AppDatabase/6.json diff --git a/app/schemas/org.schabi.newpipe.database.AppDatabase/6.json b/app/schemas/org.schabi.newpipe.database.AppDatabase/6.json new file mode 100644 index 000000000..34d457f83 --- /dev/null +++ b/app/schemas/org.schabi.newpipe.database.AppDatabase/6.json @@ -0,0 +1,737 @@ +{ + "formatVersion": 1, + "database": { + "version": 6, + "identityHash": "cc9c4d84f52f49105b1c4216b948b5f7", + "entities": [ + { + "tableName": "subscriptions", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `service_id` INTEGER NOT NULL, `url` TEXT, `name` TEXT, `avatar_url` TEXT, `subscriber_count` INTEGER, `description` TEXT, `notification_mode` INTEGER NOT NULL)", + "fields": [ + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "serviceId", + "columnName": "service_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "avatarUrl", + "columnName": "avatar_url", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "subscriberCount", + "columnName": "subscriber_count", + "affinity": "INTEGER", + "notNull": false + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "notificationMode", + "columnName": "notification_mode", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "uid" + ], + "autoGenerate": true + }, + "indices": [ + { + "name": "index_subscriptions_service_id_url", + "unique": true, + "columnNames": [ + "service_id", + "url" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_subscriptions_service_id_url` ON `${TABLE_NAME}` (`service_id`, `url`)" + } + ], + "foreignKeys": [] + }, + { + "tableName": "search_history", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`creation_date` INTEGER, `service_id` INTEGER NOT NULL, `search` TEXT, `id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL)", + "fields": [ + { + "fieldPath": "creationDate", + "columnName": "creation_date", + "affinity": "INTEGER", + "notNull": false + }, + { + "fieldPath": "serviceId", + "columnName": "service_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "search", + "columnName": "search", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "id" + ], + "autoGenerate": true + }, + "indices": [ + { + "name": "index_search_history_search", + "unique": false, + "columnNames": [ + "search" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_search_history_search` ON `${TABLE_NAME}` (`search`)" + } + ], + "foreignKeys": [] + }, + { + "tableName": "streams", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `service_id` INTEGER NOT NULL, `url` TEXT NOT NULL, `title` TEXT NOT NULL, `stream_type` TEXT NOT NULL, `duration` INTEGER NOT NULL, `uploader` TEXT NOT NULL, `uploader_url` TEXT, `thumbnail_url` TEXT, `view_count` INTEGER, `textual_upload_date` TEXT, `upload_date` INTEGER, `is_upload_date_approximation` INTEGER)", + "fields": [ + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "serviceId", + "columnName": "service_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "title", + "columnName": "title", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "streamType", + "columnName": "stream_type", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "duration", + "columnName": "duration", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "uploader", + "columnName": "uploader", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "uploaderUrl", + "columnName": "uploader_url", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "thumbnailUrl", + "columnName": "thumbnail_url", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "viewCount", + "columnName": "view_count", + "affinity": "INTEGER", + "notNull": false + }, + { + "fieldPath": "textualUploadDate", + "columnName": "textual_upload_date", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "uploadDate", + "columnName": "upload_date", + "affinity": "INTEGER", + "notNull": false + }, + { + "fieldPath": "isUploadDateApproximation", + "columnName": "is_upload_date_approximation", + "affinity": "INTEGER", + "notNull": false + } + ], + "primaryKey": { + "columnNames": [ + "uid" + ], + "autoGenerate": true + }, + "indices": [ + { + "name": "index_streams_service_id_url", + "unique": true, + "columnNames": [ + "service_id", + "url" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_streams_service_id_url` ON `${TABLE_NAME}` (`service_id`, `url`)" + } + ], + "foreignKeys": [] + }, + { + "tableName": "stream_history", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`stream_id` INTEGER NOT NULL, `access_date` INTEGER NOT NULL, `repeat_count` INTEGER NOT NULL, PRIMARY KEY(`stream_id`, `access_date`), FOREIGN KEY(`stream_id`) REFERENCES `streams`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "streamUid", + "columnName": "stream_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "accessDate", + "columnName": "access_date", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "repeatCount", + "columnName": "repeat_count", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "stream_id", + "access_date" + ], + "autoGenerate": false + }, + "indices": [ + { + "name": "index_stream_history_stream_id", + "unique": false, + "columnNames": [ + "stream_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_stream_history_stream_id` ON `${TABLE_NAME}` (`stream_id`)" + } + ], + "foreignKeys": [ + { + "table": "streams", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "stream_id" + ], + "referencedColumns": [ + "uid" + ] + } + ] + }, + { + "tableName": "stream_state", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`stream_id` INTEGER NOT NULL, `progress_time` INTEGER NOT NULL, PRIMARY KEY(`stream_id`), FOREIGN KEY(`stream_id`) REFERENCES `streams`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "streamUid", + "columnName": "stream_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "progressMillis", + "columnName": "progress_time", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "stream_id" + ], + "autoGenerate": false + }, + "indices": [], + "foreignKeys": [ + { + "table": "streams", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "stream_id" + ], + "referencedColumns": [ + "uid" + ] + } + ] + }, + { + "tableName": "playlists", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `name` TEXT, `thumbnail_url` TEXT, `display_index` INTEGER NOT NULL)", + "fields": [ + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "thumbnailUrl", + "columnName": "thumbnail_url", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "displayIndex", + "columnName": "display_index", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "uid" + ], + "autoGenerate": true + }, + "indices": [ + { + "name": "index_playlists_name", + "unique": false, + "columnNames": [ + "name" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_playlists_name` ON `${TABLE_NAME}` (`name`)" + } + ], + "foreignKeys": [] + }, + { + "tableName": "playlist_stream_join", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`playlist_id` INTEGER NOT NULL, `stream_id` INTEGER NOT NULL, `join_index` INTEGER NOT NULL, PRIMARY KEY(`playlist_id`, `join_index`), FOREIGN KEY(`playlist_id`) REFERENCES `playlists`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, FOREIGN KEY(`stream_id`) REFERENCES `streams`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED)", + "fields": [ + { + "fieldPath": "playlistUid", + "columnName": "playlist_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "streamUid", + "columnName": "stream_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "index", + "columnName": "join_index", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "playlist_id", + "join_index" + ], + "autoGenerate": false + }, + "indices": [ + { + "name": "index_playlist_stream_join_playlist_id_join_index", + "unique": true, + "columnNames": [ + "playlist_id", + "join_index" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_playlist_stream_join_playlist_id_join_index` ON `${TABLE_NAME}` (`playlist_id`, `join_index`)" + }, + { + "name": "index_playlist_stream_join_stream_id", + "unique": false, + "columnNames": [ + "stream_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_playlist_stream_join_stream_id` ON `${TABLE_NAME}` (`stream_id`)" + } + ], + "foreignKeys": [ + { + "table": "playlists", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "playlist_id" + ], + "referencedColumns": [ + "uid" + ] + }, + { + "table": "streams", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "stream_id" + ], + "referencedColumns": [ + "uid" + ] + } + ] + }, + { + "tableName": "remote_playlists", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `service_id` INTEGER NOT NULL, `name` TEXT, `url` TEXT, `thumbnail_url` TEXT, `uploader` TEXT, `stream_count` INTEGER)", + "fields": [ + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "serviceId", + "columnName": "service_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "thumbnailUrl", + "columnName": "thumbnail_url", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "uploader", + "columnName": "uploader", + "affinity": "TEXT", + "notNull": false + }, + { + "fieldPath": "streamCount", + "columnName": "stream_count", + "affinity": "INTEGER", + "notNull": false + } + ], + "primaryKey": { + "columnNames": [ + "uid" + ], + "autoGenerate": true + }, + "indices": [ + { + "name": "index_remote_playlists_name", + "unique": false, + "columnNames": [ + "name" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_remote_playlists_name` ON `${TABLE_NAME}` (`name`)" + }, + { + "name": "index_remote_playlists_service_id_url", + "unique": true, + "columnNames": [ + "service_id", + "url" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_remote_playlists_service_id_url` ON `${TABLE_NAME}` (`service_id`, `url`)" + } + ], + "foreignKeys": [] + }, + { + "tableName": "feed", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`stream_id` INTEGER NOT NULL, `subscription_id` INTEGER NOT NULL, PRIMARY KEY(`stream_id`, `subscription_id`), FOREIGN KEY(`stream_id`) REFERENCES `streams`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, FOREIGN KEY(`subscription_id`) REFERENCES `subscriptions`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED)", + "fields": [ + { + "fieldPath": "streamId", + "columnName": "stream_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "subscriptionId", + "columnName": "subscription_id", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "stream_id", + "subscription_id" + ], + "autoGenerate": false + }, + "indices": [ + { + "name": "index_feed_subscription_id", + "unique": false, + "columnNames": [ + "subscription_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_feed_subscription_id` ON `${TABLE_NAME}` (`subscription_id`)" + } + ], + "foreignKeys": [ + { + "table": "streams", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "stream_id" + ], + "referencedColumns": [ + "uid" + ] + }, + { + "table": "subscriptions", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "subscription_id" + ], + "referencedColumns": [ + "uid" + ] + } + ] + }, + { + "tableName": "feed_group", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `name` TEXT NOT NULL, `icon_id` INTEGER NOT NULL, `sort_order` INTEGER NOT NULL)", + "fields": [ + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "icon", + "columnName": "icon_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "sortOrder", + "columnName": "sort_order", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "uid" + ], + "autoGenerate": true + }, + "indices": [ + { + "name": "index_feed_group_sort_order", + "unique": false, + "columnNames": [ + "sort_order" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_feed_group_sort_order` ON `${TABLE_NAME}` (`sort_order`)" + } + ], + "foreignKeys": [] + }, + { + "tableName": "feed_group_subscription_join", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`group_id` INTEGER NOT NULL, `subscription_id` INTEGER NOT NULL, PRIMARY KEY(`group_id`, `subscription_id`), FOREIGN KEY(`group_id`) REFERENCES `feed_group`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, FOREIGN KEY(`subscription_id`) REFERENCES `subscriptions`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED)", + "fields": [ + { + "fieldPath": "feedGroupId", + "columnName": "group_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "subscriptionId", + "columnName": "subscription_id", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "group_id", + "subscription_id" + ], + "autoGenerate": false + }, + "indices": [ + { + "name": "index_feed_group_subscription_join_subscription_id", + "unique": false, + "columnNames": [ + "subscription_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_feed_group_subscription_join_subscription_id` ON `${TABLE_NAME}` (`subscription_id`)" + } + ], + "foreignKeys": [ + { + "table": "feed_group", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "group_id" + ], + "referencedColumns": [ + "uid" + ] + }, + { + "table": "subscriptions", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "subscription_id" + ], + "referencedColumns": [ + "uid" + ] + } + ] + }, + { + "tableName": "feed_last_updated", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`subscription_id` INTEGER NOT NULL, `last_updated` INTEGER, PRIMARY KEY(`subscription_id`), FOREIGN KEY(`subscription_id`) REFERENCES `subscriptions`(`uid`) ON UPDATE CASCADE ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED)", + "fields": [ + { + "fieldPath": "subscriptionId", + "columnName": "subscription_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastUpdated", + "columnName": "last_updated", + "affinity": "INTEGER", + "notNull": false + } + ], + "primaryKey": { + "columnNames": [ + "subscription_id" + ], + "autoGenerate": false + }, + "indices": [], + "foreignKeys": [ + { + "table": "subscriptions", + "onDelete": "CASCADE", + "onUpdate": "CASCADE", + "columns": [ + "subscription_id" + ], + "referencedColumns": [ + "uid" + ] + } + ] + } + ], + "views": [], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'cc9c4d84f52f49105b1c4216b948b5f7')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt b/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt index 28dea13e9..6d05a45bf 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt @@ -84,6 +84,11 @@ class DatabaseMigrationTest { true, Migrations.MIGRATION_4_5 ) + testHelper.runMigrationsAndValidate( + AppDatabase.DATABASE_NAME, Migrations.DB_VER_6, + true, Migrations.MIGRATION_5_6 + ) + val migratedDatabaseV3 = getMigratedDatabase() val listFromDB = migratedDatabaseV3.streamDAO().all.blockingFirst() diff --git a/app/src/main/java/org/schabi/newpipe/NewPipeDatabase.java b/app/src/main/java/org/schabi/newpipe/NewPipeDatabase.java index 402d4648d..fc3423994 100644 --- a/app/src/main/java/org/schabi/newpipe/NewPipeDatabase.java +++ b/app/src/main/java/org/schabi/newpipe/NewPipeDatabase.java @@ -5,6 +5,7 @@ import static org.schabi.newpipe.database.Migrations.MIGRATION_1_2; import static org.schabi.newpipe.database.Migrations.MIGRATION_2_3; 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 android.content.Context; import android.database.Cursor; @@ -24,7 +25,8 @@ public final class NewPipeDatabase { 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) + .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, + MIGRATION_5_6) .build(); } diff --git a/app/src/main/java/org/schabi/newpipe/database/AppDatabase.java b/app/src/main/java/org/schabi/newpipe/database/AppDatabase.java index 28ddc8184..563e80b17 100644 --- a/app/src/main/java/org/schabi/newpipe/database/AppDatabase.java +++ b/app/src/main/java/org/schabi/newpipe/database/AppDatabase.java @@ -1,6 +1,6 @@ package org.schabi.newpipe.database; -import static org.schabi.newpipe.database.Migrations.DB_VER_5; +import static org.schabi.newpipe.database.Migrations.DB_VER_6; import androidx.room.Database; import androidx.room.RoomDatabase; @@ -38,7 +38,7 @@ import org.schabi.newpipe.database.subscription.SubscriptionEntity; FeedEntity.class, FeedGroupEntity.class, FeedGroupSubscriptionEntity.class, FeedLastUpdatedEntity.class }, - version = DB_VER_5 + version = DB_VER_6 ) public abstract class AppDatabase extends RoomDatabase { public static final String DATABASE_NAME = "newpipe.db"; 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 7de08442c..a8f093ba0 100644 --- a/app/src/main/java/org/schabi/newpipe/database/Migrations.java +++ b/app/src/main/java/org/schabi/newpipe/database/Migrations.java @@ -23,6 +23,7 @@ public final class Migrations { public static final int DB_VER_3 = 3; public static final int DB_VER_4 = 4; public static final int DB_VER_5 = 5; + public static final int DB_VER_6 = 6; private static final String TAG = Migrations.class.getName(); public static final boolean DEBUG = MainActivity.DEBUG; @@ -184,7 +185,46 @@ public final class Migrations { @Override public void migrate(@NonNull final SupportSQLiteDatabase database) { database.execSQL("ALTER TABLE `subscriptions` ADD COLUMN `notification_mode` " - + "INTEGER NOT NULL DEFAULT 0"); + + "INTEGER NOT NULL DEFAULT 0"); + } + }; + + public static final Migration MIGRATION_5_6 = new Migration(DB_VER_5, DB_VER_6) { + @Override + public void migrate(@NonNull final SupportSQLiteDatabase database) { + try { + database.beginTransaction(); + + // create a temp table to initialize display_index + database.execSQL("CREATE TABLE `playlists_tmp` " + + "(`uid` INTEGER NOT NULL, " + + "`name` TEXT, `thumbnail_url` TEXT," + + "`display_index` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL)"); + database.execSQL("INSERT INTO `playlists_tmp` (`uid`, `name`, `thumbnail_url`)" + + "SELECT `uid`, `name`, `thumbnail_url` FROM `playlists`"); + + // drop the old table and create new one + database.execSQL("DROP TABLE `playlists`"); + database.execSQL("CREATE TABLE `playlists` " + + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " + + "`name` TEXT, `thumbnail_url` TEXT," + + "`display_index` INTEGER NOT NULL UNIQUE)"); + + // insert temp data into the new table + // set display_index start from zero + database.execSQL("INSERT INTO `playlists` SELECT * FROM `playlists_tmp`"); + database.execSQL("UPDATE `playlists` SET `display_index` = `display_index` - 1"); + + // drop tmp table + database.execSQL("DROP TABLE `playlists_tmp`"); + + // create index on the new table + database.execSQL("CREATE INDEX `index_playlists_name` ON `playlists` (`name`)"); + + database.setTransactionSuccessful(); + } finally { + database.endTransaction(); + } } }; diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java index 71abf2732..c1ae0a2b3 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java @@ -15,6 +15,7 @@ public class PlaylistEntity { 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_DISPLAY_INDEX = "display_index"; @PrimaryKey(autoGenerate = true) @ColumnInfo(name = PLAYLIST_ID) @@ -26,6 +27,9 @@ public class PlaylistEntity { @ColumnInfo(name = PLAYLIST_THUMBNAIL_URL) private String thumbnailUrl; + @ColumnInfo(name = PLAYLIST_DISPLAY_INDEX) + private long displayIndex = 0; + public PlaylistEntity(final String name, final String thumbnailUrl) { this.name = name; this.thumbnailUrl = thumbnailUrl; @@ -54,4 +58,12 @@ public class PlaylistEntity { public void setThumbnailUrl(final String thumbnailUrl) { this.thumbnailUrl = thumbnailUrl; } + + public long getDisplayIndex() { + return displayIndex; + } + + public void setDisplayIndex(final long displayIndex) { + this.displayIndex = displayIndex; + } } From c34549a47d0c39050ddd34357e43adfce56ea638 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Wed, 13 Apr 2022 21:35:38 +0800 Subject: [PATCH 02/24] Update database migrations and getter/setter --- .../6.json | 12 +++-- .../schabi/newpipe/database/Migrations.java | 44 ++++++++++++------- .../database/playlist/PlaylistLocalItem.java | 1 + .../playlist/PlaylistMetadataEntry.java | 6 ++- .../playlist/dao/PlaylistStreamDAO.java | 2 + .../playlist/model/PlaylistRemoteEntity.java | 25 +++++++++++ .../local/bookmark/BookmarkFragment.java | 1 - .../local/playlist/LocalPlaylistManager.java | 15 +++++-- 8 files changed, 83 insertions(+), 23 deletions(-) diff --git a/app/schemas/org.schabi.newpipe.database.AppDatabase/6.json b/app/schemas/org.schabi.newpipe.database.AppDatabase/6.json index 34d457f83..3ef363e30 100644 --- a/app/schemas/org.schabi.newpipe.database.AppDatabase/6.json +++ b/app/schemas/org.schabi.newpipe.database.AppDatabase/6.json @@ -2,7 +2,7 @@ "formatVersion": 1, "database": { "version": 6, - "identityHash": "cc9c4d84f52f49105b1c4216b948b5f7", + "identityHash": "9ffc14521c566beed378d77430de3f0c", "entities": [ { "tableName": "subscriptions", @@ -447,7 +447,7 @@ }, { "tableName": "remote_playlists", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `service_id` INTEGER NOT NULL, `name` TEXT, `url` TEXT, `thumbnail_url` TEXT, `uploader` TEXT, `stream_count` INTEGER)", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `service_id` INTEGER NOT NULL, `name` TEXT, `url` TEXT, `thumbnail_url` TEXT, `uploader` TEXT, `display_index` INTEGER NOT NULL, `stream_count` INTEGER)", "fields": [ { "fieldPath": "uid", @@ -485,6 +485,12 @@ "affinity": "TEXT", "notNull": false }, + { + "fieldPath": "displayIndex", + "columnName": "display_index", + "affinity": "INTEGER", + "notNull": true + }, { "fieldPath": "streamCount", "columnName": "stream_count", @@ -731,7 +737,7 @@ "views": [], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'cc9c4d84f52f49105b1c4216b948b5f7')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '9ffc14521c566beed378d77430de3f0c')" ] } } \ No newline at end of file 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 a8f093ba0..ffca6cca5 100644 --- a/app/src/main/java/org/schabi/newpipe/database/Migrations.java +++ b/app/src/main/java/org/schabi/newpipe/database/Migrations.java @@ -195,32 +195,46 @@ public final class Migrations { try { database.beginTransaction(); + // update playlists // create a temp table to initialize display_index database.execSQL("CREATE TABLE `playlists_tmp` " - + "(`uid` INTEGER NOT NULL, " + + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " + "`name` TEXT, `thumbnail_url` TEXT," - + "`display_index` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL)"); + + "`display_index` INTEGER NOT NULL DEFAULT 0)"); database.execSQL("INSERT INTO `playlists_tmp` (`uid`, `name`, `thumbnail_url`)" + "SELECT `uid`, `name`, `thumbnail_url` FROM `playlists`"); - // drop the old table and create new one + // replace the old table database.execSQL("DROP TABLE `playlists`"); - database.execSQL("CREATE TABLE `playlists` " - + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " - + "`name` TEXT, `thumbnail_url` TEXT," - + "`display_index` INTEGER NOT NULL UNIQUE)"); - - // insert temp data into the new table - // set display_index start from zero - database.execSQL("INSERT INTO `playlists` SELECT * FROM `playlists_tmp`"); - database.execSQL("UPDATE `playlists` SET `display_index` = `display_index` - 1"); - - // drop tmp table - database.execSQL("DROP TABLE `playlists_tmp`"); + 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 + database.execSQL("CREATE TABLE `remote_playlists_tmp` " + + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " + + "`service_id` INTEGER NOT NULL, `name` TEXT, `url` TEXT, " + + "`thumbnail_url` TEXT, `uploader` TEXT, " + + "`display_index` INTEGER NOT NULL DEFAULT 0," + + "`stream_count` INTEGER)"); + database.execSQL("INSERT INTO `remote_playlists_tmp` (`uid`, `service_id`, " + + "`name`, `url`, `thumbnail_url`, `uploader`, `stream_count`)" + + "SELECT `uid`, `service_id`, `name`, `url`, `thumbnail_url`, `uploader`, " + + "`stream_count` FROM `remote_playlists`"); + + // replace the old table + 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`)"); + database.setTransactionSuccessful(); } finally { database.endTransaction(); 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 43dbd89ea..bc17c51c4 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 @@ -14,6 +14,7 @@ public interface PlaylistLocalItem extends LocalItem { static List merge( final List localPlaylists, final List remotePlaylists) { + // todo: merge algorithm final List items = new ArrayList<>( localPlaylists.size() + remotePlaylists.size()); items.addAll(localPlaylists); diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java index a13894030..29bad45dc 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java @@ -2,6 +2,7 @@ package org.schabi.newpipe.database.playlist; import androidx.room.ColumnInfo; +import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_DISPLAY_INDEX; 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_THUMBNAIL_URL; @@ -15,14 +16,17 @@ public class PlaylistMetadataEntry implements PlaylistLocalItem { public final String name; @ColumnInfo(name = PLAYLIST_THUMBNAIL_URL) public final String thumbnailUrl; + @ColumnInfo(name = PLAYLIST_DISPLAY_INDEX) + public final long displayIndex; @ColumnInfo(name = PLAYLIST_STREAM_COUNT) public final long streamCount; public PlaylistMetadataEntry(final long uid, final String name, final String thumbnailUrl, - final long streamCount) { + final long displayIndex, final long streamCount) { this.uid = uid; this.name = name; this.thumbnailUrl = thumbnailUrl; + this.displayIndex = displayIndex; this.streamCount = streamCount; } 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 4941d9395..3fb96a21f 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 @@ -15,6 +15,7 @@ import java.util.List; import io.reactivex.rxjava3.core.Flowable; import static org.schabi.newpipe.database.playlist.PlaylistMetadataEntry.PLAYLIST_STREAM_COUNT; +import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_DISPLAY_INDEX; 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; @@ -75,6 +76,7 @@ public interface PlaylistStreamDAO extends BasicDAO { @Transaction @Query("SELECT " + PLAYLIST_ID + ", " + PLAYLIST_NAME + ", " + PLAYLIST_THUMBNAIL_URL + ", " + + PLAYLIST_DISPLAY_INDEX + ", " + "COALESCE(COUNT(" + JOIN_PLAYLIST_ID + "), 0) AS " + PLAYLIST_STREAM_COUNT + " FROM " + PLAYLIST_TABLE diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java index 2e9a15d7d..1fddfa732 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java @@ -31,6 +31,7 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { public static final String REMOTE_PLAYLIST_URL = "url"; public static final String REMOTE_PLAYLIST_THUMBNAIL_URL = "thumbnail_url"; public static final String REMOTE_PLAYLIST_UPLOADER_NAME = "uploader"; + public static final String REMOTE_PLAYLIST_DISPLAY_INDEX = "display_index"; public static final String REMOTE_PLAYLIST_STREAM_COUNT = "stream_count"; @PrimaryKey(autoGenerate = true) @@ -52,6 +53,9 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { @ColumnInfo(name = REMOTE_PLAYLIST_UPLOADER_NAME) private String uploader; + @ColumnInfo(name = REMOTE_PLAYLIST_DISPLAY_INDEX) + private long displayIndex; + @ColumnInfo(name = REMOTE_PLAYLIST_STREAM_COUNT) private Long streamCount; @@ -66,6 +70,19 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { this.streamCount = streamCount; } + @Ignore + public PlaylistRemoteEntity(final int serviceId, final String name, final String url, + final String thumbnailUrl, final String uploader, + final long displayIndex, final Long streamCount) { + this.serviceId = serviceId; + this.name = name; + this.url = url; + this.thumbnailUrl = thumbnailUrl; + this.uploader = uploader; + this.displayIndex = displayIndex; + this.streamCount = streamCount; + } + @Ignore public PlaylistRemoteEntity(final PlaylistInfo info) { this(info.getServiceId(), info.getName(), info.getUrl(), @@ -136,6 +153,14 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { this.uploader = uploader; } + public long getDisplayIndex() { + return displayIndex; + } + + public void setDisplayIndex(final long displayIndex) { + this.displayIndex = displayIndex; + } + public Long getStreamCount() { return streamCount; } 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 f272a8831..2f36cbd55 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 @@ -308,7 +308,6 @@ public final class BookmarkFragment extends BaseLocalListFragment { /*Do nothing on success*/ }, throwable -> showError( 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 33296aa84..aabda1bf0 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 @@ -96,12 +96,17 @@ public class LocalPlaylistManager { } public Maybe renamePlaylist(final long playlistId, final String name) { - return modifyPlaylist(playlistId, name, null); + return modifyPlaylist(playlistId, name, null, -1); } public Maybe changePlaylistThumbnail(final long playlistId, final String thumbnailUrl) { - return modifyPlaylist(playlistId, null, thumbnailUrl); + return modifyPlaylist(playlistId, null, thumbnailUrl, -1); + } + + public Maybe changePlaylistDisplayIndex(final long playlistId, + final long displayIndex) { + return modifyPlaylist(playlistId, null, null, displayIndex); } public String getPlaylistThumbnail(final long playlistId) { @@ -110,7 +115,8 @@ public class LocalPlaylistManager { private Maybe modifyPlaylist(final long playlistId, @Nullable final String name, - @Nullable final String thumbnailUrl) { + @Nullable final String thumbnailUrl, + final long displayIndex) { return playlistTable.getPlaylist(playlistId) .firstElement() .filter(playlistEntities -> !playlistEntities.isEmpty()) @@ -122,6 +128,9 @@ public class LocalPlaylistManager { if (thumbnailUrl != null) { playlist.setThumbnailUrl(thumbnailUrl); } + if (displayIndex != -1) { + playlist.setDisplayIndex(displayIndex); + } return playlistTable.update(playlist); }).subscribeOn(Schedulers.io()); } From 270a541a7c98e975d6c848fce9ee232b6620432b Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Wed, 13 Apr 2022 22:46:24 +0800 Subject: [PATCH 03/24] Implement algorithm to merge playlists --- .../database/playlist/PlaylistLocalItem.java | 58 ++++++++++++++++--- .../playlist/PlaylistMetadataEntry.java | 5 ++ .../playlist/model/PlaylistRemoteEntity.java | 1 + 3 files changed, 57 insertions(+), 7 deletions(-) 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 bc17c51c4..ae81ce3f5 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 @@ -11,18 +11,62 @@ import java.util.List; public interface PlaylistLocalItem extends LocalItem { String getOrderingName(); + long getDisplayIndex(); + static List merge( final List localPlaylists, final List remotePlaylists) { - // todo: merge algorithm - final List items = new ArrayList<>( + + // Merge localPlaylists and remotePlaylists by displayIndex. + // If two items have the same displayIndex, sort them in CASE_INSENSITIVE_ORDER. + // This algorithm is similar to the merge operation in merge sort. + + final List result = new ArrayList<>( localPlaylists.size() + remotePlaylists.size()); - items.addAll(localPlaylists); - items.addAll(remotePlaylists); + 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++; + } + addItemsWithSameIndex(result, itemsWithSameIndex); - Collections.sort(items, Comparator.comparing(PlaylistLocalItem::getOrderingName, - Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER))); + // If displayIndex does not match actual index, update displayIndex. + // This may happen when a new list is created with default displayIndex = 0. + // todo: update displayIndex - return items; + 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 displayIndex, + // 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/PlaylistMetadataEntry.java b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java index 29bad45dc..f54ffff13 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java @@ -39,4 +39,9 @@ public class PlaylistMetadataEntry implements PlaylistLocalItem { public String getOrderingName() { return name; } + + @Override + public long getDisplayIndex() { + return displayIndex; + } } diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java index 1fddfa732..454526769 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java @@ -153,6 +153,7 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { this.uploader = uploader; } + @Override public long getDisplayIndex() { return displayIndex; } From ba8370bcfd980089ef5984b6ca55e4b072a69fee Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Thu, 14 Apr 2022 12:13:42 +0800 Subject: [PATCH 04/24] Save changes to the database and bugfix --- .../database/playlist/PlaylistLocalItem.java | 14 ++-- .../playlist/dao/PlaylistRemoteDAO.java | 4 ++ .../playlist/model/PlaylistEntity.java | 6 +- .../newpipe/local/LocalItemListAdapter.java | 1 + .../local/bookmark/BookmarkFragment.java | 65 ++++++++++++++++++- .../local/playlist/LocalPlaylistManager.java | 5 +- .../local/playlist/RemotePlaylistManager.java | 15 +++++ 7 files changed, 100 insertions(+), 10 deletions(-) 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 ae81ce3f5..5bf50cd97 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 @@ -24,6 +24,12 @@ public interface PlaylistLocalItem extends LocalItem { final List result = new ArrayList<>( localPlaylists.size() + remotePlaylists.size()); final List itemsWithSameIndex = new ArrayList<>(); + + // The data from database may not be in the displayIndex order + Collections.sort(localPlaylists, + Comparator.comparingLong(PlaylistMetadataEntry::getDisplayIndex)); + Collections.sort(remotePlaylists, + Comparator.comparingLong(PlaylistRemoteEntity::getDisplayIndex)); int i = 0; int j = 0; while (i < localPlaylists.size()) { @@ -41,10 +47,6 @@ public interface PlaylistLocalItem extends LocalItem { } addItemsWithSameIndex(result, itemsWithSameIndex); - // If displayIndex does not match actual index, update displayIndex. - // This may happen when a new list is created with default displayIndex = 0. - // todo: update displayIndex - return result; } @@ -52,8 +54,8 @@ public interface PlaylistLocalItem extends LocalItem { final List itemsWithSameIndex) { if (!itemsWithSameIndex.isEmpty() && itemsWithSameIndex.get(0).getDisplayIndex() != item.getDisplayIndex()) { - // The new item has a different displayIndex, - // add previous items with same index to the result. + // The new item has a different displayIndex, add previous items with same + // index to the result. addItemsWithSameIndex(result, itemsWithSameIndex); itemsWithSameIndex.clear(); } 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 6bb849428..ade857464 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 @@ -31,6 +31,10 @@ public interface PlaylistRemoteDAO extends BasicDAO { + " WHERE " + REMOTE_PLAYLIST_SERVICE_ID + " = :serviceId") Flowable> listByService(int serviceId); + @Query("SELECT * FROM " + REMOTE_PLAYLIST_TABLE + " WHERE " + + REMOTE_PLAYLIST_ID + " = :playlistId") + Flowable> getPlaylist(long playlistId); + @Query("SELECT * FROM " + REMOTE_PLAYLIST_TABLE + " WHERE " + REMOTE_PLAYLIST_URL + " = :url AND " + REMOTE_PLAYLIST_SERVICE_ID + " = :serviceId") Flowable> getPlaylist(long serviceId, String url); diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java index c1ae0a2b3..82f697ab3 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java @@ -2,6 +2,7 @@ package org.schabi.newpipe.database.playlist.model; import androidx.room.ColumnInfo; import androidx.room.Entity; +import androidx.room.Ignore; import androidx.room.Index; import androidx.room.PrimaryKey; @@ -28,11 +29,12 @@ public class PlaylistEntity { private String thumbnailUrl; @ColumnInfo(name = PLAYLIST_DISPLAY_INDEX) - private long displayIndex = 0; + private long displayIndex; - public PlaylistEntity(final String name, final String thumbnailUrl) { + public PlaylistEntity(final String name, final String thumbnailUrl, final long displayIndex) { this.name = name; this.thumbnailUrl = thumbnailUrl; + this.displayIndex = displayIndex; } public long getUid() { diff --git a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java index 05e2fdac0..e2bfd5977 100644 --- a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java +++ b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java @@ -142,6 +142,7 @@ public class LocalItemListAdapter extends RecyclerView.Adapter subscriptions) { + + // If displayIndex does not match actual index, update displayIndex. + // This may happen when a new list is created + // or on the first run after database update + // or displayIndex is not continuous for some reason. + checkDisplayIndexUpdate(subscriptions); + handleResult(subscriptions); if (databaseSubscription != null) { databaseSubscription.request(1); @@ -212,7 +219,8 @@ public final class BookmarkFragment extends BaseLocalListFragment { /*Do nothing on success*/ }, throwable -> showError( + new ErrorInfo(throwable, + UserAction.REQUESTED_BOOKMARK, + "Changing local playlist display_index"))); + disposables.add(disposable); + } + + private void changeRemotePlaylistDisplayIndex(final long id, final long displayIndex) { + + if (remotePlaylistManager == null) { + return; + } + + if (DEBUG) { + Log.d(TAG, "Updating remote playlist id=[" + id + "] " + + "with new display_index=[" + displayIndex + "]"); + } + + final Disposable disposable = + remotePlaylistManager.changePlaylistDisplayIndex(id, displayIndex) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(longs -> { /*Do nothing on success*/ }, throwable -> showError( + new ErrorInfo(throwable, + UserAction.REQUESTED_BOOKMARK, + "Changing remote playlist display_index"))); + disposables.add(disposable); + } + + private void checkDisplayIndexUpdate(@NonNull final List result) { + for (int i = 0; i < result.size(); i++) { + final PlaylistLocalItem item = result.get(i); + if (item.getDisplayIndex() != i) { + if (item instanceof PlaylistMetadataEntry) { + changeLocalPlaylistDisplayIndex(((PlaylistMetadataEntry) item).uid, i); + } else if (item instanceof PlaylistRemoteEntity) { + changeRemotePlaylistDisplayIndex(((PlaylistRemoteEntity) item).getUid(), i); + } + } + } + } } 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 aabda1bf0..47817f9e4 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 @@ -40,8 +40,11 @@ public class LocalPlaylistManager { return Maybe.empty(); } final StreamEntity defaultStream = streams.get(0); + + // Make sure the new playlist is always on the top of bookmark. + // The index will be reassigned to non-negative number in BookmarkFragment. final PlaylistEntity newPlaylist = - new PlaylistEntity(name, defaultStream.getThumbnailUrl()); + new PlaylistEntity(name, defaultStream.getThumbnailUrl(), -1); return Maybe.fromCallable(() -> database.runInTransaction(() -> upsertStreams(playlistTable.insert(newPlaylist), streams, 0)) 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 5221139e3..b49f149d6 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 @@ -8,6 +8,7 @@ import org.schabi.newpipe.extractor.playlist.PlaylistInfo; import java.util.List; import io.reactivex.rxjava3.core.Flowable; +import io.reactivex.rxjava3.core.Maybe; import io.reactivex.rxjava3.core.Single; import io.reactivex.rxjava3.schedulers.Schedulers; @@ -33,6 +34,20 @@ public class RemotePlaylistManager { .subscribeOn(Schedulers.io()); } + public Maybe changePlaylistDisplayIndex(final long playlistId, + final long displayIndex) { + return playlistRemoteTable.getPlaylist(playlistId) + .firstElement() + .filter(playlistRemoteEntities -> !playlistRemoteEntities.isEmpty()) + .map(playlistRemoteEntities -> { + final PlaylistRemoteEntity playlist = playlistRemoteEntities.get(0); + if (displayIndex != -1) { + playlist.setDisplayIndex(displayIndex); + } + return playlistRemoteTable.update(playlist); + }).subscribeOn(Schedulers.io()); + } + public Single onBookmark(final PlaylistInfo playlistInfo) { return Single.fromCallable(() -> { final PlaylistRemoteEntity playlist = new PlaylistRemoteEntity(playlistInfo); From bfb56b4144f1fec9680dbafd5fe79420d731e0a7 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Thu, 14 Apr 2022 16:59:52 +0800 Subject: [PATCH 05/24] UI design and behavior --- .../playlist/model/PlaylistEntity.java | 1 - .../local/bookmark/BookmarkFragment.java | 207 +++++++++++++----- .../local/holder/LocalPlaylistItemHolder.java | 21 +- .../holder/RemotePlaylistItemHolder.java | 22 +- .../layout/list_playlist_bookmark_item.xml | 84 +++++++ 5 files changed, 282 insertions(+), 53 deletions(-) create mode 100644 app/src/main/res/layout/list_playlist_bookmark_item.xml diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java index 82f697ab3..cdbbdebc0 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java @@ -2,7 +2,6 @@ package org.schabi.newpipe.database.playlist.model; import androidx.room.ColumnInfo; import androidx.room.Entity; -import androidx.room.Ignore; import androidx.room.Index; import androidx.room.PrimaryKey; 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 6d2fbfbfc..63c63c1e0 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,7 @@ package org.schabi.newpipe.local.bookmark; +import static org.schabi.newpipe.util.ThemeHelper.shouldUseGridLayout; + import android.os.Bundle; import android.os.Parcelable; import android.text.InputType; @@ -12,6 +14,8 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.appcompat.app.AlertDialog; import androidx.fragment.app.FragmentManager; +import androidx.recyclerview.widget.ItemTouchHelper; +import androidx.recyclerview.widget.RecyclerView; import org.reactivestreams.Subscriber; import org.reactivestreams.Subscription; @@ -41,6 +45,7 @@ import io.reactivex.rxjava3.disposables.CompositeDisposable; import io.reactivex.rxjava3.disposables.Disposable; public final class BookmarkFragment extends BaseLocalListFragment, Void> { + private static final int MINIMUM_INITIAL_DRAG_VELOCITY = 12; @State protected Parcelable itemsListState; @@ -48,6 +53,7 @@ public final class BookmarkFragment extends BaseLocalListFragment() { @Override public void selected(final LocalItem selectedItem) { @@ -126,6 +135,14 @@ public final class BookmarkFragment extends BaseLocalListFragment - changeLocalPlaylistName( - selectedItem.uid, - dialogBinding.dialogEditText.getText().toString())) - .setNegativeButton(R.string.cancel, null) - .setNeutralButton(R.string.delete, (dialog, which) -> { - showDeleteDialog(selectedItem.name, - localPlaylistManager.deletePlaylist(selectedItem.uid)); - dialog.dismiss(); - }) - .create() - .show(); - } - - private void showDeleteDialog(final String name, final Single deleteReactor) { - if (activity == null || disposables == null) { - return; - } - - new AlertDialog.Builder(activity) - .setTitle(name) - .setMessage(R.string.delete_playlist_prompt) - .setCancelable(true) - .setPositiveButton(R.string.delete, (dialog, i) -> - disposables.add(deleteReactor - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(ignored -> { /*Do nothing on success*/ }, throwable -> - showError(new ErrorInfo(throwable, - UserAction.REQUESTED_BOOKMARK, - "Deleting playlist"))))) - .setNegativeButton(R.string.cancel, null) - .show(); - } + /*////////////////////////////////////////////////////////////////////////// + // Playlist Metadata Manipulation + //////////////////////////////////////////////////////////////////////////*/ private void changeLocalPlaylistName(final long id, final String name) { if (localPlaylistManager == null) { @@ -379,5 +350,141 @@ public final class BookmarkFragment extends BaseLocalListFragment items = itemListAdapter.getItemsList(); + for (int i = 0; i < items.size(); i++) { + final LocalItem item = items.get(i); + if (item instanceof PlaylistMetadataEntry) { + changeLocalPlaylistDisplayIndex(((PlaylistMetadataEntry) item).uid, i); + } else if (item instanceof PlaylistRemoteEntity) { + changeLocalPlaylistDisplayIndex(((PlaylistRemoteEntity) item).getUid(), i); + } + } + } + + private ItemTouchHelper.SimpleCallback getItemTouchCallback() { + int directions = ItemTouchHelper.UP | ItemTouchHelper.DOWN; + if (shouldUseGridLayout(requireContext())) { + directions |= ItemTouchHelper.LEFT | ItemTouchHelper.RIGHT; + } + return new ItemTouchHelper.SimpleCallback(directions, + ItemTouchHelper.ACTION_STATE_IDLE) { + @Override + public int interpolateOutOfBoundsScroll(@NonNull final RecyclerView recyclerView, + final int viewSize, + final int viewSizeOutOfBounds, + final int totalSize, + final long msSinceStartScroll) { + final int standardSpeed = super.interpolateOutOfBoundsScroll(recyclerView, + viewSize, viewSizeOutOfBounds, totalSize, msSinceStartScroll); + final int minimumAbsVelocity = Math.max(MINIMUM_INITIAL_DRAG_VELOCITY, + Math.abs(standardSpeed)); + return minimumAbsVelocity * (int) Math.signum(viewSizeOutOfBounds); + } + + @Override + public boolean onMove(@NonNull final RecyclerView recyclerView, + @NonNull final RecyclerView.ViewHolder source, + @NonNull final RecyclerView.ViewHolder target) { + if (source.getItemViewType() != target.getItemViewType() + || itemListAdapter == null) { + return false; + } + + // todo: is it correct + final int sourceIndex = source.getBindingAdapterPosition(); + final int targetIndex = target.getBindingAdapterPosition(); + final boolean isSwapped = itemListAdapter.swapItems(sourceIndex, targetIndex); + if (isSwapped) { + // todo + //saveChanges(); + saveImmediate(); + } + return isSwapped; + } + + @Override + public boolean isLongPressDragEnabled() { + return false; + } + + @Override + public boolean isItemViewSwipeEnabled() { + return false; + } + + @Override + public void onSwiped(@NonNull final RecyclerView.ViewHolder viewHolder, + final int swipeDir) { + } + }; + } + + /////////////////////////////////////////////////////////////////////////// + // Utils + /////////////////////////////////////////////////////////////////////////// + + private void showRemoteDeleteDialog(final PlaylistRemoteEntity item) { + showDeleteDialog(item.getName(), remotePlaylistManager.deletePlaylist(item.getUid())); + } + + private void showLocalDialog(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.name); + + final AlertDialog.Builder builder = new AlertDialog.Builder(activity); + builder.setView(dialogBinding.getRoot()) + .setPositiveButton(R.string.rename_playlist, (dialog, which) -> + changeLocalPlaylistName( + selectedItem.uid, + dialogBinding.dialogEditText.getText().toString())) + .setNegativeButton(R.string.cancel, null) + .setNeutralButton(R.string.delete, (dialog, which) -> { + showDeleteDialog(selectedItem.name, + localPlaylistManager.deletePlaylist(selectedItem.uid)); + dialog.dismiss(); + }) + .create() + .show(); + } + + private void showDeleteDialog(final String name, final Single deleteReactor) { + if (activity == null || disposables == null) { + return; + } + + new AlertDialog.Builder(activity) + .setTitle(name) + .setMessage(R.string.delete_playlist_prompt) + .setCancelable(true) + .setPositiveButton(R.string.delete, (dialog, i) -> + disposables.add(deleteReactor + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(ignored -> { /*Do nothing on success*/ }, throwable -> + showError(new ErrorInfo(throwable, + UserAction.REQUESTED_BOOKMARK, + "Deleting playlist"))))) + .setNegativeButton(R.string.cancel, null) + .show(); + } } diff --git a/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java b/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java index f8c5176ec..57a944709 100644 --- a/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java +++ b/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java @@ -1,8 +1,10 @@ package org.schabi.newpipe.local.holder; +import android.view.MotionEvent; import android.view.View; import android.view.ViewGroup; +import org.schabi.newpipe.R; import org.schabi.newpipe.database.LocalItem; import org.schabi.newpipe.database.playlist.PlaylistMetadataEntry; import org.schabi.newpipe.local.LocalItemBuilder; @@ -13,13 +15,16 @@ import org.schabi.newpipe.util.Localization; import java.time.format.DateTimeFormatter; public class LocalPlaylistItemHolder extends PlaylistItemHolder { + private final View itemHandleView; + public LocalPlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final ViewGroup parent) { - super(infoItemBuilder, parent); + this(infoItemBuilder, R.layout.list_playlist_bookmark_item, parent); } LocalPlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final int layoutId, final ViewGroup parent) { super(infoItemBuilder, layoutId, parent); + itemHandleView = itemView.findViewById(R.id.itemHandle); } @Override @@ -38,6 +43,20 @@ public class LocalPlaylistItemHolder extends PlaylistItemHolder { PicassoHelper.loadPlaylistThumbnail(item.thumbnailUrl).into(itemThumbnailView); + itemHandleView.setOnTouchListener(getOnTouchListener(item)); + super.updateFromItem(localItem, historyRecordManager, dateTimeFormatter); } + + private View.OnTouchListener getOnTouchListener(final PlaylistMetadataEntry item) { + return (view, motionEvent) -> { + view.performClick(); + if (itemBuilder != null && itemBuilder.getOnItemSelectedListener() != null + && motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { + itemBuilder.getOnItemSelectedListener().drag(item, + LocalPlaylistItemHolder.this); + } + return false; + }; + } } diff --git a/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java b/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java index 440353ac7..9ecfa6979 100644 --- a/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java +++ b/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java @@ -1,8 +1,11 @@ package org.schabi.newpipe.local.holder; import android.text.TextUtils; +import android.view.MotionEvent; +import android.view.View; import android.view.ViewGroup; +import org.schabi.newpipe.R; import org.schabi.newpipe.database.LocalItem; import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity; import org.schabi.newpipe.extractor.NewPipe; @@ -14,14 +17,17 @@ import org.schabi.newpipe.util.Localization; import java.time.format.DateTimeFormatter; public class RemotePlaylistItemHolder extends PlaylistItemHolder { + private final View itemHandleView; + public RemotePlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final ViewGroup parent) { - super(infoItemBuilder, parent); + this(infoItemBuilder, R.layout.list_playlist_bookmark_item, parent); } RemotePlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final int layoutId, final ViewGroup parent) { super(infoItemBuilder, layoutId, parent); + itemHandleView = itemView.findViewById(R.id.itemHandle); } @Override @@ -46,6 +52,20 @@ public class RemotePlaylistItemHolder extends PlaylistItemHolder { PicassoHelper.loadPlaylistThumbnail(item.getThumbnailUrl()).into(itemThumbnailView); + itemHandleView.setOnTouchListener(getOnTouchListener(item)); + super.updateFromItem(localItem, historyRecordManager, dateTimeFormatter); } + + private View.OnTouchListener getOnTouchListener(final PlaylistRemoteEntity item) { + return (view, motionEvent) -> { + view.performClick(); + if (itemBuilder != null && itemBuilder.getOnItemSelectedListener() != null + && motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { + itemBuilder.getOnItemSelectedListener().drag(item, + RemotePlaylistItemHolder.this); + } + return false; + }; + } } diff --git a/app/src/main/res/layout/list_playlist_bookmark_item.xml b/app/src/main/res/layout/list_playlist_bookmark_item.xml new file mode 100644 index 000000000..642ea2949 --- /dev/null +++ b/app/src/main/res/layout/list_playlist_bookmark_item.xml @@ -0,0 +1,84 @@ + + + + + + + + + + + + + + From 3c4882569931f91de0fc5dd0a980739781d6526c Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Fri, 15 Apr 2022 20:44:54 +0800 Subject: [PATCH 06/24] Debounced saver & bugfix & clean code --- .../database/playlist/PlaylistLocalItem.java | 4 + .../playlist/PlaylistMetadataEntry.java | 2 +- .../database/playlist/dao/PlaylistDAO.java | 14 + .../playlist/dao/PlaylistStreamDAO.java | 14 +- .../playlist/model/PlaylistEntity.java | 11 + .../playlist/model/PlaylistRemoteEntity.java | 2 +- .../newpipe/local/LocalItemListAdapter.java | 1 - .../local/bookmark/BookmarkFragment.java | 286 +++++++++++++----- .../local/dialog/PlaylistAppendDialog.java | 2 +- .../local/playlist/LocalPlaylistManager.java | 24 +- .../local/playlist/RemotePlaylistManager.java | 26 +- 11 files changed, 288 insertions(+), 98 deletions(-) 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 5bf50cd97..47c6dd617 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 @@ -45,6 +45,10 @@ public interface PlaylistLocalItem extends LocalItem { addItem(result, localPlaylists.get(i), itemsWithSameIndex); i++; } + while (j < remotePlaylists.size()) { + addItem(result, remotePlaylists.get(j), itemsWithSameIndex); + j++; + } addItemsWithSameIndex(result, itemsWithSameIndex); return result; diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java index f54ffff13..ff80049a3 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java @@ -17,7 +17,7 @@ public class PlaylistMetadataEntry implements PlaylistLocalItem { @ColumnInfo(name = PLAYLIST_THUMBNAIL_URL) public final String thumbnailUrl; @ColumnInfo(name = PLAYLIST_DISPLAY_INDEX) - public final long displayIndex; + public long displayIndex; @ColumnInfo(name = PLAYLIST_STREAM_COUNT) public final long streamCount; diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistDAO.java b/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistDAO.java index 70aaa3b2d..d8071e0af 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistDAO.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistDAO.java @@ -2,6 +2,7 @@ package org.schabi.newpipe.database.playlist.dao; import androidx.room.Dao; import androidx.room.Query; +import androidx.room.Transaction; import org.schabi.newpipe.database.BasicDAO; import org.schabi.newpipe.database.playlist.model.PlaylistEntity; @@ -36,4 +37,17 @@ public interface PlaylistDAO extends BasicDAO { @Query("SELECT COUNT(*) FROM " + PLAYLIST_TABLE) Flowable getCount(); + + @Transaction + default long upsertPlaylist(final PlaylistEntity playlist) { + final long playlistId = playlist.getUid(); + + if (playlistId == -1) { + // This situation is probably impossible. + return insert(playlist); + } else { + update(playlist); + return playlistId; + } + } } 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 3fb96a21f..0fce984f3 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 @@ -82,7 +82,19 @@ public interface PlaylistStreamDAO extends BasicDAO { + " FROM " + PLAYLIST_TABLE + " LEFT JOIN " + PLAYLIST_STREAM_JOIN_TABLE + " ON " + PLAYLIST_ID + " = " + JOIN_PLAYLIST_ID - + " GROUP BY " + 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_URL + ", " + + PLAYLIST_DISPLAY_INDEX + ", " + + "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 + + " GROUP BY " + PLAYLIST_ID + + " ORDER BY " + PLAYLIST_DISPLAY_INDEX) + Flowable> getDisplayIndexOrderedPlaylistMetadata(); } diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java index cdbbdebc0..272e8a5bc 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java @@ -2,12 +2,15 @@ package org.schabi.newpipe.database.playlist.model; import androidx.room.ColumnInfo; import androidx.room.Entity; +import androidx.room.Ignore; import androidx.room.Index; import androidx.room.PrimaryKey; 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.database.playlist.PlaylistMetadataEntry; + @Entity(tableName = PLAYLIST_TABLE, indices = {@Index(value = {PLAYLIST_NAME})}) public class PlaylistEntity { @@ -36,6 +39,14 @@ public class PlaylistEntity { this.displayIndex = displayIndex; } + @Ignore + public PlaylistEntity(final PlaylistMetadataEntry item) { + this.uid = item.uid; + this.name = item.name; + this.thumbnailUrl = item.thumbnailUrl; + this.displayIndex = item.displayIndex; + } + public long getUid() { return uid; } diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java index 454526769..adea2738b 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java @@ -54,7 +54,7 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { private String uploader; @ColumnInfo(name = REMOTE_PLAYLIST_DISPLAY_INDEX) - private long displayIndex; + private long displayIndex = -1; // Make sure the new item is on the top @ColumnInfo(name = REMOTE_PLAYLIST_STREAM_COUNT) private Long streamCount; diff --git a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java index e2bfd5977..05e2fdac0 100644 --- a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java +++ b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java @@ -142,7 +142,6 @@ public class LocalItemListAdapter extends RecyclerView.Adapter, Void> { + // todo: add to playlists, item handle should be invisible + + // Save the list 10s after the last change occurred + private static final long SAVE_DEBOUNCE_MILLIS = 10000; private static final int MINIMUM_INITIAL_DRAG_VELOCITY = 12; @State protected Parcelable itemsListState; @@ -55,6 +67,16 @@ public final class BookmarkFragment extends BaseLocalListFragment debouncedSaveSignal; + + /* Has the playlist been fully loaded from db */ + private AtomicBoolean isLoadingComplete; + /* Has the playlist been modified (e.g. items reordered or deleted) */ + private AtomicBoolean isModified; + + // Map from (uid, local/remote item) to the saved display index in the database. + private Map, Long> displayIndexInDatabase; + /////////////////////////////////////////////////////////////////////////// // Fragment LifeCycle - Creation /////////////////////////////////////////////////////////////////////////// @@ -69,6 +91,12 @@ public final class BookmarkFragment extends BaseLocalListFragment(); } @Nullable @@ -154,6 +182,10 @@ public final class BookmarkFragment extends BaseLocalListFragment subscriptions) { - - // If displayIndex does not match actual index, update displayIndex. - // This may happen when a new list is created - // or on the first run after database update - // or displayIndex is not continuous for some reason. - checkDisplayIndexUpdate(subscriptions); - - handleResult(subscriptions); + if (isModified == null || !isModified.get()) { + checkDisplayIndexModified(subscriptions); + handleResult(subscriptions); + isLoadingComplete.set(true); + } if (databaseSubscription != null) { databaseSubscription.request(1); } @@ -296,86 +338,170 @@ public final class BookmarkFragment extends BaseLocalListFragment result) { + if (isModified != null && isModified.get()) { return; } - if (DEBUG) { - Log.d(TAG, "Updating local playlist id=[" + id + "] " - + "with new display_index=[" + displayIndex + "]"); - } + displayIndexInDatabase.clear(); - final Disposable disposable = - localPlaylistManager.changePlaylistDisplayIndex(id, displayIndex) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(longs -> { /*Do nothing on success*/ }, throwable -> showError( - new ErrorInfo(throwable, - UserAction.REQUESTED_BOOKMARK, - "Changing local playlist display_index"))); - disposables.add(disposable); - } - - private void changeRemotePlaylistDisplayIndex(final long id, final long displayIndex) { - - if (remotePlaylistManager == null) { - return; - } - - if (DEBUG) { - Log.d(TAG, "Updating remote playlist id=[" + id + "] " - + "with new display_index=[" + displayIndex + "]"); - } - - final Disposable disposable = - remotePlaylistManager.changePlaylistDisplayIndex(id, displayIndex) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(longs -> { /*Do nothing on success*/ }, throwable -> showError( - new ErrorInfo(throwable, - UserAction.REQUESTED_BOOKMARK, - "Changing remote playlist display_index"))); - disposables.add(disposable); - } - - private void checkDisplayIndexUpdate(@NonNull final List result) { + // If the display index does not match actual index in the list, update the display index. + // This may happen when a new list is created + // or on the first run after database update + // or displayIndex is not continuous for some reason. + boolean isDisplayIndexModified = false; for (int i = 0; i < result.size(); i++) { final PlaylistLocalItem item = result.get(i); if (item.getDisplayIndex() != i) { - if (item instanceof PlaylistMetadataEntry) { - changeLocalPlaylistDisplayIndex(((PlaylistMetadataEntry) item).uid, i); - } else if (item instanceof PlaylistRemoteEntity) { - changeRemotePlaylistDisplayIndex(((PlaylistRemoteEntity) item).getUid(), i); - } + isDisplayIndexModified = true; } + + // Updating display index in the item does not affect the value inserts into + // database, which will be recalculated during the database update. Updating + // display index in the item here is to determine whether it is recently modified. + // Save the index read from the database. + if (item instanceof PlaylistMetadataEntry) { + + displayIndexInDatabase.put(new Pair<>(((PlaylistMetadataEntry) item).uid, + LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM), item.getDisplayIndex()); + ((PlaylistMetadataEntry) item).displayIndex = i; + + } else if (item instanceof PlaylistRemoteEntity) { + + displayIndexInDatabase.put(new Pair<>(((PlaylistRemoteEntity) item).getUid(), + LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM), + item.getDisplayIndex()); + ((PlaylistRemoteEntity) item).setDisplayIndex(i); + + } + } + + if (isDisplayIndexModified) { + saveChanges(); } } - private void saveImmediate() { - if (localPlaylistManager == null || remotePlaylistManager == null - || itemListAdapter == null) { + private void saveChanges() { + if (isModified == null || debouncedSaveSignal == null) { return; } - // todo: debounce - /* + + isModified.set(true); + debouncedSaveSignal.onNext(System.currentTimeMillis()); + } + + private Disposable getDebouncedSaver() { + if (debouncedSaveSignal == null) { + return Disposable.empty(); + } + + return debouncedSaveSignal + .debounce(SAVE_DEBOUNCE_MILLIS, TimeUnit.MILLISECONDS) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(ignored -> saveImmediate(), throwable -> + showError(new ErrorInfo(throwable, UserAction.SOMETHING_ELSE, + "Debounced saver"))); + } + + private void saveImmediate() { + if (itemListAdapter == null) { + return; + } + // List must be loaded and modified in order to save if (isLoadingComplete == null || isModified == null || !isLoadingComplete.get() || !isModified.get()) { - Log.w(TAG, "Attempting to save playlist when local playlist " - + "is not loaded or not modified: playlist id=[" + playlistId + "]"); + Log.w(TAG, "Attempting to save playlists in bookmark when bookmark " + + "is not loaded or playlists not modified"); return; } - */ - // todo: is it correct? + final List items = itemListAdapter.getItemsList(); + final List localItemsUpdate = new ArrayList<>(); + final List localItemsDeleteUid = new ArrayList<>(); + final List remoteItemsUpdate = new ArrayList<>(); + final List remoteItemsDeleteUid = new ArrayList<>(); + + // Calculate display index for (int i = 0; i < items.size(); i++) { final LocalItem item = items.get(i); + if (item instanceof PlaylistMetadataEntry) { - changeLocalPlaylistDisplayIndex(((PlaylistMetadataEntry) item).uid, i); + ((PlaylistMetadataEntry) item).displayIndex = i; + + final Long uid = ((PlaylistMetadataEntry) item).uid; + final Pair key = new Pair<>(uid, + LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM); + final Long databaseIndex = displayIndexInDatabase.remove(key); + + if (databaseIndex != null) { + if (databaseIndex != i) { + localItemsUpdate.add((PlaylistMetadataEntry) item); + } + } else { + // This should be impossible. + continue; + } } else if (item instanceof PlaylistRemoteEntity) { - changeLocalPlaylistDisplayIndex(((PlaylistRemoteEntity) item).getUid(), i); + ((PlaylistRemoteEntity) item).setDisplayIndex(i); + + final Long uid = ((PlaylistRemoteEntity) item).getUid(); + final Pair key = new Pair<>(uid, + LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM); + final Long databaseIndex = displayIndexInDatabase.remove(key); + + if (databaseIndex != null) { + if (databaseIndex != i) { + remoteItemsUpdate.add((PlaylistRemoteEntity) item); + } + } else { + // This should be impossible. + continue; + } } } + + // Find deleted items + for (final Pair key : displayIndexInDatabase.keySet()) { + if (key.second.equals(LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM)) { + localItemsDeleteUid.add(key.first); + } else if (key.second.equals(LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM)) { + remoteItemsDeleteUid.add(key.first); + } + } + + displayIndexInDatabase.clear(); + + // 1. Update local playlists + // 2. Update remote playlists + // 3. Set isModified false + disposables.add(localPlaylistManager.updatePlaylists(localItemsUpdate, localItemsDeleteUid) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(() -> disposables.add(remotePlaylistManager.updatePlaylists( + remoteItemsUpdate, remoteItemsDeleteUid) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(() -> { + if (isModified != null) { + isModified.set(false); + } + }, + throwable -> showError(new ErrorInfo(throwable, + UserAction.REQUESTED_BOOKMARK, + "Saving playlist")) + )), + throwable -> showError(new ErrorInfo(throwable, + UserAction.REQUESTED_BOOKMARK, "Saving playlist")) + )); + } private ItemTouchHelper.SimpleCallback getItemTouchCallback() { @@ -404,17 +530,26 @@ public final class BookmarkFragment extends BaseLocalListFragment { - showDeleteDialog(selectedItem.name, - localPlaylistManager.deletePlaylist(selectedItem.uid)); + showDeleteDialog(selectedItem.name, selectedItem); dialog.dismiss(); }) .create() .show(); } - private void showDeleteDialog(final String name, final Single deleteReactor) { + private void showDeleteDialog(final String name, final PlaylistLocalItem item) { if (activity == null || disposables == null) { return; } @@ -476,13 +610,7 @@ public final class BookmarkFragment extends BaseLocalListFragment - disposables.add(deleteReactor - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(ignored -> { /*Do nothing on success*/ }, throwable -> - showError(new ErrorInfo(throwable, - UserAction.REQUESTED_BOOKMARK, - "Deleting playlist"))))) + .setPositiveButton(R.string.delete, (dialog, i) -> deleteItem(item)) .setNegativeButton(R.string.cancel, null) .show(); } diff --git a/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java b/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java index a874cdd62..58a10af22 100644 --- a/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java +++ b/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java @@ -85,7 +85,7 @@ public final class PlaylistAppendDialog extends PlaylistDialog { final View newPlaylistButton = view.findViewById(R.id.newPlaylist); newPlaylistButton.setOnClickListener(ignored -> openCreatePlaylistDialog()); - playlistDisposables.add(playlistManager.getPlaylists() + playlistDisposables.add(playlistManager.getDisplayIndexOrderedPlaylists() .observeOn(AndroidSchedulers.mainThread()) .subscribe(this::onPlaylistsReceived)); } 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 47817f9e4..c68a22b01 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 @@ -41,6 +41,7 @@ public class LocalPlaylistManager { } final StreamEntity defaultStream = streams.get(0); + // Save to the database directly. // Make sure the new playlist is always on the top of bookmark. // The index will be reassigned to non-negative number in BookmarkFragment. final PlaylistEntity newPlaylist = @@ -85,10 +86,31 @@ public class LocalPlaylistManager { })).subscribeOn(Schedulers.io()); } + public Completable updatePlaylists(final List updateItems, + final List deletedItems) { + final List items = new ArrayList<>(updateItems.size()); + for (final PlaylistMetadataEntry item : updateItems) { + items.add(new PlaylistEntity(item)); + } + return Completable.fromRunnable(() -> database.runInTransaction(() -> { + for (final Long uid: deletedItems) { + playlistTable.deletePlaylist(uid); + } + for (final PlaylistEntity item: items) { + playlistTable.upsertPlaylist(item); + } + })).subscribeOn(Schedulers.io()); + } + public Flowable> getPlaylists() { return playlistStreamTable.getPlaylistMetadata().subscribeOn(Schedulers.io()); } + public Flowable> getDisplayIndexOrderedPlaylists() { + return playlistStreamTable.getDisplayIndexOrderedPlaylistMetadata() + .subscribeOn(Schedulers.io()); + } + public Flowable> getPlaylistStreams(final long playlistId) { return playlistStreamTable.getOrderedStreamsOf(playlistId).subscribeOn(Schedulers.io()); } @@ -107,7 +129,7 @@ public class LocalPlaylistManager { return modifyPlaylist(playlistId, null, thumbnailUrl, -1); } - public Maybe changePlaylistDisplayIndex(final long playlistId, + public Maybe updatePlaylistDisplayIndex(final long playlistId, final long displayIndex) { return modifyPlaylist(playlistId, null, null, displayIndex); } 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 b49f149d6..1dbd726ae 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 @@ -7,16 +7,18 @@ import org.schabi.newpipe.extractor.playlist.PlaylistInfo; 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 RemotePlaylistManager { + private final AppDatabase database; private final PlaylistRemoteDAO playlistRemoteTable; public RemotePlaylistManager(final AppDatabase db) { + database = db; playlistRemoteTable = db.playlistRemoteDAO(); } @@ -34,18 +36,16 @@ public class RemotePlaylistManager { .subscribeOn(Schedulers.io()); } - public Maybe changePlaylistDisplayIndex(final long playlistId, - final long displayIndex) { - return playlistRemoteTable.getPlaylist(playlistId) - .firstElement() - .filter(playlistRemoteEntities -> !playlistRemoteEntities.isEmpty()) - .map(playlistRemoteEntities -> { - final PlaylistRemoteEntity playlist = playlistRemoteEntities.get(0); - if (displayIndex != -1) { - playlist.setDisplayIndex(displayIndex); - } - return playlistRemoteTable.update(playlist); - }).subscribeOn(Schedulers.io()); + public Completable updatePlaylists(final List updateItems, + final List deletedItems) { + return Completable.fromRunnable(() -> database.runInTransaction(() -> { + for (final Long uid: deletedItems) { + playlistRemoteTable.deletePlaylist(uid); + } + for (final PlaylistRemoteEntity item: updateItems) { + playlistRemoteTable.upsert(item); + } + })).subscribeOn(Schedulers.io()); } public Single onBookmark(final PlaylistInfo playlistInfo) { From 0aa08a5e4049952bc830522b3053d347199ee3f7 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Fri, 15 Apr 2022 23:19:24 +0800 Subject: [PATCH 07/24] Use new item holder --- .../newpipe/local/LocalItemListAdapter.java | 21 ++++-- .../local/bookmark/BookmarkFragment.java | 18 ++--- .../LocalBookmarkPlaylistItemHolder.java | 63 ++++++++++++++++ .../local/holder/LocalPlaylistItemHolder.java | 19 +---- .../RemoteBookmarkPlaylistItemHolder.java | 71 +++++++++++++++++++ .../holder/RemotePlaylistItemHolder.java | 20 +----- 6 files changed, 163 insertions(+), 49 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/local/holder/LocalBookmarkPlaylistItemHolder.java create mode 100644 app/src/main/java/org/schabi/newpipe/local/holder/RemoteBookmarkPlaylistItemHolder.java diff --git a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java index 05e2fdac0..161d35ee5 100644 --- a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java +++ b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java @@ -13,6 +13,7 @@ import androidx.recyclerview.widget.RecyclerView; import org.schabi.newpipe.database.LocalItem; import org.schabi.newpipe.database.stream.model.StreamStateEntity; import org.schabi.newpipe.local.history.HistoryRecordManager; +import org.schabi.newpipe.local.holder.LocalBookmarkPlaylistItemHolder; import org.schabi.newpipe.local.holder.LocalItemHolder; import org.schabi.newpipe.local.holder.LocalPlaylistGridItemHolder; import org.schabi.newpipe.local.holder.LocalPlaylistItemHolder; @@ -20,6 +21,7 @@ import org.schabi.newpipe.local.holder.LocalPlaylistStreamGridItemHolder; import org.schabi.newpipe.local.holder.LocalPlaylistStreamItemHolder; import org.schabi.newpipe.local.holder.LocalStatisticStreamGridItemHolder; import org.schabi.newpipe.local.holder.LocalStatisticStreamItemHolder; +import org.schabi.newpipe.local.holder.RemoteBookmarkPlaylistItemHolder; import org.schabi.newpipe.local.holder.RemotePlaylistGridItemHolder; import org.schabi.newpipe.local.holder.RemotePlaylistItemHolder; import org.schabi.newpipe.util.FallbackViewHolder; @@ -66,6 +68,8 @@ public class LocalItemListAdapter extends RecyclerView.Adapter localItems; @@ -74,6 +78,7 @@ public class LocalItemListAdapter extends RecyclerView.Adapter, Void> { - // todo: add to playlists, item handle should be invisible // Save the list 10s after the last change occurred private static final long SAVE_DEBOUNCE_MILLIS = 10000; @@ -126,6 +125,8 @@ public final class BookmarkFragment extends BaseLocalListFragment { + view.performClick(); + if (itemBuilder != null && itemBuilder.getOnItemSelectedListener() != null + && motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { + itemBuilder.getOnItemSelectedListener().drag(item, + LocalBookmarkPlaylistItemHolder.this); + } + return false; + }; + } +} diff --git a/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java b/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java index 57a944709..2cfc94463 100644 --- a/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java +++ b/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistItemHolder.java @@ -1,6 +1,5 @@ package org.schabi.newpipe.local.holder; -import android.view.MotionEvent; import android.view.View; import android.view.ViewGroup; @@ -15,16 +14,14 @@ import org.schabi.newpipe.util.Localization; import java.time.format.DateTimeFormatter; public class LocalPlaylistItemHolder extends PlaylistItemHolder { - private final View itemHandleView; public LocalPlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final ViewGroup parent) { - this(infoItemBuilder, R.layout.list_playlist_bookmark_item, parent); + this(infoItemBuilder, R.layout.list_playlist_mini_item, parent); } LocalPlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final int layoutId, final ViewGroup parent) { super(infoItemBuilder, layoutId, parent); - itemHandleView = itemView.findViewById(R.id.itemHandle); } @Override @@ -43,20 +40,6 @@ public class LocalPlaylistItemHolder extends PlaylistItemHolder { PicassoHelper.loadPlaylistThumbnail(item.thumbnailUrl).into(itemThumbnailView); - itemHandleView.setOnTouchListener(getOnTouchListener(item)); - super.updateFromItem(localItem, historyRecordManager, dateTimeFormatter); } - - private View.OnTouchListener getOnTouchListener(final PlaylistMetadataEntry item) { - return (view, motionEvent) -> { - view.performClick(); - if (itemBuilder != null && itemBuilder.getOnItemSelectedListener() != null - && motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { - itemBuilder.getOnItemSelectedListener().drag(item, - LocalPlaylistItemHolder.this); - } - return false; - }; - } } diff --git a/app/src/main/java/org/schabi/newpipe/local/holder/RemoteBookmarkPlaylistItemHolder.java b/app/src/main/java/org/schabi/newpipe/local/holder/RemoteBookmarkPlaylistItemHolder.java new file mode 100644 index 000000000..345223b08 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/local/holder/RemoteBookmarkPlaylistItemHolder.java @@ -0,0 +1,71 @@ +package org.schabi.newpipe.local.holder; + +import android.text.TextUtils; +import android.view.MotionEvent; +import android.view.View; +import android.view.ViewGroup; + +import org.schabi.newpipe.R; +import org.schabi.newpipe.database.LocalItem; +import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity; +import org.schabi.newpipe.extractor.NewPipe; +import org.schabi.newpipe.local.LocalItemBuilder; +import org.schabi.newpipe.local.history.HistoryRecordManager; +import org.schabi.newpipe.util.Localization; +import org.schabi.newpipe.util.PicassoHelper; + +import java.time.format.DateTimeFormatter; + +public class RemoteBookmarkPlaylistItemHolder extends PlaylistItemHolder { + private final View itemHandleView; + + public RemoteBookmarkPlaylistItemHolder(final LocalItemBuilder infoItemBuilder, + final ViewGroup parent) { + this(infoItemBuilder, R.layout.list_playlist_bookmark_item, parent); + } + + RemoteBookmarkPlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final int layoutId, + final ViewGroup parent) { + super(infoItemBuilder, layoutId, parent); + itemHandleView = itemView.findViewById(R.id.itemHandle); + } + + @Override + public void updateFromItem(final LocalItem localItem, + final HistoryRecordManager historyRecordManager, + final DateTimeFormatter dateTimeFormatter) { + if (!(localItem instanceof PlaylistRemoteEntity)) { + return; + } + final PlaylistRemoteEntity item = (PlaylistRemoteEntity) localItem; + + itemTitleView.setText(item.getName()); + itemStreamCountView.setText(Localization.localizeStreamCountMini( + itemStreamCountView.getContext(), item.getStreamCount())); + // Here is where the uploader name is set in the bookmarked playlists library + if (!TextUtils.isEmpty(item.getUploader())) { + itemUploaderView.setText(Localization.concatenateStrings(item.getUploader(), + NewPipe.getNameOfService(item.getServiceId()))); + } else { + itemUploaderView.setText(NewPipe.getNameOfService(item.getServiceId())); + } + + PicassoHelper.loadPlaylistThumbnail(item.getThumbnailUrl()).into(itemThumbnailView); + + itemHandleView.setOnTouchListener(getOnTouchListener(item)); + + super.updateFromItem(localItem, historyRecordManager, dateTimeFormatter); + } + + private View.OnTouchListener getOnTouchListener(final PlaylistRemoteEntity item) { + return (view, motionEvent) -> { + view.performClick(); + if (itemBuilder != null && itemBuilder.getOnItemSelectedListener() != null + && motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { + itemBuilder.getOnItemSelectedListener().drag(item, + RemoteBookmarkPlaylistItemHolder.this); + } + return false; + }; + } +} diff --git a/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java b/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java index 9ecfa6979..d2059bfed 100644 --- a/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java +++ b/app/src/main/java/org/schabi/newpipe/local/holder/RemotePlaylistItemHolder.java @@ -1,8 +1,6 @@ package org.schabi.newpipe.local.holder; import android.text.TextUtils; -import android.view.MotionEvent; -import android.view.View; import android.view.ViewGroup; import org.schabi.newpipe.R; @@ -17,17 +15,15 @@ import org.schabi.newpipe.util.Localization; import java.time.format.DateTimeFormatter; public class RemotePlaylistItemHolder extends PlaylistItemHolder { - private final View itemHandleView; public RemotePlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final ViewGroup parent) { - this(infoItemBuilder, R.layout.list_playlist_bookmark_item, parent); + this(infoItemBuilder, R.layout.list_playlist_mini_item, parent); } RemotePlaylistItemHolder(final LocalItemBuilder infoItemBuilder, final int layoutId, final ViewGroup parent) { super(infoItemBuilder, layoutId, parent); - itemHandleView = itemView.findViewById(R.id.itemHandle); } @Override @@ -52,20 +48,6 @@ public class RemotePlaylistItemHolder extends PlaylistItemHolder { PicassoHelper.loadPlaylistThumbnail(item.getThumbnailUrl()).into(itemThumbnailView); - itemHandleView.setOnTouchListener(getOnTouchListener(item)); - super.updateFromItem(localItem, historyRecordManager, dateTimeFormatter); } - - private View.OnTouchListener getOnTouchListener(final PlaylistRemoteEntity item) { - return (view, motionEvent) -> { - view.performClick(); - if (itemBuilder != null && itemBuilder.getOnItemSelectedListener() != null - && motionEvent.getActionMasked() == MotionEvent.ACTION_DOWN) { - itemBuilder.getOnItemSelectedListener().drag(item, - RemotePlaylistItemHolder.this); - } - return false; - }; - } } From c24aed054f9da7dcde634faf570f0314d44f6a87 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Sat, 16 Apr 2022 12:00:02 +0800 Subject: [PATCH 08/24] Fix sonar warning and typo --- .../schabi/newpipe/database/Migrations.java | 16 ++--- .../newpipe/local/LocalItemListAdapter.java | 15 +++-- .../local/bookmark/BookmarkFragment.java | 64 +++++++++---------- .../LocalBookmarkPlaylistItemHolder.java | 11 +--- .../local/holder/LocalPlaylistItemHolder.java | 3 +- .../RemoteBookmarkPlaylistItemHolder.java | 19 +----- .../holder/RemotePlaylistItemHolder.java | 3 +- 7 files changed, 53 insertions(+), 78 deletions(-) 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 ffca6cca5..1013899ca 100644 --- a/app/src/main/java/org/schabi/newpipe/database/Migrations.java +++ b/app/src/main/java/org/schabi/newpipe/database/Migrations.java @@ -195,8 +195,8 @@ public final class Migrations { try { database.beginTransaction(); - // update playlists - // create a temp table to initialize display_index + // Update playlists. + // Create a temp table to initialize display_index. database.execSQL("CREATE TABLE `playlists_tmp` " + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " + "`name` TEXT, `thumbnail_url` TEXT," @@ -204,16 +204,16 @@ public final class Migrations { database.execSQL("INSERT INTO `playlists_tmp` (`uid`, `name`, `thumbnail_url`)" + "SELECT `uid`, `name`, `thumbnail_url` FROM `playlists`"); - // replace the old table + // Replace the old table. database.execSQL("DROP TABLE `playlists`"); database.execSQL("ALTER TABLE `playlists_tmp` RENAME TO `playlists`"); - // create index on the new table + // 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 + // Update remote_playlists. + // Create a temp table to initialize display_index. database.execSQL("CREATE TABLE `remote_playlists_tmp` " + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " + "`service_id` INTEGER NOT NULL, `name` TEXT, `url` TEXT, " @@ -225,11 +225,11 @@ 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. database.execSQL("DROP TABLE `remote_playlists`"); database.execSQL("ALTER TABLE `remote_playlists_tmp` RENAME TO `remote_playlists`"); - // create index on the new table + // 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` " diff --git a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java index 161d35ee5..5c22cee24 100644 --- a/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java +++ b/app/src/main/java/org/schabi/newpipe/local/LocalItemListAdapter.java @@ -256,12 +256,17 @@ public class LocalItemListAdapter extends RecyclerView.Adapter, Void> { - // Save the list 10s after the last change occurred + // Save the list 10 seconds after the last change occurred private static final long SAVE_DEBOUNCE_MILLIS = 10000; private static final int MINIMUM_INITIAL_DRAG_VELOCITY = 12; @State @@ -281,6 +281,7 @@ public final class BookmarkFragment extends BaseLocalListFragment disposables.add(remotePlaylistManager.updatePlaylists( remoteItemsUpdate, remoteItemsDeleteUid) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(() -> { - if (isModified != null) { - isModified.set(false); - } - }, - throwable -> showError(new ErrorInfo(throwable, - UserAction.REQUESTED_BOOKMARK, - "Saving playlist")) - )), + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(() -> { + if (isModified != null) { + isModified.set(false); + } + }, + throwable -> showError(new ErrorInfo(throwable, + UserAction.REQUESTED_BOOKMARK, + "Saving playlist")) + )), throwable -> showError(new ErrorInfo(throwable, UserAction.REQUESTED_BOOKMARK, "Saving playlist")) )); @@ -529,22 +527,21 @@ public final class BookmarkFragment extends BaseLocalListFragment Date: Sat, 16 Apr 2022 12:44:24 +0800 Subject: [PATCH 09/24] Fix sonar warning --- .../newpipe/local/bookmark/BookmarkFragment.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) 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 14100ef81..b19107817 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 @@ -446,12 +446,10 @@ public final class BookmarkFragment extends BaseLocalListFragment Date: Sun, 17 Apr 2022 14:53:02 +0800 Subject: [PATCH 10/24] Reuse DebounceSaver --- .../database/playlist/PlaylistLocalItem.java | 8 +- .../local/bookmark/BookmarkFragment.java | 81 +++++++------------ .../local/playlist/LocalPlaylistFragment.java | 73 ++++++----------- .../schabi/newpipe/util/DebounceSavable.java | 15 ++++ .../schabi/newpipe/util/DebounceSaver.java | 68 ++++++++++++++++ 5 files changed, 142 insertions(+), 103 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/util/DebounceSavable.java create mode 100644 app/src/main/java/org/schabi/newpipe/util/DebounceSaver.java 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 47c6dd617..0e7beba41 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 @@ -17,15 +17,15 @@ public interface PlaylistLocalItem extends LocalItem { final List localPlaylists, final List remotePlaylists) { - // Merge localPlaylists and remotePlaylists by displayIndex. - // If two items have the same displayIndex, sort them in CASE_INSENSITIVE_ORDER. + // Merge localPlaylists and remotePlaylists by display index. + // If two items have the same display index, sort them in CASE_INSENSITIVE_ORDER. // 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<>(); - // The data from database may not be in the displayIndex order + // The data from database may not be in the display index order Collections.sort(localPlaylists, Comparator.comparingLong(PlaylistMetadataEntry::getDisplayIndex)); Collections.sort(remotePlaylists, @@ -58,7 +58,7 @@ public interface PlaylistLocalItem extends LocalItem { final List itemsWithSameIndex) { if (!itemsWithSameIndex.isEmpty() && itemsWithSameIndex.get(0).getDisplayIndex() != item.getDisplayIndex()) { - // The new item has a different displayIndex, add previous items with same + // The new item has a different display index, add previous items with same // index to the result. addItemsWithSameIndex(result, itemsWithSameIndex); itemsWithSameIndex.clear(); 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 b19107817..ceeb980c7 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 @@ -35,6 +35,8 @@ import org.schabi.newpipe.local.holder.LocalBookmarkPlaylistItemHolder; import org.schabi.newpipe.local.holder.RemoteBookmarkPlaylistItemHolder; import org.schabi.newpipe.local.playlist.LocalPlaylistManager; import org.schabi.newpipe.local.playlist.RemotePlaylistManager; +import org.schabi.newpipe.util.DebounceSavable; +import org.schabi.newpipe.util.DebounceSaver; import org.schabi.newpipe.util.NavigationHelper; import org.schabi.newpipe.util.OnClickGesture; @@ -42,7 +44,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import icepick.State; @@ -50,12 +51,10 @@ 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; -import io.reactivex.rxjava3.subjects.PublishSubject; -public final class BookmarkFragment extends BaseLocalListFragment, Void> { +public final class BookmarkFragment extends BaseLocalListFragment, Void> + implements DebounceSavable { - // Save the list 10 seconds after the last change occurred - private static final long SAVE_DEBOUNCE_MILLIS = 10000; private static final int MINIMUM_INITIAL_DRAG_VELOCITY = 12; @State protected Parcelable itemsListState; @@ -66,12 +65,10 @@ public final class BookmarkFragment extends BaseLocalListFragment debouncedSaveSignal; - - /* Has the playlist been fully loaded from db */ + /* Have the bookmarked playlists been fully loaded from db */ private AtomicBoolean isLoadingComplete; - /* Has the playlist been modified (e.g. items reordered or deleted) */ - private AtomicBoolean isModified; + + private DebounceSaver debounceSaver; // Map from (uid, local/remote item) to the saved display index in the database. private Map, Long> displayIndexInDatabase; @@ -91,9 +88,8 @@ public final class BookmarkFragment extends BaseLocalListFragment(); } @@ -183,9 +179,11 @@ public final class BookmarkFragment extends BaseLocalListFragment subscriptions) { - if (isModified == null || !isModified.get()) { + if (debounceSaver == null || !debounceSaver.getIsModified()) { checkDisplayIndexModified(subscriptions); handleResult(subscriptions); isLoadingComplete.set(true); @@ -346,11 +343,11 @@ public final class BookmarkFragment extends BaseLocalListFragment result) { - if (isModified != null && isModified.get()) { + if (debounceSaver != null && debounceSaver.getIsModified()) { return; } @@ -358,8 +355,9 @@ public final class BookmarkFragment extends BaseLocalListFragment saveImmediate(), throwable -> - showError(new ErrorInfo(throwable, UserAction.SOMETHING_ELSE, - "Debounced saver"))); - } - - private void saveImmediate() { + @Override + public void saveImmediate() { if (itemListAdapter == null) { return; } // List must be loaded and modified in order to save - if (isLoadingComplete == null || isModified == null - || !isLoadingComplete.get() || !isModified.get()) { + if (isLoadingComplete == null || debounceSaver == null + || !isLoadingComplete.get() || !debounceSaver.getIsModified()) { Log.w(TAG, "Attempting to save playlists in bookmark when bookmark " + "is not loaded or playlists not modified"); return; @@ -485,8 +462,8 @@ public final class BookmarkFragment extends BaseLocalListFragment { - if (isModified != null) { - isModified.set(false); + if (debounceSaver != null) { + debounceSaver.setIsModified(false); } }, throwable -> showError(new ErrorInfo(throwable, @@ -544,7 +521,7 @@ public final class BookmarkFragment extends BaseLocalListFragment, Void> { - // Save the list 10 seconds after the last change occurred - private static final long SAVE_DEBOUNCE_MILLIS = 10000; +public class LocalPlaylistFragment extends BaseLocalListFragment, Void> + implements DebounceSavable { private static final int MINIMUM_INITIAL_DRAG_VELOCITY = 12; @State protected Long playlistId; @@ -85,13 +84,13 @@ public class LocalPlaylistFragment extends BaseLocalListFragment debouncedSaveSignal; private CompositeDisposable disposables; /* Has the playlist been fully loaded from db */ private AtomicBoolean isLoadingComplete; - /* Has the playlist been modified (e.g. items reordered or deleted) */ - private AtomicBoolean isModified; + + private DebounceSaver debounceSaver; + /* Is the playlist currently being processed to remove watched videos */ private boolean isRemovingWatched = false; @@ -109,12 +108,11 @@ public class LocalPlaylistFragment extends BaseLocalListFragment streams) { // Skip handling the result after it has been modified - if (isModified == null || !isModified.get()) { + if (debounceSaver == null || !debounceSaver.getIsModified()) { handleResult(streams); isLoadingComplete.set(true); } @@ -441,7 +441,7 @@ public class LocalPlaylistFragment extends BaseLocalListFragment saveImmediate(), throwable -> - showError(new ErrorInfo(throwable, UserAction.SOMETHING_ELSE, - "Debounced saver"))); - } - - private void saveImmediate() { + @Override + public void saveImmediate() { if (playlistManager == null || itemListAdapter == null) { return; } // List must be loaded and modified in order to save - if (isLoadingComplete == null || isModified == null - || !isLoadingComplete.get() || !isModified.get()) { + if (isLoadingComplete == null || debounceSaver == null + || !isLoadingComplete.get() || !debounceSaver.getIsModified()) { Log.w(TAG, "Attempting to save playlist when local playlist " + "is not loaded or not modified: playlist id=[" + playlistId + "]"); return; @@ -664,8 +643,8 @@ public class LocalPlaylistFragment extends BaseLocalListFragment { - if (isModified != null) { - isModified.set(false); + if (debounceSaver != null) { + debounceSaver.setIsModified(false); } }, throwable -> showError(new ErrorInfo(throwable, @@ -708,7 +687,7 @@ public class LocalPlaylistFragment extends BaseLocalListFragment + * Must set {@link DebounceSaver#setIsModified(boolean)} false in this method manually + * after the data has been saved. + */ + void saveImmediate(); + + void showError(ErrorInfo errorInfo); +} diff --git a/app/src/main/java/org/schabi/newpipe/util/DebounceSaver.java b/app/src/main/java/org/schabi/newpipe/util/DebounceSaver.java new file mode 100644 index 000000000..b17d7a29c --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/util/DebounceSaver.java @@ -0,0 +1,68 @@ +package org.schabi.newpipe.util; + +import org.schabi.newpipe.error.ErrorInfo; +import org.schabi.newpipe.error.UserAction; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; +import io.reactivex.rxjava3.disposables.Disposable; +import io.reactivex.rxjava3.subjects.PublishSubject; + +public class DebounceSaver { + + private final long saveDebounceMillis; + + private final PublishSubject debouncedSaveSignal; + + private final DebounceSavable debounceSavable; + + // Has the object been modified + private final AtomicBoolean isModified; + + + /** + * Creates a new {@code DebounceSaver}. + * + * @param saveDebounceMillis Save the object milliseconds later after the last change + * occurred. + * @param debounceSavable The object containing data to be saved. + */ + public DebounceSaver(final long saveDebounceMillis, final DebounceSavable debounceSavable) { + this.saveDebounceMillis = saveDebounceMillis; + debouncedSaveSignal = PublishSubject.create(); + this.debounceSavable = debounceSavable; + this.isModified = new AtomicBoolean(); + } + + public boolean getIsModified() { + return isModified.get(); + } + + public void setIsModified(final boolean isModified) { + this.isModified.set(isModified); + } + + public PublishSubject getDebouncedSaveSignal() { + return debouncedSaveSignal; + } + + public Disposable getDebouncedSaver() { + return debouncedSaveSignal + .debounce(saveDebounceMillis, TimeUnit.MILLISECONDS) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(ignored -> debounceSavable.saveImmediate(), throwable -> + debounceSavable.showError(new ErrorInfo(throwable, + UserAction.SOMETHING_ELSE, "Debounced saver"))); + } + + public void saveChanges() { + if (isModified == null || debouncedSaveSignal == null) { + return; + } + + isModified.set(true); + debouncedSaveSignal.onNext(System.currentTimeMillis()); + } +} From 6526ff1612ad430325a5bf8af7b425055306a0e6 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Sun, 17 Apr 2022 20:20:20 +0800 Subject: [PATCH 11/24] Add tests --- .../newpipe/database/DatabaseMigrationTest.kt | 92 ++++++++++++++- .../local/bookmark/BookmarkFragment.java | 3 +- .../playlist/PlaylistLocalItemTest.java | 105 ++++++++++++++++++ 3 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java diff --git a/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt b/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt index 6d05a45bf..73b6313db 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt @@ -13,6 +13,8 @@ import org.junit.Assert.assertNull import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.schabi.newpipe.database.playlist.model.PlaylistEntity +import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity import org.schabi.newpipe.extractor.stream.StreamType @RunWith(AndroidJUnit4::class) @@ -21,13 +23,17 @@ class DatabaseMigrationTest { private const val DEFAULT_SERVICE_ID = 0 private const val DEFAULT_URL = "https://www.youtube.com/watch?v=cDphUib5iG4" private const val DEFAULT_TITLE = "Test Title" + private const val DEFAULT_NAME = "Test Name" private val DEFAULT_TYPE = StreamType.VIDEO_STREAM private const val DEFAULT_DURATION = 480L private const val DEFAULT_UPLOADER_NAME = "Uploader Test" private const val DEFAULT_THUMBNAIL = "https://example.com/example.jpg" - private const val DEFAULT_SECOND_SERVICE_ID = 0 + private const val DEFAULT_SECOND_SERVICE_ID = 1 private const val DEFAULT_SECOND_URL = "https://www.youtube.com/watch?v=ncQU6iBn5Fc" + + private const val DEFAULT_THIRD_SERVICE_ID = 2 + private const val DEFAULT_THIRD_URL = "https://www.youtube.com/watch?v=dQw4w9WgXcQ" } @get:Rule @@ -123,6 +129,90 @@ class DatabaseMigrationTest { assertNull(secondStreamFromMigratedDatabase.isUploadDateApproximation) } + @Test + fun migrateDatabaseFrom5to6() { + val databaseInV5 = testHelper.createDatabase(AppDatabase.DATABASE_NAME, Migrations.DB_VER_5) + + val localUid1: Long + val localUid2: Long + val remoteUid1: Long + val remoteUid2: Long + databaseInV5.run { + localUid1 = insert( + "playlists", SQLiteDatabase.CONFLICT_FAIL, + ContentValues().apply { + put("name", DEFAULT_NAME + "1") + put("thumbnail_url", DEFAULT_THUMBNAIL) + } + ) + localUid2 = insert( + "playlists", SQLiteDatabase.CONFLICT_FAIL, + ContentValues().apply { + put("name", DEFAULT_NAME + "2") + put("thumbnail_url", DEFAULT_THUMBNAIL) + } + ) + delete( + "playlists", "uid = ?", + Array(1) { localUid1 } + ) + remoteUid1 = insert( + "remote_playlists", SQLiteDatabase.CONFLICT_FAIL, + ContentValues().apply { + put("service_id", DEFAULT_SERVICE_ID) + put("url", DEFAULT_URL) + } + ) + remoteUid2 = insert( + "remote_playlists", SQLiteDatabase.CONFLICT_FAIL, + ContentValues().apply { + put("service_id", DEFAULT_SECOND_SERVICE_ID) + put("url", DEFAULT_SECOND_URL) + } + ) + delete( + "remote_playlists", "uid = ?", + Array(1) { remoteUid2 } + ) + close() + } + + testHelper.runMigrationsAndValidate( + AppDatabase.DATABASE_NAME, Migrations.DB_VER_6, + true, Migrations.MIGRATION_5_6 + ) + + val migratedDatabaseV6 = getMigratedDatabase() + var localListFromDB = migratedDatabaseV6.playlistDAO().all.blockingFirst() + var remoteListFromDB = migratedDatabaseV6.playlistRemoteDAO().all.blockingFirst() + + assertEquals(1, localListFromDB.size) + assertEquals(localUid2, localListFromDB[0].uid) + assertEquals(0, localListFromDB[0].displayIndex) + assertEquals(1, remoteListFromDB.size) + assertEquals(remoteUid1, remoteListFromDB[0].uid) + assertEquals(0, remoteListFromDB[0].displayIndex) + + val localUid3 = migratedDatabaseV6.playlistDAO().insert( + PlaylistEntity(DEFAULT_NAME + "3", DEFAULT_THUMBNAIL, -1) + ) + val remoteUid3 = migratedDatabaseV6.playlistRemoteDAO().insert( + PlaylistRemoteEntity( + DEFAULT_THIRD_SERVICE_ID, DEFAULT_NAME, DEFAULT_THIRD_URL, + DEFAULT_THUMBNAIL, DEFAULT_UPLOADER_NAME, -1, 10 + ) + ) + + localListFromDB = migratedDatabaseV6.playlistDAO().all.blockingFirst() + remoteListFromDB = migratedDatabaseV6.playlistRemoteDAO().all.blockingFirst() + assertEquals(2, localListFromDB.size) + assertEquals(localUid3, localListFromDB[1].uid) + assertEquals(-1, localListFromDB[1].displayIndex) + assertEquals(2, remoteListFromDB.size) + assertEquals(remoteUid3, remoteListFromDB[1].uid) + assertEquals(-1, remoteListFromDB[1].displayIndex) + } + private fun getMigratedDatabase(): AppDatabase { val database: AppDatabase = Room.databaseBuilder( ApplicationProvider.getApplicationContext(), 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 ceeb980c7..a79719525 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 @@ -385,7 +385,7 @@ public final class BookmarkFragment extends BaseLocalListFragment localPlaylists = new ArrayList<>(); + final List remotePlaylists = new ArrayList<>(); + final List mergedPlaylists = + PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + + assertEquals(0, mergedPlaylists.size()); + } + + @Test + public void onlyLocalPlaylists() { + final List localPlaylists = new ArrayList<>(); + final List remotePlaylists = new ArrayList<>(); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 2, 1)); + localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 1, 1)); + localPlaylists.add(new PlaylistMetadataEntry(3, "name3", "", 0, 1)); + final List mergedPlaylists = + PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + + assertEquals(3, mergedPlaylists.size()); + assertEquals(0, mergedPlaylists.get(0).getDisplayIndex()); + assertEquals(1, mergedPlaylists.get(1).getDisplayIndex()); + assertEquals(2, mergedPlaylists.get(2).getDisplayIndex()); + } + + @Test + public void onlyRemotePlaylists() { + final List localPlaylists = new ArrayList<>(); + final List remotePlaylists = new ArrayList<>(); + remotePlaylists.add(new PlaylistRemoteEntity( + 1, "name1", "url1", "", "", 2, 1L)); + remotePlaylists.add(new PlaylistRemoteEntity( + 2, "name2", "url2", "", "", 1, 1L)); + remotePlaylists.add(new PlaylistRemoteEntity( + 3, "name3", "url3", "", "", 0, 1L)); + final List mergedPlaylists = + PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + + assertEquals(3, mergedPlaylists.size()); + assertEquals(0, mergedPlaylists.get(0).getDisplayIndex()); + assertEquals(1, mergedPlaylists.get(1).getDisplayIndex()); + assertEquals(2, mergedPlaylists.get(2).getDisplayIndex()); + } + + @Test + public void sameIndexWithDifferentName() { + final List localPlaylists = new ArrayList<>(); + final List remotePlaylists = new ArrayList<>(); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 0, 1)); + localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 1, 1)); + remotePlaylists.add(new PlaylistRemoteEntity( + 1, "name3", "url1", "", "", 0, 1L)); + remotePlaylists.add(new PlaylistRemoteEntity( + 2, "name4", "url2", "", "", 1, 1L)); + final List mergedPlaylists = + PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + + assertEquals(4, mergedPlaylists.size()); + assertTrue(mergedPlaylists.get(0) instanceof PlaylistMetadataEntry); + assertEquals("name1", ((PlaylistMetadataEntry) mergedPlaylists.get(0)).name); + assertTrue(mergedPlaylists.get(1) instanceof PlaylistRemoteEntity); + assertEquals("name3", ((PlaylistRemoteEntity) mergedPlaylists.get(1)).getName()); + assertTrue(mergedPlaylists.get(2) instanceof PlaylistMetadataEntry); + assertEquals("name2", ((PlaylistMetadataEntry) mergedPlaylists.get(2)).name); + assertTrue(mergedPlaylists.get(3) instanceof PlaylistRemoteEntity); + assertEquals("name4", ((PlaylistRemoteEntity) mergedPlaylists.get(3)).getName()); + } + + @Test + public void sameNameWithDifferentIndex() { + final List localPlaylists = new ArrayList<>(); + final List remotePlaylists = new ArrayList<>(); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 1, 1)); + localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 3, 1)); + remotePlaylists.add(new PlaylistRemoteEntity( + 1, "name1", "url1", "", "", 0, 1L)); + remotePlaylists.add(new PlaylistRemoteEntity( + 2, "name2", "url2", "", "", 2, 1L)); + final List mergedPlaylists = + PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + + assertEquals(4, mergedPlaylists.size()); + assertTrue(mergedPlaylists.get(0) instanceof PlaylistRemoteEntity); + assertEquals("name1", ((PlaylistRemoteEntity) mergedPlaylists.get(0)).getName()); + assertTrue(mergedPlaylists.get(1) instanceof PlaylistMetadataEntry); + assertEquals("name1", ((PlaylistMetadataEntry) mergedPlaylists.get(1)).name); + assertTrue(mergedPlaylists.get(2) instanceof PlaylistRemoteEntity); + assertEquals("name2", ((PlaylistRemoteEntity) mergedPlaylists.get(2)).getName()); + assertTrue(mergedPlaylists.get(3) instanceof PlaylistMetadataEntry); + assertEquals("name2", ((PlaylistMetadataEntry) mergedPlaylists.get(3)).name); + } +} From d32490a4be288348a8377c84dbf8c82253240bf6 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Wed, 11 May 2022 16:47:34 +0800 Subject: [PATCH 12/24] Create sub-package and default interval for DebounceSaver & sort playlists in db --- .../database/playlist/PlaylistLocalItem.java | 6 +----- .../database/playlist/dao/PlaylistRemoteDAO.java | 5 +++++ .../newpipe/local/bookmark/BookmarkFragment.java | 10 +++++----- .../local/playlist/LocalPlaylistFragment.java | 6 +++--- .../local/playlist/RemotePlaylistManager.java | 4 ++++ .../newpipe/settings/SelectPlaylistFragment.java | 4 ++-- .../util/{ => debounce}/DebounceSavable.java | 2 +- .../util/{ => debounce}/DebounceSaver.java | 15 ++++++++++++++- 8 files changed, 35 insertions(+), 17 deletions(-) rename app/src/main/java/org/schabi/newpipe/util/{ => debounce}/DebounceSavable.java (89%) rename app/src/main/java/org/schabi/newpipe/util/{ => debounce}/DebounceSaver.java (81%) 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 0e7beba41..8b01a636a 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 @@ -16,6 +16,7 @@ public interface PlaylistLocalItem extends LocalItem { static List merge( final List localPlaylists, final List remotePlaylists) { + // The playlists from the database must be in the display index order. // Merge localPlaylists and remotePlaylists by display index. // If two items have the same display index, sort them in CASE_INSENSITIVE_ORDER. @@ -25,11 +26,6 @@ public interface PlaylistLocalItem extends LocalItem { localPlaylists.size() + remotePlaylists.size()); final List itemsWithSameIndex = new ArrayList<>(); - // The data from database may not be in the display index order - Collections.sort(localPlaylists, - Comparator.comparingLong(PlaylistMetadataEntry::getDisplayIndex)); - Collections.sort(remotePlaylists, - Comparator.comparingLong(PlaylistRemoteEntity::getDisplayIndex)); int i = 0; int j = 0; while (i < localPlaylists.size()) { 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 ade857464..8118bc40f 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 @@ -11,6 +11,7 @@ import java.util.List; import io.reactivex.rxjava3.core.Flowable; +import static org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity.REMOTE_PLAYLIST_DISPLAY_INDEX; import static org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity.REMOTE_PLAYLIST_ID; import static org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity.REMOTE_PLAYLIST_SERVICE_ID; import static org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity.REMOTE_PLAYLIST_TABLE; @@ -39,6 +40,10 @@ public interface PlaylistRemoteDAO extends BasicDAO { + REMOTE_PLAYLIST_URL + " = :url AND " + REMOTE_PLAYLIST_SERVICE_ID + " = :serviceId") Flowable> getPlaylist(long serviceId, String url); + @Query("SELECT * FROM " + REMOTE_PLAYLIST_TABLE + + " ORDER BY " + REMOTE_PLAYLIST_DISPLAY_INDEX) + Flowable> getDisplayIndexOrderedPlaylists(); + @Query("SELECT " + REMOTE_PLAYLIST_ID + " FROM " + REMOTE_PLAYLIST_TABLE + " WHERE " + REMOTE_PLAYLIST_URL + " = :url " + "AND " + REMOTE_PLAYLIST_SERVICE_ID + " = :serviceId") 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 a79719525..b0833dd9c 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 @@ -35,8 +35,8 @@ import org.schabi.newpipe.local.holder.LocalBookmarkPlaylistItemHolder; import org.schabi.newpipe.local.holder.RemoteBookmarkPlaylistItemHolder; import org.schabi.newpipe.local.playlist.LocalPlaylistManager; import org.schabi.newpipe.local.playlist.RemotePlaylistManager; -import org.schabi.newpipe.util.DebounceSavable; -import org.schabi.newpipe.util.DebounceSaver; +import org.schabi.newpipe.util.debounce.DebounceSavable; +import org.schabi.newpipe.util.debounce.DebounceSaver; import org.schabi.newpipe.util.NavigationHelper; import org.schabi.newpipe.util.OnClickGesture; @@ -89,7 +89,7 @@ public final class BookmarkFragment extends BaseLocalListFragment(); } @@ -185,8 +185,8 @@ public final class BookmarkFragment extends BaseLocalListFragment> getDisplayIndexOrderedPlaylists() { + return playlistRemoteTable.getDisplayIndexOrderedPlaylists().subscribeOn(Schedulers.io()); + } + public Flowable> getPlaylist(final PlaylistInfo info) { return playlistRemoteTable.getPlaylist(info.getServiceId(), info.getUrl()) .subscribeOn(Schedulers.io()); 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 e8491d52c..cc47c3f1c 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java @@ -90,8 +90,8 @@ public class SelectPlaylistFragment extends DialogFragment { final LocalPlaylistManager localPlaylistManager = new LocalPlaylistManager(database); final RemotePlaylistManager remotePlaylistManager = new RemotePlaylistManager(database); - disposable = Flowable.combineLatest(localPlaylistManager.getPlaylists(), - remotePlaylistManager.getPlaylists(), PlaylistLocalItem::merge) + disposable = Flowable.combineLatest(localPlaylistManager.getDisplayIndexOrderedPlaylists(), + remotePlaylistManager.getDisplayIndexOrderedPlaylists(), PlaylistLocalItem::merge) .observeOn(AndroidSchedulers.mainThread()) .subscribe(this::displayPlaylists, this::onError); } diff --git a/app/src/main/java/org/schabi/newpipe/util/DebounceSavable.java b/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSavable.java similarity index 89% rename from app/src/main/java/org/schabi/newpipe/util/DebounceSavable.java rename to app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSavable.java index 189dce9c6..acc515dd6 100644 --- a/app/src/main/java/org/schabi/newpipe/util/DebounceSavable.java +++ b/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSavable.java @@ -1,4 +1,4 @@ -package org.schabi.newpipe.util; +package org.schabi.newpipe.util.debounce; import org.schabi.newpipe.error.ErrorInfo; diff --git a/app/src/main/java/org/schabi/newpipe/util/DebounceSaver.java b/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSaver.java similarity index 81% rename from app/src/main/java/org/schabi/newpipe/util/DebounceSaver.java rename to app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSaver.java index b17d7a29c..367174ab7 100644 --- a/app/src/main/java/org/schabi/newpipe/util/DebounceSaver.java +++ b/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSaver.java @@ -1,4 +1,4 @@ -package org.schabi.newpipe.util; +package org.schabi.newpipe.util.debounce; import org.schabi.newpipe.error.ErrorInfo; import org.schabi.newpipe.error.UserAction; @@ -21,6 +21,9 @@ public class DebounceSaver { // Has the object been modified private final AtomicBoolean isModified; + // Default 10 seconds + private static final long DEFAULT_SAVE_DEBOUNCE_MILLIS = 10000; + /** * Creates a new {@code DebounceSaver}. @@ -36,6 +39,16 @@ public class DebounceSaver { this.isModified = new AtomicBoolean(); } + /** + * Creates a new {@code DebounceSaver}. Save the object 10 seconds later after the last change + * occurred. + * + * @param debounceSavable The object containing data to be saved. + */ + public DebounceSaver(final DebounceSavable debounceSavable) { + this(DEFAULT_SAVE_DEBOUNCE_MILLIS, debounceSavable); + } + public boolean getIsModified() { return isModified.get(); } From ba394a7ab4d3a0c1ee705565c8543e9160d9ebb5 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Wed, 11 May 2022 18:08:14 +0800 Subject: [PATCH 13/24] Update test and Javadoc --- .../database/playlist/PlaylistLocalItem.java | 27 ++++++++++-- .../playlist/PlaylistLocalItemTest.java | 41 +++++++++++++++---- 2 files changed, 56 insertions(+), 12 deletions(-) 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 8b01a636a..352d12d6b 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 @@ -13,13 +13,34 @@ public interface PlaylistLocalItem extends LocalItem { long getDisplayIndex(); + /** + * 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 in the display index order + * @param remotePlaylists remote playlists in the display index order + * @return merged playlists + */ static List merge( final List localPlaylists, final List remotePlaylists) { - // The playlists from the database must be in the display index order. - // Merge localPlaylists and remotePlaylists by display index. - // If two items have the same display index, sort them in CASE_INSENSITIVE_ORDER. + for (int i = 1; i < localPlaylists.size(); i++) { + if (localPlaylists.get(i).getDisplayIndex() + < localPlaylists.get(i - 1).getDisplayIndex()) { + throw new IllegalArgumentException( + "localPlaylists is not in the display index order"); + } + } + + for (int i = 1; i < remotePlaylists.size(); i++) { + if (remotePlaylists.get(i).getDisplayIndex() + < remotePlaylists.get(i - 1).getDisplayIndex()) { + throw new IllegalArgumentException( + "remotePlaylists is not in the display index order"); + } + } + // This algorithm is similar to the merge operation in merge sort. final List result = new ArrayList<>( diff --git a/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java b/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java index e5f717144..98f611037 100644 --- a/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java +++ b/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java @@ -24,16 +24,26 @@ public class PlaylistLocalItemTest { public void onlyLocalPlaylists() { final List localPlaylists = new ArrayList<>(); final List remotePlaylists = new ArrayList<>(); - localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 2, 1)); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 0, 1)); localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 1, 1)); - localPlaylists.add(new PlaylistMetadataEntry(3, "name3", "", 0, 1)); + localPlaylists.add(new PlaylistMetadataEntry(3, "name3", "", 3, 1)); final List mergedPlaylists = PlaylistLocalItem.merge(localPlaylists, remotePlaylists); assertEquals(3, mergedPlaylists.size()); assertEquals(0, mergedPlaylists.get(0).getDisplayIndex()); assertEquals(1, mergedPlaylists.get(1).getDisplayIndex()); - assertEquals(2, mergedPlaylists.get(2).getDisplayIndex()); + assertEquals(3, mergedPlaylists.get(2).getDisplayIndex()); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidLocalPlaylists() { + final List localPlaylists = new ArrayList<>(); + final List remotePlaylists = new ArrayList<>(); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 2, 1)); + localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 1, 1)); + localPlaylists.add(new PlaylistMetadataEntry(3, "name3", "", 0, 1)); + PlaylistLocalItem.merge(localPlaylists, remotePlaylists); } @Test @@ -41,18 +51,31 @@ public class PlaylistLocalItemTest { final List localPlaylists = new ArrayList<>(); final List remotePlaylists = new ArrayList<>(); remotePlaylists.add(new PlaylistRemoteEntity( - 1, "name1", "url1", "", "", 2, 1L)); + 1, "name1", "url1", "", "", 1, 1L)); remotePlaylists.add(new PlaylistRemoteEntity( - 2, "name2", "url2", "", "", 1, 1L)); + 2, "name2", "url2", "", "", 2, 1L)); remotePlaylists.add(new PlaylistRemoteEntity( - 3, "name3", "url3", "", "", 0, 1L)); + 3, "name3", "url3", "", "", 4, 1L)); final List mergedPlaylists = PlaylistLocalItem.merge(localPlaylists, remotePlaylists); assertEquals(3, mergedPlaylists.size()); - assertEquals(0, mergedPlaylists.get(0).getDisplayIndex()); - assertEquals(1, mergedPlaylists.get(1).getDisplayIndex()); - assertEquals(2, mergedPlaylists.get(2).getDisplayIndex()); + assertEquals(1, mergedPlaylists.get(0).getDisplayIndex()); + assertEquals(2, mergedPlaylists.get(1).getDisplayIndex()); + assertEquals(4, mergedPlaylists.get(2).getDisplayIndex()); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidRemotePlaylists() { + final List localPlaylists = new ArrayList<>(); + final List remotePlaylists = new ArrayList<>(); + remotePlaylists.add(new PlaylistRemoteEntity( + 1, "name1", "url1", "", "", 1, 1L)); + remotePlaylists.add(new PlaylistRemoteEntity( + 2, "name2", "url2", "", "", 3, 1L)); + remotePlaylists.add(new PlaylistRemoteEntity( + 3, "name3", "url3", "", "", 0, 1L)); + PlaylistLocalItem.merge(localPlaylists, remotePlaylists); } @Test From 9ecef6f01103fc3aa0c719dd7abea4169d06c7d6 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Thu, 23 Jun 2022 19:20:16 +0800 Subject: [PATCH 14/24] Add abstract methods in PlaylistLocalItem & rename setIsModified --- .../database/playlist/PlaylistLocalItem.java | 4 ++++ .../playlist/PlaylistMetadataEntry.java | 14 +++++++++-- .../playlist/model/PlaylistEntity.java | 4 ++-- .../playlist/model/PlaylistRemoteEntity.java | 2 ++ .../local/bookmark/BookmarkFragment.java | 23 +++++++++---------- .../local/dialog/PlaylistAppendDialog.java | 4 ++-- .../local/playlist/LocalPlaylistFragment.java | 4 ++-- .../settings/SelectPlaylistFragment.java | 2 +- .../newpipe/util/debounce/DebounceSaver.java | 4 ++-- 9 files changed, 38 insertions(+), 23 deletions(-) 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 352d12d6b..3d58d3f7c 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 @@ -13,6 +13,10 @@ public interface PlaylistLocalItem extends LocalItem { long getDisplayIndex(); + 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}. diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java index ff80049a3..f1ead0fa4 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistMetadataEntry.java @@ -11,13 +11,13 @@ public class PlaylistMetadataEntry implements PlaylistLocalItem { public static final String PLAYLIST_STREAM_COUNT = "streamCount"; @ColumnInfo(name = PLAYLIST_ID) - public final long uid; + private final long uid; @ColumnInfo(name = PLAYLIST_NAME) public final String name; @ColumnInfo(name = PLAYLIST_THUMBNAIL_URL) public final String thumbnailUrl; @ColumnInfo(name = PLAYLIST_DISPLAY_INDEX) - public long displayIndex; + private long displayIndex; @ColumnInfo(name = PLAYLIST_STREAM_COUNT) public final long streamCount; @@ -44,4 +44,14 @@ public class PlaylistMetadataEntry implements PlaylistLocalItem { public long getDisplayIndex() { return displayIndex; } + + @Override + public long getUid() { + return uid; + } + + @Override + public void setDisplayIndex(final long displayIndex) { + this.displayIndex = displayIndex; + } } diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java index 272e8a5bc..508b55508 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java @@ -41,10 +41,10 @@ public class PlaylistEntity { @Ignore public PlaylistEntity(final PlaylistMetadataEntry item) { - this.uid = item.uid; + this.uid = item.getUid(); this.name = item.name; this.thumbnailUrl = item.thumbnailUrl; - this.displayIndex = item.displayIndex; + this.displayIndex = item.getDisplayIndex(); } public long getUid() { diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java index adea2738b..82baed82c 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java @@ -105,6 +105,7 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { && TextUtils.equals(getUploader(), info.getUploaderName()); } + @Override public long getUid() { return uid; } @@ -158,6 +159,7 @@ public class PlaylistRemoteEntity implements PlaylistLocalItem { return displayIndex; } + @Override public void setDisplayIndex(final long displayIndex) { this.displayIndex = displayIndex; } 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 b0833dd9c..e9cf83239 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 @@ -139,7 +139,7 @@ public final class BookmarkFragment extends BaseLocalListFragment(((PlaylistMetadataEntry) item).uid, + displayIndexInDatabase.put(new Pair<>(item.getUid(), LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM), item.getDisplayIndex()); - ((PlaylistMetadataEntry) item).displayIndex = i; + item.setDisplayIndex(i); } else if (item instanceof PlaylistRemoteEntity) { - displayIndexInDatabase.put(new Pair<>(((PlaylistRemoteEntity) item).getUid(), - LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM), - item.getDisplayIndex()); - ((PlaylistRemoteEntity) item).setDisplayIndex(i); + displayIndexInDatabase.put(new Pair<>(item.getUid(), + LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM), item.getDisplayIndex()); + item.setDisplayIndex(i); } } @@ -415,9 +414,9 @@ public final class BookmarkFragment extends BaseLocalListFragment key = new Pair<>(uid, LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM); final Long databaseIndex = displayIndexInDatabase.remove(key); @@ -463,7 +462,7 @@ public final class BookmarkFragment extends BaseLocalListFragment { if (debounceSaver != null) { - debounceSaver.setIsModified(false); + debounceSaver.setNoChangesToSave(); } }, throwable -> showError(new ErrorInfo(throwable, @@ -563,7 +562,7 @@ public final class BookmarkFragment extends BaseLocalListFragment changeLocalPlaylistName( - selectedItem.uid, + selectedItem.getUid(), dialogBinding.dialogEditText.getText().toString())) .setNegativeButton(R.string.cancel, null) .setNeutralButton(R.string.delete, (dialog, which) -> { diff --git a/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java b/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java index 58a10af22..a778e6578 100644 --- a/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java +++ b/app/src/main/java/org/schabi/newpipe/local/dialog/PlaylistAppendDialog.java @@ -147,12 +147,12 @@ public final class PlaylistAppendDialog extends PlaylistDialog { if (playlist.thumbnailUrl.equals("drawable://" + R.drawable.dummy_thumbnail_playlist)) { playlistDisposables.add(manager - .changePlaylistThumbnail(playlist.uid, streams.get(0).getThumbnailUrl()) + .changePlaylistThumbnail(playlist.getUid(), streams.get(0).getThumbnailUrl()) .observeOn(AndroidSchedulers.mainThread()) .subscribe(ignored -> successToast.show())); } - playlistDisposables.add(manager.appendToPlaylist(playlist.uid, streams) + playlistDisposables.add(manager.appendToPlaylist(playlist.getUid(), streams) .observeOn(AndroidSchedulers.mainThread()) .subscribe(ignored -> successToast.show())); 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 1b8302cac..d129e658e 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 @@ -221,7 +221,7 @@ public class LocalPlaylistFragment extends BaseLocalListFragment { if (debounceSaver != null) { - debounceSaver.setIsModified(false); + debounceSaver.setNoChangesToSave(); } }, throwable -> showError(new ErrorInfo(throwable, 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 cc47c3f1c..905a44fd1 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/SelectPlaylistFragment.java @@ -118,7 +118,7 @@ public class SelectPlaylistFragment extends DialogFragment { if (selectedItem instanceof PlaylistMetadataEntry) { final PlaylistMetadataEntry entry = ((PlaylistMetadataEntry) selectedItem); - onSelectedListener.onLocalPlaylistSelected(entry.uid, entry.name); + onSelectedListener.onLocalPlaylistSelected(entry.getUid(), entry.name); } else if (selectedItem instanceof PlaylistRemoteEntity) { final PlaylistRemoteEntity entry = ((PlaylistRemoteEntity) selectedItem); diff --git a/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSaver.java b/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSaver.java index 367174ab7..911e978ff 100644 --- a/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSaver.java +++ b/app/src/main/java/org/schabi/newpipe/util/debounce/DebounceSaver.java @@ -53,8 +53,8 @@ public class DebounceSaver { return isModified.get(); } - public void setIsModified(final boolean isModified) { - this.isModified.set(isModified); + public void setNoChangesToSave() { + isModified.set(false); } public PublishSubject getDebouncedSaveSignal() { From 4e401bc059ae1ede7f452fcfc68dd6d8c8987500 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Thu, 23 Jun 2022 20:36:21 +0800 Subject: [PATCH 15/24] Update playlists in parallel --- .../local/bookmark/BookmarkFragment.java | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) 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 e9cf83239..9b93cc3e6 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 @@ -454,21 +454,16 @@ public final class BookmarkFragment extends BaseLocalListFragment disposables.add(remotePlaylistManager.updatePlaylists( - remoteItemsUpdate, remoteItemsDeleteUid) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(() -> { - if (debounceSaver != null) { - debounceSaver.setNoChangesToSave(); - } - }, - throwable -> showError(new ErrorInfo(throwable, - UserAction.REQUESTED_BOOKMARK, - "Saving playlist")) - )), + .subscribe(() -> { + if (debounceSaver != null) { + debounceSaver.setNoChangesToSave(); + } + }, throwable -> showError(new ErrorInfo(throwable, UserAction.REQUESTED_BOOKMARK, "Saving playlist")) )); From 898a936064f19830779b0bf22f7c75b4bd4dd4df Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Thu, 23 Jun 2022 23:19:59 +0800 Subject: [PATCH 16/24] Update index modification logic & redo sorting in the merge algorithm --- .../database/playlist/PlaylistLocalItem.java | 23 ++--- .../local/bookmark/BookmarkFragment.java | 86 +++++++------------ .../local/playlist/LocalPlaylistFragment.java | 6 +- .../newpipe/util/debounce/DebounceSaver.java | 2 +- .../playlist/PlaylistLocalItemTest.java | 23 ----- 5 files changed, 39 insertions(+), 101 deletions(-) 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 3d58d3f7c..4314b0f82 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 @@ -21,29 +21,18 @@ public interface PlaylistLocalItem extends LocalItem { * 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 in the display index order - * @param remotePlaylists remote playlists in the display index order + * @param localPlaylists local playlists + * @param remotePlaylists remote playlists * @return merged playlists */ static List merge( final List localPlaylists, final List remotePlaylists) { - for (int i = 1; i < localPlaylists.size(); i++) { - if (localPlaylists.get(i).getDisplayIndex() - < localPlaylists.get(i - 1).getDisplayIndex()) { - throw new IllegalArgumentException( - "localPlaylists is not in the display index order"); - } - } - - for (int i = 1; i < remotePlaylists.size(); i++) { - if (remotePlaylists.get(i).getDisplayIndex() - < remotePlaylists.get(i - 1).getDisplayIndex()) { - throw new IllegalArgumentException( - "remotePlaylists is not in the display index order"); - } - } + 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. 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 9b93cc3e6..200fde562 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 @@ -41,9 +41,7 @@ import org.schabi.newpipe.util.NavigationHelper; import org.schabi.newpipe.util.OnClickGesture; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import icepick.State; @@ -70,8 +68,7 @@ public final class BookmarkFragment extends BaseLocalListFragment, Long> displayIndexInDatabase; + private List> deletedItems; /////////////////////////////////////////////////////////////////////////// // Fragment LifeCycle - Creation @@ -89,9 +86,9 @@ public final class BookmarkFragment extends BaseLocalListFragment(); + deletedItems = new ArrayList<>(); } @Nullable @@ -186,7 +183,8 @@ public final class BookmarkFragment extends BaseLocalListFragment(item.getUid(), + LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM)); + } else if (item instanceof PlaylistRemoteEntity) { + deletedItems.add(new Pair<>(item.getUid(), + LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM)); + } + + debounceSaver.setHasChangesToSave(); } private void checkDisplayIndexModified(@NonNull final List result) { @@ -351,9 +357,7 @@ public final class BookmarkFragment extends BaseLocalListFragment(item.getUid(), - LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM), item.getDisplayIndex()); - item.setDisplayIndex(i); - - } else if (item instanceof PlaylistRemoteEntity) { - - displayIndexInDatabase.put(new Pair<>(item.getUid(), - LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM), item.getDisplayIndex()); - item.setDisplayIndex(i); - + break; } } if (debounceSaver != null && isDisplayIndexModified) { - debounceSaver.saveChanges(); + debounceSaver.setHasChangesToSave(); } } @@ -414,43 +401,28 @@ public final class BookmarkFragment extends BaseLocalListFragment key = new Pair<>(uid, - LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM); - final Long databaseIndex = displayIndexInDatabase.remove(key); - - // The database index should not be null because inserting new item into database - // is not handled here. NullPointerException has occurred once, but I can't - // reproduce it. Enhance robustness here. - if (databaseIndex != null && databaseIndex != i) { + if (((PlaylistMetadataEntry) item).getDisplayIndex() != i) { + ((PlaylistMetadataEntry) item).setDisplayIndex(i); localItemsUpdate.add((PlaylistMetadataEntry) item); } } else if (item instanceof PlaylistRemoteEntity) { - ((PlaylistRemoteEntity) item).setDisplayIndex(i); - - final Long uid = ((PlaylistRemoteEntity) item).getUid(); - final Pair key = new Pair<>(uid, - LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM); - final Long databaseIndex = displayIndexInDatabase.remove(key); - - if (databaseIndex != null && databaseIndex != i) { + if (((PlaylistRemoteEntity) item).getDisplayIndex() != i) { + ((PlaylistRemoteEntity) item).setDisplayIndex(i); remoteItemsUpdate.add((PlaylistRemoteEntity) item); } } } // Find deleted items - for (final Pair key : displayIndexInDatabase.keySet()) { - if (key.second.equals(LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM)) { - localItemsDeleteUid.add(key.first); - } else if (key.second.equals(LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM)) { - remoteItemsDeleteUid.add(key.first); + for (final Pair item : deletedItems) { + if (item.second.equals(LocalItem.LocalItemType.PLAYLIST_LOCAL_ITEM)) { + localItemsDeleteUid.add(item.first); + } else if (item.second.equals(LocalItem.LocalItemType.PLAYLIST_REMOTE_ITEM)) { + remoteItemsDeleteUid.add(item.first); } } - displayIndexInDatabase.clear(); + deletedItems.clear(); // 1. Update local playlists // 2. Update remote playlists @@ -515,7 +487,7 @@ public final class BookmarkFragment extends BaseLocalListFragment localPlaylists = new ArrayList<>(); - final List remotePlaylists = new ArrayList<>(); - localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 2, 1)); - localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 1, 1)); - localPlaylists.add(new PlaylistMetadataEntry(3, "name3", "", 0, 1)); - PlaylistLocalItem.merge(localPlaylists, remotePlaylists); - } - @Test public void onlyRemotePlaylists() { final List localPlaylists = new ArrayList<>(); @@ -65,19 +55,6 @@ public class PlaylistLocalItemTest { assertEquals(4, mergedPlaylists.get(2).getDisplayIndex()); } - @Test(expected = IllegalArgumentException.class) - public void invalidRemotePlaylists() { - final List localPlaylists = new ArrayList<>(); - final List remotePlaylists = new ArrayList<>(); - remotePlaylists.add(new PlaylistRemoteEntity( - 1, "name1", "url1", "", "", 1, 1L)); - remotePlaylists.add(new PlaylistRemoteEntity( - 2, "name2", "url2", "", "", 3, 1L)); - remotePlaylists.add(new PlaylistRemoteEntity( - 3, "name3", "url3", "", "", 0, 1L)); - PlaylistLocalItem.merge(localPlaylists, remotePlaylists); - } - @Test public void sameIndexWithDifferentName() { final List localPlaylists = new ArrayList<>(); From 8ad7bf60d7ed3bcde5bda0d601ae5d920c042eb3 Mon Sep 17 00:00:00 2001 From: GGAutomaton <32899400+GGAutomaton@users.noreply.github.com> Date: Thu, 23 Jun 2022 23:31:56 +0800 Subject: [PATCH 17/24] Delete saveImmediate warnings & add comments --- .../org/schabi/newpipe/local/bookmark/BookmarkFragment.java | 3 +-- .../schabi/newpipe/local/playlist/LocalPlaylistFragment.java | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) 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 200fde562..4be4838ec 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 @@ -66,6 +66,7 @@ public final class BookmarkFragment extends BaseLocalListFragment> deletedItems; @@ -385,8 +386,6 @@ public final class BookmarkFragment extends BaseLocalListFragment Date: Fri, 29 Mar 2024 17:59:28 +0100 Subject: [PATCH 18/24] 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); } From 92e9c3e42eed0d8f5f29210e6527f8351edfc4da Mon Sep 17 00:00:00 2001 From: Stypox Date: Fri, 29 Mar 2024 20:43:55 +0100 Subject: [PATCH 19/24] Fix DatabaseMigrationTest Complete removal of unneeded index, and remove default value for `remote_playlists.display_index`. --- .../9.json | 25 ++-------- .../newpipe/database/DatabaseMigrationTest.kt | 47 ++++++++++++------- .../schabi/newpipe/database/Migrations.java | 7 +-- .../playlist/model/PlaylistEntity.java | 5 +- .../playlist/model/PlaylistRemoteEntity.java | 1 - 5 files changed, 37 insertions(+), 48 deletions(-) diff --git a/app/schemas/org.schabi.newpipe.database.AppDatabase/9.json b/app/schemas/org.schabi.newpipe.database.AppDatabase/9.json index 0fcd383af..aced06c0a 100644 --- a/app/schemas/org.schabi.newpipe.database.AppDatabase/9.json +++ b/app/schemas/org.schabi.newpipe.database.AppDatabase/9.json @@ -2,7 +2,7 @@ "formatVersion": 1, "database": { "version": 9, - "identityHash": "94596ea2227c63dd78b472ea4a83f1c4", + "identityHash": "7591e8039faa74d8c0517dc867af9d3e", "entities": [ { "tableName": "subscriptions", @@ -362,17 +362,7 @@ "uid" ] }, - "indices": [ - { - "name": "index_playlists_name", - "unique": false, - "columnNames": [ - "name" - ], - "orders": [], - "createSql": "CREATE INDEX IF NOT EXISTS `index_playlists_name` ON `${TABLE_NAME}` (`name`)" - } - ], + "indices": [], "foreignKeys": [] }, { @@ -511,15 +501,6 @@ ] }, "indices": [ - { - "name": "index_remote_playlists_name", - "unique": false, - "columnNames": [ - "name" - ], - "orders": [], - "createSql": "CREATE INDEX IF NOT EXISTS `index_remote_playlists_name` ON `${TABLE_NAME}` (`name`)" - }, { "name": "index_remote_playlists_service_id_url", "unique": true, @@ -743,7 +724,7 @@ "views": [], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '94596ea2227c63dd78b472ea4a83f1c4')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '7591e8039faa74d8c0517dc867af9d3e')" ] } } \ No newline at end of file diff --git a/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt b/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt index f71880366..a34cfece6 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/database/DatabaseMigrationTest.kt @@ -122,8 +122,10 @@ class DatabaseMigrationTest { ) testHelper.runMigrationsAndValidate( - AppDatabase.DATABASE_NAME, Migrations.DB_VER_6, - true, Migrations.MIGRATION_5_6 + AppDatabase.DATABASE_NAME, + Migrations.DB_VER_9, + true, + Migrations.MIGRATION_8_9 ) val migratedDatabaseV3 = getMigratedDatabase() @@ -209,6 +211,11 @@ class DatabaseMigrationTest { true, Migrations.MIGRATION_7_8 ) + testHelper.runMigrationsAndValidate( + AppDatabase.DATABASE_NAME, Migrations.DB_VER_9, + true, Migrations.MIGRATION_8_9 + ) + val migratedDatabaseV8 = getMigratedDatabase() val listFromDB = migratedDatabaseV8.searchHistoryDAO().all.blockingFirst() @@ -220,25 +227,27 @@ class DatabaseMigrationTest { @Test fun migrateDatabaseFrom8to9() { - val databaseInV5 = testHelper.createDatabase(AppDatabase.DATABASE_NAME, Migrations.DB_VER_5) + val databaseInV8 = testHelper.createDatabase(AppDatabase.DATABASE_NAME, Migrations.DB_VER_8) val localUid1: Long val localUid2: Long val remoteUid1: Long val remoteUid2: Long - databaseInV5.run { + databaseInV8.run { localUid1 = insert( "playlists", SQLiteDatabase.CONFLICT_FAIL, ContentValues().apply { put("name", DEFAULT_NAME + "1") - put("thumbnail_url", DEFAULT_THUMBNAIL) + put("is_thumbnail_permanent", false) + put("thumbnail_stream_id", -1) } ) localUid2 = insert( "playlists", SQLiteDatabase.CONFLICT_FAIL, ContentValues().apply { put("name", DEFAULT_NAME + "2") - put("thumbnail_url", DEFAULT_THUMBNAIL) + put("is_thumbnail_permanent", false) + put("thumbnail_stream_id", -1) } ) delete( @@ -267,33 +276,35 @@ class DatabaseMigrationTest { } testHelper.runMigrationsAndValidate( - AppDatabase.DATABASE_NAME, Migrations.DB_VER_6, - true, Migrations.MIGRATION_5_6 + AppDatabase.DATABASE_NAME, + Migrations.DB_VER_9, + true, + Migrations.MIGRATION_8_9 ) - val migratedDatabaseV6 = getMigratedDatabase() - var localListFromDB = migratedDatabaseV6.playlistDAO().all.blockingFirst() - var remoteListFromDB = migratedDatabaseV6.playlistRemoteDAO().all.blockingFirst() + val migratedDatabaseV9 = getMigratedDatabase() + var localListFromDB = migratedDatabaseV9.playlistDAO().all.blockingFirst() + var remoteListFromDB = migratedDatabaseV9.playlistRemoteDAO().all.blockingFirst() assertEquals(1, localListFromDB.size) assertEquals(localUid2, localListFromDB[0].uid) - assertEquals(0, localListFromDB[0].displayIndex) + assertEquals(-1, localListFromDB[0].displayIndex) assertEquals(1, remoteListFromDB.size) assertEquals(remoteUid1, remoteListFromDB[0].uid) - assertEquals(0, remoteListFromDB[0].displayIndex) + assertEquals(-1, remoteListFromDB[0].displayIndex) - val localUid3 = migratedDatabaseV6.playlistDAO().insert( - PlaylistEntity(DEFAULT_NAME + "3", DEFAULT_THUMBNAIL, -1) + val localUid3 = migratedDatabaseV9.playlistDAO().insert( + PlaylistEntity(DEFAULT_NAME + "3", false, -1, -1) ) - val remoteUid3 = migratedDatabaseV6.playlistRemoteDAO().insert( + val remoteUid3 = migratedDatabaseV9.playlistRemoteDAO().insert( PlaylistRemoteEntity( DEFAULT_THIRD_SERVICE_ID, DEFAULT_NAME, DEFAULT_THIRD_URL, DEFAULT_THUMBNAIL, DEFAULT_UPLOADER_NAME, -1, 10 ) ) - localListFromDB = migratedDatabaseV6.playlistDAO().all.blockingFirst() - remoteListFromDB = migratedDatabaseV6.playlistRemoteDAO().all.blockingFirst() + localListFromDB = migratedDatabaseV9.playlistDAO().all.blockingFirst() + remoteListFromDB = migratedDatabaseV9.playlistRemoteDAO().all.blockingFirst() assertEquals(2, localListFromDB.size) assertEquals(localUid3, localListFromDB[1].uid) assertEquals(-1, localListFromDB[1].displayIndex) 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 9d641965d..c9f630869 100644 --- a/app/src/main/java/org/schabi/newpipe/database/Migrations.java +++ b/app/src/main/java/org/schabi/newpipe/database/Migrations.java @@ -278,12 +278,13 @@ public final class Migrations { + "(`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, " + "`service_id` INTEGER NOT NULL, `name` TEXT, `url` TEXT, " + "`thumbnail_url` TEXT, `uploader` TEXT, " - + "`display_index` INTEGER NOT NULL DEFAULT 0," + + "`display_index` INTEGER NOT NULL," + "`stream_count` INTEGER)"); database.execSQL("INSERT INTO `remote_playlists_tmp` (`uid`, `service_id`, " - + "`name`, `url`, `thumbnail_url`, `uploader`, `stream_count`)" + + "`name`, `url`, `thumbnail_url`, `uploader`, `display_index`, " + + "`stream_count`)" + "SELECT `uid`, `service_id`, `name`, `url`, `thumbnail_url`, `uploader`, " - + "`stream_count` FROM `remote_playlists`"); + + "-1, `stream_count` FROM `remote_playlists`"); // Replace the old table, note that this also removes the index on the name which // we don't need anymore. diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java index cb18027d0..e0c1a06b7 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistEntity.java @@ -3,17 +3,14 @@ package org.schabi.newpipe.database.playlist.model; import androidx.room.ColumnInfo; import androidx.room.Entity; import androidx.room.Ignore; -import androidx.room.Index; import androidx.room.PrimaryKey; -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; import org.schabi.newpipe.database.playlist.PlaylistMetadataEntry; -@Entity(tableName = PLAYLIST_TABLE, - indices = {@Index(value = {PLAYLIST_NAME})}) +@Entity(tableName = PLAYLIST_TABLE) public class PlaylistEntity { public static final String DEFAULT_THUMBNAIL = "drawable://" diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java index 50c3899f1..60027a057 100644 --- a/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/playlist/model/PlaylistRemoteEntity.java @@ -21,7 +21,6 @@ import static org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity.RE @Entity(tableName = REMOTE_PLAYLIST_TABLE, indices = { - @Index(value = {REMOTE_PLAYLIST_NAME}), @Index(value = {REMOTE_PLAYLIST_SERVICE_ID, REMOTE_PLAYLIST_URL}, unique = true) }) public class PlaylistRemoteEntity implements PlaylistLocalItem { From e66e1b542c937a2df7e658102580225aededec4b Mon Sep 17 00:00:00 2001 From: Stypox Date: Fri, 29 Mar 2024 20:55:24 +0100 Subject: [PATCH 20/24] Also sort playlist duplicates by display index --- .../schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4e1c163a4..85b891770 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 @@ -154,6 +154,6 @@ public interface PlaylistStreamDAO extends BasicDAO { + " AND :streamUrl = :streamUrl" + " GROUP BY " + JOIN_PLAYLIST_ID - + " ORDER BY " + PLAYLIST_NAME + " COLLATE NOCASE ASC") + + " ORDER BY " + PLAYLIST_DISPLAY_INDEX) Flowable> getPlaylistDuplicatesMetadata(String streamUrl); } From 90979e2a8130655aacaa36b024816b832fbcbc06 Mon Sep 17 00:00:00 2001 From: Stypox Date: Fri, 29 Mar 2024 20:58:07 +0100 Subject: [PATCH 21/24] Fix PlaylistLocalItemTest --- .../local/bookmark/MergedPlaylistManager.java | 2 +- .../playlist/PlaylistLocalItemTest.java | 25 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/local/bookmark/MergedPlaylistManager.java b/app/src/main/java/org/schabi/newpipe/local/bookmark/MergedPlaylistManager.java index 6b0eda132..25eb2f652 100644 --- a/app/src/main/java/org/schabi/newpipe/local/bookmark/MergedPlaylistManager.java +++ b/app/src/main/java/org/schabi/newpipe/local/bookmark/MergedPlaylistManager.java @@ -39,7 +39,7 @@ public final class MergedPlaylistManager { * @param remotePlaylists remote playlists, already sorted by display index * @return merged playlists */ - private static List merge( + public static List merge( final List localPlaylists, final List remotePlaylists) { diff --git a/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java b/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java index ab6315d91..847c54aa8 100644 --- a/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java +++ b/app/src/test/java/org/schabi/newpipe/database/playlist/PlaylistLocalItemTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertTrue; import org.junit.Test; import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity; +import org.schabi.newpipe.local.bookmark.MergedPlaylistManager; import java.util.ArrayList; import java.util.List; @@ -15,7 +16,7 @@ public class PlaylistLocalItemTest { final List localPlaylists = new ArrayList<>(); final List remotePlaylists = new ArrayList<>(); final List mergedPlaylists = - PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + MergedPlaylistManager.merge(localPlaylists, remotePlaylists); assertEquals(0, mergedPlaylists.size()); } @@ -24,11 +25,11 @@ public class PlaylistLocalItemTest { public void onlyLocalPlaylists() { final List localPlaylists = new ArrayList<>(); final List remotePlaylists = new ArrayList<>(); - localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 0, 1)); - localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 1, 1)); - localPlaylists.add(new PlaylistMetadataEntry(3, "name3", "", 3, 1)); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", false, -1, 0, 1)); + localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", false, -1, 1, 1)); + localPlaylists.add(new PlaylistMetadataEntry(3, "name3", "", false, -1, 3, 1)); final List mergedPlaylists = - PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + MergedPlaylistManager.merge(localPlaylists, remotePlaylists); assertEquals(3, mergedPlaylists.size()); assertEquals(0, mergedPlaylists.get(0).getDisplayIndex()); @@ -47,7 +48,7 @@ public class PlaylistLocalItemTest { remotePlaylists.add(new PlaylistRemoteEntity( 3, "name3", "url3", "", "", 4, 1L)); final List mergedPlaylists = - PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + MergedPlaylistManager.merge(localPlaylists, remotePlaylists); assertEquals(3, mergedPlaylists.size()); assertEquals(1, mergedPlaylists.get(0).getDisplayIndex()); @@ -59,14 +60,14 @@ public class PlaylistLocalItemTest { public void sameIndexWithDifferentName() { final List localPlaylists = new ArrayList<>(); final List remotePlaylists = new ArrayList<>(); - localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 0, 1)); - localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 1, 1)); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", false, -1, 0, 1)); + localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", false, -1, 1, 1)); remotePlaylists.add(new PlaylistRemoteEntity( 1, "name3", "url1", "", "", 0, 1L)); remotePlaylists.add(new PlaylistRemoteEntity( 2, "name4", "url2", "", "", 1, 1L)); final List mergedPlaylists = - PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + MergedPlaylistManager.merge(localPlaylists, remotePlaylists); assertEquals(4, mergedPlaylists.size()); assertTrue(mergedPlaylists.get(0) instanceof PlaylistMetadataEntry); @@ -83,14 +84,14 @@ public class PlaylistLocalItemTest { public void sameNameWithDifferentIndex() { final List localPlaylists = new ArrayList<>(); final List remotePlaylists = new ArrayList<>(); - localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", 1, 1)); - localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", 3, 1)); + localPlaylists.add(new PlaylistMetadataEntry(1, "name1", "", false, -1, 1, 1)); + localPlaylists.add(new PlaylistMetadataEntry(2, "name2", "", false, -1, 3, 1)); remotePlaylists.add(new PlaylistRemoteEntity( 1, "name1", "url1", "", "", 0, 1L)); remotePlaylists.add(new PlaylistRemoteEntity( 2, "name2", "url2", "", "", 2, 1L)); final List mergedPlaylists = - PlaylistLocalItem.merge(localPlaylists, remotePlaylists); + MergedPlaylistManager.merge(localPlaylists, remotePlaylists); assertEquals(4, mergedPlaylists.size()); assertTrue(mergedPlaylists.get(0) instanceof PlaylistRemoteEntity); From 3cc0205def80a6732ea374655a272ac7d3457503 Mon Sep 17 00:00:00 2001 From: Stypox Date: Sat, 30 Mar 2024 14:14:31 +0100 Subject: [PATCH 22/24] Fix inconsistencies when removing playlist Remove checkDisplayIndexModified because it was causing more problems than it solved. Now when adding new playlists they won't necessarily appear at the top, but will get sorted alphabetically along with the other playlists with index -1. This will be the case until any playlist is sorted, at which point all indices are assigned and newly added playlists will appear at the top again. --- .../local/bookmark/BookmarkFragment.java | 31 +++---------------- 1 file changed, 5 insertions(+), 26 deletions(-) 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 59e2582ff..922429382 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 @@ -56,7 +56,7 @@ public final class BookmarkFragment extends BaseLocalListFragment> deletedItems; @@ -259,7 +260,6 @@ public final class BookmarkFragment extends BaseLocalListFragment subscriptions) { if (debounceSaver == null || !debounceSaver.getIsModified()) { - checkDisplayIndexModified(subscriptions); handleResult(subscriptions); isLoadingComplete.set(true); } @@ -349,30 +349,9 @@ public final class BookmarkFragment extends BaseLocalListFragment result) { - if (debounceSaver != null && debounceSaver.getIsModified()) { - return; - } - - // Check if the display index does not match the actual index in the list. - // This may happen when a new list is created - // or on the first run after database migration - // or display index is not continuous for some reason - // or the user changes the display index. - boolean isDisplayIndexModified = false; - for (int i = 0; i < result.size(); i++) { - final PlaylistLocalItem item = result.get(i); - if (item.getDisplayIndex() != i) { - isDisplayIndexModified = true; - break; - } - } - - if (debounceSaver != null && isDisplayIndexModified) { + if (debounceSaver != null) { debounceSaver.setHasChangesToSave(); + saveImmediate(); } } @@ -482,7 +461,7 @@ public final class BookmarkFragment extends BaseLocalListFragment Date: Sat, 30 Mar 2024 14:36:31 +0100 Subject: [PATCH 23/24] Fix warnings and allow moving only up and down even in grid --- .../newpipe/local/bookmark/BookmarkFragment.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) 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 922429382..a366723e0 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,7 +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; import android.os.Bundle; @@ -244,7 +243,7 @@ public final class BookmarkFragment extends BaseLocalListFragment> getPlaylistsSubscriber() { - return new Subscriber>() { + return new Subscriber<>() { @Override public void onSubscribe(final Subscription s) { showLoading(); @@ -276,7 +275,6 @@ public final class BookmarkFragment extends BaseLocalListFragment Date: Sat, 30 Mar 2024 14:46:13 +0100 Subject: [PATCH 24/24] Undo some unneeded changes to LocalPlaylistManager --- .../local/playlist/LocalPlaylistManager.java | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) 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 461ac2d0a..dd9307675 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 @@ -133,18 +133,13 @@ public class LocalPlaylistManager { } public Maybe renamePlaylist(final long playlistId, final String name) { - return modifyPlaylist(playlistId, name, THUMBNAIL_ID_LEAVE_UNCHANGED, false, -1); + return modifyPlaylist(playlistId, name, THUMBNAIL_ID_LEAVE_UNCHANGED, false); } public Maybe changePlaylistThumbnail(final long playlistId, final long thumbnailStreamId, final boolean isPermanent) { - return modifyPlaylist(playlistId, null, thumbnailStreamId, isPermanent, -1); - } - - public Maybe updatePlaylistDisplayIndex(final long playlistId, - final long displayIndex) { - return modifyPlaylist(playlistId, null, THUMBNAIL_ID_LEAVE_UNCHANGED, false, displayIndex); + return modifyPlaylist(playlistId, null, thumbnailStreamId, isPermanent); } public long getPlaylistThumbnailStreamId(final long playlistId) { @@ -168,8 +163,7 @@ public class LocalPlaylistManager { private Maybe modifyPlaylist(final long playlistId, @Nullable final String name, final long thumbnailStreamId, - final boolean isPermanent, - final long displayIndex) { + final boolean isPermanent) { return playlistTable.getPlaylist(playlistId) .firstElement() .filter(playlistEntities -> !playlistEntities.isEmpty()) @@ -182,9 +176,6 @@ public class LocalPlaylistManager { playlist.setThumbnailStreamId(thumbnailStreamId); playlist.setIsThumbnailPermanent(isPermanent); } - if (displayIndex != -1) { - playlist.setDisplayIndex(displayIndex); - } return playlistTable.update(playlist); }).subscribeOn(Schedulers.io()); }