Rework the semver-only PR (rack#83) to allow an arbitrary regex

There's no way that it would make sense to mandatorily switch from "any
digits" to "semver only" -- it would break existing users, and I'm sure it
would just annoy an alternate segment of the population.  Instead, we keep
the default behaviour, and if you want to go semver-only or some such, you
can define your own regex to do whatever you want.

I also tidied up the test fixtures a bit, and completely rewrote the
StaticCache test suite to be properly structured.  You're welcome.

lib/rack/contrib/static_cache.rb

@@ -24,6 +24,12 @@ module Rack
   #
   # You can use Rack::Deflater along with Rack::StaticCache for further improvements in page loading time.
   #
+  # If you'd like to use a non-standard version identifier in your URLs, you
+  # can set the regex to remove with the `:version_regex` option.  If you
+  # want to capture something after the regex (such as file extensions), you
+  # should capture that as `\1`.  All other captured subexpressions will be
+  # discarded.  You may find the `?:` capture modifier helpful.
+  #
   # Examples:
   #     use Rack::StaticCache, :urls => ["/images", "/css", "/js", "/documents*"], :root => "statics"
   #     will serve all requests beginning with /images, /css or /js from the
@@ -57,32 +63,29 @@ def initialize(app, options={})
       root = options[:root] || Dir.pwd
       @file_server = Rack::File.new(root)
       @cache_duration = options[:duration] || 1
-      @versioning_enabled = true
-      @versioning_enabled = options[:versioning] unless options[:versioning].nil?
+      @versioning_enabled = options.fetch(:versioning, true)
+      if @versioning_enabled
+        @version_regex = options.fetch(:version_regex, /-[\d.]+([.][a-zA-Z][\w]+)?$/)
+      end
       @duration_in_seconds = self.duration_in_seconds
       @duration_in_words    = self.duration_in_words
     end
 
     def call(env)
       path = env["PATH_INFO"]
       url = @urls.detect{ |u| path.index(u) == 0 }
-      unless url.nil?
+      if url.nil?
+        @app.call(env)
+      else
         if @versioning_enabled
-          path.sub!(/
-            -               # a literal dash
-            (\d+\.\d+\.\d+) # basic MAJOR.MINOR.PATCH semver
-            (\.[0-9a-z]+)?  # an optional file extension containing numbers or letters
-            \z              # end of string
-          /x, '\1')         # /x allows spaces in regex for comments; \1 tells sub! to delete the match
+          path.sub!(@version_regex, '\1')
         end
         status, headers, body = @file_server.call(env)
         if @no_cache[url].nil?
           headers['Cache-Control'] ="max-age=#{@duration_in_seconds}, public"
           headers['Expires'] = @duration_in_words
         end
         [status, headers, body]
-      else
-        @app.call(env)
       end
     end
 

test/spec_rack_static_cache.rb

@@ -11,91 +11,135 @@ def call(env)
 end
 
 describe "Rack::StaticCache" do
-
-  before do
-    @root = ::File.expand_path(::File.dirname(__FILE__))
+  def static_root
+    ::File.expand_path(::File.dirname(__FILE__))
   end
 
-  it "should serve files with required headers" do
-    default_app_request
-    res = @request.get("/statics/test")
-    res.ok?.must_equal(true)
-    res.body.must_match(/rubyrack/)
-    res.headers['Cache-Control'].must_equal 'max-age=31536000, public'
-    next_year = Time.now().year + 1
-    res.headers['Expires'].must_match(Regexp.new(
-        "[A-Z][a-z]{2}[,][\s][0-9]{2}[\s][A-Z][a-z]{2}[\s]" << "#{next_year}" <<
-        "[\s][0-9]{2}[:][0-9]{2}[:][0-9]{2} GMT$"))
+  def request(options)
+    options = { :root => static_root }.merge(options)
+    Rack::MockRequest.new(Rack::StaticCache.new(DummyApp.new, options))
   end
 
-  it "should return 404s if url root is known but it can't find the file" do
-    default_app_request
-    res = @request.get("/statics/foo")
-    res.not_found?.must_equal(true)
+  describe "with a default app request" do
+    def get_request(path)
+      request(:urls => ["/statics"]).get(path)
+    end
+
+    it "should serve the request successfully" do
+      get_request("/statics/test").ok?.must_equal(true)
+    end
+
+    it "should serve the correct file contents" do
+      get_request("/statics/test").body.must_match(/rubyrack/)
+    end
+
+    it "should serve the correct file contents for a file with an extension" do
+      get_request("/statics/test.html").body.must_match(/extensions rule!/)
+    end
+
+    it "should set a long Cache-Control max-age" do
+      get_request("/statics/test").headers['Cache-Control'].must_equal 'max-age=31536000, public'
+    end
+
+    it "should set a long-distant Expires header" do
+      next_year = Time.now().year + 1
+      get_request("/statics/test").headers['Expires'].must_match(
+        Regexp.new(
+          "[A-Z][a-z]{2}[,][\s][0-9]{2}[\s][A-Z][a-z]{2}[\s]" <<
+          "#{next_year}" <<
+          "[\s][0-9]{2}[:][0-9]{2}[:][0-9]{2} GMT$"
+        )
+      )
+    end
+
+    it "should return 404s if url root is known but it can't find the file" do
+      get_request("/statics/non-existent").not_found?.must_equal(true)
+    end
+
+    it "should call down the chain if url root is not known" do
+      res = get_request("/something/else")
+      res.ok?.must_equal(true)
+      res.body.must_equal "Hello World"
+    end
+
+    it "should serve files if requested with version number" do
+      res = get_request("/statics/test-0.0.1")
+      res.ok?.must_equal(true)
+    end
+
+    it "should serve the correct file contents for a file with an extension requested with a version" do
+      get_request("/statics/test-0.0.1.html").body.must_match(/extensions rule!/)
+    end
   end
 
-  it "should call down the chain if url root is not known" do
-    default_app_request
-    res = @request.get("/something/else")
-    res.ok?.must_equal(true)
-    res.body.must_equal "Hello World"
-  end
+  describe "with a custom version number regex" do
+    def get_request(path)
+      request(:urls => ["/statics"], :version_regex => /-[0-9a-f]{8}/).get(path)
+    end
 
-  it "should serve files if requested with version number and versioning is enabled" do
-    default_app_request
-    res = @request.get("/statics/test-0.0.1")
-    res.ok?.must_equal(true)
-  end
+    it "should handle requests with the custom regex" do
+      get_request("/statics/test-deadbeef").ok?.must_equal(true)
+    end
 
-  it "should change cache duration if specified thorugh option" do
-    configured_app_request
-    res = @request.get("/statics/test")
-    res.ok?.must_equal(true)
-    res.body.must_match(/rubyrack/)
-    next_next_year = Time.now().year + 2
-    res.headers['Expires'].must_match(Regexp.new("#{next_next_year}"))
-  end
+    it "should handle extensioned requests for the custom regex" do
+      get_request("/statics/test-deadbeef.html").body.must_match(/extensions rule!/)
+    end
 
-  it "should round max-age if duration is part of a year" do
-    one_week_duration_app_request
-    res = @request.get("/statics/test")
-    res.ok?.must_equal(true)
-    res.body.must_match(/rubyrack/)
-    res.headers['Cache-Control'].must_equal "max-age=606461, public"
+    it "should not handle requests for the default version regex" do
+      get_request("/statics/test-0.0.1").ok?.must_equal(false)
+    end
   end
 
-  it "should return 404s if requested with version number but versioning is disabled" do
-    configured_app_request
-    res = @request.get("/statics/test-0.0.1")
-    res.not_found?.must_equal(true)
-  end
+  describe "with custom cache duration" do
+    def get_request(path)
+      request(:urls => ["/statics"], :duration => 2).get(path)
+    end
 
-  it "should serve files with plain headers when * is added to the directory name" do
-    configured_app_request
-    res = @request.get("/documents/test")
-    res.ok?.must_equal(true)
-    res.body.must_match(/nocache/)
-    next_next_year = Time.now().year + 2
-    res.headers['Expires'].wont_match(Regexp.new("#{next_next_year}"))
+    it "should change cache duration" do
+      next_next_year = Time.now().year + 2
+      get_request("/statics/test").headers['Expires'].must_match(Regexp.new("#{next_next_year}"))
+    end
   end
 
-  def default_app_request
-    @options = {:urls => ["/statics"], :root => @root}
-    request
-  end
+  describe "with partial-year cache duration" do
+    def get_request(path)
+      request(:urls => ["/statics"], :duration => 1.0 / 52).get(path)
+    end
 
-  def one_week_duration_app_request
-    @options = {:urls => ["/statics"], :root => @root, :duration => 1.fdiv(52)}
-    request
+    it "should round max-age if duration is part of a year" do
+      get_request("/statics/test").headers['Cache-Control'].must_equal "max-age=606461, public"
+    end
   end
 
-  def configured_app_request
-    @options = {:urls => ["/statics", "/documents*"], :root => @root, :versioning => false, :duration => 2}
-    request
-  end
+  describe "with versioning disabled" do
+    def get_request(path)
+      request(:urls => ["/statics"], :versioning => false).get(path)
+    end
 
-  def request
-    @request = Rack::MockRequest.new(Rack::StaticCache.new(DummyApp.new, @options))
+    it "should return 404s if requested with version number" do
+      get_request("/statics/test-0.0.1").not_found?.must_equal(true)
+    end
   end
 
+  describe "with * suffix on directory name" do
+    def get_request(path)
+      request(:urls => ["/statics*"]).get(path)
+    end
+
+    it "should serve files OK" do
+      get_request("/statics/test").ok?.must_equal(true)
+    end
+
+    it "should serve the content" do
+      get_request("/statics/test").body.must_match(/rubyrack/)
+    end
+
+    it "should not set a max-age" do
+      get_request("/statics/test").headers['Cache-Control'].must_equal(nil)
+    end
+
+    it "should not set an Expires header" do
+      get_request("/statics/test").headers['Expires'].must_equal(nil)
+    end
+  end
 end

test/spec_rack_try_static.rb

@@ -22,23 +22,23 @@ def request(options = {})
 describe "Rack::TryStatic" do
   describe 'when file cannot be found' do
     it 'should call call app' do
-      res = request(build_options(:try => ['html'])).get('/documents')
+      res = request(build_options(:try => ['html'])).get('/statics')
       res.ok?.must_equal(true)
       res.body.must_equal "Hello World"
     end
   end
 
   describe 'when file can be found' do
     it 'should serve first found' do
-      res = request(build_options(:try => ['.html', '/index.html', '/index.htm'])).get('/documents')
+      res = request(build_options(:try => ['.html', '/index.html', '/index.htm'])).get('/statics')
       res.ok?.must_equal(true)
       res.body.strip.must_equal "index.html"
     end
   end
 
   describe 'when path_info maps directly to file' do
     it 'should serve existing' do
-      res = request(build_options(:try => ['/index.html'])).get('/documents/existing.html')
+      res = request(build_options(:try => ['/index.html'])).get('/statics/existing.html')
       res.ok?.must_equal(true)
       res.body.strip.must_equal "existing.html"
     end
@@ -48,8 +48,8 @@ def request(options = {})
     it 'should not mutate given options' do
       org_options = build_options  :try => ['/index.html']
       given_options = org_options.dup
-      request(given_options).get('/documents').ok?.must_equal(true)
-      request(given_options).get('/documents').ok?.must_equal(true)
+      request(given_options).get('/statics').ok?.must_equal(true)
+      request(given_options).get('/statics').ok?.must_equal(true)
       given_options.must_equal org_options
     end
   end

test/statics/test.html

@@ -0,0 +1 @@
+extensions rule!