tracklist: Commit the database after adding, moving, or removing tracks

Otherwise we could lose the changes if the app crashes.

Fixes: #63 ("The database isn't being committed enough")
Signed-off-by: Anna Schumaker <Anna@NoWheyCreamery.com>

emmental/db/connection.py

@@ -75,6 +75,10 @@ class Connection(GObject.GObject):
             self._sql.close()
             self.connected = False
 
+    def commit(self) -> None:
+        """Commit pending changes."""
+        self._sql.commit()
+
     def executemany(self, statement: str, *args) -> sqlite3.Cursor | None:
         """Execute several similar SQL statements at once."""
         try:

emmental/db/table.py

@@ -188,6 +188,7 @@ class Table(Gtk.FilterListModel):
     def delete(self, row: Row) -> bool:
         """Delete a Row from the Table."""
         if row in self and self.do_sql_delete(row).rowcount == 1:
+            self.sql.commit()
             self.store.remove(row)
             del self.rows[row.primary_key]
             return True

emmental/db/tracks.py

@@ -241,6 +241,7 @@ class Table(table.Table):
         track.active = True
         track.laststarted = cur.fetchone()["laststarted"]
         self.current_track = track
+        self.sql.commit()
 
     def stop_track(self, track: Track, played: bool) -> None:
         """Mark that a Track has been stopped."""
@@ -270,6 +271,8 @@ class Table(table.Table):
             self.sql.playlists.queued.remove_track(track)
             self.sql.playlists.unplayed.remove_track(track)
 
+        self.sql.commit()
+
 
 class TrackidSet(GObject.GObject):
     """Manage a set of Track IDs."""

emmental/sidebar/playlist.py

@@ -59,6 +59,7 @@ class Section(section.Section):
 
     def __add_new_playlist(self, entry: Gtk.Entry) -> None:
         if self.table.create(entry.get_text()) is not None:
+            self.table.sql.commit()
             self.extra_widget.popdown()
 
     def __entry_changed(self, entry: Gtk.Entry) -> None:

emmental/tracklist/selection.py

@@ -152,6 +152,7 @@ class MoveButtons(Gtk.Box):
 class OSD(Gtk.Overlay):
     """An Overlay with extra controls for the Tracklist."""
 
+    sql = GObject.Property(type=db.Connection)
     playlist = GObject.Property(type=playlist.playlist.Playlist)
     selection = GObject.Property(type=Gtk.SelectionModel)
 
@@ -161,7 +162,7 @@ class OSD(Gtk.Overlay):
     def __init__(self, sql: db.Connection,
                  selection: Gtk.SelectionModel, **kwargs):
         """Initialize an OSD."""
-        super().__init__(selection=selection, **kwargs)
+        super().__init__(sql=sql, selection=selection, **kwargs)
         self._add = PopoverButton(child=Adw.ButtonContent(label="Add",
                                   icon_name="list-add-symbolic"),
                                   halign=Gtk.Align.START, valign=Gtk.Align.END,
@@ -206,12 +207,14 @@ class OSD(Gtk.Overlay):
                      playlist: db.playlists.Playlist) -> None:
         for track in self.__get_selected_tracks():
             playlist.add_track(track)
+        self.sql.commit()
         self.clear_selection()
 
     def __remove_clicked(self, button: Gtk.Button) -> None:
         if self.playlist is not None:
             for track in self.__get_selected_tracks():
                 self.playlist.remove_track(track)
+            self.sql.commit()
             self.clear_selection()
 
     def __move_track_down(self, move: MoveButtons) -> None:
@@ -219,6 +222,7 @@ class OSD(Gtk.Overlay):
             index = self.selection.get_selection().get_nth(0)
             self.selection.get_model().set_incremental(False)
             self.playlist.move_track_down(self.selection[index])
+            self.sql.commit()
             self.selection.get_model().set_incremental(True)
             self.__update_visibility()
 
