[Catalyst] Unnecessary session writes

[Tomas Doran]

On 17 Dec 2008, at 13:11, Bill Moseley wrote:

> On Wed, Dec 17, 2008 at 08:34:36AM +0000, Tomas Doran wrote:
>>
>> Apologies if I wasn't being clear perviously - could you convert your
>> suggested changes and test into a diff against the distribution which
>> someone could just apply with patch, rather than having to fiddle  
>> around
>> copying chunks of code out of email?
>
> Sure, but not until we can figure out what changes really need to be
> made.

I think that not creating a session until something has been written  
into it is a totally sane default (and providing an option to always  
create one as-per the current behavior if people actually want that).

>>> But, the plugin also always writes an "expires:" key into the store
>>> (hence the /^session:/ match above), so if the goal is to not  
>>> write to
>>> the store unless there's something to store then that needs looking
>>> at, too.
>>
>> Agreed.
>
> But, if you have an application that is mostly read-only on the
> session then there's the potential of timing out an active session if
> it's not refreshed.  Depends on the application.  If using the
> application naturally writes to the session often then it's not such
> a problem.

I think that for backwards compatibility, we need to keep the session  
updated on read.

However it would be nice to provide a way to configure the session  
plugin so that reads didn't refresh the session..

> And for stores that manage expiry we don't really want that extra
> store call in addition to session store.
>

Session handling could do with refactoring as-per the authentication  
plugins, so that the store and state were not plugins themselves,  
this would make things a lot 'nicer'.

However, in the shorter term, providing people with a way to change  
the default behaviors would go a long way.

>> If you'd like to include TODO tests for that in the patch (or just  
>> fix
>> it, with no TODO in the test), then that would be fantastic.
>>
>> As you've probably seen from the mailing list already, there are a
>> couple of pending patches on Catalyst-Plugin-Session already:
>>
>> http://dev.catalyst.perl.org/repos/Catalyst/branches/Catalyst-Plugin-
>> Session/
>
> I have not paid that much attention.  What's the plan for those  
> branches?

The plan is to try and get some people to actually test them out  
(both people who have the issues these patches solve, and people who  
don't), before merging them, and then getting them released..

That's my understanding anyway, I may be wrong: the Session plugin  
isn't my baby, I'm just trying to ensure the left foot knows what the  
right foot is doing here. :)

Cheers
t0m