Skip to content

feat(doctrine): new hook for entity/document transformation #6919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mrossard
Copy link
Contributor

@mrossard mrossard commented Jan 15, 2025

Q A
Branch? main
License MIT
Doc PR TODO

Currently when using stateOptions to specify the entityClass (or documentClass) for your custom ApiResource, the standard StateProviders do exactly what you need most of the time, but you still often need to write custom providers and processors to correctly map the entity/document to the actual resource.

This adds the possibility to specify hooks (the same way you specify custom a handleLinks method) to transform the doctrine entity or document into the desired ApiResource without having to write a full custom StateProvider and/or transform the resource to the correct entity/document without writing a full custom StateProcessor.

@mrossard mrossard marked this pull request as draft January 15, 2025 11:50
@mrossard mrossard marked this pull request as ready for review January 15, 2025 12:48
@mrossard mrossard changed the title feat(doctrine): new hook for entiy/document transformation feat(doctrine): new hook for entity/document transformation Jan 15, 2025
@@ -22,6 +22,7 @@ class Options implements OptionsInterface
*/
public function __construct(
protected mixed $handleLinks = null,
protected mixed $transformModel = null,
Copy link
Member

Choose a reason for hiding this comment

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

I like the feature, I think it'd be a great fit with #6801 best would be that when the mapper is installed, we add a default transformer that uses the object mapper!

I'm just not a fan of the naming but this one is hard haha. A few ideas:

  • transformEntityToResource
  • entityToResourceMapper
  • mapper
  • resourceTransformer

I think that it's missing a way to transform a resource to an entity when persisting data (POST/PATCH) WDYT?

Last but not least, you should work on an interface that'd be used to autoconfigure the "transformEntityLocator", it would surely help with naming things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the feature, I think it'd be a great fit with #6801 best would be that when the mapper is installed, we add a default transformer that uses the object mapper!

I'm just not a fan of the naming but this one is hard haha. A few ideas:

* transformEntityToResource

* entityToResourceMapper

* mapper

* resourceTransformer

Yeah, i'm not a fan of it either - tried to find a name that works with both entities or documents, but model feels lame, and I didn't want to use mapper to avoid confusion with what you're preparing on that side.

How about resourceBuilder or something along those lines?

I think that it's missing a way to transform a resource to an entity when persisting data (POST/PATCH) WDYT?

Wouldn't it be redundant when we already have entityClass + handlelinks?

Last but not least, you should work on an interface that'd be used to autoconfigure the "transformEntityLocator", it would surely help with naming things.

I'll look into this. I was not sure about that part, actually i was wondering (and i actually left a comment in the code) if this locator trait is better by itself or if it should be merged with LinksHandlerLocatorTrait into a common OptionsHooksLocatorTrait of some sort...WDYT?

@mrossard mrossard marked this pull request as draft January 20, 2025 16:01
Copy link

stale bot commented Mar 22, 2025

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 22, 2025
@mrossard
Copy link
Contributor Author

just a comment to stop this from closing, trying to find the time to complete this.

@stale stale bot removed the stale label Mar 23, 2025
@soyuka
Copy link
Member

soyuka commented Mar 25, 2025

sorry my bad :)

@mrossard mrossard force-pushed the entityclass-transform branch 3 times, most recently from ef15c8c to 4eb09f7 Compare July 24, 2025 08:32
@mrossard mrossard force-pushed the entityclass-transform branch from 4eb09f7 to 9a2ab42 Compare July 24, 2025 08:41
@mrossard
Copy link
Contributor Author

At long last I found some time to revisit this PR. @soyuka what do you think ?

I haven't looked at how it fits in with #6801 yet, but i guess that would be a default implementation of the ResourceTransformerInterface using the Mapper symfony extension if it's available.

@soyuka
Copy link
Member

soyuka commented Jul 24, 2025

My question is more: doesn't object mapper already does the transformation hook ? I'm not sure we need a transformation hook when using the object mapper. If you mark a class with the Map attribute it'll be transformed.

@mrossard
Copy link
Contributor Author

My question is more: doesn't object mapper already does the transformation hook ? I'm not sure we need a transformation hook when using the object mapper. If you mark a class with the Map attribute it'll be transformed.

I haven't looked in depth at what ObjectMapper can do yet, and it's entirely possible what i was doing is now redundant.

Let me try to do what i had in mind with it, if it works as i hope i'll just close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants