fix(serializer): Allow nested denormalization when allow_extra_attributes=false

[calbro7]

Q	A
Branch?	4.1
Tickets	Resolves #6225
License	MIT

The linked issue was resolved for JSON LD format with #6451 but not for standard JSON, so this PR implements the same change in the standard ItemNormalizer.

The included test file demonstrates the expected behaviour, it should be possible to alter the Bar entity's someProperty via a PATCH request to edit its parent Foo object.

This test passes with the ItemNormalizer change, but fails without it due to Extra attributes are not allowed (\"id\" is unknown)..

First contribution so let me know if I've done things correctly or if I've missed anything needed for approval!

[soyuka]

This looks like it breaks one of our test would you be able to identify why? Inside CONTRIBUTING there's everything you need to run tests.

[calbro7]

Hello, possibly I'm misunderstanding what you mean, I think it always will be id, as it's not configurable is it? And it's also hardcoded eg here:

private function updateObjectToPopulate(array $data, array &$context): void
{
    try {
        $context[self::OBJECT_TO_POPULATE] = $this->iriConverter->getResourceFromIri((string) $data['id'], $context + ['fetch_data' => true]);

When/how could it be something else?

[calbro7]

This looks like it breaks one of our test would you be able to identify why? Inside CONTRIBUTING there's everything you need to run tests.

Hello, thanks for taking a look. Yep I'll get that sorted!

[calbro7]

This looks like it breaks one of our test would you be able to identify why? Inside CONTRIBUTING there's everything you need to run tests.

This was because with my changes then there were some scenarios where id shouldn't have been present in the $data passed to the parent denormalizer, but ended up being so.

I've fixed this by unsetting id before that method call (commit), but making sure that it's not unset if it legitimately should be there, to still allow for passing an id when deserializing into a new object (such as the testDenormalizeWithIdAndNoResourceClass test).

Let me know if this approach is good or if I'm overlooking any problems!

[calbro7]

Hi @soyuka - is the approach good? is there anything else necessary to get this merged (there's only one failing check, it's from php-cs-fixer in unrelated files, so I've left them as they are, per the contributing.md)?

Sorry to nudge but I just want to check if this PR is still being looked at and not forgotten about. 🙂

[soyuka]

Thanks @calbro7 !