Improve performance selecting multiple tracks

app/dbconfig.py

@@ -30,7 +30,7 @@ testing = False
 if MM_ENV == 'PRODUCTION':
     dbname = os.environ.get('MM_PRODUCTION_DBNAME', 'musicmuster_prod')
     dbuser = os.environ.get('MM_PRODUCTION_DBUSER', 'musicmuster')
-    dbpw = os.environ.get('MM_PRODUCTION_DBPW', 'xxxmusicmuster')
+    dbpw = os.environ.get('MM_PRODUCTION_DBPW', 'musicmuster')
     dbhost = os.environ.get('MM_PRODUCTION_DBHOST', 'localhost')
 elif MM_ENV == 'TESTING':
     dbname = os.environ.get('MM_TESTING_DBNAME', 'musicmuster_testing')

app/models.py

@@ -17,9 +17,10 @@ from sqlalchemy import (
     DateTime,
     Float,
     ForeignKey,
+    func,
     Integer,
     String,
-    func
+    UniqueConstraint,
 )
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm import (
@@ -39,7 +40,6 @@ from helpers import (
 )
 from log import DEBUG, ERROR
 
-
 Base: DeclarativeMeta = declarative_base()
 
 
@@ -97,9 +97,9 @@ class NoteColours(Base):
 
         for rec in (
                 session.query(NoteColours)
-                .filter(NoteColours.enabled.is_(True))
+                        .filter(NoteColours.enabled.is_(True))
-                .order_by(NoteColours.order)
+                        .order_by(NoteColours.order)
-                .all()
+                        .all()
         ):
             if rec.is_regex:
                 flags = re.UNICODE
@@ -206,8 +206,8 @@ class Playdates(Base):
 
         last_played: Optional[Playdates] = session.query(
             Playdates.lastplayed).filter(
-                (Playdates.track_id == track_id)
+            (Playdates.track_id == track_id)
-            ).order_by(Playdates.lastplayed.desc()).first()
+        ).order_by(Playdates.lastplayed.desc()).first()
         if last_played:
             return last_played[0]
         else:
