Multitype db #139
Multitype db #139
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds multi-database support across the migration subsystem: introduces new database resource types (DocumentsDB, VectorDB, legacy), Collection and Document models, and DATABASE/ENTITY/FIELD/RECORD mapping constants. Adds an abstract Attribute class and many concrete attribute/column types (text, integer, decimal, enum, vector, object, geo types, relationship, boolean, datetime, email, ip, url). Refactors Appwrite source/reader and Appwrite destination to use per-database readers/callables and per-database DSNs, updates CSV export to emit collection/document/attribute payloads, and appends database type into resource cache keys. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Migration/Destinations/Appwrite.php (1)
352-363: Normalize database type before DSN resolution.The
typefield normalizes emptygetType()to'legacy', but the DSN lookup receivesgetType()directly, which could be an empty string. If the DSN resolver doesn't handle empty strings, database metadata can point to the wrong DSN.Suggested fix
+ $databaseType = $resource->getType() ?: 'legacy'; $database = $this->dbForProject->createDocument('databases', new UtopiaDocument([ '$id' => $resource->getId(), 'name' => $resource->getDatabaseName(), 'enabled' => $resource->getEnabled(), 'search' => implode(' ', [$resource->getId(), $resource->getDatabaseName()]), '$createdAt' => $resource->getCreatedAt(), '$updatedAt' => $resource->getUpdatedAt(), 'originalId' => empty($resource->getOriginalId()) ? null : $resource->getOriginalId(), - 'type' => empty($resource->getType()) ? 'legacy' : $resource->getType(), + 'type' => $databaseType, // source and destination can be in different location - 'database' => ($this->getDatabaseDSN)($resource->getType()) + 'database' => ($this->getDatabaseDSN)($databaseType) ]));
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 459-464: The guard in createField(Column|Attribute $resource)
currently checks Resource::TYPE_DATABASE_DOCUMENTSDB and sets status with
message "Columns not supported for DocumentsDB" but the method receives both
Column and Attribute; either narrow the condition to only apply when $resource
is an instance of Column, or change the status message to accurately reflect
both types (e.g., "Columns and Attributes not supported for DocumentsDB" or
"Fields not supported for DocumentsDB"); update the check around createField and
the call to $resource->setStatus(Resource::STATUS_SKIPPED, ...) accordingly so
the behavior and message match (refer to createField, Column, Attribute,
Resource::TYPE_DATABASE_DOCUMENTSDB, setStatus, STATUS_SKIPPED).
In `@src/Migration/Resources/Database/Attribute/Decimal.php`:
- Around line 22-23: The current defaulting in Decimal.php uses PHP_FLOAT_MIN
for $min which is the smallest positive float, not a negative bound; update the
default to use -PHP_FLOAT_MAX (or set $min ??= -PHP_FLOAT_MAX) so decimal ranges
allow negative values, keeping $max ??= PHP_FLOAT_MAX unchanged and adjust any
references to $min/$max in the Decimal class or related validation to expect a
potentially negative lower bound.
In `@src/Migration/Resources/Database/Attribute/Integer.php`:
- Around line 78-91: The fromArray method for class Integer is not
reading/passing the constructor's $signed parameter, causing unsigned integers
to be lost; update Integer::fromArray to extract the signed flag (e.g.
$array['signed'] ?? false) and pass it into the Integer constructor (the
__construct parameter named $signed), and also update the method PHPDoc to
document the signed field being deserialized.
In `@src/Migration/Resources/Database/Attribute/Polygon.php`:
- Around line 58-67: In Polygon::fromArray change the handling of the 'default'
index to guard against missing keys: update the constructor call in the
fromArray method to pass $array['default'] ?? null (matching other attribute
factories' null fallback) so an undefined index notice is avoided; leave other
fields (required, createdAt, updatedAt) as-is and locate this in the fromArray
method of the Polygon class to apply the fix.
In `@src/Migration/Resources/Database/VectorDB.php`:
- Around line 14-24: The PHPDoc for the array parameter is missing the "type"
key even though the code reads $array['type'] (see use in the VectorDB
class/method handling that array); update the `@param` array shape to include
type: string so the documented shape matches runtime access (add "type: string"
to the array shape in the PHPDoc for the $array parameter).
In `@src/Migration/Sources/Appwrite.php`:
- Around line 851-857: The array construction is using $databaseName (which
holds the database type) for the 'name' field; update the assignment to use the
actual database name accessor on the $database object (e.g., replace
$databaseName with $database->getDatabaseName() or $database->getName()
depending on the available method) inside the block that builds the 'database'
array so the 'name' value reflects the real database name rather than its type.
In `@src/Migration/Sources/Appwrite/Reader/Database.php`:
- Around line 27-38: The Database class is missing implementations for
getDatabase() and logDebug() which are invoked from within the class; add a
private getDatabase(): UtopiaDatabase method that returns the correct DB
instance by invoking the stored callable $this->getDatabasesDB with
$this->projectId when callable (and falling back to $this->dbForProject), and
add a private logDebug(string $message, array $context = []): void method that
emits debug output (format the message with context) using an existing logger if
present (e.g., $this->logger) or error_log as a fallback so existing calls to
getDatabase() and logDebug() succeed.
In `@src/Migration/Transfer.php`:
- Line 361: The variable $groupDatabasesIndex is assigned from
array_search(Transfer::GROUP_DATABASES, $services) but never used; remove the
unused assignment line to eliminate the dead variable (i.e. delete the statement
that sets $groupDatabasesIndex) so only relevant variables and logic remain; if
the intent was to check presence of Transfer::GROUP_DATABASES in $services
instead, replace the assignment with an explicit conditional using
in_array(Transfer::GROUP_DATABASES, $services) where needed.
🧹 Nitpick comments (13)
src/Migration/Resources/Database/Database.php (1)
62-72: Consider includingdatabaseinjsonSerialize()for round-trip consistency.The new
$databaseproperty is not included injsonSerialize(). If resources are serialized and later reconstructed viafromArray(), thedatabasevalue will be lost. If this is intentional (e.g.,databaseis only for internal routing), consider documenting it; otherwise, add it to the serialized output.Suggested fix (if round-trip is needed)
public function jsonSerialize(): array { return [ 'id' => $this->id, 'name' => $this->name, 'createdAt' => $this->createdAt, 'updatedAt' => $this->updatedAt, 'enabled' => $this->enabled, 'type' => $this->type, + 'database' => $this->database, ]; }src/Migration/Resources/Database/Attribute/Vector.php (1)
61-72: Minor formatting nit: missing space after colon.Line 67 has
size:$array['size']without a space after the colon, unlike other named arguments. This is inconsistent with the rest of the codebase formatting.🔧 Suggested fix
public static function fromArray(array $array): self { return new self( $array['key'], Collection::fromArray($array['table'] ?? $array['collection']), required: $array['required'], - size:$array['size'], + size: $array['size'], default: $array['default'], createdAt: $array['createdAt'] ?? '', updatedAt: $array['updatedAt'] ?? '', ); }src/Migration/Resources/Database/Columns/Vector.php (1)
61-72: Minor formatting nit: missing space after colon.Line 67 has
size:$array['size']without a space after the colon, inconsistent with other named arguments in the file and codebase.🔧 Suggested fix
public static function fromArray(array $array): self { return new self( $array['key'], Table::fromArray($array['table'] ?? $array['collection']), required: $array['required'], - size:$array['size'], + size: $array['size'], default: $array['default'], createdAt: $array['createdAt'] ?? '', updatedAt: $array['updatedAt'] ?? '', ); }src/Migration/Resources/Database/Attribute/Text.php (1)
88-96: Redundant method overrides.Per the relevant code snippet from
src/Migration/Resources/Database/Attribute.php, the baseAttributeclass already definesgetSize()andgetFormat()with identical implementations. These overrides can be removed.♻️ Suggested removal
public function getType(): string { return Attribute::TYPE_STRING; } - - public function getSize(): int - { - return $this->size; - } - - public function getFormat(): string - { - return $this->format; - } }src/Migration/Resources/Database/DocumentsDB.php (1)
14-24: PHPDoc is missing thetypekey.The implementation uses
$array['type'] ?? 'legacy'on line 34, but the PHPDoc shape definition doesn't include thetypekey. Consider adding it for accuracy.📝 Suggested fix
/** * `@param` array{ * id: string, * name: string, * createdAt: string, * updatedAt: string, * enabled: bool, * originalId: string|null, + * type: string, * database: string * } $array */src/Migration/Resources/Database/Attribute/Enum.php (1)
98-101: Unnecessary array cast.The
formatOptions['elements']is always an array by construction (set in constructor line 33 and guaranteed by PHPDoc). The(array)cast is redundant.♻️ Suggested simplification
public function getElements(): array { - return (array)$this->formatOptions['elements']; + return $this->formatOptions['elements']; }src/Migration/Resources/Database/Attribute/Integer.php (1)
98-106: Return type?intis misleading.Both
getMin()andgetMax()are declared as returning?int, but they always return anintdue to the cast and because the constructor defaultsmin/maxtoPHP_INT_MIN/PHP_INT_MAX. Consider changing the return type tointfor accuracy, or removing the cast if null should actually be possible.♻️ Suggested fix (if null is never expected)
- public function getMin(): ?int + public function getMin(): int { return (int)$this->formatOptions['min']; } - public function getMax(): ?int + public function getMax(): int { return (int)$this->formatOptions['max']; }src/Migration/Resources/Database/Attribute/Decimal.php (1)
96-104: Return type inconsistency ingetMin/getMax.The return type is
?float(nullable), but the implementation always casts to(float), which will never returnnull. Either remove the nullable return type or handle the null case.Proposed fix
public function getMin(): ?float { - return (float)$this->formatOptions['min']; + return isset($this->formatOptions['min']) ? (float)$this->formatOptions['min'] : null; } public function getMax(): ?float { - return (float)$this->formatOptions['max']; + return isset($this->formatOptions['max']) ? (float)$this->formatOptions['max'] : null; }src/Migration/Resources/Database/Attribute/Relationship.php (1)
96-99: Minor:getRelatedTable()fallback is effectively dead code.Since the constructor always stores the value under
'relatedCollection', the'relatedTable'key will never exist in$this->options. The null coalescing fallback works correctly, but the first check is unnecessary.Optional simplification
public function getRelatedTable(): string { - return $this->options['relatedTable'] ?? $this->options['relatedCollection']; + return $this->options['relatedCollection']; }However, keeping the fallback may be intentional for compatibility if
fromArrayis used with data containing'relatedTable'.src/Migration/Sources/Appwrite.php (1)
92-95: PHPDoc typo:UtopiaDocumentshould be a valid type.The type hint
UtopiaDocumentappears to be incomplete. It should likely be\Utopia\Database\Documentor similar.Proposed fix
/** - * `@var` callable(UtopiaDocument $database|null): UtopiaDatabase + * `@var` callable(\Utopia\Database\Document|null $database): UtopiaDatabase */ protected mixed $getDatabasesDB;src/Migration/Destinations/Appwrite.php (3)
797-808: Drop unused $supportForSpatialAttributes (or pass into validator).
PHPMD flagged it as unused; keeping it adds noise.♻️ Suggested cleanup
- $supportForSpatialAttributes = $dbForDatabases->getAdapter()->getSupportForSpatialAttributes();
962-975: Avoid unused $value in the row scrub loop.
Small cleanup to satisfy PHPMD.♻️ Suggested cleanup
- foreach ($row as $key => $value) { + foreach ($row as $key => $_) {
1522-1525: Remove unused $columnSize.
PHPMD flags this; drop it unless you plan to use size for index lengths.♻️ Suggested cleanup
- $columnSize = $oldColumns[$columnIndex]['size'];
| $min ??= PHP_FLOAT_MIN; | ||
| $max ??= PHP_FLOAT_MAX; |
There was a problem hiding this comment.
Potential issue with PHP_FLOAT_MIN as minimum bound.
PHP_FLOAT_MIN represents the smallest positive float (~2.2e-308), not a negative minimum. This differs from Integer.php which correctly uses PHP_INT_MIN (a negative value). For decimal/float ranges, if you intend to allow negative values, consider using -PHP_FLOAT_MAX as the lower bound.
Proposed fix
- $min ??= PHP_FLOAT_MIN;
+ $min ??= -PHP_FLOAT_MAX;🤖 Prompt for AI Agents
In `@src/Migration/Resources/Database/Attribute/Decimal.php` around lines 22 - 23,
The current defaulting in Decimal.php uses PHP_FLOAT_MIN for $min which is the
smallest positive float, not a negative bound; update the default to use
-PHP_FLOAT_MAX (or set $min ??= -PHP_FLOAT_MAX) so decimal ranges allow negative
values, keeping $max ??= PHP_FLOAT_MAX unchanged and adjust any references to
$min/$max in the Decimal class or related validation to expect a potentially
negative lower bound.
There was a problem hiding this comment.
No it is same as column/integer
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Columns/Decimal.php implementation for min/max defaults
cat src/Migration/Resources/Database/Columns/Decimal.php | grep -A 30 "function __construct"
Length of output: 775
@ArnabChatterjee20k, you're absolutely right! I've verified that Columns/Decimal.php uses the identical pattern with PHP_FLOAT_MIN for the min default. The implementations are intentionally consistent, which aligns with the established pattern in this codebase where Attribute classes mirror their corresponding Column classes in constructor defaults.
I apologize for the confusion in my original review comment—it didn't account for this intentional consistency pattern. Thank you for the clarification!
🧠 Learnings used
Learnt from: ArnabChatterjee20k
Repo: utopia-php/migration PR: 114
File: src/Migration/Resources/Database/Attribute/Decimal.php:10-38
Timestamp: 2025-11-20T13:07:39.293Z
Learning: In the utopia-php/migration codebase, the user ArnabChatterjee20k maintains consistency between parallel implementations: Attribute classes (e.g., Attribute/Decimal.php) mirror their corresponding Column classes (e.g., Columns/Decimal.php) in constructor defaults and behavior, even when design decisions might be questioned.
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 81
File: src/Migration/Sources/CSV.php:215-233
Timestamp: 2025-07-02T06:24:50.844Z
Learning: In the utopia-php/migration codebase, invalid numeric values (like null) in parsed data are acceptable because the underlying database structure validator will catch and handle these validation errors, so explicit filtering during parsing is not required.
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Sources/Appwrite.php:843-851
Timestamp: 2025-06-28T09:47:08.333Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the class constructors and method parameters use the new terminology (like "relatedTable"), but the underlying data structures and API responses still use the legacy keys (like "relatedCollection"). This is an intentional design pattern to allow gradual migration while maintaining compatibility with existing data sources.
| public static function fromArray(array $array): self | ||
| { | ||
| return new self( | ||
| $array['key'], | ||
| Collection::fromArray($array['table'] ?? $array['collection']), | ||
| required: $array['required'], | ||
| default: $array['default'], | ||
| array: $array['array'], | ||
| min: $array['formatOptions']['min'] ?? null, | ||
| max: $array['formatOptions']['max'] ?? null, | ||
| createdAt: $array['createdAt'] ?? '', | ||
| updatedAt: $array['updatedAt'] ?? '', | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing signed parameter in fromArray.
The constructor accepts a $signed parameter (line 18), but fromArray doesn't extract or pass it. This causes data loss during deserialization—unsigned integers will be incorrectly reconstructed as signed.
🐛 Suggested fix
public static function fromArray(array $array): self
{
return new self(
$array['key'],
Collection::fromArray($array['table'] ?? $array['collection']),
required: $array['required'],
default: $array['default'],
array: $array['array'],
min: $array['formatOptions']['min'] ?? null,
max: $array['formatOptions']['max'] ?? null,
+ signed: $array['signed'] ?? true,
createdAt: $array['createdAt'] ?? '',
updatedAt: $array['updatedAt'] ?? '',
);
}Also update the PHPDoc to include:
* formatOptions: array{
* min: ?int,
* max: ?int
* },
+ * signed: bool,
* createdAt: string,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static function fromArray(array $array): self | |
| { | |
| return new self( | |
| $array['key'], | |
| Collection::fromArray($array['table'] ?? $array['collection']), | |
| required: $array['required'], | |
| default: $array['default'], | |
| array: $array['array'], | |
| min: $array['formatOptions']['min'] ?? null, | |
| max: $array['formatOptions']['max'] ?? null, | |
| createdAt: $array['createdAt'] ?? '', | |
| updatedAt: $array['updatedAt'] ?? '', | |
| ); | |
| } | |
| public static function fromArray(array $array): self | |
| { | |
| return new self( | |
| $array['key'], | |
| Collection::fromArray($array['table'] ?? $array['collection']), | |
| required: $array['required'], | |
| default: $array['default'], | |
| array: $array['array'], | |
| min: $array['formatOptions']['min'] ?? null, | |
| max: $array['formatOptions']['max'] ?? null, | |
| signed: $array['signed'] ?? true, | |
| createdAt: $array['createdAt'] ?? '', | |
| updatedAt: $array['updatedAt'] ?? '', | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@src/Migration/Resources/Database/Attribute/Integer.php` around lines 78 - 91,
The fromArray method for class Integer is not reading/passing the constructor's
$signed parameter, causing unsigned integers to be lost; update
Integer::fromArray to extract the signed flag (e.g. $array['signed'] ?? false)
and pass it into the Integer constructor (the __construct parameter named
$signed), and also update the method PHPDoc to document the signed field being
deserialized.
| public static function fromArray(array $array): self | ||
| { | ||
| return new self( | ||
| $array['key'], | ||
| Collection::fromArray($array['table'] ?? $array['collection']), | ||
| required: $array['required'], | ||
| default: $array['default'], | ||
| createdAt: $array['createdAt'] ?? '', | ||
| updatedAt: $array['updatedAt'] ?? '', | ||
| ); |
There was a problem hiding this comment.
Guard against missing default key.
If default is absent, this raises an undefined index notice. Use a null fallback for consistency with other attribute factories.
🛠️ Proposed fix
- default: $array['default'],
+ default: $array['default'] ?? null,🤖 Prompt for AI Agents
In `@src/Migration/Resources/Database/Attribute/Polygon.php` around lines 58 - 67,
In Polygon::fromArray change the handling of the 'default' index to guard
against missing keys: update the constructor call in the fromArray method to
pass $array['default'] ?? null (matching other attribute factories' null
fallback) so an undefined index notice is avoided; leave other fields (required,
createdAt, updatedAt) as-is and locate this in the fromArray method of the
Polygon class to apply the fix.
| /** | ||
| * @param array{ | ||
| * id: string, | ||
| * name: string, | ||
| * createdAt: string, | ||
| * updatedAt: string, | ||
| * enabled: bool, | ||
| * originalId: string|null, | ||
| * database: string | ||
| * } $array | ||
| */ |
There was a problem hiding this comment.
PHPDoc is missing the type key.
The @param array shape documents database: string but omits the type key, even though line 34 accesses $array['type']. Add type: string to the PHPDoc for consistency.
Suggested fix
/**
* `@param` array{
* id: string,
* name: string,
* createdAt: string,
* updatedAt: string,
* enabled: bool,
* originalId: string|null,
+ * type: string,
* database: string
* } $array
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @param array{ | |
| * id: string, | |
| * name: string, | |
| * createdAt: string, | |
| * updatedAt: string, | |
| * enabled: bool, | |
| * originalId: string|null, | |
| * database: string | |
| * } $array | |
| */ | |
| /** | |
| * `@param` array{ | |
| * id: string, | |
| * name: string, | |
| * createdAt: string, | |
| * updatedAt: string, | |
| * enabled: bool, | |
| * originalId: string|null, | |
| * type: string, | |
| * database: string | |
| * } $array | |
| */ |
🤖 Prompt for AI Agents
In `@src/Migration/Resources/Database/VectorDB.php` around lines 14 - 24, The
PHPDoc for the array parameter is missing the "type" key even though the code
reads $array['type'] (see use in the VectorDB class/method handling that array);
update the `@param` array shape to include type: string so the documented shape
matches runtime access (add "type: string" to the array shape in the PHPDoc for
the $array parameter).
| 'database' => [ | ||
| 'id' => $database->getId(), | ||
| 'name' => $databaseName, | ||
| 'type' => $database->getType(), | ||
| 'database' => $database->getDatabase(), | ||
| ] | ||
| ]); |
There was a problem hiding this comment.
Potential bug: 'name' => $databaseName passes the database type instead of actual name.
At line 853, $databaseName is the database type string (e.g., 'database', 'documentsdb'), not the actual database name. This should likely be $database->getDatabaseName() or similar.
Proposed fix
$newTable = self::getEntity($databaseName, [
'id' => $table['$id'],
'name' => $table['name'],
'documentSecurity' => $table['documentSecurity'],
'permissions' => $table['$permissions'],
'createdAt' => $table['$createdAt'],
'updatedAt' => $table['$updatedAt'],
'database' => [
'id' => $database->getId(),
- 'name' => $databaseName,
+ 'name' => $database->getDatabaseName(),
'type' => $database->getType(),
'database' => $database->getDatabase(),
]
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'database' => [ | |
| 'id' => $database->getId(), | |
| 'name' => $databaseName, | |
| 'type' => $database->getType(), | |
| 'database' => $database->getDatabase(), | |
| ] | |
| ]); | |
| 'database' => [ | |
| 'id' => $database->getId(), | |
| 'name' => $database->getDatabaseName(), | |
| 'type' => $database->getType(), | |
| 'database' => $database->getDatabase(), | |
| ] | |
| ]); |
🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite.php` around lines 851 - 857, The array
construction is using $databaseName (which holds the database type) for the
'name' field; update the assignment to use the actual database name accessor on
the $database object (e.g., replace $databaseName with
$database->getDatabaseName() or $database->getName() depending on the available
method) inside the block that builds the 'database' array so the 'name' value
reflects the real database name rather than its type.
| /** | ||
| * @var callable(UtopiaDocument|null): UtopiaDatabase | ||
| */ | ||
| private mixed $getDatabasesDB; | ||
|
|
||
| public function __construct( | ||
| private readonly UtopiaDatabase $dbForProject, | ||
| ?callable $getDatabasesDB = null, | ||
| private readonly ?string $projectId = null | ||
| ) { | ||
| $this->getDatabasesDB = $getDatabasesDB; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify getDatabase and logDebug methods are defined in Reader interface or related files
echo "=== Checking Reader interface ==="
cat src/Migration/Sources/Appwrite/Reader.php 2>/dev/null || echo "Reader.php not found"
echo ""
echo "=== Searching for getDatabase method definition ==="
rg -n 'function getDatabase\(' --type=php -C2
echo ""
echo "=== Searching for logDebug method definition ==="
rg -n 'function logDebug\(' --type=php -C2Repository: utopia-php/migration
Length of output: 4579
🏁 Script executed:
#!/bin/bash
# Check the Database.php class definition (parent class, traits, etc.)
echo "=== Checking Database.php class definition ==="
head -60 src/Migration/Sources/Appwrite/Reader/Database.php
echo ""
echo "=== Searching for logDebug in the entire codebase with context ==="
rg -i 'logdebug' --type=php -C2
echo ""
echo "=== Checking if Database class extends a parent or uses traits ==="
rg -n 'class Database' src/Migration/Sources/Appwrite/Reader/Database.php -A 5
echo ""
echo "=== Checking if getDatabase is called in Database.php and what parameters it takes ==="
rg -n 'getDatabase' src/Migration/Sources/Appwrite/Reader/Database.php -B2 -A2Repository: utopia-php/migration
Length of output: 9920
Fix missing method implementations: getDatabase() and logDebug()
The Database class calls $this->getDatabase() at lines 343, 398, and 496, and $this->logDebug() at lines 346 and 348, but these methods do not exist. The class implements only the Reader interface, has no parent class, and no traits that would provide these methods. The logDebug() method does not exist anywhere in the codebase, and getDatabase() is not defined in this class, its interface, or any parent. Either implement these methods in the Database class or provide them through a parent class or trait.
🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite/Reader/Database.php` around lines 27 - 38, The
Database class is missing implementations for getDatabase() and logDebug() which
are invoked from within the class; add a private getDatabase(): UtopiaDatabase
method that returns the correct DB instance by invoking the stored callable
$this->getDatabasesDB with $this->projectId when callable (and falling back to
$this->dbForProject), and add a private logDebug(string $message, array $context
= []): void method that emits debug output (format the message with context)
using an existing logger if present (e.g., $this->logger) or error_log as a
fallback so existing calls to getDatabase() and logDebug() succeed.
src/Migration/Transfer.php
Outdated
| public static function extractServices(array $services): array | ||
| { | ||
| $resources = []; | ||
| $groupDatabasesIndex = array_search(Transfer::GROUP_DATABASES, $services); |
There was a problem hiding this comment.
Remove unused variable $groupDatabasesIndex.
This variable is assigned but never used, as flagged by static analysis.
Proposed fix
- $groupDatabasesIndex = array_search(Transfer::GROUP_DATABASES, $services);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $groupDatabasesIndex = array_search(Transfer::GROUP_DATABASES, $services); |
🧰 Tools
🪛 PHPMD (2.15.0)
361-361: Avoid unused local variables such as '$groupDatabasesIndex'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In `@src/Migration/Transfer.php` at line 361, The variable $groupDatabasesIndex is
assigned from array_search(Transfer::GROUP_DATABASES, $services) but never used;
remove the unused assignment line to eliminate the dead variable (i.e. delete
the statement that sets $groupDatabasesIndex) so only relevant variables and
logic remain; if the intent was to check presence of Transfer::GROUP_DATABASES
in $services instead, replace the assignment with an explicit conditional using
in_array(Transfer::GROUP_DATABASES, $services) where needed.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Migration/Sources/Appwrite.php (1)
733-745: Root database filter skips VectorDB.Line 733 only checks
TYPE_DATABASEandTYPE_DATABASE_DOCUMENTSDB, so a VectorDB root export won’t be filtered by ID and can export all databases. AddTYPE_DATABASE_VECTORDBto the condition.🔧 Suggested fix
- if ($this->rootResourceId !== '' && ($this->rootResourceType === Resource::TYPE_DATABASE || $this->rootResourceType === Resource::TYPE_DATABASE_DOCUMENTSDB)) { + if ( + $this->rootResourceId !== '' && + \in_array($this->rootResourceType, [ + Resource::TYPE_DATABASE, + Resource::TYPE_DATABASE_DOCUMENTSDB, + Resource::TYPE_DATABASE_VECTORDB, + ], true) + ) {
🤖 Fix all issues with AI agents
In `@src/Migration/Sources/Appwrite.php`:
- Around line 1072-1084: The code calls getRecord with the database name instead
of its type which makes record creation default to Row; change the first
argument of getRecord to use $table->getDatabase()->getType() (instead of
getDatabaseName()) so getRecord receives the expected database type; update the
call site in the Appwrite migration code where getRecord(...) is invoked and
ensure related metadata (the rest of the associative array built from $table and
$table->getDatabase()) remains unchanged.
- Around line 991-1034: Remove the unused local variables that PHPMD flags:
delete the $memUsage assignment and the two $timestamp assignments around the
listRows call in the method that iterates table rows (the block using
$this->reader->listRows($table, $queries)); alternatively, if you want to keep
those diagnostics, re-enable the commented $this->logDebugTrackedProject(...)
calls or guard them behind a debug flag so the variables are actually used
(refer to the logDebugTrackedProject call sites and the $this->reader->listRows
invocation when making the change).
In `@src/Migration/Sources/Appwrite/Reader/Database.php`:
- Around line 91-97: The code reads $database->getAttribute('type') and
immediately indexes Resource::DATABASE_TYPE_RESOURCE_MAP[$databaseType], which
can throw an undefined index for missing/unexpected types; update the logic in
the block around $databaseType and $databaseSpecificResources to default to
Resource::TYPE_DATABASE when getAttribute('type') returns null/empty or when
$databaseType is not a key in Resource::DATABASE_TYPE_RESOURCE_MAP (also keep
the existing normalization for Resource::TYPE_DATABASE_LEGACY and
Resource::TYPE_DATABASE_TABLESDB), e.g. compute $databaseType with a fallback
and guard the lookup with isset/array_key_exists before assigning
$databaseSpecificResources so it never indexes an unknown key.
♻️ Duplicate comments (1)
src/Migration/Sources/Appwrite.php (1)
830-842: Database name is set to type string, not actual name.Line 839 still assigns
'name' => $databaseName, but$databaseNameholds the database type (e.g.,database,documentsdb) rather than the actual database name. This was already raised previously.🔧 Suggested fix
- 'name' => $databaseName, + 'name' => $database->getDatabaseName(),
src/Migration/Sources/Appwrite.php
Outdated
| $iterationCount++; | ||
|
|
||
| $memUsage = round(memory_get_usage(true) / 1024 / 1024, 2); | ||
| // $this->logDebugTrackedProject("Table: {$table->getName()} | Iteration: $iterationCount | Memory: {$memUsage}MB | LastRow: " . ($lastRow ? $lastRow->getId() : 'null')); | ||
|
|
||
| $queries = [ | ||
| $this->database->queryLimit($batchSize), | ||
| $this->reader->queryLimit($batchSize), | ||
| ...$this->queries, | ||
| ]; | ||
|
|
||
| $rows = []; | ||
|
|
||
| if ($lastRow) { | ||
| $queries[] = $this->database->queryCursorAfter($lastRow); | ||
| $queries[] = $this->reader->queryCursorAfter($lastRow); | ||
| } | ||
|
|
||
| $selects = ['*', '$id', '$permissions', '$updatedAt', '$createdAt']; // We want relations flat! | ||
| $manyToMany = []; | ||
|
|
||
| $attributes = $this->cache->get(Column::getName()); | ||
| foreach ($attributes as $attribute) { | ||
| /** @var Relationship $attribute */ | ||
| if ( | ||
| $attribute->getTable()->getId() === $table->getId() && | ||
| $attribute->getType() === Column::TYPE_RELATIONSHIP && | ||
| $attribute->getSide() === 'parent' && | ||
| $attribute->getRelationType() == 'manyToMany' | ||
| ) { | ||
| /** | ||
| * Blockers: | ||
| * we should use but Does not work properly: | ||
| * $selects[] = $attribute->getKey() . '.$id'; | ||
| * when selecting for a relation we get all relations not just the one we were asking. | ||
| * when selecting for a relation like select(*, relation.$id) , all relations get resolve | ||
| */ | ||
| $manyToMany[] = $attribute->getKey(); | ||
| if ($this->reader->getSupportForAttributes()) { | ||
| $attributes = $this->cache->get($fieldName); | ||
|
|
||
| foreach ($attributes as $attribute) { | ||
| /** @var Relationship $attribute */ | ||
| if ( | ||
| $attribute->getTable()->getId() === $table->getId() && | ||
| $attribute->getType() === Column::TYPE_RELATIONSHIP && | ||
| $attribute->getSide() === 'parent' && | ||
| $attribute->getRelationType() == 'manyToMany' | ||
| ) { | ||
| $manyToMany[] = $attribute->getKey(); | ||
| } | ||
| } | ||
| } | ||
| /** @var Column|Relationship $attribute */ | ||
|
|
||
| $queries[] = $this->database->querySelect($selects); | ||
| $queries[] = $this->reader->querySelect($selects); | ||
|
|
||
| $timestamp = microtime(true); | ||
| // $this->logDebugTrackedProject("BEFORE listRows() | Table: {$table->getName()} | Batch: $batchSize | Timestamp: {$timestamp}"); | ||
|
|
||
| $response = $this->database->listRows($table, $queries); | ||
| $response = $this->reader->listRows($table, $queries); | ||
|
|
||
| $timestamp = microtime(true); | ||
| // $this->logDebugTrackedProject("AFTER listRows() | Table: {$table->getName()} | Rows: " . count($response) . " | Timestamp: $timestamp"); |
There was a problem hiding this comment.
Remove unused local variables to satisfy PHPMD.
$memUsage (Line 993) and $timestamp (Lines 1028/1033) are computed but never used because the debug logs are commented out. PHPMD flags these as unused locals. Either re-enable logging behind a flag or remove the variables.
🔧 Suggested cleanup
- $memUsage = round(memory_get_usage(true) / 1024 / 1024, 2);
- // $this->logDebugTrackedProject("Table: {$table->getName()} | Iteration: $iterationCount | Memory: {$memUsage}MB | LastRow: " . ($lastRow ? $lastRow->getId() : 'null'));
+ // $this->logDebugTrackedProject("Table: {$table->getName()} | Iteration: $iterationCount | Memory: " . round(memory_get_usage(true) / 1024 / 1024, 2) . "MB | LastRow: " . ($lastRow ? $lastRow->getId() : 'null'));
- $timestamp = microtime(true);
// $this->logDebugTrackedProject("BEFORE listRows() | Table: {$table->getName()} | Batch: $batchSize | Timestamp: {$timestamp}");
- $response = $this->reader->listRows($table, $queries);
-
- $timestamp = microtime(true);
- // $this->logDebugTrackedProject("AFTER listRows() | Table: {$table->getName()} | Rows: " . count($response) . " | Timestamp: $timestamp");
+ $response = $this->reader->listRows($table, $queries);
+ // $this->logDebugTrackedProject("AFTER listRows() | Table: {$table->getName()} | Rows: " . count($response) . " | Timestamp: " . microtime(true));🧰 Tools
🪛 PHPMD (2.15.0)
993-993: Avoid unused local variables such as '$memUsage'. (undefined)
(UnusedLocalVariable)
1028-1028: Avoid unused local variables such as '$timestamp'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite.php` around lines 991 - 1034, Remove the
unused local variables that PHPMD flags: delete the $memUsage assignment and the
two $timestamp assignments around the listRows call in the method that iterates
table rows (the block using $this->reader->listRows($table, $queries));
alternatively, if you want to keep those diagnostics, re-enable the commented
$this->logDebugTrackedProject(...) calls or guard them behind a debug flag so
the variables are actually used (refer to the logDebugTrackedProject call sites
and the $this->reader->listRows invocation when making the change).
src/Migration/Sources/Appwrite.php
Outdated
| $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ | ||
| 'id' => $id, | ||
| 'table' => [ | ||
| 'id' => $table->getId(), | ||
| 'name' => $table->getTableName(), | ||
| 'rowSecurity' => $table->getRowSecurity(), | ||
| 'permissions' => $table->getPermissions(), | ||
| 'database' => [ | ||
| 'id' => $table->getDatabase()->getId(), | ||
| 'name' => $table->getDatabase()->getDatabaseName(), | ||
| 'type' => $table->getDatabase()->getType(), | ||
| 'database' => $table->getDatabase()->getDatabase(), | ||
| ] |
There was a problem hiding this comment.
Record type selection uses database name instead of database type.
Line 1072 calls getRecord($table->getDatabase()->getDatabaseName(), ...). getRecord() expects a database type (TYPE_DATABASE_DOCUMENTSDB, TYPE_DATABASE_VECTORDB, etc.), so passing the database name will cause Document DB/Vector DB records to be created as Row by default.
🔧 Suggested fix
- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [
+ $row = self::getRecord($table->getDatabase()->getType(), [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ | |
| 'id' => $id, | |
| 'table' => [ | |
| 'id' => $table->getId(), | |
| 'name' => $table->getTableName(), | |
| 'rowSecurity' => $table->getRowSecurity(), | |
| 'permissions' => $table->getPermissions(), | |
| 'database' => [ | |
| 'id' => $table->getDatabase()->getId(), | |
| 'name' => $table->getDatabase()->getDatabaseName(), | |
| 'type' => $table->getDatabase()->getType(), | |
| 'database' => $table->getDatabase()->getDatabase(), | |
| ] | |
| $row = self::getRecord($table->getDatabase()->getType(), [ | |
| 'id' => $id, | |
| 'table' => [ | |
| 'id' => $table->getId(), | |
| 'name' => $table->getTableName(), | |
| 'rowSecurity' => $table->getRowSecurity(), | |
| 'permissions' => $table->getPermissions(), | |
| 'database' => [ | |
| 'id' => $table->getDatabase()->getId(), | |
| 'name' => $table->getDatabase()->getDatabaseName(), | |
| 'type' => $table->getDatabase()->getType(), | |
| 'database' => $table->getDatabase()->getDatabase(), | |
| ] |
🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite.php` around lines 1072 - 1084, The code calls
getRecord with the database name instead of its type which makes record creation
default to Row; change the first argument of getRecord to use
$table->getDatabase()->getType() (instead of getDatabaseName()) so getRecord
receives the expected database type; update the call site in the Appwrite
migration code where getRecord(...) is invoked and ensure related metadata (the
rest of the associative array built from $table and $table->getDatabase())
remains unchanged.
| $databaseType = $database->getAttribute('type'); | ||
| if (in_array($databaseType, [Resource::TYPE_DATABASE_LEGACY,Resource::TYPE_DATABASE_TABLESDB])) { | ||
| $databaseType = Resource::TYPE_DATABASE; | ||
| } | ||
|
|
||
| if (!Resource::isSupported([Resource::TYPE_ROW, Resource::TYPE_COLUMN, Resource::TYPE_INDEX], $resources)) { | ||
| continue; | ||
| } | ||
| $databaseSpecificResources = Resource::DATABASE_TYPE_RESOURCE_MAP[$databaseType]; | ||
|
|
There was a problem hiding this comment.
Guard against missing/unknown database type before indexing the resource map.
Line 91 uses $database->getAttribute('type') without a default and immediately indexes Resource::DATABASE_TYPE_RESOURCE_MAP[$databaseType]. If older DB metadata lacks type (or has an unexpected value), this will trigger an undefined index and break the report. Consider defaulting to Resource::TYPE_DATABASE when the value is missing or not in the map.
🔧 Suggested fix
- $databaseType = $database->getAttribute('type');
- if (in_array($databaseType, [Resource::TYPE_DATABASE_LEGACY,Resource::TYPE_DATABASE_TABLESDB])) {
- $databaseType = Resource::TYPE_DATABASE;
- }
-
- $databaseSpecificResources = Resource::DATABASE_TYPE_RESOURCE_MAP[$databaseType];
+ $databaseType = $database->getAttribute('type', Resource::TYPE_DATABASE);
+ if (\in_array($databaseType, [Resource::TYPE_DATABASE_LEGACY, Resource::TYPE_DATABASE_TABLESDB], true)) {
+ $databaseType = Resource::TYPE_DATABASE;
+ }
+ if (!isset(Resource::DATABASE_TYPE_RESOURCE_MAP[$databaseType])) {
+ $databaseType = Resource::TYPE_DATABASE;
+ }
+
+ $databaseSpecificResources = Resource::DATABASE_TYPE_RESOURCE_MAP[$databaseType];🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite/Reader/Database.php` around lines 91 - 97, The
code reads $database->getAttribute('type') and immediately indexes
Resource::DATABASE_TYPE_RESOURCE_MAP[$databaseType], which can throw an
undefined index for missing/unexpected types; update the logic in the block
around $databaseType and $databaseSpecificResources to default to
Resource::TYPE_DATABASE when getAttribute('type') returns null/empty or when
$databaseType is not a key in Resource::DATABASE_TYPE_RESOURCE_MAP (also keep
the existing normalization for Resource::TYPE_DATABASE_LEGACY and
Resource::TYPE_DATABASE_TABLESDB), e.g. compute $databaseType with a fallback
and guard the lookup with isset/array_key_exists before assigning
$databaseSpecificResources so it never indexes an unknown key.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Migration/Destinations/Appwrite.php (1)
796-813: Normalized dates computed but not used.Lines 796-797 normalize the dates, but lines 811-812 use the raw resource values instead. This inconsistency could cause date format issues.
Proposed fix
$index = new UtopiaDocument([ '$id' => ID::custom($database->getSequence() . '_' . $table->getSequence() . '_' . $resource->getKey()), 'key' => $resource->getKey(), 'status' => 'available', // processing, available, failed, deleting, stuck 'databaseInternalId' => $database->getSequence(), 'databaseId' => $database->getId(), 'collectionInternalId' => $table->getSequence(), 'collectionId' => $table->getId(), 'type' => $resource->getType(), 'attributes' => $resource->getColumns(), 'lengths' => $lengths, 'orders' => $resource->getOrders(), - '$createdAt' => $resource->getCreatedAt(), - '$updatedAt' => $resource->getUpdatedAt(), + '$createdAt' => $createdAt, + '$updatedAt' => $updatedAt, ]);
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 82-84: The PHPDoc parameter name for the callable is incorrect: it
documents $getDatabasesDSN but the actual parameter is named $getDatabaseDSN;
update the PHPDoc in src/Migration/Destinations/Appwrite.php so the param
annotation matches the real parameter name ($getDatabaseDSN) (or rename the
function parameter to $getDatabasesDSN if you prefer consistency), ensuring the
callable signature stays callable(string $databaseType):string and references
the correct symbol ($getDatabaseDSN) used in the class/method.
- Around line 352-357: The code duplicates the normalizeDateTime calls for
$createdAt and $updatedAt in the Appwrite migration (duplicate lines call
normalizeDateTime twice); remove the second pair of duplicate calls so
$createdAt and $updatedAt are only set once (keep the first calls that use
normalizeDateTime($resource->getCreatedAt()) and
normalizeDateTime($resource->getUpdatedAt(), $createdAt)), ensuring the
$updatedAt uses the $createdAt value from the initial normalization.
- Around line 973-994: The foreach currently declares an unused $value when
iterating $this->rowBuffer rows; change the inner loop to iterate keys only so
$value is not declared (e.g. replace foreach ($row as $key => $value) with a
keys-only iteration such as foreach (array_keys($row->getArrayCopy()) as $key)),
keeping the surrounding logic that checks $table->getAttribute('attributes', [])
and calls $row->removeAttribute($key) intact; this removes the unused $value
while preserving the orphan-attribute removal behavior tied to
$dbForDatabases->getAdapter()->getSupportForAttributes().
- Around line 429-434: The code duplicates calls to $this->normalizeDateTime for
$createdAt and $updatedAt in the Appwrite migration class; remove the second
pair of duplicate lines so only the first calls remain (keep the first
assignments: $createdAt = $this->normalizeDateTime($resource->getCreatedAt());
and $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(),
$createdAt);), ensuring the variables are assigned once in the method in class
Appwrite (look for the normalizeDateTime calls around the existing assignments
to $createdAt and $updatedAt).
- Line 15: Remove the unused import Dom\Document from the Appwrite migration
file: locate the top-level use statement "use Dom\Document" in the Appwrite
class and delete it (or replace it with the correct import if it was intended to
be used), ensuring no references to Document remain in the class (check methods
in class Appwrite) so the file no longer contains the unused import.
♻️ Duplicate comments (1)
src/Migration/Destinations/Appwrite.php (1)
471-476: Clarify DocumentsDB guard behavior and message.The guard blocks both
ColumnandAttributeresources for DocumentsDB, but the status message only mentions "Columns". Consider updating the message to accurately reflect the intended behavior:Proposed fix
if ($resource->getTable()->getDatabase()->getType() === Resource::TYPE_DATABASE_DOCUMENTSDB) { - $resource->setStatus(Resource::STATUS_SKIPPED, 'Columns not supported for DocumentsDB'); + $resource->setStatus(Resource::STATUS_SKIPPED, 'Schema fields not supported for DocumentsDB'); return false; }
🧹 Nitpick comments (1)
src/Migration/Destinations/Appwrite.php (1)
1534-1537: Unused variable$columnSize.The variable
$columnSizeis extracted but never used. Either remove it or add a comment indicating it's reserved for future use.Proposed fix (remove unused variable)
$columnStatus = $oldColumns[$columnIndex]['status']; $columnType = $oldColumns[$columnIndex]['type']; - $columnSize = $oldColumns[$columnIndex]['size']; $columnArray = $oldColumns[$columnIndex]['array'] ?? false;
| use Appwrite\Services\Storage; | ||
| use Appwrite\Services\Teams; | ||
| use Appwrite\Services\Users; | ||
| use Dom\Document; |
There was a problem hiding this comment.
Unused import detected.
Dom\Document is imported but not used anywhere in the file. This appears to be a leftover from refactoring.
Proposed fix
-use Dom\Document;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use Dom\Document; |
🤖 Prompt for AI Agents
In `@src/Migration/Destinations/Appwrite.php` at line 15, Remove the unused import
Dom\Document from the Appwrite migration file: locate the top-level use
statement "use Dom\Document" in the Appwrite class and delete it (or replace it
with the correct import if it was intended to be used), ensuring no references
to Document remain in the class (check methods in class Appwrite) so the file no
longer contains the unused import.
| * @param UtopiaDatabase $dbForProject | ||
| * @param callable(UtopiaDocument $database):UtopiaDatabase $getDatabasesDB | ||
| * @param callable(string $databaseType):string $getDatabasesDSN |
There was a problem hiding this comment.
PHPDoc parameter name mismatch.
The PHPDoc shows $getDatabasesDSN but the actual parameter is $getDatabaseDSN (without the 's').
Proposed fix
- * `@param` callable(string $databaseType):string $getDatabasesDSN
+ * `@param` callable(string $databaseType):string $getDatabaseDSN🤖 Prompt for AI Agents
In `@src/Migration/Destinations/Appwrite.php` around lines 82 - 84, The PHPDoc
parameter name for the callable is incorrect: it documents $getDatabasesDSN but
the actual parameter is named $getDatabaseDSN; update the PHPDoc in
src/Migration/Destinations/Appwrite.php so the param annotation matches the real
parameter name ($getDatabaseDSN) (or rename the function parameter to
$getDatabasesDSN if you prefer consistency), ensuring the callable signature
stays callable(string $databaseType):string and references the correct symbol
($getDatabaseDSN) used in the class/method.
| $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); | ||
| $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); | ||
|
|
||
| $database = $this->database->createDocument('databases', new UtopiaDocument([ | ||
| $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); | ||
| $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); | ||
|
|
There was a problem hiding this comment.
Duplicate normalizeDateTime calls.
Lines 355-356 duplicate the exact same normalization already performed on lines 352-353. This appears to be a copy-paste error.
Proposed fix
$createdAt = $this->normalizeDateTime($resource->getCreatedAt());
$updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt);
- $createdAt = $this->normalizeDateTime($resource->getCreatedAt());
- $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt);
-
$database = $this->dbForProject->createDocument('databases', new UtopiaDocument([🤖 Prompt for AI Agents
In `@src/Migration/Destinations/Appwrite.php` around lines 352 - 357, The code
duplicates the normalizeDateTime calls for $createdAt and $updatedAt in the
Appwrite migration (duplicate lines call normalizeDateTime twice); remove the
second pair of duplicate calls so $createdAt and $updatedAt are only set once
(keep the first calls that use normalizeDateTime($resource->getCreatedAt()) and
normalizeDateTime($resource->getUpdatedAt(), $createdAt)), ensuring the
$updatedAt uses the $createdAt value from the initial normalization.
| $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); | ||
| $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); | ||
|
|
||
| $table = $this->database->createDocument('database_' . $database->getSequence(), new UtopiaDocument([ | ||
| $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); | ||
| $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); | ||
|
|
There was a problem hiding this comment.
Duplicate normalizeDateTime calls.
Lines 432-433 duplicate the normalization already performed on lines 429-430.
Proposed fix
$createdAt = $this->normalizeDateTime($resource->getCreatedAt());
$updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt);
- $createdAt = $this->normalizeDateTime($resource->getCreatedAt());
- $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt);
-
$dbForDatabases = ($this->getDatabasesDB)($database);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); | |
| $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); | |
| $table = $this->database->createDocument('database_' . $database->getSequence(), new UtopiaDocument([ | |
| $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); | |
| $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); | |
| $createdAt = $this->normalizeDateTime($resource->getCreatedAt()); | |
| $updatedAt = $this->normalizeDateTime($resource->getUpdatedAt(), $createdAt); | |
| $dbForDatabases = ($this->getDatabasesDB)($database); |
🤖 Prompt for AI Agents
In `@src/Migration/Destinations/Appwrite.php` around lines 429 - 434, The code
duplicates calls to $this->normalizeDateTime for $createdAt and $updatedAt in
the Appwrite migration class; remove the second pair of duplicate lines so only
the first calls remain (keep the first assignments: $createdAt =
$this->normalizeDateTime($resource->getCreatedAt()); and $updatedAt =
$this->normalizeDateTime($resource->getUpdatedAt(), $createdAt);), ensuring the
variables are assigned once in the method in class Appwrite (look for the
normalizeDateTime calls around the existing assignments to $createdAt and
$updatedAt).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Migration/Destinations/Appwrite.php (2)
796-813: Use normalized timestamps for index metadata.
$createdAt/$updatedAtare computed but not used. If the raw resource values aren’t normalized, index creation can fail validation.✅ Fix
- '$createdAt' => $resource->getCreatedAt(), - '$updatedAt' => $resource->getUpdatedAt(), + '$createdAt' => $createdAt, + '$updatedAt' => $updatedAt,
570-598: Use the target database adapter for attribute validation.The
checkAttribute()call validates against$this->dbForProject, which is the project metadata store, not the target database adapter. Since the attribute definition includes adapter-specific fields (type,format,formatOptions,filters), validation should occur against$dbForDatabasesto ensure compatibility with the target adapter's capabilities (e.g., vector support, spatial rules). Additionally,$dbForDatabasesis already initialized on line 572 but remains unused, suggesting this is the intended validation point.🔧 Proposed fix
- $this->dbForProject->checkAttribute($table, $column); + $dbForDatabases->checkAttribute($table, $column);
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 1539-1542: Remove the unused variable assignment $columnSize from
the code that reads column metadata (the block that assigns $columnStatus,
$columnType, $columnSize, $columnArray from $oldColumns[$columnIndex]);
specifically delete the line assigning $columnSize (or, if size is actually
needed, use $columnSize where appropriate), leaving only $columnStatus,
$columnType and $columnArray assignments to resolve the PHPMD unused-variable
warning.
| $columnStatus = $oldColumns[$columnIndex]['status']; | ||
| $columnType = $oldColumns[$columnIndex]['type']; | ||
| $columnSize = $oldColumns[$columnIndex]['size']; | ||
| $columnArray = $oldColumns[$columnIndex]['array'] ?? false; |
There was a problem hiding this comment.
Remove unused $columnSize.
$columnSize is assigned but never used (PHPMD).
🧹 Cleanup
- $columnSize = $oldColumns[$columnIndex]['size'];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $columnStatus = $oldColumns[$columnIndex]['status']; | |
| $columnType = $oldColumns[$columnIndex]['type']; | |
| $columnSize = $oldColumns[$columnIndex]['size']; | |
| $columnArray = $oldColumns[$columnIndex]['array'] ?? false; | |
| $columnStatus = $oldColumns[$columnIndex]['status']; | |
| $columnType = $oldColumns[$columnIndex]['type']; | |
| $columnArray = $oldColumns[$columnIndex]['array'] ?? false; |
🧰 Tools
🪛 PHPMD (2.15.0)
1541-1541: Avoid unused local variables such as '$columnSize'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In `@src/Migration/Destinations/Appwrite.php` around lines 1539 - 1542, Remove the
unused variable assignment $columnSize from the code that reads column metadata
(the block that assigns $columnStatus, $columnType, $columnSize, $columnArray
from $oldColumns[$columnIndex]); specifically delete the line assigning
$columnSize (or, if size is actually needed, use $columnSize where appropriate),
leaving only $columnStatus, $columnType and $columnArray assignments to resolve
the PHPMD unused-variable warning.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 811-812: The index document is using raw dates
($resource->getCreatedAt() / $resource->getUpdatedAt()) instead of the
previously normalized $createdAt and $updatedAt values; update the array entries
(where the index document is built in Appwrite.php) to use the normalized
$createdAt and $updatedAt variables instead of calling $resource->getCreatedAt()
and $resource->getUpdatedAt() so the stored datetimes match other create*
methods and maintain valid normalized formatting.
In `@src/Migration/Sources/Appwrite.php`:
- Around line 91-94: The PHPDoc for the property $getDatabasesDB uses a
non-existent type and invalid nullable syntax; update the docblock to reference
the correct fully-qualified document type and valid nullable form, e.g. change
the annotation to: `@var` callable(\Utopia\Database\Document|null $database):
\Utopia\Database\Document (or the correct fully-qualified return type used in
this project) so the parameter type is \Utopia\Database\Document|null and the
callable return type is fully qualified.
- Around line 644-665: The error uses the generic $resourceKey when constructing
the Exception inside the $handleExportEntityScopedResources closure; change it
to use the resolved resource type from the map (e.g. $entityResource['type'] or
the appropriate key in Resource::ENTITY_TYPE_RESOURCE_MAP) so the Exception
contains the specific resource name; update the Exception call inside the catch
block (where addError(...) is invoked) to pass the resolved resource type (from
$entityResource) instead of $resourceKey while keeping other fields
(Transfer::GROUP_DATABASES, message, code, previous) unchanged.
- Around line 893-901: The routing for column/attribute construction is
inconsistent: getEntity() returns Collection for both TYPE_DATABASE_VECTORDB and
TYPE_DATABASE_DOCUMENTSDB but the match only calls getAttribute(...) for
TYPE_DATABASE_VECTORDB and falls back to getColumn(...) for DocumentsDB; update
the match in the block that builds $col so DocumentsDB also uses
getAttribute($table, $column) (or otherwise branch by inspecting getEntity()
type) instead of defaulting to getColumn($table, $column) so Collection
instances produce Attribute objects consistently; modify the match to include
Resource::TYPE_DATABASE_DOCUMENTSDB to call getAttribute, keeping getColumn as
the default for Table-backed schemas.
🧹 Nitpick comments (3)
src/Migration/Sources/Appwrite.php (3)
1574-1582: DocumentsDB and VectorDB cases are identical — consolidate.Both
TYPE_DATABASE_DOCUMENTSDBandTYPE_DATABASE_VECTORDBreturnCollection::fromArray($entity). Merge them into a single arm.Proposed fix
public static function getEntity(string $databaseType, array $entity): Resource { switch ($databaseType) { case Resource::TYPE_DATABASE_DOCUMENTSDB: - return Collection::fromArray($entity); case Resource::TYPE_DATABASE_VECTORDB: return Collection::fromArray($entity); default: return Table::fromArray($entity); } }
1613-1623: Same consolidation opportunity ingetRecord().Both DocumentsDB and VectorDB cases return
Document::fromArray($record).Proposed fix
public static function getRecord(string $databaseType, array $record): Resource { switch ($databaseType) { case Resource::TYPE_DATABASE_DOCUMENTSDB: - return Document::fromArray($record); case Resource::TYPE_DATABASE_VECTORDB: return Document::fromArray($record); default: return Row::fromArray($record); } }
1625-1788:getColumn()andgetAttribute()are near-identical — consider extracting shared logic.These two large factory methods (~160 lines each) have the same structure and match arms, differing only in Column vs Attribute types and Table vs Collection parameter types. This is a significant amount of duplicated logic that could become a maintenance burden when adding new types.
Consider a shared private helper that takes a factory-class map or uses a common interface.
Also applies to: 1790-1952
| '$createdAt' => $resource->getCreatedAt(), | ||
| '$updatedAt' => $resource->getUpdatedAt(), |
There was a problem hiding this comment.
Index document uses raw dates instead of normalized values.
Lines 796–797 normalize $createdAt and $updatedAt, but lines 811–812 bypass them and use $resource->getCreatedAt() / $resource->getUpdatedAt() directly. This is inconsistent with all other create* methods and could produce invalid datetime formats in the stored document.
Proposed fix
- '$createdAt' => $resource->getCreatedAt(),
- '$updatedAt' => $resource->getUpdatedAt(),
+ '$createdAt' => $createdAt,
+ '$updatedAt' => $updatedAt,🤖 Prompt for AI Agents
In `@src/Migration/Destinations/Appwrite.php` around lines 811 - 812, The index
document is using raw dates ($resource->getCreatedAt() /
$resource->getUpdatedAt()) instead of the previously normalized $createdAt and
$updatedAt values; update the array entries (where the index document is built
in Appwrite.php) to use the normalized $createdAt and $updatedAt variables
instead of calling $resource->getCreatedAt() and $resource->getUpdatedAt() so
the stored datetimes match other create* methods and maintain valid normalized
formatting.
| /** | ||
| * @var callable(UtopiaDocument $database|null): UtopiaDatabase | ||
| */ | ||
| protected mixed $getDatabasesDB; |
There was a problem hiding this comment.
PHPDoc callable signature references non-existent type UtopiaDocument.
Line 92: callable(UtopiaDocument $database|null) — UtopiaDocument is not imported in this file. This should likely be \Utopia\Database\Document or simply documented differently. Also the |null syntax is not valid PHPDoc for a callable parameter.
🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite.php` around lines 91 - 94, The PHPDoc for the
property $getDatabasesDB uses a non-existent type and invalid nullable syntax;
update the docblock to reference the correct fully-qualified document type and
valid nullable form, e.g. change the annotation to: `@var`
callable(\Utopia\Database\Document|null $database): \Utopia\Database\Document
(or the correct fully-qualified return type used in this project) so the
parameter type is \Utopia\Database\Document|null and the callable return type is
fully qualified.
| $handleExportEntityScopedResources = function (string $resourceKey, callable $callback) use ($resources) { | ||
| foreach (Resource::ENTITY_TYPE_RESOURCE_MAP as $entityKey => $entityResource) { | ||
| try { | ||
| if (\in_array($entityResource[$resourceKey], $resources)) { | ||
| $callback($entityKey, $entityResource); | ||
| } | ||
| } catch (\Throwable $e) { | ||
| $this->addError( | ||
| new Exception( | ||
| $resourceKey, | ||
| Transfer::GROUP_DATABASES, | ||
| message: $e->getMessage(), | ||
| code: $e->getCode(), | ||
| previous: $e | ||
| ) | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
Error reporting uses generic key instead of specific resource type.
In the $handleExportEntityScopedResources closure, the Exception at line 653 uses $resourceKey (e.g., 'field', 'index', 'record') as the resource name. This should be the resolved resource type from the map to provide meaningful error context.
Proposed fix
} catch (\Throwable $e) {
$this->addError(
new Exception(
- $resourceKey,
+ $entityResource[$resourceKey],
Transfer::GROUP_DATABASES,
message: $e->getMessage(),
code: $e->getCode(),
previous: $e
)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $handleExportEntityScopedResources = function (string $resourceKey, callable $callback) use ($resources) { | |
| foreach (Resource::ENTITY_TYPE_RESOURCE_MAP as $entityKey => $entityResource) { | |
| try { | |
| if (\in_array($entityResource[$resourceKey], $resources)) { | |
| $callback($entityKey, $entityResource); | |
| } | |
| } catch (\Throwable $e) { | |
| $this->addError( | |
| new Exception( | |
| $resourceKey, | |
| Transfer::GROUP_DATABASES, | |
| message: $e->getMessage(), | |
| code: $e->getCode(), | |
| previous: $e | |
| ) | |
| ); | |
| return; | |
| } | |
| return false; | |
| } | |
| } | |
| return true; | |
| }; | |
| $handleExportEntityScopedResources = function (string $resourceKey, callable $callback) use ($resources) { | |
| foreach (Resource::ENTITY_TYPE_RESOURCE_MAP as $entityKey => $entityResource) { | |
| try { | |
| if (\in_array($entityResource[$resourceKey], $resources)) { | |
| $callback($entityKey, $entityResource); | |
| } | |
| } catch (\Throwable $e) { | |
| $this->addError( | |
| new Exception( | |
| $entityResource[$resourceKey], | |
| Transfer::GROUP_DATABASES, | |
| message: $e->getMessage(), | |
| code: $e->getCode(), | |
| previous: $e | |
| ) | |
| ); | |
| return false; | |
| } | |
| } | |
| return true; | |
| }; |
🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite.php` around lines 644 - 665, The error uses
the generic $resourceKey when constructing the Exception inside the
$handleExportEntityScopedResources closure; change it to use the resolved
resource type from the map (e.g. $entityResource['type'] or the appropriate key
in Resource::ENTITY_TYPE_RESOURCE_MAP) so the Exception contains the specific
resource name; update the Exception call inside the catch block (where
addError(...) is invoked) to pass the resolved resource type (from
$entityResource) instead of $resourceKey while keeping other fields
(Transfer::GROUP_DATABASES, message, code, previous) unchanged.
| } | ||
|
|
||
| switch ($column['type']) { | ||
| case Column::TYPE_STRING: | ||
| $col = match ($column['format'] ?? '') { | ||
| Column::TYPE_EMAIL => new Email( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| size: $column['size'] ?? 254, | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ), | ||
| Column::TYPE_ENUM => new Enum( | ||
| $column['key'], | ||
| $table, | ||
| elements: $column['elements'], | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| size: $column['size'] ?? UtopiaDatabase::LENGTH_KEY, | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ), | ||
| Column::TYPE_URL => new URL( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| size: $column['size'] ?? 2000, | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ), | ||
| Column::TYPE_IP => new IP( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| size: $column['size'] ?? 39, | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ), | ||
| default => new Text( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| size: $column['size'] ?? 0, | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ), | ||
| }; | ||
|
|
||
| break; | ||
| case Column::TYPE_BOOLEAN: | ||
| $col = new Boolean( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| case Column::TYPE_INTEGER: | ||
| $col = new Integer( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| min: $column['min'] ?? null, | ||
| max: $column['max'] ?? null, | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| case Column::TYPE_FLOAT: | ||
| $col = new Decimal( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| min: $column['min'] ?? null, | ||
| max: $column['max'] ?? null, | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| case Column::TYPE_RELATIONSHIP: | ||
| $col = new Relationship( | ||
| $column['key'], | ||
| $table, | ||
| relatedTable: $column['relatedTable'] ?? $column['relatedCollection'], | ||
| relationType: $column['relationType'], | ||
| twoWay: $column['twoWay'], | ||
| twoWayKey: $column['twoWayKey'], | ||
| onDelete: $column['onDelete'], | ||
| side: $column['side'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| case Column::TYPE_DATETIME: | ||
| $col = new DateTime( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| array: $column['array'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| case Column::TYPE_POINT: | ||
| $col = new Point( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| case Column::TYPE_LINE: | ||
| $col = new Line( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| case Column::TYPE_POLYGON: | ||
| $col = new Polygon( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| break; | ||
| } | ||
|
|
||
| if (!isset($col)) { | ||
| throw new Exception( | ||
| resourceName: Resource::TYPE_COLUMN, | ||
| resourceGroup: Transfer::GROUP_DATABASES, | ||
| resourceId: $column['$id'], | ||
| message: 'Unknown column type: ' . $column['type'] | ||
| ); | ||
| } | ||
| /** @var Table $table */ | ||
| $col = match($table->getDatabase()->getType()) { | ||
| Resource::TYPE_DATABASE_VECTORDB => self::getAttribute($table, $column), | ||
| default => self::getColumn($table, $column), | ||
| }; | ||
|
|
||
| $columns[] = $col; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual code at lines 893-901
echo "=== Code at lines 893-901 ==="
sed -n '893,901p' src/Migration/Sources/Appwrite.php
# Check Collection and Table class definitions
echo -e "\n=== Collection class definition ==="
rg -n "class Collection" --type php -A 5
echo -e "\n=== Table class definition ==="
rg -n "class Table" --type php -A 5
# Check getAttribute and getColumn signatures
echo -e "\n=== getAttribute method ==="
rg -n "public static function getAttribute" --type php -A 3
echo -e "\n=== getColumn method ==="
rg -n "public static function getColumn" --type php -A 3
# Check getEntity method to see what it returns for different DB types
echo -e "\n=== getEntity method ==="
rg -n "private function getEntity" --type php -B 2 -A 10
# Check the database type constants
echo -e "\n=== Database type constants ==="
rg -n "TYPE_DATABASE_" --type phpRepository: utopia-php/migration
Length of output: 6202
🏁 Script executed:
#!/bin/bash
# Get broader context around the match statement
echo "=== Context around lines 880-910 ==="
sed -n '880,910p' src/Migration/Sources/Appwrite.php
# Also check if there's a method that processes columns and what it gets from getEntity
echo -e "\n=== Context around line 893-901 with method signature ==="
sed -n '860,920p' src/Migration/Sources/Appwrite.php
# Check what getEntity returns for DocumentsDB and VectorDB
echo -e "\n=== Look for where entities are fetched ==="
rg -n "getEntity|->get\(" src/Migration/Sources/Appwrite.php --type php -B 2 -A 2 | head -60Repository: utopia-php/migration
Length of output: 5335
🏁 Script executed:
#!/bin/bash
# Find the getEntity method
echo "=== getEntity method definition ==="
rg -n "public static function getEntity" --type php -A 15
# Also check if there are any calls to getEntity with database type routing
echo -e "\n=== getEntity calls with database type context ==="
rg -n "getEntity.*TYPE_DATABASE" --type php -B 3 -A 3Repository: utopia-php/migration
Length of output: 1313
🏁 Script executed:
#!/bin/bash
# Get full getAttribute implementation
echo "=== getAttribute implementation ==="
sed -n '1790,1850p' src/Migration/Sources/Appwrite.php
# Get full getColumn implementation
echo -e "\n=== getColumn implementation ==="
sed -n '1625,1685p' src/Migration/Sources/Appwrite.php
# Check if getAttribute is used elsewhere with Collection
echo -e "\n=== getAttribute usage in codebase ==="
rg -n "getAttribute\(" src/Migration/Sources/Appwrite.php --type php | head -20Repository: utopia-php/migration
Length of output: 5895
The type mismatch claim is incorrect — no TypeError will occur at runtime.
Collection extends Table, so passing a Collection instance to getColumn(Table $table, ...) is valid. However, there is a semantic routing inconsistency: both TYPE_DATABASE_DOCUMENTSDB and TYPE_DATABASE_VECTORDB return Collection entities from getEntity() (lines 1576–1578), yet only VectorDB routes to getAttribute(), which is designed for Collection. DocumentsDB falls through to the default case, calling getColumn(Table, ...) instead, creating Column objects for data that semantically represents Attributes. Consider aligning the routing:
Suggested fix
/** `@var` Table $table */
$col = match($table->getDatabase()->getType()) {
Resource::TYPE_DATABASE_VECTORDB => self::getAttribute($table, $column),
+ Resource::TYPE_DATABASE_DOCUMENTSDB => self::getAttribute($table, $column),
default => self::getColumn($table, $column),
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } | |
| switch ($column['type']) { | |
| case Column::TYPE_STRING: | |
| $col = match ($column['format'] ?? '') { | |
| Column::TYPE_EMAIL => new Email( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| size: $column['size'] ?? 254, | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ), | |
| Column::TYPE_ENUM => new Enum( | |
| $column['key'], | |
| $table, | |
| elements: $column['elements'], | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| size: $column['size'] ?? UtopiaDatabase::LENGTH_KEY, | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ), | |
| Column::TYPE_URL => new URL( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| size: $column['size'] ?? 2000, | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ), | |
| Column::TYPE_IP => new IP( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| size: $column['size'] ?? 39, | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ), | |
| default => new Text( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| size: $column['size'] ?? 0, | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ), | |
| }; | |
| break; | |
| case Column::TYPE_BOOLEAN: | |
| $col = new Boolean( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| case Column::TYPE_INTEGER: | |
| $col = new Integer( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| min: $column['min'] ?? null, | |
| max: $column['max'] ?? null, | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| case Column::TYPE_FLOAT: | |
| $col = new Decimal( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| min: $column['min'] ?? null, | |
| max: $column['max'] ?? null, | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| case Column::TYPE_RELATIONSHIP: | |
| $col = new Relationship( | |
| $column['key'], | |
| $table, | |
| relatedTable: $column['relatedTable'] ?? $column['relatedCollection'], | |
| relationType: $column['relationType'], | |
| twoWay: $column['twoWay'], | |
| twoWayKey: $column['twoWayKey'], | |
| onDelete: $column['onDelete'], | |
| side: $column['side'], | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| case Column::TYPE_DATETIME: | |
| $col = new DateTime( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| array: $column['array'], | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| case Column::TYPE_POINT: | |
| $col = new Point( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| case Column::TYPE_LINE: | |
| $col = new Line( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| case Column::TYPE_POLYGON: | |
| $col = new Polygon( | |
| $column['key'], | |
| $table, | |
| required: $column['required'], | |
| default: $column['default'], | |
| createdAt: $column['$createdAt'] ?? '', | |
| updatedAt: $column['$updatedAt'] ?? '', | |
| ); | |
| break; | |
| } | |
| if (!isset($col)) { | |
| throw new Exception( | |
| resourceName: Resource::TYPE_COLUMN, | |
| resourceGroup: Transfer::GROUP_DATABASES, | |
| resourceId: $column['$id'], | |
| message: 'Unknown column type: ' . $column['type'] | |
| ); | |
| } | |
| /** @var Table $table */ | |
| $col = match($table->getDatabase()->getType()) { | |
| Resource::TYPE_DATABASE_VECTORDB => self::getAttribute($table, $column), | |
| default => self::getColumn($table, $column), | |
| }; | |
| $columns[] = $col; | |
| /** `@var` Table $table */ | |
| $col = match($table->getDatabase()->getType()) { | |
| Resource::TYPE_DATABASE_VECTORDB => self::getAttribute($table, $column), | |
| Resource::TYPE_DATABASE_DOCUMENTSDB => self::getAttribute($table, $column), | |
| default => self::getColumn($table, $column), | |
| }; | |
| $columns[] = $col; |
🤖 Prompt for AI Agents
In `@src/Migration/Sources/Appwrite.php` around lines 893 - 901, The routing for
column/attribute construction is inconsistent: getEntity() returns Collection
for both TYPE_DATABASE_VECTORDB and TYPE_DATABASE_DOCUMENTSDB but the match only
calls getAttribute(...) for TYPE_DATABASE_VECTORDB and falls back to
getColumn(...) for DocumentsDB; update the match in the block that builds $col
so DocumentsDB also uses getAttribute($table, $column) (or otherwise branch by
inspecting getEntity() type) instead of defaulting to getColumn($table, $column)
so Collection instances produce Attribute objects consistently; modify the match
to include Resource::TYPE_DATABASE_DOCUMENTSDB to call getAttribute, keeping
getColumn as the default for Table-backed schemas.
There was a problem hiding this comment.
so documentsdb will not be present here as there is no attribute support in documentsdb
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for DocumentsDB attribute support references
echo "=== DocumentsDB class and attribute-related code ==="
rg -n "DocumentsDB" --type php -B 2 -A 5
echo -e "\n=== Check exportFields method and its callers ==="
rg -n "exportFields|handleExportEntityScopedResources" --type php -B 3 -A 3
echo -e "\n=== Check DATABASE_TYPE_RESOURCE_MAP for field mappings ==="
rg -n "DATABASE_TYPE_RESOURCE_MAP" --type php -B 2 -A 10
echo -e "\n=== Check if DocumentsDB uses attributes or columns ==="
rg -n "TYPE_DATABASE_DOCUMENTSDB.*attribute|TYPE_DATABASE_DOCUMENTSDB.*column" --type php -i
Length of output: 12707
@ArnabChatterjee20k, you're absolutely right! I found the evidence in the codebase:
-
In
src/Migration/Resource.php(lines 108-109), there's a comment on the DocumentsDB field mapping:// HACK: not required in documentsdb but adding it for consistency in the db reader(not gonna impact) 'field' => self::TYPE_ATTRIBUTE,
-
In
src/Migration/Destinations/Appwrite.php(lines 473-474), DocumentsDB explicitly skips columns:if ($resource->getTable()->getDatabase()->getType() === Resource::TYPE_DATABASE_DOCUMENTSDB) { $resource->setStatus(Resource::STATUS_SKIPPED, 'Columns not supported for DocumentsDB');
Since DocumentsDB doesn't support attributes/columns, DocumentsDB collections won't have any attributes to export, so they'll never reach the exportFields() code path. The match statement correctly only needs to handle VectorDB → getAttribute() routing, with the default handling regular Table → getColumn().
CodeRabbit's suggestion to add TYPE_DATABASE_DOCUMENTSDB to the match was based on structural symmetry but isn't needed given the actual runtime behavior. Thanks for the clarification!
🧬 Code Graph Analysis Results
-
src/Migration/Sources/Appwrite/Reader/API.php
-
Summary: report(resources, report, resourceIds)
Description: Aggregates counts for Appwrite resources (databases, tables/collections, rows/documents, columns/attributes, indexes) based on the provided resources filter and optional resourceIds. Iterates databases and their tables to accumulate totals for each resource type. Returns null on success; throws if issues occur. Parameters:- resources: array of resource types to report on.
- report: array passed by reference to accumulate counts per resource type.
- resourceIds: optional map of resource type => list of IDs to filter on.
Returns: mixed (typically null on completion, or throws on error).
Exceptions/Errors: throws if there are errors while querying the Appwrite API or database, including permission/scope errors.
-
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: report method (high-level behavior described above).
-
Summary: listDatabases(queries)
Description: Returns an array of databases for the API reader, applying optional query constraints.
Returns: array of databases. -
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: listDatabases(array $queries = []): array
-
Summary: listTables(Database $resource, queries)
Description: Given a Database resource, returns the list of its tables/collections applying provided queries. -
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: listTables(Database $resource, array $queries = []): array
-
Summary: listColumns(Table $resource, queries)
Description: Given a Table/Collection resource, returns its columns/attributes applying provided queries. -
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: listColumns(Table $resource, array $queries = []): array
-
Summary: listIndexes(Table $resource, queries)
Description: Given a Table/Collection resource, returns its indexes applying provided queries. -
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: listIndexes(Table $resource, array $queries = []): array
-
Summary: listRows(Table $resource, queries) and getRow(Table $resource, rowId, queries)
Description: Retrieve rows/documents for a table and a single row by ID, respectively, using provided queries. -
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: listRows(Table $resource, array $queries = []): array
-
Snippet location: getRow(Table $resource, string $rowId, array $queries = []): array
-
Summary: querySelect(columns), queryEqual(column, values), queryCursorAfter(resource|Resource), queryLimit(limit)
Description: Helpers to build query objects for selecting columns, equality filters, cursorAfter semantics, and limit. -
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: querySelect(array $columns): mixed
-
Snippet location: queryEqual(string $column, array $values): mixed
-
Snippet location: queryCursorAfter(Resource|string $resource): mixed
-
Snippet location: queryLimit(int $limit): mixed
-
Summary: getSupportForAttributes()
Description: Indicates whether the underlying API supports retrieving attributes/columns for the target DB type. -
File: src/Migration/Sources/Appwrite/Reader/API.php
-
Snippet location: getSupportForAttributes(): bool
-
-
src/Migration/Sources/Appwrite.php
- Summary: getDatabase(string $databaseType, array $database), getEntity(string $databaseType, array $entity), getRecord(string $databaseType, array $record), getColumn(Table $table, mixed $column)
Description: Factory/symmetry methods that map a given database type to a corresponding resource implementation:- For DocumentsDB (Resource::TYPE_DATABASE_DOCUMENTSDB) use DocumentsDB::fromArray(...)
- For VectorDB (Resource::TYPE_DATABASE_VECTORDB) use VectorDB::fromArray(...)
- Default falls back to Database::fromArray(...)
- getEntity maps to Collection::fromArray(...) for DocumentsDB/VectorDB or Table::fromArray(...) otherwise
- getRecord maps to Document::fromArray(...) for DocumentsDB/VectorDB or Row::fromArray(...) otherwise
- getColumn builds a Column or Attribute instance depending on the column type
Returns: respective Resource instances (Database/Collection/Table, Document/Row, etc.)
- File: src/Migration/Sources/Appwrite.php
- Snippet location: public static function getDatabase(string $databaseType, array $database): Resource
- Snippet location: public static function getEntity(string $databaseType, array $entity): Resource
- Snippet location: public static function getRecord(string $databaseType, array $record): Resource
- Snippet location: public static function getColumn(Table $table, mixed $column): Column
- Summary: getDatabase(string $databaseType, array $database), getEntity(string $databaseType, array $entity), getRecord(string $databaseType, array $record), getColumn(Table $table, mixed $column)
Notes:
- The Appwrite reader/API integration includes handling for DocumentsDB and VectorDB as database types. The user comment about “documentsdb will not be present here as there is no attribute support in documentsdb” is consistent with the architecture seen in the Appwrite-related code paths, where:
- DocumentsDB is treated as a separate database type and typically has limited attribute/field support compared to legacy/VectorDB paths.
- Attribute/Column handling for DocumentsDB is distinguished from other databases (as reflected by the getColumn/getAttribute dispatch logic).
- If you need exact line references for each snippet above in your review, please specify and I can pin each to the closest line number in the provided repository context.
✏️ Learnings added
Learnt from: ArnabChatterjee20k
Repo: utopia-php/migration PR: 139
File: src/Migration/Sources/Appwrite.php:893-901
Timestamp: 2026-02-10T05:49:12.953Z
Learning: DocumentsDB (Resource::TYPE_DATABASE_DOCUMENTSDB) does not support attributes or columns. The DATABASE_TYPE_RESOURCE_MAP includes a 'field' => TYPE_ATTRIBUTE mapping for DocumentsDB only for structural consistency in the reader, but in practice DocumentsDB collections will not have attributes and the destination explicitly skips them with the message 'Columns not supported for DocumentsDB'.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Sources/Appwrite.php:843-851
Timestamp: 2025-06-28T09:47:08.333Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the class constructors and method parameters use the new terminology (like "relatedTable"), but the underlying data structures and API responses still use the legacy keys (like "relatedCollection"). This is an intentional design pattern to allow gradual migration while maintaining compatibility with existing data sources.
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Sources/Supabase.php:300-308
Timestamp: 2025-06-28T09:47:58.757Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the user ItzNotABug prefers to keep the existing query logic unchanged even if it becomes semantically incorrect with the new naming. The focus is purely on resource type renaming, not on fixing logical issues that become apparent after the terminology change.
Learnt from: ArnabChatterjee20k
Repo: utopia-php/migration PR: 114
File: src/Migration/Resources/Database/Attribute/Decimal.php:10-38
Timestamp: 2025-11-20T13:07:39.293Z
Learning: In the utopia-php/migration codebase, the user ArnabChatterjee20k maintains consistency between parallel implementations: Attribute classes (e.g., Attribute/Decimal.php) mirror their corresponding Column classes (e.g., Columns/Decimal.php) in constructor defaults and behavior, even when design decisions might be questioned.
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Resources/Database/Columns/Relationship.php:86-89
Timestamp: 2025-06-28T09:45:57.650Z
Learning: In the utopia-php/migration codebase Relationship column class, the `getRelatedTable()` method intentionally returns `$this->options['relatedCollection']` (not `relatedTable`) because the underlying API still uses "collection" terminology, even though the internal codebase has been refactored to use "table" terminology.
Learnt from: ArnabChatterjee20k
Repo: utopia-php/migration PR: 114
File: src/Migration/Resources/Database/Collection.php:14-37
Timestamp: 2025-11-20T13:12:43.482Z
Learning: In the utopia-php/migration codebase, when Collection::fromArray is called, the input array is guaranteed to contain both the 'database' key and the nested 'type' key ($array['database']['type']), so defensive guards for these keys are not necessary.
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Resources/Database/Row.php:60-60
Timestamp: 2025-06-28T09:45:36.026Z
Learning: In the utopia-php/migration codebase, the `fromArray` method is not used on Row objects, so mismatches between `jsonSerialize()` output keys and `fromArray()` input expectations for Row class are not problematic.
Learnt from: premtsd-code
Repo: utopia-php/migration PR: 142
File: src/Migration/Destinations/Appwrite.php:1542-1635
Timestamp: 2026-02-03T15:50:17.680Z
Learning: This repository targets Appwrite PHP SDK v19, and the migration code relies on enum classes Appwrite\Enums\Framework, Appwrite\Enums\BuildRuntime, and Appwrite\Enums\Adapter being present in vendor/appwrite/appwrite/src/Appwrite/Enums. The Sites mappings in src/Migration/Destinations/Appwrite.php intentionally follow the same enum pattern used for Functions.
1 participant
Multi-type db was reverted
So re-reverting the commits
Fix Double-Counting of
row/documentMigration Status CountersProblem
E2E test
testAppwriteMigrationMixedDatabaseswas failing with incorrectstatusCounters(e.g.,row.success = 2when only 1 row was migrated, orpendinggoing negative). Debug logs fromCache::add()showed the same row/document IDs being processed twice within a single migration run.Root Cause: Source + Destination Both Updated Cache for Rows/Documents
Rows and documents are special-cased in
Cacheto store aggregated counters per status (as strings), not per-resource objects. But they were being counted twice:cache->update($responseResource)during importcache->addAll($prunedResources)after destination callback returnsFor
rowanddocument,Cache::update()delegates toCache::add(), which increments the aggregated counter. So if source also adds them, it becomes double counting.Evidence in Code (Before Fix)
Source adds exported resources to cache after destination callback:
Destination updates cache for every imported resource:
Rows/documents are aggregated in cache, and
update()callsadd()for them:Solution
Skip source-side cache population for
row/document- destination import is already the single source of truth for these counters:Also clamp row/document
pendingsubtraction to prevent negative values if counts drift:Summary by CodeRabbit
New Features
Changes