diff --git a/backend/app/services/download_service.py b/backend/app/services/download_service.py index bb099a1..a00b407 100644 --- a/backend/app/services/download_service.py +++ b/backend/app/services/download_service.py @@ -274,6 +274,14 @@ class DownloadService: artist = self.sync_session.get(Artist, ctx["artist_id"]) source_row = self.sync_session.get(Source, ctx["source_id"]) + # Post-first (#856): on the native ingester the post-record is the sole + # writer of the post body/links, so the per-media import must NOT apply post + # fields. gallery-dl platforms keep writing them via the sidecar. The + # Importer is per-task, so this per-instance flag is safe. uses_native_ingester + # is the future-proof seam — a platform migrating onto the native core + # flips to post-first automatically (milestone #67, step 3). + self.importer.post_first = uses_native_ingester(ctx["platform"]) + import_summary = {"attached": 0, "skipped": 0, "errors": 0} # Archives detected but captured WITHOUT extracting any image (probe # rejected / corrupt / missing extractor backend). Surfaced on the event diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index 4b82466..88d1a76 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -149,6 +149,14 @@ class Importer: self.thumbnailer = thumbnailer self.settings = settings self.deep = deep + # Post-first ingest (#856, milestone #67): on the NATIVE core ingester the + # post-record (`upsert_post_record`) is the SOLE writer of a post's + # body/links/metadata; the per-media import only links image provenance + + # localization, NOT the post body. download_service sets this True for + # native platforms before the phase-3 media loop. Default False keeps the + # gallery-dl path writing post fields via `_apply_sidecar` (unchanged). + # Safe as a per-instance flag: Importer is constructed per Celery task. + self.post_first = False self.attachments = AttachmentStore(images_root) # phash near-dup candidate cache. Archive imports call _import_media # per-member; without this cache the per-member SELECT *FROM @@ -1110,7 +1118,14 @@ class Importer: external_post_id=epid, artist_id=artist.id, ) - self._apply_post_fields(post, sd) + # Post-first (#856): on the native path the post-record owns the body/ + # links/metadata, so the per-media import must NOT write post fields — its + # minimal sidecar carries no body, and applying it would clobber + # raw_metadata + re-run link-sync off empty data. The image still links + # provenance + localization below. gallery-dl (post_first False) is + # unchanged: its sidecar IS the only body source, so it still applies. + if not self.post_first: + self._apply_post_fields(post, sd) # Race-safe (image_record_id, post_id) upsert — mirrors the # _find_or_create_source/post savepoint pattern. The plain diff --git a/backend/app/services/patreon_downloader.py b/backend/app/services/patreon_downloader.py index ddabca7..2502d14 100644 --- a/backend/app/services/patreon_downloader.py +++ b/backend/app/services/patreon_downloader.py @@ -529,29 +529,38 @@ class PatreonDownloader: def _write_sidecar( self, post: dict, media_path: Path, *, source_url: str | None = None ) -> Path: - """Write the importer-consumed sidecar next to `media_path`. + """Write the per-media sidecar next to `media_path` — post-first (#856). - Patreon uses base-default sidecar keys (parse_sidecar maps - category->platform, id->external_post_id, title->post_title, - content->description, published_at->post_date, url->post_url). Patreon - registers no derive_post_url, so `url` is trusted as the permalink — we - pass the post's attributes.url. - - `source_url` is THIS file's CDN URL (#830 Phase 2). The importer persists - its filehash on the ImageRecord so the post body's inline `` can - be remapped to the local copy at render time. + On the native ingester the POST-RECORD (`write_post_record` → `_post.json`) + is the sole writer of the post body/links/metadata, captured once per post + BEFORE its media in the walk. So the per-media sidecar carries ONLY + image-specific identity: `category` (platform) + `id` (external_post_id, to + link provenance to the right Post) + this file's `source_url` (its CDN URL, + #830 Phase 2 — the importer persists its filehash so the body's inline + `` remaps to the local copy at render time). No body: writing it + next to every image duplicated the post body N+1× and risked divergence + (milestone #67). The importer skips post fields for these (post_first). """ return self._write_sidecar_data( - post, media_path.with_suffix(".json"), source_url=source_url + post, media_path.with_suffix(".json"), source_url=source_url, + minimal=True, ) def _write_sidecar_data( - self, post: dict, sidecar_path: Path, *, source_url: str | None = None + self, post: dict, sidecar_path: Path, *, source_url: str | None = None, + minimal: bool = False, ) -> Path: - """Serialize the post's metadata to `sidecar_path`. Shared by the - per-media sidecar (next to each file) and the post-only sidecar - (`write_post_record`, for media-less posts). `source_url` is set only for - the per-media sidecar — a media-less post has no source file.""" + """Serialize the post's metadata to `sidecar_path`. The post-only record + (`write_post_record`) writes the FULL post (body/title/date/url); the + per-media sidecar (`_write_sidecar`, minimal=True) writes only image + identity (category/id/source_url) — post-first (#856). `source_url` is set + only for the per-media sidecar — a media-less post has no source file.""" + if minimal: + data = {"category": "patreon", "id": str(post.get("id") or "")} + if source_url: + data["source_url"] = source_url + sidecar_path.write_text(json.dumps(data, indent=2)) + return sidecar_path attrs = post.get("attributes") or {} title = attrs.get("title") # Resolve the body HTML from the feed attrs: legacy flat `content`, else diff --git a/tests/test_importer_attach_in_place.py b/tests/test_importer_attach_in_place.py index 10752e1..f12c653 100644 --- a/tests/test_importer_attach_in_place.py +++ b/tests/test_importer_attach_in_place.py @@ -231,6 +231,78 @@ def test_attach_in_place_duplicate_attachment_links_both_posts(importer, db_sync assert set(linked) == {"301", "302"} # one row per post, both present +def test_attach_in_place_post_first_skips_post_fields(importer, db_sync): + """Post-first (#856, milestone #67): with post_first set (native path), the + per-media import links provenance + localization (source_filehash) but does + NOT write the post body/title — the post-record owns those. Guards the import + side even if a body sneaks into the sidecar.""" + from backend.app.models import ImageProvenance, Post + + importer.post_first = True + images_root = importer.images_root + artist = Artist(name="Pat", slug="pat") + db_sync.add(artist) + db_sync.flush() + + target = images_root / "pat" / "patreon" / "post1" / "img.jpg" + _make_jpg(target) + cdn = "https://cdn.patreon.com/" + "a" * 32 + "/x.jpg" + target.with_suffix(target.suffix + ".json").write_text( + '{"category": "patreon", "id": "900", "title": "Should NOT apply", ' + '"content": "

