[2/6] views: Make 'patch_to_mbox' return a string

The 'patch_to_mbox' function returns an object which is coverted to a
string in all places where this call occurs. The string conversion
differs between Python 2 and 3 and while one it has been updated in one
place, it was missed in two others. Resole these issues and ensure they
don't happen again by returns strings from 'patch_to_mbox' instead.

Signed-off-by: Stephen Finucane <stephen@that.guru>
---
 patchwork/views/__init__.py | 7 +++++++
 patchwork/views/bundle.py   | 9 +++------
 patchwork/views/patch.py    | 9 +++------
 patchwork/views/xmlrpc.py   | 2 +-
 4 files changed, 14 insertions(+), 13 deletions(-)

Hi,

> The 'patch_to_mbox' function returns an object which is coverted to a
> string in all places where this call occurs. The string conversion
> differs between Python 2 and 3 and while one it has been updated in one
> place, it was missed in two others. Resole these issues and ensure they
> don't happen again by returns strings from 'patch_to_mbox' instead.

Let me just reset the "Days since we encountered a bug that would have
been avoided with a rigorous type system" counter to zero.

> +    # NOTE(stephenfin) http://stackoverflow.com/a/28584090/613428
> +    if six.PY3:
> +        mail = mail.as_bytes(True).decode()
I'm a little nervous about a decode without error handling, but I'm
pretty confident all the data that comes here will be able to be decoded
fine.

Reviewed-by: Daniel Axtens <dja@axtens.net>

Regards,
Daniel

> +    else:
> +        mail = mail.as_string(True)
> +
>      return mail
> diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
> index b99287b..aea828d 100644
> --- a/patchwork/views/bundle.py
> +++ b/patchwork/views/bundle.py
> @@ -130,18 +130,15 @@ def bundle_detail(request, username, bundlename):
>  def bundle_mbox(request, username, bundlename):
>      bundle = get_object_or_404(Bundle, owner__username=username,
>                                 name=bundlename)
> -
> -    if not (request.user == bundle.owner or bundle.public):
> +    if request.user != bundle.owner and not bundle.public:
>          return HttpResponseNotFound()
>  
> -    mbox = '\n'.join([patch_to_mbox(p).as_string(True)
> -                      for p in bundle.ordered_patches()])
> -
>      response = HttpResponse(content_type='text/plain')
>      response['Content-Disposition'] = \
>          'attachment; filename=bundle-%d-%s.mbox' % (bundle.id, bundle.name)
> +    response.write('\n'.join(
> +        [patch_to_mbox(p) for p in bundle.ordered_patches()]))
>  
> -    response.write(mbox)
>      return response
>  
>  
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index 469a53a..705359f 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -26,7 +26,6 @@ from django.http import HttpResponse
>  from django.http import HttpResponseForbidden
>  from django.http import HttpResponseRedirect
>  from django.shortcuts import render, get_object_or_404
> -from django.utils import six
>  
>  from patchwork.forms import PatchForm, CreateBundleForm
>  from patchwork.models import Patch, Project, Bundle, Submission
> @@ -121,17 +120,15 @@ def patch_raw(request, patch_id):
>      response.write(patch.diff)
>      response['Content-Disposition'] = 'attachment; filename=' + \
>          patch.filename.replace(';', '').replace('\n', '')
> +
>      return response
>  
>  
>  def patch_mbox(request, patch_id):
>      patch = get_object_or_404(Patch, id=patch_id)
>      response = HttpResponse(content_type="text/plain")
> -    # NOTE(stephenfin) http://stackoverflow.com/a/28584090/613428
> -    if six.PY3:
> -        response.write(patch_to_mbox(patch).as_bytes(True).decode())
> -    else:
> -        response.write(patch_to_mbox(patch).as_string(True))
> +    response.write(patch_to_mbox(patch))
>      response['Content-Disposition'] = 'attachment; filename=' + \
>          patch.filename.replace(';', '').replace('\n', '')
> +
>      return response
> diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
> index bfbdef0..a8dc797 100644
> --- a/patchwork/views/xmlrpc.py
> +++ b/patchwork/views/xmlrpc.py
> @@ -695,7 +695,7 @@ def patch_get_mbox(patch_id):
>      """
>      try:
>          patch = Patch.objects.get(id=patch_id)
> -        return patch_to_mbox(patch).as_string(True)
> +        return patch_to_mbox(patch)
>      except Patch.DoesNotExist:
>          return ''
>  
> -- 
> 2.9.3
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

On Thu, 2017-02-02 at 13:58 +1100, Daniel Axtens wrote:
> Hi,
> 
> > The 'patch_to_mbox' function returns an object which is coverted to
> > a
> > string in all places where this call occurs. The string conversion
> > differs between Python 2 and 3 and while one it has been updated in
> > one
> > place, it was missed in two others. Resole these issues and ensure
> > they
> > don't happen again by returns strings from 'patch_to_mbox' instead.
> 
> Let me just reset the "Days since we encountered a bug that would
> have
> been avoided with a rigorous type system" counter to zero.

It might be a good idea to integrate mypy into this section of the code
in the future. For now though, this will have to do.

Stephen