Hi Niklas,
> On Feb 12, 2020, at 12:20, Niklas Keller <m...@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
pmjo...@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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
