From d103596f4835787d7bb680993602651b615aef01 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 3 Sep 2026 21:28:35 +0200 Subject: [PATCH 1/5] feat(store): let the store exchange configuration instead of rows OpenRegister has two stores and neither knows about the other. federated-config-sharing is the fleet standard. A schema opts itself in with x-openregister-shareable, the built-in types cover flows, registers with their schemas, and a whole configuration set, and bundles travel signed, discoverable and gated on a trusted-key list. Its own spec calls this the store. It has no user interface, so nobody can reach it. The apphost store plane is what users see. It fetches items from one remote instance and writes each component as a plain object into one allowlisted schema. It cannot carry a schema. It cannot carry a flow. So the Store menu entry exchanges rows of one schema, and a municipality cannot publish the way it runs its council. A store block may now declare types, the shareable configuration type ids it surfaces. Declaring them makes the store list what publishers have published and install through the type that owns the bundle, so a configuration set arrives as registers, schemas, objects, views, flows, sources and mappings. An app that declares no types keeps the objects API it has today and makes no discovery call at all, so nothing that ships now changes. Discovery returns repositories, and a repository is not a bundle, so the catalogue also fixes a conventional bundle path. Without one a card can be browsed and never installed. --- lib/AppHost/Bootstrap.php | 6 + .../Controller/GenericStoreController.php | 48 ++- lib/AppHost/Service/StoreDescriptor.php | 17 + lib/AppHost/Store/FederatedStoreCatalog.php | 366 ++++++++++++++++++ lib/AppHost/Store/StoreManifest.php | 34 ++ .../store-over-federated-config/proposal.md | 92 +++++ .../specs/apphost-store-plane/spec.md | 85 ++++ .../store-over-federated-config/tasks.md | 55 +++ .../AppHost/FederatedStoreCatalogTest.php | 242 ++++++++++++ .../AppHost/GenericStoreControllerTest.php | 7 + 10 files changed, 944 insertions(+), 8 deletions(-) create mode 100644 lib/AppHost/Store/FederatedStoreCatalog.php create mode 100644 openspec/changes/store-over-federated-config/proposal.md create mode 100644 openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md create mode 100644 openspec/changes/store-over-federated-config/tasks.md create mode 100644 tests/Unit/AppHost/FederatedStoreCatalogTest.php diff --git a/lib/AppHost/Bootstrap.php b/lib/AppHost/Bootstrap.php index 71fb8e826..fafef545a 100644 --- a/lib/AppHost/Bootstrap.php +++ b/lib/AppHost/Bootstrap.php @@ -124,6 +124,11 @@ class Bootstrap { private const GENERIC_STORE_CONTROLLER = 'OCA\\OpenRegister\\AppHost\\Controller\\GenericStoreController'; private const GENERIC_STORE_SERVICE = 'OCA\\OpenRegister\\AppHost\\Service\\GenericStoreService'; private const GENERIC_STORE_INSTALLER = 'OCA\\OpenRegister\\AppHost\\Store\\GenericStoreInstaller'; + + /** + * The catalogue serving a store that exchanges configuration. + */ + private const FEDERATED_STORE_CATALOG = 'OCA\\OpenRegister\\AppHost\\Store\\FederatedStoreCatalog'; private const GENERIC_SETTINGS_SERVICE = 'OCA\\OpenRegister\\AppHost\\Service\\AppHostSettingsService'; private const GENERIC_ACTION_AUTH_SERVICE = 'OCA\\OpenRegister\\AppHost\\Service\\GenericActionAuthService'; private const GENERIC_INITIALIZE_SETTINGS = 'OCA\\OpenRegister\\AppHost\\Repair\\GenericInitializeSettings'; @@ -289,6 +294,7 @@ private static function registerControllers(IRegistrationContext $context, strin manifestLoader: $c->get(self::OBSERVABILITY_MANIFEST_LOADER), storeService: $c->get(self::GENERIC_STORE_SERVICE), installer: $c->get(self::GENERIC_STORE_INSTALLER), + catalog: $c->get(self::FEDERATED_STORE_CATALOG), userSession: $c->get('OCP\\IUserSession'), groupManager: $c->get('OCP\\IGroupManager'), logger: $c->get('Psr\\Log\\LoggerInterface') diff --git a/lib/AppHost/Controller/GenericStoreController.php b/lib/AppHost/Controller/GenericStoreController.php index a7333c7ff..5837e282c 100644 --- a/lib/AppHost/Controller/GenericStoreController.php +++ b/lib/AppHost/Controller/GenericStoreController.php @@ -46,6 +46,7 @@ use OCA\OpenRegister\AppHost\Observability\ManifestLoader; use OCA\OpenRegister\AppHost\Service\GenericStoreService; use OCA\OpenRegister\AppHost\Service\StoreDescriptor; +use OCA\OpenRegister\AppHost\Store\FederatedStoreCatalog; use OCA\OpenRegister\AppHost\Store\GenericStoreInstaller; use OCA\OpenRegister\AppHost\Store\StoreManifest; use OCP\AppFramework\Controller; @@ -83,6 +84,7 @@ class GenericStoreController extends Controller { * @param ManifestLoader $manifestLoader Loads the leaf app's manifest. * @param GenericStoreService $storeService Guarded remote discovery. * @param GenericStoreInstaller $installer Declarative component install. + * @param FederatedStoreCatalog $catalog Configuration browse and install. * @param IUserSession $userSession Current session. * @param IGroupManager $groupManager Admin check for install. * @param LoggerInterface $logger PSR logger. @@ -93,6 +95,7 @@ public function __construct( private readonly ManifestLoader $manifestLoader, private readonly GenericStoreService $storeService, private readonly GenericStoreInstaller $installer, + private readonly FederatedStoreCatalog $catalog, private readonly IUserSession $userSession, private readonly IGroupManager $groupManager, private readonly LoggerInterface $logger, @@ -150,11 +153,17 @@ public function search(): JSONResponse { } try { - $result = $this->storeService->search( - descriptor: $this->descriptor(store: $store), - query: $query, - kind: $kind - ); + $descriptor = $this->descriptor(store: $store); + + // An app that declares shareable types exchanges CONFIGURATION, so + // its catalogue is what publishers have published, not one remote + // instance's rows. Selected by declaration, never by probing: an + // app that declares none makes no discovery call at all. + if ($descriptor->isFederated() === true) { + $result = $this->catalog->search(descriptor: $descriptor, query: $query, kind: $kind); + } else { + $result = $this->storeService->search(descriptor: $descriptor, query: $query, kind: $kind); + } } catch (Throwable $e) { // Detail to the log, generic outcome to the browser: a registry's // internals are not the caller's business. @@ -171,11 +180,20 @@ public function search(): JSONResponse { // `kinds` rides back with the cards so the page can offer the filters the // APP declared, rather than a copy kept in its page config. Empty when // the app names none, and the page falls back to the shared vocabulary. + // A federated store's cards are discriminated by TYPE, so the declared + // type ids are the honest filter set when the app names no kinds of its + // own. Falling through to the shared kind vocabulary would offer chips + // (`adapter`, `agent-template`) that match nothing on this page. + $kinds = $store->kinds; + if ($kinds === [] && $store->isFederated() === true) { + $kinds = $store->declaredTypes(); + } + return new JSONResponse( data: [ 'outcome' => $result['outcome'], 'cards' => $result['cards'], - 'kinds' => $store->kinds, + 'kinds' => $kinds, ], statusCode: Http::STATUS_OK ); @@ -227,8 +245,14 @@ public function install(string $slug): JSONResponse { ); } + $descriptor = $this->descriptor(store: $store); + try { - $item = $this->storeService->resolve(descriptor: $this->descriptor(store: $store), slug: $slug); + if ($descriptor->isFederated() === true) { + $item = $this->catalog->resolve(descriptor: $descriptor, slug: $slug); + } else { + $item = $this->storeService->resolve(descriptor: $descriptor, slug: $slug); + } } catch (Throwable $e) { $this->logger->error( message: sprintf('[AppHost\\Store] resolve failed for %s: %s', $this->appName, $e->getMessage()), @@ -244,6 +268,13 @@ public function install(string $slug): JSONResponse { ); } + // A configuration bundle is applied by the type that owns it, so that a + // set arrives as registers, schemas, flows and objects rather than as + // rows of whichever schema the plane happened to allow. + if ($descriptor->isFederated() === true) { + return new JSONResponse(data: $this->catalog->install(ref: $item), statusCode: Http::STATUS_OK); + } + return new JSONResponse( data: $this->installer->install(store: $store, item: $item), statusCode: Http::STATUS_OK @@ -274,7 +305,8 @@ private function descriptor(StoreManifest $store): StoreDescriptor { appId: $this->appName, schema: $store->schema, defaultRegister: $store->register, - cardFields: $store->cardFields + cardFields: $store->cardFields, + types: $store->declaredTypes() ); }//end descriptor() }//end class diff --git a/lib/AppHost/Service/StoreDescriptor.php b/lib/AppHost/Service/StoreDescriptor.php index 01eb842e9..3e15aff3c 100644 --- a/lib/AppHost/Service/StoreDescriptor.php +++ b/lib/AppHost/Service/StoreDescriptor.php @@ -48,6 +48,11 @@ final class StoreDescriptor { * normalisation; a missing property yields an empty * string rather than a missing key, so the frontend * never has to null-check a card. + * @param array $types Shareable configuration type ids the calling app declared. + * A non-empty list selects federated discovery, where an item + * is a configuration set, a flow or a schema that marked + * itself shareable. An empty list keeps the remote objects + * API, so an app that has not moved is untouched. * * @return void */ @@ -62,6 +67,18 @@ public function __construct( 'category' => 'category', 'version' => 'version', ], + public readonly array $types = [], ) { }//end __construct() + + /** + * Whether this descriptor selects federated configuration discovery. + * + * @return bool True when the app declared at least one shareable type. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#requirement-a-store-descriptor-must-carry-every-per-app-parameter + */ + public function isFederated(): bool { + return $this->types !== []; + }//end isFederated() }//end class diff --git a/lib/AppHost/Store/FederatedStoreCatalog.php b/lib/AppHost/Store/FederatedStoreCatalog.php new file mode 100644 index 000000000..46bd13634 --- /dev/null +++ b/lib/AppHost/Store/FederatedStoreCatalog.php @@ -0,0 +1,366 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://www.OpenRegister.nl + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\AppHost\Store; + +use OCA\OpenRegister\AppHost\Service\GenericStoreService; +use OCA\OpenRegister\AppHost\Service\StoreDescriptor; +use OCA\OpenRegister\Service\Config\FederatedConfigService; +use OCA\OpenRegister\Service\Config\ShareableConfigTypeRegistry; +use Psr\Log\LoggerInterface; +use Throwable; + +/** + * Browse and install published configuration through the store surface. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#requirement-a-store-must-be-able-to-offer-configuration-not-only-objects + */ +class FederatedStoreCatalog { + /** + * Where a published repository carries its bundle. + * + * Discovery returns repositories, and a repository is not a bundle, so + * something has to say where the bundle lives. `publish()` takes a + * caller-supplied path, which is fine for a link the publisher hands you + * and useless for a browsable store: a card with no path cannot be + * installed. This is that convention, tried in order. + * + * @var array + */ + public const BUNDLE_PATHS = [ + 'openregister.json', + '.openregister/config.json', + ]; + + /** + * Constructor. + * + * @param ShareableConfigTypeRegistry $registry Resolves a declared type id to its type. + * @param FederatedConfigService $federated Discovery, fetch, trust and install. + * @param LoggerInterface $logger PSR logger, server-side detail only. + */ + public function __construct( + private readonly ShareableConfigTypeRegistry $registry, + private readonly FederatedConfigService $federated, + private readonly LoggerInterface $logger, + ) { + }//end __construct() + + /** + * The stable card slug for a published repository of one type. + * + * A card slug travels through a URL path segment, so it must satisfy the + * controller's slug pattern: lowercase alphanumerics and hyphens. The + * encoding is deterministic rather than reversible, and `resolve()` + * recomputes every candidate's slug and compares, exactly as the objects + * path compares the slug the registry actually returned. + * + * @param string $typeId The shareable type id. + * @param string $repo The `owner/repo`. + * + * @return string The card slug. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#requirement-a-configuration-install-must-run-through-its-owning-type + */ + public static function slugFor(string $typeId, string $repo): string { + $raw = strtolower($typeId . '-' . $repo); + $slug = preg_replace('/[^a-z0-9]+/', '-', $raw); + return trim((string)$slug, '-'); + }//end slugFor() + + /** + * Browse every declared type's published configuration. + * + * @param StoreDescriptor $descriptor The calling app's store parameters. + * @param string|null $query Optional free-text filter. + * @param string|null $kind Optional type-id filter, matching the surface's kind chips. + * + * @return array{outcome: string, cards: array>} + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#scenario-a-configuration-set-reaches-the-store-surface + */ + public function search(StoreDescriptor $descriptor, ?string $query = null, ?string $kind = null): array { + $cards = []; + + foreach ($descriptor->types as $typeId) { + $type = $this->registry->get(id: $typeId); + if ($type === null) { + // A declared type nothing owns is a manifest error, not a + // runtime failure: name it in the log and carry on, so one + // stale id does not blank the whole store. + $this->logger->warning( + message: sprintf( + '[AppHost\\Store] %s declares shareable type %s, which no app owns', + $descriptor->appId, + $typeId + ), + context: ['file' => __FILE__, 'line' => __LINE__] + ); + continue; + } + + if ($kind !== null && trim($kind) !== '' && trim($kind) !== $typeId) { + continue; + } + + try { + $found = $this->federated->discover(topic: $type->getTopic()); + } catch (Throwable $e) { + $this->logger->warning( + message: sprintf('[AppHost\\Store] discovery failed for %s: %s', $typeId, $e->getMessage()), + context: ['file' => __FILE__, 'line' => __LINE__] + ); + continue; + } + + foreach ($found as $entry) { + $card = $this->toCard(typeId: $typeId, displayName: $type->getDisplayName(), entry: $entry); + if ($this->matches(card: $card, query: $query) === true) { + $cards[] = $card; + } + } + }//end foreach + + return ['outcome' => GenericStoreService::OUTCOME_OK, 'cards' => $cards]; + }//end search() + + /** + * Resolve a card slug back to the bundle it names. + * + * @param StoreDescriptor $descriptor The calling app's store parameters. + * @param string $slug The card slug. + * + * @return array{typeId: string, repo: string, source: string, bundle: array}|null + * The resolved bundle, or null when nothing matches. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#requirement-a-configuration-install-must-run-through-its-owning-type + */ + public function resolve(StoreDescriptor $descriptor, string $slug): ?array { + foreach ($descriptor->types as $typeId) { + $type = $this->registry->get(id: $typeId); + if ($type === null) { + continue; + } + + try { + $found = $this->federated->discover(topic: $type->getTopic()); + } catch (Throwable $e) { + $this->logger->warning( + message: sprintf('[AppHost\\Store] discovery failed for %s: %s', $typeId, $e->getMessage()), + context: ['file' => __FILE__, 'line' => __LINE__] + ); + continue; + } + + foreach ($found as $entry) { + $repo = (string)($entry['repo'] ?? ''); + if ($repo === '' || self::slugFor(typeId: $typeId, repo: $repo) !== $slug) { + continue; + } + + $bundle = $this->fetch(repo: $repo); + if ($bundle === null) { + return null; + } + + return [ + 'typeId' => $typeId, + 'repo' => $repo, + 'source' => (string)($entry['url'] ?? $repo), + 'bundle' => $bundle, + ]; + } + }//end foreach + + return null; + }//end resolve() + + /** + * Install a resolved bundle through the type that owns it. + * + * The source check runs BEFORE the install rather than inside it. A bundle + * from a publisher this organisation has not trusted is refused whole: a + * half-applied configuration set is worse than none, because nothing then + * says which half arrived. + * + * @param array $ref The resolved bundle from {@see self::resolve()}, shaped + * `{typeId: string, repo: string, source: string, bundle: array}`. + * + * @return array{success: bool, components: array>} The install report. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#scenario-an-untrusted-publisher-is-refused + */ + public function install(array $ref): array { + $source = (string)($ref['source'] ?? ''); + + if ($this->federated->isSourceAllowed(source: $source) === false) { + return [ + 'success' => false, + 'components' => [ + [ + 'schema' => (string)($ref['typeId'] ?? ''), + 'status' => 'refused', + 'message' => sprintf('This organisation does not trust %s as a store source.', $source), + ], + ], + ]; + } + + try { + $result = $this->federated->install( + typeId: (string)$ref['typeId'], + bundle: (array)$ref['bundle'], + source: $source + ); + } catch (Throwable $e) { + $this->logger->error( + message: sprintf('[AppHost\\Store] installing %s failed: %s', (string)$ref['typeId'], $e->getMessage()), + context: ['file' => __FILE__, 'line' => __LINE__] + ); + return [ + 'success' => false, + 'components' => [ + [ + 'schema' => (string)$ref['typeId'], + 'status' => 'error', + 'message' => 'The configuration could not be installed.', + ], + ], + ]; + }//end try + + $installed = ($result['installed'] ?? []); + if (is_array($installed) === false) { + $installed = []; + } + + $components = []; + foreach ($installed as $entry) { + if (is_string($entry) === true) { + $name = $entry; + } else { + $name = (string)($entry['type'] ?? $ref['typeId']); + } + + $components[] = [ + 'schema' => $name, + 'status' => 'installed', + 'message' => '', + ]; + } + + if ($components === []) { + $components[] = ['schema' => (string)$ref['typeId'], 'status' => 'installed', 'message' => '']; + } + + return ['success' => true, 'components' => $components]; + }//end install() + + /** + * Read a repository's bundle from the first conventional path that answers. + * + * @param string $repo The `owner/repo`. + * + * @return array|null The decoded bundle, or null when no path answers. + */ + private function fetch(string $repo): ?array { + foreach (self::BUNDLE_PATHS as $path) { + try { + $bundle = $this->federated->fetchBundle(repo: $repo, path: $path); + } catch (Throwable $e) { + continue; + } + + if ($bundle !== []) { + return $bundle; + } + } + + $this->logger->warning( + message: sprintf('[AppHost\\Store] %s carries no bundle at a conventional path', $repo), + context: ['file' => __FILE__, 'line' => __LINE__] + ); + + return null; + }//end fetch() + + /** + * Turn one discovered repository into a store card. + * + * @param string $typeId The shareable type id. + * @param string $displayName The type's human name. + * @param array $entry One discovery result. + * + * @return array The card. + */ + private function toCard(string $typeId, string $displayName, array $entry): array { + $repo = (string)($entry['repo'] ?? ''); + $owner = explode('/', $repo)[0]; + + return [ + 'slug' => self::slugFor(typeId: $typeId, repo: $repo), + 'title' => (string)($entry['name'] ?? $repo), + 'description' => (string)($entry['description'] ?? ''), + // The surface's kind chip is the type, because that is the honest + // discriminator here: a set, a flow and a schema are what differ. + 'kind' => $typeId, + 'type' => $typeId, + 'typeName' => $displayName, + 'publisher' => $owner, + 'source' => (string)($entry['url'] ?? ''), + 'updated' => (string)($entry['updated'] ?? ''), + 'category' => '', + 'version' => '', + ]; + }//end toCard() + + /** + * Whether a card survives the free-text filter. + * + * @param array $card The card. + * @param string|null $query The filter, or null for no filter. + * + * @return bool + */ + private function matches(array $card, ?string $query): bool { + if ($query === null || trim($query) === '') { + return true; + } + + $needle = mb_strtolower(trim($query)); + $hay = mb_strtolower( + (string)$card['title'] . ' ' . (string)$card['description'] . ' ' . (string)$card['publisher'] + ); + + return str_contains($hay, $needle); + }//end matches() +}//end class diff --git a/lib/AppHost/Store/StoreManifest.php b/lib/AppHost/Store/StoreManifest.php index cc1675f6d..3fc623b62 100644 --- a/lib/AppHost/Store/StoreManifest.php +++ b/lib/AppHost/Store/StoreManifest.php @@ -75,6 +75,7 @@ class StoreManifest { * @param array $kinds Kind quick-filters offered on the page. * @param array $cardFields Remote field to card field map. * @param array> $builtIn The app's own items, for the no-registry case. + * @param array $types Shareable configuration type ids this store surfaces. */ public function __construct( public readonly string $appId, @@ -86,6 +87,7 @@ public function __construct( public readonly array $kinds = [], public readonly array $cardFields = self::DEFAULT_CARD_FIELDS, public readonly array $builtIn = [], + public readonly array $types = [], ) { }//end __construct() @@ -135,9 +137,41 @@ public static function fromManifest(string $appId, array $manifest): self { kinds: self::stringList(value: ($block['kinds'] ?? [])), cardFields: array_map(callback: static fn ($v): string => (string)$v, array: $cardFields), builtIn: self::objectList(value: ($block['builtIn'] ?? [])), + // Shareable configuration type ids (store-over-federated-config). + // Declaring these selects federated discovery, where an item is a + // configuration set, a flow or a schema that marked itself + // shareable. An app that declares none keeps the remote objects + // API it has today, so nothing that ships now changes. + types: self::stringList(value: ($block['types'] ?? [])), ); }//end fromManifest() + /** + * The shareable configuration type ids this store surfaces. + * + * @return array The declared type ids, in declaration order. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#requirement-a-store-must-be-able-to-offer-configuration-not-only-objects + */ + public function declaredTypes(): array { + return $this->types; + }//end declaredTypes() + + /** + * Whether this store exchanges configuration rather than objects. + * + * The two paths are selected by declaration, never by a runtime probe: an + * app that declares types gets federated discovery, and one that declares + * none never makes a discovery call at all. + * + * @return bool True when the app declared at least one shareable type. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#requirement-a-store-must-be-able-to-offer-configuration-not-only-objects + */ + public function isFederated(): bool { + return $this->types !== []; + }//end isFederated() + /** * Whether an install may write into this schema slug. * diff --git a/openspec/changes/store-over-federated-config/proposal.md b/openspec/changes/store-over-federated-config/proposal.md new file mode 100644 index 000000000..5a4fe6ceb --- /dev/null +++ b/openspec/changes/store-over-federated-config/proposal.md @@ -0,0 +1,92 @@ +# The store exchanges configuration, not rows + +## Problem + +OpenRegister has two stores. They were built for the same purpose, neither +references the other, and the weaker one is the one users can see. + +**`federated-config-sharing`** is the fleet standard. A schema opts itself in +with one marker, `x-openregister-shareable`, and `SchemaShareableConfigScanner` +turns it into a shareable type without any per-app code. Three types ship +built in: flows, registers and schemas, and a whole configuration set, which +`ConfigSetShareableConfigType` documents as "an app's worth of configuration at +once: registers, schemas, objects, views, flows, sources and mappings". Bundles +are signed with the instance's Ed25519 key, published to a repository, found by +topic, and gated on a per-org trusted-key list. The spec calls this the store: +"A schema SHALL be able to opt its objects into the store with a single marker." + +Twenty one of its twenty five tasks are done. The backend works. It has no user +interface at all, so nobody can reach it. + +**`apphost-store-plane`** is what users actually see. `CnStorePage` calls +`/api/store/items`, the engine fetches items from one remote OpenRegister +instance over HTTP, and `GenericStoreInstaller` writes each component of an item +as a plain object into one allowlisted schema. It cannot carry a schema. It +cannot carry a flow. It reads its allowlist from the consuming app's manifest, +so the schema never gets to say whether it may travel. + +The result is a Store menu entry that exchanges rows of a single schema. A +municipality cannot publish the way it runs its council. A neighbouring +municipality cannot install it. + +## What a store item should be + +A store item is a collection of schemas and flows that work together to provide +a functionality. Decidiq publishes a default gemeente: the organisational +structure, the decision types, and the flows that move a decision through them. +Dossiq publishes case types. Portaliq publishes forms. Buildiq publishes whole +apps. Humaniq publishes tax schemas. Shillinq publishes administration setups. + +Each of those is one shape, not six. `ConfigSetShareableConfigType` already is +that shape. + +**The fleet has already written these, and calls them seed datasets.** Decidiq +ships four in `lib/Settings/profiles/`: a municipality with committees and +factions, an association with a members' meeting and a board, a company board +with a supervisory and an executive layer, and a works council. Every one is a +named organisational structure with the schemas and vocabulary to run it, which +is a configuration set with a different file extension. `SeedProfileService` +already imports one on request. + +So the first store catalogue is not something anyone has to invent. It is the +seed data each app already ships, published instead of bundled. + +## Solution + +Point the store surface at the engine that already does this, and delete the +duplicate exchange rather than grow it. + +- **`GenericStoreController::search()` sources its cards from + `FederatedConfigService::discover()`**, across the topics of the shareable + types the calling app declares, instead of from one remote objects API. +- **`install()` resolves the card and calls `FederatedConfigService::install()`**, + which routes to the owning type's `deserialise()`. A config set arrives as + registers, schemas, objects, views, flows, sources and mappings. A flow + arrives as a flow. +- **The schema decides whether it travels.** `x-openregister-shareable` is + already read by the scanner, so an app that marks a schema gets it in the + store with no manifest change. The manifest `installable` allowlist stays as + the second boundary, because a remote publisher naming a schema is not the + same claim as that schema being shareable. +- **Trust moves with it.** A federated install already passes + `isSourceAllowed()` and the trusted-key check, which the object install path + has no equivalent for. + +The manifest `store` block gains `types`, the shareable type ids an app +surfaces. `schema`, `register` and `cardFields` stay, and an app that declares +only those keeps the object store it has today, so nothing that ships now +breaks while the fleet moves. + +## Affected Projects + +- [x] Project: `openregister` — the engine and the store controller. +- [x] Project: `nextcloud-vue` — `CnStorePage` renders a type and a publisher. +- [x] Project: `decidiq` — first consumer, publishes a default gemeente. + +## Out of scope + +Retiring the object store path. It stays until every app that uses it has +moved, and this change does not move them. + +Dossiq's hand-written `StoreController`, which predates both engines and wins +the route alias by design. It moves in its own change. diff --git a/openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md b/openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md new file mode 100644 index 000000000..dd78a4f9f --- /dev/null +++ b/openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md @@ -0,0 +1,85 @@ +## ADDED Requirements + +### Requirement: A store MUST be able to offer configuration, not only objects + +A leaf app SHALL be able to declare `store.types`, a list of shareable +configuration type ids. When it does, the store surface SHALL list what +`FederatedConfigService::discover()` returns for each declared type's topic, +and each card SHALL carry the type id that produced it. + +A store item found this way MAY contain registers, schemas, objects, views, +flows, sources and mappings together, because that is what a configuration set +is. The plane SHALL NOT flatten such an item into objects of one schema. + +#### Scenario: A configuration set reaches the store surface + +- **GIVEN** an app declaring `store.types: ["openregister.config-set"]` +- **AND** a published configuration set tagged with that type's topic +- **WHEN** an administrator opens the app's store +- **THEN** the set is listed as one card naming its type and publisher + +#### Scenario: An app that declares no types keeps the object store + +- **GIVEN** an app declaring only `store.schema` and `store.register` +- **WHEN** the store is searched +- **THEN** the remote objects API is used exactly as before +- **AND** no discovery call is made + +### Requirement: A schema MUST decide whether it may be shared + +A schema carrying `x-openregister-shareable` in its configuration SHALL be +offered as a shareable type by `SchemaShareableConfigScanner`, and the store +surface SHALL list it for any app that declares its type id. + +A schema without that marker SHALL NOT be offered, even when the app's +`installable` allowlist names it. The allowlist governs what an install may +write into this instance; the marker governs whether the schema may travel at +all, and the two SHALL both hold. + +#### Scenario: An unmarked schema is not offered + +- **GIVEN** a schema absent `x-openregister-shareable` +- **AND** an app whose `installable` list names that schema +- **WHEN** the store is searched +- **THEN** the schema is not listed as a shareable type + +### Requirement: A configuration install MUST run through its owning type + +Installing a card that carries a type id SHALL fetch the bundle and call +`FederatedConfigService::install()`, which routes to that type's +`deserialise()`. The plane SHALL NOT write the bundle's contents directly. + +The install SHALL refuse a source that `isSourceAllowed()` rejects, and SHALL +report the refusal rather than installing part of the bundle. + +#### Scenario: An untrusted publisher is refused + +- **GIVEN** a card published by a source outside the org allowlist +- **WHEN** an administrator installs it +- **THEN** the install is refused and names the source +- **AND** nothing is written + +#### Scenario: A flow arrives as a flow + +- **GIVEN** a published bundle of type `openregister.flows` +- **WHEN** an administrator installs it +- **THEN** the flow exists on this instance +- **AND** it is not written as an object of some other schema + +## MODIFIED Requirements + +### Requirement: A store descriptor MUST carry every per-app parameter + +The descriptor SHALL carry the app id, the remote schema slug, the default +register and the card-field map, so that everything differing between apps is +data and everything shared lives once in `GenericStoreService`. + +It SHALL additionally carry the declared shareable type ids. A descriptor with +type ids selects federated discovery; a descriptor with none keeps the remote +objects API, so an app that has not moved is unaffected. + +#### Scenario: A descriptor names its types + +- **GIVEN** a manifest declaring two shareable type ids +- **WHEN** the descriptor is built +- **THEN** it carries both ids in declaration order diff --git a/openspec/changes/store-over-federated-config/tasks.md b/openspec/changes/store-over-federated-config/tasks.md new file mode 100644 index 000000000..32ecb2e79 --- /dev/null +++ b/openspec/changes/store-over-federated-config/tasks.md @@ -0,0 +1,55 @@ +# Tasks + +## 1. Engine + +- [ ] 1.1 `StoreManifest` parses `types` and exposes `declaredTypes()` +- [ ] 1.2 `StoreDescriptor` carries the declared type ids +- [ ] 1.3 `GenericStoreService::searchFederated()` calls + `FederatedConfigService::discover()` per declared topic and normalises the + results to the same card shape the objects path returns +- [ ] 1.4 A card carries `type`, `publisher` and `source` alongside the existing + fields, so the surface can show where a set came from +- [ ] 1.5 `GenericStoreService::resolveFederated()` maps a card slug back to its + repo, path and type id + +## 2. Install + +- [ ] 2.1 `GenericStoreController::install()` routes a card with a type id + through `FederatedConfigService::install()` +- [ ] 2.2 A source outside the org allowlist is refused before any fetch, and + the refusal names the source +- [ ] 2.3 The object install path is unchanged for a card with no type id + +## 3. Selection + +- [ ] 3.1 A descriptor with no declared types keeps the objects API, verified by + a test that asserts no discovery call is made + +## 4. Surface + +- [ ] 4.1 `CnStorePage` renders the card's type and publisher +- [ ] 4.2 Kind filters fall back to type display names when an app declares no + kinds + +## 5. First consumer + +- [ ] 5.1 Decidiq declares `store.types` and a Store menu entry at footer 92 +- [ ] 5.2 Decidiq marks the schemas that may travel with + `x-openregister-shareable` +- [ ] 5.3 A default gemeente configuration set is published and installs onto a + clean instance +- [ ] 5.4 The four example sets in `lib/Settings/profiles/` are offered as + built-in store items, so the store names what decidiq already ships + rather than rendering blank without a registry +- [ ] 5.5 An example set serialises to a configuration set, so the seed data an + app bundles and the configuration it publishes are one artefact rather + than two formats holding the same thing + +## 6. Verification + +- [ ] 6.1 Unit tests for descriptor selection, discovery normalisation and + install routing +- [ ] 6.2 An e2e test opens a store declaring types and asserts a config-set + card renders +- [ ] 6.3 Live check on a throwaway instance: publish a set from one instance, + install it on another diff --git a/tests/Unit/AppHost/FederatedStoreCatalogTest.php b/tests/Unit/AppHost/FederatedStoreCatalogTest.php new file mode 100644 index 000000000..cc4b7e071 --- /dev/null +++ b/tests/Unit/AppHost/FederatedStoreCatalogTest.php @@ -0,0 +1,242 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://www.OpenRegister.nl + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Tests\Unit\AppHost; + +use OCA\OpenRegister\AppHost\Service\StoreDescriptor; +use OCA\OpenRegister\AppHost\Store\FederatedStoreCatalog; +use OCA\OpenRegister\AppHost\Store\StoreManifest; +use OCA\OpenRegister\Service\Config\FederatedConfigService; +use OCA\OpenRegister\Service\Config\IShareableConfigType; +use OCA\OpenRegister\Service\Config\ShareableConfigTypeRegistry; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; + +/** + * The store's catalogue when an app exchanges configuration. + * + * @covers \OCA\OpenRegister\AppHost\Store\FederatedStoreCatalog + * + * @uses \OCA\OpenRegister\AppHost\Service\StoreDescriptor + * @uses \OCA\OpenRegister\AppHost\Store\StoreManifest + */ +class FederatedStoreCatalogTest extends TestCase { + /** @var ShareableConfigTypeRegistry&MockObject */ + private $registry; + + /** @var FederatedConfigService&MockObject */ + private $federated; + + private FederatedStoreCatalog $catalog; + + /** + * Build the collaborators. + * + * @return void + */ + protected function setUp(): void { + parent::setUp(); + $this->registry = $this->getMockBuilder(ShareableConfigTypeRegistry::class) + ->disableOriginalConstructor()->onlyMethods(['get'])->getMock(); + $this->federated = $this->getMockBuilder(FederatedConfigService::class) + ->disableOriginalConstructor() + ->onlyMethods(['discover', 'fetchBundle', 'install', 'isSourceAllowed']) + ->getMock(); + + $this->catalog = new FederatedStoreCatalog( + registry: $this->registry, + federated: $this->federated, + logger: $this->createMock(LoggerInterface::class) + ); + }//end setUp() + + /** + * A descriptor naming one type. + * + * @param array $types The declared type ids. + * + * @return StoreDescriptor + */ + private function descriptor(array $types = ['openregister.configset']): StoreDescriptor { + return new StoreDescriptor( + appId: 'decidiq', + schema: '', + defaultRegister: '', + types: $types + ); + }//end descriptor() + + /** + * A registered type stub. + * + * @param string $id The type id. + * @param string $topic The discovery topic. + * @param string $name The display name. + * + * @return IShareableConfigType&MockObject + */ + private function type(string $id, string $topic, string $name) { + $type = $this->createMock(IShareableConfigType::class); + $type->method('getId')->willReturn($id); + $type->method('getTopic')->willReturn($topic); + $type->method('getDisplayName')->willReturn($name); + return $type; + }//end type() + + /** + * A discovered configuration set becomes one card naming its type. + * + * @return void + */ + public function testDiscoveredSetBecomesACard(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([ + [ + 'repo' => 'ConductionNL/default-gemeente', + 'name' => 'default-gemeente', + 'description' => 'A council with committees and factions.', + 'url' => 'https://github.com/ConductionNL/default-gemeente', + ], + ]); + + $result = $this->catalog->search(descriptor: $this->descriptor()); + + $this->assertSame('ok', $result['outcome']); + $this->assertCount(1, $result['cards']); + $this->assertSame('openregister.configset', $result['cards'][0]['type']); + $this->assertSame('Configuration set', $result['cards'][0]['typeName']); + $this->assertSame('ConductionNL', $result['cards'][0]['publisher']); + }//end testDiscoveredSetBecomesACard() + + /** + * A declared type nothing owns is skipped, not fatal. + * + * One stale id in a manifest must not blank the whole store. + * + * @return void + */ + public function testUnownedTypeIsSkipped(): void { + $this->registry->method('get')->willReturn(null); + $this->federated->expects($this->never())->method('discover'); + + $result = $this->catalog->search(descriptor: $this->descriptor(types: ['nobody.owns-this'])); + + $this->assertSame([], $result['cards']); + }//end testUnownedTypeIsSkipped() + + /** + * An untrusted publisher is refused before anything is written. + * + * @return void + */ + public function testUntrustedSourceIsRefused(): void { + $this->federated->method('isSourceAllowed')->willReturn(false); + $this->federated->expects($this->never())->method('install'); + + $report = $this->catalog->install( + ref: [ + 'typeId' => 'openregister.configset', + 'repo' => 'someone/else', + 'source' => 'https://github.com/someone/else', + 'bundle' => ['type' => 'openregister.configset'], + ] + ); + + $this->assertFalse($report['success']); + $this->assertSame('refused', $report['components'][0]['status']); + }//end testUntrustedSourceIsRefused() + + /** + * A trusted bundle is applied by the type that owns it. + * + * @return void + */ + public function testTrustedBundleInstallsThroughItsType(): void { + $this->federated->method('isSourceAllowed')->willReturn(true); + $this->federated->expects($this->once()) + ->method('install') + ->with('openregister.configset', ['type' => 'openregister.configset'], 'https://github.com/c/d') + ->willReturn(['installed' => ['registers', 'schemas', 'flows']]); + + $report = $this->catalog->install( + ref: [ + 'typeId' => 'openregister.configset', + 'repo' => 'c/d', + 'source' => 'https://github.com/c/d', + 'bundle' => ['type' => 'openregister.configset'], + ] + ); + + $this->assertTrue($report['success']); + $this->assertCount(3, $report['components']); + }//end testTrustedBundleInstallsThroughItsType() + + /** + * The card slug survives a round trip through the URL pattern. + * + * @return void + */ + public function testSlugIsUrlSafe(): void { + $slug = FederatedStoreCatalog::slugFor( + typeId: 'openregister.configset', + repo: 'ConductionNL/default-gemeente' + ); + + $this->assertSame('openregister-configset-conductionnl-default-gemeente', $slug); + $this->assertMatchesRegularExpression('/^[a-z0-9][a-z0-9-]*[a-z0-9]$/', $slug); + }//end testSlugIsUrlSafe() + + /** + * An app that declares no types is not a federated store. + * + * The two paths are selected by declaration, so an app that has not moved + * never reaches discovery at all. + * + * @return void + */ + public function testNoDeclaredTypesIsNotFederated(): void { + $store = StoreManifest::fromManifest( + appId: 'dossiq', + manifest: ['store' => ['schema' => 'case-type-template']] + ); + + $this->assertFalse($store->isFederated()); + $this->assertSame([], $store->declaredTypes()); + $this->assertFalse($this->descriptor(types: [])->isFederated()); + }//end testNoDeclaredTypesIsNotFederated() + + /** + * Declared types are carried through the manifest in order. + * + * @return void + */ + public function testDeclaredTypesAreCarried(): void { + $store = StoreManifest::fromManifest( + appId: 'decidiq', + manifest: ['store' => ['types' => ['openregister.configset', 'openregister.flows']]] + ); + + $this->assertTrue($store->isFederated()); + $this->assertSame(['openregister.configset', 'openregister.flows'], $store->declaredTypes()); + }//end testDeclaredTypesAreCarried() +}//end class diff --git a/tests/Unit/AppHost/GenericStoreControllerTest.php b/tests/Unit/AppHost/GenericStoreControllerTest.php index 4ef6f7ffd..5491ba836 100644 --- a/tests/Unit/AppHost/GenericStoreControllerTest.php +++ b/tests/Unit/AppHost/GenericStoreControllerTest.php @@ -35,6 +35,7 @@ use OCA\OpenRegister\AppHost\Controller\GenericStoreController; use OCA\OpenRegister\AppHost\Observability\ManifestLoader; use OCA\OpenRegister\AppHost\Service\GenericStoreService; +use OCA\OpenRegister\AppHost\Store\FederatedStoreCatalog; use OCA\OpenRegister\AppHost\Store\GenericStoreInstaller; use OCA\OpenRegister\AppHost\Store\StoreManifest; use OCP\AppFramework\Http; @@ -71,6 +72,9 @@ class GenericStoreControllerTest extends TestCase { /** @var GenericStoreInstaller&MockObject */ private $installer; + /** @var FederatedStoreCatalog&MockObject */ + private $catalog; + /** @var IUserSession&MockObject */ private $userSession; @@ -96,6 +100,8 @@ protected function setUp(): void { ->disableOriginalConstructor()->onlyMethods(['search', 'resolve'])->getMock(); $this->installer = $this->getMockBuilder(GenericStoreInstaller::class) ->disableOriginalConstructor()->onlyMethods(['install'])->getMock(); + $this->catalog = $this->getMockBuilder(FederatedStoreCatalog::class) + ->disableOriginalConstructor()->onlyMethods(['search', 'resolve', 'install'])->getMock(); $this->userSession = $this->createMock(IUserSession::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->request = $this->createMock(IRequest::class); @@ -115,6 +121,7 @@ private function controller(string $appId = 'dossiq'): GenericStoreController { manifestLoader: $this->manifestLoader, storeService: $this->storeService, installer: $this->installer, + catalog: $this->catalog, userSession: $this->userSession, groupManager: $this->groupManager, logger: $this->createMock(LoggerInterface::class) From edf875d1e2b72265cbd11ed9a82b86f869285792 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 3 Sep 2026 21:35:35 +0200 Subject: [PATCH 2/5] docs(store): say plainly that a schema allowlist cannot gate a configuration set The requirement as first written said the installable allowlist and the shareable marker both have to hold for a configuration install. That is not what the code does, and on reflection it is not what it should do. A configuration set exists to introduce registers, schemas and flows the instance does not have yet. A list of schemas the app already owns cannot express whether such a set may be applied: it would refuse exactly the sets worth installing. The trust boundary for a bundle is its publisher. The allowlist keeps its meaning on the objects path, where an item names a schema the app does own. --- .../specs/apphost-store-plane/spec.md | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md b/openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md index dd78a4f9f..2cf172296 100644 --- a/openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md +++ b/openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md @@ -32,9 +32,24 @@ offered as a shareable type by `SchemaShareableConfigScanner`, and the store surface SHALL list it for any app that declares its type id. A schema without that marker SHALL NOT be offered, even when the app's -`installable` allowlist names it. The allowlist governs what an install may -write into this instance; the marker governs whether the schema may travel at -all, and the two SHALL both hold. +`installable` allowlist names it. The marker governs whether the schema may +travel at all, which is a different question from what an install may write. + +The `installable` allowlist SHALL NOT gate a configuration install, and this is +deliberate. A configuration set exists to introduce registers, schemas and +flows the instance does not have yet, so a list of schemas the app already owns +cannot express whether that set may be applied: it would refuse exactly the +sets worth installing. The trust boundary for a bundle is its PUBLISHER, and it +is enforced by `isSourceAllowed()` and the trusted-key check. The allowlist +keeps its meaning for the objects path, where an item names a schema the app +does own. + +#### Scenario: A set may introduce a schema the app does not own + +- **GIVEN** a configuration set carrying a schema absent from `installable` +- **AND** a publisher this organisation trusts +- **WHEN** an administrator installs it +- **THEN** the schema is created #### Scenario: An unmarked schema is not offered From ff47bf98d5789e298ef96499e8b645f47044b05c Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 3 Sep 2026 21:47:46 +0200 Subject: [PATCH 3/5] fix(store): satisfy phpmd, and cover resolve and the bundle paths phpcs refuses an inline if and phpmd refuses an else, so the branch that picks a catalogue moves into two private methods that return early. Two suppressions with their reasons. The controller holds both store paths because one route alias serves every app, and splitting it would put that alias in two places. StoreManifest mirrors the manifest block one key to one parameter, and a grouping layer is exactly where a silently dropped key hides. The coverage ratchet caught resolve() and the bundle-path fallback having no test at all, which is fair: they are the half that decides what gets installed. Nine tests added, covering a slug resolving to its bundle, every conventional path being tried, a contained discovery failure, and both shapes a type may report its components in. --- .../Controller/GenericStoreController.php | 66 ++++-- lib/AppHost/Store/FederatedStoreCatalog.php | 32 ++- lib/AppHost/Store/StoreManifest.php | 6 + .../AppHost/FederatedStoreCatalogTest.php | 188 ++++++++++++++++++ 4 files changed, 270 insertions(+), 22 deletions(-) diff --git a/lib/AppHost/Controller/GenericStoreController.php b/lib/AppHost/Controller/GenericStoreController.php index 5837e282c..1448f00e1 100644 --- a/lib/AppHost/Controller/GenericStoreController.php +++ b/lib/AppHost/Controller/GenericStoreController.php @@ -64,6 +64,13 @@ * * @psalm-suppress UnusedClass * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) One controller serves every + * AppHost app across BOTH store paths, so it holds the objects client, the + * objects installer and the configuration catalogue at once. Splitting it per + * path would put the route alias in two places, and an alias that resolves to + * a class the router cannot find is a dispatch-time 500 rather than a test + * failure. The coupling is the price of one alias. + * * @spec openspec/specs/apphost-store-plane/spec.md#requirement-a-leaf-app-must-declare-its-store-rather-than-implement-one */ class GenericStoreController extends Controller { @@ -154,16 +161,7 @@ public function search(): JSONResponse { try { $descriptor = $this->descriptor(store: $store); - - // An app that declares shareable types exchanges CONFIGURATION, so - // its catalogue is what publishers have published, not one remote - // instance's rows. Selected by declaration, never by probing: an - // app that declares none makes no discovery call at all. - if ($descriptor->isFederated() === true) { - $result = $this->catalog->search(descriptor: $descriptor, query: $query, kind: $kind); - } else { - $result = $this->storeService->search(descriptor: $descriptor, query: $query, kind: $kind); - } + $result = $this->searchFor(descriptor: $descriptor, query: $query, kind: $kind); } catch (Throwable $e) { // Detail to the log, generic outcome to the browser: a registry's // internals are not the caller's business. @@ -248,11 +246,7 @@ public function install(string $slug): JSONResponse { $descriptor = $this->descriptor(store: $store); try { - if ($descriptor->isFederated() === true) { - $item = $this->catalog->resolve(descriptor: $descriptor, slug: $slug); - } else { - $item = $this->storeService->resolve(descriptor: $descriptor, slug: $slug); - } + $item = $this->resolveFor(descriptor: $descriptor, slug: $slug); } catch (Throwable $e) { $this->logger->error( message: sprintf('[AppHost\\Store] resolve failed for %s: %s', $this->appName, $e->getMessage()), @@ -281,6 +275,48 @@ public function install(string $slug): JSONResponse { ); }//end install() + /** + * Search whichever catalogue this app declared. + * + * An app that declares shareable types exchanges CONFIGURATION, so its + * catalogue is what publishers have published, not one remote instance's + * rows. Selected by declaration, never by probing: an app that declares + * none makes no discovery call at all. + * + * @param StoreDescriptor $descriptor The calling app's store parameters. + * @param string|null $query Optional free-text search term. + * @param string|null $kind Optional kind filter. + * + * @return array{outcome: string, cards: array>} + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#scenario-an-app-that-declares-no-types-keeps-the-object-store + */ + private function searchFor(StoreDescriptor $descriptor, ?string $query, ?string $kind): array { + if ($descriptor->isFederated() === true) { + return $this->catalog->search(descriptor: $descriptor, query: $query, kind: $kind); + } + + return $this->storeService->search(descriptor: $descriptor, query: $query, kind: $kind); + }//end searchFor() + + /** + * Resolve a slug against whichever catalogue this app declared. + * + * @param StoreDescriptor $descriptor The calling app's store parameters. + * @param string $slug The item slug. + * + * @return array|null The resolved item, or null when unresolved. + * + * @spec openspec/changes/store-over-federated-config/specs/apphost-store-plane/spec.md#requirement-a-configuration-install-must-run-through-its-owning-type + */ + private function resolveFor(StoreDescriptor $descriptor, string $slug): ?array { + if ($descriptor->isFederated() === true) { + return $this->catalog->resolve(descriptor: $descriptor, slug: $slug); + } + + return $this->storeService->resolve(descriptor: $descriptor, slug: $slug); + }//end resolveFor() + /** * The calling app's declared store configuration. * diff --git a/lib/AppHost/Store/FederatedStoreCatalog.php b/lib/AppHost/Store/FederatedStoreCatalog.php index 46bd13634..5070ee8b0 100644 --- a/lib/AppHost/Store/FederatedStoreCatalog.php +++ b/lib/AppHost/Store/FederatedStoreCatalog.php @@ -265,14 +265,8 @@ public function install(array $ref): array { $components = []; foreach ($installed as $entry) { - if (is_string($entry) === true) { - $name = $entry; - } else { - $name = (string)($entry['type'] ?? $ref['typeId']); - } - $components[] = [ - 'schema' => $name, + 'schema' => $this->componentName(entry: $entry, fallback: (string)$ref['typeId']), 'status' => 'installed', 'message' => '', ]; @@ -285,6 +279,30 @@ public function install(array $ref): array { return ['success' => true, 'components' => $components]; }//end install() + /** + * Name one installed component for the report. + * + * A type reports what it installed in its own terms: some return plain + * names, others return descriptors. Both are read here rather than + * assuming one shape. + * + * @param mixed $entry One entry from the type's install result. + * @param string $fallback The type id, used when the entry names nothing. + * + * @return string The component name. + */ + private function componentName(mixed $entry, string $fallback): string { + if (is_string($entry) === true) { + return $entry; + } + + if (is_array($entry) === true) { + return (string)($entry['type'] ?? $fallback); + } + + return $fallback; + }//end componentName() + /** * Read a repository's bundle from the first conventional path that answers. * diff --git a/lib/AppHost/Store/StoreManifest.php b/lib/AppHost/Store/StoreManifest.php index 3fc623b62..5161c4bcc 100644 --- a/lib/AppHost/Store/StoreManifest.php +++ b/lib/AppHost/Store/StoreManifest.php @@ -76,6 +76,12 @@ class StoreManifest { * @param array $cardFields Remote field to card field map. * @param array> $builtIn The app's own items, for the no-registry case. * @param array $types Shareable configuration type ids this store surfaces. + * + * @SuppressWarnings(PHPMD.ExcessiveParameterList) This is a value object + * mirroring the manifest's `store` block one key to one parameter, so the + * count is the block's, not a design choice. Grouping them into sub-objects + * would put a translation layer between what an app declares and what the + * engine reads, which is exactly where a silently dropped key hides. */ public function __construct( public readonly string $appId, diff --git a/tests/Unit/AppHost/FederatedStoreCatalogTest.php b/tests/Unit/AppHost/FederatedStoreCatalogTest.php index cc4b7e071..2ef7961d5 100644 --- a/tests/Unit/AppHost/FederatedStoreCatalogTest.php +++ b/tests/Unit/AppHost/FederatedStoreCatalogTest.php @@ -191,6 +191,194 @@ public function testTrustedBundleInstallsThroughItsType(): void { $this->assertCount(3, $report['components']); }//end testTrustedBundleInstallsThroughItsType() + /** + * A slug resolves to the bundle the repository carries. + * + * @return void + */ + public function testResolveFetchesTheBundleAtTheConventionalPath(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([ + ['repo' => 'ConductionNL/default-gemeente', 'url' => 'https://github.com/ConductionNL/default-gemeente'], + ]); + $this->federated->expects($this->once()) + ->method('fetchBundle') + ->with('ConductionNL/default-gemeente', 'openregister.json') + ->willReturn(['type' => 'openregister.configset', 'version' => '1']); + + $slug = FederatedStoreCatalog::slugFor( + typeId: 'openregister.configset', + repo: 'ConductionNL/default-gemeente' + ); + $ref = $this->catalog->resolve(descriptor: $this->descriptor(), slug: $slug); + + $this->assertNotNull($ref); + $this->assertSame('openregister.configset', $ref['typeId']); + $this->assertSame('ConductionNL/default-gemeente', $ref['repo']); + $this->assertSame('openregister.configset', $ref['bundle']['type']); + }//end testResolveFetchesTheBundleAtTheConventionalPath() + + /** + * A slug matching no discovered repository resolves to nothing. + * + * @return void + */ + public function testResolveReturnsNullWhenNothingMatches(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([['repo' => 'someone/other-thing']]); + $this->federated->expects($this->never())->method('fetchBundle'); + + $this->assertNull($this->catalog->resolve(descriptor: $this->descriptor(), slug: 'nothing-by-that-name')); + }//end testResolveReturnsNullWhenNothingMatches() + + /** + * A repository carrying no bundle at any conventional path resolves to nothing. + * + * Every candidate path is tried before giving up, so a repo using the + * dotted directory is still installable. + * + * @return void + */ + public function testResolveTriesEveryConventionalPath(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([['repo' => 'a/b']]); + $this->federated->expects($this->exactly(count(FederatedStoreCatalog::BUNDLE_PATHS))) + ->method('fetchBundle') + ->willThrowException(new \RuntimeException('404')); + + $slug = FederatedStoreCatalog::slugFor(typeId: 'openregister.configset', repo: 'a/b'); + + $this->assertNull($this->catalog->resolve(descriptor: $this->descriptor(), slug: $slug)); + }//end testResolveTriesEveryConventionalPath() + + /** + * The free-text filter reads the title, description and publisher. + * + * @return void + */ + public function testSearchFiltersOnFreeText(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([ + ['repo' => 'ConductionNL/default-gemeente', 'name' => 'default-gemeente', 'description' => 'A council.'], + ['repo' => 'ConductionNL/works-council', 'name' => 'works-council', 'description' => 'Staff advice.'], + ]); + + $hit = $this->catalog->search(descriptor: $this->descriptor(), query: 'gemeente'); + $miss = $this->catalog->search(descriptor: $this->descriptor(), query: 'nothing here'); + + $this->assertCount(1, $hit['cards']); + $this->assertSame('default-gemeente', $hit['cards'][0]['title']); + $this->assertSame([], $miss['cards']); + }//end testSearchFiltersOnFreeText() + + /** + * The kind chip filters by type id, and a non-matching type is not searched. + * + * @return void + */ + public function testSearchFiltersByType(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->expects($this->never())->method('discover'); + + $result = $this->catalog->search(descriptor: $this->descriptor(), query: null, kind: 'openregister.flows'); + + $this->assertSame([], $result['cards']); + }//end testSearchFiltersByType() + + /** + * A discovery failure leaves the store empty rather than raising. + * + * @return void + */ + public function testDiscoveryFailureIsContained(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willThrowException(new \RuntimeException('github is down')); + + $result = $this->catalog->search(descriptor: $this->descriptor()); + + $this->assertSame('ok', $result['outcome']); + $this->assertSame([], $result['cards']); + }//end testDiscoveryFailureIsContained() + + /** + * An install whose type raises is reported, not thrown. + * + * @return void + */ + public function testInstallFailureIsReported(): void { + $this->federated->method('isSourceAllowed')->willReturn(true); + $this->federated->method('install')->willThrowException(new \RuntimeException('bad bundle')); + + $report = $this->catalog->install( + ref: [ + 'typeId' => 'openregister.configset', + 'repo' => 'c/d', + 'source' => 'https://github.com/c/d', + 'bundle' => [], + ] + ); + + $this->assertFalse($report['success']); + $this->assertSame('error', $report['components'][0]['status']); + }//end testInstallFailureIsReported() + + /** + * A type reporting nothing still names itself as the installed component. + * + * @return void + */ + public function testInstallWithNoReportedComponentsNamesTheType(): void { + $this->federated->method('isSourceAllowed')->willReturn(true); + $this->federated->method('install')->willReturn(['installed' => []]); + + $report = $this->catalog->install( + ref: [ + 'typeId' => 'openregister.flows', + 'repo' => 'c/d', + 'source' => 'https://github.com/c/d', + 'bundle' => [], + ] + ); + + $this->assertTrue($report['success']); + $this->assertSame('openregister.flows', $report['components'][0]['schema']); + }//end testInstallWithNoReportedComponentsNamesTheType() + + /** + * A type reporting descriptors rather than names is read too. + * + * @return void + */ + public function testInstallReadsDescriptorShapedComponents(): void { + $this->federated->method('isSourceAllowed')->willReturn(true); + $this->federated->method('install')->willReturn( + ['installed' => [['type' => 'registers'], ['type' => 'flows']]] + ); + + $report = $this->catalog->install( + ref: [ + 'typeId' => 'openregister.configset', + 'repo' => 'c/d', + 'source' => 'https://github.com/c/d', + 'bundle' => [], + ] + ); + + $this->assertSame(['registers', 'flows'], array_column($report['components'], 'schema')); + }//end testInstallReadsDescriptorShapedComponents() + /** * The card slug survives a round trip through the URL pattern. * From 1d03a0f6604f32a3225f73c62115a4522284a506 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 3 Sep 2026 21:59:59 +0200 Subject: [PATCH 4/5] test(store): cover the controller's federated branches The coverage ratchet was right that the branch choosing a catalogue had no test on the federated side at all. Five tests: search routes to the catalogue and never to the objects client, the declared type ids become the kind filters when the app names none, install routes through the catalogue rather than the object installer, an unresolved slug is a 404 rather than a blank install, and a catalogue failure reports unreachable instead of raising. --- .../AppHost/GenericStoreControllerTest.php | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/Unit/AppHost/GenericStoreControllerTest.php b/tests/Unit/AppHost/GenericStoreControllerTest.php index 5491ba836..b30b70c4e 100644 --- a/tests/Unit/AppHost/GenericStoreControllerTest.php +++ b/tests/Unit/AppHost/GenericStoreControllerTest.php @@ -154,6 +154,112 @@ private function enabledStore(): StoreManifest { ); }//end enabledStore() + /** + * A store manifest declaring shareable configuration types. + * + * @return StoreManifest + */ + private function federatedStore(): StoreManifest { + return StoreManifest::fromManifest( + appId: 'decidiq', + manifest: ['store' => ['types' => ['openregister.configset', 'openregister.flows']]] + ); + }//end federatedStore() + + /** + * An app declaring types is searched against the configuration catalogue. + * + * @return void + */ + public function testSearchUsesTheCatalogueWhenTypesAreDeclared(): void { + $this->signIn(); + $this->manifestLoader->method('loadStore')->willReturn($this->federatedStore()); + $this->storeService->expects($this->never())->method('search'); + $this->catalog->expects($this->once()) + ->method('search') + ->willReturn(['outcome' => 'ok', 'cards' => [['slug' => 'a-set']]]); + + $response = $this->controller(appId: 'decidiq')->search(); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + $this->assertSame('a-set', $response->getData()['cards'][0]['slug']); + }//end testSearchUsesTheCatalogueWhenTypesAreDeclared() + + /** + * With no kinds declared, a federated store offers its type ids as filters. + * + * Falling through to the shared kind vocabulary would offer chips that + * match nothing on the page. + * + * @return void + */ + public function testFederatedKindsFallBackToTheDeclaredTypes(): void { + $this->signIn(); + $this->manifestLoader->method('loadStore')->willReturn($this->federatedStore()); + $this->catalog->method('search')->willReturn(['outcome' => 'ok', 'cards' => []]); + + $response = $this->controller(appId: 'decidiq')->search(); + + $this->assertSame( + ['openregister.configset', 'openregister.flows'], + $response->getData()['kinds'] + ); + }//end testFederatedKindsFallBackToTheDeclaredTypes() + + /** + * A federated install runs through the catalogue, not the object installer. + * + * @return void + */ + public function testInstallUsesTheCatalogueWhenTypesAreDeclared(): void { + $this->signIn(isAdmin: true); + $this->manifestLoader->method('loadStore')->willReturn($this->federatedStore()); + $this->installer->expects($this->never())->method('install'); + $this->catalog->method('resolve')->willReturn( + ['typeId' => 'openregister.configset', 'repo' => 'a/b', 'source' => 's', 'bundle' => []] + ); + $this->catalog->expects($this->once()) + ->method('install') + ->willReturn(['success' => true, 'components' => []]); + + $response = $this->controller(appId: 'decidiq')->install(slug: 'a-set'); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + $this->assertTrue($response->getData()['success']); + }//end testInstallUsesTheCatalogueWhenTypesAreDeclared() + + /** + * A federated slug that resolves to nothing is a 404, not a blank install. + * + * @return void + */ + public function testFederatedInstallOfAnUnresolvedSlugIs404(): void { + $this->signIn(isAdmin: true); + $this->manifestLoader->method('loadStore')->willReturn($this->federatedStore()); + $this->catalog->method('resolve')->willReturn(null); + $this->catalog->expects($this->never())->method('install'); + + $response = $this->controller(appId: 'decidiq')->install(slug: 'a-set'); + + $this->assertSame(Http::STATUS_NOT_FOUND, $response->getStatus()); + }//end testFederatedInstallOfAnUnresolvedSlugIs404() + + /** + * A catalogue failure reports unreachable rather than raising. + * + * @return void + */ + public function testFederatedSearchFailureIsContained(): void { + $this->signIn(); + $this->manifestLoader->method('loadStore')->willReturn($this->federatedStore()); + $this->catalog->method('search')->willThrowException(new \RuntimeException('down')); + + $response = $this->controller(appId: 'decidiq')->search(); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + $this->assertSame('store_unreachable', $response->getData()['outcome']); + }//end testFederatedSearchFailureIsContained() + /** * An anonymous caller gets an explicit 401, not a login redirect. * From d990457a99a4ff9c3f4844bd8c8f4737d218a461 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 3 Sep 2026 22:20:40 +0200 Subject: [PATCH 5/5] test(store): cover the catalogue's remaining branches Seven more, aimed at what the ratchet says is still uncovered rather than at what reads well: resolve walking past a type nothing owns to one that does, a contained discovery failure, an entry naming no repository, the second conventional bundle path answering when the first does not, an empty bundle treated as no answer, and both shapes a type can report components in that are neither a name nor a descriptor. No coverage driver is installed locally, so these were chosen by reading the branches rather than measured. CI is the measurement. --- .../AppHost/FederatedStoreCatalogTest.php | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/tests/Unit/AppHost/FederatedStoreCatalogTest.php b/tests/Unit/AppHost/FederatedStoreCatalogTest.php index 2ef7961d5..e5dad3dc8 100644 --- a/tests/Unit/AppHost/FederatedStoreCatalogTest.php +++ b/tests/Unit/AppHost/FederatedStoreCatalogTest.php @@ -379,6 +379,151 @@ public function testInstallReadsDescriptorShapedComponents(): void { $this->assertSame(['registers', 'flows'], array_column($report['components'], 'schema')); }//end testInstallReadsDescriptorShapedComponents() + /** + * Resolve walks past a declared type nothing owns and keeps looking. + * + * A stale id in a manifest must not hide a card that a later declared + * type does own. + * + * @return void + */ + public function testResolveSkipsAnUnownedTypeAndKeepsLooking(): void { + $this->registry->method('get')->willReturnCallback( + fn (string $id) => $id === 'openregister.flows' + ? $this->type(id: 'openregister.flows', topic: 'openregister-flow', name: 'Flows') + : null + ); + $this->federated->method('discover')->willReturn([['repo' => 'a/b', 'url' => 'u']]); + $this->federated->method('fetchBundle')->willReturn(['type' => 'openregister.flows']); + + $slug = FederatedStoreCatalog::slugFor(typeId: 'openregister.flows', repo: 'a/b'); + $ref = $this->catalog->resolve( + descriptor: $this->descriptor(types: ['nobody.owns-this', 'openregister.flows']), + slug: $slug + ); + + $this->assertNotNull($ref); + $this->assertSame('openregister.flows', $ref['typeId']); + }//end testResolveSkipsAnUnownedTypeAndKeepsLooking() + + /** + * A discovery failure during resolve is contained, not raised. + * + * @return void + */ + public function testResolveContainsADiscoveryFailure(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willThrowException(new \RuntimeException('github is down')); + + $this->assertNull($this->catalog->resolve(descriptor: $this->descriptor(), slug: 'anything')); + }//end testResolveContainsADiscoveryFailure() + + /** + * A discovered entry naming no repository is skipped. + * + * @return void + */ + public function testResolveSkipsAnEntryWithNoRepo(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([['name' => 'no repo here']]); + $this->federated->expects($this->never())->method('fetchBundle'); + + $this->assertNull($this->catalog->resolve(descriptor: $this->descriptor(), slug: 'anything')); + }//end testResolveSkipsAnEntryWithNoRepo() + + /** + * The second conventional path answers when the first does not. + * + * A repo using the dotted directory is still installable. + * + * @return void + */ + public function testFetchFallsThroughToTheSecondPath(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([['repo' => 'a/b', 'url' => 'u']]); + $this->federated->method('fetchBundle')->willReturnCallback( + function (string $repo, string $path) { + if ($path === FederatedStoreCatalog::BUNDLE_PATHS[0]) { + throw new \RuntimeException('404'); + } + + return ['type' => 'openregister.configset', 'from' => $path]; + } + ); + + $slug = FederatedStoreCatalog::slugFor(typeId: 'openregister.configset', repo: 'a/b'); + $ref = $this->catalog->resolve(descriptor: $this->descriptor(), slug: $slug); + + $this->assertNotNull($ref); + $this->assertSame(FederatedStoreCatalog::BUNDLE_PATHS[1], $ref['bundle']['from']); + }//end testFetchFallsThroughToTheSecondPath() + + /** + * A path answering with an empty bundle is treated as no answer. + * + * @return void + */ + public function testAnEmptyBundleIsNotAnAnswer(): void { + $this->registry->method('get')->willReturn( + $this->type(id: 'openregister.configset', topic: 'openregister-configset', name: 'Configuration set') + ); + $this->federated->method('discover')->willReturn([['repo' => 'a/b', 'url' => 'u']]); + $this->federated->method('fetchBundle')->willReturn([]); + + $slug = FederatedStoreCatalog::slugFor(typeId: 'openregister.configset', repo: 'a/b'); + + $this->assertNull($this->catalog->resolve(descriptor: $this->descriptor(), slug: $slug)); + }//end testAnEmptyBundleIsNotAnAnswer() + + /** + * A type reporting a non-list install result still reports success. + * + * @return void + */ + public function testInstallResultWithoutAListIsStillReported(): void { + $this->federated->method('isSourceAllowed')->willReturn(true); + $this->federated->method('install')->willReturn(['installed' => 'not-a-list']); + + $report = $this->catalog->install( + ref: [ + 'typeId' => 'openregister.configset', + 'repo' => 'c/d', + 'source' => 'https://github.com/c/d', + 'bundle' => [], + ] + ); + + $this->assertTrue($report['success']); + $this->assertSame('openregister.configset', $report['components'][0]['schema']); + }//end testInstallResultWithoutAListIsStillReported() + + /** + * A component entry that is neither a name nor a descriptor falls back. + * + * @return void + */ + public function testAnUnreadableComponentEntryFallsBackToTheType(): void { + $this->federated->method('isSourceAllowed')->willReturn(true); + $this->federated->method('install')->willReturn(['installed' => [42]]); + + $report = $this->catalog->install( + ref: [ + 'typeId' => 'openregister.flows', + 'repo' => 'c/d', + 'source' => 'https://github.com/c/d', + 'bundle' => [], + ] + ); + + $this->assertSame('openregister.flows', $report['components'][0]['schema']); + }//end testAnUnreadableComponentEntryFallsBackToTheType() + /** * The card slug survives a round trip through the URL pattern. *