body should NOT apply

", ' + f'"source_url": "{cdn}"}}' + ) + + result = importer.attach_in_place(target, artist=artist) + assert result.status == "imported" + + # Image-specific work STILL happens: localization filehash + provenance link. + rec = db_sync.execute( + select(ImageRecord).where(ImageRecord.id == result.image_id) + ).scalar_one() + assert rec.source_filehash == "a" * 32 + assert rec.primary_post_id is not None + + post = db_sync.get(Post, rec.primary_post_id) + assert post.external_post_id == "900" + # Post-first: body/title NOT written by the media path (post-record owns them). + assert post.description is None + assert post.post_title is None + # Provenance link exists regardless of post_first. + prov = db_sync.execute( + select(ImageProvenance).where( + ImageProvenance.image_record_id == result.image_id + ) + ).scalars().all() + assert len(prov) == 1 + + +def test_attach_in_place_post_first_false_applies_post_fields(importer, db_sync): + """Default (gallery-dl path): the sidecar IS the body source, so post fields + ARE applied. Guards that the post_first flag actually gates this — not a no-op.""" + from backend.app.models import Post + + assert importer.post_first is False # default + images_root = importer.images_root + artist = Artist(name="Gal", slug="gal") + db_sync.add(artist) + db_sync.flush() + + target = images_root / "gal" / "patreon" / "post1" / "img.jpg" + _make_jpg(target) + target.with_suffix(target.suffix + ".json").write_text( + '{"category": "patreon", "id": "901", "title": "Applies", ' + '"content": "

body applies

"}' + ) + + result = importer.attach_in_place(target, artist=artist) + assert result.status == "imported" + rec = db_sync.get(ImageRecord, result.image_id) + post = db_sync.get(Post, rec.primary_post_id) + assert post.description == "

body applies

" + assert post.post_title == "Applies" + + def test_attach_in_place_invalid_image_returns_skipped(importer): images_root = importer.images_root bad = images_root / "fred" / "bad.jpg" diff --git a/tests/test_patreon_downloader.py b/tests/test_patreon_downloader.py index bf58416..9af8c1e 100644 --- a/tests/test_patreon_downloader.py +++ b/tests/test_patreon_downloader.py @@ -594,40 +594,34 @@ def test_range_ignored_by_server_refetches_clean(tmp_path, monkeypatch): # -- content enrichment (adaptive detail-fetch of an empty feed body) ------- -def test_enriches_empty_content_via_fetcher(tmp_path): - """An empty feed body is backfilled from the detail-fetch seam — once per - post (memoized on the shared dict) — and lands in the importer sidecar.""" +def test_media_sidecar_is_minimal_post_first(tmp_path): + """Post-first (#856): the per-media sidecar carries ONLY image identity + (category/id/source_url), NOT the post body — the post-record (`_post.json`) + owns the body and is written first in the walk. So download_post never hits + the detail fetcher (that's write_post_record's job), and the body is no longer + duplicated next to every image.""" calls: list[str] = [] - - def _fetcher(post_id: str) -> str: - calls.append(post_id) - return "

full body

" - dl = PatreonDownloader( images_root=tmp_path, cookies_path=None, validate=False, - session=_FakeSession(), content_fetcher=_fetcher, + session=_FakeSession(), + content_fetcher=lambda pid: calls.append(pid) or "

full body

", ) post = _post() - post["attributes"]["content"] = "" # feed returned an empty body + post["attributes"]["content"] = "" # even an empty feed body isn't fetched here dl.download_post(post, [_img("a.png"), _img("b.png")], "artist-x") post_dir = tmp_path / "artist-x" / "patreon" / pd_mod._post_dir_name(post) sc = find_sidecar(post_dir / "01_a.png") assert sc is not None - assert json.loads(sc.read_text())["content"] == "

full body

" - # Fetched ONCE for the post, not once per media item. - assert calls == ["1001"] - - -def test_no_enrichment_when_feed_body_present(tmp_path): - """A non-empty feed body is trusted as-is; the detail endpoint isn't hit.""" - calls: list[str] = [] - dl = PatreonDownloader( - images_root=tmp_path, cookies_path=None, validate=False, - session=_FakeSession(), - content_fetcher=lambda pid: calls.append(pid) or "x", - ) - dl.download_post(_post(), [_img("a.png")], "artist-x") # _post body is non-empty + data = json.loads(sc.read_text()) + assert data == { + "category": "patreon", + "id": "1001", + "source_url": "https://cdn.patreon.com/a.png", + } + # No post body/title next to the image; the detail endpoint is never hit here. + assert "content" not in data + assert "title" not in data assert calls == []