diff mbox

[02/15] parsemail: Make find_content() return a MailContent object

Message ID 1444387202-25735-3-git-send-email-damien.lespiau@intel.com
State Superseded
Headers show

Commit Message

Damien Lespiau Oct. 9, 2015, 10:39 a.m. UTC 
Returning tuples is not super scalable. I now want to return a Series
object along with a patch or a comment. So let's put the return values
into a class, used as a typed dictionary really.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 patchwork/bin/parsemail.py          | 29 +++++++++++------
 patchwork/tests/test_patchparser.py | 64 ++++++++++++++++++++++++++-----------
 2 files changed, 65 insertions(+), 28 deletions(-)

Comments

Stephen Finucane Oct. 10, 2015, 3:30 a.m. UTC | #1 
> Returning tuples is not super scalable. I now want to return a Series

> object along with a patch or a comment. So let's put the return values

> into a class, used as a typed dictionary really.

> 

> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>


This isn't necessary at this point, but I'm going to assume you're expanding the amount of items returned in a forthcoming patch (i.e. a series) so...

Acked-by: Stephen Finucane <stephen.finucane@intel.com>
diff mbox

Patch

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index ae588f0..e61f1f4 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -147,6 +147,7 @@  def find_pull_request(content):
         return match.group(1)
     return None
 
+
 def try_decode(payload, charset):
     try:
         payload = unicode(payload, charset)
@@ -154,6 +155,11 @@  def try_decode(payload, charset):
         return None
     return payload
 
+class MailContent:
+    def __init__(self):
+        self.patch = None
+        self.comment = None
+
 def find_content(project, mail):
     patchbuf = None
     commentbuf = ''
@@ -192,7 +198,7 @@  def find_content(project, mail):
 
             # Could not find a valid decoded payload.  Fail.
             if payload is None:
-                return (None, None)
+                return None
 
         if subtype in ['x-patch', 'x-diff']:
             patchbuf = payload
@@ -209,32 +215,31 @@  def find_content(project, mail):
             if c is not None:
                 commentbuf += c.strip() + '\n'
 
-    patch = None
-    comment = None
+    ret = MailContent()
 
     if pullurl or patchbuf:
         (name, prefixes) = clean_subject(mail.get('Subject'),
                                          [project.linkname])
