[Zope3-checkins] SVN: Zope3/branches/srichter-twisted-integration/ - Fixed issue 390. Deprecated ``IBaseRequest.body`` and

Stephan Richter srichter at cosmos.phy.tufts.edu
Thu Apr 14 12:19:14 EDT 2005


Log message for revision 29982:
  - Fixed issue 390. Deprecated ``IBaseRequest.body`` and
    ``IBaseRequest.bodyFile``. The latter was simply renamed to
    ``IBaseRequest.bodyStream``. No code assumes anymore that the input
    streams are seekable.
  
  

Changed:
  U   Zope3/branches/srichter-twisted-integration/doc/CHANGES.txt
  U   Zope3/branches/srichter-twisted-integration/doc/TWISTED-TODO.txt
  U   Zope3/branches/srichter-twisted-integration/src/zope/app/dav/mkcol.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/app/dav/propfind.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/app/dav/proppatch.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/app/fssync/browser/__init__.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/app/http/put.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/publisher/base.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/publisher/http.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/publisher/interfaces/__init__.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/publisher/publish.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/basetestiapplicationrequest.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_baserequest.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_http.py
  U   Zope3/branches/srichter-twisted-integration/src/zope/server/http/tests/test_publisherserver.py

-=-
Modified: Zope3/branches/srichter-twisted-integration/doc/CHANGES.txt
===================================================================
--- Zope3/branches/srichter-twisted-integration/doc/CHANGES.txt	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/doc/CHANGES.txt	2005-04-14 16:19:14 UTC (rev 29982)
@@ -6,8 +6,26 @@
 
   For information on future releases, see ROADMAP.txt.
 
-  Some future release (Zope X3 3.1.0)
+  Some future release (Zope X3 3.2.0)
 
+    New Features
+
+    Bug Fixes
+
+      - Fixed issue 390. Deprecated ``IBaseRequest.body`` and
+        ``IBaseRequest.bodyFile``. The latter was simply renamed to
+        ``IBaseRequest.bodyStream``. No code assumes anymore that the input
+        streams are seekable.
+
+    Much thanks to everyone who contributed to this release:
+
+      Stephan Richter
+
+
+  ------------------------------------------------------------------
+
+  Zope X3.1.0
+
     New features
 
       - Implemented a generic user preferences system.

Modified: Zope3/branches/srichter-twisted-integration/doc/TWISTED-TODO.txt
===================================================================
--- Zope3/branches/srichter-twisted-integration/doc/TWISTED-TODO.txt	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/doc/TWISTED-TODO.txt	2005-04-14 16:19:14 UTC (rev 29982)
@@ -15,45 +15,3 @@
 
 * Display log on screen. In general, we need to do logging based on the info
   from zope.conf.
