Add SessionListener::onFinishRequest method to be inline with Symfony 3.4.12

[emodric]

Fixes #465

Same issue is probably happening on master, so needs to be forward ported.

[XWB]

Wouldn't it be easier to extend AbstractSessionListener and remove the getSubscribedEvents method?

[emodric]

@XWB That wouldn't prevent the original Symfony event listener to set the response to private, one already extending the abstract listener.

[andrerom]

+1, but it does not avoid this happening again the future..

Should we add a __call() instead with the if check here?? /cc @dbu @Toflar
Maybe throw something if inner does not have what is called for.

[Toflar]

I don't think this would reduce the risk of something like this happening. Quite the opposite, I think it could even cause potential errors to remain undiscovered. __call() would blindly forward to the decorated service but potentially it should do something different 😄

[andrerom]

__call() would blindly forward to the decorated service but potentially it should do something different 😄

Well it can check which methods BaseSessionListener::getSubscribedEvents() hints about and only forward those. But yeah, I'm not saying that as a solution is pretty, just that we have a potential recurring fatal problem here.

[andrerom]

So maybe we go ahead with this as is for now, you have my +1 so if @Toflar is +1 as well it's up to you @dbu (or other maintainers like @ddeboer)

[Toflar]

Tests need to be fixed :)

[Toflar]

The test should be skipped if the method does not exist in the core session listener. See https://travis-ci.org/FriendsOfSymfony/FOSHttpCacheBundle/jobs/396800434

[emodric]

@Toflar Done, however, PHP 5.4 and 5.6 builds still fail due to OOM errors.

[Toflar]

LGTM @dbu, failing tests are unrelated and due to memory (thank you PHP 7 for giving us better memory efficiency). Thanks @emodric for the PR <3

[emodric]

Thank you @Toflar and @andrerom for review!

[dbu]

I changed the lowest build to 2.8.0 in other projects, to reduce the possibility space. Could be done here too.

Github doey not allow me to merge on the phone when tests fail. @ddeboer could your merge this and tag a patch release? Otherwise i can try to do it tonight.

[dbu]

merged in #472