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<>();