Skip to content
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

feat(symfony): object mapper with state options #6801

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

Conversation

soyuka
Copy link
Member

@soyuka soyuka commented Nov 15, 2024

Q A
Branch? main
License MIT

Still a few things left to be done but this is a first approach. This makes API Platform compatible with the Object Mapper component: symfony/symfony#51741

Copy link

stale bot commented Jan 15, 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.

Copy link
Contributor

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

I think these provider and processor are Doctrine-specific. They should be moved to the Doctrine namespace and we can consider injecting the ObjectManager of ORM or ODM to retrieve the attached object.

Unless the new symfony/mapper component is able to use a factory to create the Entity class from the DTO. It would use the Id field of the DTO to retreive the attached entity from the entity manager.

Comment on lines +213 to +216
"repositories": [
{ "type": "path", "url": "../symfony/src/Symfony/Component/ObjectMapper" },
{ "type": "path", "url": "../symfony/src/Symfony/Bundle/FrameworkBundle" }
],
Copy link
Contributor

Choose a reason for hiding this comment

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

If that can help, I use this script to link external directories in my project vendors: https://gist.github.com/GromNaN/020dd50fc3ed71d31ebc548bb75845e7

Copy link
Member Author

Choose a reason for hiding this comment

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

I built https://github.com/soyuka/pmu for that now :p

return $this->decorated->process($data, $operation, $uriVariables, $context);
}

return $this->objectMapper->map($this->decorated->process($this->objectMapper->map($data), $operation, $uriVariables, $context));
Copy link
Contributor

Choose a reason for hiding this comment

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

In PATCH and DELETE operations, you must map into the entity attached to the EntityManager when retrieved in the provider.

Otherwise the decorated doctrine processor will try to persist a new object (and maybe fail on duplicate primary key), or skip removing the non-persisted entity.

return $data;
}

$entityClass = $operation->getClass();
Copy link
Contributor

@GromNaN GromNaN Mar 15, 2025

Choose a reason for hiding this comment

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

The DenormalizeProvider already create an instance of the operation class. Mapping into the same target class as the source class doesn't seem a valid use case to me. Making this provider useful only with Doctrine.

Copy link
Member Author

Choose a reason for hiding this comment

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

The denormalizer provider is only called on write operations. but indeed they should be doctrine-oriented it would make more sense

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