Isolates standalone upload logic to prevent eager form validation (bu…

…g 672000)

apps/devhub/tests/test_views_validation.py

@@ -182,11 +182,11 @@ def test_context(self):
         eq_(r.status_code, 200)
         doc = pq(r.content)
         eq_(doc('#upload-addon').attr('data-upload-url'),
-            reverse('devhub.upload'))
+            reverse('devhub.standalone_upload'))
 
 
 class TestValidateFile(BaseUploadTest):
-    fixtures = ['base/apps', 'base/users',
+    fixtures = ['base/apps', 'base/users', 'base/addon_3615',
                 'devhub/addon-file-100456', 'base/platforms']
 
     def setUp(self):
@@ -301,6 +301,35 @@ def test_hide_validation_traceback(self, run_validator):
         eq_(data['validation'], '')
         eq_(data['error'], 'RuntimeError: simulated task error')
 
+    @mock.patch.object(waffle, 'flag_is_active')
+    @mock.patch('devhub.tasks.run_validator')
+    def test_rdf_parse_errors_are_ignored(self, run_validator,
+                                          flag_is_active):
+        run_validator.return_value = json.dumps({
+            "errors": 0,
+            "success": True,
+            "warnings": 0,
+            "notices": 0,
+            "message_tree": {},
+            "messages": [],
+            "metadata": {}
+        })
+        flag_is_active.return_value = True
+        addon = Addon.objects.get(pk=3615)
+        xpi = self.get_upload('extension.xpi')
+        d = parse_addon(xpi.path)
+        # Set up a duplicate upload:
+        addon.update(guid=d['guid'])
+        res = self.client.get(reverse('devhub.validate_addon'))
+        doc = pq(res.content)
+        upload_url = doc('#upload-addon').attr('data-upload-url')
+        with open(xpi.path, 'rb') as f:
+            # Simulate JS file upload
+            res = self.client.post(upload_url, {'upload': f}, follow=True)
+        data = json.loads(res.content)
+        # Make sure we don't see a dupe UUID error:
+        eq_(data['validation']['messages'], [])
+
 
 class TestCompatibilityResults(test_utils.TestCase):
     fixtures = ['base/users', 'devhub/addon-compat-results']
@@ -414,17 +443,19 @@ def setUp(self):
         self.app = Application.objects.get(pk=amo.FIREFOX.id)
         self.appver = AppVersion.objects.get(application=self.app,
                                              version='3.7a1pre')
-        self.upload_url = reverse('devhub.upload')
+        self.upload_url = reverse('devhub.standalone_upload')
 
     def poll_upload_status_url(self, upload_uuid):
         return reverse('devhub.upload_detail', args=[upload_uuid, 'json'])
 
-    def fake_xpi(self):
+    def fake_xpi(self, filename=None):
         """Any useless file that has a name property (for Django)."""
-        return open(get_image_path('non-animated.gif'), 'rb')
+        if not filename:
+            filename = get_image_path('non-animated.gif')
+        return open(filename, 'rb')
 
-    def upload(self):
-        with self.fake_xpi() as f:
+    def upload(self, filename=None):
+        with self.fake_xpi(filename=filename) as f:
             # Simulate how JS posts data w/ app/version from the form.
             res = self.client.post(self.upload_url,
                                    {'upload': f,
@@ -491,3 +522,18 @@ def test_compat_application_versions(self):
             empty = False
             eq_(AppVersion.objects.get(pk=id).version, ver)
         assert not empty, "Unexpected: %r" % data
+
+    @mock.patch.object(waffle, 'flag_is_active')
+    @mock.patch('devhub.tasks.run_validator')
+    def test_rdf_parse_errors_are_ignored(self, run_validator,
+                                          flag_is_active):
+        run_validator.return_value = self.compatibility_result
+        flag_is_active.return_value = True
+        addon = Addon.objects.get(pk=3615)
+        dupe_xpi = self.get_upload('extension.xpi')
+        d = parse_addon(dupe_xpi.path)
+        # Set up a duplicate upload:
+        addon.update(guid=d['guid'])
+        data = self.upload(filename=dupe_xpi.path)
+        # Make sure we don't see a dupe UUID error:
+        eq_(data['validation']['messages'], [])

apps/devhub/urls.py

@@ -152,6 +152,10 @@
     url('^upload$', views.upload, name='devhub.upload'),
     url('^upload/([^/]+)(?:/([^/]+))?$', views.upload_detail,
         name='devhub.upload_detail'),
+    url('^standalone-upload$', views.standalone_upload,
+        name='devhub.standalone_upload'),
+    url('^standalone-upload/([^/]+)$', views.standalone_upload_detail,
+        name='devhub.standalone_upload_detail'),
 
     # URLs for a single add-on.
     url('^addon/%s/' % ADDON_ID, include(detail_patterns)),

apps/devhub/views.py

@@ -512,7 +512,7 @@ def compat_application_versions(request):
 def validate_addon(request):
     return jingo.render(request, 'devhub/validate_addon.html',
                         {'title': _('Validate Add-on'),
-                         'upload_url': reverse('devhub.upload')})
+                         'upload_url': reverse('devhub.standalone_upload')})
 
 
 @login_required
@@ -521,7 +521,7 @@ def check_addon_compatibility(request):
     return jingo.render(request, 'devhub/validate_addon.html',
                         {'appversion_form': form,
                          'title': _('Check Add-on Compatibility'),
-                         'upload_url': reverse('devhub.upload')})
+                         'upload_url': reverse('devhub.standalone_upload')})
 
 
 def packager_path(name):
