From: Veronika Kabatova <vkabatov@redhat.com>
Date: Fri, 6 Apr 2018 11:24:49 +0000 (+0200)
Subject: api: Show all headers with the same key
X-Git-Tag: v2.1.0-rc1~22
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=608f7bda1f93d451aee244821f5e1ed57df2b23a;p=thirdparty%2Fpatchwork.git

api: Show all headers with the same key

While the code on our side returns all (key, value) pairs for email
headers, Django's REST framework probably uses dictionaries behind the
scenes. This means that having multiple headers with same key (eg
'Received', which is totally valid and common situation), only one of
these headers is visible in the REST API.

Let's hack around this by returning a list of values in case the key is
present multiple times.

Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
Reviewed-by: Stephen Finucane <stephen@that.guru>
---

diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
index fc7ae97b..46eab870 100644
--- a/patchwork/api/cover.py
+++ b/patchwork/api/cover.py
@@ -60,8 +60,18 @@ class CoverLetterDetailSerializer(CoverLetterListSerializer):
     headers = SerializerMethodField()
 
     def get_headers(self, instance):
+        headers = {}
+
         if instance.headers:
-            return email.parser.Parser().parsestr(instance.headers, True)
+            parsed = email.parser.Parser().parsestr(instance.headers, True)
+            for key in parsed.keys():
+                headers[key] = parsed.get_all(key)
+                # Let's return a single string instead of a list if only one
+                # header with this key is present
+                if len(headers[key]) == 1:
+                    headers[key] = headers[key][0]
+
+        return headers
 
     class Meta:
         model = CoverLetter
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 115feffa..645b0e9d 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -123,8 +123,18 @@ class PatchDetailSerializer(PatchListSerializer):
     prefixes = SerializerMethodField()
 
     def get_headers(self, patch):
+        headers = {}
+
         if patch.headers:
-            return email.parser.Parser().parsestr(patch.headers, True)
+            parsed = email.parser.Parser().parsestr(patch.headers, True)
+            for key in parsed.keys():
+                headers[key] = parsed.get_all(key)
+                # Let's return a single string instead of a list if only one
+                # header with this key is present
+                if len(headers[key]) == 1:
+                    headers[key] = headers[key][0]
+
+        return headers
 
     def get_prefixes(self, instance):
         return clean_subject(instance.name)[1]
diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py
index 3135b7e6..4c0c5284 100644
--- a/patchwork/tests/api/test_cover.py
+++ b/patchwork/tests/api/test_cover.py
@@ -17,6 +17,7 @@
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
+import email.parser
 import unittest
 
 from django.conf import settings
@@ -109,12 +110,21 @@ class TestCoverLetterAPI(APITestCase):
 
     def test_detail(self):
         """Validate we can get a specific cover letter."""
-        cover_obj = create_cover()
+        cover_obj = create_cover(
+            headers='Received: from somewhere\nReceived: from another place'
+        )
 
         resp = self.client.get(self.api_url(cover_obj.id))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertSerialized(cover_obj, resp.data)
 
+        # Make sure we don't regress and all headers with the same key are
+        # included in the response
+        parsed_headers = email.parser.Parser().parsestr(cover_obj.headers,
+                                                        True)
+        for key, value in parsed_headers.items():
+            self.assertIn(value, resp.data['headers'][key])
+
     def test_create_update_delete(self):
         user = create_maintainer()
         user.is_superuser = True
diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
index 909c1eb4..40ca7771 100644
--- a/patchwork/tests/api/test_patch.py
+++ b/patchwork/tests/api/test_patch.py
@@ -17,6 +17,7 @@
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
+import email.parser
 from email.utils import make_msgid
 import unittest
 
@@ -120,12 +121,20 @@ class TestPatchAPI(APITestCase):
     def test_detail(self):
         """Validate we can get a specific patch."""
         patch = create_patch(
-            content='Reviewed-by: Test User <test@example.com>\n')
+            content='Reviewed-by: Test User <test@example.com>\n',
+            headers='Received: from somewhere\nReceived: from another place'
+        )
 
         resp = self.client.get(self.api_url(patch.id))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertSerialized(patch, resp.data)
-        self.assertEqual(patch.headers, resp.data['headers'] or '')
+
+        # Make sure we don't regress and all headers with the same key are
+        # included in the response
+        parsed_headers = email.parser.Parser().parsestr(patch.headers, True)
+        for key, value in parsed_headers.items():
+            self.assertIn(value, resp.data['headers'][key])
+
         self.assertEqual(patch.content, resp.data['content'])
         self.assertEqual(patch.diff, resp.data['diff'])
         self.assertEqual(0, len(resp.data['tags']))