fix: close the ADR-084 contract drift behind 19 PHPUnit failures and 44 static findings

[rubenvdlinde]

What this closes

softwarecatalog was the fleet's most-red app: 12 failing jobs on development (run 31964022723, sha 53809931). This PR closes PHPUnit (all 6 cells) + phpmd + phpstan + psalm. It does not touch E2E, Hydra Gates or Quality Report — those were not diagnosed and stay red.

Measured, not inferred

Reproduced locally before any edit, and re-measured after. PHPUnit ran in-container on PHP 8.4 against a real openregister (sqdev-nc), which reproduced CI's counts exactly — Tests: 705, Assertions: 2812, Errors: 7, Failures: 12, Warnings: 3, Skipped: 20, byte-identical to the CI cell. Static analysis was run through the project's own composer scripts (composer phpmd / psalm / phpstan / phpcs), never a hand-rolled invocation — phpmd.xml extends a hydra-gates ruleset and reports different findings without it.

check	before	after	how
PHPUnit (705 tests)	7 errors + 12 failures	0	in-container PHP 8.4, real openregister
phpstan	37 errors	0	composer phpstan
psalm	4 errors	0	composer psalm
phpmd	3 findings	0	composer phpmd (exit 0)
phpcs	exit 0	exit 0	composer phpcs — warnings only, pre-existing, unchanged

Root cause

All six PHPUnit cells failed with an identical count, which looks like a class-load fatal. It is not — the suite ran to completion in every cell. The shared cause is ADR-084 contract drift: this app's use of OpenRegister diverged from the contract OpenRegister now publishes (Contract\ObjectServiceInterface / Contract\ObjectEntityInterface), and both the production code and its test doubles were pinned to the older surface.

The six distinct defects

1. The saveObject() double omitted the contract's second parameter. The contract is (object, extend, register, schema, uuid, …). MergeOrganisatieServiceTest's willReturnCallback declared (object, register, schema, uuid). PHPUnit resolves the subject's named arguments against the generated mock's own signature and then invokes the callback positionally — so the capture silently recorded extend-as-register and register-as-schema. Nothing threw; every assertion looking a save up by (schema, uuid) reported "no such save". 6 failures.

This refines the board's "a PHPUnit mock cannot observe named arguments". It can — via its own signature. What it cannot do is survive a callback whose parameter list disagrees with the contract. The failure is silent and looks like the subject not saving.

2. Three controller tests wired the fixture into a ContainerInterface double while the subject holds an injected contract. The subject was left holding a different, unconfigured mock: reads came back empty, and the organisation guard refused a caller reading their own organisation. 5 failures — one of them a cross-tenant test passing straight through the check it exists to prove.

3. getObjectService() asked for the CONCRETE class and gated on instanceof ObjectService. Anything satisfying the published interface without being that exact class — i.e. every double a leaf app can build — fell to the fail-closed arm and refused an owner. Fixed in ContractApprovalService, ContractStatusService, SbomImportService.

4. Two tests referenced RegisterMapper / MetadataHydrationHandler with no import, resolving inside the test's own namespace. 4 errors.

5. QueryLimitBoundingTest seeded only container + logger by reflection on a newInstanceWithoutConstructor() instance. Reading an uninitialised typed property is an Error, not a null, so the test died before observing the query it exists to observe.

6. UserProfileUpdatedEventListener reached past the contract into SchemaMapper, RegisterMapper and Service\Object\SaveObject\MetadataHydrationHandler to regenerate _name before saving. Redundant — ObjectService::saveObject() calls hydrateObjectMetadata() on both its create and update path (SaveObject.php:3692 / :3873). This was psalm's two UndefinedClass errors and phpmd's LongVariable + unused $registerEntity. All three dependencies removed.

Production defects found on the way

