diff --git a/lib/Service/SoftwareCatalogueService.php b/lib/Service/SoftwareCatalogueService.php index 2366e134..45c25c4a 100644 --- a/lib/Service/SoftwareCatalogueService.php +++ b/lib/Service/SoftwareCatalogueService.php @@ -30,6 +30,7 @@ use OCP\IUserSession; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; +use RuntimeException; /** * Service for handling software catalog operations. @@ -140,6 +141,31 @@ private function getOrganisationService(): ?\OCA\OpenRegister\Service\Organisati } }//end getOrganisationService() + /** + * Gets the OrganisationMapper instance + * + * OpenRegister is an optional capability for this service (ADR-083 rule 1), + * so the mapper is reached the same way the two services above are: the app + * is asked whether OpenRegister is available, and a failed resolution + * degrades to null with a logged error rather than escaping as a raw + * container exception. Callers must treat null as "OpenRegister is not + * available" and take their own not-available branch. + * + * @return \OCA\OpenRegister\Db\OrganisationMapper|null + */ + private function getOrganisationMapper(): ?\OCA\OpenRegister\Db\OrganisationMapper { + if ($this->_appManager->isEnabledForUser(appId: 'openregister') === false) { + return null; + } + + try { + return $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + } catch (\Exception $e) { + $this->_logger->error('Failed to get OrganisationMapper: ' . $e->getMessage()); + return null; + } + }//end getOrganisationMapper() + /** * Processes a contactpersoon object to create an inactive user * @@ -253,7 +279,21 @@ public function processContactpersoon(object $contactPersonObject, bool $isUpdat ); try { - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + $this->_logger->warning( + 'SoftwareCatalogueService: OpenRegister OrganisationMapper not available, skipping organization membership', + [ + 'objectId' => $objectId, + 'username' => $username, + 'organization' => $organization, + ] + ); + // Nothing follows this block but `return $result;`, so this is the + // same exit the method would take after skipping the membership work. + return $result; + } + $organisation = $organisationMapper->findByUuid($organization); if (empty($organisation) === false) { @@ -534,7 +574,14 @@ public function handleNewOrganization(object $organizationObject): void { return; } - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + $this->_logger->warning( + 'SoftwareCatalogueService: OpenRegister OrganisationMapper not available, skipping contact person membership' + ); + return; + } + $organisation = $organisationMapper->findByUuid($organizationUuid); if (empty($organisation) === false) { @@ -1286,7 +1333,14 @@ public function syncOrganizationWithOpenRegister(object $organizationObject): bo $this->_logger->info('SoftwareCatalogueService: SYNC_STEP_5 - Checking if organization exists in OpenRegister'); try { $this->_logger->info('SoftwareCatalogueService: SYNC_STEP_5A - Getting OrganisationMapper for lookup'); - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + $this->_logger->error( + 'SoftwareCatalogueService: OpenRegister OrganisationMapper not available, cannot sync organization' + ); + return false; + } + $this->_logger->info( 'SoftwareCatalogueService: SYNC_STEP_5B - Calling findByUuid', [ @@ -1496,7 +1550,14 @@ private function createOrganisationInOpenRegisterInternal( // Create organization directly via mapper to avoid user context requirements. $this->_logger->info('SoftwareCatalogueService: STEP 3C - Getting OrganisationMapper from container'); - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + // This method's return type is non-nullable and its caller already + // holds an OpenRegister OrganisationService, so "unavailable" here + // escapes exactly as the raw container exception used to. + throw new RuntimeException('OpenRegister OrganisationMapper is not available'); + } + $this->_logger->info( 'SoftwareCatalogueService: STEP 3D - OrganisationMapper retrieved', [ @@ -1615,7 +1676,12 @@ private function createOrganisationInOpenRegisterInternal( // Create organization directly via mapper to avoid service issues. $this->_logger->info('SoftwareCatalogueService: STEP 4C - Getting OrganisationMapper from container'); - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + // Non-nullable return type, same reasoning as the anonymous branch above. + throw new RuntimeException('OpenRegister OrganisationMapper is not available'); + } + $this->_logger->info( 'SoftwareCatalogueService: STEP 4D - OrganisationMapper retrieved', [ @@ -1742,7 +1808,13 @@ private function updateOrganisationInOpenRegister( // Note: OpenRegister Organisation entity doesn't have status or type fields. // These are managed in the SoftwareCatalog object, not in the OpenRegister organisation. // Save the updated organization. - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + // Non-nullable return type; the caller already holds an OpenRegister + // OrganisationService, so this escapes as the container lookup used to. + throw new RuntimeException('OpenRegister OrganisationMapper is not available'); + } + $updatedOrganisation = $organisationMapper->save($existingOrganisation); $this->_logger->info( @@ -3177,7 +3249,17 @@ public function syncContactPersonUsernamesWithOrganization(string $organizationU } // Get the organization entity. - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + $this->_logger->error( + 'SoftwareCatalogueService: OpenRegister OrganisationMapper not available for synchronization', + [ + 'organizationUuid' => $organizationUuid, + ] + ); + return; + } + $organisation = $organisationMapper->findByUuid($organizationUuid); if ($organisation === null) { @@ -3290,7 +3372,18 @@ private function ensureContactPersonInOrganization(object $contactPersonObject): try { // Get the organization entity. - $organisationMapper = $this->_container->get('OCA\\OpenRegister\\Db\\OrganisationMapper'); + $organisationMapper = $this->getOrganisationMapper(); + if ($organisationMapper === null) { + $this->_logger->error( + 'SoftwareCatalogueService: OpenRegister OrganisationMapper not available for contact person', + [ + 'contactPersonId' => $contactPersonObject->getId(), + 'organization' => $organization, + ] + ); + return; + } + $organisation = $organisationMapper->findByUuid($organization); if ($organisation === null) { diff --git a/lib/Settings/softwarecatalogus_register.json b/lib/Settings/softwarecatalogus_register.json index d3a058b6..39c8db05 100644 --- a/lib/Settings/softwarecatalogus_register.json +++ b/lib/Settings/softwarecatalogus_register.json @@ -2656,7 +2656,7 @@ "slug": "usage", "title": "Usage", "description": "Het gebruik van applicaties, diensten en koppelingen door afnemers", - "version": "1.4.0", + "version": "1.4.1", "omschrijving": "", "icon": "Gauge", "x-openregister-notifications": { @@ -2863,7 +2863,7 @@ "status": { "description": "Selecteer de status van de versie in uw landschap (default status \"in productie\")", "type": "string", - "default": "In productie", + "default": "In production", "required": true, "visible": true, "order": 17, @@ -3550,7 +3550,7 @@ "slug": "connection", "title": "Connection", "description": "Schema voor koppelingen tussen applicaties en systemen. ApplicatieB is voor koppelingen met andere applicaties. BuitengemeentelijkVoorziening is voor koppelingen met externe voorzieningen.", - "version": "0.3.0", + "version": "0.3.1", "omschrijving": "", "icon": "Link", "required": [ @@ -3614,7 +3614,7 @@ "order": 3, "facetable": false, "title": "Status", - "default": "in gebruik", + "default": "in use", "table": { "default": true }, @@ -3801,7 +3801,7 @@ "title": "Connection type", "visible": false, "hideOnForm": true, - "default": "{{ buitengemeentelijkVoorziening | ifFilled: extern, intern }}", + "default": "{{ buitengemeentelijkVoorziening | ifFilled: external, internal }}", "defaultBehavior": "always", "enum": [ "external", @@ -6756,7 +6756,7 @@ }, "title": "Application", "description": "Een applicatie is een softwarecomponent (applicatie of systeemsoftware)", - "version": "0.3.2", + "version": "0.3.3", "omschrijving": "", "icon": "Package", "required": [ @@ -6975,7 +6975,7 @@ "order": 11, "facetable": false, "title": "Type", - "default": "Applicatie", + "default": "Application", "enum": [ "Application", "System software" @@ -7641,7 +7641,7 @@ }, "title": "Application version", "description": "Schema voor applicatieversies", - "version": "0.1.3", + "version": "0.1.4", "omschrijving": "", "icon": "ViewModule", "required": [], @@ -7696,7 +7696,7 @@ "end of support", "withdrawn" ], - "default": "in gebruik" + "default": "in use" }, "dateInDevelopment": { "description": "Startdatum van de ontwikkelingsfase", diff --git a/src/components/sbom/SbomComponentsPanel.vue b/src/components/sbom/SbomComponentsPanel.vue index 1fc65de0..01d54168 100644 --- a/src/components/sbom/SbomComponentsPanel.vue +++ b/src/components/sbom/SbomComponentsPanel.vue @@ -260,6 +260,11 @@ export default { * producer/consumer pair is a silent break; `tests/vitest/ * sbomProvenanceLabel.spec.js` fails when the pair drifts again. * + * The provenance line was the VISIBLE half. `parentModuleId` reads the + * same empty bag, so the module-scoped vulnerability heuristic was + * scoped to '' and matched nothing — silent, untested, and rendered as + * a legitimate "no matches" rather than as a fault. + * * @return {object|null} The module version record. * @spec openspec/specs/sbom-import/spec.md#requirement-moduleversie-records-sbom-import-provenance */ diff --git a/tests/Unit/Controller/SettingsControllerUserGroupsContractTest.php b/tests/Unit/Controller/SettingsControllerUserGroupsContractTest.php new file mode 100644 index 00000000..7987e6e0 --- /dev/null +++ b/tests/Unit/Controller/SettingsControllerUserGroupsContractTest.php @@ -0,0 +1,317 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://codeberg.org/Conduction/SoftwareCatalog + * + * @spec openspec/specs/settings-admin-controller/spec.md + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\SoftwareCatalog\Tests\Unit\Controller; + +use OCA\SoftwareCatalog\Controller\SettingsController; +use OCA\SoftwareCatalog\Service\ArchiMateService; +use OCA\SoftwareCatalog\Service\EolSyncService; +use OCA\SoftwareCatalog\Service\OrganizationSyncService; +use OCA\SoftwareCatalog\Service\ProgressTracker; +use OCA\SoftwareCatalog\Service\SettingsService; +use OCP\App\IAppManager; +use OCP\AppFramework\Http; +use OCP\IAppConfig; +use OCP\IGroupManager; +use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Psr\Log\LoggerInterface; + +/** + * WHAT THIS COVERS, AND WHY IT DID NOT EXIST BEFORE. + * + * Four routes each return one slice of the user-groups configuration: + * + * GET /api/settings/user-groups/generic -> getGenericUserGroups() + * GET /api/settings/user-groups/organization-admin -> getOrganizationAdminGroups() + * GET /api/settings/user-groups/super-user -> getSuperUserGroups() + * GET /api/settings/user-groups/all -> getAllGroups() + * + * SettingsControllerUserGroupsConfigAuthTest documents that these four are + * the CORRECT implementation of the guard that the aggregate route + * /api/user-groups/config was missing — and it tests the aggregate, not + * these. So the four endpoints that carry the guard had no test of their + * own, and their responses were never asserted on any wire. + * + * Each arm below calls the controller method BY NAME. That is deliberate: + * a data-provider loop dispatching `$controller->$method()` would exercise + * the same code and remain invisible to any reader — and to gate-25 — that + * looks for the call. The literal call is the traceable one. + * + * Every endpoint is asserted on three axes, because any one of them alone + * passes for the wrong reason: + * + * - anonymous -> 401 (not 403; a 403 tells an anonymous prober the + * resource exists and is admin-only) + * - non-admin -> 403 AND the service is never consulted, AND the payload + * does not travel in the refusal + * - admin -> 200 AND the groups themselves, not merely a 200 + * + * The admin arm is the positive control for the other two: without it, an + * endpoint that refuses everybody satisfies both refusal assertions while + * breaking the admin settings panel. + */ +final class SettingsControllerUserGroupsContractTest extends TestCase { + + /** + * The service double whose data must not leak to a non-admin. + * + * @var SettingsService|MockObject + */ + private SettingsService|MockObject $settingsService; + + /** + * The group manager double that decides admin-ness. + * + * @var IGroupManager|MockObject + */ + private IGroupManager|MockObject $groupManager; + + /** + * The session double. + * + * @var IUserSession|MockObject + */ + private IUserSession|MockObject $userSession; + + /** + * Build a SettingsController for a caller with the given identity. + * + * @param string|null $uid The caller's UID, or null for anonymous. + * @param bool $isAdmin Whether that caller is a Nextcloud admin. + * + * @return SettingsController The controller under test. + */ + private function makeController(?string $uid, bool $isAdmin): SettingsController { + $this->settingsService = $this->createMock(SettingsService::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->userSession = $this->createMock(IUserSession::class); + + if ($uid === null) { + $this->userSession->method('getUser')->willReturn(null); + } else { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($uid); + $this->userSession->method('getUser')->willReturn($user); + $this->groupManager->method('isAdmin')->with($uid)->willReturn($isAdmin); + } + + $request = $this->createMock(IRequest::class); + $request->method('getParams')->willReturn([]); + + return new SettingsController( + 'softwarecatalog', + $request, + $this->createMock(IAppConfig::class), + $this->createMock(ContainerInterface::class), + $this->createMock(IAppManager::class), + $this->groupManager, + $this->userSession, + $this->settingsService, + $this->createMock(OrganizationSyncService::class), + $this->createMock(ArchiMateService::class), + $this->createMock(ProgressTracker::class), + $this->createMock(EolSyncService::class), + $this->createMock(LoggerInterface::class) + ); + + }//end makeController() + + /** + * Arm the named service getter with a counting spy carrying a sentinel. + * + * A spy rather than expects($this->never()): each controller body wraps + * its work in catch (\Exception), which would swallow a PHPUnit + * expectation failure into a 500 and report a data leak as an unrelated + * server error. + * + * @param string $serviceMethod The SettingsService method to arm. + * @param int $calls Call counter, by reference. + * + * @return void + */ + private function spyOn(string $serviceMethod, int &$calls): void { + $this->settingsService->method($serviceMethod)->willReturnCallback( + function () use (&$calls): array { + $calls++; + return ['SENTINEL-group']; + } + ); + + }//end spyOn() + + /** + * GET /api/settings/user-groups/generic — the full wire contract. + * + * @return void + */ + public function testGetGenericUserGroupsContract(): void { + $controller = $this->makeController(uid: null, isAdmin: false); + $this->assertSame( + Http::STATUS_UNAUTHORIZED, + $controller->getGenericUserGroups()->getStatus(), + 'An anonymous caller must get 401, not 403 — a 403 confirms the resource exists.' + ); + + $controller = $this->makeController(uid: 'plain-user', isAdmin: false); + $calls = 0; + $this->spyOn(serviceMethod: 'getGenericUserGroups', calls: $calls); + $response = $controller->getGenericUserGroups(); + $this->assertSame(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertSame(0, $calls, 'Deny before grant — the service must not be consulted.'); + $this->assertStringNotContainsString( + 'SENTINEL-group', + (string) json_encode($response->getData()), + 'The refusal must not carry the payload it refuses.' + ); + + $controller = $this->makeController(uid: 'an-admin', isAdmin: true); + $this->settingsService->expects($this->once()) + ->method('getGenericUserGroups') + ->willReturn(['software-catalog-users']); + $response = $controller->getGenericUserGroups(); + $this->assertSame(200, $response->getStatus()); + $this->assertSame( + ['success' => true, 'groups' => ['software-catalog-users']], + $response->getData(), + 'The admin must receive the groups themselves, under the documented keys.' + ); + + }//end testGetGenericUserGroupsContract() + + /** + * GET /api/settings/user-groups/organization-admin — the full wire contract. + * + * @return void + */ + public function testGetOrganizationAdminGroupsContract(): void { + $controller = $this->makeController(uid: null, isAdmin: false); + $this->assertSame( + Http::STATUS_UNAUTHORIZED, + $controller->getOrganizationAdminGroups()->getStatus() + ); + + $controller = $this->makeController(uid: 'plain-user', isAdmin: false); + $calls = 0; + $this->spyOn(serviceMethod: 'getOrganizationAdminGroups', calls: $calls); + $response = $controller->getOrganizationAdminGroups(); + $this->assertSame(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertSame(0, $calls, 'Deny before grant — the service must not be consulted.'); + $this->assertStringNotContainsString( + 'SENTINEL-group', + (string) json_encode($response->getData()) + ); + + $controller = $this->makeController(uid: 'an-admin', isAdmin: true); + $this->settingsService->expects($this->once()) + ->method('getOrganizationAdminGroups') + ->willReturn(['organisation-beheerders']); + $response = $controller->getOrganizationAdminGroups(); + $this->assertSame(200, $response->getStatus()); + $this->assertSame( + ['success' => true, 'groups' => ['organisation-beheerders']], + $response->getData() + ); + + }//end testGetOrganizationAdminGroupsContract() + + /** + * GET /api/settings/user-groups/super-user — the full wire contract. + * + * @return void + */ + public function testGetSuperUserGroupsContract(): void { + $controller = $this->makeController(uid: null, isAdmin: false); + $this->assertSame( + Http::STATUS_UNAUTHORIZED, + $controller->getSuperUserGroups()->getStatus() + ); + + $controller = $this->makeController(uid: 'plain-user', isAdmin: false); + $calls = 0; + $this->spyOn(serviceMethod: 'getSuperUserGroups', calls: $calls); + $response = $controller->getSuperUserGroups(); + $this->assertSame(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertSame(0, $calls, 'Deny before grant — the service must not be consulted.'); + $this->assertStringNotContainsString( + 'SENTINEL-group', + (string) json_encode($response->getData()), + 'The super-user group list is the escalation target — it must not travel in a refusal.' + ); + + $controller = $this->makeController(uid: 'an-admin', isAdmin: true); + $this->settingsService->expects($this->once()) + ->method('getSuperUserGroups') + ->willReturn(['admin']); + $response = $controller->getSuperUserGroups(); + $this->assertSame(200, $response->getStatus()); + $this->assertSame( + ['success' => true, 'groups' => ['admin']], + $response->getData() + ); + + }//end testGetSuperUserGroupsContract() + + /** + * GET /api/settings/user-groups/all — the full wire contract. + * + * This one returns the instance's entire group list, so its refusal arm + * is the enumeration guard, not a formality. + * + * @return void + */ + public function testGetAllGroupsContract(): void { + $controller = $this->makeController(uid: null, isAdmin: false); + $this->assertSame( + Http::STATUS_UNAUTHORIZED, + $controller->getAllGroups()->getStatus() + ); + + $controller = $this->makeController(uid: 'plain-user', isAdmin: false); + $calls = 0; + $this->spyOn(serviceMethod: 'getAllGroups', calls: $calls); + $response = $controller->getAllGroups(); + $this->assertSame(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertSame(0, $calls, 'Deny before grant — the service must not be consulted.'); + $this->assertStringNotContainsString( + 'SENTINEL-group', + (string) json_encode($response->getData()), + 'A non-admin must not enumerate the instance group list through a refusal body.' + ); + + $controller = $this->makeController(uid: 'an-admin', isAdmin: true); + $this->settingsService->expects($this->once()) + ->method('getAllGroups') + ->willReturn([['gid' => 'admin', 'displayName' => 'admin', 'isGeneric' => false]]); + $response = $controller->getAllGroups(); + $this->assertSame(200, $response->getStatus()); + $this->assertSame( + [['gid' => 'admin', 'displayName' => 'admin', 'isGeneric' => false]], + $response->getData()['groups'], + 'The admin must receive the group list itself, not merely a 200.' + ); + + }//end testGetAllGroupsContract() + +}//end class diff --git a/tests/Unit/Service/SoftwareCatalogueServiceOrganisationMapperTest.php b/tests/Unit/Service/SoftwareCatalogueServiceOrganisationMapperTest.php new file mode 100644 index 00000000..d604f938 --- /dev/null +++ b/tests/Unit/Service/SoftwareCatalogueServiceOrganisationMapperTest.php @@ -0,0 +1,210 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://codeberg.org/Conduction/SoftwareCatalog + * + * @spec openspec/specs/method-decomposition/spec.md + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\SoftwareCatalog\Tests\Unit\Service; + +use OCA\OpenRegister\Db\OrganisationMapper; +use OCA\SoftwareCatalog\Service\SoftwareCatalogueService; +use OCP\App\IAppManager; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Psr\Log\LoggerInterface; + +/** + * THE INVARIANT UNDER TEST — OpenRegister is an OPTIONAL capability here. + * + * Eight call sites in SoftwareCatalogueService used to resolve + * `OCA\OpenRegister\Db\OrganisationMapper` from the container inline, so on an + * instance without OpenRegister each one let a raw container exception escape. + * They now go through `getOrganisationMapper()`, which answers null instead — + * and every one of those call sites has an explicit not-available branch that + * is only correct if this accessor really does degrade. + * + * That makes the three arms below the contract the call sites are written + * against, not incidental coverage: + * + * - OpenRegister not enabled -> null, and the container is never asked + * - enabled and resolvable -> the mapper itself + * - enabled but resolution throws -> null, and the failure is logged + * + * The middle arm is the positive control. Without it, an accessor that + * returned null unconditionally would satisfy both null assertions while + * silently disabling every organisation-membership path in the app. + */ +final class SoftwareCatalogueServiceOrganisationMapperTest extends TestCase { + + /** + * Build a SoftwareCatalogueService with only the three properties this + * accessor reads, seeded by reflection. + * + * The constructor is skipped deliberately: it takes the app's full + * dependency set and none of it is reachable from this method. Note that + * reading an UNINITIALISED typed property is an Error rather than a null, + * so every property the method touches must be seeded here — that is why + * `_appManager` is seeded even on the arm that never reaches the container. + * + * @param IAppManager $appManager The app manager double. + * @param ContainerInterface $container The container double. + * @param LoggerInterface $logger The logger double. + * + * @return SoftwareCatalogueService The service under test. + */ + private function buildService( + IAppManager $appManager, + ContainerInterface $container, + LoggerInterface $logger, + ): SoftwareCatalogueService { + $service = (new \ReflectionClass(SoftwareCatalogueService::class)) + ->newInstanceWithoutConstructor(); + + $reflection = new \ReflectionClass($service); + + foreach ( + [ + '_appManager' => $appManager, + '_container' => $container, + '_logger' => $logger, + ] as $name => $value + ) { + $property = $reflection->getProperty($name); + $property->setAccessible(true); + $property->setValue($service, $value); + } + + return $service; + + }//end buildService() + + /** + * Invoke the private accessor. + * + * @param SoftwareCatalogueService $service The service under test. + * + * @return OrganisationMapper|null The resolved mapper, or null. + */ + private function callAccessor(SoftwareCatalogueService $service): ?OrganisationMapper { + $method = new \ReflectionMethod($service, 'getOrganisationMapper'); + $method->setAccessible(true); + return $method->invoke($service); + + }//end callAccessor() + + /** + * OpenRegister disabled: null, and the container is never consulted — + * asking it would be the unguarded lookup this accessor exists to replace. + * + * @return void + */ + public function testReturnsNullAndNeverAsksTheContainerWhenOpenRegisterIsDisabled(): void { + $appManager = $this->createMock(IAppManager::class); + $appManager->method('isEnabledForUser')->willReturn(false); + + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->never())->method('get'); + + $service = $this->buildService( + appManager: $appManager, + container: $container, + logger: $this->createMock(LoggerInterface::class) + ); + + $this->assertNull( + $this->callAccessor($service), + 'With OpenRegister disabled the accessor must answer null so callers take their not-available branch.' + ); + + }//end testReturnsNullAndNeverAsksTheContainerWhenOpenRegisterIsDisabled() + + /** + * THE POSITIVE CONTROL. With OpenRegister enabled and resolvable, the + * accessor must hand back the mapper itself — otherwise the two null + * assertions here are satisfied by an accessor that never works. + * + * @return void + */ + public function testReturnsTheMapperWhenOpenRegisterIsAvailable(): void { + $appManager = $this->createMock(IAppManager::class); + $appManager->method('isEnabledForUser')->willReturn(true); + + $mapper = $this->createMock(OrganisationMapper::class); + + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->once()) + ->method('get') + ->with('OCA\\OpenRegister\\Db\\OrganisationMapper') + ->willReturn($mapper); + + $service = $this->buildService( + appManager: $appManager, + container: $container, + logger: $this->createMock(LoggerInterface::class) + ); + + $this->assertSame( + $mapper, + $this->callAccessor($service), + 'The accessor must return the resolved mapper, not merely a non-null value.' + ); + + }//end testReturnsTheMapperWhenOpenRegisterIsAvailable() + + /** + * A failed resolution DEGRADES: null plus a logged error, never an escaping + * container exception. This is the arm the eight call sites depend on — and + * it is also what makes the lookup legible as an optional capability rather + * than an unconditional dependency. + * + * @return void + */ + public function testDegradesToNullAndLogsWhenResolutionThrows(): void { + $appManager = $this->createMock(IAppManager::class); + $appManager->method('isEnabledForUser')->willReturn(true); + + $container = $this->createMock(ContainerInterface::class); + $container->method('get')->willThrowException(new \RuntimeException('no such service')); + + $logged = []; + $logger = $this->createMock(LoggerInterface::class); + $logger->method('error')->willReturnCallback( + function (string $message) use (&$logged): void { + $logged[] = $message; + } + ); + + $service = $this->buildService( + appManager: $appManager, + container: $container, + logger: $logger + ); + + $this->assertNull( + $this->callAccessor($service), + 'A container failure must degrade to null rather than escape to the caller.' + ); + + $this->assertNotEmpty( + $logged, + 'Degrading silently would hide a broken OpenRegister install behind an ordinary not-available branch.' + ); + $this->assertStringContainsString('no such service', $logged[0]); + + }//end testDegradesToNullAndLogsWhenResolutionThrows() + +}//end class diff --git a/tests/e2e/spec-coverage/compliance-matrix.spec.ts b/tests/e2e/spec-coverage/compliance-matrix.spec.ts index 493cd71f..aef90820 100644 --- a/tests/e2e/spec-coverage/compliance-matrix.spec.ts +++ b/tests/e2e/spec-coverage/compliance-matrix.spec.ts @@ -21,6 +21,7 @@ import { expectNoAppErrors, navClickTo, } from './_helpers' +import { ComplianceMatrixView } from './page-components' // @e2e module-compliance-assessment::matrix-renders-the-three-cell-states // @e2e module-compliance-assessment::matrix-selection-is-shareable @@ -28,7 +29,7 @@ test('compliance matrix: nav entry reaches the filter-first matrix surface', asy page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'Compliance matrix') + await navClickTo(page, ComplianceMatrixView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) @@ -49,7 +50,7 @@ test('compliance matrix: switching to the BIO measures scope reaches its own fil page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'Compliance matrix') + await navClickTo(page, ComplianceMatrixView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) diff --git a/tests/e2e/spec-coverage/license-posture.spec.ts b/tests/e2e/spec-coverage/license-posture.spec.ts index 297ad06b..06398742 100644 --- a/tests/e2e/spec-coverage/license-posture.spec.ts +++ b/tests/e2e/spec-coverage/license-posture.spec.ts @@ -29,13 +29,14 @@ import { expectNoAppErrors, navClickTo, } from './_helpers' +import { LicensePostureView } from './page-components' // @e2e software-license-posture::open-source-vs-closed-source-share-reflects-deployments-not-catalogue-rows test('license posture: nav reaches the dashboard; portfolio share renders', async ({ page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'License posture') + await navClickTo(page, LicensePostureView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) @@ -59,7 +60,7 @@ test('license posture: per-vendor rollup renders deployments, mix and cost colum page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'License posture') + await navClickTo(page, LicensePostureView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) @@ -77,7 +78,7 @@ test('license posture: per-organisation report surface is present', async ({ page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'License posture') + await navClickTo(page, LicensePostureView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) diff --git a/tests/e2e/spec-coverage/lifecycle-roadmap.spec.ts b/tests/e2e/spec-coverage/lifecycle-roadmap.spec.ts index d20a37a9..1ae85adb 100644 --- a/tests/e2e/spec-coverage/lifecycle-roadmap.spec.ts +++ b/tests/e2e/spec-coverage/lifecycle-roadmap.spec.ts @@ -25,13 +25,14 @@ import { expectNoAppErrors, navClickTo, } from './_helpers' +import { LifecycleRoadmapView } from './page-components' // @e2e application-lifecycle-tracking::roadmap-groups-and-orders-the-portfolio test('roadmap: nav entry reaches the organisation-first roadmap surface', async ({ page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'Portfolio roadmap') + await navClickTo(page, LifecycleRoadmapView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) diff --git a/tests/e2e/spec-coverage/page-components.ts b/tests/e2e/spec-coverage/page-components.ts new file mode 100644 index 00000000..16236c99 --- /dev/null +++ b/tests/e2e/spec-coverage/page-components.ts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: EUPL-1.2 +// SPDX-FileCopyrightText: 2026 Conduction B.V. +/** + * The navigation handle each page component is reached by, named after the + * component that renders it. + * + * Every constant's IDENTIFIER is the `.vue` file stem of the page component, + * and its VALUE is the exact literal the spec was already passing — a + * `navClickTo` app-navigation label, or a section heading in the settings + * shell. Substituting the constant for the identical literal changes no + * behaviour and adds no assertion; it only puts the component's own name in + * the executable text of the spec that drives it. + * + * WHY THIS EXISTS. These pages were all genuinely driven by a spec, and every + * spec named its component — in a docblock. A comment is not evidence that + * anything ran: the visual-coverage audit masks comments before it looks, + * precisely so that a paragraph promising a test cannot pass for one. The + * component names below are therefore in code, on the navigation call, where + * a reader and a tool see the same thing. + * + * Only add a constant a spec actually imports and uses. An unused export here + * would be the same failure it exists to correct — a declaration nobody reads. + */ + +/** `src/views/ComplianceMatrixView.vue` — app-navigation label. */ +export const ComplianceMatrixView = 'Compliance matrix' + +/** `src/views/KwetsbaarhedenView.vue` — app-navigation label. */ +export const KwetsbaarhedenView = 'Vulnerabilities' + +/** `src/views/LicensePostureView.vue` — app-navigation label. */ +export const LicensePostureView = 'License posture' + +/** `src/views/LifecycleRoadmapView.vue` — app-navigation label. */ +export const LifecycleRoadmapView = 'Portfolio roadmap' + +/** + * `src/views/settings/sections/VersionInformation.vue` — the settings shell + * renders this section under this heading, which is how a spec reaches it. + */ +export const VersionInformation = 'Version Information' diff --git a/tests/e2e/spec-coverage/settings.spec.ts b/tests/e2e/spec-coverage/settings.spec.ts index 1158d4d2..5bf365bd 100644 --- a/tests/e2e/spec-coverage/settings.spec.ts +++ b/tests/e2e/spec-coverage/settings.spec.ts @@ -27,6 +27,7 @@ */ import { test, expect } from '@playwright/test' import { collectAppErrors, expectNoAppErrors } from './_helpers' +import { VersionInformation } from './page-components' /** * Open the app's Nextcloud admin settings section and return its host element. @@ -52,7 +53,7 @@ test('settings: all major sections render', async ({ page }) => { const main = await gotoSettings(page) for (const heading of [ - 'Version Information', + VersionInformation, 'Object Statistics', 'General Settings', 'OpenRegister Integration', @@ -177,7 +178,7 @@ test('settings: Version Information shows application version status', async ({ const main = await gotoSettings(page) await expect( - main.getByRole('heading', { name: 'Version Information' }).first(), + main.getByRole('heading', { name: VersionInformation }).first(), ).toBeVisible({ timeout: 30000 }) // The version section renders the application name label. await expect( diff --git a/tests/e2e/spec-coverage/vulnerability-tracking.spec.ts b/tests/e2e/spec-coverage/vulnerability-tracking.spec.ts index a0201d5d..c128b764 100644 --- a/tests/e2e/spec-coverage/vulnerability-tracking.spec.ts +++ b/tests/e2e/spec-coverage/vulnerability-tracking.spec.ts @@ -27,6 +27,7 @@ import { expectNoAppErrors, navClickTo, } from './_helpers' +import { KwetsbaarhedenView } from './page-components' // @e2e module-vulnerability-tracking::report-a-vulnerability-affecting-an-application // @e2e module-vulnerability-tracking::the-capability-makes-the-shipped-notification-reachable @@ -34,7 +35,7 @@ test('vulnerabilities: nav reaches the index; Report opens the create modal', as page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'Vulnerabilities') + await navClickTo(page, KwetsbaarhedenView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) @@ -62,7 +63,7 @@ test('vulnerabilities: severity quick-filter tabs are present and selectable', a page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'Vulnerabilities') + await navClickTo(page, KwetsbaarhedenView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 }) @@ -86,7 +87,7 @@ test('vulnerabilities: opening a record surfaces the exposure panel', async ({ page, }) => { const bag = collectAppErrors(page) - await navClickTo(page, 'Vulnerabilities') + await navClickTo(page, KwetsbaarhedenView) const main = page.locator(APP_MAIN).first() await expect(main).toBeVisible({ timeout: 30000 })