respect purge requests when serving ipro requests from ngx_pagespeed (#1193)

* respect purge requests when serving ipro requests from ngx_pagespeed

* Add TODO to share common base with ApacheFetch.

src/ngx_base_fetch.cc

@@ -23,6 +23,8 @@
 
 #include "ngx_pagespeed.h"
 
+#include "net/instaweb/rewriter/public/rewrite_driver.h"
+#include "net/instaweb/rewriter/public/rewrite_options.h"
 #include "net/instaweb/rewriter/public/rewrite_stats.h"
 #include "pagespeed/kernel/base/google_message_handler.h"
 #include "pagespeed/kernel/base/message_handler.h"
@@ -38,14 +40,18 @@ const char kDone = 'D';
 NgxEventConnection* NgxBaseFetch::event_connection = NULL;
 int NgxBaseFetch::active_base_fetches = 0;
 
-NgxBaseFetch::NgxBaseFetch(ngx_http_request_t* r,
+NgxBaseFetch::NgxBaseFetch(StringPiece url,
+                           ngx_http_request_t* r,
                            NgxServerContext* server_context,
                            const RequestContextPtr& request_ctx,
                            PreserveCachingHeaders preserve_caching_headers,
-                           NgxBaseFetchType base_fetch_type)
+                           NgxBaseFetchType base_fetch_type,
+                           const RewriteOptions* options)
     : AsyncFetch(request_ctx),
+      url_(url.data(), url.size()),
       request_(r),
       server_context_(server_context),
+      options_(options),
       done_called_(false),
       last_buf_sent_(false),
       references_(2),
@@ -331,4 +337,9 @@ void NgxBaseFetch::HandleDone(bool success) {
   DecrefAndDeleteIfUnreferenced();
 }
 
+bool NgxBaseFetch::IsCachedResultValid(const ResponseHeaders& headers) {
+  return OptionsAwareHTTPCacheCallback::IsCacheValid(
+      url_, *options_, request_context(), headers);
+}
+
 }  // namespace net_instaweb

src/ngx_base_fetch.h

@@ -43,6 +43,9 @@
 //    events it handles.
 //
 // When the last reference is dropped, this class will delete itself.
+//
+// TODO(jmarantz): consider sharing the cache-invalidation infrastructure
+// with ApacheFetch, using a common base class.
 
 #ifndef NGX_BASE_FETCH_H_
 #define NGX_BASE_FETCH_H_
