pixiv retired, placement reconciler removed, build race fixed, mirror pinned #257
@@ -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" }
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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"
|
||||
Reference in New Issue
Block a user