Commits

Olemis Lang  committed c86400f

TracRpc: API v2: Arg `rpcreq` phase out in favour of `req.rpc`

  • Participants
  • Parent commits b955297

Comments (0)

Files changed (1)

File t5437/t5437-protocol_api_v2-r7194.diff

 
 diff -r 369fe39e568b trunk/setup.py
 --- a/trunk/setup.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/setup.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/setup.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -28,7 +28,7 @@
      url='http://trac-hacks.org/wiki/XmlRpcPlugin',
      description='RPC interface to Trac',
      package_data={
 diff -r 369fe39e568b trunk/tracrpc/api.py
 --- a/trunk/tracrpc/api.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/api.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/api.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -21,13 +21,34 @@
      """ RPC Binary type. Currently == xmlrpclib.Binary. """
      pass
  
  RPC_TYPES = {int: 'int', bool: 'boolean', str: 'string', float: 'double',
               datetime: 'dateTime.iso8601', Binary: 'base64',
-@@ -63,8 +84,57 @@
+@@ -63,8 +84,58 @@
                     (/login)?/<path_item>. Answer to 'rpc' only if possible.
          content_type: Starts-with check of 'Content-Type' request header. """
  
 +        values included in this mapping will be ignored by the core 
 +        RPC subsystem, will be protocol-specific, and SHOULD NOT be 
 +        needed in order to invoke a given method.
-+        (TODO: reuse `req` ?)
-+        
++
 +        method  (MANDATORY): target method name (e.g. 'ticket.get')
 +        params  (OPTIONAL) : a tuple containing input positional arguments
 +        headers (OPTIONAL) : if the protocol supports custom headers set 
 +                              specific values assigned to a particular 
 +                              header in order to send a response back 
 +                              to the client.
-+        mimetype           : request MIME-type. This value will be set 
-+                              by core RPC components after calling 
++        mimetype           : request MIME-type. This value will be set
++                              by core RPC components after calling
 +                              this method so, please, ignore
-+        
++
 +        If the request cannot be parsed this method *MUST* raise 
-+        an instance of `ProtocolException` wrapping another exception 
-+        containing details about the failure.
++        an instance of `ProtocolException` optionally wrapping another 
++        exception containing details about the failure.
 +        """
 +
-+    def send_rpc_result(req, rpcreq, result):
++    def send_rpc_result(req, result):
 +        """Serialize the result of the RPC call and send it back to 
 +        the client.
 +        
-+        rpcreq  : The same object returned by `parse_rpc_request` 
-+                  (see above).
++        req     : Request object. The same mapping returned by 
++                  `parse_rpc_request` can be accessed through 
++                  `req.rpc` (see above).
 +        result  : The value returned by the target RPC method
 +        """
 +
 +    def send_rpc_error(req, rpcreq, e):
 +        """Send a fault message back to the caller. Exception type 
-+        and message are used for this purpose. This message *SHOULD* 
-+        handle `RPCError`, `PermissionError`, and `ResourceNotFound` 
-+        and subclasses. This method is called from within an exception 
-+        handler.
++        and message are used for this purpose. This method *SHOULD* 
++        handle `RPCError`, `PermissionError`, `ResourceNotFound` and 
++        their subclasses. This method is *ALWAYS* called from within 
++        an exception handler.
 +        
-+        rpcreq  : The same object returned by `parse_rpc_request` 
-+                  (see above).
++        req     : Request object. The same mapping returned by 
++                  `parse_rpc_request` can be accessed through 
++                  `req.rpc` (see above).
 +        e       : exception object describing the failure
 +        """
  
  
 diff -r 369fe39e568b trunk/tracrpc/json_rpc.py
 --- a/trunk/tracrpc/json_rpc.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/json_rpc.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/json_rpc.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -5,8 +5,9 @@
  (c) 2009      ::: www.CodeResort.com - BV Network AS (simon-code@bvnetwork.no)
  """
  class JsonRpcProtocol(Component):
      r"""
      Example `POST` request using `curl` with `Content-Type` header
-@@ -126,34 +134,47 @@
+@@ -126,34 +134,48 @@
          # Legacy path - provided for backwards compatibility:
          yield ('jsonrpc', 'application/json')
  
 +                              exception_to_unicode(e, traceback=True))
 +            raise JsonProtocolException(e, -32700)
 +
-+    def send_rpc_result(self, req, rpcreq, result):
++    def send_rpc_result(self, req, result):
 +        """Send JSON-RPC response back to the caller."""
++        rpcreq = req.rpc
 +        r_id = rpcreq.get('id')
          try:
 -            req.perm.require('XML_RPC') # Need at least XML_RPC
              try: # JSON encoding
                  self.log.debug("RPC(json) result: %s" % repr(response))
                  response = json.dumps(response, cls=TracRpcJSONEncoder)
-@@ -165,28 +186,34 @@
+@@ -165,28 +187,35 @@
                                                      traceback=True))
              response = json.dumps(self._json_error(e, r_id=r_id),
                              cls=TracRpcJSONEncoder)
 -        self._send_response(req, response + '\n', content_type)
 +        self._send_response(req, response + '\n', rpcreq['mimetype'])
 +
