diff mbox series

[1/3] parser: Don't group patches with different versions in a series

Message ID 20200304172646.49409-1-stephen@that.guru
State Superseded
Headers show
Series [1/3] parser: Don't group patches with different versions in a series | expand

Commit Message

Stephen Finucane March 4, 2020, 5:26 p.m. UTC 
As noted in #340 [1], if a patch from a series is dropped or
miscategorised, patches from a later revision of that series can end up
included in the earlier series rather than in their own series. This was
actually intentional as part of the fix for #105 [2]. However,
completely ignoring this information can be problematic. Refine things
by checking for versions and, if they don't match, using timeboxing to
try guess if they should be kept together. This would resolve the issue
seen in #340 while preventing a regression for #105.

[1] https://github.com/getpatchwork/patchwork/issues/340
[1] https://github.com/getpatchwork/patchwork/issues/105

Signed-off-by: Stephen Finucane <stephen@that.guru>
Related: #340
Related: #105
---
 patchwork/parser.py                           |  18 +++
 .../tests/series/bugs-mixed-versions.mbox     | 110 ++++++++++++++++++
 patchwork/tests/test_series.py                |  17 +++
 3 files changed, 145 insertions(+)
 create mode 100644 patchwork/tests/series/bugs-mixed-versions.mbox

Comments

Aaron Kaufman March 4, 2020, 5:32 p.m. UTC | #1 
You may want to check the most recent android revisions [google's android, not the use of your project with android] because they used almost the exact same type of time boxing-as-a-last-step when attempmting to authenticate SIDs that were trying to get a priv  upgrade.   It's on their list of issues;

As always, I've never been a fan of reinventing the wheel, so if there's some code you can borrow and python-ize, it might be worth a look.


Aaron S. Kaufman, Esq.
353 Veterans Memorial Highway, Ste 210
Commack NY 11725
631-366-2172
Fax: 631-366-1728
Email: aaron@aaronskaufman.com

-----Original Message-----
From: Patchwork <patchwork-bounces+aaron=aaronskaufman.com@lists.ozlabs.org> On Behalf Of Stephen Finucane
Sent: Wednesday, March 4, 2020 12:27 PM
To: patchwork@lists.ozlabs.org
Subject: [PATCH 1/3] parser: Don't group patches with different versions in a series

As noted in #340 [1], if a patch from a series is dropped or miscategorised, patches from a later revision of that series can end up included in the earlier series rather than in their own series. This was actually intentional as part of the fix for #105 [2]. However, completely ignoring this information can be problematic. Refine things by checking for versions and, if they don't match, using timeboxing to try guess if they should be kept together. This would resolve the issue seen in #340 while preventing a regression for #105.

[1] https://github.com/getpatchwork/patchwork/issues/340
[1] https://github.com/getpatchwork/patchwork/issues/105

Signed-off-by: Stephen Finucane <stephen@that.guru>
Related: #340
Related: #105
---
 patchwork/parser.py                           |  18 +++
 .../tests/series/bugs-mixed-versions.mbox     | 110 ++++++++++++++++++
 patchwork/tests/test_series.py                |  17 +++
 3 files changed, 145 insertions(+)
 create mode 100644 patchwork/tests/series/bugs-mixed-versions.mbox

diff --git a/patchwork/parser.py b/patchwork/parser.py index 0e88d934..45930b45 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -250,14 +250,32 @@ def _find_series_by_references(project, mail):
     Returns:
         The matching ``Series`` instance, if any
     """
+    subject = mail.get('Subject')
+    name, prefixes = clean_subject(subject, [project.linkname])
+    version = parse_version(name, prefixes)
+
     refs = find_references(mail)
     h = clean_header(mail.get('Message-Id'))
     if h:
         refs = [h] + refs
+
     for ref in refs:
         try:
             series = SeriesReference.objects.get(
                 msgid=ref[:255], project=project).series
+
+            if series.version != version:
+                # if the versions don't match, at least make sure these were
+                # sent around the same time
+                date = find_date(mail)
+                delta = datetime.timedelta(minutes=SERIES_DELAY_INTERVAL)
+                start_date = date - delta
+                end_date = date + delta
+
+                # ...and if they don't, this probably isn't our series
+                if start_date > series.date > end_date:
+                    continue
+
             # we want to return a queryset like '_find_series_by_markers'
             return Series.objects.filter(id=series.id)
         except SeriesReference.DoesNotExist:
diff --git a/patchwork/tests/series/bugs-mixed-versions.mbox b/patchwork/tests/series/bugs-mixed-versions.mbox
new file mode 100644
index 00000000..1aa639be
--- /dev/null
+++ b/patchwork/tests/series/bugs-mixed-versions.mbox
@@ -0,0 +1,110 @@
+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
+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 18 23:31:07 2016
+Return-Path: <stephenfinucane@gmail.com>
+From: Stephen Finucane <stephenfinucane@gmail.com>
+To: stephenfinucane@hotmail.com
+Subject: [PATCH v2 2/2] test: Convert to Markdown
+Date: Sun, 18 Sep 2016 23:30:55 +0100
+Message-ID: 
+<1473633055-10316-3-git-send-email-stephenfinucane@gmail.com>
+X-Mailer: git-send-email 2.7.4
+In-Reply-To: 
+<1473633055-10316-2-git-send-email-stephenfinucane@gmail.com>
+References: 
+<1473633055-10316-2-git-send-email-stephenfinucane@gmail.com>
+MIME-Version: 1.0
+Content-Type: text/plain
+Content-Length: 2589
+Lines: 58
+
+From: Stephen Finucane <stephenfinucane@hotmail.com>
+
+---
+ test.md  | 17 +++++++++++++++++
+ test.txt | 17 -----------------
+ 2 files changed, 17 insertions(+), 17 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..201bfe7
+--- /dev/null
++++ b/test.md
+@@ -0,0 +1,17 @@
++# 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.
++
++Curabitur scelerisque tempus efficitur. Maecenas aliquet elementum 
++quam, ac pharetra nisi gravida a. Nam vitae malesuada leo. Nam 
++accumsan facilisis metus sed vulputate. Ut libero odio, tempor ac 
++posuere vel, malesuada quis ex. Donec interdum ipsum et nibh molestie auctor eget in neque. Sed nec hendrerit enim.
++Etiam scelerisque non magna eu molestie. Maecenas dui quam, tempor 
++quis ipsum eu, dapibus congue dui. Integer tincidunt accumsan tortor 
++sed maximus. Praesent eu magna accumsan, consequat turpis quis, 
++fringilla ante. Vestibulum nec maximus nulla. Donec vehicula nisl at pharetra eleifend.
+diff --git a/test.txt b/test.txt
+deleted file mode 100644
+index 4478ac2..0000000
+--- a/test.txt
++++ /dev/null
+@@ -1,17 +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.
+-
+-Curabitur scelerisque tempus efficitur. Maecenas aliquet elementum 
+quam, ac -pharetra nisi gravida a. Nam vitae malesuada leo. Nam 
+accumsan facilisis metus -sed vulputate. Ut libero odio, tempor ac 
+posuere vel, malesuada quis ex. Donec -interdum ipsum et nibh molestie auctor eget in neque. Sed nec hendrerit enim.
+-Etiam scelerisque non magna eu molestie. Maecenas dui quam, tempor 
+quis ipsum -eu, dapibus congue dui. Integer tincidunt accumsan tortor 
+sed maximus. Praesent -eu magna accumsan, consequat turpis quis, 
+fringilla ante. Vestibulum nec -maximus nulla. Donec vehicula nisl at pharetra eleifend.
+--
+2.7.4
+
+
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py index 295a049d..2e86e47c 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -538,6 +538,23 @@ class RevisedSeriesTest(_BaseTestCase):
 
         self.assertSerialized(patches, [1, 1, 1])
 
+    def test_mixed_versions(self):
+        """Series with a revision sent in reply to an incompleted series.
+
+        Parse a series with two patches, one of which has been lost or
+        miscategorized, followed by a second revision of the missing patch.
+        None of the patches of the second revision should be included in the
+        first revision.
+
+          - [PATCH 1/2] test: Add some lorem ipsum
+            - [PATCH v2 2/2] test: Convert to Markdown
+        """
+        _, patches, _ = self._parse_mbox(
+            'bugs-mixed-versions.mbox', [0, 2, 0],
+        )
+
+        self.assertSerialized(patches, [1, 1])
+
 
 class SeriesTotalTest(_BaseTestCase):
 
--
2.24.1
diff mbox series

Patch

diff --git a/patchwork/parser.py b/patchwork/parser.py
index 0e88d934..45930b45 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -250,14 +250,32 @@  def _find_series_by_references(project, mail):
     Returns:
         The matching ``Series`` instance, if any
     """
+    subject = mail.get('Subject')
+    name, prefixes = clean_subject(subject, [project.linkname])
+    version = parse_version(name, prefixes)
+
     refs = find_references(mail)
     h = clean_header(mail.get('Message-Id'))
     if h:
         refs = [h] + refs
+
     for ref in refs:
         try:
             series = SeriesReference.objects.get(
                 msgid=ref[:255], project=project).series
+
+            if series.version != version:
+                # if the versions don't match, at least make sure these were
+                # sent around the same time
+                date = find_date(mail)
+                delta = datetime.timedelta(minutes=SERIES_DELAY_INTERVAL)
+                start_date = date - delta
+                end_date = date + delta
+
+                # ...and if they don't, this probably isn't our series
+                if start_date > series.date > end_date:
+                    continue
+
             # we want to return a queryset like '_find_series_by_markers'
             return Series.objects.filter(id=series.id)
         except SeriesReference.DoesNotExist:
diff --git a/patchwork/tests/series/bugs-mixed-versions.mbox b/patchwork/tests/series/bugs-mixed-versions.mbox
new file mode 100644
index 00000000..1aa639be
--- /dev/null
+++ b/patchwork/tests/series/bugs-mixed-versions.mbox
@@ -0,0 +1,110 @@ 
+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
+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 18 23:31:07 2016
+Return-Path: <stephenfinucane@gmail.com>
+From: Stephen Finucane <stephenfinucane@gmail.com>
+To: stephenfinucane@hotmail.com
+Subject: [PATCH v2 2/2] test: Convert to Markdown
+Date: Sun, 18 Sep 2016 23:30:55 +0100
+Message-ID: <1473633055-10316-3-git-send-email-stephenfinucane@gmail.com>
+X-Mailer: git-send-email 2.7.4
+In-Reply-To: <1473633055-10316-2-git-send-email-stephenfinucane@gmail.com>
+References: <1473633055-10316-2-git-send-email-stephenfinucane@gmail.com>
+MIME-Version: 1.0
+Content-Type: text/plain
+Content-Length: 2589
+Lines: 58
+
+From: Stephen Finucane <stephenfinucane@hotmail.com>
+
+---
+ test.md  | 17 +++++++++++++++++
+ test.txt | 17 -----------------
+ 2 files changed, 17 insertions(+), 17 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..201bfe7
+--- /dev/null
++++ b/test.md
+@@ -0,0 +1,17 @@
++# 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.
++
++Curabitur scelerisque tempus efficitur. Maecenas aliquet elementum quam, ac
++pharetra nisi gravida a. Nam vitae malesuada leo. Nam accumsan facilisis metus
++sed vulputate. Ut libero odio, tempor ac posuere vel, malesuada quis ex. Donec
++interdum ipsum et nibh molestie auctor eget in neque. Sed nec hendrerit enim.
++Etiam scelerisque non magna eu molestie. Maecenas dui quam, tempor quis ipsum
++eu, dapibus congue dui. Integer tincidunt accumsan tortor sed maximus. Praesent
++eu magna accumsan, consequat turpis quis, fringilla ante. Vestibulum nec
++maximus nulla. Donec vehicula nisl at pharetra eleifend.
+diff --git a/test.txt b/test.txt
+deleted file mode 100644
+index 4478ac2..0000000
+--- a/test.txt
++++ /dev/null
+@@ -1,17 +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.
+-
+-Curabitur scelerisque tempus efficitur. Maecenas aliquet elementum quam, ac
+-pharetra nisi gravida a. Nam vitae malesuada leo. Nam accumsan facilisis metus
+-sed vulputate. Ut libero odio, tempor ac posuere vel, malesuada quis ex. Donec
+-interdum ipsum et nibh molestie auctor eget in neque. Sed nec hendrerit enim.
+-Etiam scelerisque non magna eu molestie. Maecenas dui quam, tempor quis ipsum
+-eu, dapibus congue dui. Integer tincidunt accumsan tortor sed maximus. Praesent
+-eu magna accumsan, consequat turpis quis, fringilla ante. Vestibulum nec
+-maximus nulla. Donec vehicula nisl at pharetra eleifend.
+-- 
+2.7.4
+
+
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index 295a049d..2e86e47c 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -538,6 +538,23 @@  class RevisedSeriesTest(_BaseTestCase):
 
         self.assertSerialized(patches, [1, 1, 1])
 
+    def test_mixed_versions(self):
+        """Series with a revision sent in reply to an incompleted series.
+
+        Parse a series with two patches, one of which has been lost or
+        miscategorized, followed by a second revision of the missing patch.
+        None of the patches of the second revision should be included in the
+        first revision.
+
+          - [PATCH 1/2] test: Add some lorem ipsum
+            - [PATCH v2 2/2] test: Convert to Markdown
+        """
+        _, patches, _ = self._parse_mbox(
+            'bugs-mixed-versions.mbox', [0, 2, 0],
+        )
+
+        self.assertSerialized(patches, [1, 1])
+
 
 class SeriesTotalTest(_BaseTestCase):