-
-* Now that we use twisted, the input stream is not seekable anymore:
-
-  2005-04-13T21:03:16 WARNING ZopePublication Competing writes/reads at /@@/zope3_tablelayout.css
-
-  Traceback (most recent call last):
-    File "/opt/zope/Zope3/Zope3-Twisted/src/zope/publisher/publish.py", line 143, in publish
-      publication.afterCall(request, object)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/zope/app/publication/browser.py", line 70, in afterCall
-      super(BrowserPublication, self).afterCall(request, ob)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/zope/app/publication/zopepublication.py", line 161, in afterCall
-      txn.commit()
-    File "/opt/zope/Zope3/Zope3-Twisted/src/transaction/_transaction.py", line 297, in commit
-      self._commitResources(subtransaction)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/transaction/_transaction.py", line 337, in _commitResources
-      rm.commit(self)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/ZODB/Connection.py", line 320, in commit
-      self._store_objects(ObjectWriter(obj), transaction)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/ZODB/Connection.py", line 343, in _store_objects
-      s = self._storage.store(oid, serial, p, self._version, transaction)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/ZODB/FileStorage/FileStorage.py", line 659, in store
-      raise POSException.ConflictError(
-  ConflictError: database conflict error (oid 0x13, class BTrees._OOBTree.OOBTree, serial this txn started with 0x035c94d66a7aa0aa 2005-04-14 00:22:24.956019, serial currently committed 0x035c94ff459c5411 2005-04-14 01:03:16.314998)
-  Traceback (most recent call last):
-    File "/opt/python2.3/lib/python2.3/threading.py", line 416, in run
-      self.__target(*self.__args, **self.__kwargs)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/twisted/python/threadpool.py", line 145, in _worker
-      context.call(ctx, function, *args, **kwargs)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/twisted/python/context.py", line 52, in callWithContext
-      return self.currentContext().callWithContext(ctx, func, *args, **kw)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/twisted/python/context.py", line 31, in callWithContext
-      return func(*args,**kw)
-  --- <exception caught here> ---
-    File "/opt/zope/Zope3/Zope3-Twisted/src/twisted/web2/wsgi.py", line 132, in run
-      result = self.application(self.environment, self.startWSGIResponse)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/zope/app/wsgi/__init__.py", line 106, in __call__
-      publish(request)
-    File "/opt/zope/Zope3/Zope3-Twisted/src/zope/publisher/publish.py", line 159, in publish
-      newrequest = request.retry()
-    File "/opt/zope/Zope3/Zope3-Twisted/src/zope/publisher/http.py", line 370, in retry
-      self._body_instream.seek(0)
-  exceptions.AttributeError: 'InputStream' object has no attribute 'seek'

Modified: Zope3/branches/srichter-twisted-integration/src/zope/app/dav/mkcol.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/app/dav/mkcol.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/app/dav/mkcol.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -29,9 +29,7 @@
 
     def MKCOL(self):
         request = self.request
-        data = request.bodyFile
-        data.seek(0)
-        data = data.read()
+        data = request.bodyStream.read()
         if len(data):
             # We don't (yet) support a request body on MKCOL.
             request.response.setStatus(415)

Modified: Zope3/branches/srichter-twisted-integration/src/zope/app/dav/propfind.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/app/dav/propfind.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/app/dav/propfind.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -16,6 +16,7 @@
 __docformat__ = 'restructuredtext'
 
 from xml.dom import minidom
+from xml.parsers import expat
 from zope.schema import getFieldNamesInOrder, getFields
 from zope.app import zapi
 from zope.app.container.interfaces import IReadContainer
@@ -52,13 +53,16 @@
                 _avail_props[ns] = list(oprops.keys())
         self.avail_props = _avail_props
 
+        # The xmldoc attribute will be set later, if needed.
+        self.xmldoc = None
+
     def getDepth(self):
         return self._depth
 
     def setDepth(self, depth):
         self._depth = depth.lower()
 
-    def PROPFIND(self):
+    def PROPFIND(self, xmldoc=None):
         if self.content_type not in ['text/xml', 'application/xml']:
             self.request.response.setStatus(400)
             return ''
@@ -70,11 +74,14 @@
         if IReadContainer.providedBy(self.context):
             resource_url += '/'
 
-        self.request.bodyFile.seek(0, 2)
-        size = self.request.bodyFile.tell()
-        self.request.bodyFile.seek(0)
+        if xmldoc is None:
+            try:
+                xmldoc = minidom.parse(self.request.bodyStream)
+            except expat.ExpatError:
+                pass
 
-        xmldoc = size and minidom.parse(self.request.bodyFile) or None
+        self.xmldoc = xmldoc
+            
         resp = minidom.Document()
         ms = resp.createElement('multistatus')
         ms.setAttribute('xmlns', self.default_ns)
@@ -84,7 +91,8 @@
         ms.lastChild.lastChild.appendChild(resp.createTextNode(resource_url))
 
         if xmldoc is not None:
-            propname = xmldoc.getElementsByTagNameNS(self.default_ns, 'propname')
+            propname = xmldoc.getElementsByTagNameNS(
+                self.default_ns, 'propname')
             if propname:
                 self._handlePropname(resp)
             else:
@@ -111,7 +119,7 @@
             if pfind is None:
                 continue
             pfind.setDepth(subdepth)
-            value = pfind.PROPFIND()
+            value = pfind.PROPFIND(self.xmldoc)
             parsed = minidom.parseString(value)
             responses = parsed.getElementsByTagNameNS(
                 self.default_ns, 'response')

Modified: Zope3/branches/srichter-twisted-integration/src/zope/app/dav/proppatch.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/app/dav/proppatch.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/app/dav/proppatch.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -63,8 +63,7 @@
         if IReadContainer.providedBy(self.context):
             resource_url += '/'
 
-        self.request.bodyFile.seek(0)
-        xmldoc = minidom.parse(self.request.bodyFile)
+        xmldoc = minidom.parse(self.request.bodyStream)
         resp = minidom.Document()
         ms = resp.createElement('multistatus')
         ms.setAttribute('xmlns', self.default_ns)

Modified: Zope3/branches/srichter-twisted-integration/src/zope/app/fssync/browser/__init__.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/app/fssync/browser/__init__.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/app/fssync/browser/__init__.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -127,9 +127,8 @@
             shutil.rmtree(self.tempdir)
 
     def unsnarf_body(self):
-        fp = self.request.bodyFile
-        fp.seek(0)
-        uns = Unsnarfer(fp)
+        stream = self.request.bodyStream
+        uns = Unsnarfer(stream)
         uns.unsnarf(self.tempdir)
 
     def call_committer(self):

Modified: Zope3/branches/srichter-twisted-integration/src/zope/app/http/put.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/app/http/put.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/app/http/put.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -53,7 +53,7 @@
                 request.response.setStatus(501)
                 return ''
 
-        body = request.bodyFile
+        body = request.bodyStream
         name = self.context.name
         container = self.context.container
 
@@ -102,7 +102,7 @@
                 request.response.setStatus(501)
                 return ''
 
-        body = self.request.bodyFile
+        body = self.request.bodyStream
         file = self.context
         adapter = IWriteFile(file)
 

Modified: Zope3/branches/srichter-twisted-integration/src/zope/publisher/base.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/publisher/base.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/publisher/base.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -21,6 +21,8 @@
 import traceback
 from cStringIO import StringIO
 
+from zope.deprecation import deprecated
+
 from zope.interface import implements, providedBy
 from zope.interface.common.mapping import IReadMapping, IEnumerableMapping
 from zope.publisher.interfaces import NotFound
@@ -303,28 +305,43 @@
     def setTraversalStack(self, stack):
         'See IPublicationRequest'
         self._traversal_stack[:] = list(stack)
+    
 
+    def _getBodyStream(self):
+        'See zope.publisher.interfaces.IApplicationRequest'
+        return self._body_instream
+
+    bodyStream = property(_getBodyStream)
+
+    ########################################################################
+    # BBB: Deprecated; will go away in Zope 3.4
+    
     def _getBody(self):
         body = getattr(self, '_body', None)
         if body is None:
             s = self._body_instream
             if s is None:
                 return None # TODO: what should be returned here?
-            p = s.tell()
-            s.seek(0)
             body = s.read()
-            s.seek(p)
             self._body = body
         return body
 
     body = property(_getBody)
+    body = deprecated(body,
+                      'The ``body`` attribute has been deprecated. Please '
+                      'use the ``bodyStream`` attribute directly. This '
+                      'attribute will go away in Zope 3.4.')
+    
+    bodyFile = bodyStream
+    bodyFile = deprecated(bodyFile,
+                          'The ``bodyFile`` attribute has been replaced by '
+                          '``bodyStream``, which is a more accurate name. '
+                          'Streams are not necessarily files, i.e. they are '
+                          'not seekable. This attribute will go away in Zope '
+                          '3.4.')
 
-    def _getBodyFile(self):
-        'See IApplicationRequest'
-        return self._body_instream
+    ########################################################################
 
-    bodyFile = property(_getBodyFile)
-
     def __len__(self):
         'See Interface.Common.Mapping.IEnumerableMapping'
         return len(self.keys())

Modified: Zope3/branches/srichter-twisted-integration/src/zope/publisher/http.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/publisher/http.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/publisher/http.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -16,6 +16,7 @@
 $Id$
 """
 import re, time, random
+import cStringIO
 from urllib import quote, unquote, splitport
 from types import StringTypes, ClassType
 from cgi import escape
@@ -166,6 +167,37 @@
                 return default
             raise
 
+class HTTPInputStream(object):
+    """Special stream that supports caching the read data.
+
+    This is important, so that we can retry requests.
+    """
+
+    def __init__(self, stream):
+        self.stream = stream
+        self.cacheStream = cStringIO.StringIO()
+
+    def getCacheStream(self):
+        self.read()
+        self.cacheStream.seek(0)
+        return self.cacheStream
+
+    def read(self, size=-1):
+        data = self.stream.read(size)
+        self.cacheStream.write(data)
+        return data
+
+    def readline(self):
+        data = self.stream.readline()
+        self.cacheStream.write(data)
+        return data
+
+    def readlines(self, hint=None):
+        data = self.stream.readlines(hint)
+        self.cacheStream.write(''.join(data))
+        return data
+        
+
 DEFAULT_PORTS = {'http': '80', 'https': '443'}
 STAGGER_RETRIES = True
 
@@ -235,7 +267,7 @@
 
     def __init__(self, body_instream, outstream, environ, response=None):
         super(HTTPRequest, self).__init__(
-            body_instream, outstream, environ, response)
+            HTTPInputStream(body_instream), outstream, environ, response)
 
         self._orig_env = environ
         environ = sane_environment(environ)
@@ -367,10 +399,11 @@
         'See IPublisherRequest'
         count = getattr(self, '_retry_count', 0)
         self._retry_count = count + 1
-        self._body_instream.seek(0)
+
         new_response = self.response.retry()
         request = self.__class__(
-            body_instream=self._body_instream,
+            # Use the cache stream as the new input stream.
+            body_instream=self._body_instream.getCacheStream(),
             outstream=None,
             environ=self._orig_env,
             response=new_response,

Modified: Zope3/branches/srichter-twisted-integration/src/zope/publisher/interfaces/__init__.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/publisher/interfaces/__init__.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/publisher/interfaces/__init__.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -403,10 +403,21 @@
                              This is a read-only attribute.
                           """)
 