@@ -594,7 +594,7 @@ def package_addon_download(request, id):
 
 @login_required
 @post_required
-def upload(request, addon_slug=None):
+def upload(request, addon_slug=None, is_standalone=False):
     filedata = request.FILES['upload']
 
     fu = FileUpload.from_post(filedata, filedata.name, filedata.size)
@@ -612,10 +612,25 @@ def upload(request, addon_slug=None):
         and addon_slug):
         return redirect('devhub.upload_detail_for_addon',
                         addon_slug, fu.pk)
+    elif is_standalone:
+        return redirect('devhub.standalone_upload_detail', fu.pk)
     else:
         return redirect('devhub.upload_detail', fu.pk, 'json')
 
 
+@login_required
+@post_required
+def standalone_upload(request):
+    return upload(request, is_standalone=True)
+
+
+@login_required
+@json_view
+def standalone_upload_detail(request, uuid):
+    upload = get_object_or_404(FileUpload.uncached, uuid=uuid)
+    return upload_validation_context(request, upload)
+
+
 @post_required
 @dev_required
 def upload_for_addon(request, addon_id, addon):
@@ -752,23 +767,10 @@ def json_upload_detail(request, upload, addon_slug=None):
     addon = None
     if addon_slug:
         addon = get_object_or_404(Addon, slug=addon_slug)
-    if not settings.VALIDATE_ADDONS:
-        upload.task_error = ''
-        upload.validation = json.dumps({'errors': 0, 'messages': [],
-                                        'notices': 0, 'warnings': 0})
-        upload.save()
-
-    validation = json.loads(upload.validation) if upload.validation else ""
-    if waffle.flag_is_active(request, 'form-errors-in-validation') and addon:
-        url = reverse('devhub.upload_detail_for_addon',
-                      args=[addon.slug, upload.uuid])
-    else:
-        url = reverse('devhub.upload_detail', args=[upload.uuid, 'json'])
-    full_report_url = reverse('devhub.upload_detail', args=[upload.uuid])
+    result = upload_validation_context(request, upload, addon=addon)
     plat_exclude = []
-
-    if validation:
-        if validation['errors'] == 0:
+    if result['validation']:
+        if result['validation']['errors'] == 0:
             try:
                 apps = parse_addon(upload.path, addon=addon).get('apps', [])
                 app_ids = set([a.id for a in apps])
@@ -797,11 +799,31 @@ def json_upload_detail(request, upload, addon_slug=None):
                 else:
                     log.error("XPI parsing error, ignored: %s" % exc)
 
+    result['platforms_to_exclude'] = plat_exclude
+    return result
+
+
+def upload_validation_context(request, upload, addon_slug=None, addon=None):
+    if addon_slug and not addon:
+        addon = get_object_or_404(Addon, slug=addon_slug)
+    if not settings.VALIDATE_ADDONS:
+        upload.task_error = ''
+        upload.validation = json.dumps({'errors': 0, 'messages': [],
+                                        'notices': 0, 'warnings': 0})
+        upload.save()
+
+    validation = json.loads(upload.validation) if upload.validation else ""
+    if waffle.flag_is_active(request, 'form-errors-in-validation') and addon:
+        url = reverse('devhub.upload_detail_for_addon',
+                      args=[addon.slug, upload.uuid])
+    else:
+        url = reverse('devhub.upload_detail', args=[upload.uuid, 'json'])
+    full_report_url = reverse('devhub.upload_detail', args=[upload.uuid])
+
     return make_validation_result(dict(upload=upload.uuid,
                                        validation=validation,
                                        error=upload.task_error, url=url,
-                                       full_report_url=full_report_url,
-                                       platforms_to_exclude=plat_exclude))
+                                       full_report_url=full_report_url))
 
 
 @login_required

media/js/zamboni/upload.js

@@ -424,7 +424,8 @@
                         $(".platform:hidden").show();
                         $('.platform label').removeClass('platform-disabled');
                         $('input.platform').attr('disabled', false);
-                        if (results.platforms_to_exclude.length) {
+                        if (results.platforms_to_exclude &&
+                            results.platforms_to_exclude.length) {
                             // e.g. after uploading a Mobile add-on
                             var excluded = false;
                             $('input.platform').each(function(e) {