Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,4 @@ openspec/test-site-results/**/*.webp
/test-results/
/.playwright/
/playwright/.cache/
tests/e2e/.auth/
92 changes: 85 additions & 7 deletions lib/Controller/ApplicationsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -1189,24 +1189,102 @@ private function buildRewriteMap(array $companions, string $newSlug): array
*/
private function provisionPerAppRegister(string $newSlug, string $ownerUid): \OCA\OpenRegister\Db\Register
{
$registerSlug = 'openbuilt-'.$newSlug;
// Cross-user collision guard (#77): OR's register slugs are
// organisation-wide unique, so two callers wanting the same
// template+slug would otherwise share a register. The first
// attempt at scoping always namespaced by owner, but that
// breaks every existing single-tenant install (their existing
// registers are still at `openbuilt-{slug}`). So we keep the
// legacy un-namespaced slug AS LONG AS the existing register
// belongs to the caller; otherwise fall back to the
// owner-namespaced form.
$legacyRegisterSlug = 'openbuilt-'.$newSlug;

try {
return $this->registerMapper->find($registerSlug, _multitenancy: false);
$existing = $this->registerMapper->find($legacyRegisterSlug, _multitenancy: false);

$existingOwner = $this->extractRegisterOwner($existing);
if ($existingOwner === '' || $existingOwner === $ownerUid) {
return $existing;
}

// Different user owns the org-wide slug — namespace ours.
return $this->findOrCreateRegister(
slug: 'openbuilt-'.$ownerUid.'-'.$newSlug,
appSlug: $newSlug,
ownerUid: $ownerUid,
);
} catch (Throwable) {
// Legacy slug not taken — claim it for this user.
}

return $this->findOrCreateRegister(
slug: $legacyRegisterSlug,
appSlug: $newSlug,
ownerUid: $ownerUid,
);
}//end provisionPerAppRegister()

/**
* Find or create a per-app register at an exact slug.
*
* @param string $slug The register slug to find/create
* @param string $appSlug The application slug (for the title)
* @param string $ownerUid The owner UID (for the description audit trail)
*
* @return \OCA\OpenRegister\Db\Register
*/
private function findOrCreateRegister(
string $slug,
string $appSlug,
string $ownerUid,
): \OCA\OpenRegister\Db\Register {
try {
return $this->registerMapper->find($slug, _multitenancy: false);
} catch (Throwable) {
// Register does not exist yet — create it.
// Not found — create it.
}

return $this->registerMapper->createFromArray(
[
'slug' => $registerSlug,
'title' => 'OpenBuilt — '.$newSlug,
'description' => 'Per-app schema namespace for OpenBuilt app `'.$newSlug.'` (owner: '.$ownerUid.').',
'slug' => $slug,
'title' => 'OpenBuilt — '.$appSlug,
'description' => 'Per-app schema namespace for OpenBuilt app `'.$appSlug.'` (owner: '.$ownerUid.').',
'version' => '0.1.0',
'schemas' => [],
]
);
}//end provisionPerAppRegister()
}//end findOrCreateRegister()

/**
* Extract the owner UID from a Register entity, tolerating either an
* `owner` field on the entity itself or one inside an `@self` block.
*
* @param mixed $register The Register entity (or non-Register junk).
*
* @return string The owner UID, or empty string when not determinable.
*/
private function extractRegisterOwner(mixed $register): string
{
if (is_object($register) === true && method_exists($register, 'getOwner') === true) {
$owner = $register->getOwner();
if (is_string($owner) === true) {
return $owner;
}
}

if (is_object($register) === true && method_exists($register, 'jsonSerialize') === true) {
$data = $register->jsonSerialize();
if (is_array($data) === true) {
$owner = ($data['owner'] ?? ($data['@self']['owner'] ?? null));
if (is_string($owner) === true) {
return $owner;
}
}
}

return '';
}//end extractRegisterOwner()

/**
* Clone companion schemas into the per-app register.
Expand Down
45 changes: 42 additions & 3 deletions lib/Service/ApplicationCreationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,10 @@ public function createApplication(array $payload): string
$registerSlug = 'openbuilt-'.$appSlug.'-'.$versionSlug;

// 3a: Create ApplicationVersion
$versionManifest = $this->substituteRegisterSlug(
$versionManifest = $this->substituteVersionContext(
manifest: $defaultManifest,
registerSlug: $registerSlug
registerSlug: $registerSlug,
schemaSlugPrefix: $appSlug.'-'.$versionSlug.'-'
);

$versionPayload = [
Expand Down Expand Up @@ -815,6 +816,35 @@ private function loadDefaultSchemas(): array
*/
public function substituteRegisterSlug(array $manifest, string $registerSlug): array
{
return $this->substituteVersionContext(
manifest: $manifest,
registerSlug: $registerSlug,
schemaSlugPrefix: ''
);
}//end substituteRegisterSlug()

