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

[FrameworkBundle] move wiring of the property info extractor to the ObjectNormalizer #54791

Merged
merged 1 commit into from
May 1, 2024

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Apr 30, 2024

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

The PropertyNormalizer does not handle a property info extractor. It looks like the argument was accidentally added to instead of the ObjectNormalizer in #52917.

@xabbuh
Copy link
Member Author

xabbuh commented Apr 30, 2024

ping @mtarld

The PropertyNormalizer does not handle a property info extractor. It looks
like the argument was accidentally added to instead of the ObjectNormalizer
in symfony#52917.
@chalasr
Copy link
Member

chalasr commented May 1, 2024

Good catch, thanks @xabbuh.

@chalasr chalasr merged commit ad82c43 into symfony:5.4 May 1, 2024
9 of 12 checks passed
@xabbuh xabbuh deleted the pr-52917 branch May 1, 2024 16:33
This was referenced Jun 2, 2024
@coudenysj
Copy link

I think #57293 breaks v7.0.8.

Upgrading FrameworkBundle from 7.0.7 to 7.0.8 triggers this:

In ContainerBuilder.php line 964:

  [Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]
  You have requested a non-existent service "serializer.normalizer.object".


Exception trace:
  at /app/vendor/symfony/dependency-injection/ContainerBuilder.php:964
 Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition() at /app/vendor/symfony/framework-bundle/DependencyInjection/FrameworkExtension.php:1915
 Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension->registerSerializerConfiguration() at /app/vendor/symfony/framework-bundle/DependencyInjection/FrameworkExtension.php:378
 ...

Downgrading symfony/framework-bundle (v7.0.8 => v7.0.7): Extracting archive
Fixes the problem.

@xabbuh
Copy link
Member Author

xabbuh commented Jun 4, 2024

see #57294 (will be fixed with #57297 in 7.0.9)

fabpot added a commit that referenced this pull request Jun 15, 2024
…normalizers (HypeMC)

This PR was merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle] Fix setting default context for certain normalizers

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

Caused by #54791. The main problem is that `$context` defaults to `[]` instead of `$defaultContext`. There's a test to check this, but it didn't work when `circular_reference_handler` or `max_depth_handler` were not `null`.

I also found an issue with `serializer.normalizer.property`. Since it’s not tagged with `serializer.normalizer`, which, to my understanding, is intentional, it would never have the default context bound to it.

Commits
-------

f903893 [FrameworkBundle] Fix setting default context for certain normalizers
@renjinsk
Copy link

see #57294 (will be fixed with #57297 in 7.0.9)

Sorry for the bump, but this also affects v5.4.40

@xabbuh
Copy link
Member Author

xabbuh commented Jun 19, 2024

#57297 was merged into the 5.4 branch and will be part of the 5.4.41 patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants