diff --git a/src/adcp/decisioning/platform_router.py b/src/adcp/decisioning/platform_router.py index 441c969d..72ac9649 100644 --- a/src/adcp/decisioning/platform_router.py +++ b/src/adcp/decisioning/platform_router.py @@ -709,6 +709,27 @@ async def build_platform(tenant_id: str) -> DecisioningPlatform: ``get_products`` routes to the manager when wired; otherwise falls through to the lazily-resolved platform's ``get_products``. + :param proposal_stores: Optional ``{tenant_id: ProposalStore}`` — + eager per-tenant proposal store dict. Mirrors the eager + :class:`PlatformRouter`'s ``proposal_stores=`` shape for + adopters with a small known tenant set. Mutually exclusive + with ``proposal_store_factory``. Stores are dict-cheap to + hold; the lazy machinery applies only to ``platforms`` (which + wrap upstream connections / credentials and warrant the LRU + eviction). See #722. + :param proposal_store_factory: Optional + ``Callable[[str], ProposalStore | None]`` — lazy-build flavor. + **Called on every** :meth:`proposal_store_for_tenant` + **invocation** (no internal caching), and the framework's + ``proposal_dispatch`` calls the accessor 2–3× per request on + the proposal path. If your factory does non-trivial work + (opens a connection pool, reads config, etc.), wrap it with + :func:`functools.lru_cache` or your own memoization — most + ``ProposalStore`` implementations hold long-lived DB pool + references and adopters typically return the same instance + per tenant. Return ``None`` for tenants that don't need a + store (pure-catalog mode without finalize). Mutually exclusive + with ``proposal_stores``. See #722. :param cache_size: Maximum number of cached :class:`DecisioningPlatform` instances. Bounded LRU eviction past this size. Default 256. Adopters with more concurrent active tenants override. @@ -734,6 +755,8 @@ def __init__( factory: PlatformFactory, capabilities: DecisioningCapabilities, proposal_managers: Mapping[str, ProposalManager] | None = None, + proposal_stores: Mapping[str, ProposalStore] | None = None, + proposal_store_factory: Callable[[str], ProposalStore | None] | None = None, cache_size: int = 256, cache_ttl_seconds: float = 3600.0, ) -> None: @@ -749,10 +772,56 @@ def __init__( "Pass 0 for size-only eviction (no time-based expiry)." ) + if proposal_stores is not None and proposal_store_factory is not None: + raise ValueError( + "LazyPlatformRouter: pass either proposal_stores= (eager " + "per-tenant dict) or proposal_store_factory= (lazy), not " + "both. The eager dict pre-binds tenants; the factory " + "resolves on first request. See #722." + ) + self.accounts = accounts self.capabilities = capabilities self._factory = factory self._proposal_managers: dict[str, ProposalManager] = dict(proposal_managers or {}) + # #722: parity with PlatformRouter — accept proposal_stores= + # (eager) or proposal_store_factory= (lazy). The eager dict is + # the typical small-tenant-set shape; the factory composes with + # the lazy-build philosophy of LazyPlatformRouter (matches + # ``factory=`` for platforms). + self._proposal_stores: dict[str, ProposalStore] = dict(proposal_stores or {}) + self._proposal_store_factory: Callable[[str], ProposalStore | None] | None = ( + proposal_store_factory + ) + + # Cross-store consistency check on the manager-eager subset. + # ``proposal_managers`` is eager (dict-cheap), so even though + # we can't enumerate every possible tenant, we CAN walk the + # known managers and refuse to construct if a finalize-capable + # manager has no store wired for its tenant (eager dict miss + # AND no factory). Recovers ~80% of the boot-time validation + # the eager :class:`PlatformRouter` provides at line 376-386 — + # adopters migrating from eager to lazy don't silently lose + # the wiring-gap signal. The factory-only path defers to + # first-request validation (the factory might legitimately + # return ``None`` for some tenants). + for tenant_id, manager in self._proposal_managers.items(): + caps = getattr(manager, "capabilities", None) + finalize_supported = bool(getattr(caps, "finalize", False)) + if ( + finalize_supported + and tenant_id not in self._proposal_stores + and self._proposal_store_factory is None + ): + raise ValueError( + f"Tenant {tenant_id!r} wired a ProposalManager declaring " + f"finalize=True, but no ProposalStore was registered for " + f"that tenant and no proposal_store_factory was configured. " + f"Wire one via " + f"proposal_stores={{{tenant_id!r}: InMemoryProposalStore()}}, " + "supply a proposal_store_factory=, or remove the finalize " + "capability." + ) self._cache_size = cache_size self._cache_ttl = cache_ttl_seconds @@ -990,6 +1059,33 @@ def proposal_manager_for_tenant(self, tenant_id: str) -> ProposalManager | None: """Return the :class:`ProposalManager` for ``tenant_id``, or ``None``.""" return self._proposal_managers.get(tenant_id) + def proposal_store_for_tenant(self, tenant_id: str) -> ProposalStore | None: + """Return the :class:`ProposalStore` for ``tenant_id``, or ``None``. + + Resolution order: + + 1. Eager ``proposal_stores`` dict, when wired. + 2. ``proposal_store_factory(tenant_id)``, when wired. The + factory is invoked on every call — adopters who need + caching wrap the factory themselves (most ``ProposalStore`` + implementations hold long-lived pool references and are + cheap to return on repeat calls). + 3. ``None`` — the tenant has no store wired and the framework's + ``proposal_dispatch`` falls through to the v1 (no-proposal) + path. + + Sibling-API parity with :meth:`PlatformRouter.proposal_store_for_tenant` + for adopters wiring a :class:`ProposalStore` per #722. The + framework's ``proposal_dispatch`` duck-types this method via + ``hasattr(platform, "proposal_store_for_tenant")``. + """ + store = self._proposal_stores.get(tenant_id) + if store is not None: + return store + if self._proposal_store_factory is not None: + return self._proposal_store_factory(tenant_id) + return None + async def platform_for_tenant(self, tenant_id: str) -> DecisioningPlatform: """Return the platform for ``tenant_id``, building via the factory if needed. diff --git a/tests/test_lazy_platform_router.py b/tests/test_lazy_platform_router.py index f34ca32a..c283fbc0 100644 --- a/tests/test_lazy_platform_router.py +++ b/tests/test_lazy_platform_router.py @@ -710,3 +710,184 @@ async def test_missing_tenant_metadata_raises_account_not_found() -> None: with pytest.raises(AdcpError) as exc_info: await router.create_media_buy({}, ctx) assert exc_info.value.code == "ACCOUNT_NOT_FOUND" + + +# =========================================================================== +# #722: proposal_stores= / proposal_store_factory= parity with PlatformRouter +# =========================================================================== + + +class TestProposalStores: + """LazyPlatformRouter parity with PlatformRouter on proposal_stores. + + Before #722, adopters wiring LazyPlatformRouter had no way to thread + a ProposalStore into proposal_dispatch — the framework duck-types + ``hasattr(platform, "proposal_store_for_tenant")`` and silently + fell through to v1 no-proposal behavior. These tests pin the new + parity surface. + """ + + def test_eager_proposal_stores_dict_resolves_per_tenant(self) -> None: + """Eager dict — same shape PlatformRouter accepts. Per-tenant + store wired at construction; resolution is dict lookup.""" + from adcp.decisioning.proposal_store import InMemoryProposalStore + + store_a = InMemoryProposalStore() + store_b = InMemoryProposalStore() + router = LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + proposal_stores={"tenant_a": store_a, "tenant_b": store_b}, + ) + assert router.proposal_store_for_tenant("tenant_a") is store_a + assert router.proposal_store_for_tenant("tenant_b") is store_b + # Unwired tenant → None (falls through to no-proposal path in + # proposal_dispatch). + assert router.proposal_store_for_tenant("tenant_c") is None + + def test_proposal_store_factory_resolves_lazily(self) -> None: + """Factory shape — matches the ``factory=`` philosophy of the + lazy router. Invoked on every call so adopters can wrap with + their own memoization if needed.""" + from adcp.decisioning.proposal_store import InMemoryProposalStore + + invocations: list[str] = [] + + def factory(tenant_id: str) -> Any: + invocations.append(tenant_id) + return InMemoryProposalStore() + + router = LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + proposal_store_factory=factory, + ) + s1 = router.proposal_store_for_tenant("tenant_a") + s2 = router.proposal_store_for_tenant("tenant_a") + # Factory called on every invocation — no internal caching. + # Adopters who need caching wrap the factory themselves. + assert invocations == ["tenant_a", "tenant_a"] + assert s1 is not s2 + + def test_proposal_store_factory_can_return_none(self) -> None: + """Adopters with mixed tenants (some need stores, some don't) + return None from the factory for pure-catalog tenants. The + framework's proposal_dispatch falls through to the v1 path + when the accessor returns None.""" + router = LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + proposal_store_factory=lambda tid: None, + ) + assert router.proposal_store_for_tenant("tenant_a") is None + + def test_mutually_exclusive_eager_and_factory(self) -> None: + """Passing both ``proposal_stores=`` (eager dict) and + ``proposal_store_factory=`` (lazy) is a config error — they + cover the same accessor, and the precedence would be silent. + Loud-fail at construction.""" + from adcp.decisioning.proposal_store import InMemoryProposalStore + + with pytest.raises(ValueError, match="either proposal_stores=.*or proposal_store_factory="): + LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + proposal_stores={"t": InMemoryProposalStore()}, + proposal_store_factory=lambda tid: InMemoryProposalStore(), + ) + + def test_default_returns_none_for_no_store_configured(self) -> None: + """Back-compat: when neither kwarg is passed, the accessor + exists but returns None. Adopters not using proposals see the + same v1 behavior they had before #722.""" + router = LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + ) + # Method exists (so hasattr-based dispatch can find it). + assert hasattr(router, "proposal_store_for_tenant") + # Returns None for any tenant. + assert router.proposal_store_for_tenant("any") is None + + def test_finalize_manager_without_store_rejected_at_construction(self) -> None: + """Reviewer #2 caught this: even though LazyPlatformRouter + doesn't enumerate every tenant, ``proposal_managers`` IS eager + (dict-cheap). So we CAN walk the known managers and refuse to + construct when a finalize-capable manager has no store wired + (eager dict miss AND no factory). Recovers ~80% of the + boot-time validation the eager PlatformRouter has. + + Migration safety net: adopters moving from PlatformRouter to + LazyPlatformRouter don't silently lose the wiring-gap signal. + """ + from adcp.decisioning.proposal_manager import ProposalCapabilities + + class _FinalizeCapableManager: + capabilities = ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + finalize=True, + ) + + async def finalize_proposal(self, *args: Any, **kwargs: Any) -> Any: + raise NotImplementedError + + with pytest.raises(ValueError, match="finalize=True.*no ProposalStore"): + LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + proposal_managers={"t1": _FinalizeCapableManager()}, # type: ignore[dict-item] + # no proposal_stores, no proposal_store_factory + ) + + def test_finalize_manager_satisfied_by_factory(self) -> None: + """A factory is sufficient to satisfy the finalize→store + requirement at construction time. We can't verify the factory + will return a non-None store for the specific tenant (it + legitimately might not), but presence of a factory is treated + as "stores are available" — the deferred validation kicks in + at first request via ``proposal_dispatch.maybe_intercept_finalize``. + """ + from adcp.decisioning.proposal_manager import ProposalCapabilities + from adcp.decisioning.proposal_store import InMemoryProposalStore + + class _FinalizeCapableManager: + capabilities = ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + finalize=True, + ) + + async def finalize_proposal(self, *args: Any, **kwargs: Any) -> Any: + raise NotImplementedError + + # Doesn't raise. + LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + proposal_managers={"t1": _FinalizeCapableManager()}, # type: ignore[dict-item] + proposal_store_factory=lambda tid: InMemoryProposalStore(), + ) + + def test_proposal_dispatch_can_duck_type_the_accessor(self) -> None: + """Regression guard for the bug #722 closes: framework's + ``proposal_dispatch`` does ``hasattr(platform, + "proposal_store_for_tenant")``. Verify the method is + callable + returns ``None`` cleanly when no store is wired, + so the duck-type check succeeds and the dispatch doesn't + silently fall to the no-proposal path.""" + router = LazyPlatformRouter( + accounts=_make_routing_account_store({}), + factory=lambda tid: _SyncSalesPlatform(tag=tid), + capabilities=_capabilities(["sales-non-guaranteed"]), + ) + # The hasattr check the framework runs: + assert hasattr(router, "proposal_store_for_tenant") + # And the method is callable without raising on unknown tenants. + result = router.proposal_store_for_tenant("never-wired") + assert result is None