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/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/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/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() 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(