diff mbox

Fix parsing of un-numbered messages in series

Message ID 1479275924-4628-1-git-send-email-dja@axtens.net
State Accepted
Headers show

Commit Message

Daniel Axtens Nov. 16, 2016, 5:58 a.m. UTC 
Say we are sent the following:

          - [PATCH 0/2] A sample series
            - [PATCH 1/2] test: Add some lorem ipsum
            - Random message with diff

We expect that:
 1) we parse normally without errors
 2) we get a series with a cover letter and a patch
 3) the random message is orphaned

What happens is that we get an integrity error, boiling down to:

(1048, "Column 'number' cannot be null")

This is caused because we believe that the random message belongs
to the series because of the headers, but because there are no
numbers in the Subject, we pass "None" into the number field of
SeriesPatch. That turns into a null, and rightly hits an integrity
error.

Fix this by requring that a message has a series _and_ a number
before we try to add it to the series.

Add a test to verify correctness.

Reported-by: Daniel Wagner <wagi@monom.org>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 patchwork/parser.py                         |   5 +-
 patchwork/tests/series/bugs-unnumbered.mbox | 124 ++++++++++++++++++++++++++++
 patchwork/tests/test_series.py              |  21 +++++
 3 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 patchwork/tests/series/bugs-unnumbered.mbox

Comments

Stephen Finucane Nov. 18, 2016, 1:03 a.m. UTC | #1 
On Wed, 2016-11-16 at 16:58 +1100, Daniel Axtens wrote:
> Say we are sent the following:
> 
>           - [PATCH 0/2] A sample series
>             - [PATCH 1/2] test: Add some lorem ipsum
>             - Random message with diff
> 
> We expect that:
>  1) we parse normally without errors
>  2) we get a series with a cover letter and a patch
>  3) the random message is orphaned
> 
> What happens is that we get an integrity error, boiling down to:
> 
> (1048, "Column 'number' cannot be null")
> 
> This is caused because we believe that the random message belongs
> to the series because of the headers, but because there are no
> numbers in the Subject, we pass "None" into the number field of
> SeriesPatch. That turns into a null, and rightly hits an integrity
> error.
> 
> Fix this by requring that a message has a series _and_ a number
> before we try to add it to the series.
> 
> Add a test to verify correctness.
> 
> Reported-by: Daniel Wagner <wagi@monom.org>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Looks good to me. Appreciate the sanity check test.

Reviewed-by: Stephen Finucane <stephen@that.guru>

...and applied.
Daniel Wagner Nov. 18, 2016, 9:29 a.m. UTC | #2 
>> Add a test to verify correctness.
>>
>> Reported-by: Daniel Wagner <wagi@monom.org>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>
> Looks good to me. Appreciate the sanity check test.
>
> Reviewed-by: Stephen Finucane <stephen@that.guru>
>
> ...and applied.	

Updated my installation. Let's see what happens now.

Thanks,
Daniel
diff mbox

Patch

diff --git a/patchwork/parser.py b/patchwork/parser.py
index a4036ff1e547..b1544c951a3f 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -820,7 +820,10 @@  def parse_mail(mail, list_id=None):
         patch.save()
         logger.debug('Patch saved')
 
-        if series:
+        # add to a series if we have found one, and we have a numbered
+        # patch. Don't add unnumbered patches (for example diffs sent
+        # in reply, or just messages with random refs/in-reply-tos)
+        if series and x:
             series.add_patch(patch, x)
 
         return patch
