search

FriendsOfSymfony / FOSRestBundle

This Bundle provides various tools to rapidly develop RESTful API's with Symfony
http://symfony.com/doc/master/bundles/FOSRestBundle/index.html
MIT License
2.79k stars 708 forks source link

Add support for Symfony 7 where able #2400

Closed mbabker closed 3 months ago

mbabker commented 6 months ago

This PR upgrades most of the bundle to support Symfony 7. Also wrapped into this PR are:

The one feature not covered is the request body param converter, which has its own dedicated PR to deal with implementing an argument resolver replacing that.

alexander-schranz commented 5 months ago

Hello @mbabker I just got today some time to Test @sulu which has no dependency to Sensio Extra Bundle against this pull request.

https://github.com/sulu/sulu/pull/7272

Sulu itself still on Symfony 6.4 but I think it is good to know that all still works like expected also on Sulu side where the Extra Bundle is not installed. And so not any accidently bc break was introduced here which would trigger an error on such project like Sulu. Sulu uses the JMS Serializer inside FOSRest and all seems still work there also like expected.

@mbabker What do you think is required to check to move this forward?

mbabker commented 5 months ago

What do you think is required to check to move this forward?

I think for me, in priority order, the PRs to focus on are:

I treat #2401 as a blocker to this because I basically disabled the whole #[View] attribute/annotation processing and the event listener that converts a View returned from a controller into a Response on this PR since the attribute class is extending from a class in SensioFrameworkExtraBundle. So getting that one finished first lets me undo some of the changes I made in this PR to get a (mostly) passing CI.

Finishing this PR up after that gets to a good "we're most of the way there" milestone.

2398 is probably the hardest to deal with because it is impossible to make a one-for-one replacement of the existing behavior. But at the same time, it's already an optional integration, so I wouldn't let the param converter not being Symfony 7 compatible block shipping an update that adds Symfony 7 support for all other features in this bundle, and the folks relying on that can continue to use the Symfony 6.4 LTS for a little while longer.

DelphineCogi commented 3 months ago

Is there any update here ? 🙏

TheDevick commented 3 months ago

This PR is almost 5 months old and still not merged. Any updates about that?

mbabker commented 3 months ago

This specific pull request isn't moving forward until its blocker, https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2401, is addressed. From my end, nothing has been forgotten about here and there is no need for the "updates please?" comments.

mbabker commented 3 months ago

For those keeping score at home, this is now rebased on top of the recently merged #2401. There are some things that need to be undone in this PR because of the changes that the other PR worked around, those will get tackled soon.

alexander-schranz commented 3 months ago

Looks good from my side. @mbabker as you removed the draft this merge request is so not longer WIP?

W0rma commented 3 months ago

I tested this branch in one of my projects and it worked fine. @mbabker Thank you for the hard work!

mbabker commented 3 months ago

@mbabker as you removed the draft this merge request is so not longer WIP?

I knew I forgot something when updating everything else in this PR 😆

But, yeah, aside from the one major feature callout, this should be G2G for use with Symfony 5.4, 6.4, and 7.0.

goetas commented 3 months ago

Thanks for the exceptional quality of work! Released as 3.7.0 https://github.com/FriendsOfSymfony/FOSRestBundle/releases/tag/3.7.0

alexander-schranz commented 3 months ago

Thx @goetas and thank you @mbabker good work 💪