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

Impossibility of mapping in Doctrine Collection #169

Open
KoNekoD opened this issue Jun 24, 2024 · 9 comments
Open

Impossibility of mapping in Doctrine Collection #169

KoNekoD opened this issue Jun 24, 2024 · 9 comments

Comments

@KoNekoD
Copy link

KoNekoD commented Jun 24, 2024

I have difficulties with updating entities in a collection (let's omit the problem of overlapping and finding the right field by id), the problem is that instead of calling the setItems() method and transferring Collection into it, the call to the received Collection object such as setItemId() is made, which leads to an error, because Collection cannot have this method.

Below is an example of the generated code:

final class Symfony_Mapper_App_Dto_DigitalStore_SocialLinkDto_App_Entity_SocialLink extends AutoMapper\GeneratedMapper
{
    public function initialize(): void
    {
    }
    /** @param \App\Dto\DigitalStore\SocialLinkDto $value */
    public function &map($value, array $context = []): mixed
    {
        if (null === $value) {
            return $value;
        }
        /** @var \App\Entity\SocialLink|null $result */
        $result = $context['target_to_populate'] ?? null;
        if (null === $result) {
            $result = new \App\Entity\SocialLink();
        }
        $context = \AutoMapper\MapperContext::withIncrementedDepth($context);
        if (\AutoMapper\MapperContext::isAllowedAttribute($context, 'type', !isset($value->type)) && (!array_key_exists('groups', $context) || !$context['groups'])) {
            if (null !== $value->type) {
            }
            $result->setType($value->type);
        }
        if (\AutoMapper\MapperContext::isAllowedAttribute($context, 'url', !isset($value->url)) && (!array_key_exists('groups', $context) || !$context['groups'])) {
            if (null !== $value->url) {
            }
            $result->setUrl($value->url);
        }
        return $result;
    }
    public function registerMappers(AutoMapper\AutoMapperRegistryInterface $autoMapperRegistry): void
    {
    }
}

image

image

image

@Korbeil
Copy link
Member

Korbeil commented Jun 25, 2024

Hey @KoNekoD thanks for you report !
There is some "internal" classes like Doctrine collections that are impossible to map because they possess a large number of "logic" properties that cannot be mapped like your DTO/Entities are mapped.
In this case we usually do internal AST transformers, the idea is that when an AST transformer exists, we do not use the generic ObjectTransformer that will mapped properties and the AST transformer will be responsible for mapping such interfaces/objects.

For a similar and if you wanna give a try on making a PR, you can check: janephp/janephp#495
The folder is different now that AutoMapper has its own repository but you should find it quite easily 😉

@joelwurtz
Copy link
Member

joelwurtz commented Jun 25, 2024

It's strange that $result is a doctrine collection here, where does it come from ? The problem is not in this mapper but before.

I suspect you use an existing object to map the value to, and the value passed here is a doctrine collection where it should have been an item, can you show us the mapper that call this mapper with the collection ?

@KoNekoD
Copy link
Author

KoNekoD commented Jun 26, 2024

@Korbeil Thanks for your reply, but this is not a problem with Doctrine collection, it seems to be a logical error in the generated mapper, it should have read the method signature and realizing that iterable is in front of it, build an array with the required type. Maybe we need a unique internal transformer that works specifically for Doctrine Collection?

@KoNekoD
Copy link
Author

KoNekoD commented Jun 26, 2024

@joelwurtz Yes, you are right mapping is done on an existing object with deep_target_to_populate true. I think this is a logical problem because if you try to use an array it also tries to do the same with the array.

Maybe you can somehow improve the logic so that the generated code understands that there is an array in front of it and that it needs to collect an array of small elements and set them, and if there is a collection, it follows the collection methods in the same way...?

Here is the mapper that calls the problematic mapper:

<?php

final class Symfony_Mapper_App_Dto_DigitalStore_DigitalStoreUpdateProfileDto_App_Entity_DigitalStore extends AutoMapper\GeneratedMapper
{
    public function initialize(): void
    {
        $this->hydrateCallbacks['items'] = \Closure::bind(function ($object, $value) {
            $object->items = $value;
        }, null, 'App\Entity\DigitalStore');
    }
    /** @param \App\Dto\DigitalStore\DigitalStoreUpdateProfileDto $value */
    public function &map($value, array $context = []): mixed
    {
        if (null === $value) {
            return $value;
        }
        /** @var \App\Entity\DigitalStore|null $result */
        $result = $context['target_to_populate'] ?? null;
        if (null === $result) {
            $result = new \App\Entity\DigitalStore();
        }
        $context = \AutoMapper\MapperContext::withIncrementedDepth($context);
        if (\AutoMapper\MapperContext::isAllowedAttribute($context, 'profile', !isset($value->profile)) && (!array_key_exists('groups', $context) || !$context['groups'])) {
            if (null !== $value->profile) {
            }
            $result->setProfile($this->mappers['Mapper_App\Dto\DigitalStore\ProfileInputDto_App\Entity\Profile']->map($value->profile, \AutoMapper\MapperContext::withNewContext($context, 'profile', ($context['deep_target_to_populate'] ?? false) ? $result->getProfile() : null)));
        }
        if (\AutoMapper\MapperContext::isAllowedAttribute($context, 'socialLinks', !isset($value->socialLinks)) && (!array_key_exists('groups', $context) || !$context['groups'])) {
            if (null !== $value->socialLinks) {
                $values = [];
                foreach ($value->socialLinks as $key => $value_1) {
                    $values[] =& $this->mappers['Mapper_App\Dto\DigitalStore\SocialLinkDto_App\Entity\SocialLink']->map($value_1, \AutoMapper\MapperContext::withNewContext($context, 'socialLinks', ($context['deep_target_to_populate'] ?? false) ? $result->getSocialLinks() : null));
                }
            }
            $result->setSocialLinks($values);
        }
        return $result;
    }
    public function registerMappers(AutoMapper\AutoMapperRegistryInterface $autoMapperRegistry): void
    {
        $this->mappers['Mapper_App\Dto\DigitalStore\ProfileInputDto_App\Entity\Profile'] = $autoMapperRegistry->getMapper('App\Dto\DigitalStore\ProfileInputDto', 'App\Entity\Profile');
        $this->mappers['Mapper_App\Dto\DigitalStore\SocialLinkDto_App\Entity\SocialLink'] = $autoMapperRegistry->getMapper('App\Dto\DigitalStore\SocialLinkDto', 'App\Entity\SocialLink');
    }
}

I think the logical fallacy is in this point:

($context['deep_target_to_populate'] ?? false) ? $result->getSocialLinks() : null

Where it tries to use a collection to set the Item, and if you change the collection to an array it will do the same thing

@joelwurtz
Copy link
Member

joelwurtz commented Jun 26, 2024

Thanks, yeah we detect correctly, but code the code is wrongly generated, however i'm not sure what our solution should be here.

Problem is, how do we choose the correct value from the collection to map to ?

@KoNekoD
Copy link
Author

KoNekoD commented Jul 4, 2024

Hi @joelwurtz Sorry for the long reply, I was very zany and didn't notice the reply. I don't so much need to find a field to map and recreate fields in a collection. I just need to be able to map a DTO array into an array of entities and set them in a collection field using a setter, I don't see any problem if I for example use array $items in the setter signature and they will be set normally.

So to summarize, it's easy enough to set the resulting array to an entity using a setter.

@KoNekoD
Copy link
Author

KoNekoD commented Jul 4, 2024

If in the first example target_to_populate could be defined as null, then a simple mapping to a new object would occur with the following return

@KoNekoD
Copy link
Author

KoNekoD commented Jul 4, 2024

Here's how the mapper call is going on right now
$digitalStore = $this->mapper->map($data, $digitalStore, context: ['deep_target_to_populate' => true]);

This is mainly to successfully map data to a single object, I expected that for a collection it would not be able to find a mapping target and would just create a new object, but instead it just tries to use the collection as a mapping target-edentity, and this is wrong, maybe there is an option to recognize that the object type is a collection and work with it in a certain way. I would say that this is adding support for doctrine collections rather than a bug report, because the code managed to recognize the type, but the reality turned out to be different :)

@KoNekoD
Copy link
Author

KoNekoD commented Nov 4, 2024

?

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

No branches or pull requests

3 participants