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.

Sign up for GitHub

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

Jump to bottom

Uninitialized properties do not get ignored by AsPropertyMapper #257

Closed
pedrem opened this issue Jan 20, 2025 · 3 comments · Fixed by #258 or #260
Closed

Uninitialized properties do not get ignored by AsPropertyMapper #257

pedrem opened this issue Jan 20, 2025 · 3 comments · Fixed by #258 or #260

Comments

@pedrem
Copy link

pedrem commented Jan 20, 2025 •
edited
Loading

Hello!

We’re encountering an issue when mapping an object that uses a custom property mapper. Interestingly, this doesn't occur for objects without custom mapping. Here’s a simplified example:

#[AsPropertyMapper(targetClass: EntityRequest::class)]
readonly class EntityMapper
{
    #[AsPropertyMapper]
    public function mapCurrency(Request $request): ?string
    {
        // ...
        return $request->getCurrencyIso();
    }
}

When mapping from a source like:

class Request
{
    public ?string $currencyIso;
    // ...
}

The mapCurrency method is called even if the currencyIso property is not initialized, which is not the desired behavior.

For additional context, this occurs while handling a PATCH request. Ideally, we only want mapper functions to be invoked for properties that are actually initialized, like it happens for normal properties (without mappers)

Is this something that can be achieved?

Please let me know if you need more details or further clarification. Thank you!
@priyadi priyadi mentioned this issue Jan 23, 2025
Merged
@priyadi
Copy link
Member

priyadi commented Jan 23, 2025

Well, Mapper doesn't know if your custom property mapper is going to get the value from $request->getCurrencyIso(). It can get it from another property with a different name, it can be from multiple properties, or it can even be from a source outside the source object.

So, your property mapper will need to handle the uninitialized situation itself. What it needs is a way to signal the Mapper if it wishes to skip the mapping and leave the target property alone, which is provided in the related PR.

Check the example code and let me know if it would solve your problem: https://github.com/rekalogika/mapper/blob/a4cdc84496daca8e4db18736b33e72ccfe18d17a/tests/src/Services/PropertyMapper/PropertyMapperFromUnitializedVariable.php

@pedrem
Copy link
Author

pedrem commented Jan 23, 2025

That works out nicely.

Thank you for such a quick solution!

I wonder if we could add this functionality as a parameter to #[AsPropertyMapper] itself in the future. I imagine something like:

#[AsPropertyMapper(targetClass: Target::class, ignoreUninitialized: true)]

@priyadi priyadi reopened this Jan 27, 2025
@priyadi priyadi mentioned this issue Jan 27, 2025
Merged
@priyadi
Copy link
Member

priyadi commented Jan 27, 2025 

#[AsPropertyMapper(targetClass: Target::class, ignoreUninitialized: true)]

Done in the latest PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@priyadi @pedrem