fix: four production defects found by live testing the market-gap wave - #392
Merged
Merged
Conversation
…t was dead in production 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.
…as dead
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.
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"}.
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).
This was referenced Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Live-tested the merged 2026-07-23 market-gap wave on a real NC 34 instance (8080). All 396 unit tests were green, yet four defects were dead in production. All four are fixed and verified live.
1.
GET /api/facets/module→ 500 (endpoint 100% dead)FacetServicehelpers arearray-typed, but OpenRegister'ssearchObjectsPaginated()/searchObjects()returnObjectEntityinstances. The test stub returned plain arrays — textbook test-fake drift.Fix:
normalizeObject()at both fetch boundaries (fetchBaseObjects(),fetchModulesByIdentifiers()), preferringjsonSerialize()(confirmed againstObjectEntity, which merges payload +@self+ top-levelid).Regression tests added that feed ObjectEntity-shaped results for both the
moduleanddienstpaths — these would have caught it. Array-shaped tests kept.Verified live: 200,
totalMatched: 18, 35ms.Audited the sibling wave services (PortfolioReport, SbomImport, EolSync, MergeOrganisatie) — none shared the bug.
2. Portfolio report organisation picker dead (page unusable)
loadOrganisations()resolved the schema viagetSchemaConfig('organisatie'), butvoorzieningen_config.organisatie_schemais never populated, soregisterObjectType()was skipped andfetchCollection()threw before any request.Fix: register by schema slug against
voorzieningenConfig.register, mirroring the proven path inuseSelfFetchList.js(OpenRegister accepts slug or numeric id).Verified live: picker lists all 17 organisations; selecting one renders the TIME report.
3. SBOM endpoint 500 instead of 404
DoesNotExistExceptionfrom OpenRegister'sfind()escapedSbomControlleruncaught.Fix: translated to a proper 404 (
MODULE_VERSION_NOT_FOUND) on both endpoints, following the fleet convention. Tests added.4. App root 404'd — Nextcloud app-switcher entry broken for every user
dashboard#pagewas registered twice (bare/and the/{path}SPA catch-all). Both generate the same internal route name, so the catch-all displaced the root route — and its.+requirement can never match an empty path.Fix:
postfixon the catch-all. Verified live: root 200, sub-paths 200.Tests: 400 tests / 1264 assertions, 0 failures from these changes (run inside the NC container — the OCP symlink only resolves there). One pre-existing unrelated error:
PortfolioReportControllerTest::testCsvFormatReturnsDownloadResponse(SymfonyHeaderUtilsabsent in the test env; CSV export verified working live — filing separately).Also filed from this session: #391 (occ upgrade doesn't apply register schema changes — systemic), #390 (contract schema bare grants), and a priority escalation on #379.
🤖 Generated with Claude Code