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\HttpKernel
Symfony\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\HttpKernel
and 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 CreditAttribution: znerol commentedLet's see what explodes.
Comment #2
znerol CreditAttribution: znerol commentedComment #4
znerol CreditAttribution: 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 CreditAttribution: znerol commentedComment #7
znerol CreditAttribution: znerol commentedOk, the request-service (and worse
\Drupal::request
is spread all over the place. Let's take little steps: #2223593: Decouple the router.request_context service from the request service.Comment #8
znerol CreditAttribution: znerol commentedAnother one: #2223615: Use request stack in local task/action manager
Comment #9
znerol CreditAttribution: znerol commentedAdded #2223623: Use request stack in theme negotiator
Comment #10
znerol CreditAttribution: 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 CreditAttribution: znerol commentedJust discovered that there is already an issue underway which helps us to decouple the
current_user
service fromrequest
: #2180109: Change the current_user service to a proxy. Great!Comment #13
dawehnerRenaming title to not get confused again.
Comment #14
znerol CreditAttribution: znerol commentedAdded #2225539: Use request stack in admin context service
Comment #15
znerol CreditAttribution: znerol commentedOpened: #2225605: Use request stack in form builder
Comment #16
znerol CreditAttribution: znerol commentedComment #17
znerol CreditAttribution: znerol commentedComment #18
znerol CreditAttribution: znerol commentedComment #19
znerol CreditAttribution: 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 CreditAttribution: znerol commentedLet's see which tests are still blowing up.
Comment #22
sunComment #23
cordoval CreditAttribution: 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 CreditAttribution: cordoval commentedreuploading patch rebased
Comment #26
cordoval CreditAttribution: cordoval commentedComment #27
cordoval CreditAttribution: cordoval commentedwait a sec, this is messed up,
will this be Kernel from symfony rather than HttpKernel?
ping @crell, @effulgentia
Comment #28
cordoval CreditAttribution: cordoval commentedoh nevermind I was confused :), thsi is still good!
Comment #30
znerol CreditAttribution: 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 CreditAttribution: Crell commentedThis was completed by other issues.