Initial Python 3 port

This does enough Python 3 porting to make Tahrir run and do some
basic stuff under Python 3 - I've tested creating badges and
series, issuing badges, clicking around in Leaderboard and
Explore, looking at RSS and JSON views of things. This does not
break Python 2 compatibility - I'd rather not do that yet so we
can test things easily both ways and identify any differences.
We could remove Python 2 compat later.

Most of the changes are based on 2to3 suggestions and are pretty
self-explanatory. Some less obvious ones:

* The str_to_bytes and dogpile stuff: well, see
https://github.com/sqlalchemy/dogpile.cache/issues/159 . The
`sha1_mangle_key` mangler that we're using, which is provided by
dogpile, needs input as a bytestring. This is pretty awkward. It
obviously caused *some* problems even in Python 2 (as this app
explicitly uses unicodes in some places), but in Python 3 it's
worse; everywhere you see `str_to_bytes` being called is a place
where I found a crash because we wound up sending a non-encoded
`str` to `sha1_mangle_key` (or, in the case of `email_md5` and
`email_sha1`, to hashlib directly).

* map moved in Python 3; 2to3 suggests handling it with a six
move, but I preferred just replacing all the `map` uses with
comprehensions.

* 2to3 recommended a change to strip_tags, but I noticed it is
not actually used any more. It was used to sanitize HTML input
to the admin route back when it was added, but the admin route
was entirely rewritten later and the use of strip_tags was taken
out. So I just removed strip_tags and its supporting players.

* merge_dicts is used in places where we were merging two dicts
in a single expression by converting them to lists, combining
the lists, and turning the combined list back into a dict again.
You can still do this in Python 3 but you have to add extra
`list()` calls and it gets really ugly. Per
https://stackoverflow.com/questions/38987/how-to-merge-two-dictionaries-in-a-single-expression
it's also not resource-efficient, so this seems like a better
approach - it's informed by the code in that SO question but I
wrote the function myself rather than taking one from that page
to avoid technically having a tiny bit of CC-BY-SA code in this
AGPL project.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

setup.py

