Re: [PHP-DEV] RFC: Server-Side Request and Response Objects (v2)

This is only part of a thread. view whole thread
  108540
February 13, 2020 15:16 pmjones@pmjones.io ("Paul M. Jones")
Hi Niklas,

> On Feb 12, 2020, at 12:20, Niklas Keller <me@kelunik.com> wrote: > > I think the request / response API is entirely fine being solved in > userland instead of in php-src. However, since you already made such a > proposal, I want to give some feedback:
Re: userland, I gave a counterargument in my reply to Mark Randall. Even so, thanks for providing feedback in spite of that objection; I appreciate your time and effort.
> Naming > > I think we shouldn't take over the naming of the super globals, e.g. > $_GET really contains the query parameters and has nothing to do with > GET or POST, so $request->getQueryParameter(...) would be a better > name.
/me nods Yeah, naming is one of the hard problems. I considered $query as an alternative property name for $get, but in the end, the `$_GET => $get` symmetry was too great to ignore. If others here feel that $query is a better name for `$_GET` than $get, I will submit to consensus on that point. Using a getQueryParameter() method strikes a little too close to PSR-7, and thereby to charges of favoritism. But let's say we do use a method instead of a property here, call it getQuery(); then, of the following .... $foo = $request->getQuery()['foo']; // vs $foo = $request->query['foo']; .... the latter (using a property) looks and feels more appropriate to me. Thus, the RFC specifies properties over methods for ServerRequest.
> Type Safety > > I think the API should be type safe. Currently $request->get['key'] > can be null | string | string[] | ... Most parameters only appear a > single time, so a method returning the first parameter value or null > could be used instead.
This sounds a little like using `$_REQUEST` instead of `$_GET`, `$_POST`, and `$_COOKIES`. Using the "first" parameter would then depend on the order in which the superglobals get entered into the ServerRequest object, and/or we're in the business of reading and honoring the `variables_order` INI setting, at which point the logic starts sounding rather involved. So while I do get the desire for type-safety, I'd prefer to avoid all that intricacy, and go with something much closer to what PHP itself already does. That is, read $get for $_GET, $post for $_POST, etc., and just go with what is stored there.
> API Issues > > I don't see any reason to keep specials such as > $_SERVER['HTTP_X_REQUESTED_WITH'] vs. $request->requestedWith, which > is just another HTTP header.
I get that; $requestedWith in 1.x was a boolean $xhr, to let you know if `$_SERVER['HTTP_X_REQUESTED_WITH'] === 'XmlHttpRequest'`. It struck me that there might be different values in the future, and so moved to just $requestedWith instead. I am not attached to that particular property; if others agree, I am OK with removing it.
> If there's $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] => $request->method > and $_SERVER['REQUEST_METHOD'] => $request->method, how can I get the > original (actual) method?
The $method property is a calculated value: if there is a method override on a POST, $method is the override; otherwise, it is the "actual" method. So: - $request->server['REQUEST_METHOD'] is the original (actual) method, - $request->server['HTTP_X_METHOD_OVERRIDE'] is the override method, and - $request->method is the "calculated" value between them. You can see the code here: https://github.com/pmjones/ext-request/blob/2.x/php_request.c#L147-L175
> Given 'echo $content; => $response->setContent($content);', shouldn't > this rather be something like `addContent()`?
That looks like poor describing on my part in the RFC. It is more true to say that these are equivalent: echo $content; // => $response->setContent($content); $responseSender->send($response); I will try to make that more apparent in the RFC.
> How does streaming responses work? There's ServerResponseSender, but I think this should > be part of the Response API.
Here's an example: $fh = fopen('/path/to/file.ext', 'rb'); $response->setContent($fh); // ... $responseSender->send($response); When the ServerResponseSender calls $response->getContent() and detects a resource, it calls rewind() on that resource, then fpassthru(). That should stream through nicely. For more information, please see the ServerResponseSender methods at <https://github.com/pmjones/ext-request#methods-2> under the sendContent() bullet: • If the content is a resource, it is sent using rewind() and then fpassthru(). • If the content is a callable object or closure, it is invoked, and then its return value (if any) is echoed as a string; note that object returns will be cast to string at this point, invoking the __toString() method if present. • Otherwise, the content is echoed as a string; note that objects will be cast to string at this point, invoking the __toString() method if present. There are limitations to that approach, but experience has shown that it covers the vast majority of common requirements.
> The Request object should be mutable, e.g. you might want to change > the client IP to be based on a x-forwarded-header instead of the TCP > source address.
That's a great example. First, note that ServerRequest avoids setting a $clientIp property. Further, *extensions* to the ServerRequest object might well be mutable. So, to go with your example, you would be well within bounds to do something like this: class MyServerRequest extends ServerRequest { private $clientIp; public function __construct(array $globals, ?string $content = null) { parent::__construct($globals, $content); if ($this->forwarded) { $this->clientIp = // ... } } public function getClientIp() { return $this->clientIp; } } You could do that for all your custom calculations on a ServerRequest object.
> Other Commentary > >> A: It supports async exactly as much as PHP itself does. > > Not really. PHP has built-in stream_select / non-blocking streams, so > supports the tools required for async out of the box.
I will address this separately, per the resolution of our private email conversation. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108652
February 18, 2020 09:24 come.chilliet@fusiondirectory.org (=?ISO-8859-1?Q?C=F4me?= Chilliet)
Le jeudi 13 février 2020, 09:16:49 CET Paul M. Jones a écrit :
> Yeah, naming is one of the hard problems. I considered $query as an alternative property name for $get, but in the end, the `$_GET => $get` symmetry was too great to ignore. If others here feel that $query is a better name for `$_GET` than $get, I will submit to consensus on that point.
query is definitely better than get. Regarding post, I’m fine with body, parsedBody and input. I get the idea of input to mimic php://input, but if I understand things correctly, php://input is raw body, while $request->post is parsed body, so naming them alike might actually cause confusion?
> > Given 'echo $content; => $response->setContent($content);', shouldn't > > this rather be something like `addContent()`? > > That looks like poor describing on my part in the RFC. It is more true to say that these are equivalent: > > echo $content; > > // => > > $response->setContent($content); > $responseSender->send($response); > > I will try to make that more apparent in the RFC.
I still do not understand this. echo adds content to the response, it does not replace it. So the equivalent function should be $response->addContent. I would expect $response->setContent to replace the content. Can you explicit behavior for this: $response->setContent("a\n"); $response->setContent("b\n"); $responseSender->send($response); Compared to echo "a\n"; echo "b\n"; -- Côme Chilliet FusionDirectory - https://www.fusiondirectory.org
  108657
