| Message ID | 20180607130447.25186-1-stephen@that.guru |
|---|---|
| State | Accepted |
| Headers | show |
| Series | views: Raise 404 if downloading non-existent dependencies | expand |
I've briefly read through the patch, and it LGTM. Stephen Finucane <stephen@that.guru> writes: > If a patch was processed by Patchwork before series support was added, > it will not have a series associated with it. As a result, it is not > possible to extract the dependencies for that patch from the series and > a 404 should be raised. This was not previously handled correctly. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > Reported-by: John McNamara <john.mcnamara@intel.com> > Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox' endpoint") > Closes: #189 > --- > NOTE: This needs to be backported to stable/2.0 once merged to master as > it's an issue there too. > --- > patchwork/tests/test_mboxviews.py | 24 +++++++++++++++++++ > patchwork/views/patch.py | 5 ++++ > .../notes/issue-189-fe4024f33e1b5203.yaml | 7 ++++++ > 3 files changed, 36 insertions(+) > create mode 100644 releasenotes/notes/issue-189-fe4024f33e1b5203.yaml > > diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py > index 2d6cdc30..8eb3581a 100644 > --- a/patchwork/tests/test_mboxviews.py > +++ b/patchwork/tests/test_mboxviews.py > @@ -31,6 +31,7 @@ from patchwork.tests.utils import create_comment > from patchwork.tests.utils import create_patch > from patchwork.tests.utils import create_project > from patchwork.tests.utils import create_person > +from patchwork.tests.utils import create_series_patch > from patchwork.tests.utils import create_user > > > @@ -206,3 +207,26 @@ class MboxCommentPostcriptUnchangedTest(TestCase): > > self.assertContains(response, content) > self.assertNotContains(response, content + '\n') > + > + > +class MboxSeriesDependencies(TestCase): > + > + def test_patch_with_dependencies(self): > + patch_a = create_series_patch() > + patch_b = create_series_patch(series=patch_a.series) > + > + response = self.client.get('%s?series=*' % reverse( > + 'patch-mbox', args=[patch_b.patch.id])) > + > + self.assertContains(response, patch_a.patch.content) > + self.assertContains(response, patch_b.patch.content) > + > + def test_legacy_patch(self): > + """Validate a patch with non-existent dependencies raises a 404.""" > + # we're explicitly creating a patch without a series > + patch = create_patch() > + > + response = self.client.get('%s?series=*' % reverse( > + 'patch-mbox', args=[patch.id])) > + > + self.assertEqual(response.status_code, 404) > diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py > index eccb5441..cbd4ec39 100644 > --- a/patchwork/views/patch.py > +++ b/patchwork/views/patch.py > @@ -139,6 +139,11 @@ def patch_mbox(request, patch_id): > > response = HttpResponse(content_type='text/plain') > if series_id: > + if not patch.series.count(): > + raise Http404('Patch does not have an associated series. This is ' > + 'because the patch was processed with an older ' > + 'version of Patchwork. It is not possible to ' > + 'provide dependencies for this patch.') > response.write(series_patch_to_mbox(patch, series_id)) > else: > response.write(patch_to_mbox(patch)) > diff --git a/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml > new file mode 100644 > index 00000000..ce9fe93c > --- /dev/null > +++ b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml > @@ -0,0 +1,7 @@ > +--- > +fixes: > + - | > + If a patch was processed by Patchwork before series support was added, it > + will not have a series associated with it. As a result, it is not possible > + to extract the dependencies for that patch from the series. This was not > + previously handled correctly. A 404 is now raised if this occurs. > -- > 2.17.1 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On Wed, 2018-06-13 at 12:17 +1000, Daniel Axtens wrote: > I've briefly read through the patch, and it LGTM. Thanks. I've applied this now. Got two more patches to resolve usability concerns and I think we're good to cut 2.1. Stephen > Stephen Finucane <stephen@that.guru> writes: > > > If a patch was processed by Patchwork before series support was > > added, > > it will not have a series associated with it. As a result, it is > > not > > possible to extract the dependencies for that patch from the series > > and > > a 404 should be raised. This was not previously handled correctly. > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > Reported-by: John McNamara <john.mcnamara@intel.com> > > Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox' > > endpoint") > > Closes: #189 > > --- > > NOTE: This needs to be backported to stable/2.0 once merged to > > master as > > it's an issue there too. > > --- > > patchwork/tests/test_mboxviews.py | 24 > > +++++++++++++++++++ > > patchwork/views/patch.py | 5 ++++ > > .../notes/issue-189-fe4024f33e1b5203.yaml | 7 ++++++ > > 3 files changed, 36 insertions(+) > > create mode 100644 releasenotes/notes/issue-189- > > fe4024f33e1b5203.yaml > > > > diff --git a/patchwork/tests/test_mboxviews.py > > b/patchwork/tests/test_mboxviews.py > > index 2d6cdc30..8eb3581a 100644 > > --- a/patchwork/tests/test_mboxviews.py > > +++ b/patchwork/tests/test_mboxviews.py > > @@ -31,6 +31,7 @@ from patchwork.tests.utils import create_comment > > from patchwork.tests.utils import create_patch > > from patchwork.tests.utils import create_project > > from patchwork.tests.utils import create_person > > +from patchwork.tests.utils import create_series_patch > > from patchwork.tests.utils import create_user > > > > > > @@ -206,3 +207,26 @@ class > > MboxCommentPostcriptUnchangedTest(TestCase): > > > > self.assertContains(response, content) > > self.assertNotContains(response, content + '\n') > > + > > + > > +class MboxSeriesDependencies(TestCase): > > + > > + def test_patch_with_dependencies(self): > > + patch_a = create_series_patch() > > + patch_b = create_series_patch(series=patch_a.series) > > + > > + response = self.client.get('%s?series=*' % reverse( > > + 'patch-mbox', args=[patch_b.patch.id])) > > + > > + self.assertContains(response, patch_a.patch.content) > > + self.assertContains(response, patch_b.patch.content) > > + > > + def test_legacy_patch(self): > > + """Validate a patch with non-existent dependencies raises > > a 404.""" > > + # we're explicitly creating a patch without a series > > + patch = create_patch() > > + > > + response = self.client.get('%s?series=*' % reverse( > > + 'patch-mbox', args=[patch.id])) > > + > > + self.assertEqual(response.status_code, 404) > > diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py > > index eccb5441..cbd4ec39 100644 > > --- a/patchwork/views/patch.py > > +++ b/patchwork/views/patch.py > > @@ -139,6 +139,11 @@ def patch_mbox(request, patch_id): > > > > response = HttpResponse(content_type='text/plain') > > if series_id: > > + if not patch.series.count(): > > + raise Http404('Patch does not have an associated > > series. This is ' > > + 'because the patch was processed with an > > older ' > > + 'version of Patchwork. It is not > > possible to ' > > + 'provide dependencies for this patch.') > > response.write(series_patch_to_mbox(patch, series_id)) > > else: > > response.write(patch_to_mbox(patch)) > > diff --git a/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml > > b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml > > new file mode 100644 > > index 00000000..ce9fe93c > > --- /dev/null > > +++ b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml > > @@ -0,0 +1,7 @@ > > +--- > > +fixes: > > + - | > > + If a patch was processed by Patchwork before series support > > was added, it > > + will not have a series associated with it. As a result, it is > > not possible > > + to extract the dependencies for that patch from the series. > > This was not > > + previously handled correctly. A 404 is now raised if this > > occurs. > > -- > > 2.17.1 > > > > _______________________________________________ > > Patchwork mailing list > > Patchwork@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/patchwork
diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py index 2d6cdc30..8eb3581a 100644 --- a/patchwork/tests/test_mboxviews.py +++ b/patchwork/tests/test_mboxviews.py @@ -31,6 +31,7 @@ from patchwork.tests.utils import create_comment from patchwork.tests.utils import create_patch from patchwork.tests.utils import create_project from patchwork.tests.utils import create_person +from patchwork.tests.utils import create_series_patch from patchwork.tests.utils import create_user @@ -206,3 +207,26 @@ class MboxCommentPostcriptUnchangedTest(TestCase): self.assertContains(response, content) self.assertNotContains(response, content + '\n') + + +class MboxSeriesDependencies(TestCase): + + def test_patch_with_dependencies(self): + patch_a = create_series_patch() + patch_b = create_series_patch(series=patch_a.series) + + response = self.client.get('%s?series=*' % reverse( + 'patch-mbox', args=[patch_b.patch.id])) + + self.assertContains(response, patch_a.patch.content) + self.assertContains(response, patch_b.patch.content) + + def test_legacy_patch(self): + """Validate a patch with non-existent dependencies raises a 404.""" + # we're explicitly creating a patch without a series + patch = create_patch() + + response = self.client.get('%s?series=*' % reverse( + 'patch-mbox', args=[patch.id])) + + self.assertEqual(response.status_code, 404) diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index eccb5441..cbd4ec39 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -139,6 +139,11 @@ def patch_mbox(request, patch_id): response = HttpResponse(content_type='text/plain') if series_id: + if not patch.series.count(): + raise Http404('Patch does not have an associated series. This is ' + 'because the patch was processed with an older ' + 'version of Patchwork. It is not possible to ' + 'provide dependencies for this patch.') response.write(series_patch_to_mbox(patch, series_id)) else: response.write(patch_to_mbox(patch)) diff --git a/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml new file mode 100644 index 00000000..ce9fe93c --- /dev/null +++ b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + If a patch was processed by Patchwork before series support was added, it + will not have a series associated with it. As a result, it is not possible + to extract the dependencies for that patch from the series. This was not + previously handled correctly. A 404 is now raised if this occurs.
If a patch was processed by Patchwork before series support was added, it will not have a series associated with it. As a result, it is not possible to extract the dependencies for that patch from the series and a 404 should be raised. This was not previously handled correctly. Signed-off-by: Stephen Finucane <stephen@that.guru> Reported-by: John McNamara <john.mcnamara@intel.com> Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox' endpoint") Closes: #189 --- NOTE: This needs to be backported to stable/2.0 once merged to master as it's an issue there too. --- patchwork/tests/test_mboxviews.py | 24 +++++++++++++++++++ patchwork/views/patch.py | 5 ++++ .../notes/issue-189-fe4024f33e1b5203.yaml | 7 ++++++ 3 files changed, 36 insertions(+) create mode 100644 releasenotes/notes/issue-189-fe4024f33e1b5203.yaml