From f3b388fe7e1cd25a2b641388636a80f1863be11a Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Thu, 30 Jul 2026 01:29:45 +0200 Subject: [PATCH] perf(core): define bounded reference search --- docs/MODULE_ARCHITECTURE.md | 26 ++++ src/govoplan_core/api/v1/schemas.py | 2 + src/govoplan_core/core/references.py | 182 +++++++++++++++++++---- tests/test_reference_options.py | 64 ++++++++ webui/src/platform/referenceProviders.ts | 22 ++- 5 files changed, 268 insertions(+), 28 deletions(-) diff --git a/docs/MODULE_ARCHITECTURE.md b/docs/MODULE_ARCHITECTURE.md index fb3560e..6d5df01 100644 --- a/docs/MODULE_ARCHITECTURE.md +++ b/docs/MODULE_ARCHITECTURE.md @@ -91,6 +91,7 @@ The following contracts are the baseline API that modules can rely on: - capability factory contract - access DTO/protocol contracts in `govoplan_core.core.access` - resource ACL provider contract +- bounded reference-option search provider contract - single-tenant and optional batched tenant summary provider contracts - tenant delete-veto provider contract - WebUI module contribution contract @@ -346,6 +347,31 @@ full snapshot with `full: true`. A first-use `seq:0` watermark remains valid until such a floor exists, even if unrelated collections have advanced the global sequence. +### Bounded Reference Selectors + +Cross-module selectors use the module-neutral contract in +`govoplan_core.core.references`; consumers must not load an optional module's +complete directory and filter it in memory. + +- Providers receive a normalized `ReferenceSearchRequest` with `kind`, + `tenant_id`, `query`, `selected_values`, `limit`, optional opaque `cursor`, + and policy context. +- Providers apply visibility and text filtering before materializing rows and + return `ReferenceSearchPage(options, next_cursor, has_more)`. +- A page contains at most the requested bounded search results. Already-selected + references are retained in addition to that bound so historical values remain + readable and removable even when they are inactive, deleted, or outside the + current search page. +- API consumers expose `next_cursor` and `has_more`. The current searchable + selector requests the first bounded page for each query; later load-more UI + can use the same cursor without changing the provider contract. +- `access.reference_options` supplies SQL-backed account, membership, and group + searches. When it is absent, Core degrades to the legacy Access directory or + to principal-only/unavailable references without importing Access. +- The shared WebUI `apiReferenceOptionProvider` resolves selected values in + chunks of at most 200, preventing a large existing selection from turning + into an unbounded request. + ### Cursor/Keyset Pages Offset pagination remains supported for compatibility and for first page loads, diff --git a/src/govoplan_core/api/v1/schemas.py b/src/govoplan_core/api/v1/schemas.py index d54bebb..1c9ff74 100644 --- a/src/govoplan_core/api/v1/schemas.py +++ b/src/govoplan_core/api/v1/schemas.py @@ -53,6 +53,8 @@ class ReferenceOptionResponse(BaseModel): class ReferenceOptionListResponse(BaseModel): options: list[ReferenceOptionResponse] = Field(default_factory=list) provider_available: bool = True + next_cursor: str | None = None + has_more: bool = False class LoginRequest(BaseModel): diff --git a/src/govoplan_core/core/references.py b/src/govoplan_core/core/references.py index 989684e..31b0b41 100644 --- a/src/govoplan_core/core/references.py +++ b/src/govoplan_core/core/references.py @@ -12,6 +12,7 @@ from govoplan_core.core.access import ( ) +CAPABILITY_ACCESS_REFERENCE_OPTIONS = "access.reference_options" ReferenceAvailability = Literal["available", "inactive", "unavailable"] @@ -48,9 +49,17 @@ class ReferenceSearchRequest: query: str = "" selected_values: tuple[str, ...] = () limit: int = 50 + cursor: str | None = None context: Mapping[str, object] = field(default_factory=dict) +@dataclass(frozen=True, slots=True) +class ReferenceSearchPage: + options: tuple[ReferenceOption, ...] = () + next_cursor: str | None = None + has_more: bool = False + + @runtime_checkable class ReferenceOptionProvider(Protocol): """Optional module-neutral provider for typed reference selectors.""" @@ -61,7 +70,7 @@ class ReferenceOptionProvider(Protocol): principal: object, *, request: ReferenceSearchRequest, - ) -> Sequence[ReferenceOption]: + ) -> ReferenceSearchPage: ... @@ -89,38 +98,115 @@ def access_scope_reference_options( selected_values: Sequence[str] = (), limit: int = 50, administrative: bool = False, + session: object | None = None, + reference_kind: str | None = None, ) -> tuple[ReferenceOption, ...]: """Return canonical user-account or group references for definition scopes.""" + return access_scope_reference_page( + registry, + principal, + scope_type=scope_type, + query=query, + selected_values=selected_values, + limit=limit, + administrative=administrative, + session=session, + reference_kind=reference_kind, + ).options + + +def access_scope_reference_page( + registry: object | None, + principal: object, + *, + scope_type: str, + query: str = "", + selected_values: Sequence[str] = (), + limit: int = 50, + cursor: str | None = None, + administrative: bool = False, + session: object | None = None, + reference_kind: str | None = None, +) -> ReferenceSearchPage: + """Search bounded Access references while retaining selected values.""" + clean_type = str(scope_type or "").strip().casefold() if clean_type not in {"user", "group"}: raise ValueError("Scope reference type must be user or group.") + clean_kind = str(reference_kind or clean_type).strip().casefold() + if clean_kind not in ( + {"user", "membership"} if clean_type == "user" else {"group"} + ): + raise ValueError("Scope reference kind is incompatible with the scope type.") normalized_limit = max(1, min(int(limit), 200)) selected = tuple( dict.fromkeys( str(value).strip() for value in selected_values if str(value).strip() ) + )[:200] + tenant_id = str(getattr(principal, "tenant_id", "") or "") + provider = reference_option_provider( + registry, + CAPABILITY_ACCESS_REFERENCE_OPTIONS, ) - directory = _access_directory(registry) - if directory is None: - return _fallback_scope_options( + if provider is not None: + if not tenant_id: + return ReferenceSearchPage( + options=tuple( + _unavailable_option(value, kind=clean_kind) + for value in selected + ) + ) + page = provider.search_reference_options( + session, principal, - scope_type=clean_type, - query=query, - selected_values=selected, - limit=normalized_limit, + request=ReferenceSearchRequest( + kind=clean_kind, + tenant_id=tenant_id, + query=str(query or "").strip().casefold(), + selected_values=selected, + limit=normalized_limit, + cursor=str(cursor).strip() if cursor else None, + context={"administrative": administrative}, + ), + ) + return ReferenceSearchPage( + options=_bounded_page_options( + page.options, + selected_values=selected, + limit=normalized_limit, + kind=clean_kind, + ), + next_cursor=page.next_cursor, + has_more=page.has_more, + ) + + directory = _access_directory(registry) + if directory is None: + return ReferenceSearchPage( + options=_fallback_scope_options( + principal, + scope_type=clean_type, + reference_kind=clean_kind, + query=query, + selected_values=selected, + limit=normalized_limit, + ) ) - tenant_id = str(getattr(principal, "tenant_id", "") or "") if not tenant_id: - return tuple( - _unavailable_option(value, kind=clean_type) for value in selected + return ReferenceSearchPage( + options=tuple( + _unavailable_option(value, kind=clean_kind) for value in selected + ) ) if clean_type == "user": candidates = _user_options( directory.users_for_tenant(tenant_id), principal=principal, administrative=administrative, + value_kind=clean_kind, ) else: candidates = _group_options( @@ -128,17 +214,26 @@ def access_scope_reference_options( principal=principal, administrative=administrative, ) - return _filter_and_retain_options( - candidates, - query=query, - selected_values=selected, - limit=normalized_limit, - kind=clean_type, + return ReferenceSearchPage( + options=_filter_and_retain_options( + candidates, + query=query, + selected_values=selected, + limit=normalized_limit, + kind=clean_kind, + ) ) def access_scope_reference_provider_available(registry: object | None) -> bool: - return _access_directory(registry) is not None + return ( + reference_option_provider( + registry, + CAPABILITY_ACCESS_REFERENCE_OPTIONS, + ) + is not None + or _access_directory(registry) is not None + ) def validate_access_scope_reference( @@ -209,6 +304,7 @@ def _user_options( *, principal: object, administrative: bool, + value_kind: str = "user", ) -> tuple[ReferenceOption, ...]: principal_account_id = str(getattr(principal, "account_id", "") or "") options: dict[str, ReferenceOption] = {} @@ -218,11 +314,12 @@ def _user_options( inactive = user.status != "active" label = user.display_name or user.email or user.account_id detail_parts = [part for part in (user.email, "Inactive" if inactive else None) if part] - options[user.account_id] = ReferenceOption( - value=user.account_id, + value = user.id if value_kind == "membership" else user.account_id + options[value] = ReferenceOption( + value=value, label=label, description=" · ".join(detail_parts) or None, - kind="user", + kind=value_kind, availability="inactive" if inactive else "available", disabled=inactive, source_module="access", @@ -296,28 +393,58 @@ def _filter_and_retain_options( return tuple(matches) +def _bounded_page_options( + options: Sequence[ReferenceOption], + *, + selected_values: Sequence[str], + limit: int, + kind: str, +) -> tuple[ReferenceOption, ...]: + by_value = {option.value: option for option in options} + selected = set(selected_values) + bounded = [ + option for option in options if option.value not in selected + ][:limit] + returned = {option.value for option in bounded} + for value in selected_values: + if value in returned: + continue + bounded.append(by_value.get(value) or _unavailable_option(value, kind=kind)) + returned.add(value) + return tuple(bounded) + + def _fallback_scope_options( principal: object, *, scope_type: str, + reference_kind: str | None = None, query: str, selected_values: Sequence[str], limit: int, ) -> tuple[ReferenceOption, ...]: candidates: list[ReferenceOption] = [] if scope_type == "user": - account_id = str(getattr(principal, "account_id", "") or "") - if account_id: + kind = str(reference_kind or "user") + value = str( + ( + getattr(principal, "membership_id", "") + if kind == "membership" + else getattr(principal, "account_id", "") + ) + or "" + ) + if value: candidates.append( ReferenceOption( - value=account_id, + value=value, label=( str(getattr(principal, "display_name", "") or "") or str(getattr(principal, "email", "") or "") - or account_id + or value ), description="Current user · Access directory unavailable", - kind="user", + kind=kind, source_module="core", provenance={"fallback": True}, ) @@ -358,10 +485,13 @@ def _unavailable_option(value: str, *, kind: str) -> ReferenceOption: __all__ = [ + "CAPABILITY_ACCESS_REFERENCE_OPTIONS", "ReferenceAvailability", "ReferenceOption", "ReferenceOptionProvider", + "ReferenceSearchPage", "ReferenceSearchRequest", + "access_scope_reference_page", "access_scope_reference_options", "access_scope_reference_provider_available", "reference_option_provider", diff --git a/tests/test_reference_options.py b/tests/test_reference_options.py index a60f680..b149f43 100644 --- a/tests/test_reference_options.py +++ b/tests/test_reference_options.py @@ -10,6 +10,10 @@ from govoplan_core.core.access import ( UserRef, ) from govoplan_core.core.references import ( + CAPABILITY_ACCESS_REFERENCE_OPTIONS, + ReferenceOption, + ReferenceSearchPage, + access_scope_reference_page, access_scope_reference_options, validate_access_scope_reference, ) @@ -84,6 +88,38 @@ class _Registry: return self.directory if self.has_capability(name) else None +class _ReferenceProvider: + def __init__(self) -> None: + self.requests = [] + + def search_reference_options(self, session, principal, *, request): + del session, principal + self.requests.append(request) + return ReferenceSearchPage( + options=( + ReferenceOption( + value="account-1", + label="Ada", + kind="user", + source_module="access", + ), + ), + next_cursor="offset:1", + has_more=True, + ) + + +class _ProviderRegistry: + def __init__(self) -> None: + self.provider = _ReferenceProvider() + + def has_capability(self, name): + return name == CAPABILITY_ACCESS_REFERENCE_OPTIONS + + def capability(self, name): + return self.provider if self.has_capability(name) else None + + class ReferenceOptionTests(unittest.TestCase): def setUp(self) -> None: self.principal = SimpleNamespace( @@ -168,6 +204,34 @@ class ReferenceOptionTests(unittest.TestCase): self.assertEqual("account-1", options[0].value) self.assertEqual("core", options[0].source_module) + def test_bounded_provider_receives_normalized_search_and_retains_stale_selection(self) -> None: + registry = _ProviderRegistry() + + page = access_scope_reference_page( + registry, + self.principal, + scope_type="user", + query=" Ada ", + selected_values=("removed-account",), + limit=500, + cursor="offset:20", + administrative=True, + session=object(), + ) + + request = registry.provider.requests[0] + self.assertEqual("ada", request.query) + self.assertEqual(200, request.limit) + self.assertEqual("offset:20", request.cursor) + self.assertTrue(request.context["administrative"]) + self.assertEqual( + ["account-1", "removed-account"], + [option.value for option in page.options], + ) + self.assertEqual("unavailable", page.options[1].availability) + self.assertTrue(page.has_more) + self.assertEqual("offset:1", page.next_cursor) + if __name__ == "__main__": unittest.main() diff --git a/webui/src/platform/referenceProviders.ts b/webui/src/platform/referenceProviders.ts index 553b1b2..2c40c5f 100644 --- a/webui/src/platform/referenceProviders.ts +++ b/webui/src/platform/referenceProviders.ts @@ -53,8 +53,26 @@ export function apiReferenceOptionProvider( return { search: (query, context) => load(query, context.selectedValues, context.limit, context.signal), - resolve: (values, context) => - load("", values, context.limit ?? values.length, context.signal) + async resolve(values, context) { + const uniqueValues = [...new Set(values.map((value) => value.trim()).filter(Boolean))]; + const chunks: string[][] = []; + for (let index = 0; index < uniqueValues.length; index += 200) { + chunks.push(uniqueValues.slice(index, index + 200)); + } + const pages = await Promise.all( + chunks.map((chunk) => + load("", chunk, Math.min(200, Math.max(1, chunk.length)), context.signal) + ) + ); + const byValue = new Map( + pages.flat().map((option) => [option.value, option]) + ); + return uniqueValues.map( + (value) => + byValue.get(value) + ?? unavailableReferenceOption(value) + ); + } }; }