Thanks for your work on this Matt.
> 18442 will probably be the one that needs discussion. I have created a
> _load method that does everything that load does except it doesn't
> change the access time (off of which expiration is determined) nor does
> it mark the session as modified. If any parameters are expired, they
> will not be available in the session. The bugfix alters find to use the
> new _load instead of load. The bug report informs us that if the cron
> job runs at an interval less than the expiration time, sessions will
> never expire. By changing find over to use _load, the user is the only
> one who changes the access time and thus our sessions will expire
> because of the user's browsing habits and not our attempts at managing
> the sessions.
I think there is room for some refinement in this area.
"load" vs "_load" is not clear.
Already load() can take up to 3 args, plus now an option to
call "_load()" directly, effectively creating another option.
It's also proposed that find now take 4 args:
find( $dsn, \&code, \%dsn_args, \%coderef_args )
I propose instead the following:
1. A named-parameters interface for load():
load(
dsn => $dsn,
dsn_args => \%dsn_args,
query => $query,
update_atime => 1, # defaults to 1
);
2. And also a named-parameters interface for find()
find(
dsn => $dsn,
dsn_args => \%dsn_args,
code => inspect( look => 'closely'),
update_atime => 0, # defaults to 0 here!
)
Notice that "dsn" and "dsn_args" are now together, which is nice. Also
notice that the "coderef_args" is not needed. Instead, higher order
functions can used, which have worked well in the Data::FormValidator
4.0 interface. If coderefs args would be needed, a closure can be
returned whic provides them. Here's an example what "inspect()" might
look like:
sub inspect {
my %args = @_;
return sub {
my $session = shift;
my $coderef_args = \%args;
# do something...
}
}
Matt,
If you agree with these refactorings, would you mind coding them up?
Mark
|
