mod_cache: Implement remove URL via a filter.

Remove entities from the cache when re-validation receives a 404 or other
content-no-longer-present error.

Suggested by: Paul Querna, Justin Erenkrantz
Submitted by: Rudiger Plum <ruediger.pluem vodafone.com>
Reviewed by: Justin Erenkrantz

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@231487 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index d1d5510d4f070c2edc19bc51e08111df84a3ddfb..d29f1b41148b606740abff1406d58c6488c41c71 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.3.0
 
+  *) mod_cache: Remove entities from the cache when re-validation
+     receives a 404 or other content-no-longer-present error.
+     [Rüdiger Plüm ruediger.pluem vodafone.com]
+
   *) mod_disk_cache: Properly remove files from cache when needed.
      [Rüdiger Plüm ruediger.pluem vodafone.com]
 

diff --git a/modules/cache/cache_storage.c b/modules/cache/cache_storage.c
index f43bbb1fe69820714d783d1fd42b3aaf0ccce10e..641ed17532d53585d87aa25026847c651eae4a9d 100644 (file)
--- a/modules/cache/cache_storage.c
+++ b/modules/cache/cache_storage.c
@@ -28,24 +28,28 @@ extern module AP_MODULE_DECLARE_DATA cache_module;
  * delete all URL entities from the cache
  *
  */
-int cache_remove_url(request_rec *r, char *url)
+int cache_remove_url(cache_request_rec *cache, apr_pool_t *p)
 {
     cache_provider_list *list;
     apr_status_t rv;
-    char *key;
-    cache_request_rec *cache = (cache_request_rec *) 
-                         ap_get_module_config(r->request_config, &cache_module);
+    cache_handle_t *h;
 
-    rv = cache_generate_key(r,r->pool,&key);
-    if (rv != APR_SUCCESS) {
-        return rv;
-    }
+    char *key;
 
     list = cache->providers;
 
+    /* Remove the stale cache entry if present. If not, we're
+     * being called from outside of a request; remove the 
+     * non-stalle handle.
+     */
+    h = cache->stale_handle ? cache->stale_handle : cache->handle;
+    if (!h) {
+       return OK;
+    }
+
     /* for each specified cache type, delete the URL */
     while(list) {
-        list->provider->remove_url(key);
+        list->provider->remove_url(h, p);
         list = list->next;
     }
     return OK;

diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c
index 87410133ab4c938e11b71bc48794a539b79fc950..fdcfbecec3b23243ac7e59a60f1eda44a19943ba 100644 (file)
--- a/modules/cache/mod_cache.c
+++ b/modules/cache/mod_cache.c
@@ -29,6 +29,7 @@ APR_OPTIONAL_FN_TYPE(ap_cache_generate_key) *cache_generate_key;
  */
 static ap_filter_rec_t *cache_save_filter_handle;
 static ap_filter_rec_t *cache_out_filter_handle;
+static ap_filter_rec_t *cache_remove_url_filter_handle;
 
 /*
  * CACHE handler
@@ -123,6 +124,19 @@ static int cache_url_handler(request_rec *r, int lookup)
                 /* add cache_save filter to cache this request */
                 ap_add_output_filter_handle(cache_save_filter_handle, NULL, r,
                                             r->connection);
+
+                ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
+                             "Adding CACHE_REMOVE_URL filter.");
+
+                /* Add cache_remove_url filter to this request to remove a
+                 * stale cache entry if needed. Also put the current cache
+                 * request rec in the filter context, as the request that
+                 * is available later during running the filter maybe
+                 * different due to an internal redirect.
+                 */
+                cache->remove_url_filter =
+                    ap_add_output_filter_handle(cache_remove_url_filter_handle, 
+                                                cache, r, r->connection);
             }
             else if (cache->stale_headers) {
                 ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
@@ -441,11 +455,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     if (reason) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                      "cache: %s not cached. Reason: %s", url, reason);
-        /* remove this object from the cache 
-         * BillS Asks.. Why do we need to make this call to remove_url?
-         * leave it in for now..
-         */
-        cache_remove_url(r, url);
 
         /* remove this filter from the chain */
         ap_remove_output_filter(f);
@@ -546,6 +555,13 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                  "cache: Caching url: %s", url);
 
