Problem/Motivation

The method \Drupal\Core\DrupalKernel::prepareLegacyRequest() was deprecated and removed from codebase in #2869573: Remove usages of deprecated DrupalKernel::prepareLegacyRequest()
But there's still mentions and probably functionality related to it
Moreover there's mentions of bootCode() in the same comment

Steps to reproduce

$ git grep -A2 -B2 prepareLegacyRequest
core/includes/install.core.inc-507-
core/includes/install.core.inc-508-  // Initialize a route on this legacy request similar to
core/includes/install.core.inc:509:  // \Drupal\Core\DrupalKernel::prepareLegacyRequest() since normal routing
core/includes/install.core.inc-510-  // will not happen.
core/includes/install.core.inc-511-  $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('<none>'));
--
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-524-    // Explicitly call register() again on the container registered in \Drupal.
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-525-    // @todo This should already be called through
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:526:    //   DrupalKernel::prepareLegacyRequest() -> DrupalKernel::boot() but that
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-527-    //   appears to be calling a different container.
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-528-    $this->container->get('stream_wrapper_manager')->register();
--
core/tests/Drupal/Tests/BrowserTestBase.php-564-    // Explicitly call register() again on the container registered in \Drupal.
core/tests/Drupal/Tests/BrowserTestBase.php-565-    // @todo This should already be called through
core/tests/Drupal/Tests/BrowserTestBase.php:566:    //   DrupalKernel::prepareLegacyRequest() -> DrupalKernel::boot() but that
core/tests/Drupal/Tests/BrowserTestBase.php-567-    //   appears to be calling a different container.
core/tests/Drupal/Tests/BrowserTestBase.php-568-    $this->container->get('stream_wrapper_manager')->register();

$ git grep -A3 -B3 bootCode
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-516-    // Reset/rebuild all data structures after enabling the modules, primarily
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-517-    // to synchronize all data structures and caches between the test runner and
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-518-    // the child site.
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:519:    // @see \Drupal\Core\DrupalKernel::bootCode()
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-520-    // @todo Test-specific setUp() methods may set up further fixtures; find a
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-521-    //   way to execute this after setUp() is done, or to eliminate it entirely.
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php-522-    $this->resetAll();

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3453216

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

andypost created an issue. See original summary.

andypost opened merge request !8331

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs work

Needs work to fix failed tests or change comments and add todo links to follow-ups

andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: #2869573: Remove usages of deprecated DrupalKernel::prepareLegacyRequest() » #2282779: [meta] DrupalKernel has too many responsibilities
Related issues: +#2869573: Remove usages of deprecated DrupalKernel::prepareLegacyRequest()
quietone’s picture

quietone commented
Version: 11.0.x-dev » 11.x-dev

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

longwave made their first commit to this issue’s fork.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Spotted this again elsewhere, rebased, let's try to get this done now.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

The failure is

Drupal\FunctionalTests\Core\Recipe\StandardRecipeInstallTest::testStandard
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 []
+Array &0 [
+    0 => 'The file storage you selected is not a visible, readable and writable stream wrapper. Possible choices: <em class="placeholder"></em>.',
+]

This makes sense given the changes - the stream wrappers are not registered properly in the test runner container. But why does this happen for StandardRecipeInstallTest and not StandardTest, which executes the same method but installs via a profile instead of recipes?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

The bug here was in DrupalKernel.

DrupalKernel::resetContainer() ensures that stream wrappers are re-registered when a container is reset, along with propagating various other services to the new container.

DrupalKernel::rebuildContainer() which calls ::initializeContainer() does not do the same check.

These two methods should likely be unified, but not here - perhaps as a child of #2282779: [meta] DrupalKernel has too many responsibilities

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

#3583911: Clean up StreamWrapperManager::register() calls can remove those calls again, so it depends which one gets in first!

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

So all I did was apply the MR and use the same grep provided git grep -A2 -B2 prepareLegacyRequest and all instances appear to be addressed.

LGTM

godotislate’s picture

godotislate
he/him
commented
Title: clean-up outdated mentions of prepareLegacyRequest » Clean up outdated mentions of prepareLegacyRequest

godotislate closed merge request !8331

godotislate’s picture

godotislate
he/him
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 24fb26f to main. and 8799b9f to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • godotislate committed 24fb26f2 on main 
    task: #3453216 Clean up outdated mentions of prepareLegacyRequest

By:...

  • godotislate committed 8799b9f1 on 11.x 
    task: #3453216 Clean up outdated mentions of prepareLegacyRequest

By:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

cmlara’s picture

cmlara
he/him
commented

I'm seeing some regressions (at least in tests) that may be related to this issue where the Kernel appears to be trying to register a streamWrapper before all the other module services are registered into the container (the streamWrapper calls these services during construction) creating a ServiceNotFound exception.

Ref:
#3601847: D11.4, s3fs service not found

cmlara’s picture

cmlara
he/him
commented

Opened #3603333: Errors when stream wrappers instantiate services in constructors because register() is called before container is built as a core bug, confirmed this change set is breaking streamWrapper installs that worked in 11.3.