diff --git a/extension/test/artist-url-samples.json b/extension/test/artist-url-samples.json new file mode 100644 index 0000000..712edf4 --- /dev/null +++ b/extension/test/artist-url-samples.json @@ -0,0 +1,138 @@ +{ + "$comment": [ + "THE SHARED ARTIFACT for the JS<->Py artist-pattern mirror (issue #3093).", + "", + "extension/lib/platforms.js PLATFORM_ARTIST_PATTERNS and", + "backend/app/services/extension_service.py _PLATFORM_PATTERNS are two hand-kept", + "copies of one table, and they gate OPPOSITE HALVES of a single interaction:", + "the JS copy decides whether the 'Add to FC' button appears, the Python copy", + "decides whether the resulting POST is accepted. Drift is therefore never", + "cosmetic -- JS looser than Py shows a button that 400s, Py looser than JS", + "silently never offers a button for a URL the backend would take. Issue #1485", + "was the second of those, and its fix had to be applied to both files by hand.", + "", + "Neither runtime imports the other. This file is the shared artifact instead:", + "both suites read it and assert it against their OWN copy of the patterns, so", + "a change to one copy alone fails the other runtime's suite.", + " - extension/test/platforms.spec.js (vitest)", + " - tests/test_extension_artist_patterns.py (pytest)", + "", + "Lives under extension/test/ because that path is excluded from BOTH the XPI", + "file set and the extension version derivation (see scripts/packaging.sh:", + "NOT_PACKAGED_TRACKED and NOT_VERSION_RELEVANT both carry 'test/**'), so", + "adding samples here never ships bytes and never forces a re-sign.", + "", + "Adding a case: put the URL in the platform's match/no_match list with a", + "'why'. Run both suites. If only one goes red, you have found drift, which is", + "the entire point of the file.", + "", + "'slug' is read by the Python side only -- its _derive returns (platform,", + "slug) where the JS isArtistPage returns a boolean. It is not optional on a", + "match entry; the backend deriving the WRONG slug from a URL both copies", + "agree on is its own defect class, and this pins it." + ], + + "patreon": { + "match": [ + { + "url": "https://www.patreon.com/maewix", + "slug": "maewix", + "why": "bare creator root" + }, + { + "url": "https://patreon.com/maewix", + "slug": "maewix", + "why": "www is optional" + }, + { + "url": "http://patreon.com/maewix", + "slug": "maewix", + "why": "http as well as https" + }, + { + "url": "https://www.patreon.com/c/Atole", + "slug": "Atole", + "why": "#1485: the /c/ creator shape" + }, + { + "url": "https://www.patreon.com/cw/Atole", + "slug": "Atole", + "why": "#1485: /cw/ is the 'creator workspace' URL Patreon serves once you are SUBSCRIBED -- exactly when the button matters most, and exactly what the pre-#1485 pattern missed" + }, + { + "url": "https://www.patreon.com/cw/Atole/posts", + "slug": "Atole", + "why": "#1485: a creator inner page still derives the creator" + }, + { + "url": "https://www.patreon.com/Atole/membership", + "slug": "Atole", + "why": "#1485: inner page on the bare shape" + } + ], + "no_match": [ + { "url": "https://www.patreon.com/home", "why": "nav page, not a creator" }, + { "url": "https://www.patreon.com/search", "why": "nav page" }, + { "url": "https://www.patreon.com/messages", "why": "nav page" }, + { "url": "https://www.patreon.com/notifications", "why": "nav page" }, + { "url": "https://www.patreon.com/library", "why": "nav page" }, + { "url": "https://www.patreon.com/settings", "why": "nav page" }, + { "url": "https://www.patreon.com/posts", "why": "nav page; also the post-permalink prefix" }, + { "url": "https://www.patreon.com/home/anything", "why": "a nav page's inner path is still not a creator" }, + { "url": "https://www.patreon.com/settings/profile", "why": "as above" } + ] + }, + + "subscribestar": { + "match": [ + { + "url": "https://www.subscribestar.com/foobar", + "slug": "foobar", + "why": "creator root on the .com TLD" + }, + { + "url": "https://subscribestar.adult/foobar", + "slug": "foobar", + "why": "creator root on the .adult TLD" + }, + { + "url": "https://subscribestar.adult/foobar/", + "slug": "foobar", + "why": "trailing slash is tolerated" + } + ], + "no_match": [ + { "url": "https://subscribestar.adult/feed", "why": "nav page" }, + { "url": "https://subscribestar.adult/messages", "why": "nav page" }, + { "url": "https://subscribestar.adult/library", "why": "nav page" }, + { + "url": "https://subscribestar.adult/foobar/posts", + "why": "SubscribeStar's pattern end-anchors on the creator root, unlike Patreon's -- an inner page does NOT derive. Pinned so the asymmetry between the two platforms stays deliberate rather than becoming a silently-fixed bug in one copy only." + } + ] + }, + + "hentaifoundry": { + "match": [ + { + "url": "https://www.hentai-foundry.com/user/Foo", + "slug": "Foo", + "why": "user page" + }, + { + "url": "https://www.hentai-foundry.com/user/Foo/profile", + "slug": "Foo", + "why": "inner page still derives the user" + }, + { + "url": "https://hentai-foundry.com/user/Foo", + "slug": "Foo", + "why": "www is optional" + } + ], + "no_match": [ + { "url": "https://www.hentai-foundry.com/pictures/popular", "why": "gallery listing, not a user" }, + { "url": "https://www.hentai-foundry.com/", "why": "site root" } + ] + } +} diff --git a/extension/test/platforms.spec.js b/extension/test/platforms.spec.js index f869751..bee61ab 100644 --- a/extension/test/platforms.spec.js +++ b/extension/test/platforms.spec.js @@ -190,3 +190,60 @@ describe('manifest.json agrees with the platform table', () => { } }) }) + +describe('the JS<->Py artist-pattern mirror (#3093)', () => { + // PLATFORM_ARTIST_PATTERNS here and extension_service._PLATFORM_PATTERNS in + // the backend are two hand-kept copies of one table, and they gate OPPOSITE + // halves of a single interaction: this copy decides whether the "Add to FC" + // button appears, the Python copy decides whether the resulting POST is + // accepted. So JS-looser-than-Py shows a button that 400s, and + // Py-looser-than-JS never offers a button for a URL the backend would take. + // #1485 was the second of those, and its fix had to be applied to both + // files by hand. + // + // "Keep in sync by hand; reviewers catch drift" is the same guarantee + // manifest.json had before #3069, where deviantart survived seven weeks. + // + // The two-runtimes objection to a shared SOURCE file is fair, so the shared + // artifact is the SAMPLES instead: both suites read this JSON and assert it + // against their own copy of the patterns, and neither imports the other. + // The sibling half is tests/test_extension_artist_patterns.py; adding a + // sample there covers it here for free, and vice versa. + const samples = Object.fromEntries( + Object.entries( + JSON.parse(readFileSync(path.join(EXT_DIR, 'test', 'artist-url-samples.json'), 'utf8')) + ).filter(([key]) => !key.startsWith('$')) + ) + + for (const [platform, spec] of Object.entries(samples)) { + // `slug` on a match entry is read by the Python half only — isArtistPage + // answers a boolean, while the backend's _derive returns (platform, slug). + for (const { url, why } of spec.match) { + it(`shows the button on ${url} — ${why}`, () => { + expect(isArtistPage(url, platform)).toBe(true) + }) + } + for (const { url, why } of spec.no_match) { + it(`hides the button on ${url} — ${why}`, () => { + expect(isArtistPage(url, platform)).toBe(false) + }) + } + } + + it('has samples for every platform that has an artist pattern', () => { + // The guard's own coverage check: without it, deleting a platform's + // samples would make this block pass by testing less. Discord is + // deliberately in neither — it is channel-based, with no creator page to + // put a button on, so it has no artist pattern on either side. + expect(Object.keys(samples).sort()).toEqual(Object.keys(PLATFORM_ARTIST_PATTERNS).sort()) + }) + + it('has samples in both directions for every platform', () => { + // A platform with only positive samples pins half the invariant. The + // no_match half is the one that catches a pattern quietly widening. + for (const [platform, spec] of Object.entries(samples)) { + expect(spec.match.length, `${platform} match samples`).toBeGreaterThan(0) + expect(spec.no_match.length, `${platform} no_match samples`).toBeGreaterThan(0) + } + }) +}) diff --git a/tests/test_extension_artist_patterns.py b/tests/test_extension_artist_patterns.py new file mode 100644 index 0000000..68a2892 --- /dev/null +++ b/tests/test_extension_artist_patterns.py @@ -0,0 +1,121 @@ +"""The Python half of the JS<->Py artist-pattern mirror guard (#3093). + +`extension/lib/platforms.js` PLATFORM_ARTIST_PATTERNS and +`extension_service._PLATFORM_PATTERNS` are two hand-kept copies of one table. +Both files say "keep in sync by hand; reviewers catch drift" — the same +guarantee `manifest.json` had before #3069, where deviantart sat in the +manifest for seven weeks after the product dropped it. + +Drift here is worse than the manifest case, because the two copies gate +opposite halves of ONE interaction: + +- the **JS** copy decides whether the "Add to FC" button appears; +- the **Python** copy decides whether the resulting POST is accepted. + +So JS-looser-than-Py shows the operator a button that 400s, and +Py-looser-than-JS silently never offers a button for a URL the backend would +happily take. #1485 (Patreon's `/c/` and `/cw/` shapes) was exactly the +second, and its fix had to be applied to both files by hand. + +The two-runtimes objection to a shared source file is fair, so this tests the +INVARIANT rather than the source: one table of URL samples, read by both +suites and asserted against each one's own copy of the patterns. Neither +runtime imports the other. A change to one copy alone turns the other +runtime's suite red. + +The sibling half is `extension/test/platforms.spec.js`, which reads the same +file. Adding a sample there covers it here for free, and vice versa — which is +the property that makes the guard cheap enough to keep using. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from backend.app.services.extension_service import ( + ExtensionService, + UnknownPlatformError, +) + +_SAMPLES_PATH = ( + Path(__file__).resolve().parents[1] + / "extension" / "test" / "artist-url-samples.json" +) + + +def _load_samples() -> dict: + """The shared table, minus its `$comment` preamble.""" + raw = json.loads(_SAMPLES_PATH.read_text(encoding="utf-8")) + return {k: v for k, v in raw.items() if not k.startswith("$")} + + +_SAMPLES = _load_samples() + +_MATCH_CASES = [ + pytest.param(platform, entry["url"], entry["slug"], entry["why"], + id=f"{platform}-match-{i}") + for platform, spec in _SAMPLES.items() + for i, entry in enumerate(spec["match"]) +] + +_NO_MATCH_CASES = [ + pytest.param(platform, entry["url"], entry["why"], + id=f"{platform}-nomatch-{i}") + for platform, spec in _SAMPLES.items() + for i, entry in enumerate(spec["no_match"]) +] + + +def _derive(url: str) -> tuple[str, str]: + """`_derive` needs no session — it is pure regex over the URL.""" + return ExtensionService(session=None)._derive(url) + + +@pytest.mark.parametrize("platform,url,slug,why", _MATCH_CASES) +def test_creator_url_derives_the_expected_platform_and_slug(platform, url, slug, why): + """A URL the extension would show the button on must be one the backend + accepts, and it must derive the SAME creator. The slug is asserted, not + just the platform: deriving the wrong creator from a URL both copies agree + on is its own defect, and nothing else pins it.""" + assert _derive(url) == (platform, slug), why + + +@pytest.mark.parametrize("platform,url,why", _NO_MATCH_CASES) +def test_non_creator_url_derives_nothing(platform, url, why): + """The other direction, and the one that fails silently. A URL the + extension refuses to show the button on must also be one the backend + refuses — otherwise the backend is quietly looser than the button, and + nobody finds out because nothing visibly breaks. + + `_derive` is asserted to raise rather than merely to miss `platform`: it + tries every pattern in turn, so a nav page that some OTHER platform's + pattern happened to swallow would still be 'accepted by the backend', + which is the same defect wearing a different platform name. + """ + with pytest.raises(UnknownPlatformError): + _derive(url) + + +def test_the_sample_table_covers_every_platform_that_has_a_pattern(): + """The guard's own coverage check. Without it, deleting a platform's + samples would make this file pass by testing less — the failure mode that + makes absence-based tests untrustworthy (snippet #3352). + + Discord is deliberately absent from both: it has no artist pattern on + either side, because it is channel-based and has no creator page to put a + button on. + """ + from backend.app.services.extension_service import _PLATFORM_PATTERNS + + assert set(_SAMPLES) == {platform for platform, _ in _PLATFORM_PATTERNS} + + +def test_every_platform_has_samples_in_both_directions(): + """A platform with only positive samples pins half the invariant. The + no_match half is the one that catches a pattern quietly widening.""" + for platform, spec in _SAMPLES.items(): + assert spec["match"], f"{platform} has no match samples" + assert spec["no_match"], f"{platform} has no no_match samples"