mark-gerarts/automapper-plus

Registering mapping for child properties

Jean85 opened this issue · 5 comments

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?

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);

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

$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().

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?

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.