Re: [cgiapp] RFC: DBI::Session proposed docs

[Rhesa Rozendaal <suppressed>]

Just some quick initial thoughts...

Mark Stosberg wrote:
> I also want to explain why I removed a few CGI::Session methods:
>
>  expire()      | Use update( duration => '' ) instead. 
>                  Compatibility for CGI.pm cookie style could be ripped from CGI::Session.
>                  Again, this method could re-appear as syntactic sugar.

This doesn't feel right to me. It mixes the user data space with 
internal state management, and I don't think that's a good idea. First 
of all, it prevents me from having a parameter named 'duration'. Second, 
it uses named parameters for two different goals. This contradicts your 
earlier objection against CGI::Session, that it put the (ACE)TIME 
variables inside the data hash.

> ##########################################
>
> So, given everything we've discussed, does it seem worth it to you to
> have yet-another session handling module, or are you happy enough with
> CGI::Session? 

To be honest, CGI::Session has been quite adequate for our purposes. The 
only problem we have with it is how it handles expiration, but Ron's 
module already solved that for us. We occasionally had issues with the 
File storage backend not flushing sessions properly on its own, but a 
couple of explicit flush() calls fixed that as well.
Given the number of other session modules out there, I personally don't 
really have a need for yet another one.

>     Mark
>
> ##########################################
>         # expire '_IS_LOGGED_IN' flag after 10 idle minutes:
>         $ses->param('_IS_LOGGED_IN_exp => '10 min');

I really think this looks contrived. I would prefer an expire() method.

>         This is not required. It simply allows you to use custom column
>         names.
>
>           session_id             => 'session_id',
>           remote_addr        => 'remote_addr',  
>           creation_time      => 'creation_time',
>           last_access_time   => 'last_access_time', 
>           duration                   => 'duration', 

Ah, that explains why you did param(duration => '+10m'). I think these 
standard fields deserver their own accessor. $ses->duration('+10m') 
looks clearer to me. Reserve the param() method for user data.

>         For example, to add a column named "order_id" which you want to
>         allow to be expired, you would add these two columns:
>
>                 order_id                        int,
>                 order_id_exp        interval,

And another clue :)

In any case, I feel the separation between session state information and 
actual session data should be more clear.
My first thought would be to have two tables for this: one with the 
session_id, *_time and *_exp fields, and another table with the actual 
data columns. Expiring and deleting sessions would need a cascading 
delete, so that may not be such a brilliant idea. On the other hand, 
reporting would be easier, I suppose, if you're into that. And I _think_ 
it would make it simpler to have multiple data rows for one session, 
which could also be a big plus.

I saw your initial idea to use Class::DBI for this, and that you later 
rejected that.
My first reaction was also ambivalent. Class::DBI provides an excellent 
interface to manipulating databases, and I hardly do db programming 
without it anymore, but in this case you would run into difficulties 
generating the appropriate classes on the fly. Your module has to be 
very flexible in all kinds of ways, and I think cdbi is better suited to 
a more stable, fixed data model.
It would be very beneficial to have a similar interface nonetheless. 
Having accessors for each column makes for clean and readable code.

Nevertheless, I feel - like others in the discussion - that this 
proposal imposes too much structure on sessions. Our session usage is 
also quite loose, unstructured, and volatile, and wouldn't benefit much 
from a fixed database layout. We generally tend to use sessions for 
keeping shortlived data around, such as authentication fields, or caches 
for expensive operations. Definitely not stuff we're interested in 
tracking over longer periods of time. For those purposes we would write 
our own set of Class::DBI objects, since we do all our db activities in 
that framework already.

Anyway, it looks like you have an itch to scratch, so by all means go 
ahead. I'll be following your progress with interest :)

Cheers,
Rhesa

---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
             http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed