On Tue, 29 Nov 2005, Greg Stein wrote:
> On Mon, Nov 28, 2005 at 09:32:19PM -0800, Daniel Rall wrote:
> > http://svn.collab.net/viewcvs/svn/trunk/subversion/mod_dav_svn/repos.c?r1=17547&r2=17549
> > > > For (at least) version resources, we should also be setting the
> > > > Cache-Control header. The max-age should be set to some ridiculously
> > > > high number since a version resource can't change.
> > >
> > > Greg, are you referring to this specific type (from mod_dav.h)?:
> > >
> > > DAV_RESOURCE_TYPE_VERSION, /* version or baseline URL */
> Yes. Essentially that type refers to a specific <rev, path> pair.
> > Or did you mean versioned resources like files and directories?
> "version resource" is a DAV term with a specific meaning. We're
> probably talking about the same thing :-)
Yes, we are, thanks for the clarification (and thanks to sussman as
well!). I didn't understand that DAV_RESOURCE_TYPE_VERSION was an
indicator for a <rev, path> pair.
> Note that the LACKS_ETAG macro also provides an etag for REGULAR
> resources, which you can't do. Those change over time, so they
> shouldn't use Cache-Control (let the proxy use the etag and L-M header
> to see if the resource has changed).
> I'm not sure in which cases dav_svn_set_headers() is called, but
> hopefully just for GET/HEAD requests. Should be double-checked.
I've confirmed that it's not called for PROPFIND requests, and is
called for HEAD and GET requests. I haven't checked other HTTP
methods, but given that it is used in the definition of mod_dav_svn's
dav_hooks_repository, I'm fairly certain dav_svn_set_headers()
functions as desired. Here's the API it conforms to (as defined by
** If a GET is processed using a stream (open_stream, read_stream)
** rather than via a sub-request (on get_pathname), then this function
** is used to provide the repository with a way to set the headers
** in the response.
** This function may be called without a following deliver(), to
** handle a HEAD request.
** This may be NULL if handle_get is FALSE.
dav_error * (*set_headers)(request_rec *r,
const dav_resource *resource);
> Oh, and note that setting MAX_SECONDS to even just a day would be a
> big win. Make it a year if you want, but if you grow hinky with that
> duration, then something less will still be a Good Thing. (I'd go with
> a week, I think)
I can't seem to get the conditional right. 'svn cat -r2 http://...'
is apparently neither a DAV_RESOURCE_TYPE_VERSION, nor a
--- subversion/mod_dav_svn/repos.c (revision 17559)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -2176,6 +2176,11 @@
+ /* As version resources don't change, encourage caching. */
+ if (resource->type == DAV_RESOURCE_TYPE_VERSION)
+ /* Cache resource for one week (specified in seconds). */
+ apr_table_setn(r->headers_out, "Cache-Control", "max-age=604800");
/* we accept byte-ranges */
apr_table_setn(r->headers_out, "Accept-Ranges", "bytes");
Any suggestions as to how the conditional ought to be implemented?
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Tue Nov 29 23:54:55 2005