PSR-7 why no ->withAttributes() method

[Dracony]

I noticed there is no way to replace all attributes specified for ServerRequestInterface. The only way to set attributes is the withAttribute() which sets them on by one.

To replace all attributes I'd have to first do a couple of of withoutAttribute() calls and then set new ones one-by-one using withAttribute()

This wouldn't be a problem, but because ServerRequestInterface enforces immutability when working with attributes this means the ServerRequest instance has to be cloned during each of those calls. This can easily amount to 10+ needless clonings happening. Can the withAttributes() method be added to the interface to cover this particular case please? We already have a withHeaders()  to achieve that with headers.

[Dracony]

I was mistaken about the withHeaders() method being present, but my point about requiring withAttributes() still stands. The reason being that headers can usually be set during ServerRequest construction, while attributes are more likely to be modified when it is already existing ( e.g. during routing )

[Matthew Weier O'Phinney]

On Fri, May 1, 2015 at 6:43 PM, Dracony <draco...@gmail.com> wrote:
> I was mistaken about the withHeaders() method being present, but my point
> about requiring withAttributes() still stands. The reason being that headers
> can usually be set during ServerRequest construction, while attributes are
> more likely to be modified when it is already existing ( e.g. during routing
> )

We had a lengthy discussion about this some time ago on the mailing
list (look through the archives), and the rationale for omitting it is
the same as for omitting withHeaders(); to reduce ambiguity and reduce
the number of methods exposed in the interface.

What would such a method do?

- Remove all existing attributes and replace with the new set?
- Merge the new set with existing attributes? If so, does it overwrite
values if they already exist, or append them somehow (e.g., creating
an array of values, or appending a string)? If it appends, what
happens if the existing value is already an array?

The problem that occurs is: if we added the method, there were good
arguments for *both* approaches, and if we supported both, the only
way to do so was with either boolean flags (e.g., withAttributes(array
$attributes, $replace = true)) or by adding more methods (e.g.,
withAddedAttibutes() + withAttributes()). Both add cognitive overhead
to consumers, as they need to know the additional behaviors exposed.

We chose to simplify: provide a method for single attributes (and
headers) only, and support only those. This removes the ambiguity, and
reduces the overall number of methods we need to define and support.

> On Saturday, May 2, 2015 at 1:29:11 AM UTC+2, Dracony wrote:
>>
>> I noticed there is no way to replace all attributes specified for
>> ServerRequestInterface. The only way to set attributes is the
>> withAttribute() which sets them on by one.
>> To replace all attributes I'd have to first do a couple of of
>> withoutAttribute() calls and then set new ones one-by-one using
>> withAttribute()
>>
>> This wouldn't be a problem, but because ServerRequestInterface enforces
>> immutability when working with attributes this means the ServerRequest
>> instance has to be cloned during each of those calls. This can easily amount
>> to 10+ needless clonings happening. Can the withAttributes() method be added
>> to the interface to cover this particular case please? We already have a
>> withHeaders() to achieve that with headers.
>>

> --
> 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/57178288-2eec-402d-b497-11a66bc3f904%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.



--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Dracony]

Well to be fair handling appending would be rather simple: get existing, array_merge and use withAttributes() to set the results back. This still could be done with just a single "cloning". When you think of the withHeader() and withAddedHeader() example it works pretty much the same: one method to append and one method to replace. 

Even the simple act of removing the attributes requires getting all of them and calling withoutAttribute() in a loop. So in my mind having the withAttributes() perfectly fixes issues with removing all ( just set to an empty array),  merging ( get, merge, set) and replacing.

