Test request: ZSession - ZPatterns based session manager
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.
"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
At 10:51 PM 6/20/00 +0800, mike wrote:
"Phillip J. Eby" wrote:
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 :-)
I look at components in terms of "value add". Your SessionManager's value-add is that it lets you transparently create an instance of something which is associated with a cookie and which can be expired whenever it's idle for greater than a certain period of time. But the idea of a "Session" has no real value-add to me; it just makes it possible for a thing to be managed by the SessionManager. You could in fact do without the Session class altogether, and have SessionManager simply set the lastAccessed attribute directly. This would actually make SessionManager more useful, because you could use it with ZClasses that hadn't been explicitly designed to be session-based, as long as there was an AttributeProvider for lastAccessed. And you could drop the SessionSource class altogether as well, since any RackMountable/DataSkin would be acceptable as a "Session". Popular opinion to the contrary, I don't believe that there is such a thing as a "session" object. Session objects are a hack, to give you a place to put session-specific objects' data. In my opinion, if you can have session-specific objects, you don't really need a "session" object. If one can simply call CurrentCart() or CurrentGame() (where CurrentCart and CurrentGame are SessionManager specialists) to retrieve a session-specific shopping cart or gameboard, why have a "session" object? If I want them both to use the same cookie, I can certainly do that by setting the properties on both CurrentGame and CurrentCart. And there's also no chance that two session-specific objects from different vendors (such as perhaps my Games and Carts) will accidentally collide with each others attributes or sheets in the "session" object, since they're being handled by seperate specialists.
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.
Yeah, but to my way of thinking, they *should* be coding them, because if they're not it means they haven't thought their design through enough to know what *kind* of session-specific object they're making. To me, there is no such thing as a session object, only session-specific objects. In any case, it's not a big deal to make a session-specific object. Add a ZClass, base it on DataSkin, set up a "common instance" property sheet, and boom, you're done. Now set your session manager's rack to use it, and you're in business.
participants (2)
-
mike -
Phillip J. Eby