[Twisted-Python] Re: [Twisted-commits] r15860 - Move checkPreconditions() from web2.dav.static.DAVFile to web2.resource.RenderMixIn.
James Y Knight
foom at fuhm.net
Thu Feb 9 13:34:29 EST 2006
On Feb 8, 2006, at 7:49 PM, Wilfredo Sanchez wrote:
> Move checkPreconditions() from web2.dav.static.DAVFile to
> Add test suite (ported foom's tests for server.checkpreconditions()).
> Fix some bugs found by the tests. (Holy cow, checkpreconditions
> are a PITA.)
I don't like this part. You've made the precondition check code much
longer, IMO harder to understand (figuring out how the return values
of one method influenced the rest of the execution is not easy), and
pinned it to being part of Resource, rather than being reusable. I'm
not sure of the need, given that the previous implementation was
callable from Resource as it was. Why not just use it...
Nextly, I'm somewhat concerned about the change to calling the
precondition check before GET rendering instead of afterwards: I
think your use cases are probably not typical. I believe most people
will write resources which generate dynamic content, and if they're
_really good_ about it, they'll simultaneously generate an etag and a
last-modified time. This is also the only way that such information
can come out of wsgi, cgi, proxies to other servers, etc.
The way things were set up before made Not Modified responses just
"magically" show up correctly when you set those headers in your
response. No more interface was required than that, which means that
the response could come from anywhere. Now, instead, you force the
author to implement resource.etag() and resource.lastModified() in
order to get that behavior. It's really easy for file-like resources
to do so, but not so much so for most other kinds.
And the thing is, it's OK to check afterwards for the kinds of
requests a web browser makes: GET/HEAD/POST. GET/HEAD don't change
things so it's acceptable to do their check at any time, and while
POST does change things, as a practical matter nobody actually sends
conditionals on post requests, because what would it be useful for?
(and nothing would be set up to handle such a thing).
However, I'm currently ignoring conditionals on PostableResource's
http_POST, which while okay as far as real-world interoperability
goes, it is certainly not _right_. And to make things worse, the post-
processing filter doesn't confine itself to checking only GET/HEAD
requests, so the client will get back a precondition failed message,
even though the method was executed. At the very least a POST with an
If-Unmodified-Since/If-Match should return precondition failed. If
there was code on Resource to handle that automatically, I could not
have forgetten to do it.
So, um, I dunno what to conclude. :)
More information about the Twisted-Python