diff --git a/backend/app/services/gallery_dl.py b/backend/app/services/gallery_dl.py index ddaf9f7..83e8aaa 100644 --- a/backend/app/services/gallery_dl.py +++ b/backend/app/services/gallery_dl.py @@ -94,6 +94,48 @@ BACKFILL_CHUNK_SECONDS = 600 _DEFAULT_GDL_TIMEOUT_SECONDS = 870 +# --- Discord naming --------------------------------------------------------- +# +# Derived from a REAL sidecar (operator's instance, 2026-09-13), not from memory +# of gallery-dl's extractor. What gallery-dl's discord extractor actually emits +# for an attachment: `channel` is a plain STRING (the channel's name), the +# message is `message_id`, the attachment's position in it is `num`, and there +# is NO `id` key at all. +# +# The previous patterns asked for `{channel[name]}` and `{id}`. Both render as +# "None", so every Discord download since the platform was added landed in a +# directory called `None` as `_None_`. Worse, the sidecar was +# named `{filename}.json` — the attachment's ORIGINAL name — which (a) `find_ +# sidecar` can never pair with `_None_.png`, so no Discord file ever +# got a Post or a post date, and (b) collides: every `image.png` in a channel +# overwrote the same `image.json`, so the one sidecar that survived described +# whichever message happened to be written last. +# +# The fix names the sidecar EXACTLY like the media minus its extension, so +# `find_sidecar`'s first candidate (`media.with_suffix(".json")`) is the match +# and the name is unique per attachment. tests/test_gallery_dl_naming.py renders +# these patterns against a sanitized copy of the real sidecar, so a key that +# does not exist fails CI instead of silently becoming "None". +DISCORD_FILENAME = "{date:%Y%m%d}_{message_id}_{num:>02}_{filename}.{extension}" +DISCORD_DIRECTORY = ["{channel}"] + + +def sidecar_name_for(media_pattern: str) -> str | None: + """The metadata filename pattern that names a sidecar exactly like its media. + + Returns None for a pattern that does not end in `.{extension}`, since then + there is no media stem to mirror and the caller must fall back. + """ + suffix = ".{extension}" + if not media_pattern.endswith(suffix): + return None + return media_pattern[: -len(suffix)] + ".json" + + +def metadata_postprocessor(filename: str) -> dict: + return {"name": "metadata", "mode": "json", "directory": ".", "filename": filename} + + @dataclass class SourceConfig: """Per-source overrides loaded from Source.config_overrides JSON. @@ -376,8 +418,11 @@ class GalleryDLService: }, "discord": { "content_types": ["all"], - "directory": ["{channel[name]}"], - "filename": "{date:%Y%m%d}_{id}_{filename}.{extension}", + "directory": DISCORD_DIRECTORY, + "filename": DISCORD_FILENAME, + # Overrides the global `{filename}.json` sidecar for this extractor + # only — see the Discord naming note above. + "postprocessors": [metadata_postprocessor(sidecar_name_for(DISCORD_FILENAME))], "embeds": "all", "stickers": True, "reactions": False, @@ -497,6 +542,17 @@ class GalleryDLService: if source_config.filename_pattern: platform_section["filename"] = source_config.filename_pattern + # A platform that names its sidecar after its media must keep doing so + # under a per-source filename override, or the pairing breaks exactly the + # way Discord's did. No metadata wanted means no platform postprocessor + # either — the global list was already dropped above. + if "postprocessors" in platform_section: + mirrored = sidecar_name_for(platform_section.get("filename") or "") + if not source_config.save_metadata or mirrored is None: + platform_section.pop("postprocessors") + else: + platform_section["postprocessors"] = [metadata_postprocessor(mirrored)] + platform_section["metadata"] = source_config.save_metadata return config diff --git a/tests/fixtures/discord_attachment_sidecar.json b/tests/fixtures/discord_attachment_sidecar.json new file mode 100644 index 0000000..a0eb42c --- /dev/null +++ b/tests/fixtures/discord_attachment_sidecar.json @@ -0,0 +1,28 @@ +{ + "author": "example-artist", + "author_files": [], + "author_id": "100000000000000001", + "category": "discord", + "channel": "nsfw-drops", + "channel_id": "200000000000000002", + "channel_topic": "", + "channel_type": 0, + "date": "2024-07-16 16:43:23", + "extension": "png", + "filename": "image", + "files": [], + "is_thread": false, + "message": "", + "message_id": "300000000000000003", + "num": 1, + "owner_id": "100000000000000001", + "parent": "", + "parent_id": "", + "parent_type": 0, + "server": "Example Server", + "server_files": [], + "server_id": "400000000000000004", + "subcategory": "channel", + "type": "attachment", + "url": "https://cdn.discordapp.com/attachments/200000000000000002/500000000000000005/image.png" +} diff --git a/tests/test_gallery_dl_naming.py b/tests/test_gallery_dl_naming.py new file mode 100644 index 0000000..408556c --- /dev/null +++ b/tests/test_gallery_dl_naming.py @@ -0,0 +1,117 @@ +"""gallery-dl naming patterns, rendered against the metadata gallery-dl really emits. + +gallery-dl does not fail on a format field that names a key the extractor never +sets. It renders "None" and carries on. That is how every Discord download +landed in a `None/` directory as `_None_`: the patterns asked for +`{channel[name]}` and `{id}`, and the real metadata has a string `channel` and +`message_id`, with no `id` at all. The mismatch also broke sidecar pairing, so +no Discord file ever got a Post or a post date. + +The fixture keeps the key set and value TYPES of a real attachment sidecar from +the operator's instance (2026-09-13), with every value invented. Rendering +through Python's own formatter raises on a missing key or a subscript into a +string, which is the loud failure gallery-dl does not give. +""" + +import json +import string +from datetime import datetime +from pathlib import Path + +import pytest + +from backend.app.services.gallery_dl import ( + DISCORD_DIRECTORY, + DISCORD_FILENAME, + GalleryDLService, + SourceConfig, + sidecar_name_for, +) +from backend.app.utils.sidecar import find_sidecar + +_FIXTURE = Path(__file__).parent / "fixtures" / "discord_attachment_sidecar.json" + + +@pytest.fixture +def kwdict(): + data = json.loads(_FIXTURE.read_text()) + # gallery-dl hands the formatter a datetime; the JSON sidecar stores it as text. + data["date"] = datetime.strptime(data["date"], "%Y-%m-%d %H:%M:%S") + return data + + +def _render(pattern, kwdict): + return string.Formatter().vformat(pattern, (), kwdict) + + +def test_the_discord_filename_renders_from_real_keys(kwdict): + name = _render(DISCORD_FILENAME, kwdict) + assert "None" not in name + assert name == "20240716_300000000000000003_01_image.png" + + +def test_the_discord_directory_renders_from_real_keys(kwdict): + assert [_render(p, kwdict) for p in DISCORD_DIRECTORY] == ["nsfw-drops"] + + +@pytest.mark.parametrize( + "broken", ["{channel[name]}", "{date:%Y%m%d}_{id}_{filename}.{extension}"], +) +def test_the_patterns_that_shipped_would_have_failed_here(kwdict, broken): + """Positive control: the two patterns that produced `None/..._None_...` + must fail this renderer, or the tests above prove nothing.""" + with pytest.raises((KeyError, TypeError)): + _render(broken, kwdict) + + +def test_the_sidecar_is_named_exactly_like_its_media(kwdict, tmp_path): + """So `find_sidecar` pairs them on its first candidate, and two attachments + that share an original name (`image.png`) can never share a sidecar.""" + media = tmp_path / _render(DISCORD_FILENAME, kwdict) + sidecar = tmp_path / _render(sidecar_name_for(DISCORD_FILENAME), kwdict) + media.write_bytes(b"x") + sidecar.write_text("{}") + assert find_sidecar(media) == sidecar + + +def test_two_attachments_with_the_same_original_name_get_distinct_sidecars(kwdict): + other = {**kwdict, "message_id": "300000000000000099"} + pattern = sidecar_name_for(DISCORD_FILENAME) + assert _render(pattern, kwdict) != _render(pattern, other) + + +def test_sidecar_name_for_needs_an_extension_suffix(): + assert sidecar_name_for("{a}_{b}.{extension}") == "{a}_{b}.json" + assert sidecar_name_for("{a}_{b}") is None + + +# --- the config gallery-dl is actually given ----------------------------------- + + +@pytest.fixture +def gdl(tmp_path): + return GalleryDLService(images_root=tmp_path / "images", validate_files=False) + + +def _discord_section(gdl, **overrides): + cfg = gdl._build_config_for_source( + platform="discord", source_config=SourceConfig(**overrides), artist_slug="a", + ) + return cfg["extractor"]["discord"] + + +def test_discord_config_carries_the_mirrored_sidecar(gdl): + section = _discord_section(gdl) + assert section["filename"] == DISCORD_FILENAME + assert section["directory"] == DISCORD_DIRECTORY + assert section["postprocessors"][0]["filename"] == sidecar_name_for(DISCORD_FILENAME) + + +def test_a_filename_override_keeps_the_sidecar_mirrored(gdl): + section = _discord_section(gdl, filename_pattern="{message_id}_{num}.{extension}") + assert section["postprocessors"][0]["filename"] == "{message_id}_{num}.json" + + +def test_no_metadata_means_no_discord_postprocessor(gdl): + section = _discord_section(gdl, save_metadata=False) + assert "postprocessors" not in section