gpodder / mygpo

The gpodder.net webservice
http://gpodder.net/
GNU Affero General Public License v3.0
270 stars 85 forks source link

Each episode state can only be set once and never again #729

Open mogwa1 opened 2 years ago

mogwa1 commented 2 years ago

I'm the main developer of Kasts (invent.kde.org/plasma-mobile/kasts) and while debugging some sync issues I stumbled upon what I believe is a bit of a showstopper in terms of episode state syncing.

The problem is that episode states can only be uploaded exactly once to the server. This is due to this bit of code in mygpo/history/models.py

        exists = cls.objects.filter(
            user=user,
            episode=episode,
            client=client,
            action=action,
            started=started,
            stopped=stopped,
        ).exists()
        if exists:
            logger.warning(
                "Trying to save duplicate {cls} for {user} "
                "/ {episode}".format(cls=cls, user=user, episode=episode)
            )
            # if such an entry already exists, do nothing
            return

The problem is that in real-life usage the exact same episode action might happen at different times (i.e. with different timestamps). E.g. a user listens to an episode a second time. In that case the following happens:

  1. The user downloads the episode a first time --> upload DOWNLOAD action which is accepted
  2. User listens to episode thereby potentially uploading several PLAY actions all of which are accepted
  3. After the user finishes the episode, a final PLAY action is uploaded which has (started=)position=total, effectively marking the episode as "played"
  4. User decides to listen to the episode again, potentially downloading the file again --> upload DOWNLOAD action which will be rejected
  5. User listens to episode --> several PLAY actions are uploaded with a low chance of collision with previous ones unless they happen to stop playback at the exact same spot as last time; i.e. most PLAY actions get stored to the DB
  6. User finishes the episode --> PLAY action is uploaded with (started=)position=total; this will be rejected by the server.

End result: the episode state will eternally sync to other devices with the play position being set to the last point before the user finished the episode the second time around.

This is the exact same situation that I and others ran into and which confused the hell out of me (as developer). At first I couldn't figure out why certain actions were accepted and others not.

Anyway, I think the actual fix would be quite simple: if a duplicate action is detected, simply update the timestamp field on that row in the DB rather than reject the entry altogether. That would avoid an exploding number of rows in the DB due to duplicates, but would still allow a user to reset episodes to a previous state. I would submit a PR for this, but unfortunately I don't know django well enough.