make checksumming a task so it queues after image save

apps/addons/forms.py

@@ -1,5 +1,4 @@
 from datetime import datetime
-import hashlib
 import os
 import re
 
@@ -18,10 +17,10 @@ import amo
 import captcha.fields
 from amo.fields import ColorField
 from amo.urlresolvers import reverse
-from amo.utils import (LocalFileStorage, slug_validator, slugify,
-                       sorted_groupby, remove_icons)
+from amo.utils import slug_validator, slugify, sorted_groupby, remove_icons
 from addons.models import (Addon, AddonCategory, BlacklistedSlug, Category,
                            Persona)
+from addons.tasks import rereviewqueuetheme_checksum, theme_checksum
 from addons.utils import reverse_name_lookup
 from addons.widgets import IconWidgetRenderer, CategoriesSelectMultiple
 from applications.models import Application
@@ -591,14 +590,10 @@ class ThemeForm(ThemeFormBase):
         p.submit = datetime.now()
         p.author = user.username
         p.display_username = user.name
-
-        # To spot duplicate submissions.
-        # p.checksum = make_checksum(p.header_path, p.footer_path)
-        # dupe_personas = Persona.objects.filter(checksum=p.checksum)
-        # if dupe_personas.exists():
-        # p.dupe_persona = dupe_personas[0]
         p.save()
 
+        theme_checksum.delay(theme=p)
+
         # Save tags.
         for t in data['tags']:
             Tag(tag_text=t).save_tag(addon)
@@ -609,12 +604,6 @@ class ThemeForm(ThemeFormBase):
         return addon
 
 
-def make_checksum(header_path, footer_path):
-    ls = LocalFileStorage()
-    raw_checksum = ls._open(header_path).read() + ls._open(footer_path).read()
-    return hashlib.sha224(raw_checksum).hexdigest()
-
-
 class EditThemeForm(AddonFormBase):
     name = forms.CharField(max_length=50)
     slug = forms.CharField(max_length=30)
@@ -764,14 +753,7 @@ class EditThemeForm(AddonFormBase):
                 RereviewQueueTheme.objects.filter(theme=persona).delete()
                 rqt = RereviewQueueTheme.objects.create(
                     theme=persona, header=header, footer=footer)
-
-                # Check for possible duplicate theme images.
-                dupe_personas = Persona.objects.filter(
-                    checksum=make_checksum(header_dst or persona.header_path,
-                                           footer_dst or persona.footer_path))
-                if dupe_personas.exists():
-                    rqt.dupe_persona = dupe_personas[0]
-                    rqt.save()
+                rereviewqueuetheme_checksum.delay(rqt=rqt)
         except IOError as e:
             log.error(str(e))
             raise

apps/addons/tasks.py

@@ -1,3 +1,4 @@
+import hashlib
 import logging
 
 from django.conf import settings
@@ -7,9 +8,10 @@ from django.db import connection, transaction
 from celeryutils import task
 from PIL import Image
 
+from addons.models import Persona
 import amo
 from amo.decorators import set_modified_on, write
-from amo.utils import cache_ns_key, ImageCheck
+from amo.utils import cache_ns_key, ImageCheck, LocalFileStorage
 from lib.es.utils import index_objects
 from versions.models import Version
 
@@ -263,3 +265,29 @@ def update_incompatible_appversions(data, **kw):
     # Increment namespace cache of compat versions.
     for addon_id in addon_ids:
         cache_ns_key('d2c-versions:%s' % addon_id, increment=True)
+
+
+@task
+def theme_checksum(theme, **kw):
+    theme.checksum = make_checksum(theme.header_path, theme.footer_path)
+    dupe_personas = Persona.objects.filter(checksum=theme.checksum)
+    if dupe_personas.exists():
+        theme.dupe_persona = dupe_personas[0]
+    theme.save()
+
+
+@task
+def rereviewqueuetheme_checksum(rqt, **kw):
+    """Check for possible duplicate theme images."""
+    dupe_personas = Persona.objects.filter(
+        checksum=make_checksum(rqt.header_path or rqt.theme.header_path,
+                               rqt.footer_path or rqt.theme.footer_path))
+    if dupe_personas.exists():
+        rqt.dupe_persona = dupe_personas[0]
+        rqt.save()
+
+
+def make_checksum(header_path, footer_path):
+    ls = LocalFileStorage()
+    raw_checksum = ls._open(header_path).read() + ls._open(footer_path).read()
+    return hashlib.sha224(raw_checksum).hexdigest()

