Alternative Session Implementation

[Carsten Senger-3]

Hi all,

I had been joining the GetPaid sprint at the Plone conference in Budapest
for one day and we shortly discussed the session implementation. If I see
it correctly the problems of ticket 209 are resolved (except of broken
ZODBs maybe).
We could not come up with a completely sessionless cart implementation, I
think mainly because nobody knew the GetPaid design well enough. Anyway I
have implemented the session based approach using zope.app.session. It
consists of an ISession utility and an ISessionData local persistent
utility to store the data. This means that sessions (in our case those of
anonymous users) are written to the ZODB the portal is saved in. This can
give you performance problems, depending on the number of anonymous
shopping cart users. I don't know if there are bigger GetPaid installations.
But it should be possible replace the local persistent utility with another
implementation that does the loads/dumps dance internaly and saves the
data somewhere else.

Attached you find a patch converting the implementation and the tests to
use zope.app.session. The functional anon cart tests are failing atm, but I
did not have time to look at them nor do I know if they failed before
already.

..Carsten

--~--~---------~--~----~------------~-------~--~----~
GetPaid for Plone: http://www.plonegetpaid.com (overview info) | http://code.google.com/p/getpaid (code and issue tracker)
You received this message because you are subscribed to the Google Groups "getpaid-dev" group.
To post to this group, send email to getpaid-dev@...
To unsubscribe from this group, send email to getpaid-dev+unsubscribe@...

For more options, visit this group at
http://groups.google.com/group/getpaid-dev?hl=en?hl=en
-~----------~----~----~----~------~----~------~--~---

---

Index: Products/PloneGetPaid/configure.zcml
===================================================================
--- Products/PloneGetPaid/configure.zcml (Revision 3169)
+++ Products/PloneGetPaid/configure.zcml (Arbeitskopie)
@@ -222,11 +222,58 @@
    />
 
 
+<!-- zope.app.session for shopping carts -->
 
-<subscriber
-    for="*
-         getpaid.core.interfaces.IPayableCreationEvent"
-    handler=".events.payable_created"
-    />
-
+
+<adapter factory=".cart.ClientId"
+         permission="zope.Public" />    
+
+<configure package="zope.app.session">
+    
+  <adapter
+      factory=".session.Session"
+      provides="zope.app.session.interfaces.ISession"
+      permission="zope.Public"
+      />
+  
+  <adapter
+      factory=".session.Session"
+      provides="zope.traversing.interfaces.IPathAdapter"
+      name="session"
+      />
+  
+  <class class=".session.Session">
+    <allow interface=".interfaces.ISession" />
+    <implements interface="zope.traversing.interfaces.IPathAdapter" />
+  </class>
+  
+  
+  <class class=".session.PersistentSessionDataContainer">
+    <require
+        interface=".interfaces.ISessionDataContainer"
+        permission="zope.Public" />
+    <!--     <require -->
+    <!--         set_schema=".interfaces.ISessionDataContainer" -->
+    <!--         permission="zope.ManageServices" /> -->
+  </class>
+
+
+  <class class=".session.SessionData">
+    <allow interface=".interfaces.ISessionData" />
+  </class>
+  
+  <class class=".session.SessionPkgData">
+    <allow interface=".interfaces.ISessionPkgData" />
+  </class>
+  
+  <subscriber
+      for="zope.app.appsetup.IDatabaseOpenedEvent"
+      handler=".bootstrap.bootStrapSubscriber"
+      />
+  
+  <subscriber
+      for="zope.publisher.interfaces.http.IHTTPVirtualHostChangedEvent"
+      handler=".http.notifyVirtualHostChanged"
+      />
 </configure>
+</configure>
\ No newline at end of file
Index: Products/PloneGetPaid/config.py
===================================================================
--- Products/PloneGetPaid/config.py (Revision 3169)
+++ Products/PloneGetPaid/config.py (Arbeitskopie)
@@ -4,3 +4,6 @@
     PLONE3 = True
 except ImportError:
     PLONE3 = False
+
+SESSION_KEY = 'PloneGetPaid'
+CART_KEY = 'getpaid.cart'
Index: Products/PloneGetPaid/tests/test_cart.py
===================================================================
--- Products/PloneGetPaid/tests/test_cart.py (Revision 3169)
+++ Products/PloneGetPaid/tests/test_cart.py (Arbeitskopie)
@@ -5,6 +5,7 @@
 
 from zope.component import getUtility
 from Testing.ZopeTestCase import ZopeDocTestSuite
