Skip to content

fix(events): resolve the schema slug listeners compare against — 3 procest listeners had never run - #692

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/object-event-schema-slug-resolution
Aug 1, 2026
Merged

fix(events): resolve the schema slug listeners compare against — 3 procest listeners had never run#692
rubenvdlinde merged 1 commit into
developmentfrom
fix/object-event-schema-slug-resolution

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Fixes part of #690 (the issue is filed on procest but the bug class is fleet-wide).

The bug

OpenRegister object events carry the schema as an id, never a slug:

  • ObjectEntity::jsonSerialize() builds @self from getObjectArray(), which sets 'schema' => $this->schema (lib/Db/ObjectEntity.php:947).
  • $this->schema is written by SaveObject as setSchema((string) $schemaId).
  • There is no schemaSlug key on @self, and there never has been.

Three procest listeners resolved the slug by reading @self.schemaSlug (absent), then falling back to @self.schema (an id). Their strict slug comparisons therefore could never match, and their handler bodies had never executed once. Green-but-dead: no exception, no log, no failing test — while still paying full DI construction and invocation on every matching object write.

Listener Guard that never matched Consequence
BezwaarLifecycleListener in_array($slug, ['bezwaar','objection','hearingSession','advisoryReport','decision'], true) observability logging never emitted
BezwaarLegalHoldListener in_array($slug, ['objection','bezwaar'] / ['bezwaarDecision','appealDecision'], true) no Awb legal hold has ever been placed — cases under bezwaar/beroep have been eligible for OR retention/destruction the whole time
TermijnCaseCreatedListener $slug !== 'case' no AWB TermijnInstance has ever been auto-bound — termijn/dwangsom tracking never started

The fix

One shared ObjectSchemaSlugResolver (lib/Service/ObjectSchemaSlugResolver.php) rather than three per-listener variants. It turns the id the payload actually carries into the slug the handlers are written against, via OpenRegister's SchemaMapper::find() (which keeps a request-scoped cache), memoising hits and misses for the request.

OpenRegister is reached through the container rather than constructor-injected, matching SettingsService — procest carries no hard dependency on OpenRegister and still boots without it. An unresolvable schema yields '', which matches no literal, so the previous fail-closed behaviour is preserved.

What is NOT affected

The other ten procest listeners are fine and are deliberately untouched. They compare @self.schema against a *_schema app-config value, and SettingsService::reconcileSingleSchemaKey() resolves slug→id and stores (string) $schema->getId() — so they were already comparing id against id. Verified live: occ config:app:get procest bezwaar_schema116, which is exactly what @self.schema carries.

A naive "count the slug comparisons" scan reports 12 dead listeners in procest. The real number is 3.

Positive control (live, on the shared dev instance)

A negative-only control passes against a listener that does nothing at all, so both directions were measured on a real API write of a bezwaar object (register procest=17, schema bezwaar=116).

Before — proxy invoked the listener, body did nothing:

proxy subscriptions=5 dispatches=4 invoked=2 skipped=4 listenerUs=118.1
grep -c 'bezwaar-lifecycle' nextcloud.log  ->  0

The same request wrote 270 "level":0 (debug) entries, so the log channel itself was demonstrably working — the zero is the listener, not the instrument.

After — same write, same instance:

proxy subscriptions=5 dispatches=4 invoked=2 skipped=4 listenerUs=4624.5
{"app":"procest","message":"Procest bezwaar-lifecycle: observed bezwaar OCA\\OpenRegister\\Event\\ObjectCreatedEvent",
 "data":{"schema":"bezwaar","objectId":"f8288a96-...","caseId":"f9013ce5-...","status":"In behandeling"}}

"schema":"bezwaar" is the resolver turning id 116 into the slug. The independent corroboration is listenerUs 118 us -> 4624 us: the handlers are now doing real work instead of returning at the first guard. Host load average at measurement: 32.47.

Behaviour change — read before merging

These listeners have never run. Waking them is a behaviour change, not a no-op.

  • BezwaarLifecycleListener — safe. Its body is a single logger->debug(); no side effects.
  • TermijnCaseCreatedListener — writes objects. Creates an AWB TermijnInstance per new case. Correct intended behaviour, but it will start writing on every case create.
  • BezwaarLegalHoldListener — writes OpenRegister legal holds. Suppresses retention/destruction for cases under an Awb procedure. Correct and compliance-relevant, but it changes archival behaviour.

Honest limits of this verification

  • BezwaarLifecycleListener: verified end-to-end.
  • BezwaarLegalHoldListener: comparison fixed, end-to-end UNVERIFIED. After the fix a bezwaar create still produced no legal hold on the linked case. The guard is proven fixed (same resolver, same payload, same request as the verified listener), but applyHold() returns silently when resolveOr() or resolveCaseObject() yields null — resolveCaseObject() calls ObjectEntityMapper::findByUuid() with an unscoped UUID. That looks like a second, independent defect downstream of the guard and is deliberately not fixed here.
  • TermijnCaseCreatedListener: comparison fixed, end-to-end UNVERIFIED. A case create (schema 92, slug case) produced neither a TermijnInstance nor the catch branch's debug line, so something downstream of the guard is also not working. Not chased here.
  • Three slug literals cannot be valid on any instance. hearingSession and advisoryReport (BezwaarLifecycleListener) and bezwaarDecision / appealDecision (BezwaarLegalHoldListener) are camelCase; OpenRegister slugs are lower-kebab (assessment-result, grade-entry). None of these four schemas exists on the dev instance, so the comparison cannot be exercised and the literals were left alone rather than guessed at. These branches remain dead after this PR.
  • phpcs is clean on all four files. The three remaining warnings are the pre-existing "missing @spec PHPDoc tag" class-level warnings; no @spec value was invented for them.

OpenRegister object events carry the schema as an ID, never a slug.
ObjectEntity::jsonSerialize() builds @self from getObjectArray(), which
sets 'schema' => $this->schema, and SaveObject writes that field as
setSchema((string) $schemaId). There is no 'schemaSlug' key on @self and
there never has been.

Three listeners resolved the slug by reading @self.schemaSlug (absent),
then @self.schema (an id), so their strict slug comparisons could not
match and their handler bodies had never executed once — no exception,
no log, no failing test.

Adds ObjectSchemaSlugResolver: one shared, request-memoised id->slug
lookup through OpenRegister's SchemaMapper (which caches per request),
resolved via the container so procest still boots without OpenRegister.
An unresolvable schema yields '', which matches no literal, so the
previous fail-closed behaviour is preserved.

The other ten procest listeners are NOT affected: they compare
@self.schema against a *_schema app-config value, and SettingsService
stores those as ids, so they were already comparing id against id.
@rubenvdlinde
rubenvdlinde merged commit 6ff599b into development Aug 1, 2026
4 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/object-event-schema-slug-resolution branch August 1, 2026 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant