> -----Original Message-----
> From: Jon Foster [mailto:Jon.Foster_at_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
Received on 2010-09-20 12:18:10 CEST