@@ -265,7 +265,7 @@ class Playlists(Base):
         """
 
         if not row:
-            row = PlaylistTracks.next_free_row(session, self)
+            row = PlaylistTracks.next_free_row(session, self.id)
 
         PlaylistTracks(session, self.id, track_id, row)
 
@@ -294,8 +294,8 @@ class Playlists(Base):
 
         return (
             session.query(cls)
-            .filter(cls.loaded.is_(False))
+                .filter(cls.loaded.is_(False))
-            .order_by(cls.last_used.desc())
+                .order_by(cls.last_used.desc())
         ).all()
 
     @classmethod
@@ -306,8 +306,8 @@ class Playlists(Base):
 
         return (
             session.query(cls)
-            .filter(cls.loaded.is_(True))
+                .filter(cls.loaded.is_(True))
-            .order_by(cls.last_used.desc())
+                .order_by(cls.last_used.desc())
         ).all()
 
     def mark_open(self, session: Session) -> None:
@@ -317,8 +317,9 @@ class Playlists(Base):
         self.last_used = datetime.now()
         session.flush()
 
-    def move_track(self, session: Session, rows: List[int],
+    def move_track(
-                   to_playlist: "Playlists") -> None:
+            self, session: Session, rows: List[int],
+            to_playlist: "Playlists") -> None:
         """Move tracks to another playlist"""
 
         for row in rows:
@@ -354,7 +355,7 @@ class PlaylistTracks(Base):
 
     id: int = Column(Integer, primary_key=True, autoincrement=True)
     playlist_id: int = Column(Integer, ForeignKey('playlists.id'),
-                              primary_key=True)
+        primary_key=True)
     track_id: int = Column(Integer, ForeignKey('tracks.id'), primary_key=True)
     row: int = Column(Integer, nullable=False)
     tracks: RelationshipProperty = relationship("Tracks")
@@ -367,6 +368,10 @@ class PlaylistTracks(Base):
             cascade="all, delete-orphan"
         )
     )
+    # Ensure row numbers are unique within each playlist
+    __table_args__ = (UniqueConstraint
+                          ('row', 'playlist_id', name="uniquerow"),
+                      )
 
     def __init__(
             self, session: Session, playlist_id: int, track_id: int,
@@ -380,14 +385,14 @@ class PlaylistTracks(Base):
         session.flush()
 
     @staticmethod
-    def next_free_row(session: Session, playlist: Playlists) -> int:
+    def next_free_row(session: Session, playlist_id: int) -> int:
         """Return next free row number"""
 
         row: int
 
         last_row = session.query(
             func.max(PlaylistTracks.row)
-        ).filter_by(playlist_id=playlist.id).first()
+        ).filter_by(playlist_id=playlist_id).first()
         # if there are no rows, the above returns (None, ) which is True
         if last_row and last_row[0] is not None:
             row = last_row[0] + 1
@@ -396,6 +401,33 @@ class PlaylistTracks(Base):
 
         return row
 
+    @staticmethod
+    def move_rows(
+            session: Session, rows: List[int], from_playlist_id: int,
+            to_playlist_id: int) -> None:
+        """Move rows between playlists"""
+
+        # A constraint deliberately blocks duplicate (playlist_id, row)
+        # entries in database; however, unallocated rows in the database
+        # are fine (ie, we can have rows 1, 4, 6 and no 2, 3, 5).
+        # Unallocated rows will be automatically removed when the
+        # playlist is saved.
+
+        lowest_source_row: int = min(rows)
+        first_destination_free_row = PlaylistTracks.next_free_row(
+            session, to_playlist_id)
+        # Calculate offset that will put the lowest row number being
+        # moved at the first free row in destination playlist
+        offset = first_destination_free_row - lowest_source_row
+
+        session.query(PlaylistTracks).filter(
+            PlaylistTracks.playlist_id == from_playlist_id,
+            PlaylistTracks.row.in_(rows)
+        ).update({'playlist_id': to_playlist_id,
+                  'row': PlaylistTracks.row + offset},
+            False
+        )
+
 
 class Settings(Base):
     __tablename__ = 'settings'
@@ -444,24 +476,25 @@ class Tracks(Base):
     mtime: float = Column(Float, index=True)
     lastplayed: datetime = Column(DateTime, index=True, default=None)
     playlists: RelationshipProperty = relationship("PlaylistTracks",
-                                                   back_populates="tracks",
+        back_populates="tracks",
-                                                   lazy="joined")
+        lazy="joined")
     playdates: RelationshipProperty = relationship("Playdates",
-                                                   back_populates="tracks",
+        back_populates="tracks",
-                                                   lazy="joined")
+        lazy="joined")
 
-    def __init__(self,
+    def __init__(
-                 session: Session,
+            self,
-                 path: str,
+            session: Session,
-                 title: Optional[str] = None,
+            path: str,
-                 artist: Optional[str] = None,
+            title: Optional[str] = None,
-                 duration: Optional[int] = None,
+            artist: Optional[str] = None,
-                 start_gap: Optional[int] = None,
+            duration: Optional[int] = None,
-                 fade_at: Optional[int] = None,
+            start_gap: Optional[int] = None,
-                 silence_at: Optional[int] = None,
+            fade_at: Optional[int] = None,
-                 mtime: Optional[float] = None,
+            silence_at: Optional[int] = None,
-                 lastplayed: Optional[datetime] = None,
+            mtime: Optional[float] = None,
-                 ) -> None:
+            lastplayed: Optional[datetime] = None,
+            ) -> None:
         self.path = path
         self.title = title
         self.artist = artist
@@ -556,10 +589,10 @@ class Tracks(Base):
         audio: AudioSegment = get_audio_segment(self.path)
         self.duration = len(audio)
         self.fade_at = round(fade_point(audio) / 1000,
-                             Config.MILLISECOND_SIGFIGS) * 1000
+            Config.MILLISECOND_SIGFIGS) * 1000
         self.mtime = os.path.getmtime(self.path)
         self.silence_at = round(trailing_silence(audio) / 1000,
-                                Config.MILLISECOND_SIGFIGS) * 1000
+            Config.MILLISECOND_SIGFIGS) * 1000
         self.start_gap = leading_silence(audio)
         session.add(self)
         session.flush()
@@ -581,16 +614,16 @@ class Tracks(Base):
 
         return (
             session.query(cls)
-            .filter(cls.artist.ilike(f"%{text}%"))
+                .filter(cls.artist.ilike(f"%{text}%"))
-            .order_by(cls.title)
+                .order_by(cls.title)
         ).all()
 
     @classmethod
     def search_titles(cls, session: Session, text: str) -> List["Tracks"]:
         return (
             session.query(cls)
-            .filter(cls.title.ilike(f"%{text}%"))
+                .filter(cls.title.ilike(f"%{text}%"))
-            .order_by(cls.title)
+                .order_by(cls.title)
         ).all()
 
     @staticmethod

app/musicmuster.py

@@ -424,7 +424,8 @@ class Window(QMainWindow, Ui_MainWindow):
 
             # Update database for both source and destination playlists
             rows = visible_tab.get_selected_rows()
-            source_playlist.move_track(session, rows, destination_playlist)
+            PlaylistTracks.move_rows(session, rows, source_playlist.id,
+                                     destination_playlist.id)
 
             # Update destination playlist_tab if visible (if not visible, it
             # will be re-populated when it is opened)
@@ -594,10 +595,10 @@ class Window(QMainWindow, Ui_MainWindow):
             dlg = SelectPlaylistDialog(self, playlists=playlists,
                                        session=session)
             dlg.exec()
-            if dlg.plid:
+            playlist = dlg.playlist
-                p = Playlists.get_by_id(session=session, playlist_id=dlg.plid)
+            if playlist:
-                p.mark_open(session)
+                playlist.mark_open(session)
-                self.create_playlist_tab(session, p)
+                self.create_playlist_tab(session, playlist)
 
     def select_next_row(self) -> None:
         """Select next or first row in playlist"""
@@ -971,11 +972,6 @@ class SelectPlaylistDialog(QDialog):
         height = record.f_int or 600
         self.resize(width, height)
 
-        # for (plid, plname) in [(a.id, a.name) for a in playlists]:
-        #    p = QListWidgetItem()
-        #    p.setText(plname)
-        #    p.setData(Qt.UserRole, plid)
-        #    self.ui.lstPlaylists.addItem(p)
         for playlist in playlists:
             p = QListWidgetItem()
             p.setText(playlist.name)

app/playlists.py

@@ -136,6 +136,7 @@ class PlaylistTab(QTableWidget):
         self.itemSelectionChanged.connect(self._select_event)
 
         self.editing_cell: bool = False
+        self.selecting_in_progress = False
         self.cellChanged.connect(self._cell_changed)
         self.doubleClicked.connect(self._edit_cell)
         self.cellEditingStarted.connect(self._cell_edit_started)
@@ -390,8 +391,13 @@ class PlaylistTab(QTableWidget):
         # Row number will change as we delete rows so remove them in
         # reverse order.
 
-        for row in sorted(rows, reverse=True):
+        try:
-            self.removeRow(row)
+            self.selecting_in_progress = True
+            for row in sorted(rows, reverse=True):
+                self.removeRow(row)
+        finally:
+            self.selecting_in_progress = False
+            self._select_event()
 
         with Session() as session:
             self.save_playlist(session)
@@ -599,7 +605,12 @@ class PlaylistTab(QTableWidget):
     def select_played_tracks(self) -> None:
         """Select all played tracks in playlist"""
 
-        self._select_tracks(played=True)
+        try:
+            self.selecting_in_progress = True
+            self._select_tracks(played=True)
+        finally:
+            self.selecting_in_progress = False
+            self._select_event()
 
     def select_previous_row(self) -> None:
         """
