Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108893
Return-Path: <pmjones@pmjones.io>
Delivered-To: mailing list internals@lists.php.net
Received: (qmail 44218 invoked from network); 8 Mar 2020 00:09:23 -0000
Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5)
  by pb1.pair.com with SMTP; 8 Mar 2020 00:09:23 -0000
Received: from php-smtp4.php.net (localhost [127.0.0.1])
	by php-smtp4.php.net (Postfix) with ESMTP id D73D31804D3
	for <internals@lists.php.net>; Sat,  7 Mar 2020 14:29:38 -0800 (PST)
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net
X-Spam-Level: 
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW,
	SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2
X-Spam-ASN: AS29169 217.70.176.0/20
X-Spam-Virus: No
X-Envelope-From: <pmjones@pmjones.io>
Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by php-smtp4.php.net (Postfix) with ESMTPS
	for <internals@lists.php.net>; Sat,  7 Mar 2020 14:29:37 -0800 (PST)
Received: from samurai.attlocal.net (107-223-28-39.lightspeed.nsvltn.sbcglobal.net [107.223.28.39])
	(Authenticated sender: pmjones@pmjones.io)
	by relay12.mail.gandi.net (Postfix) with ESMTPSA id 9C1C7200003;
	Sat,  7 Mar 2020 22:29:35 +0000 (UTC)
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\))
In-Reply-To: <CAJvSBVOEbs4UZ6ScXmwP9-SvZWrEE6Efk+TNBHC-b+ShuozuAQ@mail.gmail.com>
Date: Sat, 7 Mar 2020 16:29:33 -0600
Cc: php internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <11C83687-E08B-4394-BEE9-94017D1EBC52@pmjones.io>
References: <50BD013E-CF72-414C-BBC0-A7A2E45CBDDB@pmjones.io>
 <1657AB79-5CAF-4BCA-96B5-1343EC703CCD@pmjones.io>
 <CAJvSBVOEbs4UZ6ScXmwP9-SvZWrEE6Efk+TNBHC-b+ShuozuAQ@mail.gmail.com>
To: Peter Bowyer <phpmailinglists@gmail.com>
X-Mailer: Apple Mail (2.3608.60.0.2.5)
Subject: Re: [PHP-DEV] RFC: Server-Side Request and Response Objects (v2)
From: pmjones@pmjones.io ("Paul M. Jones")

Hi Peter,

Thanks for the detailed critique. I'm not sure I can remedy everything =
you've pointed out, but even so ...

> On Mar 5, 2020, at 03:00, Peter Bowyer <phpmailinglists@gmail.com> =
wrote:
>=20
> I expect to vote "no". My thoughts are:
>=20
> 1. The chosen API. We have an OO approach, but headers and query =
parameters
> are accessed through an array-style mechanism.
>=20
> ...
>=20
> 2. The OO-ish approach. For a core feature which will be around for =
20+
> years I would like a pure(r) OO approach taken, and time to be taken =
to get
> that right. What you have is pragmatic, but as I say in (1) not =
something
> I'm a fan of. Rowan Tommins on 18 Feb 2020 at 20:21 expressed this =
well
> (link: https://externals.io/message/108436#108661).

And not just headers and query parameters, but the $_POST-derived =
parameters, the files, the accept-related values, and frankly everything =
else in ServerRequest.

I really do get the objection; methods, if nothing else, are just what =
we're used to.

The only thing I have to say in return is that if the only methods are =
getters on properties, that are not themselves mutable, it makes sense =
to just expose the properties directly. Especially so when they are =
arrays; then a wide range of array functions can be brought to bear on =
them, instead of adding more methods to duplicate array-related =
functionality.

Having said that, what changes would you like to see on the API, that =
you feel might be more worthy of your vote to accept the proposal? =
Getter methods on ServerRequest? Something else? (I'm not making =
promises, but perhaps knowing what you'd prefer will give me some =
ideas.)


> I'm not a fan of the way everything maps onto ServerRequest, with =
certain $_SERVER vars promoted to be part of it (but not all AFAICT).

You're correct: not every element of $_SERVER gets promoted to a =
property. The `HTTP_*` elements go into $headers, and some of *those* =
(the ones that bear restructuring into arrays) get promoted to =
properties of their own. Then, once some elements get promoted to =
properties, it makes sense to promote related elements (e.g., once you =
have $contentType and $contentCharset, might as well have $contentLength =
et al.).

What *might* you be a fan of, in terms of what gets promoted to a =
ServerRequest property, and what does not?


> 3. WRT the RFC:
> a) there is no section on where this came from. With your involvement =
in
> PSR-7 it looks to be an area you've given much thought to. Why did you
> develop ext/request? What problem(s) did you see that this is the =
answer
> to? I skimmed the thread but didn't pick out a clear answer to this.

This is one of those places where it's tough to "thread the needle." I =
want to reduce the chances of the RFC being read as a purely negative =
criticism of existing projects, but at the same time some level of =
compare-and-contrast is necessary. One set of readers wants to know =
what's so terrible about the existing projects, and another set of =
readers demands positive reasoning only within the proposal. It's a =
"damned if you do, damned if you don't" kind of situation.

My earlier attempt at answering "what problem(s) did you see?" is =
blogged here:

  =