/**
* Substitute the per-version context tokens in a manifest template.
*
* Walks all `pages[*].config` blocks. Replaces the `{registerSlug}`
* token in `register` and rewrites every non-empty `config.schema`
* to the namespaced seed slug `{schemaSlugPrefix}{originalSchemaSlug}`
* so the manifest references the actual per-version schemas created
* by {@see provisionRegister()} (openbuilt#75 — without this the
* KPI / insights cards aggregated against a non-existent schema and
* leaked the same numbers across all tiers).
*
* @param array<string,mixed> $manifest The manifest template blob
* @param string $registerSlug The per-version register slug
* @param string $schemaSlugPrefix Namespaced prefix for schema slugs (e.g. `permit-flow-development-`)
*
* @return array<string,mixed> The manifest with tokens substituted
*/
public function substituteVersionContext(
array $manifest,
string $registerSlug,
string $schemaSlugPrefix
): array {
if (isset($manifest['pages']) === false || is_array($manifest['pages']) === false) {
return $manifest;
}
Expand All @@ -831,11 +861,20 @@ public function substituteRegisterSlug(array $manifest, string $registerSlug): a
if (isset($page['config']['register']) === true && $page['config']['register'] === '{registerSlug}') {
$page['config']['register'] = $registerSlug;
}

if ($schemaSlugPrefix !== ''
&& isset($page['config']['schema']) === true
&& is_string($page['config']['schema']) === true
&& $page['config']['schema'] !== ''
&& str_starts_with($page['config']['schema'], $schemaSlugPrefix) === false
) {
$page['config']['schema'] = $schemaSlugPrefix.$page['config']['schema'];
}
}

unset($page);
return $manifest;
}//end substituteRegisterSlug()
}//end substituteVersionContext()

/**
* Get the UID of the currently authenticated user.
Expand Down
23 changes: 17 additions & 6 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,32 @@ export default defineConfig({
testDir: 'tests/e2e',
timeout: 30_000,
expect: { timeout: 5_000 },
fullyParallel: true,
// Don't run specs in parallel: Nextcloud's brute-force throttle fires
// after a handful of near-simultaneous form logins from the same IP
// and every subsequent spec falls back to the /login page. Serial
// execution with one shared storageState (via globalSetup) avoids it.
fullyParallel: false,
forbidOnly: !!process.env.CI,
retries: process.env.CI ? 1 : 0,
workers: process.env.CI ? 1 : undefined,
workers: 1,
globalSetup: './tests/e2e/global-setup.ts',
reporter: process.env.CI ? [['github'], ['html', { open: 'never' }]] : 'list',
use: {
baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8080',
// Authenticated browser context populated by globalSetup. Empty
// when login fails — specs then surface the actual login page in
// their failure snapshots.
storageState: 'tests/e2e/.auth/admin.json',
httpCredentials: {
username: process.env.NC_ADMIN_USER || 'admin',
password: process.env.NC_ADMIN_PASSWORD || 'admin',
},
extraHTTPHeaders: {
// Nextcloud rejects unauthenticated OCS/API calls without this header.
'OCS-APIRequest': 'true',
},
// Note: do NOT set `OCS-APIRequest: true` here globally. The header
// makes Nextcloud treat every request as an API call — including the
// HTML page loads — which breaks the browser-based login redirect
// (no Location header is emitted, the page stays on /login).
// Specs that need OCS-APIRequest set it on their explicit `request`
// calls (e.g. versionRouting.spec.ts, applicationDetailOverview.spec.ts).
trace: 'on-first-retry',
screenshot: 'only-on-failure',
video: 'retain-on-failure',
Expand Down
35 changes: 31 additions & 4 deletions tests/Unit/Repair/MigrateToVersionedModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,43 @@ private function step(): MigrateToVersionedModel
}//end step()

/**
* Short-circuit: versioned schema already present → no deletions.
* Short-circuit: no Application rows carry pre-spec-C shape → no deletions.
*
* The previous check probed for the `applicationVersion` schema's
* existence, but `InitializeSettings` imports that schema BEFORE
* this step runs, so the probe always fired true and the migration
* was skipped (openbuilt#69). The new check inspects Application
* row shape and only proceeds when at least one row still carries
* a legacy `manifest` / `version` / `status` / `currentVersion`
* top-level field.
*
* @return void
*/
public function testShortCircuitsWhenVersionedSchemaPresent(): void
{
// schemaMapper->find('applicationVersion', ...) succeeds (no throw) on first call.
$this->schemaMapper->method('find')->willReturn($this->createMock(Schema::class));
// Register + schema lookups succeed.
$register = $this->getMockBuilder(Register::class)
->disableOriginalConstructor()
->addMethods(['getId'])
->getMock();
$register->method('getId')->willReturn(1);
$this->registerMapper->method('find')->willReturn($register);

$this->schemaMapper->method('find')->willReturn(
$this->getMockBuilder(Schema::class)
->disableOriginalConstructor()
->addMethods(['getId'])
->getMock()
);

// Application rows already match the post-C shape (no legacy keys).
$postCRows = [
$this->mockEntity(['id' => 'uuid-a', 'slug' => 'app-a', 'name' => 'App A']),
];
$this->objectService->method('findAll')->willReturn($postCRows);

$this->objectService->expects(self::never())->method('findAll');
// No deletions should fire: rows look post-C so the short-circuit
// returns true and `run()` exits before touching either service.
$this->objectService->expects(self::never())->method('deleteObject');
$this->registerService->expects(self::never())->method('delete');

Expand Down
88 changes: 88 additions & 0 deletions tests/Unit/Service/ApplicationCreationServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,94 @@ public function substituteRegisterSlugDoesNotTouchNonTokenFields(): void
self::assertSame('some-other-register', $result['pages'][0]['config']['register']);
}//end substituteRegisterSlugDoesNotTouchNonTokenFields()

