Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "request".
There is no core 'request' service anymore. It was removed from core in #2284103: Remove the request from the container (14 July 2014).
Any usage of the the current version of the BanIP action causes a whitescreen/fatal PHP error.
The request must now be obtained through the core 'request_stack' service.
The unit test for this action is: tests/src/Unit/Integration/Action/BanIPTest.php
The test does NOT fail or detect this problem because the current test is faulty - the test mocks a core 'request' service for BanIP to use, so it does not detect that the 'request' service no longer exists.
The patch in #5 changes BanIP to use the 'request_stack' service and changes the test to mock the 'request_stack' service. After applying the patch, the Rules Action BanIP can be used successfully in the UI.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 2922804-5-request_stack.patch | 4.29 KB | tr |
Comments
Comment #2
tr commentedComment #3
tr commentedFails because the existing test is faulty - the test mocks a core 'request' service for BanIP to use. This is why the test doesn't fail when BanIP tries to use that non-existent service.
Here's the patch with a modified version of the test. This modified test mocks the real 'request_stack' core service instead.
I've never used Prophecy before, so this may take a few iterations to get the new test right ...
Comment #4
tr commentedLet's try this again ...
Comment #5
tr commentedComment #6
tr commentedComment #7
tr commentedCan someone try this and RTBC it?
1) Try using the Ban IP action. All you have to do is try to create a reaction rule with this action. It should fail with the fatal error shown in the original post.
2) Apply the patch and try it again. It should work!
Comment #8
jonathan1055 commentedGood work @TR, yes, this patch fixes the error interactively, and I can create a rule with this action. When it is triggered it works as designed and the IP address is added to the banned list, as shown when visiting /admin/config/people/ban
I have also checked it with the tests I created and it works, following slight adjustment of the text to search for - see #2664280-25: Select lists in action & condition configuration forms
Here is a pull request to match the patch in #5 https://github.com/fago/rules/pull/498
The only thing which would be nice to do is somehow disable the 'Ban IP' selection option if the Ban module is not enabled. There should be a simple way to show this text but not allow the user to pick it. However, now that each option item is an object not array (like it used to be in D7) we can't just set
'#disabled' = TRUE. This can be a follow-up issue, as it does not impact the fix here.Jonathan
Comment #10
fagoThx, agreed! Merged.
Yeah the problem of plugins requiring special modules is something which I'd prefer to solve generally, such that plugins can specify the modules they need and we handle it properly based upon that metadata.
Comment #12
jonathan1055 commentedThanks for merging. Been away, back now, hence late response.