Merge pull request #2452 from onaio/optimize-attachments-endpoint

Optimize attachments endpoint

onadata/apps/api/tests/viewsets/test_briefcase_viewset.py

@@ -301,7 +301,8 @@ def test_view_downloadSubmission(self):
             text = f.read()
             for var in ((u'{{submissionDate}}',
                          instance.date_created.isoformat()),
-                        (u'{{form_id}}', str(self.xform.id))):
+                        (u'{{form_id}}', str(self.xform.id)),
+                        (u'{{media_id}}', str(self.attachment.id))):
                 text = text.replace(*var)
             self.assertContains(response, instanceId, status_code=200)
             self.assertMultiLineEqual(response.content.decode('utf-8'), text)
@@ -466,7 +467,8 @@ def test_view_downloadSubmission_no_xmlns(self, mock_get_object):
             text = f.read()
             for var in ((u'{{submissionDate}}',
                          instance.date_created.isoformat()),
-                        (u'{{form_id}}', str(self.xform.id))):
+                        (u'{{form_id}}', str(self.xform.id)),
+                        (u'{{media_id}}', str(self.attachment.id))):
                 text = text.replace(*var)
             self.assertNotIn(
                 'transportation id="transportation_2011_07_25"'
@@ -521,7 +523,8 @@ def test_view_downloadSubmission_multiple_nodes(self, mock_get_object):
             text = f.read()
             for var in ((u'{{submissionDate}}',
                          instance.date_created.isoformat()),
-                        (u'{{form_id}}', str(self.xform.id))):
+                        (u'{{form_id}}', str(self.xform.id)),
+                        (u'{{media_id}}', str(self.attachment.id))):
                 text = text.replace(*var)
             self.assertContains(response, instanceId, status_code=200)
 

onadata/apps/logger/templates/downloadSubmission.xml

@@ -6,6 +6,6 @@
     {% for media in media_files %}<mediaFile>
         <filename>{{ media.name|safe }}</filename>
         <hash>md5:{{ media.file_hash }}</hash>
-        <downloadUrl>{{ host }}{% url "attachment_url" 'original' %}?media_file={{ media.media_file.name|safe }}</downloadUrl>
+        <downloadUrl>{{ host }}{% url "attachment_url" 'original' %}?media_file={{ media.media_file.name|safe }}&amp;attachment_id={{ media.pk|stringformat:"d" }}</downloadUrl>
     </mediaFile>{% endfor %}
 </submission>

onadata/apps/logger/tests/test_briefcase_api.py

@@ -223,9 +223,9 @@ def test_view_downloadSubmission(self):
             for var in (
                 ("{{submissionDate}}", instance.date_created.isoformat()),
                 ("{{form_id}}", str(self.xform.id)),
+                ("{{media_id}}", str(self.attachment.id)),
             ):
                 text = text.replace(*var)
-
             self.assertContains(response, instanceId, status_code=200)
             self.assertMultiLineEqual(response.content.decode("utf-8"), text)
 

onadata/apps/main/tests/fixtures/transportation/view/downloadSubmission.xml

@@ -6,6 +6,6 @@
     <mediaFile>
         <filename>1335783522563.jpg</filename>
         <hash>md5:2ca0d22073a9b6b4ebe51368b08da60c</hash>
-        <downloadUrl>http://testserver/attachment/original?media_file=bob/attachments/{{form_id}}_transportation_2011_07_25/1335783522563.jpg</downloadUrl>
+        <downloadUrl>http://testserver/attachment/original?media_file=bob/attachments/{{form_id}}_transportation_2011_07_25/1335783522563.jpg&amp;attachment_id={{media_id}}</downloadUrl>
     </mediaFile>
 </submission>

onadata/apps/viewer/tests/test_attachment_url.py

@@ -43,6 +43,24 @@ def test_attachment_has_mimetype(self):
         attachment = Attachment.objects.all().reverse()[0]
         self.assertEqual(attachment.mimetype, 'image/jpeg')
 
+    def test_attachment_url_w_media_id(self):
+        """Test attachment url with attachment id"""
+        self.assertEqual(
+            Attachment.objects.count(), self.attachment_count + 1)
+        response = self.client.get(
+            self.url, {"attachment_id": self.attachment.id})
+        self.assertEqual(response.status_code, 302)  # redirects to amazon
+
+    # pylint: disable=invalid-name
+    def test_attachment_url_w_media_id_no_redirect(self):
+        """Test attachment url with attachment id no redirect"""
+        self.assertEqual(
+            Attachment.objects.count(), self.attachment_count + 1)
+        response = self.client.get(
+            self.url, {"attachment_id": self.attachment.id,
+                       'no_redirect': 'true'})
+        self.assertEqual(response.status_code, 200)  # no redirects to amazon
+
     def tearDown(self):
         path = os.path.join(settings.MEDIA_ROOT, self.user.username)
         for root, dirs, files in os.walk(path, topdown=False):

onadata/apps/viewer/views.py

@@ -866,13 +866,17 @@ def attachment_url(request, size="medium"):
     """
     media_file = request.GET.get("media_file")
     no_redirect = request.GET.get("no_redirect")
-    if not media_file:
-        return HttpResponseNotFound(_("Attachment not found"))
+    attachment_id = request.GET.get("attachment_id")
 
-    result = Attachment.objects.filter(media_file=media_file).order_by()[0:1]
-    if not result:
+    if not media_file and not attachment_id:
         return HttpResponseNotFound(_("Attachment not found"))
-    attachment = result[0]
+    if attachment_id:
+        attachment = get_object_or_404(Attachment, pk=attachment_id)
+    else:
+        result = Attachment.objects.filter(media_file=media_file).order_by()[0:1]
+        if not result:
+            return HttpResponseNotFound(_("Attachment not found"))
+        attachment = result[0]
 
     if size == "original" and no_redirect == "true":
         response = response_with_mimetype_and_name(