[Zope-dev] Test request: ZSession - ZPatterns basedsession manager

[mike]

"Phillip J. Eby" wrote:
> 
> 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

Thanks.

> 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

I put those get/set/has stuff just for make easier life of some lazy
people (like me :-)

> 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.

Well, well, well. As for me, beter I've set up a Client abstraction
which have access to CartManager and store cart_id in the session
object. Mixing Session and Cart is not a good idea just because Client
can have several carts, for example. The composition is better than
inheritance in this case, Phillip :-)

> 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.

But without this stuff people will have to code ZClasses each time they
want sessions. Hmm... I guess the best way is splitting current session
class into two - simple Session and LazySession.

> * 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.

Moreover, I found a bug which causes infinite loop :-( You're right,
automatic removal should be optional. Will be fixed in 0.0.3 release.
> 
> * 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.

Agree with you completely.

Thanks,
Mike