+from Testing.ZopeTestCase.utils import setupCoreSessions
 from Products.Five.utilities.marker import mark
 
 from utils import optionflags
@@ -13,8 +14,24 @@
 from Products.PloneGetPaid import interfaces
 from getpaid.core.interfaces import IShoppingCartUtility
 
+
+class DummySessionAndBrowserIdManager:
+
+    __test_browser_id__ = 'funkyTestBrowserId'
+    
+    def getBrowserIdManager(self):
+        return self
+
+    def getBrowserId(self):
+        return self.__test_browser_id__
+
+
 class TestCart(PloneGetPaidTestCase):
 
+    def afterSetUp(self):
+        super(TestCart, self).afterSetUp()
+        setattr(self.app.REQUEST, 'SESSION', DummySessionAndBrowserIdManager())
+
     def mySetup(self):
         self.setRoles(('Manager',))
         id = self.portal.invokeFactory('Document', 'doc')
@@ -161,18 +178,18 @@
           ...
         ValueError: Malformed key: example:malformed:key
         """
-
+        
     def pauseBrowserSession(self):
         """Utility that disables the current session. The session can
         be resumed using resumeBrowserSession.
         """
-        self._paused_browser_id = self.portal.REQUEST.browser_id_
-        self.portal.REQUEST.browser_id_ = None
+        self._paused_browser_id = self.portal.REQUEST.SESSION.__test_browser_id__
+        self.portal.REQUEST.SESSION.__test_browser_id__ = None
 
     def resumeBrowserSession(self):
         """Utility to restore the previous session.
         """
-        self.portal.REQUEST.browser_id_ = self._paused_browser_id
+        self.portal.REQUEST.SESSION.__test_browser_id__ = self._paused_browser_id
 
 
 def test_suite():
Index: Products/PloneGetPaid/setuphandlers.py
===================================================================
--- Products/PloneGetPaid/setuphandlers.py (Revision 3169)
+++ Products/PloneGetPaid/setuphandlers.py (Arbeitskopie)
@@ -14,11 +14,11 @@
 from Products.PloneGetPaid.Extensions.install import install_plone3_portlets
 from Products.PloneGetPaid.Extensions.install import setup_payment_options
 from Products.PloneGetPaid.Extensions.install import register_shopping_cart_utility
-
 from Products.PloneGetPaid.Extensions.install import setup_addressbook
 from Products.PloneGetPaid.Extensions.install import setup_named_orders
 from Products.PloneGetPaid.Extensions.install import setup_settings
 from Products.PloneGetPaid.Extensions.install import register_shopping_cart_utility
+from Products.PloneGetPaid.Extensions.install import register_session_data_utility
 
 from Products.PloneGetPaid.config import PLONE3
 
@@ -82,6 +82,9 @@
     print >> out, "Registering shopping cart utility"
     register_shopping_cart_utility(site)
     
+    print >> out, "Registering session data utility"
+    register_session_data_utility(site)
+    
     print >> out, "Notifying Installation"
     notify_install( site )
     
Index: Products/PloneGetPaid/cart.py
===================================================================
--- Products/PloneGetPaid/cart.py (Revision 3169)
+++ Products/PloneGetPaid/cart.py (Arbeitskopie)
@@ -3,16 +3,21 @@
 
 $Id$
 """
-from zope.component import getUtility
+from zope.app.session.interfaces import IClientId
+from zope.app.session.interfaces import ISession
+from zope.component import adapts, getUtility
 from zope.interface import implements
+from zope.publisher.interfaces import IRequest
 
 from getpaid.core.cart import ShoppingCart
 from getpaid.core.interfaces import IShoppingCartUtility
 from Products.CMFCore.utils import getToolByName
+from Products.PloneGetPaid.config import CART_KEY, SESSION_KEY
 from persistent import Persistent
 from BTrees.OOBTree import OOBTree
 from AccessControl import getSecurityManager
 
+
 class ShoppingCartUtility(Persistent):
 
     implements(IShoppingCartUtility)