-+    def send_rpc_error(self, req, rpcreq, e):
++    def send_rpc_error(self, req, e):
 +        """Send a JSON-RPC fault message back to the caller. """
 +        # TODO : Check error codes and RPC exceptions
++        rpcreq = req.rpc
 +        r_id = rpcreq.get('id')
 +        response = json.dumps(self._json_error(e, r_id=r_id), \
 +                                  cls=TracRpcJSONEncoder)
          """ Makes a response dictionary that is an error. """
 diff -r 369fe39e568b trunk/tracrpc/tests/__init__.py
 --- a/trunk/tracrpc/tests/__init__.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/tests/__init__.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/tests/__init__.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -35,6 +35,7 @@
              print "Enabling RPC plugin and permissions..."
              env.config.set('components', 'tracrpc.*', 'enabled')
              self._tracadmin('permission', 'add', 'anonymous', 'XML_RPC')
              print "Created test environment: %s" % self.dirname
              parts = urllib2.urlparse.urlsplit(self.url)
-@@ -66,22 +67,55 @@
+@@ -66,22 +67,56 @@
                  os.path.realpath(__file__), '..', '..', '..', 'rpctestenv')),
                  '8765', 'http://127.0.0.1')
  
 +    
 +    TracRpcTestCase = unittest.TestCase
 +else :
++    __unittest = 1          # Do not show this module in tracebacks
 +    class TracRpcTestCase(unittest.TestCase):
 +        def setUp(self):
 +            log = rpc_testenv.getLogger()
 +
 +        assertRaises = failUnlessRaises
 diff -r 369fe39e568b trunk/tracrpc/tests/api.py
