Re: Last-Modified HTTP header from mod_dav_svn

On Mon, Nov 28, 2005 at 09:32:19PM -0800, Daniel Rall wrote:
>...
> > /* make sure the proper mtime is in the request record */
> > #if 0
> > ap_update_mtime(r, resource->info->finfo.mtime);
> > #endif
> >
> > /* ### note that these use r->filename rather than <resource> */
> > #if 0
> > ap_set_last_modified(r);
> > #endif
>
> #define's aside, this was a straight copy out of
> httpd/modules/dav/fs/repos.c, which I believe was used as the template
> for mod_dav_svn.

Right. I #if'd them out because they wouldn't work. As you noticed
:-), more coding was necessary to get that stuff to work.

> > I'm working on activating these, pulling the value of the "svn:date"
> > revprop using svn_fs_node_created_rev() and svn_fs_revision_prop().
>
> Done in r17549.
> http://svn.collab.net/viewcvs/svn/trunk/subversion/mod_dav_svn/repos.c?r1=17547&r2=17549

Ooooh! I'll take a look.

> > > 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 :-)

> Assuming so, the implementation would resemble something like the
> get_last_modified(resource) function introduced in r17549, and
> repos.c:dav_svn_set_headers() would include a header of
> "Cache-Control: max-age=MAX_SECONDS" in the response (where
> MAX_SECONDS is some big apr_time_t I haven't figured out yet).

Yup. 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.

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)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org