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 all 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
730 changes: 730 additions & 0 deletions app/schemas/org.schabi.newpipe.database.AppDatabase/9.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.ServiceList
import org.schabi.newpipe.extractor.stream.StreamType

Expand All @@ -22,13 +24,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 @@ -115,6 +121,13 @@ class DatabaseMigrationTest {
Migrations.MIGRATION_7_8
)

testHelper.runMigrationsAndValidate(
AppDatabase.DATABASE_NAME,
Migrations.DB_VER_9,
true,
Migrations.MIGRATION_8_9
)

val migratedDatabaseV3 = getMigratedDatabase()
val listFromDB = migratedDatabaseV3.streamDAO().all.blockingFirst()

Expand Down Expand Up @@ -198,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()

Expand All @@ -207,6 +225,94 @@ class DatabaseMigrationTest {
assertNotEquals(listFromDB[0].serviceId, listFromDB[1].serviceId)
}

@Test
fun migrateDatabaseFrom8to9() {
val databaseInV8 = testHelper.createDatabase(AppDatabase.DATABASE_NAME, Migrations.DB_VER_8)

val localUid1: Long
val localUid2: Long
val remoteUid1: Long
val remoteUid2: Long
databaseInV8.run {
localUid1 = insert(
"playlists", SQLiteDatabase.CONFLICT_FAIL,
ContentValues().apply {
put("name", DEFAULT_NAME + "1")
put("is_thumbnail_permanent", false)
put("thumbnail_stream_id", -1)
}
)
localUid2 = insert(
"playlists", SQLiteDatabase.CONFLICT_FAIL,
ContentValues().apply {
put("name", DEFAULT_NAME + "2")
put("is_thumbnail_permanent", false)
put("thumbnail_stream_id", -1)
}
)
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_9,
true,
Migrations.MIGRATION_8_9
)

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(-1, localListFromDB[0].displayIndex)
assertEquals(1, remoteListFromDB.size)
assertEquals(remoteUid1, remoteListFromDB[0].uid)
assertEquals(-1, remoteListFromDB[0].displayIndex)

val localUid3 = migratedDatabaseV9.playlistDAO().insert(
PlaylistEntity(DEFAULT_NAME + "3", false, -1, -1)
)
val remoteUid3 = migratedDatabaseV9.playlistRemoteDAO().insert(
PlaylistRemoteEntity(
DEFAULT_THIRD_SERVICE_ID, DEFAULT_NAME, DEFAULT_THIRD_URL,
DEFAULT_THUMBNAIL, DEFAULT_UPLOADER_NAME, -1, 10
)
)

localListFromDB = migratedDatabaseV9.playlistDAO().all.blockingFirst()
remoteListFromDB = migratedDatabaseV9.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
3 changes: 2 additions & 1 deletion app/src/main/java/org/schabi/newpipe/NewPipeDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.schabi.newpipe.database.Migrations.MIGRATION_5_6;
import static org.schabi.newpipe.database.Migrations.MIGRATION_6_7;
import static org.schabi.newpipe.database.Migrations.MIGRATION_7_8;
import static org.schabi.newpipe.database.Migrations.MIGRATION_8_9;

import android.content.Context;
import android.database.Cursor;
Expand All @@ -28,7 +29,7 @@ private static AppDatabase getDatabase(final Context context) {
return Room
.databaseBuilder(context.getApplicationContext(), AppDatabase.class, DATABASE_NAME)
.addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5,
MIGRATION_5_6, MIGRATION_6_7, MIGRATION_7_8)
MIGRATION_5_6, MIGRATION_6_7, MIGRATION_7_8, MIGRATION_8_9)
.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_8;
import static org.schabi.newpipe.database.Migrations.DB_VER_9;

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_8
version = DB_VER_9
)
public abstract class AppDatabase extends RoomDatabase {
public static final String DATABASE_NAME = "newpipe.db";
Expand Down
59 changes: 58 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 @@ -26,6 +26,7 @@ public final class Migrations {
public static final int DB_VER_6 = 6;
public static final int DB_VER_7 = 7;
public static final int DB_VER_8 = 8;
public static final int DB_VER_9 = 9;

private static final String TAG = Migrations.class.getName();
public static final boolean DEBUG = MainActivity.DEBUG;
Expand Down Expand Up @@ -187,7 +188,7 @@ 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");
}
};

