From 103a102e2677ec077d6936952181e4f7ba2e650f Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Sun, 16 Aug 2026 11:44:09 +0200 Subject: [PATCH 1/2] fix(tests): the ObjectEntity stub declared return types wider than the contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every PHPUnit cell and all four PHP quality tools were failing on development with one fatal, thrown at class load before any test ran: PHP Fatal error: Declaration of OCA\OpenRegister\Db\ObjectEntity::getUuid() must be compatible with OCA\OpenRegister\Contract\ObjectEntityInterface::getUuid(): ?string The stub implements ObjectEntityInterface but declared getUuid, getObject, getRegister and getSchema with no return type at all. A return type may be narrowed by an implementor, never widened, and an omitted type is the widest there is — so `abstract public function getUuid();` against the contract's `?string` is not loose, it is invalid. The four abstracts now carry the contract's own types, and the anonymous subclass in MergeOrganisatieServiceTest follows them; it was the only subclass in the repo. 13 of 37 jobs were red on this one declaration. --- tests/Stubs/Db/ObjectEntity.php | 24 +++++++++++++------ .../Service/MergeOrganisatieServiceTest.php | 14 +++++------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/Stubs/Db/ObjectEntity.php b/tests/Stubs/Db/ObjectEntity.php index 34311c7b..f336f7d6 100644 --- a/tests/Stubs/Db/ObjectEntity.php +++ b/tests/Stubs/Db/ObjectEntity.php @@ -147,17 +147,27 @@ protected function setter(string $name, array $args): void { /** @return int */ abstract public function getId(); - /** @return string */ - abstract public function getUuid(); + // These four carry the CONTRACT's return types, not looser ones. A + // return type may be narrowed by an implementor but never widened, and + // an omitted type is the widest there is — so declaring these untyped + // against ObjectEntityInterface's `?string` / `array` is a fatal at + // class load, which is what it was: + // Declaration of ...\Db\ObjectEntity::getUuid() must be compatible + // with ...\Contract\ObjectEntityInterface::getUuid(): ?string + // That kills the whole suite before a single test runs, which is why + // all six PHPUnit cells and all four quality tools went red together. + + /** @return string|null */ + abstract public function getUuid(): ?string; /** @return array */ - abstract public function getObject(); + abstract public function getObject(): array; - /** @return mixed */ - abstract public function getRegister(); + /** @return string|null */ + abstract public function getRegister(): ?string; - /** @return mixed */ - abstract public function getSchema(); + /** @return string|null */ + abstract public function getSchema(): ?string; /** * @param array|null $object diff --git a/tests/Unit/Service/MergeOrganisatieServiceTest.php b/tests/Unit/Service/MergeOrganisatieServiceTest.php index 7cba96a5..9bcf0ed3 100644 --- a/tests/Unit/Service/MergeOrganisatieServiceTest.php +++ b/tests/Unit/Service/MergeOrganisatieServiceTest.php @@ -794,9 +794,9 @@ public function getId() { /** * The object uuid. * - * @return string + * @return string|null */ - public function getUuid() { + public function getUuid(): ?string { return (string)$this->uuid; }//end getUuid() @@ -806,25 +806,25 @@ public function getUuid() { * * @return array */ - public function getObject() { + public function getObject(): array { return array_merge(['id' => $this->uuid], ($this->object ?? [])); }//end getObject() /** * The register id — unused by these tests. * - * @return mixed + * @return string|null */ - public function getRegister() { + public function getRegister(): ?string { return null; }//end getRegister() /** * The schema id — unused by these tests. * - * @return mixed + * @return string|null */ - public function getSchema() { + public function getSchema(): ?string { return null; }//end getSchema() From 01cc731ecae55adbdbb1e246e059a0e24123f9d0 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Sun, 16 Aug 2026 12:31:11 +0200 Subject: [PATCH 2/2] fix: the errors the class-load fatal was hiding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the stub fatal gone PHPUnit actually runs (698 tests, 2644 assertions), which exposed 53 errors that could not previously be reached. Four causes: 38 — five test helpers declared `: ObjectService` (the concrete class) while returning `createMock(ObjectServiceInterface::class)`. The helpers now declare what they return. 8 — OrganisationMembersControllerTest passed $this->container in the logger slot and omitted $organisationService entirely; the mock it needed was already built two lines up, it was just being reached through the container. 2 — OrganizationSyncService guarded two batch loops with `$this->objectService instanceof ObjectService === false`, testing the CONCRETE class against a property typed as the interface. The property is promoted, readonly and non-nullable, so it can never be unresolved: in production the guard was dead, and against any other implementation it logged "could not resolve ObjectService" and returned early — silently skipping the whole organisation batch and every contact rather than syncing them. 1 — ViewService::getObjectService() declares `?ObjectServiceInterface` but resolved `ObjectService::class` out of the container. It now asks for the contract, which is what the return type promises. --- lib/Service/OrganizationSyncService.php | 19 ++++++++++--------- lib/Service/ViewService.php | 9 +++++++-- .../OrganisationMembersControllerTest.php | 4 ++-- tests/Unit/Service/FacetServiceTest.php | 2 +- tests/Unit/Service/IntakeModerationTest.php | 4 ++-- .../Service/ReviewAggregateServiceTest.php | 2 +- tests/Unit/Service/ReviewServiceTest.php | 2 +- 7 files changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/Service/OrganizationSyncService.php b/lib/Service/OrganizationSyncService.php index 87c3228d..c063c4c4 100644 --- a/lib/Service/OrganizationSyncService.php +++ b/lib/Service/OrganizationSyncService.php @@ -21,7 +21,6 @@ namespace OCA\SoftwareCatalog\Service; use OCA\OpenRegister\Contract\ObjectServiceInterface; -use OCA\OpenRegister\Service\ObjectService; use OCA\SoftwareCatalog\Service\SoftwareCatalogue\ContactPersonHandler; use OCP\IAppConfig; use OCP\IDBConnection; @@ -384,10 +383,13 @@ public function performOrganizationsSync(int $batchSize = 50, int $maxExecutionS $rows = $qb->executeQuery()->fetchAll(); - if ($this->objectService instanceof ObjectService === false) { - $this->logger->error('OrganizationSync: could not resolve ObjectService'); - return $stats; - } + // No resolution guard here any more. $objectService is a promoted, + // readonly, non-nullable ObjectServiceInterface, so it cannot be + // unresolved by the time this runs. The old check asked + // `instanceof ObjectService` — the CONCRETE class — against a property + // typed as the interface, so any implementation other than that one + // class made it return $stats and log "could not resolve", silently + // skipping the whole batch rather than syncing it. foreach ($rows as $row) { if ((time() - $startTime) >= $maxExecutionSeconds) { @@ -508,10 +510,9 @@ public function performContactSync(int $batchSize = 100, int $maxExecutionSecond $this->logger->info('ContactSync: processing ' . count($contacts) . ' contacts with existing NC accounts'); - if ($this->objectService instanceof ObjectService === false) { - $this->logger->error('ContactSync: could not resolve ObjectService'); - return $stats; - } + // Same as above: the promoted readonly ObjectServiceInterface cannot be + // unresolved, and the old `instanceof ObjectService` check against the + // concrete class made this skip every contact instead. foreach ($contacts as $contact) { if ((time() - $startTime) >= $maxExecutionSeconds) { diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index 2ce681b3..5766e624 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -22,7 +22,6 @@ namespace OCA\SoftwareCatalog\Service; use OCA\OpenRegister\Contract\ObjectServiceInterface; -use OCA\OpenRegister\Service\ObjectService; use OCP\App\IAppManager; use OCP\IAppConfig; use OCP\ICache; @@ -1407,7 +1406,13 @@ private function getObjectService(): ?ObjectServiceInterface { } try { - return $this->container->get(ObjectService::class); + // Ask for the CONTRACT, not the concrete class. This method + // declares `?ObjectServiceInterface`, and resolving the concrete + // ObjectService returns something that only satisfies it by + // coincidence — under test it is a double of the concrete class, + // which does not implement the interface, so the declared return + // type rejected it. + return $this->container->get(ObjectServiceInterface::class); } catch (\Exception $e) { $this->logger->warning( 'Failed to get ObjectService', diff --git a/tests/Unit/Controller/OrganisationMembersControllerTest.php b/tests/Unit/Controller/OrganisationMembersControllerTest.php index fca13f43..44dd3bfe 100644 --- a/tests/Unit/Controller/OrganisationMembersControllerTest.php +++ b/tests/Unit/Controller/OrganisationMembersControllerTest.php @@ -121,8 +121,8 @@ function (string $uuid): Organisation { $this->userSession, $this->groupManager, $this->userManager, - $this->container, - $this->createMock(LoggerInterface::class) + $this->createMock(LoggerInterface::class), + $this->organisationService ); }//end makeController() diff --git a/tests/Unit/Service/FacetServiceTest.php b/tests/Unit/Service/FacetServiceTest.php index c6af92fd..033725ea 100644 --- a/tests/Unit/Service/FacetServiceTest.php +++ b/tests/Unit/Service/FacetServiceTest.php @@ -182,7 +182,7 @@ function (string $id) use ($objectService, $organisationService) { * * @return ObjectServiceInterface */ - private function makePaginatedObjectService(array $results, array &$capturedRef): ObjectService { + private function makePaginatedObjectService(array $results, array &$capturedRef): ObjectServiceInterface { $objectService = $this->createMock(ObjectServiceInterface::class); $objectService->method('searchObjectsPaginated')->willReturnCallback( function (array $query) use ($results, &$capturedRef): array { diff --git a/tests/Unit/Service/IntakeModerationTest.php b/tests/Unit/Service/IntakeModerationTest.php index 88356e28..e2011ac2 100644 --- a/tests/Unit/Service/IntakeModerationTest.php +++ b/tests/Unit/Service/IntakeModerationTest.php @@ -363,7 +363,7 @@ public function testBeoordeelingNonPendingCannotBeApproved(): void { * * @return ObjectServiceInterface The mock. */ - private function objectService(array $found): ObjectService { + private function objectService(array $found): ObjectServiceInterface { $objectService = $this->createMock(ObjectServiceInterface::class); $objectService->method('searchObjects')->willReturn($found); $objectService->method('saveObject')->willReturnCallback( @@ -383,7 +383,7 @@ function (array $object) { * * @return ObjectServiceInterface The mock. */ - private function objectServiceWithFind(ObjectEntity $entity): ObjectService { + private function objectServiceWithFind(ObjectEntity $entity): ObjectServiceInterface { $objectService = $this->createMock(ObjectServiceInterface::class); $objectService->method('find')->willReturn($entity); $objectService->method('saveObject')->willReturnCallback( diff --git a/tests/Unit/Service/ReviewAggregateServiceTest.php b/tests/Unit/Service/ReviewAggregateServiceTest.php index 9a5af9f5..c2fa5789 100644 --- a/tests/Unit/Service/ReviewAggregateServiceTest.php +++ b/tests/Unit/Service/ReviewAggregateServiceTest.php @@ -147,7 +147,7 @@ public function testInvalidSubjectTypeRejected(): void { * * @return ObjectServiceInterface The mock. */ - private function objectService(array $found): ObjectService { + private function objectService(array $found): ObjectServiceInterface { $objectService = $this->createMock(ObjectServiceInterface::class); $objectService->method('searchObjects')->willReturn($found); return $objectService; diff --git a/tests/Unit/Service/ReviewServiceTest.php b/tests/Unit/Service/ReviewServiceTest.php index 462b6e27..a351c75c 100644 --- a/tests/Unit/Service/ReviewServiceTest.php +++ b/tests/Unit/Service/ReviewServiceTest.php @@ -280,7 +280,7 @@ public function testEntityUuidReadsAMagicAccessorUuid(): void { * * @return ObjectServiceInterface The mock. */ - private function objectService(array $found): ObjectService { + private function objectService(array $found): ObjectServiceInterface { $objectService = $this->createMock(ObjectServiceInterface::class); $objectService->method('searchObjects')->willReturn($found); $objectService->method('saveObject')->willReturnCallback(