---- a/trunk/tracrpc/tests/api.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/tests/api.py	Sat Mar 27 23:40:18 2010 -0500
-@@ -9,14 +9,14 @@
- import unittest
- import urllib2
- 
--from tracrpc.tests import rpc_testenv
-+from tracrpc.tests import rpc_testenv, TracRpcTestCase
- 
- from tracrpc.api import IRPCProtocol
- 
- from trac.core import *
- from trac.test import Mock
- 
--class ProtocolProviderTestCase(unittest.TestCase):
-+class ProtocolProviderTestCase(TracRpcTestCase):
- 
-     def test_invalid_content_type(self):
-         req = urllib2.Request(rpc_testenv.url_anon,
-@@ -47,16 +47,18 @@
-             "class DummyProvider(Component):\n"
-             "    implements(IRPCProtocol)\n"
-             "    def rpc_info(self):\n"
--            "        yield ('TEST-RPC', 'No Docs!')\n"
-+            "        return ('TEST-RPC', 'No Docs!')\n"
-             "    def rpc_match(self):\n"
-             "        yield ('rpc', 'application/x-tracrpc-test')\n"
--            "    def rpc_process(self, req, content_type):\n"
-+            "    def parse_rpc_request(self, req, content_type):\n"
-+            "        return {'method' : 'system.getAPIVersion'}\n"
-+            "    def send_rpc_error(self, req, rpcreq, e):\n"
-+            "        req.send_error(None, template='', content_type=rpcreq['mimetype'],\n"
-+            "                       status=500, env=None, data='Test failure ')\n"
-+            "    def send_rpc_result(self, req, rpcreq, result):\n"
-+            "        # raise KeyError('Here')\n"
-             "        response = 'Got a result!'\n"
--            "        req.send_response(200)\n"
--            "        req.send_header('Content-Type', content_type)\n"
--            "        req.send_header('Content-Length', len(response))\n"
--            "        req.end_headers()\n"
--            "        req.write(response)\n")
-+            "        req.send(response, rpcreq['mimetype'], 200)\n")
-         rpc_testenv.restart()
-         try:
-             req = urllib2.Request(rpc_testenv.url_anon,
-@@ -65,7 +67,7 @@
-             self.assertEquals(200, resp.code)
-             self.assertEquals("Got a result!", resp.read())
-             self.assertEquals(resp.headers['Content-Type'],
--                                                'application/x-tracrpc-test')
-+                                                'application/x-tracrpc-test;charset=utf-8')
-         finally:
-             # Clean up so that provider don't affect further tests
-             os.unlink(provider)
-@@ -80,10 +82,21 @@
-             "class DummyProvider(Component):\n"
-             "    implements(IRPCProtocol)\n"
-             "    def rpc_info(self):\n"
--            "        yield ('TEST-RPC', 'No Docs!')\n"
-+            "        return ('TEST-RPC', 'No Docs!')\n"
-             "    def rpc_match(self):\n"
-             "        yield ('rpc', 'application/x-tracrpc-test')\n"
--            "    def rpc_process(self, req, content_type):\n"
-+            "    def parse_rpc_request(self, req, content_type):\n"
-+            "        return {'method' : 'system.getAPIVersion'}\n"
-+            "    def send_rpc_error(self, req, rpcreq, e):\n"
-+            "        if isinstance(e, RPCError) :\n"
-+            "            req.send_error(None, template='', \n"
-+            "                       content_type='text/plain',\n"
-+            "                       status=500, env=None, data=e.message)\n"
-+            "        else :\n"
-+            "            req.send_error(None, template='', \n"
-+            "                       content_type='text/plain',\n"
-+            "                       status=500, env=None, data='Test failure')\n"
-+            "    def send_rpc_result(self, req, rpcreq, result):\n"
-             "        raise RPCError('No good.')")
-         rpc_testenv.restart()
-         # Make the request
-@@ -100,8 +113,8 @@
-             os.unlink(provider)
-             rpc_testenv.restart()
- 
--def suite():
-+def test_suite():
-     return unittest.makeSuite(ProtocolProviderTestCase)
- 
- if __name__ == '__main__':
--    unittest.main(defaultTest='suite')
-+    unittest.main(defaultTest='test_suite')
+Binary file trunk/tracrpc/tests/api.py has changed
 diff -r 369fe39e568b trunk/tracrpc/tests/json_rpc.py
 --- a/trunk/tracrpc/tests/json_rpc.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/tests/json_rpc.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/tests/json_rpc.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -14,10 +14,10 @@
  from tracrpc.util import StringIO
  
 +    unittest.main(defaultTest='test_suite')
 diff -r 369fe39e568b trunk/tracrpc/tests/ticket.py
 --- a/trunk/tracrpc/tests/ticket.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/tests/ticket.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/tests/ticket.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -12,9 +12,9 @@
  import shutil
  import time
 +    unittest.main(defaultTest='test_suite')
 diff -r 369fe39e568b trunk/tracrpc/tests/wiki.py
 --- a/trunk/tracrpc/tests/wiki.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/tests/wiki.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/tests/wiki.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -13,10 +13,10 @@
  
  from trac.util.compat import sorted
 +    unittest.main(defaultTest='test_suite')
 diff -r 369fe39e568b trunk/tracrpc/tests/xml_rpc.py
 --- a/trunk/tracrpc/tests/xml_rpc.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/tests/xml_rpc.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/tests/xml_rpc.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -9,9 +9,9 @@
  
  import xmlrpclib
      
      def setUp(self):
          self.anon = xmlrpclib.ServerProxy(rpc_testenv.url_anon)
