fix(importer): Source = one per (artist, platform), not one per post — filesystem importer's sidecar paths now reuse the artist's existing subscription Source (or create one synthetic anchor with enabled=False) instead of fabricating a new Source per post URL. Alembic 0022 consolidates existing per-post Sources to canonical (prefers campaign URL; falls back to sidecar:<platform>:<slug>) and re-parents Posts + ImageProvenance, merging Post collisions.
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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/<id>$' (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:<platform>:<artist_slug>' 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/<token> (gallery-dl-style per-post URL)."""
|
||||
import re
|
||||
return bool(re.search(_POST_URL_RE, url or ""))
|
||||
@@ -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:<platform>:<artist_slug>' 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
|
||||
|
||||
@@ -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:<plat>:<slug>')
|
||||
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"
|
||||
|
||||
Reference in New Issue
Block a user