[Zope-dev] Test request: ZSession - ZPatterns based session manager

[Phillip J. Eby]

At 08:58 PM 6/19/00 +0800, mike wrote:
>
>http://www.zope.org/Members/RainDog/ZSession/ZSession-0.0.2.tar.gz/view
>
>Comments?
>

Now that I've had a chance to really look at this (while tracking down one
of the bugs you found), I do have a few comments.

First, nice job...  It's a good adaptation use of ZPatterns.  I do have
some suggestions for fine tuning, though.

* Rather than putting property manipulation functions right into the base
Session class, I would simply let users derive ZClasses of their own from
Session.  They could then actually give the Session a UI of its own, for
example.  Think of someone creating a Session subclass called "Shopping
Cart", with methods for viewing, checking out, adding/deleting items, etc.
Or, if they have many subsystems which want to share the same Session
objects, they could do this by having each subsystem use a different
propertysheet of the same Session object.

IMHO, the basicSheet stuff you've done, while convenient, encourages
developers to take the quick-and-dirty route of just using the Session as a
place to store variables - and that throws away a lot of the usefulness of
ZPatterns.  For one thing, you've guaranteed that Sessions have to be
stored as persistent objects in the ZODB, or at least that the basicSheet
has to support arbitrary properties.

* Make dead session removal something that doesn't happen automatically, or
at least have an option to do so.  Your default deletion algorithm would be
very expensive when executed against a non-ZODB database, and would be more
efficiently done a few times a day in large batches by a cron job.

* Delegate determining which sessions are dead to the Rack, rather than
doing it in the Specialist.  This allows an SQL implementation to make use
of indexes.  Specifically, make removeDead call:

	self._getSource().findIdleSessions(self.session_ttl)

And then provide a default implementation of findIdleSessions() in the
Specialist.  (Btw, please note that your current implementation absolutely
requires that at least *some* session data be stored persistently in the
ZODB, which if you're using a FileStorage, is a bad place for it to be.)
Now, the whole thing works the same way when first installed, but if
someone implements the session ID and lastAccessed attributes in an SQL
database, they can substitute a different implementation for
findIdleSessions() by putting it in the Rack.