-@@ -81,7 +81,7 @@
+@@ -22,44 +22,45 @@
+         # Test returned response if not XML_RPC permission
+         rpc_testenv._tracadmin('permission', 'remove', 'anonymous',
+                                 'XML_RPC')
+-        try:
++        def local_test():
+             self.anon.system.listMethods()
+             rpc_testenv._tracadmin('permission', 'add', 'anonymous',
+                                         'XML_RPC')
+             self.fail("Revoked permissions not taken effect???")
+-        except xmlrpclib.Fault, e:
+-            self.assertEquals(403, e.faultCode)
+-            self.assertTrue('XML_RPC' in e.faultString)
+-            rpc_testenv._tracadmin('permission', 'add', 'anonymous',
++
++        e = self.assertRaises(xmlrpclib.Fault, local_test)
++        self.assertEquals(403, e.faultCode)
++        self.assertTrue('XML_RPC' in e.faultString)
++        rpc_testenv._tracadmin('permission', 'add', 'anonymous',
+                                         'XML_RPC')
+ 
+     def test_method_not_found(self):
+-        try:
++        def local_test():
+             self.admin.system.doesNotExist()
+             self.fail("What? Method exists???")
+-        except xmlrpclib.Fault, e:
+-            self.assertEquals(-32601, e.faultCode)
+-            self.assertTrue("not found" in e.faultString)
++        e = self.assertRaises(xmlrpclib.Fault, local_test)
++        self.assertEquals(-32601, e.faultCode)
++        self.assertTrue("not found" in e.faultString)
+ 
+     def test_wrong_argspec(self):
+-        try:
++        def local_test():
+             self.admin.system.listMethods("hello")
+             self.fail("Oops. Wrong argspec accepted???")
+-        except xmlrpclib.Fault, e:
+-            self.assertEquals(1, e.faultCode)
+-            self.assertTrue("listMethods() takes exactly 2 arguments" \
++        e = self.assertRaises(xmlrpclib.Fault, local_test)
++        self.assertEquals(1, e.faultCode)
++        self.assertTrue("listMethods() takes exactly 2 arguments" \
+                                     in e.faultString)
+ 
+     def test_content_encoding(self):
+         test_string = "øæåØÆÅàéüoö"
+         # No encoding / encoding error
+-        try:
++        def local_test():
+             t_id = self.admin.ticket.create(test_string, test_string[::-1], {})
+             self.admin.ticket.delete(t_id)
+             self.fail("Expected ticket create to fail...")
+-        except Exception, e:
+-            self.assertTrue(isinstance(e, xmlrpclib.Fault))
+-            self.assertEquals(-32700, e.faultCode)
++        e = self.assertRaises(xmlrpclib.Fault, local_test)
++        self.assertTrue(isinstance(e, xmlrpclib.Fault))
++        self.assertEquals(-32700, e.faultCode)
+         # Unicode version (encodable)
+         from trac.util.text import to_unicode
+         test_string = to_unicode(test_string)
+@@ -81,7 +82,7 @@
          xmlrpc_now = to_xmlrpc_datetime(now)
          self.assertTrue(isinstance(xmlrpc_now, xmlrpclib.DateTime),
                  "Expected xmlprc_now to be an xmlrpclib.DateTime")
          now_from_xmlrpc = from_xmlrpc_datetime(xmlrpc_now)
          self.assertTrue(isinstance(now_from_xmlrpc, datetime),
                  "Expected now_from_xmlrpc to be a datetime")
-@@ -104,8 +104,8 @@
-             self.assertEquals(e.faultString,
-                     'Wiki page "Test" does not exist at version 10')
+@@ -90,22 +91,18 @@
+ 
+     def test_resource_not_found(self):
+         # A Ticket resource
+-        try:
+-            self.admin.ticket.get(2147483647)
+-        except Exception, e:
+-            self.assertEquals(e.faultCode, 404)
+-            self.assertEquals(e.faultString, 
+-                    'Ticket 2147483647 does not exist.')
++        e = self.assertRaises(xmlrpclib.Fault, self.admin.ticket.get, 2147483647)
++        self.assertEquals(e.faultCode, 404)
++        self.assertEquals(e.faultString, 
++                'Ticket 2147483647 does not exist.')
+         # A Wiki resource
+-        try:
+-            self.admin.wiki.getPage("Test", 10)
+-        except Exception, e:
+-            self.assertEquals(e.faultCode, 404)
+-            self.assertEquals(e.faultString,
+-                    'Wiki page "Test" does not exist at version 10')
++        e = self.assertRaises(xmlrpclib.Fault, self.admin.wiki.getPage, "Test", 10)
++        self.assertEquals(e.faultCode, 404)
++        self.assertEquals(e.faultString,
++                'Wiki page "Test" does not exist at version 10')
  
 -def suite():
 +def test_suite():
 +    unittest.main(defaultTest='test_suite')
 diff -r 369fe39e568b trunk/tracrpc/web_ui.py
 --- a/trunk/tracrpc/web_ui.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/web_ui.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/web_ui.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -6,6 +6,10 @@
  (c) 2009      ::: www.CodeResort.com - BV Network AS (simon-code@bvnetwork.no)
  """
 +    def _rpc_process(self, req, protocol, content_type):
 +        """Process incoming RPC request and finalize response."""
 +        proto_id = protocol.rpc_info()[0]
-+        rpcreq = {'mimetype': content_type}
++        rpcreq = req.rpc = {'mimetype': content_type}
 +        try :
 +            self.log.debug("RPC(%s) call by '%s'", proto_id, req.authname)
-+            rpcreq = protocol.parse_rpc_request(req, content_type)
++            rpcreq = req.rpc = protocol.parse_rpc_request(req, content_type)
 +            rpcreq['mimetype'] = content_type
-+            
++
 +            # Important ! Check after parsing RPC request to add 
 +            #             protocol-specific fields in response 
 +            #             (e.g. JSON-RPC response `id`)
 +            req.perm.require('XML_RPC') # Need at least XML_RPC
-+            
++
 +            method_name = rpcreq.get('method')
 +            if method_name is None :
 +                raise ProtocolException('Missing method name')
 +                e, tb = sys.exc_info()[-2:]
 +                raise ServiceException(e), None, tb
 +            else :
-+                protocol.send_rpc_result(req, rpcreq, result)
++                protocol.send_rpc_result(req, result)
 +        except RequestDone :
 +            raise
 +        except (RPCError, PermissionError, ResourceNotFound), e:
 +            self.log.exception("RPC(%s) Error", proto_id)
 +            try :
-+                protocol.send_rpc_error(req, rpcreq, e)
++                protocol.send_rpc_error(req, e)
 +            except RequestDone :
 +                raise
 +            except Exception, e :
 +                self.log.exception("RPC(%s) Unhandled protocol error", proto_id)
-+                self._send_unknown_error(req, rpcreq, e)
++                self._send_unknown_error(req, e)
 +        except Exception, e :
 +            self.log.exception("RPC(%s) Unhandled protocol error", proto_id)
-+            self._send_unknown_error(req, rpcreq, e)
++            self._send_unknown_error(req, e)
 +
-+    def _send_unknown_error(self, req, rpcreq, e):
++    def _send_unknown_error(self, req, e):
 +        """Last recourse if protocol cannot handle the RPC request | error"""
 +
 +        # TODO: This is the previous implementation. Please review.
 +#                status=500, env=None, data=to_unicode(e))
 +
 +        stackTrace = format_exc()
-+        method_name = rpcreq and rpcreq.get('method') or '(undefined)'
++        method_name = req.rpc and req.rpc.get('method') or '(undefined)'
 +        body = "Unhandled protocol error calling '%s'\n\n%s" % (method_name, stackTrace)
 +        req.send_error(None, template='', content_type='text/plain',
 +                            env=None, data=body, status=HTTPInternalError.code)
      def get_htdocs_dirs(self):
 diff -r 369fe39e568b trunk/tracrpc/xml_rpc.py
 --- a/trunk/tracrpc/xml_rpc.py	Sat Mar 27 18:21:41 2010 -0500
-+++ b/trunk/tracrpc/xml_rpc.py	Sat Mar 27 23:40:18 2010 -0500
++++ b/trunk/tracrpc/xml_rpc.py	Thu Apr 08 22:58:08 2010 -0500
 @@ -19,7 +19,7 @@
  from trac.util.text import to_unicode
  
  from tracrpc.util import empty, prepare_docs
  
  __all__ = ['XmlRpcProtocol']
-@@ -81,41 +81,46 @@
+@@ -81,41 +81,48 @@
          yield ('xmlrpc', 'application/xml')
          yield ('xmlrpc', 'text/xml')
  
 +            args = self._normalize_xml_input(args)
 +            return {'method' : method, 'params' : args}
 +
-+    def send_rpc_result(self, req, rpcreq, result):
++    def send_rpc_result(self, req, result):
 +        """Send the result of the XML-RPC call back to the client."""
++        rpcreq = req.rpc
 +        method = rpcreq.get('method')
 +        self.log.debug("RPC(xml) '%s' result: %s" % (
 +                                                    method, repr(result)))
 +        self._send_response(req,
 +                xmlrpclib.dumps(result, methodresponse=True), rpcreq['mimetype'])
 +
-+    def send_rpc_error(self, req, rpcreq, e):
++    def send_rpc_error(self, req, e):
 +        """Send an XML-RPC fault message back to the caller"""
++        rpcreq = req.rpc
 +        fault = None
 +        if isinstance(e, ProtocolException):
 +            fault = e._exc
              self.log.error(e)
              import traceback
              from tracrpc.util import StringIO
-@@ -123,6 +128,7 @@
+@@ -123,6 +130,7 @@
              traceback.print_exc(file = out)
              self.log.error(out.getvalue())
              err_code = hasattr(e, 'code') and e.code or 1