Shared DAV protocol definition for custom HTTP REPORTs

I've added a new svn_dav_protocol.h to house some of the magic strings
used by ra_neon, ra_serf, and mod_dav_svn. In r25481, I've done
"mergeinfo-report" and its child XML elements as an example (shown
below).

I was wondering how people feel about the naming conventions used for
REPORT names and child XML elements. Specific concerns:

a) "SVN_DAV__" prefix is the opposite of what we use in some spots
inside of mod_dav_svn (where "dav_svn" is used, as mod_dav_svn is the
backend provider module for httpd's mod_dav).

b) Child XML elements use very generic names (e.g. SVN_DAV__PATH for
"path"), and don't use a prefix specific to their parent report.

To address part of (b), Erik suggested using a more specific namespace
(e.g. SVN_DAV_REPORT__PATH). Thoughts?

On Wed, 20 Jun 2007, dlr@tigris.org wrote:
...
> Create a home for the custom HTTP REPORTs used by Subversion's WebDAV
> protocol. This information is shared by ra_neon, ra_serf, and
> mod_dav_svn.
>
> Move #defines for the "mergeinfo-report" and its child XML elements
> into this new Subversion-private header file as an example/discussion
> piece.
>
> * subversion/include/private/svn_dav_protocol.h
> Add new header file.
>
> * subversion/libsvn_ra_serf/mergeinfo.c
> * subversion/libsvn_ra_neon/mergeinfo.c
> * subversion/mod_dav_svn/version.c
> * subversion/mod_dav_svn/dav_svn.h
> * subversion/mod_dav_svn/reports/mergeinfo.c
> Use #defines for "mergeinfo-report", "mergeinfo-item",
> "mergeinfo-path", "mergeinfo-info", "path", "inherit", and
> "revision".
...
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/private/svn_dav_protocol.h?pathrev=25481
> ==============================================================================
> --- (empty file)
> +++ trunk/subversion/include/private/svn_dav_protocol.h Wed Jun 20 15:54:31 2007
> @@ -0,0 +1,45 @@
> +/*
> + * svn_dav_protocol.h: Declarations of the protocol shared by the
> + * mod_dav_svn backend for httpd's mod_dav, and its ra_neon and
> + * ra_serf RA DAV clients.
> + *
> + * ====================================================================
> + * Copyright (c) 2007 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#ifndef SVN_DAV_PROTOCOL_H
> +#define SVN_DAV_PROTOCOL_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
> +
> +/** Names for the custom HTTP REPORTs understood by mod_dav_svn, sans
> + namespace. */
> +#define SVN_DAV__MERGEINFO_REPORT "mergeinfo-report"
> +
> +/** Names for XML child elements of the custom HTTP REPORTs understood
> + by mod_dav_svn, sans namespace. */
> +#define SVN_DAV__MERGEINFO_ITEM "mergeinfo-item"
> +#define SVN_DAV__MERGEINFO_PATH "mergeinfo-path"
> +#define SVN_DAV__MERGEINFO_INFO "mergeinfo-info"
> +#define SVN_DAV__PATH "path"
> +#define SVN_DAV__INHERIT "inherit"
> +#define SVN_DAV__REVISION "revision"
> +
> +#ifdef __cplusplus
> +}
> +#endif /* __cplusplus */
> +
> +#endif /* SVN_DAV_PROTOCOL_H */
>
> Modified: trunk/subversion/libsvn_ra_neon/mergeinfo.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_neon/mergeinfo.c?pathrev=25481&r1=25480&r2=25481
> ==============================================================================
> --- trunk/subversion/libsvn_ra_neon/mergeinfo.c (original)
> +++ trunk/subversion/libsvn_ra_neon/mergeinfo.c Wed Jun 20 15:54:31 2007
> @@ -30,6 +30,7 @@
> #include "svn_path.h"
> #include "svn_xml.h"
> #include "svn_mergeinfo.h"
> +#include "private/svn_dav_protocol.h"
> #include "../libsvn_ra/ra_loader.h"
>
> #include "ra_neon.h"
> @@ -50,7 +51,7 @@
>
> static const svn_ra_neon__xml_elm_t mergeinfo_report_elements[] =
> {
> - { SVN_XML_NAMESPACE, "mergeinfo-report", ELEM_mergeinfo_report, 0 },
> + { SVN_XML_NAMESPACE, SVN_DAV__MERGEINFO_REPORT, ELEM_mergeinfo_report, 0 },
> { SVN_XML_NAMESPACE, "mergeinfo-item", ELEM_mergeinfo_item, 0 },
> { SVN_XML_NAMESPACE, "mergeinfo-path", ELEM_mergeinfo_path,
> SVN_RA_NEON__XML_CDATA },
> @@ -164,10 +165,12 @@
> svn_stringbuf_t *request_body = svn_stringbuf_create("", pool);
> struct mergeinfo_baton mb;
>
> - static const char minfo_report_head[]
> - = "<S:mergeinfo-report xmlns:S=\"" SVN_XML_NAMESPACE "\">" DEBUG_CR;
> + static const char minfo_report_head[] =
> + "<S:" SVN_DAV__MERGEINFO_REPORT " xmlns:S=\"" SVN_XML_NAMESPACE "\">"
> + DEBUG_CR;
>
> - static const char minfo_report_tail[] = "</S:mergeinfo-report>" DEBUG_CR;
> + static const char minfo_report_tail[] =
> + "</S:" SVN_DAV__MERGEINFO_REPORT ">" DEBUG_CR;
>
> /* Construct the request body. */
> svn_stringbuf_appendcstr(request_body, minfo_report_head);
>
> Modified: trunk/subversion/libsvn_ra_serf/mergeinfo.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/mergeinfo.c?pathrev=25481&r1=25480&r2=25481
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/mergeinfo.c (original)
> +++ trunk/subversion/libsvn_ra_serf/mergeinfo.c Wed Jun 20 15:54:31 2007
> @@ -21,6 +21,7 @@
>
> #include "svn_ra.h"
> #include "svn_xml.h"
> +#include "private/svn_dav_protocol.h"
> #include "../libsvn_ra/ra_loader.h"
> #include "svn_private_config.h"
> #include "svn_mergeinfo.h"
> @@ -64,24 +65,24 @@
> mergeinfo_state_e state;
>
> state = parser->state->current_state;
> - if (state == NONE && strcmp(name.name, "mergeinfo-report") == 0)
> + if (state == NONE && strcmp(name.name, SVN_DAV__MERGEINFO_REPORT) == 0)
> {
> svn_ra_serf__xml_push_state(parser, MERGE_INFO_REPORT);
> }
> else if (state == MERGE_INFO_REPORT &&
> - strcmp(name.name, "mergeinfo-item") == 0)
> + strcmp(name.name, SVN_DAV__MERGEINFO_ITEM) == 0)
> {
> svn_ra_serf__xml_push_state(parser, MERGE_INFO_ITEM);
> mergeinfo_ctx->curr_path = NULL;
> svn_stringbuf_setempty(mergeinfo_ctx->curr_info);
> }
> else if (state == MERGE_INFO_ITEM &&
> - strcmp(name.name, "mergeinfo-path") == 0)
> + strcmp(name.name, SVN_DAV__MERGEINFO_PATH) == 0)
> {
> svn_ra_serf__xml_push_state(parser, MERGE_INFO_PATH);
> }
> else if (state == MERGE_INFO_ITEM &&
> - strcmp(name.name, "mergeinfo-info") == 0)
> + strcmp(name.name, SVN_DAV__MERGEINFO_INFO) == 0)
> {
> svn_ra_serf__xml_push_state(parser, MERGE_INFO_INFO);
> }
> @@ -98,12 +99,12 @@
> state = parser->state->current_state;
>
> if (state == MERGE_INFO_REPORT &&
> - strcmp(name.name, "mergeinfo-report") == 0)
> + strcmp(name.name, SVN_DAV__MERGEINFO_REPORT) == 0)
> {
> svn_ra_serf__xml_pop_state(parser);
> }
> else if (state == MERGE_INFO_ITEM
> - && strcmp(name.name, "mergeinfo-item") == 0)
> + && strcmp(name.name, SVN_DAV__MERGEINFO_ITEM) == 0)
> {
> if (mergeinfo_ctx->curr_info && mergeinfo_ctx->curr_path)
> {
> @@ -117,12 +118,12 @@
> svn_ra_serf__xml_pop_state(parser);
> }
> else if (state == MERGE_INFO_PATH
> - && strcmp(name.name, "mergeinfo-path") == 0)
> + && strcmp(name.name, SVN_DAV__MERGEINFO_PATH) == 0)
> {
> svn_ra_serf__xml_pop_state(parser);
> }
> else if (state == MERGE_INFO_INFO
> - && strcmp(name.name, "mergeinfo-info") == 0)
> + && strcmp(name.name, SVN_DAV__MERGEINFO_INFO) == 0)
> {
> svn_ra_serf__xml_pop_state(parser);
> }
> @@ -167,10 +168,11 @@
> apr_pool_t *pool)
> {
> svn_error_t *err;
> - static const char minfo_request_head[]
> - = "<S:mergeinfo-report xmlns:S=\"" SVN_XML_NAMESPACE "\">";
> + static const char minfo_request_head[] =
> + "<S:" SVN_DAV__MERGEINFO_REPORT " xmlns:S=\"" SVN_XML_NAMESPACE "\">";
>
> - static const char minfo_request_tail[] = "</S:mergeinfo-report>";
> + static const char minfo_request_tail[] =
> + "</S:" SVN_DAV__MERGEINFO_REPORT ">";
> int i;
>
> mergeinfo_context_t *mergeinfo_ctx;
> @@ -193,9 +195,10 @@
> serf_bucket_aggregate_append(buckets, tmp);
>
> svn_ra_serf__add_tag_buckets(buckets,
> - "S:revision", apr_ltoa(pool, revision),
> + "S:" SVN_DAV__REVISION,
> + apr_ltoa(pool, revision),
> session->bkt_alloc);
> - svn_ra_serf__add_tag_buckets(buckets, "S:inherit",
> + svn_ra_serf__add_tag_buckets(buckets, "S:" SVN_DAV__INHERIT,
> svn_inheritance_to_word(inherit),
> session->bkt_alloc);
> if (paths)
> @@ -205,7 +208,7 @@
> const char *this_path =
> apr_xml_quote_string(pool, APR_ARRAY_IDX(paths, i, const char *),
> 0);
> - svn_ra_serf__add_tag_buckets(buckets, "S:path",
> + svn_ra_serf__add_tag_buckets(buckets, "S:" SVN_DAV__PATH,
> this_path, session->bkt_alloc);
> }
> }
>
> Modified: trunk/subversion/mod_dav_svn/dav_svn.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/mod_dav_svn/dav_svn.h?pathrev=25481&r1=25480&r2=25481
> ==============================================================================
> --- trunk/subversion/mod_dav_svn/dav_svn.h (original)
> +++ trunk/subversion/mod_dav_svn/dav_svn.h Wed Jun 20 15:54:31 2007
> @@ -31,6 +31,7 @@
> #include "svn_repos.h"
> #include "svn_path.h"
> #include "svn_xml.h"
> +#include "private/svn_dav_protocol.h"
> #include "mod_authz_svn.h"
>
> #ifdef __cplusplus
> @@ -499,7 +500,7 @@
> { SVN_XML_NAMESPACE, "file-revs-report" },
> { SVN_XML_NAMESPACE, "get-locks-report" },
> { SVN_XML_NAMESPACE, "replay-report" },
> - { SVN_XML_NAMESPACE, "mergeinfo-report" },
> + { SVN_XML_NAMESPACE, SVN_DAV__MERGEINFO_REPORT },
> { NULL, NULL },
> };
>
>
> Modified: trunk/subversion/mod_dav_svn/reports/mergeinfo.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/mod_dav_svn/reports/mergeinfo.c?pathrev=25481&r1=25480&r2=25481
> ==============================================================================
> --- trunk/subversion/mod_dav_svn/reports/mergeinfo.c (original)
> +++ trunk/subversion/mod_dav_svn/reports/mergeinfo.c Wed Jun 20 15:54:31 2007
> @@ -30,6 +30,7 @@
> #include "svn_xml.h"
> #include "svn_path.h"
> #include "svn_dav.h"
> +#include "private/svn_dav_protocol.h"
>
> #include "../dav_svn.h"
>
> @@ -75,14 +76,14 @@
> if (child->ns != ns)
> continue;
>
> - if (strcmp(child->name, "revision") == 0)
> + if (strcmp(child->name, SVN_DAV__REVISION) == 0)
> rev = SVN_STR_TO_REV(dav_xml_get_cdata(child, resource->pool, 1));
> - else if (strcmp(child->name, "inherit") == 0)
> + else if (strcmp(child->name, SVN_DAV__INHERIT) == 0)
> {
> inherit = svn_inheritance_from_word(
> dav_xml_get_cdata(child, resource->pool, 1));
> }
> - else if (strcmp(child->name, "path") == 0)
> + else if (strcmp(child->name, SVN_DAV__PATH) == 0)
> {
> const char *target;
> const char *rel_path = dav_xml_get_cdata(child, resource->pool, 0);
> @@ -115,8 +116,9 @@
>
> serr = dav_svn__send_xml(bb, output,
> DAV_XML_HEADER DEBUG_CR
> - "<S:mergeinfo-report xmlns:S=\"" SVN_XML_NAMESPACE
> - "\" " "xmlns:D=\"DAV:\">" DEBUG_CR);
> + "<S:" SVN_DAV__MERGEINFO_REPORT " "
> + "xmlns:S=\"" SVN_XML_NAMESPACE "\" "
> + "xmlns:D=\"DAV:\">" DEBUG_CR);
> if (serr)
> {
> derr = dav_svn__convert_err(serr, HTTP_BAD_REQUEST, serr->message,
> @@ -134,10 +136,13 @@
> hi = apr_hash_next(hi))
> {
> const char *path, *info;
> - const char itemformat[] = "<S:mergeinfo-item>" DEBUG_CR
> - "<S:mergeinfo-path>%s</S:mergeinfo-path>" DEBUG_CR
> - "<S:mergeinfo-info>%s</S:mergeinfo-info>" DEBUG_CR
> - "</S:mergeinfo-item>";
> + const char itemformat[] = "<S:" SVN_DAV__MERGEINFO_ITEM ">"
> + DEBUG_CR
> + "<S:" SVN_DAV__MERGEINFO_PATH ">%s</S:" SVN_DAV__MERGEINFO_PATH ">"
> + DEBUG_CR
> + "<S:" SVN_DAV__MERGEINFO_INFO ">%s</S:" SVN_DAV__MERGEINFO_INFO ">"
> + DEBUG_CR
> + "</S:" SVN_DAV__MERGEINFO_ITEM ">";
>
> apr_hash_this(hi, &key, NULL, &value);
> path = key;
> @@ -157,7 +162,8 @@
> }
> }
>
> - if ((serr = dav_svn__send_xml(bb, output, "</S:mergeinfo-report>"
> + if ((serr = dav_svn__send_xml(bb, output,
> + "</S:" SVN_DAV__MERGEINFO_REPORT ">"
> DEBUG_CR)))
> {
> derr = dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
>
> Modified: trunk/subversion/mod_dav_svn/version.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/mod_dav_svn/version.c?pathrev=25481&r1=25480&r2=25481
> ==============================================================================
> --- trunk/subversion/mod_dav_svn/version.c (original)
> +++ trunk/subversion/mod_dav_svn/version.c Wed Jun 20 15:54:31 2007
> @@ -32,6 +32,7 @@
> #include "svn_props.h"
> #include "svn_dav.h"
> #include "svn_base64.h"
> +#include "private/svn_dav_protocol.h"
>
> #include "dav_svn.h"
>
> @@ -978,7 +979,7 @@
> {
> return dav_svn__replay_report(resource, doc, output);
> }
> - else if (strcmp(doc->root->name, "mergeinfo-report") == 0)
> + else if (strcmp(doc->root->name, SVN_DAV__MERGEINFO_REPORT) == 0)
> {
> return dav_svn__get_mergeinfo_report(resource, doc, output);
> }
...