/**
* substituteVersionContext rewrites both register AND schema.
*
* openbuilt#75 — without this the KPI / insights cards aggregate
* against `hello-message` (the un-namespaced template slug), which
* doesn't exist in the per-version register, so counts leak the
* same numbers across all tiers.
*
* @test
*
* @return void
*/
public function substituteVersionContextNamespacesSchemaSlugAlongsideRegister(): void
{
$manifest = [
'pages' => [
['id' => 'Dashboard', 'config' => ['widgets' => []]],
['id' => 'Messages', 'config' => ['register' => '{registerSlug}', 'schema' => 'hello-message']],
],
];

$result = $this->service->substituteVersionContext(
manifest: $manifest,
registerSlug: 'openbuilt-permit-flow-development',
schemaSlugPrefix: 'permit-flow-development-'
);

self::assertSame('openbuilt-permit-flow-development', $result['pages'][1]['config']['register']);
self::assertSame('permit-flow-development-hello-message', $result['pages'][1]['config']['schema']);
}//end substituteVersionContextNamespacesSchemaSlugAlongsideRegister()

/**
* substituteVersionContext is idempotent: re-running with the same
* prefix MUST NOT double-prefix the schema slug.
*
* @test
*
* @return void
*/
public function substituteVersionContextIsIdempotentOnAlreadyNamespacedSlugs(): void
{
$manifest = [
'pages' => [
[
'id' => 'Messages',
'config' => [
'register' => 'openbuilt-permit-flow-development',
'schema' => 'permit-flow-development-hello-message',
],
],
],
];

$result = $this->service->substituteVersionContext(
manifest: $manifest,
registerSlug: 'openbuilt-permit-flow-development',
schemaSlugPrefix: 'permit-flow-development-'
);

// Schema must NOT become `permit-flow-development-permit-flow-development-hello-message`.
self::assertSame('permit-flow-development-hello-message', $result['pages'][0]['config']['schema']);
}//end substituteVersionContextIsIdempotentOnAlreadyNamespacedSlugs()

/**
* substituteVersionContext leaves schema alone when no prefix is provided
* (backwards-compat with the legacy substituteRegisterSlug entry point).
*
* @test
*
* @return void
*/
public function substituteVersionContextLeavesSchemaAloneWhenPrefixEmpty(): void
{
$manifest = [
'pages' => [
['id' => 'Messages', 'config' => ['register' => '{registerSlug}', 'schema' => 'hello-message']],
],
];

$result = $this->service->substituteVersionContext(
manifest: $manifest,
registerSlug: 'openbuilt-my-app-production',
schemaSlugPrefix: ''
);

self::assertSame('hello-message', $result['pages'][0]['config']['schema']);
}//end substituteVersionContextLeavesSchemaAloneWhenPrefixEmpty()

// -------------------------------------------------------------------------
// resolveVersionChain
// -------------------------------------------------------------------------
Expand Down
Loading
Loading