Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:97567
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain pthreads.org from 209.85.210.173 cause and error)
MIME-Version: 1.0
In-Reply-To: <0D275F6F-85D3-4E9D-AF89-03C91A23D5BD@gmail.com>
References: <D25FDAC5-A72F-4391-8414-8966124173ED@gmail.com>
 <CAL=_i_mFfPNpPZ0z48LdoKwNt4DLRpBpZbVUoCyz8Bi2PA82zg@mail.gmail.com> <0D275F6F-85D3-4E9D-AF89-03C91A23D5BD@gmail.com>
Date: Sat, 7 Jan 2017 21:41:41 +0000
Message-ID: <CAL=_i_kT6Dnty5pJFfoJy5aaffsvz_WfqdhTqJf76tatZdi9dg@mail.gmail.com>
To: Paul Jones <pmjones88@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=20cf307ac4f30c3ed1054588009d
Subject: Re: [PHP-DEV] [RFC][Discussion] ServerRequest and ServerResponse objects
From: pthreads@pthreads.org (Joe Watkins)

--20cf307ac4f30c3ed1054588009d
Content-Type: text/plain; charset=UTF-8

Evening Paul,

> My apologies for giving you cause to be surprised. I thought I had
broadcast my intentions back in September <http://news.php.net/php.
internals/96156> but obviously I failed. My bad.

The point of an internals discussion is to gather consensus that an RFC is
a good idea; let us look at the responses you got:

  * Marco said it felt like three steps backwards
  * Micheal said he felt it was covered by http extension
  * Larry said he didn't see that this adds value
  * Rowan said it was an interesting idea, but didn't see how it fitted
into the wider ecosystem
  * Davey agreed with Rowan and went on to question how it could fit in
with HTTP/2.0

That doesn't sound like a positive consensus that this is a good idea, it
is rather the opposite.

Hopefully you can see now why I am surprised that you are deciding to push
forward with something that didn't get any positive responses, and only got
a few people talking.

> FWIW, I have not found it possible to provide read-only public properties
in userland

To be clear, I didn't suggest that you write in PHP, I just stated that
this does not belong in the core.

> However, its design is born from commonalities across many different
projects, from many different authors. They all independently settled on
broadly similar solutions to the problem of dealing with PHP superglobals,
and PHP global functions for sending response elements, in a more OO-ish
way.

Of course those projects have commonalities, they are all solving the same
"problem", for the same purpose, using the same internal API ... I'm not
sure what point you are trying to make here, and "OO-ish" doesn't mean
anything.

I'll end how I started: The point of these discussions is to gather
consensus, and a consensus has emerged that this is not the right solution,
nor the right time, nor the right implementation, nor the right route.

Please think about that.

Cheers
Joe

On Sat, Jan 7, 2017 at 9:12 PM, Paul Jones <pmjones88@gmail.com> wrote:

