Skip to content

[HttpFoundation] Add $requests parameter to RequestStack constructor #57909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

alexander-schranz
Copy link
Contributor

@alexander-schranz alexander-schranz commented Aug 2, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #...
License MIT

This change make it possible to directly give requests into the constructor. This way it make it easier to write Unit tests based on RequestStack like our own RequestAnalyzer service without the need to write the requestStack into an own variable e.g.

-$requestStack = new RequestStack();
-$requestStack->push(Request::create('/'));
-$requestAnalyzer = new RequestAnalyzer($requestStack);
+$requestAnalyzer = new RequestAnalyzer(new RequestStack([Request::create('/')]));

I updated also the Symfony core tests (Undo because lowest deps then not working still for reference e16fd50).

The change in the RequestStack is:

     /*
      * @var @param Request[]
      */
     private array $requests = [];

+    /**
+     * @param Request[] $requests
+     */
+    public function __construct(array $requests = [])
+    {
+        foreach ($requests as $request) {
+            $this->push($request);
+        }
+    }
+

For BC reasons I did go of calling the push if somebody extending the RequestStack.

@carsonbot carsonbot added this to the 7.2 milestone Aug 2, 2024
@alexander-schranz alexander-schranz changed the title Feature/request stack requests via constructor [HttpFoundation] Add requests parameter to RequestStack constructor Aug 2, 2024
@alexander-schranz alexander-schranz force-pushed the feature/request-stack-requests-via-constructor branch 2 times, most recently from dff1130 to 9af95a1 Compare August 2, 2024 09:14
@alexander-schranz alexander-schranz marked this pull request as draft August 2, 2024 09:14
@alexander-schranz alexander-schranz force-pushed the feature/request-stack-requests-via-constructor branch from 9af95a1 to e16fd50 Compare August 2, 2024 09:15
@alexander-schranz alexander-schranz marked this pull request as ready for review August 2, 2024 09:18
@alexander-schranz
Copy link
Contributor Author

I was may a little bit to fast with refactor the symfony core tests as that would mean to increase the requirements for symfony/http-foundation. So reverting that part.

Still for reference: e16fd50

@OskarStark OskarStark changed the title [HttpFoundation] Add requests parameter to RequestStack constructor [HttpFoundation] Add $requests parameter to RequestStack constructor Aug 2, 2024
@xabbuh
Copy link
Member

xabbuh commented Aug 2, 2024

This looks like a reasonable change to me. Can you also add a changelog entry?

@alexander-schranz
Copy link
Contributor Author

The AppVeyor failing test seems to be unrelated to the change and happens currently als on 7.2 branch: https://ci.appveyor.com/project/fabpot/symfony/builds/50310250

@fabpot fabpot force-pushed the feature/request-stack-requests-via-constructor branch from d413641 to 7103295 Compare August 3, 2024 09:08
@fabpot
Copy link
Member

fabpot commented Aug 3, 2024

Thank you @alexander-schranz.

@fabpot fabpot merged commit 5286f27 into symfony:7.2 Aug 3, 2024
9 of 10 checks passed
@alexander-schranz alexander-schranz deleted the feature/request-stack-requests-via-constructor branch August 8, 2024 09:08
@fabpot fabpot mentioned this pull request Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants