Ticket #626 (new enhancement)

Opened 5 months ago

Last modified 4 months ago

[PATCH] ResponseCache.sendfile_proc, allows different sendfile implementations (e.g. Nginx X-Accel-Redirect)

Reported by: evansj Assigned to:
Priority: minor Milestone: 0.6.8 Incise
Component: core Keywords:
Cc:

Description

I have created a patch for ResponseCache. It allows you to configure a Proc object to handle X-Sendfile processing. This is needed for Nginx, for example, because Nginx supports X-Accel-Redirect but not X-Sendfile.

The proc object gets supplied with 4 parameters: request, response, page_cache_dir and cache filename.

The unit test I've included has an implementation which works for Nginx.

I'm using the patch in production but I've applied the patch to Radiant-0.6.4. in 0.6.4, page_cache_path() in response_cache.rb is broken for the root path "/". This is fixed in 0.6.5. I've added a unit test for that as well.

Sample code for environment.rb:

ResponseCache.defaults[:sendfile_proc] = lambda { |request, response, cache_dir, file|
  filename = file.slice(File.expand_path(cache_dir).length, file.length)
  response.headers.merge!('X-Accel-Redirect' => "/cache#{filename}.data")
}

and your nginx config file:

location /cache {
        internal;
        root /var/www/apps/example/current/cache/;
}

Change History

03/06/08 11:12:08 changed by evansj

file attachments seem to be broken.

Patch is:

Index: test/unit/response_cache_test.rb
===================================================================
--- test/unit/response_cache_test.rb	(revision 770)
+++ test/unit/response_cache_test.rb	(working copy)
@@ -152,7 +152,35 @@
     assert_equal 'text/plain', cached.headers['Content-Type']
     assert_kind_of TestResponse, result
   end
+
+  def test_page_cache_path
+    assert_equal("#{@dir}/_site-root", @cache.send(:page_cache_path, ""))
+    assert_equal("#{@dir}/_site-root", @cache.send(:page_cache_path, "/"))
+    assert_equal("#{@dir}/foo", @cache.send(:page_cache_path, "foo"))
+    assert_equal("#{@dir}/foo", @cache.send(:page_cache_path, "/foo"))
+  end
   
+  def test_send_using_sendfile_proc
+    @cache.sendfile_proc = lambda { |request, response, cache_dir, file|
+      filename = file.slice(File.expand_path(cache_dir).length, file.length)
+      response.headers.merge!('X-Accel-Redirect' => "/cache#{filename}.data")
+    }
+    
+    result = @cache.cache_response('test', response('content', 'Content-Type' => 'text/plain'))
+    cached = @cache.update_response('test', response, ActionController::TestRequest)
+    assert_equal '', cached.body
+    assert_equal "/cache/test.data", cached.headers['X-Accel-Redirect']
+    assert_equal 'text/plain', cached.headers['Content-Type']
+    assert_kind_of TestResponse, result
+    
+    result = @cache.cache_response('/', response('content', 'Content-Type' => 'text/plain'))
+    cached = @cache.update_response('/', response, ActionController::TestRequest)
+    assert_equal '', cached.body
+    assert_equal "/cache/_site-root.data", cached.headers['X-Accel-Redirect']
+    assert_equal 'text/plain', cached.headers['Content-Type']
+    assert_kind_of TestResponse, result
+  end
+  
   def test_send_cached_page_with_last_modified
     last_modified = Time.now.httpdate
     result = @cache.cache_response('test', response('content', 'Last-Modified' => last_modified))
Index: app/models/response_cache.rb
===================================================================
--- app/models/response_cache.rb	(revision 770)
+++ app/models/response_cache.rb	(working copy)
@@ -7,16 +7,18 @@
     :default_extension => '.yml',
     :perform_caching => true,
     :logger => ActionController::Base.logger,
-    :use_x_sendfile => false
+    :use_x_sendfile => false,
+    :sendfile_proc => nil
   }
   cattr_accessor :defaults
   
-  attr_accessor :directory, :expire_time, :default_extension, :perform_caching, :logger, :use_x_sendfile
+  attr_accessor :directory, :expire_time, :default_extension, :perform_caching,
+    :logger, :use_x_sendfile, :sendfile_proc
   alias :page_cache_directory :directory
   alias :page_cache_extension :default_extension
   private :benchmark, :silence, :page_cache_directory,
     :page_cache_extension 
-    
+
   # Creates a ResponseCache object with the specified options.
   #
   # Options are as follows:
@@ -26,6 +28,7 @@
   # :perform_caching   :: boolean value that turns caching on or off (defaults to true)
   # :logger            :: the application logging object (defaults to ActionController::Base.logger)
   # :use_x_sendfile    :: use X-Sendfile headers to speed up transfer of cached pages (not available on all web servers)
+  # :sendfile_proc     :: a Proc object to use for custom X-Sendfile implementations
   # 
   def initialize(options = {})
     options = options.symbolize_keys.reverse_merge(defaults)
@@ -35,6 +38,7 @@
     self.perform_caching   = options[:perform_caching]
     self.logger            = options[:logger]
     self.use_x_sendfile    = options[:use_x_sendfile]
+    self.sendfile_proc     = options[:sendfile_proc]
   end
   
   # Caches a response object for path to disk.
@@ -106,6 +110,8 @@
         response.headers.merge!(metadata['headers'] || {})
         if client_has_cache?(metadata, request)
           response.headers.merge!('Status' => '304 Not Modified')
+        elsif sendfile_proc
+          sendfile_proc[request, response, page_cache_directory, file_path]
         elsif use_x_sendfile
           response.headers.merge!('X-Sendfile' => "#{file_path}.data")
         else

03/29/08 16:01:57 changed by seancribbs

  • priority changed from major to minor.
  • milestone set to 0.6.6.

This looks good, but could you rewrite your tests as specs? Also it might be nice to do something like

ResponseCache.server_software = :nginx 

so as to simplify the process.