@@ -227,6 +231,7 @@ class OSD(Gtk.Overlay):
             index = self.selection.get_selection().get_nth(0)
             self.selection.get_model().set_incremental(False)
             self.playlist.move_track_up(self.selection[index])
+            self.sql.commit()
             self.selection.get_model().set_incremental(True)
             self.__update_visibility()
 

tests/db/test_table.py

@@ -308,9 +308,12 @@ class TestTableFunctions(tests.util.TestCase):
     def test_delete(self):
         """Test deleting rows."""
         row = self.table.create(number=1)
-        self.assertTrue(row.delete())
-        self.assertEqual(len(self.table), 0)
-        self.assertDictEqual(self.table.rows, dict())
+
+        with unittest.mock.patch.object(self.sql, "commit") as mock_commit:
+            self.assertTrue(row.delete())
+            self.assertEqual(len(self.table), 0)
+            self.assertDictEqual(self.table.rows, dict())
+            mock_commit.assert_called()
 
         self.assertFalse(row.delete())
 

tests/db/test_tracks.py

@@ -508,16 +508,20 @@ class TestTrackTable(tests.util.TestCase):
         track = self.tracks.create(self.library, pathlib.Path("/a/b/1.ogg"),
                                    self.medium, self.year)
 
-        track.start()
-        row = self.sql("SELECT laststarted FROM tracks WHERE trackid=?",
-                       track.trackid).fetchone()
-        self.assertTrue(track.active)
-        self.assertIsNotNone(track.laststarted)
-        self.assertEqual(track.laststarted, row["laststarted"])
-        self.assertEqual(self.tracks.current_track, track)
+        with unittest.mock.patch.object(self.sql, "commit",
+                                        wraps=self.sql.commit) as mock_commit:
+            track.start()
+            mock_commit.assert_called()
 
-        self.playlists.previous.remove_track.assert_called_with(track)
-        self.playlists.previous.add_track.assert_called_with(track)
+            row = self.sql("SELECT laststarted FROM tracks WHERE trackid=?",
+                           track.trackid).fetchone()
+            self.assertTrue(track.active)
+            self.assertIsNotNone(track.laststarted)
+            self.assertEqual(track.laststarted, row["laststarted"])
+            self.assertEqual(self.tracks.current_track, track)
+
+            self.playlists.previous.remove_track.assert_called_with(track)
+            self.playlists.previous.add_track.assert_called_with(track)
 
     def test_stop_started_track(self):
         """Test marking that a Track has stopped playback."""
@@ -525,31 +529,39 @@ class TestTrackTable(tests.util.TestCase):
                                    self.medium, self.year, length=10)
 
         track.start()
-        track.stop(3)
-        row = self.sql("SELECT lastplayed FROM tracks WHERE trackid=?",
-                       track.trackid).fetchone()
-        self.assertFalse(track.active)
-        self.assertEqual(track.playcount, 0)
-        self.assertIsNone(row["lastplayed"])
-        self.assertIsNone(track.lastplayed)
-        self.assertIsNone(self.tracks.current_track)
+        with unittest.mock.patch.object(self.sql, "commit",
+                                        wraps=self.sql.commit) as mock_commit:
+            track.stop(3)
+            mock_commit.assert_called()
 
-        self.playlists.most_played.reload_tracks.assert_not_called()
-        self.playlists.queued.remove_track.assert_not_called()
-        self.playlists.unplayed.remove_track.assert_not_called()
+            row = self.sql("SELECT lastplayed FROM tracks WHERE trackid=?",
+                           track.trackid).fetchone()
+            self.assertFalse(track.active)
+            self.assertEqual(track.playcount, 0)
+            self.assertIsNone(row["lastplayed"])
+            self.assertIsNone(track.lastplayed)
+            self.assertIsNone(self.tracks.current_track)
+
+            self.playlists.most_played.reload_tracks.assert_not_called()
+            self.playlists.queued.remove_track.assert_not_called()
+            self.playlists.unplayed.remove_track.assert_not_called()
 
         track.start()
