From 859842faff77143c1d7b8c82dc44774cb95e0f17 Mon Sep 17 00:00:00 2001 From: Yuri Volchkov Date: Wed, 20 Jun 2018 14:21:42 +0200 Subject: [PATCH 01/73] parsemail: ignore html part of multi-part comments Currently an html-protection present only for patch-emails. If a multi-part comment-email arrives, it messes up patchwork. In my case, the symptom was a non intended 'Signed-off-by' in the downloaded patches, with html-like junk. This patch makes parsemail skip all parts of comment which are not text/plain. Of course, this will drop html-only emails completely. But they can not be parsed anyways. Signed-off-by: Yuri Volchkov Reviewed-by: Stephen Finucane (cherry picked from commit e27ff061dc01e51967a978884a5c59152863ab9c) --- patchwork/parser.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/patchwork/parser.py b/patchwork/parser.py index 8f9af8116..a40f9314a 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -576,10 +576,13 @@ def find_comment_content(mail): """Extract content from a mail.""" commentbuf = '' - for payload, _ in _find_content(mail): + for payload, subtype in _find_content(mail): if not payload: continue + if subtype != 'plain': + continue + commentbuf += payload.strip() + '\n' commentbuf = clean_content(commentbuf) From e8cc6f8d7b17eae8783432161c7ad76a939aa43d Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 20 Jun 2018 15:25:33 +0100 Subject: [PATCH 02/73] tests: Add tests for multipart emails Ensure HTML is dropped as expected. Signed-off-by: Stephen Finucane (cherry picked from commit d6491b59d97ba7fd6a498490f4cb3b6159dd53ee) --- .../tests/mail/0019-multipart-patch.mbox | 55 +++++++++++++++++++ .../tests/mail/0020-multipart-comment.mbox | 49 +++++++++++++++++ patchwork/tests/test_parser.py | 26 +++++++++ 3 files changed, 130 insertions(+) create mode 100644 patchwork/tests/mail/0019-multipart-patch.mbox create mode 100644 patchwork/tests/mail/0020-multipart-comment.mbox diff --git a/patchwork/tests/mail/0019-multipart-patch.mbox b/patchwork/tests/mail/0019-multipart-patch.mbox new file mode 100644 index 000000000..99d23a833 --- /dev/null +++ b/patchwork/tests/mail/0019-multipart-patch.mbox @@ -0,0 +1,55 @@ +From yuri.volchkov@gmail.com Wed Jun 20 12:22:05 2018 +From: Yuri Volchkov +To: patchwork@lists.ozlabs.org +Cc: stephen@that.guru +Subject: [PATCH] parsemail: ignore html part of multi-part comments +Date: Wed, 20 Jun 2018 14:21:42 +0200 +Message-Id: <20180620122142.9917-1-yuri.volchkov@gmail.com> +Content-Type: multipart/alternative; boundary="000000000000f93f23056f12c80c" + + +--000000000000f93f23056f12c80c +Content-Type: text/plain; charset="UTF-8" +Content-Transfer-Encoding: 8bit + +Currently an html-protection present only for patch-emails. If a +multi-part comment-email arrives, it messes up patchwork. In my case, +the symptom was a non intended 'Signed-off-by' in the downloaded +patches, with html-like junk. + +This patch makes parsemail skip all parts of comment which are not +text/plain. + +Of course, this will drop html-only emails completely. But they can +not be parsed anyways. + +Signed-off-by: Yuri Volchkov +--- + patchwork/parser.py | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/patchwork/parser.py b/patchwork/parser.py +index 8f9af811..b1fb7b9c 100644 +--- a/patchwork/parser.py ++++ b/patchwork/parser.py +@@ -576,9 +576,11 @@ def find_comment_content(mail): + """Extract content from a mail.""" + commentbuf = '' + +- for payload, _ in _find_content(mail): ++ for payload, subtype in _find_content(mail): + if not payload: + continue ++ if subtype != 'plain': ++ continue + + commentbuf += payload.strip() + '\n' + +--000000000000f93f23056f12c80c +Content-Type: text/html; charset="UTF-8" +Content-Transfer-Encoding: 8bit + +
Currently an html-protection present only for patch-emails. If a
multi-part comment-email arrives, it messes up patchwork. In my case,
the symptom was a non intended 'Signed-off-by' in the downloaded
patches, with html-like junk.

This patch makes parsemail skip all parts of comment which are not
text/plain.

Of course, this will drop html-only emails completely. But they can
not be parsed anyways.

Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com>
---
 patchwork/parser.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/patchwork/parser.py b/patchwork/parser.py
index 8f9af811..b1fb7b9c 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -576,9 +576,11 @@ def find_comment_content(mail):
     """Extract content from a mail."""
     commentbuf = ''
 
-    for payload, _ in _find_content(mail):
+    for payload, subtype in _find_content(mail):
         if not payload:
             continue
+        if subtype != 'plain':
+            continue
 
         commentbuf += payload.strip() + '\n'
 
