Merged
Conversation
- Rename `next` to `connectsTo` and `more` to `hasMore` for clarity - Make `mapper` and `hydrator` properties `private(set)`, add `bindMapper()` - Remove `$changes` from Collection::persist() — identity map merge handles it - Remove EntityFactory::withChanges() and AbstractMapper::replaceTracked() - Register entities in identity map at insert time for pre-flush dedup - Preserve original pending operation in tryMergeWithIdentityMap
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #27 +/- ##
============================================
- Coverage 98.23% 97.96% -0.28%
+ Complexity 300 290 -10
============================================
Files 17 17
Lines 624 590 -34
============================================
- Hits 613 578 -35
- Misses 11 12 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR clarifies and tightens the Collection API while simplifying persistence behavior by relying on identity-map merges (instead of “persist with changes”), and it updates hydration/iteration logic and tests accordingly.
Changes:
- Rename
next→connectsToandmore→hasMoreacross Collection traversal/hydration/iteration and tests. - Make
Collection::$mapper/Collection::$hydratorprivate(set)and introduceCollection::bindMapper()for mapper binding. - Remove
EntityFactory::withChanges()and the Collectionpersist(...$changes)API, shifting partial updates to identity-map merge behavior inAbstractMapper.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/InMemoryMapper.php | Updates relationship attachment logic to use connectsTo / hasMore. |
| tests/EntityFactoryTest.php | Removes test coverage for removed EntityFactory::withChanges(). |
| tests/Collections/TypedTest.php | Updates assertions to connectsTo. |
| tests/Collections/FilteredTest.php | Updates assertions to connectsTo. |
| tests/Collections/CompositeTest.php | Updates assertions to connectsTo. |
| tests/Collections/CollectionTest.php | Updates chaining/flags to connectsTo / hasMore, switches mapper binding to bindMapper(), and rewrites persist tests around partial entities + identity map. |
| tests/AbstractMapperTest.php | Updates chaining to connectsTo and rewrites “withChanges” persistence tests to use partial entities + identity map behavior. |
| src/Hydrators/Nested.php | Updates nested hydration to traverse via connectsTo. |
| src/EntityFactory.php | Removes withChanges() implementation and now-unused imports. |
| src/Collections/Collection.php | Renames chain pointer/flag, restricts mapper/hydrator setters, adds bindMapper(), and removes persist(...$changes) support. |
| src/CollectionIterator.php | Updates recursive iteration logic to use hasMore / connectsTo. |
| src/AbstractMapper.php | Updates Filtered chaining to connectsTo, registers inserts in identity map earlier, removes replaceTracked(), and preserves pending ops when merging via identity map. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
nexttoconnectsToandmoretohasMorefor claritymapperandhydratorpropertiesprivate(set), addbindMapper()$changesfrom Collection::persist() — identity map merge handles it