Add nested spatial query tests #727
Conversation
WalkthroughInternal renaming in src/Database/Database.php replaces “field(s)” with “attribute(s)” across select and relationship processing; E2E tests update many collection/attribute names from snake_case to camelCase and add a prerequisite testers collection creation. No public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TestRunner as E2E Test
participant DB as Database
TestRunner->>DB: createCollection("testers")
Note right of DB #ddeeff: prerequisite collection
DB-->>TestRunner: created
TestRunner->>DB: createCollection("devices")
DB-->>TestRunner: created
TestRunner->>DB: run relationship tests (camelCase data)
Note right of DB #e8f5e9: relationship population & selection using updated attribute keys
DB-->>TestRunner: test results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (11)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Database/Database.php (1)
7789-7804: Fix reverse lookup query in nested relationship pathWhen we hit the reverse-lookups branch,
matchingIdsalready holds the IDs of the child documents we just resolved (e.g. comments selected viaposts.comments.author.name). The new code tries to load those children withQuery::equal($twoWayKey, $matchingIds), but$twoWayKeypoints to the parent reference field (likepost), not the child's primary key. Because the parent IDs rarely equal the child IDs, this query comes back empty, so every multi-hop filter collapses to “no matches”. For example,Query::equal('posts.comments.author.name', ['Alice'])now returns nothing even when Alice’s comments exist.We need to look up the child documents by their
$id, then readtwoWayKeyto collect the parent IDs.- [ - Query::equal($link['twoWayKey'], $matchingIds), + [ + Query::equal('$id', $matchingIds),
🧹 Nitpick comments (1)
tests/e2e/Adapter/Scopes/RelationshipTests.php (1)
3425-3674: Comprehensive spatial relationship query test with proper DB-specific handling.This new test thoroughly validates spatial queries on relationship attributes, covering:
- Distance queries (lessThan, equal, greaterThan, notEqual)
- Geometric relationship queries (contains, intersects, crosses, overlaps, touches)
- Combined spatial and regular queries
- Multiple spatial types (POINT, POLYGON, LINESTRING)
The test pragmatically acknowledges DB-specific spatial implementation differences (lines 3579-3583, 3605-3609) and verifies query execution rather than exact results where semantics vary. Proper adapter support checks and cleanup are in place.
Optional refinement: Consider splitting into smaller test methods.
At ~250 lines, this test method is quite comprehensive but might benefit from being split into focused sub-tests:
testRelationshipSpatialDistanceQueries()testRelationshipSpatialGeometricQueries()testRelationshipSpatialCombinedQueries()This would improve maintainability and make test failures easier to diagnose.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Database/Database.php(10 hunks)tests/e2e/Adapter/Scopes/CollectionTests.php(1 hunks)tests/e2e/Adapter/Scopes/RelationshipTests.php(63 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T02:04:17.803Z
Learnt from: abnegate
PR: utopia-php/database#721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.
Applied to files:
tests/e2e/Adapter/Scopes/RelationshipTests.php
🧬 Code graph analysis (3)
tests/e2e/Adapter/Scopes/CollectionTests.php (2)
src/Database/Database.php (1)
createCollection(1328-1462)src/Database/Adapter.php (1)
createCollection(525-525)
src/Database/Database.php (2)
src/Database/Query.php (2)
getValues(167-170)Query(8-1116)src/Database/Document.php (1)
getArrayCopy(423-458)
tests/e2e/Adapter/Scopes/RelationshipTests.php (4)
src/Database/Database.php (9)
createAttribute(1692-1747)Database(37-8134)createCollection(1328-1462)createRelationship(2656-2833)find(6695-6826)create(1241-1260)updateRelationship(2848-3035)count(6923-6972)sum(6987-7030)src/Database/Adapter.php (9)
createAttribute(557-557)createCollection(525-525)createRelationship(615-615)find(802-802)create(488-488)updateRelationship(631-631)getSupportForSpatialAttributes(1057-1057)count(825-825)sum(814-814)src/Database/Adapter/Pool.php (9)
createAttribute(168-171)createCollection(153-156)createRelationship(193-196)find(263-266)create(133-136)updateRelationship(198-201)getSupportForSpatialAttributes(428-431)count(273-276)sum(268-271)src/Database/Query.php (7)
Query(8-1116)distanceLessThan(1016-1019)distanceGreaterThan(1002-1005)intersects(1028-1031)crosses(1052-1055)overlaps(1076-1079)touches(1100-1103)
🪛 PHPMD (2.15.0)
tests/e2e/Adapter/Scopes/RelationshipTests.php
431-431: Avoid unused local variables such as '$user'. (undefined)
(UnusedLocalVariable)
437-437: Avoid unused local variables such as '$post1'. (undefined)
(UnusedLocalVariable)
444-444: Avoid unused local variables such as '$post2'. (undefined)
(UnusedLocalVariable)
3044-3044: Avoid unused local variables such as '$author1'. (undefined)
(UnusedLocalVariable)
3051-3051: Avoid unused local variables such as '$author2'. (undefined)
(UnusedLocalVariable)
3123-3123: Avoid unused local variables such as '$user1'. (undefined)
(UnusedLocalVariable)
3129-3129: Avoid unused local variables such as '$profile1'. (undefined)
(UnusedLocalVariable)
3170-3170: Avoid unused local variables such as '$userA'. (undefined)
(UnusedLocalVariable)
3176-3176: Avoid unused local variables such as '$comment1'. (undefined)
(UnusedLocalVariable)
3183-3183: Avoid unused local variables such as '$comment2'. (undefined)
(UnusedLocalVariable)
3223-3223: Avoid unused local variables such as '$student1'. (undefined)
(UnusedLocalVariable)
3229-3229: Avoid unused local variables such as '$course1'. (undefined)
(UnusedLocalVariable)
3462-3462: Avoid unused local variables such as '$supplier1'. (undefined)
(UnusedLocalVariable)
3481-3481: Avoid unused local variables such as '$supplier2'. (undefined)
(UnusedLocalVariable)
3500-3500: Avoid unused local variables such as '$supplier3'. (undefined)
(UnusedLocalVariable)
3923-3923: Avoid unused local variables such as '$dev1'. (undefined)
(UnusedLocalVariable)
3930-3930: Avoid unused local variables such as '$dev2'. (undefined)
(UnusedLocalVariable)
3938-3938: Avoid unused local variables such as '$project1'. (undefined)
(UnusedLocalVariable)
3947-3947: Avoid unused local variables such as '$project2'. (undefined)
(UnusedLocalVariable)
4076-4076: Avoid unused local variables such as '$dept1'. (undefined)
(UnusedLocalVariable)
4083-4083: Avoid unused local variables such as '$dept2'. (undefined)
(UnusedLocalVariable)
4090-4090: Avoid unused local variables such as '$company1'. (undefined)
(UnusedLocalVariable)
4097-4097: Avoid unused local variables such as '$company2'. (undefined)
(UnusedLocalVariable)
4104-4104: Avoid unused local variables such as '$employee1'. (undefined)
(UnusedLocalVariable)
4113-4113: Avoid unused local variables such as '$employee2'. (undefined)
(UnusedLocalVariable)
4122-4122: Avoid unused local variables such as '$project1'. (undefined)
(UnusedLocalVariable)
4130-4130: Avoid unused local variables such as '$project2'. (undefined)
(UnusedLocalVariable)
4138-4138: Avoid unused local variables such as '$task1'. (undefined)
(UnusedLocalVariable)
4147-4147: Avoid unused local variables such as '$task2'. (undefined)
(UnusedLocalVariable)
4156-4156: Avoid unused local variables such as '$task3'. (undefined)
(UnusedLocalVariable)
4280-4280: Avoid unused local variables such as '$author1'. (undefined)
(UnusedLocalVariable)
4287-4287: Avoid unused local variables such as '$author2'. (undefined)
(UnusedLocalVariable)
4294-4294: Avoid unused local variables such as '$author3'. (undefined)
(UnusedLocalVariable)
⏰ 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 (5)
tests/e2e/Adapter/Scopes/CollectionTests.php (1)
953-953: LGTM! Bug fix ensures collection exists before relationship creation.This addition correctly creates the 'testers' collection before establishing the relationship with 'devices'. Previously, the test would fail because
createRelationshiprequires both collections to exist.tests/e2e/Adapter/Scopes/RelationshipTests.php (4)
47-48: LGTM! Consistent camelCase naming convention.The attribute renaming from snake_case to camelCase (
first_name→firstName,last_name→lastName,date_of_birth→dateOfBirth,is_active→isActive) aligns with the broader refactoring effort described in the PR summary and improves code consistency.Also applies to: 62-62, 64-64
2672-2672: LGTM! Renamed attribute for consistency.The
plateNumberattribute naming (previouslyplate_number) maintains camelCase consistency with the refactoring effort.
3994-4248: LGTM! Thorough multi-depth nested relationship query validation.This test validates filtering through relationship chains up to 3 levels deep (e.g.,
tasks→projects→employees→companies), including mixed relationship types (ONE_TO_MANY, MANY_TO_ONE). The test covers:
- Single-level queries (depth 1)
- Two-level nested queries (depth 2)
- Three-level nested queries (depth 3)
- Combined queries at different depths
Assertions verify correct filtering at each depth level, and proper cleanup is performed.
4250-4400: LGTM! Count and sum operations validated with relationship filters.This test ensures aggregate operations (
count()andsum()) correctly apply relationship-based filters, covering:
- Count with single relationship filter
- Count with multiple filters (relationship + regular attribute)
- Sum with relationship filters
- Edge cases (no matches, author with no posts)
The test data setup and assertions properly validate the aggregate behavior.
Summary by CodeRabbit
Refactor
Tests