[Meta] Impact of SensioFrameworkExtraBundle deprecation

[mbabker]

In symfony/symfony#44705 it looks like there's a strong consensus for deprecating SensioFrameworkExtraBundle (which, IMO, makes sense; everything that bundle provides can be done with a core install these days, the only features it really provides are some annotations that affect the DX of how controllers are written).

If that bundle is deprecated, it does have some impact on features here, including:

• The request body param converter (deserialize the request into a DTO) - [HttpKernel] Create Attributes #[MapRequestPayload] and #[MapQueryString] to map Request input to typed objects symfony/symfony#49138
• The @View annotation (which is an extension of the @Template annotation)

The param converter can already be addressed by creating an argument resolver with an attribute for the extra configuration (IMO, with Symfony 6 being PHP 8+ and 7.4 having entered security only support, I don't think this new class would need to support annotations).

The @View annotation would either need to be decoupled from the @Template annotation, rewritten to use whatever replacement is introduced into the framework itself (if one is added), or deprecated in full.

[W0rma]

Seems like SensioFrameworkExtraBundle has been deprecated (sensiolabs/SensioFrameworkExtraBundle#783).

[mbabker]

The Symfony PR for the request body converter didn't land in 6.2, and it's hardcoded to the Symfony serializer component anyway, so it wouldn't be a full replacement for the converter here (which can use the abstraction layer to switch between the Symfony or JMS serializer as appropriate).

Also, the HttpKernel's argument value resolvers aren't allowed to return more than one value except for a variadic argument, so the feature provided here where the deserialized model can be automatically validated and the resulting violation list passed through to the controller won't be available. With that said...

When updating one of my apps to 6.2, I made a pretty simplified version of the converter already in place for this bundle:

• Only supports Symfony 6.2 and PHP 8.1
• I don't have any of the serialization context config in the constructor, the only way to configure the serialization context is through the attribute
• I'm not conditionally throwing based on optional params as all of my deserialization cases use required arguments

So if you intend to use it on something older or support other functionality, keep in mind you'll have to make changes:

<?php declare(strict_types=1);

namespace App\ArgumentResolver;

use App\Http\Attribute\RequestBody;
use FOS\RestBundle\Context\Context;
use FOS\RestBundle\Serializer\Serializer;
use JMS\Serializer\Exception\Exception as JMSSerializerException;
use JMS\Serializer\Exception\UnsupportedFormatException;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Controller\ValueResolverInterface;
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\HttpKernel\Exception\UnsupportedMediaTypeHttpException;
use Symfony\Component\Serializer\Exception\ExceptionInterface as SymfonySerializerException;

final class RequestBodyValueResolver implements ValueResolverInterface
{
    public function __construct(
        #[Autowire(service: 'fos_rest.serializer')] private readonly Serializer $serializer,
    ) {
    }

    public function resolve(Request $request, ArgumentMetadata $argument): iterable
    {
        /** @var RequestBody|null $attribute */
        $attribute = $argument->getAttributes(RequestBody::class, ArgumentMetadata::IS_INSTANCEOF)[0] ?? null;

        if ($attribute === null) {
            return [];
        }

        $this->configureContext($context = new Context(), $attribute);

        $format = $request->getContentTypeFormat();

        if ($format === null) {
            throw new UnsupportedMediaTypeHttpException();
        }

        try {
            return [$this->serializer->deserialize($request->getContent(), $argument->getType(), $format, $context)];
        } catch (UnsupportedFormatException $e) {
            throw new UnsupportedMediaTypeHttpException($e->getMessage(), $e);
        } catch (JMSSerializerException|SymfonySerializerException $e) {
            throw new BadRequestHttpException($e->getMessage(), $e);
        }
    }

    private function configureContext(Context $context, RequestBody $attribute): void
    {
        foreach ($attribute->attributes as $key => $value) {
            $context->setAttribute($key, $value);
        }

        if ($attribute->version !== null) {
            $context->setVersion($attribute->version);
        }

        if ($attribute->groups !== null) {
            $context->addGroups($attribute->groups);
        }

        if ($attribute->isMaxDepthEnabled === true) {
            $context->enableMaxDepth();
        } elseif ($attribute->isMaxDepthEnabled === false) {
            $context->disableMaxDepth();
        }

        $context->setSerializeNull($attribute->serializeNull);
    }
}

And the accompanying Attribute class:

<?php declare(strict_types=1);

namespace App\Http\Attribute;

#[\Attribute(\Attribute::TARGET_PARAMETER)]
final class RequestBody
{
    public function __construct(
        public readonly array $attributes = [],
        public readonly ?string $version = null,
        public readonly ?array $groups = null,
        public readonly ?bool $isMaxDepthEnabled = null,
        public readonly ?bool $serializeNull = null,
    ) {
    }
}

I'm now just handling validation inside my controllers instead of trying to rely on request state or prioritizing resolvers correctly or any of that fancy stuff.

====

I've personally never used the annotations/attributes that SensioFrameworkExtraBundle provided for all the other features, so I honestly have no idea how to gauge the effort to migrate the @View annotation.

[benrcole]

I have just moved a project to 6.2 and the default #[MapEntity] conversion now provided overwrote the #[ParamConverter('object', converter: 'fos_rest.request_body')] conversion I was using previously. This also generated an error because an object ID was required by Doctrine to complete the #[MapEntity] conversion whereas #[ParamConverter] will take a partially created object, I'm using this to create an object so no ID can be provided here.

I'd guess even with the replacement of the #[ParamConverter] converter here the default behaviour provided in 6.2 will still need to be disabled for this bundle to continue to function consistently, is that right?

[mbabker]

The #[MapEntity] attribute configures the EntityValueResolver and is the replacement for the DoctrineParamConverter that was provided by the SensioFrameworkExtraBundle, not the RequestBodyParamConverter provided by this bundle. I feel like I've seen a couple of issues on the Symfony repository about that exact scenario you're discussing (using the param converters to deserialize into an entity and it failing when you're trying to create a new record), but I can't find them at the moment.

[benrcole]

OK, so I just found a conflict with the default behaviour of Symfony 6.2?

I have the exact configuration described by request_body_converter_listener.rst but have needed to add #[MapEntity(disabled: true)] to my constructor so that I'm only utilising the RequestBodyParamConverter.

Perhaps it's not directly related to the deprecation of SensioFrameworkExtraBundle but without the configuration to disable the EntityValueResolver, the configuration described for request_body_converter_listener.rst doesn't function properly.

[mbabker]

The two systems have some functional differences (when they're fired in the request cycle, the param converters by default store all of their resolved values to the request's attributes bag but I don't think the value resolvers do the same), so it's not a straightforward one-for-one swap. The notes on that doc page are really only going to work if you're only using the param converters from SensioFrameworkExtraBundle; once all the value resolvers added in Symfony 6.2 come into play, those docs aren't really useful anymore (not to say they're bad, there's just a need for updates to account for the changes with Symfony 6.2 and the SensioFrameworkExtraBundle deprecation). symfony/symfony#48433 might help a bit in this type of scenario though.

[benrcole]

Thanks for the tips.

My issue was solved purely by disabling the MapEntity resolver. That's really all that needs to be a added to the documentation (for use with 6.2)

[fmarchalemisys]

Due to the hard dependency between RequestBodyParamConverter and Sensio's extra bundle, it is not possible to get rid of the latter without getting an exception from ConfigurationCheckPass.

I understand, from this discussion, that it's not possible to disable RequestBodyParamConverter.

I managed to short-circuit DoctrineParamConverter by overriding it with a class that does nothing:

class AntiDoctrineParamConverter extends DoctrineParamConverter
{
    public function apply(Request $request, ParamConverter $configuration)
    {
        return false;
    }
}

And, in services.yaml:

services:
    "sensio_framework_extra.converter.doctrine.orm":
        class: App\AntiDoctrineParamConverter

Could this results in loss of functionality with FOSRestBundle?

[mbabker]

There are ways to disable the param converter integrations, but turning those off does disable functionality (if you're on Symony 6.2, the only thing you're missing is the request body conversion, which you can restore in a few ways).

If you're aiming to keep the Sensio bundle installed but disable features that are now part of the core framework, you've got two options (depending on your needs) for the param converters specifically:

# Fully disable the param converter integration
sensio_framework_extra:
    request:
        converters: false

# Disables the Doctrine ORM param converter only
sensio_framework_extra:
    request:
        disable:
            - 'doctrine.orm'

If you're trying to remove the Sensio bundle in full (which you should be able to do, there aren't many actively maintained bundles which have a hard requirement to the Sensio bundle; even this one only has it in require-dev), then you need to check if you've set the fos_rest.body_converter.enabled config key to true; by default, the body converter (which requires the param converter API from the Sensio bundle) is disabled and needs an explicit opt-in to use it.

[fmarchalemisys]

@mbabker thanks for the quick answer.

I'm indeed trying to remove sensio's extra bundle.

fos_rest.body_converter.enabled is set to false. It does remove the dependency on ParamConverter.

But FOS\RestBundle\Controller\Annotations\View extends from Sensio\Bundle\FrameworkExtraBundle\Configuration\Template and that one looks harder to fix 🙂 .

[mbabker]

If you're using that feature, you need the Sensio bundle. That functionality hasn't been updated yet (and as my original post says, I don't use that particular feature so I'm not in a good spot to gauge the effort involved in fixing that).

[mbabker]

Reading https://symfony.com/blog/new-in-symfony-6-3-mapping-request-data-to-typed-objects this morning got me thinking about this whole migration again. The core framework features are nice, but they hard-couple you to the framework's serializer and I think one of the benefits of this bundle is it lets you work with either the Symfony or JMS serializer, so just using those is out of the question.

That gets me thinking about this bundle's features as a whole. Obviously, the FOS\RestBundle\Request\RequestBodyParamConverter has to be replaced as that only works with the SensioFrameworkExtraBundle. Any implementation is going to end up looking pretty darn close to the new Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestPayloadValueResolver class (depending on what aspects of deserialization are to be supported, does the replacement support deserializing all of the things the framework resolver does or does it continue only supporting handling $request->getContent()) with the serializer adapter from the param converter class applied instead of hard-coupled to a single component. That probably also means new attribute classes to keep this implementation from depending on the framework too closely.

That then makes me start thinking about the FOS\RestBundle\Request\ParamFetcherInterface and the FOS\RestBundle\Controller\Annotations\ParamInterface attributes. These provide a nice way of documenting parameters for a controller alongside their validation rules, and packages like NelmioApiDocBundle provide integrations building on this feature. But, in my mind, it feels like this feature just duplicates the request deserialization feature as far as data retrieval goes, but at the same time, it clearly serves a purpose. I'd personally lean toward deprecating this, but I imagine that wouldn't be the most popular decision out there.

There is still also the FOS\RestBundle\Controller\Annotations\View attribute and FOS\RestBundle\EventListener\ViewResponseListener to migrate forward. The attribute relies on the Sensio\Bundle\FrameworkExtraBundle\Configuration\Template class, but looking at the actual implementation and considering that the 3.x release dropped support for rendering a template in the FOS\RestBundle\View\ViewHandlerInterface implementations, it seems like these classes really don't need to be aware of the Sensio bundle and could just operate standalone. For the attribute, it comes with a B/C break of no longer inheriting from the Sensio bundle's class (changes instanceof type stuff), but the event listener seems like it could be migrated without too much hassle.

So I guess it's just a matter of figuring out what to migrate forward, how to do it, and what PHP/framework versions need to be supported in the newer code (i.e. Symfony 6.2 deprecates some stuff in the controller argument resolvers so only supporting 6.2+ has a simpler implementation than 5.4+).