Skip to content

Improve identity map merging, error handling, and type safety#25

Merged
alganet merged 1 commit intoRespect:masterfrom
alganet:immutable
Apr 1, 2026
Merged

Improve identity map merging, error handling, and type safety#25
alganet merged 1 commit intoRespect:masterfrom
alganet:immutable

Conversation

@alganet
Copy link
Copy Markdown
Member

@alganet alganet commented Apr 1, 2026

Replace identity map entity swapping with per-property merging: mutable entities are updated in-place, readonly entities produce a new merged instance only when properties actually differ. Normalize PK types (numeric strings cast to int) for consistent identity map lookups. Add CollectionNotBound exception, cycle-safe persist return values, and class-string annotations removing phpstan-ignore.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.39%. Comparing base (d1664a8) to head (d72a094).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #25      +/-   ##
============================================
+ Coverage     98.11%   98.39%   +0.27%     
- Complexity      283      294      +11     
============================================
  Files            16       17       +1     
  Lines           583      622      +39     
============================================
+ Hits            572      612      +40     
+ Misses           11       10       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines identity map behavior during persist() by merging entities instead of swapping object instances, improves PK lookup consistency via ID normalization, and introduces clearer error handling when collections are used without a bound mapper.

Changes:

  • Replace identity-map entity replacement with per-property merge behavior (in-place updates for mutable entities; merged instance for readonly entities only when needed).
  • Normalize primary key values (numeric strings → ints) for consistent identity map indexing and lookups.
  • Improve error handling/type-safety by adding CollectionNotBound and tightening/refining phpstan annotations/ignores.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/AbstractMapperTest.php Updates expectations to reflect merge-in-place vs merged-instance behavior and new persist() return semantics.
src/AbstractMapper.php Implements identity-map merge logic and ID normalization in identity map interactions.
src/EntityFactory.php Adds mergeEntities() and strengthens type annotations for cached reflections.
src/Collections/Collection.php Returns mapper persist() result and throws CollectionNotBound when no mapper is attached.
src/CollectionNotBound.php New dedicated exception type/message for unbound collections.
src/Hydrators/Flat.php Clarifies phpstan ignore rationale for entity instance typing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Replace identity map entity swapping with per-property merging:
mutable entities are updated in-place, readonly entities produce a
new merged instance only when properties actually differ. Normalize
PK types (numeric strings cast to int) for consistent identity map
lookups. Add CollectionNotBound exception, cycle-safe persist return
values, and class-string annotations removing phpstan-ignore.
@alganet alganet marked this pull request as ready for review April 1, 2026 04:16
@alganet alganet merged commit bc63e1e into Respect:master Apr 1, 2026
3 checks passed
@alganet alganet deleted the immutable branch April 1, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants