FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(mapper): serialize nested objects by radoslav-grencik · Pull Request #2259 · tempestphp/tempest-framework · GitHub

fix(mapper): serialize nested objects - #2259

Open
radoslav-grencik wants to merge 5 commits into
tempestphp:3.xfrom
radoslav-grencik:fix/mapper-serialize-nested-objects
Open

fix(mapper): serialize nested objects#2259
radoslav-grencik wants to merge 5 commits into
tempestphp:3.xfrom
radoslav-grencik:fix/mapper-serialize-nested-objects

Conversation

Copy link
Copy Markdown
Contributor

Fixes #2258

xHeaven commented Aug 21, 2026
edited
Loading

Copy link
Copy Markdown
Member

This seems to be a bit problematic right now. You're introducing an infinite recursion (nested objects reference each other) and also a performance regression because you are resolving all configured mappers again for every nested object.

The goal idea is good, but I'm afraid this should be a bit deeper refactor than 2 lines of change. I'll try to think of something, let's also wait for @brendt here.

Edit: added a couple failing tests for clarity.

Copy link
Copy Markdown
Contributor Author

This seems to be a bit problematic right now. You're introducing an infinite recursion (nested objects reference each other) and also a performance regression because you are resolving all configured mappers again for every nested object.

The goal idea is good, but I'm afraid this should be a bit deeper refactor than 2 lines of change. I'll try to think of something, let's also wait for @brendt here.

Edit: added a couple failing tests for clarity.

Thanks, that makes sense. I reworked the fix so nested object serialization no longer calls the public mapper pipeline recursively.

The updated approach keeps recursion inside ObjectToArrayMapper and tracks visited objects with spl_object_id(). This avoids infinite recursion for cyclic graphs; cyclic objects are left for JSON encoding to fail normally instead of hanging.

It also avoids the performance regression from repeatedly resolving configured mappers for every nested object. The new test for unused mapper resolution now passes.

I also updated ArrayOfObjectsSerializer to preserve mapping context when it is used directly.

brendt commented Aug 28, 2026

Copy link
Copy Markdown
Member

I'm can't remember if this was ever intended behavior, but I agree that it should be changed. However, I do wonder if this could be a potential breaking change, because we're suddenly changing how the mapper works… I for example remember manually mapping nested objects in some of my project exactly because of this bug. I don't believe my code would break, but I wonder if others might…

On top of that, I don't see an automated upgrade path for this change; all of that leads me to think we should target 4.x with this one

Copy link
Copy Markdown
Contributor Author

I'm can't remember if this was ever intended behavior, but I agree that it should be changed. However, I do wonder if this could be a potential breaking change, because we're suddenly changing how the mapper works… I for example remember manually mapping nested objects in some of my project exactly because of this bug. I don't believe my code would break, but I wonder if others might…

On top of that, I don't see an automated upgrade path for this change; all of that leads me to think we should target 4.x with this one

That makes sense, targeting 4.x sounds reasonable.

Is there already a plan or timeline for 4.x? I don’t see a 4.x branch yet, but I’d be happy to help.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ObjectToArrayMapper does not serialize single nested object properties

3 participants


Back | FazBrowse Home | New Git URL