From 6fc8ae3106b4fe702128727ebaf505700026133f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 31 May 2026 23:08:38 -0400 Subject: [PATCH] fix(subscriptions): hide sidecar synthetic Sources + prefer real on lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled bugs surfaced 2026-05-31 by the Subscriptions UI showing "phantom" subscriptions like `sidecar:patreon:dpmaker`: 1. `SourceService.list()` returned every Source, no filter on URL. alembic 0022 (2026-05-26) consolidated old per-post-URL Sources into one canonical row per (artist, platform); when no real campaign URL was salvageable it rewrote the canonical to `sidecar::` enabled=false as a disabled anchor. The UI then listed those anchors as if they were polls — disabled, but visible. Fix: `list()` excludes `url LIKE 'sidecar:%'` by default; `include_synthetic=True` opts back in for admin tooling. 2. `importer._source_for_sidecar` picked the lowest-id Source for (artist, platform). When alembic 0022 had rewritten a per-post row into a synthetic anchor (lower id) AND the operator later added the real subscription (higher id), every gallery-dl download silently attached its Post to the SYNTHETIC instead of the real Source. Fix: prefer a non-`sidecar:%` URL when one exists; fall back to the synthetic; only create a new synthetic when nothing exists for (artist, platform). alembic 0028 is the data half: for every (artist, platform) with both a synthetic AND a real Source, pre-merge Post+ImageProvenance collisions on the canonical, bulk-repoint Posts/ImageProvenance/ DownloadEvent.source_id onto the real Source, and delete the synthetic. Lone synthetics (no real twin) are left intact — they anchor real imported content the operator may still want; the list-filter hides them so they no longer surface as phantoms. --- ...se_sidecar_synthetics_into_real_sources.py | 190 ++++++++++++++++++ backend/app/services/importer.py | 48 +++-- backend/app/services/source_service.py | 8 + tests/test_importer_upsert_helpers.py | 33 +++ tests/test_source_service.py | 38 +++- 5 files changed, 302 insertions(+), 15 deletions(-) create mode 100644 alembic/versions/0028_collapse_sidecar_synthetics_into_real_sources.py diff --git a/alembic/versions/0028_collapse_sidecar_synthetics_into_real_sources.py b/alembic/versions/0028_collapse_sidecar_synthetics_into_real_sources.py new file mode 100644 index 0000000..5ec9267 --- /dev/null +++ b/alembic/versions/0028_collapse_sidecar_synthetics_into_real_sources.py @@ -0,0 +1,190 @@ +"""collapse-sidecar-synthetic: repoint Posts/ImageProvenance/DownloadEvents +from `sidecar::` synthetic Source anchors onto the real +Source for the same (artist, platform) when one exists, then delete the +synthetic. + +Revision ID: 0028 +Revises: 0027 +Create Date: 2026-05-31 + +Background: alembic 0022 (2026-05-26) consolidated the old per-post-URL +Source rows into one canonical Source per (artist, platform). When NO +real campaign URL was salvageable among the candidates, it rewrote the +canonical row to url='sidecar::' enabled=false as a +disabled anchor for any Posts already attached. + +That was fine while it was the only Source for that artist+platform. +But: the unique constraint on Source is (artist_id, platform, url), not +(artist_id, platform). When the operator later added the real +subscription via the UI / extension / etc., a SECOND row landed — +the real one — with id > the synthetic. Both coexisted. + +Two follow-on problems surfaced 2026-05-31: + + 1. The Subscriptions UI listed both rows. The synthetic was disabled + so the scheduler never polled it, but it looked like a phantom + subscription. (Fixed in same commit by SourceService.list filter.) + 2. importer._source_for_sidecar picked Source by `ORDER BY id ASC + LIMIT 1`, so EVERY gallery-dl download since the real Source was + added attached its Post to the SYNTHETIC anchor, not the real + Source. (Fixed in same commit by preferring non-sidecar URLs.) + +This migration is the data half of the cleanup: for every (artist, +platform) with both a synthetic AND a real Source, repoint the +synthetic's children (Posts, ImageProvenance, DownloadEvents) onto the +real Source and delete the synthetic. Reuses the same epid/provenance +collision dance from alembic 0022 because the same uniqueness +constraints fire row-by-row during bulk UPDATEs. + +Lone synthetic anchors — those where no real Source for the same +(artist, platform) exists (e.g., filesystem-imported artist with no +subscription added) — are LEFT INTACT. They anchor real imported +content; deleting them would CASCADE-delete the Posts the operator +imported. The SourceService.list filter hides them from the UI; the +operator can delete them by hand if they want the underlying imports +gone. +""" +from typing import Sequence, Union + +from alembic import op +from sqlalchemy import text + +revision: str = "0028" +down_revision: Union[str, None] = "0027" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + conn = op.get_bind() + + # Find (artist_id, platform) groups where BOTH a sidecar synthetic + # and at least one real Source exist. + groups = conn.execute(text(""" + SELECT artist_id, platform + FROM source + GROUP BY artist_id, platform + HAVING bool_or(url LIKE 'sidecar:%') + AND bool_or(url NOT LIKE 'sidecar:%') + """)).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() + + synthetic_ids = [sid for sid, url in rows if url.startswith("sidecar:")] + real_rows = [(sid, url) for sid, url in rows if not url.startswith("sidecar:")] + if not synthetic_ids or not real_rows: + continue # belt+suspenders; the GROUP BY already filtered + + # Canonical real: lowest-id non-sidecar Source. + canonical_id = real_rows[0][0] + + # STEP A: PRE-merge Post collisions on (canonical, external_post_id). + # Mirror alembic 0022's pre-merge logic — when synth has Post X + # epid=N and real has Post Y epid=N, the bulk UPDATE below would + # trip uq_post_source_external_id row-by-row. Group all Posts + # under (canonical + synthetics) by epid; for any group >1, + # pick a keep (prefer one already under canonical, else lowest + # id) and merge the rest into it. + all_posts = conn.execute( + text(""" + SELECT external_post_id, id, source_id + FROM post + WHERE source_id = :canonical OR source_id = ANY(:synths) + ORDER BY external_post_id, id + """), + {"canonical": canonical_id, "synths": synthetic_ids}, + ).fetchall() + by_epid: dict = {} + for epid, post_id, src_id in all_posts: + by_epid.setdefault(epid, []).append((post_id, src_id)) + for _epid, posts in by_epid.items(): + if len(posts) <= 1: + continue + canonical_side = [p for p in posts if p[1] == canonical_id] + keep_id = canonical_side[0][0] if canonical_side else posts[0][0] + drop_ids = [p[0] for p in posts if p[0] != keep_id] + for drop_id in drop_ids: + # Pre-delete image_provenance rows under drop_ whose + # image_record_id already has provenance under keep — + # avoids tripping uq_image_provenance_image_post (0021) + # row-by-row during the repoint UPDATE. + conn.execute( + text(""" + DELETE FROM image_provenance + WHERE post_id = :drop_ + AND image_record_id IN ( + SELECT image_record_id FROM image_provenance + WHERE post_id = :keep + ) + """), + {"keep": keep_id, "drop_": drop_id}, + ) + conn.execute( + text(""" + UPDATE image_provenance SET post_id = :keep + WHERE post_id = :drop_ + """), + {"keep": keep_id, "drop_": drop_id}, + ) + conn.execute( + text(""" + UPDATE image_record SET primary_post_id = :keep + WHERE primary_post_id = :drop_ + """), + {"keep": keep_id, "drop_": drop_id}, + ) + conn.execute( + text("DELETE FROM post WHERE id = :drop_"), + {"drop_": drop_id}, + ) + + # STEP B: Bulk reparent the remaining Posts off the synthetics. + conn.execute( + text(""" + UPDATE post SET source_id = :canonical + WHERE source_id = ANY(:synths) + """), + {"canonical": canonical_id, "synths": synthetic_ids}, + ) + + # STEP C: Reparent ImageProvenance.source_id (denormalized FK; + # no UNIQUE on source_id, safe bulk). + conn.execute( + text(""" + UPDATE image_provenance SET source_id = :canonical + WHERE source_id = ANY(:synths) + """), + {"canonical": canonical_id, "synths": synthetic_ids}, + ) + + # STEP D: Reparent any DownloadEvent.source_id. Synthetics are + # enabled=false so the scheduler never created events for them; + # this is belt+suspenders for any rows planted by manual force + # or older code paths. + conn.execute( + text(""" + UPDATE download_event SET source_id = :canonical + WHERE source_id = ANY(:synths) + """), + {"canonical": canonical_id, "synths": synthetic_ids}, + ) + + # STEP E: Drop the now-empty synthetics. + conn.execute( + text("DELETE FROM source WHERE id = ANY(:synths)"), + {"synths": synthetic_ids}, + ) + + +def downgrade() -> None: + # Lossy migration — synthetic Sources deleted, Posts repointed and + # potentially merged. No safe downgrade. + pass diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index b722f67..c66caa8 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -261,24 +261,44 @@ class Importer: def _source_for_sidecar( self, *, artist_id: int, platform: str, artist_slug: str, ) -> Source: - """Filesystem-import sidecar Source resolver. + """Sidecar-import Source resolver. Used by both filesystem imports + and gallery-dl downloads (both write sidecar JSON, both flow through + _apply_sidecar / _capture_attachment). 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 + URL polled by the FC-3 downloader). The filesystem importer used to + call _find_or_create_source(url=sd.post_url), creating 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. + Operator-flagged 2026-05-26; consolidated via alembic 0022. - 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). + Resolution order: prefer a real (non-sidecar) Source over a + synthetic anchor. When alembic 0022 ran, it may have rewritten + per-post Sources into `sidecar::` synthetic + anchors. If the operator later added the real subscription, both + rows now coexist. A naive `ORDER BY id ASC LIMIT 1` lookup would + pick the older synthetic and silently attach every gallery-dl + download to the wrong Source — operator-flagged 2026-05-31 after + the Subscriptions UI surfaced the phantom anchors. Pick the real + one when one exists; fall back to the synthetic; only create a + new synthetic when nothing exists for (artist, platform). """ - stmt = ( + real_stmt = ( + select(Source) + .where( + Source.artist_id == artist_id, + Source.platform == platform, + ~Source.url.like("sidecar:%"), + ) + .order_by(Source.id.asc()) + .limit(1) + ) + real = self.session.execute(real_stmt).scalar_one_or_none() + if real is not None: + return real + + any_stmt = ( select(Source) .where( Source.artist_id == artist_id, @@ -288,7 +308,7 @@ class Importer: .limit(1) ) return self._get_or_create( - stmt, + any_stmt, lambda: Source( artist_id=artist_id, platform=platform, diff --git a/backend/app/services/source_service.py b/backend/app/services/source_service.py index 85a6564..57a9571 100644 --- a/backend/app/services/source_service.py +++ b/backend/app/services/source_service.py @@ -151,10 +151,18 @@ class SourceService: async def list( self, artist_id: int | None = None, failing: bool = False, + include_synthetic: bool = False, ) -> list[SourceRecord]: stmt = select(Source, Artist).join(Artist, Artist.id == Source.artist_id) if artist_id is not None: stmt = stmt.where(Source.artist_id == artist_id) + if not include_synthetic: + # Filesystem-import sidecar anchors (importer._source_for_sidecar) + # have url='sidecar::' and exist only to give + # imported Posts a NOT-NULL Source FK. They aren't pollable + # feeds; the Subscriptions UI used to render them as phantom + # subscriptions. Hide by default. + stmt = stmt.where(~Source.url.like("sidecar:%")) if failing: # Worst-first so the rollup card surfaces the loudest failures. stmt = stmt.where(Source.consecutive_failures > 0).order_by( diff --git a/tests/test_importer_upsert_helpers.py b/tests/test_importer_upsert_helpers.py index 8472031..7f8882e 100644 --- a/tests/test_importer_upsert_helpers.py +++ b/tests/test_importer_upsert_helpers.py @@ -210,3 +210,36 @@ def test_source_for_sidecar_distinct_platforms_distinct_anchors( assert p.id != x.id assert p.platform == "patreon" assert x.platform == "pixiv" + + +def test_source_for_sidecar_prefers_real_over_synthetic_when_both_exist( + importer, artist_row, db_sync, +): + """When BOTH a synthetic anchor AND a real Source exist for the same + (artist, platform), the resolver must return the REAL one. This is the + fix for the 2026-05-31 phantom-subscription bug: alembic 0022 had + rewritten an older per-post Source row into a sidecar synthetic, and + the operator later added the real subscription. The old `ORDER BY + id ASC LIMIT 1` lookup picked the older synthetic (lower id), + silently attaching every gallery-dl download to the wrong Source. + """ + synthetic = Source( + artist_id=artist_row.id, platform="patreon", + url=f"sidecar:patreon:{artist_row.slug}", enabled=False, + ) + db_sync.add(synthetic) + db_sync.flush() + real = Source( + artist_id=artist_row.id, platform="patreon", + url="https://www.patreon.com/testartist", enabled=True, + ) + db_sync.add(real) + db_sync.flush() + assert synthetic.id < real.id # ordering precondition + + resolved = importer._source_for_sidecar( + artist_id=artist_row.id, platform="patreon", + artist_slug=artist_row.slug, + ) + assert resolved.id == real.id + assert resolved.url == "https://www.patreon.com/testartist" diff --git a/tests/test_source_service.py b/tests/test_source_service.py index aa310ef..f6df9f3 100644 --- a/tests/test_source_service.py +++ b/tests/test_source_service.py @@ -1,7 +1,7 @@ import pytest from sqlalchemy import select -from backend.app.models import Artist +from backend.app.models import Artist, Source from backend.app.services.source_service import ( KNOWN_PLATFORMS, ArtistNotFoundError, @@ -133,3 +133,39 @@ async def test_update_changes_fields(db): updated = await svc.update(rec.id, enabled=False, config_overrides={"videos": False}) assert updated.enabled is False assert updated.config_overrides == {"videos": False} + + +@pytest.mark.asyncio +async def test_list_hides_sidecar_synthetic_anchors(db): + """Filesystem-import synthetic Sources (url='sidecar::', + enabled=False — see importer._source_for_sidecar) used to leak into the + Subscriptions UI as phantom subscriptions because list() didn't filter + them. They aren't pollable feeds; hide by default.""" + artist = await _artist(db, "Alice") + real = Source( + artist_id=artist.id, platform="patreon", + url="https://patreon.com/alice", enabled=True, config_overrides={}, + ) + synthetic = Source( + artist_id=artist.id, platform="patreon", + url="sidecar:patreon:alice", enabled=False, config_overrides={}, + ) + db.add_all([real, synthetic]) + await db.commit() + + svc = SourceService(db) + visible = await svc.list() + visible_urls = {s.url for s in visible} + assert "https://patreon.com/alice" in visible_urls + assert "sidecar:patreon:alice" not in visible_urls + + # Same filter applies to the artist-scoped list path (the artist detail + # page hits /api/sources?artist_id=N). + artist_scoped = await svc.list(artist_id=artist.id) + assert {s.url for s in artist_scoped} == {"https://patreon.com/alice"} + + # include_synthetic=True opts back in for admin tooling. + everything = await svc.list(include_synthetic=True) + assert {s.url for s in everything} >= { + "https://patreon.com/alice", "sidecar:patreon:alice", + }