@@ -640,7 +651,12 @@ class PlaylistTab(QTableWidget):
     def select_unplayed_tracks(self) -> None:
         """Select all unplayed tracks in playlist"""
 
-        self._select_tracks(played=False)
+        try:
+            self.selecting_in_progress = True
+            self._select_tracks(played=False)
+        finally:
+            self.selecting_in_progress = False
+            self._select_event()
 
     def set_selected_as_next(self) -> None:
         """Sets the select track as next to play"""
@@ -1411,11 +1427,16 @@ class PlaylistTab(QTableWidget):
         If multiple rows are selected, display sum of durations in status bar.
         """
 
+        # If we are in the process of selecting multiple tracks, no-op here
+        if self.selecting_in_progress:
+            return
+
         # Get the row number of all selected items and put into a set
         # to deduplicate
         sel_rows: Set[int] = set([item.row() for item in self.selectedItems()])
         # If no rows are selected, we have nothing to do
         if len(sel_rows) == 0:
+            self.musicmuster.lblSumPlaytime.setText("")
             return
 
         notes_rows: Set[int] = set(self._get_notes_rows())

migrations/versions/1c4048efee96_add_constraint_to_playlist_tracks.py

@@ -0,0 +1,34 @@
+"""Add constraint to playlist_tracks
+
+Revision ID: 1c4048efee96
+Revises: 52cbded98e7c
+Create Date: 2022-03-29 19:26:27.378185
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import mysql
+
+# revision identifiers, used by Alembic.
+revision = '1c4048efee96'
+down_revision = '52cbded98e7c'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_unique_constraint('uniquerow', 'playlist_tracks', ['row', 'playlist_id'])
+    op.alter_column('playlists', 'loaded',
+               existing_type=mysql.TINYINT(display_width=1),
+               nullable=False)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('playlists', 'loaded',
+               existing_type=mysql.TINYINT(display_width=1),
+               nullable=True)
+    op.drop_constraint('uniquerow', 'playlist_tracks', type_='unique')
+    # ### end Alembic commands ###