Hi Rowan, > On Feb 19, 2020, at 16:43, Rowan Tommins <rowan.coll...@gmail.com> wrote: > >>> - When working out the details, what code should we be picturing using the >>> new classes? >> >> Not to be flippant, but: request-reading and response-writing code? > > The reason I keep trying to pin you down on this is that I am concerned about > the "jack of all trades" effect - that a feature which lacks focus can end up > being less useful than if it was refined for one job. From the point of view > of a reviewer, it's also hard to answer the question "do you think this is a > good design?" when you don't know what it's the design for. > > ... it's easier to get specific about a design fitting a goal than making an > abstract judgement about it.
The purpose of the extension is as stated in the RFC: that is, to provide "an object-oriented approach around request and response functionality already existing in PHP ... an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on." At this point, I get the impression that either one of use is underthinking things, or the other one is overthinking them -- or perhaps we just have different premises and visions. > The particular selection of fields feels rather arbitrary, though - for > instance, I've never heard of "Content-MD5", That's fair; once some of the content-related header fields came into play, it seemed reasonable to bring all of them in. > but would have expected at least some URL-related properties, like Host. Aha! If nothing else, then, this conversation has revealed a documentation flaw: specifically, my failure to document the ServerRequest $url property. That failure is now remedied: <https://github.com/pmjones/ext-request#the-url-array> > Similarly, methods can have parameters, and that can be a great way of > introducing opt-in behaviour. For instance, maybe in future someone requests > that getMethod() should have a $allow_override flag, which set to false would > ignore the X-HTTP-METHOD-OVERRIDE header. With properties, you need a > separate property for every possible combination, and you have to come up > with names for them all. I get the need for future extension, which is why the method space is left open for consumers. If they want to provide their own getter methods, calculating from the existing properties or other values, they are in the clear to do so. >> I think $uploads is a pretty distinct name, while being clearly related to >> files. > > Perhaps "descriptive" would be a better word than "distinct". I can tell > $files and $uploads apart at a glance, but the names tell me nothing about > why both exist, or which I should be using. Which is the purpose of the documentation; it describes the differences between $files and $uploads. >>>> Another "hard" problem is carrying those values around in the system once >>>> they are decoupled from global state; the objects in this RFC purport to >>>> do so. >> I answered this above, but to reiterate: "carrying around" is *one* thing >> ServerRequest does, but not *all* it does. > I'm not disputing that; I'm disputing that that particular feature is in any > way a hard problem. Your disputation is noted. >> I admit I considered this. However, it makes more sense to me in terms of >> symmetry/complementarity, and in terms of "what we need on a daily basis", >> to provide both the request object and the response object together. > > One oddity of the proposal is that the two objects aren't actually very > symmetrical. That's fair; strike "symmetry" and retain "complementarity." >> Maybe? I can similarly imagine that if new-and-different superglobals >> appear, the ServerRequest object can evolve to contain and/or translate >> between them. > > Although we can't predict the future, there are things we can do to make it > more likely we could adapt to such a change. We should make a conscious > decision whether this is a goal, or something we're happy not to focus on. The RFC states, under Future Scope, "This extension acts as an object-oriented wrapper around existing PHP request and response functionality; as the scope of that PHP functionality expands, this extension should expand with it." >>> Parsing a request body from an arbitrary source into arrays that match the >>> structure of $_POST and $_FILES would be a really useful feature. >> Yes, although one can do at least the $_POST portion with ServerRequest as >> it is now. >> ... >> Call `$request = (new ServerRequestFactory())->new();` and voila: the >> equivalent of `$_POST`, populated from JSON content, stored as >> $request->input. > > I was actually thinking of the opposite: given a request body which didn't > come from global state, but which contains data in multipart/form-data > format, extract the key-value pairs and attached files. Is that something PHP "itself" already does? If not, I have to consider it out-of-scope for this RFC. > Rather than accepting the content body as an optional constructor parameter, > what if there were two named constructors: > > - fromSuperGlobalArrays($server, $cookie, $post, $get, $files) > - fromRawRequestBody($server, $cookie, $body) If consumers/users of ext-request wish to create factories or builders to instantiate a ServerRequest instance, it is within their purview to do so. > In the first case, accessing the raw body on the object could give null, > because none is known - defaulting to global state seems like a mistake if > decoupling from global state is an explicit aim. Your point on global state is well-taken; I will try to remember in future to phrase it as "global *mutable* state." (AFAICT, php://input is not mutable, though as you correctly point out, it is global.) > if you look at pretty much any existing Request wrapper, it will make some > attempt to extract a URL from the $_SERVER array. That really feels like a > missing feature of this one right now. Yeah, my bad on not documenting it earlier -- please consider the "missing" feature "found." ;-) > I hope my comments aren't coming across as too negative. Not "negative", but as you say ... > Maybe our visions for what this should be are just too different ... I think your goals are loftier here than proposal's. -- 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
