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.

Sign up for GitHub

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

Jump to bottom

Autowired ParamConverter no longer working #29819

Closed
tacman opened this issue Jan 8, 2019 · 18 comments
Closed

Autowired ParamConverter no longer working #29819

tacman opened this issue Jan 8, 2019 · 18 comments
Labels
Bug DependencyInjection Status: Waiting feedback

Comments

@tacman
Copy link
Contributor

tacman commented Jan 8, 2019

Symfony version(s) affected: 4.2.2

Description
I have a service that implements ParamConverterInterface that worked fine a few days ago, but when I run 'composer update', I can an error message that Doctrine cannot find the entity. This may be related to #29810

How to reproduce

I've attached my composer.json/lock
setup.zip

. Follow the instructions to create a ParamConverter. run composer install with my attached files. The controller should return a valid entity. Then run composer.update, now it will fail.

Sorry I can't provide a more precise way to reproduce this problem.
@xabbuh
Copy link
Member

xabbuh commented Jan 15, 2019

What is the error that you get?

@tacman
Copy link
Contributor Author

tacman commented Jan 15, 2019 via email

@tacman
Copy link
Contributor Author

tacman commented Jan 15, 2019

I can demonstrate this error in the Symfony Demo:

git clone git@github.com:survos/symfony-demo.git
cd symfony-demo
composer install
php vendor/bin/codecept run -f

Tests fail, the key issue is that PostParamConverter isn't being called. You can also see it with

bin/console server:start

Then open http://127.0.0.1:8000/en/blog/bug-posts/test-post-to-demonstrate-bug

@xabbuh
Copy link
Member

xabbuh commented Jan 15, 2019

Thank you for investing time into this example that greatly allowed me to reproduce what you experience.

However, this is the expected behaviour. If you do not add the @ParamConverter annotation, the SensioFrameworkExtraBundle tries to retrieve the Post object based on the route placeholders. But since there is no $postSlug property this resolution fails. You need to tell the param converter which property the route placeholder should be mapped to:

/**
 * @Route("/bug-posts/{postSlug}", methods={"GET"}, name="blog_post_bug")
 * @ParamConverter("post", options={"mapping": {"postSlug": "slug"}})
 */
public function postShowBug(Post $post): Response
{
    // ...
}

I am closing here as there is nothing to fix. Thank you for understanding.

@xabbuh xabbuh closed this as completed Jan 15, 2019
@tacman
Copy link
Contributor Author

tacman commented Jan 16, 2019 •
edited

I've finally (after far too many hours) found a simple test case that demonstrates the error.

If I add this to composer.json's require section, it works:

        "symfony/dependency-injection": "4.2.x-dev#e4adc57a48d3fa7f394edfffa9e954086d7740e5",

So to see this in action:

git clone git@github.com:survos/symfony-demo.git
cd symfony-demo
composer install
php vendor/bin/server:start 

success:  http://127.0.0.1:8000/en/blog/bug-posts/test-post-to-demonstrate-bug 

git checkout di-bug
composer install
(I seem to need to clear the cache after this as well, even though composer install should do it).

fails:  http://127.0.0.1:8000/en/blog/bug-posts/test-post-to-demonstrate-bug

The only difference between the two is composer.json changing from:

   "symfony/dependency-injection": "4.2.x-dev#e4adc57a48d3fa7f394edfffa9e954086d7740e5",

to

    "symfony/dependency-injection": "^4.2",

And of course the "composer update" command to generate the lock file.

@xabbuh xabbuh reopened this Jan 16, 2019
@xabbuh
Copy link
Member

xabbuh commented Jan 16, 2019

Thank you for investing the time to investigate this a bit more and sorry for closing too early. I was under the impression that you used the param converters from the SensioFrameworkExtraBundle, but there is a custom param converter class which indeed does not work as expected anymore.

@xabbuh
Copy link
Member

xabbuh commented Jan 16, 2019

There is good news though. This seems to be caused by the same as #29836 which will be fixed in 4.2.3 since #29597 was reverted in #29853.

Can you confirm that running composer require symfony/dependency-injection:~4.2@dev solves it?

@tacman
Copy link
Contributor Author

tacman commented Jan 16, 2019 via email

@xabbuh
Copy link
Member

xabbuh commented Jan 16, 2019

I see this error on both branches:

$ php vendor/bin/codecept run -f
Codeception PHP Testing Framework v2.5.2
Powered by PHPUnit 7.5.2 by Sebastian Bergmann and contributors.
Running with seed: 


App\Tests.acceptance Tests (2) -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Testing App\Tests.acceptance
✔ BlogCept: Open blog page and see article there (0.97s)
✔ LoginCept: Login as admin to backend (1.71s)
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

App\Tests.functional Tests (14) ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Testing App\Tests.functional
✖ PublicUrlsCept: Open public urls and see requested page (1.53s)
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

App\Tests.unit Tests (25) ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Testing App\Tests.unit
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


Time: 22.08 seconds, Memory: 52.25MB

There was 1 failure:

---------
1) PublicUrlsCept: Open public urls and see requested page
 Test  tests/functional/PublicUrlsCept.php
 Step  See response code is 200
 Fail  Expected HTTP Status Code: 200 (OK). Actual Status Code: 500 (Internal Server Error)
Failed asserting that 500 matches expected 200.

Scenario Steps:

 3. $I->seeResponseCodeIs(200) at tests/functional/PublicUrlsCept.php:17
 2. $I->amOnPage("/en/blog/posts/test-post-to-demonstrate-bug") at tests/functional/PublicUrlsCept.php:16
 1. // As an Anonymous


FAILURES!
Tests: 3, Assertions: 7, Failures: 1.

But this looks expected to me. The path used here matches the route pattern for the BlogController::postShow() method where the placeholder is named slug and not postSlug and thus making your param converter fail with this error:

Route attribute postSlug is missing

@tacman
Copy link
Contributor Author

tacman commented Jan 16, 2019 via email

@tacman
Copy link
Contributor Author

tacman commented Jan 16, 2019 via email

@xabbuh
Copy link
Member

xabbuh commented Jan 16, 2019

Thank you for the confirmation. 👍 I am closing then.

P.S.: Feel free to reach out at the Symfony Slack if you need to help with the implementation.

@xabbuh xabbuh closed this as completed Jan 16, 2019
@tacman
Copy link
Contributor Author

tacman commented Jan 16, 2019 via email •
edited

@xabbuh
Copy link
Member

xabbuh commented Jan 16, 2019

Sure, if you can give me access, I can try to take a look at it tonight.

@tacman
Copy link
Contributor Author

tacman commented Jan 16, 2019 via email

@xabbuh
Copy link
Member

xabbuh commented Jan 16, 2019

CET

@fabienlem fabienlem mentioned this issue Jan 18, 2019
Closed
@gaetan-petit
Copy link

@xabbuh When upgrading SensioFrameworkExtraBundle on an application using sf 3.4 paramconverter is broken.

It seems that the faulty code was revert in 4.* but not in 3.* as you can see here : https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php#L37

Should I fill an new issue?

@xabbuh
Copy link
Member

xabbuh commented Aug 28, 2019

Please do and please also provide a small example application that allows to reproduce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DependencyInjection Status: Waiting feedback
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tacman @gaetan-petit @xabbuh @carsonbot