Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/Service/GebruikService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down
26 changes: 26 additions & 0 deletions openspec/changes/retrofit-2026-05-24-gebruik-services/design.md
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions openspec/changes/retrofit-2026-05-24-gebruik-services/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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: <id>, schema: <gebruik 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.
Original file line number Diff line number Diff line change
@@ -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)
Loading