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

[Serializer] Deserialization order of priority changed #[SerializedPath] vs. property name #51823

Closed
jerowork opened this issue Oct 3, 2023 · 0 comments

Comments

@jerowork
Copy link
Contributor

jerowork commented Oct 3, 2023

Symfony version(s) affected

6.3.5

Description

With the update of Serializer to 6.3.5, some deserialization of array to objects does behave differently (changed order of priority of configuration via attribute #[SerializedPath] vs. property name, when there is a key on root level with the same name as the private property.

Related to #49700.

How to reproduce

Example to explain changed behavior:

{
  "data": {
    "item": {
      "id": "id-1"
    }
  },
  "id": "id-2"
}
final class SomeEvent
{
    #[SerializedPath('[data][item][id]')]
    public string $id;
}

Before 6.3.5, the value of the id was id-1, with the change of #49700, the value of the id becomes id-2.

#49700 changes array_merge with array + array. It seems that the problem stated above is related to the fact that array_merge does overwrite keys differently than array + array:

$a = ['key' => 'value-a'];
$b = ['key' => 'value-b'];

var_dump(array_merge($a, $b));
// Results in:
// array(1) {
//   ["key"]=>
//   string(7) "value-b"
// }

var_dump($a + $b);
// Results in:
// array(1) {
//   ["key"]=>
//   string(7) "value-a"
// }

Possible Solution

As array_merge does behave slightly differently that array + array, the solution could be to switch array order to:

- $normalizedData = $normalizedData + $nestedData;
+ $normalizedData = $nestedData + $normalizedData;

This would result in the same, while keeping the fix (#49700) for the numeric key value.

PR: #51825

Additional Context

No response

@jerowork jerowork added the Bug label Oct 3, 2023
fabpot added a commit that referenced this issue Oct 7, 2023
This PR was merged into the 6.3 branch.

Discussion
----------

Fix order array sum normalizedData and nestedData

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #51823
| License       | MIT

### Description

With the update of Serializer to 6.3.5, some deserialization of array to objects does behave differently (changed order of priority of configuration via attribute `#[SerializedPath]` vs. property name, when there is a key on root level with the same name as the private property.

Related to #49700.

### How to reproduce

Example to explain changed behavior:

```json
{
  "data": {
    "item": {
      "id": "id-1"
    }
  },
  "id": "id-2"
}
```

```php
final class SomeEvent
{
    #[SerializedPath('[data][item][id]')]
    public string $id;
}
```

Before 6.3.5, the value of the id was `id-1`, with the change of #49700, the value of the id becomes `id-2`.

#49700 changes `array_merge` with `array + array`. It seems that the problem stated above is related to the fact that array_merge does overwrite keys differently than array + array:

```php
$a = ['key' => 'value-a'];
$b = ['key' => 'value-b'];

var_dump(array_merge($a, $b));
// Results in:
// array(1) {
//   ["key"]=>
//   string(7) "value-b"
// }

var_dump($a + $b);
// Results in:
// array(1) {
//   ["key"]=>
//   string(7) "value-a"
// }
```

### Solution

As `array_merge` does behave slightly differently that array + array, the solution could be to switch array order to:
```diff
- $normalizedData = $normalizedData + $nestedData;
+ $normalizedData = $nestedData + $normalizedData;
```

This would result in the same, while keeping the fix (#49700) for the numeric key value

Commits
-------

67f49d4 Fix order array sum normalizedData and nestedData
@fabpot fabpot closed this as completed Oct 7, 2023
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

3 participants