| Message ID | 20180406112449.17184-1-vkabatov@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | api: Show all headers with the same key | expand |
On Fri, 2018-04-06 at 13:24 +0200, vkabatov@redhat.com wrote: > From: Veronika Kabatova <vkabatov@redhat.com> > > 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> Sorry for the delay in getting to this. This looks good but I feel it needs a regression test. If you could provide one, I'd be happy to squash it in here and apply this. Cheers, Stephen > --- > patchwork/api/cover.py | 12 +++++++++++- > patchwork/api/patch.py | 12 +++++++++++- > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py > index 1064504..dd1db97 100644 > --- a/patchwork/api/cover.py > +++ b/patchwork/api/cover.py > @@ -57,8 +57,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 115feff..645b0e9 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]
On Tue, 2018-04-24 at 23:23 +0100, Stephen Finucane wrote: > On Fri, 2018-04-06 at 13:24 +0200, vkabatov@redhat.com wrote: > > From: Veronika Kabatova <vkabatov@redhat.com> > > > > 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> > > Sorry for the delay in getting to this. This looks good but I feel it > needs a regression test. If you could provide one, I'd be happy to > squash it in here and apply this. With the follow up patch added to this, I'm happy to apply it. Reviewed-by: Stephen Finucane <stephen@that.guru>
diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index 1064504..dd1db97 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -57,8 +57,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 115feff..645b0e9 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]