[PSR-17] ResponseFactoryInterface

[Rasmus Schultz]

I know it's too late to bring up something like this, but... you don't notice until you actually start using it in real code.

I wrote this class that uses PSR-17 to marshall between ResponseInterface and JSON-serializable data:

https://gist.github.com/mindplay-dk/5fc5daa1cf5efd7d7ea2be99b1ec99bf#file-responsemarshaller-php-L40-L42

See the highlighted lines?

So, whenever you need to transfer headers from one object to another, currently, you have to loop over

every individual header and manually transfer them one at a time.

This has performance implications (cloning the request object for every individual header being copied)

but also leads to ugly, repetitive loops like the one highlighted in the example above.

Makes me wish we had an argument for $headers in the ResponseFactoryInterface method.

But also makes me wonder if this was actually an oversight in PSR-7 itself?

Like, why doesn't it have a withHeaders() method that lets you apply/replace all the headers?

I guess as always it's too late to notice these things as PSRs are immutable and non-versionable? :-/

- Rasmus

[Alexandru Pătrănescu]

Hi,

In terms of packages, I can't see a reason not to have a version 1.1.0 to php-fig/http-message that will add more methods to Psr\Http\Message\MessageInterface
I guess this can be achieved by another PSR, superseding PSR-7 in a backward compatible way.

Also, I was looking at withHeader and noticed that in docblock it mentions that it can accept a string or an array of strings.

I can only guess what implementations would do with an array and that would be: add each of the strings as a value for that header.

To my knowledge, there is no mention of this behavior.

Regarding if this should have been implemented by FactoryInterface, I think Builder pattern might look better.

Alex

[Rasmus Schultz]

> I can't see a reason not to have a version 1.1.0 to php-fig/http-message that will add more methods

Adding a method to an interface is a breaking change, as it renders every existing implementation invalid.

I've brought this up on the list before - PSRs are not versioned, they're immutable. Which is a real problem when the "specification" consists of a package with a version number.

Also, PSR-7 is already a well established "brand", so there's a disincentive to start a "competing" PSR for a seemingly minor change like this - or on other words, there's a disincentive to incrementally improve the package like you would normally do if you're a maintainer.

In other words, there's no simple fix when you learn that you've made a mistake - a new PSR is the only way to go, and it will likely be really hard to get support for that... Everyone is already fully invested in PSR-7 - and it does "work", it just has this oversight that makes it clumsy and slow to add/replace headers.

If I was going to propose an entirely new PSR, I would probably take another hard look at the HTTP model in its entirety - because it was modeled after PHP's CGI interface (superglobals) first and HTTP second, there's a lot of duplicate state in the model, which gets weird at times.

Anyhow, I don't have much hope that a simple change to PSR-7 would pass and succeed as an entirely new PSR.

> I can only guess what implementations would do with an array

Headers are multi-valued, this is well covered by the spec - hence getHeaderLine() and withAddedHeader().

--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/aMetK_LAmsk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/e78ca9b9-1d0c-44c6-b629-a6f142148d72%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Woody Gilk]

I don't see what the problem with this code is. It's clear and easily understandable.

As has been discussed many times, there is very little performance penalty for lots of clones, especially in PHP >7.

--
Woody Gilk
https://shadowhand.me

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/2274cea2-1f46-404f-bb60-1b376d005d71%40googlegroups.com.

[Nicholas Ruunu]

Agree with Woody, although ResponseFactoryInterface should've added headers as an argument.

Body and headers is what a http message is really constituted of.

[Stefano Torresi]

Like, why doesn't it have a withHeaders() method that lets you apply/replace all the headers?

I guess because what that would do in every implementation is calling `withHeader` in a loop. :-P

Is this really that big of a problem? OTOH the message interface is already quite crowded.

Also, a very lengthy discussion could be made on how the `$headers` input array should or shouldn't be structured.

[Larry Garfield]

Matthew and I talked about that during the PSR-7 development. It was one of
the confusion points I had in my own implementation attempts. Specifically,
is withHeaders() "replace all headers and have just these" or is it "multi-add
these headers, as if you were looping"? Or "multi-add but replace existing
headers if the name already exists, which is different behavior than a loop on
addHeader()"?

It was just too confusing, so since a loop on withHeader() is so cheap the
decision was to go for the smaller, more self-documenting surface area.

--Larry Garfield

[Rasmus Schultz]

I did a quick benchmark, and I'm fairly satisfied it's not a performance concern (in recent versions of PHP) for normal use-cases.

https://bitbucket.org/mindplaydk/header-bench/src/master/

I still think the code to manually copy headers is ugly and verbose, but, well... may be one of those cases where ugly and verbose makes sense...

--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/aMetK_LAmsk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/1711626.aciqfvl2hO%40vulcan.