Re: RFC: Server-Side Request and Response Objects (v2)

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