An attachment's file leaves the disk one way: storage.unlink_media

The purge wrote out its own try/unlink/log next to unlink_media, which says the
same thing. It couldn't import it: unlink_media lived in the notes package, which
imports retention. unlink_media moves down to storage.py, the module about what
attachments occupy, and the purge, the delete route and sync all call it.

DRY pass #2, batch 4, F10 (#5372).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-08 14:33:11 -04:00
co-authored by Claude Opus 5.5
parent f5478a0ce0
commit a34d469e1e
5 changed files with 22 additions and 21 deletions
+1 -2
View File
@@ -35,7 +35,7 @@ from ..note_state import OWN_FIELDS, archived_for, join_state, pinned_for, posit
from .checklist import append_item, parse_items, set_item_checked
from ..responses import json_error, not_found, parse_uuid
from ..retention import purge_note
from ..storage import OVER_LIMIT_STATUS, storage_room, upload_refusal
from ..storage import OVER_LIMIT_STATUS, storage_room, unlink_media, upload_refusal
from ..unfurl_queue import schedule as schedule_unfurls
from ._bp import bp
from .body import write_body
@@ -54,7 +54,6 @@ from .helpers import (
is_empty_note,
store_attachment,
top_position,
unlink_media,
parse_list_items,
)
from .import_export import (
-10
View File
@@ -211,16 +211,6 @@ def store_attachment(
return row
def unlink_media(rel: str) -> None:
"""Remove an attachment's file, after its row is gone. A file that is already
missing or can't be removed is logged, never raised: the row was what the person
asked to delete, and failing here would undo a deletion that has committed."""
try:
(Config.media_root() / rel).unlink(missing_ok=True)
except OSError:
logger.warning("couldn't remove attachment file %s", rel, exc_info=True)
def _header_filename(name: str) -> str:
"""Sanitize a filename for a Content-Disposition header (drop quotes/newlines)."""
return re.sub(r'[\r\n"]', "", name or "")[:255] or "file"
+2 -7
View File
@@ -38,6 +38,7 @@ from .models.note_revision import NoteRevision
from .models.share import Share
from .settings import get_setting
from .share_sync import recipients, revoke
from .storage import unlink_media
logger = logging.getLogger(__name__)
@@ -79,13 +80,7 @@ async def purge_note(db, note: Note, edited_at: datetime | None = None) -> None:
"""
atts = (await db.scalars(select(NoteAttachment).where(NoteAttachment.note_id == note.id))).all()
for a in atts:
try:
(Config.media_root() / a.path).unlink(missing_ok=True)
except OSError:
# A missing or unreadable file must not strand the row: the DB record is
# what the user asked us to destroy, and a failed unlink leaving it in
# place would make the note reappear whole on the next sweep.
logger.warning("couldn't remove attachment file %s during purge", a.path, exc_info=True)
unlink_media(a.path)
await db.execute(sa_delete(NoteAttachment).where(NoteAttachment.note_id == note.id))
await db.execute(sa_delete(NoteLabel).where(NoteLabel.note_id == note.id))
await db.execute(sa_delete(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))
+17
View File
@@ -12,18 +12,24 @@ An upload that would go over the limit is answered 507 Insufficient Storage, not
(`client::upload_attachment` in the core), and a 5xx as worth another try. Over the
limit is neither: freeing space should let the file through on the next sync, with
nothing for the person to redo.
The one way an attachment's file leaves the disk is here too (`unlink_media`).
"""
from __future__ import annotations
import logging
import uuid
from sqlalchemy import func, select
from .config import Config
from .models.note import Note
from .models.note_attachment import NoteAttachment
from .models.user import User
from .settings import get_setting
logger = logging.getLogger(__name__)
GB = 1024**3
OVER_LIMIT_STATUS = 507
@@ -76,3 +82,14 @@ async def upload_refusal(db, user_id: uuid.UUID, size: int) -> tuple[str, int] |
if limit is not None and size > limit - await storage_used(db, user_id):
return over_limit_message(limit), OVER_LIMIT_STATUS
return None
def unlink_media(rel: str) -> None:
"""Remove an attachment's file, after its row is gone. A file that is already
missing or can't be removed is logged, never raised: the row was what the person
asked to delete, and failing here would undo a deletion that has committed, or
strand the row so the note reappears whole on the next sweep."""
try:
(Config.media_root() / rel).unlink(missing_ok=True)
except OSError:
logger.warning("couldn't remove attachment file %s", rel, exc_info=True)
+2 -2
View File
@@ -38,11 +38,11 @@ from .notes import (
normalize_recurrence,
)
from .notes.body import write_body
from .notes.helpers import _get_editable, _get_visible, store_attachment, unlink_media
from .notes.helpers import _get_editable, _get_visible, store_attachment
from .responses import json_error, not_found, parse_uuid
from .retention import purge_note
from .serialize import serialize_label_sync
from .storage import upload_refusal
from .storage import unlink_media, upload_refusal
from .unfurl_queue import schedule as schedule_unfurls
bp = Blueprint("sync", __name__, url_prefix="/api/sync")