-        track.stop(8)
-        row = self.sql("""SELECT lastplayed, playcount FROM tracks
-                          WHERE trackid=?""", track.trackid).fetchone()
-        self.assertEqual(row["playcount"], 1)
-        self.assertEqual(track.playcount, 1)
-        self.assertEqual(row["lastplayed"], track.laststarted)
-        self.assertEqual(track.lastplayed, track.laststarted)
+        with unittest.mock.patch.object(self.sql, "commit",
+                                        wraps=self.sql.commit) as mock_commit:
+            track.stop(8)
+            mock_commit.assert_called()
 
-        self.playlists.most_played.reload_tracks.assert_called()
-        self.playlists.queued.remove_track.assert_called_with(track)
-        self.playlists.unplayed.remove_track.assert_called_with(track)
+            row = self.sql("""SELECT lastplayed, playcount FROM tracks
+                              WHERE trackid=?""", track.trackid).fetchone()
+            self.assertEqual(row["playcount"], 1)
+            self.assertEqual(track.playcount, 1)
+            self.assertEqual(row["lastplayed"], track.laststarted)
+            self.assertEqual(track.lastplayed, track.laststarted)
+
+            self.playlists.most_played.reload_tracks.assert_called()
+            self.playlists.queued.remove_track.assert_called_with(track)
+            self.playlists.unplayed.remove_track.assert_called_with(track)
 
     def test_stop_restarted_track(self):
         """Test marking that a restarted Track has stopped playback."""

tests/sidebar/test_playlist.py

@@ -68,23 +68,28 @@ class TestPlaylists(tests.util.TestCase):
 
         with unittest.mock.patch.object(self.playlists.extra_widget,
                                         "popdown") as mock_popdown:
-            self.playlists._entry.emit("activate")
-            self.assertEqual(len(self.sql.playlists), 0)
-            mock_popdown.assert_not_called()
+            with unittest.mock.patch.object(self.sql, "commit") as mock_commit:
+                self.playlists._entry.emit("activate")
+                self.assertEqual(len(self.sql.playlists), 0)
+                mock_popdown.assert_not_called()
+                mock_commit.assert_not_called()
 
-            self.playlists._entry.set_text("Test 1")
-            self.playlists._entry.emit("activate")
-            self.assertEqual(len(self.sql.playlists), 1)
-            self.assertEqual(self.sql.playlists.get_item(0).name, "Test 1")
-            mock_popdown.assert_called()
+                self.playlists._entry.set_text("Test 1")
+                self.playlists._entry.emit("activate")
+                self.assertEqual(len(self.sql.playlists), 1)
+                self.assertEqual(self.sql.playlists.get_item(0).name, "Test 1")
+                mock_popdown.assert_called()
+                mock_commit.assert_called()
 
