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 702 forks source link

Symfony4 | Expose Services #1790

Closed juliusstoerrle closed 6 years ago

juliusstoerrle commented 7 years ago

When using SF4 we get following ServiceNotFoundException:

The "fos_rest.view_handler" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.

This is because in SF4 all Services are private by default.

I'm happy to submit a pull request, where I would make all services public which do not have a visibility set yet, or should i go through and only enable those ones that give me an ServiceNotFound?

As I'm a first time contributor, is there anything i should pay attention to? I did not find a contrib readme.

(Tagging @xabbuh as he just suggested me to do a PR for this)

xabbuh commented 7 years ago

I think we should make a case by case decision. Is each service actually meant to be used by third party code? Or is it only there for internal purposes? Services that are actually meant to be used could be turned to public to still allow users to make use of service location with the Symfony container. For the latter, we IMO don't need to do anything as users will see a deprecation with Symfony 3.4.

juliusstoerrle commented 7 years ago

Are there any services a consumer needs? I do not access the container directly in my code but still get the ServiceNotFound for "fos_rest.view_handler"

xabbuh commented 7 years ago

Does that happen when using Symfony 3.4 or 4.0? Would you be able to create a small reproducing project?

xabbuh commented 7 years ago

Are there any services a consumer needs?

I don't have any in mind. Could also be that none of them really needs to be exposed (at least there shouldn't be many that should turned into public ones).

juliusstoerrle commented 7 years ago

SF4, will try a demo but might take me a day or two

xabbuh commented 7 years ago

Could you downgrade to Symfony 3.4 and check for the exact deprecation message (there should be one)?

juliusstoerrle commented 7 years ago

Making a service public is also needed when the bundle itself accesses the container. I just had a look at the FosRestController and that might be the culprit. Look at line 34

$this->viewhandler = $this->container->get('fos_rest.view_handler');

A workaround would be to use the ControllerTrait and fetch the ViewHandler in my Concrete Controllers constructor - will do some testing and debugging

xabbuh commented 7 years ago

I had a look at the code and I wonder how this can happen. As far as I can see, the service seems to be set to public: https://github.com/FriendsOfSymfony/FOSRestBundle/blob/3e65ef5f5cef2ecc6f6a18d204dc5193f0feb272/DependencyInjection/FOSRestExtension.php#L246

juliusstoerrle commented 7 years ago

With Symfony Standard Edition 3.4-RC1 and current master I do not get any error or deprecation notice

xabbuh commented 7 years ago

Cool, thanks for confirming. I leave this ticket open though as we still have to look into other services we may try to retrieve from the container.

juliusstoerrle commented 7 years ago

I just went back to my SF4 project, and I'm still getting the ViewHandler not found error, but I can workaround that by using the ControllerTrait and a Constructor Argument ViewHandlerInterface.

juliusstoerrle commented 7 years ago

When I deserialize he is missing the fos_rest.decoder.json, setting public=truefor that service in the body_listerner.xml service config, does solve the problem. I guess the same applies to fos_rest.decoder.jsontoform and fos_rest.decoder.xml, but I am not using those two.

stof commented 7 years ago

we should not force these services to be public, but instead use Symfony 3.3+ lazy-loading features when available instead.

xabbuh commented 7 years ago

I agree with @stof. See #1793 for the implementation.

juliusstoerrle commented 7 years ago

When throwing exceptions he does not find fos_rest.serializer.exception_normalizer.jms:

(2/2) ErrorExceptionWarning: call_user_func() expects parameter 1 to be a valid callback, class 'fos_rest.serializer.exception_normalizer.jms' not found

in GraphNavigator.php (line 203)

Not sure how I can solve that one?

xabbuh commented 7 years ago

looks like that could be solved with schmittjoh/JMSSerializerBundle#618

juliusstoerrle commented 7 years ago

Pulled your changes to jms serializer and the bundle, now i got:

ServiceNotFoundException The "fos_rest.serializer.exception_normalizer.jms" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead. in Container.php (line 252)