http://paul-m-jones.com/post/2017/01/05/psr-7-vs-the-serverrequestresponse=
-rfc/

Quoting and summarizing from there:

- PSR-7 was born to answer the question, =E2=80=9CHow can we model HTTP =
messages in PHP for sending a request, and getting back a response?=E2=80=9D=
 That is, how can we standardize the model of an HTTP request message =
for sending, and the model of the returned the HTTP response, when using =
PHP as an HTTP client? ... PSR-7 [then] expanded to answer a second =
question: =E2=80=9CHow can we model HTTP messages for receiving a =
request, and sending back a response?=E2=80=9D This is in addition to =
the original goal, but idea is the same: building a standard model of =
HTTP messages.

- The proposed RFC starts out by asking a different question. It is not =
concerned with modeling HTTP messages, whether when sending or receiving =
them. Instead, it asks: =E2=80=9CHow can we take the request-related =
superglobals in PHP, and the various response-related global functions =
in PHP, and encapsulate them in objects, to make them at least a little =
more object-oriented?=E2=80=9D Because the RFC begins with a different =
question, it leads to a different answer.


> b) Did you look at how other languages and/or frameworks deal with =
requests
> and responses, and was there inspiration and lessons you took from =
their
> successes (or failures)? [For me this is a key omission in most PHP =
RFCs]

I did look at how other frameworks in PHP handle this. Rowan very =
rightly pointed out that a summary of my research would be helpful, and =
based on his suggestion I have published this comparison document:

  =
https://docs.google.com/spreadsheets/d/e/2PACX-1vQzJP00bOAMYGSVQ8QIIJkXVdA=
g-OMEfkgna7-b2IsuoWN8x_TazxEYn-yVDF2XQIqnzmHqdDO3KEKx/pubhtml

Here are some notes about it:

  https://externals.io/message/108436#108889

As far as other languages, I did the barest bit of a survey, but since =
the goal of this RFC is very directly tied to PHP itself, researching =
other frameworks in other languages was not especially fruitful.


> Thinking about porting existing codebases across, ServerRequest =
holding
> copies rather than references would make it hard to interop with =
existing
> code using the superglobals.

/me nods along

Maybe? Certainly that's true if the existing code depends on superglobal =
mutability at some point past bootstrapping the application.

But speaking from personal experience only, that kind of mutability =
dependence seems relatively rare. That is, I myself do not recall seeing =
applications that co-opt $_GET, $_POST, $_COOKIE, etc. to continuously =
modify them throughout the application. *Sometimes* I see that in $_ENV =
and $_SERVER, but even then it is a limited sort of thing that's easily =
extracted to an application-specific object.

However, if an application *does* depend on that kind of thing, you're =
right: ServerRequest would be disconnected from the superglobals, =
meaning that application would not be a candidate for migration to =
ext/request.

Do note, though, that more than half of the researched projects are =
nominally readonly in public scope, much like ServerRequest. As such, I =
am encouraged to think that the interop problem you describe is not =
often encountered.


> I've had this rewriting legacy apps that use
> $_SESSION and the new framework uses an OO session handler; it's not =
fun
> but with references can usually be made to work. In this case, what =
would
> the migration path look like? Johannes Schl=C3=BCter commented on this =
on 24 Feb
> but I didn't understand his comment (link:
> https://externals.io/message/108436#108737).

$_SESSION and the session-related functions are a whole different beast, =
one that this RFC intentionally avoids. Much as I like ext/session, it =
combines too many different concerns. As you say, you cannot simply make =
a copy of $_SESSION and have everything else keep working; it and the =
session-related functions are intimately intertwined.

But leaving aside $_SESSION, I opine that the migration path in a system =
that already treats the non-session superglobals in a read-only fashion =
might be tedious but not difficult. For example:

1. In your setup code, after you have made any changes to the =
superglobals that your application requires, instantiate ServerRequest =
via whatever object-sharing system you have in place (whether by DI =
container or by any other approach).

2. In one script (or function, or class) inject or otherwise acquire the =
ServerRequest instance; then, line by line, replace `$_GET` with =
`$request->query`, etc. Repeat for the other superglobals in that =
script/function/class. It is *almost* at the level of automated =
search-and-replace. (And along the way, you might do things like =
replacing `isset($_GET['foo']) ? $_GET['foo'] : 'default')` with =
`$request->query['foo'] ?? 'default'`.)

3. Run your regression testing (or smoke-test or spot-check) and correct =
any errors.

4. Commit, push, and notify QA.

5. Repeat 2 & 3 using the next script/function/class.

The benefit of this approach, at least if you are treating the =
superglobals as read-only, is that `$_GET` continues to work alongside =
`$request->query`, so you need not change the entire application at =
once. Baby steps and iterative improvement are possible, even preferred.

To be clear, I have not tried this approach on any applications myself. =
But, it is similar to the methodology used throughout my book on =
modernizing legacy PHP applications, and I imagine it would be pretty =
close to what would actually be needed for a migration.


> In summary, I like the idea of steering people away from superglobals,
> appreciate the work you've put in

Thanks for saying ...


> and am not persuaded by the approach.


... and noted.

After reading this, what modifications to the proposal *might* persuade =
you (if any) short of rewriting how PHP does superglobals in the first =
place?

Regardless, my thanks for your time and effort writing a good critique.


--=20
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