-            mock_popdown.reset_mock()
-            self.playlists._entry.set_text("Test 2")
-            self.playlists._entry.emit("icon-release",
-                                       Gtk.EntryIconPosition.PRIMARY)
-            self.assertEqual(len(self.sql.playlists), 2)
-            self.assertEqual(self.sql.playlists.get_item(1).name, "Test 2")
-            mock_popdown.assert_called()
+                mock_popdown.reset_mock()
+                mock_commit.reset_mock()
+                self.playlists._entry.set_text("Test 2")
+                self.playlists._entry.emit("icon-release",
+                                           Gtk.EntryIconPosition.PRIMARY)
+                self.assertEqual(len(self.sql.playlists), 2)
+                self.assertEqual(self.sql.playlists.get_item(1).name, "Test 2")
+                mock_popdown.assert_called()
+                mock_commit.assert_called()
 
         self.playlists._entry.set_text("Test 3")
         self.assertEqual(self.playlists._entry.get_icon_name(

tests/tracklist/test_selection.py

@@ -249,6 +249,7 @@ class TestOsd(tests.util.TestCase):
         self.assertIsInstance(self.osd, Gtk.Overlay)
         self.assertIsInstance(self.osd._sizegroup, Gtk.SizeGroup)
         self.assertEqual(self.osd.selection, self.selection)
+        self.assertEqual(self.osd.sql, self.sql)
         self.assertIsNone(self.osd.playlist)
 
         self.assertEqual(self.osd._sizegroup.get_mode(),
@@ -296,12 +297,14 @@ class TestOsd(tests.util.TestCase):
         with unittest.mock.patch.object(self.db_plist,
                                         "add_track") as mock_add:
             self.selection.select_all()
-            self.osd._add.popover_child.emit("playlist-selected",
-                                             self.db_plist)
-            mock_add.assert_has_calls([unittest.mock.call(self.model[0]),
-                                       unittest.mock.call(self.model[1]),
-                                       unittest.mock.call(self.model[2])])
-            self.assertEqual(self.osd.n_selected, 0)
+            with unittest.mock.patch.object(self.sql, "commit") as mock_commit:
+                self.osd._add.popover_child.emit("playlist-selected",
+                                                 self.db_plist)
+                mock_add.assert_has_calls([unittest.mock.call(self.model[0]),
+                                           unittest.mock.call(self.model[1]),
+                                           unittest.mock.call(self.model[2])])
+                mock_commit.assert_called_once()
+                self.assertEqual(self.osd.n_selected, 0)
 
     def test_remove_button(self):
         """Test the remove tracks button."""
@@ -341,16 +344,18 @@ class TestOsd(tests.util.TestCase):
         """Test clicking the remove button."""
         with unittest.mock.patch.object(self.db_plist,
                                         "remove_track") as mock_remove:
-            self.selection.select_all()
-            self.osd._remove.emit("clicked")
-            mock_remove.assert_not_called()
+            with unittest.mock.patch.object(self.sql, "commit") as mock_commit:
+                self.selection.select_all()
+                self.osd._remove.emit("clicked")
+                mock_remove.assert_not_called()
+                mock_commit.assert_not_called()
 
-            self.osd.playlist = self.playlist
-            self.selection.select_all()
-            self.osd._remove.emit("clicked")
-            mock_remove.assert_has_calls([unittest.mock.call(self.model[0]),
-                                          unittest.mock.call(self.model[1]),
-                                          unittest.mock.call(self.model[2])])
+                self.osd.playlist = self.playlist
+                self.selection.select_all()
+                self.osd._remove.emit("clicked")
+                mock_remove.assert_has_calls([unittest.mock.call(self.model[i])
+                                              for i in range(3)])
+                mock_commit.assert_called_once()
 
     def test_move_buttons(self):
         """Test the move buttons."""
@@ -372,18 +377,22 @@ class TestOsd(tests.util.TestCase):
 
         with unittest.mock.patch.object(self.playlist,
                                         "move_track_down") as mock_move_down:
-            self.osd._move.emit("move-down")
-            mock_move_down.assert_called_with(self.model[1])
-            set_incremental.assert_has_calls([unittest.mock.call(False),
-                                             unittest.mock.call(True)])
+            with unittest.mock.patch.object(self.sql, "commit") as mock_commit:
+                self.osd._move.emit("move-down")
+                mock_move_down.assert_called_with(self.model[1])
+                set_incremental.assert_has_calls([unittest.mock.call(False),
+                                                  unittest.mock.call(True)])
+                mock_commit.assert_called_once()
 
         set_incremental.reset_mock()
         with unittest.mock.patch.object(self.playlist,
                                         "move_track_up") as mock_move_up:
-            self.osd._move.emit("move-up")
-            mock_move_up.assert_called_with(self.model[1])
-            set_incremental.assert_has_calls([unittest.mock.call(False),
-                                             unittest.mock.call(True)])
+            with unittest.mock.patch.object(self.sql, "commit") as mock_commit:
+                self.osd._move.emit("move-up")
+                mock_move_up.assert_called_with(self.model[1])
+                set_incremental.assert_has_calls([unittest.mock.call(False),
+                                                  unittest.mock.call(True)])
+                mock_commit.assert_called_once()
 
     def test_move_buttons_sensitive(self):
         """Test the move button sensitivity."""