bumping mod_dav

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

bumping mod_dav

Ben Collins-Sussman
There are few 'hacks' in Subversion right now to deal with problems  
with mod_dav's API.  There are also a couple of new Subversion issues  
that could be solved with further hacks, but would be better remedied  
through a mod_dav API change.

These things are documented here:

    http://subversion.tigris.org/issues/show_bug.cgi?id=2248

Since there are rumors of 2.2 coming out soon-ish, I'm hoping that  
now would be a good time to get the mod_dav API/ABI change into httpd  
trunk.  We're not talking about a lot of code here.  If others agree,  
I can start posting patches.

(Greg Stein, I assume you're okay with this too?  I think it was your  
idea to bump mod_dav in the first place...)


Reply | Threaded
Open this post in threaded view
|

Re: bumping mod_dav

gstein
On Thu, May 26, 2005 at 11:21:53AM -0500, Ben Collins-Sussman wrote:

> There are few 'hacks' in Subversion right now to deal with problems  
> with mod_dav's API.  There are also a couple of new Subversion issues  
> that could be solved with further hacks, but would be better remedied  
> through a mod_dav API change.
>
> These things are documented here:
>
>    http://subversion.tigris.org/issues/show_bug.cgi?id=2248
>
> Since there are rumors of 2.2 coming out soon-ish, I'm hoping that  
> now would be a good time to get the mod_dav API/ABI change into httpd  
> trunk.  We're not talking about a lot of code here.  If others agree,  
> I can start posting patches.
>
> (Greg Stein, I assume you're okay with this too?  I think it was your  
> idea to bump mod_dav in the first place...)

Oh, I'm totally fine with it, but I think you're a bit late in the cycle
for 2.2. The idea is to close and release that RSN.

If you've got patches handy, and can start throwing them to the list, then
it could be possible to squeak them in.

Cheers,
-g

--
Greg Stein, http://www.lyra.org/
Reply | Threaded
Open this post in threaded view
|

Re: bumping mod_dav

Sander Striker
Greg Stein wrote:

> On Thu, May 26, 2005 at 11:21:53AM -0500, Ben Collins-Sussman wrote:
>
>>There are few 'hacks' in Subversion right now to deal with problems  
>>with mod_dav's API.  There are also a couple of new Subversion issues  
>>that could be solved with further hacks, but would be better remedied  
>>through a mod_dav API change.
>>
>>These things are documented here:
>>
>>   http://subversion.tigris.org/issues/show_bug.cgi?id=2248
>>
>>Since there are rumors of 2.2 coming out soon-ish, I'm hoping that  
>>now would be a good time to get the mod_dav API/ABI change into httpd  
>>trunk.  We're not talking about a lot of code here.  If others agree,  
>>I can start posting patches.
>>
>>(Greg Stein, I assume you're okay with this too?  I think it was your  
>>idea to bump mod_dav in the first place...)
>
>
> Oh, I'm totally fine with it, but I think you're a bit late in the cycle
> for 2.2. The idea is to close and release that RSN.

Yeah, but with our definition of RSN, we can prolly handle this.
 
> If you've got patches handy, and can start throwing them to the list, then
> it could be possible to squeak them in.

Indeed.  Ben, fire away!

Sander
Reply | Threaded
Open this post in threaded view
|

Re: bumping mod_dav

Ben Collins-Sussman

On May 26, 2005, at 12:38 PM, Sander Striker wrote:
>
>> If you've got patches handy, and can start throwing them to the  
>> list, then
>> it could be possible to squeak them in.
>>
>
> Indeed.  Ben, fire away!

OK... I'm working on the patches now.  They involve creating a
dav_provider2 structure which will be registered as version "2" with
ap_register_provider().  It means tweaking mod_dav machinery to accept
both old and new providers.

Before I make the changes, though, I want to make sure folks are okay
with the APIs I plan to change:

1.  We want mod_dav_svn to handle POST requests.

     mod_dav currently passes REPORT requests to
     provider->vsn_hooks->deliver_report().  But for POST, it merely
     validates the resource and then DECLINES.  So while it's still
     possible for a provider like mod_dav_svn to register itself to
     handle POST requests (via normal apache hooks), it's really
     inconvenient.  Does it seem reasonable to add a new do_post()
     method to the vsn_hooks vtable?

2.  Providers need to access the MERGE request body.

     At the moment, mod_dav_svn is accessing the MERGE request body
     through a hack: it registers a custom input filter which captures
     the request body in an xml doc and stashes it away for later.
     This hack can be removed if we change vsn_hooks->merge() to accept
     an xml doc.  Everyone okay with that?

3.  It's awkward to create a lock.

     mod_dav first calls lock_hooks->create_lock(), which merely
     generates a lock-token UUID.  Then it turns around and calls
     lock_hooks->append_locks() to attach the lock to the resource.
     The vtable forces the provider to separate token-generation from
     the act of locking, which is a bit odd.  I'd like to add a new
     vtable function which does everything in one step: that is, allows
     the provider to generate the token and attach the lock all at
     once.  Sound good?

Reply | Threaded
Open this post in threaded view
|

Re: bumping mod_dav

gstein
On Thu, May 26, 2005 at 08:27:50PM -0500, Ben Collins-Sussman wrote:
>...
> OK... I'm working on the patches now.  They involve creating a
> dav_provider2 structure which will be registered as version "2" with
> ap_register_provider().  It means tweaking mod_dav machinery to accept
> both old and new providers.

Yup. And for providers to supply both (so they can interop with older
servers).

> Before I make the changes, though, I want to make sure folks are okay
> with the APIs I plan to change:
>
> 1.  We want mod_dav_svn to handle POST requests.
>
>     mod_dav currently passes REPORT requests to
>     provider->vsn_hooks->deliver_report().  But for POST, it merely
>     validates the resource and then DECLINES.  So while it's still
>     possible for a provider like mod_dav_svn to register itself to
>     handle POST requests (via normal apache hooks), it's really
>     inconvenient.  Does it seem reasonable to add a new do_post()
>     method to the vsn_hooks vtable?

Knee jerk is "handle it yourself". But the more considered response notes
that mapping from a URL to a DAV resource would be a pain without mod_dav
to assist.

However, the types of responses that could be performed from a POST are
all over the map. I'm a bit wary of a vtable hook that merely gateways an
arbitrary set of functionality. Further, it would not belong to the
vsn_hooks vtable -- that is for DeltaV stuff which POST is most decidedly
_not_.

My recommendation is to have dav_method_post() hang the dav_resource()
onto the Apache request resource (r->pool's user table). mod_dav_svn can
use standard Apache handling to process the POST, and can pick up the
dav_resource in its handler. Note that you will want to order the handlers
to ensure that mod_dav_svn runs *after* mod_dav.

> 2.  Providers need to access the MERGE request body.
>...
>     This hack can be removed if we change vsn_hooks->merge() to accept
>     an xml doc.  Everyone okay with that?

Definitely.

> 3.  It's awkward to create a lock.
>
>     mod_dav first calls lock_hooks->create_lock(), which merely
>     generates a lock-token UUID.  Then it turns around and calls
>     lock_hooks->append_locks() to attach the lock to the resource.
>     The vtable forces the provider to separate token-generation from
>     the act of locking, which is a bit odd.

Not odd at all -- just for Subversion since it does not have collection
locks. For a collection lock, mod_dav will create one lock structure, and
then recursively apply that lock to all resources in the collection.
(apply meaning "append" since a resource can have multiple, non-exclusive
locks on it)

Also note that the control flow is:

1. PROVIDER: create a basic lock structure
2. MOD_DAV: fill in lock structure with data from the LOCK req body
3. MOD_DAV: validate applicability of lock based on req body params
            (specifically, exclusive vs shared locks)
4. PROVIDER: apply the lock to relevant resources

You could probably parse the body up front, and pass that into the lock
creation. But the validation step must still happen.

>     I'd like to add a new
>     vtable function which does everything in one step: that is, allows
>     the provider to generate the token and attach the lock all at
>     once.  Sound good?

Does this strategy also apply to mod_dav_fs and mod_dav_lock? IOW, they
should use the same mechanism. I'm assuming they do not require the vtable
entries to be separate?

It seems like what you're planning on doing is have dav_method_lock
examine the depth of the lock. If it is depth 0 (a member resource, or a
locknull member/collection resource), then it could use your "all in one"
vtable entry. If it is depth infinity (for a collection; depth infinity on
a member must be ignored), then it uses the old entry points.

You're going to be doing a good bit of monkeying around with the control
flow to get this one-shot vtable entry to work. Doable, yes, but probably
not as straight-forward as you intended...

Cheers,
-g

--
Greg Stein, http://www.lyra.org/
Reply | Threaded
Open this post in threaded view
|

Re: bumping mod_dav

Ben Collins-Sussman

On May 26, 2005, at 10:11 PM, Greg Stein wrote:
>>
>> 1.  We want mod_dav_svn to handle POST requests.

> [...] I'm a bit wary of a vtable hook that merely gateways an
> arbitrary set of functionality. Further, it would not belong to the
> vsn_hooks vtable -- that is for DeltaV stuff which POST is most  
> decidedly
> _not_.

Yeah, you're right.  REPORT is there because it's deltaV.  POST isn't  
webdav or deltaV at all, it's just generic HTTP.  It's probably more  
'correct' to just have mod_dav_svn register it's own POST handler.

>
>> 3.  It's awkward to create a lock.
>>
>
> Not odd at all -- just for Subversion since it does not have  
> collection
> locks. For a collection lock, mod_dav will create one lock  
> structure, and
> then recursively apply that lock to all resources in the collection.
> (apply meaning "append" since a resource can have multiple, non-
> exclusive
> locks on it)

I think I disagree here, in general.  The current API is convenient  
for the way mod_dav_fs happens to implement recursive locking  
operations:  loop over a filesystem, attach the same lock to  
everything.  But for a provider that has a locking implementation  
*built into* the filesystem, it's much nicer to assume that the  
filesystem itself will handle the looping and attaching internally.

At this point, though, we've already warped svn_fs.h to match  
mod_dav, so it would be too late to get any real benefits from an  
alternate locking API anyway.


>
>> 2.  Providers need to access the MERGE request body.
>> ...
>>     This hack can be removed if we change vsn_hooks->merge() to  
>> accept
>>     an xml doc.  Everyone okay with that?
>>
>
> Definitely.
>

At this point, it seems like diminishing returns.  It's going to be  
quite a bit of work to add a whole versioning/compatibility layer to  
mod_dav... and for what?  Just so mod_dav_svn doesn't have to snarf a  
MERGE request body via input-filter?  It seems silly to bump the  
whole provider API for one tiny change to vsn_hooks->merge().

It's probably just best to table this proposal.  I'll add this MERGE  
change to a new queue of API changes, and someday when there's at  
least 3 or 4 changes queued up, I'll propose bumping mod_dav's API  
again.  It just doesn't seem worth it right now.

Carry on, nothing to see here...