From 0bc5767a2bb848bffbfd42f416ab9e8e9a3855f1 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 26 May 2026 14:19:29 -0400 Subject: [PATCH] =?UTF-8?q?fix(importer):=20Source=20=3D=20one=20per=20(ar?= =?UTF-8?q?tist,=20platform),=20not=20one=20per=20post=20=E2=80=94=20files?= =?UTF-8?q?ystem=20importer's=20sidecar=20paths=20now=20reuse=20the=20arti?= =?UTF-8?q?st's=20existing=20subscription=20Source=20(or=20create=20one=20?= =?UTF-8?q?synthetic=20anchor=20with=20enabled=3DFalse)=20instead=20of=20f?= =?UTF-8?q?abricating=20a=20new=20Source=20per=20post=20URL.=20Alembic=200?= =?UTF-8?q?022=20consolidates=20existing=20per-post=20Sources=20to=20canon?= =?UTF-8?q?ical=20(prefers=20campaign=20URL;=20falls=20back=20to=20sidecar?= =?UTF-8?q?::)=20and=20re-parents=20Posts=20+=20ImageProve?= =?UTF-8?q?nance,=20merging=20Post=20collisions.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator-flagged 2026-05-26: Atole artist detail page showed 406 Sources where 1 was right. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../0022_source_per_artist_platform.py | 181 ++++++++++++++++++ backend/app/services/importer.py | 67 ++++++- tests/test_importer_upsert_helpers.py | 65 +++++++ 3 files changed, 307 insertions(+), 6 deletions(-) create mode 100644 alembic/versions/0022_source_per_artist_platform.py diff --git a/alembic/versions/0022_source_per_artist_platform.py b/alembic/versions/0022_source_per_artist_platform.py new file mode 100644 index 0000000..eaee97f --- /dev/null +++ b/alembic/versions/0022_source_per_artist_platform.py @@ -0,0 +1,181 @@ +"""source-collapse: one Source per (artist, platform) — consolidate junk per-post Sources + +Revision ID: 0022 +Revises: 0021 +Create Date: 2026-05-26 + +Closes the operator-flagged 2026-05-26 issue where the filesystem importer +called _find_or_create_source(url=sd.post_url), creating one Source row per +imported post URL. Operator's Atole artist had 406 Source rows where there +should have been 1 (the /cw/Atole subscription Source). + +Source represents a subscription feed (one per artist+platform — the +gallery-dl URL polled by the FC-3 downloader). Posts hang off it. The +filesystem importer was misusing Source as a per-post key. + +Migration steps per (artist_id, platform) group with >1 Source: + 1. Pick canonical — prefer a URL NOT matching '/posts/$' (real + campaign URL like /cw/Atole); else min(id). + 2. Reparent Posts and ImageProvenance off the other Sources onto canonical. + 3. Merge any Posts that collide on (canonical_source_id, external_post_id) + by repointing ImageProvenance + ImageRecord.primary_post_id to the + earliest Post, dedupe ImageProvenance, delete the loser Posts. + 4. Delete the orphan Source rows. + 5. If the canonical Source's URL still looks like a per-post URL (no + campaign URL existed among candidates), rewrite it to + 'sidecar::' so the artist detail page shows + something readable. +""" +from typing import Sequence, Union + +from alembic import op +from sqlalchemy import text + +revision: str = "0022" +down_revision: Union[str, None] = "0021" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +_POST_URL_RE = r"/posts/[^/]+$" + + +def upgrade() -> None: + conn = op.get_bind() + + # Find (artist_id, platform) groups with > 1 Source row. + groups = conn.execute(text(""" + SELECT artist_id, platform + FROM source + GROUP BY artist_id, platform + HAVING COUNT(*) > 1 + """)).fetchall() + + for artist_id, platform in groups: + rows = conn.execute( + text(""" + SELECT id, url FROM source + WHERE artist_id = :a AND platform = :p + ORDER BY id ASC + """), + {"a": artist_id, "p": platform}, + ).fetchall() + + # Canonical: first row whose URL doesn't look like a per-post URL; + # else min(id). + canonical_id = None + for sid, url in rows: + if not _matches_post_url(url): + canonical_id = sid + break + if canonical_id is None: + canonical_id = rows[0][0] + + other_ids = [sid for sid, _ in rows if sid != canonical_id] + if not other_ids: + continue + + # Reparent Posts off the other Sources. + conn.execute( + text(""" + UPDATE post SET source_id = :canonical + WHERE source_id = ANY(:others) + """), + {"canonical": canonical_id, "others": other_ids}, + ) + # Reparent ImageProvenance.source_id similarly (denormalized FK). + conn.execute( + text(""" + UPDATE image_provenance SET source_id = :canonical + WHERE source_id = ANY(:others) + """), + {"canonical": canonical_id, "others": other_ids}, + ) + + # Merge any Posts colliding on (canonical, external_post_id) after + # reparent. In practice within a single (artist, platform) group + # these should be rare — each gallery-dl post has a unique id — but + # safe to handle. + post_collisions = conn.execute( + text(""" + SELECT external_post_id, ARRAY_AGG(id ORDER BY id) AS ids + FROM post + WHERE source_id = :canonical + GROUP BY external_post_id + HAVING COUNT(*) > 1 + """), + {"canonical": canonical_id}, + ).fetchall() + for _epid, post_ids in post_collisions: + keep = post_ids[0] + drops = post_ids[1:] + conn.execute( + text(""" + UPDATE image_provenance SET post_id = :keep + WHERE post_id = ANY(:drops) + """), + {"keep": keep, "drops": drops}, + ) + conn.execute( + text(""" + UPDATE image_record SET primary_post_id = :keep + WHERE primary_post_id = ANY(:drops) + """), + {"keep": keep, "drops": drops}, + ) + # Repointed provenance rows may now collide on + # uq_image_provenance_image_post (alembic 0021). Same dedupe + # pattern: keep min(id) per (image_record_id, post_id). + conn.execute(text(""" + DELETE FROM image_provenance ip1 + USING image_provenance ip2 + WHERE ip1.image_record_id = ip2.image_record_id + AND ip1.post_id = ip2.post_id + AND ip1.id > ip2.id + """)) + conn.execute( + text("DELETE FROM post WHERE id = ANY(:drops)"), + {"drops": drops}, + ) + + # Drop the orphan Sources. + conn.execute( + text("DELETE FROM source WHERE id = ANY(:others)"), + {"others": other_ids}, + ) + + # If the canonical's URL still looks per-post (no campaign URL + # existed among the candidates), rewrite to a synthetic anchor so + # the artist detail page renders something readable. + canonical_url = conn.execute( + text("SELECT url FROM source WHERE id = :id"), + {"id": canonical_id}, + ).scalar_one() + if _matches_post_url(canonical_url): + slug = conn.execute( + text("SELECT slug FROM artist WHERE id = :id"), + {"id": artist_id}, + ).scalar_one() + conn.execute( + text(""" + UPDATE source + SET url = :new_url, enabled = false + WHERE id = :id + """), + { + "id": canonical_id, + "new_url": f"sidecar:{platform}:{slug}", + }, + ) + + +def downgrade() -> None: + # Lossy migration — orphan Sources deleted, Posts reparented, Posts + # merged. No safe downgrade. If you need to roll back the schema + # invariant, fork from 0021 and re-run filesystem imports. + pass + + +def _matches_post_url(url: str) -> bool: + """True if url ends with /posts/ (gallery-dl-style per-post URL).""" + import re + return bool(re.search(_POST_URL_RE, url or "")) diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index 997a853..e2b94eb 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -247,6 +247,62 @@ class Importer: ) ).scalar_one() + def _source_for_sidecar( + self, *, artist_id: int, platform: str, artist_slug: str, + ) -> Source: + """Filesystem-import sidecar Source resolver. + + Source represents a subscription feed (one per artist+platform — the + gallery-dl URL polled by the FC-3 downloader). The filesystem importer + used to call _find_or_create_source(url=sd.post_url), which created + one Source row per post URL — 100s of junk Sources per artist, all + with enabled=True, polluting the artist detail page and tricking the + subscription checker into trying to poll patreon post URLs as feeds. + Operator-flagged 2026-05-26. + + New behaviour: if any Source row exists for (artist_id, platform), + reuse it regardless of its URL — the artist's real subscription Source + (created by the downloader / extension / UI) is the canonical + attachment point for filesystem-imported posts. If none exists, create + ONE synthetic anchor with url='sidecar::' and + enabled=False (so the subscription checker doesn't poll it). + """ + existing = self.session.execute( + select(Source) + .where( + Source.artist_id == artist_id, + Source.platform == platform, + ) + .order_by(Source.id.asc()) + .limit(1) + ).scalar_one_or_none() + if existing is not None: + return existing + synthetic_url = f"sidecar:{platform}:{artist_slug}" + sp = self.session.begin_nested() + try: + row = Source( + artist_id=artist_id, + platform=platform, + url=synthetic_url, + enabled=False, + ) + self.session.add(row) + self.session.flush() + sp.commit() + return row + except IntegrityError: + sp.rollback() + return self.session.execute( + select(Source) + .where( + Source.artist_id == artist_id, + Source.platform == platform, + ) + .order_by(Source.id.asc()) + .limit(1) + ).scalar_one() + def _find_or_create_post( self, *, source_id: int, external_post_id: str, ) -> Post: @@ -315,9 +371,8 @@ class Importer: return None sd = parse_sidecar(data) platform = sd.platform or "unknown" - url = sd.post_url or f"sidecar:{platform}" - src = self._find_or_create_source( - artist_id=artist.id, platform=platform, url=url, + src = self._source_for_sidecar( + artist_id=artist.id, platform=platform, artist_slug=artist.slug, ) epid = sd.external_post_id or sc.stem return self._find_or_create_post( @@ -763,9 +818,9 @@ class Importer: src = explicit_source else: platform = sd.platform or "unknown" - url = sd.post_url or f"sidecar:{platform}" - src = self._find_or_create_source( - artist_id=artist.id, platform=platform, url=url, + src = self._source_for_sidecar( + artist_id=artist.id, platform=platform, + artist_slug=artist.slug, ) epid = sd.external_post_id or sc.stem diff --git a/tests/test_importer_upsert_helpers.py b/tests/test_importer_upsert_helpers.py index ca8a3bc..8472031 100644 --- a/tests/test_importer_upsert_helpers.py +++ b/tests/test_importer_upsert_helpers.py @@ -145,3 +145,68 @@ def test_find_or_create_source_recovers_from_integrity_error( artist_id=artist_row.id, platform="patreon", url=canonical_url, ) assert recovered.id == pre_existing.id + + +def test_source_for_sidecar_reuses_existing_subscription( + importer, artist_row, db_sync, +): + """The filesystem-import sidecar resolver should attach to whatever + Source already exists for (artist, platform) — the canonical subscription + Source — regardless of its URL. Without this, every imported post + spawned its own Source row. + """ + canonical = Source( + artist_id=artist_row.id, platform="patreon", + url="https://www.patreon.com/cw/testartist", enabled=True, + ) + db_sync.add(canonical) + db_sync.flush() + + resolved = importer._source_for_sidecar( + artist_id=artist_row.id, platform="patreon", + artist_slug=artist_row.slug, + ) + assert resolved.id == canonical.id + + +def test_source_for_sidecar_creates_synthetic_anchor_when_none_exists( + importer, artist_row, db_sync, +): + """No subscription Source for this (artist, platform) yet. The helper + creates one synthetic anchor (enabled=False, url='sidecar::') + so subsequent imports reuse it instead of spawning per-post Sources. + """ + resolved = importer._source_for_sidecar( + artist_id=artist_row.id, platform="pixiv", + artist_slug=artist_row.slug, + ) + assert resolved.url == f"sidecar:pixiv:{artist_row.slug}" + assert resolved.enabled is False + assert resolved.artist_id == artist_row.id + assert resolved.platform == "pixiv" + + # Second call returns the same row (no new Source spawned). + again = importer._source_for_sidecar( + artist_id=artist_row.id, platform="pixiv", + artist_slug=artist_row.slug, + ) + assert again.id == resolved.id + + +def test_source_for_sidecar_distinct_platforms_distinct_anchors( + importer, artist_row, db_sync, +): + """One synthetic anchor per (artist, platform). Different platforms get + different anchors even when no campaign Source exists for either. + """ + p = importer._source_for_sidecar( + artist_id=artist_row.id, platform="patreon", + artist_slug=artist_row.slug, + ) + x = importer._source_for_sidecar( + artist_id=artist_row.id, platform="pixiv", + artist_slug=artist_row.slug, + ) + assert p.id != x.id + assert p.platform == "patreon" + assert x.platform == "pixiv"