refactor(ingest): post-first — post-record is the sole body writer on the native path (#856)
Milestone #67 step 2. On the native core ingester the Post becomes the single authoritative record for body/links/metadata, captured once per post by the post-record; the per-media import only links image provenance + localization. Before: every per-media sidecar carried the full post body, so a post with N images wrote the body N+1 times (post-record + N media) — redundant on disk and a divergence risk (#753). gallery-dl is unchanged (its sidecar is still the only body source). - patreon_downloader: the per-media sidecar is now minimal — {category, id, source_url} only, no body. `_write_sidecar_data(minimal=True)` skips the body resolution + detail-fetch (the post-record, written first in the walk, already did it). Body no longer duplicated next to each image. - importer: new per-instance `post_first` flag (Importer is per-task). When set, `_apply_sidecar` still writes source_filehash + provenance + primary_post_id but SKIPS `_apply_post_fields` (the post-record owns body/links/raw_metadata, so applying a body-less sidecar would clobber raw_metadata + re-sync links off empty data). Default False keeps gallery-dl writing post fields. - download_service: `_phase3_persist` sets importer.post_first = uses_native_ingester(platform) — the future-proof seam, so a platform migrating onto the native core flips to post-first automatically (step 3). Media imports before post-records but both unify on external_post_id, so the post ends with its body either way. Tests: per-media sidecar is minimal + never hits the detail fetcher; attach post_first=True links provenance/localization but writes no post body/title; post_first=False (gallery-dl) still applies them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 `<img src>` 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
|
||||
`<img src>` 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
|
||||
|
||||
@@ -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": "<p>body should NOT apply</p>", '
|
||||
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": "<p>body applies</p>"}'
|
||||
)
|
||||
|
||||
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 == "<p>body applies</p>"
|
||||
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"
|
||||
|
||||
@@ -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 "<p>full body</p>"
|
||||
|
||||
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 "<p>full body</p>",
|
||||
)
|
||||
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"] == "<p>full body</p>"
|
||||
# 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 == []
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user