diff --git a/patchwork/tests/series/bugs-unnumbered.mbox b/patchwork/tests/series/bugs-unnumbered.mbox
new file mode 100644
index 000000000000..1448e99d6b23
--- /dev/null
+++ b/patchwork/tests/series/bugs-unnumbered.mbox
@@ -0,0 +1,124 @@ 
+From stephenfinucane@gmail.com Sun Sep 11 23:22:13 2016
+Return-Path: <stephenfinucane@gmail.com>
+From: Stephen Finucane <stephenfinucane@gmail.com>
+To: stephenfinucane@hotmail.com
+Subject: [PATCH 0/2] A sample series
+Date: Sun, 11 Sep 2016 23:22:02 +0100
+Message-ID: <1473632524-8585-1-git-send-email-stephenfinucane@gmail.com>
+X-Mailer: git-send-email 2.7.4
+MIME-Version: 1.0
+Content-Type: text/plain
+Content-Length: 395
+Lines: 18
+
+From: Stephen Finucane <stephenfinucane@hotmail.com>
+
+This is a sample cover letter. It doesn't really contribute to the
+conversation, but I added it anway.
+
+Stephen Finucane (2):
+  test: Add some lorem ipsum
+  test: Convert to Markdown
+
+ test.md  | 8 ++++++++
+ test.txt | 1 -
+ 2 files changed, 8 insertions(+), 1 deletion(-)
+ create mode 100644 test.md
+ delete mode 100644 test.txt
+
+-- 
+2.7.4
+
+
+From stephenfinucane@gmail.com Sun Sep 11 23:22:13 2016
+Return-Path: <stephenfinucane@gmail.com>
+From: Stephen Finucane <stephenfinucane@gmail.com>
+To: stephenfinucane@hotmail.com
+Subject: [PATCH 1/2] test: Add some lorem ipsum
+Date: Sun, 11 Sep 2016 23:22:03 +0100
+Message-ID: <1473632524-8585-2-git-send-email-stephenfinucane@gmail.com>
+X-Mailer: git-send-email 2.7.4
+In-Reply-To: <1473632524-8585-1-git-send-email-stephenfinucane@gmail.com>
+References: <1473632524-8585-1-git-send-email-stephenfinucane@gmail.com>
+MIME-Version: 1.0
+Content-Type: text/plain
+Content-Length: 670
+Lines: 22
+
+From: Stephen Finucane <stephenfinucane@hotmail.com>
+
+---
+ test.txt | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/test.txt b/test.txt
+index f75ba05..a6c61c0 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1 +1,8 @@
+ Hello, world.
++
++Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras eget eleifend
++augue. Nullam at metus venenatis, laoreet neque nec, convallis mauris.
++Pellentesque aliquam at nisi et laoreet. Duis non nisl venenatis, rhoncus risus
++id, elementum felis. In hac habitasse platea dictumst. Nam sit amet maximus
++eros. Nam quis ligula ut tortor egestas bibendum. Nunc sed purus sit amet
++tellus commodo bibendum ut vel dolor.
+-- 
+2.7.4
+
+
+From stephenfinucane@gmail.com Sun Sep 11 23:22:16 2016
+Return-Path: <stephenfinucane@gmail.com>
+From: Stephen Finucane <stephenfinucane@gmail.com>
+To: stephenfinucane@hotmail.com
+Subject: This is an orphaned patch!
+Date: Sun, 11 Sep 2016 23:22:04 +0100
+Message-ID: <1473632524-8585-3-git-send-email-stephenfinucane@gmail.com>
+X-Mailer: git-send-email 2.7.4
+In-Reply-To: <1473632524-8585-1-git-send-email-stephenfinucane@gmail.com>
+References: <1473632524-8585-1-git-send-email-stephenfinucane@gmail.com>
+MIME-Version: 1.0
+Content-Type: text/plain
+Content-Length: 1345
+Lines: 40
+
+From: Stephen Finucane <stephenfinucane@hotmail.com>
+
+---
+ test.md  | 8 ++++++++
+ test.txt | 8 --------
+ 2 files changed, 8 insertions(+), 8 deletions(-)
+ create mode 100644 test.md
+ delete mode 100644 test.txt
+
+diff --git a/test.md b/test.md
+new file mode 100644
+index 0000000..e5ff90e
+--- /dev/null
++++ b/test.md
+@@ -0,0 +1,8 @@
++# Hello, world
++
++Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras eget eleifend
++augue. Nullam at metus venenatis, laoreet neque nec, convallis mauris.
++Pellentesque aliquam at nisi et laoreet. Duis non nisl venenatis, rhoncus risus
++id, elementum felis. In hac habitasse platea dictumst. Nam sit amet maximus
++eros. Nam quis ligula ut tortor egestas bibendum. Nunc sed purus sit amet
++tellus commodo bibendum ut vel dolor.
+diff --git a/test.txt b/test.txt
+deleted file mode 100644
+index a6c61c0..0000000
+--- a/test.txt
++++ /dev/null
+@@ -1,8 +0,0 @@
+-Hello, world.
+-
+-Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras eget eleifend
+-augue. Nullam at metus venenatis, laoreet neque nec, convallis mauris.
+-Pellentesque aliquam at nisi et laoreet. Duis non nisl venenatis, rhoncus risus
+-id, elementum felis. In hac habitasse platea dictumst. Nam sit amet maximus
+-eros. Nam quis ligula ut tortor egestas bibendum. Nunc sed purus sit amet
+-tellus commodo bibendum ut vel dolor.
+-- 
+2.7.4
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index 7879db9da5fc..58abb0df303c 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -288,6 +288,27 @@  class RevisedSeriesTest(_BaseTestCase):
         self.assertSerialized(patches, [2, 2])
         self.assertSerialized(covers, [1, 1])
 
+    def test_unnumbered(self):
+        """Series with a reply with a diff but no number.
+
+        The random message with the diff should not belong to the series, as
+        it lacks a n/N label.
+
+        Input:
+
+          - [PATCH 0/2] A sample series
+            - [PATCH 1/2] test: Add some lorem ipsum
+            - Random message with diff
+
+        We expect 1 series and the random message to be orphaned.
+        """
+        covers, patches, _ = self._parse_mbox(
+            'bugs-unnumbered.mbox', [1, 2, 0])
+
+        self.assertEqual(len([p for p in patches if p.latest_series]), 1)
+        self.assertEqual(len([p for p in patches if not p.latest_series]), 1)
+        self.assertSerialized(covers, [1])
+
 
 class SeriesNameTestCase(TestCase):