> -----Original Message----- > From: Jon Foster [mailto:jon.fos...@cabot.co.uk] > Sent: maandag 20 september 2010 12:01 > To: Daniel Shahaf; Subversion Development > Subject: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code > > Hi, > > For atomic-revprop, the client needs to know if the error was > SVN_ERR_BAD_OLD_VALUE or not. For ra_local and ra_svn this is > already done; this patch does it for DAV (with Neon). > > > With mod_dav, we only have 2 ways to affect the 207 multistatus > response from a failed PROPPATCH: > > - We can set the text that appears in <D:responsedescription>, > including injecting arbitrary XML. > - We can set the <D:status> value to a (numeric) HTTP error code. > > The approach that's been discussed on-list and on IRC was to > inject the error as XML inside <D:responsedescription>. I did > actually implement that, only to discover that Neon completely > rejects the XML in that case[1]. While we would obviously fix > this in the 1.7 client code, it could break compatibility between > the 1.7 server and 1.6 and older clients. The only way to handle > this would be to introduce a new client capability, so the server > could fall back to the old code for 1.6 clients. This means we'd > have two code paths for error-handling... this gets very complex. > > > However, there is a simpler way! The <D:status> element contains > the HTTP error code, usually "500 Internal Server Error". So we > could pick a special HTTP status code to mean SVN_ERR_BAD_OLD_VALUE. > I think of old_value_p as a precondition for the operation, a bit > like "If-Modified-Since:", so I'd suggest "412 Precondition Failed". > Note that generic DAV clients won't ever see this message, because > they won't be sending old_value_p.
Nice and simple solution to this issue :) I think 'SVN_ERR_BAD_OLD_VALUE' could use a better/more generic name though. ('Old value' is only valid in the specific function context. Maybe extend this to something more like this 'precondition failed') Bert