You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by gs...@apache.org on 2003/06/04 00:09:24 UTC

cvs commit: httpd-2.0/modules/dav/main mod_dav.c mod_dav.h

gstein      2003/06/03 15:09:24

  Modified:    modules/dav/main mod_dav.c mod_dav.h
  Log:
  mod_dav improvement: make dav_method_propfind stream its response,
  rather than cache every <response> object and send the whole 207 at once.
  
  Note:  this patch doesn't affect the mod_dav provider API at all.
  Providers still return property results in text-buffers, but mod_dav
  then streams them out immediately.
  
  Submitted by: Ben Collins-Sussman <su...@collab.net>
  Reviewed by: gstein, jerenkrantz, sander
  
  * mod_dav.h (dav_walker_ctx): add a brigade field and a scratchpool field.
  
  * mod_dav.c (dav_send_one_response): new helper function to write a
      <DAV:response> into a brigade/filter.  this code has been factorized
      out of dav_send_multistatus.
  
    (dav_begin_multistatus): new factorized helper func; creates brigade
      and sends initial <multistatus> tag.
  
    (dav_send_multistatus): create brigade, call dav_begin_multistatus,
      and switch all ap_rputs calls to ap_fputs instead.  call
      dav_send_one_response when looping over response list.  use a
      subpool when iterating.
  
    (dav_method_propfind): initialize walker ctx's brigade.  initialize
      ctx's scratchpool as a subpool of r->pool.  Send a <multistatus> tag
      before calling the provider's walk() function, and a </multistatus>
      tag afterwards.
  
    (dav_stream_response): new function, originally based on
      dav_add_repsonse.  don't build linked list of responses in memory;
      just spew each response object into the brigade via
      dav_send_one_response().  take an incoming pool argument to do the
      temporary allocation and streaming.
  
    (dav_propfind_walker): pass ctx->scratchpool to dav_stream_response,
      and clear the pool when finished.
  
  Revision  Changes    Path
  1.94      +142 -67   httpd-2.0/modules/dav/main/mod_dav.c
  
  Index: mod_dav.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/dav/main/mod_dav.c,v
  retrieving revision 1.93
  retrieving revision 1.94
  diff -u -r1.93 -r1.94
  --- mod_dav.c	20 May 2003 23:09:00 -0000	1.93
  +++ mod_dav.c	3 Jun 2003 22:09:24 -0000	1.94
  @@ -453,78 +453,121 @@
       return apr_xml_quote_string(p, e_uri, 0);
   }
   
  -static void dav_send_multistatus(request_rec *r, int status,
  -                                 dav_response *first,
  -                                 apr_array_header_t *namespaces)
  +
  +/* Write a complete RESPONSE object out as a <DAV:repsonse> xml
  +   element.  Data is sent into brigade BB, which is auto-flushed into
  +   OUTPUT filter stack.  Use POOL for any temporary allocations.
  +
  +   [Presumably the <multistatus> tag has already been written;  this
  +   routine is shared by dav_send_multistatus and dav_stream_response.]
  +*/
  +static void dav_send_one_response(dav_response *response,
  +                                  apr_bucket_brigade *bb,
  +                                  ap_filter_t *output,
  +                                  apr_pool_t *pool)
  +{
  +    apr_text *t = NULL;
  +
  +    if (response->propresult.xmlns == NULL) {
  +      ap_fputs(output, bb, "<D:response>");
  +    }
  +    else {
  +      ap_fputs(output, bb, "<D:response");
  +      for (t = response->propresult.xmlns; t; t = t->next) {
  +        ap_fputs(output, bb, t->text);
  +      }
  +      ap_fputc(output, bb, '>');
  +    }
  +    
  +    ap_fputstrs(output, bb,
  +                DEBUG_CR "<D:href>",
  +                dav_xml_escape_uri(pool, response->href),
  +                "</D:href>" DEBUG_CR,
  +                NULL);
  +    
  +    if (response->propresult.propstats == NULL) {
  +      /* use the Status-Line text from Apache.  Note, this will
  +       * default to 500 Internal Server Error if first->status
  +       * is not a known (or valid) status code.
  +       */
  +      ap_fputstrs(output, bb,
  +                  "<D:status>HTTP/1.1 ",
  +                  ap_get_status_line(response->status),
  +                  "</D:status>" DEBUG_CR,
  +                  NULL);
  +    }
  +    else {
  +      /* assume this includes <propstat> and is quoted properly */
  +      for (t = response->propresult.propstats; t; t = t->next) {
  +        ap_fputs(output, bb, t->text);
  +      }
  +    }
  +    
  +    if (response->desc != NULL) {
  +      /*
  +       * We supply the description, so we know it doesn't have to
  +       * have any escaping/encoding applied to it.
  +       */
  +      ap_fputstrs(output, bb,
  +                  "<D:responsedescription>",
  +                  response->desc,
  +                  "</D:responsedescription>" DEBUG_CR,
  +                  NULL);
  +    }
  +    
  +    ap_fputs(output, bb, "</D:response>" DEBUG_CR);
  +}
  +
  +
  +/* Factorized helper function: prep request_rec R for a multistatus
  +   response and write <multistatus> tag into BB, destined for
  +   R->output_filters.  Use xml NAMESPACES in initial tag, if
  +   non-NULL. */
  +static void dav_begin_multistatus(apr_bucket_brigade *bb,
  +                                  request_rec *r, int status,
  +                                  apr_array_header_t *namespaces)
   {
       /* Set the correct status and Content-Type */
       r->status = status;
       ap_set_content_type(r, DAV_XML_CONTENT_TYPE);
   
       /* Send the headers and actual multistatus response now... */
  -    ap_rputs(DAV_XML_HEADER DEBUG_CR
  -             "<D:multistatus xmlns:D=\"DAV:\"", r);
  +    ap_fputs(r->output_filters, bb, DAV_XML_HEADER DEBUG_CR
  +             "<D:multistatus xmlns:D=\"DAV:\"");
   
       if (namespaces != NULL) {
          int i;
   
          for (i = namespaces->nelts; i--; ) {
  -           ap_rprintf(r, " xmlns:ns%d=\"%s\"", i,
  +           ap_fprintf(r->output_filters, bb, " xmlns:ns%d=\"%s\"", i,
                         APR_XML_GET_URI_ITEM(namespaces, i));
          }
       }
   
  -    /* ap_rputc('>', r); */
  -    ap_rputs(">" DEBUG_CR, r);
  +    ap_fputs(r->output_filters, bb, ">" DEBUG_CR);
  +}
   
  -    for (; first != NULL; first = first->next) {
  -        apr_text *t;
   
  -        if (first->propresult.xmlns == NULL) {
  -            ap_rputs("<D:response>", r);
  -        }
  -        else {
  -            ap_rputs("<D:response", r);
  -            for (t = first->propresult.xmlns; t; t = t->next) {
  -                ap_rputs(t->text, r);
  -            }
  -            ap_rputc('>', r);
  -        }
  +static void dav_send_multistatus(request_rec *r, int status,
  +                                 dav_response *first,
  +                                 apr_array_header_t *namespaces)
  +{
  +    apr_pool_t *subpool;
  +    apr_bucket_brigade *bb = apr_brigade_create(r->pool,
  +                                                r->connection->bucket_alloc);
   
  -        ap_rputs(DEBUG_CR "<D:href>", r);
  -        ap_rputs(dav_xml_escape_uri(r->pool, first->href), r);
  -        ap_rputs("</D:href>" DEBUG_CR, r);
  +    dav_begin_multistatus(bb, r, status, namespaces);
   
  -        if (first->propresult.propstats == NULL) {
  -            /* use the Status-Line text from Apache.  Note, this will
  -             * default to 500 Internal Server Error if first->status
  -             * is not a known (or valid) status code.
  -             */
  -            ap_rprintf(r,
  -                       "<D:status>HTTP/1.1 %s</D:status>" DEBUG_CR,
  -                       ap_get_status_line(first->status));
  -        }
  -        else {
  -            /* assume this includes <propstat> and is quoted properly */
  -            for (t = first->propresult.propstats; t; t = t->next) {
  -                ap_rputs(t->text, r);
  -            }
  -        }
  +    apr_pool_create(&subpool, r->pool);
   
  -        if (first->desc != NULL) {
  -            /*
  -             * We supply the description, so we know it doesn't have to
  -             * have any escaping/encoding applied to it.
  -             */
  -            ap_rputs("<D:responsedescription>", r);
  -            ap_rputs(first->desc, r);
  -            ap_rputs("</D:responsedescription>" DEBUG_CR, r);
  -        }
  -
  -        ap_rputs("</D:response>" DEBUG_CR, r);
  +    for (; first != NULL; first = first->next) {
  +      apr_pool_clear(subpool);
  +      dav_send_one_response(first, bb, r->output_filters, subpool);
       }
  +    apr_pool_destroy(subpool);
   
  -    ap_rputs("</D:multistatus>" DEBUG_CR, r);
  +    ap_fputs(r->output_filters, bb, "</D:multistatus>" DEBUG_CR);
  +    ap_filter_flush(bb, r->output_filters);
   }
   
   /*
  @@ -1048,6 +1091,27 @@
       return dav_created(r, NULL, "Resource", resource_state == DAV_RESOURCE_EXISTS);
   }
   
  +
  +/* Use POOL to temporarily construct a dav_response object (from WRES
  +   STATUS, and PROPSTATS) and stream it via WRES's ctx->brigade. */
  +static void dav_stream_response(dav_walk_resource *wres,
  +                                int status,
  +                                dav_get_props_result *propstats,
  +                                apr_pool_t *pool)
  +{
  +    dav_response resp = { 0 };
  +    dav_walker_ctx *ctx = wres->walk_ctx;
  +
  +    resp.href = wres->resource->uri;
  +    resp.status = status;
  +    if (propstats) {
  +        resp.propresult = *propstats;
  +    }
  +
  +    dav_send_one_response(&resp, ctx->bb, ctx->r->output_filters, pool);
  +}
  +
  +
   /* ### move this to dav_util? */
   DAV_DECLARE(void) dav_add_response(dav_walk_resource *wres,
                                      int status, dav_get_props_result *propstats)
  @@ -1066,6 +1130,7 @@
       wres->response = resp;
   }
   
  +
   /* handle the DELETE method */
   static int dav_method_delete(request_rec *r)
   {
  @@ -1837,12 +1902,14 @@
               /* some props were expected on this collection/resource */
               dav_cache_badprops(ctx);
               badprops.propstats = ctx->propstat_404;
  -            dav_add_response(wres, 0, &badprops);
  +            dav_stream_response(wres, 0, &badprops, ctx->scratchpool);
           }
           else {
               /* no props on this collection/resource */
  -            dav_add_response(wres, HTTP_OK, NULL);
  +            dav_stream_response(wres, HTTP_OK, NULL, ctx->scratchpool);
           }
  +
  +        apr_pool_clear(ctx->scratchpool);
           return NULL;
       }
       /* ### what to do about closing the propdb on server failure? */
  @@ -1858,7 +1925,13 @@
       }
       dav_close_propdb(propdb);
   
  -    dav_add_response(wres, 0, &propstats);
  +    dav_stream_response(wres, 0, &propstats, ctx->scratchpool);
  +
  +    /* at this point, ctx->scratchpool has been used to stream a
  +       single response.  this function fully controls the pool, and
  +       thus has the right to clear it for the next iteration of this
  +       callback. */
  +    apr_pool_clear(ctx->scratchpool);
   
       return NULL;
   }
  @@ -1951,6 +2024,8 @@
   
       ctx.doc = doc;
       ctx.r = r;
  +    ctx.bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
  +    apr_pool_create(&ctx.scratchpool, r->pool);
   
       /* ### should open read-only */
       if ((err = dav_open_lockdb(r, 0, &ctx.w.lockdb)) != NULL) {
  @@ -1966,6 +2041,17 @@
           ctx.w.walk_type |= DAV_WALKTYPE_LOCKNULL;
       }
   
  +    /* send <multistatus> tag, with all doc->namespaces attached.  */
  +
  +    /* NOTE: we *cannot* leave out the doc's namespaces from the
  +       initial <multistatus> tag.  if a 404 was generated for an HREF,
  +       then we need to spit out the doc's namespaces for use by the
  +       404. Note that <response> elements will override these ns0,
  +       ns1, etc, but NOT within the <response> scope for the
  +       badprops. */
  +    dav_begin_multistatus(ctx.bb, r, HTTP_MULTI_STATUS, doc->namespaces);
  +
  +    /* Have the provider walk the resource. */
       err = (*resource->hooks->walk)(&ctx.w, depth, &multi_status);
   
       if (ctx.w.lockdb != NULL) {
  @@ -1977,20 +2063,9 @@
           return dav_handle_err(r, err, NULL);
       }
   
  -    /* return a 207 (Multi-Status) response now. */
  -
  -    /* if a 404 was generated for an HREF, then we need to spit out the
  -     * doc's namespaces for use by the 404. Note that <response> elements
  -     * will override these ns0, ns1, etc, but NOT within the <response>
  -     * scope for the badprops. */
  -    /* NOTE: propstat_404 != NULL implies doc != NULL */
  -    if (ctx.propstat_404 != NULL) {
  -        dav_send_multistatus(r, HTTP_MULTI_STATUS, multi_status,
  -                             doc->namespaces);
  -    }
  -    else {
  -        dav_send_multistatus(r, HTTP_MULTI_STATUS, multi_status, NULL);
  -    }
  +    /* Finish up the multistatus response. */
  +    ap_fputs(r->output_filters, ctx.bb, "</D:multistatus>" DEBUG_CR);
  +    ap_filter_flush(ctx.bb, r->output_filters);
   
       /* the response has been sent. */
       return DONE;
  
  
  
  1.67      +6 -0      httpd-2.0/modules/dav/main/mod_dav.h
  
  Index: mod_dav.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/dav/main/mod_dav.h,v
  retrieving revision 1.66
  retrieving revision 1.67
  diff -u -r1.66 -r1.67
  --- mod_dav.h	3 Feb 2003 17:52:58 -0000	1.66
  +++ mod_dav.h	3 Jun 2003 22:09:24 -0000	1.67
  @@ -1671,6 +1671,12 @@
   
       /* ### client data... phasing out this big glom */
   
  +    /* this brigade buffers data being sent to r->output_filters */ 
  +    apr_bucket_brigade *bb;
  +
  +    /* a scratch pool, used to stream responses and iteratively cleared. */
  +    apr_pool_t *scratchpool;
  +
       request_rec *r;                 /* original request */
   
       /* for PROPFIND operations */