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

FOSRest namespace is not defined and argument #2 must be of type array #2409

Open
flohw opened this issue Apr 8, 2024 · 1 comment
Open

Comments

@flohw
Copy link
Contributor

flohw commented Apr 8, 2024

Hello there,

As discussed in #2400 I looked at the error a bit longer this morning and preferred to open an issue for better tracking instead of continuing the thread there. The exception is raised when sensio extra bundle is installed and the controller returns an object which is expected to be serialized by the bundle but is passed to twig instead.

I can confirm that by reverting the weight to 30 on kernel view event it fixes the issue.

The reason is because the View attribute extends the Template class if it exists (which exists because sensio extra bundle is installed (check made line 16 of View)

As the TemplateListener of the extra bundle has a higher priority than the ViewResponseListener, the Template listener tries to render the template defined by the template guesser (line 71 of TemplateListener) the guesser uses bundle's name, route, controller to define a name (I guess, I didn't looked at it as it's not relevant for us) that's why we get the exception FOSRest namespace is not defined.

The other exception is raised because the $parameters returned by the controller are expected to be an array of variables passed to a twig render method which FOSRest used to bypass with its ViewResponseListener (which serialize the controller response so the TemplateListener can't be used (line 85 returns true so return l86 is invoked)

I think I can provide a patch but not sure about the test strategy: should I only test the event weight with a comment or should I create a functional test which would trigger that particular case and resolve by changing the event weight?

Before providing the patch, it would be nice if someone else could double check what I explained if I didn't misunderstood the code or missed something else aka I don't have the right explanation of the bug as I'm not an expert of the whole bundle. 🙂

Thanks 🙂

ping to @mbabker and @benconda as you were the latest involved in the discussion.

@mbabker
Copy link
Contributor

mbabker commented Apr 8, 2024

I'd do a functional test here because what we want is to test to make sure the response comes back correctly even when the Sensio bundle is installed and has its view integration enabled. This diff for the tests is enough to cover that:

diff --git a/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php b/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php
index 1002f5e1..8397f718 100644
--- a/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php
+++ b/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php
@@ -32,14 +32,26 @@ class ArticleController extends AbstractFOSRestController
     }
 
     /**
-     * @Get("/articles.{_format}", name="get_article", defaults={"_format": "html"})
+     * @Get("/articles.{_format}", name="get_articles", defaults={"_format": "html"})
      *
      * @View()
      */
-    #[Get(path: '/articles.{_format}', name: 'get_article', defaults: ['_format' => 'html'])]
+    #[Get(path: '/articles.{_format}', name: 'get_articles', defaults: ['_format' => 'html'])]
     #[View]
     public function cgetAction()
     {
         return $this->view();
     }
+
+    /**
+     * @Get("/articles/{id}.{_format}", name="get_article", defaults={"_format": "html"})
+     *
+     * @View()
+     */
+    #[Get(path: '/articles/{id}.{_format}', name: 'get_article', defaults: ['_format' => 'html'])]
+    #[View]
+    public function cgetSingleAction(int $id)
+    {
+        return ['id' => $id];
+    }
 }
diff --git a/Tests/Functional/ViewResponseListenerTest.php b/Tests/Functional/ViewResponseListenerTest.php
index afcbe743..24a82c6d 100644
--- a/Tests/Functional/ViewResponseListenerTest.php
+++ b/Tests/Functional/ViewResponseListenerTest.php
@@ -35,4 +35,20 @@ class ViewResponseListenerTest extends WebTestCase
         $this->assertSame('http://localhost/hello/Post%201', $client->getResponse()->headers->get('location'));
         $this->assertStringNotContainsString('fooo', $client->getResponse()->getContent());
     }
+
+    public function testControllerReturnsView()
+    {
+        $client = $this->createClient(['test_case' => 'ViewResponseListener']);
+        $client->request('GET', '/articles.json');
+
+        $this->assertTrue($client->getResponse()->isSuccessful());
+    }
+
+    public function testControllerReturnsArray()
+    {
+        $client = $this->createClient(['test_case' => 'ViewResponseListener']);
+        $client->request('GET', '/articles/1.json');
+
+        $this->assertTrue($client->getResponse()->isSuccessful());
+    }
 }
diff --git a/Tests/Functional/app/ViewResponseListener/bundles.php b/Tests/Functional/app/ViewResponseListener/bundles.php
index 90086464..2a4b9abf 100644
--- a/Tests/Functional/app/ViewResponseListener/bundles.php
+++ b/Tests/Functional/app/ViewResponseListener/bundles.php
@@ -11,6 +11,7 @@
 
 $bundles = [
     new \Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
+    new \Symfony\Bundle\TwigBundle\TwigBundle(),
     new \FOS\RestBundle\FOSRestBundle(),
     new \FOS\RestBundle\Tests\Functional\Bundle\TestBundle\TestBundle(),
 ];

The key thing here is the view response listener's functional test app didn't have TwigBundle installed, so just adding the test cases wasn't enough on this one because the Sensio bundle will disable the view listener if Twig isn't active. Once I added that bundle to the test app, the errors came up pretty quickly.

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

No branches or pull requests

2 participants