@@ -73,40 +78,30 @@
         self._sessions[uid] = cart
         return cart
 
-
     def _getCartForSession(self, context, create=False, browser_id=None):
-        session_manager = getToolByName(context, 'session_data_manager')
-        if browser_id is None:
-            if not session_manager.hasSessionData() and not create:
-                return
-            session = session_manager.getSessionData()
-        else:
-            session = session_manager.getSessionDataByKey(browser_id)
-            if session is None:
-                return
-        if not session.has_key('getpaid.cart'):
-            if create:
-                session['getpaid.cart'] = cart = ShoppingCart()
-            else:
-                return None
-        return session['getpaid.cart']
+        session = ISession(context.REQUEST)
 
+        if browser_id is not None:
+            # we overwrite the client_id that was set during the ISession
+            # initialization
+            session.client_id = browser_id
+
+        sessiondata = session[SESSION_KEY]
+        cart = sessiondata.get(CART_KEY, None)
+        if create and cart is None:
+            sessiondata[CART_KEY] = cart = ShoppingCart()
+        return cart
+
     def _getDisposableCart(self, context, browser_id=None):
         return ShoppingCart()
         
     def _getMultiShotCart(self, context, cart_id=None):
-        session_manager = getToolByName(context, 'session_data_manager')
+        sessiondata = ISession(context.REQUEST)[SESSION_KEY]
+        key = "%s.%s" % (CART_KEY, cart_id)
+        if not sessiondata.has_key(key):
+            sessiondata[key] = ShoppingCart()
+        return sessiondata[key]
 
-        key = "getpaid.cart.%s" % cart_id
-
-        session = session_manager.getSessionData()
-
-        if not session.has_key(key):
-            session[key] = cart = ShoppingCart()
-
-        return session[key]
-
-
     def destroy(self, context, key=None):
         """ Destroy the cart.
         """
@@ -125,37 +120,26 @@
             else:
                 return self._destroyCartForSession(context)
 
-
     def _destroyCartForUser(self, context, uid):
         if self._sessions.has_key(uid):
             del self._sessions[uid]
 
-
     def _destroyCartForSession(self, context, browser_id=None):
-        session_manager = getToolByName(context, 'session_data_manager')
-        if browser_id is None:
-            if not session_manager.hasSessionData(): #nothing to destroy
-                return None
-            session = session_manager.getSessionData()
-        else:
-            session = session_manager.getSessionDataByKey(browser_id)
-            if session is None:
-                return
-        if not session.has_key('getpaid.cart'):
-            return
-        del session['getpaid.cart']
+        session = ISession(context.REQUEST)
+        if browser_id is not None:
+            # overwrite the client_id that was set
+            # during initialization
+            session.client_id = browser_id
+        sessiondata = session[SESSION_KEY]
+        if sessiondata.has_key(CART_KEY):
+            del sessiondata[CART_KEY]
 
     def _destroyMultiShotCart(self, context, cart_id=None):
-        session_manager = getToolByName(context, 'session_data_manager')
+        sessiondata = ISession(context.REQUEST)[SESSION_KEY]
+        key = "%s.%s" % (CART_KEY, cart_id)
+        if sessiondata.has_key(key):
+            del session[key]
 