apps/devhub/tests/test_forms.py

@@ -277,13 +277,14 @@ class TestThemeForm(amo.tests.TestCase):
             {'data-allowed-types': 'image/jpeg|image/png',
              'data-upload-url': footer_url})
 
-    @mock.patch('addons.forms.make_checksum')
+    @mock.patch('addons.tasks.make_checksum')
     @mock.patch('addons.tasks.create_persona_preview_images.delay')
     @mock.patch('addons.tasks.save_persona_image.delay')
     def test_success(self, save_persona_image_mock,
-                     create_persona_preview_images_mock, mock1):
+                     create_persona_preview_images_mock, make_checksum_mock):
         if not hasattr(Image.core, 'jpeg_encoder'):
             raise SkipTest
+        make_checksum_mock.return_value = 'hashyourselfbeforeyoucrashyourself'
 
         self.request.amo_user = UserProfile.objects.get(pk=2519)
 
@@ -349,7 +350,7 @@ class TestThemeForm(amo.tests.TestCase):
 
     @mock.patch('addons.tasks.create_persona_preview_images.delay')
     @mock.patch('addons.tasks.save_persona_image.delay')
-    @mock.patch('addons.forms.make_checksum')
+    @mock.patch('addons.tasks.make_checksum')
     def test_dupe_persona(self, make_checksum_mock, mock1, mock2):
         """
         Submitting persona with checksum already in db should be marked
@@ -468,7 +469,7 @@ class TestEditThemeForm(amo.tests.TestCase):
         eq_(self.form.errors,
             {'name': ['This name is already in use. Please choose another.']})
 
-    @mock.patch('addons.forms.make_checksum')
+    @mock.patch('addons.tasks.make_checksum')
     @mock.patch('addons.tasks.create_persona_preview_images.delay')
     @mock.patch('addons.tasks.save_persona_image.delay')
     def test_reupload(self, save_persona_image_mock,
@@ -498,7 +499,7 @@ class TestEditThemeForm(amo.tests.TestCase):
         eq_(rqt[0].footer, 'pending_footer.png')
         assert not rqt[0].dupe_persona
 
-    @mock.patch('addons.forms.make_checksum')
+    @mock.patch('addons.tasks.make_checksum')
     @mock.patch('addons.tasks.create_persona_preview_images.delay')
     @mock.patch('addons.tasks.save_persona_image.delay')
     def test_reupload_duplicate(self, save_persona_image_mock,

migrations/590-theme-checksums-fix.py

@@ -1,43 +1,2 @@
-#!/usr/bin/env python
-
-from celeryutils import task
-import commonware.log
-from PIL import Image
-
-from addons.forms import make_checksum
-from addons.models import Persona
-from amo.decorators import write
-from amo.utils import chunked
-
-log = commonware.log.getLogger('z.addons')
-
-
-@task
-@write
-def calc_checksum(theme_id, **kw):
-    theme = Persona.objects.get(id=theme_id)
-    header = theme.header_path
-    footer = theme.footer_path
-
-    # Delete invalid themes that are not images (e.g. PDF, EXE).
-    try:
-        Image.open(header)
-        Image.open(footer)
-    except IOError:
-        theme.addon.delete()
-        theme.delete()
-        return
-
-    # Calculate checksum and save.
-    try:
-        theme.checksum = make_checksum(header, footer)
-        theme.save()
-    except Exception as e:
-        log.error(str(e))
-
-
 def run():
-    """Calculate checksums for all themes."""
-    pks = Persona.objects.filter(checksum='').values_list('id', flat=True)
-    for chunk in chunked(pks, 1000):
-        [calc_checksum.delay(pk) for pk in chunk]
+    pass