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

AutoMapper does not handle Doctrine managed entities, leading to new instances being created for existing entities #198

Open
m4n50n opened this issue Oct 13, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@m4n50n
Copy link

m4n50n commented Oct 13, 2024

I am using AutoMapper in a Symfony project alongside Doctrine. When mapping a DTO to an entity, AutoMapper creates new instances for related entities, even if those entities already exist in the database and should be managed by Doctrine.

For example, I am mapping a Course DTO that contains a CourseType (which is an existing entity in the database). The CourseType has a valid id, and this should indicate to Doctrine that it is an existing entity, but AutoMapper is not respecting that. Instead, it creates a new CourseType instance, which is not managed by Doctrine, leading to persistence issues (e.g., Doctrine attempts to persist the new CourseType as if it were a new entity).

The only way I found to ensure that the related entity is managed by Doctrine is to manually fetch it from the database before persisting the mapped entity, which defeats the purpose of using AutoMapper to simplify this process.

What can I do? Thanks!

@AntipodTX
Copy link

i faced the same issue, but you can do better than fetch the entity from the database.
AutoMapper can map to an existing entity, so what you need is to have an Entity Proxy, if it stored in a Doctrine's IdentityMap.
For example, you want to map an UserDTO to UserEntity:

$this->map($userDTO, UserEntity::class);
...
private function map(mixed $dto, string $resourceClass): object
{
    if ($dto->getId() !== null) {
        $resourse = $this
            ->entityManager
            ->getUnitOfWork()
            ->tryGetById($dto->getId() , $resourceClass);
        if ($resource !== false) {
            return $this->autoMapper->map($dto, $resource);
        }
    }
    return $this->autoMapper->map($dto, $resourceClass);
}

@m4n50n
Copy link
Author

m4n50n commented Oct 22, 2024

i faced the same issue, but you can do better than fetch the entity from the database. AutoMapper can map to an existing entity, so what you need is to have an Entity Proxy, if it stored in a Doctrine's IdentityMap. For example, you want to map an UserDTO to UserEntity:

$this->map($userDTO, UserEntity::class);
...
private function map(mixed $dto, string $resourceClass): object
{
    if ($dto->getId() !== null) {
        $resourse = $this
            ->entityManager
            ->getUnitOfWork()
            ->tryGetById($dto->getId() , $resourceClass);
        if ($resource !== false) {
            return $this->autoMapper->map($dto, $resource);
        }
    }
    return $this->autoMapper->map($dto, $resourceClass);
}

Hi @AntipodTX , thank you very much for your reply. Finally I have decided to use mark-gerarts/automapper-plus-bundle, which has not given me any problem and while I find the time to investigate more, I leave this one.

Best regards!

@Korbeil
Copy link
Member

Korbeil commented Oct 25, 2024

Hey @m4n50n and @AntipodTX , thanks for reporting your issue !

I think the best here would be to add a Doctrine provider within the AutoMapper Bundle. I'm still not sure how to make it work correctly. Ideally the provider would contain something close to your last comment @m4n50n. But I don't think we would like that every time, how do you guys we should make it trigger ?

I was thinking about a context parameter, something like ['recover_from_database' => true] ? (name is just an idea, nothing temporary).
Also how should it work ? If no object found in database we throw an Exception ? we map onto a new object ?
There is some stuff we should define before doing such provider in my opinion !

If you have time to answer all this and give me your feedback, I can see about implementing it later on !
Baptiste

@Korbeil Korbeil added the enhancement New feature or request label Oct 25, 2024
@yobud
Copy link
Contributor

yobud commented Oct 25, 2024

Hi!

I'm just facing this issue,

In my opinion, as soon as an entity is declared as an ORM entity, should always work this way :

  • If DTO has not null identifier, it should be fetched from the database.
  • If no such id exists in database, then it should return a new empty instance of the entity (filled with the dto values of course). (this case is already working this way as even if we send an existing id, it's creating a non ORM mapped instance).

If it has to be otherwise, I think this is for specific use cases, and this should be handled by custom provider for these edge cases.

@AntipodTX
Copy link

@Korbeil

The code that you see above it is a simplified version of the code that i am using in my Transformer. I implemented the feature mentioned here , so i can collect all static metadata information about the Entity during the mapper class generation process and store it in a generated mapper class using my own PropertyTransformer. The thing is (for example) the Entity can have a composite identifier and, IMO, it is better to collect this information once instead of doing it all the time during the mapping (and the Provider's) call. I am not against the Provider, i am just thinking out laud about the performance.

@yobud

One small correction - If DTO has not null identifier, the Entity should be fetched from the database. if it is not fetched yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants