From fd92a1d1eb096dd4b1d7e481edfc6b7e6e392062 Mon Sep 17 00:00:00 2001 From: Phil Skentelbery Date: Tue, 16 Dec 2025 11:10:21 -0700 Subject: [PATCH] fix: Clean up variant files in delete_media() - v1.4.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following design approved by architect in docs/design/v1.4.0/ Changes to starpunk/media.py: - Update delete_media() to fetch and delete variant files from disk - Query media_variants table before deletion for file paths - Use best-effort cleanup with try/except for each file - Log individual file deletion failures as warnings - Update final log to show total files deleted (original + variants) - Update module docstring to reflect v1.4.0 capabilities: * 50MB max upload, 10MB max output * Image variants (thumb, small, medium, large) * Tiered resize strategy This fixes the issue where variant files were left orphaned on disk when media was deleted. The database CASCADE already deleted variant records, but the physical files remained. All tests pass: uv run pytest tests/test_media_upload.py -v (23/23) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- starpunk/media.py | 46 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/starpunk/media.py b/starpunk/media.py index e1fd534..fa39717 100644 --- a/starpunk/media.py +++ b/starpunk/media.py @@ -4,8 +4,10 @@ Media upload and management for StarPunk Per ADR-057 and ADR-058: - Social media attachment model (media at top of note) - Pillow-based image optimization -- 10MB max file size, 4096x4096 max dimensions -- Auto-resize to 2048px for performance +- 50MB max upload, 10MB max output (v1.4.0) +- Image variants: thumb, small, medium, large (v1.4.0) +- Tiered resize strategy based on input size (v1.4.0) +- 4096x4096 max dimensions - 4 images max per note """ @@ -594,9 +596,10 @@ def get_note_media(note_id: int) -> List[Dict]: def delete_media(media_id: int) -> None: """ - Delete media file and database record + Delete media file, variants, and database record Per Q8: Cleanup orphaned files + Per v1.4.0: Also cleanup variant files Args: media_id: Media ID to delete @@ -611,15 +614,38 @@ def delete_media(media_id: int) -> None: return media_path = row[0] + media_dir = Path(current_app.config.get('DATA_PATH', 'data')) / 'media' - # Delete database record (cascade will delete note_media entries) + # Get variant paths before deleting (v1.4.0) + variant_rows = db.execute( + "SELECT path FROM media_variants WHERE media_id = ?", + (media_id,) + ).fetchall() + + # Delete database record (cascade will delete media_variants and note_media entries) db.execute("DELETE FROM media WHERE id = ?", (media_id,)) db.commit() - # Delete file from disk - media_dir = Path(current_app.config.get('DATA_PATH', 'data')) / 'media' - full_path = media_dir / media_path + # Delete files from disk (best-effort cleanup) + deleted_count = 0 - if full_path.exists(): - full_path.unlink() - current_app.logger.info(f"Deleted media file: {media_path}") + # Delete original file + full_path = media_dir / media_path + try: + if full_path.exists(): + full_path.unlink() + deleted_count += 1 + except OSError as e: + current_app.logger.warning(f"Failed to delete media file {media_path}: {e}") + + # Delete variant files (v1.4.0) + for variant_row in variant_rows: + variant_path = media_dir / variant_row[0] + try: + if variant_path.exists(): + variant_path.unlink() + deleted_count += 1 + except OSError as e: + current_app.logger.warning(f"Failed to delete variant file {variant_row[0]}: {e}") + + current_app.logger.info(f"Deleted media {media_id}: {deleted_count} file(s) removed from disk")