bottlepy / bottle

bottle.py is a fast and simple micro-framework for python web-applications.
http://bottlepy.org/
MIT License
8.39k stars 1.47k forks source link

E-Tags for static_file() and Request/Response #48

Open defnull opened 14 years ago

defnull commented 14 years ago

Bottles send_file() helper already supports Last-Modfied and If-Modified-Since headers and sends a 304 Not Modified response if applicable. A file-name and modification-time based Etag and If-None-Match support would be easy to implement.

I also want to add support for etags and mtime to normal handler functions. The Idea: Request.check_mtime() and Request.check_etag() methods that automatically abort execution of the handler function (by throwing a HTTPResponse(code=304) exception) on a match and do nothing but adding the header to the response object on a non-match. I'm not sure about the API and its hard to predict side-effects, though.

sgala commented 14 years ago

I'll give it a thought, the static ones look easy enough.

The main problem I see for the normal, not send_file, execution path is how to implement them in the handlres. Bottle is oriented to functional handlers, so callbacks for etag or mtime does not look easy to integrate. Are you thinking in a function in bottle that the app could call with a mtime and/or etag, and set header(s) and optionally short circuit?

I think that if the app is the one that gives a mtime and/or ETag, the checks are safe, in the sense that the contract of the function would be precisely this. For applications that can easily predict date or etag (for instance a blog can do that with the "updated" and the slug fields for entries) this would save at least bandwidth.

defnull commented 14 years ago

Yes, I meant that the Request.check_mtime() and Request.check_etag() methods get called explicitly by the callback handler functions.

sgala commented 14 years ago

I have been testing an implementation (but no tests, I just tested it with homepage/app:

http://github.com/sgala/bottle/commit/58c50bb5dab390a18209450db93762ae11611a31

Please comment in the code or here if you find issues.

The next commit in that branch adds a middleware to "gzip-transfer-encode" pages exceeding a threshold size. Mostly taken from the paste middleware, which is broken BTW.

Both things seem to work in my limited tests. I guess positive and negative tests for the check check_mtime and check_etag whould not be difficult to add. I just have little habit of writing bottle tests :(

defnull commented 14 years ago

I don"t like the API yet... Perhaps the exception-based approach is not the best one (or should not be the only one). How about:

response.etag = 'abc' # defaults to None response.mtime = ... # defaults to NOW if response.conditional(request_or_environ_dict): return # response.status is already set to 304 by .conditional() return expensive_page_generation()

I'll leave this open until we find a good solution. Thanks for the code and ideas.