diff --git a/backend/src/cms_backend/shuttle/move_files.py b/backend/src/cms_backend/shuttle/move_files.py index 50c3f59..ee4b3e2 100644 --- a/backend/src/cms_backend/shuttle/move_files.py +++ b/backend/src/cms_backend/shuttle/move_files.py @@ -70,12 +70,13 @@ def move_book_files(session: OrmSession, book: Book): book.needs_file_operation = False return - # start with copies - while len(target_locations) > len(current_locations): - current_location = current_locations[0] - target_location = target_locations[0] + # Grab one valid source file to copy from since it's the same + # file spread across multiple locations with the same book + # data + source_location = current_locations[0] - current_path = current_location.full_local_path( + for target_location in target_locations: + source_path = source_location.full_local_path( ShuttleContext.local_warehouse_paths ) target_path = target_location.full_local_path( @@ -83,58 +84,42 @@ def move_book_files(session: OrmSession, book: Book): ) target_path.parent.mkdir(parents=True, exist_ok=True) - shutil.copy(current_path, target_path) - logger.debug(f"Copied book {book.id} from {current_path} to {target_path}") + shutil.copy(source_path, target_path) + logger.debug(f"Copied book {book.id} from {source_path} to {target_path}") book.events.append( - f"{getnow()}: copied book from {current_location.full_str} to " + f"{getnow()}: copied book from {source_location.full_str} to " f"{target_location.full_str}" ) - target_locations.remove(target_location) target_location.status = "current" - # continue with moves - while len(current_locations) > 0 and len(target_locations) > 0: - current_location = current_locations[0] - target_location = target_locations[0] - - current_path = current_location.full_local_path( - ShuttleContext.local_warehouse_paths - ) - target_path = target_location.full_local_path( - ShuttleContext.local_warehouse_paths - ) - - target_path.parent.mkdir(parents=True, exist_ok=True) - shutil.copy(current_path, target_path) - current_path.unlink() - logger.debug(f"Moved book {book.id} from {current_path} to {target_path}") - book.events.append( - f"{getnow()}: moved book from {current_location.full_str} to " - f"{target_location.full_str}" - ) - current_locations.remove(current_location) - target_locations.remove(target_location) - book.locations.remove(current_location) - session.delete(current_location) - session.flush() - target_location.status = "current" - - # cleanup phase: delete extra current locations - while len(current_locations) > 0: - current_location = current_locations[0] - current_path = current_location.full_local_path( + # After making one copy, delete one of the current locations + # that is not the original to avoid filling disk up with copies + if len(current_locations) > 1: + loc_to_delete = current_locations.pop() + del_path = loc_to_delete.full_local_path( + ShuttleContext.local_warehouse_paths + ) + del_path.unlink(missing_ok=True) + logger.debug(f"Deleted old location for book {book.id} at {del_path}") + book.events.append( + f"{getnow()}: deleted old location {loc_to_delete.full_str}" + ) + book.locations.remove(loc_to_delete) + session.delete(loc_to_delete) + + # Cleanup the original source location and any extra location (if we + # started with more currents than targets) + for current_location in current_locations: + del_path = current_location.full_local_path( ShuttleContext.local_warehouse_paths ) - - current_path.unlink(missing_ok=True) - logger.debug( - f"Deleted extra current location for book {book.id} at {current_path}" - ) + del_path.unlink(missing_ok=True) + logger.debug(f"Deleted old location for book {book.id} at {del_path}") book.events.append( f"{getnow()}: deleted old location {current_location.full_str}" ) - current_locations.remove(current_location) book.locations.remove(current_location) session.delete(current_location) book.needs_file_operation = False + session.flush() diff --git a/backend/tests/shuttle/test_move_files.py b/backend/tests/shuttle/test_move_files.py index 5ab9811..e4f25cf 100644 --- a/backend/tests/shuttle/test_move_files.py +++ b/backend/tests/shuttle/test_move_files.py @@ -141,9 +141,9 @@ def test_move_book_files_copy_operation( assert book.needs_processing is False assert book.has_error is False assert book.needs_file_operation is False + assert sum(1 for loc in book.locations if loc.status == "current") == 2 assert any("copied book from" in event for event in book.events) - # One target should now be current - assert sum(1 for loc in book.locations if loc.status == "current") >= 1 + assert any("deleted old location" in event for event in book.events) def test_move_book_files_move_operation( @@ -180,14 +180,15 @@ def test_move_book_files_move_operation( mock_context.local_warehouse_paths = {warehouse.id: Path("/warehouse")} move_book_files(dbsession, book) - # Should have moved once + # Should have copied once and unlinked once assert mock_copy.call_count == 1 assert mock_unlink.call_count == 1 assert book.needs_processing is False assert book.has_error is False assert book.needs_file_operation is False - assert any("moved book from" in event for event in book.events) + assert any("copied book from" in event for event in book.events) + assert any("deleted old location" in event for event in book.events) # Current location should be removed assert len([loc for loc in book.locations if loc.status == "current"]) == 1 @@ -229,14 +230,16 @@ def test_move_book_files_delete_operation( mock_context.local_warehouse_paths = {warehouse.id: Path("/warehouse")} move_book_files(dbsession, book) - # Should have deleted one extra location + # Should have copied once and deleted two extra locations assert mock_copy.call_count == 1 assert mock_unlink.call_count == 2 assert book.needs_processing is False assert book.has_error is False assert book.needs_file_operation is False + assert any("copied book from" in event for event in book.events) assert any("deleted old location" in event for event in book.events) + assert len([loc for loc in book.locations if loc.status == "current"]) == 1 def test_move_book_files_updates_book_locations(