From 3c8ef31938693c4720c77f2a0d8c8ed47ee77373 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Sun, 24 May 2026 20:50:29 +0200 Subject: [PATCH] retrofit: draft gebruik-services spec + annotate 3 methods Reverse-spec for GebruikService (read-side facade for gebruik records). New 'gebruik-services' capability with 2 REQs: paginated gebruik retrieval + application UUID extraction. Sync-side stays under method-decomposition#REQ-DECOMP-011. Notes flag getApplicationIds() malformed else-if / unconditional getObject() call for follow-up. Refs #285 --- lib/Service/GebruikService.php | 6 ++ .../design.md | 26 +++++++++ .../proposal.md | 16 ++++++ .../specs/gebruik-services/spec.md | 56 +++++++++++++++++++ .../tasks.md | 4 ++ 5 files changed, 108 insertions(+) create mode 100644 openspec/changes/retrofit-2026-05-24-gebruik-services/design.md create mode 100644 openspec/changes/retrofit-2026-05-24-gebruik-services/proposal.md create mode 100644 openspec/changes/retrofit-2026-05-24-gebruik-services/specs/gebruik-services/spec.md create mode 100644 openspec/changes/retrofit-2026-05-24-gebruik-services/tasks.md diff --git a/lib/Service/GebruikService.php b/lib/Service/GebruikService.php index edbad2f6..e1534def 100644 --- a/lib/Service/GebruikService.php +++ b/lib/Service/GebruikService.php @@ -48,6 +48,8 @@ public function __construct( * @return array The resulting configuration parameters. * * @throws Exception When configuration cannot be retrieved. + * + * @spec openspec/changes/retrofit-2026-05-24-gebruik-services/tasks.md#task-1 */ private function getGebruiksConfiguration(): array { @@ -125,6 +127,8 @@ private function getObjectService(): ObjectService * @return array The result set of gebruiken. * * @throws \OCP\DB\Exception When database query fails. + * + * @spec openspec/changes/retrofit-2026-05-24-gebruik-services/tasks.md#task-1 */ public function getGebruiken(array $options): array { @@ -174,6 +178,8 @@ function ($object) { * @return array The resulting ids. * * @throws \OCP\DB\Exception When database query fails. + * + * @spec openspec/changes/retrofit-2026-05-24-gebruik-services/tasks.md#task-2 */ public function getApplicationIds(array $options): array { diff --git a/openspec/changes/retrofit-2026-05-24-gebruik-services/design.md b/openspec/changes/retrofit-2026-05-24-gebruik-services/design.md new file mode 100644 index 00000000..2b46cae9 --- /dev/null +++ b/openspec/changes/retrofit-2026-05-24-gebruik-services/design.md @@ -0,0 +1,26 @@ +# Design — Retrofit Gebruik Services + +Retrofit change. Tasks describe retroactive annotation, not new implementation work. + +## Context + +`GebruikService` is the read-side facade for gebruik (usage) records — distinct from the sync-side `GebruikSyncService`, which is already in Bucket 1 (`method-decomposition#REQ-DECOMP-011`). The companion `getGebruiksConfiguration()` and `getObjectService()` are private plumbing folded into the public REQs. + +## Decisions + +- **Cluster mode** — no existing capability covers read-side gebruik behaviour. +- **2 REQs / 3 methods** — `getGebruiksConfiguration` is plumbing for both public methods; folded into the REQ descriptions and not annotated separately because it is private. +- **Bug flagged in Notes for `getApplicationIds()`.** The implementation has a malformed if/else-if/unconditional-call sequence that always invokes `getObject()` even when `jsonSerialize` was used or when `getId` doesn't exist. The REQ describes the contract (UUID extraction with fallback order), not the bug. Follow-up issue tracked. +- **`@SuppressWarnings(PHPMD.*)` posture** — the class has no suppressions; PHPMD passes against it cleanly. + +## Out of scope + +- Fixing the `getApplicationIds()` ObjectEntity bug. +- Adding hardcoded fallback configuration (deliberately removed). +- Refactoring against OR abstractions (ADR-022) — covered by `softwarecatalog-adopt-or-abstractions` change. + +## References + +- Umbrella: ConductionNL/softwarecatalog#285 +- Coverage report: openspec/coverage-report.md (2026-05-24) +- Source: lib/Service/GebruikService.php diff --git a/openspec/changes/retrofit-2026-05-24-gebruik-services/proposal.md b/openspec/changes/retrofit-2026-05-24-gebruik-services/proposal.md new file mode 100644 index 00000000..19c0d885 --- /dev/null +++ b/openspec/changes/retrofit-2026-05-24-gebruik-services/proposal.md @@ -0,0 +1,16 @@ +# Retrofit — gebruik-services + +Describes observed behavior of 3 public methods in `GebruikService` as 2 new REQs under a new `gebruik-services` capability. Code already exists — this change retroactively specifies it. The companion `GebruikSyncService` is already covered by `method-decomposition#REQ-DECOMP-011`. + +## Affected code units + +- lib/Service/GebruikService.php::getGebruiksConfiguration +- lib/Service/GebruikService.php::getGebruiken +- lib/Service/GebruikService.php::getApplicationIds + +## Approach + +- Describe configuration resolution via SettingsService (no hardcoded fallback) and OpenRegister search behaviour. +- Notes flag the `getApplicationIds()` early-fall-through bug (empty `else if` followed by an unconditionally executed `getObject()` call) — observed-but-suspicious. + +Source: openspec/coverage-report.md generated 2026-05-24. Umbrella: ConductionNL/softwarecatalog#285. diff --git a/openspec/changes/retrofit-2026-05-24-gebruik-services/specs/gebruik-services/spec.md b/openspec/changes/retrofit-2026-05-24-gebruik-services/specs/gebruik-services/spec.md new file mode 100644 index 00000000..6c606006 --- /dev/null +++ b/openspec/changes/retrofit-2026-05-24-gebruik-services/specs/gebruik-services/spec.md @@ -0,0 +1,56 @@ +--- +status: draft +retrofit: true +--- + +# Gebruik Services Specification + +## Purpose + +Captures observed behavior of `OCA\SoftwareCatalog\Service\GebruikService`, the read-side service that exposes `gebruik` (usage) records from the voorzieningen register via OpenRegister. The service requires the OpenRegister app to be installed and the voorzieningen register/schema configuration to be set in the admin panel — there is no hardcoded fallback. Reverse-spec'd from existing code 2026-05-24. + +## ADDED Requirements + +### REQ-001: The system SHALL return paginated gebruik records for the active register/schema configuration + +`getGebruiken(options)` MUST resolve the voorzieningen register + gebruik schema via `SettingsService::getVoorzieningenConfig()` and apply them to the search query as `@self.register` / `@self.schema`. The `extend` and `_extend` query options MUST be normalised to a string-array `_extend` (string input is split on commas + trimmed). The service MUST delegate to `ObjectService::searchObjectsPaginated` with `_rbac: false, _multitenancy: false` and return the paginated envelope. Each result MUST be stripped of the `interneAantekening` field before being returned. If configuration is missing OR if OpenRegister is not installed, the service MUST throw an `Exception` rather than return an empty result. + +#### Scenario: Configured search returns paginated envelope +- GIVEN the voorzieningen register and gebruik schema are configured +- AND OpenRegister is installed +- WHEN `getGebruiken(['limit' => 10])` is called +- THEN the call MUST delegate to `ObjectService::searchObjectsPaginated` with `query['@self'] = { register: , schema: }` +- AND the returned array MUST contain `results` (each without `interneAantekening`) and pagination metadata + +#### Scenario: Extend string is split into array +- GIVEN configuration is in place +- WHEN `getGebruiken(['extend' => 'modules,diensten'])` is called +- THEN the query forwarded to ObjectService MUST contain `_extend: ['modules', 'diensten']` +- AND the original `extend` key MUST be removed + +#### Scenario: Missing configuration throws +- GIVEN `SettingsService::getVoorzieningenConfig()` returns an array without `register` or `gebruik_schema` +- WHEN `getGebruiken([])` is called +- THEN the call MUST throw an `Exception` whose message references the missing voorzieningen configuration + +#### Scenario: OpenRegister not installed throws +- GIVEN OpenRegister is not in the installed-apps list +- WHEN `getGebruiken([])` is called +- THEN the call MUST throw an `Exception` whose message says "OpenRegister app is not installed" + +### REQ-002: The system SHALL return application UUIDs scoped to the active register + +`getApplicationIds(options)` MUST resolve the voorzieningen register + module (applicatie) schema via SettingsService and run a paginated search via `ObjectService::searchObjectsPaginated` with `_rbac: false, _multitenancy: false`. For each result it MUST extract the application's UUID — prefer `$object['@self']['id']`, fall back to `$object['id']`, fall back to `null`. The returned value MUST be the bare array of UUIDs (not the full paginated envelope). + +#### Scenario: Each application result yields a UUID +- GIVEN configuration is in place and the search returns 3 ObjectEntity instances +- WHEN `getApplicationIds([])` is called +- THEN the returned array MUST have length 3 +- AND each entry MUST be either the `@self.id` value of the corresponding object or its `id` + +## Notes + +- **Bug — `getApplicationIds()` ObjectEntity branch is broken.** The current `array_map` callback for non-array `$object` has an empty `else if (method_exists($object, 'getId') === true) { }` block (line 197-198) followed by an *unconditionally indented* `$object = $object->getObject();` call (line 200). The indentation suggests it was meant to be inside the `else if`. As written, every non-array, non-jsonSerialize-supporting object hits `getObject()` regardless of method existence — and the if/else-if structure means `jsonSerialize`-supporting objects also fall through into `getObject()`. Observed behaviour is "best-effort UUID extraction" but the implementation is buggy — REQ-002 describes the *contract* (UUID extraction in `@self.id` → `id` → null order), not the buggy implementation. See follow-up issue. +- **No hardcoded fallback by design.** `getGebruiksConfiguration()` explicitly logs + throws when the admin panel has not been configured — this is the intended behaviour after a 2025 cleanup that removed hardcoded defaults. +- **`_rbac: false, _multitenancy: false`** — gebruik records are scoped per-organisation at the data model level; the service bypasses OR's RBAC + multi-tenancy because the active org is implicit in the data structure. Documented for security review. +- **Acceptance Criteria:** Covered indirectly by `GebruikController` Newman tests; no direct unit coverage at retrofit time. diff --git a/openspec/changes/retrofit-2026-05-24-gebruik-services/tasks.md b/openspec/changes/retrofit-2026-05-24-gebruik-services/tasks.md new file mode 100644 index 00000000..f191fb4d --- /dev/null +++ b/openspec/changes/retrofit-2026-05-24-gebruik-services/tasks.md @@ -0,0 +1,4 @@ +# Tasks + +- [x] task-1: gebruik-services#REQ-001 — System SHALL return paginated gebruik records for the active register/schema configuration (retroactive annotation) +- [x] task-2: gebruik-services#REQ-002 — System SHALL return application UUIDs scoped to the active register (retroactive annotation)