fix(subscriptions): hide sidecar synthetic Sources + prefer real on lookup
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:<plat>:<slug>` 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.
This commit is contained in:
@@ -0,0 +1,190 @@
|
|||||||
|
"""collapse-sidecar-synthetic: repoint Posts/ImageProvenance/DownloadEvents
|
||||||
|
from `sidecar:<platform>:<slug>` 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:<platform>:<slug>' 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
|
||||||
@@ -261,24 +261,44 @@ class Importer:
|
|||||||
def _source_for_sidecar(
|
def _source_for_sidecar(
|
||||||
self, *, artist_id: int, platform: str, artist_slug: str,
|
self, *, artist_id: int, platform: str, artist_slug: str,
|
||||||
) -> Source:
|
) -> 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
|
Source represents a subscription feed (one per artist+platform — the
|
||||||
gallery-dl URL polled by the FC-3 downloader). The filesystem importer
|
URL polled by the FC-3 downloader). The filesystem importer used to
|
||||||
used to call _find_or_create_source(url=sd.post_url), which created
|
call _find_or_create_source(url=sd.post_url), creating one Source
|
||||||
one Source row per post URL — 100s of junk Sources per artist, all
|
row per post URL — 100s of junk Sources per artist, all with
|
||||||
with enabled=True, polluting the artist detail page and tricking the
|
enabled=True, polluting the artist detail page and tricking the
|
||||||
subscription checker into trying to poll patreon post URLs as feeds.
|
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),
|
Resolution order: prefer a real (non-sidecar) Source over a
|
||||||
reuse it regardless of its URL — the artist's real subscription Source
|
synthetic anchor. When alembic 0022 ran, it may have rewritten
|
||||||
(created by the downloader / extension / UI) is the canonical
|
per-post Sources into `sidecar:<platform>:<slug>` synthetic
|
||||||
attachment point for filesystem-imported posts. If none exists, create
|
anchors. If the operator later added the real subscription, both
|
||||||
ONE synthetic anchor with url='sidecar:<platform>:<artist_slug>' and
|
rows now coexist. A naive `ORDER BY id ASC LIMIT 1` lookup would
|
||||||
enabled=False (so the subscription checker doesn't poll it).
|
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)
|
select(Source)
|
||||||
.where(
|
.where(
|
||||||
Source.artist_id == artist_id,
|
Source.artist_id == artist_id,
|
||||||
@@ -288,7 +308,7 @@ class Importer:
|
|||||||
.limit(1)
|
.limit(1)
|
||||||
)
|
)
|
||||||
return self._get_or_create(
|
return self._get_or_create(
|
||||||
stmt,
|
any_stmt,
|
||||||
lambda: Source(
|
lambda: Source(
|
||||||
artist_id=artist_id,
|
artist_id=artist_id,
|
||||||
platform=platform,
|
platform=platform,
|
||||||
|
|||||||
@@ -151,10 +151,18 @@ class SourceService:
|
|||||||
|
|
||||||
async def list(
|
async def list(
|
||||||
self, artist_id: int | None = None, failing: bool = False,
|
self, artist_id: int | None = None, failing: bool = False,
|
||||||
|
include_synthetic: bool = False,
|
||||||
) -> list[SourceRecord]:
|
) -> list[SourceRecord]:
|
||||||
stmt = select(Source, Artist).join(Artist, Artist.id == Source.artist_id)
|
stmt = select(Source, Artist).join(Artist, Artist.id == Source.artist_id)
|
||||||
if artist_id is not None:
|
if artist_id is not None:
|
||||||
stmt = stmt.where(Source.artist_id == artist_id)
|
stmt = stmt.where(Source.artist_id == artist_id)
|
||||||
|
if not include_synthetic:
|
||||||
|
# Filesystem-import sidecar anchors (importer._source_for_sidecar)
|
||||||
|
# have url='sidecar:<platform>:<slug>' 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:
|
if failing:
|
||||||
# Worst-first so the rollup card surfaces the loudest failures.
|
# Worst-first so the rollup card surfaces the loudest failures.
|
||||||
stmt = stmt.where(Source.consecutive_failures > 0).order_by(
|
stmt = stmt.where(Source.consecutive_failures > 0).order_by(
|
||||||
|
|||||||
@@ -210,3 +210,36 @@ def test_source_for_sidecar_distinct_platforms_distinct_anchors(
|
|||||||
assert p.id != x.id
|
assert p.id != x.id
|
||||||
assert p.platform == "patreon"
|
assert p.platform == "patreon"
|
||||||
assert x.platform == "pixiv"
|
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"
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import pytest
|
import pytest
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
|
|
||||||
from backend.app.models import Artist
|
from backend.app.models import Artist, Source
|
||||||
from backend.app.services.source_service import (
|
from backend.app.services.source_service import (
|
||||||
KNOWN_PLATFORMS,
|
KNOWN_PLATFORMS,
|
||||||
ArtistNotFoundError,
|
ArtistNotFoundError,
|
||||||
@@ -133,3 +133,39 @@ async def test_update_changes_fields(db):
|
|||||||
updated = await svc.update(rec.id, enabled=False, config_overrides={"videos": False})
|
updated = await svc.update(rec.id, enabled=False, config_overrides={"videos": False})
|
||||||
assert updated.enabled is False
|
assert updated.enabled is False
|
||||||
assert updated.config_overrides == {"videos": 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:<platform>:<slug>',
|
||||||
|
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",
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user