February 18, 2020 13:33 pmjones@pmjones.io ("Paul M. Jones")
Hi Côme,

> On Feb 18, 2020, at 03:24, Côme Chilliet chilliet@fusiondirectory.org> wrote: > > Le jeudi 13 février 2020, 09:16:49 CET Paul M. Jones a écrit : > >> Yeah, naming is one of the hard problems. I considered $query as an alternative property name for $get, but in the end, the `$_GET => $get` symmetry was too great to ignore. If others here feel that $query is a better name for `$_GET` than $get, I will submit to consensus on that point. > > query is definitely better than get.
Excellent.
> Regarding post, I’m fine with body, parsedBody and input. > > I get the idea of input to mimic php://input, but if I understand things correctly, php://input is raw body, while $request->post is parsed body, so naming them alike might actually cause confusion?
Might, might not. I don't think there is any "good" name here, only names that are less-bad than others.
> I still do not understand this. > echo adds content to the response, it does not replace it. > So the equivalent function should be $response->addContent. > > I would expect $response->setContent to replace the content.
Ah, I see what you are getting at now ...
> Can you explicit behavior for this: > > $response->setContent("a\n"); > $response->setContent("b\n"); > $responseSender->send($response); > > Compared to > > echo "a\n"; > echo "b\n";
.... the output would be "b\n". As you say, setContent() replaces whatever content is already in the ServerResponse. While the comparison for a single echo is accurate, the comparison for multiple echoes would be: $content = "a\n"; $content .= "b\n"; $response->setContent($content); $responseSender->send($content); Does that help to clarify? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108659
February 18, 2020 14:10 come.chilliet@fusiondirectory.org (=?ISO-8859-1?Q?C=F4me?= Chilliet)
Le mardi 18 février 2020, 07:33:37 CET Paul M. Jones a écrit :
> ... the output would be "b\n". As you say, setContent() replaces whatever content is already in the ServerResponse. While the comparison for a single echo is accurate, the comparison for multiple echoes would be: > > $content = "a\n"; > $content .= "b\n"; > $response->setContent($content); > $responseSender->send($content); > > Does that help to clarify?
Yes, but to me that means we also need an addContent method. Otherwise people will have to carry a global $content along side $response, or use setContent(getContent().$additionalContent). -- Côme Chilliet FusionDirectory - https://www.fusiondirectory.org
  108660
February 18, 2020 14:56 pmjones@pmjones.io ("Paul M. Jones")
> On Feb 18, 2020, at 08:10, Côme Chilliet chilliet@fusiondirectory.org> wrote: > > to me that means we also need an addContent method.
I can see why we'd think that; it's symmetrical, if nothing else. Even so, none of the researched implementations have a method like that. As far as I can recall, they call have setContent() and getContent() equivalents, but no addContent() equivalent. They all work much like you point out here ...
> Otherwise people will have to carry a global $content along side $response, or use setContent(getContent().$additionalContent).
.... although usually it's not a global $content. Instead, the $content is built up from a template or other subsystem of some sort, and then assigned to the response when complete. For example: $content = $template->render(); $response->setContent($content); So, I am reluctant to add something that no other implementations, across many years and many authors, have actually found a need for. Any further thoughts on this? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php