GeometryCast - don't parse Geometry #151
Conversation
📝 WalkthroughWalkthroughAdds an early-return to GeometryCast::get: if the input is already a Geometry instance, assert its type and return it immediately; otherwise existing null handling and WKT/WKB parsing remain unchanged. No public API or set() behavior changes. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GeometryCast
participant Parser as WKT/WKB Parser
Caller->>GeometryCast: get(value)
alt value instanceof Geometry
GeometryCast-->>Caller: assertGeometryType(value) then return value
else
GeometryCast->>Parser: parse(value)
Parser-->>GeometryCast: Geometry
GeometryCast-->>Caller: return parsed Geometry (after type assertion)
end
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Cast/GeometryCast.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Cast/GeometryCast.php (2)
src/Data/Geometries/Geometry.php (1)
Geometry(13-73)tests/Models/Geometry.php (1)
Geometry(10-24)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Cast/GeometryCast.php (2)
41-46: Document thrown exception in get()/set() PHPDocBoth methods can now throw
\InvalidArgumentExceptionwhen an already-instantiated Geometry doesn’t match the configured cast type. Please reflect this in the docblocks./** * Cast the given value. * * @param Model $model + * @throws \InvalidArgumentException If the provided Geometry does not match the configured cast type */ public function get($model, string $key, mixed $value, array $attributes)/** * Prepare the given value for storage. * * @param Model $model + * @throws \InvalidArgumentException If the provided Geometry does not match the configured cast type */ public function set($model, string $key, mixed $value, array $attributes)Also applies to: 70-75
51-54: Add regression tests for subtype mismatch and pass-throughRecommend adding tests to cover:
- Dirty attribute contains a wrong Geometry subtype for a specific cast (expect
InvalidArgumentException).- Dirty attribute contains the correct subtype (expect the same instance to be returned, no parsing).
I can draft these tests against an example model (e.g., point field cast to
Point::class) if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Cast/GeometryCast.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Cast/GeometryCast.php (2)
src/Data/Geometries/Geometry.php (1)
Geometry(13-73)tests/Models/Geometry.php (1)
Geometry(10-24)
🔇 Additional comments (1)
src/Cast/GeometryCast.php (1)
51-54: Good fix: assert subtype before early-returning GeometryEarly return now enforces the caster’s target subtype via
assertGeometryType()before returning, which both fixes the parsing error and preserves the cast contract. Looks correct and minimal.
|
Hey! Thanks for the contribution, would you mind adding a test case for this? Thanks! |
|
Ok silly me. I was working off 2.0.0 not the main branch and the last pull request #148 fixed this problem. Sorry to waste your time and thank you for the package. |
|
No worries! Thanks! That reminds me to do a release :) |
2 participants
Models on
Model::create()will return the dirty attribute, not data from the database.This change prevents a $value that is already a Geometry from being passed to the parser and causing an error.
{ "message": "Cannot use object of type Clickbar\\Magellan\\Data\\Geometries\\MultiPoint as array", "exception": "Error", "file": "/Users/Gnative/Development/Sites/Contours/source/http/contours/vendor/clickbar/laravel-magellan/src/IO/Parser/WKB/Scanner.php", "line": 17, "trace": [ { "file": "/Users/Gnative/Development/Sites/Contours/source/http/contours/vendor/clickbar/laravel-magellan/src/IO/Parser/WKB/WKBParser.php", "line": 28, "function": "__construct", "class": "Clickbar\\Magellan\\IO\\Parser\\WKB\\Scanner", "type": "->" }, { "file": "/Users/Gnative/Development/Sites/Contours/source/http/contours/vendor/clickbar/laravel-magellan/src/Cast/GeometryCast.php", "line": 52, "function": "parse", "class": "Clickbar\\Magellan\\IO\\Parser\\WKB\\WKBParser", "type": "->" }, { "file": "/Users/Gnative/Development/Sites/Contours/source/http/contours/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php", "line": 886, "function": "get", "class": "Clickbar\\Magellan\\Cast\\GeometryCast", "type": "->" }, ] }Summary by CodeRabbit