-    body = Attribute("""The body of the request as a string""")
+    bodyStream = Attribute(
+        """The stream that provides the data of the request.
 
-    bodyFile = Attribute("""The body of the request as a file""")
+        The data returned by the stream will not include any possible header
+        information, which should have been stripped by the server (or
+        previous layer) before.
 
+        Also, the body stream might already be read and not return any
+        data. This is commonly done when retrieving the data for the ``body``
+        attribute.
+
+        If you access this stream directly to retrieve data, it will not be
+        possible by other parts of the framework to access the data of the
+        request via the ``body`` attribute.""")
+
     debug = Attribute("""Debug flags (see IDebugFlags).""")
 
     def __getitem__(key):

Modified: Zope3/branches/srichter-twisted-integration/src/zope/publisher/publish.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/publisher/publish.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/publisher/publish.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -169,7 +169,8 @@
                     else:
                         raise
 
-            except:
+            except Error, e:
+                import pdb; pdb.set_trace()
                 # Bad exception handler or retry method.
                 # Re-raise after outputting the response.
                 if handle_errors:

Modified: Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/basetestiapplicationrequest.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/basetestiapplicationrequest.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/basetestiapplicationrequest.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -29,7 +29,7 @@
 
     def testHaveCustomTestsForIApplicationRequest(self):
         # Make sure that tests are defined for things we can't test here
