8000 [4.0][HttpFoundation] Future of the Request class · Issue #17640 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[4.0][HttpFoundation] Future of the Request class #17640

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.

Sign up for GitHub

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

Jump to bottom
Closed
GuilhemN opened this issue Feb 1, 2016 · 12 comments
Closed

[4.0][HttpFoundation] Future of the Request class #17640

GuilhemN opened this issue Feb 1, 2016 · 12 comments
Labels
Feature HttpFoundation

Comments

@GuilhemN
Copy link
Contributor
GuilhemN commented Feb 1, 2016

Just as a reminder for 4.0 to not forget #17379.

This PR makes Request::getMimeType(), Request::getFormat() and Request::setFormat() static.

@Tobion can you add to this issue the 4.0 milestone please?
@Tobion Tobion added this to the 4.0 milestone Feb 1, 2016
@dunglas
Copy link
Member
dunglas commented Feb 1, 2016

In 4.0 the best would be to extract all the logic from Request (and Response) to make it a true value object.

@GuilhemN
Copy link
Contributor Author
GuilhemN commented Feb 1, 2016

I agree and step by step we can even imagine to use the psr implementation one day.

@GuilhemN
Copy link
Contributor Author
GuilhemN commented Feb 2, 2016

I think we can use this issue to talk about the future of the Request in general (maybe rename this issue?).

So I see several functions to move or change:

  • move createFromGlobals to a RequestFactory class (with setFactory and createRequestFromFactory)
  • move create to RequestFactory
  • change the visibility of initialize to private
  • see if we can replace duplicate by the functions with* of the psr implementation
  • move normalizeQueryString to something like QueryStringNormalizer.
  • move getClientIps and getClientIp to an IpResolver
  • move getMimeType, getMimeTypes, getFormat, setFormat to a MimeTypeResolver or MimeTypeBag

If I forgot something or if you don't agree with one of this point, please just tell it ;-)

@GuilhemN GuilhemN changed the title [4.0] Make some Request methods static [4.0][HttpFoundation] Future of the Request class Feb 2, 2016
@stof
Copy link
Member
stof commented Feb 2, 2016

@Ener-Getick the issue with using PSR is the immutability, which requires to change the whole architecture of HttpKernel (and of other parts using the Request in Symfony). Renaming some methods to with* will not make HttpFoundation implement PSR-7 (it would only create confusion by making people think it works the same).
So please stop bringing this argument each time a change in the Request is mentionned. It only creates confusion for people following a discussion.

@GuilhemN
Copy link
Contributor Author
GuilhemN commented Feb 2, 2016

@stof I'm not talking about making the Request immutable but I agree this change would be confusing...
I'll remove it, let's just talk about the other propositions.

@GuilhemN
Copy link
Contributor Author
GuilhemN commented Feb 2, 2016

BTW #6406 also talks about refactoring in part the Request but that's more about the methods related to uri management.

@ghost
Copy link
ghost commented Feb 4, 2016

We should think about to remove the get() function of Request. Related discussion: #17657

@klinki
Copy link
klinki commented Feb 10, 2016

I have another proposal.

I noticed you mentioned creating RequestFactory class. I agree, it would be great. And it would be great to have method which reads request input from php://input stream. Because right now, PHP doesn't read any data for application/json MIME type (and probably for any other, different then application/x-www-form-urlencoded).

@GuilhemN
Copy link
Contributor Author
GuilhemN commented Feb 10, 2016 8000

@klinki I love this idea, would be amazing to have such thing in the core (we can even imagine parsing the content asynchronously).

edit: i'll create a PR and we'll see what happens

@wouterj
Copy link
Member
wouterj commented Feb 10, 2016

@klinki afaik, Request#getContent() reads from php://input, right? https://github.com/symfony/http-foundation/blob/6edfe4a63b1f5c0a3d0c6190998c9089784ea883/Request.php#L1494-L1533

@HeahDude HeahDude mentioned this issue Apr 28, 2016
Closed
@fabpot fabpot removed this from the 4.0 milestone Oct 18, 2016
@chillu chillu mentioned this issue May 31, 2017
Open
@acasademont
Copy link
Contributor

Is this still a thing? Maybe for SF5?

@fabpot
Copy link
Member
fabpot commented Aug 6, 2019

Closing this meta issue.

@fabpot fabpot closed this as completed Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature HttpFoundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@fabpot @dunglas @javiereguiluz @stof @Tobion @wouterj @acasademont @klinki @GuilhemN
0