From 2bf1bc64e7671b74c919388b26b19f350de11e50 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Fri, 28 Mar 2025 09:57:36 +0000 Subject: [PATCH] WIP: Unify function to move rows within/between playlists --- app/repository.py | 152 ++++++++++++++------------------------- tests/test_repository.py | 18 ++--- 2 files changed, 63 insertions(+), 107 deletions(-) diff --git a/app/repository.py b/app/repository.py index 9e619e4..240cc71 100644 --- a/app/repository.py +++ b/app/repository.py @@ -283,20 +283,19 @@ def _check_playlist_integrity( if fix: log.debug(msg) plr.row_number = idx - session.commit() else: raise ApplicationError(msg) -def _move_rows( - session: Session, playlist_id: int, starting_row: int, move_by: int +def _shift_rows( + session: Session, playlist_id: int, starting_row: int, shift_by: int ) -> None: """ - Move rows from starting_row by move_by. If move_by is +ve, move rows - down; if -ve, move them up. + Shift rows from starting_row by shift_by. If shift_by is +ve, shift rows + down; if -ve, shift them up. """ - log.debug(f"(_move_rows_down({playlist_id=}, {starting_row=}, {move_by=}") + log.debug(f"(_shift_rows_down({playlist_id=}, {starting_row=}, {shift_by=}") session.execute( update(PlaylistRows) @@ -304,32 +303,49 @@ def _move_rows( (PlaylistRows.playlist_id == playlist_id), (PlaylistRows.row_number >= starting_row), ) - .values(row_number=PlaylistRows.row_number + move_by) + .values(row_number=PlaylistRows.row_number + shift_by) ) - session.commit() -def move_rows_to_playlist( - from_rows: list[int], from_playlist_id: int, to_row: int, to_playlist_id: int +def move_rows( + from_rows: list[int], from_playlist_id: int, to_row: int, to_playlist_id: int | None = None ) -> None: """ - Move rows between playlists. + Move rows with or between playlists. + + Algorithm: + - Sanity check row numbers + - Check there are no playlist rows with playlist_id == PENDING_MOVE + - Put rows to be moved into PENDING_MOVE playlist + - Resequence remaining row numbers + - Make space for moved rows + - Move the PENDING_MOVE rows back and fixup row numbers + - Sanity check row numbers """ log.debug( f"move_rows_to_playlist({from_rows=}, {from_playlist_id=}, {to_row=}, {to_playlist_id=})" ) + # If to_playlist_id isn't specified, we're moving within the one + # playlist. + if to_playlist_id is None: + to_playlist_id = from_playlist_id + with db.Session() as session: # Sanity check row numbers _check_playlist_integrity(session, from_playlist_id, fix=False) - _check_playlist_integrity(session, to_playlist_id, fix=False) + if from_playlist_id != to_playlist_id: + _check_playlist_integrity(session, to_playlist_id, fix=False) # Check there are no playlist rows with playlist_id == PENDING_MOVE pending_move_rows = get_playlist_rows(Config.PLAYLIST_PENDING_MOVE) if pending_move_rows: raise ApplicationError(f"move_rows_to_playlist: {pending_move_rows=}") + # We need playlist length if we're moving within a playlist. Get + # that now before we remove rows. + from_playlist_length = len(get_playlist_rows(from_playlist_id)) # Put rows to be moved into PENDING_MOVE playlist session.execute( update(PlaylistRows) @@ -339,17 +355,30 @@ def move_rows_to_playlist( ) .values(playlist_id=Config.PLAYLIST_PENDING_MOVE) ) - session.commit() # Resequence remaining row numbers _check_playlist_integrity(session, from_playlist_id, fix=True) + session.commit() - # Make space for moved rows. - _move_rows(session, to_playlist_id, to_row, len(from_rows)) + # Make space for moved rows. If moving within one playlist, + # determning where to make the space is non-trivial. For example, + # if the playlist has ten entries and we're moving four of them + # to row 8, after we've moved the rows to the + # PLAYLIST_PENDING_MOVE there will only be six entries left. + # Clearly we can't make space at row 8... + space_row = to_row + if to_playlist_id == from_playlist_id: + overflow = max(to_row + len(from_rows) - from_playlist_length, 0) + if overflow != 0: + space_row = ( + to_row - overflow - len([a for a in from_rows if a > to_row]) + ) + + _shift_rows(session, to_playlist_id, space_row, len(from_rows)) # Move the PENDING_MOVE rows back and fixup row numbers update_list: list[dict[str, int]] = [] - next_row = to_row + next_row = space_row # PLAYLIST_PENDING_MOVE may have gaps so don't check it for row_to_move in get_playlist_rows( Config.PLAYLIST_PENDING_MOVE, check_playlist_itegrity=False @@ -365,92 +394,16 @@ def move_rows_to_playlist( session.commit() # Sanity check row numbers - _check_playlist_integrity(session, from_playlist_id, fix=False) - _check_playlist_integrity(session, to_playlist_id, fix=False) - - -def move_rows_within_playlist( - playlist_id: int, from_rows: list[int], to_row: int -) -> None: - """ - Move rows within a playlist. - - Algorithm: - - Sanity check row numbers - - Check there are no playlist rows with playlist_id == PENDING_MOVE - - Put rows to be moved into PENDING_MOVE playlist - - Resequence remaining row numbers - - Make space for moved rows - - Move the PENDING_MOVE rows back and fixup row numbers - - Sanity check row numbers - """ - - log.debug(f"move_rows_within_playlist({playlist_id=}, {from_rows=}, {to_row=})") - - with db.Session() as session: - # Sanity check row numbers - _check_playlist_integrity(session, playlist_id, fix=False) - - # Check there are no playlist rows with playlist_id == PENDING_MOVE - pending_move_rows = get_playlist_rows(Config.PLAYLIST_PENDING_MOVE) - if pending_move_rows: - raise ApplicationError(f"move_rows_within_playlist: {pending_move_rows=}") - - # Get length of playlist - playlist_length = len(get_playlist_rows(playlist_id)) - - # Put rows to be moved into PENDING_MOVE playlist - session.execute( - update(PlaylistRows) - .where( - PlaylistRows.playlist_id == playlist_id, - PlaylistRows.row_number.in_(from_rows), - ) - .values(playlist_id=Config.PLAYLIST_PENDING_MOVE) - ) - session.commit() - - # Resequence remaining row numbers - _check_playlist_integrity(session, playlist_id, fix=True) - - # Make space for moved rows. Determning where to make the space - # is non-trivial. For example, if the playlist has ten entries - # and we're moving four of them to row 8, after we've moved the - # rows to the PLAYLIST_PENDING_MOVE there will only be six - # entries left. Clearly we can't make space at row 8... - overflow = max(to_row + len(from_rows) - playlist_length, 0) - if overflow == 0: - space_row = to_row - else: - space_row = to_row - overflow - len([a for a in from_rows if a > to_row]) - _move_rows(session, playlist_id, space_row, len(from_rows)) - - # Move the PENDING_MOVE rows back and fixup row numbers - update_list: list[dict[str, int]] = [] - next_row = space_row - # PLAYLIST_PENDING_MOVE may have gaps so don't check it - for row_to_move in get_playlist_rows( - Config.PLAYLIST_PENDING_MOVE, check_playlist_itegrity=False - ): - update_list.append( - {"id": row_to_move.playlistrow_id, "row_number": next_row} - ) - update_list.append( - {"id": row_to_move.playlistrow_id, "playlist_id": playlist_id} - ) - next_row += 1 - session.execute(update(PlaylistRows), update_list) - session.commit() - - # Sanity check row numbers - _check_playlist_integrity(session, playlist_id, fix=False) + _check_playlist_integrity(session, from_playlist_id, fix=False) + if from_playlist_id != to_playlist_id: + _check_playlist_integrity(session, to_playlist_id, fix=False) def update_row_numbers( playlist_id: int, id_to_row_number: list[dict[int, int]] ) -> None: """ - Update playlistrows rownumbers for pass playlistrow_ids + Update playlistrows rownumbers for passed playlistrow_ids playlist_id is only needed for sanity checking """ @@ -683,8 +636,11 @@ def insert_row( _check_playlist_integrity(session, playlist_id, fix=False) # Make space for new row - _move_rows( - session=session, playlist_id=playlist_id, starting_row=row_number, move_by=1 + _shift_rows( + session=session, + playlist_id=playlist_id, + starting_row=row_number, + shift_by=1, ) playlist_row = PlaylistRows.insert_row( diff --git a/tests/test_repository.py b/tests/test_repository.py index 16521e8..3ac0d21 100644 --- a/tests/test_repository.py +++ b/tests/test_repository.py @@ -135,7 +135,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 10 (playlist, model) = self.create_rows("test_move_rows_test1", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [3], 5) + repository.move_rows([3], playlist.playlist_id, 5) # Check we have all rows and plr_rownums are correct new_order = [] @@ -149,7 +149,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 10 (playlist, model) = self.create_rows("test_move_rows_test2", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [4], 3) + repository.move_rows([4], playlist.playlist_id, 3) # Check we have all rows and plr_rownums are correct new_order = [] @@ -163,7 +163,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 10 (playlist, model) = self.create_rows("test_move_rows_test3", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [4], 2) + repository.move_rows([4], playlist.playlist_id, 2) # Check we have all rows and plr_rownums are correct new_order = [] @@ -177,7 +177,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 11 (playlist, model) = self.create_rows("test_move_rows_test4", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [1, 4, 5, 10], 8) + repository.move_rows([1, 4, 5, 10], playlist.playlist_id, 8) # Check we have all rows and plr_rownums are correct new_order = [] @@ -191,7 +191,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 11 (playlist, model) = self.create_rows("test_move_rows_test5", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [3, 6], 5) + repository.move_rows([3, 6], playlist.playlist_id, 5) # Check we have all rows and plr_rownums are correct new_order = [] @@ -205,7 +205,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 11 (playlist, model) = self.create_rows("test_move_rows_test6", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [3, 5, 6], 8) + repository.move_rows([3, 5, 6], playlist.playlist_id, 8) # Check we have all rows and plr_rownums are correct new_order = [] @@ -219,7 +219,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 11 (playlist, model) = self.create_rows("test_move_rows_test7", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [7, 8, 10], 5) + repository.move_rows([7, 8, 10], playlist.playlist_id, 5) # Check we have all rows and plr_rownums are correct new_order = [] @@ -234,7 +234,7 @@ class MyTestCase(unittest.TestCase): number_of_rows = 11 (playlist, model) = self.create_rows("test_move_rows_test8", number_of_rows) - repository.move_rows_within_playlist(playlist.playlist_id, [0, 1, 2, 3], 0) + repository.move_rows([0, 1, 2, 3], playlist.playlist_id, 0) # Check we have all rows and plr_rownums are correct new_order = [] @@ -250,7 +250,7 @@ class MyTestCase(unittest.TestCase): (playlist_src, model_src) = self.create_rows("src playlist", number_of_rows) (playlist_dst, model_dst) = self.create_rows("dst playlist", number_of_rows) - repository.move_rows_to_playlist( + repository.move_rows( rows_to_move, playlist_src.playlist_id, to_row, playlist_dst.playlist_id )