Remove usages of deprecated DrupalKernel::prepareLegacyRequest()

Doesn't apply anymore.

1. This is looking good - nice work!

2. +++ b/core/authorize.php
   @@ -65,7 +67,17 @@ function authorize_access_allowed(Request $request) {
   +  $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('<none>'));
   +  $request->attributes->set(RouteObjectInterface::ROUTE_NAME, '<none>');
   +  $container->get('request_stack')->push($request);
   

   The pushing the request onto the stack is done in preHandle() already :) So we could move the request setting stuff above $kernel->preHandle().

   Also we should see how much of this is really needed.

3. +++ b/core/lib/Drupal/Core/DrupalKernel.php
   @@ -753,6 +753,7 @@ public function prepareLegacyRequest(Request $request) {
   +    @trigger_error(__METHOD__ . ' is deprecated Drupal 8.0.x and will be removed before 9.0.0. Use DrupalKernel::boot() and DrupalKernel::preHandle() instead.', E_USER_DEPRECATED);
   

   __NAMESPACE__ . '\DrupalKernel::prepareLegacyRequest...' is the usual construction for this.

4. +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
   @@ -386,13 +388,17 @@ protected function initUserSession() {
        $this->kernel = DrupalKernel::createFromRequest($request, $this->classLoader, 'prod', TRUE);
   -
   +    $this->kernel->boot();
   +    $this->kernel->preHandle($request);
        // Force the container to be built from scratch instead of loaded from the
        // disk. This forces us to not accidentally load the parent site.
   -    $this->kernel->invalidateContainer();
   -
   -    $this->kernel->prepareLegacyRequest($request);
   -    return \Drupal::getContainer();
   +    $container = $this->kernel->rebuildContainer();
   +    // Add our request to the stack and route context.
   +    $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('<none>'));
   +    $request->attributes->set(RouteObjectInterface::ROUTE_NAME, '<none>');
   +    $container->get('request_stack')->push($request);
   +    $container->get('router.request_context')->fromRequest($request);
   +    return $container;
   

   booting and then rebuilding feels really odd - I'm sure this can be done in a more optimal way.

5. +++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
   @@ -341,10 +343,14 @@ private function bootKernel() {
   +    $kernel->preHandle($request);
   ...
   +    $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('<none>'));
   +    $request->attributes->set(RouteObjectInterface::ROUTE_NAME, '<none>');
   

   Let's move these above the preHandle and then we can remove the push to the stack.

I've tried to address #21. Things might break.

Let's add some test coverage because we can.

At the moment in kernel tests the request stack has 3 requests (all the same mind you). But that seems worth testing that we don't mess that up again.

The interdiff is the test only patch.

I don't think it's the job of this issue to do add documentation to DrupalKernel::preHandle() - also the situations where we are calling it are not really to be duplicated in contrib or anywhere. Usually preHandle() is called by part of the handle() - ie. by \Drupal\Core\StackMiddleware\KernelPreHandle - as that class says it's really about preparing the environment if page caching as not kicked in.

Nice spot on the copy pasta. Fix in the patch.

This request manipulations reminds me about related #2613044: Requests are pushed onto the request stack twice, popped once

https://gitlab.com/weitzman/drupal-test-traits/blob/master/src/DrupalTra... needs an update as well after this.

https://github.com/drush-ops/drush/blob/master/src/Boot/DrupalBoot8.php#... is another one.

@jibran both of them already need an update. This is not deprecating the method. It already is deprecated.

#28 hah I came here too to point to #2613044: Requests are pushed onto the request stack twice, popped once :)

+++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
@@ -358,8 +362,6 @@ private function bootKernel() {
-    $this->container->get('request_stack')->push($request);

This is the only push/pop-related change and definitely won't fix #2613044: Requests are pushed onto the request stack twice, popped once. I was sneakily hoping this would also fix #2613044…

This method may already be deprecated, but it was never clear how to replace it. This patch answers that question - thanks. I will update both Drush and DTT accordingly.

Drush and Drupal Test Traits have both been updated for this change. Releases will come soonish.

+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -753,6 +753,7 @@ public function prepareLegacyRequest(Request $request) {
+    @trigger_error(__NAMESPACE__ . '\DrupalKernel::prepareLegacyRequest is deprecated Drupal 8.0.x and will be removed before 9.0.0. Use DrupalKernel::boot() and DrupalKernel::preHandle() instead.', E_USER_DEPRECATED);

+++ b/core/lib/Drupal/Core/DrupalKernelInterface.php
@@ -132,8 +132,8 @@ public function invalidateContainer();
+   * @deprecated in Drupal 8.0.x and will be removed before 9.0.0. Use
+   *   DrupalKernel::boot() and DrupalKernel::preHandle() instead.

+++ b/core/tests/Drupal/KernelTests/Core/DrupalKernel/DrupalKernelTest.php
@@ -191,4 +191,24 @@ public function testPreventChangeOfSitePath() {
+   * @expectedDeprecation Drupal\Core\DrupalKernel::prepareLegacyRequest is deprecated Drupal 8.0.x and will be removed before 9.0.0. Use DrupalKernel::boot() and DrupalKernel::preHandle() instead.

Deprecation message needs to be updated regarding CS

updating deprecations, adding a quick CR.

Updated the IS with the current fix.

This looks ready to me, all feedback has been addressed and all tests are green.

@moshe weitzman++ for updating Drush & Drupal Test Traits!

And yay for RTBC! #24 added explicit backwards compatibility test coverage. I was first confused by all the

$request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('<none>'));

lines, but that's what the deprecated method used to do. So this makes sense.

+++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
@@ -446,7 +450,6 @@ protected function rebuildAll() {
     // @todo This should already be called through

This was the reason #20 added a Needs followup issue tag, and since this line is still there, we still need that follow-up.

+++ b/core/authorize.php
@@ -65,7 +67,15 @@ function authorize_access_allowed(Request $request) {
+  $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('<none>'));
+  $request->attributes->set(RouteObjectInterface::ROUTE_NAME, '<none>');

I wonder whether we could put this in Kernel::preHandle - it feels like knowledge that outside consumers should not need to know

Let's see whether that's do-able

If this works, our recommended approach would just be ->boot()->preHandle($request) because of chaining

Added #3074414: Expand documentation of DrupalKernel::preHandle to indicate when it should be used, and when ::handle() is a better option for that, I agree - we should make it clear when to use what

Guess not, back to RTBC for the patch at #36

Committed the patch at #36

Committed 8181767 and pushed to 8.8.x. Thanks!

Published change record

Created 11.x follow-up to #3453216: Clean up outdated mentions of prepareLegacyRequest