fix(events): event_message UUID links (post-cutover) — fixes pull/messages 500#991
Open
rubenvdlinde wants to merge 5 commits into
Open
fix(events): event_message UUID links (post-cutover) — fixes pull/messages 500#991rubenvdlinde wants to merge 5 commits into
rubenvdlinde wants to merge 5 commits into
Conversation
…fixes 500 on pull/messages
The event_message schema carries BOTH the legacy integer FK columns
(eventId/consumerId/subscriptionId) and the post-cutover UUID reference
fields (event/consumer/subscription). The OR migrator correctly populates
the UUID fields, but the runtime code still used the integer FK fields:
- createEventMessage() wrote the event/subscription UUIDs INTO the integer
columns;
- pullEvents() + EventsController::{subscriptionMessages,messages} FILTERED
the integer columns with a UUID.
Postgres rejects a UUID in an integer column
(`invalid input syntax for type integer`), so GET
/events/subscriptions/{id}/pull and /messages returned 500 (3 Newman
assertion failures). Switched all four sites to the UUID fields
(event/subscription/consumer). Newman: 94/94 assertions pass; the
regression E2E passes.
Note: the vestigial integer FK columns are now unused for new data — a
schema-hygiene cleanup (dropping them) is a separate follow-up.
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
May 26, 2026 07:35
calculateExpires() returns a ?\DateTime. Every call site formats it with
?->format('c') before saving to OpenRegister — except the error/test
sync-log data at line ~1056, which passed the raw DateTime. OR's
`expires` property is string|null, so saving an unformatted DateTime
threw a schema-validation error that escaped the controller's
`catch (Exception)` (it is not an \Exception), producing a corrupt HTTP
response — POST /synchronizations/{id}/test failed in CI with
"Parse Error: Invalid status code" (2 Newman assertions). Now formatted
like the others; the dry-run returns a clean response.
Contributor
Quality Report — ConductionNL/openconnector @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 148/148 | |||
| npm | ✅ | ✅ 581/581 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ❌ | ||||
| Playwright | ❌ |
Quality workflow — 2026-05-26 07:50 UTC
Download the full PDF report from the workflow artifacts.
The synchronization_log and synchronization_contract_log schemas are
immutable/append-only in OpenRegister; the engine's legacy create-then-update
log pattern was rejected with 405 SCHEMA_APPEND_ONLY, breaking every sync run
and POST /synchronizations/{id}/test. Refactor the engine to build each log
fully in memory and persist it exactly once (append), correlating child
contract-logs to their parent via a generated executionId field rather than
the parent's OR-assigned id. Adds executionId to the two sync log schemas.
Verified locally: Newman 94/94; bogus-sync /test returns 400 (was 405).
Spec: openspec/changes/openconnector-write-once-immutable-logging.
- manifest-pages: add 'roadmap' to the allowed page types (FeaturesRoadmap), sync MANIFEST_PAGES to the current 25 pages, drop the removed Import page. - manifest-pages console gate: replace the blanket 'ignore all 500/503' with URL-scoped filtering (NC-core user_status, OR graphql under php -S, removed /api/settings, __nonexistent__ sentinel) so a real openconnector endpoint 5xx still fails the gate; surface the failing URL in error messages. - synced-from-leaf: make the beforeAll seed robust (skip cleanly on failure rather than cascade) and dispose the API context on the early-return path. Verified locally: manifest-pages 27/27, synced-from-leaf 3/3.
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.
The bug (post-migration fallout)
GET /events/subscriptions/{id}/pulland/messagesreturned 500, failing 3 Newman assertions (the "Integration Tests (Newman)" CI check).Root cause: the
event_messageschema carries both the legacy integer FK columns (eventId/consumerId/subscriptionId) and the post-cutover UUID reference fields (event/consumer/subscription). The OR migrator correctly populates the UUID fields (it maps legacysubscription_id→subscription), but the runtime code still used the integer fields:EventService::createEventMessage()wrote the event/subscription UUIDs into the integer columns;EventService::pullEvents()andEventsController::{subscriptionMessages,messages}filtered the integer columns with a UUID.Postgres rejects a UUID in an integer column (
invalid input syntax for type integer: "868d34ce-…") → 500.Fix
Switched all four sites to the UUID reference fields (
event/subscription/consumer). This is purely a post-cutover follow-through — the migrator already moved to these fields; the service/controller hadn't.Verification
Against a fresh post-cutover instance (cutover migrations applied):
pull/messagesfor a real subscription: 500 → 200.Audited the rest
Checked every other
*Id => getUuid()write:synchronization_log/synchronization_contract/job_loguse string FK fields (fine), andcall_logwrites use the stringsourcefield (fine).event_messagewas the only schema with the integer-FK/UUID mismatch.The now-unused integer FK columns (
event_message.{eventId,consumerId,subscriptionId},call_log.{sourceId,actionId,synchronizationId}) are harmless for new data; dropping them is a separate schema-hygiene follow-up.🤖 Generated with Claude Code