> Hi all,
>
> Replying to both Marco Pivetta and Joe Watkins here, as some of their
> critiques overlap. I apologize in advance for the length of this email.
>
> * * *
>
> On Jan 6, 2017, at 10:47, Marco Pivetta <ocramius@gmail.com> wrote:
>
> > Bundling the [PSR-7] interface is no biggie, so I suggest steering away
> and going that way
>
> It's a bigger deal than one might think. But again, I have no desire to
> highlight the negatives of PSR-7 in this discussion.
>
> I'd much rather discuss the RFC on its own merits, which Marco does next:
>
> > There are several technical issues too:
> >
> >  * A magic constructor that fetches data from global state: add a
> factory for that, leave the constructor open for modifications or new
> instantiation ways, or make it private.
>
> I get that. John and I tried both variations, and found that the
> static-factory version was clunkier than we expected. For example, if
> ServerRequest uses a static factory, then users extending the ServerRequest
> class will additionally have to write their own factory. Having the
> constructor just "felt" better when we used it. But, I am willing to
> revisit that decision if needed.
>
> As far as global state: yeah, I hear you. Since ServerRequest is intended
> to encapsulate the superglobals, it seemed reasonable to have it default to
> copying the superglobals internally. Note that the `__contruct(array
> $globals = [])` signature allows for a complete override of whatever you
> want. (The vagaries of JIT population come into play here as well.)
>
>
> >  * Magic read-only properties that go against the language semantics:
> make 'em accessors, or do whatever you want, but don't invent new language
> semantics just for your own extension.
>
> You're right that it's not often seen. However, public read-only
> properties are not a new invention for this RFC. They already exist, such
> as in PDOStatement::$queryString <https://secure.php.net/pdostatement>.
>
> FWIW, the use of a public read-only property turns out to be very
> practical in daily work. Instead of variations on ...
>
>     $request->getPost(); // returns the whole array
>     $request->getPost($key); // returns a value in the array
>     $request->getPost($key)[$subkey]; // returns a sub-value in the array
>     $request->getPost($key, $default); // returns a value, or a default if
> not present
>
> ... you work with the property like any other array:
>
>     $request->post;
>     $request->post[$key];
>     $request->post[$key][$sub];
>     $request->post[$key] ?? $default;
>
> It's just that the property is read-only, and you get an exception if you
> try to change it (same as with PDOStatement::$queryString, if I recall
> correctly.)
>
>
>
> >  * A series of non-exhaustive properties that give some "helper" API to
> see if it's XHR or not, violating basic SRP. Are you abstracting a request
> or what you want to do with the request? You can define separate utility
> functions for that. Yes, functions.
>
> /me nods
>
> They are definitely non-exhaustive; the list of properties was put
> together from several reference projects that have classes similar to
> ServerRequest (more on that below).
>
> When some of those projects had a use for for a particular value, I
> figured it made sense to include it here. (If you or others want to provide
> patches to make it more exhaustive, so much the better!)
>
> As for being a violation of the single-responsibility principle, I don't
> know. It seems reasonable to think that if the object contains (e.g.)
> `$request->server['HTTP_FORWARDED']`, it might do well to contain a
> parsed version of the same information as an array in `$request->forwarded`.
>
>
> >  * As per current API, I'd expect the ctor to throw an exception when
> used outside the web SAPI context
>
> I will defer to John Boehr on this one. I do wonder how difficult that
> would make it to test at the command-line, or include in command-line
> testing environments (e.g. PHPUnit).
>
>
> >  * You use the *with* prefix for setting values, while you know exactly
> that *with* was used to differentiate from a mutable and a non-mutable API
> in the existing request abstractions: this makes things confusing again
>
> I'm sorry, I don't understand the criticism here. The
> `ServerRequest::with*()` methods do in fact provide an immutable API (a
> pretty strict one at that). That is, they return a new copy of the
> ServerRequest object with the changed values, leaving the original copy
> unchanged. Maybe I'm not getting what you're saying ... ?
>
>
> >  * There's no interface for these classes, and they are not marked as
> final, as far as I can see. This means that users will extend them (unwise
> choice), and with that they will have a class with reflection bits in the
> extension (extremely bad idea!). An interface and final are needed here.
>
> Extensibility is an intended option. Many classes in PHP can be extended,
> to good effect (e.g., PDO).  But I am willing to be hear about why this is
> a bad call -- can you expand on your reasoning?
>
> > I am obviously defensive and biased here
>
> Sure; anyone would be. Thanks for being up-front about it though. :-)
>
> * * *
>
> On Jan 7, 2017, at 06:00, Joe Watkins <pthreads@pthreads.org> wrote:
> >
> > Regardless of the details of the implementation, I feel it necessary to
> point out that this is a surprising RFC.
>
> My apologies for giving you cause to be surprised. I thought I had
> broadcast my intentions back in September <http://news.php.net/php.
> internals/96156> but obviously I failed. My bad.
>
>
> > There are extensions that are absolutely a core part of the ecosystem
> that remain outside of php-src because that is where they belong. xdebug is
> one, apc was another, redis, memcached, and a whole list of others besides.
>
> Noted. Honestly I figure at best this extension has only 1 chance in 3 of
> getting accepted.
>
> However, as I think it is central to the operation of PHP in its primary
> use cases (i.e., reading superglobals and sending back response
> headers/cookies/content), taking that chance seems worthwhile.
>
>
> >   * it is not restricted by what is possible outside of php-src (as
> phpdbg was)
>
> FWIW, I have not found it possible to provide read-only public properties
> in userland, especially when extending a parent class that wants to enforce
> the read-only nature of those properties. This is central to the intent of
> ServerRequest.
>
> Further, as far as I know, it is not possible to detect if a variable is a
> reference in userland. Doing so is a prerequsite to enforcing immutability,
> which ServerRequest intends to do.
>
> I am of course happy to be corrected on this, as it would solve other
> problems I have had elsewhere.
>
> * * *
>
> Both Joe and Marco had a similar observation.
>
> Marco said:
>
> > if something new is being designed for inclusion in php-src, and it has
> to reach millions of developers, it better be *reeeeeealli* good. ... IMO
> needs another few design iterations, and a lot more adoption, in order to
> become interesting. Orrrrr you provide us with a factory for an
> implementation of these concepts that already has adoption, traction and
> extremely careful design behind it.
>
> Joe said:
>
> >
> >   * it doesn't have any user base
> >
> > ...
> >
> > I think if you really want to push this forward, the best place to do
> that is outside the core, where you can pick a release schedule not bound
> by php-src, where the API can shift because of the will of consumers,
> rather than internals (dis)ability to agree on anything so contrived as how
> we should handle HTTP transactions.
> >
> > The day may come where an abstraction is so popular that we should
> consider merging it into core, dedicating our time to maintaining it, and
> even possibly allow it to deprecate and completely replace the current API
> ...
>
> You are both completely correct, of course, that this extension has so
> little adoption as to be virtually unknown. It is, in itself, a infant.
>
> However, its design is born from commonalities across many different
> projects, from many different authors. They all independently settled on
> broadly similar solutions to the problem of dealing with PHP superglobals,
> and PHP global functions for sending response elements, in a more OO-ish
> way.
>
> Links to references, and longer explanation of the "broadly similar
> solutions" are in my blog post at <http://paul-m-jones.com/archives/6494>.
> Meanwhile, note that the reference projects include Aura, Cake, Code
> Igniter, Horde, Joomla, Lithium, MediaWiki, Phalcon, Symfony (and thereby
> Drupal and Laravel), Yaf, Yii, and Zend Framework. That's a mere dozen that
> are themselves representative of wide swaths of other parts of PHP userland.
>
> So it is true that this extension, per se, is not widely adopted. But it
> is also true the collection of ideas and solutions it represents is widely
> vetted and broadly agreed-upon in intent, if not in the names of specific
> classes, methods, and properties.
>
>
> --
> Paul M. Jones
> pmjones88@gmail.com
> 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
>
>

--20cf307ac4f30c3ed1054588009d--