Skip to content
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
19 changes: 10 additions & 9 deletions lib/Service/OrganizationSyncService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 7 additions & 2 deletions lib/Service/ViewService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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',
Expand Down
24 changes: 17 additions & 7 deletions tests/Stubs/Db/ObjectEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<string,mixed> */
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<string,mixed>|null $object
Expand Down
4 changes: 2 additions & 2 deletions tests/Unit/Controller/OrganisationMembersControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Service/FacetServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions tests/Unit/Service/IntakeModerationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down
14 changes: 7 additions & 7 deletions tests/Unit/Service/MergeOrganisatieServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -806,25 +806,25 @@ public function getUuid() {
*
* @return array<string, mixed>
*/
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()

Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Service/ReviewAggregateServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Service/ReviewServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading