Add Python 3 support

Author: Burak Yigit Kaya

phabricator/__init__.py

@@ -14,21 +14,23 @@
 except:
     __version__ = 'unknown'
 
+import collections
 import copy
 import hashlib
-import httplib
 import json
 import os.path
 import re
 import socket
 import time
-import urllib
-import urlparse
 
-from collections import defaultdict
+from ._compat import (
+    MutableMapping, iteritems, string_types, httplib, urlparse, urlencode,
+)
+
 
 __all__ = ['Phabricator']
 
+
 # Default phabricator interfaces
 INTERFACES = json.loads(open(os.path.join(os.path.dirname(__file__), 'interfaces.json'), 'r').read())
 
@@ -79,11 +81,11 @@
     'pair': tuple,
 
     # str types
-    'str': basestring,
-    'string': basestring,
-    'phid': basestring,
-    'guids': basestring,
-    'type': basestring,
+    'str': string_types,
+    'string': string_types,
+    'phid': string_types,
+    'guids': string_types,
+    'type': string_types,
 }
 
 STR_RE = re.compile(r'([a-zA-Z_]+)')
@@ -108,9 +110,9 @@ def map_param_type(param_type):
         sub_match = STR_RE.match(sub_type)
         sub_type = sub_match.group(0).lower()
 
-        return [PARAM_TYPE_MAP.setdefault(sub_type, basestring)]
+        return [PARAM_TYPE_MAP.setdefault(sub_type, string_types)]
 
-    return PARAM_TYPE_MAP.setdefault(main_type, basestring)
+    return PARAM_TYPE_MAP.setdefault(main_type, string_types)
 
 
 def parse_interfaces(interfaces):