-        patch = Patch(name = name, pull_url = pullurl, content = patchbuf,
+        ret.patch = Patch(name = name, pull_url = pullurl, content = patchbuf,
                     date = mail_date(mail), headers = mail_headers(mail))
 
     if commentbuf:
         # If this is a new patch, we defer setting comment.patch until
         # patch has been saved by the caller
-        if patch:
-            comment = Comment(date = mail_date(mail),
+        if ret.patch:
+            ret.comment = Comment(date = mail_date(mail),
                     content = clean_content(commentbuf),
                     headers = mail_headers(mail))
 
         else:
             cpatch = find_patch_for_comment(project, mail)
             if not cpatch:
-                return (None, None)
-            comment = Comment(patch = cpatch, date = mail_date(mail),
+                return ret
+            ret.comment = Comment(patch = cpatch, date = mail_date(mail),
                     content = clean_content(commentbuf),
                     headers = mail_headers(mail))
 
-    return (patch, comment)
+    return ret
 
 def find_patch_for_comment(project, mail):
     # construct a list of possible reply message ids
@@ -371,7 +376,11 @@  def parse_mail(mail):
 
     (author, save_required) = find_author(mail)
 
-    (patch, comment) = find_content(project, mail)
+    content = find_content(project, mail)
+    if not content:
+        return 0
+    patch = content.patch
+    comment = content.comment
 
     if patch:
         # we delay the saving until we know we have a patch.
diff --git a/patchwork/tests/test_patchparser.py b/patchwork/tests/test_patchparser.py
index eb83220..a70def6 100644
--- a/patchwork/tests/test_patchparser.py
+++ b/patchwork/tests/test_patchparser.py
@@ -43,7 +43,9 @@  class InlinePatchTest(PatchTest):
     def setUp(self):
         self.orig_patch = read_patch(self.patch_filename)
         email = create_email(self.test_comment + '\n' + self.orig_patch)
-        (self.patch, self.comment) = find_content(self.project, email)
+        content = find_content(self.project, email)
+        self.patch = content.patch
+        self.comment = content.comment
 
     def testPatchPresence(self):
         self.assertTrue(self.patch is not None)
@@ -68,7 +70,9 @@  class AttachmentPatchTest(InlinePatchTest):
         email = create_email(self.test_comment, multipart = True)
         attachment = MIMEText(self.orig_patch, _subtype = self.content_subtype)
         email.attach(attachment)
-        (self.patch, self.comment) = find_content(self.project, email)
+        content = find_content(self.project, email)
+        self.patch = content.patch
+        self.comment = content.comment
 
 class AttachmentXDiffPatchTest(AttachmentPatchTest):
     content_subtype = 'x-diff'
@@ -81,7 +85,9 @@  class UTF8InlinePatchTest(InlinePatchTest):
         self.orig_patch = read_patch(self.patch_filename, self.patch_encoding)
         email = create_email(self.test_comment + '\n' + self.orig_patch,
                              content_encoding = self.patch_encoding)
-        (self.patch, self.comment) = find_content(self.project, email)
+        content = find_content(self.project, email)
+        self.patch = content.patch
+        self.comment = content.comment
 
 class NoCharsetInlinePatchTest(InlinePatchTest):
     """ Test mails with no content-type or content-encoding header """
@@ -92,7 +98,9 @@  class NoCharsetInlinePatchTest(InlinePatchTest):
         email = create_email(self.test_comment + '\n' + self.orig_patch)
         del email['Content-Type']
         del email['Content-Transfer-Encoding']
-        (self.patch, self.comment) = find_content(self.project, email)
+        content = find_content(self.project, email)
+        self.patch = content.patch
+        self.comment = content.comment
 
 class SignatureCommentTest(InlinePatchTest):
     patch_filename = '0001-add-line.patch'
@@ -103,8 +111,9 @@  class SignatureCommentTest(InlinePatchTest):
         email = create_email( \
                 self.test_comment + '\n' + \
                 '-- \nsig\n' + self.orig_patch)
-        (self.patch, self.comment) = find_content(self.project, email)
-
+        content = find_content(self.project, email)
+        self.patch = content.patch
+        self.comment = content.comment
 
 class ListFooterTest(InlinePatchTest):
     patch_filename = '0001-add-line.patch'
@@ -117,7 +126,9 @@  class ListFooterTest(InlinePatchTest):
                 '_______________________________________________\n' + \
                 'Linuxppc-dev mailing list\n' + \
                 self.orig_patch)
-        (self.patch, self.comment) = find_content(self.project, email)
+        content = find_content(self.project, email)
+        self.patch = content.patch
+        self.comment = content.comment
 
 
 class DiffWordInCommentTest(InlinePatchTest):
@@ -190,8 +201,8 @@  class SubjectEncodingTest(PatchTest):
         self.email = message_from_string(mail)
 
     def testSubjectEncoding(self):
-        (patch, comment) = find_content(self.project, self.email)
-        self.assertEquals(patch.name, self.subject)
+        content = find_content(self.project, self.email)
+        self.assertEquals(content.patch.name, self.subject)
 
 class SubjectUTF8QPEncodingTest(SubjectEncodingTest):
     subject = u'test s\xfcbject'
@@ -359,7 +370,10 @@  class GitPullTest(MBoxPatchTest):
     mail_file = '0001-git-pull-request.mbox'
 
     def testGitPullRequest(self):
-        (patch, comment) = find_content(self.project, self.mail)
+        content = find_content(self.project, self.mail)
+        patch = content.patch
+        comment = content.comment
+
         self.assertTrue(patch is not None)
         self.assertTrue(patch.pull_url is not None)
         self.assertTrue(patch.content is None)
@@ -372,7 +386,10 @@  class GitPullWithDiffTest(MBoxPatchTest):
     mail_file = '0003-git-pull-request-with-diff.mbox'
 
     def testGitPullWithDiff(self):
-        (patch, comment) = find_content(self.project, self.mail)
+        content = find_content(self.project, self.mail)
+        patch = content.patch
+        comment = content.comment
+
         self.assertTrue(patch is not None)
         self.assertEqual('git://git.kernel.org/pub/scm/linux/kernel/git/tip/' +
              'linux-2.6-tip.git x86-fixes-for-linus', patch.pull_url)
@@ -394,7 +411,10 @@  class GitRenameOnlyTest(MBoxPatchTest):
     mail_file = '0008-git-rename.mbox'
 
     def testGitRename(self):
-        (patch, comment) = find_content(self.project, self.mail)
+        content = find_content(self.project, self.mail)
+        patch = content.patch
+        comment = content.comment
+
         self.assertTrue(patch is not None)
         self.assertTrue(comment is not None)
         self.assertEqual(patch.content.count("\nrename from "), 2)
@@ -404,7 +424,10 @@  class GitRenameWithDiffTest(MBoxPatchTest):
     mail_file = '0009-git-rename-with-diff.mbox'
 
     def testGitRename(self):
-        (patch, comment) = find_content(self.project, self.mail)
+        content = find_content(self.project, self.mail)
+        patch = content.patch
+        comment = content.comment
+
         self.assertTrue(patch is not None)
         self.assertTrue(comment is not None)
         self.assertEqual(patch.content.count("\nrename from "), 2)
@@ -415,7 +438,10 @@  class CVSFormatPatchTest(MBoxPatchTest):
     mail_file = '0007-cvs-format-diff.mbox'
 
     def testPatch(self):
-        (patch, comment) = find_content(self.project, self.mail)
+        content = find_content(self.project, self.mail)
+        patch = content.patch
+        comment = content.comment
+
         self.assertTrue(patch is not None)
         self.assertTrue(comment is not None)
         self.assertTrue(patch.content.startswith('Index'))
@@ -427,15 +453,17 @@  class CharsetFallbackPatchTest(MBoxPatchTest):
     mail_file = '0010-invalid-charset.mbox'
 
     def testPatch(self):
-        (patch, comment) = find_content(self.project, self.mail)
-        self.assertTrue(patch is not None)
-        self.assertTrue(comment is not None)
+        content = find_content(self.project, self.mail)
+        self.assertTrue(content.patch is not None)
+        self.assertTrue(content.comment is not None)
 
 class NoNewlineAtEndOfFilePatchTest(MBoxPatchTest):
     mail_file = '0011-no-newline-at-end-of-file.mbox'
 
     def testPatch(self):
-        (patch, comment) = find_content(self.project, self.mail)
+        content = find_content(self.project, self.mail)
+        patch = content.patch
+        comment = content.comment
         self.assertTrue(patch is not None)
         self.assertTrue(comment is not None)
         self.assertTrue(patch.content.startswith('diff --git a/tools/testing/selftests/powerpc/Makefile'))