at Container->get('fos_rest.serializer.exception_normalizer.jms')in getJmsSerializerService.php (line 80) at srcDevDebugProjectContainer->{closure}()in ServiceLocator.php (line 57) at ServiceLocator->get('fos_rest.serializer.exception_normalizer.jms')in LazyHandlerRegistry.php (line 57) at LazyHandlerRegistry->getHandler(1, 'Exception', 'json')in GraphNavigator.php (line 202) at GraphNavigator->accept(object(Exception), array('name' => 'Exception', 'params' => array()), object(SerializationContext))in Serializer.php (line 193) at Serializer->visit(object(JsonSerializationVisitor), object(SerializationContext), object(Exception), 'json', null)in Serializer.php (line 112) at Serializer->JMS\Serializer{closure}(object(JsonSerializationVisitor)) at call_user_func(object(Closure), object(JsonSerializationVisitor))in Some.php (line 89) at Some->map(object(Closure))in Serializer.php (line 115) at Serializer->serialize(object(Exception), 'json', object(SerializationContext))in JMSSerializerAdapter.php (line 59) at JMSSerializerAdapter->serialize(object(Exception), 'json', object(SerializationContext))in ViewHandler.php (line 453) at ViewHandler->initResponse(object(View), 'json')in ViewHandler.php (line 416) at ViewHandler->createResponse(object(View), object(Request), 'json')in ViewHandler.php (line 300) at ViewHandler->handle(object(View))in ExceptionController.php (line 71)

Anything I can do to debug this?

xabbuh commented 7 years ago

Can you publish a small example project that makes it possible to reproduce it?

juliusstoerrle commented 7 years ago

see https://github.com/juliusstoerrle/sf4-rest-error. My fos_rest config might be wrong too.

xabbuh commented 7 years ago

Thank you! I am going to check that.

xabbuh commented 7 years ago

@juliusstoerrle Just to give you quick feedback: I was able to reproduce the issue you reported with your application. Thanks for that. 👍 I also managed to identify one issue, but there's still some things more to fix. I'll keep you updated.

xabbuh commented 7 years ago

Please take a look at #1805 which will solve the first issue I was able to identify.

juliusstoerrle commented 7 years ago

Just pulled your changes into the example project, and there I'm getting following error: (I guess thats one of those additional issues you reported 2 days ago.)

FatalThrowableError

Type error: Argument 1 passed to FOS\RestBundle\Serializer\JMSHandlerRegistry::__construct() must implement interface JMS\Serializer\Handler\HandlerRegistryInterface, instance of Symfony\Component\DependencyInjection\ServiceLocator given, called in C:\xampp\htdocs\my-project\var\cache\dev\ContainerGfhFFM6\getJmsSerializerService.php on line 90

As stated in my comment in the PR, I do not get any errors in my main project, even through dependencies and config are identical.

xabbuh commented 7 years ago

@juliusstoerrle I guess that's just related to the pending changes in schmittjoh/JMSSerializerBundle#618.

juliusstoerrle commented 6 years ago

~Hey, when throwing exceptions I currently get:~

~> The "fos_rest.serializer.exception_normalizer.jms" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead~

~See https://github.com/juliusstoerrle/sf4-rest-error for the code~

By bad, when changing from SF4 Rcs to the release I also changed the fos rest version which resulted in not pulling the changes #1805

sanchobouillant commented 6 years ago

@juliusstoerrle how did you fix your issue ? I have the same

juliusstoerrle commented 6 years ago

@sanchobouillant Do you mean the issue with the service: fos_rest.serializer.exception_normalizer.jms?

That is solved by the PR #1805, but as that one is still open you need to put following version constraint into your composer and run composer update: "friendsofsymfony/rest-bundle": "dev-master#dcd5add5153af7e479e97a233efa6fdcd0164879 as 2.3",

Let me know if you need something else :)

sanchobouillant commented 6 years ago

that's solved my fos_rest.serializer.exception_normalizer.jms problem :D

Do you have any solution for the HandlerRegistryInterface issue ?

deniskhmel commented 6 years ago

Hi. How can i resolve this in my tests: The "fos_rest.view_handler" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead: 3x 2x in UserAddressControllerTest::testPostAddress from AppBundle\Tests\Controller\Api

juliusstoerrle commented 6 years ago

