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()
 
 
@@ -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)
 
@@ -317,7 +317,8 @@ class Playlists(Base):
         self.last_used = datetime.now()
         session.flush()
 
-    def move_track(self, session: Session, rows: List[int],
+    def move_track(
+            self, session: Session, rows: List[int],
             to_playlist: "Playlists") -> None:
         """Move tracks to another playlist"""
 
@@ -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'
@@ -450,7 +482,8 @@ class Tracks(Base):
         back_populates="tracks",
         lazy="joined")
 
-    def __init__(self,
+    def __init__(
+            self,
             session: Session,
             path: str,
             title: Optional[str] = None,

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.
 
+        try:
+            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"""
 
+        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"""
 
+        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 ###