added spatial type migration support#110
Conversation
WalkthroughAdds spatial support: new point/linestring/polygon constants in Column and Database, new Point/Line/Polygon column classes with constructors and fromArray factories, Appwrite source updated to instantiate them during export, destination mapping extended to UtopiaDatabase VAR_POINT/VAR_LINESTRING/VAR_POLYGON, and a spatial index type constant. Changes
Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Export as Appwrite::exportColumns
participant Switch as type switch
participant Point as Point
participant Line as Line
participant Poly as Polygon
rect rgba(220,240,255,0.25)
Export->>Switch: evaluate column.type
alt type == 'point'
Switch-->>Point: new Point(key, table, required, default, createdAt, updatedAt)
Switch-->>Export: add Point column
else type == 'linestring'
Switch-->>Line: new Line(key, table, required, default, createdAt, updatedAt)
Switch-->>Export: add Line column
else type == 'polygon'
Switch-->>Poly: new Polygon(key, table, required, default, createdAt, updatedAt)
Switch-->>Export: add Polygon column
else
Switch-->>Export: existing handling (unchanged)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
src/Migration/Sources/Appwrite.php (1)
951-977: Confirm whether spatial attributes can be arrays; constructors don’t accept array flag.Other types pass array: $column['array']; spatial constructors don’t expose it. If Appwrite never returns arrays for spatial types, ignore. Otherwise, extend constructors to accept the array flag for consistency.
src/Migration/Resources/Database/Columns/Polygon.php (1)
13-15: Tiny style nit: extra spacing around type hints.Let Pint normalize spacing.
Apply:
- bool $required = false, - ?array $default = null, + bool $required = false, + ?array $default = null,src/Migration/Resources/Database/Columns/Line.php (4)
1-3: Enable strict types.Add declare(strict_types=1); for safer scalar coercions.
<?php +declare(strict_types=1); namespace Utopia\Migration\Resources\Database\Columns;
28-56: Fix docblock accuracy and clarify default shape.
- Mark createdAt/updatedAt as optional (you already default them).
- Consider documenting the expected geometry shape for defaults (e.g., LineString: array of [x,y] tuples, optional srid).
/** * @param array{ ... - * required: bool, - * default: ?array<mixed>, - * createdAt: string, - * updatedAt: string, + * required: bool, + * default?: array<array<float>>|null, // e.g. [[x1,y1],[x2,y2],...], optional SRID via options/formatOptions + * createdAt?: string, + * updatedAt?: string, * } $array * @return self */
58-68: Harden fromArray: guard missing table/collection and coalesce optional fields.Prevents undefined-index notices and produces a clearer error when table metadata is absent.
return new self( $array['key'], - Table::fromArray($array['table'] ?? $array['collection']), - required: $array['required'], - default: $array['default'], - createdAt: $array['createdAt'] ?? '', - updatedAt: $array['updatedAt'] ?? '', + Table::fromArray($array['table'] ?? $array['collection'] ?? throw new \InvalidArgumentException('Missing table/collection for Line::fromArray')), + required: (bool)($array['required'] ?? false), + default: $array['default'] ?? null, + createdAt: (string)($array['createdAt'] ?? ''), + updatedAt: (string)($array['updatedAt'] ?? ''), );
8-9: Optional naming alignment with geometry terminology.Consider LineString (or an alias) to match the underlying 'linestring' type and common GIS vocabulary. Not blocking.
Example alias (outside this file or in a small stub file):
<?php namespace Utopia\Migration\Resources\Database\Columns; class LineString extends Line {}src/Migration/Resources/Database/Columns/Point.php (4)
1-3: Enable strict types.Add declare(strict_types=1); for consistency and safer type handling.
<?php +declare(strict_types=1); namespace Utopia\Migration\Resources\Database\Columns;
28-56: Tighten phpdoc and describe expected default.
- Mark createdAt/updatedAt optional.
- Document expected default as a [x,y] pair (and optional srid if applicable).
/** * @param array{ ... - * required: bool, - * default: ?array<mixed>, - * createdAt: string, - * updatedAt: string, + * required: bool, + * default?: array{0: float, 1: float}|null, // [x,y], optional SRID via options/formatOptions + * createdAt?: string, + * updatedAt?: string, * } $array * @return self */
58-68: Make fromArray resilient to incomplete payloads.Same hardening as Line: guard table/collection and coalesce optional fields.
return new self( $array['key'], - Table::fromArray($array['table'] ?? $array['collection']), - required: $array['required'], - default: $array['default'], - createdAt: $array['createdAt'] ?? '', - updatedAt: $array['updatedAt'] ?? '', + Table::fromArray($array['table'] ?? $array['collection'] ?? throw new \InvalidArgumentException('Missing table/collection for Point::fromArray')), + required: (bool)($array['required'] ?? false), + default: $array['default'] ?? null, + createdAt: (string)($array['createdAt'] ?? ''), + updatedAt: (string)($array['updatedAt'] ?? ''), );
8-26: Optional: validate default geometry shape.If you expect strict geometry shapes, consider normalizing/validating in the constructor or fromArray (e.g., ensure [x,y] floats for Point).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Migration/Resources/Database/Column.php(1 hunks)src/Migration/Resources/Database/Columns/Line.php(1 hunks)src/Migration/Resources/Database/Columns/Point.php(1 hunks)src/Migration/Resources/Database/Columns/Polygon.php(1 hunks)src/Migration/Sources/Appwrite.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/Migration/Resources/Database/Columns/Polygon.php (3)
src/Migration/Resources/Database/Column.php (3)
Column(8-155)__construct(40-55)getType(85-85)src/Migration/Resources/Database/Table.php (1)
Table(8-110)src/Migration/Resources/Database/Columns/Line.php (3)
__construct(10-26)fromArray(58-68)getType(70-73)
src/Migration/Resources/Database/Columns/Line.php (4)
src/Migration/Resources/Database/Column.php (3)
Column(8-155)__construct(40-55)getType(85-85)src/Migration/Resources/Database/Table.php (1)
Table(8-110)src/Migration/Resources/Database/Columns/Point.php (3)
__construct(10-26)fromArray(58-68)getType(70-73)src/Migration/Resources/Database/Columns/Polygon.php (3)
__construct(10-26)fromArray(58-68)getType(70-73)
src/Migration/Resources/Database/Columns/Point.php (4)
src/Migration/Resources/Database/Column.php (3)
Column(8-155)__construct(40-55)getType(85-85)src/Migration/Resources/Database/Table.php (1)
Table(8-110)src/Migration/Resources/Database/Columns/Line.php (3)
__construct(10-26)fromArray(58-68)getType(70-73)src/Migration/Resources/Database/Columns/Polygon.php (3)
__construct(10-26)fromArray(58-68)getType(70-73)
src/Migration/Sources/Appwrite.php (4)
src/Migration/Resources/Database/Columns/Line.php (1)
Line(8-74)src/Migration/Resources/Database/Columns/Point.php (1)
Point(8-74)src/Migration/Resources/Database/Columns/Polygon.php (1)
Polygon(8-74)src/Migration/Resources/Database/Column.php (1)
Column(8-155)
🪛 GitHub Actions: Linter
src/Migration/Sources/Appwrite.php
[error] 1-1: Pint lint failed. PSR-12 'no_break_comment' violation detected. Command './vendor/bin/pint --test handling the lint event' exited with code 1.
🔇 Additional comments (6)
src/Migration/Resources/Database/Column.php (1)
21-23: Spatial type constants added — looks good.The new POINT/LINE/POLYGON constants align with the added column classes and Appwrite export mapping.
src/Migration/Sources/Appwrite.php (1)
29-31: Imports for spatial columns — OK.New use statements correctly reference Line, Point, and Polygon.
src/Migration/Resources/Database/Columns/Polygon.php (1)
1-74: Polygon column implementation — solid.Constructor, fromArray, and getType are consistent with other column classes.
src/Migration/Resources/Database/Columns/Line.php (2)
8-26: Constructor looks good.Forwarding only the relevant args to the parent is consistent with other column types.
70-73: LGTM on type mapping.Returns Column::TYPE_LINE ("linestring") as expected.
src/Migration/Resources/Database/Columns/Point.php (1)
70-73: LGTM on type mapping.Returns Column::TYPE_POINT as intended.
| case Column::TYPE_POINT: | ||
| $col = new Point( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| case Column::TYPE_LINE: | ||
| $col = new Line( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); | ||
| case Column::TYPE_POLYGON: | ||
| $col = new Polygon( | ||
| $column['key'], | ||
| $table, | ||
| required: $column['required'], | ||
| default: $column['default'], | ||
| createdAt: $column['$createdAt'] ?? '', | ||
| updatedAt: $column['$updatedAt'] ?? '', | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reset $col each iteration to avoid stale value on unknown types.
Currently, if an unknown type appears, isset($col) can be true from a previous loop, silently appending the wrong Column. Initialize $col to null before the switch.
Add just before the switch:
$col = null;
switch ($column['type']) {
// ...
}🤖 Prompt for AI Agents
In src/Migration/Sources/Appwrite.php around lines 951 to 977, $col can retain a
value from a previous iteration if the current $column['type'] doesn't match any
case; initialize $col to null immediately before the switch to avoid appending a
stale Column for unknown types, and ensure any later logic that uses $col checks
for null before adding it to the collection.
Missing breaks cause fall-through (all spatial types become POLYGON) and trigger Pint no_break_comment.
Add breaks after each case. This fixes the functional bug and the linter error.
Apply:
case Column::TYPE_POINT:
$col = new Point(
$column['key'],
$table,
required: $column['required'],
default: $column['default'],
createdAt: $column['$createdAt'] ?? '',
updatedAt: $column['$updatedAt'] ?? '',
);
+ break;
case Column::TYPE_LINE:
$col = new Line(
$column['key'],
$table,
required: $column['required'],
default: $column['default'],
createdAt: $column['$createdAt'] ?? '',
updatedAt: $column['$updatedAt'] ?? '',
);
+ break;
case Column::TYPE_POLYGON:
$col = new Polygon(
$column['key'],
$table,
required: $column['required'],
default: $column['default'],
createdAt: $column['$createdAt'] ?? '',
updatedAt: $column['$updatedAt'] ?? '',
);
+ break;🤖 Prompt for AI Agents
In src/Migration/Sources/Appwrite.php around lines 951 to 977, each switch case
for spatial column types (POINT, LINE, POLYGON) is missing break statements
causing fall-through so all types become POLYGON and triggering the Pint
no_break_comment lint error; add a break; statement immediately after
constructing the $col for the POINT and LINE cases (and ensure break remains
after POLYGON if intended) so each case returns only its own instantiation and
the linter warning is resolved.
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)
432-447: Bug: broken match arm for string-like types (syntax/logic error).The grouped arm for TYPE_STRING/IP/EMAIL/URL lacks the => mapping, causing a parse error and/or incorrect mapping. Map them to VAR_STRING before the new spatial arms.
Apply this patch:
$type = match ($resource->getType()) { Column::TYPE_DATETIME => UtopiaDatabase::VAR_DATETIME, Column::TYPE_BOOLEAN => UtopiaDatabase::VAR_BOOLEAN, Column::TYPE_INTEGER => UtopiaDatabase::VAR_INTEGER, Column::TYPE_FLOAT => UtopiaDatabase::VAR_FLOAT, Column::TYPE_RELATIONSHIP => UtopiaDatabase::VAR_RELATIONSHIP, - Column::TYPE_STRING, - Column::TYPE_IP, - Column::TYPE_EMAIL, - Column::TYPE_URL, + Column::TYPE_STRING, + Column::TYPE_IP, + Column::TYPE_EMAIL, + Column::TYPE_URL + => UtopiaDatabase::VAR_STRING, Column::TYPE_POINT => UtopiaDatabase::VAR_POINT, Column::TYPE_LINE => UtopiaDatabase::VAR_LINESTRING, Column::TYPE_POLYGON => UtopiaDatabase::VAR_POLYGON, Column::TYPE_ENUM => UtopiaDatabase::VAR_STRING, default => throw new \Exception('Invalid resource type '.$resource->getType()), };
🧹 Nitpick comments (1)
src/Migration/Resources/Database/Database.php (1)
15-17: Non-blocking: Avoid duplicating type constants; rely on Column::TYPE_ instead.*These file-scope TYPE_POINT/TYPE_LINE/TYPE_POLYGON constants duplicate the Column class constants and can drift. Consider removing these and referencing Column::TYPE_* where needed.
📜 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 (3)
src/Migration/Destinations/Appwrite.php(1 hunks)src/Migration/Resources/Database/Database.php(1 hunks)src/Migration/Sources/Appwrite.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Migration/Sources/Appwrite.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/Migration/Destinations/Appwrite.php (1)
src/Migration/Resources/Database/Column.php (1)
Column(8-155)
| Column::TYPE_POINT => UtopiaDatabase::VAR_POINT, | ||
| Column::TYPE_LINE => UtopiaDatabase::VAR_LINESTRING, | ||
| Column::TYPE_POLYGON => UtopiaDatabase::VAR_POLYGON, | ||
| Column::TYPE_ENUM => UtopiaDatabase::VAR_STRING, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm dependency support for spatial VAR_ constants.*
Ensure your utopia-php/database dependency exposes VAR_POINT, VAR_LINESTRING, and VAR_POLYGON; otherwise this will fatal at runtime.
Run from repo root:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show required version constraint
fd -HI '^composer\.json$' -x cat {} | sed -n '1,200p' | rg -n '"utopia-php/database"'
# If composer.lock exists, print the resolved version
if [ -f composer.lock ]; then
python - <<'PY'
import json,sys
lock=json.load(open('composer.lock'))
for p in lock.get('packages',[]):
if p.get('name')=='utopia-php/database':
print('utopia-php/database resolved version:', p.get('version'))
break
PY
fiLength of output: 84
Ensure utopia-php/database supports spatial VAR_ constants*
Package utopia-php/database v2.0.0 lacks VAR_POINT, VAR_LINESTRING, and VAR_POLYGON; this will fatal at runtime — add these constants to the Database class or upgrade to a version that includes them. (github.com)
🤖 Prompt for AI Agents
In src/Migration/Destinations/Appwrite.php around lines 442 to 445, the code
maps spatial column types to UtopiaDatabase::VAR_POINT, VAR_LINESTRING, and
VAR_POLYGON but utopia-php/database v2.0.0 does not define these constants;
either (A) add these constants to the Database class in the utopia-php/database
package (declare public const VAR_POINT = 'point'; public const VAR_LINESTRING =
'linestring'; public const VAR_POLYGON = 'polygon') and run composer update in
this project, or (B) upgrade the utopia-php/database dependency to a version
that already includes these constants and update composer.json accordingly;
after either change, run composer install, update any imports/namespaces if
necessary, and run tests to verify no runtime fatal occurs.
2 participants
Summary by CodeRabbit