mark-gerarts / automapper-plus

An AutoMapper for PHP
MIT License
551 stars 30 forks source link

CustomConstructor callable should have destinationClass parameter #40

Closed Toilal closed 5 years ago

Toilal commented 5 years ago

Maybe I miss something, but I try to implement a custom constructor that grabs entities from a Doctrine repository.

But it doesn't seem to be possible because the constructor callback doesn't get $destinationClass as parameter to retrieve which class to instanciate.

https://github.com/mark-gerarts/automapper-plus/blob/e0a411b4556865c6b9323b1af30814374df321fd/src/AutoMapper.php#L69-L76

I think $destinationClass should be passed as a parameter to the constructor callable.

mark-gerarts commented 5 years ago

Could you give a bit more context for this, with maybe a small example? It sounds like you want to have some sort of factory class, for which it would be more suited to use a custom mapper (docs) than a custom constructor. In the snippet you linked you can see that a custom mapper does in fact receive the destination class as a parameter.

If you have to use a constructor, this can be set for specific mappings, so you would know your destination class there:

$config->registerMapping(A::class, B:class)->beConstructedUsing(
    // Destination class is B
);

Another option is the $context. The custom constructor gets the context passed a third parameter. This is an array containing the destination object. You could use a get_class here. For example:

$config->registerMapping(A::class, B:class)->beConstructedUsing(
    function (A $src, AutoMapperInterface $mapper, array $context = []) {
        $destinationClass = get_class($context[AutoMapper::DESTINATION_CONTEXT]);
    }
);

If none of this works for your situation, and you can convince me to add it as a parameter, I'll see if I can implement it :)

Toilal commented 5 years ago

I didn't noticed the AutoMapper::DESTINATION_CONTEXT thing inside docs.

I just give it a try to this option, but it doesn't work. In the constructor callable, context is still empty in the debugger. When reading sources, it seems to be defined only when invoking mapToObject, and is not defined when using map function. Maybe a new AutoMapper::DESTINATION_CONTEXT_CLASS key could be introduced, as AutoMapper::DESTINATION_CONTEXT seems to contain the target instance (of course, no instance is available until this is actually constructed).

So now, using a custom mapper is an option, but it seems overkilled as I only want to define a custom way of constructing instances. As a workaround, I'll implement a custom mapper that creates destination instances using my custom behavior and delegates to the default mapper using doMap.

More about my use case: I try to integrate AutoMapper with Doctrine inside a symfony project. The idea is to retrieve object instances from the database instead of constructing them from scratch, it will really help to implement an CRUD API with a DTO layer.

class AutoMapperConfigurator implements AutoMapperConfiguratorInterface
{
    /**
     * @var EntityManagerInterface
     */
    private $em;

    public function __construct(EntityManagerInterface $em)
    {
        $this->em = $em;
    }

    public function findEntityConstructor($source, AutoMapperInterface $mapper, $context): FindEntityOperation
    {
        // This function loads an entity from the DTO ($source) instead of creating an instance.
         return FindEntityOperation::constructor($this->em, $source, $mapper, $context);
    }

 public function configure(AutoMapperConfigInterface $config): void
    {
    $config->registerMapping(UserDTO::class, User::class)
            ->beConstructedUsing(array($this, 'findEntityConstructor'))
    }
Toilal commented 5 years ago

I have tried to implement a custom mapper, but I can't find a way to invoke the default mapping strategy after creating the instance.

<?php

namespace App\AutoMapper;

use AutoMapperPlus\MapperInterface;
use Doctrine\ORM\EntityManagerInterface;

class FindEntityMapper implements MapperInterface
{
    /**
     * @var EntityManagerInterface
     */
    private $em;

    public function __construct(EntityManagerInterface $em)
    {
        $this->em = $em;
    }

    public function map($source, string $targetClass)
    {
        $op = new FindEntityOperation($this->em, $targetClass);
        $entity = $op->findEntity($source);
        if (!$entity) {
            // In a perfect world, I would like to honor skipConstructor/dontSkipConstructor option,
            // but it doesn't seems to be possible.
            $entity = new $targetClass;
        }
        return $this->mapToObject($source, $entity);
    }

    public function mapToObject($source, $destination)
    {
        // TODO: How to invoke the configured mapping ?
    }
}
Toilal commented 5 years ago

I have opened a pull request that solves my issue.

mark-gerarts commented 5 years ago

I see, using a custom mapper wouldn't work (without some hacky stuff) since you need to fall back to the regular mapper. In this case it would make more sense to decorate the AutoMapper.

However, I can see the usefulness of adding the destination class to the context, so I'm going to merge your PR (#41). This way you can choose which solution you prefer, be it decorating or using the constructor.

Thanks for the discussion!