--
2.17.1
+ +--000000000000f93f23056f12c80c-- + diff --git a/patchwork/tests/mail/0020-multipart-comment.mbox b/patchwork/tests/mail/0020-multipart-comment.mbox new file mode 100644 index 000000000..7a696a559 --- /dev/null +++ b/patchwork/tests/mail/0020-multipart-comment.mbox @@ -0,0 +1,49 @@ +From stephenfinucane@hotmail.com Wed Jun 20 13:35:48 2018 +From: Stephen Finucane +To: "stephen@that.guru" +Subject: Re: [PATCH] parsemail: ignore html part of multi-part comments +Date: Wed, 20 Jun 2018 13:35:37 +0000 +Message-ID: +References: <20180620122142.9917-1-yuri.volchkov@gmail.com> +In-Reply-To: <20180620122142.9917-1-yuri.volchkov@gmail.com> +Content-Type: multipart/alternative; + boundary="_000_DB5PR03MB18774049A0E62D211988EC8CA3770DB5PR03MB1877eurp_" +MIME-Version: 1.0 + + +--_000_DB5PR03MB18774049A0E62D211988EC8CA3770DB5PR03MB1877eurp_ +Content-Type: text/plain; charset="iso-8859-1" +Content-Transfer-Encoding: 8bit + +Yup, this looks sensible to me. Replying from Outlook's awful HTML editor to get +a sample comment to test with. + +Stephen + + +--_000_DB5PR03MB18774049A0E62D211988EC8CA3770DB5PR03MB1877eurp_ +Content-Type: text/html; charset="iso-8859-1" +Content-Transfer-Encoding: 8bit + + + + + + + +
+Yup, this looks sensible to me. Replying from Outlook's awful HTML editor to get a sample comment to test with.
+
+
+
+
+Stephen
+
+
+
+
+ + + +--_000_DB5PR03MB18774049A0E62D211988EC8CA3770DB5PR03MB1877eurp_-- + diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index 5ba06c0f3..e99cf214e 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -36,6 +36,7 @@ from patchwork.parser import clean_subject from patchwork.parser import get_or_create_author from patchwork.parser import find_patch_content as find_content +from patchwork.parser import find_comment_content from patchwork.parser import find_project from patchwork.parser import find_series from patchwork.parser import parse_mail as _parse_mail @@ -632,6 +633,14 @@ def test_no_subject(self): self.assertTrue(diff is not None) self.assertTrue(message is not None) + def test_html_multipart(self): + """Validate parsing a mail with multiple parts.""" + diff, message = self._find_content('0019-multipart-patch.mbox') + self.assertTrue(diff is not None) + self.assertTrue(message is not None) + self.assertFalse(' Date: Mon, 1 Oct 2018 22:36:41 +0100 Subject: [PATCH 03/73] templates: Avoid recursive call We had registered an event handler on a checkbox in table header which would call a function, 'checkboxes', on all checkboxes within that table. This function, in turn, causes does its work and then triggers event handlers for all modified checkboxes which include the original table header checkbox. This resulted in the original event calling itself recursively. Resolve this by only modifying the checkboxes in the table body. Signed-off-by: Stephen Finucane Fixes: 44fe7bae ("js: Allow shift-select of checkboxes") (cherry picked from commit ae154148c78a75ff73c3c22f0ff0c6b3a3d01408) --- patchwork/templates/patchwork/patch-list.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patchwork/templates/patchwork/patch-list.html b/patchwork/templates/patchwork/patch-list.html index 71c1ba927..f0f12cc81 100644 --- a/patchwork/templates/patchwork/patch-list.html +++ b/patchwork/templates/patchwork/patch-list.html @@ -41,9 +41,9 @@ $('#check-all').change(function(e) { if(this.checked) { - $('#patchlist').checkboxes('check'); + $('#patchlist > tbody').checkboxes('check'); } else { - $('#patchlist').checkboxes('uncheck'); + $('#patchlist > tbody').checkboxes('uncheck'); } e.preventDefault(); }); From 6ac5bac532b718137d9f0090b55192a74a6971fe Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sat, 29 Sep 2018 22:33:36 +0100 Subject: [PATCH 04/73] views: Populate bundles for 'todo' view Signed-off-by: Stephen Finucane Closes: #213 (cherry picked from commit caab3e3ec93079b7dc6b120e353535983e66ba79) --- patchwork/views/user.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/patchwork/views/user.py b/patchwork/views/user.py index 4a4958ab1..0c2914b3b 100644 --- a/patchwork/views/user.py +++ b/patchwork/views/user.py @@ -235,8 +235,10 @@ def todo_list(request, project_id): filter_settings=filter_settings, patches=patches) - context['action_required_states'] = \ - State.objects.filter(action_required=True).all() + context['bundles'] = request.user.bundles.all() + context['action_required_states'] = State.objects.filter( + action_required=True).all() + return render(request, 'patchwork/todo-list.html', context) From 07020c49e6ba364e6ee5e4f6678635268dc2a976 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 27 Sep 2018 15:34:07 +0100 Subject: [PATCH 05/73] filters: Return dictionaries This makes it a little easier to figure out what filters are active and will be used in a future patch. Signed-off-by: Stephen Finucane (cherry picked from commit 0bd13fe1d4d077e7b1fa101d08c914da04387d0a) --- patchwork/filters.py | 13 ++++++++----- patchwork/templates/patchwork/filters.html | 2 +- patchwork/views/__init__.py | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/patchwork/filters.py b/patchwork/filters.py index 8d0f82f2d..f6e483947 100644 --- a/patchwork/filters.py +++ b/patchwork/filters.py @@ -19,6 +19,8 @@ from __future__ import absolute_import +import collections + from django.contrib.auth.models import User from django.utils.html import escape from django.utils.safestring import mark_safe @@ -459,7 +461,7 @@ def set_project(self, project): self.project = project def filter_conditions(self): - kwargs = {} + kwargs = collections.OrderedDict() for f in self._filters: if f.applied: kwargs.update(f.kwargs()) @@ -472,11 +474,11 @@ def apply(self, queryset): return queryset.filter(**kwargs) def params(self): - return [(f.param, f.key()) for f in self._filters - if f.key() is not None] + return collections.OrderedDict([ + (f.param, f.key()) for f in self._filters if f.key() is not None]) def querystring(self, remove=None): - params = dict(self.params()) + params = self.params() for (k, v) in self.values.items(): if k not in params: @@ -497,7 +499,8 @@ def querystring_without_filter(self, filter): return self.querystring(filter) def applied_filters(self): - return [x for x in self._filters if x.applied] + return collections.OrderedDict([ + (x.param, x) for x in self._filters if x.applied]) def available_filters(self): return self._filters diff --git a/patchwork/templates/patchwork/filters.html b/patchwork/templates/patchwork/filters.html index 5331ac855..e760310bd 100644 --- a/patchwork/templates/patchwork/filters.html +++ b/patchwork/templates/patchwork/filters.html @@ -130,7 +130,7 @@
Show patches with: {% if filters.applied_filters %} - {% for filter in filters.applied_filters %} + {% for filter in filters.applied_filters.values %} {{ filter.name }} = {{ filter.condition }} {% if not filter.forced %} Date: Sat, 29 Sep 2018 21:11:22 +0100 Subject: [PATCH 06/73] filters: Pre-populate delegate, submitter filters This appears to have got lost in the transition to 'selectize.js'. In brief, this will ensure that a previously selected delegate or submitter is still enabled post-filtering. For more information, see [1]. [1] https://stackoverflow.com/a/45124779 Signed-off-by: Stephen Finucane Closes: #78 (cherry picked from commit bd528a9026d8b45588c8117f1a3120461d417c69) --- patchwork/templates/patchwork/filters.html | 71 +++++++++------------- 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/patchwork/templates/patchwork/filters.html b/patchwork/templates/patchwork/filters.html index e760310bd..9b0c4cb2e 100644 --- a/patchwork/templates/patchwork/filters.html +++ b/patchwork/templates/patchwork/filters.html @@ -16,8 +16,6 @@ form.style['display'] = 'block'; filterform_displayed = true; } - - } Selectize.define('enter_key_submit', function (options) { @@ -39,9 +37,6 @@ $(document).ready(function() { $('#submitter_input').selectize({ - valueField: 'pk', - labelField: 'name', - searchField: ['name', 'email'], plugins: ['enter_key_submit'], maxItems: 1, persist: false, @@ -52,43 +47,37 @@ this.$input.closest('form').submit(); }, this); }, - render: { - option: function(item, escape) { - if (item.name) - return '
' + escape(item.name) + ' <' + - escape(item.email) + '>' + '
'; - return '
' + escape(item.email) + '
'; +{% if "submitter" in filters.applied_filters %} +{% with submitter_filter=filters.applied_filters.submitter %} + options: [ + { + value: "{{ submitter_filter.key }}", + text: "{{ submitter_filter.condition }}", }, - item: function(item, escape) { - if (item.name) - return '
' + escape(item.name) + '
'; - return '
' + escape(item.email) + '
'; - } - }, + ], + items: ["{{ submitter_filter.key }}"], +{% endwith %} +{% endif %} load: function(query, callback) { if (query.length < 4) return callback(); req = $.ajax({ - url: '{% url 'api-submitters' %}?q=' + - encodeURIComponent(query) + '&l=10', + url: "{% url 'api-submitters' %}", + data: {q: query, l: 10}, error: function() { callback(); }, success: function(res) { - callback(res); + callback($.map(res, function (obj) { + return {value: obj.pk, text: `${obj.name} <${obj.email}>`}; + })); } }); } }); -}); - -$(document).ready(function() { $('#delegate_input').selectize({ - valueField: 'pk', - labelField: 'name', - searchField: ['name'], plugins: ['enter_key_submit'], maxItems: 1, persist: false, @@ -99,26 +88,28 @@ this.$input.closest('form').submit(); }, this); }, - render: { - option: function(item, escape) { - if (item.email) - return '
' + escape(item.name) + ' <' + - escape(item.email) + '>' + '
'; - return '
' + escape(item.name) + '
'; +{% if "delegate" in filters.applied_filters %} +{% with delegate_filter=filters.applied_filters.delegate %} + options: [ + { + value: "{{ delegate_filter.key }}", + text: "{{ delegate_filter.condition }}", }, - item: function(item, escape) { - return '
' + escape(item.name) + '
'; - } - }, + ], + items: ["{{ delegate_filter.key }}"], +{% endwith %} +{% endif %} load: function(query, callback) { req = $.ajax({ - url: '{% url 'api-delegates' %}?q=' + - encodeURIComponent(query) + '&l=10', + url: "{% url 'api-delegates' %}", + data: {q: query, l: 10}, error: function() { callback(); }, success: function(res) { - callback(res); + callback($.map(res, function (obj) { + return {value: obj.pk, text: obj.name}; + })); } }); } @@ -167,5 +158,3 @@
- - From f26930218d3c2dcfcec2ca61475056d47ff32c83 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 12 Sep 2018 09:46:56 -0600 Subject: [PATCH 07/73] REST: Only list checks for the given patch This is either a regression or it never worked. In any case, fix it and add a test to ensure it doesn't happen again. Signed-off-by: Stephen Finucane Reviewed-by: Veronika Kabatova Closes: #203 (cherry picked from commit e80216a96eac6b1f1ed10aa9effd1d3120f2c3a1) --- patchwork/api/check.py | 3 ++- patchwork/tests/api/test_check.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/patchwork/api/check.py b/patchwork/api/check.py index 8753c7de4..1cfa5375b 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -83,7 +83,8 @@ class CheckMixin(object): filter_class = CheckFilterSet def get_queryset(self): - return Check.objects.prefetch_related('patch', 'user') + patch_id = self.kwargs['patch_id'] + return Check.objects.prefetch_related('user').filter(patch=patch_id) class CheckListCreate(CheckMixin, ListCreateAPIView): diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py index 43181af3d..57d5b775b 100644 --- a/patchwork/tests/api/test_check.py +++ b/patchwork/tests/api/test_check.py @@ -54,9 +54,9 @@ def setUp(self): self.user = create_maintainer(project) self.patch = create_patch(project=project) - def _create_check(self): + def _create_check(self, patch=None): values = { - 'patch': self.patch, + 'patch': patch if patch else self.patch, 'user': self.user, } return create_check(**values) @@ -75,6 +75,7 @@ def test_list(self): self.assertEqual(0, len(resp.data)) check_obj = self._create_check() + self._create_check(create_patch()) # second, unrelated patch resp = self.client.get(self.api_url()) self.assertEqual(status.HTTP_200_OK, resp.status_code) From 4159ba3176d56934a44c9146984adf19a4d20b03 Mon Sep 17 00:00:00 2001 From: Petr Vorel Date: Thu, 26 Jul 2018 09:11:14 +0200 Subject: [PATCH 08/73] pwclient: Fix pwclient am output formatting repr() print unicode prefix for string: $ pwclient git-am N Applying patch #N using u'git am' Remove it: $ pwclient git-am N Applying patch #918868 using "git am" git mixes single and double quotes, use double quotes which are more frequently used. Signed-off-by: Petr Vorel Signed-off-by: Daniel Axtens (cherry picked from commit ea5847ada3ac79908a2b251839e77e1b2f3dc6d2) --- patchwork/bin/pwclient | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patchwork/bin/pwclient b/patchwork/bin/pwclient index 79137b0d0..2020a8c0d 100755 --- a/patchwork/bin/pwclient +++ b/patchwork/bin/pwclient @@ -327,8 +327,8 @@ def action_apply(rpc, patch_id, apply_cmd=None): print('Applying patch #%d to current directory' % patch_id) apply_cmd = ['patch', '-p1'] else: - print('Applying patch #%d using %s' % - (patch_id, repr(' '.join(apply_cmd)))) + print('Applying patch #%d using "%s"' % + (patch_id, ' '.join(apply_cmd))) print('Description: %s' % patch['name']) s = rpc.patch_get_mbox(patch_id) From 8060b9a6719bcfe80f7f23143447167dc02c23d5 Mon Sep 17 00:00:00 2001 From: Jiri Benc Date: Thu, 28 Jun 2018 15:42:11 -0400 Subject: [PATCH 09/73] parser: fix parsing of patches with headings Some people tend to use lines full of '=' as a fancy way to format headings in their commit messages in a rst-like style. However, the current parser treats such lines as a beginning of a diff. The only currently used tool that produces diffs with '=' lines is quilt in the default configuration. However, even with quilt, the diff looks this way: Index: dir/file =================================================================== --- dir.orig/file +++ dir/file @@ ...etc... It's enough to match on the "Index:" line. The state of the state machine is kept at 1 when it encounters the '=' line, thus it's safe to remove the match on '=' completely. [This prevents us from properly parsing metadata out of the changelog. -dcz ] Signed-off-by: Jiri Benc Reviewed-by: Stephen Finucane (cherry picked from commit 67faf96ab96d93252c89967ef766bcbe8214c0fc) --- patchwork/parser.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/patchwork/parser.py b/patchwork/parser.py index a40f9314a..a2db40373 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -745,7 +745,7 @@ def parse_patch(content): # state specified the line we just saw, and what to expect next state = 0 # 0: text - # 1: suspected patch header (diff, ====, Index:) + # 1: suspected patch header (diff, Index:) # 2: patch header line 1 (---) # 3: patch header line 2 (+++) # 4: patch hunk header line (@@ line) @@ -753,7 +753,7 @@ def parse_patch(content): # 6: patch meta header (rename from/rename to) # # valid transitions: - # 0 -> 1 (diff, ===, Index:) + # 0 -> 1 (diff, Index:) # 0 -> 2 (---) # 1 -> 2 (---) # 2 -> 3 (+++) @@ -776,7 +776,7 @@ def parse_patch(content): line += '\n' if state == 0: - if line.startswith('diff ') or line.startswith('===') \ + if line.startswith('diff ') \ or line.startswith('Index: '): state = 1 buf += line From 7a69a77dcced02e5f9de3f563f9545fc9297c204 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 14 Oct 2018 14:32:25 +0100 Subject: [PATCH 10/73] docs: Add release notes for recent bug fixes We need to start adding these as part of the bug fix itself. Signed-off-by: Stephen Finucane (cherry picked from commit 67565bf528d6347edd6561677f61639adb35ba55) --- releasenotes/notes/issue-203-ece01ae49ceac712.yaml | 6 ++++++ releasenotes/notes/issue-223-0757db6ac886374f.yaml | 6 ++++++ releasenotes/notes/issue-78-accd1f9db45a2b71.yaml | 6 ++++++ 3 files changed, 18 insertions(+) create mode 100644 releasenotes/notes/issue-203-ece01ae49ceac712.yaml create mode 100644 releasenotes/notes/issue-223-0757db6ac886374f.yaml create mode 100644 releasenotes/notes/issue-78-accd1f9db45a2b71.yaml diff --git a/releasenotes/notes/issue-203-ece01ae49ceac712.yaml b/releasenotes/notes/issue-203-ece01ae49ceac712.yaml new file mode 100644 index 000000000..d8609cc0e --- /dev/null +++ b/releasenotes/notes/issue-203-ece01ae49ceac712.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + An issue that resulted in checks for all patches being listed for each + patch is resolved. + (`#203 `__) diff --git a/releasenotes/notes/issue-223-0757db6ac886374f.yaml b/releasenotes/notes/issue-223-0757db6ac886374f.yaml new file mode 100644 index 000000000..e7a5c5509 --- /dev/null +++ b/releasenotes/notes/issue-223-0757db6ac886374f.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + It's now possible to assign patches to existing bundles from a user's TODO + page. + (`#213 `__) diff --git a/releasenotes/notes/issue-78-accd1f9db45a2b71.yaml b/releasenotes/notes/issue-78-accd1f9db45a2b71.yaml new file mode 100644 index 000000000..ba805a338 --- /dev/null +++ b/releasenotes/notes/issue-78-accd1f9db45a2b71.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The delegate and submitter fields will remain populated when moving + between different pages or changing filters. + (`#78 `__) From 1552b7d539695e9c2c86b96b220c2c9d2e150284 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sat, 13 Oct 2018 17:10:49 +0100 Subject: [PATCH 11/73] REST: Don't allow settings of some project fields These should only be configurable by superusers as invalid configuration can break things. Signed-off-by: Stephen Finucane Closes: #217 (cherry picked from commit 530999bf7c286bd3990e63790958338ef65a25a8) --- patchwork/api/project.py | 9 +++++---- patchwork/tests/api/test_project.py | 11 ++++++++++- releasenotes/notes/issue-217-676f3f737e46320e.yaml | 7 +++++++ 3 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/issue-217-676f3f737e46320e.yaml diff --git a/patchwork/api/project.py b/patchwork/api/project.py index 6f1affad9..3609f7336 100644 --- a/patchwork/api/project.py +++ b/patchwork/api/project.py @@ -30,9 +30,9 @@ class ProjectSerializer(BaseHyperlinkedModelSerializer): - link_name = CharField(max_length=255, source='linkname') - list_id = CharField(max_length=255, source='listid') - list_email = CharField(max_length=200, source='listemail') + link_name = CharField(max_length=255, source='linkname', read_only=True) + list_id = CharField(max_length=255, source='listid', read_only=True) + list_email = CharField(max_length=200, source='listemail', read_only=True) maintainers = UserProfileSerializer(many=True, read_only=True, source='maintainer_project') @@ -41,7 +41,8 @@ class Meta: fields = ('id', 'url', 'name', 'link_name', 'list_id', 'list_email', 'web_url', 'scm_url', 'webscm_url', 'maintainers', 'subject_match') - read_only_fields = ('name', 'maintainers', 'subject_match') + read_only_fields = ('name', 'link_name', 'list_id', 'list_email', + 'maintainers', 'subject_match') versioned_fields = { '1.1': ('subject_match', ), } diff --git a/patchwork/tests/api/test_project.py b/patchwork/tests/api/test_project.py index 129cedb74..10044de4e 100644 --- a/patchwork/tests/api/test_project.py +++ b/patchwork/tests/api/test_project.py @@ -143,7 +143,7 @@ def test_create(self): def test_update(self): """Ensure updates can be performed by maintainers.""" project = create_project() - data = {'linkname': 'TEST'} + data = {'web_url': 'TEST'} # an anonymous user resp = self.client.patch(self.api_url(project.id), data) @@ -160,6 +160,15 @@ def test_update(self): self.client.force_authenticate(user=user) resp = self.client.patch(self.api_url(project.id), data) self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(resp.data['web_url'], 'TEST') + + # ...with the exception of some read-only fields + resp = self.client.patch(self.api_url(project.id), { + 'link_name': 'test'}) + # NOTE(stephenfin): This actually returns HTTP 200 due to + # https://github.com/encode/django-rest-framework/issues/1655 + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertNotEqual(resp.data['link_name'], 'test') def test_delete(self): """Ensure deletions are rejected.""" diff --git a/releasenotes/notes/issue-217-676f3f737e46320e.yaml b/releasenotes/notes/issue-217-676f3f737e46320e.yaml new file mode 100644 index 000000000..ecf4a118f --- /dev/null +++ b/releasenotes/notes/issue-217-676f3f737e46320e.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + A project's ``list_email``, ``list_id`` and ``link_name`` fields can no + longer be updated via the REST API. This is a superuser-only operation + that, for now, should only be done via the admin interface. + (`#217 `__) From 1bd87ee1c40dbae9d35ac0b5380e6dd036c60e88 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 11 Oct 2018 14:53:25 +0100 Subject: [PATCH 12/73] REST: Validate patch delegate At present, only users who are maintainers of projects can be delegated a project. Validate this. This is currently broken due to #216 but that will be fixed in a future change. Signed-off-by: Stephen Finucane (cherry picked from commit b690746d3f21fac69dc6f2aadd81bdd38ace225a) --- patchwork/api/patch.py | 9 +++++++++ patchwork/tests/api/test_patch.py | 18 +++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 9d890eb10..6b5cc0d1a 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -25,6 +25,7 @@ from rest_framework.relations import RelatedField from rest_framework.reverse import reverse from rest_framework.serializers import SerializerMethodField +from rest_framework.serializers import ValidationError from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import PatchworkPermission @@ -113,6 +114,14 @@ def get_tags(self, instance): # model return {} + def validate_delegate(self, value): + """Check that the delgate is a maintainer of the patch's project.""" + if not self.instance.project.maintainer_project.filter( + id=value.id).exists(): + raise ValidationError("User '%s' is not a maintainer for project " + "'%s'" % (value, self.instance.project)) + return value + class Meta: model = Patch fields = ('id', 'url', 'web_url', 'project', 'msgid', 'date', 'name', diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 27b992484..399150019 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -218,12 +218,15 @@ def test_update(self): # maintainer user = create_maintainer(project) self.client.force_authenticate(user=user) - resp = self.client.patch(self.api_url(patch.id), {'state': state.name}) - self.assertEqual(status.HTTP_200_OK, resp.status_code) + resp = self.client.patch(self.api_url(patch.id), { + 'state': state.name, 'delegate': user.id}) + self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) self.assertEqual(Patch.objects.get(id=patch.id).state, state) + # TODO(stephenfin): This is currently broken due to #216 + # self.assertEqual(Patch.objects.get(id=patch.id).delegate, user) def test_update_invalid(self): - """Ensure we handle invalid Patch states.""" + """Ensure we handle invalid Patch updates.""" project = create_project() state = create_state() patch = create_patch(project=project, state=state) @@ -236,6 +239,15 @@ def test_update_invalid(self): self.assertContains(resp, 'Expected one of: %s.' % state.name, status_code=status.HTTP_400_BAD_REQUEST) + # invalid delegate + user_b = create_user() + resp = self.client.patch(self.api_url(patch.id), + {'delegate': user_b.id}) + # TODO(stephenfin): This is currently broken due to #216 + # self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) + # self.assertContains(resp, "User '%s' is not a maintainer" % user_b, + # status_code=status.HTTP_400_BAD_REQUEST) + def test_delete(self): """Ensure deletions are always rejected.""" project = create_project() From e1ae71a9f6a33c636a4d6babca36236d8a6c29ac Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 11 Oct 2018 11:47:56 +0100 Subject: [PATCH 13/73] REST: Allow setting of values using embedded serializers Unfortunately, the use of embedded serializers for some fields breaks the ability to update these fields, either via the HTML interface (where the widget is totally busted) or via a client like 'git-pw'. What we actually want is to be able to update these fields like normal primary key but show them using the embedded serializer. We do just this by using a modified variant of the PrimaryKeyRelatedField and using the serializers simply for displaying. Signed-off-by: Stephen Finucane Closes: #216 (cherry picked from commit 1a0021a21a8bfe822b469da7fb3e5f0ab6dcaed1) --- patchwork/api/embedded.py | 272 +++++++++++------- patchwork/tests/api/test_patch.py | 10 +- .../notes/issue-216-d3bf9d1baa100f74.yaml | 5 + 3 files changed, 174 insertions(+), 113 deletions(-) create mode 100644 releasenotes/notes/issue-216-d3bf9d1baa100f74.yaml diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index 1d5aba84a..314a27fed 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -23,14 +23,54 @@ nested fields. """ +from collections import OrderedDict + from rest_framework.serializers import CharField from rest_framework.serializers import SerializerMethodField +from rest_framework.serializers import PrimaryKeyRelatedField from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import CheckHyperlinkedIdentityField from patchwork import models +class SerializedRelatedField(PrimaryKeyRelatedField): + """ + A read-write field that expects a primary key for writes and returns a + serialized version of the underlying field on reads. + """ + + def use_pk_only_optimization(self): + # We're using embedded serializers so we want the whole object + return False + + def get_queryset(self): + return self._Serializer.Meta.model.objects.all() + + def get_choices(self, cutoff=None): + # Override this so we don't call 'to_representation', which no longer + # returns a flat value + queryset = self.get_queryset() + if queryset is None: + # Ensure that field.choices returns something sensible + # even when accessed with a read-only field. + return {} + + if cutoff is not None: + queryset = queryset[:cutoff] + + return OrderedDict([ + ( + item.pk, + self.display_value(item) + ) + for item in queryset + ]) + + def to_representation(self, data): + return self._Serializer(context=self.context).to_representation(data) + + class MboxMixin(BaseHyperlinkedModelSerializer): """Embed a link to the mbox URL. @@ -55,132 +95,150 @@ def get_web_url(self, instance): return request.build_absolute_uri(instance.get_absolute_url()) -class BundleSerializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): - - class Meta: - model = models.Bundle - fields = ('id', 'url', 'web_url', 'name', 'mbox') - read_only_fields = fields - versioned_field = { - '1.1': ('web_url', ), - } - extra_kwargs = { - 'url': {'view_name': 'api-bundle-detail'}, - } +class BundleSerializer(SerializedRelatedField): + + class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): + + class Meta: + model = models.Bundle + fields = ('id', 'url', 'web_url', 'name', 'mbox') + read_only_fields = fields + versioned_field = { + '1.1': ('web_url', ), + } + extra_kwargs = { + 'url': {'view_name': 'api-bundle-detail'}, + } + + +class CheckSerializer(SerializedRelatedField): + + class _Serializer(BaseHyperlinkedModelSerializer): + + url = CheckHyperlinkedIdentityField('api-check-detail') + + def to_representation(self, instance): + data = super(CheckSerializer._Serializer, self).to_representation( + instance) + data['state'] = instance.get_state_display() + return data + + class Meta: + model = models.Check + fields = ('id', 'url', 'date', 'state', 'target_url', 'context') + read_only_fields = fields + extra_kwargs = { + 'url': {'view_name': 'api-check-detail'}, + } + + +class CoverLetterSerializer(SerializedRelatedField): + + class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): + + class Meta: + model = models.CoverLetter + fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') + read_only_fields = fields + versioned_field = { + '1.1': ('web_url', 'mbox', ), + } + extra_kwargs = { + 'url': {'view_name': 'api-cover-detail'}, + } + + +class PatchSerializer(SerializedRelatedField): + + class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): + + class Meta: + model = models.Patch + fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') + read_only_fields = fields + versioned_field = { + '1.1': ('web_url', ), + } + extra_kwargs = { + 'url': {'view_name': 'api-patch-detail'}, + } -class CheckSerializer(BaseHyperlinkedModelSerializer): +class PersonSerializer(SerializedRelatedField): - url = CheckHyperlinkedIdentityField('api-check-detail') + class _Serializer(BaseHyperlinkedModelSerializer): - def to_representation(self, instance): - data = super(CheckSerializer, self).to_representation(instance) - data['state'] = instance.get_state_display() - return data + class Meta: + model = models.Person + fields = ('id', 'url', 'name', 'email') + read_only_fields = fields + extra_kwargs = { + 'url': {'view_name': 'api-person-detail'}, + } - class Meta: - model = models.Check - fields = ('id', 'url', 'date', 'state', 'target_url', 'context') - read_only_fields = fields - extra_kwargs = { - 'url': {'view_name': 'api-check-detail'}, - } +class ProjectSerializer(SerializedRelatedField): + class _Serializer(BaseHyperlinkedModelSerializer): -class CoverLetterSerializer(MboxMixin, WebURLMixin, - BaseHyperlinkedModelSerializer): + link_name = CharField(max_length=255, source='linkname') + list_id = CharField(max_length=255, source='listid') + list_email = CharField(max_length=200, source='listemail') - class Meta: - model = models.CoverLetter - fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') - read_only_fields = fields - versioned_field = { - '1.1': ('web_url', 'mbox', ), - } - extra_kwargs = { - 'url': {'view_name': 'api-cover-detail'}, - } + class Meta: + model = models.Project + fields = ('id', 'url', 'name', 'link_name', 'list_id', + 'list_email', 'web_url', 'scm_url', 'webscm_url') + read_only_fields = fields + extra_kwargs = { + 'url': {'view_name': 'api-project-detail'}, + } -class PatchSerializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): +class SeriesSerializer(SerializedRelatedField): - class Meta: - model = models.Patch - fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') - read_only_fields = fields - versioned_field = { - '1.1': ('web_url', ), - } - extra_kwargs = { - 'url': {'view_name': 'api-patch-detail'}, - } + class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): + class Meta: + model = models.Series + fields = ('id', 'url', 'date', 'name', 'version', 'mbox') + read_only_fields = fields + versioned_field = { + '1.1': ('web_url', ), + } + extra_kwargs = { + 'url': {'view_name': 'api-series-detail'}, + } -class PersonSerializer(BaseHyperlinkedModelSerializer): - class Meta: - model = models.Person - fields = ('id', 'url', 'name', 'email') - read_only_fields = fields - extra_kwargs = { - 'url': {'view_name': 'api-person-detail'}, - } +class UserSerializer(SerializedRelatedField): + class _Serializer(BaseHyperlinkedModelSerializer): -class ProjectSerializer(BaseHyperlinkedModelSerializer): + class Meta: + model = models.User + fields = ('id', 'url', 'username', 'first_name', 'last_name', + 'email') + read_only_fields = fields + extra_kwargs = { + 'url': {'view_name': 'api-user-detail'}, + } - link_name = CharField(max_length=255, source='linkname') - list_id = CharField(max_length=255, source='listid') - list_email = CharField(max_length=200, source='listemail') - class Meta: - model = models.Project - fields = ('id', 'url', 'name', 'link_name', 'list_id', 'list_email', - 'web_url', 'scm_url', 'webscm_url') - read_only_fields = fields - extra_kwargs = { - 'url': {'view_name': 'api-project-detail'}, - } +class UserProfileSerializer(SerializedRelatedField): + class _Serializer(BaseHyperlinkedModelSerializer): -class SeriesSerializer(MboxMixin, WebURLMixin, - BaseHyperlinkedModelSerializer): + username = CharField(source='user.username') + first_name = CharField(source='user.first_name') + last_name = CharField(source='user.last_name') + email = CharField(source='user.email') - class Meta: - model = models.Series - fields = ('id', 'url', 'date', 'name', 'version', 'mbox') - read_only_fields = fields - versioned_field = { - '1.1': ('web_url', ), - } - extra_kwargs = { - 'url': {'view_name': 'api-series-detail'}, - } - - -class UserSerializer(BaseHyperlinkedModelSerializer): - - class Meta: - model = models.User - fields = ('id', 'url', 'username', 'first_name', 'last_name', 'email') - read_only_fields = fields - extra_kwargs = { - 'url': {'view_name': 'api-user-detail'}, - } - - -class UserProfileSerializer(BaseHyperlinkedModelSerializer): - - username = CharField(source='user.username') - first_name = CharField(source='user.first_name') - last_name = CharField(source='user.last_name') - email = CharField(source='user.email') - - class Meta: - model = models.UserProfile - fields = ('id', 'url', 'username', 'first_name', 'last_name', 'email') - read_only_fields = fields - extra_kwargs = { - 'url': {'view_name': 'api-user-detail'}, - } + class Meta: + model = models.UserProfile + fields = ('id', 'url', 'username', 'first_name', 'last_name', + 'email') + read_only_fields = fields + extra_kwargs = { + 'url': {'view_name': 'api-user-detail'}, + } diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 399150019..017707149 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -222,8 +222,7 @@ def test_update(self): 'state': state.name, 'delegate': user.id}) self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) self.assertEqual(Patch.objects.get(id=patch.id).state, state) - # TODO(stephenfin): This is currently broken due to #216 - # self.assertEqual(Patch.objects.get(id=patch.id).delegate, user) + self.assertEqual(Patch.objects.get(id=patch.id).delegate, user) def test_update_invalid(self): """Ensure we handle invalid Patch updates.""" @@ -243,10 +242,9 @@ def test_update_invalid(self): user_b = create_user() resp = self.client.patch(self.api_url(patch.id), {'delegate': user_b.id}) - # TODO(stephenfin): This is currently broken due to #216 - # self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) - # self.assertContains(resp, "User '%s' is not a maintainer" % user_b, - # status_code=status.HTTP_400_BAD_REQUEST) + self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) + self.assertContains(resp, "User '%s' is not a maintainer" % user_b, + status_code=status.HTTP_400_BAD_REQUEST) def test_delete(self): """Ensure deletions are always rejected.""" diff --git a/releasenotes/notes/issue-216-d3bf9d1baa100f74.yaml b/releasenotes/notes/issue-216-d3bf9d1baa100f74.yaml new file mode 100644 index 000000000..c3756aa00 --- /dev/null +++ b/releasenotes/notes/issue-216-d3bf9d1baa100f74.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + An issue that prevented updating of delegates using the REST API is + resolved. (`#216 `__) From 7af94038002c86345ef0659b100cdaef11b84bcb Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 14 Oct 2018 15:29:47 +0100 Subject: [PATCH 14/73] REST: Allow unsetting of delegate While we recently fixed setting of this field via the API, we didn't resolve unsetting. Fix this now. Signed-off-by: Stephen Finucane (cherry picked from commit 7be99581f1d1b35274987e43ec57272daff94f02) --- patchwork/api/patch.py | 5 ++++- patchwork/tests/api/test_patch.py | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 6b5cc0d1a..7b8e12e39 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -82,7 +82,7 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): project = ProjectSerializer(read_only=True) state = StateField() submitter = PersonSerializer(read_only=True) - delegate = UserSerializer() + delegate = UserSerializer(allow_null=True) mbox = SerializerMethodField() series = SeriesSerializer(many=True, read_only=True) comments = SerializerMethodField() @@ -116,6 +116,9 @@ def get_tags(self, instance): def validate_delegate(self, value): """Check that the delgate is a maintainer of the patch's project.""" + if not value: + return value + if not self.instance.project.maintainer_project.filter( id=value.id).exists(): raise ValidationError("User '%s' is not a maintainer for project " diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 017707149..6a8df512e 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -224,6 +224,13 @@ def test_update(self): self.assertEqual(Patch.objects.get(id=patch.id).state, state) self.assertEqual(Patch.objects.get(id=patch.id).delegate, user) + # (who can unset fields too) + # we need to send as JSON due to https://stackoverflow.com/q/30677216/ + resp = self.client.patch(self.api_url(patch.id), {'delegate': None}, + format='json') + self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) + self.assertIsNone(Patch.objects.get(id=patch.id).delegate) + def test_update_invalid(self): """Ensure we handle invalid Patch updates.""" project = create_project() From 9be793e8af151ce73bbd9587d5791341080363a4 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 14 Oct 2018 14:46:14 +0100 Subject: [PATCH 15/73] Release 2.1.1 Signed-off-by: Stephen Finucane --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index c0a4a8615..1f71f4c13 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 2, 0, 'alpha', 0) +VERSION = (2, 1, 1) __version__ = get_latest_version(VERSION) From 32248ce9c4ac1093eba9ae6bcac1e5d0ac5a62e3 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 14 Oct 2018 14:47:10 +0100 Subject: [PATCH 16/73] Post-release version bump Signed-off-by: Stephen Finucane --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 1f71f4c13..72416b32a 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 1) +VERSION = (2, 1, 2, 'alpha', 0) __version__ = get_latest_version(VERSION) From 877a52413167e7b3baea6ede0a300f812919e7ad Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 26 Oct 2018 10:39:29 +0100 Subject: [PATCH 17/73] tox: Disable W504 ("line break after binary operator") This was introduced in a recent version of 'pycodestyle'. The documentation notes [1] that it is mutually exclusive with W503, which we do enforce, suggesting that we disable one or the other. Avoid the churn and stick to the older rule, which I personally prefer. [1] http://pycodestyle.pycqa.org/en/latest/intro.html#error-codes NOTE(stephenfin): This isn't technically required here, but it does help us maintain a passing 'pep8' tox target on this branch which is generally useful for catching things like SyntaxErrors. Signed-off-by: Stephen Finucane (cherry picked from commit 498a8eeaa6c9d1dca8ab29a845325cff1463e815) --- tox.ini | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index a64d95875..327b9e7c1 100644 --- a/tox.ini +++ b/tox.ini @@ -39,7 +39,12 @@ deps = flake8 commands = flake8 {posargs} patchwork patchwork/bin/pwclient [flake8] -ignore = E129, F405 +# Some rules are ignored as their use makes the code more difficult to read: +# +# E129 visually indented line with same indent as next logical line +# F405 'name' may be undefined, or defined from star imports: 'module' +# W504 line break after binary operator +ignore = E129, F405, W504 exclude = ./patchwork/migrations [testenv:docs] From 7d0b1ddef17283388a1f39099d6c04dadfbd66c5 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 24 Jun 2018 20:55:55 +0100 Subject: [PATCH 18/73] Resolve Python 3.6 warnings DeprecationWarning: invalid escape sequence \d Signed-off-by: Stephen Finucane Reviewed-by: Daniel Axtens Signed-off-by: Daniel Axtens (cherry picked from commit 67c76281c25efd88dfda1a86f4b277e8f1bfbb53) --- patchwork/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/parser.py b/patchwork/parser.py index a2db40373..40a80f267 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -486,7 +486,7 @@ def parse_version(subject, subject_prefixes): Returns: version if found, else 1 """ - regex = re.compile('^[vV](\d+)$') + regex = re.compile(r'^[vV](\d+)$') m = _find_matching_prefix(subject_prefixes, regex) if m: return int(m.group(1)) From 4f1dc4febf7b03449d5d284c5ffa558401a16af0 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 4 Nov 2018 14:25:03 +0000 Subject: [PATCH 19/73] Don't passthrough 'Content-Type: multipart/signed' header We don't GPG signatures, therefore this header is incorrect. Stop passing it through. Test for the other dropped header are also included. Signed-off-by: Stephen Finucane Acked-by: Veronika Kabatova Closes: #221 (cherry picked from commit 22093692a80f9c028dc424cb1e664d449d0dcc4e) --- patchwork/tests/test_mboxviews.py | 15 +++++++++++++++ patchwork/views/utils.py | 6 ++++++ 2 files changed, 21 insertions(+) diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py index 8eb3581ad..dabbb99ca 100644 --- a/patchwork/tests/test_mboxviews.py +++ b/patchwork/tests/test_mboxviews.py @@ -125,6 +125,21 @@ def test_header_passthrough_listid(self): header = 'List-Id: Patchwork development ' self._test_header_passthrough(header) + def _test_header_dropped(self, header): + patch = create_patch(headers=header + '\n') + response = self.client.get(reverse('patch-mbox', args=[patch.id])) + self.assertNotContains(response, header) + + def test_header_dropped_content_transfer_encoding(self): + """Validate dropping of 'Content-Transfer-Encoding' header.""" + header = 'Content-Transfer-Encoding: quoted-printable' + self._test_header_dropped(header) + + def test_header_dropped_content_type_multipart_signed(self): + """Validate dropping of 'Content-Type=multipart/signed' header.""" + header = 'Content-Type: multipart/signed' + self._test_header_dropped(header) + def test_patchwork_id_header(self): """Validate inclusion of generated 'X-Patchwork-Id' header.""" patch = create_patch() diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py index 2357ab86d..fb0195ceb 100644 --- a/patchwork/views/utils.py +++ b/patchwork/views/utils.py @@ -99,8 +99,14 @@ def _submission_to_mbox(submission): orig_headers = HeaderParser().parsestr(str(submission.headers)) for key, val in orig_headers.items(): + # we set this ourselves if key == 'Content-Transfer-Encoding': continue + # we don't save GPG signatures described in RFC1847 [1] so this + # Content-Type value is invalid + # [1] https://tools.ietf.org/html/rfc1847 + if key == 'Content-Type' and val == 'multipart/signed': + continue mail[key] = val if 'Date' not in mail: From 85c31c5a3222d4b075568bdba55698eba7344fa8 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 26 Oct 2018 21:35:03 +0100 Subject: [PATCH 20/73] REST: Show 'web_url' in embedded series responses Signed-off-by: Stephen Finucane Closes: #224 Fixes: 9c179bf4c ("REST: Add 'web_url' link to API responses") (cherry picked from commit ee58fb3be6eaa0deeae966ef3b95fc757df4519b) --- patchwork/api/embedded.py | 3 ++- releasenotes/notes/issue-224-8f1c4207aa273ac6.yaml | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/issue-224-8f1c4207aa273ac6.yaml diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index 314a27fed..f1105b892 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -201,7 +201,8 @@ class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): class Meta: model = models.Series - fields = ('id', 'url', 'date', 'name', 'version', 'mbox') + fields = ('id', 'url', 'web_url', 'date', 'name', 'version', + 'mbox') read_only_fields = fields versioned_field = { '1.1': ('web_url', ), diff --git a/releasenotes/notes/issue-224-8f1c4207aa273ac6.yaml b/releasenotes/notes/issue-224-8f1c4207aa273ac6.yaml new file mode 100644 index 000000000..f6e9ccaa1 --- /dev/null +++ b/releasenotes/notes/issue-224-8f1c4207aa273ac6.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + API resources with embedded series were not showing the ``web_url`` value + for these series. This is now shown. From 63165ec62db3e3e1a3c3bdc057e54a256579e7c5 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 28 Oct 2018 13:31:12 +0000 Subject: [PATCH 21/73] REST: Ensure patch exists for check creation Signed-off-by: Stephen Finucane Closes: #226 (cherry picked from commit 93ce847c359234f987285edbecedc534c7cde50e) --- patchwork/api/check.py | 8 ++++- patchwork/tests/api/test_check.py | 31 ++++++++++++++++++- .../notes/issue-226-27ea72266d3ee9ac.yaml | 7 +++++ 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml diff --git a/patchwork/api/check.py b/patchwork/api/check.py index 1cfa5375b..594ecd4bf 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -17,6 +17,8 @@ # along with Patchwork; if not, write to the Free Software # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +from django.http import Http404 +from django.shortcuts import get_object_or_404 from rest_framework.exceptions import PermissionDenied from rest_framework.generics import ListCreateAPIView from rest_framework.generics import RetrieveAPIView @@ -84,6 +86,10 @@ class CheckMixin(object): def get_queryset(self): patch_id = self.kwargs['patch_id'] + + if not Patch.objects.filter(pk=self.kwargs['patch_id']).exists(): + raise Http404 + return Check.objects.prefetch_related('user').filter(patch=patch_id) @@ -94,7 +100,7 @@ class CheckListCreate(CheckMixin, ListCreateAPIView): ordering = 'id' def create(self, request, patch_id, *args, **kwargs): - p = Patch.objects.get(id=patch_id) + p = get_object_or_404(Patch, id=patch_id) if not p.is_editable(request.user): raise PermissionDenied() request.patch = p diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py index 57d5b775b..bc06e86e5 100644 --- a/patchwork/tests/api/test_check.py +++ b/patchwork/tests/api/test_check.py @@ -90,6 +90,12 @@ def test_list(self): resp = self.client.get(self.api_url(), {'user': 'otheruser'}) self.assertEqual(0, len(resp.data)) + def test_list_invalid_patch(self): + """Ensure we get a 404 for a non-existent patch.""" + resp = self.client.get( + reverse('api-check-list', kwargs={'patch_id': '99999'})) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) + def test_detail(self): """Validate we can get a specific check.""" check = self._create_check() @@ -112,12 +118,21 @@ def test_create(self): self.assertEqual(1, Check.objects.all().count()) self.assertSerialized(Check.objects.first(), resp.data) + def test_create_no_permissions(self): + """Ensure creations are rejected by standard users.""" + check = { + 'state': 'success', + 'target_url': 'http://t.co', + 'description': 'description', + 'context': 'context', + } + user = create_user() self.client.force_authenticate(user=user) resp = self.client.post(self.api_url(), check) self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) - def test_create_invalid(self): + def test_create_invalid_state(self): """Ensure we handle invalid check states.""" check = { 'state': 'this-is-not-a-valid-state', @@ -131,6 +146,20 @@ def test_create_invalid(self): self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) self.assertEqual(0, Check.objects.all().count()) + def test_create_invalid_patch(self): + """Ensure we handle non-existent patches.""" + check = { + 'state': 'success', + 'target_url': 'http://t.co', + 'description': 'description', + 'context': 'context', + } + + self.client.force_authenticate(user=self.user) + resp = self.client.post( + reverse('api-check-list', kwargs={'patch_id': '99999'}), check) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) + def test_update_delete(self): """Ensure updates and deletes aren't allowed""" check = self._create_check() diff --git a/releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml b/releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml new file mode 100644 index 000000000..8f891e04b --- /dev/null +++ b/releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Showing checks for a non-existant patch was returning an empty response + instead of a HTTP 404. Similarly, attempting to create a new check against + this patch would result in a HTTP 5xx error instead of a HTTP 404. Both + issues are now resolved. From 62b0f040689526ea06c8923afcacf08593be999d Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 28 Oct 2018 13:31:34 +0000 Subject: [PATCH 22/73] REST: Ensure submission exists for comment listing Signed-off-by: Stephen Finucane Closes: #225 (cherry picked from commit 3b12675c88ce81800df9e0921f299cf2a082b2aa) --- patchwork/api/comment.py | 5 +++++ patchwork/tests/api/test_comment.py | 12 ++++++++++++ releasenotes/notes/issue-225-94215600c1b23f6e.yaml | 6 ++++++ 3 files changed, 23 insertions(+) create mode 100644 releasenotes/notes/issue-225-94215600c1b23f6e.yaml diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py index 5a5adb1d0..e0068353e 100644 --- a/patchwork/api/comment.py +++ b/patchwork/api/comment.py @@ -19,6 +19,7 @@ import email.parser +from django.http import Http404 from rest_framework.generics import ListAPIView from rest_framework.serializers import SerializerMethodField @@ -26,6 +27,7 @@ from patchwork.api.base import PatchworkPermission from patchwork.api.embedded import PersonSerializer from patchwork.models import Comment +from patchwork.models import Submission class CommentListSerializer(BaseHyperlinkedModelSerializer): @@ -78,6 +80,9 @@ class CommentList(ListAPIView): lookup_url_kwarg = 'pk' def get_queryset(self): + if not Submission.objects.filter(pk=self.kwargs['pk']).exists(): + raise Http404 + return Comment.objects.filter( submission=self.kwargs['pk'] ).select_related('submitter') diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py index f79ea4695..5fcb9463b 100644 --- a/patchwork/tests/api/test_comment.py +++ b/patchwork/tests/api/test_comment.py @@ -75,6 +75,12 @@ def test_list(self): with self.assertRaises(NoReverseMatch): self.client.get(self.api_url(cover_obj, version='1.0')) + def test_list_invalid_cover(self): + """Ensure we get a 404 for a non-existent cover letter.""" + resp = self.client.get( + reverse('api-cover-comment-list', kwargs={'pk': '99999'})) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) + @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API') class TestPatchComments(APITestCase): @@ -113,3 +119,9 @@ def test_list(self): # check we can't access comments using the old version of the API with self.assertRaises(NoReverseMatch): self.client.get(self.api_url(patch_obj, version='1.0')) + + def test_list_invalid_patch(self): + """Ensure we get a 404 for a non-existent patch.""" + resp = self.client.get( + reverse('api-patch-comment-list', kwargs={'pk': '99999'})) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) diff --git a/releasenotes/notes/issue-225-94215600c1b23f6e.yaml b/releasenotes/notes/issue-225-94215600c1b23f6e.yaml new file mode 100644 index 000000000..035e38d83 --- /dev/null +++ b/releasenotes/notes/issue-225-94215600c1b23f6e.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Showing comments for a non-existant patch or cover letter was returning an + empty response instead of a HTTP 404. This issue is resolved for both + resources. From 3a98b6b10ae87fd1804179537349037110706984 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 16 Nov 2018 21:17:53 +0100 Subject: [PATCH 23/73] REST: Fix typo in embedded serializers This was preventing the v1.1 fields from getting dropped as intended. NOTE(stephenfin): 'patchwork/tests/api/test_series.py' was modified to ensure the created series had an associated cover letter and patch. Signed-off-by: Stephen Finucane Closes: #237 (cherry picked from commit 6e48cb627c2731405f991a54085f0148c472d0e6) --- patchwork/api/embedded.py | 8 ++++---- patchwork/tests/api/test_series.py | 18 +++++++++++++++--- .../notes/issue-237-48b9442c31e74b9d.yaml | 5 +++++ 3 files changed, 24 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/issue-237-48b9442c31e74b9d.yaml diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index f1105b892..bc4fd0fe5 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -103,7 +103,7 @@ class Meta: model = models.Bundle fields = ('id', 'url', 'web_url', 'name', 'mbox') read_only_fields = fields - versioned_field = { + versioned_fields = { '1.1': ('web_url', ), } extra_kwargs = { @@ -140,7 +140,7 @@ class Meta: model = models.CoverLetter fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') read_only_fields = fields - versioned_field = { + versioned_fields = { '1.1': ('web_url', 'mbox', ), } extra_kwargs = { @@ -156,7 +156,7 @@ class Meta: model = models.Patch fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') read_only_fields = fields - versioned_field = { + versioned_fields = { '1.1': ('web_url', ), } extra_kwargs = { @@ -204,7 +204,7 @@ class Meta: fields = ('id', 'url', 'web_url', 'date', 'name', 'version', 'mbox') read_only_fields = fields - versioned_field = { + versioned_fields = { '1.1': ('web_url', ), } extra_kwargs = { diff --git a/patchwork/tests/api/test_series.py b/patchwork/tests/api/test_series.py index 11324bc3f..23d8cd28f 100644 --- a/patchwork/tests/api/test_series.py +++ b/patchwork/tests/api/test_series.py @@ -122,13 +122,19 @@ def test_list(self): def test_list_old_version(self): """Validate that newer fields are dropped for older API versions.""" - create_series() + cover_obj = create_cover() + series_obj = create_series() + series_obj.add_cover_letter(cover_obj) + create_series_patch(series=series_obj) resp = self.client.get(self.api_url(version='1.0')) self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(1, len(resp.data)) self.assertIn('url', resp.data[0]) self.assertNotIn('web_url', resp.data[0]) + self.assertNotIn('web_url', resp.data[0]['cover_letter']) + self.assertNotIn('mbox', resp.data[0]['cover_letter']) + self.assertNotIn('web_url', resp.data[0]['patches'][0]) def test_detail(self): """Validate we can get a specific series.""" @@ -141,11 +147,17 @@ def test_detail(self): self.assertSerialized(series, resp.data) def test_detail_version_1_0(self): - series = create_series() + cover_obj = create_cover() + series_obj = create_series() + series_obj.add_cover_letter(cover_obj) + create_series_patch(series=series_obj) - resp = self.client.get(self.api_url(series.id, version='1.0')) + resp = self.client.get(self.api_url(series_obj.id, version='1.0')) self.assertIn('url', resp.data) self.assertNotIn('web_url', resp.data) + self.assertNotIn('web_url', resp.data['cover_letter']) + self.assertNotIn('mbox', resp.data['cover_letter']) + self.assertNotIn('web_url', resp.data['patches'][0]) def test_create_update_delete(self): """Ensure creates, updates and deletes aren't allowed""" diff --git a/releasenotes/notes/issue-237-48b9442c31e74b9d.yaml b/releasenotes/notes/issue-237-48b9442c31e74b9d.yaml new file mode 100644 index 000000000..541f44a0c --- /dev/null +++ b/releasenotes/notes/issue-237-48b9442c31e74b9d.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fields added in API v1.1 are now consistently excluded when requesting API + v1.0, as was intended. From 413bd043df90d22d38a01bc79b64d96e9dba5d3c Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sat, 3 Nov 2018 21:47:51 +0000 Subject: [PATCH 24/73] models: Ensure UserProfile.user is configured Django Admin seems to be doing something funky with how it's handling the creation of a User's corresponding UserProfile instance when modelled as an inline field. Re-setting the UserProfile.user attribute seems to resolve the issue, so do just that. Signed-off-by: Stephen Finucane Closes: #110 (cherry picked from commit b1c2e80187008aff472c6509c67f44583cd3334c) --- patchwork/models.py | 1 + releasenotes/notes/issue-110-a5bb3184bf831280.yaml | 5 +++++ 2 files changed, 6 insertions(+) create mode 100644 releasenotes/notes/issue-110-a5bb3184bf831280.yaml diff --git a/patchwork/models.py b/patchwork/models.py index 6268f5b72..1a2de60a5 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -212,6 +212,7 @@ def __str__(self): def _user_saved_callback(sender, created, instance, **kwargs): try: profile = instance.profile + profile.user = instance except UserProfile.DoesNotExist: profile = UserProfile(user=instance) profile.save() diff --git a/releasenotes/notes/issue-110-a5bb3184bf831280.yaml b/releasenotes/notes/issue-110-a5bb3184bf831280.yaml new file mode 100644 index 000000000..16a0fa5f0 --- /dev/null +++ b/releasenotes/notes/issue-110-a5bb3184bf831280.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Assigning maintained projects when creating a new user in the admin page + was causing an error. This is now resolved. From aa6f4f0385a3deae8925615ccfcc4fb262e819a1 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 4 Nov 2018 14:06:03 +0000 Subject: [PATCH 25/73] parser: Ensure whitespace is stripped for long headers RFC2822 states that long headers can be wrapped using CRLF followed by WSP [1]. For example: Subject: Foo bar, baz Should be parsed as: Foo bar,baz While we were stripping the former, we were not stripping the latter. This mean that we ended up with the following: Foo bar, baz Resolve this. Signed-off-by: Stephen Finucane Closes: #197 (cherry picked from commit 841f966b8d54b2f51ab1c498eed6e5391f2546a9) --- patchwork/parser.py | 1 + patchwork/tests/test_parser.py | 2 ++ releasenotes/notes/issue-197-4f7594db1e4c9887.yaml | 7 +++++++ 3 files changed, 10 insertions(+) create mode 100644 releasenotes/notes/issue-197-4f7594db1e4c9887.yaml diff --git a/patchwork/parser.py b/patchwork/parser.py index 40a80f267..0d335b71b 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -56,6 +56,7 @@ def normalise_space(value): + value = ''.join(re.split(r'\n\s+', value)) whitespace_re = re.compile(r'\s+') return whitespace_re.sub(' ', value).strip() diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index e99cf214e..b9336d377 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -846,6 +846,8 @@ def test_clean_subject(self): self.assertEqual(clean_subject('[PATCH] meep'), ('meep', [])) self.assertEqual(clean_subject("[PATCH] meep \n meep"), ('meep meep', [])) + self.assertEqual(clean_subject("[PATCH] meep,\n meep"), + ('meep,meep', [])) self.assertEqual(clean_subject('[PATCH RFC] meep'), ('[RFC] meep', ['RFC'])) self.assertEqual(clean_subject('[PATCH,RFC] meep'), diff --git a/releasenotes/notes/issue-197-4f7594db1e4c9887.yaml b/releasenotes/notes/issue-197-4f7594db1e4c9887.yaml new file mode 100644 index 000000000..2777fbc2f --- /dev/null +++ b/releasenotes/notes/issue-197-4f7594db1e4c9887.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Long headers can be wrapped using CRLF followed by WSP (whitespace). This + whitespace was not being stripped, resulting in errant whitespace being + saved for the patch subject. This is resolved though existing patches and + cover letters will need to be updated manually. From b00cb3cb8a675ca846d3439c45fae0df53ec342d Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sat, 22 Dec 2018 22:54:05 +0000 Subject: [PATCH 26/73] tests: Add tests for 'series-completed' event In commit 087fe50, we resolved an issue with 'series-completed' events. That entire fix doesn't apply here due to commit 76505e9, but the tests can be included to ensure we don't ever regress. Signed-off-by: Stephen Finucane Stable-Only --- patchwork/tests/test_events.py | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/patchwork/tests/test_events.py b/patchwork/tests/test_events.py index 70d563de3..c5543bb85 100644 --- a/patchwork/tests/test_events.py +++ b/patchwork/tests/test_events.py @@ -42,7 +42,7 @@ def assertEventFields(self, event, parent_type='patch', **fields): self.assertIsNone(field) -class PatchCreateTest(_BaseTestCase): +class PatchCreatedTest(_BaseTestCase): def test_patch_created(self): """No series, so patch dependencies implicitly exist.""" @@ -170,7 +170,7 @@ def test_patch_delegated(self): self.assertEventFields(events[3], previous_delegate=delegate_b) -class CheckCreateTest(_BaseTestCase): +class CheckCreatedTest(_BaseTestCase): def test_check_created(self): check = utils.create_check() @@ -181,7 +181,7 @@ def test_check_created(self): self.assertEventFields(events[0]) -class CoverCreateTest(_BaseTestCase): +class CoverCreatedTest(_BaseTestCase): def test_cover_created(self): cover = utils.create_cover() @@ -192,7 +192,7 @@ def test_cover_created(self): self.assertEventFields(events[0]) -class SeriesCreateTest(_BaseTestCase): +class SeriesCreatedTest(_BaseTestCase): def test_series_created(self): series = utils.create_series() @@ -201,3 +201,28 @@ def test_series_created(self): self.assertEqual(events[0].category, Event.CATEGORY_SERIES_CREATED) self.assertEqual(events[0].project, series.project) self.assertEventFields(events[0]) + + +class SeriesChangedTest(_BaseTestCase): + + def test_series_completed(self): + """Validate 'series-completed' events.""" + series = utils.create_series(total=2) + + # the series has no patches associated with it so it's not yet complete + events = _get_events(series=series) + self.assertNotIn(Event.CATEGORY_SERIES_COMPLETED, + [x.category for x in events]) + + # create the second of two patches in the series; series is still not + # complete + utils.create_series_patch(series=series, number=2) + events = _get_events(series=series) + self.assertNotIn(Event.CATEGORY_SERIES_COMPLETED, + [x.category for x in events]) + + # now create the first patch, which will "complete" the series + utils.create_series_patch(series=series, number=1) + events = _get_events(series=series) + self.assertIn(Event.CATEGORY_SERIES_COMPLETED, + [x.category for x in events]) From 0c60d688d0ac3cf41477970b9c8d299ef1d4227b Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Thu, 28 Feb 2019 15:29:53 +1100 Subject: [PATCH 27/73] parser: recognise git commit consisting only of empty new file Commits with only an empty new file are liable to be missed. The parser state machine doesn't recognise the headers "new file mode" and "index": teach it about them. Add a test to demonstrate. It's a little bit academic as you don't usually send patches like that but sometimes you do, especially if you're a snowpatch dev :) Closes: #256 Reported-by: Andrew Donnellan Signed-off-by: Daniel Axtens Reviewed-by: Andrew Donnellan Reviewed-by: Stephen Finucane (cherry picked from commit 9dd0b5ba843ccac5cf768a1c34226abe8b85cf6d) --- patchwork/parser.py | 10 +++--- .../tests/mail/0021-git-empty-new-file.mbox | 32 +++++++++++++++++++ patchwork/tests/test_parser.py | 5 +++ 3 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 patchwork/tests/mail/0021-git-empty-new-file.mbox diff --git a/patchwork/parser.py b/patchwork/parser.py index 0d335b71b..bafb8d886 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -751,7 +751,7 @@ def parse_patch(content): # 3: patch header line 2 (+++) # 4: patch hunk header line (@@ line) # 5: patch hunk content - # 6: patch meta header (rename from/rename to) + # 6: patch meta header (rename from/rename to/new file/index) # # valid transitions: # 0 -> 1 (diff, Index:) @@ -761,7 +761,7 @@ def parse_patch(content): # 3 -> 4 (@@ line) # 4 -> 5 (patch content) # 5 -> 1 (run out of lines from @@-specifed count) - # 1 -> 6 (rename from / rename to) + # 1 -> 6 (rename from / rename to / new file / index) # 6 -> 2 (---) # 6 -> 1 (other text) # @@ -791,7 +791,8 @@ def parse_patch(content): if line.startswith('--- '): state = 2 - if line.startswith(('rename from ', 'rename to ')): + if line.startswith(('rename from ', 'rename to ', + 'new file mode ', 'index ')): state = 6 elif state == 2: if line.startswith('+++ '): @@ -852,7 +853,8 @@ def fn(x): else: state = 5 elif state == 6: - if line.startswith(('rename to ', 'rename from ')): + if line.startswith(('rename to ', 'rename from ', + 'new file mode ', 'index ')): patchbuf += buf + line buf = '' elif line.startswith('--- '): diff --git a/patchwork/tests/mail/0021-git-empty-new-file.mbox b/patchwork/tests/mail/0021-git-empty-new-file.mbox new file mode 100644 index 000000000..c3be48e6e --- /dev/null +++ b/patchwork/tests/mail/0021-git-empty-new-file.mbox @@ -0,0 +1,32 @@ +From andrew.donnellan@au1.ibm.com Thu Feb 28 00:37:42 2019 +Delivered-To: dja@axtens.net +Received: by 2002:a4a:2812:0:0:0:0:0 with SMTP id h18csp2242ooa; + Wed, 27 Feb 2019 16:37:59 -0800 (PST) +From: Andrew Donnellan +Subject: [snowpatch] [PATCH 1/3] Test commit; please ignore +To: Daniel Axtens +Date: Thu, 28 Feb 2019 11:37:42 +1100 +User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 + Thunderbird/60.5.1 +MIME-Version: 1.0 +Content-Language: en-AU + + +Doing some snowpatching. +--- + banana | 0 + 1 file changed, 0 insertions(+), 0 deletions(-) + create mode 100644 banana + +diff --git a/banana b/banana +new file mode 100644 +index 000000000000..e69de29bb2d1 +-- +2.11.0 + +_______________________________________________ +snowpatch mailing list +snowpatch@lists.ozlabs.org +https://lists.ozlabs.org/listinfo/snowpatch + + diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index b9336d377..a4b3d7109 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -597,6 +597,11 @@ def test_git_rename_with_diff(self): self.assertEqual(diff.count("\nrename to "), 2) self.assertEqual(diff.count('\n-a\n+b'), 1) + def test_git_new_empty_file(self): + diff, message = self._find_content('0021-git-empty-new-file.mbox') + self.assertTrue(diff is not None) + self.assertTrue(message is not None) + def test_cvs_format(self): diff, message = self._find_content('0007-cvs-format-diff.mbox') self.assertTrue(diff.startswith('Index')) From 641b1232169023b74c3bf900c1a6f2347743ad72 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 30 Apr 2019 16:02:59 +1000 Subject: [PATCH 28/73] REST: Check.user is not read-only We only support 'Check' creation - not check updating. As a result, there's no real reason that the 'Check.user' field should be read-only and this is causing an issue with Django REST Framework 3.7. Simply remove the attribute and extend the tests to validate things are working as expected. Signed-off-by: Stephen Finucane Reviewed-by: Daniel Axtens Signed-off-by: Daniel Axtens (cherry picked from commit c9cc59dac70d76971a5342ca53e2b13eb93592de) --- patchwork/api/check.py | 2 +- patchwork/tests/api/test_check.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/patchwork/api/check.py b/patchwork/api/check.py index 594ecd4bf..d76573a52 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -46,7 +46,7 @@ class CheckSerializer(HyperlinkedModelSerializer): url = CheckHyperlinkedIdentityField('api-check-detail') patch = HiddenField(default=CurrentPatchDefault()) - user = UserSerializer(read_only=True, default=CurrentUserDefault()) + user = UserSerializer(default=CurrentUserDefault()) def run_validation(self, data): for val, label in Check.STATE_CHOICES: diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py index bc06e86e5..f5a8eca15 100644 --- a/patchwork/tests/api/test_check.py +++ b/patchwork/tests/api/test_check.py @@ -67,6 +67,7 @@ def assertSerialized(self, check_obj, check_json): self.assertEqual(check_obj.target_url, check_json['target_url']) self.assertEqual(check_obj.context, check_json['context']) self.assertEqual(check_obj.description, check_json['description']) + self.assertEqual(check_obj.user.id, check_json['user']['id']) def test_list(self): """Validate we can list checks on a patch.""" From ca544ba9e19766b1b38089f111e6d0917ceeb2b3 Mon Sep 17 00:00:00 2001 From: Andrew Donnellan Date: Tue, 30 Apr 2019 16:03:00 +1000 Subject: [PATCH 29/73] REST: Add new setting for maximum API page size In 41790caf59ad ("REST: Limit max page size") we limited the maximum page size to the default page size in the settings. This turns out to be rather restrictive, as we usually want to keep the default page size low, but an administrator may want to allow API clients to fetch more than that per request. Add a new setting, MAX_REST_RESULTS_PER_PAGE, to set the maximum page size. Closes: #202 ("Separate max API page size and default API page size into different settings") Suggested-by: Stewart Smith Suggested-by: Joel Stanley Signed-off-by: Andrew Donnellan [dja: set to 250 as per mailing list discussion] Signed-off-by: Daniel Axtens (cherry picked from commit 8fe11180a1a59f6e8e5a4441b21a3d9831f0b69d) --- docs/deployment/configuration.rst | 8 +++++++- patchwork/api/base.py | 3 ++- patchwork/settings/base.py | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/deployment/configuration.rst b/docs/deployment/configuration.rst index 347485636..e599522a4 100644 --- a/docs/deployment/configuration.rst +++ b/docs/deployment/configuration.rst @@ -88,7 +88,13 @@ Enable the :doc:`REST API <../api/rest>`. The number of items to include in REST API responses by default. This can be overridden by the ``per_page`` parameter for some endpoints. -.. versionadded:: 2.0 +``MAX_REST_RESULTS_PER_PAGE`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The maximum number of items that can be requested in a REST API request using +the ``per_page`` parameter. + +.. versionadded:: 2.2 ``COMPAT_REDIR`` ~~~~~~~~~~~~~~~~ diff --git a/patchwork/api/base.py b/patchwork/api/base.py index 8c38d5a1d..bf452f78b 100644 --- a/patchwork/api/base.py +++ b/patchwork/api/base.py @@ -36,7 +36,8 @@ class LinkHeaderPagination(PageNumberPagination): https://tools.ietf.org/html/rfc5988#section-5 https://developer.github.com/guides/traversing-with-pagination """ - page_size = max_page_size = settings.REST_RESULTS_PER_PAGE + page_size = settings.REST_RESULTS_PER_PAGE + max_page_size = settings.MAX_REST_RESULTS_PER_PAGE page_size_query_param = 'per_page' def get_paginated_response(self, data): diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py index 4b0d55138..45c66da79 100644 --- a/patchwork/settings/base.py +++ b/patchwork/settings/base.py @@ -220,6 +220,7 @@ ENABLE_REST_API = True REST_RESULTS_PER_PAGE = 30 +MAX_REST_RESULTS_PER_PAGE = 250 # Set to True to enable redirections or URLs from previous versions # of patchwork From ea9301b76d0fc67b02097cabebd23efbe52300e7 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 30 Apr 2019 16:03:01 +1000 Subject: [PATCH 30/73] notifications: fix notification expiry when no user is associated It's possible that an EmailConfirmation object will have no associated user (eg, for email opt-out, which does not require a user object). In this case, we will see a NULL value for EmailConfirmation.user_id. However, having a NULL value appear in a SQL 'IN' clause will match every value. This means that once one of these null-user EmailConfirmations is present, we will never expire any non-active user accounts. This change adds a filter for a valid user_id when we query for active EmailConfirmation objects. This means we'll have a valid values set to use in the pending_confs set. Signed-off-by: Jeremy Kerr [dja: fix pep8 issue] Signed-off-by: Daniel Axtens (cherry picked from commit d0b79d9dee04aee13c8d64a193a7818f72eeca3b) --- patchwork/notifications.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patchwork/notifications.py b/patchwork/notifications.py index a5f642352..dce525aba 100644 --- a/patchwork/notifications.py +++ b/patchwork/notifications.py @@ -109,7 +109,8 @@ def expire_notifications(): EmailConfirmation.objects.filter(q).delete() # remove inactive users with no pending confirmation - pending_confs = EmailConfirmation.objects.values('user') + pending_confs = (EmailConfirmation.objects + .filter(user__isnull=False).values('user')) users = User.objects.filter(is_active=False).exclude(id__in=pending_confs) # delete users From b7aa6f1f62c56dade7dd524ebcba58546fb07f6b Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 30 Apr 2019 16:03:02 +1000 Subject: [PATCH 31/73] REST: Handle JSON requests This was raising an attribute error when switching tests to use JSON bodies instead of form-data. AttributeError: 'dict' object has no attribute '_mutable' The easy fix is to check if it's a dictionary and avoid the mutability check if so. NOTE(daxtens): This is needed for JSON bodies sent by regular users, not just the tests. Signed-off-by: Stephen Finucane (cherry picked from commit dc48fbce99efe7d13987a3f510f7dee389636eba) --- patchwork/api/check.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/patchwork/api/check.py b/patchwork/api/check.py index d76573a52..67062132f 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -50,7 +50,12 @@ class CheckSerializer(HyperlinkedModelSerializer): def run_validation(self, data): for val, label in Check.STATE_CHOICES: - if label == data['state']: + if label != data['state']: + continue + + if isinstance(data, dict): # json request + data['state'] = val + else: # form-data request # NOTE(stephenfin): 'data' is essentially 'request.POST', which # is immutable by default. However, there's no good reason for # this to be this way [1], so temporarily unset that mutability @@ -61,7 +66,8 @@ def run_validation(self, data): data._mutable = True # noqa data['state'] = val data._mutable = mutable # noqa - break + + break return super(CheckSerializer, self).run_validation(data) def to_representation(self, instance): From d963655ccd89c68648dbe3b2843a4a7b06aa78ce Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Tue, 30 Apr 2019 16:03:03 +1000 Subject: [PATCH 32/73] REST: Handle regular form data requests for checks 08d1459a4a40 ("Add REST API validation using OpenAPI schema") moved all API requests to JSON blobs rather than form data. dc48fbce99ef ("REST: Handle JSON requests") attempted to change the check serialiser to handle this. However, because both a JSON dict and a QueryDict satisfy isinstance(data, dict), everything was handled as JSON and the old style requests were broken. Found in the process of debugging issues from the OzLabs PW & Snowpatch crew - I'm not sure if they actually hit this one, but kudos to them anyway as we wouldn't have found it without them. NOTE(daxtens): This does not need the new tests as we do not have 08d1459a4a40, so we just need the fix to the API. We do not add a JSON test to stable. Fixes: dc48fbce99ef ("REST: Handle JSON requests") Signed-off-by: Daniel Axtens (cherry picked from commit 666de29ebada5990a8d69f4d71d6bb271e1a68c3) --- patchwork/api/check.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/patchwork/api/check.py b/patchwork/api/check.py index 67062132f..62e6fd19e 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -19,6 +19,7 @@ from django.http import Http404 from django.shortcuts import get_object_or_404 +from django.http.request import QueryDict from rest_framework.exceptions import PermissionDenied from rest_framework.generics import ListCreateAPIView from rest_framework.generics import RetrieveAPIView @@ -53,9 +54,7 @@ def run_validation(self, data): if label != data['state']: continue - if isinstance(data, dict): # json request - data['state'] = val - else: # form-data request + if isinstance(data, QueryDict): # form-data request # NOTE(stephenfin): 'data' is essentially 'request.POST', which # is immutable by default. However, there's no good reason for # this to be this way [1], so temporarily unset that mutability @@ -66,6 +65,8 @@ def run_validation(self, data): data._mutable = True # noqa data['state'] = val data._mutable = mutable # noqa + else: # json request + data['state'] = val break return super(CheckSerializer, self).run_validation(data) From 233e6849f636c67048ef81290d48a168f7d1c766 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Tue, 30 Apr 2019 16:03:04 +1000 Subject: [PATCH 33/73] REST: A check must specify a state The Ozlabs crew noticed that a check without a state caused a KeyError in data['state']. Mark state as mandatory, check for it, and add a test. NOTE(daxtens): Swagger changes are excluded from the backport Reported-by: Russell Currey Reported-by: Jeremy Kerr Signed-off-by: Daniel Axtens (cherry picked from commit 7a20ccda99e48dab643d1fbd7e170fe3e4c47185) --- patchwork/api/check.py | 4 ++++ patchwork/tests/api/test_check.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/patchwork/api/check.py b/patchwork/api/check.py index 62e6fd19e..1498abbbf 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -26,6 +26,7 @@ from rest_framework.serializers import CurrentUserDefault from rest_framework.serializers import HiddenField from rest_framework.serializers import HyperlinkedModelSerializer +from rest_framework.serializers import ValidationError from patchwork.api.base import CheckHyperlinkedIdentityField from patchwork.api.base import MultipleFieldLookupMixin @@ -50,6 +51,9 @@ class CheckSerializer(HyperlinkedModelSerializer): user = UserSerializer(default=CurrentUserDefault()) def run_validation(self, data): + if 'state' not in data or data['state'] == '': + raise ValidationError({'state': ["A check must have a state."]}) + for val, label in Check.STATE_CHOICES: if label != data['state']: continue diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py index f5a8eca15..e3ad099cf 100644 --- a/patchwork/tests/api/test_check.py +++ b/patchwork/tests/api/test_check.py @@ -147,6 +147,22 @@ def test_create_invalid_state(self): self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) self.assertEqual(0, Check.objects.all().count()) + def test_create_missing_state(self): + """Create a check using invalid values. + + Ensure we handle the state being absent. + """ + check = { + 'target_url': 'http://t.co', + 'description': 'description', + 'context': 'context', + } + + self.client.force_authenticate(user=self.user) + resp = self.client.post(self.api_url(), check) + self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) + self.assertEqual(0, Check.objects.all().count()) + def test_create_invalid_patch(self): """Ensure we handle non-existent patches.""" check = { From b99531224fcbadcdb9fea85179d1a7ea1dad4462 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 24 Jun 2018 20:55:48 +0100 Subject: [PATCH 34/73] Add support for Django REST Framework 3.7, 3.8 No breaking changes that concern us here. Signed-off-by: Stephen Finucane Reviewed-by: Daniel Axtens Signed-off-by: Daniel Axtens (backported from commit a27f36fb784867e434527b8e65064ae3bdb12c82 to not include changes to requirements-*.txt, just release note and tox.ini changes.) Signed-off-by: Daniel Axtens --- README.rst | 2 +- .../notes/django-rest-framework-3-7-bc6ad5df8bc54afc.yaml | 6 ++++++ .../notes/django-rest-framework-3-8-23865db833b4d188.yaml | 6 ++++++ tox.ini | 5 +++-- 4 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/django-rest-framework-3-7-bc6ad5df8bc54afc.yaml create mode 100644 releasenotes/notes/django-rest-framework-3-8-23865db833b4d188.yaml diff --git a/README.rst b/README.rst index 4a970ec5a..ddc4b9812 100644 --- a/README.rst +++ b/README.rst @@ -45,7 +45,7 @@ Requirements - Django (1.8 - 1.11) -- Django REST Framework (3.2 - 3.6) +- Django REST Framework (3.4 - 3.8) - Django Filters (1.0) diff --git a/releasenotes/notes/django-rest-framework-3-7-bc6ad5df8bc54afc.yaml b/releasenotes/notes/django-rest-framework-3-7-bc6ad5df8bc54afc.yaml new file mode 100644 index 000000000..4bf92c999 --- /dev/null +++ b/releasenotes/notes/django-rest-framework-3-7-bc6ad5df8bc54afc.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + `Django REST Framework 3.7 + `_ is now + supported. diff --git a/releasenotes/notes/django-rest-framework-3-8-23865db833b4d188.yaml b/releasenotes/notes/django-rest-framework-3-8-23865db833b4d188.yaml new file mode 100644 index 000000000..dc2d2c8f8 --- /dev/null +++ b/releasenotes/notes/django-rest-framework-3-8-23865db833b4d188.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + `Django REST Framework 3.8 + `_ is now + supported. diff --git a/tox.ini b/tox.ini index 327b9e7c1..5c741d374 100644 --- a/tox.ini +++ b/tox.ini @@ -10,8 +10,9 @@ deps = django19: django>=1.9,<1.10 django110: django>=1.10,<1.11 django111: django>=1.11,<2.0 - django{18,19,110}: djangorestframework>=3.4,<3.7 - django111: djangorestframework>=3.6,<3.7 + django{18,19}: djangorestframework>=3.4,<3.7 + django110: djangorestframework>=3.4,<3.9 + django111: djangorestframework>=3.6,<3.9 django{18,19,110,111}: django-filter>=1.0,<1.1 setenv = DJANGO_SETTINGS_MODULE = patchwork.settings.dev From 4a4862f6d18797ab44f860cf7d669121b2eda7c9 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 24 Jun 2018 20:55:49 +0100 Subject: [PATCH 35/73] Add support for django-filter 1.1 There's one warning to handle here. Signed-off-by: Stephen Finucane Signed-off-by: Daniel Axtens (backported from commit 4ad2558f884bf33201e179a171ad3821a62126a1 to not include changes to requirements-*.txt, just release note, code and tox.ini changes.) Signed-off-by: Daniel Axtens --- README.rst | 2 +- patchwork/api/filters.py | 5 +++-- patchwork/compat.py | 16 ++++++++++++++++ .../django-filter-1-1-6a6b0ab8cbe0faca.yaml | 6 ++++++ tox.ini | 3 ++- 5 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/django-filter-1-1-6a6b0ab8cbe0faca.yaml diff --git a/README.rst b/README.rst index ddc4b9812..94ac32c3b 100644 --- a/README.rst +++ b/README.rst @@ -47,7 +47,7 @@ Requirements - Django REST Framework (3.4 - 3.8) -- Django Filters (1.0) +- Django Filters (1.0 - 1.1) Development Installation ------------------------ diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py index 73353d900..ddf527fd8 100644 --- a/patchwork/api/filters.py +++ b/patchwork/api/filters.py @@ -26,6 +26,7 @@ from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField from django.forms.widgets import MultipleHiddenInput +from patchwork.compat import NAME_FIELD from patchwork.models import Bundle from patchwork.models import Check from patchwork.models import CoverLetter @@ -151,8 +152,8 @@ class UserFilter(ModelMultipleChoiceFilter): class TimestampMixin(FilterSet): # TODO(stephenfin): These should filter on a 'updated_at' field instead - before = IsoDateTimeFilter(name='date', lookup_expr='lt') - since = IsoDateTimeFilter(name='date', lookup_expr='gte') + before = IsoDateTimeFilter(lookup_expr='lt', **{NAME_FIELD: 'date'}) + since = IsoDateTimeFilter(lookup_expr='gte', **{NAME_FIELD: 'date'}) class SeriesFilterSet(TimestampMixin, FilterSet): diff --git a/patchwork/compat.py b/patchwork/compat.py index 38caa4e86..3bbff447d 100644 --- a/patchwork/compat.py +++ b/patchwork/compat.py @@ -41,6 +41,22 @@ from rest_framework.filters import DjangoFilterBackend # noqa +# NAME_FIELD +# +# The django-filter library renamed 'Filter.name' to 'Filter.field_name' in +# 1.1. +# +# https://django-filter.readthedocs.io/en/master/guide/migration.html#migrating-to-2-0 + +if settings.ENABLE_REST_API: + import django_filters # noqa + + if django_filters.VERSION >= (1, 1): + NAME_FIELD = 'field_name' + else: + NAME_FIELD = 'name' + + # reverse, reverse_lazy # # The reverse and reverse_lazy functions have been moved to django.urls in diff --git a/releasenotes/notes/django-filter-1-1-6a6b0ab8cbe0faca.yaml b/releasenotes/notes/django-filter-1-1-6a6b0ab8cbe0faca.yaml new file mode 100644 index 000000000..3beb9c084 --- /dev/null +++ b/releasenotes/notes/django-filter-1-1-6a6b0ab8cbe0faca.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + `django-filter 1.1 + `_ is + now supported. diff --git a/tox.ini b/tox.ini index 5c741d374..c242c97f4 100644 --- a/tox.ini +++ b/tox.ini @@ -13,7 +13,8 @@ deps = django{18,19}: djangorestframework>=3.4,<3.7 django110: djangorestframework>=3.4,<3.9 django111: djangorestframework>=3.6,<3.9 - django{18,19,110,111}: django-filter>=1.0,<1.1 + django18: django-filter>=1.0,<1.1 + django{19,110,111}: django-filter>=1.0,<1.2 setenv = DJANGO_SETTINGS_MODULE = patchwork.settings.dev PYTHONDONTWRITEBYTECODE = 1 From 9b4d7a2cf4b845de162f80ef9c44144b32263e41 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 18 Oct 2018 17:45:21 +0100 Subject: [PATCH 36/73] Add support for Django REST Framework 3.9 As with 3.7 and 3.8, there are no breaking changes we need to be concerned with here. Signed-off-by: Stephen Finucane (backported from commit fab5571e2f20323feed84609a52778a0f8e41e32 to not include changes to requirements-*.txt, just release note and tox.ini changes.) Signed-off-by: Daniel Axtens --- .../notes/django-rest-framework-3-9-0afb78322dd82367.yaml | 6 ++++++ tox.ini | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/django-rest-framework-3-9-0afb78322dd82367.yaml diff --git a/releasenotes/notes/django-rest-framework-3-9-0afb78322dd82367.yaml b/releasenotes/notes/django-rest-framework-3-9-0afb78322dd82367.yaml new file mode 100644 index 000000000..e65531808 --- /dev/null +++ b/releasenotes/notes/django-rest-framework-3-9-0afb78322dd82367.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + `Django REST Framework 3.9 + `_ is now + supported. diff --git a/tox.ini b/tox.ini index c242c97f4..ed9e77f96 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,7 @@ deps = django111: django>=1.11,<2.0 django{18,19}: djangorestframework>=3.4,<3.7 django110: djangorestframework>=3.4,<3.9 - django111: djangorestframework>=3.6,<3.9 + django111: djangorestframework>=3.6,<3.10 django18: django-filter>=1.0,<1.1 django{19,110,111}: django-filter>=1.0,<1.2 setenv = From bc4c011e894780e8b98920c61c2a647ee30e56d5 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Tue, 30 Apr 2019 15:48:18 +1000 Subject: [PATCH 37/73] Release 2.1.2 Signed-off-by: Daniel Axtens --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 72416b32a..241f496cf 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 2, 'alpha', 0) +VERSION = (2, 1, 2) __version__ = get_latest_version(VERSION) From 8742e9e353107f6acd9e93a0b14c052c5c578df2 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Tue, 30 Apr 2019 15:50:06 +1000 Subject: [PATCH 38/73] Post-release version bump Signed-off-by: Daniel Axtens --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 241f496cf..36b1bfee8 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 2) +VERSION = (2, 1, 3, 'alpha', 0) __version__ = get_latest_version(VERSION) From b58236e9cd93f1a033768b427d601ea0de96ccfb Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Tue, 14 May 2019 16:11:25 +1000 Subject: [PATCH 39/73] Revert "parser: Ensure whitespace is stripped for long headers" This reverts commit 841f966b8d54b2f51ab1c498eed6e5391f2546a9. In July 2018, we received a report of OzLabs patchwork mangling emails that have subjects containing words with internal commas, like "Insert DT binding for foo,bar" (#197). Stephen took a look and came up with the comment this reverts. Quoting the commit message: RFC2822 states that long headers can be wrapped using CRLF followed by WSP [1]. For example: Subject: Foo bar, baz Should be parsed as: Foo bar,baz As it turns out, this is not the case. Journey with me to section 2.2.3 of RFC 2822: 2.2.3. Long Header Fields Each header field is logically a single line of characters comprising the field name, the colon, and the field body. For convenience however, and to deal with the 998/78 character limitations per line, the field body portion of a header field can be split into a multiple line representation; this is called "folding". The general rule is that wherever this standard allows for folding white space (not simply WSP characters), a CRLF may be inserted before any WSP. For example, the header field: Subject: This is a test can be represented as: Subject: This is a test So the issue with the example in the reverted commit is that there is no folding white space in "bar,baz", so it's not valid to split it. These are valid: Subject: Foo bar,baz Subject: Foo bar,baz but splitting "bar,baz" into "bar,\n baz" is not valid. What then is correct unfolding behaviour? Quoting the RFC again: The process of moving from this folded multiple-line representation of a header field to its single line representation is called "unfolding". Unfolding is accomplished by simply removing any CRLF that is immediately followed by WSP. Each header field should be treated in its unfolded form for further syntactic and semantic evaluation. In other words, the unfolding rule requires you to strip the CRLF, but it does not permit you to strip the WSP. Indeed, if "bar,\n baz" is received, the correct unfolding is "bar, baz". If you do strip the WSP, you end up mashing words together, such as in https://patchwork.ozlabs.org/patch/1097852/ So revert the commit, restoring original behaviour, but keep a corrected version of the test. This presents a big question though: how did Rob's email up with a mangled subject line? To answer this question, you end up having to learn about OzLabs Patchwork and how it differs from Patchwork the project. OzLabs Patchwork (patchwork.ozlabs.org) is an installation of Patchwork. Part of what makes it so useful for so many projects is a little intervening layer that can massage some mail to make it end up in the right project. Email that lands in the device tree project is an example of email that goes through this process. I only learned about this today and I haven't looked in any detail at precisely what is done to the mail. The script is not part of the Patchwork project. This intervening filter is a Python script that runs - and this is an important detail - in Python 2.7. Ignoring all the details, the filter basically operates in a pipe between the mail program and patchwork's parsemail, like (mail from system) | filter.py | parsemail At it's very simplest, filter.py acts as follows: import email import sys mail = email.parse_from_file(sys.stdin) sys.stdout.write(mail.as_string()) Fascinatingly, if you take Rob's email from #197 and put it through this process, you can see that it is getting mangled: Before: Subject: [PATCH v2 3/4] dt-bindings: sound: wm8994: document wlf,csnaddr-pd property After: Subject: [PATCH v2 3/4] dt-bindings: sound: wm8994: document wlf, csnaddr-pd property You can see that python27 has incorrectly wrapped the header, breaking where there is not a foldable space. Python3 does not have this issue. To summarise: - part of the magic of OzLabs PW is a filter to make sure mail gets to the right place. This isn't part of the Patchwork project and so is usually invisible to patchwork developers. - the filter is written in python27. The email module in py27 has a bug that incorrectly breaks subjects around commas within words. - patchwork correctly unfolds those broken subjects with a space after the comma. - the extra space was interpreted as a bug in patchwork, leading to a misinterpretation of the spec to strip out the whitespace that was believed to be in error. - that broke other wrapped subjects. To solve this, revert the commit and I'll work with jk to get the filter script into py3 compatibility. (Given that py27 sunsets in ~7mo, trying to fix it is not worth it.) Closes: #273 Signed-off-by: Daniel Axtens Signed-off-by: Stephen Finucane [stephenfin: Use a new release note instead of editing the original one] (cherry picked from commit 0c7d45e35fe409dd056ccc4558808a8e97ffaf0e) --- patchwork/parser.py | 1 - patchwork/tests/test_parser.py | 2 +- releasenotes/notes/issue-273-2bb8d2bf5fa9a57e.yaml | 8 ++++++++ 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/issue-273-2bb8d2bf5fa9a57e.yaml diff --git a/patchwork/parser.py b/patchwork/parser.py index bafb8d886..e064b0ffc 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -56,7 +56,6 @@ def normalise_space(value): - value = ''.join(re.split(r'\n\s+', value)) whitespace_re = re.compile(r'\s+') return whitespace_re.sub(' ', value).strip() diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index a4b3d7109..8255c657e 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -852,7 +852,7 @@ def test_clean_subject(self): self.assertEqual(clean_subject("[PATCH] meep \n meep"), ('meep meep', [])) self.assertEqual(clean_subject("[PATCH] meep,\n meep"), - ('meep,meep', [])) + ('meep, meep', [])) self.assertEqual(clean_subject('[PATCH RFC] meep'), ('[RFC] meep', ['RFC'])) self.assertEqual(clean_subject('[PATCH,RFC] meep'), diff --git a/releasenotes/notes/issue-273-2bb8d2bf5fa9a57e.yaml b/releasenotes/notes/issue-273-2bb8d2bf5fa9a57e.yaml new file mode 100644 index 000000000..506de0db9 --- /dev/null +++ b/releasenotes/notes/issue-273-2bb8d2bf5fa9a57e.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + `#197`__ was the result of a issue with OzLabs instance and not Patchwork + itself, and the fix included actually ended up corrupting subjects for + everyone. It has now been reverted. + + __ https://github.com/getpatchwork/patchwork/issues/197 From 7b141134eb97163d285daf928ab322fde27d7dc8 Mon Sep 17 00:00:00 2001 From: Petr Vorel Date: Sun, 5 May 2019 22:20:05 +0200 Subject: [PATCH 40/73] parser: Add missing extended header lines Patchwork didn't recognise some patches due missing some extended header lines (e.g. "old mode" and "new mode" for renaming file mode, see [1]). Thus adding all modes from git doc [2]. [1] https://lists.ozlabs.org/pipermail/patchwork/2019-April/005741.html [2] https://git-scm.com/docs/git-diff#_generating_patches_with_p Suggested-by: Veronika Kabatova Signed-off-by: Petr Vorel Closes: #267 (cherry picked from commit 753e4572d7e8b836daf6087820c49637b3b23602) --- patchwork/parser.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/patchwork/parser.py b/patchwork/parser.py index e064b0ffc..0fedaa651 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -760,7 +760,7 @@ def parse_patch(content): # 3 -> 4 (@@ line) # 4 -> 5 (patch content) # 5 -> 1 (run out of lines from @@-specifed count) - # 1 -> 6 (rename from / rename to / new file / index) + # 1 -> 6 (extended header lines) # 6 -> 2 (---) # 6 -> 1 (other text) # @@ -790,7 +790,15 @@ def parse_patch(content): if line.startswith('--- '): state = 2 - if line.startswith(('rename from ', 'rename to ', + # extended header lines + # @see https://git-scm.com/docs/git-diff#_generating_patches_with_p + if line.startswith(('old mode ', 'new mode ', + 'deleted file mode ', + 'new file mode ', + 'copy from ', 'copy to ', + 'rename from ', 'rename to ', + 'similarity index ', + 'dissimilarity index ', 'new file mode ', 'index ')): state = 6 elif state == 2: From f205495b6a9c2a0879984b9367a5b5d0e2443058 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 4 Jun 2019 12:55:38 +0100 Subject: [PATCH 41/73] parser: Include extended header lines in diff Commit 753e4572d updated the parser to consider additional header lines when deciding where a patch message ends and the diff begins. However, these additional lines were not captured meaning these patches didn't have a diff associated with them and they therefore weren't patches in the Patchwork sense of the term. Correct this and add a test. Signed-off-by: Stephen Finucane (cherry picked from commit fc1d7505991474b07638a87c0834157d12e1e476) --- patchwork/parser.py | 10 +++++++- .../tests/mail/0022-git-mode-change.mbox | 23 +++++++++++++++++++ patchwork/tests/test_parser.py | 5 ++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 patchwork/tests/mail/0022-git-mode-change.mbox diff --git a/patchwork/parser.py b/patchwork/parser.py index 0fedaa651..fb4b3a0ab 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -860,7 +860,15 @@ def fn(x): else: state = 5 elif state == 6: - if line.startswith(('rename to ', 'rename from ', + # extended header lines + # @see https://git-scm.com/docs/git-diff#_generating_patches_with_p + if line.startswith(('old mode ', 'new mode ', + 'deleted file mode ', + 'new file mode ', + 'copy from ', 'copy to ', + 'rename from ', 'rename to ', + 'similarity index ', + 'dissimilarity index ', 'new file mode ', 'index ')): patchbuf += buf + line buf = '' diff --git a/patchwork/tests/mail/0022-git-mode-change.mbox b/patchwork/tests/mail/0022-git-mode-change.mbox new file mode 100644 index 000000000..bf280bb8c --- /dev/null +++ b/patchwork/tests/mail/0022-git-mode-change.mbox @@ -0,0 +1,23 @@ +From linux-kbuild Sun Apr 07 23:09:09 2019 +From: Petr Vorel +Date: Sun, 07 Apr 2019 23:09:09 +0000 +To: linux-kbuild +Subject: [PATCH 1/1] kconfig: Make nconf-cfg.sh executable +Message-Id: <20190407230909.20668-1-pvorel@suse.cz> +X-MARC-Message: https://marc.info/?l=linux-kbuild&m=155467856208923 + +Although it's not required for the build *conf-cfg.sh scripts to be +executable (they're run by CONFIG_SHELL), let's be consistent with other +scripts. + +Signed-off-by: Petr Vorel +--- + scripts/kconfig/nconf-cfg.sh | 0 + 1 file changed, 0 insertions(+), 0 deletions(-) + mode change 100644 => 100755 scripts/kconfig/nconf-cfg.sh + +diff --git a/scripts/kconfig/nconf-cfg.sh b/scripts/kconfig/nconf-cfg.sh +old mode 100644 +new mode 100755 +-- +2.20.1 diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index 8255c657e..4df1e1577 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -602,6 +602,11 @@ def test_git_new_empty_file(self): self.assertTrue(diff is not None) self.assertTrue(message is not None) + def test_git_mode_change(self): + diff, message = self._find_content('0022-git-mode-change.mbox') + self.assertTrue(diff is not None) + self.assertTrue(message is not None) + def test_cvs_format(self): diff, message = self._find_content('0007-cvs-format-diff.mbox') self.assertTrue(diff.startswith('Index')) From 9f24264144df514d7ea6b9ee082a4530a083039b Mon Sep 17 00:00:00 2001 From: Petr Vorel Date: Tue, 4 Jun 2019 17:56:39 +0200 Subject: [PATCH 42/73] parser: Remove duplicity commit fc1d750 copied lines added in 753e457. Make sense to define it on single place (DRY). Signed-off-by: Petr Vorel Reviewed-by: Stephen Finucane (cherry picked from commit ecbe3fc5f1c91cfa19dab77d527897e2122f5096) --- patchwork/parser.py | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/patchwork/parser.py b/patchwork/parser.py index fb4b3a0ab..01bdd9f9f 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -52,6 +52,15 @@ SERIES_DELAY_INTERVAL = 10 +# @see https://git-scm.com/docs/git-diff#_generating_patches_with_p +EXTENDED_HEADER_LINES = ( + 'old mode ', 'new mode ', + 'deleted file mode ', 'new file mode ', + 'copy from ', 'copy to ', + 'rename from ', 'rename to ', + 'similarity index ', 'dissimilarity index ', + 'new file mode ', 'index ') + logger = logging.getLogger(__name__) @@ -789,17 +798,7 @@ def parse_patch(content): buf += line if line.startswith('--- '): state = 2 - - # extended header lines - # @see https://git-scm.com/docs/git-diff#_generating_patches_with_p - if line.startswith(('old mode ', 'new mode ', - 'deleted file mode ', - 'new file mode ', - 'copy from ', 'copy to ', - 'rename from ', 'rename to ', - 'similarity index ', - 'dissimilarity index ', - 'new file mode ', 'index ')): + if line.startswith(EXTENDED_HEADER_LINES): state = 6 elif state == 2: if line.startswith('+++ '): @@ -860,16 +859,7 @@ def fn(x): else: state = 5 elif state == 6: - # extended header lines - # @see https://git-scm.com/docs/git-diff#_generating_patches_with_p - if line.startswith(('old mode ', 'new mode ', - 'deleted file mode ', - 'new file mode ', - 'copy from ', 'copy to ', - 'rename from ', 'rename to ', - 'similarity index ', - 'dissimilarity index ', - 'new file mode ', 'index ')): + if line.startswith(EXTENDED_HEADER_LINES): patchbuf += buf + line buf = '' elif line.startswith('--- '): From b0b7e5dab2d1a8016cb90836134f42174ac5df31 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Tue, 4 Jun 2019 18:31:39 -0300 Subject: [PATCH 43/73] filters: re-add the possibility of filtering undelegated patches The filters.py redesign that happened for patchwork 1.1 removed a functionality that we use a lot: to filter patches that weren't delegated to anyone. Also, it is a way harder to find someone to delegate with a free text input. Use, instead a combo-box just like before. Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Stephen Finucane Fixes: f439f541 ("Add delegate filter autocomplete support") Closes: #60 [stephenfin: Rework release note and fix some style issues] (cherry picked from commit e99490cf28e8104f42746d71f11618f4e6f427e1) --- patchwork/filters.py | 36 +++++++++++++++++- patchwork/templates/patchwork/filters.html | 38 ------------------- .../notes/issue-60-9d4fc111242f7db6.yaml | 8 ++++ 3 files changed, 42 insertions(+), 40 deletions(-) create mode 100644 releasenotes/notes/issue-60-9d4fc111242f7db6.yaml diff --git a/patchwork/filters.py b/patchwork/filters.py index f6e483947..2ddeb18d3 100644 --- a/patchwork/filters.py +++ b/patchwork/filters.py @@ -375,6 +375,7 @@ def url_without_me(self): class DelegateFilter(Filter): param = 'delegate' + no_delegate_str = 'Nobody' AnyDelegate = 1 def __init__(self, filters): @@ -391,6 +392,11 @@ def _set_key(self, key): if not key: return + if key == self.no_delegate_str: + self.delegate_match = key + self.applied = True + return + try: self.delegate = User.objects.get(id=int(key)) except (ValueError, User.DoesNotExist): @@ -410,6 +416,9 @@ def kwargs(self): if self.delegate: return {'delegate': self.delegate} + if self.delegate_match == self.no_delegate_str: + return {'delegate__username__isnull': True} + if self.delegate_match: return {'delegate__username__icontains': self.delegate_match} return {} @@ -422,8 +431,31 @@ def condition(self): return '' def _form(self): - return mark_safe('') + delegates = User.objects.filter( + profile__maintainer_projects__isnull=False) + + out = '' + return mark_safe(out) def key(self): if self.delegate: diff --git a/patchwork/templates/patchwork/filters.html b/patchwork/templates/patchwork/filters.html index 9b0c4cb2e..a689b2eb5 100644 --- a/patchwork/templates/patchwork/filters.html +++ b/patchwork/templates/patchwork/filters.html @@ -76,44 +76,6 @@ }); } }); - - $('#delegate_input').selectize({ - plugins: ['enter_key_submit'], - maxItems: 1, - persist: false, - onInitialize: function() { - this.on('submit', function() { - if (!this.items.length) - this.$input.val(this.lastValue); - this.$input.closest('form').submit(); - }, this); - }, -{% if "delegate" in filters.applied_filters %} -{% with delegate_filter=filters.applied_filters.delegate %} - options: [ - { - value: "{{ delegate_filter.key }}", - text: "{{ delegate_filter.condition }}", - }, - ], - items: ["{{ delegate_filter.key }}"], -{% endwith %} -{% endif %} - load: function(query, callback) { - req = $.ajax({ - url: "{% url 'api-delegates' %}", - data: {q: query, l: 10}, - error: function() { - callback(); - }, - success: function(res) { - callback($.map(res, function (obj) { - return {value: obj.pk, text: obj.name}; - })); - } - }); - } - }); }); diff --git a/releasenotes/notes/issue-60-9d4fc111242f7db6.yaml b/releasenotes/notes/issue-60-9d4fc111242f7db6.yaml new file mode 100644 index 000000000..798865927 --- /dev/null +++ b/releasenotes/notes/issue-60-9d4fc111242f7db6.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + In the past, Patchwork used to support filtering patches that weren't + delegated to anyone. This feature was removed in v1.1.0, as part of a patch + designed to support delegation to anyone. However, that feature didn't scale + and was later removed. The ability to delegate to anyone is now itself + re-introduced. From 12b4a194a9a07f9ecc87bb9a8ed6f950ec4d3476 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 5 Jun 2019 11:42:45 +0100 Subject: [PATCH 44/73] Release 2.1.3 Signed-off-by: Stephen Finucane --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 36b1bfee8..0c2d2c973 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 3, 'alpha', 0) +VERSION = (2, 1, 3) __version__ = get_latest_version(VERSION) From d7f7f9ea8f5ec1f754b00b49850d9d980b5d12e1 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 5 Jun 2019 11:44:35 +0100 Subject: [PATCH 45/73] Post-release version bump Signed-off-by: Stephen Finucane --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 0c2d2c973..692f12b36 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 3) +VERSION = (2, 1, 4, 'alpha', 0) __version__ = get_latest_version(VERSION) From 19aad6096a13603c088461dcc7cf534289b8479a Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 19 Sep 2018 21:03:33 +0100 Subject: [PATCH 46/73] parser: Handle IntegrityError for cover letters, comments This was already done for patches but cover letters and comments were not handled correctly, resulting in errors while parsing archives. While we're here, we slightly modify how these exceptions are handle. Rather than simply ignoring them, as we were doing, we raise a custom exception. This allows us to specifically identify these types of exceptions, print a log and still skip them (which we want, as seen in commit d2eb1f6d2). While we're here, we change from separate create-save calls to a combined create-save call for both created CoverLetter and Comment objects. We were already doing this for patches. Signed-off-by: Stephen Finucane (cherry picked from commit 300ba3eb283ab1af3dc5924e5ed540159c3ee0bc) --- patchwork/management/commands/parsearchive.py | 15 ++++-- patchwork/management/commands/parsemail.py | 8 ++- patchwork/parser.py | 50 ++++++++++++------- 3 files changed, 48 insertions(+), 25 deletions(-) diff --git a/patchwork/management/commands/parsearchive.py b/patchwork/management/commands/parsearchive.py index 5468d35ee..bffb46479 100644 --- a/patchwork/management/commands/parsearchive.py +++ b/patchwork/management/commands/parsearchive.py @@ -26,6 +26,7 @@ from patchwork import models from patchwork.parser import parse_mail +from patchwork.parser import DuplicateMailError logger = logging.getLogger(__name__) @@ -48,6 +49,7 @@ def handle(self, *args, **options): models.CoverLetter: 0, models.Comment: 0, } + duplicates = 0 dropped = 0 errors = 0 @@ -90,10 +92,13 @@ def handle(self, *args, **options): results[type(obj)] += 1 else: dropped += 1 - except ValueError: - # TODO(stephenfin): Perhaps we should store the broken patch - # somewhere for future reference? + except DuplicateMailError as exc: + duplicates += 1 + logger.warning('Duplicate mail for message ID %s', exc.msgid) + except (ValueError, Exception) as exc: errors += 1 + logger.warning('Invalid mail: %s', exc.message, + extra={'mail': mail.as_string()}) if (i % 10) == 0: self.stdout.write('%06d/%06d\r' % (i, count), ending='') @@ -104,6 +109,7 @@ def handle(self, *args, **options): ' %(covers)4d cover letters\n' ' %(patches)4d patches\n' ' %(comments)4d comments\n' + ' %(duplicates)4d duplicates\n' ' %(dropped)4d dropped\n' ' %(errors)4d errors\n' 'Total: %(new)s new entries' % { @@ -111,8 +117,9 @@ def handle(self, *args, **options): 'covers': results[models.CoverLetter], 'patches': results[models.Patch], 'comments': results[models.Comment], + 'duplicates': duplicates, 'dropped': dropped, 'errors': errors, - 'new': count - dropped - errors, + 'new': count - duplicates - dropped - errors, }) mbox.close() diff --git a/patchwork/management/commands/parsemail.py b/patchwork/management/commands/parsemail.py index f62fb4f51..9098047d2 100644 --- a/patchwork/management/commands/parsemail.py +++ b/patchwork/management/commands/parsemail.py @@ -25,6 +25,7 @@ from django.utils import six from patchwork.parser import parse_mail +from patchwork.parser import DuplicateMailError logger = logging.getLogger(__name__) @@ -79,7 +80,10 @@ def handle(self, *args, **options): result = parse_mail(mail, options['list_id']) if result is None: logger.warning('Nothing added to database') - except Exception: - logger.exception('Error when parsing incoming email', + except DuplicateMailError as exc: + logger.warning('Duplicate mail for message ID %s', exc.msgid) + except (ValueError, Exception) as exc: + logger.exception('Error when parsing incoming email: %s', + exc.message, extra={'mail': mail.as_string()}) sys.exit(1) diff --git a/patchwork/parser.py b/patchwork/parser.py index 01bdd9f9f..1304e95ea 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -64,6 +64,12 @@ logger = logging.getLogger(__name__) +class DuplicateMailError(Exception): + + def __init__(self, msgid): + self.msgid = msgid + + def normalise_space(value): whitespace_re = re.compile(r'\s+') return whitespace_re.sub(' ', value).strip() @@ -1036,8 +1042,7 @@ def parse_mail(mail, list_id=None): state=find_state(mail)) logger.debug('Patch saved') except IntegrityError: - logger.error("Duplicate mail for message ID %s" % msgid) - return None + raise DuplicateMailError(msgid=msgid) # if we don't have a series marker, we will never have an existing # series to match against. @@ -1143,15 +1148,18 @@ def parse_mail(mail, list_id=None): logger.error("Multiple SeriesReferences for %s" " in project %s!" % (msgid, project.name)) - cover_letter = CoverLetter( - msgid=msgid, - project=project, - name=name[:255], - date=date, - headers=headers, - submitter=author, - content=message) - cover_letter.save() + try: + cover_letter = CoverLetter.objects.create( + msgid=msgid, + project=project, + name=name[:255], + date=date, + headers=headers, + submitter=author, + content=message) + except IntegrityError: + raise DuplicateMailError(msgid=msgid) + logger.debug('Cover letter saved') series.add_cover_letter(cover_letter) @@ -1167,14 +1175,18 @@ def parse_mail(mail, list_id=None): author = get_or_create_author(mail) - comment = Comment( - submission=submission, - msgid=msgid, - date=date, - headers=headers, - submitter=author, - content=message) - comment.save() + + try: + comment = Comment.objects.create( + submission=submission, + msgid=msgid, + date=date, + headers=headers, + submitter=author, + content=message) + except IntegrityError: + raise DuplicateMailError(msgid=msgid) + logger.debug('Comment saved') return comment From 92ccdeb85118c59d1c0a3481aee89a9e455bcca4 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 21 Sep 2018 17:59:48 +0100 Subject: [PATCH 47/73] settings: Don't configure logging for parsearchive A recent change added additional ERROR level logs to the 'parsearchive' tool. This highlighted an issue, whereby we had configured all modules in 'patchwork.management.command' to email administrators on ERROR logs. We clearly shouldn't be doing this for the 'parsearchive' command or for anything other than 'parsemail', so fix this. Along the way, we remove a now-unnecessary 'extra' argument to one of the logging calls in 'parsearchive' and resolve a pep8 issue. Signed-off-by: Stephen Finucane Fixes: 133091da ("parsearchive: Fix logging") Cc: Daniel Axtens (cherry picked from commit aa266a28c6a022ad6952fac8f36afff0c540dccf) --- patchwork/management/commands/parsearchive.py | 3 +-- patchwork/parser.py | 1 - patchwork/settings/base.py | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/patchwork/management/commands/parsearchive.py b/patchwork/management/commands/parsearchive.py index bffb46479..4e2bb46ac 100644 --- a/patchwork/management/commands/parsearchive.py +++ b/patchwork/management/commands/parsearchive.py @@ -97,8 +97,7 @@ def handle(self, *args, **options): logger.warning('Duplicate mail for message ID %s', exc.msgid) except (ValueError, Exception) as exc: errors += 1 - logger.warning('Invalid mail: %s', exc.message, - extra={'mail': mail.as_string()}) + logger.warning('Invalid mail: %s', exc.message) if (i % 10) == 0: self.stdout.write('%06d/%06d\r' % (i, count), ending='') diff --git a/patchwork/parser.py b/patchwork/parser.py index 1304e95ea..a4762dbb4 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -1175,7 +1175,6 @@ def parse_mail(mail, list_id=None): author = get_or_create_author(mail) - try: comment = Comment.objects.create( submission=submission, diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py index 45c66da79..23b9fc95b 100644 --- a/patchwork/settings/base.py +++ b/patchwork/settings/base.py @@ -193,7 +193,7 @@ 'level': 'DEBUG', 'propagate': False, }, - 'patchwork.management.commands': { + 'patchwork.management.commands.parsemail': { 'handlers': ['console', 'mail_admins'], 'level': 'INFO', 'propagate': True, From 3e66958169c7e7ac20bf0cc316e1c2803606aa8e Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 21 Oct 2018 11:51:25 +0100 Subject: [PATCH 48/73] templates: Fix pwclientrc sample INI files should use 'key = value' values, not 'key: value'. Correct this and fix some other whitespace issues. Signed-off-by: Stephen Finucane Closes: #277 (cherry picked from commit 5ee0c0fdb0fc85f6e03ed55b04c918fa2ee0659b) --- patchwork/templates/patchwork/pwclientrc | 6 +++--- releasenotes/notes/issue-277-5bfda7ad1f72f267.yaml | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/issue-277-5bfda7ad1f72f267.yaml diff --git a/patchwork/templates/patchwork/pwclientrc b/patchwork/templates/patchwork/pwclientrc index 96464c1be..7d466d890 100644 --- a/patchwork/templates/patchwork/pwclientrc +++ b/patchwork/templates/patchwork/pwclientrc @@ -8,8 +8,8 @@ # default={{ project.linkname }} [{{ project.linkname }}] -url= {{scheme}}://{{site.domain}}{% url 'xmlrpc' %} +url = {{ scheme }}://{{ site.domain }}{% url 'xmlrpc' %} {% if user.is_authenticated %} -username: {{ user.username }} -password: +username = {{ user.username }} +password = {% endif %} diff --git a/releasenotes/notes/issue-277-5bfda7ad1f72f267.yaml b/releasenotes/notes/issue-277-5bfda7ad1f72f267.yaml new file mode 100644 index 000000000..da9460d60 --- /dev/null +++ b/releasenotes/notes/issue-277-5bfda7ad1f72f267.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The ``pwclientrc`` samples generated by Patchwork were previously not valid + INI files. This issue is resolved. (`#277 + `__) From c400741bae2477d8329a24b3498d059c28ced0d6 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Fri, 5 Jul 2019 11:30:28 +1000 Subject: [PATCH 49/73] templatetags: Do not mark output of msgid tag as safe The msgid template tag exists to remove angle brackets from either side of the Message-ID header. It also marks its output as safe, meaning it does not get autoescaped by Django templating. Its output is not safe. A maliciously crafted email can include HTML tags inside the Message-ID header, and as long as the angle brackets are not at the start and end of the header, we will quite happily render them. Rather than using mark_safe(), use escape() to explicitly escape the Message-ID. Signed-off-by: Andrew Donnellan (backported from 133a6c90e9826376be0f12f2ae6c2d7b076bdba0) Signed-off-by: Daniel Axtens --- patchwork/templatetags/patch.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patchwork/templatetags/patch.py b/patchwork/templatetags/patch.py index 4350e092e..577c78375 100644 --- a/patchwork/templatetags/patch.py +++ b/patchwork/templatetags/patch.py @@ -21,6 +21,7 @@ from __future__ import absolute_import from django import template +from django.utils.html import escape from django.utils.safestring import mark_safe from django.template.defaultfilters import stringfilter @@ -65,4 +66,4 @@ def state_class(state): @register.filter @stringfilter def msgid(value): - return mark_safe(value.strip('<>')) + return escape(value.strip('<>')) From 95ccc2d4ded0a8f001ff93a45708df1519f6346b Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Fri, 5 Jul 2019 11:33:58 +1000 Subject: [PATCH 50/73] tests: Add test for unescaped values in patch detail page Add a test to check whether we are escaping values from the Patch model on the patch detail page. This test shouldn't be relied upon as proof that we've escaped everything correctly, but may help catch regressions. Signed-off-by: Andrew Donnellan (backported from df80e690bcc32d483875dcb36b488764c89ec9b6) Signed-off-by: Daniel Axtens --- patchwork/tests/test_detail.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/patchwork/tests/test_detail.py b/patchwork/tests/test_detail.py index 5d8534eae..fa3207cfa 100644 --- a/patchwork/tests/test_detail.py +++ b/patchwork/tests/test_detail.py @@ -66,6 +66,23 @@ def test_series_dropdown(self): response, reverse('series-mbox', kwargs={'series_id': series_.id})) + def test_escaping(self): + # Warning: this test doesn't guarantee anything - it only tests some + # fields + unescaped_string = 'blahTESTblah' + patch = create_patch() + patch.diff = unescaped_string + patch.commit_ref = unescaped_string + patch.pull_url = unescaped_string + patch.name = unescaped_string + patch.msgid = unescaped_string + patch.headers = unescaped_string + patch.content = unescaped_string + patch.save() + requested_url = reverse('patch-detail', kwargs={'patch_id': patch.id}) + response = self.client.get(requested_url) + self.assertNotIn('TEST'.encode('utf-8'), response.content) + class CommentRedirectTest(TestCase): From 6774a89048c04ac8cdebf7ba694ea913ec949ce9 Mon Sep 17 00:00:00 2001 From: Andrew Donnellan Date: Fri, 5 Jul 2019 13:27:41 +1000 Subject: [PATCH 51/73] filters: Escape State names when generating selector HTML States with names containing special characters are not correctly escaped when generating the select list. Use escape() to fix this. Signed-off-by: Andrew Donnellan (cherry picked from commit b3fa0c402e060622a5ed539a465d2fa98b1d2e13) Signed-off-by: Daniel Axtens --- patchwork/filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/filters.py b/patchwork/filters.py index 2ddeb18d3..1358cc0be 100644 --- a/patchwork/filters.py +++ b/patchwork/filters.py @@ -254,7 +254,7 @@ def _form(self): selected = ' selected="true"' out += '' % ( - state.id, selected, state.name) + state.id, selected, escape(state.name)) out += '' return mark_safe(out) From 2d9ce1d90bf56d0adedbc744aa38786ff86cc211 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Fri, 5 Jul 2019 15:21:26 +1000 Subject: [PATCH 52/73] docs: Add a release note for CVE-2019-13122 Signed-off-by: Daniel Axtens --- .../notes/CVE-2019-13122-e9c63aa346ed15c2.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 releasenotes/notes/CVE-2019-13122-e9c63aa346ed15c2.yaml diff --git a/releasenotes/notes/CVE-2019-13122-e9c63aa346ed15c2.yaml b/releasenotes/notes/CVE-2019-13122-e9c63aa346ed15c2.yaml new file mode 100644 index 000000000..48afac050 --- /dev/null +++ b/releasenotes/notes/CVE-2019-13122-e9c63aa346ed15c2.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + CVE-2019-13122 has been fixed. Andrew Donnellan discovered an XSS + via the message-id field. A malicious user could send a patch with + a message ID that included a script tag. Because of the quirks of + the email RFCs, such a message ID can survive being sent through + many mail systems, including Gmail, and be parsed and stored by + Patchwork. When a user viewed a patch detail page for the patch + with this message id, the script would be run. This is fixed by + properly escaping the field before it is rendered. \ No newline at end of file From 1ad5e6f28e28432677955b9437d5c8f51dc3bf6c Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Fri, 5 Jul 2019 15:27:32 +1000 Subject: [PATCH 53/73] Release 2.1.4 Signed-off-by: Daniel Axtens --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 692f12b36..06d3db460 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 4, 'alpha', 0) +VERSION = (2, 1, 4) __version__ = get_latest_version(VERSION) From b4592c96704f656475b93afbb801991b71ba411b Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Fri, 5 Jul 2019 15:27:50 +1000 Subject: [PATCH 54/73] Post-release version bump Signed-off-by: Daniel Axtens --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 06d3db460..d7f3b0bfb 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 4) +VERSION = (2, 1, 5, 'alpha', 0) __version__ = get_latest_version(VERSION) From 6a716c4267dfbb0122fb20071b70ff560bef3b3a Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Wed, 21 Aug 2019 15:15:44 +1000 Subject: [PATCH 55/73] mbox: do not copy Content-Type into exported mbox Daniel reports a patch + comment combination that breaks in git am. The patch reports a Content-Type of US-ASCII, while the comment adds a Ack with UTF-8 characters. The exported mbox contains both the original Content-Type, and a UTF-8 Content-Type that we set. However, because the US-ASCII one occurs later, git am honours it instead of ours, and chokes on the UTF-8 characters. Strip out any subsequent Content-Type:s. We normalise things to UTF-8 and should not allow it to be overridden. Add a test for this, based on the original report. Reported-by: Daniel Borkmann Signed-off-by: Daniel Axtens --- .../series/bugs-multiple-content-types.mbox | 172 ++++++++++++++++++ patchwork/tests/test_series.py | 11 ++ patchwork/views/utils.py | 4 +- 3 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 patchwork/tests/series/bugs-multiple-content-types.mbox diff --git a/patchwork/tests/series/bugs-multiple-content-types.mbox b/patchwork/tests/series/bugs-multiple-content-types.mbox new file mode 100644 index 000000000..f7006b447 --- /dev/null +++ b/patchwork/tests/series/bugs-multiple-content-types.mbox @@ -0,0 +1,172 @@ +From mboxrd@z Thu Jan 1 00:00:00 1970 +Return-Path: +X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on + aws-us-west-2-korg-lkml-1.web.codeaurora.org +X-Spam-Level: +X-Spam-Status: No, score=-9.7 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, + INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, + URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no + version=3.4.0 +Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) + by smtp.lore.kernel.org (Postfix) with ESMTP id A702DC3A5A2 + for ; Tue, 20 Aug 2019 01:33:12 +0000 (UTC) +Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) + by mail.kernel.org (Postfix) with ESMTP id 8717B22DA7 + for ; Tue, 20 Aug 2019 01:33:12 +0000 (UTC) +Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand + id S1728887AbfHTBdL (ORCPT ); + Mon, 19 Aug 2019 21:33:11 -0400 +Received: from szxga05-in.huawei.com ([45.249.212.191]:4731 "EHLO huawei.com" + rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP + id S1728627AbfHTBdL (ORCPT ); + Mon, 19 Aug 2019 21:33:11 -0400 +Received: from DGGEMS411-HUB.china.huawei.com (unknown [172.30.72.59]) + by Forcepoint Email with ESMTP id EF227A58CA1FC4ADCFA3; + Tue, 20 Aug 2019 09:33:03 +0800 (CST) +Received: from localhost.localdomain.localdomain (10.175.113.25) by + DGGEMS411-HUB.china.huawei.com (10.3.19.211) with Microsoft SMTP Server id + 14.3.439.0; Tue, 20 Aug 2019 09:32:56 +0800 +From: YueHaibing +To: , , + , , + , , , + , +CC: YueHaibing , , + , +Subject: [PATCH -next] bpf: Use PTR_ERR_OR_ZERO in xsk_map_inc() +Date: Tue, 20 Aug 2019 01:36:52 +0000 +Message-ID: <20190820013652.147041-1-yuehaibing@huawei.com> +X-Mailer: git-send-email 2.20.1 +MIME-Version: 1.0 +Content-Type: text/plain; charset=US-ASCII +Content-Transfer-Encoding: 7BIT +X-Originating-IP: [10.175.113.25] +X-CFilter-Loop: Reflected +Sender: netdev-owner@vger.kernel.org +Precedence: bulk +List-ID: +X-Mailing-List: netdev@vger.kernel.org +Archived-At: +List-Archive: +List-Post: + +Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR + +Signed-off-by: YueHaibing +--- + kernel/bpf/xskmap.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c +index 4cc28e226398..942c662e2eed 100644 +--- a/kernel/bpf/xskmap.c ++++ b/kernel/bpf/xskmap.c +@@ -21,7 +21,7 @@ int xsk_map_inc(struct xsk_map *map) + struct bpf_map *m = &map->map; + + m = bpf_map_inc(m, false); +- return IS_ERR(m) ? PTR_ERR(m) : 0; ++ return PTR_ERR_OR_ZERO(m); + } + + void xsk_map_put(struct xsk_map *map) + + +From mboxrd@z Thu Jan 1 00:00:00 1970 +Return-Path: +X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on + aws-us-west-2-korg-lkml-1.web.codeaurora.org +X-Spam-Level: +X-Spam-Status: No, score=-8.2 required=3.0 tests=FROM_EXCESS_BASE64, + HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, + SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable + autolearn_force=no version=3.4.0 +Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) + by smtp.lore.kernel.org (Postfix) with ESMTP id 02AB1C3A59E + for ; Tue, 20 Aug 2019 07:28:35 +0000 (UTC) +Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) + by mail.kernel.org (Postfix) with ESMTP id CC7942082F + for ; Tue, 20 Aug 2019 07:28:34 +0000 (UTC) +Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand + id S1729348AbfHTH2e (ORCPT ); + Tue, 20 Aug 2019 03:28:34 -0400 +Received: from mga18.intel.com ([134.134.136.126]:58020 "EHLO mga18.intel.com" + rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP + id S1729047AbfHTH2e (ORCPT ); + Tue, 20 Aug 2019 03:28:34 -0400 +X-Amp-Result: SKIPPED(no attachment in message) +X-Amp-File-Uploaded: False +Received: from orsmga007.jf.intel.com ([10.7.209.58]) + by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Aug 2019 00:28:33 -0700 +X-ExtLoop1: 1 +X-IronPort-AV: E=Sophos;i="5.64,408,1559545200"; + d="scan'208";a="169001452" +Received: from arappl-mobl2.ger.corp.intel.com (HELO btopel-mobl.ger.intel.com) ([10.252.53.140]) + by orsmga007.jf.intel.com with ESMTP; 20 Aug 2019 00:28:27 -0700 +Subject: Re: [PATCH -next] bpf: Use PTR_ERR_OR_ZERO in xsk_map_inc() +To: YueHaibing , magnus.karlsson@intel.com, + jonathan.lemon@gmail.com, ast@kernel.org, daniel@iogearbox.net, + kafai@fb.com, songliubraving@fb.com, yhs@fb.com, + john.fastabend@gmail.com +Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, + kernel-janitors@vger.kernel.org +References: <20190820013652.147041-1-yuehaibing@huawei.com> +From: =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= +Message-ID: <93fafdab-8fb3-0f2b-8f36-0cf297db3cd9@intel.com> +Date: Tue, 20 Aug 2019 09:28:26 +0200 +User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 + Thunderbird/60.8.0 +MIME-Version: 1.0 +In-Reply-To: <20190820013652.147041-1-yuehaibing@huawei.com> +Content-Type: text/plain; charset=utf-8; format=flowed +Content-Language: en-US +Content-Transfer-Encoding: 8bit +Sender: netdev-owner@vger.kernel.org +Precedence: bulk +List-ID: +X-Mailing-List: netdev@vger.kernel.org +Archived-At: +List-Archive: +List-Post: + +On 2019-08-20 03:36, YueHaibing wrote: +> Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR +> +> Signed-off-by: YueHaibing +> --- +> kernel/bpf/xskmap.c | 2 +- +> 1 file changed, 1 insertion(+), 1 deletion(-) +> +> diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c +> index 4cc28e226398..942c662e2eed 100644 +> --- a/kernel/bpf/xskmap.c +> +++ b/kernel/bpf/xskmap.c +> @@ -21,7 +21,7 @@ int xsk_map_inc(struct xsk_map *map) +> struct bpf_map *m = &map->map; +> +> m = bpf_map_inc(m, false); +> - return IS_ERR(m) ? PTR_ERR(m) : 0; +> + return PTR_ERR_OR_ZERO(m); +> } +> +> void xsk_map_put(struct xsk_map *map) +> + +Acked-by: Björn Töpel + +Thanks for the patch! + +For future patches: Prefix AF_XDP socket work with "xsk:" and use "PATCH +bpf-next" to let the developers know what tree you're aiming for. + + + +Cheers! +Björn + + +> +> + + + diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py index 9b5c01291..515a5bd14 100644 --- a/patchwork/tests/test_series.py +++ b/patchwork/tests/test_series.py @@ -25,6 +25,7 @@ from patchwork import models from patchwork import parser from patchwork.tests import utils +from patchwork.views.utils import patch_to_mbox TEST_SERIES_DIR = os.path.join(os.path.dirname(__file__), 'series') @@ -273,6 +274,16 @@ def test_no_references_no_cover(self): self.assertSerialized(patches, [2]) self.assertSerialized(covers, [1]) + def test_multiple_content_types(self): + """Test what happens when a patch and comment have different + Content-Type headers.""" + + _, patches, _ = self._parse_mbox( + 'bugs-multiple-content-types.mbox', [0, 1, 1]) + + patch = patches[0] + self.assertEqual(patch_to_mbox(patch).count('Content-Type:'), 1) + class RevisedSeriesTest(_BaseTestCase): """Tests for a series plus a single revision. diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py index fb0195ceb..7376bae8b 100644 --- a/patchwork/views/utils.py +++ b/patchwork/views/utils.py @@ -99,8 +99,8 @@ def _submission_to_mbox(submission): orig_headers = HeaderParser().parsestr(str(submission.headers)) for key, val in orig_headers.items(): - # we set this ourselves - if key == 'Content-Transfer-Encoding': + # we set these ourselves + if key in ['Content-Type', 'Content-Transfer-Encoding']: continue # we don't save GPG signatures described in RFC1847 [1] so this # Content-Type value is invalid From 47744ffaaf22e095e6d9c3321833c3dcfccce2b1 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Wed, 18 Sep 2019 16:17:28 +1000 Subject: [PATCH 56/73] parsearchive, mail: use repr() to get a human readable exception Currently if we have particular types of error in mail parsing in parsearchive or parsemail, we print exc.message, which doesn't always work: Traceback (most recent call last): File ".../patchwork/management/commands/parsearchive.py", line 90, in handle obj = parse_mail(msg, options['list_id']) File ".../patchwork/parser.py", line 961, in parse_mail raise ValueError("Missing 'Message-Id' header") ValueError: Missing 'Message-Id' header During handling of the above exception, another exception occurred: Traceback (most recent call last): File "manage.py", line 11, in execute_from_command_line(sys.argv) File ".../django/core/management/__init__.py", line 381, in execute_from_command_line utility.execute() File ".../django/core/management/__init__.py", line 375, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File ".../django/core/management/base.py", line 323, in run_from_argv self.execute(*args, **cmd_options) File ".../django/core/management/base.py", line 364, in execute output = self.handle(*args, **options) File ".../patchwork/management/commands/parsearchive.py", line 100, in handle logger.warning('Invalid mail: %s', exc.message) AttributeError: 'ValueError' object has no attribute 'message' repr(exc) will work. Use it. Signed-off-by: Daniel Axtens Reviewed-by: Stephen Finucane (cherry picked from commit 319b6f9d750c220d47c1044246d4e97fd500c6d4) --- patchwork/management/commands/parsearchive.py | 2 +- patchwork/management/commands/parsemail.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/patchwork/management/commands/parsearchive.py b/patchwork/management/commands/parsearchive.py index 4e2bb46ac..51951cb46 100644 --- a/patchwork/management/commands/parsearchive.py +++ b/patchwork/management/commands/parsearchive.py @@ -97,7 +97,7 @@ def handle(self, *args, **options): logger.warning('Duplicate mail for message ID %s', exc.msgid) except (ValueError, Exception) as exc: errors += 1 - logger.warning('Invalid mail: %s', exc.message) + logger.warning('Invalid mail: %s', repr(exc)) if (i % 10) == 0: self.stdout.write('%06d/%06d\r' % (i, count), ending='') diff --git a/patchwork/management/commands/parsemail.py b/patchwork/management/commands/parsemail.py index 9098047d2..f31f533e3 100644 --- a/patchwork/management/commands/parsemail.py +++ b/patchwork/management/commands/parsemail.py @@ -84,6 +84,6 @@ def handle(self, *args, **options): logger.warning('Duplicate mail for message ID %s', exc.msgid) except (ValueError, Exception) as exc: logger.exception('Error when parsing incoming email: %s', - exc.message, + repr(exc), extra={'mail': mail.as_string()}) sys.exit(1) From a938f952023a7381816107392fcc10100f5b054c Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 28 Oct 2018 17:14:45 +0000 Subject: [PATCH 57/73] tests: Split up tests in 'test_patch' This is essentially commit ad54f526869804abcdcea8744d90ae85bd0ce34c ("tests: Add 'store_samples' decorator to 'test_patch'") from master without the 'store_samples' decorator. It's included to avoid merge conflicts with future patches. Signed-off-by: Stephen Finucane Stable-Only --- patchwork/tests/api/test_patch.py | 137 ++++++++++++++++++++---------- 1 file changed, 94 insertions(+), 43 deletions(-) diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 6a8df512e..03fd92654 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -72,73 +72,96 @@ def assertSerialized(self, patch_obj, patch_json): self.assertEqual(patch_obj.project.id, patch_json['project']['id']) - def test_list(self): - """Validate we can list a patch.""" + def test_list_empty(self): + """List patches when none are present.""" resp = self.client.get(self.api_url()) self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(0, len(resp.data)) + def _create_patch(self): person_obj = create_person(email='test@example.com') project_obj = create_project(linkname='myproject') state_obj = create_state(name='Under Review') patch_obj = create_patch(state=state_obj, project=project_obj, submitter=person_obj) - # anonymous user + return patch_obj + + def test_list_anonymous(self): + """List patches as anonymous user.""" + patch = self._create_patch() + resp = self.client.get(self.api_url()) self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(1, len(resp.data)) patch_rsp = resp.data[0] - self.assertSerialized(patch_obj, patch_rsp) + self.assertSerialized(patch, patch_rsp) self.assertNotIn('headers', patch_rsp) self.assertNotIn('content', patch_rsp) self.assertNotIn('diff', patch_rsp) - # authenticated user + def test_list_authenticated(self): + """List patches as an authenticated user.""" + patch = self._create_patch() user = create_user() + self.client.force_authenticate(user=user) resp = self.client.get(self.api_url()) self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(1, len(resp.data)) patch_rsp = resp.data[0] - self.assertSerialized(patch_obj, patch_rsp) + self.assertSerialized(patch, patch_rsp) - # test filtering by state - resp = self.client.get(self.api_url(), {'state': 'under-review'}) - self.assertEqual([patch_obj.id], [x['id'] for x in resp.data]) - resp = self.client.get(self.api_url(), {'state': 'missing-state'}) - self.assertEqual(0, len(resp.data)) + def test_list_filter_state(self): + """Filter patches by state.""" + self._create_patch() + user = create_user() + + state_obj_b = create_state(name='New') + create_patch(state=state_obj_b) + state_obj_c = create_state(name='RFC') + create_patch(state=state_obj_c) + + self.client.force_authenticate(user=user) + resp = self.client.get(self.api_url(), [('state', 'under-review'), + ('state', 'new')]) + self.assertEqual(2, len(resp.data)) + + def test_list_filter_project(self): + """Filter patches by project.""" + patch = self._create_patch() + user = create_user() + + self.client.force_authenticate(user=user) - # test filtering by project resp = self.client.get(self.api_url(), {'project': 'myproject'}) - self.assertEqual([patch_obj.id], [x['id'] for x in resp.data]) + self.assertEqual([patch.id], [x['id'] for x in resp.data]) + resp = self.client.get(self.api_url(), {'project': 'invalidproject'}) self.assertEqual(0, len(resp.data)) + def test_list_filter_submitter(self): + """Filter patches by submitter.""" + patch = self._create_patch() + submitter = patch.submitter + user = create_user() + + self.client.force_authenticate(user=user) + # test filtering by submitter, both ID and email - resp = self.client.get(self.api_url(), {'submitter': person_obj.id}) - self.assertEqual([patch_obj.id], [x['id'] for x in resp.data]) + resp = self.client.get(self.api_url(), {'submitter': submitter.id}) + self.assertEqual([patch.id], [x['id'] for x in resp.data]) + resp = self.client.get(self.api_url(), { 'submitter': 'test@example.com'}) - self.assertEqual([patch_obj.id], [x['id'] for x in resp.data]) + self.assertEqual([patch.id], [x['id'] for x in resp.data]) + resp = self.client.get(self.api_url(), { 'submitter': 'test@example.org'}) self.assertEqual(0, len(resp.data)) - state_obj_b = create_state(name='New') - create_patch(state=state_obj_b) - state_obj_c = create_state(name='RFC') - create_patch(state=state_obj_c) - - resp = self.client.get(self.api_url()) - self.assertEqual(3, len(resp.data)) - resp = self.client.get(self.api_url(), [('state', 'under-review')]) - self.assertEqual(1, len(resp.data)) - resp = self.client.get(self.api_url(), [('state', 'under-review'), - ('state', 'new')]) - self.assertEqual(2, len(resp.data)) - def test_list_version_1_0(self): + """List patches using API v1.0.""" create_patch() resp = self.client.get(self.api_url(version='1.0')) @@ -148,7 +171,7 @@ def test_list_version_1_0(self): self.assertNotIn('web_url', resp.data[0]) def test_detail(self): - """Validate we can get a specific patch.""" + """Show a specific patch.""" patch = create_patch( content='Reviewed-by: Test User \n', headers='Received: from somewhere\nReceived: from another place' @@ -199,27 +222,43 @@ def test_create(self): resp = self.client.post(self.api_url(), patch) self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - def test_update(self): - """Ensure updates can be performed by maintainers.""" - project = create_project() - patch = create_patch(project=project) + def test_update_anonymous(self): + """Update patch as anonymous user. + + Ensure updates can be performed by maintainers. + """ + patch = create_patch() state = create_state() - # anonymous user resp = self.client.patch(self.api_url(patch.id), {'state': state.name}) self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) - # authenticated user + def test_update_non_maintainer(self): + """Update patch as non-maintainer. + + Ensure updates can be performed by maintainers. + """ + patch = create_patch() + state = create_state() user = create_user() + self.client.force_authenticate(user=user) resp = self.client.patch(self.api_url(patch.id), {'state': state.name}) self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) - # maintainer + def test_update_maintainer(self): + """Update patch as maintainer. + + Ensure updates can be performed by maintainers. + """ + project = create_project() + patch = create_patch(project=project) + state = create_state() user = create_maintainer(project) + self.client.force_authenticate(user=user) - resp = self.client.patch(self.api_url(patch.id), { - 'state': state.name, 'delegate': user.id}) + resp = self.client.patch(self.api_url(patch.id), + {'state': state.name, 'delegate': user.id}) self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) self.assertEqual(Patch.objects.get(id=patch.id).state, state) self.assertEqual(Patch.objects.get(id=patch.id).delegate, user) @@ -231,22 +270,34 @@ def test_update(self): self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) self.assertIsNone(Patch.objects.get(id=patch.id).delegate) - def test_update_invalid(self): - """Ensure we handle invalid Patch updates.""" + def test_update_invalid_state(self): + """Update patch with invalid fields. + + Ensure we handle invalid Patch updates. + """ project = create_project() state = create_state() patch = create_patch(project=project, state=state) user = create_maintainer(project) - # invalid state self.client.force_authenticate(user=user) resp = self.client.patch(self.api_url(patch.id), {'state': 'foobar'}) self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) self.assertContains(resp, 'Expected one of: %s.' % state.name, status_code=status.HTTP_400_BAD_REQUEST) - # invalid delegate + def test_update_invalid_delegate(self): + """Update patch with invalid fields. + + Ensure we handle invalid Patch updates. + """ + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user_a = create_maintainer(project) user_b = create_user() + + self.client.force_authenticate(user=user_a) resp = self.client.patch(self.api_url(patch.id), {'delegate': user_b.id}) self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) From 9d6c86ef720521605d7e8d9938c2795ad7b9e110 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sat, 21 Sep 2019 19:06:14 +0100 Subject: [PATCH 58/73] Fix issue with delegation of patch via REST API There have been reports of people being unable to delegate patches to themselves, despite being a maintainer or the project to which the patch is associated. The issue is a result of how we do a check for whether the user is a maintainer of the patch's project [1]. This check is checking if a given 'User.id' is in the list of items referenced by 'Project.maintainer_project'. However, 'Project.maintainer_project' is a backref to 'UserProfile.maintainer_projects'. This means we're comparing 'User.id' and 'UserProfile.id'. Boo. This wasn't seen in testing since we've had a post-save callback [2] for some time that ensures we always create a 'UserProfile' object whenever we create a 'User' object. This also means we won't have an issue on deployments initially deployed after that post-save callback was added, a 'User' with id=N will always have a corresponding 'UserProfile' with id=N. However, that's not true for older deployments such as the ozlabs.org one. [1] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/api/patch.py#L108-L111 [2] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/models.py#L204-L210 Signed-off-by: Stephen Finucane Closes: #313 Reported-by: Bjorn Helgaas (cherry picked from commit ab35df8c33a178b3b2349c1e4727393b94f5e916) --- patchwork/api/patch.py | 4 ++-- patchwork/tests/api/test_patch.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 7b8e12e39..f772c6543 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -119,8 +119,8 @@ def validate_delegate(self, value): if not value: return value - if not self.instance.project.maintainer_project.filter( - id=value.id).exists(): + if not value.profile.maintainer_projects.only('id').filter( + id=self.instance.project.id).exists(): raise ValidationError("User '%s' is not a maintainer for project " "'%s'" % (value, self.instance.project)) return value diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 03fd92654..f183ca9a1 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -286,6 +286,30 @@ def test_update_invalid_state(self): self.assertContains(resp, 'Expected one of: %s.' % state.name, status_code=status.HTTP_400_BAD_REQUEST) + def test_update_legacy_delegate(self): + """Regression test for bug #313.""" + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user_a = create_maintainer(project) + + # create a user (User), then delete the associated UserProfile and save + # the user to ensure a new profile is generated + user_b = create_user() + self.assertEqual(user_b.id, user_b.profile.id) + user_b.profile.delete() + user_b.save() + user_b.profile.maintainer_projects.add(project) + user_b.profile.save() + self.assertNotEqual(user_b.id, user_b.profile.id) + + self.client.force_authenticate(user=user_a) + resp = self.client.patch(self.api_url(patch.id), + {'delegate': user_b.id}) + self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) + self.assertEqual(Patch.objects.get(id=patch.id).state, state) + self.assertEqual(Patch.objects.get(id=patch.id).delegate, user_b) + def test_update_invalid_delegate(self): """Update patch with invalid fields. From 8a8eb858a65428778a6df056031b2fd943e5f815 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 24 Sep 2019 18:03:44 +0100 Subject: [PATCH 59/73] Release 2.1.5 Signed-off-by: Stephen Finucane --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index d7f3b0bfb..91e1771d9 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 5, 'alpha', 0) +VERSION = (2, 1, 5) __version__ = get_latest_version(VERSION) From fcac7790c5e32858c0998f9013bbf5c3795efec4 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 24 Sep 2019 18:05:25 +0100 Subject: [PATCH 60/73] Post-release version bump Signed-off-by: Stephen Finucane --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 91e1771d9..2b82e1aac 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 5) +VERSION = (2, 1, 6, 'alpha', 0) __version__ = get_latest_version(VERSION) From ea6a7bb00ec76cbb1edce53f9bba33cc87db52a3 Mon Sep 17 00:00:00 2001 From: Ali Alnubani Date: Tue, 29 Oct 2019 07:34:46 +0000 Subject: [PATCH 61/73] sql: Fix table lists [ backported from master commit 1a36ed51a8e1 ] The patch adds missing commas in the table lists where missing, and removes where unnecessary. This fixes errors such as the following when feeding the script to psql: psql:lib/sql/grant-all.postgres.sql:37: ERROR: syntax error at or near "patchwork_emailconfirmation" LINE 19: patchwork_emailconfirmation, ... Signed-off-by: Ali Alnubani Reviewed-by: Stephen Finucane Fixes: ca0e79d4db34 ("sql: Sort 'grant-all' scripts alphabetically") Signed-off-by: Daniel Axtens --- lib/sql/grant-all.postgres.sql | 24 +++++++++---------- .../sql-fix-table-lists-77667621052b2f72.yaml | 4 ++++ 2 files changed, 16 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/sql-fix-table-lists-77667621052b2f72.yaml diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index 27f55c96d..7fb3ac833 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -18,7 +18,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_check, patchwork_comment, patchwork_coverletter, - patchwork_delegationrule + patchwork_delegationrule, patchwork_emailconfirmation, patchwork_emailoptout, patchwork_patch, @@ -33,7 +33,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_submission, patchwork_tag, patchwork_userprofile, - patchwork_userprofile_maintainer_projects, + patchwork_userprofile_maintainer_projects TO "www-data"; GRANT SELECT, UPDATE ON auth_group_id_seq, @@ -49,7 +49,7 @@ GRANT SELECT, UPDATE ON patchwork_bundlepatch_id_seq, patchwork_check_id_seq, patchwork_comment_id_seq, - patchwork_delegationrule_id_seq + patchwork_delegationrule_id_seq, patchwork_emailconfirmation_id_seq, patchwork_patch_id_seq, patchwork_patchtag_id_seq, @@ -61,7 +61,7 @@ GRANT SELECT, UPDATE ON patchwork_state_id_seq, patchwork_tag_id_seq, patchwork_userprofile_id_seq, - patchwork_userprofile_maintainer_projects_id_seq, + patchwork_userprofile_maintainer_projects_id_seq TO "www-data"; -- allow the mail user (in this case, 'nobody') to add submissions (patches, @@ -69,32 +69,32 @@ TO "www-data"; GRANT INSERT, SELECT ON patchwork_comment, patchwork_coverletter, - patchwork_event + patchwork_event, patchwork_seriespatch, patchwork_seriesreference, - patchwork_submission, + patchwork_submission TO "nobody"; GRANT INSERT, SELECT, UPDATE, DELETE ON patchwork_patch, patchwork_patchtag, - patchwork_person - patchwork_series, + patchwork_person, + patchwork_series TO "nobody"; GRANT SELECT ON - patchwork_delegationrule + patchwork_delegationrule, patchwork_project, patchwork_state, - patchwork_tag, + patchwork_tag TO "nobody"; GRANT UPDATE, SELECT ON patchwork_comment_id_seq, - patchwork_event_id_seq + patchwork_event_id_seq, patchwork_patch_id_seq, patchwork_patchtag_id_seq, patchwork_person_id_seq, patchwork_series_id_seq, patchwork_seriespatch_id_seq, - patchwork_seriesreference_id_seq, + patchwork_seriesreference_id_seq TO "nobody"; COMMIT; diff --git a/releasenotes/notes/sql-fix-table-lists-77667621052b2f72.yaml b/releasenotes/notes/sql-fix-table-lists-77667621052b2f72.yaml new file mode 100644 index 000000000..8eaa9f48b --- /dev/null +++ b/releasenotes/notes/sql-fix-table-lists-77667621052b2f72.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + An sql error was fixed in `lib/sql/grant-all.postgres.sql`. From da24345592cd34729e62d3c7a377e5bdbc0def5f Mon Sep 17 00:00:00 2001 From: Andrew Donnellan Date: Mon, 21 Oct 2019 18:37:31 +1100 Subject: [PATCH 62/73] templates: Fix mismatched close tags There's a rather than in the bundle list. Fix it. Signed-off-by: Andrew Donnellan Signed-off-by: Daniel Axtens (cherry picked from commit 005ba0644dab1f386adaf4e603422091bb31cdca) --- patchwork/templates/patchwork/bundles.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/templates/patchwork/bundles.html b/patchwork/templates/patchwork/bundles.html index 749aaedcf..1bb3b0da7 100644 --- a/patchwork/templates/patchwork/bundles.html +++ b/patchwork/templates/patchwork/bundles.html @@ -14,7 +14,7 @@

Bundles

Bundle Project Public - Patches + Patches Download Delete From 37179103a0ddff805fa1d70aa769b76a2820aa4d Mon Sep 17 00:00:00 2001 From: Ali Alnubani Date: Mon, 21 Oct 2019 15:06:25 +0000 Subject: [PATCH 63/73] docs: Fix link to deployment guide The old format redirects to a nonexistent page when there are multiple versions of the docs. Signed-off-by: Ali Alnubani Signed-off-by: Daniel Axtens (cherry picked from commit 01ae4d15c628038534792f9758152e87820d5e57) --- docs/development/installation.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/development/installation.rst b/docs/development/installation.rst index 7f1788181..940f73036 100644 --- a/docs/development/installation.rst +++ b/docs/development/installation.rst @@ -3,8 +3,8 @@ Installation This document describes the necessary steps to configure Patchwork in a development environment. If you are interested in deploying Patchwork in a -production environment, refer to `the deployment guide -`__ instead. +production environment, refer to :doc:`the deployment guide +` instead. To begin, you should clone Patchwork: From 86137cc1fa42dabfe715cae669f746fe28b9b234 Mon Sep 17 00:00:00 2001 From: Konstantin Ryabitsev Date: Sat, 16 Nov 2019 11:16:00 -0500 Subject: [PATCH 64/73] Improve pull request URL matching regex When git-request-pull output is pasted into a mail client instead of mailed directly, the ref part of the pull URL may end up wrapped to the next line. Example: https://lore.kernel.org/r/294422a4-37b2-def5-5d32-8988f27c3a5b@gmail.com/ This change properly parses URLs both with and without newlines. Signed-off-by: Konstantin Ryabitsev Reviewed-by: Andrew Donnellan (cherry picked from commit 8c229caa71d58e971708ac7ebdb02d6858cd2a4c) --- patchwork/parser.py | 4 +- .../0023-git-pull-request-newline-in-url.mbox | 48 +++++++++++++++++++ patchwork/tests/test_parser.py | 9 ++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 patchwork/tests/mail/0023-git-pull-request-newline-in-url.mbox diff --git a/patchwork/parser.py b/patchwork/parser.py index a4762dbb4..e71d2de7c 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -892,11 +892,11 @@ def fn(x): def parse_pull_request(content): git_re = re.compile(r'^The following changes since commit.*' + r'^are available in the git repository at:\n' - r'^\s*([\S]+://[^\n]+)$', + r'^\s*([\w+-]+(?:://|@)[\w/.@:~-]+[\s\\]*[\w/._-]*)\s*$', re.DOTALL | re.MULTILINE | re.IGNORECASE) match = git_re.search(content) if match: - return match.group(1) + return re.sub('\s+', ' ', match.group(1)).strip() return None diff --git a/patchwork/tests/mail/0023-git-pull-request-newline-in-url.mbox b/patchwork/tests/mail/0023-git-pull-request-newline-in-url.mbox new file mode 100644 index 000000000..74c29ce47 --- /dev/null +++ b/patchwork/tests/mail/0023-git-pull-request-newline-in-url.mbox @@ -0,0 +1,48 @@ +From mboxrd@z Thu Jan 1 00:00:00 1970 +To: soc@kernel.org +From: Matthias Brugger +Subject: [GIT PULL] soc: updates for v5.5 +Message-ID: <294422a4-37b2-def5-5d32-8988f27c3a5b@gmail.com> +Date: Mon, 11 Nov 2019 13:23:51 +0100 + +Hi Olof and Arnd, + +Please have a look on the following updates of drivers/soc for v5.5 + +Thanks a lot, +Matthias + +--- + +The following changes since commit 54ecb8f7028c5eb3d740bb82b0f1d90f2df63c5c: + + Linux 5.4-rc1 (2019-09-30 10:35:40 -0700) + +are available in the Git repository at: + + https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/ +tags/v5.4-next-soc + +for you to fetch changes up to 662c9d55c5ccb37f3920ecab9720f2ebf2a6ca18: + + soc: mediatek: Refactor bus protection control (2019-11-07 10:11:04 +0100) + +---------------------------------------------------------------- +refactor code of mtk-scpsys + +---------------------------------------------------------------- +Weiyi Lu (5): + soc: mediatek: Refactor polling timeout and documentation + soc: mediatek: Refactor regulator control + soc: mediatek: Refactor clock control + soc: mediatek: Refactor sram control + soc: mediatek: Refactor bus protection control + + drivers/soc/mediatek/mtk-scpsys.c | 214 ++++++++++++++++++++++++++------------ + 1 file changed, 146 insertions(+), 68 deletions(-) + +_______________________________________________ +linux-arm-kernel mailing list +linux-arm-kernel@lists.infradead.org +http://lists.infradead.org/mailman/listinfo/linux-arm-kernel + diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index 4df1e1577..19e391dfa 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -583,6 +583,15 @@ def test_git_pull_with_diff(self): diff.startswith('diff --git a/arch/x86/include/asm/smp.h'), diff) + def test_git_pull_newline_in_url(self): + diff, message = self._find_content( + '0023-git-pull-request-newline-in-url.mbox') + pull_url = parse_pull_request(message) + self.assertEqual( + 'https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/' + 'linux.git/ tags/v5.4-next-soc', + pull_url) + def test_git_rename(self): diff, _ = self._find_content('0008-git-rename.mbox') self.assertTrue(diff is not None) From aa75400ff7cf34553eaf22289ce00060a64e86b5 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sat, 30 Nov 2019 15:40:15 +0000 Subject: [PATCH 65/73] parser: Fix style issues NOTE(stephenfin): Modified to fix another latent style issue that's popping up here. Signed-off-by: Stephen Finucane Fixes: 8c229caa ("Improve pull request URL matching regex") (cherry picked from commit 3ed6a1434dd90f073f5db5a6e85a80469aaaed40) --- patchwork/parser.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/patchwork/parser.py b/patchwork/parser.py index e71d2de7c..889a3721e 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -167,7 +167,7 @@ def clean_header(header): # on Py2, we want to do unicode(), on Py3, str(). # That gets us the decoded, un-wrapped header. if six.PY2: - header_str = unicode(sane_header) + header_str = unicode(sane_header) # noqa else: header_str = str(sane_header) @@ -890,13 +890,14 @@ def fn(x): def parse_pull_request(content): - git_re = re.compile(r'^The following changes since commit.*' + - r'^are available in the git repository at:\n' - r'^\s*([\w+-]+(?:://|@)[\w/.@:~-]+[\s\\]*[\w/._-]*)\s*$', - re.DOTALL | re.MULTILINE | re.IGNORECASE) + git_re = re.compile( + r'^The following changes since commit.*' + r'^are available in the git repository at:\n' + r'^\s*([\w+-]+(?:://|@)[\w/.@:~-]+[\s\\]*[\w/._-]*)\s*$', + re.DOTALL | re.MULTILINE | re.IGNORECASE) match = git_re.search(content) if match: - return re.sub('\s+', ' ', match.group(1)).strip() + return re.sub(r'\s+', ' ', match.group(1)).strip() return None From 58051431a33f87f5bcec58c1fbdc7e0fcb26fdc6 Mon Sep 17 00:00:00 2001 From: Konstantin Ryabitsev Date: Mon, 27 Jan 2020 12:56:19 -0500 Subject: [PATCH 66/73] Handle pull requests with random trailing space Another fix for copy-pasted pull requests, this time for cases when something is copy-pasted from a terminal and retains all the bogus trailing whitespace. Example: https://lore.kernel.org/r/043eb5b2-a302-4de6-a3e8-8238e49483b1@ti.com Signed-off-by: Konstantin Ryabitsev Reviewed-by: Stephen Finucane (cherry picked from commit 1633afe5b46042d522777f66b1959a82298d0ab2) --- patchwork/parser.py | 2 +- .../0024-git-pull-request-trailing-space.mbox | 60 +++++++++++++++++++ patchwork/tests/test_parser.py | 9 +++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 patchwork/tests/mail/0024-git-pull-request-trailing-space.mbox diff --git a/patchwork/parser.py b/patchwork/parser.py index 889a3721e..705c0a07f 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -892,7 +892,7 @@ def fn(x): def parse_pull_request(content): git_re = re.compile( r'^The following changes since commit.*' - r'^are available in the git repository at:\n' + r'^are available in the git repository at:\s*\n' r'^\s*([\w+-]+(?:://|@)[\w/.@:~-]+[\s\\]*[\w/._-]*)\s*$', re.DOTALL | re.MULTILINE | re.IGNORECASE) match = git_re.search(content) diff --git a/patchwork/tests/mail/0024-git-pull-request-trailing-space.mbox b/patchwork/tests/mail/0024-git-pull-request-trailing-space.mbox new file mode 100644 index 000000000..d62d070b8 --- /dev/null +++ b/patchwork/tests/mail/0024-git-pull-request-trailing-space.mbox @@ -0,0 +1,60 @@ +From mboxrd@z Thu Jan 1 00:00:00 1970 +To: Linux ARM Kernel List +From: XXX XXX +Subject: [GIT PULL] DaVinci SoC updates for v5.6 +Message-ID: <043eb5b2-a302-4de6-a3e8-8238e49483b1@ti.com> +Date: Tue, 14 Jan 2020 23:48:54 +0530 +Content-Type: text/plain; charset="us-ascii" +Content-Transfer-Encoding: 7bit + +The following changes since commit e42617b825f8073569da76dc4510bfa019b1c35a: + + Linux 5.5-rc1 (2019-12-08 14:57:55 -0800) + +are available in the Git repository at: + + git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v5.6/soc + +for you to fetch changes up to 5e06d19694a463a012c2589e29078196eb209448: + + ARM: davinci: dm644x-evm: Add Fixed regulators needed for tlv320aic33 (2020-01-13 17:36:26 +0530) + +---------------------------------------------------------------- +DaVinci SoC updates for v5.6 include migrating DM365 SoC to use +drivers/clocksource based driver for timer. This leads to removal +of machine specific timer driver. + +There are two patches adding missing fixed regulators for audio codecs +on DM365 and DM644x EVMs. + +---------------------------------------------------------------- +Bartosz Golaszewski (3): + clocksource: davinci: only enable clockevents once tim34 is initialized + ARM: davinci: dm365: switch to using the clocksource driver + ARM: davinci: remove legacy timer support + +Peter Ujfalusi (2): + ARM: davinci: dm365-evm: Add Fixed regulators needed for tlv320aic3101 + ARM: davinci: dm644x-evm: Add Fixed regulators needed for tlv320aic33 + + arch/arm/mach-davinci/Makefile | 3 +- + arch/arm/mach-davinci/board-dm365-evm.c | 20 ++ + arch/arm/mach-davinci/board-dm644x-evm.c | 20 ++ + arch/arm/mach-davinci/devices-da8xx.c | 1 - + arch/arm/mach-davinci/devices.c | 19 -- + arch/arm/mach-davinci/dm365.c | 22 +- + arch/arm/mach-davinci/include/mach/common.h | 17 -- + arch/arm/mach-davinci/include/mach/time.h | 33 --- + arch/arm/mach-davinci/time.c | 400 ---------------------------- + drivers/clocksource/timer-davinci.c | 8 +- + 10 files changed, 60 insertions(+), 483 deletions(-) + delete mode 100644 arch/arm/mach-davinci/include/mach/time.h + delete mode 100644 arch/arm/mach-davinci/time.c +~ +~ + +_______________________________________________ +linux-arm-kernel mailing list +linux-arm-kernel@lists.infradead.org +http://lists.infradead.org/mailman/listinfo/linux-arm-kernel + diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index 19e391dfa..216ab4813 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -592,6 +592,15 @@ def test_git_pull_newline_in_url(self): 'linux.git/ tags/v5.4-next-soc', pull_url) + def test_git_pull_trailing_space(self): + diff, message = self._find_content( + '0024-git-pull-request-trailing-space.mbox') + pull_url = parse_pull_request(message) + self.assertEqual( + 'git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/' + 'linux-davinci.git tags/davinci-for-v5.6/soc', + pull_url) + def test_git_rename(self): diff, _ = self._find_content('0008-git-rename.mbox') self.assertTrue(diff is not None) From f40bcd03401084a9687a9b1f7a0c210eb2e7e42e Mon Sep 17 00:00:00 2001 From: Mete Polat Date: Wed, 29 Jan 2020 20:01:22 +0100 Subject: [PATCH 67/73] REST: Fix duplicate project queries Eliminates duplicate project queries caused by calling get_absolute_url() in the embedded serializers. Following foreign keys with 'series__project' will cache the project of the series as well as the series itself. Signed-off-by: Mete Polat Signed-off-by: Stephen Finucane Closes: #335 (backported from commit ec00daae4d79bf2560034e1b2bc3cf76a98a3212 dropped all the tests, they clash horribly) Signed-off-by: Daniel Axtens --- patchwork/api/cover.py | 3 ++- patchwork/api/event.py | 4 ++-- patchwork/api/patch.py | 5 +++-- patchwork/api/series.py | 5 +++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index b497fd853..8a9e61e29 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -103,7 +103,8 @@ class CoverLetterList(ListAPIView): ordering = 'id' def get_queryset(self): - return CoverLetter.objects.all().prefetch_related('series')\ + return CoverLetter.objects.all().prefetch_related('series', + 'series__project')\ .select_related('project', 'submitter')\ .defer('content', 'headers') diff --git a/patchwork/api/event.py b/patchwork/api/event.py index cce25a75e..e354ae2c0 100644 --- a/patchwork/api/event.py +++ b/patchwork/api/event.py @@ -96,7 +96,7 @@ class EventList(ListAPIView): def get_queryset(self): return Event.objects.all()\ - .prefetch_related('project', 'patch', 'series', 'cover', - 'previous_state', 'current_state', + .prefetch_related('project', 'patch__project', 'series__project', + 'cover', 'previous_state', 'current_state', 'previous_delegate', 'current_delegate', 'created_check') diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index f772c6543..c55bf9c91 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -187,8 +187,9 @@ class PatchList(ListAPIView): def get_queryset(self): return Patch.objects.all()\ - .prefetch_related('series', 'check_set')\ - .select_related('project', 'state', 'submitter', 'delegate')\ + .prefetch_related('series', 'check_set', 'series__project')\ + .select_related('project', 'state', 'submitter', 'delegate', + )\ .defer('content', 'diff', 'headers') diff --git a/patchwork/api/series.py b/patchwork/api/series.py index 14768efbc..627b87434 100644 --- a/patchwork/api/series.py +++ b/patchwork/api/series.py @@ -69,8 +69,9 @@ class SeriesMixin(object): serializer_class = SeriesSerializer def get_queryset(self): - return Series.objects.all().prefetch_related('patches',)\ - .select_related('submitter', 'cover_letter', 'project') + return Series.objects.all()\ + .prefetch_related('patches__project',)\ + .select_related('submitter', 'cover_letter__project', 'project') class SeriesList(SeriesMixin, ListAPIView): From 913f1957f9341d5edc7c0b78c9c6d4a5b78f27fb Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Wed, 18 Mar 2020 00:59:13 +1100 Subject: [PATCH 68/73] REST: massively improve the patch counting query under filters The DRF web view counts the patches as part of pagination. The query it uses is a disaster zone: SELECT Count(*) FROM ( SELECT DISTINCT `patchwork_submission`.`id` AS Col1, `patchwork_submission`.`msgid` AS Col2, `patchwork_submission`.`date` AS Col3, `patchwork_submission`.`submitter_id` AS Col4, `patchwork_submission`.`project_id` AS Col5, `patchwork_submission`.`name` AS Col6, `patchwork_patch`.`submission_ptr_id` AS Col7, `patchwork_patch`.`commit_ref` AS Col8, `patchwork_patch`.`pull_url` AS Col9, `patchwork_patch`.`delegate_id` AS Col10, `patchwork_patch`.`state_id` AS Col11, `patchwork_patch`.`archived` AS Col12, `patchwork_patch`.`hash` AS Col13, `patchwork_patch`.`patch_project_id` AS Col14, `patchwork_patch`.`series_id` AS Col15, `patchwork_patch`.`number` AS Col16, `patchwork_patch`.`related_id` AS Col17 FROM `patchwork_patch` INNER JOIN `patchwork_submission` ON (`patchwork_patch`.`submission_ptr_id`=`patchwork_submission`.`id`) WHERE `patchwork_submission`.`project_id`=1 ) This is because django-filters adds a DISTINCT qualifier on a ModelMultiChoiceFilter by default. I guess it makes sense and they do a decent job of justifying it, but it causes the count to be made with this awful subquery. (The justification is that they don't know if you're filtering on a to-many relationship, in which case there could be duplicate values that need to be removed.) While fixing that, we can also tell the filter to filter on patch_project rather than submission's project, which allows us in some cases to avoid the join entirely. The resultant SQL is beautiful when filtering by project only: SELECT COUNT(*) AS `__count` FROM `patchwork_patch` WHERE `patchwork_patch`.`patch_project_id` = 1 On my test setup (2x canonical kernel mailing list in the db, warm cache, my laptop) this query goes from >1s to ~10ms, a ~100x improvement. If we filter by project and date the query is still nice, but still also very slow: SELECT COUNT(*) AS `__count` FROM `patchwork_patch` INNER JOIN `patchwork_submission` ON (`patchwork_patch`.`submission_ptr_id`=`patchwork_submission`.`id`) WHERE (`patchwork_patch`.`patch_project_id`=1 AND `patchwork_submission`.`date`>='2010-11-01 00:00:00') This us from ~1.3s to a bit under 400ms - still not ideal, but I'll take the 3x improvement! Reported-by: Konstantin Ryabitsev Signed-off-by: Daniel Axtens Reviewed-by: Stephen Finucane (backported from commit 97155c0bc8881787f6c536031b678a4c3f89bda6 old django-filters uses 'name' instead of 'field_name') Signed-off-by: Daniel Axtens --- patchwork/api/filters.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py index ddf527fd8..d374b5852 100644 --- a/patchwork/api/filters.py +++ b/patchwork/api/filters.py @@ -182,7 +182,8 @@ class Meta: class PatchFilterSet(TimestampMixin, FilterSet): - project = ProjectFilter(queryset=Project.objects.all()) + project = ProjectFilter(queryset=Project.objects.all(), distinct=False, + name='patch_project') # NOTE(stephenfin): We disable the select-based HTML widgets for these # filters as the resulting query is _huge_ series = BaseFilter(queryset=Series.objects.all(), From 4e3100fb3d268b46aebe71dd59c0d8360624cabf Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Wed, 18 Mar 2020 00:59:14 +1100 Subject: [PATCH 69/73] REST: fix patch listing query The patch listing query is punishingly slow under even very simple filters. The new data model in 3.0 will help _a lot_, so this is a simple fix: I did try indexes but haven't got really deeply into the weeds of what we can do with them. Move a number of things from select_related to prefetch_related: we trade off one big, inefficient query for a slightly larger number of significantly more efficient queries. On my laptop with 2 copies of the canonical kernel team list loaded into the database, and considering only the API view (the JSON view is always faster) with warm caches and considering the entire set of SQL queries: - /api/patches/?project=1 ~1.4-1.5s -> <100ms, something like 14x better - /api/patches/?project=1&since=2010-11-01T00:00:00 ~1.7-1.8s -> <560ms, something like 3x better (now dominated by the counting query only invoked on the HTML API view, not the pure JSON API view.) The things I moved: * project: this was generating SQL that looked like: INNER JOIN `patchwork_project` T5 ON (`patchwork_submission`.`project_id` = T5.`id`) This is correct but we've already had to join the patchwork_submission table and perhaps as a result it seems to be inefficient. * series__project: Likewise we've already had to join the series table, doing another join is possibly why it is inefficient. * delegate: I do not know why this was tanking performance. I think it might relate to the strategy mysql was using. Reported-by: Konstantin Ryabitsev Signed-off-by: Daniel Axtens Reviewed-by: Stephen Finucane (backported from commit 98a2d051372dcedb889c4cb94ebd8ed7b399b522 - dropped tests, it depends on a test we don't carry - rejigged to suit old M:N series model) Signed-off-by: Daniel Axtens --- patchwork/api/patch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index c55bf9c91..f593316d9 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -187,9 +187,9 @@ class PatchList(ListAPIView): def get_queryset(self): return Patch.objects.all()\ - .prefetch_related('series', 'check_set', 'series__project')\ - .select_related('project', 'state', 'submitter', 'delegate', - )\ + .prefetch_related('series', 'check_set', 'project', + 'delegate', 'series__project')\ + .select_related('state', 'submitter')\ .defer('content', 'diff', 'headers') From 90d85e24f1207daee22f46ab7eca4e367616dbd4 Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Wed, 18 Mar 2020 00:59:15 +1100 Subject: [PATCH 70/73] REST: extend performance improvements to other parts of the API We can trivially extend what we've just done to other parts of the API. I haven't done much by way of benchmark but we're seeing multiple 'x's pretty much across the board when filtering. Signed-off-by: Daniel Axtens Reviewed-by: Stephen Finucane (backported from commit 046aa155c3bf827691bab9e1df8916c969a30d54 - dropped tests, it depends on a test we don't carry - rejigged to suit old M:N series model and old API) Signed-off-by: Daniel Axtens --- patchwork/api/cover.py | 3 ++- patchwork/api/filters.py | 36 ++++++++++++++++++++---------------- patchwork/api/series.py | 4 ++-- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index 8a9e61e29..edc002982 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -104,8 +104,9 @@ class CoverLetterList(ListAPIView): def get_queryset(self): return CoverLetter.objects.all().prefetch_related('series', + 'project', 'series__project')\ - .select_related('project', 'submitter')\ + .select_related('submitter')\ .defer('content', 'headers') diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py index d374b5852..886f06ee7 100644 --- a/patchwork/api/filters.py +++ b/patchwork/api/filters.py @@ -158,8 +158,8 @@ class TimestampMixin(FilterSet): class SeriesFilterSet(TimestampMixin, FilterSet): - submitter = PersonFilter(queryset=Person.objects.all()) - project = ProjectFilter(queryset=Project.objects.all()) + submitter = PersonFilter(queryset=Person.objects.all(), distinct=False) + project = ProjectFilter(queryset=Project.objects.all(), distinct=False) class Meta: model = Series @@ -168,12 +168,12 @@ class Meta: class CoverLetterFilterSet(TimestampMixin, FilterSet): - project = ProjectFilter(queryset=Project.objects.all()) + project = ProjectFilter(queryset=Project.objects.all(), distinct=False) # NOTE(stephenfin): We disable the select-based HTML widgets for these # filters as the resulting query is _huge_ series = BaseFilter(queryset=Project.objects.all(), - widget=MultipleHiddenInput) - submitter = PersonFilter(queryset=Person.objects.all()) + widget=MultipleHiddenInput, distinct=False) + submitter = PersonFilter(queryset=Person.objects.all(), distinct=False) class Meta: model = CoverLetter @@ -187,10 +187,10 @@ class PatchFilterSet(TimestampMixin, FilterSet): # NOTE(stephenfin): We disable the select-based HTML widgets for these # filters as the resulting query is _huge_ series = BaseFilter(queryset=Series.objects.all(), - widget=MultipleHiddenInput) - submitter = PersonFilter(queryset=Person.objects.all()) - delegate = UserFilter(queryset=User.objects.all()) - state = StateFilter(queryset=State.objects.all()) + widget=MultipleHiddenInput, distinct=False) + submitter = PersonFilter(queryset=Person.objects.all(), distinct=False) + delegate = UserFilter(queryset=User.objects.all(), distinct=False) + state = StateFilter(queryset=State.objects.all(), distinct=False) class Meta: model = Patch @@ -200,7 +200,7 @@ class Meta: class CheckFilterSet(TimestampMixin, FilterSet): - user = UserFilter(queryset=User.objects.all()) + user = UserFilter(queryset=User.objects.all(), distinct=False) class Meta: model = Check @@ -213,13 +213,17 @@ class EventFilterSet(TimestampMixin, FilterSet): # filters as the resulting query is _huge_ # TODO(stephenfin): We should really use an AJAX widget of some form here project = ProjectFilter(queryset=Project.objects.all(), - widget=MultipleHiddenInput) + widget=MultipleHiddenInput, + distinct=False) series = BaseFilter(queryset=Series.objects.all(), - widget=MultipleHiddenInput) + widget=MultipleHiddenInput, + distinct=False) patch = BaseFilter(queryset=Patch.objects.all(), - widget=MultipleHiddenInput) + widget=MultipleHiddenInput, + distinct=False) cover = BaseFilter(queryset=CoverLetter.objects.all(), - widget=MultipleHiddenInput) + widget=MultipleHiddenInput, + distinct=False) class Meta: model = Event @@ -228,8 +232,8 @@ class Meta: class BundleFilterSet(FilterSet): - project = ProjectFilter(queryset=Project.objects.all()) - owner = UserFilter(queryset=User.objects.all()) + project = ProjectFilter(queryset=Project.objects.all(), distinct=False) + owner = UserFilter(queryset=User.objects.all(), distinct=False) class Meta: model = Bundle diff --git a/patchwork/api/series.py b/patchwork/api/series.py index 627b87434..6b95310e3 100644 --- a/patchwork/api/series.py +++ b/patchwork/api/series.py @@ -70,8 +70,8 @@ class SeriesMixin(object): def get_queryset(self): return Series.objects.all()\ - .prefetch_related('patches__project',)\ - .select_related('submitter', 'cover_letter__project', 'project') + .prefetch_related('patches__project', 'cover_letter__project')\ + .select_related('submitter', 'project') class SeriesList(SeriesMixin, ListAPIView): From eddea66d243df4d6137a70d321fcf08fcd45e25c Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Wed, 18 Mar 2020 00:59:16 +1100 Subject: [PATCH 71/73] REST: Add release note for faster queries Didn't quite seem like it fit anywhere else in the series. I want the release note mostly because I hope to backport this to stable. Signed-off-by: Daniel Axtens Reviewed-by: Stephen Finucane (cherry picked from commit 271d91341e0e9ebea13fcfc46fb2f68106753c66) Signed-off-by: Daniel Axtens --- .../notes/faster-api-db-queries-a1b5face736fe5b8.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 releasenotes/notes/faster-api-db-queries-a1b5face736fe5b8.yaml diff --git a/releasenotes/notes/faster-api-db-queries-a1b5face736fe5b8.yaml b/releasenotes/notes/faster-api-db-queries-a1b5face736fe5b8.yaml new file mode 100644 index 000000000..6c4f61140 --- /dev/null +++ b/releasenotes/notes/faster-api-db-queries-a1b5face736fe5b8.yaml @@ -0,0 +1,4 @@ +fixes: + - | + Queries to the REST API with filters are now significantly faster: slow + database queries were reworked. From 2dd5d6c82d51c3f6114e512f48264d595810d56a Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Tue, 14 Apr 2020 16:46:20 +1000 Subject: [PATCH 72/73] Release 2.1.6 Signed-off-by: Daniel Axtens --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 2b82e1aac..8f043f64b 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 6, 'alpha', 0) +VERSION = (2, 1, 6) __version__ = get_latest_version(VERSION) From 2a940ff834df53fd2241f3d32c1edb6d553c8ffc Mon Sep 17 00:00:00 2001 From: Daniel Axtens Date: Tue, 14 Apr 2020 16:46:25 +1000 Subject: [PATCH 73/73] Post-release version bump Signed-off-by: Daniel Axtens --- patchwork/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 8f043f64b..132c0d123 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -19,7 +19,7 @@ from patchwork.version import get_latest_version -VERSION = (2, 1, 6) +VERSION = (2, 1, 7, 'alpha', 0) __version__ = get_latest_version(VERSION)