toastdriven / restless

A lightweight REST miniframework for Python.
http://restless.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
832 stars 107 forks source link

Class-level `http_methods` attribute breaks Resource subclasses #103

Open donspaulding opened 7 years ago

donspaulding commented 7 years ago

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.