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

Registering mapping for child properties #36

Open
Jean85 opened this issue Feb 28, 2019 · 5 comments
Open

Registering mapping for child properties #36

Jean85 opened this issue Feb 28, 2019 · 5 comments

Comments

@Jean85
Copy link

Jean85 commented Feb 28, 2019

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
Copy link
Owner

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
Copy link
Contributor

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
Copy link

$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
Copy link
Owner

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
Copy link

juliocgs commented Apr 16, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants