httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Davi Arnaut <d...@haxent.com.br>
Subject [patch 03/16] shrink cache_save_filter
Date Wed, 20 Sep 2006 02:33:56 GMT
Move parts of cache_save_filter() to a smaller check_cacheable_request()
function that is easier to read and understand. Also, a useless assignment
is removed.


Index: modules/cache/mod_cache.c
===================================================================
--- modules/cache/mod_cache.c.orig
+++ modules/cache/mod_cache.c
@@ -313,65 +313,11 @@
  *   Finally, pass the data to the next filter (the network or whatever)
  */
 
-static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
+static const char* check_cacheable_request(request_rec *r,
+    cache_request_rec *cache, cache_server_conf *conf)
 {
-    int rv = !OK;
-    int date_in_errhdr = 0;
-    request_rec *r = f->r;
-    cache_request_rec *cache;
-    cache_server_conf *conf;
-    const char *cc_out, *cl;
-    const char *exps, *lastmods, *dates, *etag;
-    apr_time_t exp, date, lastmod, now;
-    apr_off_t size;
-    cache_info *info = NULL;
-    char *reason;
-    apr_pool_t *p;
-
-    conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
-                                                      &cache_module);
-
-    /* Setup cache_request_rec */
-    cache = (cache_request_rec *) ap_get_module_config(r->request_config,
-                                                       &cache_module);
-    if (!cache) {
-        /* user likely configured CACHE_SAVE manually; they should really use
-         * mod_cache configuration to do that
-         */
-        cache = apr_pcalloc(r->pool, sizeof(cache_request_rec));
-        ap_set_module_config(r->request_config, &cache_module, cache);
-    }
-
-    reason = NULL;
-    p = r->pool;
-    /*
-     * Pass Data to Cache
-     * ------------------
-     * This section passes the brigades into the cache modules, but only
-     * if the setup section (see below) is complete.
-     */
-    if (cache->block_response) {
-        /* We've already sent down the response and EOS.  So, ignore
-         * whatever comes now.
-         */
-        return APR_SUCCESS;
-    }
-
-    /* have we already run the cachability check and set up the
-     * cached file handle?
-     */
-    if (cache->in_checked) {
-        /* pass the brigades into the cache, then pass them
-         * up the filter stack
-         */
-        rv = cache->provider->store_body(cache->handle, r, in);
-        if (rv != APR_SUCCESS) {
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
-                         "cache: Cache provider's store_body failed!");
-            ap_remove_output_filter(f);
-        }
-        return ap_pass_brigade(f->next, in);
-    }
+    apr_time_t exp, lastmod;
+    const char *exps, *lastmods, *etag, *cc_out, *reason = NULL;
 
     /*
      * Setup Data in Cache
@@ -439,11 +385,11 @@
          * We include 304 Not Modified here too as this is the origin server
          * telling us to serve the cached copy.
          */
-        reason = apr_psprintf(p, "Response status %d", r->status);
+        reason = apr_psprintf(r->pool, "Response status %d", r->status);
     }
     else if (exps != NULL && exp == APR_DATE_BAD) {
         /* if a broken Expires header is present, don't cache it */
-        reason = apr_pstrcat(p, "Broken expires header: ", exps, NULL);
+        reason = apr_pstrcat(r->pool, "Broken expires header: ", exps, NULL);
     }
     else if (exp != APR_DATE_BAD && exp < r->request_time)
     {
@@ -522,20 +468,16 @@
         reason = "r->no_cache present";
     }
 
-    if (reason) {
-        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
-                     "cache: %s not cached. Reason: %s", r->unparsed_uri,
-                     reason);
+    cache->exp = exp;
+    cache->lastmod = lastmod;
 
-        /* remove this filter from the chain */
-        ap_remove_output_filter(f);
-
-        /* ship the data up the stack */
-        return ap_pass_brigade(f->next, in);
-    }
+    return reason;
+}
 
-    /* Make it so that we don't execute this path again. */
-    cache->in_checked = 1;
+static apr_off_t request_cl(request_rec *r, apr_bucket_brigade *in)
+{
+    const char *cl;
+    apr_off_t size;
 
     /* Set the content length if known.
      */
@@ -580,48 +522,18 @@
         }
     }
 
-    /* It's safe to cache the response.
-     *
-     * There are two possiblities at this point:
-     * - cache->handle == NULL. In this case there is no previously
-     * cached entity anywhere on the system. We must create a brand
-     * new entity and store the response in it.
-     * - cache->stale_handle != NULL. In this case there is a stale
-     * entity in the system which needs to be replaced by new
-     * content (unless the result was 304 Not Modified, which means
-     * the cached entity is actually fresh, and we should update
-     * the headers).
-     */
-
-    /* Did we have a stale cache entry that really is stale? */
-    if (cache->stale_handle) {
-        if (r->status == HTTP_NOT_MODIFIED) {
-            /* Oh, hey.  It isn't that stale!  Yay! */
-            cache->handle = cache->stale_handle;
-            info = &cache->handle->cache_obj->info;
-            rv = OK;
-        }
-        else {
-            /* Oh, well.  Toss it. */
-            cache->provider->remove_entity(cache->stale_handle);
-            /* Treat the request as if it wasn't conditional. */
-            cache->stale_handle = NULL;
-        }
-    }
+    return size;
+}
 
-    /* no cache handle, create a new entity */
-    if (!cache->handle) {
-        rv = cache_create_entity(r, size);
-        info = apr_pcalloc(r->pool, sizeof(cache_info));
-        /* We only set info->status upon the initial creation. */
-        info->status = r->status;
-    }
+static void prepare_cacheable_request(request_rec *r, cache_request_rec *cache,
+    cache_info *info, cache_server_conf *conf)
+{
+    const char *dates;
+    int date_in_errhdr = 0;
+    apr_time_t exp, lastmod, now, date;
 
-    if (rv != OK) {
-        /* Caching layer declined the opportunity to cache the response */
-        ap_remove_output_filter(f);
-        return ap_pass_brigade(f->next, in);
-    }
+    exp = cache->exp;
+    lastmod = cache->lastmod;
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                  "cache: Caching url: %s", r->unparsed_uri);
@@ -690,7 +602,6 @@
     if (lastmod != APR_DATE_BAD && lastmod > date) {
         /* if it's in the future, then replace by date */
         lastmod = date;
-        lastmods = dates;
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
                      r->server,
                      "cache: Last modified is in the future, "
@@ -729,7 +640,129 @@
             apr_table_set(r->headers_out, "Expires", expire_hdr);
         }
     }
+
     info->expire = exp;
+}
+
+static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
+{
+    int rv = !OK;
+    request_rec *r = f->r;
+    cache_request_rec *cache;
+    cache_server_conf *conf;
+    apr_off_t size;
+    cache_info *info = NULL;
+    const char *reason;
+    apr_pool_t *p;
+
+    conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
+                                                      &cache_module);
+
+    /* Setup cache_request_rec */
+    cache = (cache_request_rec *) ap_get_module_config(r->request_config,
+                                                       &cache_module);
+    if (!cache) {
+        /* user likely configured CACHE_SAVE manually; they should really use
+         * mod_cache configuration to do that
+         */
+        cache = apr_pcalloc(r->pool, sizeof(cache_request_rec));
+        ap_set_module_config(r->request_config, &cache_module, cache);
+    }
+
+    p = r->pool;
+
+    /*
+     * Pass Data to Cache
+     * ------------------
+     * This section passes the brigades into the cache modules, but only
+     * if the setup section (see below) is complete.
+     */
+    if (cache->block_response) {
+        /* We've already sent down the response and EOS.  So, ignore
+         * whatever comes now.
+         */
+        return APR_SUCCESS;
+    }
+
+    /* have we already run the cachability check and set up the
+     * cached file handle?
+     */
+    if (cache->in_checked) {
+        /* pass the brigades into the cache, then pass them
+         * up the filter stack
+         */
+        rv = cache->provider->store_body(cache->handle, r, in);
+        if (rv != APR_SUCCESS) {
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
+                         "cache: Cache provider's store_body failed!");
+            ap_remove_output_filter(f);
+        }
+        return ap_pass_brigade(f->next, in);
+    }
+
+    reason = check_cacheable_request(r, cache, conf);
+
+    if (reason) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                     "cache: %s not cached. Reason: %s", r->unparsed_uri,
+                     reason);
+
+        /* remove this filter from the chain */
+        ap_remove_output_filter(f);
+
+        /* ship the data up the stack */
+        return ap_pass_brigade(f->next, in);
+    }
+
+    /* Make it so that we don't execute this path again. */
+    cache->in_checked = 1;
+
+    size = request_cl(r, in);
+
+    /* It's safe to cache the response.
+     *
+     * There are two possiblities at this point:
+     * - cache->handle == NULL. In this case there is no previously
+     * cached entity anywhere on the system. We must create a brand
+     * new entity and store the response in it.
+     * - cache->stale_handle != NULL. In this case there is a stale
+     * entity in the system which needs to be replaced by new
+     * content (unless the result was 304 Not Modified, which means
+     * the cached entity is actually fresh, and we should update
+     * the headers).
+     */
+
+    /* Did we have a stale cache entry that really is stale? */
+    if (cache->stale_handle) {
+        if (r->status == HTTP_NOT_MODIFIED) {
+            /* Oh, hey.  It isn't that stale!  Yay! */
+            cache->handle = cache->stale_handle;
+            info = &cache->handle->cache_obj->info;
+            rv = OK;
+        }
+        else {
+            /* Oh, well.  Toss it. */
+            cache->provider->remove_entity(cache->stale_handle);
+            /* Treat the request as if it wasn't conditional. */
+            cache->stale_handle = NULL;
+        }
+    }
+
+    /* no cache handle, create a new entity */
+    if (!cache->handle) {
+        rv = cache_create_entity(r, size);
+        info = apr_pcalloc(r->pool, sizeof(cache_info));
+        /* We only set info->status upon the initial creation. */
+        info->status = r->status;
+    }
+
+    if (rv != OK) {
+        /* Caching layer declined the opportunity to cache the response */
+        ap_remove_output_filter(f);
+        return ap_pass_brigade(f->next, in);
+    }
+
+    prepare_cacheable_request(r, cache, info, conf);
 
     /* We found a stale entry which wasn't really stale. */
     if (cache->stale_handle) {

--

Mime
View raw message