Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort bookmarked playlists #8221

Merged
merged 26 commits into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
96d6b30
Migrate database
GGAutomaton Apr 13, 2022
c34549a
Update database migrations and getter/setter
GGAutomaton Apr 13, 2022
270a541
Implement algorithm to merge playlists
GGAutomaton Apr 13, 2022
813f551
Merge branch 'TeamNewPipe:dev' into feature-7870
GGAutomaton Apr 13, 2022
ba8370b
Save changes to the database and bugfix
GGAutomaton Apr 14, 2022
bfb56b4
UI design and behavior
GGAutomaton Apr 14, 2022
3c48825
Debounced saver & bugfix & clean code
GGAutomaton Apr 15, 2022
0aa08a5
Use new item holder
GGAutomaton Apr 15, 2022
c24aed0
Fix sonar warning and typo
GGAutomaton Apr 16, 2022
bd1aae8
Fix sonar warning
GGAutomaton Apr 16, 2022
bb5390d
Reuse DebounceSaver
GGAutomaton Apr 17, 2022
6526ff1
Add tests
GGAutomaton Apr 17, 2022
d32490a
Create sub-package and default interval for DebounceSaver & sort play…
GGAutomaton May 11, 2022
ba394a7
Update test and Javadoc
GGAutomaton May 11, 2022
9ecef6f
Add abstract methods in PlaylistLocalItem & rename setIsModified
GGAutomaton Jun 23, 2022
4e401bc
Update playlists in parallel
GGAutomaton Jun 23, 2022
898a936
Update index modification logic & redo sorting in the merge algorithm
GGAutomaton Jun 23, 2022
8ad7bf6
Delete saveImmediate warnings & add comments
GGAutomaton Jun 23, 2022
e1ce3fe
Merge branch 'dev' into pr8221
Stypox Mar 29, 2024
4591c09
Apply review
Stypox Mar 29, 2024
92e9c3e
Fix DatabaseMigrationTest
Stypox Mar 29, 2024
e66e1b5
Also sort playlist duplicates by display index
Stypox Mar 29, 2024
90979e2
Fix PlaylistLocalItemTest
Stypox Mar 29, 2024
3cc0205
Fix inconsistencies when removing playlist
Stypox Mar 30, 2024
c9051d3
Fix warnings and allow moving only up and down even in grid
Stypox Mar 30, 2024
38d4887
Undo some unneeded changes to LocalPlaylistManager
Stypox Mar 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
743 changes: 743 additions & 0 deletions app/schemas/org.schabi.newpipe.database.AppDatabase/6.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -84,6 +90,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()

Expand Down Expand Up @@ -118,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(),
Expand Down
4 changes: 3 additions & 1 deletion app/src/main/java/org/schabi/newpipe/NewPipeDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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;
Expand All @@ -24,7 +25,8 @@ private 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();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -38,7 +38,7 @@
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";
Expand Down
56 changes: 55 additions & 1 deletion app/src/main/java/org/schabi/newpipe/database/Migrations.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -184,7 +185,60 @@ public void migrate(@NonNull final SupportSQLiteDatabase database) {
@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();

// 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,"
+ "`display_index` INTEGER NOT NULL DEFAULT 0)");
database.execSQL("INSERT INTO `playlists_tmp` (`uid`, `name`, `thumbnail_url`)"
+ "SELECT `uid`, `name`, `thumbnail_url` FROM `playlists`");

// Replace the old table.
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.
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();
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,68 @@
public interface PlaylistLocalItem extends LocalItem {
String getOrderingName();

long getDisplayIndex();

static List<PlaylistLocalItem> merge(
final List<PlaylistMetadataEntry> localPlaylists,
final List<PlaylistRemoteEntity> remotePlaylists) {
final List<PlaylistLocalItem> items = new ArrayList<>(

// 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<PlaylistLocalItem> result = new ArrayList<>(
localPlaylists.size() + remotePlaylists.size());
items.addAll(localPlaylists);
items.addAll(remotePlaylists);
final List<PlaylistLocalItem> 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()) {
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);

Collections.sort(items, Comparator.comparing(PlaylistLocalItem::getOrderingName,
Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)));
return result;
}

static void addItem(final List<PlaylistLocalItem> result, final PlaylistLocalItem item,
final List<PlaylistLocalItem> itemsWithSameIndex) {
if (!itemsWithSameIndex.isEmpty()
&& itemsWithSameIndex.get(0).getDisplayIndex() != item.getDisplayIndex()) {
// The new item has a different display index, add previous items with same
// index to the result.
addItemsWithSameIndex(result, itemsWithSameIndex);
itemsWithSameIndex.clear();
}
itemsWithSameIndex.add(item);
}

return items;
static void addItemsWithSameIndex(final List<PlaylistLocalItem> result,
final List<PlaylistLocalItem> itemsWithSameIndex) {
if (itemsWithSameIndex.size() > 1) {
Collections.sort(itemsWithSameIndex,
Comparator.comparing(PlaylistLocalItem::getOrderingName,
Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)));
}
result.addAll(itemsWithSameIndex);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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;
Expand All @@ -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 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;
}

Expand All @@ -35,4 +39,9 @@ public LocalItemType getLocalItemType() {
public String getOrderingName() {
return name;
}

@Override
public long getDisplayIndex() {
return displayIndex;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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;
Expand Down Expand Up @@ -36,4 +37,17 @@ default Flowable<List<PlaylistEntity>> listByService(final int serviceId) {

@Query("SELECT COUNT(*) FROM " + PLAYLIST_TABLE)
Flowable<Long> 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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ public interface PlaylistRemoteDAO extends BasicDAO<PlaylistRemoteEntity> {
+ " WHERE " + REMOTE_PLAYLIST_SERVICE_ID + " = :serviceId")
Flowable<List<PlaylistRemoteEntity>> listByService(int serviceId);

@Query("SELECT * FROM " + REMOTE_PLAYLIST_TABLE + " WHERE "
+ REMOTE_PLAYLIST_ID + " = :playlistId")
Flowable<List<PlaylistRemoteEntity>> getPlaylist(long playlistId);

@Query("SELECT * FROM " + REMOTE_PLAYLIST_TABLE + " WHERE "
+ REMOTE_PLAYLIST_URL + " = :url AND " + REMOTE_PLAYLIST_SERVICE_ID + " = :serviceId")
Flowable<List<PlaylistRemoteEntity>> getPlaylist(long serviceId, String url);
Expand Down
Loading