Merge branch 'devel' into 'master'

Handle size parameter and correct behaviour with non existing OpenID/Email (return default)

See merge request oliver/ivatar!34

ivatar/ivataraccount/admin.py

@@ -4,12 +4,16 @@ Register models in admin
 from django.contrib import admin
 
 from . models import Photo, ConfirmedEmail, UnconfirmedEmail
-from . models import ConfirmedOpenId, OpenIDNonce, OpenIDAssociation
+from . models import ConfirmedOpenId, UnconfirmedOpenId
+from . models import OpenIDNonce, OpenIDAssociation
+from . models import UserPreference
 
 # Register models in admin
 admin.site.register(Photo)
 admin.site.register(ConfirmedEmail)
 admin.site.register(UnconfirmedEmail)
 admin.site.register(ConfirmedOpenId)
+admin.site.register(UnconfirmedOpenId)
+admin.site.register(UserPreference)
 admin.site.register(OpenIDNonce)
 admin.site.register(OpenIDAssociation)

ivatar/ivataraccount/models.py

@@ -80,7 +80,7 @@ class UserPreference(models.Model):
     )
 
     def __str__(self):
-        return '<UserPreference (%i) for %s>' % (self.pk, self.user)
+        return 'Preference (%i) for %s' % (self.pk, self.user)
 
 
 class BaseAccountModel(models.Model):
@@ -244,6 +244,9 @@ class Photo(BaseAccountModel):
 
         return HttpResponseRedirect(reverse_lazy('profile'))
 
+    def __str__(self):
+        return '%s (%i) from %s' % (self.format, self.pk, self.user)
+
 
 class ConfirmedEmailManager(models.Manager):  # pylint: disable=too-few-public-methods
     '''
@@ -312,6 +315,9 @@ class ConfirmedEmail(BaseAccountModel):
         self.digest_sha256 = hashlib.sha256(self.email.strip().lower().encode('utf-8')).hexdigest()
         return super().save(force_insert, force_update, using, update_fields)
 
+    def __str__(self):
+        return '%s (%i) from %s' % (self.email, self.pk, self.user)
+
 
 class UnconfirmedEmail(BaseAccountModel):
     '''
@@ -334,6 +340,9 @@ class UnconfirmedEmail(BaseAccountModel):
         self.verification_key = hash_object.hexdigest()
         super(UnconfirmedEmail, self).save(force_insert, force_update, using, update_fields)
 
+    def __str__(self):
+        return '%s (%i) from %s' % (self.email, self.pk, self.user)
+
 
 class UnconfirmedOpenId(BaseAccountModel):
     '''
@@ -348,6 +357,9 @@ class UnconfirmedOpenId(BaseAccountModel):
         verbose_name = _('unconfirmed OpenID')
         verbose_name_plural = ('unconfirmed_OpenIDs')
 
+    def __str__(self):
+        return '%s (%i) from %s' % (self.openid, self.pk, self.user)
+
 
 class ConfirmedOpenId(BaseAccountModel):
     '''
@@ -395,6 +407,9 @@ class ConfirmedOpenId(BaseAccountModel):
         self.digest = hashlib.sha256(lowercase_url.encode('utf-8')).hexdigest()
         return super().save(force_insert, force_update, using, update_fields)
 
+    def __str__(self):
+        return '%s (%i) (%s)' % (self.openid, self.pk, self.user)
+
 
 class OpenIDNonce(models.Model):
     '''
@@ -405,6 +420,9 @@ class OpenIDNonce(models.Model):
     timestamp = models.IntegerField()
     salt = models.CharField(max_length=128)
 
+    def __str__(self):
+        return '%s (%i) (timestamp: %i)' % (self.server_url, self.pk, self.timestamp)
+
 
 class OpenIDAssociation(models.Model):
     '''
@@ -417,6 +435,9 @@ class OpenIDAssociation(models.Model):
     lifetime = models.IntegerField()
     assoc_type = models.TextField(max_length=64)
 
+    def __str__(self):
+        return '%s (%i) (%s, lifetime: %i)' % (self.server_url, self.pk, self.assoc_type, self.lifetime)
+
 
 class DjangoOpenIDStore(OpenIDStore):
     '''
@@ -424,10 +445,10 @@ class DjangoOpenIDStore(OpenIDStore):
     related to OpenID authentications. This one uses our Django models.
     '''
 
-    def storeAssociation(self, server_url, association):  # pragma: no cover
+    @staticmethod
+    def storeAssociation(server_url, association):  # pragma: no cover
         '''
         Helper method to store associations
-        TODO: Could be moved to classmethod
         '''
         assoc = OpenIDAssociation(
             server_url=server_url,
@@ -472,10 +493,11 @@ class DjangoOpenIDStore(OpenIDStore):
             return None
         return associations[-1][1]
 
-    def removeAssociation(self, server_url, handle):  # pragma: no cover
+
+    @staticmethod
+    def removeAssociation(server_url, handle):  # pragma: no cover
         '''
         Helper method to remove associations
-        TODO: Could be moved to classmethod
         '''
         assocs = list(
             OpenIDAssociation.objects.filter(  # pylint: disable=no-member
@@ -485,10 +507,10 @@ class DjangoOpenIDStore(OpenIDStore):
             assoc.delete()
         return assocs_exist
 
-    def useNonce(self, server_url, timestamp, salt):  # pragma: no cover
+    @staticmethod
+    def useNonce(server_url, timestamp, salt):  # pragma: no cover
         '''
         Helper method to 'use' nonces
-        TODO: Could be moved to classmethod
         '''
         # Has nonce expired?
         if abs(timestamp - time.time()) > oidnonce.SKEW:
@@ -505,18 +527,18 @@ class DjangoOpenIDStore(OpenIDStore):
         nonce.delete()
         return False
 
-    def cleanupNonces(self):  # pragma: no cover
+    @staticmethod
+    def cleanupNonces():  # pragma: no cover
         '''
         Helper method to cleanup nonces
-        TODO: Could be moved to classmethod
         '''
         timestamp = int(time.time()) - oidnonce.SKEW
         OpenIDNonce.objects.filter(timestamp__lt=timestamp).delete()  # pylint: disable=no-member
 
-    def cleanupAssociations(self):  # pragma: no cover
+    @staticmethod
+    def cleanupAssociations():  # pragma: no cover
         '''
         Helper method to cleanup associations
-        TODO: Could be moved to classmethod
         '''
         OpenIDAssociation.objects.extra(  # pylint: disable=no-member
             where=['issued + lifetimeint < (%s)' % time.time()]).delete()

ivatar/ivataraccount/test_views.py

@@ -1059,7 +1059,7 @@ class Tester(TestCase):  # pylint: disable=too-many-public-methods
             self.user.photo_set.first(),
             'set_photo did not work!?')
 
-    def test_avatar_url_mail(self, do_upload_and_confirm=True):
+    def test_avatar_url_mail(self, do_upload_and_confirm=True, size=(80, 80)):
         '''
         Test fetching avatar via mail
         '''
@@ -1068,18 +1068,21 @@ class Tester(TestCase):  # pylint: disable=too-many-public-methods
             self.test_confirm_email()
         urlobj = urlsplit(
             libravatar_url(
-                email=self.user.confirmedemail_set.first().email)
+                email=self.user.confirmedemail_set.first().email,
+                size=size[0],
             )
-        url = urlobj.path
+        )
+        url = '%s?%s' % (urlobj.path, urlobj.query)
         response = self.client.get(url, follow=True)
         self.assertEqual(
             response.status_code,
             200,
             'unable to fetch avatar?')
+        photodata = Image.open(BytesIO(response.content))
         self.assertEqual(
-            response.content,
+            photodata.size,
-            self.user.photo_set.first().data,
+            size,
-            'Why is this not the same data?')
+            'Why is this not the correct size?')
 
     def test_avatar_url_openid(self):
         '''
@@ -1088,18 +1091,21 @@ class Tester(TestCase):  # pylint: disable=too-many-public-methods
         self.test_assign_photo_to_openid()
         urlobj = urlsplit(
             libravatar_url(
-                openid=self.user.confirmedopenid_set.first().openid)
+                openid=self.user.confirmedopenid_set.first().openid,
+                size=80,
             )
-        url = urlobj.path
+        )
+        url = '%s?%s' % (urlobj.path, urlobj.query)
         response = self.client.get(url, follow=True)
         self.assertEqual(
             response.status_code,
             200,
             'unable to fetch avatar?')
+        photodata = Image.open(BytesIO(response.content))
         self.assertEqual(
-            response.content,
+            photodata.size,
-            self.user.photo_set.first().data,
+            (80, 80),
-            'Why is this not the same data?')
+            'Why is this not the correct size?')
 
     def test_avatar_url_inexisting_mail_digest(self):  # pylint: disable=invalid-name
         '''
@@ -1109,14 +1115,20 @@ class Tester(TestCase):  # pylint: disable=too-many-public-methods
         self.test_confirm_email()
         urlobj = urlsplit(
             libravatar_url(
-                email=self.user.confirmedemail_set.first().email)
+                email=self.user.confirmedemail_set.first().email,
+                size=80,
+            )
         )
         # Simply delete it, then it digest is 'correct', but
         # the hash is no longer there
         self.user.confirmedemail_set.first().delete()