@deniskhmel What version of the bundle are you on and can you please post of UserAddressControllerTest::testPostAddress

juliusstoerrle commented 6 years ago

@sanchobouillant I think that was solved by https://github.com/schmittjoh/JMSSerializerBundle/pull/618. Are you on the latest Version of JMSSerializerBundle (^2.3 is mine). But I can't recall exactly what I did.

deniskhmel commented 6 years ago

yes, "jms/serializer-bundle": "^2.3", deprications appears just after method: $client->request('GET', '/api/addresses/');

juliusstoerrle commented 6 years ago

@deniskhmel I think you might mixed up my last two comments :) Does your Controller extend from FosRestController? If so I worked around the deprecation by creating my own base controller class:

<?php
declare(strict_types=1);

namespace App\Controller;

use FOS\RestBundle\Controller\ControllerTrait;
use FOS\RestBundle\View\ViewHandlerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\Controller as FrameworkController;

abstract class Controller extends FrameworkController
{
    use ControllerTrait;

    /**
     * @param ViewHandlerInterface $handler
     */
    public function __construct(ViewHandlerInterface $handler)
    {
        $this->setViewHandler($handler);
    }

}

The Problem here is that the FosRestController wants to fetch the view handler service directly from the container, but that does not work from SF4 onward because the Service is registered as private.

@xabbuh Where you able to track down why the service is private? (Re: https://github.com/FriendsOfSymfony/FOSRestBundle/issues/1790#issuecomment-346330143)

deniskhmel commented 6 years ago

This is not controller - this is test for controller

// use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Liip\FunctionalTestBundle\Test\WebTestCase;

class UserAddressControllerTest  extends WebTestCase
{
    public function testGetAddresses()
        {
            $client = static::createClient();
            $client->request('GET', '/api/addresses/');
        ...
    }
}

Before i tried to extends from: use Symfony\Bundle\FrameworkBundle\Test\WebTestCase - the same result;

juliusstoerrle commented 6 years ago

@deniskhmel Yes but the test is calling a controller (the one it is supposed to test). It's not the test which is the problem here.

deniskhmel commented 6 years ago

I made new base class controller, as you show above and extends my controller from it. Now i got error: {"code":500,"message":"A \"ViewHandlerInterface\" instance must be set when using the FOSRestBundle \"ControllerTrait\"."}

juliusstoerrle commented 6 years ago

@deniskhmel Thats unexpected, I can not see how that can happen, ViewHandlerInterface is not an optional argument of the constructor.

sanchobouillant commented 6 years ago

@juliusstoerrle I try with 2.3 and dev-master. Still have the issue.

I will create a new issue

XWB commented 6 years ago

A similar issue is present in FOSCommentBundle:

The "fos_rest.view_handler" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead: 23x
    5x in ApiTest::testReplyToComment from FOS\CommentBundle\Tests\Functional
    4x in ApiTest::testGetCommentFlatSorted from FOS\CommentBundle\Tests\Functional
    3x in ApiTest::testAddCommentToThread from FOS\CommentBundle\Tests\Functional
    2x in ApiTest::testGetThreadFormAndSubmit from FOS\CommentBundle\Tests\Functional
    2x in ApiTest::testGetEmptyThread from FOS\CommentBundle\Tests\Functional
    2x in ApiTest::testGetCommentTree from FOS\CommentBundle\Tests\Functional
    2x in ApiTest::testGetCommentTreeDepth from FOS\CommentBundle\Tests\Functional
    2x in ApiTest::testGetCommentFlat from FOS\CommentBundle\Tests\Functional
    1x in ApiTest::testGetThread from FOS\CommentBundle\Tests\Functional
andrzepakula commented 6 years ago

Fos rest is not compatible with symfony 4. It does not use controller as service ( https://symfony.com/doc/3.1/controller/service.html ) I have the same error. FosRestController should inherit from abstract controller and inject fos_rest_view_handler. Is a fixed planned?

eloyekunle commented 6 years ago

What is the fix for this?

xabbuh commented 6 years ago

@XWB Thanks for the hint with the FOSCommentBundle builds. It allowed me to find the root cause which was now fixed by #1840.