Problem/Motivation

In #3209617: [Symfony 6] Symfony\Component\HttpFoundation\RequestStack::getMasterRequest() is deprecated, use getMainRequest() instead we added a BC shim that supports RequestStack that could have either getMainRequest() or getMasterRequest() methods.

Now we have moved to Symfony 6 we can drop the code that handles the getMasterRequest() method and use the Symfony class directly again.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Postponed until 11.0.x-dev is open.

CommentFileSizeAuthor
#5 3265121-5.patch5.07 KBandypost
#5 interdiff.txt3.43 KBandypost
#2 3265121-2.patch1.64 KBlongwave

Issue fork drupal-3265121

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

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 3265121-2.patch1.64 KB

Status: Needs review » Needs work

The last submitted patch, 2: 3265121-2.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: » #3213895: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 10 branch
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt3.43 KB
new 3265121-5.patch5.07 KB

Removed legacy test and fixed mentions

andypost’s picture

andypost
he/him
Primary language Russian
commented

FYI RendererPlaceholdersTest has local failures because of stricter types on PHP 8

The \Drupal\Core\Render\Renderer::doCallback needs to relax second argument "array" type to allow pass tests or this tests and assertions could be removed

Failed asserting that exception of type "ArgumentCountError" matches expected exception "AssertionError". Message was: "Too few arguments to function Drupal\Tests\Core\Render\RecursivePlaceholdersTest::callback(), 0 passed and exactly 1 expected" at
/var/www/html/web/core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php:1173
/var/www/html/web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php:101
/var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:775
/var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:345
/var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:201
/var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:145
/var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:567
/var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php:146
/var/www/html/web/core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php:952
spokje’s picture

spokje
Primary language Dutch
commented

FCI (=For Committers Information): A full PHPStan run on this one would be nice, since it currently makes patches/MRs that change the baseline fail (#3227033-128: Remove Quick Edit from core and #3227033-130: Remove Quick Edit from core)

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Green TestBot and neither it nor me could find any other references to the shim.

RTBC for me.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Filed follow-up #3265288: Add assertion for #lazy_builder callback argument

neclimdul’s picture

neclimdul
He/Him
commented
Status: Reviewed & tested by the community » Needs work

Don't really want to leave this empty class lying around for a full release cycle but I'm not sure we can remove the entire class without deprecating it can we? The class would be expected to exist. Facets is at least still using it.

spokje’s picture

spokje
Primary language Dutch
commented
Title: Remove Symfony 4 RequestStack BC shim » [PP-1] Remove Symfony 4 RequestStack BC shim
Status: Needs work » Postponed
Related issues: +#3265356: Deprecate Symfony 4 RequestStack BC shim for removal in D11

Ouch....Everybody must have seen so much "Remove All The Deprecation Things!"-issues by now that we've become blind to the fact that not everything is nicely deprecated already.

Let's deprecate this in 9.4.x 10.0.x before we remove.

I've opened #3265356: Deprecate Symfony 4 RequestStack BC shim for removal in D11 for that and am postponing this issue on it.

Thanks @necliimdul for being eagle-eyed and spotting this.

spokje’s picture

spokje
Primary language Dutch
commented

Postponed until 11.0.x-dev is open.

spokje’s picture

spokje
Primary language Dutch
commented
Title: [PP-1] Remove Symfony 4 RequestStack BC shim » [PP-1] Remove Symfony 4 RequestStack BC shim in 11.0.x
Issue summary: View changes
andypost’s picture

andypost
he/him
Primary language Russian
commented
Title: [PP-1] Remove Symfony 4 RequestStack BC shim in 11.0.x » Remove Symfony 4 RequestStack BC shim in 11.0.x
Issue tags: +Deprecation Removal, +Drupal 11
Parent issue: #3213895: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 10 branch »
Related issues: +#3213895: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 10 branch

Setting postponed til 11.0.x open according to #3265356-35: Deprecate Symfony 4 RequestStack BC shim for removal in D11

andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: » #3213895: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 10 branch
Related issues: -#3213895: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 10 branch
andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: #3213895: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 10 branch »
Related issues: +#3213895: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 10 branch

sorry it's for 11.x

andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: » #3295574: [meta] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 11 branch
quietone’s picture

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

Moving to 11.x-dev

andypost’s picture

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

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

sakthi_dev’s picture

sakthi_dev commented

It's already changed in 11.x. Please correct me if I'm wrong.

andypost’s picture

andypost
he/him
Primary language Russian
commented

It needs to remove remain after https://git.drupalcode.org/project/drupal/-/commit/bc964dc

andypost’s picture

andypost
he/him
Primary language Russian
commented

It just needs rm core/lib/Drupal/Core/Http/RequestStack.php core/tests/Drupal/Tests/Core/Http/RequestStackLegacyTest.php

andypost opened merge request !6854

andypost’s picture

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

let's see if that enough

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Searched for \Core\Http\RequestStack and only found in that test that was deleted.

So removal seemed good.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed 60d5162 and pushed to 11.x. Thanks!

  • longwave committed 60d51622 on 11.x 
    Issue #3265121 by andypost, longwave, Spokje, neclimdul, smustgrave:...

longwave closed merge request !6854

Status: Fixed » Closed (fixed)

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