You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2011/03/04 13:29:10 UTC
[PATCH v2] HTTPv2 allow client to control transaction name in protocol
"C. Michael Pilato" <cm...@collab.net> writes:
> Just a thought: Have you considered expanding the scope of the private
> resource space rather than using the magic prefix hack? You could add
> ".../!svn/vtxn/UUID" and ".../!svn/vtxr/UUID/..." to be alternate ways to
> address transactions and transaction roots (the "v" there being a shortcut
> for "virtual"). This is *effectively* the same approach as yours -- there's
> a different prefix here. But the prefix is a clearly defined piece of the
> protocol, not just some magic bit buried in mod_dav_svn's codebase.
This looks promising, it's quite straightforward to implement. New
log/patch follows:
Extend Subversion's v2 HTTP protocol to include URIs that allow the
client to define the transaction name visible in on the wire.
If the client sends, or a proxy injects, an SVN-VTxn-Name header with
the POST request it defines the transaction name to be returned to the
client in the POST response. If the client recieves the new
SVN-VTxn-Name header it uses that name in the new URIs in the requests
that make up the commit.
By default the client will not send the new header.
* subversion/mod_dav_svn/dav_svn.h
(struct dav_svn_root): Add vtxn_name member.
(dav_svn__get_vtxn_stub, dav_svn__get_vtxn_root_stub): New.
* subversion/mod_dav_svn/mod_dav_svn.c
(dav_svn__get_vtxn_stub, dav_svn__get_vtxn_root_stub): New.
* subversion/mod_dav_svn/posts/create_txn.c
(dav_svn__post_create_txn): Get vtxn_name from header, if sent, and
store vtxn_name:txn_name mapping in the activity database.
* subversion/mod_dav_svn/version.c
(get_option): Add vtxn stub and vtxn root stub headers.
(merge): Delete visible_txn_name:txn_name mapping after commit.
* subversion/mod_dav_svn/repos.c
(parse_vtxnstub_uri, parse_vtxnroot_uri): New.
(special_subdirs): Add vtxn and vtxr.
(remove_resource): Delete mapping when aborting the transaction.
* subversion/libsvn_ra_serf/ra_serf.h
(struct svn_ra_serf__session_t): Add vtxn_stub and vtxn_root_stub.
* subversion/libsvn_ra_serf/commit.c
(setup_post_headers): New.
(post_headers_iterator_callback): Handle vtxn name header.
(open_root): Set header_delegate.
* subversion/libsvn_ra_serf/options.c
(capabilities_headers_iterator_callback): Support vtxn stub and
vtxn root stub headers.
* subversion/libsvn_ra_neon/options.c
(parse_capabilities): Support vtxn stub and vtxn root stub headers.
* subversion/include/svn_dav.h
(SVN_DAV_VTXN_STUB_HEADER, SVN_DAV_VTXN_ROOT_STUB_HEADER,
SVN_DAV_VTXN_NAME_HEADER): New.
* notes/http-and-webdav/http-protocol-v2.txt: Update.
Index: notes/http-and-webdav/http-protocol-v2.txt
===================================================================
--- notes/http-and-webdav/http-protocol-v2.txt (revision 1077861)
+++ notes/http-and-webdav/http-protocol-v2.txt (working copy)
@@ -128,6 +128,14 @@
Various read- and write-type requests can be issued against
these resources (MKCOL, PUT, PROPFIND, PROPPATCH, GET, etc.).
+ - alternate transaction resource (!svn/vtxn/VTXN-NAME)
+ alternate transaction root resource (!svn/vtxr/VTXN-NAME/[PATH])
+
+ Alternative names for the transaction based on a virtual, or
+ visible, name supplied by the client when the transaction
+ was created. The client supplied name is optional, if not
+ supplied these resource names are not valid.
+
* Opening an RA session:
ra_serf will send an OPTIONS request when creating a new
@@ -220,12 +228,17 @@
- no more need to "discover" the activity URI; !svn/act/ is gone.
- - client no longer creates an activity UUID itself.
+ - client no longer needs to create an activity UUID itself.
- instead, POST returns the name of the transaction it created,
- which can then be appended to the transaction stub and
- transaction root stub as necessary.
+ as TXN-NAME, which can then be appended to the transaction
+ stub and transaction root stub as necessary.
+ - if the client does choose to supply a UUID with the POST
+ request then the POST returns that UUID as VTXN-NAME, and the
+ client then uses that with the alternate transaction stub and
+ transaction root stub in subsequent requests.
+
- Once the commit transaction is created, the client is free to
send write requests against transaction resources it constructs
itself. This eliminates the CHECKOUT requests, and also
Index: subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- subversion/mod_dav_svn/mod_dav_svn.c (revision 1077861)
+++ subversion/mod_dav_svn/mod_dav_svn.c (working copy)
@@ -626,6 +626,22 @@
}
+const char *
+dav_svn__get_vtxn_stub(request_rec *r)
+{
+ return apr_pstrcat(r->pool, dav_svn__get_special_uri(r), "/vtxn",
+ (char *)NULL);
+}
+
+
+const char *
+dav_svn__get_vtxn_root_stub(request_rec *r)
+{
+ return apr_pstrcat(r->pool, dav_svn__get_special_uri(r), "/vtxr",
+ (char *)NULL);
+}
+
+
svn_boolean_t
dav_svn__get_autoversioning_flag(request_rec *r)
{
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h (revision 1077861)
+++ subversion/mod_dav_svn/dav_svn.h (working copy)
@@ -202,6 +202,15 @@
*/
const char *txn_name;
+ /* The optional vtxn name supplied by an HTTPv2 client and
+ used in subsequent requests. This may be NULL if the client
+ is not using a vtxn name.
+
+ PRIVATE resources that directly represent either a txn or
+ txn-root use this field.
+ */
+ const char *vtxn_name;
+
/* If the root is part of a transaction, this contains the FS's transaction
handle. It may be NULL if this root corresponds to a specific revision.
It may also be NULL if we have not opened the transaction yet.
@@ -392,7 +401,13 @@
/* For accessing transaction properties (typically "!svn/txr") */
const char *dav_svn__get_txn_root_stub(request_rec *r);
+/* For accessing transaction resources (typically "!svn/vtxn") */
+const char *dav_svn__get_vtxn_stub(request_rec *r);
+/* For accessing transaction properties (typically "!svn/vtxr") */
+const char *dav_svn__get_vtxn_root_stub(request_rec *r);
+
+
/*** activity.c ***/
/* Create a new transaction based on HEAD in REPOS, setting *PTXN_NAME
Index: subversion/mod_dav_svn/version.c
===================================================================
--- subversion/mod_dav_svn/version.c (revision 1077861)
+++ subversion/mod_dav_svn/version.c (working copy)
@@ -250,6 +250,12 @@
apr_table_set(r->headers_out, SVN_DAV_TXN_STUB_HEADER,
apr_pstrcat(resource->pool, repos_root_uri, "/",
dav_svn__get_txn_stub(r), (char *)NULL));
+ apr_table_set(r->headers_out, SVN_DAV_VTXN_ROOT_STUB_HEADER,
+ apr_pstrcat(resource->pool, repos_root_uri, "/",
+ dav_svn__get_vtxn_root_stub(r), (char *)NULL));
+ apr_table_set(r->headers_out, SVN_DAV_VTXN_STUB_HEADER,
+ apr_pstrcat(resource->pool, repos_root_uri, "/",
+ dav_svn__get_vtxn_stub(r), (char *)NULL));
}
return NULL;
@@ -1427,6 +1433,12 @@
svn_error_clear(serr);
serr = SVN_NO_ERROR;
}
+
+ /* HTTPv2 doesn't send DELETE after a successful MERGE so if
+ using the optional vtxn name mapping then delete it here. */
+ if (source->info->root.vtxn_name)
+ dav_svn__delete_activity(source->info->repos,
+ source->info->root.vtxn_name);
}
else
{
Index: subversion/mod_dav_svn/posts/create_txn.c
===================================================================
--- subversion/mod_dav_svn/posts/create_txn.c (revision 1077861)
+++ subversion/mod_dav_svn/posts/create_txn.c (working copy)
@@ -37,6 +37,7 @@
ap_filter_t *output)
{
const char *txn_name;
+ const char *vtxn_name;
dav_error *derr;
request_rec *r = resource->info->r;
@@ -47,7 +48,20 @@
/* Build a "201 Created" response with header that tells the
client our new transaction's name. */
- apr_table_set(r->headers_out, SVN_DAV_TXN_NAME_HEADER, txn_name);
+ vtxn_name = apr_table_get(r->headers_in, SVN_DAV_VTXN_NAME_HEADER);
+ if (vtxn_name && vtxn_name[0])
+ {
+ /* If the client supplied a vtxn name then store a mapping from
+ the client name to the FS transaction name in the activity
+ database. */
+ if ((derr = dav_svn__store_activity(resource->info->repos,
+ vtxn_name, txn_name)))
+ return derr;
+ apr_table_set(r->headers_out, SVN_DAV_VTXN_NAME_HEADER, vtxn_name);
+ }
+ else
+ apr_table_set(r->headers_out, SVN_DAV_TXN_NAME_HEADER, txn_name);
+
r->status = HTTP_CREATED;
return NULL;
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 1077861)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -491,7 +491,25 @@
return FALSE;
}
+static int
+parse_vtxnstub_uri(dav_resource_combined *comb,
+ const char *path,
+ const char *label,
+ int use_checked_in)
+{
+ /* format: !svn/vtxn/TXN_NAME */
+ if (parse_txnstub_uri(comb, path, label, use_checked_in))
+ return TRUE;
+
+ comb->priv.root.vtxn_name = comb->priv.root.txn_name;
+ comb->priv.root.txn_name = dav_svn__get_txn(comb->priv.repos,
+ comb->priv.root.vtxn_name);
+
+ return FALSE;
+}
+
+
static int
parse_txnroot_uri(dav_resource_combined *comb,
const char *path,
@@ -541,7 +559,25 @@
return FALSE;
}
+static int
+parse_vtxnroot_uri(dav_resource_combined *comb,
+ const char *path,
+ const char *label,
+ int use_checked_in)
+{
+ /* format: !svn/vtxr/TXN_NAME/[PATH] */
+ if (parse_txnroot_uri(comb, path, label, use_checked_in))
+ return TRUE;
+
+ comb->priv.root.vtxn_name = comb->priv.root.txn_name;
+ comb->priv.root.txn_name = dav_svn__get_txn(comb->priv.repos,
+ comb->priv.root.vtxn_name);
+
+ return FALSE;
+}
+
+
static int
parse_wrk_baseline_uri(dav_resource_combined *comb,
const char *path,
@@ -621,6 +657,8 @@
{ "rvr", parse_revroot_uri, 1, TRUE, DAV_SVN_RESTYPE_REVROOT_COLLECTION },
{ "txn", parse_txnstub_uri, 1, FALSE, DAV_SVN_RESTYPE_TXN_COLLECTION},
{ "txr", parse_txnroot_uri, 1, TRUE, DAV_SVN_RESTYPE_TXNROOT_COLLECTION},
+ { "vtxn", parse_vtxnstub_uri, 1, FALSE, DAV_SVN_RESTYPE_TXN_COLLECTION},
+ { "vtxr", parse_vtxnroot_uri, 1, TRUE, DAV_SVN_RESTYPE_TXNROOT_COLLECTION},
{ NULL } /* sentinel */
};
@@ -3783,11 +3821,13 @@
if (resource->type == DAV_RESOURCE_TYPE_PRIVATE
&& resource->info->restype == DAV_SVN_RESTYPE_TXN_COLLECTION)
{
- /* We'll assume that no activity was created to map to this
- transaction. */
- return dav_svn__abort_txn(resource->info->repos,
- resource->info->root.txn_name,
- resource->pool);
+ if (resource->info->root.vtxn_name)
+ return dav_svn__delete_activity(resource->info->repos,
+ resource->info->root.vtxn_name);
+ else
+ return dav_svn__abort_txn(resource->info->repos,
+ resource->info->root.txn_name,
+ resource->pool);
}
/* ### note that the parent was checked out at some point, and this
Index: subversion/include/svn_dav.h
===================================================================
--- subversion/include/svn_dav.h (revision 1077861)
+++ subversion/include/svn_dav.h (working copy)
@@ -133,6 +133,11 @@
* from a "txn root"). (HTTP protocol v2 only) */
#define SVN_DAV_TXN_STUB_HEADER "SVN-Txn-Stub"
+/** Companion to @c SVN_DAV_TXN_STUB_HEADER, used when a POST request
+ * returns @c SVN_DAV_VTXN_NAME_HEADER in response to a client
+ * supplied name. (HTTP protocol v2 only) */
+#define SVN_DAV_VTXN_STUB_HEADER "SVN-VTxn-Stub"
+
/** This header provides an opaque URI which represents the root
* directory of a Subversion transaction (revision-in-progress),
* similar to the concept of a "txn root" in the libsvn_fs API. The
@@ -141,6 +146,11 @@
* protocol v2 only) */
#define SVN_DAV_TXN_ROOT_STUB_HEADER "SVN-Txn-Root-Stub"
+/** Companion to @c SVN_DAV_TXN_ROOT_STUB_HEADER, used when a POST
+ * request returns @c SVN_DAV_VTXN_NAME_HEADER in response to a
+ * client supplied name. (HTTP protocol v2 only) */
+#define SVN_DAV_VTXN_ROOT_STUB_HEADER "SVN-VTxn-Root-Stub"
+
/** This header is used in the POST response to tell the client the
* name of the Subversion transaction created by the request. It can
* then be appended to the transaction stub and transaction root stub
@@ -148,6 +158,12 @@
* transaction. (HTTP protocol v2 only) */
#define SVN_DAV_TXN_NAME_HEADER "SVN-Txn-Name"
+/** This header is used in the POST request, to pass a client supplied
+ * alternative transaction name to the server, and in the the POST
+ * response, to tell the client that the alternative transaction
+ * resource names should be used. (HTTP protocol v2 only) */
+#define SVN_DAV_VTXN_NAME_HEADER "SVN-VTxn-Name"
+
/**
* @name Fulltext MD5 headers
*
Index: subversion/libsvn_ra_neon/ra_neon.h
===================================================================
--- subversion/libsvn_ra_neon/ra_neon.h (revision 1077861)
+++ subversion/libsvn_ra_neon/ra_neon.h (working copy)
@@ -146,6 +146,8 @@
const char *rev_root_stub; /* for accessing REV/PATH pairs */
const char *txn_stub; /* for accessing transactions (i.e. txnprops) */
const char *txn_root_stub; /* for accessing TXN/PATH pairs */
+ const char *vtxn_stub; /* for accessing transactions (i.e. txnprops) */
+ const char *vtxn_root_stub; /* for accessing TXN/PATH pairs */
/*** End HTTP v2 stuff ***/
Index: subversion/libsvn_ra_neon/options.c
===================================================================
--- subversion/libsvn_ra_neon/options.c (revision 1077861)
+++ subversion/libsvn_ra_neon/options.c (working copy)
@@ -253,6 +253,14 @@
{
ras->txn_stub = apr_pstrdup(ras->pool, val);
}
+ if ((val = ne_get_response_header(req, SVN_DAV_VTXN_ROOT_STUB_HEADER)))
+ {
+ ras->vtxn_root_stub = apr_pstrdup(ras->pool, val);
+ }
+ if ((val = ne_get_response_header(req, SVN_DAV_VTXN_STUB_HEADER)))
+ {
+ ras->vtxn_stub = apr_pstrdup(ras->pool, val);
+ }
}
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 1077861)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -195,6 +195,8 @@
const char *rev_root_stub; /* for accessing REV/PATH pairs */
const char *txn_stub; /* for accessing transactions (i.e. txnprops) */
const char *txn_root_stub; /* for accessing TXN/PATH pairs */
+ const char *vtxn_stub; /* for accessing transactions (i.e. txnprops) */
+ const char *vtxn_root_stub; /* for accessing TXN/PATH pairs */
/*** End HTTP v2 stuff ***/
Index: subversion/libsvn_ra_serf/commit.c
===================================================================
--- subversion/libsvn_ra_serf/commit.c (revision 1077861)
+++ subversion/libsvn_ra_serf/commit.c (working copy)
@@ -1141,6 +1141,21 @@
}
static svn_error_t *
+setup_post_headers(serf_bucket_t *headers,
+ void *baton,
+ apr_pool_t *pool)
+{
+#ifdef SVN_SERF_SEND_VTXN_NAME
+ /* Enable this to exercise the VTXN-NAME code based on a client
+ supplied transaction name. */
+ serf_bucket_headers_set(headers, SVN_DAV_VTXN_NAME_HEADER,
+ svn_uuid_generate(pool));
+#endif
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
setup_delete_headers(serf_bucket_t *headers,
void *baton,
apr_pool_t *pool)
@@ -1254,6 +1269,18 @@
prc_cc->txn_root_url =
svn_path_url_add_component2(sess->txn_root_stub, val, prc_cc->pool);
}
+
+ if (svn_cstring_casecmp(key, SVN_DAV_VTXN_NAME_HEADER) == 0)
+ {
+ /* Build out vtxn and vtxn-root URLs using the vtxn name we're
+ given, and store the whole lot of it in the commit context. */
+ prc_cc->txn_name = apr_pstrdup(prc_cc->pool, val);
+ prc_cc->txn_url =
+ svn_path_url_add_component2(sess->vtxn_stub, val, prc_cc->pool);
+ prc_cc->txn_root_url =
+ svn_path_url_add_component2(sess->vtxn_root_stub, val, prc_cc->pool);
+ }
+
return 0;
}
@@ -1308,6 +1335,8 @@
handler->body_type = SVN_SKEL_MIME_TYPE;
handler->body_delegate = create_txn_post_body;
handler->body_delegate_baton = NULL;
+ handler->header_delegate = setup_post_headers;
+ handler->header_delegate_baton = NULL;
handler->path = ctx->session->me_resource;
handler->conn = ctx->session->conns[0];
handler->session = ctx->session;
Index: subversion/libsvn_ra_serf/options.c
===================================================================
--- subversion/libsvn_ra_serf/options.c (revision 1077861)
+++ subversion/libsvn_ra_serf/options.c (working copy)
@@ -373,6 +373,14 @@
{
orc->session->txn_root_stub = apr_pstrdup(orc->session->pool, val);
}
+ else if (svn_cstring_casecmp(key, SVN_DAV_VTXN_STUB_HEADER) == 0)
+ {
+ orc->session->vtxn_stub = apr_pstrdup(orc->session->pool, val);
+ }
+ else if (svn_cstring_casecmp(key, SVN_DAV_VTXN_ROOT_STUB_HEADER) == 0)
+ {
+ orc->session->vtxn_root_stub = apr_pstrdup(orc->session->pool, val);
+ }
else if (svn_cstring_casecmp(key, SVN_DAV_REPOS_UUID_HEADER) == 0)
{
orc->session->uuid = apr_pstrdup(orc->session->pool, val);
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> Wait a minute. The svn client won't ever send this. So I'm not sure
> what you mean by "client send path" here, or "requiring ... the use of
> a proxy".
>
> Your request for this feature is entirely predicated on a proxy. So of
> course you need a proxy. And the client is never sending this header.
By default a standard Subversion client won't send the header, but any
client could send it -- it's part of the protocol. The patch includes
some conditional code that causes the svn client to send the header.
That code is not normally compiled, but if it is compiled it allows
testing both the server behaviour to VTXN-NAME, and the svn client
behaviour when it receives VTXN-NAME, without requiring a proxy that
adds VTXN-NAME.
The patch has also been tested within WANdisco using their proxy.
>> And it may be more convenient for the proxy as well, but I've not looked
>> at any proxy implementations so I can't say for certain.
>
> What?! You must be looking at a proxy implementation. That is why you
> want this feature.
Not me, other people do that. People who work on WANdisco's proxy
described the problem in terms of the HTTP protocol.
As far as I am concerned it is more convenient for the server to return
the header, as I can test using a small amount of extra code to send
VTXN-NAME. If the server doesn't return the header then I need more
test code.
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Mar 9, 2011 at 06:43, Philip Martin <ph...@wandisco.com> wrote:
> Philip Martin <ph...@wandisco.com> writes:
>
>> Greg Stein <gs...@gmail.com> writes:
>>
>>> The proxy is the one altering the request/response flow. From the
>>> server's standpoint, the client (in this case, the proxy) already
>>> knows the name. Why should the server tell it what it already knows?
>>> Why should it put that useless information onto the wire?
>>
>> It's convenient to implement it that way, a very simple change in the
>> client send path allows testing the client recieve path. If the server
>> doesn't send the header then testing will require a more complicated
>> client change, or the use of a proxy that sends the header.
Wait a minute. The svn client won't ever send this. So I'm not sure
what you mean by "client send path" here, or "requiring ... the use of
a proxy".
Your request for this feature is entirely predicated on a proxy. So of
course you need a proxy. And the client is never sending this header.
> And it may be more convenient for the proxy as well, but I've not looked
> at any proxy implementations so I can't say for certain.
What?! You must be looking at a proxy implementation. That is why you
want this feature.
I don't get it. First you mention "a proxy wants to do this, so I need
<this>", and then you say "but I haven't looked at proxies".
This seems inconsistent.
-g
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Greg Stein <gs...@gmail.com> writes:
>
>> The proxy is the one altering the request/response flow. From the
>> server's standpoint, the client (in this case, the proxy) already
>> knows the name. Why should the server tell it what it already knows?
>> Why should it put that useless information onto the wire?
>
> It's convenient to implement it that way, a very simple change in the
> client send path allows testing the client recieve path. If the server
> doesn't send the header then testing will require a more complicated
> client change, or the use of a proxy that sends the header.
And it may be more convenient for the proxy as well, but I've not looked
at any proxy implementations so I can't say for certain.
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> The proxy is the one altering the request/response flow. From the
> server's standpoint, the client (in this case, the proxy) already
> knows the name. Why should the server tell it what it already knows?
> Why should it put that useless information onto the wire?
It's convenient to implement it that way, a very simple change in the
client send path allows testing the client recieve path. If the server
doesn't send the header then testing will require a more complicated
client change, or the use of a proxy that sends the header.
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Mar 9, 2011 at 04:40, Philip Martin <ph...@wandisco.com> wrote:
> Greg Stein <gs...@gmail.com> writes:
>...
>> Basically, the server is responding with somebody the requestor
>> already knows. So I wonder which approach is "best". It seems to be
>> kinda six-of-one/half-dozen-of-another. I suspect the server just
>> needs an if/else, so that might not be nearly the burden relative
>> modifying the proxy to add that header.
>
> The server needs an "if" in order to avoid sending TXN-NAME when it
> receives VTXN-NAME (and not sending TXN-NAME is the main reason for this
> change). If the server doesn't send VTXN-NAME then the response is not
> complete and the server is relying on the the proxy to make it complete.
> I think it is better if the server generates a complete response.
The proxy is the one altering the request/response flow. From the
server's standpoint, the client (in this case, the proxy) already
knows the name. Why should the server tell it what it already knows?
Why should it put that useless information onto the wire?
Cheers,
-g
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> On Tue, Mar 8, 2011 at 11:08, Philip Martin <ph...@wandisco.com> wrote:
>> VTXN operation:
>>
>> - client sends POST
>> - proxy adds SVN-VTxn-Name:UUID
>> - server creates transaction called TXN-NAME
>> - server replies SVN-VTxn-Name:UUID
>> - proxy passes
>
> or:
>
> - proxy adds: SVN-VTxn-Name:UUID
>
>> - client sends !svn/vtxn/UUID
>> - proxy passes
>> - server extracts UUID and maps to TXN-NAME
>
> So my question is whether to have the server do it, or have the proxy do it.
>
> Basically, the server is responding with somebody the requestor
> already knows. So I wonder which approach is "best". It seems to be
> kinda six-of-one/half-dozen-of-another. I suspect the server just
> needs an if/else, so that might not be nearly the burden relative
> modifying the proxy to add that header.
The server needs an "if" in order to avoid sending TXN-NAME when it
receives VTXN-NAME (and not sending TXN-NAME is the main reason for this
change). If the server doesn't send VTXN-NAME then the response is not
complete and the server is relying on the the proxy to make it complete.
I think it is better if the server generates a complete response.
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 8, 2011 at 11:08, Philip Martin <ph...@wandisco.com> wrote:
> Greg Stein <gs...@gmail.com> writes:
>
>> And to be clear: the server *could* just remain silent, and the proxy
>> would insert the SVN-VTxn-Name header in the response back to the
>> client, right? Would that be an improvement/simplification?
>
> I don't think so.
>
> Normal operation:
>
> - client sends POST
> - server creates transaction called TXN-NAME
> - server replies SVN-Txn-Name:TXN-NAME
> - client send !svn/txn/TXN-NAME
> - server extracts TXN-NAME
>
> VTXN operation:
>
> - client sends POST
> - proxy adds SVN-VTxn-Name:UUID
> - server creates transaction called TXN-NAME
> - server replies SVN-VTxn-Name:UUID
> - proxy passes
or:
- proxy adds: SVN-VTxn-Name:UUID
> - client sends !svn/vtxn/UUID
> - proxy passes
> - server extracts UUID and maps to TXN-NAME
So my question is whether to have the server do it, or have the proxy do it.
Basically, the server is responding with somebody the requestor
already knows. So I wonder which approach is "best". It seems to be
kinda six-of-one/half-dozen-of-another. I suspect the server just
needs an if/else, so that might not be nearly the burden relative
modifying the proxy to add that header.
Cheers,
-g
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> And to be clear: the server *could* just remain silent, and the proxy
> would insert the SVN-VTxn-Name header in the response back to the
> client, right? Would that be an improvement/simplification?
I don't think so.
Normal operation:
- client sends POST
- server creates transaction called TXN-NAME
- server replies SVN-Txn-Name:TXN-NAME
- client send !svn/txn/TXN-NAME
- server extracts TXN-NAME
VTXN operation:
- client sends POST
- proxy adds SVN-VTxn-Name:UUID
- server creates transaction called TXN-NAME
- server replies SVN-VTxn-Name:UUID
- proxy passes
- client sends !svn/vtxn/UUID
- proxy passes
- server extracts UUID and maps to TXN-NAME
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Greg Stein <gs...@gmail.com>.
On Sat, Mar 5, 2011 at 13:16, Philip Martin <ph...@wandisco.com> wrote:
> Greg Stein <gs...@gmail.com> writes:
>
>>> If the client sends, or a proxy injects, an SVN-VTxn-Name header with
>>> the POST request it defines the transaction name to be returned to the
>>> client in the POST response. If the client recieves the new
>>> SVN-VTxn-Name header it uses that name in the new URIs in the requests
>>> that make up the commit.
>>
>> I don't understand why the *client* needs to read that header. The
>> base URI that the server returns already has the proper txn name,
>> right?
>>
>> Sending *to* the server creates the "client-provided" feature that you
>> want to retain. I just don't understand the other direction.
>
> By default the client doesn't send the header, it receives SVN-Txn-Name
> containing the server generated name and uses that in the txn/txr URIs.
> A client could send SVN-VTxn-Name but most clients will not. If a proxy
> inserts an SVN-VTxn-Name header into the request, the server replies
> with an SVN-VTxn-Name header containing the proxy generated name. The
> client then receives SVN-VTxn-Name containing the proxy generated name
> and uses that with the vtxn/vtxr URIs.
Ah. Gotcha. Thanks.
And to be clear: the server *could* just remain silent, and the proxy
would insert the SVN-VTxn-Name header in the response back to the
client, right? Would that be an improvement/simplification?
Cheers,
-g
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
>> If the client sends, or a proxy injects, an SVN-VTxn-Name header with
>> the POST request it defines the transaction name to be returned to the
>> client in the POST response. If the client recieves the new
>> SVN-VTxn-Name header it uses that name in the new URIs in the requests
>> that make up the commit.
>
> I don't understand why the *client* needs to read that header. The
> base URI that the server returns already has the proper txn name,
> right?
>
> Sending *to* the server creates the "client-provided" feature that you
> want to retain. I just don't understand the other direction.
By default the client doesn't send the header, it receives SVN-Txn-Name
containing the server generated name and uses that in the txn/txr URIs.
A client could send SVN-VTxn-Name but most clients will not. If a proxy
inserts an SVN-VTxn-Name header into the request, the server replies
with an SVN-VTxn-Name header containing the proxy generated name. The
client then receives SVN-VTxn-Name containing the proxy generated name
and uses that with the vtxn/vtxr URIs.
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Mar 4, 2011 at 07:29, Philip Martin <ph...@wandisco.com> wrote:
>...
> Extend Subversion's v2 HTTP protocol to include URIs that allow the
> client to define the transaction name visible in on the wire.
>
> If the client sends, or a proxy injects, an SVN-VTxn-Name header with
> the POST request it defines the transaction name to be returned to the
> client in the POST response. If the client recieves the new
> SVN-VTxn-Name header it uses that name in the new URIs in the requests
> that make up the commit.
I don't understand why the *client* needs to read that header. The
base URI that the server returns already has the proper txn name,
right?
Sending *to* the server creates the "client-provided" feature that you
want to retain. I just don't understand the other direction.
Cheers,
-g
Re: [PATCH v2] HTTPv2 allow client to control transaction name in
protocol
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 03/04/2011 07:29 AM, Philip Martin wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>
>> Just a thought: Have you considered expanding the scope of the private
>> resource space rather than using the magic prefix hack? You could add
>> ".../!svn/vtxn/UUID" and ".../!svn/vtxr/UUID/..." to be alternate ways to
>> address transactions and transaction roots (the "v" there being a shortcut
>> for "virtual"). This is *effectively* the same approach as yours -- there's
>> a different prefix here. But the prefix is a clearly defined piece of the
>> protocol, not just some magic bit buried in mod_dav_svn's codebase.
>
> This looks promising, it's quite straightforward to implement. New
> log/patch follows:
Oh, that's *so* much more satisfying. Most of this kind of work is
copy-n-paste-n-tweak, I know, but I gave it a once-over anyway. Looks good
to me!
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH v2] HTTPv2 allow client to control transaction name in
protocol
Posted by Julian Foad <ju...@wandisco.com>.
Philip Martin wrote:
> Julian Foad <ju...@wandisco.com> writes:
>
> > One thing that's not 100% clear from the protocol doc update is whether
> > the server sends *both* txn names in response, or just the "V" version.
> > If it sends both, then we need to specify whether the client has to use
> > the "V" version or can choose to use either one, or can mix accesses
> > arbitrarily using either. I can't think of any reason the server would
> > need to send both, so can we keep things simple by specifying that it
> > doesn't?
>
> The server sends one or the other, not both.
Good.
> > Additionally, this response will contain some new URL stub values:
> >
> > SVN-Rev-Stub: /REPOS-ROOT/!svn/rev
> > SVN-Rev-Root-Stub: /REPOS-ROOT/!svn/rvr
> > SVN-Txn-Stub: /REPOS-ROOT/!svn/txn
> > SVN-Txn-Root-Stub: /REPOS-ROOT/!svn/txr
> >
> > Should it send "vtxn" and "vtxr" stubs too, or instead?
>
> Those are not in the POST response.
Yes - that's the OPTIONS response.
> The server already sends SVN-Txn-Stub and SVN-Txn-Root-Stub during
> capabilities negotiation, the patch makes it send SVN-VTxn-Stub and
> SVN-VTxn-Root stub as well.
OK, good. Just doc tweaks then.
Thanks.
- Julian
> > (I don't
> > understand why the protocol needs to send these stubs explicitly at all:
> > is there some reason why these cannot just be constructed by the
> > client?)
>
Re: [PATCH v2] HTTPv2 allow client to control transaction name in protocol
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@wandisco.com> writes:
> One thing that's not 100% clear from the protocol doc update is whether
> the server sends *both* txn names in response, or just the "V" version.
> If it sends both, then we need to specify whether the client has to use
> the "V" version or can choose to use either one, or can mix accesses
> arbitrarily using either. I can't think of any reason the server would
> need to send both, so can we keep things simple by specifying that it
> doesn't?
The server sends one or the other, not both.
> Additionally, this response will contain some new URL stub values:
>
> SVN-Rev-Stub: /REPOS-ROOT/!svn/rev
> SVN-Rev-Root-Stub: /REPOS-ROOT/!svn/rvr
> SVN-Txn-Stub: /REPOS-ROOT/!svn/txn
> SVN-Txn-Root-Stub: /REPOS-ROOT/!svn/txr
>
> Should it send "vtxn" and "vtxr" stubs too, or instead?
Those are not in the POST response.
The server already sends SVN-Txn-Stub and SVN-Txn-Root-Stub during
capabilities negotiation, the patch makes it send SVN-VTxn-Stub and
SVN-VTxn-Root stub as well.
> (I don't
> understand why the protocol needs to send these stubs explicitly at all:
> is there some reason why these cannot just be constructed by the
> client?)
--
Philip
Re: [PATCH v2] HTTPv2 allow client to control transaction name in
protocol
Posted by Julian Foad <ju...@wandisco.com>.
On Fri, 2011-03-04, Philip Martin wrote:
> Extend Subversion's v2 HTTP protocol to include URIs that allow the
> client to define the transaction name visible in on the wire.
>
> If the client sends, or a proxy injects, an SVN-VTxn-Name header with
> the POST request it defines the transaction name to be returned to the
> client in the POST response. If the client recieves the new
> SVN-VTxn-Name header it uses that name in the new URIs in the requests
> that make up the commit.
>
> By default the client will not send the new header.
>
> * subversion/mod_dav_svn/dav_svn.h
> (struct dav_svn_root): Add vtxn_name member.
> (dav_svn__get_vtxn_stub, dav_svn__get_vtxn_root_stub): New.
>
> * subversion/mod_dav_svn/mod_dav_svn.c
> (dav_svn__get_vtxn_stub, dav_svn__get_vtxn_root_stub): New.
>
> * subversion/mod_dav_svn/posts/create_txn.c
> (dav_svn__post_create_txn): Get vtxn_name from header, if sent, and
> store vtxn_name:txn_name mapping in the activity database.
>
> * subversion/mod_dav_svn/version.c
> (get_option): Add vtxn stub and vtxn root stub headers.
> (merge): Delete visible_txn_name:txn_name mapping after commit.
>
> * subversion/mod_dav_svn/repos.c
> (parse_vtxnstub_uri, parse_vtxnroot_uri): New.
> (special_subdirs): Add vtxn and vtxr.
> (remove_resource): Delete mapping when aborting the transaction.
>
> * subversion/libsvn_ra_serf/ra_serf.h
> (struct svn_ra_serf__session_t): Add vtxn_stub and vtxn_root_stub.
>
> * subversion/libsvn_ra_serf/commit.c
> (setup_post_headers): New.
> (post_headers_iterator_callback): Handle vtxn name header.
> (open_root): Set header_delegate.
>
> * subversion/libsvn_ra_serf/options.c
> (capabilities_headers_iterator_callback): Support vtxn stub and
> vtxn root stub headers.
>
> * subversion/libsvn_ra_neon/options.c
> (parse_capabilities): Support vtxn stub and vtxn root stub headers.
>
> * subversion/include/svn_dav.h
> (SVN_DAV_VTXN_STUB_HEADER, SVN_DAV_VTXN_ROOT_STUB_HEADER,
> SVN_DAV_VTXN_NAME_HEADER): New.
>
> * notes/http-and-webdav/http-protocol-v2.txt: Update.
>
> Index: notes/http-and-webdav/http-protocol-v2.txt
> ===================================================================
> --- notes/http-and-webdav/http-protocol-v2.txt (revision 1077861)
> +++ notes/http-and-webdav/http-protocol-v2.txt (working copy)
> @@ -128,6 +128,14 @@
> Various read- and write-type requests can be issued against
> these resources (MKCOL, PUT, PROPFIND, PROPPATCH, GET, etc.).
>
> + - alternate transaction resource (!svn/vtxn/VTXN-NAME)
> + alternate transaction root resource (!svn/vtxr/VTXN-NAME/[PATH])
> +
> + Alternative names for the transaction based on a virtual, or
> + visible, name supplied by the client when the transaction
> + was created. The client supplied name is optional, if not
> + supplied these resource names are not valid.
> +
> * Opening an RA session:
>
> ra_serf will send an OPTIONS request when creating a new
> @@ -220,12 +228,17 @@
>
> - no more need to "discover" the activity URI; !svn/act/ is gone.
>
> - - client no longer creates an activity UUID itself.
> + - client no longer needs to create an activity UUID itself.
>
> - instead, POST returns the name of the transaction it created,
> - which can then be appended to the transaction stub and
> - transaction root stub as necessary.
> + as TXN-NAME, which can then be appended to the transaction
> + stub and transaction root stub as necessary.
>
> + - if the client does choose to supply a UUID with the POST
> + request then the POST returns that UUID as VTXN-NAME, and the
> + client then uses that with the alternate transaction stub and
> + transaction root stub in subsequent requests.
One thing that's not 100% clear from the protocol doc update is whether
the server sends *both* txn names in response, or just the "V" version.
If it sends both, then we need to specify whether the client has to use
the "V" version or can choose to use either one, or can mix accesses
arbitrarily using either. I can't think of any reason the server would
need to send both, so can we keep things simple by specifying that it
doesn't?
Other parts of this doc may need updating:
* Opening an RA session:
[...]
Additionally, this response will contain some new URL stub values:
SVN-Rev-Stub: /REPOS-ROOT/!svn/rev
SVN-Rev-Root-Stub: /REPOS-ROOT/!svn/rvr
SVN-Txn-Stub: /REPOS-ROOT/!svn/txn
SVN-Txn-Root-Stub: /REPOS-ROOT/!svn/txr
Should it send "vtxn" and "vtxr" stubs too, or instead? (I don't
understand why the protocol needs to send these stubs explicitly at all:
is there some reason why these cannot just be constructed by the
client?)
* Commits
[...]
Specific new changes:
- The activity-UUID-to-Subversion-txn-name abstraction is gone.
We now expose the Subversion txn names explicitly through the
protocol.
[...]
- client no longer creates an activity UUID itself.
- Julian