Updated: Comment #N
Problem/Motivation
Drupal still uses a copy of the HttpKernel class which once was part of the Symfony Framework Bundle. However this class is long gone (deprecated in 2.2 and removed in 2.3) and therefore we also should get rid of our copy.
There are two candidates we could go for as a replacement for the custom http kernel:
Symfony\Component\HttpKernel\HttpKernelSymfony\Component\HttpKernel\DependencyInjection\ContainerAwareHttpKernel
In fact the latter is the successor of the HttpKernel we copied over from the framework bundle and only differs from the former that it still manages the request scope.
Regrettably just swapping out our custom kernel with ContainerAwareHttpKernel causes problems all over the place. Unlike our implementation it sets the request to NULL before leaving the request scope. In order to make our services compatible with the ContainerAwareHttpKernel we'd need to adapt them such they accept NULL for the $request parameter.
The other alternative (switching to Symfony\Component\HttpKernel\HttpKernel) requires us to convert all our services to the recently introduced request_stack. Given that Symfony moves away from request service as well as from container scopes, and that we have to touch all our request-dependent services anyway, this seems the better alternative.
Proposed resolution
- Convert all request-dependent services to
request_stack - Get rid of the request-scope.
- Remove
Drupal\Core\HttpKerneland replace it withSymfony\Component\HttpKernel\HttpKernel
Remaining tasks
User interface changes
API changes
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | 2223189-25.patch | 23.37 KB | cordoval |
| #23 | 2223189-23.patch | 399.89 KB | cordoval |
| #20 | 2223189-remove-http-kernel-20.patch | 24.44 KB | znerol |
| #4 | 2223189-remove-http-kernel-3.patch | 56.28 KB | znerol |
Comments
Comment #1
znerol commentedLet's see what explodes.
Comment #2
znerol commentedComment #4
znerol commentedWe probably will need to go a looong way and convert everything to the new RequestStack before being able to get rid of this legacy. Let's hope that this time we don't hit a fatal error...
Comment #5
znerol commentedComment #7
znerol commentedOk, the request-service (and worse
\Drupal::requestis spread all over the place. Let's take little steps: #2223593: Decouple the router.request_context service from the request service.Comment #8
znerol commentedAnother one: #2223615: Use request stack in local task/action manager
Comment #9
znerol commentedAdded #2223623: Use request stack in theme negotiator
Comment #10
znerol commentedAnother: #2223631: Use request stack in database flood backend
Comment #11
dawehnerI wonder whether we should make our code more request passing along like.
Comment #12
znerol commentedJust discovered that there is already an issue underway which helps us to decouple the
current_userservice fromrequest: #2180109: Change the current_user service to a proxy. Great!Comment #13
dawehnerRenaming title to not get confused again.
Comment #14
znerol commentedAdded #2225539: Use request stack in admin context service
Comment #15
znerol commentedOpened: #2225605: Use request stack in form builder
Comment #16
znerol commentedComment #17
znerol commentedComment #18
znerol commentedComment #19
znerol commentedThree new ones: #2236167: Use request stack in cache contexts, #2236207: Use router request context instead of request service in system breadcrumb builder, #2236245: Use request stack in system manager.
#2226573: Remove the optional $request argument from outbound path processor badly needs review. This will help uncoupling
UrlGenerator(and therefore alsourl()fromrequest.Comment #20
znerol commentedLet's see which tests are still blowing up.
Comment #22
sunComment #23
cordoval commentedI redid the changes on 8.x bleeding
there were some wrong things i corrected, from now on I talked with @crell and he told me to create a child issue and work on the propagation of the stuff
Comment #25
cordoval commentedreuploading patch rebased
Comment #26
cordoval commentedComment #27
cordoval commentedwait a sec, this is messed up,
will this be Kernel from symfony rather than HttpKernel?
ping @crell, @effulgentia
Comment #28
cordoval commentedoh nevermind I was confused :), thsi is still good!
Comment #30
znerol commentedFiled #2286971: Remove dependency of current_user on request and authentication manager.
Also #2226573: Remove the optional $request argument from outbound path processor still is important and still needs review.
Comment #31
Crell commentedThis was completed by other issues.