+    /* We are actually caching this response. So it does not
+     * make sense to remove this entity any more.
+     */
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                 "cache: Removing CACHE_REMOVE_URL filter.");
+    ap_remove_output_filter(cache->remove_url_filter);
+
     /*
      * We now want to update the cache file header information with
      * the new date, last modified, expire and content length and write
@@ -714,6 +730,57 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     return ap_pass_brigade(f->next, in);
 }
 
+/*
+ * CACHE_REMOVE_URL filter
+ * ---------------
+ *
+ * This filter gets added in the quick handler every time the CACHE_SAVE filter
+ * gets inserted. Its purpose is to remove a confirmed stale cache entry from
+ * the cache.
+ *
+ * CACHE_REMOVE_URL has to be a protocol filter to ensure that is run even if
+ * the response is a canned error message, which removes the content filters
+ * and thus the CACHE_SAVE filter from the chain.
+ *
+ * CACHE_REMOVE_URL expects cache request rec within its context because the
+ * request this filter runs on can be different from the one whose cache entry
+ * should be removed, due to internal redirects.
+ *
+ * Note that CACHE_SAVE_URL (as a content-set filter, hence run before the
+ * protocol filters) will remove this filter if it decides to cache the file.
+ * Therefore, if this filter is left in, it must mean we need to toss any
+ * existing files.
+ */
+static int cache_remove_url_filter(ap_filter_t *f, apr_bucket_brigade *in)
+{
+    request_rec *r = f->r;
+    cache_request_rec *cache;
+
+    /* Setup cache_request_rec */
+    cache = (cache_request_rec *) f->ctx;
+
+    if (!cache) {
+        /* user likely configured CACHE_REMOVE_URL manually; they should really 
+         * use mod_cache configuration to do that. So:
+         * 1. Remove ourselves 
+         * 2. Do nothing and bail out
+         */
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                     "cache: CACHE_REMOVE_URL enabled unexpectedly");
+        ap_remove_output_filter(f);
+        return ap_pass_brigade(f->next, in);
+    }
+    /*
+     * Now remove this cache entry from the cache
+     */
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                 "cache: Removing url %s from the cache", f->r->unparsed_uri);
+    cache_remove_url(cache, r->pool);
+    /* remove ourselves */
+    ap_remove_output_filter(f);
+    return ap_pass_brigade(f->next, in);
+}
+
 /* -------------------------------------------------------------- */
 /* Setup configurable data */
 
@@ -967,6 +1034,7 @@ static int cache_post_config(apr_pool_t *p, apr_pool_t *plog,
     return OK;
 }
 
+
 static const command_rec cache_cmds[] =
 {
     /* XXX
@@ -1033,6 +1101,15 @@ static void register_hooks(apr_pool_t *p)
                                   cache_out_filter, 
                                   NULL,
                                   AP_FTYPE_CONTENT_SET+1);
+    /* CACHE_REMOVE_URL has to be a protocol filter to ensure that is
+     * run even if the response is a canned error message, which
+     * removes the content filters.
+     */
+    cache_remove_url_filter_handle =
+        ap_register_output_filter("CACHE_REMOVE_URL",
+                                  cache_remove_url_filter,
+                                  NULL,
+                                  AP_FTYPE_PROTOCOL);
     ap_hook_post_config(cache_post_config, NULL, NULL, APR_HOOK_REALLY_FIRST);
 }
 

diff --git a/modules/cache/mod_cache.h b/modules/cache/mod_cache.h
index 63416be36abd1a3c056f2586d1c1b774bace0b14..2e6fcf1d20d23cf93c45f4e5c03a8903333a24e7 100644 (file)
--- a/modules/cache/mod_cache.h
+++ b/modules/cache/mod_cache.h
@@ -197,7 +197,7 @@ typedef struct {
                            const char *urlkey, apr_off_t len);
     int (*open_entity) (cache_handle_t *h, request_rec *r,
                            const char *urlkey);
-    int (*remove_url) (const char *urlkey);
+    int (*remove_url) (cache_handle_t *h, apr_pool_t *p);
 } cache_provider;
 
 /* A linked-list of authn providers. */
@@ -225,6 +225,7 @@ typedef struct {
     apr_time_t exp;                     /* expiration */
     apr_time_t lastmod;                 /* last-modified time */
     cache_info *info;                   /* current cache info */
+    ap_filter_t *remove_url_filter;     /* Enable us to remove the filter */
 } cache_request_rec;
 
 
@@ -271,7 +272,7 @@ CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t *pool,
 /**
  * cache_storage.c
  */
-int cache_remove_url(request_rec *r, char *url);
+int cache_remove_url(cache_request_rec *cache, apr_pool_t *p);
 int cache_create_entity(request_rec *r, char *url, apr_off_t size);
 int cache_select_url(request_rec *r, char *url);
 apr_status_t cache_generate_key_default( request_rec *r, apr_pool_t*p, char**key );

diff --git a/modules/cache/mod_mem_cache.c b/modules/cache/mod_mem_cache.c
index f1be48d47100a92d04224fda97f57bd76ef2ef8b..efe0a18d734be537eff3c7387c2a429341806cb2 100644 (file)
--- a/modules/cache/mod_mem_cache.c
+++ b/modules/cache/mod_mem_cache.c
@@ -601,7 +601,7 @@ static int unserialize_table( cache_header_tbl_t *ctbl,
 /* remove_url()
  * Notes:
  */
-static int remove_url(const char *key) 
+static int remove_url(cache_handle_t *h, apr_pool_t *p) 
 {
     cache_object_t *obj;
     int cleanup = 0;
@@ -609,8 +609,8 @@ static int remove_url(const char *key)
     if (sconf->lock) {
         apr_thread_mutex_lock(sconf->lock);
     }
-  
-    obj = cache_find(sconf->cache_cache, key);       
+ 
+    obj = h->cache_obj; 
     if (obj) {
         cache_remove(sconf->cache_cache, obj);
         /* For performance, cleanup cache object after releasing the lock */