fix(controllers): accept string UUIDs on /sources/test, /jobs/run|test, /synchronizations/{id}/run|test (closes #839)#840
Closed
rubenvdlinde wants to merge 3 commits into
Conversation
…t, /synchronizations/{id}/run|test
The chain-B/C OR cutover replaced auto-increment integer ids with UUIDs,
but five action-controller methods kept `int $id` parameters:
SourcesController::test lib/Controller/SourcesController.php:225
JobsController::run lib/Controller/JobsController.php:181
JobsController::test lib/Controller/JobsController.php:225
SynchronizationsController::test lib/Controller/SynchronizationsController.php:228
SynchronizationsController::run lib/Controller/SynchronizationsController.php:277
PHP coerces the inbound UUID string ("77637bae-01ec-4909-9f47-f2236fe7c3b1")
to int by taking the leading digits → 77. ObjectService::find(id: "77",
schema: '<schema>') then raises DoesNotExistException → the catch returns
404 Not Found. Every action-button click in the UI + every Newman
fixture POST to /sources/test/{uuid} silently 404s with no log written.
This is the root cause of the empty openconnector dashboard reported in
#838 — the journey tests "pass" because they
only assert response status, not semantic success, but the underlying
CallService / JobService / SynchronizationService never actually runs.
Switch `int $id` → `string $id` on all 5 methods. Drop the now-redundant
`(string) $id` cast at the `find()` call site (the param is already
a string). Update docblocks (`@param int $id` → `@param string $id ...UUID...`).
Discovered while investigating an empty openconnector dashboard.
Companion bugs filed at OR side:
- ConductionNL/openregister#1614 — honor x-openregister-archival annotation
- ConductionNL/openregister#1615 — eager magic-table creation on schema import
Closes #839.
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
…`sourceId` integer The openconnector_register.json descriptor declares `call_log.sourceId` as a legacy integer FK kept "during transition per chain-A REQ-A-008 (cleanup tracked at #821)", and a parallel `source` property typed `string format=uuid` ($ref to source schema) as the post-chain-B canonical reference. `CallService::call()` (and its 3 short-circuit paths for disabled source / missing location / rate-limited) write `'sourceId' => $source->getUuid()` — a UUID string being shoved into the legacy integer column. Postgres rejects: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for type integer: "77637bae-01ec-4909-9f47-f2236fe7c3b1" Switch all four `'sourceId'` keys to `'source'` so the UUID lands in the correct column. The legacy integer column stays empty (NULL) — #821 tracks its removal once nothing reads it anymore. ## Verified end-to-end on the local container (After ConductionNL/openregister PR #1616 lands the eager magic-table creation that materialises `oc_openregister_table_65_223`:) TRUNCATE oc_openregister_table_65_223; // PHP CLI: CallService->call(source: $src, endpoint: '/get'); → row inserted with `source = '77637bae-...'`, status_code = 200 GET /api/objects/openregister/api/objects/openconnector/call_log → returns the row (after a manual UPDATE _owner='admin' workaround pending ConductionNL/openregister#1617 — system-context attribution) Same fix needed for the JobService / SynchronizationService write paths (jobId / synchronizationId → job / synchronization) — filed separately as #841 since those services have additional complexity (event_message also has the integer-FK problem on eventId / consumerId / subscriptionId). Companion changes shipping together in this PR: - 5 action-controllers `int $id` → `string $id` (the original commit) - This CallService field-name fix Both are "chain-B/C cutover type drift" bugs.
…ns' into fix/action-controllers-uuid-param-type
rubenvdlinde
added a commit
to ConductionNL/openregister
that referenced
this pull request
May 22, 2026
… (#1616) `ImportHandler::importFromApp` previously called `MagicMapper::ensureTableForRegisterSchema` only from inside the seed-objects loop (line 3187, formerly 3174). Schemas with an empty seed-objects array therefore never got their per-schema magic table provisioned during register import. The fallback was lazy: the first write against the schema would trigger `ensureTableForRegisterSchema` inside `MagicMapper::saveObject`. For entity-style schemas (source, job, mapping, …) users write to them directly so the table appears on first use. For **log-style schemas** (call_log, job_log, synchronization_log, synchronization_contract_log in ConductionNL/openconnector — and similar in other apps) writes only happen from internal services. If anything earlier in the chain swallowed an error (controller validation, find lookup, etc.), the service never reached saveObject and the table stayed missing forever. In practice: openconnector dashboards rendered empty even after thousands of Newman + Playwright invocations because every `POST /sources/test/{id}` 404'd at the controller (separate bug, ConductionNL/openconnector#840), AND if it ever reached CallService the saveObject would have thrown `relation oc_openregister_table_65_223 does not exist`. ## Fix Provision the magic table for **every** Schema bound to the Register inside `autoCreateRegisterIfApplication()`, right after the Register is created / reconciled and its Configuration is synced. The loop iterates the schemas already in scope, is idempotent (the underlying helper short-circuits when the table already exists per MagicTableHandler line 109-112), and non-fatal (any per-schema failure logs a warning instead of aborting the whole import). The original seed-objects-loop pre-create at line 3225 stays in place as a defensive no-op for back-compat. ## Verified end-to-end on a chain-E openconnector instance occ maintenance:repair --include-expensive → OpenConnector Repair step fires importFromApp → autoCreateRegisterIfApplication runs the new loop → All 15 openconnector schemas now have magic tables: source / job / mapping / synchronization (entity) call_log / job_log / synchronization_log / synchronization_contract_log (log — previously missing) … plus 7 others. CallService->call() then succeeds and writes to call_log.
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.
Summary
Closes #839. The chain-B/C OR cutover swapped auto-increment integer ids for UUIDs, but five action-controller methods kept
int $idparameters:SourcesController::testJobsController::runJobsController::testSynchronizationsController::testSynchronizationsController::runPHP coerces the inbound UUID string (
"77637bae-01ec-4909-9f47-f2236fe7c3b1") to int by taking the leading digits →77.ObjectService::find(id: "77", schema: '<schema>')then raisesDoesNotExistException→ the controller's try/catch returns404 Not Found.Why this matters
This is the root cause of the empty openconnector dashboard reported on #838. The Newman/Playwright journey tests "pass" because they only assert response shape (Newman scripts check 200/4xx, not semantic success), but the underlying
CallService::call(),JobService::executeJob(),SynchronizationService::synchronize()never actually runs — so no call_log / job_log / synchronization_log row gets written, and the dashboard charts have nothing to plot even after thousands of test invocations.Every Run-now / Test-connection button click from the action-row-handlers we just added in #830 was also silently 404'ing for the same reason.
Fix
Five
int $id→string $idparameter changes + drop the now-redundant(string) $idcast at thefind()call site + update the@param int $iddocblocks to@param string $id ...UUID.... No behavioural changes elsewhere.Related upstream gaps
This patch makes the controllers reach
saveObjectagain, but two OR-side issues will still block log persistence end-to-end:oc_openregister_table_65_223(call_log) magic table doesn't exist on this instance because OR only lazy-creates magic tables for schemas with seed data. CallService writes will throwrelation does not existuntil that lands.x-openregister-archivalretention annotation declared in the openconnector descriptor is silently stripped (vocabulary gap). Logs aren't immutable and don't get retention-swept.Filed as separate issues; this PR only addresses the openconnector-internal typing bug.
Test plan
https://httpbin.org, POST/api/sources/test/{uuid}→ returns the actual HTTP test result (not 404)oc_openregister_table_65_223and is visible viaGET /api/objects/openregister/api/objects/openconnector/call_log