Class-level `http_methods` attribute breaks Resource subclasses
donspaulding opened this issue · 0 comments
http_methods
is defined as a class-level attribute on the Resource class.
But then the documentation for Custom Endpoints recommends overriding http_methods
in your resource's __init__
method.
The trouble with this technique is that there is only ever a single copy of http_methods
and the last call to .update(your_custom_endpoints_dict)
will clobber other resource definitions across the entire site.
This works fine for the example in the documentation, because it is defining a new endpoint to be hooked up to a urlconf using cls.as_view('schema')
. As long as no other resource similarly overrides the schema
http_method with a different mapping, this will work.
But in the following situation, it breaks down a bit:
class CustomEndpointResource(Resource):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.http_methods.update({
'detail': {
'GET': 'detail',
'OPTIONS': 'cors_preflight'
}
})
class InnocentBystanderResource(Resource):
def update(self):
return {"error": "sometimes reached, sometimes not"}
In this example, we want to add the ability for the CustomEndpointResource
to respond to OPTIONS
requests, for CORS purposes. We expect that when we call self.http_methods.update()
we're only updating the http_methods on our own resource. Instead, we're updating our parent class' attribute Resource.http_methods
. A race condition ensues where until some request hits CustomEndpointResource
, the global http_methods
will still contain the default mapping of the PUT
HTTP-level method to the update
method on InnocentBystanderResource
, but once a request comes in for CustomEndpointResource
, all further requests to InnocentBystanderResource
will fail with an error similar to:
restless.exceptions.MethodNotImplemented: Unsupported method 'PUT' for detail endpoint.
A simple fix for this is to simply instantiate a copy of http_methods
inside the __init__
method of the Resource class, but other techniques would work as well. I'm sure this is an intentional API choice, but it contains a nasty gotcha bug for this corner case.