[Catalyst] Plugin::Authentication overrides $c->req->user

[Tomas Doran]

On 20 Feb 2009, at 22:57, Daniel Westermann-Clark wrote:

> On 2009-02-11 21:53:48 +0000, Tomas Doran wrote:
>>> Why not just add a remote_user() method on $c->req instead? It's a
>>> little more typing, but is more explicit about where the value comes
>>> from and doesn't potentially break any existing apps.
>>
>> Patches on 5.80 welcome :)
>
> Attached is a set of patches to add support for $c->req->remote_user,
> including a basic test.

Good stuff, thanks. I've branched 5.80 trunk and applied your Runtime  
change, and then I've fiddled the 'do we warn' logic to be a bit  
safer. Have a look and let me know what you think?

http://dev.catalystframework.org/repos/Catalyst/Catalyst-Runtime/5.80/ 
branches/deprecate_req_user/

Test for the new functionality looks fine.

> There is also a deprecation warning for non-Catalyst packages calling
> $c->req->user.

Is anything in the current test suite triggering the new warning? If  
so, can you switch it over to be calling ->remote_user instead, and  
can you add a call to read ->user which provokes the warning, and  
test you get the expected warning (see t/deprecated.t r9354 - you  
could just add the warning test here/to that app which already has  
its global logger overridden?)

> I'm not sure about the engine patches, but it seemed like a
> forward-compatible way to add support now for the new method.

They look totally sane to me. Lets get Runtime right and a nod from  
andyg before applying them however :)

Feel free to supply another patch against the branch, or hop on irc  
and grab a commit bit so you can commit yourself?

Cheers
t0m