mark-gerarts / automapper-plus

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

Registering mapping for child properties #36

Open Jean85 opened 5 years ago

Jean85 commented 5 years ago

I'm currently trying to use the automapper to solve this problem:

I have multiple objects to map, and I have multiple \DateTime properties in those; I would like to map all of those to the same format\DTO. I have set up everything correctly, but every time that a source object has a \DateTime property, I'm forced to call ->forMember('field', Operation::mapTo(DateTimeDto)).

Is there any way to map that just once and for all?

mark-gerarts commented 5 years ago

Hi @Jean85, that's a valid point. Unfortunately there's no way to do that (yet), but since it's a nice thing to have I'm willing to implement it. I'm however not sure on what a good API would be to configure this. Maybe something like:

$options->setDefaultOperationFor(\DateTime::class, Operation::mapTo(DateTimeDto));

What are your thoughts about this?


As I was typing this out I think I figured out a way to do this. This is a hacky workaround, so I'm definitely still interested in implementing a clean way, but maybe it'll help you for now :)


$workaround = new class extends DefaultMappingOperation implements MapperAwareOperation
{
    use MapperAwareTrait;

    public function getSourceValue($source, string $propertyName)
    {
        $sourceValue = parent::getSourceValue($source, $propertyName);

        // Check if we're dealing with a datetime, and then perform our custom
        // operation. Otherwise resort to the defaults.
        return $sourceValue instanceof \DateTime
            ? $this->mapper->map($sourceValue, DateTimeDto::class)
            : $sourceValue;
    }
};

$config->getOptions()->setDefaultMappingOperation($workaround);
BoShurik commented 5 years ago

Maybe we can use existing AutoMapperPlus\Configuration\AutoMapperConfigInterface::registerMapping?

$config->registerMapping(\DateTime::class, DateTimeDto::class);

And use this mapping for all DateTime objects if there is no explicit mapping for property

juliocgs commented 5 years ago
$workaround = new class extends DefaultMappingOperation implements MapperAwareOperation
{
    use MapperAwareTrait;

    public function getSourceValue($source, string $propertyName)
    {
        $sourceValue = parent::getSourceValue($source, $propertyName);

        // Check if we're dealing with a datetime, and then perform our custom
        // operation. Otherwise resort to the defaults.
        return $sourceValue instanceof \DateTime
            ? $this->mapper->map($sourceValue, DateTimeDto::class)
            : $sourceValue;
    }
};

$config->getOptions()->setDefaultMappingOperation($workaround);

Hi, any news on this? Also, unfortunately the workaround does not work when the property is mapped using Operation::fromProperty().

mark-gerarts commented 5 years ago

You are right, the workaround only applies to the default operation (and some other classes). I've looked into the issue a bit deeper, and a real fix looks a bit more difficult than I first expected. Internally the automapper doesn't care what type the source properties are, it only looks at the property names. Because of this reason, it would introduce quite a bit of overhead to type-check every property and see if there are defaults registered for the particular combination. I'm afraid this would be a pretty big performance hit.

As I said, the mapper works with property names, so it would be perfectly possible to allow the registering of default operations based this. For example something like:

 $options->setDefaultOperationFor('date', Operation::mapTo(DateTimeDto::class));

Would this alternative still be interesting to implement?

juliocgs commented 5 years ago

Hi.

Instead of a method where we set default mapping to a class. It is possible to use the mappings that are already registered? That way, there is no reason to always type-check for defaults. Whenever a child property is going to be processed, it's type is checked and it's looked in all the mappings already registered. Unless Operation::mapTo() is used, because the type is already given.

Just like @BoShurik suggested.

It would really be interisting to have a feature like this. But if would be a huge performance problem, then it is better not to.