Implement removal of duplicates from local playlists

[onelharrison]

What is it?

• Bugfix (user facing)
• Feature (user facing)
• Codebase improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

• Remove Duplicate Entries From Queue  #5558

Before/After Screenshots/Screen Record

Before

After

On click "Remove duplicates" menu item, the appropriate popup menu is displayed

On click "Yes", duplicates in playlist are removed

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

• I read the contribution guidelines.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[litetex]

Problems I saw while quickly taking a look at this PR:

1. I don't think the function will be used very often. The PR implements a workaround for the bugs Cleared Playlist Doesn't Stay Cleared #5557 and Show a warning before adding a duplicate video to a playlist #4186 - I would like to see fixes for these bugs instead because they are the primary cause for duplicates in a playlist.
2. The code should be refactored out of the UI if possible. Also using the Java streaming API would be great.
3. I don't think it's a good idea to compare thumbnails to determine if a video is a duplicate
4. I think running the de-duplication on database level and then refreshing/reloading the UI would be the most performant solution here

[Stypox]

About 4.: I agree, and a query like this should work to get the items. I took part of this query, added a GROUP BY to group items with the same stream id together (which also solves 3.) and then only kept the minimum join index among each group (which means the first instance of the video in the playlist is kept).

"SELECT " + JOIN_STREAM_ID + ", MIN(" + JOIN_INDEX + ")"
+ " FROM " + PLAYLIST_STREAM_JOIN_TABLE
+ " WHERE " + JOIN_PLAYLIST_ID + " = :playlistId"
+ " GROUP BY " + JOIN_STREAM_ID

[litetex]

@Stypox

I don't think you need a query.
Something like

DELETE 
FROM playlist_stream_join
WHERE playlist_id = <playlistId>
AND stream_id IN (
    SELECT stream_id
    FROM playlist_stream_join
    WHERE playlist_id = <playlistId>
    GROUP BY stream_id
    HAVING COUNT(*) > 1
)
AND join_index NOT IN (
    SELECT join_index
    FROM playlist_stream_join
    WHERE playlist_id = <playlistId>
    GROUP BY stream_id
    HAVING COUNT(*) > 1
)

should also be able to de-duplicate the data.
However the join_index should probably be rebuilt after that.

[onelharrison]

I agree that this type of code should be refactored out of the UI [2] and that thumbnail comparison is not ideal for de-duplication [3] (although, [3] isn't really what's happening in this implementation). Still, the thumbnail comparison is unnecessary in the current implementation (and an un-pushed revision using Java steams). Admittedly, I'm new to programming on the Android platform and used the structure of the "Remove Watched" feature to guide this implementation. The thumbnail check is being used to reset the playlist's thumbnail in the event the first stream in a playlist is removed. However, the implementation(s) of this "Remove duplicates" feature preserve the first occurrence of a duplicate stream, so there isn't a need to reset the playlist's thumbnail.

Regarding [1], I'm not sure how this feature relates to #5557, but I do see the connection to #4186. I view duplicate removal as a two-part problem. The first part is ensuring that duplicates aren't added to playlists going forward. This would be addressed by a solution to #4186. The second part is removing duplicates that already exist (at least from view), which is what this PR is about.

@litetex despite your reservations about how frequently this feature will be used, am I to understand that [4] is saying that the preferred implementation is on touch of the "Remove duplicates" menu item, the query in #7703 (comment) is to be executed, followed by rebuilding the join index, then a refresh/reload of the UI?

I'm not sure how to implement any of that, but I'm willing to do it. Some reference examples of those three things (run SQL from UI action, rebuild join index, and refresh/reload UI) would serve as good starting points for me. I'm also assuming that "refresh" and "reload" in Android development mean the same thing, but please to correct me if I'm wrong.

If we go down this path, I would expect that this feature would have a relatively short lifespan (maybe a year or so) during which users could remove duplicates from their playlists and going forward, the fix to #4186 would be what disallows duplicate entries.

I would also like for us to consider @Stypox's approach a bit more. Instead of implementing this feature to clean up the user's databases, what if we left their database untouched, implemented #4186, and only rendered the distinct playlist items?

[Iron-E]

1. I don't think the function will be used very often. The PR implements a workaround for the bugs Cleared Playlist Doesn't Stay Cleared #5557 and Show a warning before adding a duplicate video to a playlist #4186 - I would like to see fixes for these bugs instead because they are the primary cause for duplicates in a playlist.

It's true that this option wouldn't be used often after those mentioned features are implemented, but their approaches are different. For example, even if I would be warned about adding more duplicates to my existing playlists, that wouldn't get rid of the ones that are already there.

[Stypox]

#5557 has been fixed separately in #8706, so this PR shouldn't need to fix it. Anyway, I think it's better to implement #4186 first, which I guess is simpler and more user-friendly (less cluttering options under hidden menus). Then if we see there is still need for removing duplicates from playlists, we can always implement that, too. Hence I am closing this PR, @onelharrison if you wish to work on #4186 feel free to do so :-)

[Iron-E]

I think it's better to implement #4186 first, which I guess is simpler and more user-friendly (less cluttering options under hidden menus).

Unless I'm misunderstanding: as I said in #7703 (comment), even if #4186 is implemented, playlists with pre-existing duplicates won't be affected. It would help future playlists, but nothing that already has duplicates, which is still a separate issue.

[Stypox]

Well, in the long term playlists with already some duplicates would not be a problem anymore