From abf95d500739878d5160962e2e59167aebec42eb Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 11:51:12 +0100 Subject: [PATCH 01/36] Schema Skip/Upsert via pre-check + updatedAt gate on attribute/index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-migration needs the destination to tolerate existing schema resources. The database library stays minimal — migration owns the reconciliation decision since it has both source and destination metadata in hand. Per-resource behavior: - Database: pre-check `databases` metadata. If exists, hydrate sequence from the existing document and return. Both Skip and Upsert tolerate unconditionally — databases contain the entire tree of collections + rows, dropping would destroy everything. - Table: pre-check `database_{sequence}` metadata. Same pattern — tolerate unconditionally, hydrate sequence, never drop. Attribute and index reconciliation happens per-resource at the lower layer. - Attribute: pre-check `attributes` metadata by composite id ({databaseSeq}_{tableSeq}_{key}). Skip tolerates. Upsert compares source's updatedAt vs destination's — if source is strictly newer the column is dropped (via `dbForDatabases->deleteAttribute`) and recreated so the spec matches source. Column data is wiped on drop; rows will be repopulated by the row-level Upsert path that follows. - Index: same pattern against `indexes` metadata. Drop is cheap because indexes carry no user data — only rebuild time. Approach is pre-check rather than try/catch for control flow: migration is sequential single-writer, so the race condition justification for try/catch doesn't apply, and pre-check reads top-to-bottom with no exception-as-control-flow. Row-level dispatch (unchanged, already committed): OnDuplicate::Upsert → $dbForDatabases->upsertDocuments(...) OnDuplicate::Skip → $dbForDatabases->skipDuplicates(fn () => createDocuments(...)) OnDuplicate::Fail → plain createDocuments Both row-level primitives are existing library APIs — no database-library changes are required for this feature. Helpers: shouldTolerateSchemaDuplicate() // onDuplicate !== Fail sourceSpecIsNewer($src, $dst) // strtotime compare, false on empty --- src/Migration/Destinations/Appwrite.php | 117 +++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 893bf987..c18eb63c 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -422,6 +422,20 @@ protected function createDatabase(Database $resource): bool $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); + // Skip/Upsert: pre-check an existing database. Databases contain the + // entire tree of collections + rows, so they are never destructively + // reconciled — both modes simply hydrate the sequence from the + // existing metadata document and return. Downstream resources + // (tables/columns/rows) locate the existing underlying schema via the + // hydrated sequence. + if ($this->shouldTolerateSchemaDuplicate()) { + $existing = $this->dbForProject->getDocument('databases', $resource->getId()); + if (!$existing->isEmpty()) { + $resource->setSequence($existing->getSequence()); + return true; + } + } + $database = $this->dbForProject->createDocument('databases', new UtopiaDocument([ '$id' => $resource->getId(), 'name' => $resource->getDatabaseName(), @@ -456,6 +470,32 @@ protected function createDatabase(Database $resource): bool return true; } + /** + * Skip/Upsert both tolerate an existing schema resource on re-migration. + * Upsert differs from Skip only for leaf resources (attributes, indexes) + * where the updatedAt comparison gates a drop+recreate — containers + * (databases, tables) are always tolerate-only because their data is + * preserved. + */ + private function shouldTolerateSchemaDuplicate(): bool + { + return $this->onDuplicate !== OnDuplicate::Fail; + } + + /** + * Upsert reconciliation: source's updatedAt strictly later than + * destination's signals the spec changed since last sync — leaf resource + * should be dropped + recreated. Equal or earlier: dest is up-to-date, + * tolerate. + */ + private function sourceSpecIsNewer(string $sourceUpdatedAt, string $destUpdatedAt): bool + { + if ($sourceUpdatedAt === '' || $destUpdatedAt === '') { + return false; + } + return \strtotime($sourceUpdatedAt) > \strtotime($destUpdatedAt); + } + /** * @throws AuthorizationException * @throws DatabaseException @@ -503,6 +543,21 @@ protected function createEntity(Table $resource): bool $dbForDatabases->create(); } + // Skip/Upsert: pre-check an existing table. Like databases, tables + // contain user data (rows), so both modes simply hydrate the sequence + // from the existing metadata document. Attribute/index reconciliation + // happens per-resource at a lower layer. + if ($this->shouldTolerateSchemaDuplicate()) { + $existing = $this->dbForProject->getDocument( + 'database_' . $database->getSequence(), + $resource->getId() + ); + if (!$existing->isEmpty()) { + $resource->setSequence($existing->getSequence()); + return true; + } + } + $table = $this->dbForProject->createDocument('database_' . $database->getSequence(), new UtopiaDocument([ '$id' => $resource->getId(), 'databaseInternalId' => $database->getSequence(), @@ -642,9 +697,38 @@ protected function createField(Column|Attribute $resource): bool $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); $dbForDatabases = ($this->getDatabasesDB)($database); + + // Skip/Upsert: pre-check against `attributes` metadata. Skip tolerates + // unconditionally; Upsert tolerates unless source's updatedAt is + // strictly newer — in which case the column is dropped + recreated so + // the spec matches source. Column data is wiped on drop; rows are + // repopulated via row-level Upsert below. + $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); + if ($this->shouldTolerateSchemaDuplicate()) { + $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); + if (!$existingAttr->isEmpty()) { + if ($this->onDuplicate === OnDuplicate::Skip + || !$this->sourceSpecIsNewer($updatedAt, $existingAttr->getUpdatedAt()) + ) { + // Destination already up-to-date; hydrate caches and skip. + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + return true; + } + // Source spec newer: drop then recreate under the create flow. + $dbForDatabases->deleteAttribute( + 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), + $resource->getKey() + ); + $this->dbForProject->deleteDocument('attributes', $attributeMetaId); + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + } + } + try { $column = new UtopiaDocument([ - '$id' => ID::custom($database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey()), + '$id' => ID::custom($attributeMetaId), 'key' => $resource->getKey(), 'databaseInternalId' => $database->getSequence(), 'databaseId' => $database->getId(), @@ -920,6 +1004,37 @@ protected function createIndex(Index $resource): bool ); } + // Skip/Upsert: pre-check against `indexes` metadata. Same rule as + // attributes — Skip tolerates unconditionally; Upsert tolerates unless + // source's updatedAt is strictly newer, in which case the index is + // dropped + recreated. Index drops are non-destructive (no row data + // lives in indexes) so rebuild cost is just the index build time. + $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); + if ($this->shouldTolerateSchemaDuplicate()) { + $existingIdx = $this->dbForProject->getDocument('indexes', $indexMetaId); + if (!$existingIdx->isEmpty()) { + if ($this->onDuplicate === OnDuplicate::Skip + || !$this->sourceSpecIsNewer($updatedAt, $existingIdx->getUpdatedAt()) + ) { + $this->dbForProject->purgeCachedDocument( + 'database_' . $database->getSequence(), + $table->getId() + ); + return true; + } + // Source spec newer: drop then recreate. + $dbForDatabases->deleteIndex( + 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), + $resource->getKey() + ); + $this->dbForProject->deleteDocument('indexes', $indexMetaId); + $this->dbForProject->purgeCachedDocument( + 'database_' . $database->getSequence(), + $table->getId() + ); + } + } + $index = $this->dbForProject->createDocument('indexes', $index); try { From fc6e5f7d0210c46aad168a863279bc8a8dd65011 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 13:26:22 +0100 Subject: [PATCH 02/36] sourceSpecIsNewer: explicit false check on strtotime Guard against malformed datetime strings (e.g. "0000-00-00 00:00:00") that PHP's strtotime() returns false for. Without the explicit check, false gets loose-compared as 0 and the helper silently returns false, meaning a corrupted source updatedAt would always tolerate the existing destination entry and never trigger drop+recreate. Appwrite itself always emits parseable RFC 3339 timestamps, so this is mainly defensive for non-Appwrite sources (Supabase, NHost, CSV). Flagged by greptile P2. --- src/Migration/Destinations/Appwrite.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index c18eb63c..8ac8ca61 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -493,7 +493,16 @@ private function sourceSpecIsNewer(string $sourceUpdatedAt, string $destUpdatedA if ($sourceUpdatedAt === '' || $destUpdatedAt === '') { return false; } - return \strtotime($sourceUpdatedAt) > \strtotime($destUpdatedAt); + $src = \strtotime($sourceUpdatedAt); + $dst = \strtotime($destUpdatedAt); + if ($src === false || $dst === false) { + // Conservative: any unparseable timestamp → don't drop+recreate. + // Matters for non-Appwrite sources that may emit malformed dates + // (e.g. "0000-00-00 00:00:00"); Appwrite itself always emits + // parseable RFC 3339. + return false; + } + return $src > $dst; } /** From 4e8100ab83471f9d69b9b1eea2cec3d81aa7b2c1 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 13:50:24 +0100 Subject: [PATCH 03/36] Upsert drop: clean up child-side metadata for two-way relationships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a first-run migration createField writes TWO metadata documents for a two-way relationship attribute: parent-side at {dbSeq}_{tableSeq}_{key} (line 763) and child-side at {dbSeq}_{relatedTableSeq}_{twoWayKey} (line 819). Migration operates directly on the database via $dbForProject — it doesn't rely on Appwrite's attribute-event workers to derive the child side — so both documents are physically written by migration itself. The Upsert drop block was only removing the parent-side document + column. On the recreate pass that followed, line 819 re-wrote the child-side document and immediately hit DuplicateException because the stale child doc from the previous migration run was still sitting there. The inner catch rolled back the parent and the whole attribute aborted with "Attribute already exists" — breaking Upsert re-migration for every two-way relationship whose source spec had changed. Fix: in the Upsert drop branch, when the resource is a two-way relationship, also delete the child-side metadata document and the child-side physical column on the related table, then purge caches. Both deletes are wrapped in try/catch to tolerate the case where a prior interrupted run (or utopia-php/database's relationship handling) already cleaned one side — the goal is to guarantee a clean slate for the downstream recreate, not to require both sides to still exist. Flagged by greptile P1. Reproducing scenario: - OnDuplicate::Upsert - Destination already contains the attribute - Source's two-way relationship spec was modified between runs (source.updatedAt > dest.updatedAt) --- src/Migration/Destinations/Appwrite.php | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 8ac8ca61..f01c77ea 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -732,6 +732,37 @@ protected function createField(Column|Attribute $resource): bool $this->dbForProject->deleteDocument('attributes', $attributeMetaId); $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + + // Two-way relationships: the first-run createField wrote TWO + // metadata docs (parent at line 763, child at line 819). The + // drop above only removed the parent; without cleaning up the + // child, the recreate path hits DuplicateException at line 819 + // and the whole attribute re-migration fails. + $resourceOptions = $resource->getOptions(); + if ($type === UtopiaDatabase::VAR_RELATIONSHIP && !empty($resourceOptions['twoWay'])) { + // $relatedTable was populated at the top of createField + // inside the same VAR_RELATIONSHIP branch. + $childTwoWayKey = $resourceOptions['twoWayKey'] ?? ''; + if ($childTwoWayKey !== '') { + $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $childTwoWayKey; + try { + $this->dbForProject->deleteDocument('attributes', $childMetaId); + } catch (\Throwable) { + // Child metadata already gone — interrupted prior run, nothing to do. + } + try { + $dbForDatabases->deleteAttribute( + 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(), + $childTwoWayKey + ); + } catch (\Throwable) { + // Child column already gone — the parent's deleteAttribute may + // have cascaded via utopia-php/database's relationship handling. + } + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); + $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence()); + } + } } } From cfd6b987af2e25ddaa41f38a3a39e2535e1b0337 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 13:56:01 +0100 Subject: [PATCH 04/36] Move tolerate/reconcile logic onto OnDuplicate enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DestinationAppwrite's shouldTolerateSchemaDuplicate() and sourceSpecIsNewer() were private helpers that encoded mode-specific behavior. Moving them onto OnDuplicate itself puts the behavior on the type that owns it and makes the call sites self-documenting: $this->onDuplicate->toleratesSchemaDuplicate() $this->onDuplicate->shouldReconcileSchema($src, $dst) shouldReconcileSchema() also subsumes the explicit Skip check at the call sites — it returns false for Skip and Fail, so the compound "`Skip || !sourceSpecIsNewer(...)`" condition collapses to a single "`!shouldReconcileSchema(...)`". No behavior change. Lint + PHPStan L3 clean. --- src/Migration/Destinations/Appwrite.php | 54 ++++------------------ src/Migration/Destinations/OnDuplicate.php | 31 +++++++++++++ 2 files changed, 39 insertions(+), 46 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index f01c77ea..d68b6c43 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -428,7 +428,7 @@ protected function createDatabase(Database $resource): bool // existing metadata document and return. Downstream resources // (tables/columns/rows) locate the existing underlying schema via the // hydrated sequence. - if ($this->shouldTolerateSchemaDuplicate()) { + if ($this->onDuplicate->toleratesSchemaDuplicate()) { $existing = $this->dbForProject->getDocument('databases', $resource->getId()); if (!$existing->isEmpty()) { $resource->setSequence($existing->getSequence()); @@ -470,41 +470,6 @@ protected function createDatabase(Database $resource): bool return true; } - /** - * Skip/Upsert both tolerate an existing schema resource on re-migration. - * Upsert differs from Skip only for leaf resources (attributes, indexes) - * where the updatedAt comparison gates a drop+recreate — containers - * (databases, tables) are always tolerate-only because their data is - * preserved. - */ - private function shouldTolerateSchemaDuplicate(): bool - { - return $this->onDuplicate !== OnDuplicate::Fail; - } - - /** - * Upsert reconciliation: source's updatedAt strictly later than - * destination's signals the spec changed since last sync — leaf resource - * should be dropped + recreated. Equal or earlier: dest is up-to-date, - * tolerate. - */ - private function sourceSpecIsNewer(string $sourceUpdatedAt, string $destUpdatedAt): bool - { - if ($sourceUpdatedAt === '' || $destUpdatedAt === '') { - return false; - } - $src = \strtotime($sourceUpdatedAt); - $dst = \strtotime($destUpdatedAt); - if ($src === false || $dst === false) { - // Conservative: any unparseable timestamp → don't drop+recreate. - // Matters for non-Appwrite sources that may emit malformed dates - // (e.g. "0000-00-00 00:00:00"); Appwrite itself always emits - // parseable RFC 3339. - return false; - } - return $src > $dst; - } - /** * @throws AuthorizationException * @throws DatabaseException @@ -556,7 +521,7 @@ protected function createEntity(Table $resource): bool // contain user data (rows), so both modes simply hydrate the sequence // from the existing metadata document. Attribute/index reconciliation // happens per-resource at a lower layer. - if ($this->shouldTolerateSchemaDuplicate()) { + if ($this->onDuplicate->toleratesSchemaDuplicate()) { $existing = $this->dbForProject->getDocument( 'database_' . $database->getSequence(), $resource->getId() @@ -713,13 +678,11 @@ protected function createField(Column|Attribute $resource): bool // the spec matches source. Column data is wiped on drop; rows are // repopulated via row-level Upsert below. $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); - if ($this->shouldTolerateSchemaDuplicate()) { + if ($this->onDuplicate->toleratesSchemaDuplicate()) { $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); if (!$existingAttr->isEmpty()) { - if ($this->onDuplicate === OnDuplicate::Skip - || !$this->sourceSpecIsNewer($updatedAt, $existingAttr->getUpdatedAt()) - ) { - // Destination already up-to-date; hydrate caches and skip. + if (!$this->onDuplicate->shouldReconcileSchema($updatedAt, $existingAttr->getUpdatedAt())) { + // Skip, or Upsert with dest up-to-date — tolerate. $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); return true; @@ -1050,12 +1013,11 @@ protected function createIndex(Index $resource): bool // dropped + recreated. Index drops are non-destructive (no row data // lives in indexes) so rebuild cost is just the index build time. $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); - if ($this->shouldTolerateSchemaDuplicate()) { + if ($this->onDuplicate->toleratesSchemaDuplicate()) { $existingIdx = $this->dbForProject->getDocument('indexes', $indexMetaId); if (!$existingIdx->isEmpty()) { - if ($this->onDuplicate === OnDuplicate::Skip - || !$this->sourceSpecIsNewer($updatedAt, $existingIdx->getUpdatedAt()) - ) { + if (!$this->onDuplicate->shouldReconcileSchema($updatedAt, $existingIdx->getUpdatedAt())) { + // Skip, or Upsert with dest up-to-date — tolerate. $this->dbForProject->purgeCachedDocument( 'database_' . $database->getSequence(), $table->getId() diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 31a0e49f..2e3918c4 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -18,4 +18,35 @@ public static function values(): array { return \array_map(fn (self $case) => $case->value, self::cases()); } + + /** + * Skip and Upsert tolerate an existing schema resource on re-migration; + * Fail rethrows. Callers gate their pre-check on this before issuing a + * `getDocument` against the destination metadata — on Fail mode the + * library's own `DuplicateException` surfaces from the create call below. + */ + public function toleratesSchemaDuplicate(): bool + { + return $this !== self::Fail; + } + + /** + * Upsert reconciliation trigger: true iff mode is Upsert AND source's + * updatedAt is strictly newer than destination's. Skip and Fail always + * return false (they don't reconcile). Unparseable timestamps → false + * (conservative: preserve the existing destination rather than risk a + * destructive drop on garbage input). + */ + public function shouldReconcileSchema(string $sourceUpdatedAt, string $destUpdatedAt): bool + { + if ($this !== self::Upsert) { + return false; + } + $src = \strtotime($sourceUpdatedAt); + $dst = \strtotime($destUpdatedAt); + if ($src === false || $dst === false) { + return false; + } + return $src > $dst; + } } From 001682168f7d87932c56635a0d0a45b927febc33 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Wed, 22 Apr 2026 14:15:22 +0100 Subject: [PATCH 05/36] Collapse tolerate/reconcile into one decision point: resolveSchemaAction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OnDuplicate previously exposed two methods (toleratesSchemaDuplicate + shouldReconcileSchema) that call sites had to combine. That split meant the decision was spread across two methods and the call site — adding a new mode later would require updating all three places in sync. Replace with a single resolveSchemaAction() that returns one of three SchemaAction enum cases (Create / Tolerate / DropAndRecreate). The SchemaAction enum lives in the same file as OnDuplicate so the complete decision surface is in one place. For data-preserving containers (databases, tables) the method accepts allowDrop=false so it can never return DropAndRecreate — callers that use that path don't need to worry about accidentally dropping the container. In practice the container call sites just use the inline `$this->onDuplicate !== OnDuplicate::Fail` gate and then a simple "is empty?" branch; resolveSchemaAction() is reserved for the leaves (attributes, indexes) that actually benefit from the three-way choice. Fail-mode short-circuit is preserved at every call site: no getDocument pre-check when onDuplicate is Fail, so first-time migrations (the common default-mode path) pay zero overhead. The short-circuit stays at the call site rather than inside the enum method so the "don't query on Fail" invariant is visible where the query is issued. Call sites: - createDatabase / createEntity: inline `!== Fail` gate, tolerate on existing. No drop ever. - createField: Fail short-circuit, else resolveSchemaAction + match. DropAndRecreate triggers the two-way relationship cleanup. - createIndex: same pattern, no two-way concern. --- src/Migration/Destinations/Appwrite.php | 80 +++++++++++++--------- src/Migration/Destinations/OnDuplicate.php | 65 +++++++++++++----- 2 files changed, 94 insertions(+), 51 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index d68b6c43..992b0e2e 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -428,7 +428,11 @@ protected function createDatabase(Database $resource): bool // existing metadata document and return. Downstream resources // (tables/columns/rows) locate the existing underlying schema via the // hydrated sequence. - if ($this->onDuplicate->toleratesSchemaDuplicate()) { + // + // Fail mode short-circuits: no pre-check, let the create below throw + // DuplicateException as designed. Saves N metadata reads on the + // common first-time-migration path. + if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument('databases', $resource->getId()); if (!$existing->isEmpty()) { $resource->setSequence($existing->getSequence()); @@ -520,8 +524,8 @@ protected function createEntity(Table $resource): bool // Skip/Upsert: pre-check an existing table. Like databases, tables // contain user data (rows), so both modes simply hydrate the sequence // from the existing metadata document. Attribute/index reconciliation - // happens per-resource at a lower layer. - if ($this->onDuplicate->toleratesSchemaDuplicate()) { + // happens per-resource at a lower layer. Fail short-circuits. + if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument( 'database_' . $database->getSequence(), $resource->getId() @@ -672,22 +676,26 @@ protected function createField(Column|Attribute $resource): bool $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); $dbForDatabases = ($this->getDatabasesDB)($database); - // Skip/Upsert: pre-check against `attributes` metadata. Skip tolerates - // unconditionally; Upsert tolerates unless source's updatedAt is - // strictly newer — in which case the column is dropped + recreated so - // the spec matches source. Column data is wiped on drop; rows are - // repopulated via row-level Upsert below. + // Skip/Upsert: pre-check against `attributes` metadata via one + // resolveSchemaAction call. Fail mode short-circuits to preserve + // zero-overhead fresh-migration behavior. $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); - if ($this->onDuplicate->toleratesSchemaDuplicate()) { + if ($this->onDuplicate !== OnDuplicate::Fail) { $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); - if (!$existingAttr->isEmpty()) { - if (!$this->onDuplicate->shouldReconcileSchema($updatedAt, $existingAttr->getUpdatedAt())) { - // Skip, or Upsert with dest up-to-date — tolerate. - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); - return true; - } - // Source spec newer: drop then recreate under the create flow. + $action = $this->onDuplicate->resolveSchemaAction( + !$existingAttr->isEmpty(), + $updatedAt, + $existingAttr->getUpdatedAt(), + ); + + if ($action === SchemaAction::Tolerate) { + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + return true; + } + + if ($action === SchemaAction::DropAndRecreate) { + // Drop then fall through to the normal create flow below. $dbForDatabases->deleteAttribute( 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), $resource->getKey() @@ -727,6 +735,7 @@ protected function createField(Column|Attribute $resource): bool } } } + // SchemaAction::Create → fall through to the normal create flow. } try { @@ -1007,24 +1016,28 @@ protected function createIndex(Index $resource): bool ); } - // Skip/Upsert: pre-check against `indexes` metadata. Same rule as - // attributes — Skip tolerates unconditionally; Upsert tolerates unless - // source's updatedAt is strictly newer, in which case the index is - // dropped + recreated. Index drops are non-destructive (no row data - // lives in indexes) so rebuild cost is just the index build time. + // Skip/Upsert: pre-check against `indexes` metadata via one + // resolveSchemaAction call. Same rule as attributes, except that + // index drops are non-destructive (no row data lives in indexes) so + // rebuild cost is just the index build time. Fail short-circuits. $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); - if ($this->onDuplicate->toleratesSchemaDuplicate()) { + if ($this->onDuplicate !== OnDuplicate::Fail) { $existingIdx = $this->dbForProject->getDocument('indexes', $indexMetaId); - if (!$existingIdx->isEmpty()) { - if (!$this->onDuplicate->shouldReconcileSchema($updatedAt, $existingIdx->getUpdatedAt())) { - // Skip, or Upsert with dest up-to-date — tolerate. - $this->dbForProject->purgeCachedDocument( - 'database_' . $database->getSequence(), - $table->getId() - ); - return true; - } - // Source spec newer: drop then recreate. + $action = $this->onDuplicate->resolveSchemaAction( + !$existingIdx->isEmpty(), + $updatedAt, + $existingIdx->getUpdatedAt(), + ); + + if ($action === SchemaAction::Tolerate) { + $this->dbForProject->purgeCachedDocument( + 'database_' . $database->getSequence(), + $table->getId() + ); + return true; + } + + if ($action === SchemaAction::DropAndRecreate) { $dbForDatabases->deleteIndex( 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), $resource->getKey() @@ -1035,6 +1048,7 @@ protected function createIndex(Index $resource): bool $table->getId() ); } + // SchemaAction::Create → fall through to the normal create flow. } $index = $this->dbForProject->createDocument('indexes', $index); diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 2e3918c4..ddb3d768 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -2,6 +2,23 @@ namespace Utopia\Migration\Destinations; +/** + * Caller branches on one of these three outcomes after asking + * {@see OnDuplicate::resolveSchemaAction()} what to do with a possibly + * pre-existing schema resource on the destination. + */ +enum SchemaAction +{ + /** Resource doesn't exist — run the normal create flow. */ + case Create; + + /** Resource exists; leave it alone (Skip, or Upsert with dest up-to-date). */ + case Tolerate; + + /** Resource exists; Upsert mode + source strictly newer — drop + recreate. */ + case DropAndRecreate; +} + /** * Behavior when a destination row with an existing ID is encountered. */ @@ -20,30 +37,42 @@ public static function values(): array } /** - * Skip and Upsert tolerate an existing schema resource on re-migration; - * Fail rethrows. Callers gate their pre-check on this before issuing a - * `getDocument` against the destination metadata — on Fail mode the - * library's own `DuplicateException` surfaces from the create call below. + * Single decision point for schema-level reconciliation on re-migration. + * + * Callers typically short-circuit on Fail mode before invoking this (to + * avoid the destination metadata lookup entirely — the library's own + * DuplicateException surfaces from the create call as designed). + * + * For containers whose data must be preserved (databases, tables), + * callers pass $allowDrop = false so the method never returns + * DropAndRecreate — only Create or Tolerate. */ - public function toleratesSchemaDuplicate(): bool - { - return $this !== self::Fail; + public function resolveSchemaAction( + bool $exists, + string $sourceUpdatedAt = '', + string $destUpdatedAt = '', + bool $allowDrop = true, + ): SchemaAction { + if (!$exists) { + return SchemaAction::Create; + } + return match ($this) { + self::Fail => SchemaAction::Create, // caller's create flow will throw + self::Skip => SchemaAction::Tolerate, + self::Upsert => ($allowDrop && $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt)) + ? SchemaAction::DropAndRecreate + : SchemaAction::Tolerate, + }; } /** - * Upsert reconciliation trigger: true iff mode is Upsert AND source's - * updatedAt is strictly newer than destination's. Skip and Fail always - * return false (they don't reconcile). Unparseable timestamps → false - * (conservative: preserve the existing destination rather than risk a - * destructive drop on garbage input). + * Unparseable timestamps → false (conservative: preserve the existing + * destination rather than risk a destructive drop on garbage input). */ - public function shouldReconcileSchema(string $sourceUpdatedAt, string $destUpdatedAt): bool + private function sourceIsNewer(string $source, string $dest): bool { - if ($this !== self::Upsert) { - return false; - } - $src = \strtotime($sourceUpdatedAt); - $dst = \strtotime($destUpdatedAt); + $src = \strtotime($source); + $dst = \strtotime($dest); if ($src === false || $dst === false) { return false; } From 6ee988247b186f03447191ef21bcd6ede740cb28 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 23 Apr 2026 00:00:57 +0100 Subject: [PATCH 06/36] Refactor: extract deleteAttributeCompletely, drop dead $allowDrop, behavioral comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review improvements to the schema-tolerance work: 1. OnDuplicate::resolveSchemaAction — remove the $allowDrop parameter. No caller passes it; all four call sites rely on the default. Containers (databases, tables) bypass the method entirely with an inline "!== Fail" check. The parameter advertised tuning that wasn't real, confusing readers about what was actually gated. Docstring tightened to mention that this method is only safe for leaf resources. 2. Extract deleteAttributeCompletely() as a reusable primitive that fully removes an attribute from destination — both metadata documents AND physical columns, mirroring createField's two-document write for two-way relationships. The Upsert drop block in createField shrinks from ~40 nested lines to a single method call; the two-way cleanup is now a named concern that future non-Upsert paths (Merge mode, explicit rollback, etc.) can reuse without duplication. 3. Replace line-number comments ("the first-run createField wrote TWO metadata docs (parent at line 763, child at line 819)") with behavioral descriptions ("createField writes a second `attributes` document on the related table"). Line references rot on every edit; behavioral descriptions don't. Pure refactor — no functional change. Pint + PHPStan L3 clean. --- src/Migration/Destinations/Appwrite.php | 117 ++++++++++++++------- src/Migration/Destinations/OnDuplicate.php | 11 +- 2 files changed, 85 insertions(+), 43 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 992b0e2e..079a948c 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -695,45 +695,14 @@ protected function createField(Column|Attribute $resource): bool } if ($action === SchemaAction::DropAndRecreate) { - // Drop then fall through to the normal create flow below. - $dbForDatabases->deleteAttribute( - 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), - $resource->getKey() + $this->deleteAttributeCompletely( + $database, + $table, + $resource, + $type, + $relatedTable ?? null, + $dbForDatabases, ); - $this->dbForProject->deleteDocument('attributes', $attributeMetaId); - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); - - // Two-way relationships: the first-run createField wrote TWO - // metadata docs (parent at line 763, child at line 819). The - // drop above only removed the parent; without cleaning up the - // child, the recreate path hits DuplicateException at line 819 - // and the whole attribute re-migration fails. - $resourceOptions = $resource->getOptions(); - if ($type === UtopiaDatabase::VAR_RELATIONSHIP && !empty($resourceOptions['twoWay'])) { - // $relatedTable was populated at the top of createField - // inside the same VAR_RELATIONSHIP branch. - $childTwoWayKey = $resourceOptions['twoWayKey'] ?? ''; - if ($childTwoWayKey !== '') { - $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $childTwoWayKey; - try { - $this->dbForProject->deleteDocument('attributes', $childMetaId); - } catch (\Throwable) { - // Child metadata already gone — interrupted prior run, nothing to do. - } - try { - $dbForDatabases->deleteAttribute( - 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(), - $childTwoWayKey - ); - } catch (\Throwable) { - // Child column already gone — the parent's deleteAttribute may - // have cascaded via utopia-php/database's relationship handling. - } - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence()); - } - } } // SchemaAction::Create → fall through to the normal create flow. } @@ -1225,6 +1194,78 @@ protected function createRecord(Row $resource, bool $isLast): bool return true; } + /** + * Fully remove an attribute from destination — both metadata documents + * and the physical column(s) — so the caller can create it fresh without + * colliding with any remnants. + * + * For plain attributes this is a parent-side cleanup. For two-way + * relationship attributes, createField writes a second `attributes` + * document on the related table (under {dbSeq}_{relatedTableSeq}_{twoWayKey}) + * in addition to the parent-side document. Dropping only the parent + * side leaves the child document dangling and a subsequent recreate + * collides on it with DuplicateException. This method mirrors + * createField's two-doc write so the "reconcile" path is symmetric. + * + * Physical cleanup on the related table is best-effort: the library's + * deleteAttribute on the parent side can cascade to the child column + * depending on relationship handling, so the second deleteAttribute may + * no-op or NotFoundException — both swallowed. + * + * @param UtopiaDocument|null $relatedTable null when the attribute isn't + * a two-way relationship; + * required otherwise. + */ + private function deleteAttributeCompletely( + UtopiaDocument $database, + UtopiaDocument $table, + Column|Attribute $resource, + string $type, + ?UtopiaDocument $relatedTable, + UtopiaDatabase $dbForDatabases, + ): void { + $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); + $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); + + // Parent side. + $dbForDatabases->deleteAttribute($collectionId, $resource->getKey()); + $this->dbForProject->deleteDocument('attributes', $attributeMetaId); + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $dbForDatabases->purgeCachedCollection($collectionId); + + // Child side, only for two-way relationships. createField writes a + // second `attributes` document keyed on the related table; mirror + // that write here. + if ($type !== UtopiaDatabase::VAR_RELATIONSHIP || $relatedTable === null) { + return; + } + $options = $resource->getOptions(); + if (empty($options['twoWay'])) { + return; + } + $twoWayKey = $options['twoWayKey'] ?? ''; + if ($twoWayKey === '') { + return; + } + + $childCollectionId = 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(); + $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $twoWayKey; + + try { + $this->dbForProject->deleteDocument('attributes', $childMetaId); + } catch (\Throwable) { + // Child metadata already gone — interrupted prior run. + } + try { + $dbForDatabases->deleteAttribute($childCollectionId, $twoWayKey); + } catch (\Throwable) { + // Physical column already gone — parent-side deleteAttribute may + // have cascaded via utopia-php/database relationship handling. + } + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); + $dbForDatabases->purgeCachedCollection($childCollectionId); + } + /** * @throws \Exception */ diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index ddb3d768..84937a8a 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -43,15 +43,16 @@ public static function values(): array * avoid the destination metadata lookup entirely — the library's own * DuplicateException surfaces from the create call as designed). * - * For containers whose data must be preserved (databases, tables), - * callers pass $allowDrop = false so the method never returns - * DropAndRecreate — only Create or Tolerate. + * Only safe to call for leaf resources (attributes, indexes) that can be + * dropped to reconcile. Containers whose data must be preserved + * (databases, tables) should use the simpler "tolerate existing" inline + * check instead — this method's DropAndRecreate outcome would destroy + * their user data. */ public function resolveSchemaAction( bool $exists, string $sourceUpdatedAt = '', string $destUpdatedAt = '', - bool $allowDrop = true, ): SchemaAction { if (!$exists) { return SchemaAction::Create; @@ -59,7 +60,7 @@ public function resolveSchemaAction( return match ($this) { self::Fail => SchemaAction::Create, // caller's create flow will throw self::Skip => SchemaAction::Tolerate, - self::Upsert => ($allowDrop && $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt)) + self::Upsert => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) ? SchemaAction::DropAndRecreate : SchemaAction::Tolerate, }; From cfba224f73f00d236a748553d87381a4cf77d0f6 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 23 Apr 2026 00:09:03 +0100 Subject: [PATCH 07/36] Add unit tests for OnDuplicate::resolveSchemaAction; harden zero-date handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveSchemaAction is the load-bearing decision point for re-migration tolerance. Unit test locks the full mode × existence × timestamp matrix so a future refactor can't silently shift the mapping. 14 tests, sub-millisecond to run, no destination spin-up required. Coverage: - Not-exists returns Create for all modes. - Fail + exists → Create (routed through create flow so library DuplicateException surfaces as designed, not silently tolerated). - Skip ignores timestamps entirely — "don't touch" contract enforced by verifying both timestamp orderings produce Tolerate. - Upsert with source strictly newer → DropAndRecreate. - Upsert with dest newer or equal timestamps → Tolerate (strict > gate avoids unnecessary destructive rebuild when nothing has changed). - Upsert with unparseable timestamps → Tolerate (conservative, dataset covers empty strings, '0000-00-00 00:00:00', and non-date garbage). - OnDuplicate::values() case ordering guarded against accidental rename/reorder that would break SDK param validators. Writing the test surfaced a real gap in sourceIsNewer: PHP's strtotime() accepts '0000-00-00 00:00:00' leniently and returns a large negative epoch (-62169984000) rather than false. The previous `=== false` check missed the MySQL zero-date sentinel and would have returned true when source was a valid date and dest was '0000-00-00', triggering a destructive drop on garbage input. Harden sourceIsNewer to additionally reject non-positive epochs (<= 0) — any legitimate Appwrite updatedAt is well past 1970-01-01. --- src/Migration/Destinations/OnDuplicate.php | 11 +- .../Unit/General/OnDuplicateTest.php | 140 ++++++++++++++++++ 2 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 tests/Migration/Unit/General/OnDuplicateTest.php diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 84937a8a..19116b24 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -67,14 +67,19 @@ public function resolveSchemaAction( } /** - * Unparseable timestamps → false (conservative: preserve the existing - * destination rather than risk a destructive drop on garbage input). + * Unparseable or clearly-invalid timestamps → false (conservative: + * preserve the existing destination rather than risk a destructive drop + * on garbage input). PHP's strtotime() accepts some malformed dates + * leniently — '0000-00-00 00:00:00' for example parses to a large + * negative epoch rather than returning false — so we also reject + * non-positive epochs. Any legitimate Appwrite updatedAt is well past + * 1970-01-01. */ private function sourceIsNewer(string $source, string $dest): bool { $src = \strtotime($source); $dst = \strtotime($dest); - if ($src === false || $dst === false) { + if ($src === false || $dst === false || $src <= 0 || $dst <= 0) { return false; } return $src > $dst; diff --git a/tests/Migration/Unit/General/OnDuplicateTest.php b/tests/Migration/Unit/General/OnDuplicateTest.php new file mode 100644 index 00000000..adebac13 --- /dev/null +++ b/tests/Migration/Unit/General/OnDuplicateTest.php @@ -0,0 +1,140 @@ +assertSame( + SchemaAction::Create, + $mode->resolveSchemaAction(exists: false), + "{$mode->value} on non-existing resource must return Create", + ); + } + } + + public function testFailExistsReturnsCreateSoCallerDDLThrows(): void + { + // Fail is routed through Create (not Tolerate) so the caller's normal + // create flow runs and the library surfaces DuplicateException as + // designed. Returning Tolerate here would silently hide the error. + $this->assertSame( + SchemaAction::Create, + OnDuplicate::Fail->resolveSchemaAction(exists: true), + ); + } + + public function testSkipAlwaysToleratesExisting(): void + { + // Skip must ignore timestamps entirely — it's the "don't touch" + // contract. Exercise both orderings to prove the comparison isn't + // consulted. + $this->assertSame( + SchemaAction::Tolerate, + OnDuplicate::Skip->resolveSchemaAction( + exists: true, + sourceUpdatedAt: '2026-01-01T00:00:00.000+00:00', + destUpdatedAt: '2020-01-01T00:00:00.000+00:00', + ), + ); + $this->assertSame( + SchemaAction::Tolerate, + OnDuplicate::Skip->resolveSchemaAction( + exists: true, + sourceUpdatedAt: '2020-01-01T00:00:00.000+00:00', + destUpdatedAt: '2026-01-01T00:00:00.000+00:00', + ), + ); + } + + public function testUpsertSourceStrictlyNewerReconciles(): void + { + $this->assertSame( + SchemaAction::DropAndRecreate, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', + destUpdatedAt: '2026-04-23T09:59:59.000+00:00', + ), + ); + } + + public function testUpsertDestNewerTolerates(): void + { + $this->assertSame( + SchemaAction::Tolerate, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceUpdatedAt: '2026-04-23T09:00:00.000+00:00', + destUpdatedAt: '2026-04-23T10:00:00.000+00:00', + ), + ); + } + + public function testUpsertEqualTimestampsTolerates(): void + { + // Strict > comparison: equal means dest is already in sync, skip the + // drop. Avoids unnecessary destructive rebuild when the user hasn't + // touched source since the last migration. + $stamp = '2026-04-23T10:00:00.000+00:00'; + $this->assertSame( + SchemaAction::Tolerate, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceUpdatedAt: $stamp, + destUpdatedAt: $stamp, + ), + ); + } + + /** + * @return array + */ + public static function unparseableTimestampPairs(): array + { + return [ + 'both empty' => ['', ''], + 'source empty' => ['', '2026-04-23T10:00:00.000+00:00'], + 'dest empty' => ['2026-04-23T10:00:00.000+00:00', ''], + 'source zero-date' => ['0000-00-00 00:00:00', '2026-04-23T10:00:00.000+00:00'], + 'dest zero-date' => ['2026-04-23T10:00:00.000+00:00', '0000-00-00 00:00:00'], + 'source garbage' => ['not-a-date', '2026-04-23T10:00:00.000+00:00'], + 'dest garbage' => ['2026-04-23T10:00:00.000+00:00', 'not-a-date'], + ]; + } + + #[DataProvider('unparseableTimestampPairs')] + public function testUpsertUnparseableTimestampsTolerate(string $source, string $dest): void + { + // Conservative: unparseable timestamps preserve existing destination + // rather than risk a destructive drop on garbage input. Any + // non-Appwrite source that emits malformed dates gets handled safely. + $this->assertSame( + SchemaAction::Tolerate, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceUpdatedAt: $source, + destUpdatedAt: $dest, + ), + ); + } + + public function testValuesListsAllCasesInDeclarationOrder(): void + { + // The values() helper is consumed by API/SDK param validators; this + // tests protects against an accidental case-rename or reorder. + $this->assertSame(['fail', 'skip', 'upsert'], OnDuplicate::values()); + } +} From e08d531d7cab9c6aeed4fa28cfe326449b0dd3b0 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 23 Apr 2026 06:05:36 +0100 Subject: [PATCH 08/36] Upsert: update database/table metadata in-place when source is newer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the Upsert reconciliation to cover container metadata drift. Previously databases and tables always tolerated an existing destination under Upsert — source renames, enable toggles, and (for tables) permission changes silently never reached destination on re-migration. Now Upsert + source strictly newer triggers an updateDocument on the container's own metadata doc, leaving children (rows, sub-resources) untouched. Mechanism: - New SchemaAction::UpdateInPlace case alongside Create / Tolerate / DropAndRecreate. - OnDuplicate::resolveSchemaAction gains a canDrop parameter (default false — safer default, destructive reconciliation must be opt-in): canDrop: true → DropAndRecreate on Upsert-newer (attributes, indexes) canDrop: false → UpdateInPlace on Upsert-newer (databases, tables) - createField / createIndex pass canDrop: true. - createDatabase / createEntity rely on the default and handle SchemaAction::UpdateInPlace by calling updateDocument with a scoped field set: Database → name, search, enabled, type, originalId, database, $updatedAt Table → name, search, enabled, $permissions, documentSecurity, $updatedAt Immutable fields ($id, $createdAt, internal sequences) are never touched. Source-wins semantics for table permissions follow the full-Upsert contract: if the caller wants destination drift preserved across re-runs, they should pick Skip mode, not Upsert. Row Upsert already works this way; containers now match. Unit tests cover both canDrop branches and confirm Skip never returns UpdateInPlace regardless of timestamps (reinforcing the "don't touch" contract). 16 tests, pint + phpstan L3 clean. --- src/Migration/Destinations/Appwrite.php | 67 ++++++++++++++++--- src/Migration/Destinations/OnDuplicate.php | 36 +++++++--- .../Unit/General/OnDuplicateTest.php | 35 +++++++++- 3 files changed, 120 insertions(+), 18 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 079a948c..0d4b0768 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -424,20 +424,40 @@ protected function createDatabase(Database $resource): bool // Skip/Upsert: pre-check an existing database. Databases contain the // entire tree of collections + rows, so they are never destructively - // reconciled — both modes simply hydrate the sequence from the - // existing metadata document and return. Downstream resources - // (tables/columns/rows) locate the existing underlying schema via the - // hydrated sequence. + // reconciled — under Upsert-newer the container metadata is updated + // in place (name, enabled, etc.) but children are untouched. // // Fail mode short-circuits: no pre-check, let the create below throw // DuplicateException as designed. Saves N metadata reads on the // common first-time-migration path. if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument('databases', $resource->getId()); - if (!$existing->isEmpty()) { + $action = $this->onDuplicate->resolveSchemaAction( + !$existing->isEmpty(), + $updatedAt, + $existing->getUpdatedAt(), + // canDrop: false by default — databases hold child tables + rows. + ); + + if ($action === SchemaAction::Tolerate) { + $resource->setSequence($existing->getSequence()); + return true; + } + + if ($action === SchemaAction::UpdateInPlace) { + $this->dbForProject->updateDocument('databases', $existing->getId(), new UtopiaDocument([ + 'name' => $resource->getDatabaseName(), + 'search' => implode(' ', [$resource->getId(), $resource->getDatabaseName()]), + 'enabled' => $resource->getEnabled(), + 'type' => empty($resource->getType()) ? 'legacy' : $resource->getType(), + 'originalId' => empty($resource->getOriginalId()) ? null : $resource->getOriginalId(), + 'database' => $resource->getDatabase(), + '$updatedAt' => $updatedAt, + ])); $resource->setSequence($existing->getSequence()); return true; } + // SchemaAction::Create → fall through to the normal create flow. } $database = $this->dbForProject->createDocument('databases', new UtopiaDocument([ @@ -522,18 +542,45 @@ protected function createEntity(Table $resource): bool } // Skip/Upsert: pre-check an existing table. Like databases, tables - // contain user data (rows), so both modes simply hydrate the sequence - // from the existing metadata document. Attribute/index reconciliation - // happens per-resource at a lower layer. Fail short-circuits. + // contain user data (rows) so they are never destructively + // reconciled — under Upsert-newer the container metadata is + // updated in place (name, enabled, permissions, etc.) but child + // rows are untouched. Attribute/index reconciliation happens + // per-resource at a lower layer. Fail short-circuits. if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument( 'database_' . $database->getSequence(), $resource->getId() ); - if (!$existing->isEmpty()) { + $action = $this->onDuplicate->resolveSchemaAction( + !$existing->isEmpty(), + $updatedAt, + $existing->getUpdatedAt(), + // canDrop: false by default — tables hold child rows. + ); + + if ($action === SchemaAction::Tolerate) { + $resource->setSequence($existing->getSequence()); + return true; + } + + if ($action === SchemaAction::UpdateInPlace) { + $this->dbForProject->updateDocument( + 'database_' . $database->getSequence(), + $existing->getId(), + new UtopiaDocument([ + 'name' => $resource->getTableName(), + 'search' => implode(' ', [$resource->getId(), $resource->getTableName()]), + 'enabled' => $resource->getEnabled(), + '$permissions' => Permission::aggregate($resource->getPermissions()), + 'documentSecurity' => $resource->getRowSecurity(), + '$updatedAt' => $updatedAt, + ]) + ); $resource->setSequence($existing->getSequence()); return true; } + // SchemaAction::Create → fall through to the normal create flow. } $table = $this->dbForProject->createDocument('database_' . $database->getSequence(), new UtopiaDocument([ @@ -686,6 +733,7 @@ protected function createField(Column|Attribute $resource): bool !$existingAttr->isEmpty(), $updatedAt, $existingAttr->getUpdatedAt(), + canDrop: true, // attributes are leaves; column data repopulates via row Upsert ); if ($action === SchemaAction::Tolerate) { @@ -996,6 +1044,7 @@ protected function createIndex(Index $resource): bool !$existingIdx->isEmpty(), $updatedAt, $existingIdx->getUpdatedAt(), + canDrop: true, // indexes are leaves; carry no user data ); if ($action === SchemaAction::Tolerate) { diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 19116b24..6e53218c 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -3,7 +3,7 @@ namespace Utopia\Migration\Destinations; /** - * Caller branches on one of these three outcomes after asking + * Caller branches on one of these outcomes after asking * {@see OnDuplicate::resolveSchemaAction()} what to do with a possibly * pre-existing schema resource on the destination. */ @@ -15,8 +15,22 @@ enum SchemaAction /** Resource exists; leave it alone (Skip, or Upsert with dest up-to-date). */ case Tolerate; - /** Resource exists; Upsert mode + source strictly newer — drop + recreate. */ + /** + * Resource exists; Upsert mode + source strictly newer + resource is a + * leaf (attribute/index) — drop + recreate. Data-preserving containers + * get {@see self::UpdateInPlace} instead. + */ case DropAndRecreate; + + /** + * Resource exists; Upsert mode + source strictly newer + resource is a + * container (database/table) — update metadata in place without + * touching children. Callers should only overwrite fields that are + * safe to source-wins (name, enabled, search, permissions, etc.) and + * must never touch immutable fields ($id, $createdAt, internal + * sequences). + */ + case UpdateInPlace; } /** @@ -43,16 +57,22 @@ public static function values(): array * avoid the destination metadata lookup entirely — the library's own * DuplicateException surfaces from the create call as designed). * - * Only safe to call for leaf resources (attributes, indexes) that can be - * dropped to reconcile. Containers whose data must be preserved - * (databases, tables) should use the simpler "tolerate existing" inline - * check instead — this method's DropAndRecreate outcome would destroy - * their user data. + * $canDrop picks the Upsert-newer reconciliation strategy; default + * false is the safe option — destructive reconciliation requires + * explicit opt-in at the call site: + * - true → DropAndRecreate (attributes, indexes; column data is + * repopulated by the follow-up row Upsert, or is pure + * metadata for indexes). + * - false → UpdateInPlace (databases, tables; their child rows and + * sub-resources must be preserved, so destructive + * reconciliation is replaced with an updateDocument on the + * container's own metadata document). */ public function resolveSchemaAction( bool $exists, string $sourceUpdatedAt = '', string $destUpdatedAt = '', + bool $canDrop = false, ): SchemaAction { if (!$exists) { return SchemaAction::Create; @@ -61,7 +81,7 @@ public function resolveSchemaAction( self::Fail => SchemaAction::Create, // caller's create flow will throw self::Skip => SchemaAction::Tolerate, self::Upsert => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) - ? SchemaAction::DropAndRecreate + ? ($canDrop ? SchemaAction::DropAndRecreate : SchemaAction::UpdateInPlace) : SchemaAction::Tolerate, }; } diff --git a/tests/Migration/Unit/General/OnDuplicateTest.php b/tests/Migration/Unit/General/OnDuplicateTest.php index adebac13..a4f57c0a 100644 --- a/tests/Migration/Unit/General/OnDuplicateTest.php +++ b/tests/Migration/Unit/General/OnDuplicateTest.php @@ -59,14 +59,47 @@ public function testSkipAlwaysToleratesExisting(): void ); } - public function testUpsertSourceStrictlyNewerReconciles(): void + public function testUpsertLeafSourceNewerDropsAndRecreates(): void { + // canDrop: true → leaf resource (attribute, index). Column data is + // repopulated by the follow-up row Upsert, or the resource is pure + // metadata (index) so destructive reconciliation is safe. $this->assertSame( SchemaAction::DropAndRecreate, OnDuplicate::Upsert->resolveSchemaAction( exists: true, sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', destUpdatedAt: '2026-04-23T09:59:59.000+00:00', + canDrop: true, + ), + ); + } + + public function testUpsertContainerSourceNewerUpdatesInPlace(): void + { + // Default canDrop: false → container resource (database, table). + // Container's metadata doc gets updateDocument; children (tables, + // rows) are preserved untouched. + $this->assertSame( + SchemaAction::UpdateInPlace, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', + destUpdatedAt: '2026-04-23T09:59:59.000+00:00', + ), + ); + } + + public function testSkipNeverUpdatesContainerEvenWhenSourceNewer(): void + { + // Skip is strict "don't touch" — must never return UpdateInPlace, + // only Tolerate, regardless of timestamps or resource kind. + $this->assertSame( + SchemaAction::Tolerate, + OnDuplicate::Skip->resolveSchemaAction( + exists: true, + sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', + destUpdatedAt: '2026-04-23T09:59:59.000+00:00', ), ); } From 2bf1c83283300595e1d26d4f81a6c551219c8bee Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 23 Apr 2026 07:00:58 +0100 Subject: [PATCH 09/36] Trim verbose comments, keep only essential 'why' explanations --- src/Migration/Destinations/Appwrite.php | 64 +++------------------- src/Migration/Destinations/OnDuplicate.php | 55 +++---------------- 2 files changed, 17 insertions(+), 102 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 0d4b0768..7b06c729 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -422,21 +422,13 @@ protected function createDatabase(Database $resource): bool $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); - // Skip/Upsert: pre-check an existing database. Databases contain the - // entire tree of collections + rows, so they are never destructively - // reconciled — under Upsert-newer the container metadata is updated - // in place (name, enabled, etc.) but children are untouched. - // - // Fail mode short-circuits: no pre-check, let the create below throw - // DuplicateException as designed. Saves N metadata reads on the - // common first-time-migration path. + // Fail mode skips the pre-check so library's DuplicateException surfaces on re-migration. if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument('databases', $resource->getId()); $action = $this->onDuplicate->resolveSchemaAction( !$existing->isEmpty(), $updatedAt, $existing->getUpdatedAt(), - // canDrop: false by default — databases hold child tables + rows. ); if ($action === SchemaAction::Tolerate) { @@ -457,7 +449,6 @@ protected function createDatabase(Database $resource): bool $resource->setSequence($existing->getSequence()); return true; } - // SchemaAction::Create → fall through to the normal create flow. } $database = $this->dbForProject->createDocument('databases', new UtopiaDocument([ @@ -541,12 +532,6 @@ protected function createEntity(Table $resource): bool $dbForDatabases->create(); } - // Skip/Upsert: pre-check an existing table. Like databases, tables - // contain user data (rows) so they are never destructively - // reconciled — under Upsert-newer the container metadata is - // updated in place (name, enabled, permissions, etc.) but child - // rows are untouched. Attribute/index reconciliation happens - // per-resource at a lower layer. Fail short-circuits. if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument( 'database_' . $database->getSequence(), @@ -556,7 +541,6 @@ protected function createEntity(Table $resource): bool !$existing->isEmpty(), $updatedAt, $existing->getUpdatedAt(), - // canDrop: false by default — tables hold child rows. ); if ($action === SchemaAction::Tolerate) { @@ -580,7 +564,6 @@ protected function createEntity(Table $resource): bool $resource->setSequence($existing->getSequence()); return true; } - // SchemaAction::Create → fall through to the normal create flow. } $table = $this->dbForProject->createDocument('database_' . $database->getSequence(), new UtopiaDocument([ @@ -723,9 +706,6 @@ protected function createField(Column|Attribute $resource): bool $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); $dbForDatabases = ($this->getDatabasesDB)($database); - // Skip/Upsert: pre-check against `attributes` metadata via one - // resolveSchemaAction call. Fail mode short-circuits to preserve - // zero-overhead fresh-migration behavior. $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); if ($this->onDuplicate !== OnDuplicate::Fail) { $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); @@ -733,7 +713,7 @@ protected function createField(Column|Attribute $resource): bool !$existingAttr->isEmpty(), $updatedAt, $existingAttr->getUpdatedAt(), - canDrop: true, // attributes are leaves; column data repopulates via row Upsert + canDrop: true, ); if ($action === SchemaAction::Tolerate) { @@ -752,7 +732,6 @@ protected function createField(Column|Attribute $resource): bool $dbForDatabases, ); } - // SchemaAction::Create → fall through to the normal create flow. } try { @@ -1033,10 +1012,6 @@ protected function createIndex(Index $resource): bool ); } - // Skip/Upsert: pre-check against `indexes` metadata via one - // resolveSchemaAction call. Same rule as attributes, except that - // index drops are non-destructive (no row data lives in indexes) so - // rebuild cost is just the index build time. Fail short-circuits. $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); if ($this->onDuplicate !== OnDuplicate::Fail) { $existingIdx = $this->dbForProject->getDocument('indexes', $indexMetaId); @@ -1044,7 +1019,7 @@ protected function createIndex(Index $resource): bool !$existingIdx->isEmpty(), $updatedAt, $existingIdx->getUpdatedAt(), - canDrop: true, // indexes are leaves; carry no user data + canDrop: true, ); if ($action === SchemaAction::Tolerate) { @@ -1066,7 +1041,6 @@ protected function createIndex(Index $resource): bool $table->getId() ); } - // SchemaAction::Create → fall through to the normal create flow. } $index = $this->dbForProject->createDocument('indexes', $index); @@ -1244,26 +1218,11 @@ protected function createRecord(Row $resource, bool $isLast): bool } /** - * Fully remove an attribute from destination — both metadata documents - * and the physical column(s) — so the caller can create it fresh without - * colliding with any remnants. + * Drop an attribute (metadata doc + physical column) so it can be recreated. * - * For plain attributes this is a parent-side cleanup. For two-way - * relationship attributes, createField writes a second `attributes` - * document on the related table (under {dbSeq}_{relatedTableSeq}_{twoWayKey}) - * in addition to the parent-side document. Dropping only the parent - * side leaves the child document dangling and a subsequent recreate - * collides on it with DuplicateException. This method mirrors - * createField's two-doc write so the "reconcile" path is symmetric. - * - * Physical cleanup on the related table is best-effort: the library's - * deleteAttribute on the parent side can cascade to the child column - * depending on relationship handling, so the second deleteAttribute may - * no-op or NotFoundException — both swallowed. - * - * @param UtopiaDocument|null $relatedTable null when the attribute isn't - * a two-way relationship; - * required otherwise. + * Two-way relationships require mirroring: createField writes a second + * `attributes` document on the related table keyed by twoWayKey. Dropping + * only the parent leaves that child doc dangling and the recreate collides. */ private function deleteAttributeCompletely( UtopiaDocument $database, @@ -1276,15 +1235,11 @@ private function deleteAttributeCompletely( $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); - // Parent side. $dbForDatabases->deleteAttribute($collectionId, $resource->getKey()); $this->dbForProject->deleteDocument('attributes', $attributeMetaId); $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection($collectionId); - // Child side, only for two-way relationships. createField writes a - // second `attributes` document keyed on the related table; mirror - // that write here. if ($type !== UtopiaDatabase::VAR_RELATIONSHIP || $relatedTable === null) { return; } @@ -1303,13 +1258,12 @@ private function deleteAttributeCompletely( try { $this->dbForProject->deleteDocument('attributes', $childMetaId); } catch (\Throwable) { - // Child metadata already gone — interrupted prior run. + // already gone } try { $dbForDatabases->deleteAttribute($childCollectionId, $twoWayKey); } catch (\Throwable) { - // Physical column already gone — parent-side deleteAttribute may - // have cascaded via utopia-php/database relationship handling. + // parent-side delete may have cascaded } $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); $dbForDatabases->purgeCachedCollection($childCollectionId); diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 6e53218c..1c90ec67 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -3,39 +3,16 @@ namespace Utopia\Migration\Destinations; /** - * Caller branches on one of these outcomes after asking - * {@see OnDuplicate::resolveSchemaAction()} what to do with a possibly - * pre-existing schema resource on the destination. + * Outcome of {@see OnDuplicate::resolveSchemaAction()}. */ enum SchemaAction { - /** Resource doesn't exist — run the normal create flow. */ case Create; - - /** Resource exists; leave it alone (Skip, or Upsert with dest up-to-date). */ case Tolerate; - - /** - * Resource exists; Upsert mode + source strictly newer + resource is a - * leaf (attribute/index) — drop + recreate. Data-preserving containers - * get {@see self::UpdateInPlace} instead. - */ case DropAndRecreate; - - /** - * Resource exists; Upsert mode + source strictly newer + resource is a - * container (database/table) — update metadata in place without - * touching children. Callers should only overwrite fields that are - * safe to source-wins (name, enabled, search, permissions, etc.) and - * must never touch immutable fields ($id, $createdAt, internal - * sequences). - */ case UpdateInPlace; } -/** - * Behavior when a destination row with an existing ID is encountered. - */ enum OnDuplicate: string { case Fail = 'fail'; @@ -51,22 +28,11 @@ public static function values(): array } /** - * Single decision point for schema-level reconciliation on re-migration. - * - * Callers typically short-circuit on Fail mode before invoking this (to - * avoid the destination metadata lookup entirely — the library's own - * DuplicateException surfaces from the create call as designed). + * Schema-level reconciliation decision. * - * $canDrop picks the Upsert-newer reconciliation strategy; default - * false is the safe option — destructive reconciliation requires - * explicit opt-in at the call site: - * - true → DropAndRecreate (attributes, indexes; column data is - * repopulated by the follow-up row Upsert, or is pure - * metadata for indexes). - * - false → UpdateInPlace (databases, tables; their child rows and - * sub-resources must be preserved, so destructive - * reconciliation is replaced with an updateDocument on the - * container's own metadata document). + * $canDrop = true → leaves (attributes, indexes) get DropAndRecreate on Upsert-newer. + * $canDrop = false → containers (databases, tables) get UpdateInPlace on Upsert-newer. + * Default is false so destructive reconciliation requires explicit opt-in. */ public function resolveSchemaAction( bool $exists, @@ -78,7 +44,7 @@ public function resolveSchemaAction( return SchemaAction::Create; } return match ($this) { - self::Fail => SchemaAction::Create, // caller's create flow will throw + self::Fail => SchemaAction::Create, self::Skip => SchemaAction::Tolerate, self::Upsert => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) ? ($canDrop ? SchemaAction::DropAndRecreate : SchemaAction::UpdateInPlace) @@ -87,13 +53,8 @@ public function resolveSchemaAction( } /** - * Unparseable or clearly-invalid timestamps → false (conservative: - * preserve the existing destination rather than risk a destructive drop - * on garbage input). PHP's strtotime() accepts some malformed dates - * leniently — '0000-00-00 00:00:00' for example parses to a large - * negative epoch rather than returning false — so we also reject - * non-positive epochs. Any legitimate Appwrite updatedAt is well past - * 1970-01-01. + * strtotime() accepts '0000-00-00' leniently (returns a large negative + * epoch, not false), so reject non-positive epochs too. */ private function sourceIsNewer(string $source, string $dest): bool { From bb21912e8cb501e5144b6bc6e1d0c9a6918a88d1 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 23 Apr 2026 16:35:27 +0100 Subject: [PATCH 10/36] Mark Tolerate outcomes as SKIPPED so counters reflect no-op re-migrations --- src/Migration/Destinations/Appwrite.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 7b06c729..fdc70f14 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -433,7 +433,8 @@ protected function createDatabase(Database $resource): bool if ($action === SchemaAction::Tolerate) { $resource->setSequence($existing->getSequence()); - return true; + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; } if ($action === SchemaAction::UpdateInPlace) { @@ -545,7 +546,8 @@ protected function createEntity(Table $resource): bool if ($action === SchemaAction::Tolerate) { $resource->setSequence($existing->getSequence()); - return true; + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; } if ($action === SchemaAction::UpdateInPlace) { @@ -719,7 +721,8 @@ protected function createField(Column|Attribute $resource): bool if ($action === SchemaAction::Tolerate) { $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); - return true; + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; } if ($action === SchemaAction::DropAndRecreate) { @@ -1027,7 +1030,8 @@ protected function createIndex(Index $resource): bool 'database_' . $database->getSequence(), $table->getId() ); - return true; + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; } if ($action === SchemaAction::DropAndRecreate) { From 36fdf269fe1c6fac2a1d80f5b913493c18989727 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Fri, 24 Apr 2026 05:55:29 +0100 Subject: [PATCH 11/36] Upsert orphan cleanup + nullable timestamps + refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Track attribute/index keys seen from source per (db, table); after rows land per table (and in a final sweep at end-of-run) drop destination attributes/indexes whose keys source did not emit. Source rename (delete+create) on source now propagates as a drop on destination without leaving the old column orphaned. Gated to Upsert mode only. - resolveSchemaAction now accepts nullable timestamps — attribute docs with missing $updatedAt no longer throw TypeError, they fall through to Tolerate (same conservative handling as zero-date/garbage input). - Tolerate branches mark resources as SKIPPED instead of implicit SUCCESS so re-migration counters reflect 'already exists' no-ops. - Internal refactor: consolidated 3 orphan-tracker state fields into one $orphanScopes map; extracted dropOrphansByKind(), tolerateMissing(), dropTwoWayMirror() helpers; renamed deleteAttributeCompletely to dropAttributeForRecreate. --- src/Migration/Destinations/Appwrite.php | 264 +++++++++++++++++- src/Migration/Destinations/OnDuplicate.php | 21 +- .../Unit/General/OnDuplicateTest.php | 5 +- 3 files changed, 271 insertions(+), 19 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index fdc70f14..4dbf091f 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -85,6 +85,23 @@ class Appwrite extends Destination */ private array $rowBuffer = []; + /** + * Upsert-mode orphan tracking. For each (database, table) scope we saw + * during the migration, records the attribute + index keys that came + * from source plus the docs/handles needed to reach the destination. + * Scopes are removed from this map after their cleanup has run, so a + * final sweep at end-of-migration only visits tables that had no rows. + * + * @var array, + * indexKeys: list, + * }> + */ + private array $orphanScopes = []; + /** * @param string $project * @param string $endpoint @@ -122,6 +139,22 @@ public function __construct( $this->getDatabasesDB = $getDatabasesDB; } + /** + * Upsert-mode orphan cleanup runs after a successful migration only. + * A thrown exception from the source/import loop short-circuits here + * so mid-migration failures preserve the destination as-is. + */ + #[Override] + public function run( + array $resources, + callable $callback, + string $rootResourceId = '', + string $rootResourceType = '', + ): void { + parent::run($resources, $callback, $rootResourceId, $rootResourceType); + $this->cleanupUpsertOrphans(); + } + public static function getName(): string { return 'Appwrite'; @@ -708,6 +741,8 @@ protected function createField(Column|Attribute $resource): bool $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); $dbForDatabases = ($this->getDatabasesDB)($database); + $this->trackOrphanCandidate($database, $table, 'attributeKeys', $resource->getKey(), $dbForDatabases); + $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); if ($this->onDuplicate !== OnDuplicate::Fail) { $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); @@ -726,7 +761,7 @@ protected function createField(Column|Attribute $resource): bool } if ($action === SchemaAction::DropAndRecreate) { - $this->deleteAttributeCompletely( + $this->dropAttributeForRecreate( $database, $table, $resource, @@ -963,6 +998,8 @@ protected function createIndex(Index $resource): bool $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); + $this->trackOrphanCandidate($database, $table, 'indexKeys', $resource->getKey(), $dbForDatabases); + $index = new UtopiaDocument([ '$id' => ID::custom($database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey()), 'key' => $resource->getKey(), @@ -1170,6 +1207,13 @@ protected function createRecord(Row $resource, bool $isLast): bool $databaseInternalId = $database->getSequence(); $tableInternalId = $table->getSequence(); $dbForDatabases = ($this->getDatabasesDB)($database); + + // Reconcile the destination SCHEMA before rows land: drops + // attributes/indexes source no longer declares so the + // Structure validator doesn't reject rows on orphan required + // columns. Distinct from the payload-shape pass below, which + // strips extra fields off the incoming ROW documents. + $this->cleanupUpsertOrphansForScope($this->orphanScopeKey($database, $table)); /** * This is in case an attribute was deleted from Appwrite attributes collection but was not deleted from the table * When creating an archive we select * which will include orphan attribute from the schema @@ -1228,7 +1272,7 @@ protected function createRecord(Row $resource, bool $isLast): bool * `attributes` document on the related table keyed by twoWayKey. Dropping * only the parent leaves that child doc dangling and the recreate collides. */ - private function deleteAttributeCompletely( + private function dropAttributeForRecreate( UtopiaDocument $database, UtopiaDocument $table, Column|Attribute $resource, @@ -1251,28 +1295,226 @@ private function deleteAttributeCompletely( if (empty($options['twoWay'])) { return; } - $twoWayKey = $options['twoWayKey'] ?? ''; + $twoWayKey = (string) ($options['twoWayKey'] ?? ''); if ($twoWayKey === '') { return; } + $this->dropTwoWayMirror($database, $relatedTable, $twoWayKey, $dbForDatabases); + } + + /** + * Drop the child-side `attributes` metadata doc + physical column that + * createField writes for two-way relationships. Shared by the DropAndRecreate + * path (pre-fetched $relatedTable) and the orphan cleanup path. + */ + private function dropTwoWayMirror( + UtopiaDocument $database, + UtopiaDocument $relatedTable, + string $twoWayKey, + UtopiaDatabase $dbForDatabases, + ): void { $childCollectionId = 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(); $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $twoWayKey; - try { - $this->dbForProject->deleteDocument('attributes', $childMetaId); - } catch (\Throwable) { - // already gone + $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $childMetaId)); + $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($childCollectionId, $twoWayKey)); + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); + $dbForDatabases->purgeCachedCollection($childCollectionId); + } + + /** + * Deterministic per-(database, table) key used to index the orphan + * tracker. Built from sequences so it's stable across calls but unique + * within the run. + */ + private function orphanScopeKey(UtopiaDocument $database, UtopiaDocument $table): string + { + return $database->getSequence() . ':' . $table->getSequence(); + } + + /** + * Records that $key was seen for this (database, table) during the run. + * $kind selects the tracker bucket ('attributeKeys' or 'indexKeys'). Only + * Upsert mode accumulates; Skip/Fail short-circuit. + */ + private function trackOrphanCandidate( + UtopiaDocument $database, + UtopiaDocument $table, + string $kind, + string $key, + UtopiaDatabase $dbForDatabases, + ): void { + if ($this->onDuplicate !== OnDuplicate::Upsert) { + return; } - try { - $dbForDatabases->deleteAttribute($childCollectionId, $twoWayKey); - } catch (\Throwable) { - // parent-side delete may have cascaded + $scopeKey = $this->orphanScopeKey($database, $table); + if (!isset($this->orphanScopes[$scopeKey])) { + $this->orphanScopes[$scopeKey] = [ + 'database' => $database, + 'table' => $table, + 'dbForDatabases' => $dbForDatabases, + 'attributeKeys' => [], + 'indexKeys' => [], + ]; + } + $this->orphanScopes[$scopeKey][$kind][] = $key; + } + + /** + * End-of-migration sweep: cleans up any scope not already handled by a + * per-table cleanup in createRecord (i.e. tables that had no rows). + * Skipped if the migration failed before run() completed. + */ + private function cleanupUpsertOrphans(): void + { + foreach (\array_keys($this->orphanScopes) as $scopeKey) { + $this->cleanupUpsertOrphansForScope($scopeKey); + } + } + + /** + * Drops destination attributes/indexes whose keys weren't observed from + * source for this scope. Called per-table from createRecord before rows + * land so the Structure validator sees the post-cleanup schema. After + * running, the scope is removed from the tracker so the end-of-run + * sweep doesn't re-do the work. + */ + private function cleanupUpsertOrphansForScope(string $scopeKey): void + { + if ($this->onDuplicate !== OnDuplicate::Upsert) { + return; } + if (!isset($this->orphanScopes[$scopeKey])) { + return; + } + + $scope = $this->orphanScopes[$scopeKey]; + $database = $scope['database']; + $table = $scope['table']; + $dbForDatabases = $scope['dbForDatabases']; + + $this->dropOrphansByKind( + 'attributes', + $scope['attributeKeys'], + $database, + $table, + fn (UtopiaDocument $doc) => $this->dropOrphanAttribute($database, $table, $doc, $dbForDatabases), + ); + + $this->dropOrphansByKind( + 'indexes', + $scope['indexKeys'], + $database, + $table, + fn (UtopiaDocument $doc) => $this->dropOrphanIndex( + $database, + $table, + (string) $doc->getAttribute('key'), + $dbForDatabases, + ), + ); + + unset($this->orphanScopes[$scopeKey]); + } + + /** + * Finds destination metadata docs in $metaCollection belonging to this + * (database, table) and invokes $drop for each one whose key isn't in + * $processedKeys. Shared by the attribute and index cleanup paths. + * + * @param list $processedKeys + * @param callable(UtopiaDocument): void $drop + */ + private function dropOrphansByKind( + string $metaCollection, + array $processedKeys, + UtopiaDocument $database, + UtopiaDocument $table, + callable $drop, + ): void { + $destDocs = $this->dbForProject->find($metaCollection, [ + Query::equal('databaseInternalId', [$database->getSequence()]), + Query::equal('collectionInternalId', [$table->getSequence()]), + Query::limit(PHP_INT_MAX), + ]); + foreach ($destDocs as $destDoc) { + if (!\in_array($destDoc->getAttribute('key'), $processedKeys, true)) { + $drop($destDoc); + } + } + } + + /** + * Drop an orphan attribute (metadata doc + physical column). Uses the + * destination's own metadata document as the source of truth for type + * and relationship options, since there's no source resource to consult. + */ + private function dropOrphanAttribute( + UtopiaDocument $database, + UtopiaDocument $table, + UtopiaDocument $attrDoc, + UtopiaDatabase $dbForDatabases, + ): void { + $key = (string) $attrDoc->getAttribute('key'); + $type = (string) $attrDoc->getAttribute('type'); + $options = $attrDoc->getAttribute('options', []); + $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); + + $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($collectionId, $key)); + $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $attrDoc->getId())); + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $dbForDatabases->purgeCachedCollection($collectionId); + + if ($type !== UtopiaDatabase::VAR_RELATIONSHIP || empty($options['twoWay'])) { + return; + } + $twoWayKey = (string) ($options['twoWayKey'] ?? ''); + $relatedTableId = (string) ($options['relatedCollection'] ?? ''); + if ($twoWayKey === '' || $relatedTableId === '') { + return; + } + $relatedTable = $this->dbForProject->getDocument('database_' . $database->getSequence(), $relatedTableId); + if ($relatedTable->isEmpty()) { + return; + } + + $childCollectionId = 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(); + $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $twoWayKey; + $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $childMetaId)); + $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($childCollectionId, $twoWayKey)); $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); $dbForDatabases->purgeCachedCollection($childCollectionId); } + private function dropOrphanIndex( + UtopiaDocument $database, + UtopiaDocument $table, + string $indexKey, + UtopiaDatabase $dbForDatabases, + ): void { + $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); + $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $indexKey; + + $this->tolerateMissing(fn () => $dbForDatabases->deleteIndex($collectionId, $indexKey)); + $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('indexes', $indexMetaId)); + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + } + + /** + * Swallows deletion errors — a prior interrupted run (or parent-side + * cascade via utopia-php/database relationship handling) may already + * have removed the target. + */ + private function tolerateMissing(callable $fn): void + { + try { + $fn(); + } catch (\Throwable) { + // already gone + } + } + /** * @throws \Exception */ diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 1c90ec67..a40b50e1 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -3,7 +3,10 @@ namespace Utopia\Migration\Destinations; /** - * Outcome of {@see OnDuplicate::resolveSchemaAction()}. + * Outcome of {@see OnDuplicate::resolveSchemaAction()}. Declared alongside + * OnDuplicate because the two are designed together — any code that uses + * SchemaAction necessarily imports OnDuplicate (as the source of the + * decision), so the shared file satisfies autoloading in practice. */ enum SchemaAction { @@ -20,11 +23,11 @@ enum OnDuplicate: string case Upsert = 'upsert'; /** - * @return array + * @return list */ public static function values(): array { - return \array_map(fn (self $case) => $case->value, self::cases()); + return \array_values(\array_map(fn (self $case) => $case->value, self::cases())); } /** @@ -36,8 +39,8 @@ public static function values(): array */ public function resolveSchemaAction( bool $exists, - string $sourceUpdatedAt = '', - string $destUpdatedAt = '', + ?string $sourceUpdatedAt = null, + ?string $destUpdatedAt = null, bool $canDrop = false, ): SchemaAction { if (!$exists) { @@ -54,10 +57,14 @@ public function resolveSchemaAction( /** * strtotime() accepts '0000-00-00' leniently (returns a large negative - * epoch, not false), so reject non-positive epochs too. + * epoch, not false), so reject non-positive epochs too. Null/empty are + * treated as unknown → tolerate rather than risk a destructive drop. */ - private function sourceIsNewer(string $source, string $dest): bool + private function sourceIsNewer(?string $source, ?string $dest): bool { + if ($source === null || $source === '' || $dest === null || $dest === '') { + return false; + } $src = \strtotime($source); $dst = \strtotime($dest); if ($src === false || $dst === false || $src <= 0 || $dst <= 0) { diff --git a/tests/Migration/Unit/General/OnDuplicateTest.php b/tests/Migration/Unit/General/OnDuplicateTest.php index a4f57c0a..42ffa296 100644 --- a/tests/Migration/Unit/General/OnDuplicateTest.php +++ b/tests/Migration/Unit/General/OnDuplicateTest.php @@ -139,8 +139,11 @@ public static function unparseableTimestampPairs(): array { return [ 'both empty' => ['', ''], + 'both null' => [null, null], 'source empty' => ['', '2026-04-23T10:00:00.000+00:00'], 'dest empty' => ['2026-04-23T10:00:00.000+00:00', ''], + 'source null' => [null, '2026-04-23T10:00:00.000+00:00'], + 'dest null' => ['2026-04-23T10:00:00.000+00:00', null], 'source zero-date' => ['0000-00-00 00:00:00', '2026-04-23T10:00:00.000+00:00'], 'dest zero-date' => ['2026-04-23T10:00:00.000+00:00', '0000-00-00 00:00:00'], 'source garbage' => ['not-a-date', '2026-04-23T10:00:00.000+00:00'], @@ -149,7 +152,7 @@ public static function unparseableTimestampPairs(): array } #[DataProvider('unparseableTimestampPairs')] - public function testUpsertUnparseableTimestampsTolerate(string $source, string $dest): void + public function testUpsertUnparseableTimestampsTolerate(?string $source, ?string $dest): void { // Conservative: unparseable timestamps preserve existing destination // rather than risk a destructive drop on garbage input. Any From c2b871519bbdaf7b3a2d947baa4e88a4b407c85d Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Fri, 24 Apr 2026 07:24:44 +0100 Subject: [PATCH 12/36] Rename orphan-tracker identifiers to 'table' vocabulary - $orphanScopes -> $orphansByTable - orphanScopeKey() -> tableIdentity() - cleanupUpsertOrphansForScope() -> cleanupUpsertOrphansForTable() - $scopeKey local -> $tableId - $scope local -> $tracked (what source declared for this table) No behavior change. The old names were consumer-oriented jargon; the new names name the actual unit (one entry per table). --- src/Migration/Destinations/Appwrite.php | 58 +++++++++++++------------ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 4dbf091f..eff802d2 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -86,11 +86,13 @@ class Appwrite extends Destination private array $rowBuffer = []; /** - * Upsert-mode orphan tracking. For each (database, table) scope we saw - * during the migration, records the attribute + index keys that came - * from source plus the docs/handles needed to reach the destination. - * Scopes are removed from this map after their cleanup has run, so a - * final sweep at end-of-migration only visits tables that had no rows. + * Upsert-mode orphan tracking, keyed by (database, table). Each entry + * holds the attribute + index keys source declared for that table plus + * the docs/handles needed to reach the destination at cleanup time. + * Orphans are the complement: anything the destination has whose key + * is NOT in this record. Entries are removed from the map after their + * cleanup has run, so the final sweep at end-of-migration only visits + * tables that had no rows. * * @var array, * }> */ - private array $orphanScopes = []; + private array $orphansByTable = []; /** * @param string $project @@ -1213,7 +1215,7 @@ protected function createRecord(Row $resource, bool $isLast): bool // Structure validator doesn't reject rows on orphan required // columns. Distinct from the payload-shape pass below, which // strips extra fields off the incoming ROW documents. - $this->cleanupUpsertOrphansForScope($this->orphanScopeKey($database, $table)); + $this->cleanupUpsertOrphansForTable($this->tableIdentity($database, $table)); /** * This is in case an attribute was deleted from Appwrite attributes collection but was not deleted from the table * When creating an archive we select * which will include orphan attribute from the schema @@ -1325,10 +1327,10 @@ private function dropTwoWayMirror( /** * Deterministic per-(database, table) key used to index the orphan - * tracker. Built from sequences so it's stable across calls but unique - * within the run. + * tracker (`$orphansByTable`). Built from sequences so it's stable + * across calls but unique within the run. */ - private function orphanScopeKey(UtopiaDocument $database, UtopiaDocument $table): string + private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): string { return $database->getSequence() . ':' . $table->getSequence(); } @@ -1348,9 +1350,9 @@ private function trackOrphanCandidate( if ($this->onDuplicate !== OnDuplicate::Upsert) { return; } - $scopeKey = $this->orphanScopeKey($database, $table); - if (!isset($this->orphanScopes[$scopeKey])) { - $this->orphanScopes[$scopeKey] = [ + $tableId = $this->tableIdentity($database, $table); + if (!isset($this->orphansByTable[$tableId])) { + $this->orphansByTable[$tableId] = [ 'database' => $database, 'table' => $table, 'dbForDatabases' => $dbForDatabases, @@ -1358,45 +1360,45 @@ private function trackOrphanCandidate( 'indexKeys' => [], ]; } - $this->orphanScopes[$scopeKey][$kind][] = $key; + $this->orphansByTable[$tableId][$kind][] = $key; } /** - * End-of-migration sweep: cleans up any scope not already handled by a + * End-of-migration sweep: cleans up any table not already handled by a * per-table cleanup in createRecord (i.e. tables that had no rows). * Skipped if the migration failed before run() completed. */ private function cleanupUpsertOrphans(): void { - foreach (\array_keys($this->orphanScopes) as $scopeKey) { - $this->cleanupUpsertOrphansForScope($scopeKey); + foreach (\array_keys($this->orphansByTable) as $tableId) { + $this->cleanupUpsertOrphansForTable($tableId); } } /** * Drops destination attributes/indexes whose keys weren't observed from - * source for this scope. Called per-table from createRecord before rows + * source for this table. Called per-table from createRecord before rows * land so the Structure validator sees the post-cleanup schema. After - * running, the scope is removed from the tracker so the end-of-run + * running, the entry is removed from the tracker so the end-of-run * sweep doesn't re-do the work. */ - private function cleanupUpsertOrphansForScope(string $scopeKey): void + private function cleanupUpsertOrphansForTable(string $tableId): void { if ($this->onDuplicate !== OnDuplicate::Upsert) { return; } - if (!isset($this->orphanScopes[$scopeKey])) { + if (!isset($this->orphansByTable[$tableId])) { return; } - $scope = $this->orphanScopes[$scopeKey]; - $database = $scope['database']; - $table = $scope['table']; - $dbForDatabases = $scope['dbForDatabases']; + $tracked = $this->orphansByTable[$tableId]; + $database = $tracked['database']; + $table = $tracked['table']; + $dbForDatabases = $tracked['dbForDatabases']; $this->dropOrphansByKind( 'attributes', - $scope['attributeKeys'], + $tracked['attributeKeys'], $database, $table, fn (UtopiaDocument $doc) => $this->dropOrphanAttribute($database, $table, $doc, $dbForDatabases), @@ -1404,7 +1406,7 @@ private function cleanupUpsertOrphansForScope(string $scopeKey): void $this->dropOrphansByKind( 'indexes', - $scope['indexKeys'], + $tracked['indexKeys'], $database, $table, fn (UtopiaDocument $doc) => $this->dropOrphanIndex( @@ -1415,7 +1417,7 @@ private function cleanupUpsertOrphansForScope(string $scopeKey): void ), ); - unset($this->orphanScopes[$scopeKey]); + unset($this->orphansByTable[$tableId]); } /** From 791dbb16d713bf51901273ce3775fb321c83eab6 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Fri, 24 Apr 2026 07:57:57 +0100 Subject: [PATCH 13/36] Two-way relationships: track processed pairs, skip partner side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Processing one side of a two-way relationship already reconciles both metadata docs and both physical columns (via createRelationship on Create/DropAndRecreate, or via the mirror in dropTwoWayMirror on DropAndRecreate). Re-processing the partner side would either waste work or — worse — drop the mirror after the partner table's rows already migrated, destroying relationship data. Track a canonical pair-key (sorted (table, key) tuples scoped to the database) as soon as createField finishes a two-way attribute. On the partner's second pass the check at the top of createField sees the pair in $processedTwoWayPairs and short-circuits with STATUS_SKIPPED. Covers the scenario where one side's source _updatedAt is newer than the other: whichever side is processed first wins; the partner is SKIPPED regardless of its own timestamp. Data safety preferred over honoring a second, potentially-divergent schema decision. --- src/Migration/Destinations/Appwrite.php | 65 +++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index eff802d2..62a55563 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -104,6 +104,18 @@ class Appwrite extends Destination */ private array $orphansByTable = []; + /** + * Set of two-way relationship pair identities already reconciled during + * this run. Keyed by a canonical pair-key independent of which side is + * being processed. Used by createField to short-circuit the partner's + * second pass — the first side's action already reconciled both sides + * (Tolerate no-ops, DropAndRecreate drops + recreates both via + * dropTwoWayMirror + createRelationship). + * + * @var array + */ + private array $processedTwoWayPairs = []; + /** * @param string $project * @param string $endpoint @@ -745,6 +757,20 @@ protected function createField(Column|Attribute $resource): bool $this->trackOrphanCandidate($database, $table, 'attributeKeys', $resource->getKey(), $dbForDatabases); + // Two-way relationships have mirrored metadata + columns on both + // tables. Processing one side already reconciles both (Tolerate + // no-ops, DropAndRecreate drops + recreates both via + // dropTwoWayMirror + createRelationship). When the partner side + // comes through, skip it — re-processing would either double-drop + // (destroying row data on tables whose rows already migrated) or + // waste work. The first side always wins; subsequent partner is + // idempotent from the orchestrator's perspective. + $twoWayPairKey = $this->twoWayPairKey($database, $table, $resource, $type); + if ($twoWayPairKey !== null && isset($this->processedTwoWayPairs[$twoWayPairKey])) { + $resource->setStatus(Resource::STATUS_SKIPPED, 'Two-way partner already reconciled'); + return false; + } + $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); if ($this->onDuplicate !== OnDuplicate::Fail) { $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); @@ -759,6 +785,9 @@ protected function createField(Column|Attribute $resource): bool $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + if ($twoWayPairKey !== null) { + $this->processedTwoWayPairs[$twoWayPairKey] = true; + } return false; } @@ -940,6 +969,10 @@ protected function createField(Column|Attribute $resource): bool $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + if ($twoWayPairKey !== null) { + $this->processedTwoWayPairs[$twoWayPairKey] = true; + } + return true; } @@ -1335,6 +1368,38 @@ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): return $database->getSequence() . ':' . $table->getSequence(); } + /** + * Canonical identity for a two-way relationship pair — same string + * regardless of which side (parent or child) is being processed. + * Returns null when the resource isn't a two-way relationship (no + * pair-level tracking needed). + */ + private function twoWayPairKey( + UtopiaDocument $database, + UtopiaDocument $table, + Column|Attribute $resource, + string $type, + ): ?string { + if ($type !== UtopiaDatabase::VAR_RELATIONSHIP) { + return null; + } + $options = $resource->getOptions(); + if (empty($options['twoWay'])) { + return null; + } + $twoWayKey = (string) ($options['twoWayKey'] ?? ''); + $relatedTableId = (string) ($options['relatedCollection'] ?? ''); + if ($twoWayKey === '' || $relatedTableId === '') { + return null; + } + + $thisSide = $table->getId() . '::' . $resource->getKey(); + $partnerSide = $relatedTableId . '::' . $twoWayKey; + $pair = [$thisSide, $partnerSide]; + \sort($pair); + return $database->getSequence() . '@' . \implode('<->', $pair); + } + /** * Records that $key was seen for this (database, table) during the run. * $kind selects the tracker bucket ('attributeKeys' or 'indexKeys'). Only From 6e6f8255fa52c8053fa2b9ad877c7797d93e32c5 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Fri, 24 Apr 2026 11:33:58 +0100 Subject: [PATCH 14/36] Relationships: route to UpdateInPlace, delete dead drop-mirror code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relationships can't take the DropAndRecreate path cleanly: - utopia's deleteAttribute throws for VAR_RELATIONSHIP - Two-way drops cascade to the partner table's physical column and destroy its row data (partner's rows migrated earlier in the run won't get re-migrated) Route relationships through SchemaAction::UpdateInPlace instead: - canDrop: !$isRelationship on resolveSchemaAction - New updateRelationshipInPlace() helper calls $dbForDatabases->updateRelationship() for the fields utopia supports (newTwoWayKey, twoWay, onDelete) + updateDocument() on Appwrite's application-level doc - relationType changes can't be updated in place and would silently diverge utopia's internal metadata from Appwrite's doc; throw a clear error asking the caller to drop+recreate manually Each side of a two-way is authoritative for its own Appwrite-level doc. utopia's updateRelationship cascades internal metadata on both sides in one call and is idempotent for same-target writes, so neither side's pass stomps the other. Dead-code cleanup: delete dropTwoWayMirror() and the relationship branch of dropAttributeForRecreate — both were only reachable from paths that no longer fire. Simplifies dropAttributeForRecreate signature (drops unused $type and $relatedTable params). Fix dropOrphanAttribute for relationship orphans: deleteAttribute throws for VAR_RELATIONSHIP, so use deleteRelationship which cascades both sides' physical columns + utopia metadata. Clean the two Appwrite-level docs separately. All 34 unit tests pass, pint + phpstan L3 clean. --- src/Migration/Destinations/Appwrite.php | 207 ++++++++++++------------ 1 file changed, 102 insertions(+), 105 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 62a55563..d4cf2db2 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -104,18 +104,6 @@ class Appwrite extends Destination */ private array $orphansByTable = []; - /** - * Set of two-way relationship pair identities already reconciled during - * this run. Keyed by a canonical pair-key independent of which side is - * being processed. Used by createField to short-circuit the partner's - * second pass — the first side's action already reconciled both sides - * (Tolerate no-ops, DropAndRecreate drops + recreates both via - * dropTwoWayMirror + createRelationship). - * - * @var array - */ - private array $processedTwoWayPairs = []; - /** * @param string $project * @param string $endpoint @@ -757,19 +745,11 @@ protected function createField(Column|Attribute $resource): bool $this->trackOrphanCandidate($database, $table, 'attributeKeys', $resource->getKey(), $dbForDatabases); - // Two-way relationships have mirrored metadata + columns on both - // tables. Processing one side already reconciles both (Tolerate - // no-ops, DropAndRecreate drops + recreates both via - // dropTwoWayMirror + createRelationship). When the partner side - // comes through, skip it — re-processing would either double-drop - // (destroying row data on tables whose rows already migrated) or - // waste work. The first side always wins; subsequent partner is - // idempotent from the orchestrator's perspective. - $twoWayPairKey = $this->twoWayPairKey($database, $table, $resource, $type); - if ($twoWayPairKey !== null && isset($this->processedTwoWayPairs[$twoWayPairKey])) { - $resource->setStatus(Resource::STATUS_SKIPPED, 'Two-way partner already reconciled'); - return false; - } + // Relationships route to UpdateInPlace, not DropAndRecreate: + // utopia's deleteAttribute throws for VAR_RELATIONSHIP, and + // two-way drops would cascade to the partner table's physical + // column. Non-relationship attrs keep DropAndRecreate. + $isRelationship = $type === UtopiaDatabase::VAR_RELATIONSHIP; $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); if ($this->onDuplicate !== OnDuplicate::Fail) { @@ -778,28 +758,31 @@ protected function createField(Column|Attribute $resource): bool !$existingAttr->isEmpty(), $updatedAt, $existingAttr->getUpdatedAt(), - canDrop: true, + canDrop: !$isRelationship, ); if ($action === SchemaAction::Tolerate) { $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); - if ($twoWayPairKey !== null) { - $this->processedTwoWayPairs[$twoWayPairKey] = true; - } return false; } - if ($action === SchemaAction::DropAndRecreate) { - $this->dropAttributeForRecreate( + if ($action === SchemaAction::UpdateInPlace) { + $this->updateRelationshipInPlace( $database, $table, $resource, $type, - $relatedTable ?? null, + $updatedAt, + $existingAttr, $dbForDatabases, ); + return true; + } + + if ($action === SchemaAction::DropAndRecreate) { + $this->dropAttributeForRecreate($database, $table, $resource, $dbForDatabases); } } @@ -969,10 +952,6 @@ protected function createField(Column|Attribute $resource): bool $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); - if ($twoWayPairKey !== null) { - $this->processedTwoWayPairs[$twoWayPairKey] = true; - } - return true; } @@ -1301,18 +1280,16 @@ protected function createRecord(Row $resource, bool $isLast): bool } /** - * Drop an attribute (metadata doc + physical column) so it can be recreated. - * - * Two-way relationships require mirroring: createField writes a second - * `attributes` document on the related table keyed by twoWayKey. Dropping - * only the parent leaves that child doc dangling and the recreate collides. + * Drop a non-relationship attribute (metadata doc + physical column) so + * it can be recreated. Not used for relationships — those route through + * UpdateInPlace since utopia's deleteAttribute throws for VAR_RELATIONSHIP + * and dropping a relationship column would cascade data loss to the + * partner table's rows. */ private function dropAttributeForRecreate( UtopiaDocument $database, UtopiaDocument $table, Column|Attribute $resource, - string $type, - ?UtopiaDocument $relatedTable, UtopiaDatabase $dbForDatabases, ): void { $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); @@ -1322,40 +1299,79 @@ private function dropAttributeForRecreate( $this->dbForProject->deleteDocument('attributes', $attributeMetaId); $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection($collectionId); - - if ($type !== UtopiaDatabase::VAR_RELATIONSHIP || $relatedTable === null) { - return; - } - $options = $resource->getOptions(); - if (empty($options['twoWay'])) { - return; - } - $twoWayKey = (string) ($options['twoWayKey'] ?? ''); - if ($twoWayKey === '') { - return; - } - - $this->dropTwoWayMirror($database, $relatedTable, $twoWayKey, $dbForDatabases); } /** - * Drop the child-side `attributes` metadata doc + physical column that - * createField writes for two-way relationships. Shared by the DropAndRecreate - * path (pre-fetched $relatedTable) and the orphan cleanup path. + * Reconcile a relationship attribute's metadata on destination without + * dropping its physical column — keeps row data intact. + * + * Two layers must stay in sync: utopia-php/database's internal + * `_metadata` collection (used for row validation) and Appwrite's + * application-level `attributes` doc (used by the Appwrite API). Each + * side's own pass is authoritative for its own Appwrite-level doc; + * utopia's updateRelationship cascades internal metadata on both sides + * in one call and is idempotent across passes for same-target writes. + * + * relationType changes aren't supported by updateRelationship (they'd + * require a physical schema change). Fail fast rather than silently + * diverge utopia's view from Appwrite's view. */ - private function dropTwoWayMirror( + private function updateRelationshipInPlace( UtopiaDocument $database, - UtopiaDocument $relatedTable, - string $twoWayKey, + UtopiaDocument $table, + Column|Attribute $resource, + string $type, + string $updatedAt, + UtopiaDocument $existingAttr, UtopiaDatabase $dbForDatabases, ): void { - $childCollectionId = 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(); - $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $twoWayKey; + $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); + $sourceOptions = $resource->getOptions(); + $destOptions = $existingAttr->getAttribute('options', []); - $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $childMetaId)); - $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($childCollectionId, $twoWayKey)); - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); - $dbForDatabases->purgeCachedCollection($childCollectionId); + if ( + isset($sourceOptions['relationType'], $destOptions['relationType']) + && $sourceOptions['relationType'] !== $destOptions['relationType'] + ) { + throw new Exception( + resourceName: $resource->getName(), + resourceGroup: $resource->getGroup(), + resourceId: $resource->getId(), + message: 'Changing relationType on a migrated relationship is not supported; drop and recreate the relationship on the destination manually before re-running the migration.', + ); + } + + $dbForDatabases->updateRelationship( + collection: $collectionId, + id: $resource->getKey(), + newTwoWayKey: ($sourceOptions['twoWayKey'] ?? null) !== ($destOptions['twoWayKey'] ?? null) + ? (string) ($sourceOptions['twoWayKey'] ?? '') + : null, + twoWay: ($sourceOptions['twoWay'] ?? null) !== ($destOptions['twoWay'] ?? null) + ? (bool) ($sourceOptions['twoWay'] ?? false) + : null, + onDelete: ($sourceOptions['onDelete'] ?? null) !== ($destOptions['onDelete'] ?? null) + ? (string) ($sourceOptions['onDelete'] ?? '') + : null, + ); + + $this->dbForProject->updateDocument('attributes', $existingAttr->getId(), new UtopiaDocument([ + 'key' => $resource->getKey(), + 'type' => $type, + 'size' => $resource->getSize(), + 'required' => $resource->isRequired(), + 'signed' => $resource->isSigned(), + 'default' => $resource->getDefault(), + 'array' => $resource->isArray(), + 'format' => $resource->getFormat(), + 'formatOptions' => $resource->getFormatOptions(), + 'filters' => $resource->getFilters(), + 'options' => $sourceOptions, + '$updatedAt' => $updatedAt, + ])); + + $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $dbForDatabases->purgeCachedCollection($collectionId); } /** @@ -1368,38 +1384,6 @@ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): return $database->getSequence() . ':' . $table->getSequence(); } - /** - * Canonical identity for a two-way relationship pair — same string - * regardless of which side (parent or child) is being processed. - * Returns null when the resource isn't a two-way relationship (no - * pair-level tracking needed). - */ - private function twoWayPairKey( - UtopiaDocument $database, - UtopiaDocument $table, - Column|Attribute $resource, - string $type, - ): ?string { - if ($type !== UtopiaDatabase::VAR_RELATIONSHIP) { - return null; - } - $options = $resource->getOptions(); - if (empty($options['twoWay'])) { - return null; - } - $twoWayKey = (string) ($options['twoWayKey'] ?? ''); - $relatedTableId = (string) ($options['relatedCollection'] ?? ''); - if ($twoWayKey === '' || $relatedTableId === '') { - return null; - } - - $thisSide = $table->getId() . '::' . $resource->getKey(); - $partnerSide = $relatedTableId . '::' . $twoWayKey; - $pair = [$thisSide, $partnerSide]; - \sort($pair); - return $database->getSequence() . '@' . \implode('<->', $pair); - } - /** * Records that $key was seen for this (database, table) during the run. * $kind selects the tracker bucket ('attributeKeys' or 'indexKeys'). Only @@ -1516,6 +1500,12 @@ private function dropOrphansByKind( * Drop an orphan attribute (metadata doc + physical column). Uses the * destination's own metadata document as the source of truth for type * and relationship options, since there's no source resource to consult. + * + * Relationships use deleteRelationship (which cascades to both sides's + * physical columns + utopia's internal metadata) rather than + * deleteAttribute (which throws for relationship types). Appwrite's + * application-level `attributes` metadata docs on both sides are + * cleaned separately. */ private function dropOrphanAttribute( UtopiaDocument $database, @@ -1528,7 +1518,11 @@ private function dropOrphanAttribute( $options = $attrDoc->getAttribute('options', []); $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); - $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($collectionId, $key)); + if ($type === UtopiaDatabase::VAR_RELATIONSHIP) { + $this->tolerateMissing(fn () => $dbForDatabases->deleteRelationship($collectionId, $key)); + } else { + $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($collectionId, $key)); + } $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $attrDoc->getId())); $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); $dbForDatabases->purgeCachedCollection($collectionId); @@ -1546,12 +1540,15 @@ private function dropOrphanAttribute( return; } - $childCollectionId = 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(); + // Physical column on the related table was already dropped by + // deleteRelationship above. Only the Appwrite-level metadata doc + // remains to clean up. $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $twoWayKey; $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $childMetaId)); - $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($childCollectionId, $twoWayKey)); $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); - $dbForDatabases->purgeCachedCollection($childCollectionId); + $dbForDatabases->purgeCachedCollection( + 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(), + ); } private function dropOrphanIndex( From 786a27b596c5f67174f8649ea4ff7cb45720d0b7 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 07:39:40 +0100 Subject: [PATCH 15/36] Schema reconciliation: createdAt-aware actions + SDK-aligned in-place updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveSchemaAction now distinguishes 'source resource was deleted+ recreated' (createdAt diff) from 'source resource was edited' (updatedAt-newer with createdAt unchanged). The first triggers DropAndRecreate; the second triggers UpdateInPlace. Containers (databases, tables) get UpdateInPlace in both cases (canDrop=false) since dropping would cascade child loss. UpdateInPlace is now SDK-aligned. New helpers updateAttributeInPlace (non-relationships) and refactored updateRelationshipInPlace return bool: true if the source change maps to an SDK updateXAttribute endpoint (required, default, size, min/max, elements, newKey, onDelete), false to fall through to DropAndRecreate. Non-SDK fields (type, array, signed, format, formatOptions, filters) and structural relationship changes (relationType, twoWay toggle, twoWayKey, relatedCollection) automatically route to drop+recreate. Both relationship types (one-way and two-way) now use deleteRelationship on the DropAndRecreate path. One-way is a clean drop (no partner data to lose). Two-way drops both columns; partner row data is restored by the subsequent Upsert row pass. canDrop on attributes is now uniformly true. Indexes have no SDK in-place primitive, so UpdateInPlace consults indexSpecMatches: matching specs tolerate (filters phantom updatedAt drift), differing specs drop and recreate. Maintainability passes: extracted databaseCollectionId, tableCollectionId, attributeIndexMetaId, purgeTableCaches, arraysDifferOnKeys helpers. SDK-boundary fields lifted to ATTRIBUTE_NON_SDK_FIELDS and RELATIONSHIP_STRUCTURAL_FIELDS class constants — single point of change when SDK adds new endpoints. Test matrix expanded: createdAt diff × canDrop × mode × unparseable timestamp combinations. --- src/Migration/Destinations/Appwrite.php | 437 +++++++++++++----- src/Migration/Destinations/OnDuplicate.php | 63 ++- .../Unit/General/OnDuplicateTest.php | 138 ++++-- 3 files changed, 479 insertions(+), 159 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index d4cf2db2..62166ab7 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -63,6 +63,33 @@ class Appwrite extends Destination { + /** + * Attribute fields not reachable via Appwrite SDK's per-type + * updateXAttribute endpoints. Source-side changes to any of these force + * DropAndRecreate because no in-place SDK call can express the change. + * Update if the SDK adds new endpoints (e.g. `array` toggle). + */ + private const ATTRIBUTE_NON_SDK_FIELDS = [ + 'type', + 'array', + 'signed', + 'format', + 'formatOptions', + 'filters', + ]; + + /** + * Relationship `options` fields the SDK's updateRelationshipAttribute + * does not expose (it allows only `onDelete` and `newKey`). Any change + * here is a structural relationship swap that requires drop+recreate. + */ + private const RELATIONSHIP_STRUCTURAL_FIELDS = [ + 'relationType', + 'twoWay', + 'twoWayKey', + 'relatedCollection', + ]; + protected Client $client; protected string $project; @@ -462,8 +489,12 @@ protected function createDatabase(Database $resource): bool $existing = $this->dbForProject->getDocument('databases', $resource->getId()); $action = $this->onDuplicate->resolveSchemaAction( !$existing->isEmpty(), + $createdAt, + $existing->getCreatedAt(), $updatedAt, $existing->getUpdatedAt(), + // Containers (databases) can't be dropped — children would be lost. + canDrop: false, ); if ($action === SchemaAction::Tolerate) { @@ -513,7 +544,7 @@ protected function createDatabase(Database $resource): bool ); $this->dbForProject->createCollection( - 'database_' . $database->getSequence(), + $this->databaseCollectionId($database), $columns, $indexes ); @@ -570,13 +601,17 @@ protected function createEntity(Table $resource): bool if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument( - 'database_' . $database->getSequence(), + $this->databaseCollectionId($database), $resource->getId() ); $action = $this->onDuplicate->resolveSchemaAction( !$existing->isEmpty(), + $createdAt, + $existing->getCreatedAt(), $updatedAt, $existing->getUpdatedAt(), + // Containers (tables) can't be dropped — children would be lost. + canDrop: false, ); if ($action === SchemaAction::Tolerate) { @@ -587,7 +622,7 @@ protected function createEntity(Table $resource): bool if ($action === SchemaAction::UpdateInPlace) { $this->dbForProject->updateDocument( - 'database_' . $database->getSequence(), + $this->databaseCollectionId($database), $existing->getId(), new UtopiaDocument([ 'name' => $resource->getTableName(), @@ -603,7 +638,7 @@ protected function createEntity(Table $resource): bool } } - $table = $this->dbForProject->createDocument('database_' . $database->getSequence(), new UtopiaDocument([ + $table = $this->dbForProject->createDocument($this->databaseCollectionId($database), new UtopiaDocument([ '$id' => $resource->getId(), 'databaseInternalId' => $database->getSequence(), 'databaseId' => $resource->getDatabase()->getId(), @@ -619,7 +654,7 @@ protected function createEntity(Table $resource): bool $resource->setSequence($table->getSequence()); $dbForDatabases->createCollection( - 'database_' . $database->getSequence() . '_collection_' . $resource->getSequence(), + $this->tableCollectionId($database, $table), permissions: $resource->getPermissions(), documentSecurity: $resource->getRowSecurity() ); @@ -681,7 +716,7 @@ protected function createField(Column|Attribute $resource): bool } $table = $this->dbForProject->getDocument( - 'database_' . $database->getSequence(), + $this->databaseCollectionId($database), $resource->getTable()->getId(), ); @@ -726,7 +761,7 @@ protected function createField(Column|Attribute $resource): bool if ($type === UtopiaDatabase::VAR_RELATIONSHIP) { $resource->getOptions()['side'] = UtopiaDatabase::RELATION_SIDE_PARENT; $relatedTable = $this->dbForProject->getDocument( - 'database_' . $database->getSequence(), + $this->databaseCollectionId($database), $resource->getOptions()['relatedCollection'] ); if ($relatedTable->isEmpty()) { @@ -745,10 +780,10 @@ protected function createField(Column|Attribute $resource): bool $this->trackOrphanCandidate($database, $table, 'attributeKeys', $resource->getKey(), $dbForDatabases); - // Relationships route to UpdateInPlace, not DropAndRecreate: - // utopia's deleteAttribute throws for VAR_RELATIONSHIP, and - // two-way drops would cascade to the partner table's physical - // column. Non-relationship attrs keep DropAndRecreate. + // Both relationship types use deleteRelationship (not deleteAttribute, + // which throws for VAR_RELATIONSHIP). One-way: clean drop. Two-way: + // drops both columns; partner row data is restored by the subsequent + // Upsert row pass. $isRelationship = $type === UtopiaDatabase::VAR_RELATIONSHIP; $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); @@ -756,32 +791,34 @@ protected function createField(Column|Attribute $resource): bool $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); $action = $this->onDuplicate->resolveSchemaAction( !$existingAttr->isEmpty(), + $createdAt, + $existingAttr->getCreatedAt(), $updatedAt, $existingAttr->getUpdatedAt(), - canDrop: !$isRelationship, + canDrop: true, ); if ($action === SchemaAction::Tolerate) { - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + $this->purgeTableCaches($database, $table, $dbForDatabases); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; } + // UpdateInPlace tries an SDK-equivalent update first; returns + // false if the source change isn't expressible in place (see + // ATTRIBUTE_NON_SDK_FIELDS / RELATIONSHIP_STRUCTURAL_FIELDS). + // Falls through to DropAndRecreate in that case. if ($action === SchemaAction::UpdateInPlace) { - $this->updateRelationshipInPlace( - $database, - $table, - $resource, - $type, - $updatedAt, - $existingAttr, - $dbForDatabases, - ); - return true; + $applied = $isRelationship + ? $this->updateRelationshipInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases) + : $this->updateAttributeInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases); + + if ($applied) { + return true; + } } - if ($action === SchemaAction::DropAndRecreate) { + if ($action === SchemaAction::DropAndRecreate || $action === SchemaAction::UpdateInPlace) { $this->dropAttributeForRecreate($database, $table, $resource, $dbForDatabases); } } @@ -827,13 +864,11 @@ protected function createField(Column|Attribute $resource): bool message: 'Attribute limit exceeded', ); } catch (\Throwable $e) { - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + $this->purgeTableCaches($database, $table, $dbForDatabases); throw $e; } - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + $this->purgeTableCaches($database, $table, $dbForDatabases); $options = $resource->getOptions(); $twoWayKey = null; @@ -887,8 +922,7 @@ protected function createField(Column|Attribute $resource): bool message: 'Column limit exceeded', ); } catch (\Throwable $e) { - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence()); + $this->purgeTableCaches($database, $relatedTable, $dbForDatabases); throw $e; } } @@ -897,8 +931,8 @@ protected function createField(Column|Attribute $resource): bool switch ($type) { case UtopiaDatabase::VAR_RELATIONSHIP: if (!$dbForDatabases->createRelationship( - collection: 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), - relatedCollection: 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(), + collection: $this->tableCollectionId($database, $table), + relatedCollection: $this->tableCollectionId($database, $relatedTable), type: $options['relationType'], twoWay: $options['twoWay'], id: $resource->getKey(), @@ -915,7 +949,7 @@ protected function createField(Column|Attribute $resource): bool break; default: if (!$dbForDatabases->createAttribute( - 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), + $this->tableCollectionId($database, $table), $resource->getKey(), $type, $resource->getSize(), @@ -946,11 +980,10 @@ protected function createField(Column|Attribute $resource): bool } if ($type === UtopiaDatabase::VAR_RELATIONSHIP && $options['twoWay']) { - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $relatedTable->getId()); } - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection('database_' . $database->getSequence() . '_collection_' . $table->getSequence()); + $this->purgeTableCaches($database, $table, $dbForDatabases); return true; } @@ -975,7 +1008,7 @@ protected function createIndex(Index $resource): bool } $table = $this->dbForProject->getDocument( - 'database_' . $database->getSequence(), + $this->databaseCollectionId($database), $resource->getTable()->getId(), ); if ($table->isEmpty()) { @@ -1066,35 +1099,40 @@ protected function createIndex(Index $resource): bool ); } - $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); + $indexMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); if ($this->onDuplicate !== OnDuplicate::Fail) { $existingIdx = $this->dbForProject->getDocument('indexes', $indexMetaId); $action = $this->onDuplicate->resolveSchemaAction( !$existingIdx->isEmpty(), + $createdAt, + $existingIdx->getCreatedAt(), $updatedAt, $existingIdx->getUpdatedAt(), canDrop: true, ); if ($action === SchemaAction::Tolerate) { - $this->dbForProject->purgeCachedDocument( - 'database_' . $database->getSequence(), - $table->getId() - ); + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; } - if ($action === SchemaAction::DropAndRecreate) { - $dbForDatabases->deleteIndex( - 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), - $resource->getKey() - ); + // Indexes have no SDK in-place update — utopia provides no + // updateIndex primitive, and the underlying DB requires drop+ + // recreate for any structural change. UpdateInPlace from + // resolveSchemaAction (createdAt-same + updatedAt-newer) is + // treated as a possibly-phantom edit; if the spec actually + // matches existing, tolerate; otherwise drop and recreate. + if ($action === SchemaAction::UpdateInPlace && $this->indexSpecMatches($existingIdx, $resource)) { + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Index spec unchanged'); + return false; + } + + if ($action === SchemaAction::DropAndRecreate || $action === SchemaAction::UpdateInPlace) { + $dbForDatabases->deleteIndex($this->tableCollectionId($database, $table), $resource->getKey()); $this->dbForProject->deleteDocument('indexes', $indexMetaId); - $this->dbForProject->purgeCachedDocument( - 'database_' . $database->getSequence(), - $table->getId() - ); + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); } } @@ -1102,7 +1140,7 @@ protected function createIndex(Index $resource): bool try { $result = $dbForDatabases->createIndex( - 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(), + $this->tableCollectionId($database, $table), $resource->getKey(), $resource->getType(), $resource->getColumns(), @@ -1129,10 +1167,7 @@ protected function createIndex(Index $resource): bool ); } - $this->dbForProject->purgeCachedDocument( - 'database_' . $database->getSequence(), - $table->getId() - ); + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); return true; } @@ -1214,7 +1249,7 @@ protected function createRecord(Row $resource, bool $isLast): bool ); $table = $this->dbForProject->getDocument( - 'database_' . $database->getSequence(), + $this->databaseCollectionId($database), $resource->getTable()->getId(), ); @@ -1280,11 +1315,14 @@ protected function createRecord(Row $resource, bool $isLast): bool } /** - * Drop a non-relationship attribute (metadata doc + physical column) so - * it can be recreated. Not used for relationships — those route through - * UpdateInPlace since utopia's deleteAttribute throws for VAR_RELATIONSHIP - * and dropping a relationship column would cascade data loss to the - * partner table's rows. + * Drop an attribute (metadata doc + physical column) so it can be recreated. + * + * Branches by type because utopia's deleteAttribute throws for relationships; + * relationships use deleteRelationship, which handles both one-way (partner + * cleanup is a silent no-op since no partner attribute exists) and two-way + * (partner attribute and physical column both removed). For two-way, the + * partner-side Appwrite-level meta doc is also deleted here since + * deleteRelationship only touches utopia's internal metadata. */ private function dropAttributeForRecreate( UtopiaDocument $database, @@ -1292,31 +1330,49 @@ private function dropAttributeForRecreate( Column|Attribute $resource, UtopiaDatabase $dbForDatabases, ): void { - $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); + $collectionId = $this->tableCollectionId($database, $table); $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); + $isRelationship = $resource->getType() === Column::TYPE_RELATIONSHIP; + + if ($isRelationship) { + $dbForDatabases->deleteRelationship($collectionId, $resource->getKey()); + } else { + $dbForDatabases->deleteAttribute($collectionId, $resource->getKey()); + } - $dbForDatabases->deleteAttribute($collectionId, $resource->getKey()); $this->dbForProject->deleteDocument('attributes', $attributeMetaId); - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection($collectionId); + + // For two-way relationships, also remove the partner-side Appwrite meta + // doc — deleteRelationship only touches utopia's internal _metadata. + if ($isRelationship && ($resource->getOptions()['twoWay'] ?? false)) { + $relatedTableId = $resource->getOptions()['relatedCollection'] ?? null; + $twoWayKey = $resource->getOptions()['twoWayKey'] ?? null; + if ($relatedTableId !== null && $twoWayKey !== null) { + $relatedTable = $this->dbForProject->getDocument( + $this->databaseCollectionId($database), + $relatedTableId, + ); + if (!$relatedTable->isEmpty()) { + $partnerMetaId = $this->attributeIndexMetaId($database, $relatedTable, $twoWayKey); + $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $partnerMetaId)); + } + } + } + + $this->purgeTableCaches($database, $table, $dbForDatabases); } /** - * Reconcile a relationship attribute's metadata on destination without - * dropping its physical column — keeps row data intact. - * - * Two layers must stay in sync: utopia-php/database's internal - * `_metadata` collection (used for row validation) and Appwrite's - * application-level `attributes` doc (used by the Appwrite API). Each - * side's own pass is authoritative for its own Appwrite-level doc; - * utopia's updateRelationship cascades internal metadata on both sides - * in one call and is idempotent across passes for same-target writes. + * Apply an SDK-equivalent in-place update for a non-relationship attribute. + * Returns true if applied, false if the source spec changed in a way the + * SDK can't update in place — caller falls through to DropAndRecreate. * - * relationType changes aren't supported by updateRelationship (they'd - * require a physical schema change). Fail fast rather than silently - * diverge utopia's view from Appwrite's view. + * SDK-updatable per `updateXAttribute` endpoints: required, default, size + * (string/varchar), min/max (integer/float), elements (enum), newKey rename. + * Anything else (type, array, signed, format, formatOptions, filters) + * requires drop+recreate and returns false. */ - private function updateRelationshipInPlace( + private function updateAttributeInPlace( UtopiaDocument $database, UtopiaDocument $table, Column|Attribute $resource, @@ -1324,35 +1380,40 @@ private function updateRelationshipInPlace( string $updatedAt, UtopiaDocument $existingAttr, UtopiaDatabase $dbForDatabases, - ): void { - $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); - $sourceOptions = $resource->getOptions(); - $destOptions = $existingAttr->getAttribute('options', []); + ): bool { + $sourceFields = [ + 'type' => $type, + 'array' => $resource->isArray(), + 'signed' => $resource->isSigned(), + 'format' => $resource->getFormat(), + 'formatOptions' => $resource->getFormatOptions(), + 'filters' => $resource->getFilters(), + ]; - if ( - isset($sourceOptions['relationType'], $destOptions['relationType']) - && $sourceOptions['relationType'] !== $destOptions['relationType'] - ) { - throw new Exception( - resourceName: $resource->getName(), - resourceGroup: $resource->getGroup(), - resourceId: $resource->getId(), - message: 'Changing relationType on a migrated relationship is not supported; drop and recreate the relationship on the destination manually before re-running the migration.', - ); + $existingFields = []; + foreach (self::ATTRIBUTE_NON_SDK_FIELDS as $field) { + $existingFields[$field] = $existingAttr->getAttribute($field); } - $dbForDatabases->updateRelationship( - collection: $collectionId, + if ($this->arraysDifferOnKeys($sourceFields, $existingFields, self::ATTRIBUTE_NON_SDK_FIELDS)) { + return false; + } + + // Apply the SDK-updatable subset via utopia. Pass existing values for + // non-SDK fields explicitly so utopia doesn't trigger an ALTER for + // fields that haven't changed. + $dbForDatabases->updateAttribute( + collection: $this->tableCollectionId($database, $table), id: $resource->getKey(), - newTwoWayKey: ($sourceOptions['twoWayKey'] ?? null) !== ($destOptions['twoWayKey'] ?? null) - ? (string) ($sourceOptions['twoWayKey'] ?? '') - : null, - twoWay: ($sourceOptions['twoWay'] ?? null) !== ($destOptions['twoWay'] ?? null) - ? (bool) ($sourceOptions['twoWay'] ?? false) - : null, - onDelete: ($sourceOptions['onDelete'] ?? null) !== ($destOptions['onDelete'] ?? null) - ? (string) ($sourceOptions['onDelete'] ?? '') - : null, + type: $type, + size: $resource->getSize(), + required: $resource->isRequired(), + default: $resource->getDefault(), + signed: $existingAttr->getAttribute('signed'), + array: $existingAttr->getAttribute('array'), + format: $resource->getFormat(), + formatOptions: $resource->getFormatOptions(), + filters: $existingAttr->getAttribute('filters'), ); $this->dbForProject->updateDocument('attributes', $existingAttr->getId(), new UtopiaDocument([ @@ -1366,12 +1427,91 @@ private function updateRelationshipInPlace( 'format' => $resource->getFormat(), 'formatOptions' => $resource->getFormatOptions(), 'filters' => $resource->getFilters(), - 'options' => $sourceOptions, '$updatedAt' => $updatedAt, ])); - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); - $dbForDatabases->purgeCachedCollection($collectionId); + $this->purgeTableCaches($database, $table, $dbForDatabases); + return true; + } + + /** + * Apply an SDK-equivalent in-place update for a relationship attribute. + * Returns true if applied, false if the source spec changed in a way the + * SDK can't update in place — caller falls through to DropAndRecreate. + * + * SDK's updateRelationshipAttribute exposes only `onDelete` and `newKey`. + * Structural changes (relationType, twoWay toggle, twoWayKey, relatedCollection) + * require drop+recreate and return false. + * + * One-way relationships hit a utopia bug in updateRelationship's + * partner-cascade (`updateAttributeMeta` throws because no partner exists). + * For one-way + onDelete change → return false → DropAndRecreate handles + * it cleanly via deleteRelationship. + */ + private function updateRelationshipInPlace( + UtopiaDocument $database, + UtopiaDocument $table, + Column|Attribute $resource, + string $type, + string $updatedAt, + UtopiaDocument $existingAttr, + UtopiaDatabase $dbForDatabases, + ): bool { + $sourceOptions = $resource->getOptions(); + $destOptions = $existingAttr->getAttribute('options', []); + + if ($this->arraysDifferOnKeys($sourceOptions, $destOptions, self::RELATIONSHIP_STRUCTURAL_FIELDS)) { + return false; + } + + $isTwoWay = (bool) ($destOptions['twoWay'] ?? false); + $onDeleteChanged = ($sourceOptions['onDelete'] ?? null) !== ($destOptions['onDelete'] ?? null); + + // One-way + any actual change → utopia's partner-cascade throws. + // DropAndRecreate handles one-way cleanly. + if (!$isTwoWay && $onDeleteChanged) { + return false; + } + + if ($onDeleteChanged) { + $dbForDatabases->updateRelationship( + collection: $this->tableCollectionId($database, $table), + id: $resource->getKey(), + onDelete: (string) ($sourceOptions['onDelete'] ?? ''), + ); + } + + // Refresh source-side Appwrite-level meta. Options preserve dest's + // structural fields verbatim — they didn't change. + $this->dbForProject->updateDocument('attributes', $existingAttr->getId(), new UtopiaDocument([ + 'key' => $resource->getKey(), + 'type' => $type, + 'options' => array_merge($destOptions, [ + 'onDelete' => $sourceOptions['onDelete'] ?? $destOptions['onDelete'] ?? null, + ]), + '$updatedAt' => $updatedAt, + ])); + + $this->purgeTableCaches($database, $table, $dbForDatabases); + return true; + } + + /** + * True iff the destination's existing index spec matches what the source + * resource is asking for on the user-settable fields (type, indexed + * columns, sort orders). Used to detect phantom updatedAt drift on + * indexes (which have no in-place update primitive); when specs match we + * tolerate, otherwise fall through to DropAndRecreate. + * + * `lengths` is intentionally not compared: it's derived per-column from + * the adapter's index validator, not user-settable on the Index resource, + * so two equivalent specs always produce the same lengths array. + */ + private function indexSpecMatches(UtopiaDocument $existingIdx, Index $resource): bool + { + return $existingIdx->getAttribute('type') === $resource->getType() + && $existingIdx->getAttribute('attributes') === $resource->getColumns() + && $existingIdx->getAttribute('orders') === $resource->getOrders(); } /** @@ -1384,6 +1524,70 @@ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): return $database->getSequence() . ':' . $table->getSequence(); } + /** + * utopia-php/database collection id for the database itself (lookup target + * for table documents inside this database). + */ + private function databaseCollectionId(UtopiaDocument $database): string + { + return 'database_' . $database->getSequence(); + } + + /** + * utopia-php/database collection id for a specific table (lookup target + * for row documents inside this table). Used as the `collection` argument + * to most `$dbForDatabases` calls. + */ + private function tableCollectionId(UtopiaDocument $database, UtopiaDocument $table): string + { + return $this->databaseCollectionId($database) . '_collection_' . $table->getSequence(); + } + + /** + * Composite id for an attribute or index meta document in the platform DB + * (`attributes` / `indexes` collections). Built deterministically from + * sequences + key, mirroring how Appwrite stores meta docs. + */ + private function attributeIndexMetaId(UtopiaDocument $database, UtopiaDocument $table, string $key): string + { + return $database->getSequence() . '_' . $table->getSequence() . '_' . $key; + } + + /** + * Invalidate platform + per-database caches for a table after any + * structural change (attribute/index/permissions). Required because the + * Appwrite UI reads cached collection metadata; stale cache shows the + * pre-change schema until evicted. Called after Tolerate, UpdateInPlace, + * and DropAndRecreate paths so subsequent reads see the current state. + */ + private function purgeTableCaches( + UtopiaDocument $database, + UtopiaDocument $table, + UtopiaDatabase $dbForDatabases, + ): void { + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); + $dbForDatabases->purgeCachedCollection($this->tableCollectionId($database, $table)); + } + + /** + * True iff the two array-shaped specs disagree on any of the listed keys + * (treating missing as null). Used to detect "the SDK can't express this + * change in place" — caller falls through to DropAndRecreate when true. + * + * @param array $a + * @param array $b + * @param list $keys + */ + private function arraysDifferOnKeys(array $a, array $b, array $keys): bool + { + foreach ($keys as $key) { + if (($a[$key] ?? null) !== ($b[$key] ?? null)) { + return true; + } + } + return false; + } + /** * Records that $key was seen for this (database, table) during the run. * $kind selects the tracker bucket ('attributeKeys' or 'indexKeys'). Only @@ -1516,7 +1720,7 @@ private function dropOrphanAttribute( $key = (string) $attrDoc->getAttribute('key'); $type = (string) $attrDoc->getAttribute('type'); $options = $attrDoc->getAttribute('options', []); - $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); + $collectionId = $this->tableCollectionId($database, $table); if ($type === UtopiaDatabase::VAR_RELATIONSHIP) { $this->tolerateMissing(fn () => $dbForDatabases->deleteRelationship($collectionId, $key)); @@ -1524,7 +1728,7 @@ private function dropOrphanAttribute( $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($collectionId, $key)); } $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $attrDoc->getId())); - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); $dbForDatabases->purgeCachedCollection($collectionId); if ($type !== UtopiaDatabase::VAR_RELATIONSHIP || empty($options['twoWay'])) { @@ -1535,7 +1739,7 @@ private function dropOrphanAttribute( if ($twoWayKey === '' || $relatedTableId === '') { return; } - $relatedTable = $this->dbForProject->getDocument('database_' . $database->getSequence(), $relatedTableId); + $relatedTable = $this->dbForProject->getDocument($this->databaseCollectionId($database), $relatedTableId); if ($relatedTable->isEmpty()) { return; } @@ -1543,12 +1747,9 @@ private function dropOrphanAttribute( // Physical column on the related table was already dropped by // deleteRelationship above. Only the Appwrite-level metadata doc // remains to clean up. - $childMetaId = $database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $twoWayKey; + $childMetaId = $this->attributeIndexMetaId($database, $relatedTable, $twoWayKey); $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $childMetaId)); - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $relatedTable->getId()); - $dbForDatabases->purgeCachedCollection( - 'database_' . $database->getSequence() . '_collection_' . $relatedTable->getSequence(), - ); + $this->purgeTableCaches($database, $relatedTable, $dbForDatabases); } private function dropOrphanIndex( @@ -1557,12 +1758,12 @@ private function dropOrphanIndex( string $indexKey, UtopiaDatabase $dbForDatabases, ): void { - $collectionId = 'database_' . $database->getSequence() . '_collection_' . $table->getSequence(); + $collectionId = $this->tableCollectionId($database, $table); $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $indexKey; $this->tolerateMissing(fn () => $dbForDatabases->deleteIndex($collectionId, $indexKey)); $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('indexes', $indexMetaId)); - $this->dbForProject->purgeCachedDocument('database_' . $database->getSequence(), $table->getId()); + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); } /** diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index a40b50e1..c709c71a 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -33,12 +33,22 @@ public static function values(): array /** * Schema-level reconciliation decision. * - * $canDrop = true → leaves (attributes, indexes) get DropAndRecreate on Upsert-newer. - * $canDrop = false → containers (databases, tables) get UpdateInPlace on Upsert-newer. - * Default is false so destructive reconciliation requires explicit opt-in. + * createdAt-different → the source-side resource was deleted+recreated, so + * it's a different physical incarnation. Destination should follow suit + * with DropAndRecreate (or UpdateInPlace if $canDrop is false — containers). + * + * Same createdAt + newer updatedAt → metadata-only edit on the same + * resource. UpdateInPlace; the caller checks whether the specific changed + * fields are SDK-updatable and falls back to DropAndRecreate if not. + * + * $canDrop = true → leaves (attributes, indexes) can be dropped. + * $canDrop = false → containers (databases, tables) get UpdateInPlace + * even on createdAt-different (drop would lose children). */ public function resolveSchemaAction( bool $exists, + ?string $sourceCreatedAt = null, + ?string $destCreatedAt = null, ?string $sourceUpdatedAt = null, ?string $destUpdatedAt = null, bool $canDrop = false, @@ -49,12 +59,53 @@ public function resolveSchemaAction( return match ($this) { self::Fail => SchemaAction::Create, self::Skip => SchemaAction::Tolerate, - self::Upsert => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) - ? ($canDrop ? SchemaAction::DropAndRecreate : SchemaAction::UpdateInPlace) - : SchemaAction::Tolerate, + self::Upsert => $this->resolveUpsertAction( + $sourceCreatedAt, + $destCreatedAt, + $sourceUpdatedAt, + $destUpdatedAt, + $canDrop, + ), }; } + private function resolveUpsertAction( + ?string $sourceCreatedAt, + ?string $destCreatedAt, + ?string $sourceUpdatedAt, + ?string $destUpdatedAt, + bool $canDrop, + ): SchemaAction { + if ($this->createdAtDiffers($sourceCreatedAt, $destCreatedAt)) { + return $canDrop ? SchemaAction::DropAndRecreate : SchemaAction::UpdateInPlace; + } + + if ($this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt)) { + return SchemaAction::UpdateInPlace; + } + + return SchemaAction::Tolerate; + } + + /** + * Detect whether the source-side resource has a different physical + * createdAt than the destination — i.e., the source resource was deleted + * and recreated since the last migration. Null/empty/zero-date treated as + * unknown → false (don't trigger destructive action on uncertain input). + */ + private function createdAtDiffers(?string $source, ?string $dest): bool + { + if ($source === null || $source === '' || $dest === null || $dest === '') { + return false; + } + $src = \strtotime($source); + $dst = \strtotime($dest); + if ($src === false || $dst === false || $src <= 0 || $dst <= 0) { + return false; + } + return $src !== $dst; + } + /** * strtotime() accepts '0000-00-00' leniently (returns a large negative * epoch, not false), so reject non-positive epochs too. Null/empty are diff --git a/tests/Migration/Unit/General/OnDuplicateTest.php b/tests/Migration/Unit/General/OnDuplicateTest.php index 42ffa296..c8b10e1a 100644 --- a/tests/Migration/Unit/General/OnDuplicateTest.php +++ b/tests/Migration/Unit/General/OnDuplicateTest.php @@ -9,8 +9,19 @@ /** * OnDuplicate::resolveSchemaAction is the load-bearing decision point for - * re-migration tolerance. These tests lock the mode × existence × timestamp - * matrix so a future refactor can't silently shift the mapping. + * re-migration tolerance. These tests lock the mode × existence × createdAt × + * updatedAt × canDrop matrix so a future refactor can't silently shift the + * mapping. + * + * Decision rules under Upsert (with $exists = true): + * - createdAt differs → resource was deleted+recreated on source. Caller can + * drop and recreate (canDrop=true) or update in place (canDrop=false; for + * containers like databases/tables which would lose children on drop). + * - createdAt same + updatedAt newer → metadata-only edit. UpdateInPlace. + * The caller checks whether changed fields are SDK-updatable; if not it + * falls through to DropAndRecreate. + * - createdAt same + updatedAt equal/older → Tolerate. + * - Unparseable createdAt or updatedAt → conservative; treat as if no diff. */ class OnDuplicateTest extends TestCase { @@ -38,13 +49,15 @@ public function testFailExistsReturnsCreateSoCallerDDLThrows(): void public function testSkipAlwaysToleratesExisting(): void { - // Skip must ignore timestamps entirely — it's the "don't touch" - // contract. Exercise both orderings to prove the comparison isn't - // consulted. + // Skip must ignore createdAt/updatedAt entirely — it's the "don't + // touch" contract. Exercise both source-newer and dest-newer to prove + // comparisons aren't consulted. $this->assertSame( SchemaAction::Tolerate, OnDuplicate::Skip->resolveSchemaAction( exists: true, + sourceCreatedAt: '2020-01-01T00:00:00.000+00:00', + destCreatedAt: '2026-01-01T00:00:00.000+00:00', sourceUpdatedAt: '2026-01-01T00:00:00.000+00:00', destUpdatedAt: '2020-01-01T00:00:00.000+00:00', ), @@ -59,81 +72,120 @@ public function testSkipAlwaysToleratesExisting(): void ); } - public function testUpsertLeafSourceNewerDropsAndRecreates(): void + public function testUpsertCreatedAtDiffersOnLeafDropsAndRecreates(): void { - // canDrop: true → leaf resource (attribute, index). Column data is - // repopulated by the follow-up row Upsert, or the resource is pure - // metadata (index) so destructive reconciliation is safe. + // canDrop: true + createdAt differs → leaf was deleted/recreated on + // source. Drop the destination's incarnation and create fresh. $this->assertSame( SchemaAction::DropAndRecreate, OnDuplicate::Upsert->resolveSchemaAction( exists: true, - sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', - destUpdatedAt: '2026-04-23T09:59:59.000+00:00', + sourceCreatedAt: '2026-04-23T10:00:00.000+00:00', + destCreatedAt: '2026-04-20T10:00:00.000+00:00', canDrop: true, ), ); } - public function testUpsertContainerSourceNewerUpdatesInPlace(): void + public function testUpsertCreatedAtDiffersOnContainerUpdatesInPlace(): void + { + // Default canDrop: false (containers like databases/tables). Even on + // createdAt diff, drop is forbidden — children would be lost. Fall + // back to UpdateInPlace. + $this->assertSame( + SchemaAction::UpdateInPlace, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceCreatedAt: '2026-04-23T10:00:00.000+00:00', + destCreatedAt: '2026-04-20T10:00:00.000+00:00', + ), + ); + } + + public function testUpsertCreatedAtSameUpdatedAtNewerUpdatesInPlace(): void { - // Default canDrop: false → container resource (database, table). - // Container's metadata doc gets updateDocument; children (tables, - // rows) are preserved untouched. + // Same createdAt + newer updatedAt → metadata-only edit on the same + // physical resource. UpdateInPlace; caller checks whether the changed + // fields are SDK-updatable and falls back to DropAndRecreate if not. $this->assertSame( SchemaAction::UpdateInPlace, OnDuplicate::Upsert->resolveSchemaAction( exists: true, + sourceCreatedAt: '2026-04-20T10:00:00.000+00:00', + destCreatedAt: '2026-04-20T10:00:00.000+00:00', sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', destUpdatedAt: '2026-04-23T09:59:59.000+00:00', + canDrop: true, ), ); } - public function testSkipNeverUpdatesContainerEvenWhenSourceNewer(): void + public function testUpsertCreatedAtSameUpdatedAtEqualTolerates(): void { - // Skip is strict "don't touch" — must never return UpdateInPlace, - // only Tolerate, regardless of timestamps or resource kind. + // Same createdAt + same updatedAt → nothing changed. Skip the work. + $created = '2026-04-20T10:00:00.000+00:00'; + $updated = '2026-04-23T10:00:00.000+00:00'; $this->assertSame( SchemaAction::Tolerate, - OnDuplicate::Skip->resolveSchemaAction( + OnDuplicate::Upsert->resolveSchemaAction( exists: true, - sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', - destUpdatedAt: '2026-04-23T09:59:59.000+00:00', + sourceCreatedAt: $created, + destCreatedAt: $created, + sourceUpdatedAt: $updated, + destUpdatedAt: $updated, + canDrop: true, ), ); } - public function testUpsertDestNewerTolerates(): void + public function testUpsertCreatedAtSameUpdatedAtOlderTolerates(): void { + // Dest is ahead — don't roll back. Avoids overwriting newer + // destination edits with stale source data. $this->assertSame( SchemaAction::Tolerate, OnDuplicate::Upsert->resolveSchemaAction( exists: true, + sourceCreatedAt: '2026-04-20T10:00:00.000+00:00', + destCreatedAt: '2026-04-20T10:00:00.000+00:00', sourceUpdatedAt: '2026-04-23T09:00:00.000+00:00', destUpdatedAt: '2026-04-23T10:00:00.000+00:00', + canDrop: true, ), ); } - public function testUpsertEqualTimestampsTolerates(): void + public function testUpsertNoTimestampsTolerates(): void { - // Strict > comparison: equal means dest is already in sync, skip the - // drop. Avoids unnecessary destructive rebuild when the user hasn't - // touched source since the last migration. - $stamp = '2026-04-23T10:00:00.000+00:00'; + // No timestamps provided at all → no information to act on. Conservative: + // Tolerate rather than risk destructive action on uncertain input. $this->assertSame( SchemaAction::Tolerate, OnDuplicate::Upsert->resolveSchemaAction( exists: true, - sourceUpdatedAt: $stamp, - destUpdatedAt: $stamp, + canDrop: true, + ), + ); + } + + public function testUpsertOnlyUpdatedAtNewerWithoutCreatedAtUpdatesInPlace(): void + { + // CreatedAt missing on either side (e.g. older Appwrite versions, CSV + // sources): can't detect "recreated", so the only signal is updatedAt. + // Newer source → UpdateInPlace path (caller validates SDK-updatability). + $this->assertSame( + SchemaAction::UpdateInPlace, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', + destUpdatedAt: '2026-04-23T09:59:59.000+00:00', + canDrop: true, ), ); } /** - * @return array + * @return array */ public static function unparseableTimestampPairs(): array { @@ -152,11 +204,27 @@ public static function unparseableTimestampPairs(): array } #[DataProvider('unparseableTimestampPairs')] - public function testUpsertUnparseableTimestampsTolerate(?string $source, ?string $dest): void + public function testUpsertUnparseableCreatedAtDoesNotTriggerDrop(?string $source, ?string $dest): void + { + // Unparseable createdAt → don't trigger DropAndRecreate. With + // updatedAt also absent, falls through to Tolerate. Safety net for + // sources that emit malformed timestamps. + $this->assertSame( + SchemaAction::Tolerate, + OnDuplicate::Upsert->resolveSchemaAction( + exists: true, + sourceCreatedAt: $source, + destCreatedAt: $dest, + canDrop: true, + ), + ); + } + + #[DataProvider('unparseableTimestampPairs')] + public function testUpsertUnparseableUpdatedAtTolerates(?string $source, ?string $dest): void { - // Conservative: unparseable timestamps preserve existing destination - // rather than risk a destructive drop on garbage input. Any - // non-Appwrite source that emits malformed dates gets handled safely. + // Conservative: unparseable updatedAt preserves existing destination + // rather than risk a destructive update on garbage input. $this->assertSame( SchemaAction::Tolerate, OnDuplicate::Upsert->resolveSchemaAction( @@ -170,7 +238,7 @@ public function testUpsertUnparseableTimestampsTolerate(?string $source, ?string public function testValuesListsAllCasesInDeclarationOrder(): void { // The values() helper is consumed by API/SDK param validators; this - // tests protects against an accidental case-rename or reorder. + // protects against an accidental case-rename or reorder. $this->assertSame(['fail', 'skip', 'upsert'], OnDuplicate::values()); } } From d7f56b03b8743e2a1998a140130fbfd0a5cfd40b Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 11:30:28 +0100 Subject: [PATCH 16/36] Replace inline meta-id and collection-id literals with helpers --- src/Migration/Destinations/Appwrite.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 62166ab7..f97423e9 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -786,7 +786,7 @@ protected function createField(Column|Attribute $resource): bool // Upsert row pass. $isRelationship = $type === UtopiaDatabase::VAR_RELATIONSHIP; - $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); + $attributeMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); if ($this->onDuplicate !== OnDuplicate::Fail) { $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); $action = $this->onDuplicate->resolveSchemaAction( @@ -881,7 +881,7 @@ protected function createField(Column|Attribute $resource): bool try { $twoWayAttribute = new UtopiaDocument([ - '$id' => ID::custom($database->getSequence() . '_' . $relatedTable->getSequence() . '_' . $twoWayKey), + '$id' => ID::custom($this->attributeIndexMetaId($database, $relatedTable, $twoWayKey)), 'key' => $twoWayKey, 'databaseInternalId' => $database->getSequence(), 'databaseId' => $database->getId(), @@ -1048,7 +1048,7 @@ protected function createIndex(Index $resource): bool $this->trackOrphanCandidate($database, $table, 'indexKeys', $resource->getKey(), $dbForDatabases); $index = new UtopiaDocument([ - '$id' => ID::custom($database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey()), + '$id' => ID::custom($this->attributeIndexMetaId($database, $table, $resource->getKey())), 'key' => $resource->getKey(), 'status' => 'available', // processing, available, failed, deleting, stuck 'databaseInternalId' => $database->getSequence(), @@ -1253,8 +1253,6 @@ protected function createRecord(Row $resource, bool $isLast): bool $resource->getTable()->getId(), ); - $databaseInternalId = $database->getSequence(); - $tableInternalId = $table->getSequence(); $dbForDatabases = ($this->getDatabasesDB)($database); // Reconcile the destination SCHEMA before rows land: drops @@ -1289,7 +1287,7 @@ protected function createRecord(Row $resource, bool $isLast): bool } } } - $collectionId = 'database_' . $databaseInternalId . '_collection_' . $tableInternalId; + $collectionId = $this->tableCollectionId($database, $table); match ($this->onDuplicate) { OnDuplicate::Upsert => $dbForDatabases->skipRelationshipsExistCheck( @@ -1331,7 +1329,7 @@ private function dropAttributeForRecreate( UtopiaDatabase $dbForDatabases, ): void { $collectionId = $this->tableCollectionId($database, $table); - $attributeMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey(); + $attributeMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); $isRelationship = $resource->getType() === Column::TYPE_RELATIONSHIP; if ($isRelationship) { @@ -1759,7 +1757,7 @@ private function dropOrphanIndex( UtopiaDatabase $dbForDatabases, ): void { $collectionId = $this->tableCollectionId($database, $table); - $indexMetaId = $database->getSequence() . '_' . $table->getSequence() . '_' . $indexKey; + $indexMetaId = $this->attributeIndexMetaId($database, $table, $indexKey); $this->tolerateMissing(fn () => $dbForDatabases->deleteIndex($collectionId, $indexKey)); $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('indexes', $indexMetaId)); From 2cd053471e3580c339b29c7640b6e449d48ac6d1 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 11:35:43 +0100 Subject: [PATCH 17/36] OnDuplicate: extract parseTimestamp() helper --- src/Migration/Destinations/OnDuplicate.php | 48 +++++++++++----------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index c709c71a..5c9b7fa4 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -88,39 +88,41 @@ private function resolveUpsertAction( } /** - * Detect whether the source-side resource has a different physical - * createdAt than the destination — i.e., the source resource was deleted - * and recreated since the last migration. Null/empty/zero-date treated as - * unknown → false (don't trigger destructive action on uncertain input). + * Whether source and destination have different physical createdAt values. + * Unknown (null/empty/zero-date) on either side → false. */ private function createdAtDiffers(?string $source, ?string $dest): bool { - if ($source === null || $source === '' || $dest === null || $dest === '') { - return false; - } - $src = \strtotime($source); - $dst = \strtotime($dest); - if ($src === false || $dst === false || $src <= 0 || $dst <= 0) { - return false; - } - return $src !== $dst; + $src = $this->parseTimestamp($source); + $dst = $this->parseTimestamp($dest); + return $src !== null && $dst !== null && $src !== $dst; } /** - * strtotime() accepts '0000-00-00' leniently (returns a large negative - * epoch, not false), so reject non-positive epochs too. Null/empty are - * treated as unknown → tolerate rather than risk a destructive drop. + * Whether source's timestamp is strictly newer than destination's. + * Unknown (null/empty/zero-date) on either side → false. */ private function sourceIsNewer(?string $source, ?string $dest): bool { - if ($source === null || $source === '' || $dest === null || $dest === '') { - return false; + $src = $this->parseTimestamp($source); + $dst = $this->parseTimestamp($dest); + return $src !== null && $dst !== null && $src > $dst; + } + + /** + * strtotime accepts '0000-00-00' leniently (returns a large negative epoch, + * not false), so non-positive epochs are rejected too. Null/empty/garbage + * → null so callers treat the comparison as unknown. + */ + private function parseTimestamp(?string $value): ?int + { + if ($value === null || $value === '') { + return null; } - $src = \strtotime($source); - $dst = \strtotime($dest); - if ($src === false || $dst === false || $src <= 0 || $dst <= 0) { - return false; + $epoch = \strtotime($value); + if ($epoch === false || $epoch <= 0) { + return null; } - return $src > $dst; + return $epoch; } } From 7c43d6ace9d214e39566175ff83292c79f4e904e Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 12:21:05 +0100 Subject: [PATCH 18/36] Use match() for SchemaAction dispatch in 4 create methods --- src/Migration/Destinations/Appwrite.php | 154 +++++++++++++----------- 1 file changed, 86 insertions(+), 68 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index f97423e9..4fe0bea2 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -497,24 +497,29 @@ protected function createDatabase(Database $resource): bool canDrop: false, ); - if ($action === SchemaAction::Tolerate) { - $resource->setSequence($existing->getSequence()); - $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); - return false; - } - - if ($action === SchemaAction::UpdateInPlace) { - $this->dbForProject->updateDocument('databases', $existing->getId(), new UtopiaDocument([ - 'name' => $resource->getDatabaseName(), - 'search' => implode(' ', [$resource->getId(), $resource->getDatabaseName()]), - 'enabled' => $resource->getEnabled(), - 'type' => empty($resource->getType()) ? 'legacy' : $resource->getType(), - 'originalId' => empty($resource->getOriginalId()) ? null : $resource->getOriginalId(), - 'database' => $resource->getDatabase(), - '$updatedAt' => $updatedAt, - ])); - $resource->setSequence($existing->getSequence()); - return true; + $earlyReturn = match ($action) { + SchemaAction::Tolerate => (function () use ($resource, $existing): bool { + $resource->setSequence($existing->getSequence()); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; + })(), + SchemaAction::UpdateInPlace => (function () use ($resource, $existing, $updatedAt): bool { + $this->dbForProject->updateDocument('databases', $existing->getId(), new UtopiaDocument([ + 'name' => $resource->getDatabaseName(), + 'search' => implode(' ', [$resource->getId(), $resource->getDatabaseName()]), + 'enabled' => $resource->getEnabled(), + 'type' => empty($resource->getType()) ? 'legacy' : $resource->getType(), + 'originalId' => empty($resource->getOriginalId()) ? null : $resource->getOriginalId(), + 'database' => $resource->getDatabase(), + '$updatedAt' => $updatedAt, + ])); + $resource->setSequence($existing->getSequence()); + return true; + })(), + SchemaAction::Create, SchemaAction::DropAndRecreate => null, + }; + if ($earlyReturn !== null) { + return $earlyReturn; } } @@ -614,27 +619,32 @@ protected function createEntity(Table $resource): bool canDrop: false, ); - if ($action === SchemaAction::Tolerate) { - $resource->setSequence($existing->getSequence()); - $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); - return false; - } - - if ($action === SchemaAction::UpdateInPlace) { - $this->dbForProject->updateDocument( - $this->databaseCollectionId($database), - $existing->getId(), - new UtopiaDocument([ - 'name' => $resource->getTableName(), - 'search' => implode(' ', [$resource->getId(), $resource->getTableName()]), - 'enabled' => $resource->getEnabled(), - '$permissions' => Permission::aggregate($resource->getPermissions()), - 'documentSecurity' => $resource->getRowSecurity(), - '$updatedAt' => $updatedAt, - ]) - ); - $resource->setSequence($existing->getSequence()); - return true; + $earlyReturn = match ($action) { + SchemaAction::Tolerate => (function () use ($resource, $existing): bool { + $resource->setSequence($existing->getSequence()); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; + })(), + SchemaAction::UpdateInPlace => (function () use ($resource, $existing, $database, $updatedAt): bool { + $this->dbForProject->updateDocument( + $this->databaseCollectionId($database), + $existing->getId(), + new UtopiaDocument([ + 'name' => $resource->getTableName(), + 'search' => implode(' ', [$resource->getId(), $resource->getTableName()]), + 'enabled' => $resource->getEnabled(), + '$permissions' => Permission::aggregate($resource->getPermissions()), + 'documentSecurity' => $resource->getRowSecurity(), + '$updatedAt' => $updatedAt, + ]) + ); + $resource->setSequence($existing->getSequence()); + return true; + })(), + SchemaAction::Create, SchemaAction::DropAndRecreate => null, + }; + if ($earlyReturn !== null) { + return $earlyReturn; } } @@ -798,24 +808,25 @@ protected function createField(Column|Attribute $resource): bool canDrop: true, ); - if ($action === SchemaAction::Tolerate) { - $this->purgeTableCaches($database, $table, $dbForDatabases); - $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); - return false; - } - - // UpdateInPlace tries an SDK-equivalent update first; returns - // false if the source change isn't expressible in place (see - // ATTRIBUTE_NON_SDK_FIELDS / RELATIONSHIP_STRUCTURAL_FIELDS). - // Falls through to DropAndRecreate in that case. - if ($action === SchemaAction::UpdateInPlace) { - $applied = $isRelationship + // UpdateInPlace tries an SDK-equivalent update first; returns null + // when the source change isn't expressible in place (see + // ATTRIBUTE_NON_SDK_FIELDS / RELATIONSHIP_STRUCTURAL_FIELDS) so it + // falls through to the DropAndRecreate drop step below. + $earlyReturn = match ($action) { + SchemaAction::Tolerate => (function () use ($resource, $database, $table, $dbForDatabases): bool { + $this->purgeTableCaches($database, $table, $dbForDatabases); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; + })(), + SchemaAction::UpdateInPlace => ($isRelationship ? $this->updateRelationshipInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases) - : $this->updateAttributeInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases); - - if ($applied) { - return true; - } + : $this->updateAttributeInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases)) + ? true + : null, + SchemaAction::DropAndRecreate, SchemaAction::Create => null, + }; + if ($earlyReturn !== null) { + return $earlyReturn; } if ($action === SchemaAction::DropAndRecreate || $action === SchemaAction::UpdateInPlace) { @@ -1111,22 +1122,29 @@ protected function createIndex(Index $resource): bool canDrop: true, ); - if ($action === SchemaAction::Tolerate) { - $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); - $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); - return false; - } - // Indexes have no SDK in-place update — utopia provides no // updateIndex primitive, and the underlying DB requires drop+ // recreate for any structural change. UpdateInPlace from // resolveSchemaAction (createdAt-same + updatedAt-newer) is - // treated as a possibly-phantom edit; if the spec actually - // matches existing, tolerate; otherwise drop and recreate. - if ($action === SchemaAction::UpdateInPlace && $this->indexSpecMatches($existingIdx, $resource)) { - $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); - $resource->setStatus(Resource::STATUS_SKIPPED, 'Index spec unchanged'); - return false; + // treated as a possibly-phantom edit; if the spec matches + // existing, tolerate; otherwise drop and recreate. + $earlyReturn = match ($action) { + SchemaAction::Tolerate => (function () use ($resource, $database, $table): bool { + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; + })(), + SchemaAction::UpdateInPlace => $this->indexSpecMatches($existingIdx, $resource) + ? (function () use ($resource, $database, $table): bool { + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Index spec unchanged'); + return false; + })() + : null, + SchemaAction::DropAndRecreate, SchemaAction::Create => null, + }; + if ($earlyReturn !== null) { + return $earlyReturn; } if ($action === SchemaAction::DropAndRecreate || $action === SchemaAction::UpdateInPlace) { From a36d95f86d24b4f024a419191e69c3c034a593eb Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 12:29:25 +0100 Subject: [PATCH 19/36] =?UTF-8?q?Trim=20verbose=20code=20comments=20?= =?UTF-8?q?=E2=80=94=20keep=20only=20WHY=20content?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Migration/Destinations/Appwrite.php | 189 +++------------------ src/Migration/Destinations/OnDuplicate.php | 32 +--- 2 files changed, 30 insertions(+), 191 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 4fe0bea2..80b22c71 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -63,12 +63,7 @@ class Appwrite extends Destination { - /** - * Attribute fields not reachable via Appwrite SDK's per-type - * updateXAttribute endpoints. Source-side changes to any of these force - * DropAndRecreate because no in-place SDK call can express the change. - * Update if the SDK adds new endpoints (e.g. `array` toggle). - */ + /** Fields the SDK's per-type updateXAttribute endpoints don't expose; a change here forces DropAndRecreate. */ private const ATTRIBUTE_NON_SDK_FIELDS = [ 'type', 'array', @@ -78,11 +73,7 @@ class Appwrite extends Destination 'filters', ]; - /** - * Relationship `options` fields the SDK's updateRelationshipAttribute - * does not expose (it allows only `onDelete` and `newKey`). Any change - * here is a structural relationship swap that requires drop+recreate. - */ + /** Fields the SDK's updateRelationshipAttribute doesn't expose (only onDelete/newKey are SDK-reachable). */ private const RELATIONSHIP_STRUCTURAL_FIELDS = [ 'relationType', 'twoWay', @@ -113,13 +104,10 @@ class Appwrite extends Destination private array $rowBuffer = []; /** - * Upsert-mode orphan tracking, keyed by (database, table). Each entry - * holds the attribute + index keys source declared for that table plus - * the docs/handles needed to reach the destination at cleanup time. - * Orphans are the complement: anything the destination has whose key - * is NOT in this record. Entries are removed from the map after their - * cleanup has run, so the final sweep at end-of-migration only visits - * tables that had no rows. + * Upsert-mode orphan tracking, keyed by (database, table). Orphans are + * destination keys not in `attributeKeys` / `indexKeys`. Entries removed + * after their cleanup runs so the end-of-run sweep only visits tables + * that had no rows. * * @var arraygetDatabasesDB = $getDatabasesDB; } - /** - * Upsert-mode orphan cleanup runs after a successful migration only. - * A thrown exception from the source/import loop short-circuits here - * so mid-migration failures preserve the destination as-is. - */ + /** Orphan cleanup runs only after a successful migration — a mid-run throw preserves the destination as-is. */ #[Override] public function run( array $resources, @@ -484,7 +468,6 @@ protected function createDatabase(Database $resource): bool $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); - // Fail mode skips the pre-check so library's DuplicateException surfaces on re-migration. if ($this->onDuplicate !== OnDuplicate::Fail) { $existing = $this->dbForProject->getDocument('databases', $resource->getId()); $action = $this->onDuplicate->resolveSchemaAction( @@ -493,7 +476,6 @@ protected function createDatabase(Database $resource): bool $existing->getCreatedAt(), $updatedAt, $existing->getUpdatedAt(), - // Containers (databases) can't be dropped — children would be lost. canDrop: false, ); @@ -615,7 +597,6 @@ protected function createEntity(Table $resource): bool $existing->getCreatedAt(), $updatedAt, $existing->getUpdatedAt(), - // Containers (tables) can't be dropped — children would be lost. canDrop: false, ); @@ -790,10 +771,8 @@ protected function createField(Column|Attribute $resource): bool $this->trackOrphanCandidate($database, $table, 'attributeKeys', $resource->getKey(), $dbForDatabases); - // Both relationship types use deleteRelationship (not deleteAttribute, - // which throws for VAR_RELATIONSHIP). One-way: clean drop. Two-way: - // drops both columns; partner row data is restored by the subsequent - // Upsert row pass. + // Both rel types route through deleteRelationship — deleteAttribute throws for VAR_RELATIONSHIP. + // Two-way drop loses partner row data, restored by the subsequent Upsert row pass. $isRelationship = $type === UtopiaDatabase::VAR_RELATIONSHIP; $attributeMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); @@ -808,10 +787,7 @@ protected function createField(Column|Attribute $resource): bool canDrop: true, ); - // UpdateInPlace tries an SDK-equivalent update first; returns null - // when the source change isn't expressible in place (see - // ATTRIBUTE_NON_SDK_FIELDS / RELATIONSHIP_STRUCTURAL_FIELDS) so it - // falls through to the DropAndRecreate drop step below. + // UpdateInPlace returns null when the source change isn't SDK-expressible — falls through to the drop step below. $earlyReturn = match ($action) { SchemaAction::Tolerate => (function () use ($resource, $database, $table, $dbForDatabases): bool { $this->purgeTableCaches($database, $table, $dbForDatabases); @@ -1122,12 +1098,7 @@ protected function createIndex(Index $resource): bool canDrop: true, ); - // Indexes have no SDK in-place update — utopia provides no - // updateIndex primitive, and the underlying DB requires drop+ - // recreate for any structural change. UpdateInPlace from - // resolveSchemaAction (createdAt-same + updatedAt-newer) is - // treated as a possibly-phantom edit; if the spec matches - // existing, tolerate; otherwise drop and recreate. + // Indexes have no in-place primitive — UpdateInPlace tolerates if spec matches, else drop+recreate. $earlyReturn = match ($action) { SchemaAction::Tolerate => (function () use ($resource, $database, $table): bool { $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); @@ -1273,11 +1244,7 @@ protected function createRecord(Row $resource, bool $isLast): bool $dbForDatabases = ($this->getDatabasesDB)($database); - // Reconcile the destination SCHEMA before rows land: drops - // attributes/indexes source no longer declares so the - // Structure validator doesn't reject rows on orphan required - // columns. Distinct from the payload-shape pass below, which - // strips extra fields off the incoming ROW documents. + // Drop schema-level orphans before rows land so the Structure validator doesn't reject on orphan required columns. $this->cleanupUpsertOrphansForTable($this->tableIdentity($database, $table)); /** * This is in case an attribute was deleted from Appwrite attributes collection but was not deleted from the table @@ -1331,14 +1298,8 @@ protected function createRecord(Row $resource, bool $isLast): bool } /** - * Drop an attribute (metadata doc + physical column) so it can be recreated. - * - * Branches by type because utopia's deleteAttribute throws for relationships; - * relationships use deleteRelationship, which handles both one-way (partner - * cleanup is a silent no-op since no partner attribute exists) and two-way - * (partner attribute and physical column both removed). For two-way, the - * partner-side Appwrite-level meta doc is also deleted here since - * deleteRelationship only touches utopia's internal metadata. + * Drop attribute (meta doc + physical column). Relationships route through + * deleteRelationship since deleteAttribute throws for VAR_RELATIONSHIP. */ private function dropAttributeForRecreate( UtopiaDocument $database, @@ -1358,8 +1319,7 @@ private function dropAttributeForRecreate( $this->dbForProject->deleteDocument('attributes', $attributeMetaId); - // For two-way relationships, also remove the partner-side Appwrite meta - // doc — deleteRelationship only touches utopia's internal _metadata. + // deleteRelationship only touches utopia's _metadata; clear the Appwrite-level partner meta doc too. if ($isRelationship && ($resource->getOptions()['twoWay'] ?? false)) { $relatedTableId = $resource->getOptions()['relatedCollection'] ?? null; $twoWayKey = $resource->getOptions()['twoWayKey'] ?? null; @@ -1378,16 +1338,7 @@ private function dropAttributeForRecreate( $this->purgeTableCaches($database, $table, $dbForDatabases); } - /** - * Apply an SDK-equivalent in-place update for a non-relationship attribute. - * Returns true if applied, false if the source spec changed in a way the - * SDK can't update in place — caller falls through to DropAndRecreate. - * - * SDK-updatable per `updateXAttribute` endpoints: required, default, size - * (string/varchar), min/max (integer/float), elements (enum), newKey rename. - * Anything else (type, array, signed, format, formatOptions, filters) - * requires drop+recreate and returns false. - */ + /** Returns false when the source change isn't SDK-expressible — caller falls through to DropAndRecreate. */ private function updateAttributeInPlace( UtopiaDocument $database, UtopiaDocument $table, @@ -1415,9 +1366,7 @@ private function updateAttributeInPlace( return false; } - // Apply the SDK-updatable subset via utopia. Pass existing values for - // non-SDK fields explicitly so utopia doesn't trigger an ALTER for - // fields that haven't changed. + // Pass existing values for non-SDK fields so utopia doesn't trigger an ALTER for unchanged fields. $dbForDatabases->updateAttribute( collection: $this->tableCollectionId($database, $table), id: $resource->getKey(), @@ -1451,18 +1400,8 @@ private function updateAttributeInPlace( } /** - * Apply an SDK-equivalent in-place update for a relationship attribute. - * Returns true if applied, false if the source spec changed in a way the - * SDK can't update in place — caller falls through to DropAndRecreate. - * - * SDK's updateRelationshipAttribute exposes only `onDelete` and `newKey`. - * Structural changes (relationType, twoWay toggle, twoWayKey, relatedCollection) - * require drop+recreate and return false. - * - * One-way relationships hit a utopia bug in updateRelationship's - * partner-cascade (`updateAttributeMeta` throws because no partner exists). - * For one-way + onDelete change → return false → DropAndRecreate handles - * it cleanly via deleteRelationship. + * Returns false when the source change isn't SDK-expressible — caller falls through to DropAndRecreate. + * One-way + onDelete change is also rejected: utopia's updateRelationship partner-cascade throws on one-way. */ private function updateRelationshipInPlace( UtopiaDocument $database, @@ -1483,8 +1422,6 @@ private function updateRelationshipInPlace( $isTwoWay = (bool) ($destOptions['twoWay'] ?? false); $onDeleteChanged = ($sourceOptions['onDelete'] ?? null) !== ($destOptions['onDelete'] ?? null); - // One-way + any actual change → utopia's partner-cascade throws. - // DropAndRecreate handles one-way cleanly. if (!$isTwoWay && $onDeleteChanged) { return false; } @@ -1497,8 +1434,6 @@ private function updateRelationshipInPlace( ); } - // Refresh source-side Appwrite-level meta. Options preserve dest's - // structural fields verbatim — they didn't change. $this->dbForProject->updateDocument('attributes', $existingAttr->getId(), new UtopiaDocument([ 'key' => $resource->getKey(), 'type' => $type, @@ -1513,15 +1448,8 @@ private function updateRelationshipInPlace( } /** - * True iff the destination's existing index spec matches what the source - * resource is asking for on the user-settable fields (type, indexed - * columns, sort orders). Used to detect phantom updatedAt drift on - * indexes (which have no in-place update primitive); when specs match we - * tolerate, otherwise fall through to DropAndRecreate. - * * `lengths` is intentionally not compared: it's derived per-column from - * the adapter's index validator, not user-settable on the Index resource, - * so two equivalent specs always produce the same lengths array. + * the adapter's index validator and not settable on the Index resource. */ private function indexSpecMatches(UtopiaDocument $existingIdx, Index $resource): bool { @@ -1530,52 +1458,27 @@ private function indexSpecMatches(UtopiaDocument $existingIdx, Index $resource): && $existingIdx->getAttribute('orders') === $resource->getOrders(); } - /** - * Deterministic per-(database, table) key used to index the orphan - * tracker (`$orphansByTable`). Built from sequences so it's stable - * across calls but unique within the run. - */ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): string { return $database->getSequence() . ':' . $table->getSequence(); } - /** - * utopia-php/database collection id for the database itself (lookup target - * for table documents inside this database). - */ private function databaseCollectionId(UtopiaDocument $database): string { return 'database_' . $database->getSequence(); } - /** - * utopia-php/database collection id for a specific table (lookup target - * for row documents inside this table). Used as the `collection` argument - * to most `$dbForDatabases` calls. - */ private function tableCollectionId(UtopiaDocument $database, UtopiaDocument $table): string { return $this->databaseCollectionId($database) . '_collection_' . $table->getSequence(); } - /** - * Composite id for an attribute or index meta document in the platform DB - * (`attributes` / `indexes` collections). Built deterministically from - * sequences + key, mirroring how Appwrite stores meta docs. - */ private function attributeIndexMetaId(UtopiaDocument $database, UtopiaDocument $table, string $key): string { return $database->getSequence() . '_' . $table->getSequence() . '_' . $key; } - /** - * Invalidate platform + per-database caches for a table after any - * structural change (attribute/index/permissions). Required because the - * Appwrite UI reads cached collection metadata; stale cache shows the - * pre-change schema until evicted. Called after Tolerate, UpdateInPlace, - * and DropAndRecreate paths so subsequent reads see the current state. - */ + /** Stale platform/per-database cache shows the pre-change schema; evict after every structural change. */ private function purgeTableCaches( UtopiaDocument $database, UtopiaDocument $table, @@ -1586,10 +1489,6 @@ private function purgeTableCaches( } /** - * True iff the two array-shaped specs disagree on any of the listed keys - * (treating missing as null). Used to detect "the SDK can't express this - * change in place" — caller falls through to DropAndRecreate when true. - * * @param array $a * @param array $b * @param list $keys @@ -1604,11 +1503,7 @@ private function arraysDifferOnKeys(array $a, array $b, array $keys): bool return false; } - /** - * Records that $key was seen for this (database, table) during the run. - * $kind selects the tracker bucket ('attributeKeys' or 'indexKeys'). Only - * Upsert mode accumulates; Skip/Fail short-circuit. - */ + /** $kind is 'attributeKeys' or 'indexKeys'. */ private function trackOrphanCandidate( UtopiaDocument $database, UtopiaDocument $table, @@ -1632,11 +1527,7 @@ private function trackOrphanCandidate( $this->orphansByTable[$tableId][$kind][] = $key; } - /** - * End-of-migration sweep: cleans up any table not already handled by a - * per-table cleanup in createRecord (i.e. tables that had no rows). - * Skipped if the migration failed before run() completed. - */ + /** End-of-migration sweep — only visits tables that had no rows (rest were cleaned per-table in createRecord). */ private function cleanupUpsertOrphans(): void { foreach (\array_keys($this->orphansByTable) as $tableId) { @@ -1644,13 +1535,7 @@ private function cleanupUpsertOrphans(): void } } - /** - * Drops destination attributes/indexes whose keys weren't observed from - * source for this table. Called per-table from createRecord before rows - * land so the Structure validator sees the post-cleanup schema. After - * running, the entry is removed from the tracker so the end-of-run - * sweep doesn't re-do the work. - */ + /** Called per-table from createRecord before rows land so Structure validator sees the post-cleanup schema. */ private function cleanupUpsertOrphansForTable(string $tableId): void { if ($this->onDuplicate !== OnDuplicate::Upsert) { @@ -1690,10 +1575,6 @@ private function cleanupUpsertOrphansForTable(string $tableId): void } /** - * Finds destination metadata docs in $metaCollection belonging to this - * (database, table) and invokes $drop for each one whose key isn't in - * $processedKeys. Shared by the attribute and index cleanup paths. - * * @param list $processedKeys * @param callable(UtopiaDocument): void $drop */ @@ -1716,17 +1597,7 @@ private function dropOrphansByKind( } } - /** - * Drop an orphan attribute (metadata doc + physical column). Uses the - * destination's own metadata document as the source of truth for type - * and relationship options, since there's no source resource to consult. - * - * Relationships use deleteRelationship (which cascades to both sides's - * physical columns + utopia's internal metadata) rather than - * deleteAttribute (which throws for relationship types). Appwrite's - * application-level `attributes` metadata docs on both sides are - * cleaned separately. - */ + /** Reads dest's own meta doc as source of truth — there's no source resource for an orphan. */ private function dropOrphanAttribute( UtopiaDocument $database, UtopiaDocument $table, @@ -1760,9 +1631,7 @@ private function dropOrphanAttribute( return; } - // Physical column on the related table was already dropped by - // deleteRelationship above. Only the Appwrite-level metadata doc - // remains to clean up. + // deleteRelationship already dropped the partner physical column; only the Appwrite-level meta doc remains. $childMetaId = $this->attributeIndexMetaId($database, $relatedTable, $twoWayKey); $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $childMetaId)); $this->purgeTableCaches($database, $relatedTable, $dbForDatabases); @@ -1782,11 +1651,7 @@ private function dropOrphanIndex( $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); } - /** - * Swallows deletion errors — a prior interrupted run (or parent-side - * cascade via utopia-php/database relationship handling) may already - * have removed the target. - */ + /** Swallows deletion errors — a prior run or relationship cascade may have already removed the target. */ private function tolerateMissing(callable $fn): void { try { diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 5c9b7fa4..744b2e15 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -2,12 +2,6 @@ namespace Utopia\Migration\Destinations; -/** - * Outcome of {@see OnDuplicate::resolveSchemaAction()}. Declared alongside - * OnDuplicate because the two are designed together — any code that uses - * SchemaAction necessarily imports OnDuplicate (as the source of the - * decision), so the shared file satisfies autoloading in practice. - */ enum SchemaAction { case Create; @@ -31,19 +25,8 @@ public static function values(): array } /** - * Schema-level reconciliation decision. - * - * createdAt-different → the source-side resource was deleted+recreated, so - * it's a different physical incarnation. Destination should follow suit - * with DropAndRecreate (or UpdateInPlace if $canDrop is false — containers). - * - * Same createdAt + newer updatedAt → metadata-only edit on the same - * resource. UpdateInPlace; the caller checks whether the specific changed - * fields are SDK-updatable and falls back to DropAndRecreate if not. - * - * $canDrop = true → leaves (attributes, indexes) can be dropped. - * $canDrop = false → containers (databases, tables) get UpdateInPlace - * even on createdAt-different (drop would lose children). + * $canDrop = false (containers like databases/tables) forces UpdateInPlace + * even on createdAt-different — dropping would orphan children. */ public function resolveSchemaAction( bool $exists, @@ -87,10 +70,6 @@ private function resolveUpsertAction( return SchemaAction::Tolerate; } - /** - * Whether source and destination have different physical createdAt values. - * Unknown (null/empty/zero-date) on either side → false. - */ private function createdAtDiffers(?string $source, ?string $dest): bool { $src = $this->parseTimestamp($source); @@ -98,10 +77,6 @@ private function createdAtDiffers(?string $source, ?string $dest): bool return $src !== null && $dst !== null && $src !== $dst; } - /** - * Whether source's timestamp is strictly newer than destination's. - * Unknown (null/empty/zero-date) on either side → false. - */ private function sourceIsNewer(?string $source, ?string $dest): bool { $src = $this->parseTimestamp($source); @@ -111,8 +86,7 @@ private function sourceIsNewer(?string $source, ?string $dest): bool /** * strtotime accepts '0000-00-00' leniently (returns a large negative epoch, - * not false), so non-positive epochs are rejected too. Null/empty/garbage - * → null so callers treat the comparison as unknown. + * not false), so non-positive epochs are rejected too. */ private function parseTimestamp(?string $value): ?int { From c76de9acffa7fddac7cb1e969ebde63c053abcd3 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 13:57:33 +0100 Subject: [PATCH 20/36] Two-way relationships: sync partner-side onDelete on UpdateInPlace utopia's updateRelationship updates the physical constraint on both sides, but the Appwrite-level attributes meta doc on the partner side keeps the old options.onDelete value. Reads from the related collection's attribute list (Appwrite API, validators, subsequent re-migration pre-checks) see stale data. Mirrors the partner-side cleanup pattern already used in dropAttributeForRecreate. One-way unaffected (onDelete change on one-way already routes to DropAndRecreate). --- src/Migration/Destinations/Appwrite.php | 47 +++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 80b22c71..d1efd098 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -1444,9 +1444,56 @@ private function updateRelationshipInPlace( ])); $this->purgeTableCaches($database, $table, $dbForDatabases); + + // utopia's updateRelationship synced the physical constraint on both sides but + // not the Appwrite-level partner meta doc — refresh it so reads from the related + // collection don't see stale onDelete. + if ($isTwoWay) { + $this->refreshTwoWayPartnerOnDelete($database, $destOptions, $sourceOptions, $updatedAt, $dbForDatabases); + } + return true; } + private function refreshTwoWayPartnerOnDelete( + UtopiaDocument $database, + array $destOptions, + array $sourceOptions, + string $updatedAt, + UtopiaDatabase $dbForDatabases, + ): void { + $relatedTableId = (string) ($destOptions['relatedCollection'] ?? ''); + $partnerKey = (string) ($destOptions['twoWayKey'] ?? ''); + if ($relatedTableId === '' || $partnerKey === '') { + return; + } + + $relatedTable = $this->dbForProject->getDocument( + $this->databaseCollectionId($database), + $relatedTableId, + ); + if ($relatedTable->isEmpty()) { + return; + } + + $partnerMeta = $this->dbForProject->getDocument( + 'attributes', + $this->attributeIndexMetaId($database, $relatedTable, $partnerKey), + ); + if ($partnerMeta->isEmpty()) { + return; + } + + $partnerOptions = $partnerMeta->getAttribute('options', []); + $this->dbForProject->updateDocument('attributes', $partnerMeta->getId(), new UtopiaDocument([ + 'options' => array_merge($partnerOptions, [ + 'onDelete' => $sourceOptions['onDelete'] ?? $partnerOptions['onDelete'] ?? null, + ]), + '$updatedAt' => $updatedAt, + ])); + $this->purgeTableCaches($database, $relatedTable, $dbForDatabases); + } + /** * `lengths` is intentionally not compared: it's derived per-column from * the adapter's index validator and not settable on the Index resource. From c13e77d562e775b66ab8c03ee5c8163e53c3067e Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 14:42:58 +0100 Subject: [PATCH 21/36] Two-way relationships: restore partner-side dedup via pair-key set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source emits both sides of a two-way relationship as separate Column resources, but processing one side already reconciles both physical columns + both Appwrite-level meta docs (utopia's createRelationship/ deleteRelationship cascade). Without dedup, the partner pass can fire DropAndRecreate again — partner's source createdAt is independent of parent's and may not match what we just stamped on dest's partner meta (parent's createdAt). Reintroduces the pair-key dedup that existed in 791dbb1 and was removed in 6e6f825 when relationships routed exclusively through UpdateInPlace. Commit 786a27b reopened the DropAndRecreate path for createdAt-different without restoring the dedup; this fixes that gap. Pair-key is canonical (sorted, scoped to dbSeq) so both sides resolve to the same string. Set after Tolerate, after UpdateInPlace success, and after the create-path completion. Skip check at the top of createField. --- src/Migration/Destinations/Appwrite.php | 61 +++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index d1efd098..7eedb82d 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -119,6 +119,18 @@ class Appwrite extends Destination */ private array $orphansByTable = []; + /** + * Two-way relationship pairs already reconciled this run, keyed by a + * canonical pair-key. Source emits both sides as separate Column + * resources, but processing one side already reconciles both physical + * columns + both Appwrite-level meta docs. Without this set, the + * partner pass can re-trigger DropAndRecreate and destroy partner-table + * rows already migrated. + * + * @var array + */ + private array $processedTwoWayPairs = []; + /** * @param string $project * @param string $endpoint @@ -775,6 +787,17 @@ protected function createField(Column|Attribute $resource): bool // Two-way drop loses partner row data, restored by the subsequent Upsert row pass. $isRelationship = $type === UtopiaDatabase::VAR_RELATIONSHIP; + // Two-way relationships emit two source-side resources but reconcile both + // physical columns + both Appwrite-level meta docs in one pass. Skip the + // partner — re-processing can fire DropAndRecreate again and destroy + // partner-table rows already migrated this run (parent and partner have + // independent source createdAt values that may not match). + $twoWayPairKey = $this->twoWayPairKey($database, $table, $resource, $type); + if ($twoWayPairKey !== null && isset($this->processedTwoWayPairs[$twoWayPairKey])) { + $resource->setStatus(Resource::STATUS_SKIPPED, 'Two-way partner already reconciled'); + return false; + } + $attributeMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); if ($this->onDuplicate !== OnDuplicate::Fail) { $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); @@ -802,6 +825,9 @@ protected function createField(Column|Attribute $resource): bool SchemaAction::DropAndRecreate, SchemaAction::Create => null, }; if ($earlyReturn !== null) { + if ($twoWayPairKey !== null) { + $this->processedTwoWayPairs[$twoWayPairKey] = true; + } return $earlyReturn; } @@ -972,6 +998,10 @@ protected function createField(Column|Attribute $resource): bool $this->purgeTableCaches($database, $table, $dbForDatabases); + if ($twoWayPairKey !== null) { + $this->processedTwoWayPairs[$twoWayPairKey] = true; + } + return true; } @@ -1510,6 +1540,37 @@ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): return $database->getSequence() . ':' . $table->getSequence(); } + /** + * Canonical pair-key for a two-way relationship — same string regardless + * of which side (parent or partner) is being processed. Returns null for + * non-two-way attributes (no pair-level tracking needed). + */ + private function twoWayPairKey( + UtopiaDocument $database, + UtopiaDocument $table, + Column|Attribute $resource, + string $type, + ): ?string { + if ($type !== UtopiaDatabase::VAR_RELATIONSHIP) { + return null; + } + $options = $resource->getOptions(); + if (empty($options['twoWay'])) { + return null; + } + $twoWayKey = (string) ($options['twoWayKey'] ?? ''); + $relatedTableId = (string) ($options['relatedCollection'] ?? ''); + if ($twoWayKey === '' || $relatedTableId === '') { + return null; + } + + $thisSide = $table->getId() . '::' . $resource->getKey(); + $partnerSide = $relatedTableId . '::' . $twoWayKey; + $pair = [$thisSide, $partnerSide]; + \sort($pair); + return $database->getSequence() . '@' . \implode('<->', $pair); + } + private function databaseCollectionId(UtopiaDocument $database): string { return 'database_' . $database->getSequence(); From 09c1b2133d3ec42d11aee345f003d067d1de9ae5 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 17:27:12 +0100 Subject: [PATCH 22/36] Maintainability pass from PR review Four targeted fixes from a code review pass on the schema-reconciliation code, picked for clear correctness or future-debug-cost improvements: 1. META_DATABASES / META_ATTRIBUTES / META_INDEXES class constants. ~30 bare collection-name string literals are now phpstan-checkable and a typo no longer compiles. Avoided false-positive replacements in getAttribute('attributes', []) / collectionStructure['indexes'] etc., which name UtopiaDocument fields, not collections. 2. resetRunState() called at top of run(). $rowBuffer, $orphansByTable, and $processedTwoWayPairs are instance state that previously leaked across run() invocations on a reused Appwrite destination instance. A second run on the same instance would inherit the first run's processed-pair set and silently SKIP unrelated two-way attributes. 3. tolerateMissing() renamed to bestEffort(). The previous name implied it caught only NotFound errors, but the implementation swallows all \Throwable. Honest naming prevents 'why didn't the orphan get cleaned up' debugging time later. 4. Inconsistent partner-side error message: 'Column limit exceeded' on line 949 was a copy-paste drift from the parent block's correct 'Attribute limit exceeded'. Same conceptual failure should surface the same user-visible message. 5. TwoWayPartner value object + resolveTwoWayPartner() helper. Three sites (dropAttributeForRecreate, refreshTwoWayPartnerOnDelete, dropOrphanAttribute) independently extracted relatedCollection / twoWayKey from options with subtly different fallback handling (?? null vs ?? ''). Single helper returns ?TwoWayPartner with relatedTable / partnerKey / partnerMetaId; null when not two-way or the partner table has gone away. Inconsistent fallbacks were a P1 bug waiting to ship. All four are P1 reviewer concerns. Larger items (split createField, template-method for the pre-check skeleton) deferred to a follow-up PR per the user's explicit instruction. 31/31 unit tests pass, pint clean, phpstan level 3 clean. --- src/Migration/Destinations/Appwrite.php | 166 +++++++++++-------- src/Migration/Destinations/TwoWayPartner.php | 15 ++ 2 files changed, 110 insertions(+), 71 deletions(-) create mode 100644 src/Migration/Destinations/TwoWayPartner.php diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 7eedb82d..c206c35d 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -63,6 +63,11 @@ class Appwrite extends Destination { + /** Names of the platform-DB collections holding Appwrite schema metadata. */ + private const META_DATABASES = 'databases'; + private const META_ATTRIBUTES = 'attributes'; + private const META_INDEXES = 'indexes'; + /** Fields the SDK's per-type updateXAttribute endpoints don't expose; a change here forces DropAndRecreate. */ private const ATTRIBUTE_NON_SDK_FIELDS = [ 'type', @@ -176,10 +181,19 @@ public function run( string $rootResourceId = '', string $rootResourceType = '', ): void { + $this->resetRunState(); parent::run($resources, $callback, $rootResourceId, $rootResourceType); $this->cleanupUpsertOrphans(); } + /** Per-run state must not leak across run() invocations on a reused instance. */ + private function resetRunState(): void + { + $this->rowBuffer = []; + $this->orphansByTable = []; + $this->processedTwoWayPairs = []; + } + public static function getName(): string { return 'Appwrite'; @@ -481,7 +495,7 @@ protected function createDatabase(Database $resource): bool $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); if ($this->onDuplicate !== OnDuplicate::Fail) { - $existing = $this->dbForProject->getDocument('databases', $resource->getId()); + $existing = $this->dbForProject->getDocument(self::META_DATABASES, $resource->getId()); $action = $this->onDuplicate->resolveSchemaAction( !$existing->isEmpty(), $createdAt, @@ -498,7 +512,7 @@ protected function createDatabase(Database $resource): bool return false; })(), SchemaAction::UpdateInPlace => (function () use ($resource, $existing, $updatedAt): bool { - $this->dbForProject->updateDocument('databases', $existing->getId(), new UtopiaDocument([ + $this->dbForProject->updateDocument(self::META_DATABASES, $existing->getId(), new UtopiaDocument([ 'name' => $resource->getDatabaseName(), 'search' => implode(' ', [$resource->getId(), $resource->getDatabaseName()]), 'enabled' => $resource->getEnabled(), @@ -517,7 +531,7 @@ protected function createDatabase(Database $resource): bool } } - $database = $this->dbForProject->createDocument('databases', new UtopiaDocument([ + $database = $this->dbForProject->createDocument(self::META_DATABASES, new UtopiaDocument([ '$id' => $resource->getId(), 'name' => $resource->getDatabaseName(), 'enabled' => $resource->getEnabled(), @@ -575,7 +589,7 @@ protected function createEntity(Table $resource): bool } $database = $this->dbForProject->getDocument( - 'databases', + self::META_DATABASES, $resource->getDatabase()->getId() ); @@ -705,7 +719,7 @@ protected function createField(Column|Attribute $resource): bool }; $database = $this->dbForProject->getDocument( - 'databases', + self::META_DATABASES, $resource->getTable()->getDatabase()->getId(), ); @@ -800,7 +814,7 @@ protected function createField(Column|Attribute $resource): bool $attributeMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); if ($this->onDuplicate !== OnDuplicate::Fail) { - $existingAttr = $this->dbForProject->getDocument('attributes', $attributeMetaId); + $existingAttr = $this->dbForProject->getDocument(self::META_ATTRIBUTES, $attributeMetaId); $action = $this->onDuplicate->resolveSchemaAction( !$existingAttr->isEmpty(), $createdAt, @@ -861,7 +875,7 @@ protected function createField(Column|Attribute $resource): bool $this->dbForProject->checkAttribute($table, $column); - $column = $this->dbForProject->createDocument('attributes', $column); + $column = $this->dbForProject->createDocument(self::META_ATTRIBUTES, $column); } catch (DuplicateException) { throw new Exception( resourceName: $resource->getName(), @@ -915,9 +929,9 @@ protected function createField(Column|Attribute $resource): bool '$updatedAt' => $updatedAt, ]); - $this->dbForProject->createDocument('attributes', $twoWayAttribute); + $this->dbForProject->createDocument(self::META_ATTRIBUTES, $twoWayAttribute); } catch (DuplicateException) { - $this->dbForProject->deleteDocument('attributes', $column->getId()); + $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $column->getId()); throw new Exception( resourceName: $resource->getName(), @@ -926,13 +940,13 @@ protected function createField(Column|Attribute $resource): bool message: 'Attribute already exists', ); } catch (LimitException) { - $this->dbForProject->deleteDocument('attributes', $column->getId()); + $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $column->getId()); throw new Exception( resourceName: $resource->getName(), resourceGroup: $resource->getGroup(), resourceId: $resource->getId(), - message: 'Column limit exceeded', + message: 'Attribute limit exceeded', ); } catch (\Throwable $e) { $this->purgeTableCaches($database, $relatedTable, $dbForDatabases); @@ -978,10 +992,10 @@ protected function createField(Column|Attribute $resource): bool } } } catch (\Throwable) { - $this->dbForProject->deleteDocument('attributes', $column->getId()); + $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $column->getId()); if (isset($twoWayAttribute)) { - $this->dbForProject->deleteDocument('attributes', $twoWayAttribute->getId()); + $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $twoWayAttribute->getId()); } throw new Exception( @@ -1012,7 +1026,7 @@ protected function createField(Column|Attribute $resource): bool protected function createIndex(Index $resource): bool { $database = $this->dbForProject->getDocument( - 'databases', + self::META_DATABASES, $resource->getTable()->getDatabase()->getId(), ); if ($database->isEmpty()) { @@ -1038,7 +1052,7 @@ protected function createIndex(Index $resource): bool } $dbForDatabases = ($this->getDatabasesDB)($database); - $count = $this->dbForProject->count('indexes', [ + $count = $this->dbForProject->count(self::META_INDEXES, [ Query::equal('collectionInternalId', [$table->getSequence()]), Query::equal('databaseInternalId', [$database->getSequence()]) ], $dbForDatabases->getLimitForIndexes()); @@ -1118,7 +1132,7 @@ protected function createIndex(Index $resource): bool $indexMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); if ($this->onDuplicate !== OnDuplicate::Fail) { - $existingIdx = $this->dbForProject->getDocument('indexes', $indexMetaId); + $existingIdx = $this->dbForProject->getDocument(self::META_INDEXES, $indexMetaId); $action = $this->onDuplicate->resolveSchemaAction( !$existingIdx->isEmpty(), $createdAt, @@ -1150,12 +1164,12 @@ protected function createIndex(Index $resource): bool if ($action === SchemaAction::DropAndRecreate || $action === SchemaAction::UpdateInPlace) { $dbForDatabases->deleteIndex($this->tableCollectionId($database, $table), $resource->getKey()); - $this->dbForProject->deleteDocument('indexes', $indexMetaId); + $this->dbForProject->deleteDocument(self::META_INDEXES, $indexMetaId); $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); } } - $index = $this->dbForProject->createDocument('indexes', $index); + $index = $this->dbForProject->createDocument(self::META_INDEXES, $index); try { $result = $dbForDatabases->createIndex( @@ -1176,7 +1190,7 @@ protected function createIndex(Index $resource): bool ); } } catch (\Throwable $th) { - $this->dbForProject->deleteDocument('indexes', $index->getId()); + $this->dbForProject->deleteDocument(self::META_INDEXES, $index->getId()); throw new Exception( resourceName: $resource->getName(), @@ -1263,7 +1277,7 @@ protected function createRecord(Row $resource, bool $isLast): bool if ($isLast) { try { $database = $this->dbForProject->getDocument( - 'databases', + self::META_DATABASES, $resource->getTable()->getDatabase()->getId(), ); @@ -1347,21 +1361,13 @@ private function dropAttributeForRecreate( $dbForDatabases->deleteAttribute($collectionId, $resource->getKey()); } - $this->dbForProject->deleteDocument('attributes', $attributeMetaId); + $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $attributeMetaId); // deleteRelationship only touches utopia's _metadata; clear the Appwrite-level partner meta doc too. - if ($isRelationship && ($resource->getOptions()['twoWay'] ?? false)) { - $relatedTableId = $resource->getOptions()['relatedCollection'] ?? null; - $twoWayKey = $resource->getOptions()['twoWayKey'] ?? null; - if ($relatedTableId !== null && $twoWayKey !== null) { - $relatedTable = $this->dbForProject->getDocument( - $this->databaseCollectionId($database), - $relatedTableId, - ); - if (!$relatedTable->isEmpty()) { - $partnerMetaId = $this->attributeIndexMetaId($database, $relatedTable, $twoWayKey); - $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $partnerMetaId)); - } + if ($isRelationship) { + $partner = $this->resolveTwoWayPartner($database, $resource->getOptions()); + if ($partner !== null) { + $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner->partnerMetaId)); } } @@ -1411,7 +1417,7 @@ private function updateAttributeInPlace( filters: $existingAttr->getAttribute('filters'), ); - $this->dbForProject->updateDocument('attributes', $existingAttr->getId(), new UtopiaDocument([ + $this->dbForProject->updateDocument(self::META_ATTRIBUTES, $existingAttr->getId(), new UtopiaDocument([ 'key' => $resource->getKey(), 'type' => $type, 'size' => $resource->getSize(), @@ -1464,7 +1470,7 @@ private function updateRelationshipInPlace( ); } - $this->dbForProject->updateDocument('attributes', $existingAttr->getId(), new UtopiaDocument([ + $this->dbForProject->updateDocument(self::META_ATTRIBUTES, $existingAttr->getId(), new UtopiaDocument([ 'key' => $resource->getKey(), 'type' => $type, 'options' => array_merge($destOptions, [ @@ -1485,6 +1491,10 @@ private function updateRelationshipInPlace( return true; } + /** + * @param array $destOptions + * @param array $sourceOptions + */ private function refreshTwoWayPartnerOnDelete( UtopiaDocument $database, array $destOptions, @@ -1492,36 +1502,24 @@ private function refreshTwoWayPartnerOnDelete( string $updatedAt, UtopiaDatabase $dbForDatabases, ): void { - $relatedTableId = (string) ($destOptions['relatedCollection'] ?? ''); - $partnerKey = (string) ($destOptions['twoWayKey'] ?? ''); - if ($relatedTableId === '' || $partnerKey === '') { + $partner = $this->resolveTwoWayPartner($database, $destOptions); + if ($partner === null) { return; } - $relatedTable = $this->dbForProject->getDocument( - $this->databaseCollectionId($database), - $relatedTableId, - ); - if ($relatedTable->isEmpty()) { - return; - } - - $partnerMeta = $this->dbForProject->getDocument( - 'attributes', - $this->attributeIndexMetaId($database, $relatedTable, $partnerKey), - ); + $partnerMeta = $this->dbForProject->getDocument(self::META_ATTRIBUTES, $partner->partnerMetaId); if ($partnerMeta->isEmpty()) { return; } $partnerOptions = $partnerMeta->getAttribute('options', []); - $this->dbForProject->updateDocument('attributes', $partnerMeta->getId(), new UtopiaDocument([ + $this->dbForProject->updateDocument(self::META_ATTRIBUTES, $partnerMeta->getId(), new UtopiaDocument([ 'options' => array_merge($partnerOptions, [ 'onDelete' => $sourceOptions['onDelete'] ?? $partnerOptions['onDelete'] ?? null, ]), '$updatedAt' => $updatedAt, ])); - $this->purgeTableCaches($database, $relatedTable, $dbForDatabases); + $this->purgeTableCaches($database, $partner->relatedTable, $dbForDatabases); } /** @@ -1540,6 +1538,38 @@ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): return $database->getSequence() . ':' . $table->getSequence(); } + /** + * Resolve a two-way relationship's partner-side context (related table doc, + * partner attribute key, partner meta-doc id) from a relationship's options. + * Returns null if not two-way, options are missing the partner pointer, + * or the partner table no longer exists. + * + * @param array $options + */ + private function resolveTwoWayPartner(UtopiaDocument $database, array $options): ?TwoWayPartner + { + if (empty($options['twoWay'])) { + return null; + } + $relatedTableId = (string) ($options['relatedCollection'] ?? ''); + $partnerKey = (string) ($options['twoWayKey'] ?? ''); + if ($relatedTableId === '' || $partnerKey === '') { + return null; + } + $relatedTable = $this->dbForProject->getDocument( + $this->databaseCollectionId($database), + $relatedTableId, + ); + if ($relatedTable->isEmpty()) { + return null; + } + return new TwoWayPartner( + $relatedTable, + $partnerKey, + $this->attributeIndexMetaId($database, $relatedTable, $partnerKey), + ); + } + /** * Canonical pair-key for a two-way relationship — same string regardless * of which side (parent or partner) is being processed. Returns null for @@ -1659,7 +1689,7 @@ private function cleanupUpsertOrphansForTable(string $tableId): void $dbForDatabases = $tracked['dbForDatabases']; $this->dropOrphansByKind( - 'attributes', + self::META_ATTRIBUTES, $tracked['attributeKeys'], $database, $table, @@ -1667,7 +1697,7 @@ private function cleanupUpsertOrphansForTable(string $tableId): void ); $this->dropOrphansByKind( - 'indexes', + self::META_INDEXES, $tracked['indexKeys'], $database, $table, @@ -1718,31 +1748,25 @@ private function dropOrphanAttribute( $collectionId = $this->tableCollectionId($database, $table); if ($type === UtopiaDatabase::VAR_RELATIONSHIP) { - $this->tolerateMissing(fn () => $dbForDatabases->deleteRelationship($collectionId, $key)); + $this->bestEffort(fn () => $dbForDatabases->deleteRelationship($collectionId, $key)); } else { - $this->tolerateMissing(fn () => $dbForDatabases->deleteAttribute($collectionId, $key)); + $this->bestEffort(fn () => $dbForDatabases->deleteAttribute($collectionId, $key)); } - $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $attrDoc->getId())); + $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $attrDoc->getId())); $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); $dbForDatabases->purgeCachedCollection($collectionId); - if ($type !== UtopiaDatabase::VAR_RELATIONSHIP || empty($options['twoWay'])) { - return; - } - $twoWayKey = (string) ($options['twoWayKey'] ?? ''); - $relatedTableId = (string) ($options['relatedCollection'] ?? ''); - if ($twoWayKey === '' || $relatedTableId === '') { + if ($type !== UtopiaDatabase::VAR_RELATIONSHIP) { return; } - $relatedTable = $this->dbForProject->getDocument($this->databaseCollectionId($database), $relatedTableId); - if ($relatedTable->isEmpty()) { + $partner = $this->resolveTwoWayPartner($database, $options); + if ($partner === null) { return; } // deleteRelationship already dropped the partner physical column; only the Appwrite-level meta doc remains. - $childMetaId = $this->attributeIndexMetaId($database, $relatedTable, $twoWayKey); - $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('attributes', $childMetaId)); - $this->purgeTableCaches($database, $relatedTable, $dbForDatabases); + $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner->partnerMetaId)); + $this->purgeTableCaches($database, $partner->relatedTable, $dbForDatabases); } private function dropOrphanIndex( @@ -1754,13 +1778,13 @@ private function dropOrphanIndex( $collectionId = $this->tableCollectionId($database, $table); $indexMetaId = $this->attributeIndexMetaId($database, $table, $indexKey); - $this->tolerateMissing(fn () => $dbForDatabases->deleteIndex($collectionId, $indexKey)); - $this->tolerateMissing(fn () => $this->dbForProject->deleteDocument('indexes', $indexMetaId)); + $this->bestEffort(fn () => $dbForDatabases->deleteIndex($collectionId, $indexKey)); + $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_INDEXES, $indexMetaId)); $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); } /** Swallows deletion errors — a prior run or relationship cascade may have already removed the target. */ - private function tolerateMissing(callable $fn): void + private function bestEffort(callable $fn): void { try { $fn(); diff --git a/src/Migration/Destinations/TwoWayPartner.php b/src/Migration/Destinations/TwoWayPartner.php new file mode 100644 index 00000000..afb1cea4 --- /dev/null +++ b/src/Migration/Destinations/TwoWayPartner.php @@ -0,0 +1,15 @@ + Date: Tue, 28 Apr 2026 03:30:55 +0100 Subject: [PATCH 23/36] Inline TwoWayPartner as a typed array shape The 3-field readonly value object was thin enough that an array shape (typed via phpdoc) covers the same phpstan-checkable surface without a separate class file. Helper return type narrows from ?TwoWayPartner to ?array{relatedTable, partnerKey, partnerMetaId} and call sites switch from $partner->field to $partner['field']. --- src/Migration/Destinations/Appwrite.php | 26 ++++++++++---------- src/Migration/Destinations/TwoWayPartner.php | 15 ----------- 2 files changed, 13 insertions(+), 28 deletions(-) delete mode 100644 src/Migration/Destinations/TwoWayPartner.php diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index c206c35d..839fcb03 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -1367,7 +1367,7 @@ private function dropAttributeForRecreate( if ($isRelationship) { $partner = $this->resolveTwoWayPartner($database, $resource->getOptions()); if ($partner !== null) { - $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner->partnerMetaId)); + $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner['partnerMetaId'])); } } @@ -1507,7 +1507,7 @@ private function refreshTwoWayPartnerOnDelete( return; } - $partnerMeta = $this->dbForProject->getDocument(self::META_ATTRIBUTES, $partner->partnerMetaId); + $partnerMeta = $this->dbForProject->getDocument(self::META_ATTRIBUTES, $partner['partnerMetaId']); if ($partnerMeta->isEmpty()) { return; } @@ -1519,7 +1519,7 @@ private function refreshTwoWayPartnerOnDelete( ]), '$updatedAt' => $updatedAt, ])); - $this->purgeTableCaches($database, $partner->relatedTable, $dbForDatabases); + $this->purgeTableCaches($database, $partner['relatedTable'], $dbForDatabases); } /** @@ -1539,14 +1539,14 @@ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): } /** - * Resolve a two-way relationship's partner-side context (related table doc, - * partner attribute key, partner meta-doc id) from a relationship's options. + * Resolve a two-way relationship's partner-side context from options. * Returns null if not two-way, options are missing the partner pointer, * or the partner table no longer exists. * * @param array $options + * @return array{relatedTable: UtopiaDocument, partnerKey: string, partnerMetaId: string}|null */ - private function resolveTwoWayPartner(UtopiaDocument $database, array $options): ?TwoWayPartner + private function resolveTwoWayPartner(UtopiaDocument $database, array $options): ?array { if (empty($options['twoWay'])) { return null; @@ -1563,11 +1563,11 @@ private function resolveTwoWayPartner(UtopiaDocument $database, array $options): if ($relatedTable->isEmpty()) { return null; } - return new TwoWayPartner( - $relatedTable, - $partnerKey, - $this->attributeIndexMetaId($database, $relatedTable, $partnerKey), - ); + return [ + 'relatedTable' => $relatedTable, + 'partnerKey' => $partnerKey, + 'partnerMetaId' => $this->attributeIndexMetaId($database, $relatedTable, $partnerKey), + ]; } /** @@ -1765,8 +1765,8 @@ private function dropOrphanAttribute( } // deleteRelationship already dropped the partner physical column; only the Appwrite-level meta doc remains. - $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner->partnerMetaId)); - $this->purgeTableCaches($database, $partner->relatedTable, $dbForDatabases); + $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner['partnerMetaId'])); + $this->purgeTableCaches($database, $partner['relatedTable'], $dbForDatabases); } private function dropOrphanIndex( diff --git a/src/Migration/Destinations/TwoWayPartner.php b/src/Migration/Destinations/TwoWayPartner.php deleted file mode 100644 index afb1cea4..00000000 --- a/src/Migration/Destinations/TwoWayPartner.php +++ /dev/null @@ -1,15 +0,0 @@ - Date: Tue, 28 Apr 2026 03:44:46 +0100 Subject: [PATCH 24/36] Lock-in test for ATTRIBUTE_NON_SDK_FIELDS via SDK reflection Reflects on appwrite/appwrite's TablesDB and Databases services to build the set of meta-doc fields the SDK can mutate via update*Column / update*Attribute endpoints, then asserts no overlap with our ATTRIBUTE_NON_SDK_FIELDS constant. When the SDK ships a new mutable param (e.g. exposing 'signed' or 'array' on Update), CI fails so we sync the constant. Catches drift in CI rather than at runtime. PARAM_TO_META_FIELD documents the SDK-vs-meta-doc vocabulary mismatch: - xdefault -> default (PHP keyword workaround) - min/max/elements -> null (sub-fields of formatOptions, not direct) - onDelete -> null (sub-field of relationship options) Currently observed SDK-exposed top-level fields: required, default, size. Constant correctly lists: type, array, signed, format, formatOptions, filters. Zero overlap, test passes. --- .../AppwriteAttributeSdkBoundaryTest.php | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php diff --git a/tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php b/tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php new file mode 100644 index 00000000..3b2fdddc --- /dev/null +++ b/tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php @@ -0,0 +1,96 @@ + + */ + private const PARAM_TO_META_FIELD = [ + 'xdefault' => 'default', + 'min' => null, + 'max' => null, + 'elements' => null, + 'onDelete' => null, + ]; + + public function testSdkExposedFieldsAreNotInNonSdkConstant(): void + { + $sdkExposed = $this->collectSdkExposedMetaFields(); + $nonSdk = $this->readNonSdkConstant(); + + $overlap = \array_intersect($sdkExposed, $nonSdk); + $this->assertSame( + [], + \array_values($overlap), + 'ATTRIBUTE_NON_SDK_FIELDS marks these fields as non-SDK, but the appwrite/appwrite ' + . 'SDK exposes them via updateXColumn/updateXAttribute: ' . \implode(', ', $overlap) + . '. Either drop these from ATTRIBUTE_NON_SDK_FIELDS or update PARAM_TO_META_FIELD ' + . 'in this test if the SDK param doesn\'t correspond to a top-level meta-doc field.', + ); + } + + /** + * @return list meta-doc field names the SDK can mutate + */ + private function collectSdkExposedMetaFields(): array + { + $fields = []; + foreach ([TablesDB::class, Databases::class] as $service) { + $service = new ReflectionClass($service); + foreach ($service->getMethods(ReflectionMethod::IS_PUBLIC) as $method) { + if (!\preg_match('/^update.+(Column|Attribute)$/', $method->getName())) { + continue; + } + foreach ($method->getParameters() as $param) { + $name = $param->getName(); + if (\in_array($name, self::IGNORED_PARAMS, true)) { + continue; + } + $mapped = \array_key_exists($name, self::PARAM_TO_META_FIELD) + ? self::PARAM_TO_META_FIELD[$name] + : $name; + if ($mapped === null) { + continue; + } + $fields[$mapped] = true; + } + } + } + return \array_keys($fields); + } + + /** + * @return list + */ + private function readNonSdkConstant(): array + { + /** @var list $value */ + $value = (new ReflectionClass(AppwriteDestination::class))->getConstant('ATTRIBUTE_NON_SDK_FIELDS'); + return $value; + } +} From c8d17898458de4a297ba03fd1a126f7aa13227af Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 04:12:58 +0100 Subject: [PATCH 25/36] Spec-match guard: skip drop when source and dest already match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an unconditional spec-match check after resolveSchemaAction returns DropAndRecreate or UpdateInPlace. If source's full attribute/table/db/ index spec already matches dest, action is forced to Tolerate — no DDL, no in-place call, no row-data wipe. Closes the gap where a user dropping + recreating a column on source with the same spec triggered a wasteful drop+recreate cascade on dest (createdAt advanced even though spec was unchanged). Row pass under Upsert mode still propagates source row values via upsertDocuments. Helpers: databaseSpecMatches, tableSpecMatches, attributeSpecMatches (handles relationship via RELATIONSHIP_STRUCTURAL_FIELDS + onDelete), indexSpecMatches (existing). Inserted as a single override at each of the four pre-check sites. createIndex: removed the inline indexSpecMatches IIFE inside UpdateInPlace since the unified outer guard now subsumes it. The 'Index spec unchanged' status message merges into the generic 'Already exists on destination' — slight loss of fidelity, gain in symmetry across the four create methods. 47/47 unit tests pass, pint clean, phpstan level 3 clean. --- src/Migration/Destinations/Appwrite.php | 83 ++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 839fcb03..c648a9d4 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -504,6 +504,9 @@ protected function createDatabase(Database $resource): bool $existing->getUpdatedAt(), canDrop: false, ); + if ($action !== SchemaAction::Create && $this->databaseSpecMatches($existing, $resource)) { + $action = SchemaAction::Tolerate; + } $earlyReturn = match ($action) { SchemaAction::Tolerate => (function () use ($resource, $existing): bool { @@ -625,6 +628,9 @@ protected function createEntity(Table $resource): bool $existing->getUpdatedAt(), canDrop: false, ); + if ($action !== SchemaAction::Create && $this->tableSpecMatches($existing, $resource)) { + $action = SchemaAction::Tolerate; + } $earlyReturn = match ($action) { SchemaAction::Tolerate => (function () use ($resource, $existing): bool { @@ -823,6 +829,9 @@ protected function createField(Column|Attribute $resource): bool $existingAttr->getUpdatedAt(), canDrop: true, ); + if ($action !== SchemaAction::Create && $this->attributeSpecMatches($existingAttr, $resource, $type, $isRelationship)) { + $action = SchemaAction::Tolerate; + } // UpdateInPlace returns null when the source change isn't SDK-expressible — falls through to the drop step below. $earlyReturn = match ($action) { @@ -1141,22 +1150,18 @@ protected function createIndex(Index $resource): bool $existingIdx->getUpdatedAt(), canDrop: true, ); + if ($action !== SchemaAction::Create && $this->indexSpecMatches($existingIdx, $resource)) { + $action = SchemaAction::Tolerate; + } - // Indexes have no in-place primitive — UpdateInPlace tolerates if spec matches, else drop+recreate. + // Indexes have no in-place primitive — any action other than Tolerate falls through to drop+recreate. $earlyReturn = match ($action) { SchemaAction::Tolerate => (function () use ($resource, $database, $table): bool { $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; })(), - SchemaAction::UpdateInPlace => $this->indexSpecMatches($existingIdx, $resource) - ? (function () use ($resource, $database, $table): bool { - $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); - $resource->setStatus(Resource::STATUS_SKIPPED, 'Index spec unchanged'); - return false; - })() - : null, - SchemaAction::DropAndRecreate, SchemaAction::Create => null, + SchemaAction::UpdateInPlace, SchemaAction::DropAndRecreate, SchemaAction::Create => null, }; if ($earlyReturn !== null) { return $earlyReturn; @@ -1522,6 +1527,66 @@ private function refreshTwoWayPartnerOnDelete( $this->purgeTableCaches($database, $partner['relatedTable'], $dbForDatabases); } + private function databaseSpecMatches(UtopiaDocument $existing, Database $resource): bool + { + $sourceType = empty($resource->getType()) ? 'legacy' : $resource->getType(); + $sourceOriginalId = empty($resource->getOriginalId()) ? null : $resource->getOriginalId(); + + return $existing->getAttribute('name') === $resource->getDatabaseName() + && $existing->getAttribute('enabled') === $resource->getEnabled() + && $existing->getAttribute('type') === $sourceType + && $existing->getAttribute('originalId') === $sourceOriginalId + && $existing->getAttribute('database') === $resource->getDatabase(); + } + + private function tableSpecMatches(UtopiaDocument $existing, Table $resource): bool + { + if ($existing->getAttribute('name') !== $resource->getTableName() + || $existing->getAttribute('enabled') !== $resource->getEnabled() + || $existing->getAttribute('documentSecurity') !== $resource->getRowSecurity()) { + return false; + } + + $sourcePerms = Permission::aggregate($resource->getPermissions()); + /** @var list $destPerms */ + $destPerms = $existing->getAttribute('$permissions', []); + \sort($sourcePerms); + \sort($destPerms); + return $sourcePerms === $destPerms; + } + + /** + * Compares the full attribute spec including SDK-reachable fields. Used to + * short-circuit DropAndRecreate / UpdateInPlace when source and destination + * already match — no DDL needed regardless of timestamp drift. + */ + private function attributeSpecMatches(UtopiaDocument $existing, Column|Attribute $resource, string $type, bool $isRelationship): bool + { + if ($existing->getAttribute('type') !== $type) { + return false; + } + if ($isRelationship) { + $sourceOptions = $resource->getOptions(); + /** @var array $destOptions */ + $destOptions = $existing->getAttribute('options', []); + foreach (self::RELATIONSHIP_STRUCTURAL_FIELDS as $field) { + if (($sourceOptions[$field] ?? null) !== ($destOptions[$field] ?? null)) { + return false; + } + } + return ($sourceOptions['onDelete'] ?? null) === ($destOptions['onDelete'] ?? null); + } + + return $existing->getAttribute('size') === $resource->getSize() + && $existing->getAttribute('required') === $resource->isRequired() + && $existing->getAttribute('default') === $resource->getDefault() + && $existing->getAttribute('array') === $resource->isArray() + && $existing->getAttribute('signed') === $resource->isSigned() + && $existing->getAttribute('format') === $resource->getFormat() + && $existing->getAttribute('formatOptions') === $resource->getFormatOptions() + && $existing->getAttribute('filters') === $resource->getFilters(); + } + /** * `lengths` is intentionally not compared: it's derived per-column from * the adapter's index validator and not settable on the Index resource. From 89078ae8deeece9ec1fa16b6e738765e8a4b9240 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 04:24:38 +0100 Subject: [PATCH 26/36] Trim verbose comments and helper docblocks Mostly mechanical reduction of multi-line WHY blocks to one-liners and removal of obvious WHAT comments. Keeps load-bearing rationale (e.g. 'utopia syncs both physical sides; partner meta has to be refreshed by hand') as one short line per concern. --- src/Migration/Destinations/Appwrite.php | 43 ++++--------------------- 1 file changed, 7 insertions(+), 36 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index c648a9d4..44b252c9 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -125,12 +125,7 @@ class Appwrite extends Destination private array $orphansByTable = []; /** - * Two-way relationship pairs already reconciled this run, keyed by a - * canonical pair-key. Source emits both sides as separate Column - * resources, but processing one side already reconciles both physical - * columns + both Appwrite-level meta docs. Without this set, the - * partner pass can re-trigger DropAndRecreate and destroy partner-table - * rows already migrated. + * Two-way pairs already reconciled this run; partner pass short-circuits. * * @var array */ @@ -803,15 +798,9 @@ protected function createField(Column|Attribute $resource): bool $this->trackOrphanCandidate($database, $table, 'attributeKeys', $resource->getKey(), $dbForDatabases); - // Both rel types route through deleteRelationship — deleteAttribute throws for VAR_RELATIONSHIP. - // Two-way drop loses partner row data, restored by the subsequent Upsert row pass. $isRelationship = $type === UtopiaDatabase::VAR_RELATIONSHIP; - // Two-way relationships emit two source-side resources but reconcile both - // physical columns + both Appwrite-level meta docs in one pass. Skip the - // partner — re-processing can fire DropAndRecreate again and destroy - // partner-table rows already migrated this run (parent and partner have - // independent source createdAt values that may not match). + // Source emits both sides of a two-way; processing one side reconciles both. Partner skip. $twoWayPairKey = $this->twoWayPairKey($database, $table, $resource, $type); if ($twoWayPairKey !== null && isset($this->processedTwoWayPairs[$twoWayPairKey])) { $resource->setStatus(Resource::STATUS_SKIPPED, 'Two-way partner already reconciled'); @@ -833,7 +822,6 @@ protected function createField(Column|Attribute $resource): bool $action = SchemaAction::Tolerate; } - // UpdateInPlace returns null when the source change isn't SDK-expressible — falls through to the drop step below. $earlyReturn = match ($action) { SchemaAction::Tolerate => (function () use ($resource, $database, $table, $dbForDatabases): bool { $this->purgeTableCaches($database, $table, $dbForDatabases); @@ -1293,12 +1281,9 @@ protected function createRecord(Row $resource, bool $isLast): bool $dbForDatabases = ($this->getDatabasesDB)($database); - // Drop schema-level orphans before rows land so the Structure validator doesn't reject on orphan required columns. + // Drop schema orphans before rows land so the Structure validator doesn't reject on orphan required columns. $this->cleanupUpsertOrphansForTable($this->tableIdentity($database, $table)); - /** - * This is in case an attribute was deleted from Appwrite attributes collection but was not deleted from the table - * When creating an archive we select * which will include orphan attribute from the schema - */ + // Strip row payload fields the table doesn't declare — guards against orphans surviving in source archives. if ($dbForDatabases->getAdapter()->getSupportForAttributes()) { foreach ($this->rowBuffer as $row) { foreach ($row as $key => $value) { @@ -1486,9 +1471,7 @@ private function updateRelationshipInPlace( $this->purgeTableCaches($database, $table, $dbForDatabases); - // utopia's updateRelationship synced the physical constraint on both sides but - // not the Appwrite-level partner meta doc — refresh it so reads from the related - // collection don't see stale onDelete. + // utopia syncs both physical sides; partner's Appwrite-level meta doc has to be refreshed by hand. if ($isTwoWay) { $this->refreshTwoWayPartnerOnDelete($database, $destOptions, $sourceOptions, $updatedAt, $dbForDatabases); } @@ -1555,11 +1538,7 @@ private function tableSpecMatches(UtopiaDocument $existing, Table $resource): bo return $sourcePerms === $destPerms; } - /** - * Compares the full attribute spec including SDK-reachable fields. Used to - * short-circuit DropAndRecreate / UpdateInPlace when source and destination - * already match — no DDL needed regardless of timestamp drift. - */ + /** Full-spec equality: short-circuits both DropAndRecreate and UpdateInPlace to Tolerate when nothing changed. */ private function attributeSpecMatches(UtopiaDocument $existing, Column|Attribute $resource, string $type, bool $isRelationship): bool { if ($existing->getAttribute('type') !== $type) { @@ -1604,10 +1583,6 @@ private function tableIdentity(UtopiaDocument $database, UtopiaDocument $table): } /** - * Resolve a two-way relationship's partner-side context from options. - * Returns null if not two-way, options are missing the partner pointer, - * or the partner table no longer exists. - * * @param array $options * @return array{relatedTable: UtopiaDocument, partnerKey: string, partnerMetaId: string}|null */ @@ -1635,11 +1610,7 @@ private function resolveTwoWayPartner(UtopiaDocument $database, array $options): ]; } - /** - * Canonical pair-key for a two-way relationship — same string regardless - * of which side (parent or partner) is being processed. Returns null for - * non-two-way attributes (no pair-level tracking needed). - */ + /** Canonical pair-key — same string regardless of which side is being processed. */ private function twoWayPairKey( UtopiaDocument $database, UtopiaDocument $table, From 36a1acf75027a22b0ad1a7374b8f88c94ab30f68 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 05:16:28 +0100 Subject: [PATCH 27/36] Spec-match: order-independent assoc-array equality + comments - valuesMatch helper: ksort assoc arrays before === so {min,max} matches {max,min}. Lists (filters, columns) keep order-sensitive comparison since order is semantically meaningful there. - Apply via attributeSpecMatches (formatOptions, relationship options, onDelete) and arraysDifferOnKeys (used by updateAttributeInPlace and updateRelationshipInPlace). - One-line comment on each spec-match guard documenting why the Create case is excluded (no existing dest doc to match against). --- src/Migration/Destinations/Appwrite.php | 40 ++++++++++++++++++------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 44b252c9..1a1acfbd 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -499,6 +499,7 @@ protected function createDatabase(Database $resource): bool $existing->getUpdatedAt(), canDrop: false, ); + // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->databaseSpecMatches($existing, $resource)) { $action = SchemaAction::Tolerate; } @@ -623,6 +624,7 @@ protected function createEntity(Table $resource): bool $existing->getUpdatedAt(), canDrop: false, ); + // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->tableSpecMatches($existing, $resource)) { $action = SchemaAction::Tolerate; } @@ -818,6 +820,7 @@ protected function createField(Column|Attribute $resource): bool $existingAttr->getUpdatedAt(), canDrop: true, ); + // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->attributeSpecMatches($existingAttr, $resource, $type, $isRelationship)) { $action = SchemaAction::Tolerate; } @@ -1138,6 +1141,7 @@ protected function createIndex(Index $resource): bool $existingIdx->getUpdatedAt(), canDrop: true, ); + // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->indexSpecMatches($existingIdx, $resource)) { $action = SchemaAction::Tolerate; } @@ -1549,21 +1553,21 @@ private function attributeSpecMatches(UtopiaDocument $existing, Column|Attribute /** @var array $destOptions */ $destOptions = $existing->getAttribute('options', []); foreach (self::RELATIONSHIP_STRUCTURAL_FIELDS as $field) { - if (($sourceOptions[$field] ?? null) !== ($destOptions[$field] ?? null)) { + if (!$this->valuesMatch($sourceOptions[$field] ?? null, $destOptions[$field] ?? null)) { return false; } } - return ($sourceOptions['onDelete'] ?? null) === ($destOptions['onDelete'] ?? null); + return $this->valuesMatch($sourceOptions['onDelete'] ?? null, $destOptions['onDelete'] ?? null); } - return $existing->getAttribute('size') === $resource->getSize() - && $existing->getAttribute('required') === $resource->isRequired() - && $existing->getAttribute('default') === $resource->getDefault() - && $existing->getAttribute('array') === $resource->isArray() - && $existing->getAttribute('signed') === $resource->isSigned() - && $existing->getAttribute('format') === $resource->getFormat() - && $existing->getAttribute('formatOptions') === $resource->getFormatOptions() - && $existing->getAttribute('filters') === $resource->getFilters(); + return $existing->getAttribute('size') === $resource->getSize() + && $existing->getAttribute('required') === $resource->isRequired() + && $existing->getAttribute('default') === $resource->getDefault() + && $existing->getAttribute('array') === $resource->isArray() + && $existing->getAttribute('signed') === $resource->isSigned() + && $existing->getAttribute('format') === $resource->getFormat() + && $this->valuesMatch($existing->getAttribute('formatOptions'), $resource->getFormatOptions()) + && $existing->getAttribute('filters') === $resource->getFilters(); } /** @@ -1670,13 +1674,27 @@ private function purgeTableCaches( private function arraysDifferOnKeys(array $a, array $b, array $keys): bool { foreach ($keys as $key) { - if (($a[$key] ?? null) !== ($b[$key] ?? null)) { + if (!$this->valuesMatch($a[$key] ?? null, $b[$key] ?? null)) { return true; } } return false; } + /** + * `===` on associative arrays is order-sensitive on keys; ksort both sides + * before comparing so {min, max} matches {max, min}. Lists (numeric keys) + * are left alone — order is semantically meaningful for filters/columns. + */ + private function valuesMatch(mixed $a, mixed $b): bool + { + if (\is_array($a) && \is_array($b) && !\array_is_list($a) && !\array_is_list($b)) { + \ksort($a); + \ksort($b); + } + return $a === $b; + } + /** $kind is 'attributeKeys' or 'indexKeys'. */ private function trackOrphanCandidate( UtopiaDocument $database, From 6ec2c45a4077d9452ae32e4faa1b49375ad77cae Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 05:43:11 +0100 Subject: [PATCH 28/36] =?UTF-8?q?Drop=20createdAt=20handling=20=E2=80=94?= =?UTF-8?q?=20spec-match=20guard=20+=20sourceIsNewer=20cover=20everything?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveSchemaAction's createdAt branch became redundant once the spec-match guard landed. The flow: - resolveSchemaAction returns Create / Tolerate / UpdateInPlace based purely on existence and updatedAt-newer. - The destination's spec-match guard overrides UpdateInPlace to Tolerate when source and dest have identical spec. - Inner attribute/index spec checks fall through to drop+recreate when the SDK can't express the change in place. End-state behavior on every scenario is identical (verified before this commit). The createdAt path was extra work that produced the same outcome via a longer route. Removed: - $sourceCreatedAt / $destCreatedAt / $canDrop params from resolveSchemaAction - createdAtDiffers private helper - SchemaAction::DropAndRecreate enum case (no longer returned by resolveSchemaAction; inner fallthrough drops directly) - 4 call sites in Appwrite.php drop the createdAt args + canDrop: named arg + DropAndRecreate match arm + DropAndRecreate post-match if-clause - ~13 createdAt-specific unit tests; remaining 19 test the exists × mode × updatedAt matrix --- src/Migration/Destinations/Appwrite.php | 32 ++--- src/Migration/Destinations/OnDuplicate.php | 44 +------ .../Unit/General/OnDuplicateTest.php | 123 +++--------------- 3 files changed, 33 insertions(+), 166 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 1a1acfbd..d215452d 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -68,7 +68,7 @@ class Appwrite extends Destination private const META_ATTRIBUTES = 'attributes'; private const META_INDEXES = 'indexes'; - /** Fields the SDK's per-type updateXAttribute endpoints don't expose; a change here forces DropAndRecreate. */ + /** Fields the SDK's per-type updateXAttribute endpoints don't expose; a change here forces drop+recreate. */ private const ATTRIBUTE_NON_SDK_FIELDS = [ 'type', 'array', @@ -493,11 +493,8 @@ protected function createDatabase(Database $resource): bool $existing = $this->dbForProject->getDocument(self::META_DATABASES, $resource->getId()); $action = $this->onDuplicate->resolveSchemaAction( !$existing->isEmpty(), - $createdAt, - $existing->getCreatedAt(), $updatedAt, $existing->getUpdatedAt(), - canDrop: false, ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->databaseSpecMatches($existing, $resource)) { @@ -523,7 +520,7 @@ protected function createDatabase(Database $resource): bool $resource->setSequence($existing->getSequence()); return true; })(), - SchemaAction::Create, SchemaAction::DropAndRecreate => null, + SchemaAction::Create => null, }; if ($earlyReturn !== null) { return $earlyReturn; @@ -618,11 +615,8 @@ protected function createEntity(Table $resource): bool ); $action = $this->onDuplicate->resolveSchemaAction( !$existing->isEmpty(), - $createdAt, - $existing->getCreatedAt(), $updatedAt, $existing->getUpdatedAt(), - canDrop: false, ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->tableSpecMatches($existing, $resource)) { @@ -651,7 +645,7 @@ protected function createEntity(Table $resource): bool $resource->setSequence($existing->getSequence()); return true; })(), - SchemaAction::Create, SchemaAction::DropAndRecreate => null, + SchemaAction::Create => null, }; if ($earlyReturn !== null) { return $earlyReturn; @@ -814,11 +808,8 @@ protected function createField(Column|Attribute $resource): bool $existingAttr = $this->dbForProject->getDocument(self::META_ATTRIBUTES, $attributeMetaId); $action = $this->onDuplicate->resolveSchemaAction( !$existingAttr->isEmpty(), - $createdAt, - $existingAttr->getCreatedAt(), $updatedAt, $existingAttr->getUpdatedAt(), - canDrop: true, ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->attributeSpecMatches($existingAttr, $resource, $type, $isRelationship)) { @@ -836,7 +827,7 @@ protected function createField(Column|Attribute $resource): bool : $this->updateAttributeInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases)) ? true : null, - SchemaAction::DropAndRecreate, SchemaAction::Create => null, + SchemaAction::Create => null, }; if ($earlyReturn !== null) { if ($twoWayPairKey !== null) { @@ -845,7 +836,7 @@ protected function createField(Column|Attribute $resource): bool return $earlyReturn; } - if ($action === SchemaAction::DropAndRecreate || $action === SchemaAction::UpdateInPlace) { + if ($action === SchemaAction::UpdateInPlace) { $this->dropAttributeForRecreate($database, $table, $resource, $dbForDatabases); } } @@ -1135,11 +1126,8 @@ protected function createIndex(Index $resource): bool $existingIdx = $this->dbForProject->getDocument(self::META_INDEXES, $indexMetaId); $action = $this->onDuplicate->resolveSchemaAction( !$existingIdx->isEmpty(), - $createdAt, - $existingIdx->getCreatedAt(), $updatedAt, $existingIdx->getUpdatedAt(), - canDrop: true, ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->indexSpecMatches($existingIdx, $resource)) { @@ -1153,13 +1141,13 @@ protected function createIndex(Index $resource): bool $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; })(), - SchemaAction::UpdateInPlace, SchemaAction::DropAndRecreate, SchemaAction::Create => null, + SchemaAction::UpdateInPlace, SchemaAction::Create => null, }; if ($earlyReturn !== null) { return $earlyReturn; } - if ($action === SchemaAction::DropAndRecreate || $action === SchemaAction::UpdateInPlace) { + if ($action === SchemaAction::UpdateInPlace) { $dbForDatabases->deleteIndex($this->tableCollectionId($database, $table), $resource->getKey()); $this->dbForProject->deleteDocument(self::META_INDEXES, $indexMetaId); $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); @@ -1368,7 +1356,7 @@ private function dropAttributeForRecreate( $this->purgeTableCaches($database, $table, $dbForDatabases); } - /** Returns false when the source change isn't SDK-expressible — caller falls through to DropAndRecreate. */ + /** Returns false when the source change isn't SDK-expressible — caller falls through to drop+recreate. */ private function updateAttributeInPlace( UtopiaDocument $database, UtopiaDocument $table, @@ -1430,7 +1418,7 @@ private function updateAttributeInPlace( } /** - * Returns false when the source change isn't SDK-expressible — caller falls through to DropAndRecreate. + * Returns false when the source change isn't SDK-expressible — caller falls through to drop+recreate. * One-way + onDelete change is also rejected: utopia's updateRelationship partner-cascade throws on one-way. */ private function updateRelationshipInPlace( @@ -1542,7 +1530,7 @@ private function tableSpecMatches(UtopiaDocument $existing, Table $resource): bo return $sourcePerms === $destPerms; } - /** Full-spec equality: short-circuits both DropAndRecreate and UpdateInPlace to Tolerate when nothing changed. */ + /** Full-spec equality: short-circuits UpdateInPlace to Tolerate when nothing changed. */ private function attributeSpecMatches(UtopiaDocument $existing, Column|Attribute $resource, string $type, bool $isRelationship): bool { if ($existing->getAttribute('type') !== $type) { diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 744b2e15..69b4cdf8 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -6,7 +6,6 @@ enum SchemaAction { case Create; case Tolerate; - case DropAndRecreate; case UpdateInPlace; } @@ -25,16 +24,14 @@ public static function values(): array } /** - * $canDrop = false (containers like databases/tables) forces UpdateInPlace - * even on createdAt-different — dropping would orphan children. + * Coarse routing for re-migration. The caller follows up with a spec-match + * check that overrides UpdateInPlace to Tolerate when source and dest + * already have identical spec — see DestinationAppwrite for the full flow. */ public function resolveSchemaAction( bool $exists, - ?string $sourceCreatedAt = null, - ?string $destCreatedAt = null, ?string $sourceUpdatedAt = null, ?string $destUpdatedAt = null, - bool $canDrop = false, ): SchemaAction { if (!$exists) { return SchemaAction::Create; @@ -42,41 +39,12 @@ public function resolveSchemaAction( return match ($this) { self::Fail => SchemaAction::Create, self::Skip => SchemaAction::Tolerate, - self::Upsert => $this->resolveUpsertAction( - $sourceCreatedAt, - $destCreatedAt, - $sourceUpdatedAt, - $destUpdatedAt, - $canDrop, - ), + self::Upsert => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) + ? SchemaAction::UpdateInPlace + : SchemaAction::Tolerate, }; } - private function resolveUpsertAction( - ?string $sourceCreatedAt, - ?string $destCreatedAt, - ?string $sourceUpdatedAt, - ?string $destUpdatedAt, - bool $canDrop, - ): SchemaAction { - if ($this->createdAtDiffers($sourceCreatedAt, $destCreatedAt)) { - return $canDrop ? SchemaAction::DropAndRecreate : SchemaAction::UpdateInPlace; - } - - if ($this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt)) { - return SchemaAction::UpdateInPlace; - } - - return SchemaAction::Tolerate; - } - - private function createdAtDiffers(?string $source, ?string $dest): bool - { - $src = $this->parseTimestamp($source); - $dst = $this->parseTimestamp($dest); - return $src !== null && $dst !== null && $src !== $dst; - } - private function sourceIsNewer(?string $source, ?string $dest): bool { $src = $this->parseTimestamp($source); diff --git a/tests/Migration/Unit/General/OnDuplicateTest.php b/tests/Migration/Unit/General/OnDuplicateTest.php index c8b10e1a..cc326258 100644 --- a/tests/Migration/Unit/General/OnDuplicateTest.php +++ b/tests/Migration/Unit/General/OnDuplicateTest.php @@ -9,19 +9,10 @@ /** * OnDuplicate::resolveSchemaAction is the load-bearing decision point for - * re-migration tolerance. These tests lock the mode × existence × createdAt × - * updatedAt × canDrop matrix so a future refactor can't silently shift the - * mapping. - * - * Decision rules under Upsert (with $exists = true): - * - createdAt differs → resource was deleted+recreated on source. Caller can - * drop and recreate (canDrop=true) or update in place (canDrop=false; for - * containers like databases/tables which would lose children on drop). - * - createdAt same + updatedAt newer → metadata-only edit. UpdateInPlace. - * The caller checks whether changed fields are SDK-updatable; if not it - * falls through to DropAndRecreate. - * - createdAt same + updatedAt equal/older → Tolerate. - * - Unparseable createdAt or updatedAt → conservative; treat as if no diff. + * re-migration tolerance. The destination then runs a spec-match guard that + * overrides UpdateInPlace to Tolerate when source and dest already have + * identical spec — see DestinationAppwrite. These tests lock the + * mode × existence × updatedAt matrix. */ class OnDuplicateTest extends TestCase { @@ -49,15 +40,11 @@ public function testFailExistsReturnsCreateSoCallerDDLThrows(): void public function testSkipAlwaysToleratesExisting(): void { - // Skip must ignore createdAt/updatedAt entirely — it's the "don't - // touch" contract. Exercise both source-newer and dest-newer to prove - // comparisons aren't consulted. + // Skip must ignore updatedAt entirely — it's the "don't touch" contract. $this->assertSame( SchemaAction::Tolerate, OnDuplicate::Skip->resolveSchemaAction( exists: true, - sourceCreatedAt: '2020-01-01T00:00:00.000+00:00', - destCreatedAt: '2026-01-01T00:00:00.000+00:00', sourceUpdatedAt: '2026-01-01T00:00:00.000+00:00', destUpdatedAt: '2020-01-01T00:00:00.000+00:00', ), @@ -72,73 +59,36 @@ public function testSkipAlwaysToleratesExisting(): void ); } - public function testUpsertCreatedAtDiffersOnLeafDropsAndRecreates(): void + public function testUpsertSourceNewerUpdatesInPlace(): void { - // canDrop: true + createdAt differs → leaf was deleted/recreated on - // source. Drop the destination's incarnation and create fresh. - $this->assertSame( - SchemaAction::DropAndRecreate, - OnDuplicate::Upsert->resolveSchemaAction( - exists: true, - sourceCreatedAt: '2026-04-23T10:00:00.000+00:00', - destCreatedAt: '2026-04-20T10:00:00.000+00:00', - canDrop: true, - ), - ); - } - - public function testUpsertCreatedAtDiffersOnContainerUpdatesInPlace(): void - { - // Default canDrop: false (containers like databases/tables). Even on - // createdAt diff, drop is forbidden — children would be lost. Fall - // back to UpdateInPlace. + // Source updatedAt strictly newer than dest → UpdateInPlace. The + // caller (DestinationAppwrite) follows up with attribute/index spec + // checks and falls through to drop+recreate when the SDK can't + // express the change. $this->assertSame( SchemaAction::UpdateInPlace, OnDuplicate::Upsert->resolveSchemaAction( exists: true, - sourceCreatedAt: '2026-04-23T10:00:00.000+00:00', - destCreatedAt: '2026-04-20T10:00:00.000+00:00', - ), - ); - } - - public function testUpsertCreatedAtSameUpdatedAtNewerUpdatesInPlace(): void - { - // Same createdAt + newer updatedAt → metadata-only edit on the same - // physical resource. UpdateInPlace; caller checks whether the changed - // fields are SDK-updatable and falls back to DropAndRecreate if not. - $this->assertSame( - SchemaAction::UpdateInPlace, - OnDuplicate::Upsert->resolveSchemaAction( - exists: true, - sourceCreatedAt: '2026-04-20T10:00:00.000+00:00', - destCreatedAt: '2026-04-20T10:00:00.000+00:00', sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', destUpdatedAt: '2026-04-23T09:59:59.000+00:00', - canDrop: true, ), ); } - public function testUpsertCreatedAtSameUpdatedAtEqualTolerates(): void + public function testUpsertSourceEqualTolerates(): void { - // Same createdAt + same updatedAt → nothing changed. Skip the work. - $created = '2026-04-20T10:00:00.000+00:00'; - $updated = '2026-04-23T10:00:00.000+00:00'; + $when = '2026-04-23T10:00:00.000+00:00'; $this->assertSame( SchemaAction::Tolerate, OnDuplicate::Upsert->resolveSchemaAction( exists: true, - sourceCreatedAt: $created, - destCreatedAt: $created, - sourceUpdatedAt: $updated, - destUpdatedAt: $updated, - canDrop: true, + sourceUpdatedAt: $when, + destUpdatedAt: $when, ), ); } - public function testUpsertCreatedAtSameUpdatedAtOlderTolerates(): void + public function testUpsertSourceOlderTolerates(): void { // Dest is ahead — don't roll back. Avoids overwriting newer // destination edits with stale source data. @@ -146,11 +96,8 @@ public function testUpsertCreatedAtSameUpdatedAtOlderTolerates(): void SchemaAction::Tolerate, OnDuplicate::Upsert->resolveSchemaAction( exists: true, - sourceCreatedAt: '2026-04-20T10:00:00.000+00:00', - destCreatedAt: '2026-04-20T10:00:00.000+00:00', sourceUpdatedAt: '2026-04-23T09:00:00.000+00:00', destUpdatedAt: '2026-04-23T10:00:00.000+00:00', - canDrop: true, ), ); } @@ -158,29 +105,10 @@ public function testUpsertCreatedAtSameUpdatedAtOlderTolerates(): void public function testUpsertNoTimestampsTolerates(): void { // No timestamps provided at all → no information to act on. Conservative: - // Tolerate rather than risk destructive action on uncertain input. + // Tolerate rather than risk a destructive update on uncertain input. $this->assertSame( SchemaAction::Tolerate, - OnDuplicate::Upsert->resolveSchemaAction( - exists: true, - canDrop: true, - ), - ); - } - - public function testUpsertOnlyUpdatedAtNewerWithoutCreatedAtUpdatesInPlace(): void - { - // CreatedAt missing on either side (e.g. older Appwrite versions, CSV - // sources): can't detect "recreated", so the only signal is updatedAt. - // Newer source → UpdateInPlace path (caller validates SDK-updatability). - $this->assertSame( - SchemaAction::UpdateInPlace, - OnDuplicate::Upsert->resolveSchemaAction( - exists: true, - sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', - destUpdatedAt: '2026-04-23T09:59:59.000+00:00', - canDrop: true, - ), + OnDuplicate::Upsert->resolveSchemaAction(exists: true), ); } @@ -203,23 +131,6 @@ public static function unparseableTimestampPairs(): array ]; } - #[DataProvider('unparseableTimestampPairs')] - public function testUpsertUnparseableCreatedAtDoesNotTriggerDrop(?string $source, ?string $dest): void - { - // Unparseable createdAt → don't trigger DropAndRecreate. With - // updatedAt also absent, falls through to Tolerate. Safety net for - // sources that emit malformed timestamps. - $this->assertSame( - SchemaAction::Tolerate, - OnDuplicate::Upsert->resolveSchemaAction( - exists: true, - sourceCreatedAt: $source, - destCreatedAt: $dest, - canDrop: true, - ), - ); - } - #[DataProvider('unparseableTimestampPairs')] public function testUpsertUnparseableUpdatedAtTolerates(?string $source, ?string $dest): void { From aa1e7c706326542a050354635f4290df43f06443 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 07:03:39 +0100 Subject: [PATCH 29/36] Fix two greptile-flagged P1 bugs in createField MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Stale $table after dropAttributeForRecreate caused false LimitException $table is loaded once at the top of createField. After dropAttributeForRecreate runs (UpdateInPlace fallthrough), the meta doc + physical column are gone from the DB but the in-memory $table object still has the dropped entry in its 'attributes' array. utopia's checkAttribute clones $table and appends the new attribute, then asks the adapter for getCountOfAttributes — at the adapter ceiling this throws LimitException even though the net change is zero. Fix: re-fetch $table after dropAttributeForRecreate. 2. Partner meta-doc deleted from wrong table when relatedCollection changes dropAttributeForRecreate looked up the partner meta via $resource->getOptions() — source's options. But it runs precisely when RELATIONSHIP_STRUCTURAL_FIELDS differ (relatedCollection, twoWayKey, etc), so source's options point to the NEW partner table while dest's existing partner meta lives on the OLD partner table. The OLD partner meta was left orphaned forever. Fix: pass $existingAttr through to dropAttributeForRecreate and use its 'options' (dest's current state) for partner resolution. --- src/Migration/Destinations/Appwrite.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index d215452d..eae78d99 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -837,7 +837,14 @@ protected function createField(Column|Attribute $resource): bool } if ($action === SchemaAction::UpdateInPlace) { - $this->dropAttributeForRecreate($database, $table, $resource, $dbForDatabases); + $this->dropAttributeForRecreate($database, $table, $resource, $dbForDatabases, $existingAttr); + // Re-fetch $table: the in-memory copy still holds the dropped + // attribute, so checkAttribute below would over-count and trip + // the adapter's LimitException at the column ceiling. + $table = $this->dbForProject->getDocument( + $this->databaseCollectionId($database), + $table->getId(), + ); } } @@ -1332,6 +1339,7 @@ private function dropAttributeForRecreate( UtopiaDocument $table, Column|Attribute $resource, UtopiaDatabase $dbForDatabases, + ?UtopiaDocument $existingAttr = null, ): void { $collectionId = $this->tableCollectionId($database, $table); $attributeMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); @@ -1346,8 +1354,13 @@ private function dropAttributeForRecreate( $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $attributeMetaId); // deleteRelationship only touches utopia's _metadata; clear the Appwrite-level partner meta doc too. + // Use dest's options ($existingAttr): we drop precisely when relatedCollection/twoWayKey differ, + // so source's options point to the NEW partner; the OLD partner meta lives where dest currently has it. if ($isRelationship) { - $partner = $this->resolveTwoWayPartner($database, $resource->getOptions()); + $options = $existingAttr !== null && !$existingAttr->isEmpty() + ? (array) $existingAttr->getAttribute('options', []) + : $resource->getOptions(); + $partner = $this->resolveTwoWayPartner($database, $options); if ($partner !== null) { $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner['partnerMetaId'])); } From 4c3965bc89557109d80ee1443f0efa59c07c8465 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 07:11:04 +0100 Subject: [PATCH 30/36] Tighten dropAttributeForRecreate: required $existingAttr, trimmed comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit $existingAttr is always non-empty at the only call site (UpdateInPlace fallthrough requires $exists=true). Drops the unreachable nullable fallback path and the defensive isEmpty() check. Comments collapsed to one line each — the WHY is small enough to fit. --- src/Migration/Destinations/Appwrite.php | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index eae78d99..ee70a8c4 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -838,13 +838,8 @@ protected function createField(Column|Attribute $resource): bool if ($action === SchemaAction::UpdateInPlace) { $this->dropAttributeForRecreate($database, $table, $resource, $dbForDatabases, $existingAttr); - // Re-fetch $table: the in-memory copy still holds the dropped - // attribute, so checkAttribute below would over-count and trip - // the adapter's LimitException at the column ceiling. - $table = $this->dbForProject->getDocument( - $this->databaseCollectionId($database), - $table->getId(), - ); + // Reload $table — in-memory copy still holds the dropped attribute, so checkAttribute would over-count. + $table = $this->dbForProject->getDocument($this->databaseCollectionId($database), $table->getId()); } } @@ -1330,16 +1325,13 @@ protected function createRecord(Row $resource, bool $isLast): bool return true; } - /** - * Drop attribute (meta doc + physical column). Relationships route through - * deleteRelationship since deleteAttribute throws for VAR_RELATIONSHIP. - */ + /** Relationships route through deleteRelationship since deleteAttribute throws for VAR_RELATIONSHIP. */ private function dropAttributeForRecreate( UtopiaDocument $database, UtopiaDocument $table, Column|Attribute $resource, UtopiaDatabase $dbForDatabases, - ?UtopiaDocument $existingAttr = null, + UtopiaDocument $existingAttr, ): void { $collectionId = $this->tableCollectionId($database, $table); $attributeMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); @@ -1353,14 +1345,9 @@ private function dropAttributeForRecreate( $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $attributeMetaId); - // deleteRelationship only touches utopia's _metadata; clear the Appwrite-level partner meta doc too. - // Use dest's options ($existingAttr): we drop precisely when relatedCollection/twoWayKey differ, - // so source's options point to the NEW partner; the OLD partner meta lives where dest currently has it. + // Use dest's options for partner lookup — drop fires when relatedCollection/twoWayKey differ, so source points to the NEW partner. if ($isRelationship) { - $options = $existingAttr !== null && !$existingAttr->isEmpty() - ? (array) $existingAttr->getAttribute('options', []) - : $resource->getOptions(); - $partner = $this->resolveTwoWayPartner($database, $options); + $partner = $this->resolveTwoWayPartner($database, (array) $existingAttr->getAttribute('options', [])); if ($partner !== null) { $this->bestEffort(fn () => $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $partner['partnerMetaId'])); } From d37efed0d977a5f0c164861e58fc807763b1115f Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 07:55:53 +0100 Subject: [PATCH 31/36] Track partner key for two-way orphan cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile #2: source-side filter (Sources/Appwrite::exportColumns) skips RELATION_SIDE_CHILD, so the partner meta-doc created inline inside the parent's createField never reaches the partner table's createField path — and therefore never gets registered with trackOrphanCandidate. If any other column on the partner table is migrated, orphan cleanup sees the partner key absent from the tracked set and deletes it, breaking the relationship's child side. Adds a trackOrphanCandidate call right after the inline createDocument so the partner key is registered against $relatedTable. Phpstan note: adding the strict-typed call inside the existing relationship block shifted phpstan's narrowing analysis and made it flag two pre-existing $relatedTable usages (createRelationship at line 957 and purgeCachedDocument at line 1004). Both are reachable only when type === VAR_RELATIONSHIP, where $relatedTable is guaranteed defined. Suppressed with @phpstan-ignore-next-line on those two specific lines. --- src/Migration/Destinations/Appwrite.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index ee70a8c4..c710570e 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -923,6 +923,7 @@ protected function createField(Column|Attribute $resource): bool ]); $this->dbForProject->createDocument(self::META_ATTRIBUTES, $twoWayAttribute); + $this->trackOrphanCandidate($database, $relatedTable, 'attributeKeys', $twoWayKey, $dbForDatabases); } catch (DuplicateException) { $this->dbForProject->deleteDocument(self::META_ATTRIBUTES, $column->getId()); @@ -952,6 +953,7 @@ protected function createField(Column|Attribute $resource): bool case UtopiaDatabase::VAR_RELATIONSHIP: if (!$dbForDatabases->createRelationship( collection: $this->tableCollectionId($database, $table), + // @phpstan-ignore-next-line — $relatedTable is set when type is VAR_RELATIONSHIP. relatedCollection: $this->tableCollectionId($database, $relatedTable), type: $options['relationType'], twoWay: $options['twoWay'], @@ -1000,6 +1002,7 @@ protected function createField(Column|Attribute $resource): bool } if ($type === UtopiaDatabase::VAR_RELATIONSHIP && $options['twoWay']) { + // @phpstan-ignore-next-line — $relatedTable is set when type is VAR_RELATIONSHIP. $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $relatedTable->getId()); } From 7d71505ec0a5731f322bbdc83cc9bee32053df2e Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Tue, 28 Apr 2026 08:10:11 +0100 Subject: [PATCH 32/36] createIndex: run pre-check + drop before count and IndexValidator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile: $count (META_INDEXES query) and IndexValidator both ran BEFORE the pre-check / drop block. On an Upsert recreate path: - The to-be-dropped index was included in the count, so a table at getLimitForIndexes() falsely threw 'Index limit reached' even though the net change is zero (one drop, one add). - IndexValidator was initialized with $tableIndexes from the in-memory $table — including the existing index — so the new index spec could be flagged as a conflict against itself ('Invalid index: …') before the drop ran. Reordered createIndex so: 1. Pre-check + drop block runs first (Tolerate returns early). 2. $table is reloaded after a drop so embedded $tableIndexes reflects the post-drop state. 3. count, validateFieldsForIndexes, build $index, IndexValidator all run AFTER the drop, against the fresh $table. Mirrors the attribute path's structure where checkAttribute already runs after dropAttributeForRecreate. --- src/Migration/Destinations/Appwrite.php | 85 +++++++++++++------------ 1 file changed, 45 insertions(+), 40 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index c710570e..a9254e57 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -1048,6 +1048,50 @@ protected function createIndex(Index $resource): bool } $dbForDatabases = ($this->getDatabasesDB)($database); + $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); + $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); + + $this->trackOrphanCandidate($database, $table, 'indexKeys', $resource->getKey(), $dbForDatabases); + + $indexMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); + + // Pre-check + drop runs BEFORE count/validator so the to-be-dropped index isn't included in + // the limit count or in IndexValidator's $tableIndexes (otherwise an Upsert recreate at the + // ceiling throws "Index limit reached" or "Invalid index" even though the net change is zero). + if ($this->onDuplicate !== OnDuplicate::Fail) { + $existingIdx = $this->dbForProject->getDocument(self::META_INDEXES, $indexMetaId); + $action = $this->onDuplicate->resolveSchemaAction( + !$existingIdx->isEmpty(), + $updatedAt, + $existingIdx->getUpdatedAt(), + ); + // Spec match → skip work. Create excluded; nothing on dest to match against. + if ($action !== SchemaAction::Create && $this->indexSpecMatches($existingIdx, $resource)) { + $action = SchemaAction::Tolerate; + } + + // Indexes have no in-place primitive — any action other than Tolerate falls through to drop+recreate. + $earlyReturn = match ($action) { + SchemaAction::Tolerate => (function () use ($resource, $database, $table): bool { + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); + return false; + })(), + SchemaAction::UpdateInPlace, SchemaAction::Create => null, + }; + if ($earlyReturn !== null) { + return $earlyReturn; + } + + if ($action === SchemaAction::UpdateInPlace) { + $dbForDatabases->deleteIndex($this->tableCollectionId($database, $table), $resource->getKey()); + $this->dbForProject->deleteDocument(self::META_INDEXES, $indexMetaId); + $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); + // Reload $table — in-memory copy still holds the dropped index, so IndexValidator below would over-count. + $table = $this->dbForProject->getDocument($this->databaseCollectionId($database), $table->getId()); + } + } + $count = $this->dbForProject->count(self::META_INDEXES, [ Query::equal('collectionInternalId', [$table->getSequence()]), Query::equal('databaseInternalId', [$database->getSequence()]) @@ -1069,13 +1113,8 @@ protected function createIndex(Index $resource): bool $this->validateFieldsForIndexes($resource, $table, $lengths); } - $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); - $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); - - $this->trackOrphanCandidate($database, $table, 'indexKeys', $resource->getKey(), $dbForDatabases); - $index = new UtopiaDocument([ - '$id' => ID::custom($this->attributeIndexMetaId($database, $table, $resource->getKey())), + '$id' => ID::custom($indexMetaId), 'key' => $resource->getKey(), 'status' => 'available', // processing, available, failed, deleting, stuck 'databaseInternalId' => $database->getSequence(), @@ -1116,7 +1155,6 @@ protected function createIndex(Index $resource): bool $dbForDatabases->getAdapter()->getSupportForFulltextIndex() ); - if (!$validator->isValid($index)) { throw new Exception( resourceName: $resource->getName(), @@ -1126,39 +1164,6 @@ protected function createIndex(Index $resource): bool ); } - $indexMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); - if ($this->onDuplicate !== OnDuplicate::Fail) { - $existingIdx = $this->dbForProject->getDocument(self::META_INDEXES, $indexMetaId); - $action = $this->onDuplicate->resolveSchemaAction( - !$existingIdx->isEmpty(), - $updatedAt, - $existingIdx->getUpdatedAt(), - ); - // Spec match → skip work. Create excluded; nothing on dest to match against. - if ($action !== SchemaAction::Create && $this->indexSpecMatches($existingIdx, $resource)) { - $action = SchemaAction::Tolerate; - } - - // Indexes have no in-place primitive — any action other than Tolerate falls through to drop+recreate. - $earlyReturn = match ($action) { - SchemaAction::Tolerate => (function () use ($resource, $database, $table): bool { - $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); - $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); - return false; - })(), - SchemaAction::UpdateInPlace, SchemaAction::Create => null, - }; - if ($earlyReturn !== null) { - return $earlyReturn; - } - - if ($action === SchemaAction::UpdateInPlace) { - $dbForDatabases->deleteIndex($this->tableCollectionId($database, $table), $resource->getKey()); - $this->dbForProject->deleteDocument(self::META_INDEXES, $indexMetaId); - $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); - } - } - $index = $this->dbForProject->createDocument(self::META_INDEXES, $index); try { From 5520cb2585ce2498699937e6ef9e0706cfcd1f00 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 30 Apr 2026 11:21:31 +0100 Subject: [PATCH 33/36] Rename SDK-boundary constants to IMMUTABLE_FIELDS; fix META doc Maintainer review feedback: - ATTRIBUTE_NON_SDK_FIELDS -> ATTRIBUTE_IMMUTABLE_FIELDS - RELATIONSHIP_STRUCTURAL_FIELDS -> RELATIONSHIP_IMMUTABLE_FIELDS - META_* docblock 'platform-DB' -> 'project-DB' (these collections live in dbForProject, not the platform-level DB) Naming shift: "NON_SDK"/"STRUCTURAL" describes the cause (SDK doesn't expose them), "IMMUTABLE" describes the effect (can't change in place, only via drop+recreate). The latter is the audience-facing semantic. SDK-boundary lock-in test (AppwriteAttributeSdkBoundaryTest) still reflects on the renamed constant; updated its references. --- src/Migration/Destinations/Appwrite.php | 18 +++++++++--------- .../AppwriteAttributeSdkBoundaryTest.php | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index a9254e57..ca0b9cd3 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -63,13 +63,13 @@ class Appwrite extends Destination { - /** Names of the platform-DB collections holding Appwrite schema metadata. */ + /** Names of the project-DB collections holding Appwrite schema metadata. */ private const META_DATABASES = 'databases'; private const META_ATTRIBUTES = 'attributes'; private const META_INDEXES = 'indexes'; - /** Fields the SDK's per-type updateXAttribute endpoints don't expose; a change here forces drop+recreate. */ - private const ATTRIBUTE_NON_SDK_FIELDS = [ + /** Attribute fields the SDK can't update in place (no per-type updateX endpoint exposes them); a change here forces drop+recreate. */ + private const ATTRIBUTE_IMMUTABLE_FIELDS = [ 'type', 'array', 'signed', @@ -78,8 +78,8 @@ class Appwrite extends Destination 'filters', ]; - /** Fields the SDK's updateRelationshipAttribute doesn't expose (only onDelete/newKey are SDK-reachable). */ - private const RELATIONSHIP_STRUCTURAL_FIELDS = [ + /** Relationship options fields the SDK can't update in place (only onDelete/newKey are SDK-reachable); a change here forces drop+recreate. */ + private const RELATIONSHIP_IMMUTABLE_FIELDS = [ 'relationType', 'twoWay', 'twoWayKey', @@ -1384,11 +1384,11 @@ private function updateAttributeInPlace( ]; $existingFields = []; - foreach (self::ATTRIBUTE_NON_SDK_FIELDS as $field) { + foreach (self::ATTRIBUTE_IMMUTABLE_FIELDS as $field) { $existingFields[$field] = $existingAttr->getAttribute($field); } - if ($this->arraysDifferOnKeys($sourceFields, $existingFields, self::ATTRIBUTE_NON_SDK_FIELDS)) { + if ($this->arraysDifferOnKeys($sourceFields, $existingFields, self::ATTRIBUTE_IMMUTABLE_FIELDS)) { return false; } @@ -1441,7 +1441,7 @@ private function updateRelationshipInPlace( $sourceOptions = $resource->getOptions(); $destOptions = $existingAttr->getAttribute('options', []); - if ($this->arraysDifferOnKeys($sourceOptions, $destOptions, self::RELATIONSHIP_STRUCTURAL_FIELDS)) { + if ($this->arraysDifferOnKeys($sourceOptions, $destOptions, self::RELATIONSHIP_IMMUTABLE_FIELDS)) { return false; } @@ -1548,7 +1548,7 @@ private function attributeSpecMatches(UtopiaDocument $existing, Column|Attribute $sourceOptions = $resource->getOptions(); /** @var array $destOptions */ $destOptions = $existing->getAttribute('options', []); - foreach (self::RELATIONSHIP_STRUCTURAL_FIELDS as $field) { + foreach (self::RELATIONSHIP_IMMUTABLE_FIELDS as $field) { if (!$this->valuesMatch($sourceOptions[$field] ?? null, $destOptions[$field] ?? null)) { return false; } diff --git a/tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php b/tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php index 3b2fdddc..a079ea61 100644 --- a/tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php +++ b/tests/Migration/Unit/Destinations/AppwriteAttributeSdkBoundaryTest.php @@ -10,12 +10,12 @@ use Utopia\Migration\Destinations\Appwrite as AppwriteDestination; /** - * Lock-in for the SDK boundary that drives ATTRIBUTE_NON_SDK_FIELDS. + * Lock-in for the SDK boundary that drives ATTRIBUTE_IMMUTABLE_FIELDS. * * Reflects on appwrite/appwrite's TablesDB + Databases services. If the SDK * ships a new updateXColumn / updateXAttribute endpoint that exposes a * previously-non-SDK field, this test fails so DestinationAppwrite's - * ATTRIBUTE_NON_SDK_FIELDS constant can be synced. Catches drift in CI + * ATTRIBUTE_IMMUTABLE_FIELDS constant can be synced. Catches drift in CI * rather than at runtime. */ class AppwriteAttributeSdkBoundaryTest extends TestCase @@ -47,9 +47,9 @@ public function testSdkExposedFieldsAreNotInNonSdkConstant(): void $this->assertSame( [], \array_values($overlap), - 'ATTRIBUTE_NON_SDK_FIELDS marks these fields as non-SDK, but the appwrite/appwrite ' + 'ATTRIBUTE_IMMUTABLE_FIELDS marks these fields as non-SDK, but the appwrite/appwrite ' . 'SDK exposes them via updateXColumn/updateXAttribute: ' . \implode(', ', $overlap) - . '. Either drop these from ATTRIBUTE_NON_SDK_FIELDS or update PARAM_TO_META_FIELD ' + . '. Either drop these from ATTRIBUTE_IMMUTABLE_FIELDS or update PARAM_TO_META_FIELD ' . 'in this test if the SDK param doesn\'t correspond to a top-level meta-doc field.', ); } @@ -90,7 +90,7 @@ private function collectSdkExposedMetaFields(): array private function readNonSdkConstant(): array { /** @var list $value */ - $value = (new ReflectionClass(AppwriteDestination::class))->getConstant('ATTRIBUTE_NON_SDK_FIELDS'); + $value = (new ReflectionClass(AppwriteDestination::class))->getConstant('ATTRIBUTE_IMMUTABLE_FIELDS'); return $value; } } From b8ae7bc953d1897994eaa0de7e677687a19e5133 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 30 Apr 2026 11:24:12 +0100 Subject: [PATCH 34/36] Rename OnDuplicate::Upsert -> OnDuplicate::Overwrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer review: align with Appwrite's terminology (skip / overwrite / fail). Public-API breaking change — value 'upsert' becomes 'overwrite', so any pre-1.9.3 consumer of OnDuplicate::Upsert or the 'upsert' string needs to update. Renamed in this repo: - enum case + value - all OnDuplicate::Upsert references in DestinationAppwrite - internal helpers cleanupUpsertOrphans(ForTable) -> cleanupOverwriteOrphans(ForTable) - comment / docblock mentions of 'Upsert-mode' / 'Upsert recreate' - unit test method names testUpsert* -> testOverwrite* Cross-repo follow-up needed in appwrite/appwrite: - app/controllers/api/migrations.php (3 endpoint param descriptions) - src/Appwrite/Platform/Workers/Migrations.php (OnDuplicate reference) - e2e tests passing 'onDuplicate' => 'upsert' - console UI hardcoded option strings (separate repo) --- src/Migration/Destinations/Appwrite.php | 20 ++++++++--------- src/Migration/Destinations/OnDuplicate.php | 4 ++-- .../Unit/General/OnDuplicateTest.php | 22 +++++++++---------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index ca0b9cd3..fb25bb55 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -109,7 +109,7 @@ class Appwrite extends Destination private array $rowBuffer = []; /** - * Upsert-mode orphan tracking, keyed by (database, table). Orphans are + * Overwrite-mode orphan tracking, keyed by (database, table). Orphans are * destination keys not in `attributeKeys` / `indexKeys`. Entries removed * after their cleanup runs so the end-of-run sweep only visits tables * that had no rows. @@ -178,7 +178,7 @@ public function run( ): void { $this->resetRunState(); parent::run($resources, $callback, $rootResourceId, $rootResourceType); - $this->cleanupUpsertOrphans(); + $this->cleanupOverwriteOrphans(); } /** Per-run state must not leak across run() invocations on a reused instance. */ @@ -1056,7 +1056,7 @@ protected function createIndex(Index $resource): bool $indexMetaId = $this->attributeIndexMetaId($database, $table, $resource->getKey()); // Pre-check + drop runs BEFORE count/validator so the to-be-dropped index isn't included in - // the limit count or in IndexValidator's $tableIndexes (otherwise an Upsert recreate at the + // the limit count or in IndexValidator's $tableIndexes (otherwise an Overwrite recreate at the // ceiling throws "Index limit reached" or "Invalid index" even though the net change is zero). if ($this->onDuplicate !== OnDuplicate::Fail) { $existingIdx = $this->dbForProject->getDocument(self::META_INDEXES, $indexMetaId); @@ -1284,7 +1284,7 @@ protected function createRecord(Row $resource, bool $isLast): bool $dbForDatabases = ($this->getDatabasesDB)($database); // Drop schema orphans before rows land so the Structure validator doesn't reject on orphan required columns. - $this->cleanupUpsertOrphansForTable($this->tableIdentity($database, $table)); + $this->cleanupOverwriteOrphansForTable($this->tableIdentity($database, $table)); // Strip row payload fields the table doesn't declare — guards against orphans surviving in source archives. if ($dbForDatabases->getAdapter()->getSupportForAttributes()) { foreach ($this->rowBuffer as $row) { @@ -1311,7 +1311,7 @@ protected function createRecord(Row $resource, bool $isLast): bool $collectionId = $this->tableCollectionId($database, $table); match ($this->onDuplicate) { - OnDuplicate::Upsert => $dbForDatabases->skipRelationshipsExistCheck( + OnDuplicate::Overwrite => $dbForDatabases->skipRelationshipsExistCheck( fn () => $dbForDatabases->upsertDocuments($collectionId, $this->rowBuffer) ), OnDuplicate::Skip => $dbForDatabases->skipDuplicates( @@ -1699,7 +1699,7 @@ private function trackOrphanCandidate( string $key, UtopiaDatabase $dbForDatabases, ): void { - if ($this->onDuplicate !== OnDuplicate::Upsert) { + if ($this->onDuplicate !== OnDuplicate::Overwrite) { return; } $tableId = $this->tableIdentity($database, $table); @@ -1716,17 +1716,17 @@ private function trackOrphanCandidate( } /** End-of-migration sweep — only visits tables that had no rows (rest were cleaned per-table in createRecord). */ - private function cleanupUpsertOrphans(): void + private function cleanupOverwriteOrphans(): void { foreach (\array_keys($this->orphansByTable) as $tableId) { - $this->cleanupUpsertOrphansForTable($tableId); + $this->cleanupOverwriteOrphansForTable($tableId); } } /** Called per-table from createRecord before rows land so Structure validator sees the post-cleanup schema. */ - private function cleanupUpsertOrphansForTable(string $tableId): void + private function cleanupOverwriteOrphansForTable(string $tableId): void { - if ($this->onDuplicate !== OnDuplicate::Upsert) { + if ($this->onDuplicate !== OnDuplicate::Overwrite) { return; } if (!isset($this->orphansByTable[$tableId])) { diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 69b4cdf8..3ba945c0 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -13,7 +13,7 @@ enum OnDuplicate: string { case Fail = 'fail'; case Skip = 'skip'; - case Upsert = 'upsert'; + case Overwrite = 'overwrite'; /** * @return list @@ -39,7 +39,7 @@ public function resolveSchemaAction( return match ($this) { self::Fail => SchemaAction::Create, self::Skip => SchemaAction::Tolerate, - self::Upsert => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) + self::Overwrite => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) ? SchemaAction::UpdateInPlace : SchemaAction::Tolerate, }; diff --git a/tests/Migration/Unit/General/OnDuplicateTest.php b/tests/Migration/Unit/General/OnDuplicateTest.php index cc326258..0141cb5d 100644 --- a/tests/Migration/Unit/General/OnDuplicateTest.php +++ b/tests/Migration/Unit/General/OnDuplicateTest.php @@ -59,7 +59,7 @@ public function testSkipAlwaysToleratesExisting(): void ); } - public function testUpsertSourceNewerUpdatesInPlace(): void + public function testOverwriteSourceNewerUpdatesInPlace(): void { // Source updatedAt strictly newer than dest → UpdateInPlace. The // caller (DestinationAppwrite) follows up with attribute/index spec @@ -67,7 +67,7 @@ public function testUpsertSourceNewerUpdatesInPlace(): void // express the change. $this->assertSame( SchemaAction::UpdateInPlace, - OnDuplicate::Upsert->resolveSchemaAction( + OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', destUpdatedAt: '2026-04-23T09:59:59.000+00:00', @@ -75,12 +75,12 @@ public function testUpsertSourceNewerUpdatesInPlace(): void ); } - public function testUpsertSourceEqualTolerates(): void + public function testOverwriteSourceEqualTolerates(): void { $when = '2026-04-23T10:00:00.000+00:00'; $this->assertSame( SchemaAction::Tolerate, - OnDuplicate::Upsert->resolveSchemaAction( + OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: $when, destUpdatedAt: $when, @@ -88,13 +88,13 @@ public function testUpsertSourceEqualTolerates(): void ); } - public function testUpsertSourceOlderTolerates(): void + public function testOverwriteSourceOlderTolerates(): void { // Dest is ahead — don't roll back. Avoids overwriting newer // destination edits with stale source data. $this->assertSame( SchemaAction::Tolerate, - OnDuplicate::Upsert->resolveSchemaAction( + OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: '2026-04-23T09:00:00.000+00:00', destUpdatedAt: '2026-04-23T10:00:00.000+00:00', @@ -102,13 +102,13 @@ public function testUpsertSourceOlderTolerates(): void ); } - public function testUpsertNoTimestampsTolerates(): void + public function testOverwriteNoTimestampsTolerates(): void { // No timestamps provided at all → no information to act on. Conservative: // Tolerate rather than risk a destructive update on uncertain input. $this->assertSame( SchemaAction::Tolerate, - OnDuplicate::Upsert->resolveSchemaAction(exists: true), + OnDuplicate::Overwrite->resolveSchemaAction(exists: true), ); } @@ -132,13 +132,13 @@ public static function unparseableTimestampPairs(): array } #[DataProvider('unparseableTimestampPairs')] - public function testUpsertUnparseableUpdatedAtTolerates(?string $source, ?string $dest): void + public function testOverwriteUnparseableUpdatedAtTolerates(?string $source, ?string $dest): void { // Conservative: unparseable updatedAt preserves existing destination // rather than risk a destructive update on garbage input. $this->assertSame( SchemaAction::Tolerate, - OnDuplicate::Upsert->resolveSchemaAction( + OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: $source, destUpdatedAt: $dest, @@ -150,6 +150,6 @@ public function testValuesListsAllCasesInDeclarationOrder(): void { // The values() helper is consumed by API/SDK param validators; this // protects against an accidental case-rename or reorder. - $this->assertSame(['fail', 'skip', 'upsert'], OnDuplicate::values()); + $this->assertSame(['fail', 'skip', 'overwrite'], OnDuplicate::values()); } } From dfb0224ce18310a5bd7dd2676201909e6dec6525 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 30 Apr 2026 11:59:43 +0100 Subject: [PATCH 35/36] createRecord: reload $table after orphan cleanup before field-strip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile #3167268161: cleanupOverwriteOrphansForTable removes orphan attributes from dest's META_ATTRIBUTES + physical schema, but the in-memory $table object still holds them in its 'attributes' array. The field-strip loop right after iterates $table->getAttribute( 'attributes') to decide which row payload fields to keep — finds the orphan as 'valid' and leaves it in the row. Subsequent upsertDocuments hits the post-cleanup schema, structure validator rejects the row. Fix: reload $table immediately after orphan cleanup. Mirrors the attribute-path and index-path stale-$table fixes. --- src/Migration/Destinations/Appwrite.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index c23de53d..5b8047c6 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -1288,6 +1288,11 @@ protected function createRecord(Row $resource, bool $isLast): bool // Drop schema orphans before rows land so the Structure validator doesn't reject on orphan required columns. $this->cleanupOverwriteOrphansForTable($this->tableIdentity($database, $table)); + // Reload $table — in-memory copy still holds the dropped attributes, so the strip loop below would over-keep. + $table = $this->dbForProject->getDocument( + $this->databaseCollectionId($database), + $resource->getTable()->getId(), + ); // Strip row payload fields the table doesn't declare — guards against orphans surviving in source archives. if ($dbForDatabases->getAdapter()->getSupportForAttributes()) { foreach ($this->rowBuffer as $row) { From d7c3d0536dbb9e37a4894cd0b3b0134a483e04e8 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Thu, 30 Apr 2026 12:08:08 +0100 Subject: [PATCH 36/36] Rename SchemaAction cases to match Appwrite terms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Jake's review on PR #171 (line 9 of OnDuplicate.php — SchemaAction enum): align the SchemaAction case names with Appwrite's user-facing terms (skip / overwrite / fail). - SchemaAction::Tolerate -> SchemaAction::Skip - SchemaAction::UpdateInPlace -> SchemaAction::Overwrite - SchemaAction::Create kept (no Appwrite equivalent; means 'no existing dest doc, run the create flow') Now the inner machinery uses the same vocabulary as the user-facing OnDuplicate enum (Fail / Skip / Overwrite). Comments and test method names updated to match (Tolerate -> Skip, UpdateInPlace -> Overwrite where they refer to the action — the test name 'UpdatesInPlace' as a verb phrase stays). --- src/Migration/Destinations/Appwrite.php | 32 ++++++++--------- src/Migration/Destinations/OnDuplicate.php | 12 +++---- .../Unit/General/OnDuplicateTest.php | 34 +++++++++---------- 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/Migration/Destinations/Appwrite.php b/src/Migration/Destinations/Appwrite.php index 5b8047c6..41023b1a 100644 --- a/src/Migration/Destinations/Appwrite.php +++ b/src/Migration/Destinations/Appwrite.php @@ -501,16 +501,16 @@ protected function createDatabase(Database $resource): bool ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->databaseSpecMatches($existing, $resource)) { - $action = SchemaAction::Tolerate; + $action = SchemaAction::Skip; } $earlyReturn = match ($action) { - SchemaAction::Tolerate => (function () use ($resource, $existing): bool { + SchemaAction::Skip => (function () use ($resource, $existing): bool { $resource->setSequence($existing->getSequence()); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; })(), - SchemaAction::UpdateInPlace => (function () use ($resource, $existing, $updatedAt): bool { + SchemaAction::Overwrite => (function () use ($resource, $existing, $updatedAt): bool { $this->dbForProject->updateDocument(self::META_DATABASES, $existing->getId(), new UtopiaDocument([ 'name' => $resource->getDatabaseName(), 'search' => implode(' ', [$resource->getId(), $resource->getDatabaseName()]), @@ -623,16 +623,16 @@ protected function createEntity(Table $resource): bool ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->tableSpecMatches($existing, $resource)) { - $action = SchemaAction::Tolerate; + $action = SchemaAction::Skip; } $earlyReturn = match ($action) { - SchemaAction::Tolerate => (function () use ($resource, $existing): bool { + SchemaAction::Skip => (function () use ($resource, $existing): bool { $resource->setSequence($existing->getSequence()); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; })(), - SchemaAction::UpdateInPlace => (function () use ($resource, $existing, $database, $updatedAt): bool { + SchemaAction::Overwrite => (function () use ($resource, $existing, $database, $updatedAt): bool { $this->dbForProject->updateDocument( $this->databaseCollectionId($database), $existing->getId(), @@ -816,16 +816,16 @@ protected function createField(Column|Attribute $resource): bool ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->attributeSpecMatches($existingAttr, $resource, $type, $isRelationship)) { - $action = SchemaAction::Tolerate; + $action = SchemaAction::Skip; } $earlyReturn = match ($action) { - SchemaAction::Tolerate => (function () use ($resource, $database, $table, $dbForDatabases): bool { + SchemaAction::Skip => (function () use ($resource, $database, $table, $dbForDatabases): bool { $this->purgeTableCaches($database, $table, $dbForDatabases); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; })(), - SchemaAction::UpdateInPlace => ($isRelationship + SchemaAction::Overwrite => ($isRelationship ? $this->updateRelationshipInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases) : $this->updateAttributeInPlace($database, $table, $resource, $type, $updatedAt, $existingAttr, $dbForDatabases)) ? true @@ -839,7 +839,7 @@ protected function createField(Column|Attribute $resource): bool return $earlyReturn; } - if ($action === SchemaAction::UpdateInPlace) { + if ($action === SchemaAction::Overwrite) { $this->dropAttributeForRecreate($database, $table, $resource, $dbForDatabases, $existingAttr); // Reload $table — in-memory copy still holds the dropped attribute, so checkAttribute would over-count. $table = $this->dbForProject->getDocument($this->databaseCollectionId($database), $table->getId()); @@ -1070,23 +1070,23 @@ protected function createIndex(Index $resource): bool ); // Spec match → skip work. Create excluded; nothing on dest to match against. if ($action !== SchemaAction::Create && $this->indexSpecMatches($existingIdx, $resource)) { - $action = SchemaAction::Tolerate; + $action = SchemaAction::Skip; } - // Indexes have no in-place primitive — any action other than Tolerate falls through to drop+recreate. + // Indexes have no in-place primitive — any action other than Skip falls through to drop+recreate. $earlyReturn = match ($action) { - SchemaAction::Tolerate => (function () use ($resource, $database, $table): bool { + SchemaAction::Skip => (function () use ($resource, $database, $table): bool { $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); $resource->setStatus(Resource::STATUS_SKIPPED, 'Already exists on destination'); return false; })(), - SchemaAction::UpdateInPlace, SchemaAction::Create => null, + SchemaAction::Overwrite, SchemaAction::Create => null, }; if ($earlyReturn !== null) { return $earlyReturn; } - if ($action === SchemaAction::UpdateInPlace) { + if ($action === SchemaAction::Overwrite) { $dbForDatabases->deleteIndex($this->tableCollectionId($database, $table), $resource->getKey()); $this->dbForProject->deleteDocument(self::META_INDEXES, $indexMetaId); $this->dbForProject->purgeCachedDocument($this->databaseCollectionId($database), $table->getId()); @@ -1545,7 +1545,7 @@ private function tableSpecMatches(UtopiaDocument $existing, Table $resource): bo return $sourcePerms === $destPerms; } - /** Full-spec equality: short-circuits UpdateInPlace to Tolerate when nothing changed. */ + /** Full-spec equality: short-circuits Overwrite to Skip when nothing changed. */ private function attributeSpecMatches(UtopiaDocument $existing, Column|Attribute $resource, string $type, bool $isRelationship): bool { if ($existing->getAttribute('type') !== $type) { diff --git a/src/Migration/Destinations/OnDuplicate.php b/src/Migration/Destinations/OnDuplicate.php index 3ba945c0..a88ebf80 100644 --- a/src/Migration/Destinations/OnDuplicate.php +++ b/src/Migration/Destinations/OnDuplicate.php @@ -5,8 +5,8 @@ enum SchemaAction { case Create; - case Tolerate; - case UpdateInPlace; + case Skip; + case Overwrite; } enum OnDuplicate: string @@ -25,7 +25,7 @@ public static function values(): array /** * Coarse routing for re-migration. The caller follows up with a spec-match - * check that overrides UpdateInPlace to Tolerate when source and dest + * check that overrides Overwrite to Skip when source and dest * already have identical spec — see DestinationAppwrite for the full flow. */ public function resolveSchemaAction( @@ -38,10 +38,10 @@ public function resolveSchemaAction( } return match ($this) { self::Fail => SchemaAction::Create, - self::Skip => SchemaAction::Tolerate, + self::Skip => SchemaAction::Skip, self::Overwrite => $this->sourceIsNewer($sourceUpdatedAt, $destUpdatedAt) - ? SchemaAction::UpdateInPlace - : SchemaAction::Tolerate, + ? SchemaAction::Overwrite + : SchemaAction::Skip, }; } diff --git a/tests/Migration/Unit/General/OnDuplicateTest.php b/tests/Migration/Unit/General/OnDuplicateTest.php index 0141cb5d..3b7e4b11 100644 --- a/tests/Migration/Unit/General/OnDuplicateTest.php +++ b/tests/Migration/Unit/General/OnDuplicateTest.php @@ -10,7 +10,7 @@ /** * OnDuplicate::resolveSchemaAction is the load-bearing decision point for * re-migration tolerance. The destination then runs a spec-match guard that - * overrides UpdateInPlace to Tolerate when source and dest already have + * overrides Overwrite to Skip when source and dest already have * identical spec — see DestinationAppwrite. These tests lock the * mode × existence × updatedAt matrix. */ @@ -29,20 +29,20 @@ public function testNotExistsAlwaysCreates(): void public function testFailExistsReturnsCreateSoCallerDDLThrows(): void { - // Fail is routed through Create (not Tolerate) so the caller's normal + // Fail is routed through Create (not Skip) so the caller's normal // create flow runs and the library surfaces DuplicateException as - // designed. Returning Tolerate here would silently hide the error. + // designed. Returning Skip here would silently hide the error. $this->assertSame( SchemaAction::Create, OnDuplicate::Fail->resolveSchemaAction(exists: true), ); } - public function testSkipAlwaysToleratesExisting(): void + public function testSkipAlwaysSkipsExisting(): void { // Skip must ignore updatedAt entirely — it's the "don't touch" contract. $this->assertSame( - SchemaAction::Tolerate, + SchemaAction::Skip, OnDuplicate::Skip->resolveSchemaAction( exists: true, sourceUpdatedAt: '2026-01-01T00:00:00.000+00:00', @@ -50,7 +50,7 @@ public function testSkipAlwaysToleratesExisting(): void ), ); $this->assertSame( - SchemaAction::Tolerate, + SchemaAction::Skip, OnDuplicate::Skip->resolveSchemaAction( exists: true, sourceUpdatedAt: '2020-01-01T00:00:00.000+00:00', @@ -61,12 +61,12 @@ public function testSkipAlwaysToleratesExisting(): void public function testOverwriteSourceNewerUpdatesInPlace(): void { - // Source updatedAt strictly newer than dest → UpdateInPlace. The + // Source updatedAt strictly newer than dest → Overwrite. The // caller (DestinationAppwrite) follows up with attribute/index spec // checks and falls through to drop+recreate when the SDK can't // express the change. $this->assertSame( - SchemaAction::UpdateInPlace, + SchemaAction::Overwrite, OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: '2026-04-23T10:00:00.000+00:00', @@ -75,11 +75,11 @@ public function testOverwriteSourceNewerUpdatesInPlace(): void ); } - public function testOverwriteSourceEqualTolerates(): void + public function testOverwriteSourceEqualSkips(): void { $when = '2026-04-23T10:00:00.000+00:00'; $this->assertSame( - SchemaAction::Tolerate, + SchemaAction::Skip, OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: $when, @@ -88,12 +88,12 @@ public function testOverwriteSourceEqualTolerates(): void ); } - public function testOverwriteSourceOlderTolerates(): void + public function testOverwriteSourceOlderSkips(): void { // Dest is ahead — don't roll back. Avoids overwriting newer // destination edits with stale source data. $this->assertSame( - SchemaAction::Tolerate, + SchemaAction::Skip, OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: '2026-04-23T09:00:00.000+00:00', @@ -102,12 +102,12 @@ public function testOverwriteSourceOlderTolerates(): void ); } - public function testOverwriteNoTimestampsTolerates(): void + public function testOverwriteNoTimestampsSkips(): void { // No timestamps provided at all → no information to act on. Conservative: - // Tolerate rather than risk a destructive update on uncertain input. + // Skip rather than risk a destructive update on uncertain input. $this->assertSame( - SchemaAction::Tolerate, + SchemaAction::Skip, OnDuplicate::Overwrite->resolveSchemaAction(exists: true), ); } @@ -132,12 +132,12 @@ public static function unparseableTimestampPairs(): array } #[DataProvider('unparseableTimestampPairs')] - public function testOverwriteUnparseableUpdatedAtTolerates(?string $source, ?string $dest): void + public function testOverwriteUnparseableUpdatedAtSkips(?string $source, ?string $dest): void { // Conservative: unparseable updatedAt preserves existing destination // rather than risk a destructive update on garbage input. $this->assertSame( - SchemaAction::Tolerate, + SchemaAction::Skip, OnDuplicate::Overwrite->resolveSchemaAction( exists: true, sourceUpdatedAt: $source,