• ContactpersonenController passed silent: true TWICE in one saveObject() call — a merge artefact from "restore two parameters my dangling-reference pass shadowed".
• GebruikSyncService passed id: where the contract's parameter is uuid:. The name was corrected, not dropped.
• ContactpersoonService tested findSilent(...) === null. findSilent() declares a non-nullable ObjectEntityInterface and lets DoesNotExistException out, so the distinct "not found" entry was unreachable and every miss came back carrying an error key. Now caught explicitly.
• ContactPersonHandler::findContactPersonByUsername() — private, no caller, and called findAll($filters, $registerId, $schemaId) positionally against findAll(array $config, bool $_rbac, bool $_multitenancy). The register id would have landed in $_rbac and the search run unscoped. Deleted, reasoning recorded in place.
• OrganizationHandler had one saveObject() with no register/schema at all, leaving the write to whatever scope the service happened to carry. Now falls back to the entity's own coordinates.

Named arguments: none removed

Rule 7a is gate-enforced, so explicitly: no named argument was stripped anywhere in this change. The only name that changed is id: → uuid: in GebruikSyncService, which corrects the name to the published signature. Where arguments disappear from a call it is because the parameter itself was deleted (unused ContainerInterface on two constructors; three dead parameters on one private helper). The saveObject() double was repaired by adding the contract's ?array $extend = [] — mirroring the real surface, which is the prescribed repair. Verified by diffing every removed name: line.

The stub documented the opposite of the truth

tests/Stubs/Db/ObjectEntity.php's header said getOrganisation() is magic on the real entity, so declaring it inverts method_exists(). ADR-084 changed that: the real ObjectEntity implements ObjectEntityInterface, and an interface method cannot be served by __call(), so the real class declares all six concretely (openregister lib/Db/ObjectEntity.php:833). The stub now mirrors it, keeps the backing organisation property (what Entity::getter() and readOwningOrganisation() key on), and the header says so. testTheMagicEntityDoubleMatchesTheRealObjectEntityAccessorShape was pinning the pre-ADR-084 shape; it is re-pointed at the current one and now pins both halves, so the softwarecatalog#490 data-loss path cannot come back.

Six setObject() call sites

setObject(), setOrganisation() and getId() are implementation-only accessors reached through Entity::__call() and are not on ObjectEntityInterface. Six sites pushed a payload into the entity and read it straight back out. The payload is now threaded through explicitly. saveObject() is PUT-semantic, so every unchanged field is still carried forward.

One deliberate suppression

OrganizationSyncService::__construct has 11 parameters (phpmd threshold 10). Suppressed with the reasoning in place, matching ContactpersonenController: $container is not dead weight there (it lazily resolves IUserManager/IGroupManager), so removing it takes the count to twelve, and hiding a pair behind a parameter object hides a dependency the composition root must state anyway. The real remedy is splitting a 2,700-line service — a refactor, not a quality-gate change.

Still red — not addressed

• E2E Tests (Playwright) — not diagnosed. The job log names only the failing selector; the cause is in the playwright-report artifact's data/*.md, which I did not open.
• Hydra Gates — not diagnosed. Note the gate package moved mid-afternoon (18fe6f9 → f935e2c); the base must be re-run against the same gate-package sha before its numbers mean anything.
• Quality Report — downstream aggregator; should follow once its inputs are green.

Verification note

The final full-suite PHPUnit run measured 1 remaining failure (ContractApprovalServiceTest::testAuthorizeSubmitOwningAanbodBeheerderIsAuthorized), which the getObjectService() contract fix then addressed. That fix, and the phpstan/psalm/phpmd work that followed it, were verified by the static tools but not re-verified against the full PHPUnit suite — the shared test server was taken out of service before I could re-run it. CI on this PR is the first full-suite measurement of the final tree.

[github-actions]

Quality Report — ConductionNL/softwarecatalog @ f94ddcc

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-vue-demi		✅
test-l10n		✅
format		✅
composer			✅	✅ 130/130
npm			✅	✅ 704/704
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					✅
Newman					⏭️
Playwright					❌
Hydra gates					❌

---

Quality workflow — 2026-08-16 20:48 UTC

Download the full PDF report from the workflow artifacts.