updated type of the spatial validator#687
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Pass old to update on next, rename to upsert
WalkthroughThe Spatial validator now reports it handles array data: isArray() returns true, and getType() returns TYPE_ARRAY. No other validation logic or public signatures were changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Database/Validator/Spatial.php (1)
113-117: Polygon closure check is too strict for numeric strings vs numbers.Using
!==can fail when first/last points are numerically equal but differ by type (e.g., "1" vs 1). Consider numeric comparison with tolerance.Apply:
- // Check that the ring is closed (first point == last point) - if ($ring[0] !== $ring[count($ring) - 1]) { + // Check that the ring is closed (first point ~= last point) + $first = array_map('floatval', $ring[0]); + $last = array_map('floatval', $ring[count($ring) - 1]); + $epsilon = 1e-9; + if (abs($first[0] - $last[0]) > $epsilon || abs($first[1] - $last[1]) > $epsilon) { $this->message = "Ring #{$ringIndex} must be closed (first point must equal last point)"; return false; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/Database/Validator/Spatial.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Database/Validator/Spatial.php (2)
src/Database/Validator/Queries.php (1)
getType(165-168)src/Database/Validator/Structure.php (1)
getType(427-430)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Setup & Build Docker Image
🔇 Additional comments (2)
src/Database/Validator/Spatial.php (2)
142-145: Confirm no external dependencies on the ‘spatial’ type
Ripgrep found nogetType() === 'spatial'checks orTYPE_SPATIALusages in src/; sincegetType()now returnsTYPE_ARRAY, manually audit any serializers, schema generators, SDKs, or client code that might expect a'spatial'literal.
137-140: Spatial::isArray() now returns true – verify no upstream logic misinterprets arrays as element collectionsSpatial::isArray() was flipped from
falsetotrue. Search for any code usingisArray()(or alongsidegetType()) to gate or iterate inputs before validation—ensure WKT strings aren’t being rejected or their elements processed individually.
|
wrong branch so closing as it would lead to breaking change |
Summary by CodeRabbit