More 'detached session' fixes

Don't try to work out how tracks may have moved. Instead, delete all
track from current playlist and re-add them.

app/model.py

@@ -166,6 +166,73 @@ class Playlists(Base):
     def __repr__(self):
         return (f"<Playlist(id={self.id}, name={self.name}>")
 
+    def add_track(self, session, track, row=None):
+        """
+        Add track to playlist at given row.
+        If row=None, add to end of playlist
+        """
+
+        if not row:
+            row = PlaylistTracks.new_row(session, self.id)
+
+        glue = PlaylistTracks(row=row)
+        glue.track_id = track.id
+        self.tracks.append(glue)
+        session.commit()
+
+    def close(self, session):
+        "Record playlist as no longer loaded"
+
+        self.loaded = False
+        session.add(self)
+        session.commit()
+
+    @staticmethod
+    def get_all_closed_playlists(session):
+        "Returns a  list of all playlists not currently open"
+
+        return (
+            session.query(Playlists)
+            .filter(
+                (Playlists.loaded == False) |  # noqa E712
+                (Playlists.loaded == None)
+            )
+            .order_by(Playlists.last_used.desc())
+        ).all()
+
+    @staticmethod
+    def get_all_playlists(session):
+        "Returns a list of all playlists"
+
+        return session.query(Playlists).all()
+
+    @staticmethod
+    def get_last_used(session):
+        """
+        Return a list of playlists marked "loaded", ordered by loaded date.
+        """
+
+        return (
+            session.query(Playlists)
+            .filter(Playlists.loaded == True)   # noqa E712
+            .order_by(Playlists.last_used.desc())
+        ).all()
+
+    def get_notes(self):
+        return [a.note for a in self.notes]
+
+    @staticmethod
+    def get_playlist(session, playlist_id):
+        return (
+            session.query(Playlists)
+            .filter(
+                Playlists.id == playlist_id   # noqa E712
+            )
+        ).one()
+
+    def get_tracks(self):
+        return [a.tracks for a in self.tracks]
+
     @staticmethod
     def new(session, name):
         DEBUG(f"Playlists.new(name={name})")
@@ -186,75 +253,6 @@ class Playlists(Base):
 
         return p
 
-    def close(self, session):
-        "Record playlist as no longer loaded"
-
-        self.loaded = False
-        session.add(self)
-        session.commit()
-
-    @staticmethod
-    def get_last_used(session):
-        """
-        Return a list of playlists marked "loaded", ordered by loaded date.
-        """
-
-        return (
-            session.query(Playlists)
-            .filter(Playlists.loaded == True)   # noqa E712
-            .order_by(Playlists.last_used.desc())
-        ).all()
-
-    @staticmethod
-    def get_all_playlists(session):
-        "Returns a list of all playlists"
-
-        return session.query(Playlists).all()
-
-    @staticmethod
-    def get_all_closed_playlists(session):
-        "Returns a list of all playlists not currently open"
-
-        return (
-            session.query(Playlists)
-            .filter(
-                (Playlists.loaded == False) |  # noqa E712
-                (Playlists.loaded == None)
-            )
-            .order_by(Playlists.last_used.desc())
-        ).all()
-
-    # Not currently used 1 June 2021
-    # @staticmethod
-    # def get_name(session, plid):
-    #     """
-    #     Return name of playlist with id 'plid'
-    #     """
-
-    #     return (
-    #         session.query(Playlists.name)
-    #         .filter(Playlists.id == plid)
-    #     ).one()[0]
-
-    def add_track(self, session, track, row=None):
-        """
-        Add track to playlist at given row.
-        If row=None, add to end of playlist
-        """
-
-        if not row:
-            row = PlaylistTracks.new_row(session, self.id)
-
-        glue = PlaylistTracks(row=row)
-        glue.track_id = track.id
-        self.tracks.append(glue)
-
-    def get_notes(self):
-        return [a.note for a in self.notes]
-
-    def get_tracks(self):
-        return [a.tracks for a in self.tracks]
-
 
 class PlaylistTracks(Base):
     __tablename__ = 'playlisttracks'
@@ -266,13 +264,6 @@ class PlaylistTracks(Base):
     tracks = relationship("Tracks", back_populates="playlists")
     playlists = relationship("Playlists", back_populates="tracks")
 