[Matthew Weier O'Phinney]

On May 1, 2015 8:05 PM, "Dracony" <draco...@gmail.com> wrote:
>
> Well to be fair handling appending would be rather simple: get existing, array_merge and use withAttributes() to set the results back.

You completely missed my point: the problem is that you're assuming it should behave a certain way (overwrite), while others indicated such a method should behave quite differently (merge). Because expectations differed, we either needed to have methods to support each use case, or remove the method entirely. We opted for the latter.

I do not disagree that implementing either case is relatively easy; my point was that, to reduce the size of the interface and to reduce ambiguity of expectations, we choose to remove such methods.

> To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/aa5af324-6288-4c0d-9ec2-4d1f8cecb0ea%40googlegroups.com.

[Roman Tsjupa]

What are the pros of having it append attributes vs replace them ? As I already proved appending can be easily achieved by getting and replacing. While the reverse is not true

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/_DOlNaYrVbY/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/CAJp_myWMZ3J6Vi57qvD63dP5vNYdb%2BN9%2BPkzU%2BacsNJeuZUSxA%40mail.gmail.com.

[Larry Garfield]

I was one of those who asked for withAttributes() to be removed, after implementing Stacker.  In short, I kept having the temptation to write withAttributes($stuff_im_adding), because that's how it would logically read, even though that would replace any that were already there.

It's not a question of what versions are possible to implement.  It's a matter of DX: withAttributes() is confusing for someone who is using the interface in an IDE rather than actually reading the spec.  Does it replace or append?  It *could* be either.  Either are completely logical conclusions for someone to draw, and if they draw the wrong one you get fun subtle bugs.  So go with neither, and there's no confusion.

If ~5 function calls are the bottle neck in your application, your application is already faster than 99% of the PHP software out there so you don't need to worry anyway. :-)

--Larry Garfield

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CANamvv3n7KJdLEsBavpr6VngCiho5Q5XypNntStuLs0L%3DLd98g%40mail.gmail.com.

[Paul M. Jones]

> On May 2, 2015, at 13:35, Larry Garfield <la...@garfieldtech.com> wrote:
>
> If ~5 function calls are the bottle neck in your application, your application is already faster than 99% of the PHP software out there so you don't need to worry anyway. :-)

Hear, hear.


--
Paul M. Jones
pmjo...@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

[Woody Gilk]

On Sat, May 2, 2015 at 1:35 PM, Larry Garfield <la...@garfieldtech.com> wrote:
> If ~5 function calls are the bottle neck in your application, your
> application is already faster than 99% of the PHP software out there so you
> don't need to worry anyway. :-)

That's really not the point. The point is developer time... if 100,000
developers all have to spend a minute writing those exact same lines,
there is a problem. I'm not sure that "I didn't read the docs" is a
valid reason not to implement something, unless it completely breaks
the expected pattern.

Perhaps it would be good to have a withNewAttributes() method to make
it completely clear that it is a _replace_ operation?

--
Woody Gilk
http://about.me/shadowhand

[Matthew Weier O'Phinney]

And this steps directly into the issues I've already mentioned; we now
have a different set of verbiage than we have elsewhere, and the
intent is *still* unclear (what does "new" imply? replace? or add the
options that do not exist already? what about those that do?).

The fact that there's debate over this at all is why we chose not to
add the method.

[Roman Tsjupa]

Why would anyone think that withAttributes() somehow implies appending ? Lets look at header methids, we have withHeader() and withAddedHeader() . This makes it clear that only methods containing Added add things instead of replacing.

Every other with* method replaces contents, how would anyone be confused by that method name.

Furthemore this is an amazing nonissue, since even if someone somehow misreads the docblock he will notice how the behavior actually works after first try.

So what you say is that its better to sacrifice a widely used usecase and force people to write same code to remove attrs one by one and add new ones, while doing unnecessary clone() calls all the time, just so some random person hypothetically doesnt get confused ?

This is silly.

--
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/_DOlNaYrVbY/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/CAJp_myUdK-HM19YG5mjru5Q-udRBP8yMh6EdQujqvQ4Li_BFFg%40mail.gmail.com.

[Woody Gilk]

So choose a word that isn't ambiguous.

--
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/CAJp_myUdK-HM19YG5mjru5Q-udRBP8yMh6EdQujqvQ4Li_BFFg%40mail.gmail.com.

[Christopher Pitt]

withAttributes() and withAddedAttributes()?

[Roman Tsjupa]

withAttributes that replaces attributes

On May 3, 2015 10:12 PM, "Christopher Pitt" <cgp...@gmail.com> wrote:

withAttributes() and withAddedAttributes()?

--
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/_DOlNaYrVbY/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/adc7b7bc-6f95-4466-8d02-3b10435c4145%40googlegroups.com.

[Matthew Weier O'Phinney]

On Sun, May 3, 2015 at 4:35 PM, Roman Tsjupa <draco...@gmail.com> wrote:
> withAttributes that replaces attributes
>
> On May 3, 2015 10:12 PM, "Christopher Pitt" <cgp...@gmail.com> wrote:
>>
>> withAttributes() and withAddedAttributes()?

I've done a poor job of explaining the problem, and why there's a case
to be made for each of replace or merge.

In the case of both attributes and headers, it is common to want to do
either of the following two options:

- Replace the entire set of items
- Add items en masse *without changing existing items*.

Let me explain the latter.

Consider that you've got a set of application attributes you want to
inject early on; these might be things like the application base path,
the path to local bundles/modules/what-have-you, maybe some sane
defaults for things like encoding, current version, etc.

Then consider that you perform routing, and want to inject matched
parameters into the attributes. In many cases, routes may contain
multiple parameters: /:controller/:action/:identifier would create
three parameters.

What happens when you call withAttributes($routeMatches)?

That stuff you provided at the start of the application, where does
that stuff go? Are those all removed? or do we merge the $routeMatches
with them? That's really the heart of the "merging problem" I was
alluding to (though whether or not the existing values should get
merged with the new values in the case of equal keys is a non-trivial
question as well).

Yes, you could say, "withAttributes()" always replaces. But then
you're left with a rather ugly paradigm that will be quite common:

$request->withAttributes(array_merge($request->getAttributes(),
$routeMatches);

(And should that be array_merge? or array_merge_recursive? or
array_replace_recursive()?)

If we say it merges by default, then how do you replace with the new set?

foreach ($request->getAttributes() as $key => $value) {
$request = $request->withoutAttribute($key);
}
$request = $request->withAttributes($routeMatches);

I think you're seeing a pattern here, or, at least, I hope you are:
having such a method does not guarantee less effort by the developer;
to the contrary, for even basic things, these methods require as much
or more code than what we currently have:

foreach ($routeMatches as $key => $value) {
$request = $request->withAttribute($key, $value);
}

"Why don't we just add another method?" you and others ask. Because
then the user has to know how each works, and the methods become
redundant with the API we have already exposed.

And, to be honest, we then have to debate whether or not the one that
merges should do so recursively, and what algorithm is most
appropriate, and the next thing that happens is that another six
months has gone by, and we'll likely have decided that what we have
*right* *now* actually already works, *without* ambiguity.

[Christopher Pitt]

"Why don't we just add another method?" you and others ask. Because 
then the user has to know how each works, and the methods become 
redundant with the API we have already exposed. 

They do vastly different things though. There are situations in which either or both would be useful. 

And, to be honest, we then have to debate whether or not the one that 
merges should do so recursively, and what algorithm is most 

appropriate...

These are implementation details. Can attributes or headers have nested arrays or parameters? Not sure these are good reasons not to have the additional method.

[Paul M. Jones]

> On May 3, 2015, at 21:19, Christopher Pitt <cgp...@gmail.com> wrote:
>
>> These are implementation details. Can attributes or headers have nested arrays or parameters? Not sure these are good reasons not to have the additional method.

At the of merely gainsaying, they really are good reasons. Much as I'd like to see a withAttributes() that works "the way I expect", there are at least three reasonable expectations, each of which is useful and all of which are mutually exclusive. Leaving it out is the way to go at this point.

[Roman Tsjupa]

The case for replace vs add is easily solved if you consider that  you can use "replace" to achieve "add" but not vice versa. So replacing is more versatile and thus superior.

As for the array_merge problem this again is a huge non-issue since it is not going to be called within the implementation anyway. The code that uses the interface can manipulate attributes however it likes before calling the withAttributes method. This makes array_merge vs array_merge_recursive entirely the problem of your hipothetical router library not the interface implementation one .

--
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/_DOlNaYrVbY/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/B45A5138-D542-480E-89F2-E0B28CDB6EE6%40gmail.com.

[Jason Judge]

On Monday, 4 May 2015 03:14:30 UTC+1, Matthew Weier O'Phinney wrote:

On Sun, May 3, 2015 at 4:35 PM, Roman Tsjupa <draco...@gmail.com> wrote:
> withAttributes that replaces attributes
>
> On May 3, 2015 10:12 PM, "Christopher Pitt" <cgp...@gmail.com> wrote:
>>
>> withAttributes() and withAddedAttributes()?

...

Yes, you could say, "withAttributes()" always replaces. But then
you're left with a rather ugly paradigm that will be quite common:

    $request->withAttributes(array_merge($request->getAttributes(),
$routeMatches);

I've had to do this a few times in Laravel 4 when redirecting with session variables from different sources. It feels dirty /shudder

Another example if merging in bulk is Content Security Policy (CSP). An application may have many rules collected together by different packages. These rules define what the end user's browser is allowed to access from within the delivered page. Then right at the end, there will be a bunch of headers to add to whatever headers are already defined, to deliver those rules to the browser. I started writing a package to handle these rules last year, and it would be great to incorporate features to make it talk PSR-7. I'm not asking for additional methods though - stick to the essentials. Just giving another example of where you may want to merge rather than replace.
 

(And should that be array_merge? or array_merge_recursive? or
array_replace_recursive()?)

...

[Roman Tsjupa]

Well so you would say array_merge feels more dirty then removing and adding attributes one by one ?

--

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/_DOlNaYrVbY/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/053b4a63-1ab8-40bc-b302-493ad1649a3d%40googlegroups.com.

[Jason Judge]

On Tuesday, 5 May 2015 11:18:02 UTC+1, Dracony wrote:

Well so you would say array_merge feels more dirty then removing and adding attributes one by one ?

Adding the values one-by-one would have been better, if that was an option for the `Response::redirect()` action. And maybe it was an option in the underlying response or session object. The fact that `with*()` replaced all existing session variables instead of merging, side-tracked me enough already. And that's the problem - it was ambiguous, and maybe there are some conventions as to what "with*" means, but it is certainly not in the documentation.

Anyway, I don't want to take this discussion off on a tangent about one particular implementation, but if I had only the option to add one parameter at a time, then it would have been much less ambiguous and simple to code for.

--------

I do have a more general question about the cloning needed when adding headers to an immutable request object. If adding or removing a header involves creating a clone of the object, then what happens to the earlier clones that other variables may be holding from when they were assigned earlier? I'm just trying to understand what immutability means for coding patterns and techniques in PHP.  I may just be misunderstanding what happens when several variables point to an object and you replace that object through *one* of those variables.
 

[Dracony]

Well adding the word "replace" to the documentation block would fix that problem )

[Márk Sági-Kazár]

I think you are asking wrong questions. Why and How does not matter. The point is that it causes confusion and not just in theory.

[Matthew Weier O'Phinney]

On Tue, May 5, 2015 at 7:40 AM, Jason Judge <jason...@consil.co.uk> wrote:
> I do have a more general question about the cloning needed when adding
> headers to an immutable request object. If adding or removing a header
> involves creating a clone of the object, then what happens to the earlier
> clones that other variables may be holding from when they were assigned
> earlier? I'm just trying to understand what immutability means for coding
> patterns and techniques in PHP. I may just be misunderstanding what happens
> when several variables point to an object and you replace that object
> through *one* of those variables.

It depends on where they are assigned. :)

Essentially, once no more references to the value are present, PHP
will GC them. Until then, they will stick around. When you do chain
assignment:

$request = $request
->withHeader('X-Foo', 'bar')
->withHeader('X-Bar', 'baz');

the intermediary will likely undergo GC "soonish" as there are no
lingering references to it.

> 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/14300c28-71e8-4f1f-a88e-c2090997de9a%40googlegroups.com.

>
> For more options, visit https://groups.google.com/d/optout.

[Roman Tsjupa]

But you also have to consider the cloning overhead. Remember that using "clone" is only one possible implementation. If it's implemented by calling the constructor that's already some overhead.

Another overhead example comes with your withHeader() example. The interface demands headers be case insensitive, so the implementation must keep a map of lowercased header names to ensure that ( you do that in your implementation too btw). If 'clone' is being used it's easy to transfer old map to the new obect, but if a constructor based approach is used, such a map would be reconstructed in each "clone". And that is already a m*n situation

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAJp_myV%3Drzd5W%3Dbsnpp0Q5M9H0zW49h6Ut2FfH-AUiJFrC5UFQ%40mail.gmail.com.