> On 14 Mar 2020, at 01:40, Mike Schinkel <m...@newclarity.net> wrote:
> 
> 
>> On Mar 13, 2020, at 10:55 AM, Paul M. Jones <pmjo...@pmjones.io> wrote:
>> 
>> One other alternative John & I contemplated was 
>> `Web{Request,Response,ResponseSender}` -- do you think that might be a 
>> reasonable alternative to HTTP, one that is "adjacent" but not 
>> overly-specific? That would net us:
>> 
>> - WebRequest
>> - WebResponse
>> - WebResponseSender
>> 
>> I didn't like the look of it previously, and I don't think I like the look 
>> of it now, but ... (/me shrugs).
>> 
>> 
>>> However, that also may be confusing if people expect it to be a construct 
>>> for making outgoing requests. 
>> 
>> Yes, that's another tricky bit in the naming -- making a distinction between 
>> the objects as representative of client operations (send request, receive 
>> response) and server operations (receive request, send response). Thus the 
>> current `Server` prefix (however unsatisfactory it may be) to indicate their 
>> operational context.
>> 
>> Your `Incoming` and `Outgoing` prefixes, minus the HTTP, would net us:
>> 
>> - IncomingRequest
>> - OutgoingResponse
>> - OutgoingResponseSender
>> 
>> I will need to ponder on those.
>> 
>>> The user land implementation I’ve been using ’solves’ this by using a 
>>> `HTTP` namespace, and then provides `Request` and `Response` (for an 
>>> outgoing - i.e. curl - HTTP Request, and the corresponding HTTP Response) 
>>> objects and then `CurrentRequest` and `CurrentResponse` for what your RFC 
>>> proposes (i.e. the active request made to php). 
>> 
>> Yes, userland does operate that way. However, I think adding an HTTP 
>> namespace to PHP itself is something to be avoided, so emulating userland 
>> here is not an option.
>> 
>> 
>>> As with anything any of us has written, I’m not 100% sold on 
>>> ‘Current{Request,Response}` even after writing it, but I think it’s at 
>>> least a little more specific about what they do, when the namespace is 
>>> taken into account.
>> 
>> `Current{...}` is not something we had previously considered; that would net 
>> us, in the global namespace:
>> 
>> - CurrentRequest
>> - CurrentResponse
>> - CurrentResponseSender
>> 
>> I will need to ponder on those as well.
>> 
>> Any further thoughts or opinions on this, Stephen? Or from anyone else?
> 
> One issue that I have is, if we are going to fine-tune the naming to make 
> sure it matches  *exactly* then I think that CurrentRequest->server does not 
> make sense.
> 
> Maybe if you choose one of these names you should break out the 
> server-specific items into their own class/object?
> 
> -Mike
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
> 

Hi Mike,

(I realise some of these points are possibly more addressed to Paul than 
yourself, this is just where my brain went when I dug into what you were 
mentioning)

I apologise if I’ve missed part of the discussion but what do you mean by “make 
sure it matches *exactly*. Do you mean how `->server` is listed as being a copy 
of the `$_SERVER` super global? If so, can someone point me to the specific 
logic behind that, given how many parts of it are already exposed via 
‘dedicated’ properties of the proposed class?  From what I can see (and I may 
have missed some) the parts of $_SERVER not exposed in some way “directly” on 
ServerRequest (nee CurrentRequest) are the actual “server” parts: the 
`SERVER_*` keys, doc root, PHP_SELF; and the ‘client’ parts: REMOTE_*; plus few 
random stragglers like PATH_INFO, and for some reason REQUEST_TIME[_FLOAT]?

Can those two things not be organised as a hash of those values, under `server` 
and `client` (or `remote` if you want to keep the terminology - yes I know it 
will be the originating TCP connection host not necessarily the browser host)? 
As I said, I’ve missed some of the discussion but I fail to see the benefit of 
a class to make all the details of a web request and it’s response available…. 
And then just stick the existing superglobals in it untouched.

Things like the query params and body params make sense to be accessible 
essentially (albeit with better names) as-is, because they’re a hash of unknown 
shape by their very nature - but the keys in _SERVER (barring http headers, 
which are already special cased) are known, and have pretty clear definition of 
their meaning.

Is the proposal really suggesting that a developer would still need to do 
`if(!empty($request->server[‘HTTPS’]) && $request->server[‘HTTPS’] !== ‘off’) 
{…}` rather than just providing a `secureTransport` property (or `https` if you 
prefer)? 

One last point, regarding the ‘break out a server specific class’ part. I don’t 
think it’s “wrong” to access these properties from something that is ostensibly 
related to the “current request”, but it feels quite ‘wonky’ to me, the way 
it’s proposed with the full ->server array just copied as-is, AND exposed via 
dedicated properties.




Cheers

Stephen
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to