diff --git a/backend/app/services/membership_reconcile.py b/backend/app/services/membership_reconcile.py index cd0df5e..c6a0d44 100644 --- a/backend/app/services/membership_reconcile.py +++ b/backend/app/services/membership_reconcile.py @@ -49,7 +49,7 @@ from .membership_roster import ( get_sync_state, has_paid_access, identity_keys_for_source, - match_kind, + pair_sources_with_memberships, roster_is_fresh, url_tail, ) @@ -113,20 +113,12 @@ async def reconcile( .where(Source.platform == platform) )).all() - # Nested loop rather than a SQL join: the match is a predicate over a JSON - # blob and a derived URL handle, neither of which is indexable, and both - # sides are tens of rows on any real library. Keeping it in Python means ONE - # definition of identity (`match_kind`) instead of a second one in SQL that - # could drift from it. - pairs: dict[int, tuple[PlatformMembership, str]] = {} - matched_membership_ids: set[int] = set() - for source, _artist in rows: - for m in memberships: - kind = match_kind(source, m) - if kind: - pairs[source.id] = (m, kind) - matched_membership_ids.add(m.id) - break + # The join itself lives in `membership_roster` beside `match_kind`, so C5's + # gated-reason annotation pairs sources with memberships by exactly the same + # rule this card sorts them by. Two copies would let the Subscriptions row + # and this card disagree about which creator a source IS. + pairs = pair_sources_with_memberships([s for s, _a in rows], memberships) + matched_membership_ids = {m.id for m, _kind in pairs.values()} subscribed_not_tracked = [] for m in memberships: diff --git a/backend/app/services/membership_roster.py b/backend/app/services/membership_roster.py index 8e9c43e..2c52888 100644 --- a/backend/app/services/membership_roster.py +++ b/backend/app/services/membership_roster.py @@ -352,6 +352,127 @@ def match_kind(source: Source, membership: PlatformMembership) -> str | None: return None +def pair_sources_with_memberships( + sources: list[Source], memberships: list[PlatformMembership], +) -> dict[int, tuple[PlatformMembership, str]]: + """Source id -> the membership it is the same creator as, and how we know. + + Extracted from C4's reconcile loop when C5 became its second caller. It is + a nested loop rather than a SQL join because the match is a predicate over + a JSON blob and a derived URL handle, neither of which is indexable, and + both sides are tens of rows on any real library. Keeping it in Python means + ONE definition of identity (`match_kind`) instead of a second one in SQL + that could drift from it. + + First match wins, which is `match_kind`'s ordering doing its job: a source + with a cached campaign id can only pair with the membership holding that + id, so an ambiguous handle never outvotes it. + """ + pairs: dict[int, tuple[PlatformMembership, str]] = {} + for source in sources: + for m in memberships: + kind = match_kind(source, m) + if kind: + pairs[source.id] = (m, kind) + break + return pairs + + +# --------------------------------------------------------------------------- +# Why the posts are invisible (#387 C5) +# --------------------------------------------------------------------------- +# +# A3 made a tier-gated source say "47 posts you can't see". These are the words +# the roster is allowed to add to that count — and ONLY to that count. +# +# THE LINE: the roster ANNOTATES the gated flag, it never produces it. +# `current_user_can_view` (read per post by `patreon_client.post_is_gated`) is +# the authoritative per-post signal, and entitled-tier data cannot stand in for +# it — a creator can gate a post behind an access rule that maps onto no tier +# name at all. So nothing here may suppress a download, skip a walk, or decide +# a post is inaccessible. It explains a skip that ALREADY happened. Getting +# that backwards would make FC silently stop fetching content the operator is +# paying for, which is the worst failure available in this milestone. +# `test_no_fetch_path_can_read_the_roster` pins that structurally. +GATED_LAPSED = "lapsed" # the membership ended — resubscribe, or disable +GATED_TIER = "tier" # paying, but this tier doesn't reach these posts +GATED_FREE = "free" # a current FREE follow — nobody is paying for access + + +def gated_reason( + platform: str, status: str | None, *, is_free_member: bool = False, +) -> str | None: + """Why a tier-gated source's posts are out of reach, if the roster knows. + + None means "no words beyond the count" and is the answer for every case + where the roster is not evidence: a status this code has not been taught, + and (at the call site) a campaign absent from the roster or a roster too + stale to trust. Absence is not evidence — the same discipline as + `test_post_is_gated_only_on_explicit_false`. + + `is_free_member` is read AFTER the status axis, not folded into it, which + is why `has_paid_access` is called here with it forced off. The two axes + are independent in Patreon's payload, and collapsing them loses a real + distinction: a current free follower has not lost anything, so telling them + "you're not a patron any more" would be a false sentence about a state they + were never in. + """ + by_status = has_paid_access(platform, status, is_free_member=False) + if by_status is None: + return None + if not by_status: + return GATED_LAPSED + return GATED_FREE if is_free_member else GATED_TIER + + +async def gated_reasons_for_sources( + session: AsyncSession, sources: list[Source], *, now: datetime | None = None, +) -> dict[int, str]: + """The reason word for each of these sources, where the roster has one. + + Callers pass ONLY the sources already known to be tier-gated: the question + "why can't I see these posts" is meaningless for a source whose posts are + all visible, and asking it anyway would put roster data on rows that have + no gated state for it to annotate. + + Sources with no entry in the result get A3's bare count, which is the + correct degraded rendering for all three of: platform never swept, roster + stale, campaign not in the roster. + """ + if not sources: + return {} + + platforms = {s.platform for s in sources} + # Per platform, because freshness is per platform: a working Patreon sweep + # must not lend its credibility to a SubscribeStar roster that has never + # run. Same gate as C4's `tracked_not_subscribed`, for the same reason. + fresh = { + p for p in platforms + if roster_is_fresh(await get_sync_state(session, p), now=now) + } + if not fresh: + return {} + + memberships = (await session.execute( + select(PlatformMembership).where( + PlatformMembership.platform.in_(sorted(fresh)) + ) + )).scalars().all() + pairs = pair_sources_with_memberships( + [s for s in sources if s.platform in fresh], memberships, + ) + + reasons: dict[int, str] = {} + for source_id, (m, _kind) in pairs.items(): + reason = gated_reason( + m.platform, m.status, + is_free_member=bool((m.details or {}).get("is_free_member")), + ) + if reason is not None: + reasons[source_id] = reason + return reasons + + async def source_for_membership( session: AsyncSession, membership: PlatformMembership, ) -> Source | None: diff --git a/backend/app/services/source_service.py b/backend/app/services/source_service.py index e4c6172..22a49ba 100644 --- a/backend/app/services/source_service.py +++ b/backend/app/services/source_service.py @@ -19,6 +19,7 @@ from ..models import ( ) from .db_helpers import failing_sources_clause from .gallery_dl import ErrorType +from .membership_roster import gated_reasons_for_sources from .platforms import known_platform_keys from .scheduler_service import compute_next_check_at @@ -92,6 +93,12 @@ class SourceRecord: # source, so it is joined in by `list()` only — None everywhere else, which # the UI renders as the bare no-access state with no fabricated number. tier_gated_count: int | None = None + # Milestone #387 C5: WHY those posts are out of reach, when the learned + # roster can say — "lapsed" / "tier" / "free", or None for "no words beyond + # the count". Joined in by `list()` beside the count, and for the same + # reason: it annotates a gated state rather than producing one. Nothing on + # a fetch path may read it (see `membership_roster.gated_reason`). + gated_reason: str | None = None def to_dict(self) -> dict: return { @@ -116,6 +123,7 @@ class SourceRecord: "backfill_recapture": self.backfill_recapture, "backfill_posts": self.backfill_posts, "tier_gated_count": self.tier_gated_count, + "gated_reason": self.gated_reason, } @@ -243,6 +251,7 @@ class SourceService: def _build_record( self, source: Source, artist: Artist, settings: ImportSettings, gated_counts: dict[int, int] | None = None, + gated_reasons: dict[int, str] | None = None, ) -> SourceRecord: nxt = compute_next_check_at(source, artist, settings) co = source.config_overrides or {} @@ -268,6 +277,7 @@ class SourceService: backfill_recapture=bool(co.get("_backfill_recapture")), backfill_posts=int(co.get("_backfill_posts", 0)), tier_gated_count=(gated_counts or {}).get(source.id), + gated_reason=(gated_reasons or {}).get(source.id), ) async def _row_to_record(self, source: Source) -> SourceRecord: @@ -302,12 +312,15 @@ class SourceService: stmt = stmt.order_by(Artist.name.asc(), Source.id.asc()) rows = (await self.session.execute(stmt)).all() settings = await self._load_settings() - # Only tier-gated rows need the join — on a healthy library that is an - # empty list and _tier_gated_counts short-circuits without a query. - gated_counts = await self._tier_gated_counts( - [s.id for s, _a in rows if s.error_type == ErrorType.TIER_LIMITED] - ) - return [self._build_record(s, a, settings, gated_counts) for s, a in rows] + # Only tier-gated rows need either join — on a healthy library that is + # an empty list and both helpers short-circuit without a query. + gated = [s for s, _a in rows if s.error_type == ErrorType.TIER_LIMITED] + gated_counts = await self._tier_gated_counts([s.id for s in gated]) + gated_reasons = await gated_reasons_for_sources(self.session, gated) + return [ + self._build_record(s, a, settings, gated_counts, gated_reasons) + for s, a in rows + ] async def get(self, source_id: int) -> SourceRecord | None: source = (await self.session.execute( diff --git a/frontend/src/components/subscriptions/SourceHealthDot.vue b/frontend/src/components/subscriptions/SourceHealthDot.vue index b84b7db..304d564 100644 --- a/frontend/src/components/subscriptions/SourceHealthDot.vue +++ b/frontend/src/components/subscriptions/SourceHealthDot.vue @@ -11,6 +11,9 @@
Last checked: {{ lastCheckedText }}
Next check: {{ nextCheckText }}
{{ noAccessText }}
+
+ {{ noAccessReason }} +
Failures: {{ source.consecutive_failures }}
@@ -53,6 +56,19 @@ const noAccessText = computed(() => { : "Some posts are behind a tier you don't hold" }) +// #387 C5: the learned roster's explanation for the line above, when it has +// one. The backend sends null for every case where the roster is not evidence — +// campaign absent, roster stale, never swept, status not yet characterised — so +// there is deliberately NO fallback sentence here. A default would turn "we +// don't know why" into a reason, which is the one thing this step must not do. +const GATED_REASONS = { + lapsed: "You're not a patron any more — resubscribe, or disable this source.", + tier: "Your tier doesn't cover these posts — upgrade, or leave them be.", + free: "You follow this creator for free — these posts are for paying patrons.", +} + +const noAccessReason = computed(() => GATED_REASONS[props.source.gated_reason] || null) + const ariaLabel = computed(() => `source health: ${level.value}`) const lastCheckedText = computed(() => formatRelative(props.source.last_checked_at)) @@ -92,6 +108,12 @@ const truncatedError = computed(() => { .fc-health-tip__gated { color: rgb(var(--v-theme-info, 33 150 243)); } +/* The reason is subordinate to the count it explains: same block, quieter, so + a tooltip that has one does not read as two separate findings. */ +.fc-health-tip__why { + color: rgb(var(--v-theme-on-surface-variant)); + max-width: 24rem; +} .fc-health-tip__err { margin-top: 0.25rem; color: rgb(var(--v-theme-error, 244 67 54)); diff --git a/frontend/test/components/sourceHealthDot.spec.js b/frontend/test/components/sourceHealthDot.spec.js index a0f41af..4e0ad83 100644 --- a/frontend/test/components/sourceHealthDot.spec.js +++ b/frontend/test/components/sourceHealthDot.spec.js @@ -56,6 +56,69 @@ describe('SourceHealthDot', () => { expect(w.text()).toContain("tier you don't hold") }) + // Milestone #387 C5. The count says WHAT; these say WHY — a much stronger + // claim, so the cases that must stay silent are pinned alongside the ones + // that speak. + + function gated (extra) { + return mountComponent(SourceHealthDot, { + stubs: { VTooltip: VTooltipStub }, + props: { + source: { + ...checked, consecutive_failures: 0, + error_type: 'tier_limited', tier_gated_count: 47, ...extra, + }, + }, + }) + } + + it('a lapsed membership says the subscription ended', () => { + expect(gated({ gated_reason: 'lapsed' }).text()).toContain('not a patron any more') + }) + + it('an active membership says the tier is the limit, not the subscription', () => { + const text = gated({ gated_reason: 'tier' }).text() + expect(text).toContain("tier doesn't cover these posts") + expect(text).not.toContain('not a patron any more') + }) + + it('a free follow is not described as a lapsed subscription', () => { + const text = gated({ gated_reason: 'free' }).text() + expect(text).toContain('follow this creator for free') + expect(text).not.toContain('not a patron any more') + }) + + it('no reason means the count stands alone, with no invented explanation', () => { + // null covers all four not-evidence cases the backend collapses into it: + // campaign absent, roster stale, never swept, status uncharacterised. + const text = gated({ gated_reason: null }).text() + expect(text).toContain('47 posts') + expect(text).not.toContain('patron') + expect(text).not.toContain('tier doesn') + }) + + it('a reason the frontend has not been taught renders nothing', () => { + // The backend's status vocabulary grows per platform (D1). An unknown word + // must degrade to the bare count, never to `undefined` in the tooltip. + const text = gated({ gated_reason: 'some_future_word' }).text() + expect(text).toContain('47 posts') + expect(text).not.toContain('undefined') + }) + + it('a reason never appears on a source that is not gated', () => { + const w = mountComponent(SourceHealthDot, { + stubs: { VTooltip: VTooltipStub }, + props: { + source: { + ...checked, consecutive_failures: 0, + error_type: null, gated_reason: 'lapsed', + }, + }, + }) + // The roster annotates a gated state; it never asserts one on its own. + expect(w.text()).not.toContain('not a patron any more') + }) + it('a genuinely failing source still grades as a failure, gated or not', () => { const w = mountComponent(SourceHealthDot, { stubs: { VTooltip: VTooltipStub }, diff --git a/tests/roster_builders.py b/tests/roster_builders.py new file mode 100644 index 0000000..8efd634 --- /dev/null +++ b/tests/roster_builders.py @@ -0,0 +1,64 @@ +"""Row builders for the learned membership roster (#387 phase C). + +Three test files were each constructing `PlatformMembership` and +`MembershipSync` rows with their own private helper — C4's reconcile tests, +E4's suggestion tests, and C5's gated-reason tests — and the three had already +started to drift apart in which fields they defaulted. That matters more here +than for ordinary test plumbing: every one of these tests turns on the exact +shape of a membership row (a `details["campaign"]["vanity"]` that the identity +join reads, an `is_free_member` flag that changes what the operator is told), +so three builders means three slightly different ideas of what a membership +looks like, and a test that passes against a row the sweep would never write. + +`campaign=` rather than the column's own `external_campaign_id=`: it is what +the majority of call sites already say, and the full name earns nothing in a +builder whose only subject is memberships. +""" +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +from backend.app.models import MembershipSync, PlatformMembership + +# The shape a real Patreon sweep writes, per the C0 capture (Scribe note +# #3886): a vanity nested under `details.campaign`, which is where +# `PlatformMembership.vanity_or_none` reads it from. +DEFAULT_VANITY = "maewix" +DEFAULT_URL = f"https://www.patreon.com/{DEFAULT_VANITY}" + + +async def membership( + db, *, campaign="c1", platform="patreon", status="active_patron", + display_name="Maewix", url=DEFAULT_URL, details=None, **kw, +) -> PlatformMembership: + """One observed membership. + + `details` defaults to the vanity-bearing shape rather than to `{}`, because + a row with no vanity cannot be matched by handle and would quietly make + every identity test a campaign-id test. + """ + m = PlatformMembership( + platform=platform, + external_campaign_id=campaign, + status=status, + display_name=display_name, + url=url, + details={"campaign": {"vanity": DEFAULT_VANITY}} if details is None else details, + **kw, + ) + db.add(m) + await db.flush() + return m + + +async def synced(db, *, platform="patreon", ago=timedelta(hours=1)) -> MembershipSync: + """A successful sweep this recently — what makes a roster FRESH. + + Pass `ago` past `ROSTER_STALE_AFTER` to build the stale case; omit the call + entirely for never-synced. Those are three different states and every + consumer of the roster has to tell them apart. + """ + state = MembershipSync(platform=platform, last_success_at=datetime.now(UTC) - ago) + db.add(state) + await db.flush() + return state diff --git a/tests/test_artist_membership_suggestions.py b/tests/test_artist_membership_suggestions.py index 6e3c7fd..27dc520 100644 --- a/tests/test_artist_membership_suggestions.py +++ b/tests/test_artist_membership_suggestions.py @@ -31,6 +31,7 @@ from backend.app.services.artist_membership_service import ( name_signal, weighted_score, ) +from tests.roster_builders import membership as _membership pytestmark = pytest.mark.integration @@ -126,17 +127,6 @@ async def _artist_with_discord(db, name, slug): return a -async def _membership(db, **kw): - kw.setdefault("platform", "patreon") - kw.setdefault("external_campaign_id", "c1") - kw.setdefault("url", "https://www.patreon.com/maewix") - kw.setdefault("details", {"campaign": {"vanity": "maewix"}}) - m = PlatformMembership(**kw) - db.add(m) - await db.flush() - return m - - @pytest.mark.asyncio async def test_a_matching_name_proposes_the_link(db): artist = await _artist_with_discord(db, "Maewix", "maewix") diff --git a/tests/test_gated_reason.py b/tests/test_gated_reason.py new file mode 100644 index 0000000..94acd73 --- /dev/null +++ b/tests/test_gated_reason.py @@ -0,0 +1,387 @@ +"""Milestone 387 C5: why the posts are invisible, in the roster's own words. + +A3 gave a tier-gated source a count — "47 posts you can't see". The roster can +turn that into a reason, and the whole risk of this step is that a reason is a +much stronger claim than a count. So most of what follows pins refusals: + +* an unrecognised status says NOTHING rather than guessing lapsed; +* a campaign absent from the roster says nothing, because absence is not + evidence (the same discipline as `test_post_is_gated_only_on_explicit_false`); +* a stale roster degrades to the bare count rather than asserting last week's + reason as today's; +* a current FREE follower is not told they used to be a patron. + +And the one that matters most: no fetch path can read the roster at all. That +is asserted structurally, not by inspection — the roster explains a skip that +already happened, and must never be able to cause one. +""" +from __future__ import annotations + +import ast +from datetime import timedelta +from pathlib import Path + +import pytest + +from backend.app.models import Artist, Source +from backend.app.services.gallery_dl import ErrorType +from backend.app.services.membership_roster import ( + GATED_FREE, + GATED_LAPSED, + GATED_TIER, + ROSTER_STALE_AFTER, + gated_reason, + gated_reasons_for_sources, +) +from tests.roster_builders import membership as _membership +from tests.roster_builders import synced as _synced + +# --- the pure mapping: status word -> the words we are entitled to say ------- +# +# No database, so these run in the unit lane. `gated_reason` is where the claim +# is actually decided; everything below it is plumbing. + + +def test_a_former_patron_is_told_the_membership_ended(): + assert gated_reason("patreon", "former_patron") == GATED_LAPSED + + +def test_an_active_patron_is_told_the_tier_does_not_reach_these_posts(): + assert gated_reason("patreon", "active_patron") == GATED_TIER + + +def test_a_current_free_follower_is_not_told_they_used_to_be_a_patron(): + """`free` and `lapsed` are different states and must not share a sentence. + + Both make `has_paid_access` False, which is why the reason is derived from + the status axis first rather than from that boolean: "you're not a patron + any more" is a false statement about someone who never was one. + """ + assert gated_reason("patreon", "active_patron", is_free_member=True) == GATED_FREE + assert gated_reason("patreon", "former_patron", is_free_member=True) == GATED_LAPSED + + +@pytest.mark.parametrize("status", [None, "declined_patron", "", "wat"]) +def test_an_unrecognised_status_says_nothing(status): + """Unknown is not lapsed. + + `declined_patron` is in here on purpose: it is the plausible-looking word + the C0 capture proved is NOT in the `patron_status` vocabulary. If someone + adds it to MEMBERSHIP_STATUS on the strength of the request filter, this + test is where that shows up. + """ + assert gated_reason("patreon", status) is None + + +def test_a_platform_that_has_never_been_characterised_says_nothing(): + """SubscribeStar and FANBOX (D1) inherit silence, not a Patreon guess.""" + assert gated_reason("subscribestar", "active_patron") is None + + +# --- the join, against the database ---------------------------------------- + + +async def _artist(db, name="Maewix"): + a = Artist(name=name, slug=name.lower().replace(" ", "")) + db.add(a) + await db.flush() + return a + + +async def _gated_source(db, artist, *, url="https://www.patreon.com/maewix", + platform="patreon", overrides=None): + s = Source( + artist_id=artist.id, platform=platform, url=url, enabled=True, + error_type=ErrorType.TIER_LIMITED, config_overrides=overrides, + ) + db.add(s) + await db.flush() + return s + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_matched_active_membership_explains_the_gate(db): + artist = await _artist(db) + source = await _gated_source(db, artist) + await _membership(db) + await _synced(db) + + assert await gated_reasons_for_sources(db, [source]) == {source.id: GATED_TIER} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_lapsed_membership_explains_the_gate(db): + artist = await _artist(db) + source = await _gated_source(db, artist) + await _membership(db, status="former_patron") + await _synced(db) + + assert await gated_reasons_for_sources(db, [source]) == {source.id: GATED_LAPSED} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_campaign_absent_from_the_roster_adds_no_words(db): + """The third case in the step: absence is not evidence. + + The sweep succeeded an hour ago and this creator is simply not in it. That + could mean the subscription lapsed — or that the sweep's pagination is + incomplete, or that the creator renamed. The count stands alone. + """ + artist = await _artist(db) + source = await _gated_source(db, artist) + await _membership(db, campaign="somebody-else", + url="https://www.patreon.com/someoneelse", + details={"campaign": {"vanity": "someoneelse"}}) + await _synced(db) + + assert await gated_reasons_for_sources(db, [source]) == {} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_stale_roster_degrades_to_the_bare_count(db): + """Same data as the passing case, one stale sync state. + + A reason is a claim about NOW. Last week's roster cannot make it. + """ + artist = await _artist(db) + source = await _gated_source(db, artist) + await _membership(db, status="former_patron") + await _synced(db, ago=ROSTER_STALE_AFTER + timedelta(hours=1)) + + assert await gated_reasons_for_sources(db, [source]) == {} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_never_synced_platform_degrades_to_the_bare_count(db): + artist = await _artist(db) + source = await _gated_source(db, artist) + await _membership(db, status="former_patron") + # No MembershipSync row at all. + + assert await gated_reasons_for_sources(db, [source]) == {} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_one_platforms_fresh_sweep_does_not_vouch_for_anothers(db): + """Freshness is per platform, so a working Patreon sweep cannot lend its + credibility to a SubscribeStar roster that has never run (D1's future).""" + artist = await _artist(db) + patreon = await _gated_source(db, artist) + other = await _gated_source( + db, artist, platform="subscribestar", + url="https://subscribestar.adult/maewix", + ) + await _membership(db, status="former_patron") + await _membership(db, platform="subscribestar", campaign="s1", + status="former_patron", + url="https://subscribestar.adult/maewix", + details={"campaign": {"vanity": "maewix"}}) + await _synced(db) # patreon only + + reasons = await gated_reasons_for_sources(db, [patreon, other]) + assert reasons == {patreon.id: GATED_LAPSED} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_the_cached_campaign_id_wins_over_a_stale_url(db): + """Inherited from `match_kind` via the shared join — asserted here so C5 + keeps identity in step with C4 rather than quietly growing its own.""" + artist = await _artist(db) + source = await _gated_source( + db, artist, url="https://www.patreon.com/old-handle", + overrides={"patreon_campaign_id": "c1"}, + ) + await _membership(db, campaign="c1", url="https://www.patreon.com/new-handle", + details={"campaign": {"vanity": "new-handle"}}, + status="former_patron") + await _synced(db) + + assert await gated_reasons_for_sources(db, [source]) == {source.id: GATED_LAPSED} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_no_sources_asks_the_database_nothing(db): + assert await gated_reasons_for_sources(db, []) == {} + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_the_source_list_carries_the_reason_to_the_ui(db): + """End to end through `SourceService.list`, because the field being on the + record is not the same as the field reaching the payload.""" + from backend.app.services.source_service import SourceService + + artist = await _artist(db) + source = await _gated_source(db, artist) + await _membership(db, status="former_patron") + await _synced(db) + await db.commit() + + records = await SourceService(db).list() + row = next(r.to_dict() for r in records if r.id == source.id) + assert row["gated_reason"] == GATED_LAPSED + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_source_that_is_not_gated_gets_no_reason(db): + """The roster annotates a gated state; it never puts one on a healthy row. + + A source walking fine whose creator the operator stopped paying for has + nothing to explain — and saying "you're not a patron any more" beside a + healthy row would be the roster asserting inaccessibility on its own. + """ + from backend.app.services.source_service import SourceService + + artist = await _artist(db) + source = Source( + artist_id=artist.id, platform="patreon", enabled=True, + url="https://www.patreon.com/maewix", error_type=None, + ) + db.add(source) + await db.flush() + await _membership(db, status="former_patron") + await _synced(db) + await db.commit() + + records = await SourceService(db).list() + row = next(r.to_dict() for r in records if r.id == source.id) + assert row["gated_reason"] is None + assert row["tier_gated_count"] is None + + +# --- the line that must not be crossed -------------------------------------- + +_APP = Path(__file__).resolve().parents[1] / "backend" / "app" + +# The modules that FETCH: the walk, the downloaders, the per-platform clients +# and ingesters. Named as ROOTS of an import walk rather than as the place the +# assertion looks — the check follows their transitive first-party imports, so +# it still holds when the logic inside them moves (rule #167). +_FETCH_ROOTS = [ + "services/download_service.py", + "services/ingest_core.py", + "services/native_ingest_common.py", + "services/gallery_dl.py", + "services/refetch_service.py", +] + +# Reading ANY of these is reading the roster. +_ROSTER_MODULES = {"services.membership_roster", "services.membership_reconcile"} + + +def _first_party_imports(path: Path) -> set[str]: + """Every `backend.app.*` module this file imports, as a dotted path + relative to `backend/app` — absolute and relative forms both.""" + tree = ast.parse(path.read_text()) + here = path.relative_to(_APP).with_suffix("").parts + if here and here[-1] == "__init__": + # A package's `__init__` IS the package, so `from .x import y` inside it + # resolves one level shallower than the file path suggests. Getting this + # wrong silently under-resolves every relative import in every package + # and would make the guard below unable to fail. + here = here[:-1] + out: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom): + if node.level: + # `from ..models import X` -> walk up from this module's package + base = list(here[: len(here) - node.level]) + mod = base + (node.module.split(".") if node.module else []) + elif node.module and node.module.startswith("backend.app."): + mod = node.module[len("backend.app."):].split(".") + else: + continue + out.add(".".join(mod)) + # `from .membership_roster import x` and + # `from . import membership_roster` must both resolve to the module. + for alias in node.names: + out.add(".".join([*mod, alias.name])) + elif isinstance(node, ast.Import): + for alias in node.names: + if alias.name.startswith("backend.app."): + out.add(alias.name[len("backend.app."):]) + return out + + +def _reachable_from(roots: list[str]) -> set[str]: + seen: set[str] = set() + queue = [r[:-3].replace("/", ".") for r in roots] + while queue: + mod = queue.pop() + if mod in seen: + continue + seen.add(mod) + for candidate in (_APP / Path(*mod.split(".")) / "__init__.py", + _APP / Path(*mod.split(".")).with_suffix(".py")): + if candidate.is_file(): + queue.extend(_first_party_imports(candidate) - seen) + break + return seen + + +def test_the_fetch_roots_all_exist(): + """Falsifiability guard for the guard below. + + A reachability check passes trivially if its roots resolve to nothing, so a + renamed module would silently turn the real assertion into a no-op that + still reads as coverage. + """ + missing = [r for r in _FETCH_ROOTS if not (_APP / r).is_file()] + assert not missing, f"fetch roots moved or were renamed: {missing}" + + +# The falsifiability controls. FC runs no local verification (rule #85), so a +# guard cannot be falsified by hand before it is committed — it has to carry its +# own proof that it is capable of failing, and re-prove it on every run. These +# two assert the walker finds roster imports that ARE there, one direct and one +# through a hop, so the real assertion below can never pass merely because the +# walk resolved nothing. + + +def test_the_walk_sees_a_direct_roster_import(): + """`source_service` legitimately reads the roster — it is what annotates the + Subscriptions rows. If this stops holding the walker has gone blind, not the + dependency gone away.""" + assert "services.membership_roster" in _reachable_from(["services/source_service.py"]) + + +def test_the_walk_follows_more_than_one_hop(): + """api/sources.py -> services/source_service.py -> services/membership_roster.py. + + A one-hop walker would pass the guard below on a fetch path that reaches the + roster through any intermediate module, which is the likeliest way this ever + actually regresses. + """ + reachable = _reachable_from(["api/sources.py"]) + assert "services.source_service" in reachable + assert "services.membership_roster" in reachable + + +def test_no_fetch_path_can_read_the_roster(): + """The roster explains a skip. It must never be able to cause one. + + Entitled-tier data says which tiers the account holds, not which posts + those tiers unlock — `current_user_can_view` is the only per-post truth. A + fetch path that could consult the roster could decide not to fetch + something the operator is paying for, silently, and that is the worst + failure available in this milestone. + + Import reachability rather than a call-site scan: a module that cannot + reach the roster cannot consult it, and unlike a grep for a function name + this keeps holding when the functions are renamed. + """ + reachable = _reachable_from(_FETCH_ROOTS) + assert not (reachable & _ROSTER_MODULES), ( + "a fetch path can now reach the membership roster: " + f"{sorted(reachable & _ROSTER_MODULES)}. The roster may annotate a " + "gated state, never produce one — see membership_roster.gated_reason." + ) diff --git a/tests/test_membership_reconcile.py b/tests/test_membership_reconcile.py index 95bde7f..083b2dd 100644 --- a/tests/test_membership_reconcile.py +++ b/tests/test_membership_reconcile.py @@ -10,11 +10,11 @@ So most of what follows pins refusals: the gate that empties the bucket when the roster cannot be trusted, the tri-state that keeps "unknown" out of "lapsed", and the per-row basis that stops the weakest claim sounding like the strongest. """ -from datetime import UTC, datetime, timedelta +from datetime import timedelta import pytest -from backend.app.models import Artist, MembershipSync, PlatformMembership, Source +from backend.app.models import Artist, MembershipSync, Source from backend.app.services.membership_reconcile import ( BASIS_ABSENT_EXACT, BASIS_ABSENT_HANDLE, @@ -23,6 +23,8 @@ from backend.app.services.membership_reconcile import ( reconcile_all, ) from backend.app.services.membership_roster import ROSTER_STALE_AFTER +from tests.roster_builders import membership as _membership +from tests.roster_builders import synced as _synced pytestmark = pytest.mark.integration @@ -44,25 +46,6 @@ async def _source(db, artist, *, url, platform="patreon", enabled=True, override return s -async def _membership( - db, *, campaign="c1", platform="patreon", status="active_patron", - display_name="Maewix", url="https://www.patreon.com/maewix", details=None, -): - m = PlatformMembership( - platform=platform, external_campaign_id=campaign, status=status, - display_name=display_name, url=url, - details={"campaign": {"vanity": "maewix"}} if details is None else details, - ) - db.add(m) - await db.flush() - return m - - -async def _synced(db, *, platform="patreon", ago=timedelta(hours=1)): - db.add(MembershipSync(platform=platform, last_success_at=datetime.now(UTC) - ago)) - await db.flush() - - # --- the join --------------------------------------------------------------