Complete sf5 compatibility

[l-vo]

I had a problem when migrating a project to Symfony 5.

The container crashes during compilation because setContainer of CaptchaController was not called (and it's normal because CaptchaController is not autowired). To fix this, this PR:

• Remove extends AbstractController since CaptchaController doesn't use its services/methods anymore
• Add controller.service_arguments tag to the controller to allow it to be registered even if it doesn't extends AbstractController anymore.

I'm suggesting some other changes to match better with the sf4/sf5 DI philosophy:

• Services only need to be public if they are retrieved with $container->get('myservice'); I think none of the services of this bundle are concerned.
• The controller alias is not needed, the controller.service_arguments tag (and previously the AbstractController parent class) allows to make the service public.

[ViniTou]

@Gregwar
Can we get any status with that? As I can confirm that it breaks bundle on sf5.

[Gregwar]

@ViniTou does this PR fixes SF5 compatibility for you ?

[ViniTou]

@Gregwar
Yes,
localy i just added

        calls:
            - [setContainer, ["@service_container"]]

to controller definition but I agree with @l-vo that we do not need whole container there.
So if you don't care about BC breaks that comes from removing abstract class and making those services private, then yes I would merge it.

[vibby]

Can we know the status for that PR, it would be real great to have that bundle deprecation-less in sf5 ;)

[Gregwar]

@vibby apparently the original PR is now conflicting
@l-vo if you fix that I will give it a try and merge it

[Gregwar]

(I am sorry with delays I can have sometime, don't hesitate to ping me more 😅)

[l-vo]

I'm having a look this week end. I'm going to try to improve it deprecating public service instead of removing it.

[l-vo]

@vibby what deprecation(s) are you talking about ? This PR aims to remove useless things (original problem with container injection has been fixed by fec0ebb), but basically it doesn't fix any deprecation.

[vibby]

Hello. I am with version v1.1.8 (6088ad3db59bc226423ad1476a9f0424b19b1866).
The deprecation appears in symfony profiler : ````Since symfony/dependency-injection 5.1: The "Psr\Container\ContainerInterface" autowiring alias is deprecated. Define it explicitly in your app if you want to keep using it. It is being referenced by the "gregwar_captcha.controller" service.```

Warning hides when I remove the the autowire: true (line 20) in vendor/gregwar/captcha-bundle/Resources/config/services.yml
The line still exists in https://github.com/Gregwar/CaptchaBundle/blob/master/Resources/config/services.yml
The feature seems to works great without that line, maybe a legacy ?

[l-vo]

@vibby thank you for your feedback. A fix for the deprecation is proposed in #218. autowire: true is not a legacy, @Gregwar added it to solve the container injection problem presented in this PR. The fix actually does the job but it injects the whole container instead of the subset of services needed by AbstractController (it's why you have a deprecation). The application seems work properly without the fix because the problem arises only if you use the captcha reload feature 🙂.

@Gregwar I close this PR in favor of #218 (due to the possible BC breaks). But I still think AbstractController dependency is useless and should be removed and the public services should be turned to private. But it would be better to do that in a major version. If you want to go this way I can help 🙂