Allow DAV clients to request that the server transmit all property changes inline, even when not in send-all mode, and which teaches ra_serf to do so. * subversion/mod_dav_svn/reports/update.c (update_ctx_t): Add 'include_props' member. (item_baton_t): Add 'fetch_props' member. (close_helper): Add 'pool' parameter, and do property name XML-quoting here. If the item's 'fetch_props' flag is set, transmit this fact to the client. (maybe_start_update_report): Include "inline-prop=true" attribute in the report response tag if the client so requested that functionality. (send_propchange): New helper function. (upd_change_xxx_prop): Don't XML-quote property names here. Rework logic for comprehensibility, making use of send_propchange(). (upd_close_directory, upd_close_file): Update calls to close_helper(). (dav_svn__update_report): Set 'include_props' when in send-all mode, and also when the client explicitly requests that props be included inline. * subversion/libsvn_ra_serf/update.c (report_context_t): New 'add_props_included' member. (start_report): Look for "inline-props=true" on the report response tag, setting add_props_included to TRUE if found. Only set fetch_props for added files and directories if add_props_included isn't set. (make_update_reporter): Add "yes" XML tag to the REPORT request. Patch by: cmpilato --This line, and those below, will be ignored-- Index: subversion/libsvn_ra_serf/update.c =================================================================== --- subversion/libsvn_ra_serf/update.c (revision 1378669) +++ subversion/libsvn_ra_serf/update.c (working copy) @@ -313,6 +313,10 @@ /* Do we want the server to send copyfrom args or not? */ svn_boolean_t send_copyfrom_args; + /* Is the server including properties inline for newly added + files/dirs? */ + svn_boolean_t add_props_included; + /* Path -> lock token mapping. */ apr_hash_t *lock_path_tokens; @@ -1391,8 +1395,14 @@ state = parser->state->current_state; - if (state == NONE && strcmp(name.name, "target-revision") == 0) + if (state == NONE && strcmp(name.name, "update-report") == 0) { + const char *val = svn_xml_get_attr_value("inline-props", attrs); + if (val && (strcmp(val, "true") == 0)) + ctx->add_props_included = TRUE; + } + else if (state == NONE && strcmp(name.name, "target-revision") == 0) + { const char *rev; rev = svn_xml_get_attr_value("rev", attrs); @@ -1531,8 +1541,12 @@ /* Mark that we don't have a base. */ info->base_rev = SVN_INVALID_REVNUM; dir->base_rev = info->base_rev; - dir->fetch_props = TRUE; + /* If the server isn't included properties for added items, + we'll need to fetch them ourselves. */ + if (! ctx->add_props_included) + dir->fetch_props = TRUE; + dir->repos_relpath = svn_relpath_join(dir->parent_dir->repos_relpath, dir->base_name, dir->pool); } @@ -1588,9 +1602,13 @@ info = push_state(parser, ctx, ADD_FILE); info->base_rev = SVN_INVALID_REVNUM; - info->fetch_props = TRUE; info->fetch_file = TRUE; + /* If the server isn't included properties for added items, + we'll need to fetch them ourselves. */ + if (! ctx->add_props_included) + info->fetch_props = TRUE; + info->base_name = apr_pstrdup(info->pool, file_name); info->name = NULL; @@ -1906,7 +1924,7 @@ /* At this point, we should have the checked-in href. * If needed, create the PROPFIND to retrieve the dir's properties. */ - if (!SVN_IS_VALID_REVNUM(info->dir->base_rev) || info->dir->fetch_props) + if (info->dir->fetch_props) { /* Unconditionally set fetch_props now. */ info->dir->fetch_props = TRUE; @@ -2785,6 +2803,10 @@ make_simple_xml_tag(&buf, "S:recursive", "no", scratch_pool); } + /* Subversion 1.8+ servers can be told to send properties for newly + added items inline even when doing a skelta response. */ + make_simple_xml_tag(&buf, "S:include-props", "yes", scratch_pool); + make_simple_xml_tag(&buf, "S:depth", svn_depth_to_word(depth), scratch_pool); SVN_ERR(svn_io_file_write_full(report->body_file, buf->data, buf->len, Index: subversion/mod_dav_svn/reports/update.c =================================================================== --- subversion/mod_dav_svn/reports/update.c (revision 1378667) +++ subversion/mod_dav_svn/reports/update.c (working copy) @@ -81,6 +81,10 @@ /* True iff client requested all data inline in the report. */ svn_boolean_t send_all; + /* True iff client requested that properties be transmitted + inline. (This is implied when "send_all" is set.) */ + svn_boolean_t include_props; + /* SVNDIFF version to send to client. */ int svndiff_version; @@ -119,6 +123,10 @@ /* File/dir copied? */ svn_boolean_t copyfrom; + /* Does the client need to fetch additional properties for this + item? */ + svn_boolean_t fetch_props; + /* Array of const char * names of removed properties. (Used only for copied files/dirs in skelta mode.) */ apr_array_header_t *removed_props; @@ -432,7 +440,7 @@ static svn_error_t * -close_helper(svn_boolean_t is_dir, item_baton_t *baton) +close_helper(svn_boolean_t is_dir, item_baton_t *baton, apr_pool_t *pool) { if (baton->uc->resource_walk) return SVN_NO_ERROR; @@ -446,14 +454,21 @@ for (i = 0; i < baton->removed_props->nelts; i++) { - /* We already XML-escaped the property name in change_xxx_prop. */ qname = APR_ARRAY_IDX(baton->removed_props, i, const char *); + qname = apr_xml_quote_string(pool, qname, 1); SVN_ERR(dav_svn__brigade_printf(baton->uc->bb, baton->uc->output, "" DEBUG_CR, qname)); } } + /* If our client need to fetch properties, let it know. */ + if (baton->fetch_props) + SVN_ERR(dav_svn__brigade_printf(baton->uc->bb, baton->uc->output, + "" DEBUG_CR)); + + + /* Let's tie it off, nurse. */ if (baton->added) SVN_ERR(dav_svn__brigade_printf(baton->uc->bb, baton->uc->output, "" DEBUG_CR, @@ -473,13 +488,13 @@ { if ((! uc->resource_walk) && (! uc->started_update)) { - SVN_ERR(dav_svn__brigade_printf(uc->bb, uc->output, - DAV_XML_HEADER DEBUG_CR - "" DEBUG_CR, - uc->send_all ? "send-all=\"true\"" : "")); + SVN_ERR(dav_svn__brigade_printf( + uc->bb, uc->output, + DAV_XML_HEADER DEBUG_CR "" DEBUG_CR, + uc->send_all ? "send-all=\"true\"" : "", + uc->include_props ? "inline-props=\"true\"" : "")); uc->started_update = TRUE; } @@ -593,90 +608,120 @@ static svn_error_t * +send_propchange(item_baton_t *b, + const char *name, + const svn_string_t *value, + apr_pool_t *pool) +{ + const char *qname; + + /* Ensure that the property name is XML-safe. + apr_xml_quote_string() doesn't realloc if there is nothing to + quote, so dup the name, but only if necessary. */ + qname = apr_xml_quote_string(b->pool, name, 1); + if (qname == name) + qname = apr_pstrdup(b->pool, name); + + if (value) + { + const char *qval; + + if (svn_xml_is_xml_safe(value->data, value->len)) + { + svn_stringbuf_t *tmp = NULL; + svn_xml_escape_cdata_string(&tmp, value, pool); + qval = tmp->data; + SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output, + "", + qname)); + } + else + { + qval = svn_base64_encode_string2(value, TRUE, pool)->data; + SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output, + "" DEBUG_CR, + qname)); + } + + SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output, qval)); + SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output, + "" DEBUG_CR)); + } + else /* value is null, so this is a prop removal */ + { + SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output, + "" + DEBUG_CR, + qname)); + } + + return SVN_NO_ERROR; +} + +static svn_error_t * upd_change_xxx_prop(void *baton, const char *name, const svn_string_t *value, apr_pool_t *pool) { item_baton_t *b = baton; - const char *qname; /* Resource walks say nothing about props. */ if (b->uc->resource_walk) return SVN_NO_ERROR; - /* Else this not a resource walk, so either send props or cache them - to send later, depending on whether this is a modern report - response or not. */ + /* If we get here, this not a resource walk, so either send props or + cache them to send later, depending on whether this is a modern + report response or not. */ - qname = apr_xml_quote_string(b->pool, name, 1); - - /* apr_xml_quote_string doesn't realloc if there is nothing to - quote, so dup the name, but only if necessary. */ - if (qname == name) - qname = apr_pstrdup(b->pool, name); - /* Even if we are not in send-all mode we have the prop changes already, so send them to the client now instead of telling the client to fetch them later. */ - if (b->uc->send_all || !b->added) + if (b->uc->send_all) { - if (value) + SVN_ERR(send_propchange(b, name, value, pool)); + } + else + { + if (b->added) { - const char *qval; + /* This is an addition in "skelta" (that is, "not send-all") + mode so there is no strict need for an inline response. + Clients will assume that added objects need all to have + all their properties explicitly fetched from the + server. */ - if (svn_xml_is_xml_safe(value->data, value->len)) + /* That said, beginning in Subversion 1.8, clients might + request even in skelta mode that we transmit properties + on newly added files explicitly. */ + if ((! b->copyfrom) && value && b->uc->include_props) { - svn_stringbuf_t *tmp = NULL; - svn_xml_escape_cdata_string(&tmp, value, pool); - qval = tmp->data; - SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output, - "", - qname)); + SVN_ERR(send_propchange(b, name, value, pool)); } - else + + /* Now, if the object is actually a copy and this is a + property removal, we'll still need to cache (and later + transmit) property removals, because fetching the + object's current property set alone isn't sufficient to + communicate the fact that additional properties were, in + fact, removed from the copied base object in order to + arrive at that set. */ + if (b->copyfrom && (! value)) { - qval = svn_base64_encode_string2(value, TRUE, pool)->data; - SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output, - "" DEBUG_CR, - qname)); + if (! b->removed_props) + b->removed_props = apr_array_make(b->pool, 1, sizeof(name)); + + APR_ARRAY_PUSH(b->removed_props, const char *) = name; } - - SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output, qval)); - SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output, - "" DEBUG_CR)); } - else /* value is null, so this is a prop removal */ + else { - SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output, - "" - DEBUG_CR, - qname)); + /* "skelta" mode non-addition. Just send the change. */ + SVN_ERR(send_propchange(b, name, value, pool)); } } - else if (!value) - { - /* This is an addition in "skelta" (that is, "not send-all") - mode so there is no strict need for an inline response. - Clients will assume that added objects need all to have all - their properties explicitly fetched from the server. */ - /* Now, if the object is actually a copy, we'll still need to - cache (and later transmit) property removals, because - fetching the object's current property set alone isn't - sufficient to communicate the fact that additional properties - were, in fact, removed from the copied base object in order - to arrive at that set. */ - if (b->copyfrom) - { - if (! b->removed_props) - b->removed_props = apr_array_make(b->pool, 1, sizeof(name)); - - APR_ARRAY_PUSH(b->removed_props, const char *) = qname; - } - } - return SVN_NO_ERROR; } @@ -684,7 +729,7 @@ static svn_error_t * upd_close_directory(void *dir_baton, apr_pool_t *pool) { - return close_helper(TRUE /* is_dir */, dir_baton); + return close_helper(TRUE /* is_dir */, dir_baton, pool); } @@ -845,7 +890,7 @@ text_checksum)); } - return close_helper(FALSE /* is_dir */, file); + return close_helper(FALSE /* is_dir */, file, pool); } @@ -944,6 +989,7 @@ && (strcmp(this_attr->value, "true") == 0)) { uc.send_all = TRUE; + uc.include_props = TRUE; break; } } @@ -1066,6 +1112,14 @@ if (strcmp(cdata, "no") == 0) text_deltas = FALSE; } + if (child->ns == ns && strcmp(child->name, "include-props") == 0) + { + cdata = dav_xml_get_cdata(child, resource->pool, 1); + if (! *cdata) + return malformed_element_error(child->name, resource->pool); + if (strcmp(cdata, "no") != 0) + uc.include_props = TRUE; + } } if (!saw_depth && !saw_recursive && (requested_depth == svn_depth_unknown))