-    @staticmethod
-    def new_row(session, playlist_id):
-        "Return row number > largest existing row number"
-
-        last_row = session.query(func.max(PlaylistTracks.row)).one()[0]
-        return last_row + 1
-
     @staticmethod
     def add_track(session, playlist_id, track_id, row):
         DEBUG(
@@ -305,6 +296,24 @@ class PlaylistTracks(Base):
         record.row = new_row
         session.commit()
 
+    @staticmethod
+    def new_row(session, playlist_id):
+        "Return row number > largest existing row number"
+
+        last_row = session.query(func.max(PlaylistTracks.row)).one()[0]
+        return last_row + 1
+
+    @staticmethod
+    def remove_all_tracks(session, playlist_id):
+        """
+        Remove all tracks from passed playlist_id
+        """
+
+        session.query(PlaylistTracks).filter(
+            PlaylistTracks.playlist_id == playlist_id,
+        ).delete()
+        session.commit()
+
     @staticmethod
     def remove_track(session, playlist_id, row):
         DEBUG(
@@ -425,13 +434,6 @@ class Tracks(Base):
 
         return [a[0] for a in session.query(Tracks.path).all()]
 
-    # Not used as of 1 June 2021
-    # @classmethod
-    # def get_all_tracks(cls):
-    #     "Return a list of all tracks"
-
-    #     return session.query(cls).all()
-
     @staticmethod
     def get_path(session, id):
         try:

app/musicmuster.py

@@ -217,7 +217,7 @@ class Window(QMainWindow, Ui_MainWindow):
         # Get output filename
         pathspec = QFileDialog.getSaveFileName(
             self, 'Save Playlist',
-            directory=f"{self.visible_playlist().db.name}.m3u",
+            directory=f"{self.visible_playlist().name}.m3u",
             filter="M3U files (*.m3u);;All files (*.*)"
         )
         if not pathspec:
@@ -227,19 +227,23 @@ class Window(QMainWindow, Ui_MainWindow):
 
         if not path.endswith(".m3u"):
             path += ".m3u"
-        with open(path, "w") as f:
-            # Required directive on first line
-            f.write("#EXTM3U\n")
-            for track in self.visible_playlist().db.get_tracks():
-                f.write(
-                    "#EXTINF:"
-                    f"{int(track.duration / 1000)},"
-                    f"{track.title} - "
-                    f"{track.artist}"
-                    "\n"
-                    f"{track.path}"
-                    "\n"
-                )
+
+        # Get playlist db object
+        with Session() as session:
+            p = Playlists.get_playlist(session, self.visible_playlist().id)
+            with open(path, "w") as f:
+                # Required directive on first line
+                f.write("#EXTM3U\n")
+                for track in p.get_tracks():
+                    f.write(
+                        "#EXTINF:"
+                        f"{int(track.duration / 1000)},"
+                        f"{track.title} - "
+                        f"{track.artist}"
+                        "\n"
+                        f"{track.path}"
+                        "\n"
+                    )
 
     def fade(self):
         "Fade currently playing track"
@@ -261,7 +265,7 @@ class Window(QMainWindow, Ui_MainWindow):
         if ok:
             with Session() as session:
                 note = self.create_note(session, dlg.textValue())
-                self.visible_playlist().add_note(note)
+                self.visible_playlist().add_note(session, note)
 
     def load_last_playlists(self):
         "Load the playlists that we loaded at end of last session"
@@ -655,7 +659,8 @@ class DbDialog(QDialog):
 
         item = self.ui.matchList.currentItem()
         track_id = item.data(Qt.UserRole)
-        self.add_track(track_id)
+        with Session() as session:
+            self.add_track(session, track_id)
 
     def add_selected_and_close(self):
         self.add_selected()
@@ -678,14 +683,14 @@ class DbDialog(QDialog):
 
     def double_click(self, entry):
         track_id = entry.data(Qt.UserRole)
-        self.add_track(track_id)
+        with Session() as session:
+            self.add_track(session, track_id)
         # Select search text to make it easier for next search
         self.select_searchtext()
 
-    def add_track(self, track_id):
-        with Session() as session:
-            track = Tracks.track_from_id(session, track_id)
-            self.parent().visible_playlist().add_to_playlist(session, track)
+    def add_track(self, session, track_id):
+        track = Tracks.track_from_id(session, track_id)
+        self.parent().visible_playlist().add_to_playlist(session, track)
         # Select search text to make it easier for next search
         self.select_searchtext()
 

app/playlists.py

@@ -17,7 +17,7 @@ import os
 from config import Config
 from datetime import datetime, timedelta
 from log import DEBUG, ERROR
-from model import Notes, PlaylistTracks, Session, Settings, Tracks
+from model import Notes, Playlists, PlaylistTracks, Session, Settings, Tracks
 
 
 class Playlist(QTableWidget):
@@ -222,7 +222,7 @@ class Playlist(QTableWidget):
         Notes object.
         """
 
-        DEBUG(f"add_to_playlist(session={session}, data={data})")
+        DEBUG(f"playlists.add_to_playlist(session={session}, data={data})")
 
         if isinstance(data, Tracks):
             self.add_track(session, data, repaint=repaint)
@@ -241,7 +241,13 @@ class Playlist(QTableWidget):
             row = self.currentRow()
         else:
             row = self.rowCount()
-        DEBUG(f"add_track(track={track}), row={row}")
+        DEBUG(f"playlists.add_track(track={track}), row={row}")
+
+        # We need to add ourself to the session
+        us_in_db = session.query(Playlists).filter(
+            Playlists.id == self.id).one()
+
+        us_in_db.add_track(session, track, row)
 
         self.insertRow(row)
 
@@ -273,13 +279,13 @@ class Playlist(QTableWidget):
         "Clear current track"
 
         self._meta_clear_current()
-        self._repaint(save_playlist=False)
+        self._repaint()
 
     def clear_next(self):
         "Clear next track"
 
         self._meta_clear_next()
-        self._repaint(save_playlist=False)
+        self._repaint()
 
     def get_next_track_id(self):
         "Return next track id"
@@ -319,7 +325,7 @@ class Playlist(QTableWidget):
         for row in sorted(rows, reverse=True):
             self.removeRow(row)
 
-        self._repaint(save_playlist=False)
+        self._repaint()
 
     def get_selected_title(self):
         "Return title of selected row or None"
@@ -346,18 +352,24 @@ class Playlist(QTableWidget):
         scroll_to = self.item(current_row, self.COL_INDEX)
         self.scrollToItem(scroll_to, QAbstractItemView.PositionAtCenter)
         next_track_id = self._mark_next_track()
-        self._repaint(save_playlist=False)
+        self._repaint()
         return next_track_id
 
     def play_stopped(self):
         self._meta_clear_current()
         self.current_track_start_time = None
-        self._repaint(save_playlist=False)
+        self._repaint()
 
     def populate(self, session):
-        # add them in row order. We don't mandate that an item will be
+        # add tracks and notes in row order.
+        # We don't mandate that an item will be
         # on its specified row, only that it will be above
         # larger-numbered row items, and below lower-numbered ones.
+
+        # First, save our id for the future
+        self.id = self.db.id
+        self.name = self.db.name
+
         data = []
 
         for t in self.db.tracks:
@@ -381,7 +393,7 @@ class Playlist(QTableWidget):
 
     def repaint(self):
         # Called when we change tabs
-        self._repaint(save_playlist=False)
+        self._repaint()
 
     def select_next_track(self):
         """
@@ -509,10 +521,10 @@ class Playlist(QTableWidget):
                 if row in self._meta_get_notes():
                     Notes.delete_note(session, id)
                 else:
-                    PlaylistTracks.remove_track(session, self.db.id, row)
+                    PlaylistTracks.remove_track(session, self.id, row)
                 self.removeRow(row)
 
-        self._save_playlist(session)
+            self._save_playlist(session)
         self._repaint()
 
     def _drop_on(self, event):
@@ -696,7 +708,7 @@ class Playlist(QTableWidget):
         else:
             title = ""
         DEBUG(
-            f"playlist[{self.db.id}:{self.db.name}]._meta_set(row={row}, "
+            f"playlist[{self.id}:{self.name}]._meta_set(row={row}, "
             f"title={title}, metadata={metadata})"
         )
         if row is None:
@@ -718,14 +730,14 @@ class Playlist(QTableWidget):
         track_id = self._get_row_id(row)
         if track_id:
             self._meta_set_next(self.currentRow())
-            self._repaint(save_playlist=False)
+            self._repaint()
             self.master_process.set_next_track(track_id)
 
     def _repaint(self, clear_selection=True):
         "Set row colours, fonts, etc"
 
         DEBUG(
-            f"playlist[{self.db.id}:{self.db.name}]."
+            f"playlist[{self.id}:{self.name}]."
             f"_repaint(clear_selection={clear_selection}"
         )
 
@@ -810,15 +822,21 @@ class Playlist(QTableWidget):
 
     def _save_playlist(self, session):
         """
-        Save playlist to database. We do this by correcting differences
-        between the on-screen (definitive) playlist and that in the
-        database.
+        Save playlist to database.
 
-        We treat the notes rows and the tracks rows differently. Notes must
-        appear only once in only one playlist. Tracks can appear multiple
-        times in one playlist and in multiple playlists.
+        For notes: check the database entry is correct and update it if
+        necessary. Playlists:Note is one:many, so there is only one notes
+        appearance in all playlists.
+
+        For tracks: erase the playlist tracks and recreate. This is much
+        simpler than trying to correct any Playlists:Tracks many:many
+        errors.
         """
 
+        # We need to add ourself to the session
+        us_in_db = session.query(Playlists).filter(
+            Playlists.id == self.id).one()
+
         # Notes first
         # Create dictionaries indexed by note_id
         playlist_notes = {}
@@ -834,7 +852,7 @@ class Playlist(QTableWidget):
             playlist_notes[note_id] = row
 
         # Database
-        for note in self.db.notes:
+        for note in us_in_db.notes:
             database_notes[note.id] = note.row
 
         # Notes to add to database
@@ -843,14 +861,14 @@ class Playlist(QTableWidget):
         for note_id in set(playlist_notes.keys()) - set(database_notes.keys()):
             ERROR(
                 f"_save_playlist(): Note.id={note_id} "
-                f"missing from playlist {self} in database"
+                f"missing from playlist {us_in_db} in database"
             )
 
         # Notes to remove from database
         for note_id in set(database_notes.keys()) - set(playlist_notes.keys()):
             DEBUG(
                 f"_save_playlist(): Delete note note_id={note_id} "
-                f"from playlist {self} in database"
+                f"from playlist {us_in_db} in database"
             )
             Notes.delete_note(session, note_id)
 
@@ -864,55 +882,15 @@ class Playlist(QTableWidget):
                 )
                 Notes.update_note(session, note_id, playlist_notes[note_id])
 
-        # Now check tracks
-        # Create dictionaries indexed by row
-        playlist_tracks = {}
-        database_tracks = {}
-
-        # Playlist
+        # Tracks
+        # Remove all tracks for us in datbase
+        PlaylistTracks.remove_all_tracks(session, self.id)
+        # Iterate on-screen playlist and add tracks back in
         for row in range(self.rowCount()):
             if row in notes_rows:
                 continue
-            playlist_tracks[row] = self._get_row_id(row)
-
-        # Database
-        # Workaround for issue #10
-        # for track in self.db.tracks:
-        for track in session.query(PlaylistTracks).filter(
-                PlaylistTracks.playlist_id == self.db.id).all():
-            database_tracks[track.row] = track.track_id
-
-        # Tracks rows to add to database
-        for row in (
-                set(set(playlist_tracks.keys()) - set(database_tracks.keys()))
-        ):
-            DEBUG(f"_save_playlist(): row {row} missing from database")
-            PlaylistTracks.add_track(session, self.db.id,
-                                     playlist_tracks[row], row)
-
-        # Track rows to remove from database
-        for row in (
-                set(database_tracks.keys()) - set(playlist_tracks.keys())
-        ):
-            track = database_tracks[row]
-            DEBUG(
-                f"_save_playlist(): row {row} in database not playlist "
-                f"(track={track})"
-            )
-            PlaylistTracks.remove_track(session, self.db.id, row)
-
-        # Track rows to update in database
-        for row in (
-                set(playlist_tracks.keys()) & set(database_tracks.keys())
-        ):
-            if playlist_tracks[row] != database_tracks[row]:
-                DEBUG(
-                    "_save_playlist(): Update row={row} in database for "
-                    f"playlist {self} from track={database_tracks[row]} "
-                    f"to track={playlist_tracks[row]}"
-                )
-                PlaylistTracks.update_row_track(
-                    session, self.db.id, row, playlist_tracks[row])
+            PlaylistTracks.add_track(
+                session, self.id, self._get_row_id(row), row)
 
     def _set_column_widths(self):