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

PHPUnit was red in all six matrix cells with an IDENTICAL count
(Tests 705, Errors 7, Failures 12, Warnings 3, Skipped 20), which looked
like a class-load fatal. It was not: the suite ran to completion in every
cell. The shared cause is that this app's consumption of OpenRegister
drifted from the contract OpenRegister now publishes
(OCA\OpenRegister\Contract\ObjectServiceInterface / ObjectEntityInterface),
and both the production code and its doubles were pinned to the older
surface.

Measured, in-container on PHP 8.4 against the same openregister:
  PHPUnit  705 tests: 19 red -> 1 red -> 0 red
  phpstan  37 errors -> 0
  psalm     4 errors -> 0
  phpmd     3 findings -> 0
  phpcs     exit 0 before and after (warnings only, pre-existing)

Six distinct defects, not one:

1. The saveObject() double omitted the contract's second parameter.
   ObjectServiceInterface::saveObject() 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 that looked a save up by (schema, uuid) reported "no such
   save". Six failures. The callback now mirrors the contract position for
   position.

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

3. getObjectService() asked the container for the CONCRETE class and gated
   on `instanceof ObjectService`. Anything that satisfies 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 and SbomImportService:
   ask for the contract, narrow on the contract.

4. Two tests referenced RegisterMapper / MetadataHydrationHandler with no
   import, so they resolved inside the test's own namespace. Four errors.
   The listener's dependency on both is gone (see 6), so the imports went
   with it.