-        url = urlobj.path
+        url = '%s?%s' % (urlobj.path, urlobj.query)
-        self.assertRaises(Exception, lambda:
+        response = self.client.get(url, follow=True)
-                          self.client.get(url, follow=True))
+        self.assertEqual(
+            response['Content-Type'],
+            'image/png',
+            'Content type wrong!?')
+        # Eventually one should check if the data is the same
 
     def test_crop_photo(self):
         '''
@@ -1135,6 +1147,6 @@ class Tester(TestCase):  # pylint: disable=too-many-public-methods
             response.status_code,
             200,
             'unable to crop?')
-        self.test_avatar_url_mail(do_upload_and_confirm=False)
+        self.test_avatar_url_mail(do_upload_and_confirm=False, size=(20, 20))
         img = Image.open(BytesIO(self.user.photo_set.first().data))
         self.assertEqual(img.size, (20, 20), 'cropped to 20x20, but resulting image isn\'t 20x20!?')

ivatar/ivataraccount/views.py

@@ -533,7 +533,7 @@ class CropPhotoView(TemplateView):
     model = Photo
 
     def get(self, request, *args, **kwargs):
-        photo = self.model.objects.get(pk=kwargs['pk'], user=request.user)
+        photo = self.model.objects.get(pk=kwargs['pk'], user=request.user)  # pylint: disable=no-member
         email = request.GET.get('email')
         openid = request.GET.get('openid')
         return render(self.request, self.template_name, {
@@ -543,7 +543,7 @@ class CropPhotoView(TemplateView):
         })
 
     def post(self, request, *args, **kwargs):
-        photo = self.model.objects.get(pk=kwargs['pk'], user=request.user)
+        photo = self.model.objects.get(pk=kwargs['pk'], user=request.user)  # pylint: disable=no-member
         dimensions = {
             'x': int(request.POST['x']),
             'y': int(request.POST['y']),

ivatar/views.py

@@ -1,29 +1,42 @@
 '''
 views under /
 '''
-import io
+from io import BytesIO
+from os import path
+from PIL import Image
 from django.views.generic.base import TemplateView
 from django.http import HttpResponse
-from . ivataraccount.models import ConfirmedEmail, ConfirmedOpenId
 from django.core.exceptions import ObjectDoesNotExist
+from ivatar.settings import AVATAR_MAX_SIZE, JPEG_QUALITY
+from . ivataraccount.models import ConfirmedEmail, ConfirmedOpenId
+from . ivataraccount.models import pil_format
 
 
 class AvatarImageView(TemplateView):
     '''
-    View to return (binary) image, based for OpenID/Email (both by digest)
+    View to return (binary) image, based on OpenID/Email (both by digest)
     '''
+    # TODO: Do cache resize images!! Memcached?
 
     def get(self, request, *args, **kwargs):
         '''
         Override get from parent class
         '''
         model = ConfirmedEmail
+        size = 80
+        imgformat = 'png'
+        obj = None
+        if 's' in request.GET:
+            size = request.GET['s']
+        size = int(size)
+        if size > int(AVATAR_MAX_SIZE):
+            size = int(AVATAR_MAX_SIZE)
         if len(kwargs['digest']) == 32:
             # Fetch by digest from mail
             pass
         elif len(kwargs['digest']) == 64:
-            if ConfirmedOpenId.objects.filter(
+            if ConfirmedOpenId.objects.filter(  # pylint: disable=no-member
-              digest=kwargs['digest']).count():  # pylint: disable=no-member
+                    digest=kwargs['digest']).count():
                 # Fetch by digest from OpenID
                 model = ConfirmedOpenId
         else:  # pragma: no cover
@@ -36,15 +49,26 @@ class AvatarImageView(TemplateView):
             try:
                 obj = model.objects.get(digest_sha256=kwargs['digest'])
             except ObjectDoesNotExist:
-                # TODO: Use default!?
+                pass
-                raise Exception('Mail/openid ("%s") does not exist"' %
+
-                                kwargs['digest'])
+        if not obj or not obj.photo:
-        if not obj.photo:
+            static_img = path.join('static', 'img', 'mm', '%s%s' % (str(size), '.png'))
-            # That is hacky, but achieves what we want :-)
+            if path.isfile(static_img):
-            attr = getattr(obj, 'email', obj.openid)
+                photodata = Image.open(static_img)
-            # TODO: Use default!?
+            else:
-            raise Exception('No photo assigned to "%s"' % attr)
+                # TODO: Resize it!?
+                static_img = path.join('static', 'img', 'mm', '512.png')
+        else:
+            imgformat = obj.photo.format
+            photodata = Image.open(BytesIO(obj.photo.data))
+
+        photodata.thumbnail((size, size), Image.ANTIALIAS)
+        data = BytesIO()
+        photodata.save(data, pil_format(imgformat), quality=JPEG_QUALITY)
+        data.seek(0)
 
         return HttpResponse(
-            io.BytesIO(obj.photo.data),
+            data,
-            content_type='image/%s' % obj.photo.format)
+            content_type='image/%s' % imgformat)
+        # One eventually also wants to check if the DATA is correct,
+        # not only the size