-        self.test_IApplicationRequest_body
+        self.test_IApplicationRequest_bodyStream
 
     def testEnvironment(self):
         request = self._Test__new(foo='Foo', bar='Bar')

Modified: Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_baserequest.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_baserequest.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_baserequest.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -40,15 +40,12 @@
     def _Test__expectedViewType(self):
         return None # we don't expect
 
-    def test_IApplicationRequest_body(self):
+    def test_IApplicationRequest_bodyStream(self):
         from zope.publisher.base import BaseRequest
 
         request = BaseRequest(StringIO('spam'), StringIO(), {})
-        self.assertEqual(request.body, 'spam')
+        self.assertEqual(request.bodyStream.read(), 'spam')
 
-        request = BaseRequest(StringIO('spam'), StringIO(), {})
-        self.assertEqual(request.bodyFile.read(), 'spam')
-
     def test_IPublicationRequest_getPositionalArguments(self):
         self.assertEqual(self._Test__new().getPositionalArguments(), ())
 

Modified: Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_http.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_http.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/publisher/tests/test_http.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -20,7 +20,7 @@
 
 from zope.interface import implements
 from zope.publisher.interfaces.logginginfo import ILoggingInfo
-from zope.publisher.http import HTTPRequest, HTTPResponse
+from zope.publisher.http import HTTPRequest, HTTPResponse, HTTPInputStream
 from zope.publisher.publish import publish
 from zope.publisher.base import DefaultPublication
 from zope.publisher.interfaces.http import IHTTPRequest, IHTTPResponse
