From f5c8b3ebf3db8f010ca4c87233ff0113f5e8f040 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Fri, 24 Jul 2026 08:27:12 +0200 Subject: [PATCH 1/4] =?UTF-8?q?fix(facets):=20normalize=20ObjectEntity=20r?= =?UTF-8?q?esults=20in=20FacetService=20=E2=80=94=20endpoint=20was=20dead?= =?UTF-8?q?=20in=20production?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /apps/softwarecatalog/api/facets/module returned HTTP 500 on every real request: FacetService::objectIdentifier(): Argument #1 ($object) must be of type array, OCA\OpenRegister\Db\ObjectEntity given. Root cause: OpenRegister's real ObjectService::searchObjectsPaginated() and searchObjects() return `results` as OCA\OpenRegister\Db\ObjectEntity instances, not plain arrays. Every downstream FacetService method (objectIdentifier(), extractRelatedIdentifiers(), extractRelatedNames(), ...) is array-typed. The unit test double fed the service plain arrays only, so the mismatch never surfaced in tests — classic test-fake drift — and the endpoint shipped 100% broken against real data. Fix: add a single normalizeObject() boundary in FacetService and map every OpenRegister search result through it before it enters any other method. Prefers jsonSerialize() (mirrors the real ObjectEntity contract — merges payload properties with @self metadata and the top-level id), falls back to getObject(), then to a raw array cast. Applied at both places FacetService consumes OR search results: fetchBaseObjects() (searchObjectsPaginated) and fetchModulesByIdentifiers() (searchObjects, the dienst-schema module batch lookup). Audited PortfolioReportService, SbomImportService, EolSyncService, and MergeOrganisatieService for the same assumption — all four already normalize correctly (normalizeResults()/normalizeRow() helpers or explicit ->getObject() calls), so they do not share this bug. Added FakeObjectEntity, a minimal JsonSerializable fake mirroring the real ObjectEntity's jsonSerialize() contract, and two regression tests that feed searchObjectsPaginated()/searchObjects() ObjectEntity-shaped results through both boundaries and assert identical facet counts to the array-shaped tests. --- lib/Service/FacetService.php | 58 +++++++- tests/Unit/Service/FacetServiceTest.php | 173 ++++++++++++++++++++++++ 2 files changed, 229 insertions(+), 2 deletions(-) diff --git a/lib/Service/FacetService.php b/lib/Service/FacetService.php index bd9a2326..7c6a8e25 100644 --- a/lib/Service/FacetService.php +++ b/lib/Service/FacetService.php @@ -437,7 +437,7 @@ private function fetchBaseObjects(ObjectService $objectService, string $schema, $pagedQuery['_page'] = $page; $paginated = $objectService->searchObjectsPaginated($pagedQuery); - $results = $paginated['results'] ?? []; + $results = array_map([$this, 'normalizeObject'], $paginated['results'] ?? []); $allObjects = array_merge($allObjects, $results); $totalPages = (int) ($paginated['pages'] ?? 1); @@ -464,6 +464,52 @@ private function fetchBaseObjects(ObjectService $objectService, string $schema, }//end fetchBaseObjects() + /** + * Normalize a single OpenRegister search result into a plain data-bag + * array, tolerant of the several shapes OpenRegister can hand back. + * + * `ObjectService::searchObjectsPaginated()`/`searchObjects()` return + * `OCA\OpenRegister\Db\ObjectEntity` instances in production (confirmed + * live: `ObjectEntity` given where an `array` was assumed), NOT plain + * arrays — only the unit-test stub returned arrays, which is how this + * shipped green and dead. This is the single boundary every OR search + * result MUST pass through before any other FacetService method (all of + * which are `array`-typed) touches it. + * + * Preference order: already an array → returned as-is. An `ObjectEntity` + * (or any object exposing `jsonSerialize()`) → `jsonSerialize()`, which + * merges the payload properties with `@self` metadata AND mirrors the id + * at the top level (see `ObjectEntity::jsonSerialize()`), so both the + * payload fields `extractRelatedIdentifiers()`/`extractRelatedNames()` + * read AND the `id`/`@self.id` shapes `objectIdentifier()` reads survive. + * An object exposing only `getObject()` → that (payload only, `id` + * still present at top level per `ObjectEntity::getObject()`). Anything + * else → cast to array as a last-resort fallback. + * + * @param mixed $object A single OpenRegister search result entry. + * + * @return array The normalized data-bag array. + * + * @spec openspec/specs/gemma-faceted-search/spec.md#requirement-facet-aggregation-endpoint-returns-gemma-dimension-counts + */ + private function normalizeObject(mixed $object): array + { + if (is_array($object) === true) { + return $object; + } + + if (is_object($object) === true && method_exists($object, 'jsonSerialize') === true) { + return (array) $object->jsonSerialize(); + } + + if (is_object($object) === true && method_exists($object, 'getObject') === true) { + return (array) $object->getObject(); + } + + return (array) $object; + + }//end normalizeObject() + /** * Resolve the module object(s) backing each base object's GEMMA dimensions. * @@ -567,8 +613,16 @@ private function fetchModulesByIdentifiers(ObjectService $objectService, array $ return []; } + if (is_array($results) === false) { + // Defensive: `searchObjects()`'s declared return type is + // `array|int` (a `count`-mode caller could pass a shape that + // resolves to an int); this call site is never in count mode, + // but stay defensive rather than fatal on an unexpected shape. + return []; + } + $byId = []; - foreach ($results as $module) { + foreach (array_map([$this, 'normalizeObject'], $results) as $module) { $key = $this->objectIdentifier(object: $module); $byId[$key] = $module; } diff --git a/tests/Unit/Service/FacetServiceTest.php b/tests/Unit/Service/FacetServiceTest.php index 3f4ad88b..32597eb2 100644 --- a/tests/Unit/Service/FacetServiceTest.php +++ b/tests/Unit/Service/FacetServiceTest.php @@ -38,6 +38,50 @@ use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; +/** + * Minimal fake mirroring the runtime contract of the REAL + * `OCA\OpenRegister\Db\ObjectEntity` (not the test-only abstract stub in + * `tests/Stubs/Db/ObjectEntity.php`, which cannot be instantiated directly): + * `jsonSerialize()` merges the payload properties with `@self` metadata and + * mirrors `id` at the top level, exactly as + * `OCA\OpenRegister\Db\ObjectEntity::jsonSerialize()` does in production. + * + * `searchObjectsPaginated()`/`searchObjects()` return real `ObjectEntity` + * instances in production, never plain arrays — the OLD test double fed + * `FacetService` plain arrays only, which is exactly how the production 500 + * (`FacetService::objectIdentifier(): Argument #1 ($object) must be of type + * array, OCA\OpenRegister\Db\ObjectEntity given`) shipped green. + * + * @category Test + * @package OCA\SoftwareCatalog\Tests\Unit\Service + */ +final class FakeObjectEntity implements \JsonSerializable +{ + + /** + * @param string $id The object's identifier. + * @param array $payload The object's payload properties (no `id`/`@self`). + */ + public function __construct(private readonly string $id, private readonly array $payload) + { + } + + /** + * Mirrors `ObjectEntity::jsonSerialize()`: payload + `@self.id` + top-level `id`. + * + * @return array + */ + public function jsonSerialize(): array + { + $data = $this->payload; + $data['@self'] = ['id' => $this->id]; + $data['id'] = $this->id; + + return $data; + + }//end jsonSerialize() +}//end class + /** * Unit tests for FacetService. * @@ -275,6 +319,78 @@ public function testGetFacetsAggregatesDirectModuleFields(): void }//end testGetFacetsAggregatesDirectModuleFields() + /** + * REGRESSION: the SAME assertions as + * `testGetFacetsAggregatesDirectModuleFields()`, but `searchObjectsPaginated()` + * returns `FakeObjectEntity` instances (mirroring the real OpenRegister + * `ObjectEntity` contract) instead of plain arrays — this is the shape + * production actually hands back. Proves `FacetService` normalizes at + * the `fetchBaseObjects()` boundary rather than assuming `array` + * (the exact defect that produced the live 500: + * `objectIdentifier(): Argument #1 ($object) must be of type array, + * OCA\OpenRegister\Db\ObjectEntity given`). + * + * @spec openspec/specs/gemma-faceted-search/spec.md#requirement-facet-aggregation-endpoint-returns-gemma-dimension-counts + * + * @return void + */ + public function testGetFacetsAggregatesDirectModuleFieldsWhenResultsAreObjectEntityInstances(): void + { + $modules = [ + new FakeObjectEntity( + id: 'm1', + payload: [ + 'referentieComponenten' => [['identifier' => 'rc-1']], + 'standaardVersies' => [['name' => 'StUF-ZKN']], + ] + ), + new FakeObjectEntity( + id: 'm2', + payload: [ + 'referentieComponenten' => [['identifier' => 'rc-1']], + 'standaardVersies' => [['name' => 'StUF-ZKN']], + ] + ), + new FakeObjectEntity( + id: 'm3', + payload: [ + 'referentieComponenten' => ['rc-2'], + 'standaardVersies' => [], + ] + ), + ]; + + $captured = []; + $objectService = $this->makePaginatedObjectService(results: $modules, capturedRef: $captured); + + $archiMateService = $this->createMock(ArchiMateService::class); + $archiMateService->method('getElementObjects')->willReturn( + [ + ['identifier' => 'rc-1', 'name' => 'Zaakregistratiecomponent', 'domein' => 'Bedrijfsvoering'], + ['identifier' => 'rc-2', 'name' => 'Klantcontactcomponent', 'domein' => 'Dienstverlening'], + ] + ); + $archiMateService->method('getRelationshipObjects')->willReturn([]); + + $service = $this->makeService(objectService: $objectService, archiMateService: $archiMateService); + + $result = $service->getFacets(schema: 'module'); + + $refCompByValue = array_column($result['referentiecomponent'], 'count', 'value'); + $this->assertSame(2, $refCompByValue['Zaakregistratiecomponent']); + $this->assertSame(1, $refCompByValue['Klantcontactcomponent']); + + $standaardByValue = array_column($result['standaard'], 'count', 'value'); + $this->assertSame(2, $standaardByValue['StUF-ZKN']); + + $domeinByValue = array_column($result['domein'], 'count', 'value'); + $this->assertSame(2, $domeinByValue['Bedrijfsvoering']); + $this->assertSame(1, $domeinByValue['Dienstverlening']); + + $this->assertSame(3, $result['_meta']['totalMatched']); + + }//end testGetFacetsAggregatesDirectModuleFieldsWhenResultsAreObjectEntityInstances() + /** * `applicatieservice` is resolved via a `relation` connecting a referentiecomponent * element to an element with gemmaType === 'Applicatieservice'. @@ -607,4 +723,61 @@ function (array $query) use (&$capturedPaginated): array { $this->assertSame(1, $refCompByValue['Zaakregistratiecomponent']); }//end testGetFacetsResolvesDienstFacetsTransitivelyViaModules() + + /** + * REGRESSION: the SAME scenario as + * `testGetFacetsResolvesDienstFacetsTransitivelyViaModules()`, but BOTH + * OpenRegister boundaries return `FakeObjectEntity` instances instead of + * plain arrays — the bounded base-object page (`searchObjectsPaginated()`, + * the `dienst` results) AND the batch module lookup + * (`searchObjects()`, resolved inside `fetchModulesByIdentifiers()`). + * Proves normalization happens at BOTH boundaries `FacetService` + * consumes OpenRegister search results from, not just the first one. + * + * @spec openspec/specs/gemma-faceted-search/spec.md#requirement-facet-aggregation-endpoint-returns-gemma-dimension-counts + * + * @return void + */ + public function testGetFacetsResolvesDienstFacetsWhenBothLookupsReturnObjectEntityInstances(): void + { + $objectService = $this->createMock(ObjectService::class); + + $capturedPaginated = []; + $objectService->method('searchObjectsPaginated')->willReturnCallback( + function (array $query) use (&$capturedPaginated): array { + $capturedPaginated[] = $query; + return [ + 'results' => [ + new FakeObjectEntity(id: 'd1', payload: ['modules' => [['id' => 'm1']]]), + ], + 'total' => 1, + 'page' => 1, + 'pages' => 1, + ]; + } + ); + $objectService->method('searchObjects')->willReturn( + [ + new FakeObjectEntity( + id: 'm1', + payload: ['referentieComponenten' => [['identifier' => 'rc-1']], 'standaardVersies' => []] + ), + ] + ); + + $archiMateService = $this->createMock(ArchiMateService::class); + $archiMateService->method('getElementObjects')->willReturn( + [['identifier' => 'rc-1', 'name' => 'Zaakregistratiecomponent']] + ); + $archiMateService->method('getRelationshipObjects')->willReturn([]); + + $service = $this->makeService(objectService: $objectService, archiMateService: $archiMateService); + + $result = $service->getFacets(schema: 'dienst'); + + $refCompByValue = array_column($result['referentiecomponent'], 'count', 'value'); + $this->assertSame(1, $refCompByValue['Zaakregistratiecomponent']); + $this->assertSame(1, $result['_meta']['totalMatched']); + + }//end testGetFacetsResolvesDienstFacetsWhenBothLookupsReturnObjectEntityInstances() }//end class From 8e9bbd47b5994bfc515d5268b2f8ab63c18abda4 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Fri, 24 Jul 2026 09:18:32 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(portfolio-report):=20register=20organis?= =?UTF-8?q?atie=20by=20schema=20slug=20=E2=80=94=20picker=20was=20dead?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Portfolio rationalization organisation picker rendered "No results" on every real instance despite the register holding real Gemeente/Samenwerking/ supplier organisations. Console: Error fetching organisatie collection: Object type "organisatie" is not registered in the store. Root cause: loadOrganisations() resolved the schema via objectStore.getSchemaConfig('organisatie'), which only succeeds when the voorzieningenConfig blob's organisatie_schema key holds a NUMERIC schema id. That key is only ever populated by the voorzieningen auto-configure flow for module/compliancy/moduleVersie/sbomComponent — organisatie_schema (along with dienst/contactpersoon/gebruik/contract/koppeling/beoordeeling/suite/sector) is always empty on instances that never ran a legacy manual config step, so getSchemaConfig() silently produced no config and registerObjectType() was never called before fetchCollection() threw. The manifest-driven Organisaties index page never hits this: the shared library's self-fetch path (useSelfFetchList.js) registers the type using the SCHEMA SLUG itself ('organisatie') as the id, not a numeric id resolved from a config blob — OpenRegister's objects endpoint accepts a schema slug or a numeric id interchangeably. loadOrganisations() now follows that same proven path: register 'organisatie' as both the store key and the schema id against voorzieningenConfig.register (which IS reliably populated), with no dependency on organisatie_schema ever being set. Live-verified: the picker now lists all 17 organisations (4 Gemeente, 2 Samenwerking, suppliers) and selecting one successfully loads the TIME quadrant report. Frontend bundle rebuilt (npm run build) so the fix is live. --- src/views/organisaties/PortfolioReport.vue | 45 +++++++++++++++++----- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/src/views/organisaties/PortfolioReport.vue b/src/views/organisaties/PortfolioReport.vue index 9b1428dd..b904b27b 100644 --- a/src/views/organisaties/PortfolioReport.vue +++ b/src/views/organisaties/PortfolioReport.vue @@ -352,6 +352,33 @@ export default { /** * Load the organisation collection the picker depends on. + * + * `getSchemaConfig('organisatie')` cannot be used here: it only + * resolves a type from the `voorzieningenConfig` blob when that + * blob's `_schema` key holds a NUMERIC schema id, and + * `organisatie_schema` is unset on instances where the voorzieningen + * auto-configure flow never wrote it (it only ever populates + * `module`/`compliancy`/`moduleVersie`/`sbomComponent` — see + * `SettingsService::normalizeVoorzieningenConfig()`). That is exactly + * why the picker rendered "No results" despite the register holding + * real `Gemeente`/`Samenwerking`/supplier organisations: the schema + * never got registered, so `fetchCollection()` threw + * "Object type ... is not registered" before any request was even + * sent. + * + * The manifest-driven `Organisaties` index page (`src/manifest.json` + * `pages[].config` for route `/organisaties`) never hits this gap: the + * shared library's self-fetch path + * (`node_modules/@conduction/nextcloud-vue/src/components/CnIndexPage/useSelfFetchList.js`) + * calls `registerObjectType(type, props.schema, props.register, ...)` + * using the SCHEMA SLUG itself (`'organisatie'`) as the id — which + * OpenRegister's `/api/objects/{register}/{schemaSlugOrId}` accepts + * interchangeably with a numeric id — rather than resolving a numeric + * schema id from a config blob first. This mirrors that proven path: + * register the slug directly against the voorzieningen register id + * (which IS reliably populated — `voorzieningenConfig.register`), + * with no dependency on `organisatie_schema` ever being set. + * * @return {Promise} */ async loadOrganisations() { @@ -359,17 +386,17 @@ export default { if (!objectStore.settings && typeof objectStore.fetchSettings === 'function') { await objectStore.fetchSettings() } + const voorzieningenConfig = objectStore.settings?.voorzieningen + || objectStore.settings?.voorzieningenConfig + || {} + const registerId = voorzieningenConfig.register if (typeof objectStore.registerObjectType === 'function' + && registerId && !objectStore.objectTypeRegistry?.organisatie) { - let cfg = null - try { - cfg = objectStore.getSchemaConfig?.('organisatie') - } catch (cfgError) { - // getSchemaConfig throws when no schema/register resolves; fall through. - } - if (cfg?.register && cfg?.schema) { - objectStore.registerObjectType('organisatie', cfg.schema, cfg.register) - } + objectStore.registerObjectType('organisatie', 'organisatie', registerId, { + registerSlug: 'voorzieningen', + schemaSlug: 'organisatie', + }) } if (typeof objectStore.fetchCollection === 'function') { await objectStore.fetchCollection('organisatie', { _limit: 1000 }) From 88b81b6e2789f7790a66bfdf31344bf74fdcc1a3 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Fri, 24 Jul 2026 09:18:46 +0200 Subject: [PATCH 3/4] fix(sbom): translate DoesNotExistException to 404 in SbomController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/moduleversies/{uuid}/sbom for a non-existent moduleVersie uuid 500'd with an uncaught OCP\AppFramework\Db\DoesNotExistException ("Object with identifier '...' not found in any magic table"). Root cause: SbomImportService::getStatus() and ::importForModuleVersie() (via authorizeManage() -> resolveParentModuleUuid(), and via its own find() call) both call OpenRegister's real ObjectService::find(). Despite its ?ObjectEntity return type suggesting null on a miss, and despite the existing "if ($moduleVersie !== null)"/"if ($moduleVersie === null) throw RuntimeException" guards already written under that assumption, find()'s cross-table fallback lookup re-throws DoesNotExistException instead of returning null for a well-formed but unresolvable uuid. Both controller methods let that exception escape uncaught. Fixed by catching DoesNotExistException in both getSbomImportStatus() and importSbom() (the whole method body, since the exception can originate from either the authorization guard or the import call) and translating it to a 404 JSONResponse with error: MODULE_VERSION_NOT_FOUND — following the same try/catch-at-the-find()-boundary pattern already used elsewhere in the fleet (e.g. launchpad's DashboardMetadataController::loadDashboard()). Added two regression tests asserting 404 (not 500) for a non-existent moduleVersieUuid on both endpoints. Live-verified via authenticated fetch: the endpoint now returns 404 with {"message":"moduleVersie not found: ...", "error":"MODULE_VERSION_NOT_FOUND"}. --- lib/Controller/SbomController.php | 74 +++++++++++++++----- tests/Unit/Controller/SbomControllerTest.php | 65 +++++++++++++++++ 2 files changed, 123 insertions(+), 16 deletions(-) diff --git a/lib/Controller/SbomController.php b/lib/Controller/SbomController.php index 83686f33..e97a5f6e 100644 --- a/lib/Controller/SbomController.php +++ b/lib/Controller/SbomController.php @@ -41,6 +41,7 @@ use OCA\SoftwareCatalog\Exception\UnsupportedSbomFormatException; use OCA\SoftwareCatalog\Service\SbomImportService; use OCP\AppFramework\Controller; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; @@ -94,6 +95,21 @@ public function __construct( /** * Import an SBOM for a `moduleVersie`. Multipart upload only. * + * `DoesNotExistException` is caught here (not just the explicit + * `RuntimeException` "moduleVersie not found" guard already inside + * `SbomImportService::importForModuleVersie()`): OpenRegister's real + * `ObjectService::find()` does not reliably return `null` for a missing + * object the way the unit-test stub does — for a well-formed but + * non-existent uuid its cross-table fallback lookup re-throws + * `OCP\AppFramework\Db\DoesNotExistException` instead. That exception can + * originate from `authorizeManage()` (via + * `SbomImportService::resolveParentModuleUuid()`, for a non-admin/ + * manage-group caller) or from `importForModuleVersie()` itself, so the + * whole method body is covered by one outer try/catch rather than + * threading a guard through each call site individually. Uncaught, this + * escaped as a 500 (confirmed live for `GET .../sbom` on + * `getSbomImportStatus()` — the same defect class applies here). + * * @param string $moduleVersieUuid The target moduleVersie's uuid. * * @return JSONResponse The import result summary, or a 400/401/403/404/422/500. @@ -106,23 +122,28 @@ public function __construct( #[NoCSRFRequired] public function importSbom(string $moduleVersieUuid): JSONResponse { - $guard = $this->authorizeManage(moduleVersieUuid: $moduleVersieUuid); - if ($guard instanceof JSONResponse) { - return $guard; - } + try { + $guard = $this->authorizeManage(moduleVersieUuid: $moduleVersieUuid); + if ($guard instanceof JSONResponse) { + return $guard; + } - $validated = $this->validateUpload(moduleVersieUuid: $moduleVersieUuid); - if ($validated instanceof JSONResponse) { - return $validated; - } + $validated = $this->validateUpload(moduleVersieUuid: $moduleVersieUuid); + if ($validated instanceof JSONResponse) { + return $validated; + } - try { $result = $this->importService->importForModuleVersie( moduleVersieUuid: $moduleVersieUuid, rawJson: $validated['contents'], format: $validated['format'], fileName: $validated['fileName'] ); + } catch (DoesNotExistException $e) { + return new JSONResponse( + data: ['message' => 'moduleVersie not found: '.$moduleVersieUuid, 'error' => 'MODULE_VERSION_NOT_FOUND'], + statusCode: Http::STATUS_NOT_FOUND + ); } catch (UnsupportedSbomFormatException $e) { return new JSONResponse( data: ['message' => $e->getMessage(), 'error' => 'UNSUPPORTED_SBOM_FORMAT'], @@ -228,12 +249,26 @@ private function validateUpload(string $moduleVersieUuid): array|JSONResponse * Read SBOM import status/provenance for a `moduleVersie`, optionally * including a `progress-tracking` snapshot when `operationId` is given. * + * Confirmed live (500 on a non-existent `moduleVersieUuid`): + * `SbomImportService::getStatus()` calls OpenRegister's real + * `ObjectService::find()`, which — despite its `?ObjectEntity` return + * type suggesting `null` on a miss (and despite the local + * `if ($moduleVersie !== null)` guard already inside `getStatus()`) — + * can re-throw `OCP\AppFramework\Db\DoesNotExistException` from its + * cross-table fallback lookup for a well-formed but unresolvable uuid, + * rather than returning `null`. Uncaught, that propagated straight + * through this controller method as a 500. Caught here and translated + * to a proper 404 — the endpoint is a plain "read status for this uuid" + * lookup, so a missing `moduleVersie` is an ordinary not-found, not a + * server error. + * * @param string $moduleVersieUuid The target moduleVersie's uuid. * - * @return JSONResponse `{sbomLastImportedAt, sbomFormat, sbomFileName, progress}`. + * @return JSONResponse `{sbomLastImportedAt, sbomFormat, sbomFileName, progress}`, or a 404. * * @NoAdminRequired * @spec openspec/specs/sbom-import/spec.md#requirement-large-imports-run-in-bounded-batches-with-progress-reporting + * @spec openspec/specs/sbom-import/spec.md#requirement-moduleversie-records-sbom-import-provenance */ #[NoAdminRequired] public function getSbomImportStatus(string $moduleVersieUuid): JSONResponse @@ -247,12 +282,19 @@ public function getSbomImportStatus(string $moduleVersieUuid): JSONResponse $operationId = null; } - return new JSONResponse( - data: $this->importService->getStatus( - moduleVersieUuid: $moduleVersieUuid, - operationId: $operationId - ) - ); + try { + return new JSONResponse( + data: $this->importService->getStatus( + moduleVersieUuid: $moduleVersieUuid, + operationId: $operationId + ) + ); + } catch (DoesNotExistException $e) { + return new JSONResponse( + data: ['message' => 'moduleVersie not found: '.$moduleVersieUuid, 'error' => 'MODULE_VERSION_NOT_FOUND'], + statusCode: Http::STATUS_NOT_FOUND + ); + } }//end getSbomImportStatus() /** diff --git a/tests/Unit/Controller/SbomControllerTest.php b/tests/Unit/Controller/SbomControllerTest.php index f53281fb..b2284af5 100644 --- a/tests/Unit/Controller/SbomControllerTest.php +++ b/tests/Unit/Controller/SbomControllerTest.php @@ -27,6 +27,7 @@ use OCA\SoftwareCatalog\Controller\SbomController; use OCA\SoftwareCatalog\Service\SbomImportService; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\IGroupManager; use OCP\IRequest; @@ -306,4 +307,68 @@ public function testStatusRefusesUnauthenticated(): void $this->assertSame(Http::STATUS_UNAUTHORIZED, $response->getStatus()); }//end testStatusRefusesUnauthenticated() + + /** + * REGRESSION: a non-existent `moduleVersieUuid` returns 404, not 500. + * + * Confirmed live: `GET /apps/softwarecatalog/api/moduleversies/{uuid}/sbom` + * for a well-formed but non-existent uuid 500'd with an uncaught + * `OCP\AppFramework\Db\DoesNotExistException` ("Object with identifier + * '...' not found in any magic table") — OpenRegister's real + * `ObjectService::find()` re-throws instead of returning `null` for this + * shape, unlike the assumption `SbomImportService::getStatus()`'s own + * `if ($moduleVersie !== null)` guard was written under. + * + * @return void + */ + public function testStatusReturns404ForNonExistentModuleVersie(): void + { + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturn(null); + $this->userSession = $this->createMock(IUserSession::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->importService = $this->createMock(SbomImportService::class); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('caller-uid'); + $this->userSession->method('getUser')->willReturn($user); + + $this->importService->method('getStatus') + ->willThrowException(new DoesNotExistException("Object with identifier '00000000-0000-0000-0000-000000000000' not found in any magic table")); + + $controller = new SbomController( + $request, + $this->userSession, + $this->groupManager, + $this->importService, + $this->createMock(LoggerInterface::class) + ); + + $response = $controller->getSbomImportStatus('00000000-0000-0000-0000-000000000000'); + + $this->assertSame(Http::STATUS_NOT_FOUND, $response->getStatus()); + $this->assertSame('MODULE_VERSION_NOT_FOUND', $response->getData()['error'] ?? null); + }//end testStatusReturns404ForNonExistentModuleVersie() + + /** + * REGRESSION: `importSbom()` also translates a `DoesNotExistException` + * escaping `SbomImportService::importForModuleVersie()` (same underlying + * OpenRegister `find()` behaviour as `testStatusReturns404ForNonExistentModuleVersie()`) + * to 404, not 500. + * + * @return void + */ + public function testImportReturns404ForNonExistentModuleVersie(): void + { + $upload = $this->uploadedFile('{"bomFormat":"CycloneDX","specVersion":"1.6","components":[]}'); + + $controller = $this->makeController(isAdmin: true, memberGroups: [], uploadedFile: $upload); + $this->importService->method('importForModuleVersie') + ->willThrowException(new DoesNotExistException("Object with identifier '00000000-0000-0000-0000-000000000000' not found in any magic table")); + + $response = $controller->importSbom('00000000-0000-0000-0000-000000000000'); + + $this->assertSame(Http::STATUS_NOT_FOUND, $response->getStatus()); + $this->assertSame('MODULE_VERSION_NOT_FOUND', $response->getData()['error'] ?? null); + }//end testImportReturns404ForNonExistentModuleVersie() }//end class From 330a821eb07f0838ad57fd26246bd97f070a4362 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Fri, 24 Jul 2026 09:20:43 +0200 Subject: [PATCH 4/4] fix(routes): postfix the SPA catch-all so the app root stops 404ing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dashboard#page was registered twice (bare '/' and the '/{path}' SPA catch-all). Both generate the same internal route name, so the catch-all silently displaced the bare-root route — and its own path requirement ('.+') can never match an empty path, so /apps/softwarecatalog/ 404'd for every user, breaking the Nextcloud app-switcher entry point. Found by live testing on 8080; verified fixed (root 200, sub-paths 200). --- appinfo/routes.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index e2e20d98..21eb6272 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -273,6 +273,13 @@ ['name' => 'portfolioReport#index', 'url' => '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/api/portfolio-report', 'verb' => 'GET'], // SPA catch-all — serves the Vue app for any frontend route (history mode routing) - ['name' => 'dashboard#page', 'url' => '/{path}', 'verb' => 'GET', 'requirements' => ['path' => '.+'], 'defaults' => ['path' => '']], + // `postfix` keeps this SPA catch-all from colliding with the bare-root + // `dashboard#page` route above: both entries target the same + // controller#method, so without a postfix they generate the same + // internal route name and the later one silently displaces the first — + // which 404'd the app's own entry point (`/apps/softwarecatalog/`) for + // every user, because this route's `path` requirement ('.+') can never + // match an empty path. + ['name' => 'dashboard#page', 'url' => '/{path}', 'verb' => 'GET', 'requirements' => ['path' => '.+'], 'defaults' => ['path' => ''], 'postfix' => 'spa'], ], ];