documents db + vector db migration #114
Conversation
WalkthroughAdds per-project and per-database contexts to source and destination Appwrite adapters via callables, replacing many global Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Files/areas to pay extra attention to:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
ArnabChatterjee20k
changed the title
separated database object into dbForProject to interact with project metadata and dbForDatabase to work with the targetted database
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Migration/Destinations/Appwrite.php (1)
411-428: Consider extracting repeated database/table lookup pattern.The sequence (fetch database document → fetch table document → invoke factory) appears in
createTable,createColumn,createIndex, andcreateRow. Extracting this into a helper method would reduce duplication and centralize error handling.Example helper:
private function getDbForDatabase(Database|string $databaseId): array { $database = $this->dbForProject->getDocument( 'databases', is_string($databaseId) ? $databaseId : $databaseId->getId() ); if ($database->isEmpty()) { throw new Exception( message: 'Database not found', ); } $dbForDatabase = call_user_func( $this->getDatabaseDB, $database->getAttribute('database') ); return [$database, $dbForDatabase]; }Then replace repeated blocks with:
[$database, $dbForDatabase] = $this->getDbForDatabase($resource->getDatabase()->getId());Also applies to: 533-577, 734-877, 994-998
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Migration/Destinations/Appwrite.php(32 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
🧬 Code graph analysis (1)
src/Migration/Destinations/Appwrite.php (4)
src/Migration/Resources/Database/Database.php (2)
__construct(21-32)getDatabase(99-102)src/Migration/Sources/Appwrite.php (1)
__construct(70-103)src/Migration/Resources/Database/Table.php (1)
getDatabase(91-94)bin/MigrationCLI.php (1)
getDatabase(265-399)
🔇 Additional comments (3)
src/Migration/Destinations/Appwrite.php (3)
60-63: Property type is correct for callable storage.The
mixedtype is appropriate here sincecallablecannot be used as a property type in PHP 8.1+. The docblock provides the precise signature.
78-101: Constructor correctly implements dual-handle pattern.The refactoring from a single database handle to separate
$dbForProject(promoted property) and a factory callable is architecturally sound:
- Project-level metadata operations use
$dbForProject- Database-specific operations obtain handles on-demand via the factory
- The callable cannot be promoted due to PHP type system limitations, so line 100 assignment is necessary
341-351: New 'database' field correctly propagates database context.Adding the 'database' field (line 350) enables downstream operations to obtain the appropriate database handle via the factory. This aligns with the broader PR objective to support per-database routing.
| } | ||
|
|
||
| $table = $this->database->createDocument('database_' . $database->getSequence(), new UtopiaDocument([ | ||
| $dbForDatabase = call_user_func($this->getDatabaseDB, $database->getAttribute('database')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify factory handles missing or invalid database identifiers.
The factory is invoked with $database->getAttribute('database') without checking if the attribute exists or is valid. If getAttribute('database') returns null, an empty string, or an invalid identifier, the factory might fail unexpectedly.
Run the following script to check if there are safeguards elsewhere in the codebase for the 'database' attribute:
Also applies to: 533-533, 734-734, 994-994
🏁 Script executed:
#!/bin/bash
# Description: Verify that the 'database' attribute is always set and validated.
# Search for Database resource creation to see if 'database' is always provided
rg -nP -A 5 'new Database\(' --type=php
# Search for validation or null checks on the 'database' attribute
rg -nP "getAttribute\('database'\)" --type=php -C 3Length of output: 8320
Validate database identifier before invoking factory
Ensure $database->getAttribute('database') returns a non-null, valid identifier before calling $this->getDatabaseDB; add a guard or fallback to avoid runtime failures. Applies to lines 411, 533, 734 and 994 in src/Migration/Destinations/Appwrite.php.
🤖 Prompt for AI Agents
In src/Migration/Destinations/Appwrite.php around lines 411, 533, 734 and 994,
the calls pass $database->getAttribute('database') directly into the factory
($this->getDatabaseDB) without validating the identifier; add a guard that reads
the attribute into a local variable, verify it's non-null, a non-empty string
(or valid expected type/format), and only call
call_user_func($this->getDatabaseDB, $id) when valid; if invalid, handle
explicitly (throw a clear InvalidArgumentException or log and skip/fallback to a
default DB) so you avoid runtime failures and make the error path deterministic.
There was a problem hiding this comment.
Actionable comments posted: 2
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)
82-109: Fix parameter order: required callable after optional triggers PHP 8.1 deprecation.PHP 8.1 emits a deprecation (and future versions will reject it) when a required parameter (
$getDatabaseDB) follows an optional one ($dbForProject = null). The pipeline already fails on this. Reorder the parameters or give the callable a default so that required parameters precede optional ones.Apply this diff:
- protected ?UtopiaDatabase $dbForProject = null, - callable $getDatabaseDB + protected ?UtopiaDatabase $dbForProject = null, + ?callable $getDatabaseDB = nulland keep the runtime validation in Lines 99‑105 to enforce the presence of the callable only for
SOURCE_DATABASE, adjusting property/default handling accordingly.
🧹 Nitpick comments (2)
src/Migration/Resources/Database/Document.php (1)
1-13: Add#[Override]attribute for consistency.The Document class extends Row and returns
Resource::TYPE_DOCUMENTfromgetName(), providing backward compatibility for the legacy Document terminology. However, it's missing the#[Override]attribute that Collection.php uses for the same pattern.For consistency with Collection.php, consider adding the
#[Override]attribute:+use Override; + use Utopia\Migration\Resource; class Document extends Row { + #[Override] public static function getName(): string { return Resource::TYPE_DOCUMENT; } }Based on learnings.
src/Migration/Resources/Database/DocumentsDB.php (1)
1-13: Add#[Override]attribute for consistency.The DocumentsDB class extends Database and returns
Resource::TYPE_DOCUMENTSDB_DATABASEfromgetName(). For consistency with Collection.php, consider adding the#[Override]attribute to make the override explicit.Apply this diff:
+use Override; + use Utopia\Migration\Resource; class DocumentsDB extends Database { + #[Override] public static function getName(): string { return Resource::TYPE_DOCUMENTSDB_DATABASE; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
composer.json(1 hunks)src/Migration/Destinations/Appwrite.php(33 hunks)src/Migration/Resource.php(2 hunks)src/Migration/Resources/Database/Collection.php(1 hunks)src/Migration/Resources/Database/Document.php(1 hunks)src/Migration/Resources/Database/DocumentsDB.php(1 hunks)src/Migration/Sources/Appwrite.php(16 hunks)src/Migration/Sources/Appwrite/Reader.php(1 hunks)src/Migration/Sources/Appwrite/Reader/API.php(1 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Migration/Destinations/Appwrite.php
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-28T09:47:08.333Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Resources/Database/Collection.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:47:58.757Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Resources/Database/Collection.php
🧬 Code graph analysis (7)
src/Migration/Sources/Appwrite/Reader.php (2)
src/Migration/Sources/Appwrite/Reader/API.php (1)
getSupportForAttributes(244-247)src/Migration/Sources/Appwrite/Reader/Database.php (1)
getSupportForAttributes(416-419)
src/Migration/Sources/Appwrite/Reader/Database.php (2)
src/Migration/Sources/Appwrite/Reader.php (1)
getSupportForAttributes(109-109)src/Migration/Sources/Appwrite/Reader/API.php (1)
getSupportForAttributes(244-247)
src/Migration/Resources/Database/Collection.php (2)
src/Migration/Resource.php (2)
Resource(5-239)getName(111-111)src/Migration/Resources/Database/Table.php (1)
Table(8-110)
src/Migration/Sources/Appwrite/Reader/API.php (2)
src/Migration/Sources/Appwrite/Reader.php (1)
getSupportForAttributes(109-109)src/Migration/Sources/Appwrite/Reader/Database.php (1)
getSupportForAttributes(416-419)
src/Migration/Resources/Database/Document.php (2)
src/Migration/Resource.php (2)
Resource(5-239)getName(111-111)src/Migration/Resources/Database/Row.php (1)
Row(8-98)
src/Migration/Resources/Database/DocumentsDB.php (1)
src/Migration/Resource.php (2)
Resource(5-239)getName(111-111)
src/Migration/Sources/Appwrite.php (9)
src/Migration/Sources/Appwrite/Reader/Database.php (13)
Database(21-431)report(27-109)queryLimit(411-414)queryEqual(369-372)queryCursorAfter(380-409)listDatabases(111-114)listTables(116-155)listColumns(157-217)listIndexes(219-264)getSupportForAttributes(416-419)querySelect(359-362)listRows(266-314)getRow(316-353)src/Migration/Resources/Database/Database.php (6)
Database(19-103)getName(74-77)getDatabase(99-102)getDatabaseName(84-87)getType(94-97)fromArray(45-57)src/Migration/Resources/Database/Document.php (2)
Document(7-13)getName(9-12)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/DocumentsDB.php (1)
DocumentsDB(7-13)src/Migration/Sources/Appwrite/Reader.php (12)
report(21-21)queryLimit(108-108)queryEqual(92-92)queryCursorAfter(100-100)listDatabases(29-29)listTables(38-38)listColumns(47-47)listIndexes(56-56)getSupportForAttributes(109-109)querySelect(83-83)listRows(65-65)getRow(75-75)src/Migration/Sources/Appwrite/Reader/API.php (12)
report(28-118)queryLimit(239-242)queryEqual(221-224)queryCursorAfter(230-237)listDatabases(123-126)listTables(131-138)listColumns(146-154)listIndexes(162-170)getSupportForAttributes(244-247)querySelect(211-214)listRows(179-187)getRow(196-205)src/Migration/Resources/Database/Table.php (4)
getDatabase(91-94)Table(8-110)getTableName(96-99)getRowSecurity(101-104)src/Migration/Resources/Database/Row.php (3)
getTable(86-89)fromArray(53-61)Row(8-98)
🪛 GitHub Actions: CodeQL
src/Migration/Sources/Appwrite.php
[error] 83-83: Deprecated in PHP 8.1: Required parameter $getDatabaseDB follows optional parameter $dbForProject.
🪛 PHPMD (2.15.0)
src/Migration/Sources/Appwrite.php
123-132: Avoid unused private methods such as 'createReaderForDatabase'. (undefined)
(UnusedPrivateMethod)
🔇 Additional comments (7)
src/Migration/Sources/Appwrite/Reader.php (1)
109-110: LGTM: Interface expansion for attribute support.The addition of
getSupportForAttributes(): boolexpands the Reader interface to expose attribute support capabilities. This is a breaking change, but the context shows that both implementations (API and Database readers) have been updated accordingly.src/Migration/Resources/Database/Collection.php (1)
1-15: LGTM: Legacy resource type support.The Collection class extends Table and returns
Resource::TYPE_COLLECTIONfromgetName(), providing backward compatibility for the legacy Collection terminology. This aligns with the codebase's pattern of supporting both current (Table/Column/Row) and legacy (Collection/Attribute/Document) terminology during the migration period.Based on learnings.
src/Migration/Resource.php (2)
33-34: LGTM: New resource type constant added.The
TYPE_DOCUMENTSDB_DATABASEconstant follows the existing naming pattern and is properly defined.
75-75: LGTM: Resource type registered.The new
TYPE_DOCUMENTSDB_DATABASEconstant is correctly added to theALL_RESOURCESarray, ensuring it's recognized throughout the system.src/Migration/Sources/Appwrite/Reader/API.php (1)
244-247: LGTM: Interface implementation for attribute support.The
getSupportForAttributes()method correctly implements the Reader interface, returningtrueto indicate that the API reader supports attribute operations.src/Migration/Sources/Appwrite/Reader/Database.php (1)
416-419: LGTM: Capability delegation to adapter.The
getSupportForAttributes()method correctly implements the Reader interface by delegating to the underlying database adapter'sgetSupportForAttributes()method, allowing adapter-specific capability reporting.composer.json (1)
29-29: Revert to stable version before merging
Confirm thefeat-documentsdbbranch has been merged and tagged as a stable release (e.g., 1.4.5), then update composer.json to reference that version instead of the dev alias.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/Migration/Resources/Database/Database.php (1)
64-71: Include the new$databasefield in serialization.We now persist a
$databaseidentifier, butjsonSerialize()still omits it. Any cache/export round-trip will drop the value, and subsequentfromArray()calls will fail the new constructor argument. Please add'database' => $this->databaseto the payload.Apply this diff:
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/Table.php (1)
50-61: Avoidnew static()to satisfy CodeQL.The late-static binding here causes the same “unsafe dynamic object instantiation” failure flagged by the CodeQL pipeline. Please revert to a concrete instantiation (e.g.
new self(...)) and let subclasses such asCollectionoverridefromArray()explicitly, or introduce a small factory that instantiates only vetted subclasses. This removes the security warning while still supporting the new resources.src/Migration/Resources/Database/Row.php (1)
53-60: Replacenew static()to clear the CodeQL failure.Just like the other resources, this
new staticis tripping the pipeline’s security rule. Please refactor to a concrete instantiation (new self) and, if needed, overridefromArray()in subclasses such asDocumentso they construct their own instances. That keeps the behaviour while satisfying CodeQL.src/Migration/Sources/Appwrite/Reader/Database.php (1)
1-1: Fix PSR-12 formatting violation.The linter detected a
single_space_around_constructviolation. Ensure there's exactly one space around language constructs (e.g.,if,foreach,while).src/Migration/Destinations/Appwrite.php (1)
1-1: Fix PSR-12 formatting violation.The linter detected a
single_space_around_constructviolation. Ensure proper spacing around language constructs.src/Migration/Sources/Appwrite.php (1)
1-1: Fix PSR-12 formatting violation.The linter detected a
single_space_around_constructviolation. Ensure proper spacing around language constructs.
♻️ Duplicate comments (3)
src/Migration/Destinations/Appwrite.php (1)
413-413: Validate database identifier before invoking factory.As flagged in a previous review,
$database->getAttribute('database')is passed directly to$this->getDatabaseDBwithout validation. If the attribute is missing, null, or empty, the factory might fail unexpectedly.Refer to the past review comment for the recommended fix: extract the attribute, validate it's non-null and non-empty, then call the factory. Apply this pattern at lines 413, 541, 742, and 977.
Also applies to: 541-541, 742-742, 977-977
src/Migration/Sources/Appwrite.php (2)
791-804: Pass database type, not database name, to getEntity().As flagged in a previous review,
getEntity()expects the database type (e.g.,Resource::TYPE_DOCUMENTSDB_DATABASE) to decide betweenCollection::fromArray()andTable::fromArray(), but here we pass$databaseName. This causes Document DB tables to be emitted asTableresources.Apply this diff:
- $newTable = self::getEntity($databaseName, [ + $newTable = self::getEntity($database->getType(), [Refer to the past review comment for additional context.
1189-1205: Pass database type, not database name, to getRecord().Similar to the issue above,
getRecord()expects the database type to distinguish betweenDocument::fromArray()andRow::fromArray(), but here we pass$table->getDatabase()->getDatabaseName(). This causes Documents DB rows to be emitted asRowresources.Apply this diff:
- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ + $row = self::getRecord($table->getDatabase()->getType(), [Refer to the past review comment for details.
🧹 Nitpick comments (3)
src/Migration/Sources/Appwrite/Reader/Database.php (2)
90-90: Address TODO: optimize table counting.The comment "todo: replacing this with a single list tables call downwards with +1 in each table" suggests a performance optimization opportunity. Consider whether this should be addressed now or tracked separately.
Would you like me to generate a refactored version that uses a single
listTablescall with a count query, or should this be tracked as a separate issue?
129-133: Clarify TODO: attribute support check.The comment "todo: apply check get support for attributes" is unclear. If this refers to conditionally counting attributes based on database capabilities, please clarify the intent.
Do you want me to investigate what this TODO should accomplish and propose an implementation?
src/Migration/Sources/Appwrite.php (1)
123-132: createReaderForDatabase method appears unused.PHPMD flags
createReaderForDatabase()as unused. If this method is intended for future use or external invocation, consider marking itprotectedor adding a comment explaining its purpose. Otherwise, remove it to reduce maintenance burden.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Migration/Destinations/Appwrite.php(38 hunks)src/Migration/Resource.php(3 hunks)src/Migration/Resources/Database/Database.php(3 hunks)src/Migration/Resources/Database/Row.php(1 hunks)src/Migration/Resources/Database/Table.php(1 hunks)src/Migration/Sources/Appwrite.php(17 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(9 hunks)src/Migration/Transfer.php(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-06-28T09:45:36.026Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Resources/Database/Row.phpsrc/Migration/Resources/Database/Table.php
📚 Learning: 2025-06-28T09:47:58.757Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite/Reader/Database.php
📚 Learning: 2025-06-28T09:47:08.333Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
🧬 Code graph analysis (5)
src/Migration/Transfer.php (1)
src/Migration/Resource.php (1)
Resource(5-257)
src/Migration/Resources/Database/Database.php (4)
src/Migration/Resources/Database/Row.php (1)
fromArray(53-61)src/Migration/Resources/Database/Table.php (2)
fromArray(50-62)getDatabase(91-94)src/Migration/Sources/Appwrite.php (1)
getDatabase(1787-1795)src/Migration/Sources/Appwrite/Reader/Database.php (1)
getDatabase(40-47)
src/Migration/Sources/Appwrite/Reader/Database.php (9)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)__construct(21-32)getDatabase(99-102)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/Document.php (1)
Document(7-13)src/Migration/Destinations/Appwrite.php (1)
__construct(78-101)src/Migration/Resources/Database/Row.php (1)
__construct(16-24)src/Migration/Resources/Database/Table.php (2)
__construct(20-32)getDatabase(91-94)src/Migration/Sources/Appwrite.php (3)
__construct(77-109)getDatabase(1787-1795)report(190-223)src/Migration/Resource.php (1)
Resource(5-257)src/Migration/Sources/Appwrite/Reader.php (2)
report(21-21)getSupportForAttributes(109-109)
src/Migration/Destinations/Appwrite.php (7)
src/Migration/Resources/Database/Database.php (3)
__construct(21-32)getDatabase(99-102)getName(74-77)src/Migration/Resources/Database/Row.php (4)
__construct(16-24)getTable(86-89)Row(8-98)getName(76-79)src/Migration/Sources/Appwrite.php (3)
__construct(77-109)getDatabase(1787-1795)getName(111-114)src/Migration/Sources/Appwrite/Reader/Database.php (2)
__construct(30-35)getDatabase(40-47)src/Migration/Cache.php (2)
__construct(24-27)get(188-195)src/Migration/Resource.php (2)
Resource(5-257)getName(129-129)src/Migration/Resources/Database/Table.php (2)
getDatabase(91-94)Table(8-110)
src/Migration/Sources/Appwrite.php (9)
src/Migration/Resources/Database/Database.php (6)
Database(19-103)getName(74-77)getDatabase(99-102)getType(94-97)getDatabaseName(84-87)fromArray(45-57)src/Migration/Sources/Appwrite/Reader/Database.php (14)
Database(23-491)report(49-142)queryLimit(463-466)queryEqual(408-411)queryCursorAfter(419-461)listDatabases(144-147)getDatabase(40-47)listTables(149-188)listColumns(190-250)listIndexes(252-297)getSupportForAttributes(468-471)querySelect(398-401)listRows(299-350)getRow(352-392)src/Migration/Resources/Database/Document.php (2)
Document(7-13)getName(9-12)src/Migration/Resource.php (3)
Resource(5-257)getName(129-129)getId(148-151)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/DocumentsDB.php (1)
DocumentsDB(7-13)src/Migration/Sources/Appwrite/Reader/API.php (12)
report(28-118)queryLimit(239-242)queryEqual(221-224)queryCursorAfter(230-237)listDatabases(123-126)listTables(131-138)listColumns(146-154)listIndexes(162-170)getSupportForAttributes(244-247)querySelect(211-214)listRows(179-187)getRow(196-205)src/Migration/Resources/Database/Table.php (5)
getDatabase(91-94)Table(8-110)getTableName(96-99)getRowSecurity(101-104)fromArray(50-62)src/Migration/Resources/Database/Row.php (3)
getTable(86-89)fromArray(53-61)Row(8-98)
🪛 GitHub Actions: CodeQL
src/Migration/Resources/Database/Row.php
[error] 55-55: Unsafe usage of new static().
src/Migration/Resources/Database/Database.php
[error] 47-47: Unsafe usage of new static().
src/Migration/Resources/Database/Table.php
[error] 52-52: Unsafe usage of new static().
src/Migration/Sources/Appwrite.php
[error] 83-83: Deprecated in PHP 8.1: Required parameter $getDatabaseDB follows optional parameter $dbForProject.
[error] 748-748: PHPDoc tag @param references unknown parameter: $databases
[error] 1718-1718: PHPDoc tag @param references unknown parameter: $documentsDBDatabases
🪛 GitHub Actions: Linter
src/Migration/Resource.php
[error] 1-1: PSR-12: no_whitespace_in_blank_line violation detected. (File: src/Migration/Resource.php)
src/Migration/Sources/Appwrite/Reader/Database.php
[error] 1-1: PSR-12: single_space_around_construct violation detected. (File: src/Migration/Sources/Appwrite/Reader/Database.php)
src/Migration/Destinations/Appwrite.php
[error] 1-1: PSR-12: single_space_around_construct violation detected. (File: src/Migration/Destinations/Appwrite.php)
src/Migration/Sources/Appwrite.php
[error] 1-1: PSR-12: single_space_around_construct violation detected. (File: src/Migration/Sources/Appwrite.php)
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
984-984: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
src/Migration/Sources/Appwrite.php
123-132: Avoid unused private methods such as 'createReaderForDatabase'. (undefined)
(UnusedPrivateMethod)
677-677: Avoid unused parameters such as '$resources'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (16)
src/Migration/Sources/Appwrite/Reader/Database.php (7)
25-35: Constructor accepts optional per-database factory callable.The addition of
?callable $getDatabaseDB = nullenables per-database resolution for multi-database contexts. The implementation correctly stores and uses this callable.
40-47: Database resolution helper correctly implements fallback.The
getDatabase()method properly routes to per-database instances when aDatabaseResourceis provided and the factory is available, falling back todbForProjectotherwise. This design supports both legacy single-database and new multi-database workflows.
331-335: Correctly routes row queries to per-database instance.The change from
$this->dbForProject->find()to$dbInstance = $this->getDatabase($resource->getDatabase()); $dbInstance->find()properly routes row data operations to the appropriate database context.
425-461: Extended cursor handling for Collection and Document resources.The addition of
CollectionResourceandDocumentResourcecases inqueryCursorAfter()correctly handles the new resource types. The logic appropriately distinguishes between metadata (stored indbForProject) and per-database data (retrieved viagetRow()).
468-471: New method delegates attribute support check to adapter.The
getSupportForAttributes()method correctly delegates to the underlying database adapter, enabling callers to determine whether the database supports schema-based attributes.
480-490: Per-database resource counting with proper routing.The refactored
countResources()method correctly routes counts to the appropriate database instance when aDatabaseResourceis provided, while falling back todbForProjectfor metadata tables. This aligns with the dual-context architecture.
78-83: Database type normalization is correct The normalization ofTYPE_DATABASE_LEGACYandTYPE_DATABASE_TABLESDBtoTYPE_DATABASEaligns with the keys inDATABASE_TYPE_RESOURCE_MAPand theisSupportedlogic, so resource filtering remains unaffected.src/Migration/Destinations/Appwrite.php (3)
60-63: Constructor properly accepts per-database factory.The updated constructor signature correctly accepts
$dbForProject,$getDatabaseDB, and stores the callable for later use. The docblock annotations are clear and accurate.Also applies to: 78-101
121-121: DocumentsDB resource type added to supported resources.The addition of
Resource::TYPE_DOCUMENTSDB_DATABASEcorrectly extends the destination's capabilities to handle Documents DB migrations.
982-1003: Orphan attribute filtering logic is correct but complex.The new logic filters out attributes not present in the table's schema when
getSupportForAttributes()returns true. This prevents errors when importing data with orphaned columns. The implementation correctly iterates through each row and removes unrecognized keys.Note: PHPMD flags
$valueas unused on line 984, but this is a false positive—the loop iterates over$rowto access keys, not values.src/Migration/Sources/Appwrite.php (6)
69-72: Constructor correctly accepts and validates getDatabaseDB callable.The constructor properly stores the
getDatabaseDBcallable and validates that it's non-null when$source === SOURCE_DATABASE. This ensures the required dependencies are available for database-based sources.Also applies to: 77-109
128-128: Verify getDatabaseDB callable handles null database parameter.In
createReaderForDatabase(), the code callscall_user_func($this->getDatabaseDB, new UtopiaDocument(['database' => $database])), but$databaseisDatabase|null. If null, the callable receives['database' => null], which might not be valid.Ensure the factory callable gracefully handles a null
databasekey, or guard this call to only invoke the factory when$databaseis non-null.
158-158: DocumentsDB resource type added to supported resources.The addition of
Resource::TYPE_DOCUMENTSDB_DATABASEcorrectly extends the source's capabilities for Documents DB migrations.
299-307: reportDatabases correctly instantiates reader for metadata access.The use of a reader (API or Database) with
$this->dbForProjectfor reporting is appropriate, as the report logic queries metadata across all databases.
646-669: exportGroupDatabases split by database type.The refactored logic correctly separates exports into
exportTablesDB()for legacy/tablesdb databases andexportDocumentsDB()for Documents DB. This aligns with the new per-database-type resource mapping.
1787-1795: Helper methods correctly map database types to resource classes.The new
getDatabase(),getEntity(), andgetRecord()helpers properly switch between Documents DB resources (DocumentsDB,Collection,Document) and legacy/tablesdb resources (Database,Table,Row) based on the database type. This centralizes the mapping logic.Also applies to: 1815-1823, 1854-1862
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
src/Migration/Destinations/Appwrite.php (2)
413-413: Validate database identifier before factory invocation.Similar to a previous review comment, the code passes
$databaseto the factory without validating that$database->getAttribute('database')is non-null/non-empty. Consider adding validation before the factory call.
447-447: Use constant instead of magic string.As flagged in a previous review, use
Resource::TYPE_DOCUMENTSDB_DATABASEinstead of the hardcoded string'documents'for consistency and maintainability.Apply this diff:
- if ($resource->getTable()->getDatabase()->getType() === 'documents') { + if ($resource->getTable()->getDatabase()->getType() === Resource::TYPE_DOCUMENTSDB_DATABASE) {src/Migration/Sources/Appwrite.php (5)
77-109: Fix deprecated parameter order for PHP 8.1+ compatibility.The pipeline correctly flags that required parameter
$getDatabaseDBfollows optional parameter$dbForProject, which is deprecated in PHP 8.1 and will cause a fatal error in PHP 9.0.Reorder parameters to place all required parameters before optional ones:
public function __construct( protected string $project, protected string $endpoint, protected string $key, + callable $getDatabaseDB, protected string $source = self::SOURCE_API, - protected ?UtopiaDatabase $dbForProject = null, - callable $getDatabaseDB + protected ?UtopiaDatabase $dbForProject = null ) {Important: Update all call sites to match the new parameter order.
734-740: Fix PHPDoc: remove unknown parameter.The pipeline correctly flags that PHPDoc references non-existent parameter
$databases. Remove this line from the docblock./** * @param string $databaseName * @param int $batchSize - * @param array<Resource> $databases * @throws Exception */
1170-1186: Pass database type instead of database name to getRecord().The code passes
$table->getDatabase()->getDatabaseName()togetRecord(), butgetRecord()expects the database type to determine whether to create a Document or Row resource.For DocumentsDB databases, this will incorrectly create Row resources instead of Document resources.
- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ + $row = self::getRecord($table->getDatabase()->getType(), [ 'id' => $id,
1692-1699: Fix PHPDoc: remove unknown parameter.The pipeline correctly flags that PHPDoc references non-existent parameter
$documentsDBDatabases. Remove this line from the docblock./** * Export DocumentsDB databases (collections and documents) * * @param int $batchSize * @param array $resources - * @param array $documentsDBDatabases */
779-792: Pass database type instead of database name to getEntity().The code passes
$databaseName(which isDatabase::getName()orDocumentsDB::getName()) togetEntity(), butgetEntity()expects the database type to determine whether to create a Table or Collection resource.For DocumentsDB databases, this will incorrectly create Table resources instead of Collection resources.
foreach ($response as $table) { - $newTable = self::getEntity($databaseName, [ + $newTable = self::getEntity($database->getType(), [ 'id' => $table['$id'], 'name' => $table['name'],Based on learnings from past reviews, this maintains compatibility while correctly routing DocumentsDB tables to Collection resources.
src/Migration/Resource.php (1)
33-38: Past PSR-12 whitespace issue may still need attention.A previous review flagged that the blank lines between these constants contain stray spaces, violating the PSR-12
no_whitespace_in_blank_linerule. Ensure blank lines 34 and 36 are truly empty (no spaces or tabs).
🧹 Nitpick comments (2)
src/Migration/Sources/Appwrite/Reader/Database.php (1)
40-47: Verify database identifier is valid before factory invocation.The code passes
$databaseResource->getDatabase()directly to the factory without validating it's non-null or non-empty. IfgetDatabase()returns an invalid identifier, the factory might fail unexpectedly.Consider adding validation:
private function getDatabase(?DatabaseResource $databaseResource = null): UtopiaDatabase { if ($this->getDatabaseDB !== null && $databaseResource !== null) { + $databaseId = $databaseResource->getDatabase(); + if (empty($databaseId)) { + return $this->dbForProject; + } return call_user_func($this->getDatabaseDB, new UtopiaDocument(['database' => $databaseResource->getDatabase()])); } return $this->dbForProject; }src/Migration/Destinations/Appwrite.php (1)
541-706: Consider caching the per-database instance.The method calls
call_user_func($this->getDatabaseDB, $database)multiple times (lines 541, 584, 644, 652, 670, 702). Consider caching the result in a local variable at the start of the method to improve efficiency and readability.Example:
$dbForDatabase = call_user_func($this->getDatabaseDB, $database); // Then use $dbForDatabase throughout instead of repeated calls
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Migration/Destinations/Appwrite.php(38 hunks)src/Migration/Resource.php(3 hunks)src/Migration/Sources/Appwrite.php(17 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(9 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
📚 Learning: 2025-06-28T09:47:58.757Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite/Reader/Database.php
📚 Learning: 2025-06-28T09:47:08.333Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/Appwrite.php
🧬 Code graph analysis (3)
src/Migration/Destinations/Appwrite.php (3)
src/Migration/Sources/Appwrite.php (2)
__construct(77-109)getDatabase(1768-1776)src/Migration/Sources/Appwrite/Reader/Database.php (3)
__construct(30-35)getDatabase(40-47)getSupportForAttributes(468-471)src/Migration/Resource.php (1)
Resource(5-257)
src/Migration/Sources/Appwrite/Reader/Database.php (10)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)__construct(21-32)getDatabase(99-102)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/Document.php (1)
Document(7-13)src/Migration/Destinations/Appwrite.php (1)
__construct(78-101)src/Migration/Sources/Appwrite.php (3)
__construct(77-109)getDatabase(1768-1776)report(187-220)src/Migration/Resources/Database/Row.php (1)
__construct(16-24)src/Migration/Resources/Database/Table.php (2)
__construct(20-32)getDatabase(91-94)src/Migration/Sources/Appwrite/Reader/API.php (3)
__construct(19-23)report(28-118)getSupportForAttributes(244-247)src/Migration/Resource.php (2)
Resource(5-257)isSupported(133-146)src/Migration/Sources/Appwrite/Reader.php (2)
report(21-21)getSupportForAttributes(109-109)
src/Migration/Sources/Appwrite.php (9)
src/Migration/Sources/Appwrite/Reader/Database.php (14)
Database(23-491)report(49-142)queryLimit(463-466)queryEqual(408-411)queryCursorAfter(419-461)listDatabases(144-147)getDatabase(40-47)listTables(149-188)listColumns(190-250)listIndexes(252-297)getSupportForAttributes(468-471)querySelect(398-401)listRows(299-350)getRow(352-392)src/Migration/Resources/Database/Database.php (6)
Database(19-103)getName(74-77)getDatabase(99-102)getType(94-97)getDatabaseName(84-87)fromArray(45-57)src/Migration/Resources/Database/Document.php (2)
Document(7-13)getName(9-12)src/Migration/Resource.php (4)
Resource(5-257)getName(129-129)isSupported(133-146)getId(148-151)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/DocumentsDB.php (1)
DocumentsDB(7-13)src/Migration/Sources/Appwrite/Reader/API.php (12)
report(28-118)queryLimit(239-242)queryEqual(221-224)queryCursorAfter(230-237)listDatabases(123-126)listTables(131-138)listColumns(146-154)listIndexes(162-170)getSupportForAttributes(244-247)querySelect(211-214)listRows(179-187)getRow(196-205)src/Migration/Resources/Database/Table.php (4)
getDatabase(91-94)Table(8-110)getTableName(96-99)fromArray(50-62)src/Migration/Resources/Database/Row.php (3)
getTable(86-89)fromArray(53-61)Row(8-98)
🪛 GitHub Actions: CodeQL
src/Migration/Sources/Appwrite.php
[error] 83-83: Deprecated in PHP 8.1: Required parameter $getDatabaseDB follows optional parameter $dbForProject.
[error] 740-740: PHPDoc tag @param references unknown parameter: $databases
[error] 1699-1699: PHPDoc tag @param references unknown parameter: $documentsDBDatabases
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
984-984: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (13)
src/Migration/Resource.php (1)
97-109: LGTM! Clear database type mappings.The
DATABASE_TYPE_RESOURCE_MAPprovides a clean separation between TablesDB (table/column/row) and DocumentsDB (collection/document/attribute) resource types. The structure supports the per-database architecture introduced in this PR.src/Migration/Sources/Appwrite/Reader/Database.php (4)
76-122: LGTM! Robust per-database reporting logic.The report method correctly:
- Normalizes legacy database types to TYPE_DATABASE
- Uses DATABASE_TYPE_RESOURCE_MAP for database-specific resource types
- Passes DatabaseResource context to countResources for accurate per-database counts
331-335: LGTM! Correct per-database context usage.Both
listRows()andgetRow()correctly:
- Fetch metadata (database/table info) from
dbForProject- Use
getDatabase()to obtain the per-database instance for actual row data operationsThis separation ensures metadata queries use the project database while row data uses the appropriate database instance.
Also applies to: 384-391
425-455: LGTM! Clear resource type routing with helpful comments.The
queryCursorAfter()method correctly routes different resource types:
- Metadata resources (Database, Table, Column, Index) →
dbForProject- Data resources (Row, Document) →
getRow()which uses per-database contextThe inline comments explaining the routing are helpful for maintainability.
468-490: LGTM! Correct adapter delegation and per-database counting.Both methods implement correct logic:
getSupportForAttributes()delegates to the project database adaptercountResources()routes to per-database instance when DatabaseResource is provided, otherwise uses dbForProject for metadatasrc/Migration/Destinations/Appwrite.php (4)
78-101: LGTM! Constructor accepts per-database factory.The constructor correctly accepts both
$dbForProjectand the$getDatabaseDBcallable, storing the factory for later use in database-specific operations.
120-121: LGTM! DocumentsDB added to supported resources.Correctly adds
Resource::TYPE_DOCUMENTSDB_DATABASEto the supported resources list, enabling DocumentsDB migrations.
742-885: LGTM! Correct per-database context usage.The method correctly:
- Uses
dbForProjectfor metadata operations (index documents, counts)- Obtains
dbForDatabasevia factory for adapter limits and actual index creation
982-1007: LGTM! Robust orphan attribute filtering.The attribute filtering logic correctly:
- Checks if the database adapter supports attributes
- Filters out attributes not in the table schema
- Preserves system attributes (those starting with
$)The comment clearly explains why this filtering is necessary.
src/Migration/Sources/Appwrite.php (4)
122-129: LGTM! Clean reader factory pattern.The
createReader()method provides a clean abstraction for creating the appropriate reader based on the source type, correctly passing the per-database factory to DatabaseReader.
700-717: LGTM! Correct database type normalization and resource creation.The method correctly:
- Normalizes legacy database types to TYPE_DATABASE
- Uses the
getDatabase()helper with the correct database type- Passes all necessary database attributes including the new
databasefield
1768-1776: LGTM! Clean database resource factory.The
getDatabase()helper correctly routes to DocumentsDB or Database resource based on the database type.
1796-1843: LGTM! Correct entity and record factory helpers.Both
getEntity()andgetRecord()correctly route to Collection/Document or Table/Row based on the database type. This design enables proper resource type handling for DocumentsDB vs TablesDB databases.
ArnabChatterjee20k
force-pushed
the
feat-documents-db
branch
from
8bc1eb8 to
1a45ca8
Compare
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 (1)
src/Migration/Resources/Database/Row.php (1)
55-61: Replace unsafe new static() and add subclass override.Use new self() here to satisfy static analysis. Implement Document::fromArray() to preserve late binding when invoked on Document.
- return new static( + return new self( $array['id'], Table::fromArray($array['table'] ?? $array['collection']), $array['data'], $array['permissions'] ?? [] );Add in src/Migration/Resources/Database/Document.php:
public static function fromArray(array $array): self { return new self( $array['id'], Table::fromArray($array['table'] ?? $array['collection']), $array['data'], $array['permissions'] ?? [] ); }
♻️ Duplicate comments (9)
src/Migration/Destinations/Appwrite.php (3)
446-450: Use constant instead of 'documents' magic string.Replace with Resource::TYPE_DOCUMENTSDB_DATABASE.
- if ($resource->getTable()->getDatabase()->getType() === 'documents') { + if ($resource->getTable()->getDatabase()->getType() === Resource::TYPE_DOCUMENTSDB_DATABASE) {
413-416: Validate database DSN before invoking factory.Ensure the UtopiaDocument has a non-empty 'database' attribute; handle invalid DSNs deterministically.
- $dbForDatabase = call_user_func($this->getDatabaseDB, $database); + $dsn = (string) $database->getAttribute('database', ''); + if ($dsn === '') { + throw new Exception( + resourceName: $resource->getName(), + resourceGroup: $resource->getGroup(), + resourceId: $resource->getId(), + message: 'Invalid or missing database DSN' + ); + } + $dbForDatabase = \call_user_func($this->getDatabaseDB, $database);Also applies to: 541-542, 742-743, 977-978
982-1001: Remove unused foreach value to satisfy PHPMD.Iterate keys only.
- foreach ($row as $key => $value) { + foreach (\array_keys($row->getArrayCopy()) as $key) { if (\str_starts_with($key, '$')) { continue; }src/Migration/Sources/Appwrite.php (5)
77-85: Fix deprecated parameter order (PHP 8.1+).Required parameter must not follow optional.
public function __construct( protected string $project, protected string $endpoint, protected string $key, protected string $source = self::SOURCE_API, - protected ?UtopiaDatabase $dbForProject = null, - callable $getDatabaseDB + callable $getDatabaseDB, + protected ?UtopiaDatabase $dbForProject = null ) {
735-741: PHPDoc references unknown parameter.Remove @param for non-existent $databases.
- /** - * @param string $databaseName - * @param int $batchSize - * @param array<Resource> $databases - * @throws Exception - */ + /** + * @param string $databaseName + * @param int $batchSize + * @throws Exception + */
779-793: Pass database type, not name, to getEntity().Ensures Collections are emitted for DocumentsDB.
- $newTable = self::getEntity($databaseName, [ + $newTable = self::getEntity($database->getType(), [
1169-1186: Pass database type, not name, to getRecord().Ensures Documents are emitted for DocumentsDB.
- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ + $row = self::getRecord($table->getDatabase()->getType(), [
1692-1700: PHPDoc references unknown parameter.Remove @param for non-existent $documentsDBDatabases.
- /** - * Export DocumentsDB databases (collections and documents) - * - * @param int $batchSize - * @param array $resources - * @param array $documentsDBDatabases - */ + /** + * Export DocumentsDB databases (collections and documents) + * + * @param int $batchSize + * @param array $resources + */src/Migration/Resources/Database/Database.php (1)
47-56: Fix unsafe late-static instantiation and default missing 'database'.
- Replace new static(...) with new self(...) to satisfy CodeQL/PHPStan.
- Default database to '' when key missing to keep BC and avoid TypeError.
- return new static( + return new self( $array['id'], $array['name'], createdAt: $array['createdAt'] ?? '', updatedAt: $array['updatedAt'] ?? '', enabled: $array['enabled'] ?? true, originalId: $array['originalId'] ?? '', type: $array['type'] ?? 'legacy', - database: $array['database'] + database: $array['database'] ?? '' );
🧹 Nitpick comments (3)
src/Migration/Resources/Database/Database.php (1)
34-43: Docblock: mark database as optional.Reflects the defensive default applied above.
- * database: string + * database?: stringsrc/Migration/Destinations/Appwrite.php (1)
60-64: Type the database resolver.Use callable type instead of mixed.
- protected mixed $getDatabaseDB; + protected callable $getDatabaseDB;src/Migration/Sources/Appwrite.php (1)
69-73: Type the database resolver property.Prefer callable|null over mixed.
- protected mixed $getDatabaseDB; + protected ?callable $getDatabaseDB;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Migration/Destinations/Appwrite.php(38 hunks)src/Migration/Resource.php(3 hunks)src/Migration/Resources/Database/Database.php(3 hunks)src/Migration/Resources/Database/Row.php(1 hunks)src/Migration/Resources/Database/Table.php(1 hunks)src/Migration/Sources/Appwrite.php(17 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(8 hunks)src/Migration/Transfer.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Migration/Resources/Database/Table.php
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-06-28T09:45:36.026Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Resources/Database/Row.php
📚 Learning: 2025-06-28T09:47:58.757Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite/Reader/Database.php
📚 Learning: 2025-06-28T09:47:08.333Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
🧬 Code graph analysis (5)
src/Migration/Transfer.php (1)
src/Migration/Resource.php (1)
Resource(5-257)
src/Migration/Sources/Appwrite/Reader/Database.php (10)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)__construct(21-32)getDatabase(99-102)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/Document.php (1)
Document(7-13)src/Migration/Destinations/Appwrite.php (1)
__construct(78-101)src/Migration/Resources/Database/Row.php (1)
__construct(16-24)src/Migration/Resources/Database/Table.php (2)
__construct(20-32)getDatabase(91-94)src/Migration/Sources/Appwrite.php (3)
__construct(77-109)getDatabase(1768-1776)report(187-220)src/Migration/Sources/Appwrite/Reader/API.php (5)
__construct(19-23)listDatabases(123-126)report(28-118)listTables(131-138)getSupportForAttributes(244-247)src/Migration/Resource.php (1)
Resource(5-257)src/Migration/Sources/Appwrite/Reader.php (4)
listDatabases(29-29)report(21-21)listTables(38-38)getSupportForAttributes(109-109)
src/Migration/Destinations/Appwrite.php (8)
src/Migration/Resources/Database/Database.php (4)
__construct(21-32)getDatabase(99-102)getType(94-97)getName(74-77)src/Migration/Resources/Database/Row.php (4)
__construct(16-24)getTable(86-89)Row(8-98)getName(76-79)src/Migration/Sources/Appwrite.php (3)
__construct(77-109)getDatabase(1768-1776)getName(111-114)src/Migration/Sources/Appwrite/Reader/Database.php (3)
__construct(30-35)getDatabase(40-47)getSupportForAttributes(478-481)src/Migration/Cache.php (2)
__construct(24-27)get(188-195)src/Migration/Resource.php (2)
Resource(5-257)getName(129-129)src/Migration/Resources/Database/Table.php (2)
getDatabase(91-94)Table(8-110)src/Migration/Resources/Database/Column.php (3)
Column(8-155)getTable(97-100)getType(85-85)
src/Migration/Resources/Database/Database.php (4)
src/Migration/Resources/Database/Row.php (1)
fromArray(53-61)src/Migration/Resources/Database/Table.php (2)
fromArray(50-62)getDatabase(91-94)src/Migration/Resources/Database/Index.php (1)
fromArray(76-89)src/Migration/Sources/Appwrite.php (1)
getDatabase(1768-1776)
src/Migration/Sources/Appwrite.php (11)
src/Migration/Resources/Database/Database.php (6)
Database(19-103)getName(74-77)getDatabase(99-102)getType(94-97)getDatabaseName(84-87)fromArray(45-57)src/Migration/Sources/Appwrite/Reader/Database.php (14)
Database(23-501)report(49-152)queryLimit(473-476)queryEqual(418-421)queryCursorAfter(429-471)listDatabases(154-157)getDatabase(40-47)listTables(159-198)listColumns(200-260)listIndexes(262-307)getSupportForAttributes(478-481)querySelect(408-411)listRows(309-360)getRow(362-402)src/Migration/Resources/Database/Document.php (2)
Document(7-13)getName(9-12)src/Migration/Resource.php (4)
Resource(5-257)getName(129-129)isSupported(133-146)getId(148-151)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/DocumentsDB.php (1)
DocumentsDB(7-13)src/Migration/Sources/Appwrite/Reader.php (12)
report(21-21)queryLimit(108-108)queryEqual(92-92)queryCursorAfter(100-100)listDatabases(29-29)listTables(38-38)listColumns(47-47)listIndexes(56-56)getSupportForAttributes(109-109)querySelect(83-83)listRows(65-65)getRow(75-75)src/Migration/Sources/Appwrite/Reader/API.php (12)
report(28-118)queryLimit(239-242)queryEqual(221-224)queryCursorAfter(230-237)listDatabases(123-126)listTables(131-138)listColumns(146-154)listIndexes(162-170)getSupportForAttributes(244-247)querySelect(211-214)listRows(179-187)getRow(196-205)src/Migration/Cache.php (1)
get(188-195)src/Migration/Resources/Database/Table.php (5)
getDatabase(91-94)Table(8-110)getTableName(96-99)getRowSecurity(101-104)fromArray(50-62)src/Migration/Resources/Database/Row.php (3)
getTable(86-89)fromArray(53-61)Row(8-98)
🪛 GitHub Actions: CodeQL
src/Migration/Resources/Database/Row.php
[error] 55-55: PHPStan: Unsafe usage of new static().
src/Migration/Resources/Database/Database.php
[error] 47-47: PHPStan: Unsafe usage of new static().
src/Migration/Sources/Appwrite.php
[error] 83-83: PHPStan: Deprecated in PHP 8.1: Required parameter $getDatabaseDB follows optional parameter $dbForProject.
[error] 740-740: PHPStan: PHPDoc tag @param references unknown parameter: $databases
[error] 1699-1699: PHPStan: PHPDoc tag @param references unknown parameter: $documentsDBDatabases
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
984-984: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (2)
src/Migration/Resource.php (1)
33-38: Additions look consistent and useful.New resource types and DATABASE_TYPE_RESOURCE_MAP are coherent with downstream usage.
Also applies to: 74-80, 97-109
src/Migration/Transfer.php (1)
37-46: LGTM.Databases group now includes DocumentsDB entities/records. No issues.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (9)
src/Migration/Destinations/Appwrite.php (3)
984-984: Remove unused loop variable.Static analysis correctly identifies that
$valueis unused in the foreach loop at line 984. Consider using only the key or a throwaway variable name.Apply this diff:
- foreach ($row as $key => $value) { + foreach ($row as $key => $_) {Or iterate only over keys:
- foreach ($row as $key => $value) { + foreach (array_keys($row->getArrayCopy()) as $key) {
447-450: Use constant instead of hardcoded 'documents' string.The check
$resource->getTable()->getDatabase()->getType() === 'documents'uses a magic string. This should use the appropriate constant for consistency and maintainability.Apply this diff:
- if ($resource->getTable()->getDatabase()->getType() === 'documents') { + if ($resource->getTable()->getDatabase()->getType() === Resource::TYPE_DOCUMENTSDB_DATABASE) { $resource->setStatus(Resource::STATUS_SKIPPED, 'Columns not supported for documents database'); return false; }
413-413: Validate database identifier before invoking factory.At multiple locations (lines 413, 541, 742, 977),
$database->getAttribute('database')is passed directly to the factory callable without validation. If the attribute is null or empty, the factory may fail unexpectedly.Consider adding validation before each factory invocation. Example for line 413:
+ $databaseDSN = $database->getAttribute('database'); + if (!\is_string($databaseDSN) || $databaseDSN === '') { + throw new Exception( + resourceName: $resource->getName(), + resourceGroup: $resource->getGroup(), + resourceId: $resource->getId(), + message: 'Database DSN is required', + ); + } - $dbForDatabase = call_user_func($this->getDatabasesDB, $database); + $dbForDatabase = call_user_func($this->getDatabasesDB, $database);Apply similar guards at lines 541, 742, and 977.
Also applies to: 541-541, 742-742, 977-977
src/Migration/Sources/Appwrite.php (5)
77-109: Fix deprecated parameter order in PHP 8.1.Required parameter
$getDatabasesDBfollows optional parameter$dbForProject, which is deprecated in PHP 8.1 and will cause a fatal error in PHP 9.0.Apply this diff to reorder parameters:
public function __construct( protected string $project, protected string $endpoint, protected string $key, protected string $source = self::SOURCE_API, - protected ?UtopiaDatabase $dbForProject = null, - callable $getDatabasesDB + callable $getDatabasesDB, + protected ?UtopiaDatabase $dbForProject = null ) {Important: Ensure all call sites are updated to match the new parameter order.
779-795: Pass database type instead of name to getEntity.At line 779,
getEntityis called with$databaseName(the cache key/resource name like"Database"or"DocumentsDB"), butgetEntityexpects the database type (e.g.,Resource::TYPE_DATABASEorResource::TYPE_DOCUMENTSDB_DATABASE) to determine whether to return aTableorCollectionresource.Apply this diff:
- $newTable = self::getEntity($databaseName, [ + $newTable = self::getEntity($database->getType(), [This ensures DocumentsDB tables are correctly mapped to
Collectionresources and regular tables toTableresources.
1170-1186: Pass database type instead of name to getRecord.At line 1170,
getRecordis called with$table->getDatabase()->getDatabaseName(), butgetRecordexpects the database type to decide betweenDocumentandRowresources. Using the name will cause all records to default toRow.Apply this diff:
- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ + $row = self::getRecord($table->getDatabase()->getType(), [This ensures DocumentsDB rows become
Documentresources as intended, and regular rows remainRowresources.
734-741: Fix PHPDoc: remove unknown parameter.The PHPDoc references parameter
$databases, but the method signature forexportEntitieshas no such parameter.Apply this diff:
/** * @param string $databaseName * @param int $batchSize - * @param array<Resource> $databases * @throws Exception */
1692-1699: Fix PHPDoc: remove unknown parameter.The PHPDoc references parameter
$documentsDBDatabases, but the method signature forexportDocumentsDBhas no such parameter.Apply this diff:
/** * Export DocumentsDB databases (collections and documents) * * @param int $batchSize * @param array $resources - * @param array $documentsDBDatabases */src/Migration/Sources/Appwrite/Reader/Database.php (1)
40-47: Guard per-database resolver against empty or null DSN.The
getDatabasehelper does not validate that$databaseDSNis a non-empty string before invoking the factory callable. If$databaseDSNis an empty string, the factory may fail unexpectedly or return an incorrect database instance.Apply this diff to add validation:
private function getDatabase(?string $databaseDSN = null): UtopiaDatabase { - if ($this->getDatabasesDB !== null && $databaseDSN !== null) { + if ($this->getDatabasesDB !== null && \is_string($databaseDSN) && $databaseDSN !== '') { return call_user_func($this->getDatabasesDB, new UtopiaDocument(['database' => $databaseDSN])); } return $this->dbForProject; }This ensures the factory is only called with valid, non-empty DSNs, and falls back to
dbForProjectwhen the DSN is null or empty.
🧹 Nitpick comments (1)
src/Migration/Sources/Appwrite/Reader/Database.php (1)
25-35: Type the callable property for clarity.The
$getDatabasesDBproperty is declared asmixedwith a docblock. Consider using a more specific type hint for better type safety.Apply this diff:
- private mixed $getDatabasesDB; + private ?callable $getDatabasesDB;This clarifies that the property holds either a callable or null, improving type safety and IDE support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Migration/Destinations/Appwrite.php(38 hunks)src/Migration/Sources/Appwrite.php(17 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(8 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-28T09:47:08.333Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite.phpsrc/Migration/Sources/Appwrite/Reader/Database.php
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
📚 Learning: 2025-06-28T09:47:58.757Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Sources/Appwrite/Reader/Database.php
🧬 Code graph analysis (3)
src/Migration/Sources/Appwrite.php (6)
src/Migration/Sources/Appwrite/Reader/Database.php (14)
Database(23-501)report(49-152)queryLimit(473-476)queryEqual(418-421)queryCursorAfter(429-471)listDatabases(154-157)getDatabase(40-47)listTables(159-198)listColumns(200-260)listIndexes(262-307)getSupportForAttributes(478-481)querySelect(408-411)listRows(309-360)getRow(362-402)src/Migration/Resources/Database/Database.php (6)
Database(19-103)getName(74-77)getDatabase(99-102)getType(94-97)getDatabaseName(84-87)fromArray(45-57)src/Migration/Resources/Database/Document.php (2)
Document(7-13)getName(9-12)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/DocumentsDB.php (1)
DocumentsDB(7-13)src/Migration/Sources/Appwrite/Reader/API.php (12)
report(28-118)queryLimit(239-242)queryEqual(221-224)queryCursorAfter(230-237)listDatabases(123-126)listTables(131-138)listColumns(146-154)listIndexes(162-170)getSupportForAttributes(244-247)querySelect(211-214)listRows(179-187)getRow(196-205)
src/Migration/Destinations/Appwrite.php (6)
src/Migration/Sources/Appwrite.php (3)
__construct(77-109)getDatabase(1768-1776)getName(111-114)src/Migration/Sources/Appwrite/Reader/Database.php (3)
__construct(30-35)getDatabase(40-47)getSupportForAttributes(478-481)src/Migration/Resources/Database/Database.php (4)
__construct(21-32)getDatabase(99-102)getType(94-97)getName(74-77)src/Migration/Cache.php (2)
__construct(24-27)get(188-195)src/Migration/Resource.php (2)
Resource(5-257)getName(129-129)src/Migration/Sources/Appwrite/Reader.php (1)
getSupportForAttributes(109-109)
src/Migration/Sources/Appwrite/Reader/Database.php (12)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)__construct(21-32)getDatabase(99-102)src/Migration/Resources/Database/Collection.php (1)
Collection(8-15)src/Migration/Resources/Database/Column.php (2)
Column(8-155)__construct(40-55)src/Migration/Resources/Database/Document.php (1)
Document(7-13)src/Migration/Destinations/Appwrite.php (1)
__construct(78-101)src/Migration/Sources/Appwrite.php (3)
__construct(77-109)getDatabase(1768-1776)report(187-220)src/Migration/Resources/Database/Table.php (2)
__construct(20-32)getDatabase(91-94)src/Migration/Resources/Database/Row.php (1)
__construct(16-24)src/Migration/Sources/Appwrite/Reader/API.php (5)
__construct(19-23)listDatabases(123-126)report(28-118)listTables(131-138)getSupportForAttributes(244-247)src/Migration/Transfer.php (1)
__construct(107-116)src/Migration/Resources/Database/Index.php (1)
__construct(29-41)src/Migration/Resource.php (3)
Resource(5-257)isSupported(133-146)getId(148-151)
🪛 GitHub Actions: CodeQL
src/Migration/Sources/Appwrite.php
[error] 83-83: Deprecated in PHP 8.1: Required parameter $getDatabasesDB follows optional parameter $dbForProject.
[error] 740-740: PHPDoc tag @param references unknown parameter: $databases
[error] 1699-1699: PHPDoc tag @param references unknown parameter: $documentsDBDatabases
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
984-984: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (9)
src/Migration/Sources/Appwrite/Reader/Database.php (3)
76-149: Per-database reporting logic is well-structured.The report method correctly normalizes legacy database types and routes resource counting to the appropriate database instances using
DATABASE_TYPE_RESOURCE_MAP. The per-database context propagation is properly implemented.
478-481: New public method getSupportForAttributes is correctly implemented.The method delegates to the adapter and provides a clean interface for checking attribute support.
341-359: Per-database routing verified
Database::fromArray()requires thedatabasekey (no default in constructor), sogetDatabase()always returns the configured DSN.listRowscorrectly routes queries per database.src/Migration/Destinations/Appwrite.php (2)
78-101: Constructor correctly accepts per-database factory.The constructor properly stores the
$getDatabasesDBcallable and validates its presence for database sources.
382-437: Per-database entity creation is correctly implemented.The
createEntitymethod properly retrieves the per-database instance using the factory and routes table creation to the correct database context.src/Migration/Sources/Appwrite.php (4)
122-129: Reader factory method is well-implemented.The
createReadermethod correctly instantiates the appropriate reader based on the source type.
667-732: Database export logic correctly uses static helper.The
exportDatabasesmethod properly normalizes database types and constructs Database or DocumentsDB resources using thegetDatabasehelper.
1768-1843: Static mapping helpers are well-structured.The
getDatabase,getEntity, andgetRecordhelpers correctly map database types to the appropriate resource classes (Database/DocumentsDB, Table/Collection, Row/Document). Good use of switch statements for clarity.
1617-1754: Per-database export methods are correctly implemented.The
exportTablesDBandexportDocumentsDBmethods properly separate export flows for Tables/Columns/Rows vs Collections/Documents, routing to the appropriate entity types.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/Migration/Destinations/Appwrite.php (1)
413-416: Validate database identifier before creating per‑DB adapterEnsure the database doc has a valid 'database' attribute before calling the factory; fail fast with a clear error.
- $dbForDatabase = call_user_func($this->getDatabasesDB, $database); + $dsn = $database->getAttribute('database'); + if (!\is_string($dsn) || $dsn === '') { + throw new Exception( + resourceName: $resource->getName(), + resourceGroup: $resource->getGroup(), + resourceId: $resource->getId(), + message: 'Invalid or missing database identifier (DSN)' + ); + } + $dbForDatabase = call_user_func($this->getDatabasesDB, $database);src/Migration/Resources/Database/Database.php (1)
55-56: Handle missing 'database' key to preserve BCUse a fallback to avoid TypeError when callers omit 'database'.
Apply:
- database: $array['database'] + database: $array['database'] ?? ''src/Migration/Sources/Appwrite.php (1)
1167-1183: Wrong discriminator passed to getRecord; Documents become RowsPass the database type, not the name, so documents route to Document::fromArray().
- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ + $row = self::getRecord($table->getDatabase()->getType(), [
🧹 Nitpick comments (2)
src/Migration/Resources/Database/Collection.php (1)
30-41: Defensive default for rowSecurityIf both rowSecurity and documentSecurity are absent, null is passed to a bool parameter. Default to false.
Apply:
- rowSecurity: $array['rowSecurity'] ?? $array['documentSecurity'], + rowSecurity: $array['rowSecurity'] ?? ($array['documentSecurity'] ?? false),src/Migration/Destinations/Appwrite.php (1)
982-999: Remove unused loop variable$value is unused; avoid noise and PHPMD warning.
- foreach ($row as $key => $value) { + foreach ($row as $key => $_) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Migration/Destinations/Appwrite.php(38 hunks)src/Migration/Resource.php(3 hunks)src/Migration/Resources/Database/Collection.php(1 hunks)src/Migration/Resources/Database/Database.php(4 hunks)src/Migration/Resources/Database/Document.php(1 hunks)src/Migration/Resources/Database/DocumentsDB.php(1 hunks)src/Migration/Sources/Appwrite.php(17 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Migration/Resources/Database/Document.php
- src/Migration/Resources/Database/DocumentsDB.php
- src/Migration/Resource.php
- src/Migration/Sources/Appwrite/Reader/Database.php
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-28T09:47:08.333Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Resources/Database/Collection.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:47:58.757Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Resources/Database/Collection.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
🧬 Code graph analysis (4)
src/Migration/Resources/Database/Collection.php (5)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)getName(74-77)fromArray(45-57)src/Migration/Resource.php (2)
Resource(5-257)getName(129-129)src/Migration/Resources/Database/Table.php (3)
Table(8-110)getName(81-84)fromArray(50-62)src/Migration/Resources/Database/Document.php (2)
getName(9-12)fromArray(41-50)src/Migration/Resources/Database/DocumentsDB.php (3)
getName(9-12)fromArray(25-37)DocumentsDB(7-38)
src/Migration/Destinations/Appwrite.php (8)
src/Migration/Resources/Database/Database.php (3)
__construct(21-32)getDatabase(99-102)getName(74-77)src/Migration/Sources/Appwrite.php (3)
__construct(76-108)getDatabase(1764-1772)getName(110-113)src/Migration/Sources/Appwrite/Reader/Database.php (3)
__construct(30-35)getDatabase(40-47)getSupportForAttributes(478-481)src/Migration/Resources/Database/Row.php (4)
__construct(16-24)getTable(86-89)Row(8-98)getName(76-79)src/Migration/Resources/Database/Table.php (3)
__construct(20-32)getDatabase(91-94)Table(8-110)src/Migration/Cache.php (2)
__construct(24-27)get(188-195)src/Migration/Resource.php (2)
Resource(5-257)getName(129-129)src/Migration/Resources/Database/Column.php (2)
Column(8-155)getTable(97-100)
src/Migration/Sources/Appwrite.php (10)
src/Migration/Resources/Database/Database.php (6)
Database(19-103)getName(74-77)getDatabase(99-102)getType(94-97)getDatabaseName(84-87)fromArray(45-57)src/Migration/Sources/Appwrite/Reader/Database.php (14)
Database(23-501)report(49-152)queryLimit(473-476)queryEqual(418-421)queryCursorAfter(429-471)listDatabases(154-157)getDatabase(40-47)listTables(159-198)listColumns(200-260)listIndexes(262-307)getSupportForAttributes(478-481)querySelect(408-411)listRows(309-360)getRow(362-402)src/Migration/Resources/Database/Collection.php (3)
Collection(7-43)getName(9-12)fromArray(30-42)src/Migration/Resources/Database/Document.php (3)
Document(7-51)getName(9-12)fromArray(41-50)src/Migration/Resources/Database/DocumentsDB.php (2)
DocumentsDB(7-38)fromArray(25-37)src/Migration/Resources/Database/Row.php (4)
getName(76-79)getTable(86-89)fromArray(53-61)Row(8-98)src/Migration/Resources/Database/Table.php (6)
getName(81-84)getDatabase(91-94)Table(8-110)getTableName(96-99)getRowSecurity(101-104)fromArray(50-62)src/Migration/Sources/Appwrite/Reader.php (12)
report(21-21)queryLimit(108-108)queryEqual(92-92)queryCursorAfter(100-100)listDatabases(29-29)listTables(38-38)listColumns(47-47)listIndexes(56-56)getSupportForAttributes(109-109)querySelect(83-83)listRows(65-65)getRow(75-75)src/Migration/Sources/Appwrite/Reader/API.php (12)
report(28-118)queryLimit(239-242)queryEqual(221-224)queryCursorAfter(230-237)listDatabases(123-126)listTables(131-138)listColumns(146-154)listIndexes(162-170)getSupportForAttributes(244-247)querySelect(211-214)listRows(179-187)getRow(196-205)src/Migration/Cache.php (1)
get(188-195)
src/Migration/Resources/Database/Database.php (3)
src/Migration/Sources/Appwrite.php (1)
getDatabase(1764-1772)src/Migration/Sources/Appwrite/Reader/Database.php (1)
getDatabase(40-47)src/Migration/Resources/Database/Table.php (1)
getDatabase(91-94)
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
983-983: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (6)
src/Migration/Resources/Database/Collection.php (1)
9-12: Resource name OKReturns the correct type for Collection.
src/Migration/Resources/Database/Database.php (1)
98-102: Getter addition LGTMgetDatabase() accessor is straightforward and consistent.
src/Migration/Destinations/Appwrite.php (1)
121-122: Support matrix updatedIncluding TYPE_DOCUMENTSDB_DATABASE here is correct.
src/Migration/Sources/Appwrite.php (3)
76-83: Constructor parameter order fixedRequired callable precedes optional args; resolves PHP 8.1 deprecation.
631-657: Conditional export split LGTMBranching to export relational vs documents dbs is correct and aligns with cache usage.
697-705: Legacy constants verified Resource::TYPE_DATABASE_LEGACY and Resource::TYPE_DATABASE_TABLESDB are declared in src/Migration/Resource.php; no action needed.
src/Migration/Sources/Appwrite.php
Outdated
| if ($databaseType === Resource::TYPE_DATABASE_LEGACY || $databaseType === Resource::TYPE_DATABASE_TABLESDB) { | ||
| $databaseType = Resource::TYPE_DATABASE; | ||
| } | ||
|
|
||
| $databases[] = $newDatabase; | ||
| if (Resource::isSupported($databaseType, $resources)) { | ||
| $newDatabase = self::getDatabase($databaseType, [ | ||
| 'id' => $database['$id'], | ||
| 'name' => $database['name'], | ||
| 'createdAt' => $database['$createdAt'], | ||
| 'updatedAt' => $database['$updatedAt'], | ||
| 'type' => $databaseType, | ||
| 'database' => $database['database'] | ||
| ]); | ||
| $databases[] = $newDatabase; |
There was a problem hiding this comment.
DSN may be absent in API mode; add safe default
When using APIReader, database['database'] might be missing. Default to empty string to avoid fromArray crash.
- 'database' => $database['database']
+ 'database' => $database['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.
| if ($databaseType === Resource::TYPE_DATABASE_LEGACY || $databaseType === Resource::TYPE_DATABASE_TABLESDB) { | |
| $databaseType = Resource::TYPE_DATABASE; | |
| } | |
| $databases[] = $newDatabase; | |
| if (Resource::isSupported($databaseType, $resources)) { | |
| $newDatabase = self::getDatabase($databaseType, [ | |
| 'id' => $database['$id'], | |
| 'name' => $database['name'], | |
| 'createdAt' => $database['$createdAt'], | |
| 'updatedAt' => $database['$updatedAt'], | |
| 'type' => $databaseType, | |
| 'database' => $database['database'] | |
| ]); | |
| $databases[] = $newDatabase; | |
| if ($databaseType === Resource::TYPE_DATABASE_LEGACY || $databaseType === Resource::TYPE_DATABASE_TABLESDB) { | |
| $databaseType = Resource::TYPE_DATABASE; | |
| } | |
| if (Resource::isSupported($databaseType, $resources)) { | |
| $newDatabase = self::getDatabase($databaseType, [ | |
| 'id' => $database['$id'], | |
| 'name' => $database['name'], | |
| 'createdAt' => $database['$createdAt'], | |
| 'updatedAt' => $database['$updatedAt'], | |
| 'type' => $databaseType, | |
| 'database' => $database['database'] ?? '' | |
| ]); | |
| $databases[] = $newDatabase; |
🤖 Prompt for AI Agents
In src/Migration/Sources/Appwrite.php around lines 700 to 713, the code assumes
$database['database'] exists and will crash when absent (APIReader mode); change
the array entry to supply a safe default by using the null coalescing operator
(or equivalent) so 'database' => $database['database'] ?? '' is passed to
getDatabase, ensuring fromArray won't fail when the DSN/key is missing.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Migration/Destinations/Appwrite.php (2)
982-1001: Remove unused loop variable.The
$valuevariable is declared but never used in the loop. This is flagged by static analysis.Apply this diff:
- foreach ($row as $key => $value) { + foreach ($row as $key => $_) { if (\str_starts_with($key, '$')) { continue; }Based on learnings
446-449: DocumentsDB detection will never trigger; columns won't be skipped.The check uses
getName()which returns the resource type (TYPE_DATABASEorTYPE_DOCUMENTSDB_DATABASE) based on the polymorphic class type. However, when Table resources are deserialized viaTable::fromArray(), the database reference is always instantiated as aDatabaseobject (notDocumentsDB), sogetName()will returnTYPE_DATABASEeven for DocumentsDB databases.Use
getType()instead, which checks the database'stypeproperty:- if ($resource->getTable()->getDatabase()->getName() === Resource::TYPE_DOCUMENTSDB_DATABASE) { + if ($resource->getTable()->getDatabase()->getType() === 'documents') { $resource->setStatus(Resource::STATUS_SKIPPED, 'Columns not supported for DocumentsDB'); return false; }Note: If a constant exists for the database type 'documents', use that instead of the magic string for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Migration/Destinations/Appwrite.php(38 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
🧬 Code graph analysis (1)
src/Migration/Destinations/Appwrite.php (7)
src/Migration/Resources/Database/Row.php (4)
__construct(16-24)getTable(86-89)getName(76-79)Row(8-98)src/Migration/Resources/Database/Table.php (3)
__construct(20-32)getDatabase(91-94)Table(8-110)src/Migration/Sources/Appwrite/Reader/Database.php (3)
__construct(30-35)getDatabase(40-47)getSupportForAttributes(478-481)src/Migration/Sources/Appwrite.php (3)
__construct(76-108)getDatabase(1764-1772)getName(110-113)src/Migration/Cache.php (2)
__construct(24-27)get(188-195)src/Migration/Resources/Database/Database.php (3)
__construct(21-32)getDatabase(99-102)getName(74-77)src/Migration/Resource.php (2)
Resource(5-257)getName(129-129)
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
983-983: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (1)
src/Migration/Destinations/Appwrite.php (1)
60-100: LGTM! Constructor properly accepts per-database context support.The addition of the
$getDatabasesDBcallable and$dbForProjectparameter enables proper per-database context switching throughout the destination implementation.
…s database migration
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 (1)
src/Migration/Destinations/Appwrite.php (1)
75-109: Fix PHPDoc parameter name mismatch.The PHPDoc at line 81 references
$getDatabaseDSN, but the actual parameter at line 90 is named$getDatabasesDSN(note the extra 's'). This mismatch is causing the PHPStan pipeline failure.Apply this diff to fix the PHPDoc:
- * @param callable(string $databaseType):string $getDatabaseDSN + * @param callable(string $databaseType):string $getDatabasesDSN
♻️ Duplicate comments (1)
src/Migration/Destinations/Appwrite.php (1)
990-1011: Remove unused loop variable.Static analysis correctly identifies that
$valueat line 992 is unused. The loop only requires the key to check and remove orphan attributes.Apply this diff to remove the unused variable:
- foreach ($this->rowBuffer as $row) { - foreach ($row as $key => $value) { + foreach ($this->rowBuffer as $row) { + foreach ($row as $key => $_) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Migration/Destinations/Appwrite.php(38 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-28T09:45:57.650Z
Learnt from: ItzNotABug
PR: utopia-php/migration#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.
Applied to files:
src/Migration/Destinations/Appwrite.php
🧬 Code graph analysis (1)
src/Migration/Destinations/Appwrite.php (7)
src/Migration/Resources/Database/Database.php (4)
__construct(21-32)getType(94-97)getDatabase(99-102)getName(74-77)src/Migration/Resources/Database/Row.php (4)
__construct(16-24)getTable(86-89)getName(76-79)Row(8-98)src/Migration/Resources/Database/Table.php (3)
__construct(20-32)Table(8-110)getDatabase(91-94)src/Migration/Sources/Appwrite/Reader/Database.php (3)
__construct(30-35)getDatabase(40-47)getSupportForAttributes(478-481)src/Migration/Sources/Appwrite.php (3)
__construct(76-108)getDatabase(1764-1772)getName(110-113)src/Migration/Cache.php (2)
__construct(24-27)get(188-195)src/Migration/Resource.php (2)
Resource(5-257)getName(129-129)
🪛 GitHub Actions: CodeQL
src/Migration/Destinations/Appwrite.php
[error] 84-84: PHPStan: PHPDoc tag @param references unknown parameter: $getDatabaseDSN.
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
992-992: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (5)
src/Migration/Destinations/Appwrite.php (5)
119-149: LGTM!The addition of
Resource::TYPE_DOCUMENTSDB_DATABASEto the supported resources list correctly reflects the new DocumentsDB support introduced in this PR.
234-327: LGTM!The import flow correctly handles the per-project context (
$this->dbForProject) and routesResource::TYPE_DOCUMENTSDB_DATABASEto the appropriate handler.
334-383: LGTM!The
createDatabasemethod correctly uses the project-scoped context ($this->dbForProject) and invokes thegetDatabaseDSNcallable to populate the database field, enabling cross-location source/destination mapping.
723-927: LGTM!The
createIndexmethod correctly separates concerns: it uses$this->dbForProjectfor project-scoped document operations and$dbForDatabase(obtained via the callable) for database-scoped index creation and adapter-specific limits.
391-446: <!-- [scratchpad]
[task review_file_1/1 inspect_getCallbacks/2]
Inspect getDatabaseDSN usage.
[observations]
- Found assignment at lines 89-91.
- No calls to getDatabaseDSN in Appwrite.php.
[pending]- Determine intended use of getDatabaseDSN callback.
[analysis]- The class receives getDatabaseDSN but never uses it in createEntity.
- The getDatabasesDB callback likely includes DSN resolution.
- Cannot confirm that getDatabasesDB validates the DSN attribute.
[actions]- Recommend verifying that the getDatabasesDB implementation handles missing DSN.
[done]Verify DSN handling in getDatabasesDB callback.
Before callingcall_user_func($this->getDatabasesDB, $database), ensure the injected callback either:
- Validates
$database->getAttribute('database')(or database type) and throws a clear exception on missing/invalid DSN, or- Leverages
$this->getDatabaseDSNto resolve and validate the DSN.If the callback does not include these checks, add a pre-flight validation:
$databaseType = $database->getAttribute('database'); if (empty($databaseType)) { throw new Exception( resourceName: $resource->getName(), resourceGroup: $resource->getGroup(), resourceId: $resource->getId(), message: 'Database type attribute is missing or invalid', ); } $dsn = call_user_func($this->getDatabaseDSN, $databaseType);Then pass the full document to
getDatabasesDB.
ArnabChatterjee20k
changed the title
* updated resource exporting way and removed redundancy in code
* removed redundancy in exporting of entity driven resources
* fixed many to many relationship bug
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Migration/Sources/Appwrite.php (2)
637-712: Arrow function parameter mismatch inhandleExportEntityScopedResourcescauses runtime errors
$handleExportEntityScopedResourcesalways calls the callback with two arguments:($entityKey, $entityResource), but the callbacks forfieldandindexare declared with only one parameter. In modern PHP this results in anArgumentCountErrorat runtime.
field:fn ($entityKey) => $this->exportFields($entityKey, $batchSize)index:fn ($entityKey) => $this->exportIndexes($entityKey, $batchSize)Called as:
$callback($entityKey, $entityResource).Update those arrow functions to accept (and ignore) the second parameter.
- // field - if (!$handleExportEntityScopedResources('field', fn ($entityKey) => $this->exportFields($entityKey, $batchSize))) { + // field + if (!$handleExportEntityScopedResources( + 'field', + fn (string $entityKey, array $_) => $this->exportFields($entityKey, $batchSize) + )) { return; } - // index - if (!$handleExportEntityScopedResources('index', fn ($entityKey) => $this->exportIndexes($entityKey, $batchSize))) { + // index + if (!$handleExportEntityScopedResources( + 'index', + fn (string $entityKey, array $_) => $this->exportIndexes($entityKey, $batchSize) + )) { return; } // record - if (!$handleExportEntityScopedResources('record', fn ($entityKey, $entityResource) => $this->exportRecords($entityKey, $entityResource['field'], $batchSize))) { + if ( + !$handleExportEntityScopedResources( + 'record', + fn (string $entityKey, array $entityResource) => $this->exportRecords($entityKey, $entityResource['field'], $batchSize) + ) + ) { return; }
79-125: Critical:Appwriteconstructor calls have incorrect argument order and types in both README and MigrationCLIVerification reveals the new
callable $getDatabasesDBwiring is broken:
- README.md:32 – Only passes 3 arguments; missing the
$getDatabasesDBcallable entirely.- bin/MigrationCLI.php:213–219 – Arguments in wrong order and types:
- arg 4 is
Appwrite::SOURCE_DATABASE(string constant), but constructor expectscallable $getDatabasesDB- arg 5 is
$this->getDatabase('source')(database object), but constructor expectsstring $sourceBoth call sites must be corrected to match the new constructor signature:
__construct(string $project, string $endpoint, string $key, callable $getDatabasesDB, string $source = self::SOURCE_API, ?UtopiaDatabase $dbForProject = null, array $queries = []).
♻️ Duplicate comments (3)
src/Migration/Sources/Appwrite/Reader/Database.php (1)
25-47: Improve type safety and guard against empty database DSN.Two improvements needed for the per-database resolver:
- Type the callable explicitly instead of
mixedfor better type safety and IDE support.- Guard against empty DSN string to prevent factory invocation with invalid identifiers.
Apply this diff:
/** - * @var callable(UtopiaDocument|null): UtopiaDatabase + * @var ?callable(UtopiaDocument): UtopiaDatabase */ - private mixed $getDatabasesDB; + private $getDatabasesDB; public function __construct( private readonly UtopiaDatabase $dbForProject, ?callable $getDatabasesDB = null ) { $this->getDatabasesDB = $getDatabasesDB; } /** * Get the appropriate database instance for the given database DSN */ private function getDatabase(?string $databaseDSN = null): UtopiaDatabase { - if ($this->getDatabasesDB !== null && $databaseDSN !== null) { - return ($this->getDatabasesDB)(new UtopiaDocument(['database' => $databaseDSN])); + if ($this->getDatabasesDB !== null && !empty($databaseDSN)) { + return ($this->getDatabasesDB)(new UtopiaDocument(['database' => $databaseDSN])); } return $this->dbForProject; }This ensures the factory is only called with valid, non-empty DSN values and improves type clarity.
src/Migration/Sources/Appwrite.php (2)
721-784: Default DSN for API mode to avoid undefined index / type error inexportDatabasesIn
exportDatabases(), the payload forgetDatabase()unconditionally accesses$database['database']:'database' => $database['database']When using the API reader (
APIReader::listDatabases()), the Appwrite API response may not include adatabase/DSN field. In that case this access will:
- Trigger an undefined index notice.
- Pass
nullintoDatabase::fromArray(), which expects a non‑nullablestring $databaseand will cause aTypeError.Using a safe default (empty string) avoids crashes when DSN is not available, which matches the previous review guidance for this path.
- 'database' => $database['database'] + 'database' => $database['database'] ?? ''Also applies to: 751-767
791-858: Database type vs name confusion breaks DocumentsDB/VectorDB entity & record mappingThere are a couple of intertwined issues in
exportEntities()andexportRecords()that affect DocumentsDB/VectorDB:
Wrong discriminator passed into
getEntity()$newTable = self::getEntity($databaseName, [ ... ]);Here
$databaseNameis actually the cache key ('database' | 'documentsdb' | 'vectordb'), not necessarily the runtime type of$database. While it currently happens to match, it’s safer and clearer to pass the actual type from theDatabaseresource.Using
getDatabaseName()as the discriminator forgetRecord()$row = self::getRecord($table->getDatabase()->getDatabaseName(), [ ... ]);
getRecord()switches on the database type:switch ($databaseType) { case Resource::TYPE_DATABASE_DOCUMENTSDB: case Resource::TYPE_DATABASE_VECTORDB: return Document::fromArray($record); default: return Row::fromArray($record); }Passing
getDatabaseName()here couples behaviour to whatever was placed in thenamefield of the nesteddatabasearray inexportEntities(). With the current code, that field holds the cache key / type string rather than the human database name, and any future change (e.g. making it the real name as requested in previous reviews) will silently downgrade DocumentsDB/VectorDB records back toRow.To make this robust and match the intent of DocumentsDB/VectorDB:
- Use the database resource type (
$database->getType()) as the discriminator for both entities and records.- Store the real database name in the nested
database['name']field.Concretely:
- private function exportEntities(string $databaseName, int $batchSize): void + private function exportEntities(string $databaseName, int $batchSize): void { $databases = $this->cache->get($databaseName); foreach ($databases as $database) { /** @var Database $database */ @@ - $response = $this->reader->listTables($database, $queries); + $response = $this->reader->listTables($database, $queries); foreach ($response as $table) { - $newTable = self::getEntity($databaseName, [ + $newTable = self::getEntity($database->getType(), [ 'id' => $table['$id'], 'name' => $table['name'], 'documentSecurity' => $table['documentSecurity'], 'permissions' => $table['$permissions'], 'createdAt' => $table['$createdAt'], 'updatedAt' => $table['$updatedAt'], 'database' => [ 'id' => $database->getId(), - 'name' => $databaseName, - 'type' => $database->getType(), - 'database' => $database->getDatabase(), + // Human-readable database name + 'name' => $database->getDatabaseName(), + // Discriminator for DocumentsDB / VectorDB behaviour + 'type' => $database->getType(), + // DSN / underlying database identifier + 'database' => $database->getDatabase(), ] ]);- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ + $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(), + 'name' => $table->getDatabase()->getDatabaseName(), 'type' => $table->getDatabase()->getType(), 'database' => $table->getDatabase()->getDatabase(), ] ], 'data' => $row, 'permissions' => $permissions ]);This aligns entity/record construction with
Resource::DATABASE_TYPE_RESOURCE_MAPand ensures DocumentsDB/VectorDB rows becomeDocumentresources as intended, independent of how the human‑readable database name is set.Also applies to: 825-840, 1052-1068
🧹 Nitpick comments (14)
src/Migration/Resources/Database/VectorDB.php (1)
14-36: AlignfromArrayphpdoc with actual array shape
fromArray()reads$array['type']but the phpdoc for$arraydoesn’t declare atypekey, while other keys that are treated as optional in code (createdAt,updatedAt,enabled,originalId,database) are documented as mandatory.Consider updating the docblock to something like:
/** * @param array{ * id: string, * name: string, * type?: string, * createdAt?: string, * updatedAt?: string, * enabled?: bool, * originalId?: string|null, * database?: string * } $array */This keeps static analysis and IDE hints in sync with the actual usage.
src/Migration/Resources/Database/Attribute/Email.php (1)
8-36: Consider addingfromArrayforIPto match other attribute subclasses
IP) currently only customize the constructor andgetType(), while many other attribute classes (e.g.Integer,Point,Polygon,ObjectType,Line,Vector,Decimal) also expose afromArray()factory that mirrors their JSON shape.If you expect to deserialize email/IP attributes from arrays the same way, it may be worth adding:
public static function fromArray(array $array): self { return new self( $array['key'], Collection::fromArray($array['table'] ?? $array['collection']), required: $array['required'], default: $array['default'] ?? null, array: $array['array'], size: $array['size'] ?? 254, createdAt: $array['createdAt'] ?? '', updatedAt: $array['updatedAt'] ?? '', ); }(and the analogous method for
IP) to keep the attribute API consistent.src/Migration/Resources/Database/Attribute/Integer.php (1)
10-106: RevisitIntegermin/max defaults,sizeinference, and accessorsThe implementation works, but a few details could be sharpened:
- Unsigned vs signed defaults: When
$min/$maxare omitted, you always fall back toPHP_INT_MIN/PHP_INT_MAX, regardless of$signed. Forsigned === falseit’s more intuitive for the default range to start at 0. Example:if ($min === null) { $min = $signed ? PHP_INT_MIN : 0; } if ($max === null) { $max = PHP_INT_MAX; // or a DB-specific upper bound }
sizecalculation andsigned:sizeis currently8whenever$max > 2147483647. This may over‑promote unsigned 32‑bit ranges (0–4,294,967,295) to 8 bytes even though many engines store them in 4 bytes regardless of signedness. If you care about mirroring the storage type, you might want to factor$signed(or explicit DB type info) into this decision instead of a hard threshold.
getMin/getMaxreturn types: Because the constructor always fillsformatOptions['min']/['max'],getMin()/getMax()never actually returnnull, despite the?intreturn type. You could either:
- change the signatures to
int, or- allow
nullto propagate (skip the defaults) if “no constraint” is a valid state you care about.None of these are blockers, but tightening them would make the API more predictable for consumers.
src/Migration/Resources/Database/DocumentsDB.php (1)
14-36: AlignDocumentsDB::fromArrayphpdoc with its actual input shapeThe implementation assumes optional
typeanddatabasekeys and falls back to'legacy', but the phpdoc doesn’t mentiontypeat all and documentsdatabaseas required. This makes the declared array shape stricter/different than reality and can confuse static analysis or callers.Consider updating the phpdoc to reflect the real, optional shape, e.g.:
- * originalId: string|null, - * database: string + * originalId: string|null, + * type?: string, + * database?: stringsrc/Migration/Resources/Database/Attribute/Decimal.php (1)
41-88: MakeDecimal::fromArraymore robust and consistent withInteger
fromArray()currently assumesformatOptions['min']and['max']are always present:min: $array['formatOptions']['min'], max: $array['formatOptions']['max'],Integer’s
fromArray()uses?? null, which is safer if the keys are missing or coming from older payloads. You could mirror that here:- min: $array['formatOptions']['min'], - max: $array['formatOptions']['max'], + min: $array['formatOptions']['min'] ?? null, + max: $array['formatOptions']['max'] ?? null,This keeps the constructor signature unchanged while avoiding notices on partial inputs.
src/Migration/Resources/Database/Attribute/Boolean.php (1)
10-71: Boolean attribute wiring looks good; consider updating nesteddatabasedocConstructor and
fromArray()are consistent with the other simple attributes (e.g.DateTime), and the Collection resolution via$array['table'] ?? $array['collection']matches the new Collection model.The only minor nit is that the phpdoc for the nested
databasearray omits thetypefield, even thoughCollection::fromArray()now relies ondatabase.type. If all callers will provide that, you might want to reflect it in the docblock for static analysis.src/Migration/Resources/Database/Attribute/Vector.php (1)
10-71: Vector attribute implementation is consistent; docs could mentiondatabase.typeThe Vector attribute’s constructor and
fromArray()mirror the existingColumns\Vectorand other attribute implementations, and correctly useCollection::fromArray($array['table'] ?? $array['collection']). The use ofsizeanddefaultis straightforward and fits the baseAttributeAPI.As with
Boolean, the nesteddatabasephpdoc doesn’t mention thetypekey, even thoughCollection::fromArray()now expects it. If that key is required for all inputs, updating the docblocks here (and in other attribute classes) would keep array-shape annotations accurate.src/Migration/Transfer.php (1)
357-377: Remove unused variable.Static analysis correctly identifies that
$groupDatabasesIndex(line 360) is assigned but never used. Remove it unless you plan to implement additional grouping logic.Apply this diff:
public static function extractServices(array $services): array { $resources = []; - $groupDatabasesIndex = array_search(Transfer::GROUP_DATABASES, $services); foreach ($services as $service) { $resources = match ($service) {src/Migration/Destinations/Appwrite.php (3)
799-807: Remove unused spatial support variable.Static analysis correctly identifies that
$supportForSpatialAttributes(line 799) is assigned but never used. The variable can be removed safely.Apply this diff:
$maxIndexLength = $dbForDatabases->getAdapter()->getMaxIndexLength(); $internalIndexesKeys = $dbForDatabases->getAdapter()->getInternalIndexesKeys(); $supportForIndexArray = $dbForDatabases->getAdapter()->getSupportForIndexArray(); - $supportForSpatialAttributes = $dbForDatabases->getAdapter()->getSupportForSpatialAttributes(); $supportForSpatialIndexNull = $dbForDatabases->getAdapter()->getSupportForSpatialIndexNull();
933-954: Remove unused loop variable.The loop at line 935 declares
$valuebut only uses$key. Use the underscore convention to indicate the value is intentionally unused.Apply this diff:
foreach ($this->rowBuffer as $row) { - foreach ($row as $key => $value) { + foreach ($row as $key => $_) { if (\str_starts_with($key, '$')) { continue; }
1403-1493: Consider using extracted column size for validation.In
validateFieldsForIndexes(), the variable$columnSize(line 1465) is extracted but never used. If column size validation is needed for index creation, implement it; otherwise, remove the unused variable assignment.If size validation is not needed, apply this diff:
$columnStatus = $oldColumns[$columnIndex]['status']; $columnType = $oldColumns[$columnIndex]['type']; - $columnSize = $oldColumns[$columnIndex]['size']; $columnArray = $oldColumns[$columnIndex]['array'] ?? false;If you plan to add size-based validation for indexes in the future, keep the variable and add a comment explaining the deferred validation.
src/Migration/Sources/Appwrite.php (3)
865-914: Export flows for fields, indexes, and records: overall logic good after argument fixOnce the callback arity issue is fixed (see earlier comment), the refactored
exportFields,exportIndexes, andexportRecordsflows look consistent:
- They rely on
Resource::ENTITY_TYPE_RESOURCE_MAPand the cache keys to decide which entities to process.- The decision to treat VectorDB fields as attributes (
getAttribute) while keeping others as columns (getColumn) matches the resource maps and the retrieved learnings about terminology vs underlying data.- The many‑to‑many handling in
exportRecordsremains scoped to schema‑based DBs and usesgetSupportForAttributes()so it won't run against unsupported adapters.No additional functional issues stand out here beyond the database type/name confusion already covered for
getRecord().Also applies to: 921-966, 974-1081
1499-1521: Helper factoriesgetDatabase/getEntity/getRecord: types and mappings align with resource constantsThese private helpers correctly centralize mapping between raw arrays and typed resources:
getDatabase()switches on the database type and returnsDatabase,DocumentsDB, orVectorDB.getEntity()switches on database type and returnsTableorCollection.getRecord()switches on database type and returnsRoworDocument.With the earlier suggested change to pass
Database::getType()intogetEntity()/getRecord(), these helpers give you a clean single point for resource fan‑out when new database types are added.Also applies to: 1539-1592
1593-1756: Column/attribute factory helpers look consistent with prior naming & relationship semanticsThe new
getColumn()andgetAttribute()helpers:
- Cover all relevant scalar and geometric types (string/email/enum/url/ip, bool, int, float/decimal, datetime, point/line/polygon, object, vector).
- Correctly thread through
required,default,array,size,min/max, and timestamps with sensible fallbacks.- Honour the existing relationship semantics by preferring
relatedTablebut falling back torelatedCollection, which matches the documented pattern of using legacy keys under the hood while exposing new terminology.
This is in line with the retrieved learnings about gradual terminology migration in this codebase.I don’t see functional issues here; these helpers should make future extensions much easier.
Also applies to: 1758-1920
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
composer.json(1 hunks)src/Migration/Cache.php(1 hunks)src/Migration/Destinations/Appwrite.php(41 hunks)src/Migration/Resource.php(3 hunks)src/Migration/Resources/Database/Attribute.php(1 hunks)src/Migration/Resources/Database/Attribute/Boolean.php(1 hunks)src/Migration/Resources/Database/Attribute/DateTime.php(1 hunks)src/Migration/Resources/Database/Attribute/Decimal.php(1 hunks)src/Migration/Resources/Database/Attribute/Email.php(1 hunks)src/Migration/Resources/Database/Attribute/Enum.php(1 hunks)src/Migration/Resources/Database/Attribute/IP.php(1 hunks)src/Migration/Resources/Database/Attribute/Integer.php(1 hunks)src/Migration/Resources/Database/Attribute/Line.php(1 hunks)src/Migration/Resources/Database/Attribute/ObjectType.php(1 hunks)src/Migration/Resources/Database/Attribute/Point.php(1 hunks)src/Migration/Resources/Database/Attribute/Polygon.php(1 hunks)src/Migration/Resources/Database/Attribute/Relationship.php(1 hunks)src/Migration/Resources/Database/Attribute/Text.php(1 hunks)src/Migration/Resources/Database/Attribute/URL.php(1 hunks)src/Migration/Resources/Database/Attribute/Vector.php(1 hunks)src/Migration/Resources/Database/Collection.php(1 hunks)src/Migration/Resources/Database/Column.php(1 hunks)src/Migration/Resources/Database/Columns/ObjectType.php(1 hunks)src/Migration/Resources/Database/Columns/Vector.php(1 hunks)src/Migration/Resources/Database/DocumentsDB.php(1 hunks)src/Migration/Resources/Database/VectorDB.php(1 hunks)src/Migration/Sources/Appwrite.php(19 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(7 hunks)src/Migration/Transfer.php(5 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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: 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.
📚 Learning: 2025-06-28T09:47:08.333Z
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.
Applied to files:
src/Migration/Resources/Database/Column.phpsrc/Migration/Resources/Database/Columns/ObjectType.phpsrc/Migration/Resources/Database/Attribute/Line.phpsrc/Migration/Resources/Database/Attribute/Polygon.phpsrc/Migration/Resources/Database/Attribute/Text.phpsrc/Migration/Resources/Database/Attribute/Vector.phpsrc/Migration/Resources/Database/VectorDB.phpsrc/Migration/Resources/Database/DocumentsDB.phpsrc/Migration/Resource.phpsrc/Migration/Resources/Database/Collection.phpsrc/Migration/Resources/Database/Attribute/Relationship.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Resources/Database/Attribute/ObjectType.phpsrc/Migration/Transfer.phpsrc/Migration/Resources/Database/Attribute.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Destinations/Appwrite.php
📚 Learning: 2025-06-28T09:47:58.757Z
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.
Applied to files:
src/Migration/Resources/Database/Attribute/Line.phpsrc/Migration/Cache.phpsrc/Migration/Resources/Database/DocumentsDB.phpsrc/Migration/Resource.phpsrc/Migration/Resources/Database/Collection.phpsrc/Migration/Resources/Database/Attribute/Relationship.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Resources/Database/Attribute/ObjectType.phpsrc/Migration/Transfer.phpsrc/Migration/Resources/Database/Attribute.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Destinations/Appwrite.php
📚 Learning: 2025-07-30T12:06:02.331Z
Learnt from: abnegate
Repo: utopia-php/migration PR: 0
File: :0-0
Timestamp: 2025-07-30T12:06:02.331Z
Learning: In the utopia-php/migration codebase, the Utopia Database package does not have a Memory adapter. When testing classes that require a Database instance (like CSV), use PHPUnit's createMock() method to create proper mocks instead of trying to instantiate real database adapters.
Applied to files:
src/Migration/Resources/Database/VectorDB.phpsrc/Migration/Resources/Database/DocumentsDB.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Destinations/Appwrite.php
📚 Learning: 2025-06-28T09:45:57.650Z
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.
Applied to files:
src/Migration/Resources/Database/Collection.phpsrc/Migration/Resources/Database/Attribute/Relationship.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Destinations/Appwrite.php
📚 Learning: 2025-06-28T09:45:36.026Z
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.
Applied to files:
src/Migration/Resources/Database/Collection.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Destinations/Appwrite.php
📚 Learning: 2025-06-28T09:47:11.436Z
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Sources/Appwrite/Reader/API.php:8-8
Timestamp: 2025-06-28T09:47:11.436Z
Learning: In the Appwrite migration codebase, commented-out Tables service references (import statements and constructor parameters) are intentionally kept for future implementation when the Tables service becomes available in the Appwrite SDK, rather than being dead code that should be removed.
Applied to files:
src/Migration/Sources/Appwrite.php
🧬 Code graph analysis (26)
src/Migration/Resources/Database/Columns/ObjectType.php (3)
src/Migration/Resources/Database/Column.php (3)
Column(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Table.php (1)
Table(8-110)src/Migration/Resources/Database/Columns/Vector.php (3)
__construct(10-28)fromArray(61-72)getType(74-77)
src/Migration/Resources/Database/Attribute/Email.php (3)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (1)
Collection(7-50)src/Migration/Resources/Database/Attribute/Text.php (3)
Text(9-97)__construct(11-33)getType(83-86)
src/Migration/Resources/Database/Attribute/Line.php (3)
src/Migration/Resources/Database/Attribute.php (2)
Attribute(8-158)__construct(43-58)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)src/Migration/Resources/Database/Attribute/Vector.php (2)
__construct(10-28)fromArray(61-72)
src/Migration/Cache.php (2)
src/Migration/Resources/Database/Database.php (2)
getDatabase(99-102)getType(94-97)src/Migration/Resources/Database/Table.php (1)
getDatabase(91-94)
src/Migration/Resources/Database/Columns/Vector.php (4)
src/Migration/Resources/Database/Column.php (3)
Column(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Table.php (1)
Table(8-110)src/Migration/Resources/Database/Attribute/Vector.php (4)
Vector(8-78)__construct(10-28)fromArray(61-72)getType(74-77)src/Migration/Resources/Database/Columns/ObjectType.php (3)
__construct(10-26)fromArray(58-68)getType(70-73)
src/Migration/Resources/Database/Attribute/Polygon.php (3)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)src/Migration/Resources/Database/Attribute/Point.php (3)
__construct(10-26)fromArray(58-68)getType(70-73)
src/Migration/Resources/Database/Attribute/Point.php (2)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)
src/Migration/Resources/Database/Attribute/Text.php (2)
src/Migration/Resources/Database/Attribute.php (5)
Attribute(8-158)__construct(43-58)getType(88-88)getSize(105-108)getFormat(130-133)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)
src/Migration/Resources/Database/Attribute/Decimal.php (3)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)src/Migration/Resources/Database/Attribute/Integer.php (5)
__construct(10-41)fromArray(78-91)getType(93-96)getMin(98-101)getMax(103-106)
src/Migration/Resources/Database/Attribute/Boolean.php (4)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)src/Migration/Resources/Database/Attribute/Vector.php (3)
__construct(10-28)fromArray(61-72)getType(74-77)src/Migration/Resources/Database/Attribute/DateTime.php (3)
__construct(10-29)fromArray(67-78)getType(31-34)
src/Migration/Resources/Database/Attribute/Vector.php (3)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)src/Migration/Resources/Database/Columns/Vector.php (4)
Vector(8-78)__construct(10-28)fromArray(61-72)getType(74-77)
src/Migration/Resources/Database/Attribute/Integer.php (4)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)src/Migration/Resources/Database/Attribute/Vector.php (3)
__construct(10-28)fromArray(61-72)getType(74-77)src/Migration/Resources/Database/Attribute/Decimal.php (5)
__construct(10-39)fromArray(76-89)getType(91-94)getMin(96-99)getMax(101-104)
src/Migration/Resources/Database/VectorDB.php (2)
src/Migration/Resource.php (1)
Resource(5-278)src/Migration/Resources/Database/Collection.php (1)
fromArray(31-49)
src/Migration/Resources/Database/Attribute/DateTime.php (2)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)
src/Migration/Resources/Database/DocumentsDB.php (3)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)getName(74-77)fromArray(45-57)src/Migration/Resource.php (2)
Resource(5-278)getName(150-150)src/Migration/Resources/Database/Collection.php (2)
getName(9-12)fromArray(31-49)
src/Migration/Resources/Database/Collection.php (5)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)getName(74-77)fromArray(45-57)src/Migration/Resource.php (2)
Resource(5-278)getName(150-150)src/Migration/Resources/Database/Table.php (3)
Table(8-110)getName(81-84)fromArray(50-62)src/Migration/Resources/Database/DocumentsDB.php (3)
getName(9-12)fromArray(25-37)DocumentsDB(7-38)src/Migration/Resources/Database/VectorDB.php (1)
VectorDB(7-38)
src/Migration/Resources/Database/Attribute/IP.php (3)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (1)
Collection(7-50)src/Migration/Resources/Database/Attribute/Text.php (3)
Text(9-97)__construct(11-33)getType(83-86)
src/Migration/Resources/Database/Attribute/Relationship.php (2)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)
src/Migration/Sources/Appwrite/Reader/Database.php (6)
src/Migration/Resources/Database/Database.php (3)
Database(19-103)__construct(21-32)getDatabase(99-102)src/Migration/Resources/Database/Collection.php (1)
Collection(7-50)src/Migration/Resources/Database/Document.php (1)
Document(7-51)src/Migration/Sources/Appwrite.php (2)
__construct(97-127)getDatabase(1511-1521)src/Migration/Resources/Database/Table.php (2)
__construct(20-32)getDatabase(91-94)src/Migration/Resource.php (1)
Resource(5-278)
src/Migration/Resources/Database/Attribute/ObjectType.php (2)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)
src/Migration/Transfer.php (1)
src/Migration/Resource.php (1)
Resource(5-278)
src/Migration/Resources/Database/Attribute/URL.php (3)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (1)
Collection(7-50)src/Migration/Resources/Database/Attribute/Text.php (2)
Text(9-97)getType(83-86)
src/Migration/Resources/Database/Attribute.php (3)
src/Migration/Resource.php (3)
Resource(5-278)getName(150-150)getGroup(152-152)src/Migration/Transfer.php (2)
Transfer(5-378)__construct(140-149)src/Migration/Resources/Database/Table.php (1)
Table(8-110)
src/Migration/Sources/Appwrite.php (5)
src/Migration/Sources/Appwrite/Reader/Database.php (14)
Database(23-506)report(49-157)queryLimit(478-481)queryEqual(423-426)queryCursorAfter(434-476)listDatabases(159-162)getDatabase(40-47)listTables(164-203)listColumns(205-265)listIndexes(267-312)getSupportForAttributes(483-486)querySelect(413-416)listRows(314-365)getRow(367-407)src/Migration/Resources/Database/Database.php (5)
Database(19-103)getDatabase(99-102)getType(94-97)getDatabaseName(84-87)fromArray(45-57)src/Migration/Resource.php (2)
Resource(5-278)getId(169-172)src/Migration/Sources/Appwrite/Reader.php (12)
report(21-21)queryLimit(108-108)queryEqual(92-92)queryCursorAfter(100-100)listDatabases(29-29)listTables(38-38)listColumns(47-47)listIndexes(56-56)getSupportForAttributes(109-109)querySelect(83-83)listRows(65-65)getRow(75-75)src/Migration/Sources/Appwrite/Reader/API.php (12)
report(28-118)queryLimit(239-242)queryEqual(221-224)queryCursorAfter(230-237)listDatabases(123-126)listTables(131-138)listColumns(146-154)listIndexes(162-170)getSupportForAttributes(244-247)querySelect(211-214)listRows(179-187)getRow(196-205)
src/Migration/Destinations/Appwrite.php (3)
src/Migration/Sources/Appwrite/Reader/Database.php (4)
Database(23-506)__construct(30-35)getDatabase(40-47)getSupportForAttributes(483-486)src/Migration/Sources/Appwrite.php (3)
__construct(97-127)getDatabase(1511-1521)getName(129-132)src/Migration/Cache.php (2)
__construct(24-27)get(189-196)
src/Migration/Resources/Database/Attribute/Enum.php (2)
src/Migration/Resources/Database/Attribute.php (3)
Attribute(8-158)__construct(43-58)getType(88-88)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)
🪛 PHPMD (2.15.0)
src/Migration/Transfer.php
360-360: Avoid unused local variables such as '$groupDatabasesIndex'. (undefined)
(UnusedLocalVariable)
src/Migration/Destinations/Appwrite.php
799-799: Avoid unused local variables such as '$supportForSpatialAttributes'. (undefined)
(UnusedLocalVariable)
935-935: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
1465-1465: Avoid unused local variables such as '$columnSize'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (23)
composer.json (1)
29-29: Verify rationale for downgrading utopia-php/database from 4. to 3.*.*This is a major version downgrade which is atypical. The AI summary indicates substantial architectural changes (per-database contexts, new resource types). Ensure this downgrade is intentional and that version 3.* provides all necessary functionality without security or stability concerns.
Please clarify:
- Why is version 4.* incompatible with the new per-database migration architecture?
- Does version 3.* lack features introduced in 4.* that could impact this PR or dependent code?
- Are there any security considerations with using an older version?
Consider searching the PR description or related issues for context on this version constraint decision.
src/Migration/Resources/Database/Column.php (1)
25-26: New TYPE_OBJECT / TYPE_VECTOR constants are consistentThe added constants align with the new Column subclasses and mirror the Attribute type set; no issues spotted.
src/Migration/Resources/Database/Columns/ObjectType.php (1)
10-73: ObjectType column correctly wires into Column and legacy collection payloadsConstructor delegation looks correct, and
fromArray()cleanly supports bothtableand legacycollectionstructures while routing throughTable::fromArray(), matching the established migration pattern for terminology changes.getType()returningColumn::TYPE_OBJECTis consistent with the new column type set. Based on learningssrc/Migration/Resources/Database/Columns/Vector.php (1)
10-77: Vector column implementation is consistent with the attribute/vector ecosystemThe constructor and
fromArray()correctly mirror the existingAttribute\Vectorpattern, including support for bothtableand legacycollectionpayloads, andgetType()returningColumn::TYPE_VECTORties it neatly into the new type set. Based on learningssrc/Migration/Resources/Database/Attribute.php (1)
8-157: New Attribute base class cleanly mirrors Column and supports the table/collection migrationThe shared type constants, constructor shape, serialization, and getters are consistent with
Column, and usingTableas the associated entity while concrete subclasses acceptCollection(aTablesubclass) matches the intended terminology migration (table/column vs collection/attribute) without breaking legacy data flows. Based on learningssrc/Migration/Cache.php (1)
45-50: Including database type in TABLE/COLLECTION cache keys improves disambiguationPrefixing the key with
$resource->getDatabase()->getType()will prevent collisions between tables/collections that share sequences but live under different database types (e.g., traditional DB vs documents/vector DB), while leaving the rest of the cache behavior unchanged.src/Migration/Resources/Database/Attribute/URL.php (1)
8-36: URL attribute specialization is correctly layered on top of TextThe constructor cleanly reuses
Textwhile setting an appropriate default size andformat: 'url', andgetType()returningAttribute::TYPE_URLwires it into the Attribute type system as expected.src/Migration/Resources/Database/Attribute/IP.php (1)
8-36: IP attribute implementation looks consistent withTextThe IP attribute correctly specializes
Textby fixingformat: 'ip'and returningAttribute::TYPE_IPfromgetType(). This matches the baseAttribute/Textcontracts and json serialization behavior.No changes needed here from my side.
src/Migration/Resources/Database/Attribute/Line.php (1)
8-73:Linegeometry attribute matches the common attribute patternConstructor,
fromArray()(includingtable/collectionhandling), andgetType()are all consistent with the other geometry/object attributes (Point,Polygon,ObjectType). Nothing stands out as problematic here.src/Migration/Resources/Database/Attribute/Polygon.php (1)
8-73:Polygongeometry attribute implementation is consistent and straightforwardThe constructor,
fromArray()array shape, andgetType()correctly mirror the pattern used byPoint/Line, with TYPE_POLYGON as the discriminator. No issues spotted.src/Migration/Resources/Database/Attribute/ObjectType.php (1)
8-73:ObjectTypeattribute cleanly integrates with the sharedAttributeAPIThe constructor and
fromArray()mirror the patterns used by other complex attributes andgetType()correctly returnsAttribute::TYPE_OBJECT. This should fit smoothly into the existing serialization/deserialization flow.src/Migration/Resources/Database/Attribute/Point.php (1)
8-73:Pointgeometry attribute aligns with other spatial typesConstructor,
fromArray()(includingtable/collectionhandling), andgetType()follow the same conventions asLineandPolygon, with TYPE_POINT as the discriminator. Looks good.src/Migration/Resources/Database/Attribute/Text.php (1)
1-97: LGTM! Attribute implementation follows established pattern.The Text attribute class correctly extends Attribute, implements the required abstract methods, and handles backward compatibility with both 'table' and 'collection' keys in fromArray. The constructor properly forwards all parameters to the parent using named arguments.
src/Migration/Resources/Database/Attribute/Enum.php (1)
1-102: LGTM! Enum attribute implementation is consistent.The Enum class correctly encapsulates enum-specific behavior by storing elements in formatOptions and provides proper accessor methods. The fromArray method handles backward compatibility with table/collection keys as expected.
src/Migration/Resources/Database/Attribute/DateTime.php (1)
1-79: LGTM! DateTime attribute is properly implemented.The DateTime class correctly sets the datetime filter and implements all required methods following the established attribute pattern.
src/Migration/Resource.php (1)
33-130: LGTM! Resource type mappings correctly support multiple database backends.The new database type constants (TYPE_DATABASE_DOCUMENTSDB, TYPE_DATABASE_VECTORDB) and their corresponding resource mappings enable the migration system to handle different database backends with appropriate entity/field/record terminology. The HACK comment on line 108 regarding the 'field' mapping for documentsdb is understood.
src/Migration/Resources/Database/Attribute/Relationship.php (1)
1-125: LGTM! Relationship attribute correctly implements terminology bridging.The Relationship class properly handles the terminology transition by accepting
relatedTablein the constructor but storing it asrelatedCollectionin the options (line 27), then providing fallback access ingetRelatedTable()(line 98). This design pattern maintains compatibility with existing data sources while using modern terminology in the API surface.Based on learnings.
src/Migration/Sources/Appwrite/Reader/Database.php (1)
49-505: Per-database context handling is well-implemented.The enhanced
report()method correctly enumerates databases, normalizes legacy types, and counts per-database resources using the appropriate database instances. ThelistRows(),getRow(), andcountResources()methods properly route through per-database contexts when DatabaseResource is provided, falling back to the project database for metadata operations.src/Migration/Transfer.php (1)
15-64: LGTM! New database-specific resource groups are well-organized.The addition of granular database groups (GROUP_DATABASES_TABLES_DB, GROUP_DATABASES_DOCUMENTS_DB, GROUP_DATABASES_VECTOR_DB) and their corresponding resource arrays enables selective targeting of specific database backends during migration. The structure is clear and consistent.
src/Migration/Destinations/Appwrite.php (3)
62-111: Per-database context infrastructure properly initialized.The constructor correctly accepts and stores both per-project (
$dbForProject) and per-database resolution callables ($getDatabasesDB,$getDatabaseDSN), enabling the destination to route operations to the appropriate database instance based on context.
289-327: Resource routing correctly handles multiple database types.The
importDatabaseResource()method properly routes the new database resource types (TYPE_DATABASE_DOCUMENTSDB, TYPE_DATABASE_VECTORDB) and uses updated method names (createEntity, createField, createRecord) that align with the abstraction layer.
458-463: Correctly skip column creation for DocumentsDB.The guard at line 460 properly uses
getType()to detect DocumentsDB and skip column creation, which is appropriate since DocumentsDB doesn't require explicit column definitions.src/Migration/Sources/Appwrite.php (1)
295-303: Database reporting delegation looks correctDelegating
reportDatabases()to$this->reader->report()is a nice simplification and keeps API vs DB modes consistent. As long as bothAPIReaderandDatabaseReaderimplementreport()for the new database types, this should be fine.
… connecting via databases dsn
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Migration/Sources/Appwrite.php (2)
825-838: Incorrect database name in nested payload.Line 834 uses
$databaseNamefor the'name'field, but$databaseNameis actually the database type (e.g.,TYPE_DATABASE,TYPE_DATABASE_DOCUMENTSDB) passed from line 683, not the actual database name. This will result in the wrong value being stored.Apply this diff to use the actual database name:
'database' => [ 'id' => $database->getId(), - 'name' => $databaseName, + 'name' => $database->getDatabaseName(), 'type' => $database->getType(), 'database' => $database->getDatabase(), ]
1050-1066: Wrong parameter passed togetRecord.Line 1050 calls
getRecordwith$table->getDatabase()->getDatabaseName(), butgetRecordexpects the database type as the first parameter (per the signature at line 1580). This will cause incorrect routing betweenDocumentandRowresources.Apply this diff to pass the database type:
- $row = self::getRecord($table->getDatabase()->getDatabaseName(), [ + $row = self::getRecord($table->getDatabase()->getType(), [ 'id' => $id, 'table' => [
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Migration/Sources/Appwrite.php(19 hunks)src/Migration/Sources/CSV.php(8 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.272Z
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/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.
📚 Learning: 2025-06-28T09:47:08.333Z
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.
Applied to files:
src/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-07-30T12:06:02.331Z
Learnt from: abnegate
Repo: utopia-php/migration PR: 0
File: :0-0
Timestamp: 2025-07-30T12:06:02.331Z
Learning: In the utopia-php/migration codebase, the Utopia Database package does not have a Memory adapter. When testing classes that require a Database instance (like CSV), use PHPUnit's createMock() method to create proper mocks instead of trying to instantiate real database adapters.
Applied to files:
src/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:47:58.757Z
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.
Applied to files:
src/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-11-20T13:07:39.272Z
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.272Z
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.
Applied to files:
src/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-07-02T06:24:50.844Z
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.
Applied to files:
src/Migration/Sources/CSV.php
📚 Learning: 2025-06-28T09:45:36.026Z
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.
Applied to files:
src/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-11-20T13:12:43.446Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/migration PR: 114
File: src/Migration/Resources/Database/Collection.php:14-37
Timestamp: 2025-11-20T13:12:43.446Z
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.
Applied to files:
src/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:47:11.436Z
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Sources/Appwrite/Reader/API.php:8-8
Timestamp: 2025-06-28T09:47:11.436Z
Learning: In the Appwrite migration codebase, commented-out Tables service references (import statements and constructor parameters) are intentionally kept for future implementation when the Tables service becomes available in the Appwrite SDK, rather than being dead code that should be removed.
Applied to files:
src/Migration/Sources/Appwrite.php
📚 Learning: 2025-06-28T09:45:57.650Z
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.
Applied to files:
src/Migration/Sources/Appwrite.php
🧬 Code graph analysis (2)
src/Migration/Sources/CSV.php (6)
src/Migration/Resource.php (2)
Resource(5-278)isSupported(154-167)src/Migration/Sources/Appwrite.php (3)
Appwrite(72-1920)getEntity(1539-1549)getRecord(1580-1590)src/Migration/Sources/Appwrite/Reader/Database.php (3)
listDatabases(159-162)queryEqual(423-426)queryLimit(478-481)src/Migration/Sources/Appwrite/Reader/API.php (3)
listDatabases(123-126)queryEqual(221-224)queryLimit(239-242)src/Migration/Sources/Appwrite/Reader.php (3)
listDatabases(29-29)queryEqual(92-92)queryLimit(108-108)src/Migration/Resources/Database/Column.php (1)
Column(8-158)
src/Migration/Sources/Appwrite.php (7)
src/Migration/Sources/Appwrite/Reader/Database.php (14)
Database(23-506)report(49-157)queryLimit(478-481)queryEqual(423-426)queryCursorAfter(434-476)listDatabases(159-162)getDatabase(40-47)listTables(164-203)listColumns(205-265)listIndexes(267-312)getSupportForAttributes(483-486)querySelect(413-416)listRows(314-365)getRow(367-407)src/Migration/Resources/Database/Collection.php (2)
Collection(7-50)fromArray(31-49)src/Migration/Resources/Database/DocumentsDB.php (2)
DocumentsDB(7-38)fromArray(25-37)src/Migration/Resources/Database/Table.php (5)
Table(8-110)getDatabase(91-94)getTableName(96-99)getRowSecurity(101-104)fromArray(50-62)src/Migration/Resources/Database/VectorDB.php (1)
VectorDB(7-38)src/Migration/Resource.php (3)
Resource(5-278)getId(169-172)getPermissions(233-236)src/Migration/Resources/Database/Column.php (4)
getType(88-88)getTable(100-103)Column(8-158)getKey(95-98)
🔇 Additional comments (14)
src/Migration/Sources/CSV.php (6)
59-65: LGTM!The addition of
TYPE_DOCUMENTcorrectly expands support for document-based databases, aligning with the per-database context changes in this PR.
107-109: LGTM!The use of
isSupported()correctly checks whether the CSV source should handle any of the requested resources, properly utilizing the expanded resource type support.
136-149: LGTM!The database lookup logic correctly queries for the target database, validates its existence, and normalizes legacy database types to the current type constants.
151-175: LGTM!The payload construction correctly builds the nested database and table structures required by the
fromArraymethods, and thegetEntitycall properly routes to the appropriate resource type based on the database type.
340-347: LGTM!The extended type parsing correctly handles the new geometric and vector types by decoding JSON representations, with appropriate null-safe fallback when decoding fails.
355-360: LGTM!The
getRecordcall correctly constructs row/document resources with the appropriate payload structure, enabling proper routing betweenRowandDocumenttypes based on the database type.src/Migration/Sources/Appwrite.php (8)
97-127: LGTM!The constructor correctly accepts the
$getDatabasesDBcallable as a required parameter before optional parameters, and properly initializes the reader based on the source type, enabling per-database context routing.
157-160: LGTM!The addition of
TYPE_DATABASE_DOCUMENTSDBandTYPE_DATABASE_VECTORDBto supported resources correctly expands the source's capabilities to handle document and vector databases.
299-302: LGTM!The delegation to
$this->reader->report()correctly abstracts the reporting logic, allowing different implementations for API and Database readers.
639-714: LGTM!The refactored export logic elegantly handles multiple database types through
DATABASE_TYPE_RESOURCE_MAPiteration and the helper closure pattern, making the code more maintainable and extensible for future database types.
757-764: Verify thatdatabasekey exists in API mode.Line 763 accesses
$database['database']without a null coalescing operator. If this key is missing (particularly in API mode), this will cause a PHP notice or error.Apply this diff if the key might be absent:
- 'database' => $database['database'] + 'database' => $database['database'] ?? ''
1509-1519: LGTM!The helper methods (
getDatabase,getEntity,getRecord) correctly route to appropriate resource classes based on database type, providing a clean abstraction layer for per-database resource construction.Also applies to: 1539-1549, 1580-1590
1592-1755: LGTM!The
getColumnmethod comprehensively handles all column types with appropriate parameter mapping, including proper fallback for legacy keys (e.g.,relatedCollection→relatedTable) and sensible defaults for missing optional fields.
1757-1919: LGTM!The
getAttributemethod mirrors thegetColumnimplementation for document/vector database contexts, maintaining consistency across the parallel Column/Attribute implementations. Based on learnings.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.