Expand Down Expand Up @@ -245,6 +246,62 @@ public void migrate(@NonNull final SupportSQLiteDatabase database) {
}
};

public static final Migration MIGRATION_8_9 = new Migration(DB_VER_8, DB_VER_9) {
@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, `is_thumbnail_permanent` INTEGER NOT NULL, "
+ "`thumbnail_stream_id` INTEGER NOT NULL, "
+ "`display_index` INTEGER NOT NULL)");
database.execSQL("INSERT INTO `playlists_tmp` "
+ "(`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, 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`");


// 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,"
+ "`stream_count` INTEGER)");
database.execSQL("INSERT INTO `remote_playlists_tmp` (`uid`, `service_id`, "
+ "`name`, `url`, `thumbnail_url`, `uploader`, `display_index`, "
+ "`stream_count`)"
+ "SELECT `uid`, `service_id`, `name`, `url`, `thumbnail_url`, `uploader`, "
+ "-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.
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 UNIQUE INDEX `index_remote_playlists_service_id_url` "
+ "ON `remote_playlists` (`service_id`, `url`)");

database.setTransactionSuccessful();
} finally {
database.endTransaction();
}
}
};

private Migrations() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@ 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,
final boolean isThumbnailPermanent,
final long thumbnailStreamId,
final long displayIndex,
final long streamCount,
final long timesStreamIsContained) {
super(uid, name, thumbnailUrl, streamCount);
super(uid, name, thumbnailUrl, isThumbnailPermanent, thumbnailStreamId, displayIndex,
streamCount);
this.timesStreamIsContained = timesStreamIsContained;
}
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,13 @@
package org.schabi.newpipe.database.playlist;

import org.schabi.newpipe.database.LocalItem;
import org.schabi.newpipe.database.playlist.model.PlaylistRemoteEntity;

import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public interface PlaylistLocalItem extends LocalItem {
String getOrderingName();

static List<PlaylistLocalItem> merge(
final List<PlaylistMetadataEntry> localPlaylists,
final List<PlaylistRemoteEntity> remotePlaylists) {
return Stream.concat(localPlaylists.stream(), remotePlaylists.stream())
.sorted(Comparator.comparing(PlaylistLocalItem::getOrderingName,
Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)))
.collect(Collectors.toList());
}
long getDisplayIndex();

long getUid();

void setDisplayIndex(long displayIndex);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,40 @@

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_PERMANENT;
import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_THUMBNAIL_STREAM_ID;
import static org.schabi.newpipe.database.playlist.model.PlaylistEntity.PLAYLIST_THUMBNAIL_URL;

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_PERMANENT)
private final boolean isThumbnailPermanent;
@ColumnInfo(name = PLAYLIST_THUMBNAIL_STREAM_ID)
private final long thumbnailStreamId;
@ColumnInfo(name = PLAYLIST_THUMBNAIL_URL)
public final String thumbnailUrl;
@ColumnInfo(name = PLAYLIST_DISPLAY_INDEX)
private 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 boolean isThumbnailPermanent, final long thumbnailStreamId,
final long displayIndex, final long streamCount) {
this.uid = uid;
this.name = name;
this.thumbnailUrl = thumbnailUrl;
this.isThumbnailPermanent = isThumbnailPermanent;
this.thumbnailStreamId = thumbnailStreamId;
this.displayIndex = displayIndex;
this.streamCount = streamCount;
}

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

public boolean isThumbnailPermanent() {
return isThumbnailPermanent;
}

public long getThumbnailStreamId() {
return thumbnailStreamId;
}

@Override
public long getDisplayIndex() {
return displayIndex;
}

@Override
public long getUid() {
return uid;
}

@Override
public void setDisplayIndex(final long displayIndex) {
this.displayIndex = displayIndex;
}
}
Loading
Loading