[Twisted-Python] Re: [Twisted-commits] r15860 - Move checkPreconditions() from web2.dav.static.DAVFile to web2.resource.RenderMixIn.

[James Y Knight]

On Feb 8, 2006, at 7:49 PM, Wilfredo Sanchez wrote:
> Move checkPreconditions() from web2.dav.static.DAVFile to  
> web2.resource.RenderMixIn.
> 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. :)

James