diff --git a/alembic/versions/0114_fold_misfiled_attachment_posts.py b/alembic/versions/0114_fold_misfiled_attachment_posts.py new file mode 100644 index 0000000..4356898 --- /dev/null +++ b/alembic/versions/0114_fold_misfiled_attachment_posts.py @@ -0,0 +1,79 @@ +"""Fold the undated shell posts a misfiled attachment created into the real post. + +#4435. A non-image file (an archive, a pdf) downloaded for one of an artist's +Discord channels was filed under the artist's FIRST Discord source: the +attachment path looked the source up by (artist, platform), which takes the +lowest id. That created an undated, url-less post there holding only the +attachment, and the message's real post record then created the dated post +under the right source. The importer now uses the source it was downloading +for; this repairs the pairs it left. + +A shell is folded only when all of this holds: + +* it has no date and no url, and nothing synthesized it; +* another post of the same artist, platform and external id HAS a date; +* no image is linked to the shell (it held an attachment and nothing else). + +Its attachments move to the dated post, dropping any the dated post already +has (same sha256, which the per-post unique forbids twice), and the shell is +deleted. A shell with no dated twin is left alone: which channel it belongs to +is not recorded anywhere but the file name. The downgrade does nothing. + +Revision ID: 0114 +Revises: 0113 +Create Date: 2026-09-25 + +""" +import sqlalchemy as sa +from alembic import op + +revision = "0114" +down_revision = "0113" +branch_labels = None +depends_on = None + +_PAIRS = """ + SELECT DISTINCT ON (shell.id) shell.id AS shell_id, real.id AS real_id + FROM post shell + JOIN source ss ON ss.id = shell.source_id + JOIN post real + ON real.artist_id = shell.artist_id + AND real.external_post_id = shell.external_post_id + AND real.id <> shell.id + AND real.post_date IS NOT NULL + JOIN source rs ON rs.id = real.source_id AND rs.platform = ss.platform + WHERE shell.post_date IS NULL + AND shell.post_url IS NULL + AND shell.synthesized_by IS NULL + AND NOT EXISTS (SELECT 1 FROM image_provenance ip WHERE ip.post_id = shell.id) + ORDER BY shell.id, real.id +""" + + +def fold_misfiled_attachment_posts(conn) -> int: + """The data step, on a plain connection, so a test can run it directly. + Returns how many shells were folded.""" + pairs = conn.execute(sa.text(_PAIRS)).all() + for shell_id, real_id in pairs: + conn.execute(sa.text(""" + DELETE FROM post_attachment pa + WHERE pa.post_id = :shell + AND EXISTS ( + SELECT 1 FROM post_attachment keep + WHERE keep.post_id = :real AND keep.sha256 = pa.sha256 + ) + """), {"shell": shell_id, "real": real_id}) + conn.execute( + sa.text("UPDATE post_attachment SET post_id = :real WHERE post_id = :shell"), + {"shell": shell_id, "real": real_id}, + ) + conn.execute(sa.text("DELETE FROM post WHERE id = :shell"), {"shell": shell_id}) + return len(pairs) + + +def upgrade() -> None: + fold_misfiled_attachment_posts(op.get_bind()) + + +def downgrade() -> None: + pass diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index 8a3b4f4..9e28d07 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -426,11 +426,19 @@ class Importer: return self._upsert_artist(name) if name else None def _post_for_sidecar( - self, source: Path, artist: Artist | None + self, source: Path, artist: Artist | None, + *, source_row: Source | None = None, ) -> Post | None: """If a sidecar sits next to `source`, ensure its Source+Post exist (idempotent) and return the Post — so attachments can link - to the same Post the per-member _apply_sidecar will reuse.""" + to the same Post the per-member _apply_sidecar will reuse. + + `source_row` is the subscription being downloaded, when there is one, + and wins over the (artist, platform) lookup — as it does in + `upsert_post_record`. The lookup takes the artist's FIRST source on the + platform, which is right only while an artist has one: a Discord artist + has one per channel, and every non-image file from a later channel was + filed under the first as an undated second post (#4435).""" sc = find_sidecar(source) if sc is None or artist is None: return None @@ -442,10 +450,13 @@ class Importer: log.warning("sidecar parse failed for %s: %s", sc, exc) return None sd = parse_sidecar(data) - platform = sd.platform or "unknown" - src = self._lookup_source_for_sidecar( - artist_id=artist.id, platform=platform, - ) + if source_row is not None: + src = source_row + else: + platform = sd.platform or "unknown" + src = self._lookup_source_for_sidecar( + artist_id=artist.id, platform=platform, + ) epid = sd.external_post_id or sc.stem return self._find_or_create_post( source_id=src.id if src else None, @@ -543,7 +554,7 @@ class Importer: # nothing silently vanishes, matching extract_archive's # fail-soft contract. artist_use = artist if artist is not None else self._resolve_artist(source) - post = self._post_for_sidecar(source, artist_use) + post = self._post_for_sidecar(source, artist_use, source_row=source_row) self._capture_attachment( source, post=post, artist=artist_use, resolved=True, ) @@ -552,7 +563,7 @@ class Importer: return ImportResult(status="attached", error=reason) artist_use = artist if artist is not None else self._resolve_artist(source) - post = self._post_for_sidecar(source, artist_use) + post = self._post_for_sidecar(source, artist_use, source_row=source_row) member_ids: list[int] = [] # Every member image touched (new + superseded + deduped), so the # from_attachment_id stamp below covers files that already existed in the @@ -1218,7 +1229,10 @@ class Importer: path, artist=artist, source_row=source, ) if not is_supported(path): - post = self._post_for_sidecar(path, artist) if artist else None + post = ( + self._post_for_sidecar(path, artist, source_row=source) + if artist else None + ) return self._capture_attachment( path, post=post, artist=artist, resolved=True, ) diff --git a/tests/test_fold_misfiled_attachment_posts_migration.py b/tests/test_fold_misfiled_attachment_posts_migration.py new file mode 100644 index 0000000..ebc8546 --- /dev/null +++ b/tests/test_fold_misfiled_attachment_posts_migration.py @@ -0,0 +1,106 @@ +"""Migration 0114 (#4435): the undated shell post a misfiled attachment made on +the artist's first Discord source is folded into the real, dated post.""" +import importlib.util +from datetime import UTC, datetime +from pathlib import Path + +import pytest +from sqlalchemy import select + +from backend.app.models import ( + Artist, + ImageProvenance, + Post, + PostAttachment, + Source, +) +from tests.factories import make_image as _img + +pytestmark = pytest.mark.integration + +_MIGRATION = ( + Path(__file__).resolve().parents[1] + / "alembic" / "versions" / "0114_fold_misfiled_attachment_posts.py" +) + + +def _fold(): + spec = importlib.util.spec_from_file_location("m0114", _MIGRATION) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod.fold_misfiled_attachment_posts + + +def _source(db, artist, channel): + s = Source( + artist_id=artist.id, platform="discord", + url=f"https://discord.com/channels/1/{channel}", + ) + db.add(s) + db.flush() + return s + + +def _post(db, artist, source, epid, when=None): + p = Post( + artist_id=artist.id, source_id=source.id, external_post_id=epid, + post_date=when, + post_url=f"https://discord.com/channels/1/x/{epid}" if when else None, + ) + db.add(p) + db.flush() + return p + + +def _attach(db, post, sha, name): + db.add(PostAttachment( + post_id=post.id, artist_id=post.artist_id, sha256=sha, + path=f"/att/{sha}", original_filename=name, ext=".rar", size_bytes=1, + )) + db.flush() + + +def test_shells_fold_into_their_dated_twin_and_nothing_else_moves(db_sync): + sent = datetime(2024, 12, 26, 1, 42, tzinfo=UTC) + artist = Artist(name="Yellow", slug="yellow") + db_sync.add(artist) + db_sync.flush() + first = _source(db_sync, artist, 100) + second = _source(db_sync, artist, 200) + + # The #4435 shape: shell on the first source, real post on the second. + shell = _post(db_sync, artist, first, "555") + real = _post(db_sync, artist, second, "555", sent) + _attach(db_sync, shell, "a" * 64, "pack.rar") + # A shell whose attachment the real post already has: dropped, not doubled. + shell2 = _post(db_sync, artist, first, "556") + real2 = _post(db_sync, artist, second, "556", sent) + _attach(db_sync, shell2, "b" * 64, "same.rar") + _attach(db_sync, real2, "b" * 64, "same.rar") + # Left alone: no dated twin, and an undated post that holds an image. + lonely = _post(db_sync, artist, first, "557") + _attach(db_sync, lonely, "c" * 64, "lonely.rar") + with_image = _post(db_sync, artist, first, "558") + _post(db_sync, artist, second, "558", sent) + img = _img(db_sync, "d" * 64) + db_sync.add(ImageProvenance(image_record_id=img.id, post_id=with_image.id)) + db_sync.flush() + shell_id, shell2_id = shell.id, shell2.id + + folded = _fold()(db_sync.connection()) + db_sync.expire_all() + + assert folded == 2 + assert db_sync.get(Post, shell_id) is None + assert db_sync.get(Post, shell2_id) is None + owners = dict(db_sync.execute( + select(PostAttachment.original_filename, PostAttachment.post_id) + ).all()) + assert owners["pack.rar"] == real.id + assert owners["lonely.rar"] == lonely.id + kept = db_sync.execute( + select(PostAttachment.id).where(PostAttachment.post_id == real2.id) + ).scalars().all() + assert len(kept) == 1 + assert db_sync.get(Post, lonely.id) is not None + assert db_sync.get(Post, with_image.id) is not None diff --git a/tests/test_importer_attach_in_place.py b/tests/test_importer_attach_in_place.py index ab08a06..47ae24d 100644 --- a/tests/test_importer_attach_in_place.py +++ b/tests/test_importer_attach_in_place.py @@ -435,3 +435,48 @@ def test_attach_in_place_non_media_routes_to_attachment(importer, db_sync): ).scalar_one() assert row.ext == ".txt" assert row.artist_id == artist.id + + +def test_a_non_media_file_lands_on_the_source_being_downloaded(importer, db_sync): + """#4435: a Discord artist has one source per channel. A non-media file + downloaded for the SECOND channel was filed under the artist's first + Discord source (the (artist, platform) lookup takes the lowest id), as an + undated shell post; the real post record then made a second post under + the right source. The attachment must follow the source it was fetched for.""" + from backend.app.models import Post, PostAttachment, Source + + images_root = importer.images_root + artist = Artist(name="Yara", slug="yara") + db_sync.add(artist) + db_sync.flush() + first = Source( + artist_id=artist.id, platform="discord", + url="https://discord.com/channels/1/100", + ) + second = Source( + artist_id=artist.id, platform="discord", + url="https://discord.com/channels/1/200", + ) + db_sync.add_all([first, second]) + db_sync.flush() + + rar = images_root / "yara" / "discord" / "rewards" / "20241226_555_01_pack.rar" + rar.parent.mkdir(parents=True, exist_ok=True) + rar.write_bytes(b"not a real archive, so it is kept as an attachment") + rar.with_suffix(rar.suffix + ".json").write_text( + '{"category": "discord", "id": "555", "message_id": "555"}' + ) + + result = importer.attach_in_place(rar, artist=artist, source=second) + assert result.status == "attached" + + owner = db_sync.execute( + select(Post.source_id, Post.external_post_id) + .join(PostAttachment, PostAttachment.post_id == Post.id) + .where(PostAttachment.original_filename == rar.name) + ).one() + assert owner.source_id == second.id + assert owner.external_post_id == "555" + assert db_sync.execute( + select(func.count()).select_from(Post).where(Post.source_id == first.id) + ).scalar_one() == 0