5. QueryLimitBoundingTest seeded only `container` and `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. It now seeds
   `objectService` too.

6. UserProfileUpdatedEventListener reached past the contract into
   SchemaMapper, RegisterMapper and Service\Object\SaveObject\
   MetadataHydrationHandler to regenerate `_name` before saving. That was
   redundant - ObjectService::saveObject() calls hydrateObjectMetadata() on
   both its create and its update path - and it is what psalm reported as
   two UndefinedClass errors and phpmd as a LongVariable plus an unused
   $registerEntity. All three dependencies removed.

Production defects found on the way, each fixed at the call site:

- ContactpersonenController passed `silent: true` TWICE in one saveObject()
  call (a merge artefact); psalm InvalidNamedArgument, phpstan duplicate.
- 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 the mapper's
  DoesNotExistException out, so the distinct "not found" entry was
  unreachable and every miss came back carrying an `error` key instead. Now
  caught explicitly.
- ContactPersonHandler::findContactPersonByUsername() was private, had 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 with the 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. It now
  falls back to the entity's own coordinates.

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

Two constructors dropped an unused ContainerInterface (phpstan: "never
read, only written") - ADR-084 replaced the lazy lookup with the injected
contract. lib/AppInfo/Application.php's hand-written factories updated to
match; tests/Unit/AppInfo/CompositionRootArgumentsTest.php covers that.

tests/Stubs/Db/ObjectEntity.php's header documented the OPPOSITE of the
current truth. It said getOrganisation() is magic on the real entity, so
declaring it here 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. The stub mirrors it, keeps the backing `organisation` property
(that is what Entity::getter() and readOwningOrganisation() key on), and
the header now says so. testTheMagicEntityDoubleMatchesTheRealObjectEntity
AccessorShape was asserting the pre-ADR-084 shape and is re-pointed at the
current one, pinning BOTH halves so the softwarecatalog#490 data-loss path
cannot come back.

No named argument was removed anywhere in this change. `id:` -> `uuid:` is
a NAME correction to match the published signature.

E2E Tests, Hydra Gates and Quality Report are NOT addressed here and remain
red; they were not diagnosed.

Author: rubenvdlinde

lib/AppInfo/Application.php

@@ -377,7 +377,6 @@ function ($container) {
 				return new GebruikSyncService(
 					logger: $container->get('Psr\Log\LoggerInterface'),
 					settingsService: $container->get(SettingsService::class),
-					container: $container,
 					objectService: $container->get(ObjectServiceInterface::class),
 				);
 			}
@@ -738,7 +737,6 @@ function ($container) {
 					userManager: $container->get('OCP\IUserManager'),
 					groupManager: $container->get('OCP\IGroupManager'),
 					userSession: $container->get('OCP\IUserSession'),
-					container: $container,
 					secureRandom: $container->get('OCP\Security\ISecureRandom'),
 					logger: $container->get('Psr\Log\LoggerInterface'),
 					// ADR-084 added these two to the constructor; this hand-written

lib/Controller/ContactpersonenController.php

@@ -33,7 +33,6 @@
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Security\ISecureRandom;
-use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
 use OCA\OpenRegister\Contract\ObjectServiceInterface;
 use OCA\OpenRegister\Service\OrganisationService;
@@ -111,13 +110,6 @@ class ContactpersonenController extends Controller {
 	 */
 	private IUserSession $userSession;
 
-	/**
-	 * Container for dependency injection.
-	 *
-	 * @var ContainerInterface
-	 */
-	private ContainerInterface $container;
-
 	/**
 	 * Contactpersoon service for business logic.
 	 *
@@ -136,7 +128,6 @@ class ContactpersonenController extends Controller {
 	 * @param IUserManager $userManager User manager
 	 * @param IGroupManager $groupManager Group manager
 	 * @param IUserSession $userSession User session
-	 * @param ContainerInterface $container Container for DI
 	 * @param ISecureRandom $secureRandom Secure random generator
 	 * @param LoggerInterface $logger Logger instance
 	 * @param ObjectServiceInterface $objectService OpenRegister object access (ADR-022/ADR-084 —
@@ -154,7 +145,6 @@ public function __construct(
 		IUserManager $userManager,
 		IGroupManager $groupManager,
 		IUserSession $userSession,
-		ContainerInterface $container,
 		ISecureRandom $secureRandom,
 		LoggerInterface $logger,
 		private readonly ObjectServiceInterface $objectService,
@@ -167,7 +157,6 @@ public function __construct(
 		$this->userManager = $userManager;
 		$this->groupManager = $groupManager;
 		$this->userSession = $userSession;
-		$this->container = $container;
 		$this->secureRandom = $secureRandom;
 		$this->logger = $logger;
 	}//end __construct()
@@ -515,7 +504,12 @@ public function convertToUser(string $contactPersonId): JSONResponse {
 
 			$contactData = $this->normaliseContactDataForPersist(contactData: $contactData);
 
-			$contactPersonObject->setObject($contactData);
+			// No local `setObject()` on the entity here. `setObject()` is not on
+			// the published ObjectEntityInterface (ADR-084) — it is an
+			// implementation-only accessor reached through Entity::__call() — and
+			// mutating the in-memory copy changed nothing anyway: the save below
+			// sends `$contactData` itself, and the entity is only read again for
+			// its UUID.
 
 			// Debug logging to understand data types before save.
 			$lastNameValue = $contactData['achternaam'] ?? 'not set';
@@ -550,8 +544,7 @@ public function convertToUser(string $contactPersonId): JSONResponse {
 				schema: $schemaId,
 				uuid: $contactPersonObject->getUuid(),
 				silent: true,
-				silent: true,
-			_validation: false
+				_validation: false
 			);
 
 			$this->logger->info(

lib/EventListener/UserProfileUpdatedEventListener.php

@@ -26,9 +26,6 @@
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
 use OCA\OpenRegister\Contract\ObjectServiceInterface;
-use OCA\OpenRegister\Db\SchemaMapper;
-use OCA\OpenRegister\Db\RegisterMapper;
-use OCA\OpenRegister\Service\Object\SaveObject\MetadataHydrationHandler;
 
 /**
  * Syncs user profile changes to the corresponding contactpersoon object.
@@ -58,16 +55,10 @@ class UserProfileUpdatedEventListener implements IEventListener {
 	 *
 	 * @param ContainerInterface $container DI container for lazy service resolution.
 	 * @param ObjectServiceInterface $objectService OpenRegister object access (ADR-084 contract).
-	 * @param SchemaMapper $schemaMapper Resolves the contactpersoon schema.
-	 * @param RegisterMapper $registerMapper Resolves the register the schema lives in.
-	 * @param MetadataHydrationHandler $metadataHydrationHandler Hydrates `@self` metadata on read.
 	 */
 	public function __construct(
 		private readonly ContainerInterface $container,
 		private readonly ObjectServiceInterface $objectService,
-		private readonly SchemaMapper $schemaMapper,
-		private readonly RegisterMapper $registerMapper,
-		private readonly MetadataHydrationHandler $metadataHydrationHandler,
 	) {
 	}//end __construct()
 
@@ -207,16 +198,16 @@ private function syncToContactPerson(UserProfileUpdatedEvent $event, LoggerInter
 			]
 		);
 
-		// Merge the patch into existing data and save directly via mapper to skip schema validation.
-		// Schema validation can reject existing data with legacy values (e.g. notificaties enum).
+		// Merge the patch into the existing payload. `saveObject()` is
+		// PUT-semantic, so the FULL payload must be carried forward, not only
+		// the changed keys. Validation is skipped on the save because schema
+		// validation can reject pre-existing legacy values (e.g. notificaties
+		// enum) that this listener never touched.
 		$mergedObject = array_merge($contactData, $patch);
-		$contactPerson->setObject($mergedObject);
 
 		$this->persistContactPersonPatch(
 			contactPerson: $contactPerson,
-			register:       (int)$register,
-			schema:         (int)$contactPersonSchema,
-			logger:         $logger
+			object:        $mergedObject
 		);
 
 		$logger->info(
@@ -278,63 +269,39 @@ private function buildContactPatch(
 	}//end buildContactPatch()
 
 	/**
-	 * Persist the patched contactpersoon entity, regenerating `_name`
-	 * metadata first when the schema is loadable.
+	 * Persist the patched contactpersoon payload through the published
+	 * OpenRegister contract.
 	 *
-	 * @param object $contactPerson The contactpersoon entity.
-	 * @param int $register The voorzieningen register id.
-	 * @param int $schema The contactpersoon schema id.
-	 * @param LoggerInterface $logger Logger for hydration warnings.
+	 * `_name` metadata is NOT regenerated here. This method used to load the
+	 * schema through `SchemaMapper` and call
+	 * `MetadataHydrationHandler::hydrateObjectMetadata()` itself — both of them
+	 * OpenRegister internals that ADR-084's published contract deliberately does
+	 * not expose, and neither of which a leaf app can load in its own unit
+	 * tests. It was also redundant: `ObjectService::saveObject()` calls
+	 * `hydrateObjectMetadata()` on both its create and its update path before
+	 * handing the entity to `objectEntityMapper`, so the hydration happens
+	 * exactly once either way — it was simply being done twice, one layer too
+	 * deep.
+	 *
+	 * @param object $contactPerson The contactpersoon entity (read for its coordinates).
+	 * @param array $object The full merged payload to store (PUT-semantic).
 	 *
 	 * @return void
 	 */
 	private function persistContactPersonPatch(
 		object $contactPerson,
-		int $register,
-		int $schema,
-		LoggerInterface $logger,
+		array $object,
 	): void {
-		$schemaEntity = null;
-		$registerEntity = null;
-		try {
-			$schemaEntity = $this->schemaMapper->find(id: $schema, _rbac: false, _multitenancy: false);
-			$registerEntity = $this->registerMapper->find(id: $register, _rbac: false, _multitenancy: false);
-		} catch (\Exception $e) {
-			$logger->warning(
-				'[UserProfileUpdatedEventListener] Could not load schema/register entities for _name hydration',
-				[
-					'error' => $e->getMessage(),
-				]
-			);
-		}
-
-		if ($schemaEntity !== null) {
-			$this->metadataHydrationHandler->hydrateObjectMetadata(entity: $contactPerson, schema: $schemaEntity);
-			$logger->debug(
-				'[UserProfileUpdatedEventListener] Regenerated _name metadata',
-				[
-					'newName' => $contactPerson->getName(),
-				]
-			);
-		}
-
-		// Persist through the published contract rather than OpenRegister's Db
-		// layer. The comment this replaces worried that a plain save would touch
-		// "just the blob table" — it does not: ObjectService::saveObject() calls
-		// metaHydrationHandler->hydrateObjectMetadata() and then
-		// objectEntityMapper->update(entity:, register:, schema:), which IS the
-		// magic-mapper route. This listener was hand-rolling OpenRegister's own
-		// save pipeline, one layer too deep.
 		$this->objectService->saveObject(
-			object: $contactPerson->getObject(),
+			object: $object,
 			register: $contactPerson->getRegister(),
 			schema: $contactPerson->getSchema(),
 			uuid: $contactPerson->getUuid(),
 			silent: true,
 			_validation: false
 		);
 
-	}//end persistContactpersoonPatch()
+	}//end persistContactPersonPatch()
 
 	/**
 	 * Find a contactpersoon by username, falling back to a case-insensitive email search.

lib/Service/AanbodService.php

@@ -374,9 +374,12 @@ public function acceptAanbod(string $aanbodId, array $options = []): array {
 			);
 
 			// Save the updated object with RBAC and multitenancy disabled.
-			$existingAanbod->setObject($aanbodData);
+			// The PAYLOAD is what is sent — `setObject()` on the entity is not on
+			// the published ObjectEntityInterface (ADR-084) and the mutated copy
+			// was never read back anyway; `saveObject()` is PUT-semantic, so
+			// $aanbodData already carries every unchanged field forward.
 			$updatedAanbod = $objectService->saveObject(
-				object: $existingAanbod,
+				object: $aanbodData,
 				register: $existingAanbod->getRegister(),
 				schema: $existingAanbod->getSchema(),
 				uuid: $aanbodId,
@@ -593,12 +596,12 @@ private function resolvePartyId(mixed $partyInfo): ?string {
 	 * @param ObjectServiceInterface $objectService The OpenRegister object service
 	 * @param string $aanbodId The UUID of the aanbod object
 	 *
-	 * @return \OCA\OpenRegister\Db\ObjectEntity|null The found object or null
+	 * @return \OCA\OpenRegister\Contract\ObjectEntityInterface|null The found object or null
 	 */
 	private function findAanbodObject(
 		ObjectServiceInterface $objectService,
 		string $aanbodId,
-	): ?\OCA\OpenRegister\Db\ObjectEntity {
+	): ?\OCA\OpenRegister\Contract\ObjectEntityInterface {
 		$voorzieningenConfig = $this->settingsService->getVoorzieningenConfig();
 		$registerId = $voorzieningenConfig['register'] ?? null;
 

lib/Service/AangebodenGebruikService.php

@@ -992,9 +992,11 @@ public function setGebruikSelfToActiveOrg(string $gebruikId, array $options = []
 
 			// Save the updated object with RBAC and multitenancy disabled.
 			// Use register/schema from the found entity for correct table routing.
-			$existingGebruik->setObject($gebruikData);
+			// The PAYLOAD is what is sent — `setObject()` is not on the published
+			// ObjectEntityInterface (ADR-084), and `saveObject()` is PUT-semantic,
+			// so $gebruikData already carries every unchanged field forward.
 			$updatedGebruik = $objectService->saveObject(
-				object: $existingGebruik,
+				object: $gebruikData,
 				register: $existingGebruik->getRegister(),
 				schema: $existingGebruik->getSchema(),
 				uuid: $gebruikId,
@@ -1049,12 +1051,12 @@ public function setGebruikSelfToActiveOrg(string $gebruikId, array $options = []
 	 * @param ObjectServiceInterface $objectService The OpenRegister object service
 	 * @param string $objectId The UUID of the object to find
 	 *
-	 * @return \OCA\OpenRegister\Db\ObjectEntity|null The found object or null
+	 * @return \OCA\OpenRegister\Contract\ObjectEntityInterface|null The found object or null
 	 */
 	private function findGebruikOrIntegration(
 		ObjectServiceInterface $objectService,
 		string $objectId,
-	): ?\OCA\OpenRegister\Db\ObjectEntity {
+	): ?\OCA\OpenRegister\Contract\ObjectEntityInterface {
 		$voorzieningenConfig = $this->settingsService->getVoorzieningenConfig();
 		$registerId = $voorzieningenConfig['register'] ?? null;
 

lib/Service/ContactpersoonService.php

@@ -25,6 +25,7 @@
 use OCA\SoftwareCatalog\Service\SoftwareCatalogue\GroupHandler;
 use OCA\SoftwareCatalog\Service\SoftwareCatalogue\HierarchyHandler;
 use OCP\App\IAppManager;
+use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IAppConfig;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
@@ -1026,15 +1027,24 @@ public function getBulkUserInfo(array $contactPersonIds): array {
 					}
 
 					// Find the contactpersoon object with register and schema specified.
-					$contactObject = $objectService->findSilent(
-						id: $contactPersonId,
-						_extend: [],
-						files: false,
-						register: $contactRegister,
-						schema: $contactSchema
-					);
-
-					if ($contactObject === null) {
+					//
+					// A MISS RAISES, it does not return null: `findSilent()` is
+					// declared `ObjectEntityInterface` (non-nullable) on the
+					// published contract and OpenRegister lets the mapper's
+					// DoesNotExistException out. The `=== null` test this replaces
+					// could therefore never be true — the distinct "not found"
+					// entry below was unreachable, and every missing contactpersoon
+					// fell through to the generic error arm and came back carrying
+					// an `error` key instead.
+					try {
+						$contactObject = $objectService->findSilent(
+							id: $contactPersonId,
+							_extend: [],
+							files: false,
+							register: $contactRegister,
+							schema: $contactSchema
+						);
+					} catch (DoesNotExistException $e) {
 						$this->logger->warning(
 							'ContactpersoonService: Contactpersoon not found for bulk user info',
 							['contactpersoonId' => $contactPersonId]
@@ -1045,7 +1055,7 @@ public function getBulkUserInfo(array $contactPersonIds): array {
 							'groups' => [],
 						];
 						continue;
-					}
+					}//end try
 
 					$contactData = $contactObject->getObject();
 					$username = $contactData['username'] ?? null;

lib/Service/ContractApprovalService.php

@@ -38,7 +38,6 @@
 namespace OCA\SoftwareCatalog\Service;
 
 use OCA\OpenRegister\Contract\ObjectServiceInterface;
-use OCA\OpenRegister\Service\ObjectService;
 use OCP\EventDispatcher\IEventDispatcher;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
@@ -501,7 +500,7 @@ private function buildSubjectLabel(array $data): string {
 	 *
 	 * @param string $contractUuid The contract uuid.
 	 *
-	 * @return \OCA\OpenRegister\Db\ObjectEntity|null The object, or null.
+	 * @return \OCA\OpenRegister\Contract\ObjectEntityInterface|null The object, or null.
 	 *
 	 * @spec openspec/specs/contract-decision-delegation/spec.md
 	 */
@@ -540,7 +539,7 @@ private function loadContract(string $contractUuid) {
 	/**
 	 * Persist a mutated contract object back to the OR store.
 	 *
-	 * @param \OCA\OpenRegister\Db\ObjectEntity $contract The contract entity.
+	 * @param \OCA\OpenRegister\Contract\ObjectEntityInterface $contract The contract entity.
 	 * @param array $data The mutated object data.
 	 *
 	 * @return void
@@ -572,8 +571,14 @@ private function persistContract($contract, array $data): void {
 	 */
 	private function getObjectService(): ?ObjectServiceInterface {
 		try {
-			$service = $this->container->get('OCA\OpenRegister\Service\ObjectService');
-			if ($service instanceof ObjectService) {
+			// Ask for the CONTRACT and narrow on the CONTRACT (ADR-084). Asking
+			// for the concrete class and gating on `instanceof ObjectService`
+			// made this method return null for anything that satisfies the
+			// published interface without being that exact class — which is
+			// every double a leaf app can build, so the ownership check silently
+			// fell through to its fail-closed arm and refused an owner.
+			$service = $this->container->get(ObjectServiceInterface::class);
+			if ($service instanceof ObjectServiceInterface) {
 				return $service;
 			}
 		} catch (\Throwable $e) {

lib/Service/ContractStatusService.php

@@ -28,7 +28,6 @@
 
 use DateTimeImmutable;
 use OCA\OpenRegister\Contract\ObjectServiceInterface;
-use OCA\OpenRegister\Service\ObjectService;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
 
@@ -189,8 +188,11 @@ public function expirePastContracts(?DateTimeImmutable $now = null): int {
 	 */
 	private function getObjectService(): ?ObjectServiceInterface {
 		try {
-			$service = $this->container->get('OCA\OpenRegister\Service\ObjectService');
-			if ($service instanceof ObjectService) {
+			// Ask for the CONTRACT and narrow on the CONTRACT (ADR-084) — see
+			// ContractApprovalService::getObjectService() for why gating on the
+			// concrete class is a silent fail-closed.
+			$service = $this->container->get(ObjectServiceInterface::class);
+			if ($service instanceof ObjectServiceInterface) {
 				return $service;
 			}
 		} catch (\Throwable $e) {