-        key = "getpaid.cart.%s" % cart_id
-        session = session_manager.getSessionData()
-
-        if not session.has_key(key):
-            return
-
-        del session[key]
-
     def getKey(self, context):
         """Return key that can be used to recover the cart for the
         current user or session.
@@ -164,15 +148,12 @@
         if uid is not None:
             return 'user:%s' % uid
         else:
-            session_manager = getToolByName(context, 'session_data_manager')
-            if not session_manager.hasSessionData():
+            session = ISession(context.REQUEST)
+            sessiondata = session[SESSION_KEY]
+            if not sessiondata.has_key(CART_KEY):
                 return None
-            session = session_manager.getSessionData()
-            if not session.has_key('getpaid.cart'):
-                return None
-            return 'session:%s' % session.token
+            return 'session:%s' % session.client_id
 
-
     def _decodeKey(self, key):
         try:
             name, value = key.split(':', 1)
@@ -185,3 +166,19 @@
 
     def manage_fixupOwnershipAfterAdd(self):
         pass
+
+
+class ClientId(str):
+    """See zope.app.interfaces.utilities.session.IClientId
+
+    An IClientId Utility that uses Zope2's browser_id_manager
+    implementation.
+    """
+    implements(IClientId)
+    adapts(IRequest)
+    
+    def __new__(cls, request):
+        return str.__new__(
+            cls, request.SESSION.getBrowserIdManager().getBrowserId()
+            )
+    
Index: Products/PloneGetPaid/Extensions/install.py
===================================================================
--- Products/PloneGetPaid/Extensions/install.py (Revision 3169)
+++ Products/PloneGetPaid/Extensions/install.py (Arbeitskopie)
@@ -12,6 +12,9 @@
 from zope.event import notify
 from zope.app.component.hooks import setSite
 from zope.app.component.interfaces import ISite
+from zope.app.session.interfaces import ISessionDataContainer
+from zope.app.session.session import PersistentSessionDataContainer
+
 from Products.PloneGetPaid import generations, preferences, addressbook, namedorder
 from Products.PloneGetPaid.interfaces import IGetPaidManagementOptions, IAddressBookUtility, INamedOrderUtility
 from Products.PloneGetPaid.config import PLONE3
@@ -61,6 +64,21 @@
             # BBB for Zope 2.9
             sm.registerUtility(interface=IOrderManager, utility=manager)
 
+def register_session_data_utility( self ):
+    portal = getToolByName( self, 'portal_url').getPortalObject()
+    sm = portal.getSiteManager()
+    is_already_registered = [u for u in sm.getUtilitiesFor(ISessionDataContainer)]
+    if not len(is_already_registered):
+        session_data = PersistentSessionDataContainer()
+        session_data.timeout = 604800 # 7 days
+        session_data.resolution = 100
+        sm.registerUtility(session_data, ISessionDataContainer)
+        try:
+            sm.registerUtility(component=session_data, provided=ISessionDataContainer)
+        except TypeError:
+            # BBB for Zope 2.9
+            sm.registerUtility(interface=ISessionDataContainer, utility=session_data)
+
 def setup_intid( self ):
     portal = getToolByName(self, 'portal_url').getPortalObject()
     try:

---

[Matt Halstead-3]

Hi Carsten,

On Nov 4, 12:22 pm, Carsten Senger <sen...@...> wrote:
> Hi all,
>
> I had been joining the GetPaid sprint at the Plone conference in Budapest
> for one day and we shortly discussed the session implementation. If I see
> it correctly the problems of ticket 209 are resolved (except of broken
> ZODBs maybe).

I just committed the patch the michael dunstan provided - it was
previously not committed, that does seem to resolve the source of the
issue for non-broken _session storages.

> We could not come up with a completely sessionless cart implementation, I
> think mainly because nobody knew the GetPaid design well enough. Anyway I
> have implemented the session based approach using zope.app.session. It
> consists of an ISession utility and an ISessionData local persistent
> utility to store the data. This means that sessions (in our case those of
> anonymous users) are written to the ZODB the portal is saved in. This can
> give you performance problems, depending on the number of anonymous
> shopping cart users. I don't know if there are bigger GetPaid installations.
> But it should be possible replace the local persistent utility with another
> implementation that does the loads/dumps dance internaly and saves the
> data somewhere else.

Can you elaborate on why session storage needed to be reimplemented?

>
> Attached you find a patch converting the implementation and the tests to
> use zope.app.session. The functional anon cart tests are failing atm, but I
> did not have time to look at them nor do I know if they failed before
> already.

It would be great if you could create a branch with your changes and
fix the tests to work.

>
> ..Carsten
>
>  Products.PloneGetPaid.session.patch
> 13KViewDownload

cheers
Matt

--
GetPaid for Plone: http://www.plonegetpaid.com (overview info) | http://code.google.com/p/getpaid (code and issue tracker)
You received this message because you are subscribed to the Google Groups "getpaid-dev" group.
To post to this group, send email to getpaid-dev@...
To unsubscribe from this group, send email to getpaid-dev+unsubscribe@...

For more options, visit this group at
http://groups.google.com/group/getpaid-dev?hl=en?hl=en

[silviot]

On Fri, Nov 13, 2009 at 8:24 AM, Matt Halstead <matt.halstead@...> wrote:

> But it should be possible replace the local persistent utility with another
> implementation that does the loads/dumps dance internaly and saves the
> data somewhere else.

Can you elaborate on why session storage needed to be reimplemented?

I can help about this. I had a conversation with Carsten and other people at the sprint in Budapest. My point about issue 209 is that, even if it is now resolved, it was initially caused by a bad design decision: storing carts in a non persistent database. This decision makes anonymous customers lose their carts when Zope (or zeo in a zeo cluster) is restarted; site owners never want customers to lose their carts. And the storage of carts in the temporary zodb requires the loads/dumps dance that seems to be very error-prone and leads to hard-to-trace bugs. 

          Silvio

--
GetPaid for Plone: http://www.plonegetpaid.com (overview info) | http://code.google.com/p/getpaid (code and issue tracker)
You received this message because you are subscribed to the Google Groups "getpaid-dev" group.
To post to this group, send email to getpaid-dev@...
To unsubscribe from this group, send email to getpaid-dev+unsubscribe@...
 
For more options, visit this group at
http://groups.google.com/group/getpaid-dev?hl=en?hl=en

[Michael Dunstan]

On Fri, Nov 13, 2009 at 9:45 PM, Silvio <silviot@...> wrote:
> And the storage of carts in the
> temporary zodb requires the loads/dumps dance that seems to be very
> error-prone and leads to hard-to-trace bugs.

FWIW - independent of the functional use case in discussion here -
ZODB 3.9 has an option which can be used to prevent cross database
references from being created in the first place.
allow-implicit-cross-references. So that these kinds of bugs become
trivial to trace.

Here is patch that enables the same functionality for ZODB 3.8
http://code.google.com/p/getpaid/issues/detail?id=209#c52

--
Michael Dunstan

--
GetPaid for Plone: http://www.plonegetpaid.com (overview info) | http://code.google.com/p/getpaid (code and issue tracker)
You received this message because you are subscribed to the Google Groups "getpaid-dev" group.
To post to this group, send email to getpaid-dev@...
To unsubscribe from this group, send email to getpaid-dev+unsubscribe@...

For more options, visit this group at
http://groups.google.com/group/getpaid-dev?hl=en?hl=en

[Matt Halstead-3]

On Nov 13, 9:45 pm, Silvio <silv...@...> wrote:

> On Fri, Nov 13, 2009 at 8:24 AM, Matt Halstead <matt.halst...@...>wrote:
>
>
>
> >  > But it should be possible replace the local persistent utility with
> > another
> > > implementation that does the loads/dumps dance internaly and saves the
> > > data somewhere else.
>
> > Can you elaborate on why session storage needed to be reimplemented?
>
> I can help about this. I had a conversation with Carsten and other people at
> the sprint in Budapest. My point about issue 209 is that, even if it is now
> resolved, it was initially caused by a bad design decision: storing carts in
> a non persistent database. This decision makes anonymous customers lose
> their carts when Zope (or zeo in a zeo cluster) is restarted; site owners
> never want customers to lose their carts.

I'd agree with that. They will lose their carts after the session time
out, but at least that's a constant. I have a couple of questions
about the proposed implementation:

should the session data container be configured with an explicit
pkg_id so that we can control its settings just for the shop and not
any other use of sessions?

should timeout and resolution be configurable through the web?

we should also create an upgrade step that:

1) calls register_session_data_utility( self ) from the Install.py so
people don't have to do a full product reinstall to update the session
management

2) identifies unusable carts in the _sessions persistent storage and
removes them since these will still cause problems.

If this is the direction people want to head, I'd be quite keen to see
this in action in the next few weeks. I don't mind making a test
branch if you or Carsten don't have time.

> And the storage of carts in the
> temporary zodb requires the loads/dumps dance that seems to be very
> error-prone and leads to hard-to-trace bugs.

>
>           Silvio

--
GetPaid for Plone: http://www.plonegetpaid.com (overview info) | http://code.google.com/p/getpaid (code and issue tracker)
You received this message because you are subscribed to the Google Groups "getpaid-dev" group.
To post to this group, send email to getpaid-dev@...
To unsubscribe from this group, send email to getpaid-dev+unsubscribe@...

For more options, visit this group at
http://groups.google.com/group/getpaid-dev?hl=en?hl=en