fix: a non-image file lands on the source it was downloaded for, and the undated shells it left are folded into their real posts (#4435)
CI and images / lint (push) Successful in 3s
CI and images / extension-version (push) Successful in 3s
CI and images / extension-test (push) Successful in 18s
CI and images / frontend-build (push) Successful in 22s
CI and images / backend-lint-and-test (push) Successful in 32s
CI and images / integration (push) Successful in 2m22s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 5s
CI and images / build-web (push) Successful in 1m39s
CI and images / smoke-web (push) Successful in 54s
CI and images / promote (push) Successful in 1s
CI and images / lint (push) Successful in 3s
CI and images / extension-version (push) Successful in 3s
CI and images / extension-test (push) Successful in 18s
CI and images / frontend-build (push) Successful in 22s
CI and images / backend-lint-and-test (push) Successful in 32s
CI and images / integration (push) Successful in 2m22s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 5s
CI and images / build-web (push) Successful in 1m39s
CI and images / smoke-web (push) Successful in 54s
CI and images / promote (push) Successful in 1s
The attachment path looked the post's source up by (artist, platform), taking the artist's lowest-id source. A Discord artist has one source per channel, so every archive or pdf from a later channel became an undated second post on the first channel's source. _post_for_sidecar now takes the downloading source, as upsert_post_record and _apply_sidecar already did. Migration 0114 re-points each shell's attachments to its dated twin and deletes the shell. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
@@ -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
|
||||||
@@ -426,11 +426,19 @@ class Importer:
|
|||||||
return self._upsert_artist(name) if name else None
|
return self._upsert_artist(name) if name else None
|
||||||
|
|
||||||
def _post_for_sidecar(
|
def _post_for_sidecar(
|
||||||
self, source: Path, artist: Artist | None
|
self, source: Path, artist: Artist | None,
|
||||||
|
*, source_row: Source | None = None,
|
||||||
) -> Post | None:
|
) -> Post | None:
|
||||||
"""If a sidecar sits next to `source`, ensure its Source+Post
|
"""If a sidecar sits next to `source`, ensure its Source+Post
|
||||||
exist (idempotent) and return the Post — so attachments can link
|
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)
|
sc = find_sidecar(source)
|
||||||
if sc is None or artist is None:
|
if sc is None or artist is None:
|
||||||
return None
|
return None
|
||||||
@@ -442,6 +450,9 @@ class Importer:
|
|||||||
log.warning("sidecar parse failed for %s: %s", sc, exc)
|
log.warning("sidecar parse failed for %s: %s", sc, exc)
|
||||||
return None
|
return None
|
||||||
sd = parse_sidecar(data)
|
sd = parse_sidecar(data)
|
||||||
|
if source_row is not None:
|
||||||
|
src = source_row
|
||||||
|
else:
|
||||||
platform = sd.platform or "unknown"
|
platform = sd.platform or "unknown"
|
||||||
src = self._lookup_source_for_sidecar(
|
src = self._lookup_source_for_sidecar(
|
||||||
artist_id=artist.id, platform=platform,
|
artist_id=artist.id, platform=platform,
|
||||||
@@ -543,7 +554,7 @@ class Importer:
|
|||||||
# nothing silently vanishes, matching extract_archive's
|
# nothing silently vanishes, matching extract_archive's
|
||||||
# fail-soft contract.
|
# fail-soft contract.
|
||||||
artist_use = artist if artist is not None else self._resolve_artist(source)
|
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(
|
self._capture_attachment(
|
||||||
source, post=post, artist=artist_use, resolved=True,
|
source, post=post, artist=artist_use, resolved=True,
|
||||||
)
|
)
|
||||||
@@ -552,7 +563,7 @@ class Importer:
|
|||||||
return ImportResult(status="attached", error=reason)
|
return ImportResult(status="attached", error=reason)
|
||||||
|
|
||||||
artist_use = artist if artist is not None else self._resolve_artist(source)
|
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] = []
|
member_ids: list[int] = []
|
||||||
# Every member image touched (new + superseded + deduped), so the
|
# Every member image touched (new + superseded + deduped), so the
|
||||||
# from_attachment_id stamp below covers files that already existed in 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,
|
path, artist=artist, source_row=source,
|
||||||
)
|
)
|
||||||
if not is_supported(path):
|
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(
|
return self._capture_attachment(
|
||||||
path, post=post, artist=artist, resolved=True,
|
path, post=post, artist=artist, resolved=True,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -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
|
||||||
@@ -435,3 +435,48 @@ def test_attach_in_place_non_media_routes_to_attachment(importer, db_sync):
|
|||||||
).scalar_one()
|
).scalar_one()
|
||||||
assert row.ext == ".txt"
|
assert row.ext == ".txt"
|
||||||
assert row.artist_id == artist.id
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user