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