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

skip_uninitialized_values not working ? #189

Open
TheoD02 opened this issue Sep 15, 2024 · 8 comments
Open

skip_uninitialized_values not working ? #189

TheoD02 opened this issue Sep 15, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@TheoD02
Copy link
Contributor

TheoD02 commented Sep 15, 2024

I am trying to map a payload for a PATCH endpoint and would like to configure it so that:

  • If a property is uninitialized, it is skipped during the update.
  • If a property is explicitly set to null, it should update the field to null (if the field is nullable).

However, the current behavior of skip_uninitialized_values is not working.

I have also tried using skip_null_values, but this is problematic because it skips both uninitialized and null values. In cases where a field is nullable (e.g., birthdate), this prevents setting a nullable field to null during a PATCH operation.

Example:

Here’s a basic example of my User entity:

class User
{
    private string $name;

    private ?\DateTimeInterface $birthdate = null; // Nullable
}

In a PATCH request, I would like the following behavior:

  • If birthdate is not provided in the payload, it should remain unchanged.
  • If birthdate is explicitly set to null in the payload, it should be updated to null in the entity.

Here’s what the request payload might look like:

{
    "name": "John Doe",
    "birthdate": null
}

Expected Behavior:

  • Properties that are uninitialized (e.g., not provided in the payload) should be skipped in the update.
  • Nullable properties (like birthdate) should be updated to null if explicitly set to null.

Current Behavior:

  • skip_uninitialized_values does not skip uninitialized properties as expected.
  • skip_null_values skips both uninitialized and null values, making it impossible to set a nullable field to null in a PATCH request.

Steps to Reproduce:

  1. Create a User entity with a nullable field (e.g., birthdate).
  2. Attempt to perform a PATCH request where some fields are not provided and others are explicitly set to null.
  3. Observe that uninitialized fields are not skipped correctly, or nullable fields can't be updated to null.

Questions:

  • Is there a way to achieve this behavior using the current options?
@MrMeshok
Copy link
Contributor

class User
{
    private string $name;

    private ?\DateTimeInterface $birthdate = null; // Nullable
}

When you type property with default value it already initialized on creation, so if you just remove = null it should work as expected.

@TheoD02
Copy link
Contributor Author

TheoD02 commented Sep 16, 2024

class User
{
    private string $name;

    private ?\DateTimeInterface $birthdate = null; // Nullable
}

When you type property with default value it already initialized on creation, so if you just remove = null it should work as expected.

Thanks for answer :)

Yes for that case, but in case you only want to update partially a resource, if birthdate is present, but name is not.
An error will occur because mapper will attempt to map name but is uninitialized

@MrMeshok
Copy link
Contributor

Can you provide full example with configuration for AutoMapper? I am testing with AutoMapper::create() without any configuration and have no errors

@TheoD02
Copy link
Contributor Author

TheoD02 commented Sep 16, 2024

Check here:
https://phpsandbox.io/n/w5wwq#public/index.php

If I want to only update partially a resource, by setting only birthdate or lastName that will not work because other properties are uninitialized.

If birthdate is set to null or date, it should update value, otherwise it should not, for example

@MrMeshok
Copy link
Contributor

I see now, for me, it's working because I was mapping from array/stdClass.
When I was working with PATCH requests, I did this kind of mapping by hand. Yeah, it's kinda ugly with several if statements, but you can control all the logic

@TheoD02
Copy link
Contributor Author

TheoD02 commented Sep 17, 2024

I see now, for me, it's working because I was mapping from array/stdClass. When I was working with PATCH requests, I did this kind of mapping by hand. Yeah, it's kinda ugly with several if statements, but you can control all the logic

OK, thanks for your time, make it by hand need some effort that is not really needed in my case ^^

I think the preferable solution will be to split skip_uninitialized_values and skip_null_values to support both approach

@joelwurtz
Copy link
Member

Indeed we don't support skip_uninitialized_values yet, would be a nice addition, will add this to roadmap

@joelwurtz joelwurtz added the enhancement New feature or request label Oct 22, 2024
@Korbeil
Copy link
Member

Korbeil commented Oct 22, 2024

I started working on this mater and got something working, I need to fix some tests but the feature is there if you wanna take a look at it: #200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants