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

[Sf 6.4 RC2] Serialization problem #52744

Closed
COil opened this issue Nov 26, 2023 · 13 comments
Closed

[Sf 6.4 RC2] Serialization problem #52744

COil opened this issue Nov 26, 2023 · 13 comments

Comments

@COil
Copy link
Contributor

COil commented Nov 26, 2023

Symfony version(s) affected

6.4 RC2

Description

Hello, I have an API Platform route which returns a collection :

[
  {
    "id": 2,
    "type": "snippet",
    "name": "Test if a Doctrine entity is already persisted in the database",
    "datePublished": "2018-09-11T00:00:00+02:00",
    "dateModified": "2018-09-23T00:00:00+02:00",
    "author": "COil"
  },

It works well with Symfony 6.4 RC1, but with RC2 it returns an empty list :

[[],[],[],[],[],[],[],[],[],[]]

It comes from the serializer component, I get the bug when only updating this component :

composer up symfony/serializer                           ✔   39s  ⬢ 18.17.0   8.2.12  19:26:00
Loading composer repositories with package information
Restricting packages listed in "symfony/symfony" to "6.4.*"
Updating dependencies
Lock file operations: 0 installs, 1 update, 0 removals
  - Upgrading symfony/serializer (v6.4.0-RC1 => v6.4.0-RC2)

How to reproduce

up / down

Possible Solution

serializer component

Additional Context

  • PHP 8.2
  • Symfony 6.4 RC2
  • API Patform 3.1.7
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Nov 26, 2023

Thanks fr testing RCs :)

Can you please share a reproducer? The current description is not actionable I fear.

@COil
Copy link
Contributor Author

COil commented Nov 26, 2023

Hello Nicolas, :)
I can't give a reproducer for now, but the root cause is quite clear as doing :

composer up symfony/serializer

then :

git checkout composer.lock
composer install

Allows to trigger the bug or not. (OK with RC1, NOK with RC2).

I'll try to create a reproducer with MicroSymfony if I can find time this week.
Most of the code is in the blog post:
https://www.strangebuzz.com/en/blog/replacing-manual-api-endpoints-with-api-platform-3-in-a-symfony-application

It was also OK with all 6.4 beta versions.

@jdecool
Copy link
Contributor

jdecool commented Nov 26, 2023

I've got the same error on a project.

You can reproduce the error with this project sample.
The first commit use the RC1 without problems. The second commit use latest Symfony version (RC2).

The project is just a new project symfony new with orm and api dependency.

I also confirm that only the symfony/serializer is responsible of the bug.

@COil
Copy link
Contributor Author

COil commented Nov 26, 2023

Thanks for the reproducer @jdecool :)

@nicolas-grekas
Copy link
Member

/cc @mtarld maybe?

@mtarld
Copy link
Contributor

mtarld commented Nov 27, 2023

Hey! I checked a little bit and it appears that it is tightly related to API Platform.

Indeed, this behavior was introducted in #52680 (which still does make sense to me).

But API Platform do ignore the attribute extraction, and I don't know why, maybe @soyuka or @dunglas have an idea? 🙂

@soyuka
Copy link
Contributor

soyuka commented Nov 27, 2023

We don't use extractAttributes but we do use getAllowedAttributes but I don't get the link between this issue and extractAttributes can you add more information @mtarld ?

@okorneliuk
Copy link
Contributor

okorneliuk commented Nov 27, 2023

What I've found so far - when I have this:

<operation
                class="ApiPlatform\Metadata\Get"
                ...
                provider="App\State\MembershipProvider"
                output="App\Dto\MembershipResponse"
            />

and this

class MembershipProvider implements ProviderInterface
{
   ...
    public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
    {
        return new App\Dto\MembershipResponse('foo');
    }
}

the issue is reproduced, but when I remove/change the output of the operation, it works as expected.

Looks like the issue in API platform indeed.

@bendavies
Copy link
Contributor

bendavies commented Nov 27, 2023

this is not solely an api platform problem.
There is an issue just using raw serializer:

Given a serialization config like:

App\Entity\Foo:
  attributes:
    id:
      groups: ['read']
    bar.id:
      serialized_name: bar_id
      groups: ['read']

where Bar is a relation on Foo, bar_id is not serialized in RC2, but was in RC1.

@tacman
Copy link
Contributor

tacman commented Nov 27, 2023

Here's a repo that shows the issue. Works with serializer RC1, but breaks with RC2.

git clone git@github.com:survos-sites/dt-demo && cd dt-demo
composer install 
bin/console importmap:install
symfony server:start -d
symfony open:local --path=/congress/api_grid

Now change symfony/serializer to RC2, composer update, and it go to the same URL, it's broken.

composer req symfony/serializer:v6.4.0-RC2
symfony open:local --path=/congress/api_grid

@xabbuh
Copy link
Member

xabbuh commented Nov 27, 2023

@bendavies Your example broke with #52680 (bar.id is only part of the array returned by getAllowedAttributes() but extractAttributes() does not list it).

/cc @mtarld

@tacman
Copy link
Contributor

tacman commented Nov 27, 2023

FWIW, I'm experiencing the problem even if I explicitly list serialization groups, so it's not simply that only using the default ApiPlatform() attribute. In other project:

#[ApiResource(
    shortName: 'member',
    operations: [new Get(), new Put(), new Delete(), new Patch(), new GetCollection()],
    normalizationContext: [
        'groups' => ['member.read', 'member_extended', 'member_main', 'rp'],
    ],
    denormalizationContext: [
        'groups' => ['member.write'],
    ],
)]

#[Gedmo\Loggable]
#[Groups(['member.read'])]
class Member implements Loggable, RouteParametersInterface

@soyuka
Copy link
Contributor

soyuka commented Nov 27, 2023

@xabbuh indeed and this is the behavior API Platform is based on, we don't use extractAttributes() and we rely only ongetAllowedAttributes. I think it's a broken compatibility as if attributes are not set the array_intersect will take no attributes. I'd suggest to add if (false !== $allowedAttributes && $attributes) to the mentioned patch but we'll wait for @mtarld to be available (he's in a flight rn).

nicolas-grekas added a commit that referenced this issue Nov 28, 2023
…es only (mtarld)

This PR was merged into the 5.4 branch.

Discussion
----------

[Serializer] Fix normalization relying on allowed attributes only

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #52744
| License       | MIT

Commits
-------

ca16751 [Serializer] Fix normalization relying on allowed attributes only
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

10 participants