klein icon indicating copy to clipboard operation
klein copied to clipboard

There are problems with registerAdapter(KleinRequest, Request, IKleinRequest)

Open exarkun opened this issue 12 years ago • 1 comments

app.py registers KleinRequest as an adapter from twisted.web.server.Request to IKleinRequest.

It looks like the purpose of this is to create an extra object where some extra Klein-specific state can can live. The state is per-request so Klein wants to associate it with a twisted.web.server.Request instance and be able to retrieve it from that instance later.

It so happens that with this adapter registered IKleinRequest(someRequest) is IKleinRequest(someRequest) - providing the desired persistent state storage and access. However, this is only because twisted.web.server.Request subclasses Componentized.

It's somewhat questionable whether "is a Componentized subclass" is really meant to be part of the public interface of the Twisted Web request object (though public or not it clearly is currently part of that interface). In practice, the fact that zero Twisted Web unit tests fail if twisted.web.server.Request is made to not be a Componentized subclass makes relying on this property at least risky.

Apart from that, this dependency seems to make unit testing Klein-using applications more difficult.

To exercise the request dispatch logic it becomes necessary to manufacture a suitably test-friendly IRequest implementation which also has the same persistent adapter behavior of Componentized (probably by subclassing Componentized) and then register KleinRequest as an adapter for that class as well.

I suspect this is a deeper reliance on some internals of Klein than the Klein developers would have aimed for in their testing story.

Notice that Klein itself probably suffers from some testing issues related to this. DummyRequest in test_app.py is not a Componentized so the behavior presented when adapting it to IKleinRequest does not accurately reflect the behavior that real Klein code will get in non-test usage.

I think a good change to make to Klein would be to explicitly wrap a KleinRequest around the underlying IRequest provider and then pass it around wherever it is needed.

Perhaps this means KleinRequest should also be a proxy for IRequest to the wrapped object (yay proxyForInterface) so that only one request object actually needs to be passed everywhere.

exarkun avatar Dec 20 '13 17:12 exarkun

I'm totally +1 on replacing the abuse of Componentized with proxyForInterface.

dreid avatar Dec 20 '13 17:12 dreid