2f66de2928
Operator-asked 2026-06-01 after the Dymkens orphan investigation (Scribe plan #540). The pre-2030 sidecar-synthetic Source pattern (`sidecar:<platform>:<slug>` enabled=false rows) existed solely to satisfy `Post.source_id NOT NULL`, and leaked into the Subscriptions UI as phantom subscriptions. Now the data model says what's true: filesystem-imported content with no live subscription has NULL source_id, full stop. ## Schema (alembic 0030) - `post.artist_id` — NEW NOT NULL FK to artist (CASCADE). Backfilled from source.artist_id in the migration. Indexed for the artist-filter queries. - `post.source_id` — NOT NULL → nullable; FK ondelete CASCADE → SET NULL. Deleting a Source detaches its Posts instead of destroying archived content (subscription ends, archive stays). - `image_provenance.source_id` — same nullable + SET NULL. - Partial unique index `uq_post_artist_external_id_null_source` on (artist_id, external_post_id) WHERE source_id IS NULL — guards filesystem-import dedup since the existing source-bound unique ignores NULLs (Postgres treats NULL != NULL). - Sidecar synthetic Sources deleted: NULL out FKs in post, image_provenance first, then DELETE FROM source WHERE url LIKE 'sidecar:%'. The Dymkens cleanup. ## Model + service changes - `Post.source_id` → `Mapped[int | None]`; new `Post.artist_id` denormalized. - `ImageProvenance.source_id` → `Mapped[int | None]`. - Importer: `_source_for_sidecar` (synthetic-creating) → `_lookup_source_for_sidecar` (returns None when no subscription). `_find_or_create_post` takes required `artist_id`; matches on (source_id, external_post_id) for source-bound posts or (artist_id, external_post_id) for NULL-source posts. - Service queries switched off the Source detour to use Post.artist_id directly: post_feed_service.scroll/around/get_post (LEFT JOIN to Source so NULL-source posts surface); artist_service date_row/ activity/post_count; provenance_service.for_image/for_post (LEFT JOIN); gallery_service._provenance_exists_where_artist via Post.artist_id instead of ImageProvenance.source_id → Source. - `_to_dict` and provenance dict-builders emit `"source": null` for NULL-source rows. ## Frontend - `ProvenancePanel.vue` + `PostCard.vue`: render `e.source?.platform ?? 'filesystem import'` so NULL-source posts get a clear "filesystem import" affordance instead of a NaN crash. ## Tests - `test_importer_upsert_helpers`: removed the four synthetic-anchor tests; added `_find_or_create_post_idempotent_with_null_source` (dedup via the partial unique index) and `_lookup_source_for_sidecar_returns_*` (existing-subscription + none cases). The existing `_find_or_create_post_idempotent` now also passes `artist_id` and asserts it. - 8 other test files updated: every direct `Post(...)` construction gains `artist_id=<artist>.id`. The `_seed_post` helper in `test_post_feed_service` looks up artist_id from the source row so callsites stay one-arg. ## Verification on deploy After alembic 0030 runs: - `SELECT COUNT(*) FROM source WHERE url LIKE 'sidecar:%'` → 0. - `SELECT COUNT(*) FROM post WHERE source_id IS NULL` → count of filesystem-imported posts (Dymkens + any other historical). - Every `post.artist_id` non-null; consistent with source.artist_id for source-bound rows. - Subscriptions tab: no Dymkens phantom row. - Artist detail → Posts/Gallery: Dymkens's content still reachable via Post.artist_id. - Provenance panel renders "filesystem import" chip for NULL-source posts; PostCard same. ## Out of scope - UI to manage/delete orphan NULL-source Posts. Data model is right; UI follows if operator wants it.
217 lines
7.5 KiB
Python
217 lines
7.5 KiB
Python
"""Tests for Importer._find_or_create_source / _find_or_create_post —
|
|
the race-safe savepoint-based helpers that replaced the previous
|
|
check-then-insert pattern.
|
|
|
|
Operator-flagged 2026-05-26: concurrent workers processing different
|
|
files in the same post both found no existing Source row, then both
|
|
INSERTed, tripping uq_source_artist_platform_url and poisoning the
|
|
session with `psycopg.errors.UniqueViolation`. The new helpers wrap
|
|
the INSERT in a savepoint and recover from IntegrityError by
|
|
re-selecting the row that the concurrent op committed.
|
|
|
|
Tests cover:
|
|
- idempotent return: same (artist_id, platform, url) → same Source row
|
|
- idempotent return for Post: same (source_id, external_post_id) → same Post
|
|
- IntegrityError recovery: monkeypatched flush raises once, helper
|
|
finds the row a concurrent op committed
|
|
"""
|
|
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
from sqlalchemy import select
|
|
from sqlalchemy.exc import IntegrityError
|
|
|
|
from backend.app.models import Artist, ImportSettings, Post, Source
|
|
from backend.app.services.importer import Importer
|
|
from backend.app.services.thumbnailer import Thumbnailer
|
|
|
|
pytestmark = pytest.mark.integration
|
|
|
|
|
|
@pytest.fixture
|
|
def importer(db_sync, tmp_path):
|
|
import_root = tmp_path / "import"
|
|
images_root = tmp_path / "images"
|
|
import_root.mkdir()
|
|
images_root.mkdir()
|
|
settings = db_sync.execute(
|
|
select(ImportSettings).where(ImportSettings.id == 1)
|
|
).scalar_one()
|
|
return Importer(
|
|
session=db_sync,
|
|
images_root=images_root,
|
|
import_root=import_root,
|
|
thumbnailer=Thumbnailer(images_root=images_root),
|
|
settings=settings,
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def artist_row(db_sync):
|
|
a = Artist(name="TestArtist", slug="testartist")
|
|
db_sync.add(a)
|
|
db_sync.flush()
|
|
return a
|
|
|
|
|
|
def test_find_or_create_source_creates_then_returns_existing(importer, artist_row):
|
|
s1 = importer._find_or_create_source(
|
|
artist_id=artist_row.id, platform="patreon",
|
|
url="https://www.patreon.com/posts/test-1",
|
|
)
|
|
s2 = importer._find_or_create_source(
|
|
artist_id=artist_row.id, platform="patreon",
|
|
url="https://www.patreon.com/posts/test-1",
|
|
)
|
|
assert s1.id == s2.id
|
|
|
|
|
|
def test_find_or_create_source_distinct_urls_yield_distinct_rows(
|
|
importer, artist_row,
|
|
):
|
|
a = importer._find_or_create_source(
|
|
artist_id=artist_row.id, platform="patreon",
|
|
url="https://www.patreon.com/posts/a",
|
|
)
|
|
b = importer._find_or_create_source(
|
|
artist_id=artist_row.id, platform="patreon",
|
|
url="https://www.patreon.com/posts/b",
|
|
)
|
|
assert a.id != b.id
|
|
|
|
|
|
def test_find_or_create_post_idempotent(importer, artist_row, db_sync):
|
|
src = importer._find_or_create_source(
|
|
artist_id=artist_row.id, platform="patreon",
|
|
url="https://www.patreon.com/posts/post-test",
|
|
)
|
|
p1 = importer._find_or_create_post(
|
|
source_id=src.id, external_post_id="ext-001",
|
|
artist_id=artist_row.id,
|
|
)
|
|
p2 = importer._find_or_create_post(
|
|
source_id=src.id, external_post_id="ext-001",
|
|
artist_id=artist_row.id,
|
|
)
|
|
assert p1.id == p2.id
|
|
assert p1.artist_id == artist_row.id
|
|
|
|
|
|
def test_find_or_create_post_idempotent_with_null_source(
|
|
importer, artist_row, db_sync,
|
|
):
|
|
"""Post.source_id is nullable since alembic 0030 — filesystem-imported
|
|
posts with no live subscription have NULL source_id. The partial
|
|
unique index `uq_post_artist_external_id_null_source` on
|
|
(artist_id, external_post_id) WHERE source_id IS NULL guards the
|
|
dedup; the helper matches on the same (artist_id, external_post_id)
|
|
keys when source_id is None."""
|
|
p1 = importer._find_or_create_post(
|
|
source_id=None, external_post_id="fs-001",
|
|
artist_id=artist_row.id,
|
|
)
|
|
p2 = importer._find_or_create_post(
|
|
source_id=None, external_post_id="fs-001",
|
|
artist_id=artist_row.id,
|
|
)
|
|
assert p1.id == p2.id
|
|
assert p1.source_id is None
|
|
assert p1.artist_id == artist_row.id
|
|
|
|
|
|
def test_find_or_create_source_recovers_from_integrity_error(
|
|
importer, artist_row, db_sync, monkeypatch,
|
|
):
|
|
"""Simulate the race: another worker has already inserted a Source row
|
|
matching our (artist_id, platform, url) just before our flush would
|
|
have. Our flush raises IntegrityError; the helper rolls back the
|
|
savepoint and re-selects, returning the row the concurrent op created.
|
|
"""
|
|
canonical_url = "https://www.patreon.com/posts/race-141226276"
|
|
pre_existing = Source(
|
|
artist_id=artist_row.id, platform="patreon", url=canonical_url,
|
|
)
|
|
db_sync.add(pre_existing)
|
|
db_sync.flush()
|
|
|
|
# Force a fresh select within the helper to MISS the existing row by
|
|
# detaching it from the identity map; SQLAlchemy's first-level cache
|
|
# would otherwise return the pre_existing row immediately.
|
|
# Easier: monkeypatch the FIRST select inside the helper to return
|
|
# None on first call, real result on subsequent. We do that by
|
|
# patching session.execute with a single-shot wrapper.
|
|
real_execute = db_sync.execute
|
|
skip_count = [0]
|
|
|
|
def execute_with_first_select_miss(stmt, *args, **kwargs):
|
|
# Strip-down heuristic: the first SELECT issued by the helper is
|
|
# the existence check. Force it to return a "no row" result.
|
|
result = real_execute(stmt, *args, **kwargs)
|
|
if skip_count[0] == 0:
|
|
skip_count[0] += 1
|
|
# Wrap result so .scalar_one_or_none() returns None for this
|
|
# one call, then unwrap on subsequent uses.
|
|
class _ForcedMiss:
|
|
def scalar_one_or_none(self):
|
|
return None
|
|
|
|
def scalar_one(self):
|
|
return result.scalar_one()
|
|
|
|
def __getattr__(self, name):
|
|
return getattr(result, name)
|
|
return _ForcedMiss()
|
|
return result
|
|
|
|
monkeypatch.setattr(db_sync, "execute", execute_with_first_select_miss)
|
|
|
|
recovered = importer._find_or_create_source(
|
|
artist_id=artist_row.id, platform="patreon", url=canonical_url,
|
|
)
|
|
assert recovered.id == pre_existing.id
|
|
|
|
|
|
def test_lookup_source_for_sidecar_returns_existing_subscription(
|
|
importer, artist_row, db_sync,
|
|
):
|
|
"""The sidecar-import Source lookup returns the existing Source for
|
|
(artist, platform) when one exists — letting filesystem-imported
|
|
content attach to the artist's real subscription instead of being
|
|
orphaned.
|
|
"""
|
|
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._lookup_source_for_sidecar(
|
|
artist_id=artist_row.id, platform="patreon",
|
|
)
|
|
assert resolved is not None
|
|
assert resolved.id == canonical.id
|
|
|
|
|
|
def test_lookup_source_for_sidecar_returns_none_when_no_subscription(
|
|
importer, artist_row, db_sync,
|
|
):
|
|
"""Alembic 0030 made Post.source_id nullable; the importer no
|
|
longer creates synthetic `sidecar:<platform>:<slug>` anchor rows
|
|
when no real subscription exists. The lookup returns None and the
|
|
caller carries that through as a NULL source_id on the new Post.
|
|
"""
|
|
resolved = importer._lookup_source_for_sidecar(
|
|
artist_id=artist_row.id, platform="pixiv",
|
|
)
|
|
assert resolved is None
|
|
|
|
# Verify no Source row was created as a side effect.
|
|
count = db_sync.execute(
|
|
select(Source).where(
|
|
Source.artist_id == artist_row.id, Source.platform == "pixiv",
|
|
)
|
|
).all()
|
|
assert count == []
|