Skip to content

perf(events): declare OrderCustomerListener's register/schema interest - #3

Merged
rubenvdlinde merged 2 commits into
mainfrom
perf/filtered-event-subscription
Aug 1, 2026
Merged

perf(events): declare OrderCustomerListener's register/schema interest#3
rubenvdlinde merged 2 commits into
mainfrom
perf/filtered-event-subscription

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What

OrderCustomerListener stamps order.customer from the session, and only for petstore/order objects. But ObjectCreatingEvent is dispatched from OpenRegister's central write path for every object on the instance, so every create constructed the listener and ran the two mapper lookups (SchemaMapper::find() + RegisterMapper::find()) that isPetstoreOrder() needs before rejecting the object.

It now declares register petstore / schema order through OpenRegister's ObjectEventSubscription.

Evidence for the declaration

private const REGISTER_SLUG = 'petstore';
private const SCHEMA_SLUG   = 'order';

private function isPetstoreOrder(object $entity): bool
{
    …
    return ($schemaSlug === self::SCHEMA_SLUG && $registerSlug === self::REGISTER_SLUG);
}

handle() returns immediately when isPetstoreOrder() is false — nothing else in the listener runs. Both values are compile-time constants, not config.

ObjectEventSubscription ANDs the two filters, which is exactly the listener's own &&.

Slug verification (dev instance)

oc_openregister_registers slug=petstore -> id 212, schemas [427, 428]  (category, pet)
oc_openregister_schemas   slug=order    -> id 1585   (member of register 18 = decidesk)

Both slugs resolve, but ⚠️ the combination does not exist on the dev instance: the installed petstore register carries only category and pet; the only order schema belongs to decidesk. lib/Settings/petstore_register.json does ship an order schema ("slug": "order"), so this is a stale/partial register import on the dev instance rather than a code problem.

This makes the change behaviour-neutral either way: the current isPetstoreOrder() guard requires the same register+schema combination, so the listener body cannot run on this instance today, and it cannot after this change. On an instance with a fully imported petstore register both the guard and the declaration match the same objects.

⚠️ petstore is not bind-mounted from this working tree, so this was not exercised against the running container.

Safety

  • Guarded on class_exists('\OCA\OpenRegister\Event\ObjectEventSubscription') — openregister#2223 is not merged, so on an instance without it this degrades to the exact registerEventListener() call it replaced. No hard cross-app reference.
  • ObjectCreatingEvent carries getObject(), so OpenRegister's proxy can identify the object and the filter is real (not fail-open), and the proxy passes the original event object through, so setModifiedData() still reaches OR's write path.
  • isPetstoreOrder() is left in place as defence in depth.

Branch note

petstore has no development branch — this PR targets main, the repository default.

Checks

  • php -l: clean
  • phpcs --standard=phpcs.xml lib/AppInfo/Application.php: clean, 0 errors 0 warnings, same as baseline
  • phpstan analyse lib/AppInfo/Application.php: No errors
  • psalm lib/AppInfo/Application.php: 4 UndefinedClass errors before and after (autoloader artefacts of analysing a single file — petstore has no local vendor/, so a sibling app's vendor was mounted); no delta
  • phpmd lib/AppInfo/Application.php text phpmd.xml: no findings
  • Full composer check:strict not run — petstore has no installed vendor/ in this working tree; the checks above were run in a nextcloud:34 container with a sibling app's toolchain and petstore's own phpcs.xml / phpmd.xml / phpstan.neon / custom sniffs.

OrderCustomerListener stamps `order.customer` only for petstore/order
objects (its own REGISTER_SLUG / SCHEMA_SLUG), but ObjectCreatingEvent is
dispatched from OpenRegister's central write path for every object on the
instance — so every create constructed the listener and ran the two mapper
lookups isPetstoreOrder() needs before rejecting the object.

Declare petstore/order through OpenRegister's ObjectEventSubscription,
guarded on class_exists so an instance whose OpenRegister predates the
mechanism falls back to the global registration it replaced. The
in-listener guard stays in place as defence in depth.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/petstore @ 8235c0d

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm ✅ 612/612
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-07-31 12:00 UTC

Download the full PDF report from the workflow artifacts.

…-independent

Declaring the filtered subscription in register() was boot-order sensitive:
Nextcloud enables each app's autoloader immediately before calling that app's
own register(), so OpenRegister's ObjectEventSubscription was only autoloadable
to apps registering after it. The class_exists() guard then failed silently and
the app fell back to an unfiltered registration that looked identical to a
working narrowing.

boot() runs only after every app's register() has completed, so the guard
resolves regardless of this app's position. The fallback now logs a warning
naming the app and listener instead of degrading silently.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/petstore @ 06bf125

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm ✅ 612/612
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-07-31 21:17 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 09d3f56 into main Aug 1, 2026
21 of 26 checks passed
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