@@ -22,6 +22,7 @@ requires = [
     "requests",
     "rdflib<=4.1.2",
     "pytz",
+    "six",
 
     # For qrcode to work from PyPI, you also need Pillow.
     # This is handled for us in Fedora because python-qrcode pulls in the

tahrir/__init__.py

@@ -1,13 +1,10 @@
 from __future__ import print_function
+from __future__ import absolute_import
 
 import os
 import hashlib
 
-try:
-    import configparser as ConfigParser
-except ImportError:
-    import ConfigParser
-
+from six.moves.configparser import ConfigParser
 
 import dogpile.cache
 import dogpile.cache.util
@@ -24,6 +21,7 @@ from .utils import (
     make_avatar_method,
     make_relative_time_property,
     make_openid_identifier_property,
+    str_to_bytes,
 )
 from . import notifications
 
@@ -40,12 +38,12 @@ def main(global_config, **settings):
     """
 
     cache = dogpile.cache.make_region(
-        key_mangler=dogpile.cache.util.sha1_mangle_key)
+        key_mangler=lambda x: dogpile.cache.util.sha1_mangle_key(str_to_bytes(x)))
     tahrir_api.model.Person.avatar_url = make_avatar_method(cache)
     tahrir_api.model.Person.email_md5 = property(
-        lambda self: hashlib.md5(self.email).hexdigest())
+        lambda self: hashlib.md5(str_to_bytes(self.email)).hexdigest())
     tahrir_api.model.Person.email_sha1 = property(
-        lambda self: hashlib.sha1(self.email).hexdigest())
+        lambda self: hashlib.sha1(str_to_bytes(self.email)).hexdigest())
 
     identifier = settings.get('tahrir.openid_identifier')
     tahrir_api.model.Person.openid_identifier =\
@@ -94,7 +92,7 @@ def main(global_config, **settings):
         secret_path = settings.get('secret_config_path', default_path)
         # TODO: There is a better way to log this message than print.
         print("Reading secrets from %r" % secret_path)
-        parser = ConfigParser.ConfigParser()
+        parser = ConfigParser()
         parser.read(secret_path)
         secret_config = dict(parser.items("tahrir"))
         settings.update(secret_config)
@@ -208,10 +206,7 @@ def groupfinder(userid, request):
     is listed as an admin in the config file (tahrir.ini).
     This is the callback function used by the authorization
     policy."""
-    admins = map(
-        str.strip,
-        request.registry.settings['tahrir.admin'].split(','),
-    )
+    admins = [admin.strip() for admin in request.registry.settings['tahrir.admin'].split(',')]
     if userid in admins:
         return ['group:admins']
     else:

tahrir/app.py

@@ -1,3 +1,4 @@
+from __future__ import absolute_import
 from pyramid.security import Allow, Deny, Everyone
 
 

tahrir/custom_openid.py

@@ -1,3 +1,4 @@
+from __future__ import absolute_import
 import velruse.api
 import velruse.providers.openid as vr
 

tahrir/events.py

@@ -1,3 +1,4 @@
+from __future__ import absolute_import
 from pyramid.events import (
     subscriber,
     BeforeRender,

tahrir/notifications.py

@@ -1,3 +1,4 @@
+from __future__ import absolute_import
 import pyramid.threadlocal
 from pyramid.settings import asbool
 

tahrir/templates/index.mak

@@ -25,7 +25,7 @@
     <div class="shadow">
       <h1 class="section-header">Weekly Leaders</h1>
       <div class="padded-content">
-        % for person in weekly_leaders.keys()[:n]:
+        % for person in list(weekly_leaders.keys())[:n]:
           <div class="grid-container">
             ${self.functions.avatar_thumbnail(person, 64, 33)}
             <div class="grid-66 text-64">
@@ -46,7 +46,7 @@
     <div class="shadow">
       <h1 class="section-header">Monthly Leaders</h1>
       <div class="padded-content">
-        % for person in monthly_leaders.keys()[:n]:
+        % for person in list(monthly_leaders.keys())[:n]:
           <div class="grid-container">
             ${self.functions.avatar_thumbnail(person, 64, 33)}
             <div class="grid-66 text-64">

tahrir/templates/report.mak

@@ -6,7 +6,7 @@
       <div class="padded-content">
         <p>Looking for the <a href="${request.route_url('leaderboard')}">all-time leaderboard?</a></p>
         <table>
-          % for person, stats in user_to_rank.items()[:25]:
+          % for person, stats in list(user_to_rank.items())[:25]:
             <tr>
               <td style="width: 20px;">
                   <span class="big-text">#${stats['rank']}</span>

tahrir/utils.py

@@ -1,18 +1,15 @@
+from __future__ import absolute_import
 import cgi
 
-try:
-    from html.parser import HTMLParser
-except ImportError:
-    from HTMLParser import HTMLParser
-
 import math
 import time
 import datetime
 import dateutil.relativedelta
-import urllib
+import six.moves.urllib.parse
 from hashlib import md5, sha256
 
 import pyramid.threadlocal
+import six
 
 libravatar = None
 try:
@@ -21,40 +18,6 @@ except ImportError:
     pass
 
 
-class MLStripper(HTMLParser):
-
-    def __init__(self):
-        self.reset()
-        self.fed = []
-
-    def handle_data(self, d):
-        self.fed.append(d)
-
-    def get_data(self):
-        return ''.join(self.fed)
-
-
-def _strip_tags(html):
-    s = MLStripper()
-    s.feed(html)
-    return s.get_data()
-
-
-def strip_tags(_d):
-    d = {}
-    for k, v in _d.items():
-        if type(v) == dict:
-            d[k] = strip_tags(v)
-        elif type(v) == list:
-            d[k] = map(strip_tags, v)
-        elif isinstance(v, cgi.FieldStorage):
-            d[k] = v
-        else:
-            d[k] = _strip_tags(v)
-
-    return d
-
-
 def generate_badge_yaml(postdict):
     return "%YAML 1.2\n"\
          "---\n"\
@@ -107,7 +70,7 @@ def make_avatar_method(cache):
             # Make it big so we can downscale it as we please
             query['s'] = 312
 
-        query = urllib.urlencode(query)
+        query = six.moves.urllib.parse.urlencode(query)
 
         # Use md5 for emails, and sha256 for openids.
         # We're really using openids, so...
@@ -132,9 +95,7 @@ def make_avatar_method(cache):
 
     def avatar_method(self, size):
         # dogpile.cache can barf on unicode, so do this ourselves.
-        ident = self.openid_identifier
-        if isinstance(ident, unicode):
-            ident = ident.encode('utf-8')
+        ident = str_to_bytes(self.openid_identifier)
         # Call the cached workhorse function
         return _avatar_function(ident, size)
 
@@ -197,3 +158,24 @@ def make_openid_identifier_property(identifier):
         return "http://%s.%s" % (self.nickname, domain)
 
     return openid_identifier
+
+
+def merge_dicts(dict1, dict2):
+    """
+    Combine two dicts, in a way that works with Python 2 and 3. In
+    3.5+ you can just do z = {**x, **y}, so when we no longer care
+    about compatibility before 3.5 we can replace use of this.
+    """
+    ret = dict1.copy()
+    ret.update(dict2)
+    return ret
+
+
+def str_to_bytes(input):
+    """If input is unicode-type (unicode on Python 2, str on Python
+    3), encodes it and returns the result. Otherwise just passes it
+    through. Needed to deal with dogpile key mangling.
+    """
+    if isinstance(input, six.text_type):
+        input = input.encode('utf-8')
+    return input

tahrir/views.py

@@ -1,3 +1,4 @@
+from __future__ import absolute_import
 import re
 import random
 import types
@@ -8,6 +9,7 @@ import velruse
 import qrcode as qrcode_module
 import docutils.examples
 import markupsafe
+import six
 from datetime import date
 from datetime import datetime
 from datetime import timedelta
@@ -44,7 +46,7 @@ from pyramid.settings import asbool
 
 import tahrir_api.model as m
 
-from tahrir.utils import generate_badge_yaml
+from tahrir.utils import generate_badge_yaml, merge_dicts
 from tahrir_api.utils import convert_name_to_id
 
 
@@ -586,7 +588,7 @@ def leaderboard_json(request):
         leaderboard = leaderboard[:limit]
 
     ret = [
-        dict(user_to_rank[p].items() + [('nickname', p.nickname)])
+        merge_dicts(user_to_rank[p], {'nickname': p.nickname})
         for p in leaderboard if p in user_to_rank
     ]
 
@@ -908,12 +910,11 @@ def badge_rss(request):
     if not badge:
         raise HTTPNotFound("No such badge %r" % badge_id)
 
-    comparator = lambda x, y: cmp(x.issued_on, y.issued_on)
     # this gives us the assertions sorted *earliest first*. feedgen's
     # default when adding entries is to prepend - put the new item at
     # the top of the feed. so as we iterate over this and add items to
     # the feed, we add each newer assertion to the front of the feed
-    sorted_assertions = sorted(badge.assertions, cmp=comparator)
+    sorted_assertions = sorted(badge.assertions, key=lambda x: x.issued_on)
 
     feed = FeedGenerator()
     feed.title("Badges Feed for %s" % badge.name)
@@ -976,12 +977,11 @@ def user_rss(request):
     if user.opt_out == True and user.email != authenticated_userid(request):
         raise HTTPNotFound("User %r has opted out." % user_id)
 
-    comparator = lambda x, y: cmp(x.issued_on, y.issued_on)
     # this gives us the assertions sorted *earliest first*. feedgen's
     # default when adding entries is to prepend - put the new item at
     # the top of the feed. so as we iterate over this and add items to
     # the feed, we add each newer assertion to the front of the feed
-    sorted_assertions = sorted(user.assertions, cmp=comparator)
+    sorted_assertions = sorted(user.assertions, key=lambda x: x.issued_on)
 
     feed = FeedGenerator()
     feed.title("Badges Feed for %s" % user.nickname)
@@ -1131,10 +1131,10 @@ def _user_json_generator(request, user):
 
     assertions = []
     for assertion in user.assertions:
-        issued = {'issued': float(assertion.issued_on.strftime('%s'))}.items()
+        issued = {'issued': float(assertion.issued_on.strftime('%s'))}
         _badged = _badge_json_generator(
-            request, assertion.badge, withasserts=False).items()
-        assertions.append(dict(issued + _badged))
+            request, assertion.badge, withasserts=False)
+        assertions.append(merge_dicts(issued, _badged))
 
     return {
         'user': user.nickname,
@@ -1542,7 +1542,7 @@ def award_from_csv(request):
             # If there is an @ sign in the first value, it is the email.
             awards[values[0].strip()] = values[1].strip()
 
-    for email, badge_id in awards.iteritems():
+    for email, badge_id in six.iteritems(awards):
         # First, if the person doesn't exist, we automatically
         # create the person in this special case.
         if not request.db.person_exists(email=email):
@@ -1559,7 +1559,7 @@ def award_from_csv(request):
     return HTTPFound(location=request.route_url('admin'))
 
 
-@view_config(context=unicode)
+@view_config(context=six.text_type)
 def html(context, request):
     return Response(context)
 
@@ -1670,7 +1670,7 @@ def modify_rst(rst):
     try:
         # The rst features we need were introduced in this version
         minimum = [0, 9]
-        version = map(int, docutils.__version__.split('.'))
+        version = [int(elem) for elem in docutils.__version__.split('.')]
 
         # If we're at or later than that version, no need to downgrade
         if version >= minimum:

tahrir/widgets.py

@@ -1,3 +1,4 @@
+from __future__ import absolute_import
 import tahrir_api.model as m
 
 import hashlib
@@ -16,7 +17,7 @@ log = logging.getLogger(__name__)
 def scale_to_standard_size(filename):
     try:
         import magickwand.image as magick
-    except Exception, e:
+    except Exception as e:
         log.warn(str(e))
         log.warn("Did not scale image to standard size")
         return

tests/test_utils.py

@@ -13,3 +13,25 @@ class TestSingularize(unittest.TestCase):
     def test_singularize_value_1(self):
         """Test that the trailing letter is returned when value is 1."""
         self.assertEqual(utils.singularize('cats', 1), 'cat')
+
+
+class TestMergeDicts(unittest.TestCase):
+    """Test the merge_dicts() function."""
+
+    def test_merge_dicts(self):
+        dict1 = {1: 'a'}
+        dict2 = {2: ['b']}
+        self.assertEqual(utils.merge_dicts(dict1, dict2), {1: 'a', 2: ['b']})
+
+
+class TestStrToBytes(unittest.TestCase):
+    """Test the str_to_bytes() function."""
+
+    def test_str_to_bytes_str(self):
+        self.assertEqual(utils.str_to_bytes('foo'), b'foo')
+
+    def test_str_to_bytes_bytes(self):
+        self.assertEqual(utils.str_to_bytes(b'foo'), b'foo')
+
+    def test_str_to_bytes_other(self):
+        self.assertEqual(utils.str_to_bytes(['list']), ['list'])