@@ -59,6 +62,7 @@ extern "C" {
 #include "ngx_server_context.h"
 
 #include "net/instaweb/http/public/async_fetch.h"
+#include "net/instaweb/rewriter/public/rewrite_options.h"
 #include "pagespeed/kernel/base/string.h"
 #include "pagespeed/kernel/http/headers.h"
 
@@ -74,10 +78,12 @@ enum NgxBaseFetchType {
 
 class NgxBaseFetch : public AsyncFetch {
  public:
-  NgxBaseFetch(ngx_http_request_t* r, NgxServerContext* server_context,
+  NgxBaseFetch(StringPiece url, ngx_http_request_t* r,
+               NgxServerContext* server_context,
                const RequestContextPtr& request_ctx,
                PreserveCachingHeaders preserve_caching_headers,
-               NgxBaseFetchType base_fetch_type);
+               NgxBaseFetchType base_fetch_type,
+               const RewriteOptions* options);
   virtual ~NgxBaseFetch();
 
   // Statically initializes event_connection, require for PSOL and nginx to
@@ -125,6 +131,8 @@ class NgxBaseFetch : public AsyncFetch {
   ngx_http_request_t* request() { return request_; }
   NgxBaseFetchType base_fetch_type() { return base_fetch_type_; }
 
+  bool IsCachedResultValid(const ResponseHeaders& headers) override;
+
  private:
   virtual bool HandleWrite(const StringPiece& sp, MessageHandler* handler);
   virtual bool HandleFlush(MessageHandler* handler);
@@ -156,9 +164,11 @@ class NgxBaseFetch : public AsyncFetch {
   // Live count of NgxBaseFetch instances that are currently in use.
   static int active_base_fetches;
 
+  GoogleString url_;
   ngx_http_request_t* request_;
   GoogleString buffer_;
   NgxServerContext* server_context_;
+  const RewriteOptions* options_;
   bool done_called_;
   bool last_buf_sent_;
   // How many active references there are to this fetch. Starts at two,

src/ngx_pagespeed.cc

@@ -1635,10 +1635,12 @@ void ps_release_base_fetch(ps_request_ctx_t* ctx) {
 }
 
 // TODO(chaizhenhua): merge into NgxBaseFetch ctor
-void ps_create_base_fetch(ps_request_ctx_t* ctx,
+void ps_create_base_fetch(StringPiece url,
+                          ps_request_ctx_t* ctx,
                           RequestContextPtr request_context,
                           RequestHeaders* request_headers,
-                               NgxBaseFetchType type) {
+                          NgxBaseFetchType type,
+                          const RewriteOptions* options) {
   CHECK(ctx->base_fetch == NULL) << "Pre-existing base fetch!";
 
   ngx_http_request_t* r = ctx->r;
@@ -1648,9 +1650,9 @@ void ps_create_base_fetch(ps_request_ctx_t* ctx,
   // it, and call Done() on the associated parent (Proxy or Resource) fetch. If
   // we fail before creating the associated fetch then we need to call Done() on
   // the BaseFetch ourselves.
-  ctx->base_fetch = new NgxBaseFetch(r, cfg_s->server_context,
-                                     request_context,
-                                     ctx->preserve_caching_headers, type);
+  ctx->base_fetch = new NgxBaseFetch(url, r, cfg_s->server_context, request_context,
+                                     ctx->preserve_caching_headers, type,
+                                     options);
   ctx->base_fetch->SetRequestHeadersTakingOwnership(request_headers);
 }
 
@@ -1915,16 +1917,17 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
   if (pagespeed_resource) {
     // TODO(jefftk): Set using_spdy appropriately.  See
     // ProxyInterface::ProxyRequestCallback
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kPageSpeedResource);
+    ps_create_base_fetch(url.Spec(), ctx, request_context,
+                         request_headers.release(), kPageSpeedResource,
+                         options);
     ResourceFetch::Start(
         url,
         custom_options.release() /* null if there aren't custom options */,
         cfg_s->server_context, ctx->base_fetch);
     return ps_async_wait_response(r);
   } else if (is_an_admin_handler) {
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kAdminPage);
+    ps_create_base_fetch(url.Spec(), ctx, request_context,
+                         request_headers.release(), kAdminPage, options);
     QueryParams query_params;
     query_params.ParseFromUrl(url);
 
@@ -1972,8 +1975,8 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
 
     if (options->domain_lawyer()->MapOriginUrl(
             url, &mapped_url, &host_header, &is_proxy) && is_proxy) {
-      ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                          kPageSpeedProxy);
+      ps_create_base_fetch(url.Spec(), ctx, request_context,
+                           request_headers.release(), kPageSpeedProxy, options);
 
       RewriteDriver* driver;
       if (custom_options.get() == NULL) {
@@ -1997,8 +2000,8 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
   }
 
   if (html_rewrite) {
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kHtmlTransform);
+    ps_create_base_fetch(url.Spec(), ctx, request_context,
+                         request_headers.release(), kHtmlTransform, options);
     // Do not store driver in request_context, it's not safe.
     RewriteDriver* driver;
 
@@ -2041,8 +2044,8 @@ ngx_int_t ps_resource_handler(ngx_http_request_t* r,
   if (options->in_place_rewriting_enabled() &&
       options->enabled() &&
       options->IsAllowed(url.Spec())) {
-    ps_create_base_fetch(ctx, request_context, request_headers.release(),
-                         kIproLookup);
+    ps_create_base_fetch(url.Spec(), ctx, request_context, request_headers.release(),
+                         kIproLookup, options);
 
     // Do not store driver in request_context, it's not safe.
     RewriteDriver* driver;

test/nginx_system_test.sh

@@ -48,6 +48,7 @@ PRIMARY_HOSTNAME="localhost:$PRIMARY_PORT"
 SECONDARY_HOSTNAME="localhost:$SECONDARY_PORT"
 
 SERVER_ROOT="$MOD_PAGESPEED_DIR/src/install/"
+export APACHE_DOC_ROOT="$SERVER_ROOT"
 
 # We need check and check_not before we source SYSTEM_TEST_FILE that provides
 # them.