@@ -119,9 +121,9 @@ def parse_interfaces(interfaces):
     This performs the logic of parsing the non-standard params dict
         and then returning a dict Resource can understand
     """
-    parsed_interfaces = defaultdict(dict)
+    parsed_interfaces = collections.defaultdict(dict)
 
-    for m, d in interfaces.iteritems():
+    for m, d in iteritems(interfaces):
         app, func = m.split('.', 1)
 
         method = parsed_interfaces[app][func] = {}
@@ -133,7 +135,7 @@ def parse_interfaces(interfaces):
         method['optional'] = {}
         method['required'] = {}
 
-        for name, type_info in dict(d['params']).iteritems():
+        for name, type_info in iteritems(dict(d['params'])):
             # Usually in the format: <optionality> <param_type>
             info_pieces = type_info.split(' ', 1)
 
@@ -176,42 +178,29 @@ class InvalidAccessToken(APIError):
     pass
 
 
-class Result(object):
+class Result(MutableMapping):
     def __init__(self, response):
         self.response = response
 
-    def __repr__(self):
-        return '<%s: %s>' % (self.__class__.__name__, repr(self.response))
-
-    def __iter__(self):
-        for r in self.response:
-            yield r
-
     def __getitem__(self, key):
         return self.response[key]
 
-    def __getattr__(self, key):
-        return self.response[key]
+    __getattr__ = __getitem__
+
+    def __setitem__(self, key, value):
+        self.response[key] = value
 
-    def __getstate__(self):
-        return self.response
+    def __delitem__(self, key):
+        del self.response[key]
 
-    def __setstate__(self, state):
-        self.response = state
+    def __iter__(self):
+        return iter(self.response)
 
     def __len__(self):
         return len(self.response.keys())
 
-    def keys(self):
-        return self.response.keys()
-
-    def iteritems(self):
-        for k, v in self.response.iteritems():
-            yield k, v
-
-    def itervalues(self):
-        for v in self.response.itervalues():
-            yield v
+    def __repr__(self):
+        return '<%s: %s>' % (type(self).__name__, repr(self.response))
 
 
 class Resource(object):
@@ -240,17 +229,17 @@ def validate_kwarg(key, target):
             # Always allow list
             if isinstance(key, list):
                 return all([validate_kwarg(x, target[0]) for x in key])
-            return isinstance(key, target)
-
-        for k in resource.get('required', []):
-            if k not in [x.split(':')[0] for x in kwargs.keys()]:
-                raise ValueError('Missing required argument: %s' % k)
-            if isinstance(kwargs.get(k), list) and not isinstance(resource['required'][k], list):
-                raise ValueError('Wrong argument type: %s is not a list' % k)
-            elif not validate_kwarg(kwargs.get(k), resource['required'][k]):
-                if isinstance(resource['required'][k], list):
-                    raise ValueError('Wrong arguemnt type: %s is not a list of %ss' % (k, resource['required'][k][0]))
-                raise ValueError('Wrong arguemnt type: %s is not a %s' % (k, resource['required'][k]))
+            return isinstance(key, tuple(target) if isinstance(target, list) else target)
+
+        for key, val in resource.get('required', {}).items():
+            if key not in [x.split(':')[0] for x in kwargs.keys()]:
+                raise ValueError('Missing required argument: %s' % key)
+            if isinstance(kwargs.get(key), list) and not isinstance(val, list):
+                raise ValueError('Wrong argument type: %s is not a list' % key)
+            elif not validate_kwarg(kwargs.get(key), val):
+                if isinstance(val, list):
+                    raise ValueError('Wrong argument type: %s is not a list of %ss' % (key, val[0]))
+                raise ValueError('Wrong argument type: %s is not a %s' % (key, val))
 
         conduit = self.api.conduit
 
@@ -280,7 +269,7 @@ def validate_kwarg(key, target):
             'Content-Type': 'application/x-www-form-urlencoded'
         }
 
-        body = urllib.urlencode({
+        body = urlencode({
             "params": json.dumps(kwargs),
             "output": self.api.response_format
         })
@@ -355,7 +344,8 @@ def connect(self):
         }
 
     def generate_hash(self, token):
-        return hashlib.sha1(token + self.api.certificate).hexdigest()
+        source_string = (token + self.api.certificate).encode('utf-8')
+        return hashlib.sha1(source_string).hexdigest()
 
     def update_interfaces(self):
         query = Resource(api=self, method='conduit', endpoint='query')

phabricator/_compat.py

@@ -0,0 +1,36 @@
+import sys
+
+PY3 = sys.version_info[0] >= 3
+
+try:
+    from collections.abc import MutableMapping
+except ImportError:
+    from collections import MutableMapping
+
+try:
+    import httplib
+except ImportError:
+    import http.client as httplib
+
+try:
+    import urlparse
+except ImportError:
+    import urllib.parse as urlparse
+
+try:
+    from urllib import urlencode
+except ImportError:
+    from urllib.parse import urlencode
+
+if PY3:
+    str_type = str
+    string_types = str,
+
+    def iteritems(d, **kw):
+        return iter(d.items(**kw))
+else:
+    str_type = unicode
+    string_types = basestring,
+
+    def iteritems(d, **kw):
+        return d.iteritems(**kw)

phabricator/tests.py

@@ -1,77 +1,100 @@
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+
+try:
+    from StringIO import StringIO
+except ImportError:
+    from io import StringIO
+
+try:
+    import unittest.mock as mock
+except ImportError:
+    import mock
+
+
 import phabricator
-import unittest
-from StringIO import StringIO
-from mock import patch, Mock
+
 
 RESPONSES = {
     'conduit.connect': '{"result":{"connectionID":1759,"sessionKey":"lwvyv7f6hlzb2vawac6reix7ejvjty72svnir6zy","userPHID":"PHID-USER-6ij4rnamb2gsfpdkgmny"},"error_code":null,"error_info":null}',
     'user.whoami': '{"result":{"phid":"PHID-USER-6ij4rnamz2gxfpbkamny","userName":"testaccount","realName":"Test Account"},"error_code":null,"error_info":null}',
     'maniphest.find': '{"result":{"PHID-TASK-4cgpskv6zzys6rp5rvrc":{"id":"722","phid":"PHID-TASK-4cgpskv6zzys6rp5rvrc","authorPHID":"PHID-USER-5022a9389121884ab9db","ownerPHID":"PHID-USER-5022a9389121884ab9db","ccPHIDs":["PHID-USER-5022a9389121884ab9db","PHID-USER-ba8aeea1b3fe2853d6bb"],"status":"3","priority":"Needs Triage","title":"Relations should be two-way","description":"When adding a differential revision you can specify Maniphest Tickets to add the relation. However, this doesnt add the relation from the ticket -> the differently.(This was added via the commit message)","projectPHIDs":["PHID-PROJ-358dbc2e601f7e619232","PHID-PROJ-f58a9ac58c333f106a69"],"uri":"https:\/\/secure.phabricator.com\/T722","auxiliary":[],"objectName":"T722","dateCreated":"1325553508","dateModified":"1325618490"}},"error_code":null,"error_info":null}'
 }
 
+CERTIFICATE = (
+    'fdhcq3zsyijnm4h6gmh43zue5umsmng5t4dlwodvmiz4cnc6fl6f'
+    'zrvjbfg2ftktrcddan7b3xtgmfge2afbrh4uwam6pfxpq5dbkhbl'
+    '6mgaijdzpq5efw2ynlnjhoeqyh6dakl4yg346gbhabzkcxreu7hc'
+    'jhw6vo6wwa7ky2sjdk742khlgsakwtme6sr2dfkhlxxkcqw3jngy'
+    'rq5zj7m6m7hnscuzlzsviawnvg47pe7l4hxiexpbb5k456r'
+)
+
 # Protect against local user's .arcrc interference.
 phabricator.ARCRC = {}
 
+
 class PhabricatorTest(unittest.TestCase):
     def setUp(self):
         self.api = phabricator.Phabricator(username='test', certificate='test', host='http://localhost')
-        self.api.certificate = "fdhcq3zsyijnm4h6gmh43zue5umsmng5t4dlwodvmiz4cnc6fl6f" + \
-                               "zrvjbfg2ftktrcddan7b3xtgmfge2afbrh4uwam6pfxpq5dbkhbl" + \
-                               "6mgaijdzpq5efw2ynlnjhoeqyh6dakl4yg346gbhabzkcxreu7hc" + \
-                               "jhw6vo6wwa7ky2sjdk742khlgsakwtme6sr2dfkhlxxkcqw3jngy" + \
-                               "rq5zj7m6m7hnscuzlzsviawnvg47pe7l4hxiexpbb5k456r"
+        self.api.certificate = CERTIFICATE
 
     def test_generate_hash(self):
         token = '12345678'
         hashed = self.api.generate_hash(token)
-        self.assertEquals(hashed, 'f8d3bea4e58a2b2967d93d5b307bfa7c693b2e7f')
+        self.assertEqual(hashed, 'f8d3bea4e58a2b2967d93d5b307bfa7c693b2e7f')
 
-    @patch('phabricator.httplib.HTTPConnection')
+    @mock.patch('phabricator.httplib.HTTPConnection')
     def test_connect(self, mock_connection):
-        mock = mock_connection.return_value = Mock()
-        mock.getresponse.return_value = StringIO(RESPONSES['conduit.connect'])
+        mock_obj = mock_connection.return_value = mock.Mock()
+        mock_obj.getresponse.return_value = StringIO(RESPONSES['conduit.connect'])
 
         api = phabricator.Phabricator(username='test', certificate='test', host='http://localhost')
         api.connect()
-        self.assertTrue('sessionKey' in api.conduit.keys())
-        self.assertTrue('connectionID' in api.conduit.keys())
+        keys = api.conduit.keys()
+        self.assertIn('sessionKey', keys)
+        self.assertIn('connectionID', keys)
 
-    @patch('phabricator.httplib.HTTPConnection')
+    @mock.patch('phabricator.httplib.HTTPConnection')
     def test_user_whoami(self, mock_connection):
-        mock = mock_connection.return_value = Mock()
-        mock.getresponse.return_value = StringIO(RESPONSES['user.whoami'])
+        mock_obj = mock_connection.return_value = mock.Mock()
+        mock_obj.getresponse.return_value = StringIO(RESPONSES['user.whoami'])
 
         api = phabricator.Phabricator(username='test', certificate='test', host='http://localhost')
         api.conduit = True
 
-        self.assertEqual('testaccount', api.user.whoami()['userName'])
+        self.assertEqual(api.user.whoami()['userName'], 'testaccount')
 
-    @patch('phabricator.httplib.HTTPConnection')
+    @mock.patch('phabricator.httplib.HTTPConnection')
     def test_maniphest_find(self, mock_connection):
-        mock = mock_connection.return_value = Mock()
-        mock.getresponse.return_value = StringIO(RESPONSES['maniphest.find'])
+        mock_obj = mock_connection.return_value = mock.Mock()
+        mock_obj.getresponse.return_value = StringIO(RESPONSES['maniphest.find'])
 
         api = phabricator.Phabricator(username='test', certificate='test', host='http://localhost')
         api.conduit = True
 
-        result = api.maniphest.find(ownerphids=["PHID-USER-5022a9389121884ab9db"])
-        self.assertEqual(1, len(result))
+        result = api.maniphest.find(ownerphids=['PHID-USER-5022a9389121884ab9db'])
+        self.assertEqual(len(result), 1)
 
         # Test iteration
-        self.assertTrue(isinstance([x for x in result], list))
+        self.assertIsInstance([x for x in result], list)
 
         # Test getattr
-        self.assertEqual("3", result["PHID-TASK-4cgpskv6zzys6rp5rvrc"]["status"])
+        self.assertEqual(result['PHID-TASK-4cgpskv6zzys6rp5rvrc']['status'], '3')
 
     def test_validation(self):
         self.api.conduit = True
 
+        self.assertRaises(ValueError, self.api.differential.find)
+        with self.assertRaises(ValueError):
+            self.api.differential.find(query=1)
+        with self.assertRaises(ValueError):
+            self.api.differential.find(query='1')
+        with self.assertRaises(ValueError):
+            self.api.differential.find(query='1', guids='1')
         with self.assertRaises(ValueError):
-            self.assertRaises(ValueError, self.api.differential.find())
-            self.assertRaises(ValueError, self.api.differential.find(query=1))
-            self.assertRaises(ValueError, self.api.differential.find(query="1"))
-            self.assertRaises(ValueError, self.api.differential.find(query="1", guids="1"))
-            self.assertRaises(ValueError, self.api.differential.find(query="1", guids=["1"]))
+            self.api.differential.find(query='1', guids=['1'])
 
 
 if __name__ == '__main__':

setup.py

@@ -1,7 +1,17 @@
 #!/usr/bin/env python
 
+import sys
+
 from setuptools import setup, find_packages
 
+tests_requires = ['pytest']
+
+if sys.version_info[:2] < (2, 7):
+    tests_requires.append('unittest2')
+
+if sys.version_info[:2] <= (3, 3):
+    tests_requires.append('mock')
+
 setup(
     name='phabricator',
     version='0.5.0',
@@ -12,13 +22,20 @@
     packages=find_packages(),
     zip_safe=False,
     test_suite='nose.collector',
-    install_requires=[''],
-    tests_require=['nose', 'unittest2', 'mock'],
+    tests_require=tests_requires,
     include_package_data=True,
     classifiers=[
         'Intended Audience :: Developers',
         'Intended Audience :: System Administrators',
         'Operating System :: OS Independent',
-        'Topic :: Software Development'
+        'Topic :: Software Development',
+        'Programming Language :: Python',
+        'Programming Language :: Python :: 2',
+        'Programming Language :: Python :: 2.6',
+        'Programming Language :: Python :: 2.7',
+        'Programming Language :: Python :: 3',
+        'Programming Language :: Python :: 3.3',
+        'Programming Language :: Python :: 3.4',
+        'Programming Language :: Python :: 3.5',
     ],
 )