fix(scope): clear the pending schema ref on the save path too - #2803
Merged
rubenvdlinde merged 2 commits intoAug 22, 2026
Conversation
added 2 commits
August 22, 2026 17:05
#2792 made the pending ref single-use and cleared it in find(). saveObject() and patchObject() do not go through find() — they call setContextFromParameters(), which was left as it was. So the leak is still live on the write path, which is where it is most visible. MEASURED on buildiq's E2E, 2026-08-22 12:26 UTC (after #2792 landed at 11:09): POST /index.php/apps/docudesk/api/templates Failed to create template: Schema slug "application" is not carried by register "docudesk" (id 19), which carries 9 schema(s). 1 schema(s) elsewhere on this instance carry this slug; none of them is served here, because naming a register makes it a boundary. Docudesk asked for register 19 / schema 18 (`template`) — the seed writes that pair and verifies it on read-back immediately before. `application` is openbuild's schema, left pending by an unrelated earlier call on the same shared ObjectService instance. setRegister() then consumed it and re-resolved it inside docudesk's register, which correctly refused a schema that register does not carry. The refusal is right; the ref should never have been there. find() already guards this, with the same reasoning in its own comment. The fix belongs in the shared helper rather than a second copy at the third call site: when a caller supplies BOTH register and schema, it has named the pair outright, so any pending ref is stale by definition and is dropped before resolution. Order-independence is untouched — a caller that genuinely does setSchema($s)->setRegister($r) still re-resolves — and the boundary is exactly as strict, because the schema is still resolved inside the register named here. Two failing fleet E2E suites trace to this: buildiq's template create (above) and integriq's flow halting immediately after its `write` step.
find() already has a regression test for this leak. saveObject() and
patchObject() do not go through find(), so nothing covered the path that was
still broken.
Drives setContextFromParameters() by reflection rather than saveObject(): the
leak lives entirely in context resolution, and the rest of a save needs a dozen
more collaborators that would obscure what is being asserted.
Paired control, against the parent commit's lib/:
without the fix SchemaNotInRegisterException: Schema slug "application" is
not carried by register "docudesk" (id 19) …
with the fix OK (2 tests, 2 assertions)
That exception is the production message verbatim, from buildiq's E2E on
2026-08-22 12:26 UTC.
The wider tests/Unit/Service/ suite reports 8 errors both WITH and WITHOUT this
change (LeafProvidersMetadataTest, DbalObjectSourceProviderTest,
TextExtractionServiceTest) — pre-existing, measured both directions rather than
assumed.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| composer | ✅ | ✅ 175/175 | |||
| npm | ✅ | ✅ 528/528 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-22 15:55 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 22, 2026
#2803 landed mid-flight — a FOURTH pass at this one mechanism, found from the other end (a docudesk template create failing with openbuild's `application` slug). It guards `setContextFromParameters()`, the path a caller naming BOTH register and schema takes. This branch guards the seven public entry points. The two are complementary, not competing: #2803 covers the "names both" case wherever it is reached from, this covers every operation that supplies its own scope at all. Both test sets are kept. They are different regressions from different apps against the same root cause, and either would have caught only its own.
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.
#2792made the pending schema ref single-use and cleared it infind().saveObject()andpatchObject()do not go throughfind()— they callsetContextFromParameters(), which was left as it was. So the leak is still live on the write path, which is where it is most visible.Measured on
development, after #2792buildiq E2E, 2026-08-22 12:26 UTC (#2792 landed 11:09):
Docudesk asked for register 19 / schema 18 (
template) — its seed writes that pair and verifies it on read-back immediately before.applicationis openbuild's schema, left pending by an unrelated earlier call on the same sharedObjectServiceinstance.setRegister()consumed it and re-resolved it inside docudesk's register, which correctly refused a schema that register does not carry.The refusal is right. The ref should never have been there.
The fix
When a caller supplies both register and schema it has named the pair outright, so any pending ref is stale by definition and is dropped before resolution — in the shared helper, rather than as a second copy of
find()'s guard at the third call site.setSchema($s)->setRegister($r)still re-resolves.Verification
SchemaNotInRegisterException … "application" … register "docudesk" (id 19)— the production message verbatimOK (2 tests, 2 assertions)tests/Unit/Service/[OK] No errors/ 0 errors / cleanFleet impact
Two failing E2E suites trace here: buildiq's template create (above) and integriq's flow halting immediately after its
writestep. Both are ondevelopmentnow.Full diagnosis thread, including two wrong attributions of mine that this replaces, is on #2774. The server log is available on failing E2E runs as the
nextcloud-log-e2eartifact (ConductionNL/.github#554) — that capture is what made this findable.