Daniel Axtens [Mon, 27 Mar 2017 05:01:13 +0000 (16:01 +1100)]
parser: cut off patch names after 255 characters
Our model limits the length of patch names to 255 characters.
Enforce this cutoff in the parser, otherwise we throw an
exception and fail to store patches with stupidly long titles.
Reported-by: Russell Currey <ruscur@russell.cc> Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Thu, 23 Mar 2017 13:25:00 +0000 (13:25 +0000)]
views: Use bundle-detail for public URLs
In 'd1c605f', we reworked the 'bundle-list' view to use the new
'Bundle.get_mbox_url' function instead of the 'Bundle.public_url'.
However, these are not the same thing. The latter referred to the
'bundle-detail' view, while the former referred to the 'bundle-mbox'
view.
The easiest fix would be to simply revert that patch. However, it turns
out that 'public_url' isn't actually needed. Commit '5d0140ef' removed a
divide between public and non-public URLs for bundles, meaning we can
actually use an existing function - 'get_absolute_url' - instead.
This also presents the opportunity to clean up the 'bundle-list' page,
favouring a simple public/is-not-public marker and only a single URL.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: d1c605f9 ("views: Stop using Bundle.public_url")
Closes-bug: #92
Stephen Finucane [Thu, 23 Mar 2017 13:24:59 +0000 (13:24 +0000)]
xmlrpc: Remove 'bundle_to_dict'
Turns out this function is not called anywhere nor has it been called
since being introduced in '83964878'. Given that we're now focused on
the REST API, we can simply remove this.
Signed-off-by: Stephen Finucane <stephen@that.guru>
It is possible to download a patch mbox with all dependencies. Now make
it possible to download the entire series. This takes the form of the
following URL when using the default routes:
/series/{seriesID}/mbox/
Like the equivalent patch and bundle links, this will return a 404 if
'{seriesID}' does not match an existing series' ID. However, a 404 will
also be returned in the series is not complete, as indicated by
Series.total > Series.received_total. You can override this behavior by
providing the 'force' parameter:
/series/{seriesID}/mbox/?force=1
Note that there are no current plans to provide a series-specific view,
a.k.a.
/series/{seriesID}/
As a result, this particular URL will continue to return a 404.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
The 'patch_to_mbox' function returns an object which is coverted to a
string in all places where this call occurs. The string conversion
differs between Python 2 and 3 and while it has been updated in one
place, it was missed in two others. Resolve these issues and ensure they
don't happen again by returning strings from 'patch_to_mbox' instead.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Ralf Baechle [Tue, 14 Mar 2017 18:09:12 +0000 (19:09 +0100)]
parsemail.sh: Fix usage via stdin.
Commit e56391f66cb8 ("trivial: Standardize variable naming in scripts")
sneaked in a checked for missing command line arguments in parsemail.sh
which broke typical argument-less usage for mail delivery. Revert that
again.
Signed-off-by: Ralf Baechle <ralf@linux-mips.org> Reviewed-by: Stephen Finucane <stephen@that.guru> Fixes: e56391f6 ("trivial: Standardize variable naming in scripts")
Stephen Finucane [Tue, 21 Feb 2017 16:22:41 +0000 (11:22 -0500)]
REST: Add '/bundle' endpoint
I had initially resisted adding a '/bundle' endpoint to the API as I
wanted to kill this feature in favour of series. However, series are not
a like-for-like replacement for bundles. Among other things, series do
not provide the composability of bundles: bundles can be manually
created, meaning you can use bundles to group not only multiple patches
but also multiple series (or at least the patches in those series).
Seeing as we're not getting rid of this feature, we should expose it via
the API. Bundles are little unusual, in that they can be public or
private, thus, we should only show bundles that are public or belonging
to the currently authenticated user, if any. For now, this is a
read-only endpoint. We may well allow creation of bundles via the API
once we figure out how to do this cleanly.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Sun, 26 Feb 2017 23:57:22 +0000 (23:57 +0000)]
views: Allow use of basic auth for bundle mboxes
API clients are going to talk using basic auth. We also need to do this
for bundles. The alternative is to provide another endpoint for bundles
in the API but that seems unnecessary.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Apparently different environments raise different return codes. Handle
the invalid return codes thrown by subprocess, presumably related to how
Travis checks out things from git.
Instead of exposing differing fields depending on the event category,
expose a consistent set of 'id', 'category', 'date' and 'payload'. This
ensures we can loop through events in a somewhat standardized fashion.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Daniel Axtens <dja@axtens.net>
This is a list only endpoint as it's expected that we would kill events
after a certain duration and would have no reason to allow indexing of
past events.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Daniel Axtens <dja@axtens.net>
At the moment, Series.received_all is only true is the number of patches
received exactly matches the expected number of patches. However, there
are cases where there may in fact be more patches in the final series
than expected. For example, one could send patches with a subject of
'PATCH 5/4' in-reply-to an original four patch series. The parser
handles this correctly, so the 'received_all' property should too.
We still need to support patterns like '5/n', which are in use and do
not appear to be supported since 'c21b305'. This can be done at a later
stage.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Tue, 21 Feb 2017 05:45:29 +0000 (16:45 +1100)]
REST: allow fetching of subject prefixes
Some mailing lists accept patches for multiple projects, and use
a subject prefix to differentiate the projects.
Therefore, for snowpatch, it's useful to be able to fetch the
subject prefixes.
Export subject prefixes in the REST API.
Signed-off-by: Daniel Axtens <dja@axtens.net> Tested-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Sat, 19 Nov 2016 18:31:32 +0000 (18:31 +0000)]
urls: Move 'help/about' to 'about'
This is the standard URL for such pages. This involves removing the
pwclient help page, but this is migrated to the project summary page
and detailed in the documentation.
Permanent redirects are included to prevent dead links.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
This is reflected in the migration but not the model.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: c1d8a0d ("Fix series.project migration logic") Reviewed-by: Andy Doan <andy.doan@linaro.org>
It would appear that object-based mixins don't work with django-filter's
FilterSet. This should probably be fixed upstream, but for now let's
live with the extra duplication.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 0fc32337 ("REST: Integrate django-filter support") Reviewed-by: Andy Doan <andy.doan@linaro.org>
Django 1.10.3, 1.9.11 and 1.8.16 changed default behavior for
ALLOWED_HOSTS to prevent DNS rebinding attacks [1]. Unfortunately this
also means we can't access the development Docker or Vagrant installs
by IP address. Sidestep the issue by wildcarding the 'ALLOWED_HOSTS'
setting for development, thus allowing connections from any IP.
This causes two issues. Firstly, on fresh installs you see the following
error message:
"Table 'patchwork.patchwork_state' doesn't exist"
Secondly, any new states created when the process is running will not be
reflected in the API until the server process is restarted.
Resolve this issue by moving the step into a method, thus ensuring it's
continuously refreshed. It doesn't seem possible to add tests to prevent
this regressing but some similarly useful tests are included to at least
validate the behavior of that field.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-By: Denis Laxalde <denis@laxalde.org> Fixes: a2993505 ("REST: Make 'Patch.state' editable")
Closes-bug: #67
Closes-bug: #80
Per feedback from FOSDEM, this is still confusing some people. Clarify
things.
You might think we could just strip of the offending prefixes but that
might not always be the thing to do. Other VCSs don't include these
prefixes and both 'a' and 'b' are valid folder names. The risk of false
positives might be small, but it's enough to discourage us from doing
this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Daniel Axtens [Wed, 8 Feb 2017 04:16:58 +0000 (15:16 +1100)]
Fix double use of mail directory in parsemail-batch.sh
2d142b2c0a27 ("bin: Run scripts through shellcheck") changes an
ls to a find. find output includes the directory name, unlike ls.
This breaks the script, because it prepends the mail directory
later on.
Remove the prepending in the script - leave it exclusively to find.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: stephen@that.guru Fixes: 2d142b2c0a27 ("bin: Run scripts through shellcheck")
Stephen Finucane [Mon, 23 Jan 2017 09:07:29 +0000 (09:07 +0000)]
migrations: Don't use 'noop'
The 'noop' operation was introduced in Django 1.8, meaning this
operation won't work on Django 1.7. Travis caught this, though I don't
know how it was missed locally.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: e3cbe493 ("models: Add 'project' field to Series")
Stephen Finucane [Sun, 22 Jan 2017 22:33:51 +0000 (22:33 +0000)]
Revert "requirements: Test older versions of DRF"
This partially reverts commit febad055fb6609369f1a465a5eec323549c5c065.
While Django REST Framework works with Django 1.6 and 1.7, the versions
of Django Filters that provide DRF integration do not [1]. It doesn't
really make sense to enable only partial REST API support (i.e. no
filtering) for users with older Django versions and this approach will
cause far too much confusion among users. Better to just drop REST API
support for users with these insecure versions and encourage them to
update to supported versions of Django should they wish to use these
features.
This details the various top level elements that Patchwork exposes.
This could be stored as source code documentation, but then users would
need to look at the source.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 11 Nov 2016 23:19:32 +0000 (23:19 +0000)]
REST: Integrate django-filter support
This mostly works out of the box, thanks to Django REST Framework.
Mostly unique fields, like name or email, are excluded as these will be
handled separately.
Signed-off-by: Stephen Finucane <stephen@that.guru>
This is helpful for filtering. We use RunPython because folks are likely
to have few if any Series objects existing yet. In addition, we update
the unique constraints for SeriesReference as it's now possible to
handle messages with duplicate message IDs.
The update is included in parser to ensure this applies immediately.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Wed, 16 Nov 2016 23:25:00 +0000 (23:25 +0000)]
docs: Update swagger definition
There are a couple of changes needed:
- Add the '/users' endpoint
- Add PATCH support for the '/projects' endpoint
- Add information on authentication
- Clean up some documentation
- Remove some non-implemented parameters
This is still not complete, but it bears a lot closer resemblence to
reality now.
Signed-off-by: Stephen Finucane <stephen@that.guru>
This is necessary due to the N:N mapping of series and patches: it's
possible for a patch to belong to many series, and a series usually
contains many patches. This means it is not possible to rely on the
patch endpoint alone.
It is also necessary to add a cover letter endpoint, such that the
series body can include this.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Thu, 17 Nov 2016 01:00:04 +0000 (01:00 +0000)]
REST: Make 'User.first_name', 'last_name' editable
In an ideal world, everything that can be done with the UI should also
be doable from the command line. The first and last names of the User
are low-value fields to allow customization of, but they are easily
implmented and move us towards the above goal.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Wed, 23 Nov 2016 15:53:50 +0000 (15:53 +0000)]
REST: Make 'Patch.state' editable
The 'Patch.state' field is exposed by the API, but is not editable.
Tests exist that suggest the field is editable, but they lie as they
don't actually validate the field is changed (it hasn't). Make this
field editable, using a custom field type to allow said user to submit a
string representation of the state rather than an integer id.
This has the side effect of changing the output representation of the
'state' field for the '/patches' endpoint to a slugified representation,
i.e.:
"state": "under-review",
Instead of:
"state": "Under review",
The same slugified representation will be used in PATCH requests. This
seems more consistent with how APIs generally do this stuff making it a
"good thing" (TM).
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org> Reviewed-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Thu, 24 Nov 2016 12:55:42 +0000 (12:55 +0000)]
REST: Use generic views instead of ViewSets
ViewSet provide an easy way to define an API, but they don't offer much
flexibility. To get them to work as expected, a lot of hacks were
required. Generic views provide a more verbose, but ultimately easier to
understand, version. Using generic views lets us remove the dependency
of drf-nested-routers, bringing us back down to two main dependencies.
It also lets us remove the AuthenticatedReadOnly permission class, as
the DRF provides a similar permission class that can be used with
generic views.
The main user facing change is that invalid methods, such as POST on an
endpoint that doesn't allow object creation, will now return a HTTP 405
(Method Not Allowed) error code rather than the HTTP 403 (Forbidden)
error code previously returned. This is the semantically correct option
and should have been used all along.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Fri, 28 Oct 2016 18:02:41 +0000 (19:02 +0100)]
REST: Resolve performance issues with tags
The list comprehension used to generate a list of tags resulted in a
significant number of duplicated queries. Resolve this by copying the
approach taken to minimize patch queries in the UI.
This changes the output of the response in two ways. First, counts for
all tag patches are now shown, even if the count is 0. Secondly, a
dictionary is returned, with the tag name as the key, rather than a
list. As such, the output for a typical patch is transformed from:
Stephen Finucane [Wed, 23 Nov 2016 19:41:12 +0000 (19:41 +0000)]
REST: Explicitly define fields
Explicitly define included fields (using 'Meta.fields') rather than
those that should be excluded (using 'Meta.exclude'). This ensure fields
that are exposed by the API don't change unless we explicitly change
them
The only side-effect of this change should be a consistent ordering of
the output - the ordering used in the 'Meta.fields' will be consistently
used, thus ordering is now important and some fields are moved around
to reflect this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Wed, 23 Nov 2016 19:35:40 +0000 (19:35 +0000)]
REST: Use SerializerMethod field
Use 'SerializerMethodField' to override how we generate fields, rather
than hacking the 'to_representation' method. This is the more idiomatic
way to do this.
The Meta class for the Project and Patch serializers are moved to the
bottom of the serializers and some variables renamed, both for
consistency's sake.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Thu, 24 Nov 2016 09:33:53 +0000 (09:33 +0000)]
REST: Remove '_url' suffixes
This was a design decision made when implementing the REST API. The
idea was that these items were URLs to related objects and should be
indicated as such. However, this was a faulty assumption as the
Patchwork API, unlike other some other APIs (GitHub), does not also
include a full representation of said objects, like so:
Since there is no intention to support this design yet, there isn't
really any reason to fight django-rest-framework in appending these
suffixes. Simply remove them. This changes the output for most endpoints
from something like this:
Stephen Finucane [Mon, 31 Oct 2016 18:23:29 +0000 (18:23 +0000)]
REST: Create 'api' directory
Move all REST API-related code into an 'api' directory. This allows us
to break the existing files into endpoint-based files and will allow us
to split the API into a different Django app in the future. This
involves simply shuffling code around for now, so there no functional
changes introduced.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Yann E. MORIN [Thu, 22 Dec 2016 21:09:32 +0000 (22:09 +0100)]
bin/pwclient: accept alternate http_proxy forms
The forms for http_proxy (and the likes) is not very well defined nor
documented in any authoritaive place. However, there are two common
forms: http://host:port or http://host:port/
Currently, the code chokes on the latter (e.g. with
http_proxy=http://127.0.0.1:8080/ ):
[...]
File "/usr/lib/python2.7/httplib.py", line 792, in _get_hostport
raise InvalidURL("nonnumeric port: '%s'" % host[i+1:])
httplib.InvalidURL: nonnumeric port: '8080/'
Chop off any slash character in the port definition to accept the second
form. If there is no '/' in there, it still works.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Signed-off-by: Stephen Finucane <stephenfinucane@hotmail.com>
Stephen Finucane [Fri, 23 Dec 2016 19:40:59 +0000 (19:40 +0000)]
forms: 'False' != False
Forms cast boolean values to strings, and attempting to coerce using the
'bool' function does not correctly return them to true boolean values.
Correct this by doing a string comparison instead.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 0abde97aa ("forms: Use TypedChoiceField")
Stephen Finucane [Fri, 23 Dec 2016 20:21:56 +0000 (20:21 +0000)]
tests: Set 'date' when bulk creating patches
Apparently Django 1.8+ changed how patches with identical 'date' fields
were ordered, meaning tests that worked on these versions failed on
previous versions.
Florian Fainelli [Tue, 13 Dec 2016 18:47:37 +0000 (10:47 -0800)]
xmlrpc: Expose patch hash to patch_get
Expose the patch object hash value to xmlrpc::patch_get. This is
particuarly helpful if some patche(s) have been submitted several times
but changed from e.g: RFC to a proper official patch submission. The
hash would typically be identical, but numbers would not.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> Reviewed-by: Stephen Finucane <stephen@that.guru>