Revert #127
Merged
Revert #127
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughThis PR removes legacy database types and many database-related resource classes (attributes, collection, document, DocumentsDB, VectorDB), simplifies cache key generation by removing database type, and refactors source/destination readers and exporters to use a single concrete UtopiaDatabase instance. Constructors and numerous method signatures were changed (notably in Appwrite destination and Appwrite/Database readers), several public constants and resource groups in Transfer.php were removed, and CSV/source export flows were updated to use Database/Table/Row objects. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Migration/Sources/Appwrite/Reader/Database.php (1)
5-5: Remove unused importUtopia\Console.The
Consoleclass is imported but never used in this file. This is causing the pipeline lint failure.-use Utopia\Console;src/Migration/Sources/Appwrite.php (1)
1234-1234: Fix undefined variable$entityName.The variable
$entityNameis never defined inexportRows(), causing a PHP error. This was flagged by the PHPStan pipeline failure.This appears to be leftover from the entity-to-table terminology refactoring. Consider removing this debug log line or fixing the variable reference.
-$this->logDebugTrackedProject("exportRecords completed | Entity: {$entityName}"); +$this->logDebugTrackedProject("exportRows completed");
🧹 Nitpick comments (2)
src/Migration/Destinations/Appwrite.php (2)
799-802: Remove unused variable$columnSize.The
$columnSizevariable is assigned but never used. This was flagged by static analysis.$columnStatus = $oldColumns[$columnIndex]['status']; $columnType = $oldColumns[$columnIndex]['type']; -$columnSize = $oldColumns[$columnIndex]['size']; $columnArray = $oldColumns[$columnIndex]['array'] ?? false;
969-988: Remove unused$valuevariable in foreach loop.The
$valuevariable is never used in the loop body; only$keyis needed. This was flagged by static analysis.foreach ($this->rowBuffer as $row) { - foreach ($row as $key => $value) { + foreach ($row as $key => $_) { if (\str_starts_with($key, '$')) { continue; }
📜 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 (33)
src/Migration/Cache.php(0 hunks)src/Migration/Destinations/Appwrite.php(34 hunks)src/Migration/Resource.php(0 hunks)src/Migration/Resources/Database/Attribute.php(0 hunks)src/Migration/Resources/Database/Attribute/Boolean.php(0 hunks)src/Migration/Resources/Database/Attribute/DateTime.php(0 hunks)src/Migration/Resources/Database/Attribute/Decimal.php(0 hunks)src/Migration/Resources/Database/Attribute/Email.php(0 hunks)src/Migration/Resources/Database/Attribute/Enum.php(0 hunks)src/Migration/Resources/Database/Attribute/IP.php(0 hunks)src/Migration/Resources/Database/Attribute/Integer.php(0 hunks)src/Migration/Resources/Database/Attribute/Line.php(0 hunks)src/Migration/Resources/Database/Attribute/ObjectType.php(0 hunks)src/Migration/Resources/Database/Attribute/Point.php(0 hunks)src/Migration/Resources/Database/Attribute/Polygon.php(0 hunks)src/Migration/Resources/Database/Attribute/Relationship.php(0 hunks)src/Migration/Resources/Database/Attribute/Text.php(0 hunks)src/Migration/Resources/Database/Attribute/URL.php(0 hunks)src/Migration/Resources/Database/Attribute/Vector.php(0 hunks)src/Migration/Resources/Database/Collection.php(0 hunks)src/Migration/Resources/Database/Column.php(0 hunks)src/Migration/Resources/Database/Columns/ObjectType.php(0 hunks)src/Migration/Resources/Database/Columns/Vector.php(0 hunks)src/Migration/Resources/Database/Database.php(0 hunks)src/Migration/Resources/Database/Document.php(0 hunks)src/Migration/Resources/Database/DocumentsDB.php(0 hunks)src/Migration/Resources/Database/VectorDB.php(0 hunks)src/Migration/Sources/Appwrite.php(14 hunks)src/Migration/Sources/Appwrite/Reader.php(0 hunks)src/Migration/Sources/Appwrite/Reader/API.php(0 hunks)src/Migration/Sources/Appwrite/Reader/Database.php(5 hunks)src/Migration/Sources/CSV.php(7 hunks)src/Migration/Transfer.php(1 hunks)
💤 Files with no reviewable changes (28)
- src/Migration/Resources/Database/Collection.php
- src/Migration/Sources/Appwrite/Reader/API.php
- src/Migration/Resources/Database/Attribute/Decimal.php
- src/Migration/Resources/Database/Column.php
- src/Migration/Resources/Database/DocumentsDB.php
- src/Migration/Resources/Database/Columns/ObjectType.php
- src/Migration/Resources/Database/Attribute/Line.php
- src/Migration/Resources/Database/Attribute/URL.php
- src/Migration/Resources/Database/Attribute/IP.php
- src/Migration/Cache.php
- src/Migration/Resources/Database/Columns/Vector.php
- src/Migration/Resources/Database/Document.php
- src/Migration/Sources/Appwrite/Reader.php
- src/Migration/Resources/Database/VectorDB.php
- src/Migration/Resources/Database/Attribute/Enum.php
- src/Migration/Resources/Database/Attribute/Point.php
- src/Migration/Resources/Database/Attribute/ObjectType.php
- src/Migration/Resources/Database/Attribute/DateTime.php
- src/Migration/Resources/Database/Attribute/Email.php
- src/Migration/Resources/Database/Attribute/Vector.php
- src/Migration/Resources/Database/Database.php
- src/Migration/Resources/Database/Attribute/Integer.php
- src/Migration/Resources/Database/Attribute/Boolean.php
- src/Migration/Resource.php
- src/Migration/Resources/Database/Attribute/Relationship.php
- src/Migration/Resources/Database/Attribute/Polygon.php
- src/Migration/Resources/Database/Attribute/Text.php
- src/Migration/Resources/Database/Attribute.php
🧰 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: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Sources/Appwrite.php:843-851
Timestamp: 2025-06-28T09:47:08.333Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the class constructors and method parameters use the new terminology (like "relatedTable"), but the underlying data structures and API responses still use the legacy keys (like "relatedCollection"). This is an intentional design pattern to allow gradual migration while maintaining compatibility with existing data sources.
Learnt from: ItzNotABug
Repo: utopia-php/migration PR: 80
File: src/Migration/Sources/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.
📚 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/Destinations/Appwrite.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Transfer.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/Destinations/Appwrite.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Transfer.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/Destinations/Appwrite.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-11-20T13:07:39.293Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/migration PR: 114
File: src/Migration/Resources/Database/Attribute/Decimal.php:10-38
Timestamp: 2025-11-20T13:07:39.293Z
Learning: In the utopia-php/migration codebase, the user ArnabChatterjee20k maintains consistency between parallel implementations: Attribute classes (e.g., Attribute/Decimal.php) mirror their corresponding Column classes (e.g., Columns/Decimal.php) in constructor defaults and behavior, even when design decisions might be questioned.
Applied to files:
src/Migration/Destinations/Appwrite.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite/Reader/Database.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/Destinations/Appwrite.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/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/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/Sources/CSV.phpsrc/Migration/Sources/Appwrite/Reader/Database.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.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/Appwrite.php
📚 Learning: 2025-11-20T13:12:43.482Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/migration PR: 114
File: src/Migration/Resources/Database/Collection.php:14-37
Timestamp: 2025-11-20T13:12:43.482Z
Learning: In the utopia-php/migration codebase, when Collection::fromArray is called, the input array is guaranteed to contain both the 'database' key and the nested 'type' key ($array['database']['type']), so defensive guards for these keys are not necessary.
Applied to files:
src/Migration/Sources/CSV.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/Appwrite.php
🧬 Code graph analysis (4)
src/Migration/Destinations/Appwrite.php (9)
src/Migration/Sources/Appwrite.php (1)
getName(113-116)src/Migration/Resources/Database/Database.php (3)
getType(91-94)getName(71-74)getGroup(76-79)src/Migration/Resources/Database/Column.php (5)
Column(8-155)getType(85-85)getName(80-83)getGroup(87-90)getTable(97-100)src/Migration/Resources/Database/Index.php (4)
getType(129-132)getName(109-112)getGroup(114-117)getTable(124-127)src/Migration/Resources/Auth/User.php (2)
getName(83-86)getGroup(153-156)src/Migration/Exception.php (1)
Exception(5-65)src/Migration/Resource.php (5)
Resource(5-236)getSequence(151-154)getId(127-130)getName(108-108)getGroup(110-110)src/Migration/Resources/Database/Table.php (4)
Table(8-110)getName(81-84)getGroup(86-89)getDatabase(91-94)bin/MigrationCLI.php (1)
getDatabase(265-399)
src/Migration/Sources/Appwrite/Reader/Database.php (2)
src/Migration/Sources/Appwrite/Reader/API.php (3)
report(28-118)listDatabases(123-126)listTables(131-138)src/Migration/Resource.php (3)
Resource(5-236)isSupported(112-125)getId(127-130)
src/Migration/Transfer.php (1)
src/Migration/Exception.php (1)
Exception(5-65)
src/Migration/Sources/Appwrite.php (1)
src/Migration/Sources/Appwrite/Reader/Database.php (11)
queryLimit(413-416)queryEqual(371-374)queryCursorAfter(382-411)listDatabases(113-116)Database(23-428)listTables(118-157)listColumns(159-219)listIndexes(221-266)querySelect(361-364)listRows(268-316)getRow(318-355)
🪛 GitHub Actions: CodeQL
src/Migration/Sources/Appwrite.php
[error] 1234-1234: PHPStan: Undefined variable: $entityName.
🪛 GitHub Actions: Linter
src/Migration/Sources/Appwrite/Reader/Database.php
[error] 1-1: Pint lint failed. Command './vendor/bin/pint --test' returned exit code 1 due to PSR-12: no_unused_imports.
🪛 PHPMD (2.15.0)
src/Migration/Destinations/Appwrite.php
801-801: Avoid unused local variables such as '$columnSize'. (undefined)
(UnusedLocalVariable)
970-970: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (10)
src/Migration/Sources/Appwrite/Reader/Database.php (1)
25-27: LGTM!The simplified constructor accepting a single
UtopiaDatabaseinstance is cleaner and aligns with the database-centric refactoring approach.src/Migration/Sources/CSV.php (3)
43-53: LGTM!Constructor simplification and direct
DatabaseReaderinstantiation align well with the database-centric refactoring.
134-136: LGTM!Creating lightweight
DatabaseandTableobjects with just the required IDs is appropriate for the CSV import context where full metadata isn't needed.
314-319: LGTM!Row creation using the new
Rowresource class is cleaner than the previous approach.src/Migration/Transfer.php (2)
37-43: LGTM!The consolidated
GROUP_DATABASES_RESOURCESconstant properly reflects the simplified database resource model.
62-66: Good backward compatibility consideration.Keeping legacy types (
TYPE_DOCUMENT,TYPE_ATTRIBUTE,TYPE_COLLECTION) inALL_PUBLIC_RESOURCESmaintains backward compatibility for existing migrations.src/Migration/Destinations/Appwrite.php (2)
72-77: LGTM!The constructor now properly accepts a concrete
UtopiaDatabaseinstance, simplifying dependency management and aligning with the database-centric architecture.
426-443: LGTM!The column type mapping comprehensively covers all supported types with proper mapping to
UtopiaDatabaseconstants.src/Migration/Sources/Appwrite.php (2)
98-110: LGTM!The switch-based initialization properly guards against missing database for
SOURCE_DATABASEand provides clear error messaging.
1146-1165: LGTM!The many-to-many relationship handling properly filters for parent-side relationship columns before processing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.