@@ -48,6 +48,49 @@
         return self._id
 
 
+data = '''\
+line 1
+line 2
+line 3'''
+    
+    
+
+class HTTPInputStreamTests(unittest.TestCase):
+
+    def setUp(self):
+        self.stream = HTTPInputStream(StringIO(data))
+
+    def testRead(self):
+        output = ''
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        output += self.stream.read(5)
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        output += self.stream.read()
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        self.assertEqual(data, self.stream.cacheStream.getvalue())
+
+    def testReadLine(self):
+        output = self.stream.readline()
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        output += self.stream.readline()
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        output += self.stream.readline()
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        output += self.stream.readline()
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        self.assertEqual(data, self.stream.cacheStream.getvalue())
+
+    def testReadLines(self):
+        output = ''.join(self.stream.readlines(4))
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        output += ''.join(self.stream.readlines())
+        self.assertEqual(output, self.stream.cacheStream.getvalue())
+        self.assertEqual(data, self.stream.cacheStream.getvalue())
+
+    def testGetChacheStream(self):
+        self.stream.read(5)
+        self.assertEqual(data, self.stream.getCacheStream().getvalue())        
+
 class HTTPTests(unittest.TestCase):
 
     _testEnv =  {
@@ -602,6 +645,7 @@
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(ConcreteHTTPTests))
     suite.addTest(unittest.makeSuite(TestHTTPResponse))
+    suite.addTest(unittest.makeSuite(HTTPInputStreamTests))
     return suite
 
 

Modified: Zope3/branches/srichter-twisted-integration/src/zope/server/http/tests/test_publisherserver.py
===================================================================
--- Zope3/branches/srichter-twisted-integration/src/zope/server/http/tests/test_publisherserver.py	2005-04-14 14:06:47 UTC (rev 29981)
+++ Zope3/branches/srichter-twisted-integration/src/zope/server/http/tests/test_publisherserver.py	2005-04-14 16:19:14 UTC (rev 29982)
@@ -138,6 +138,7 @@
         h.endheaders()
         if request_body:
             h.send(request_body)
+        #import pdb; pdb.set_trace()
         response = h.getresponse()
         length = int(response.getheader('Content-Length', '0'))
         if length:



More information about the Zope3-Checkins mailing list