This repository was archived by the owner on May 29, 2026. It is now read-only.
feat(openspec): runtime-schema-api — runtime CRUD on schemas + registers#1464
Merged
Conversation
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ❌ 1/153 denied | |||
| npm | ✅ | ✅ 598/598 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
❌ Denied composer licenses
| Package | Version | License |
|---|---|---|
| dompdf/dompdf | v3.1.5 | LGPL-2.1 |
Quality workflow — 2026-05-11 10:16 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
May 11, 2026
countSearchObjects(['@self' => ['schema' => $id]]) only returns a real count when BOTH register AND schema IDs are present in the @self filter; single-axis queries silently return 0. That made the DELETE safety guard on schemas and registers a no-op — `DELETE /schemas/{id}` and `DELETE /registers/{id}` succeeded even when objects were attached, because `objectCount` was always 0 and the 409 branch never fired. Switch both destroy() controllers to ObjectEntityMapper::getStatistics() on the single-axis path (registerId|schemaId), which is the same handler the rest of the controller already uses for stats display (see SchemasController.php:288, 924 and RegistersController.php:366, 399). After the fix, deleting a schema/register with attached objects without `?force=true` returns 409 with the orphan count, matching the runtime-schema-api ADR. Also apply PHPCS auto-fixes flagged on PR #1464 review: - RegistersController.php:539,648 — missing `//end try` end-comments. - ImportHandler.php:2419-2423 — docblock param-type spacing. PHPCS now reports zero errors on the touched files.
rubenvdlinde
requested review from
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
Runtime CRUD on /api/schemas and /api/registers with cache invalidation and declarative-engine reload, so callers (especially OpenBuilt's schema editor in chain spec #4) can author schemas at runtime instead of shipping them as static JSON in lib/Settings/{app}_register.json. Plus two follow-ups from the bootstrap-openbuilt smoke test (2026-05-11, commit 3138e4c): - importFromApp auto-creates a Register from x-openregister.app + info.title + info.description for application-type configurations - ObjectService::searchObjectsBySlug helper resolves register/schema slugs to numeric IDs before delegating to searchObjects; existing searchObjects docblock updated to make the numeric-ID-only contract explicit 5 artifacts: - proposal.md (kind=mixed, chained to openbuilt-schema-editor) - specs/runtime-schema-api/spec.md (new capability) - specs/data-import-export/spec.md (delta for importFromApp behaviour) - design.md (5 decisions, migration plan) - tasks.md (8 task groups, 28 tasks) openspec validate --strict: passes.
…a/register CRUD
Add invalidate() on SchemaCacheHandler and a new RegisterCacheHandler so the
runtime CRUD path on /api/schemas and /api/registers re-reads from the database
on the next request in the same PHP worker. Hooks the request-scoped find cache
on each mapper as well as the persistent schema cache row.
Add ?force=true delete guard on SchemasController::destroy and
RegistersController::destroy: returns HTTP 409 with structured
{error, objectCount} when objects still reference the entity, only deletes
when ?force=true is passed explicitly (and logs WARNING with audit data).
Spec: openregister-runtime-schema-api (tasks 1, 2.1, 2.2, 4.1, 4.2, 4.3)
…mports When ImportHandler::importFromApp processes an OAS document carrying x-openregister.type=application, derive a Register from x-openregister.app (slug), info.title (title), and info.description (description), and attach every imported schema's numeric ID to the resulting Register's schemas[] field. Lookup is idempotent on (slug, organisation) so a re-import on the same slug+org pair updates the existing Register row rather than inserting a duplicate. The Configuration row's registers[] field is kept in sync so the (Configuration, Schemas, Register) triple stays consistent. Closes the OpenBuilt smoke-test foot-gun: schemas exist but no Register wraps them, so slug-aware searches return zero. Citizen developers no longer need a manual POST /api/registers after the import. Spec: openregister-runtime-schema-api (tasks 5.1, 5.2, 5.3) Spec delta: data-import-export
…vice Add a slug-aware bridge to ObjectService::searchObjects() that resolves register and schema slugs to numeric IDs via the mappers (scoped to the caller's organisation by the standard multi-tenancy filter) and delegates to the numeric-ID search path. Closes the OpenBuilt smoke-test foot-gun where callers passed slugs in @self.register / @self.schema and got zero results back — the numeric-ID contract on searchObjects() is now strict, so any future misuse fails loudly at the call site rather than silently returning empty. Slug-aware callers (controller layer, OpenCatalogi, softwarecatalog) get a one-method-call upgrade path. Multi-tenancy honours the same _multitenancy flag every other lookup uses; a slug that exists in another organisation but not the caller's throws DoesNotExistException via the mappers' standard organisation filter. Also documents the numeric-ID contract inline on searchObjects() so the foot-gun cannot reappear silently.
countSearchObjects(['@self' => ['schema' => $id]]) only returns a real count when BOTH register AND schema IDs are present in the @self filter; single-axis queries silently return 0. That made the DELETE safety guard on schemas and registers a no-op — `DELETE /schemas/{id}` and `DELETE /registers/{id}` succeeded even when objects were attached, because `objectCount` was always 0 and the 409 branch never fired. Switch both destroy() controllers to ObjectEntityMapper::getStatistics() on the single-axis path (registerId|schemaId), which is the same handler the rest of the controller already uses for stats display (see SchemasController.php:288, 924 and RegistersController.php:366, 399). After the fix, deleting a schema/register with attached objects without `?force=true` returns 409 with the orphan count, matching the runtime-schema-api ADR. Also apply PHPCS auto-fixes flagged on PR #1464 review: - RegistersController.php:539,648 — missing `//end try` end-comments. - ImportHandler.php:2419-2423 — docblock param-type spacing. PHPCS now reports zero errors on the touched files.
…uto-Register Adds spec-specific unit tests for the new behaviour introduced by change openregister-runtime-schema-api: - tests/Unit/Service/ObjectServiceSearchBySlugTest.php (3 tests) covers REQ 'ObjectService.searchObjectsBySlug resolves slugs at the slug-aware layer': resolves both slugs into the numeric @self block, throws DoesNotExistException identifying register-side failures, throws DoesNotExistException identifying schema-side failures. - tests/Unit/Service/Configuration/ImportHandlerApplicationTypeTest.php (3 tests) covers data-import-export delta REQ 'importFromApp auto-creates a Register for application-type configurations': first import creates Register with derived slug/title/schemas[], re-import is idempotent on (slug, org) and only updates, library/untyped configs are skipped. - tests/bootstrap-spec-docker.php — Docker bootstrap that resolves OCA\OpenRegister\* to the spec worktree instead of the main custom_apps mount, so spec branch tests can run against the live NC container without disturbing the main mount.
…sters Adds spec-specific unit tests for the runtime-schema-api guard introduced in feature/runtime-schema-api: - tests/Unit/Controller/SchemasDestroySafetyTest.php (3 tests) covers REQ 'Runtime schema deletion is guarded by object count': DELETE returns 409 with schema-has-objects + objectCount when objects attached and force=false, DELETE with force=true succeeds + invalidates cache + logs WARNING with orphan count, DELETE on unused schema succeeds (baseline). - tests/Unit/Controller/RegistersDestroySafetyTest.php (3 tests) covers REQ 'Same CRUD guarantees apply to /api/registers': DELETE returns 409 with register-has-objects + objectCount when objects attached and force=false, DELETE with force=true succeeds + invalidates cache + logs WARNING with orphan count, DELETE on unused register succeeds (baseline).
…ntegration
Adds the canonical integration test referenced by tasks.md §4.4 — a
service-level round-trip through SchemasController that proves the cache
invalidation chain holds in the same PHP worker:
- POST /api/schemas + immediate GET — new schema visible (cache invalidated)
- PUT /api/schemas/{id} + same-worker re-read — new description observed
- DELETE without ?force=true returns 409 when objects exist
- DELETE with ?force=true returns 200; cache cleared; schema gone
- ObjectService::searchObjectsBySlug resolves a fresh slug-pair in-worker
Uses real services from \OC::\$server (SchemaMapper, RegisterMapper,
MagicMapper, ObjectService, SchemaCacheHandler, FacetCacheHandler,
SchemasController) and mocks only IRequest as a thin data carrier. Whole
suite is skipped automatically when Nextcloud isn't bootstrapped so the
host \`composer test:unit\` run stays green.
phpunit.xml updated so the Integration Tests testsuite picks up both the
existing tests/integration (newman) and the new tests/Integration (php).
…DELETE safety
Adds tests/integration/openregister.postman_collection.json — a focused,
spec-scoped Newman collection that exercises the end-to-end HTTP round-trip
the unit + service-level tests can only assert in isolation:
1. POST /api/registers seed register
2. POST /api/schemas create schema with `x-openregister-lifecycle`
3. GET /api/schemas/{id} immediate read sees the new schema (cache cleared)
4. PUT /api/schemas/{id} update description; chained GET sees update
5. PUT /api/registers/{id} attach schema to register
6. POST /api/objects/{r}/{s} create object against the schema
7. GET /api/objects/{r}/{s}/{u} read object back
8. DELETE /api/schemas/{id} no `?force=true` → 409 (schema-has-objects)
9. DELETE /api/schemas/{id}?force=true → 200; chained GET → 404
10. DELETE /api/registers/{id}?force=true → 200
Also updates openspec/.../tasks.md: 4.4 + 6.4 now closed (integration test
landed); 5.4 cross-references the unit-level ImportHandler coverage and
explicitly defers full importFromApp upload coverage to a follow-up.
Run inside the container:
docker exec -u 33 nextcloud newman run \
/var/www/html/custom_apps/openregister/tests/integration/openregister.postman_collection.json \
--env-var base_url=http://localhost --env-var admin_user=admin --env-var admin_password=admin
Post-rebase onto development: - psalm: 11 UnusedBaselineEntry errors removed (entries no longer match) - phpstan: baseline regenerated, net -50 lines (1455 errors tracked, matches current source after rebase reconciled both branches)
rubenvdlinde
force-pushed
the
feature/runtime-schema-api
branch
from
11f78f6 to
9b59a26
Compare
- RegistersController gained a 19th ctor arg (RegisterCacheHandler); update RegistersControllerTest + RegistersDestroySafetyTest to pass it (plus the previously-missing ContainerInterface/IGroupManager in the DELETE-safety test). - SchemasController create/update/destroy now route schema-cache cleanup through SchemaCacheHandler::invalidate(); update SchemasControllerTest expectations and add getStatistics() stubs for the new DELETE-safety object count. Use real Schema entities so the magic getId() resolves. - SchemasDestroySafetyTest: append the 14th ctor arg (ContainerInterface). - ObjectServiceSearchBySlugTest: pass the dateTimeNormalizer ctor arg. - RegistersControllerTest destroy/* tests: use real Register entities and stub getStatistics() so RegisterCacheHandler::invalidate(int) is fed a real id.
Contributor
Author
CI status notesPushed
Pre-existing reds — NOT caused by this PR, not blockingThe same failures exist on
After this push the PR-introduced PHPUnit reds are resolved (verified locally against the live NC source: 163 controller tests + 9 new safety tests green). The remaining red is the pre-existing eslint job, which is not in scope for this change. |
Contributor
Author
Re-run after
|
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
OpenSpec change artifacts for
openregister-runtime-schema-api— spec #3of the 9-spec OpenBuilt chain. This change ships runtime CRUD on
/api/schemasand/api/registerswith proper cache invalidation anddeclarative-engine reload, so non-admin callers (starting with
OpenBuilt's schema editor in chain spec #4) can author schemas at
runtime without shipping a PHP register PR.
Two follow-ups from the
bootstrap-openbuiltsmoke test (2026-05-11,commit 3138e4c titled "Runtime smoke test — fix 5 real bugs") are
folded into this spec:
importFromAppauto-creates a Register fromx-openregister.app+info.title+info.descriptionwhen the imported configurationcarries
x-openregister.type=application. Idempotent on(slug, organisationId).ObjectService::searchObjectsBySlughelper resolves register/schemaslugs to numeric IDs before delegating to
searchObjects. Theexisting
searchObjectsdocblock is updated to state thenumeric-IDs-only contract explicitly so the next misuse fails loudly,
not silently.
Artifacts
openspec/changes/openregister-runtime-schema-api/proposal.md(kind=mixed, chained to
openbuilt-schema-editor)openspec/changes/openregister-runtime-schema-api/specs/runtime-schema-api/spec.md(new capability, 6 requirements)
openspec/changes/openregister-runtime-schema-api/specs/data-import-export/spec.md(delta for the importFromApp Register-auto-create behaviour)
openspec/changes/openregister-runtime-schema-api/design.md(5 decisions covering cache invalidation, engine reload,
auto-Register, searchObjectsBySlug ergonomics, multi-tenant slug
resolution; backwards-compatible migration plan)
openspec/changes/openregister-runtime-schema-api/tasks.md(8 task groups, 28 tasks)
Validation
openspec validate openregister-runtime-schema-api --strict— passes.Test plan
reasoning behind each decision
(auto-Register + searchObjectsBySlug) are scoped correctly
smoke test against the resulting implementation and confirm
the manual
POST /api/registersstep is no longer needed