fix: address Greptile review feedback on migration resource refactor

- Request V25 filter no longer scrubs a non-composite resourceId so the
  UID validator can surface a proper error instead of an empty value.
- V25 schema migration writes resourceType/resourceId only after the
  internal-ID lookup succeeds; if the database/collection no longer
  exists, the document is left untouched so the patch can be retried.
- Worker getCompoundResourceId() returns null when only parentResourceId
  is set, instead of feeding the migration library a malformed
  parent-only ID.
- processMigrationResourceStats() guards against missing/unresolvable
  parent or leaf IDs and skips the stats update with a warning, instead
  of casting null to '' and querying database_0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jake Barnby
2026-05-01 13:04:20 +12:00
co-authored by Claude Opus 4.7
parent fb4930741e
commit 5462998c2c
3 changed files with 41 additions and 17 deletions
+15 -7
View File
@@ -91,26 +91,34 @@ class V25 extends Migration
[$parentId, $childId] = \explode(':', $resourceId, 2);
$parentResourceType = $document->getAttribute('resourceType');
$document
->setAttribute('resourceId', $childId)
->setAttribute('resourceType', 'collection')
->setAttribute('parentResourceId', $parentId)
->setAttribute('parentResourceType', $parentResourceType);
$parentResourceInternalId = '';
$resourceInternalId = '';
try {
$database = $this->dbForProject->getDocument('databases', $parentId);
if (!$database->isEmpty()) {
$document->setAttribute('parentResourceInternalId', (string) $database->getSequence());
$parentResourceInternalId = (string) $database->getSequence();
$collection = $this->dbForProject->getDocument('database_' . $database->getSequence(), $childId);
if (!$collection->isEmpty()) {
$document->setAttribute('resourceInternalId', (string) $collection->getSequence());
$resourceInternalId = (string) $collection->getSequence();
}
}
} catch (Throwable $th) {
Console::warning("Failed to backfill internal IDs for migration {$document->getId()}: {$th->getMessage()}");
// Lookup failed — leave the document untouched so the original
// composite resourceId is preserved and the doc can be retried.
return $document;
}
$document
->setAttribute('resourceId', $childId)
->setAttribute('resourceInternalId', $resourceInternalId)
->setAttribute('resourceType', 'collection')
->setAttribute('parentResourceId', $parentId)
->setAttribute('parentResourceInternalId', $parentResourceInternalId)
->setAttribute('parentResourceType', $parentResourceType);
return $document;
}
}
+19 -6
View File
@@ -610,9 +610,10 @@ class Migrations extends Action
}
/**
* Returns a "{parentResourceId}:{resourceId}" string when both are set, or just
* the available ID otherwise. The utopia-php/migration library expects this
* compound shape on its source/destination constructors.
* Returns a "{parentResourceId}:{resourceId}" string when both are set,
* or the leaf resourceId on its own otherwise. Returning a parent-only
* value would feed the utopia-php/migration library a malformed
* compound, so an unanchored parent yields null instead.
*/
protected function getCompoundResourceId(Document $migration): ?string
{
@@ -623,7 +624,7 @@ class Migrations extends Action
return $parentResourceId . ':' . $resourceId;
}
return $resourceId ?? $parentResourceId;
return !empty($resourceId) ? $resourceId : null;
}
/**
@@ -916,8 +917,20 @@ class Migrations extends Action
$tableInternalId = $resources['tableId'];
if ($source === CSV::getName()) {
$database = $authorization->skip(fn () => $this->dbForProject->getDocument('databases', (string) $parentResourceId));
$table = $authorization->skip(fn () => $this->dbForProject->getDocument('database_' . $database->getSequence(), (string) $resourceId));
if (empty($parentResourceId) || empty($resourceId)) {
Console::warning("Skipping CSV migration usage stats: missing parent/leaf resource ID (parent: '{$parentResourceId}', leaf: '{$resourceId}')");
return;
}
$database = $authorization->skip(fn () => $this->dbForProject->getDocument('databases', $parentResourceId));
if ($database->isEmpty()) {
Console::warning("Skipping CSV migration usage stats: database '{$parentResourceId}' not found");
return;
}
$table = $authorization->skip(fn () => $this->dbForProject->getDocument('database_' . $database->getSequence(), $resourceId));
if ($table->isEmpty()) {
Console::warning("Skipping CSV migration usage stats: collection '{$resourceId}' not found in database '{$parentResourceId}'");
return;
}
$databaseInternalId = (int) $database->getSequence();
$tableInternalId = (int) $table->getSequence();
}
+7 -4
View File
@@ -27,12 +27,15 @@ class V25 extends Filter
return $content;
}
if (\str_contains($content['resourceId'], ':')) {
[$databaseId, $collectionId] = \explode(':', $content['resourceId'], 2);
$content['databaseId'] = $content['databaseId'] ?? $databaseId;
$content['collectionId'] = $content['collectionId'] ?? $collectionId;
if (!\str_contains($content['resourceId'], ':')) {
// Leave malformed resourceId in place so the new UID validator
// surfaces it to the caller instead of silently scrubbing it.
return $content;
}
[$databaseId, $collectionId] = \explode(':', $content['resourceId'], 2);
$content['databaseId'] = $content['databaseId'] ?? $databaseId;
$content['collectionId'] = $content['collectionId'] ?? $collectionId;
unset($content['resourceId']);
return $content;