Optimized middleware implementation and specs

ahmadvaroqua · Dec 10, 2010 · 87addbd · 87addbd
1 parent 9e127ca
commit 87addbd
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 33 deletions.
diff --git a/lib/pdfkit/middleware.rb b/lib/pdfkit/middleware.rb
@@ -11,8 +11,8 @@ def initialize(app, options = {}, conditions = {})
     def call(env)
       @request    = Rack::Request.new(env)
       @render_pdf = false
-      status, headers, response = @app.call(env)
       set_request_to_render_as_pdf(env) if render_as_pdf?
+      status, headers, response = @app.call(env)
 
       if @render_pdf && headers['Content-Type'] =~ /text\/html|application\/xhtml\+xml/
         body = response.respond_to?(:body) ? response.body : response.join
@@ -40,7 +40,7 @@ def translate_paths(body, env)
     end
 
     def render_as_pdf?
-      request_path_is_pdf = @request.path.match(/\.pdf$/)
+      request_path_is_pdf = @request.path.match(%r{\.pdf$})
 
       if request_path_is_pdf && @conditions[:only]
         rules = [@conditions[:only]].flatten
@@ -58,9 +58,8 @@ def render_as_pdf?
 
     def set_request_to_render_as_pdf(env)
       @render_pdf = true
-
-      path = Pathname(env['PATH_INFO'])
-      %w[PATH_INFO REQUEST_URI].each { |e| env[e] = path.to_s.sub(/#{path.extname}$/,'') }
+      path = @request.path.sub(%r{\.pdf$}, '')
+      %w[PATH_INFO REQUEST_URI].each { |e| env[e] = path }
       env['HTTP_ACCEPT'] = concat(env['HTTP_ACCEPT'], Rack::Mime.mime_type('.html'))
     end
 

diff --git a/spec/middleware_spec.rb b/spec/middleware_spec.rb
@@ -4,10 +4,9 @@ def app; Rack::Lint.new(@app); end
 
 def mock_app(options = {}, conditions = {})
   main_app = lambda { |env|
-    request = Rack::Request.new(env)
+    @env = env
     headers = {'Content-Type' => "text/html"}
-    headers['Set-Cookie'] = "id=1; path=/\ntoken=abc; path=/; secure; HttpOnly"
-    [200, headers, ['Hello world!']]
+    [200, headers, @body || ['Hello world!']]
   }
 
   builder = Rack::Builder.new
@@ -108,51 +107,49 @@ def mock_app(options = {}, conditions = {})
 
       end
     end
+
+    describe "remove .pdf from PATH_INFO and REQUEST_URI" do
+      before { mock_app }
+
+      context "matching" do
+        specify do
+          get 'http://www.example.org/public/file.pdf'
+          @env["PATH_INFO"].should == "/public/file"
+          @env["REQUEST_URI"].should == "/public/file"
+        end
+        specify do
+          get 'http://www.example.org/public/file.txt'
+          @env["PATH_INFO"].should == "/public/file.txt"
+          @env["REQUEST_URI"].should be_nil
+        end
+      end
+
+    end
   end
 
   describe "#translate_paths" do
-
     before do
       @pdf = PDFKit::Middleware.new({})
-      @env = {'REQUEST_URI' => 'http://example.com/document.pdf', 'rack.url_scheme' => 'http', 'HTTP_HOST' => 'example.com'}
+      @env = { 'REQUEST_URI' => 'http://example.com/document.pdf', 'rack.url_scheme' => 'http', 'HTTP_HOST' => 'example.com' }
     end
-
+    
     it "should correctly parse relative url with single quotes" do
       @body = %{<html><head><link href='/stylesheets/application.css' media='screen' rel='stylesheet' type='text/css' /></head><body><img alt='test' src="/test.png" /></body></html>}
       body = @pdf.send :translate_paths, @body, @env
       body.should == "<html><head><link href='http://example.com/stylesheets/application.css' media='screen' rel='stylesheet' type='text/css' /></head><body><img alt='test' src=\"http://example.com/test.png\" /></body></html>"
     end
-
+    
     it "should correctly parse relative url with double quotes" do
       @body = %{<link href="/stylesheets/application.css" media="screen" rel="stylesheet" type="text/css" />}
       body = @pdf.send :translate_paths, @body, @env
       body.should == "<link href=\"http://example.com/stylesheets/application.css\" media=\"screen\" rel=\"stylesheet\" type=\"text/css\" />"
     end
-
+    
     it "should return the body even if there are no valid substitutions found" do
       @body = "NO MATCH"
       body = @pdf.send :translate_paths, @body, @env
       body.should == "NO MATCH"
     end
   end
 
-  describe "#set_request_to_render_as_pdf" do
-
-    before do      
-      @pdf = PDFKit::Middleware.new({})
-
-      @pdf_env = {'PATH_INFO' => Pathname.new("file.pdf"), 'REQUEST_URI' => Pathname.new("file.pdf")}
-      @non_pdf_env = {'PATH_INFO' => Pathname.new("file.txt"), 'REQUEST_URI' => Pathname.new("file.txt")}
-    end
-
-    it "should replace .pdf in PATH_INFO when the extname is .pdf" do
-      @pdf.send :set_request_to_render_as_pdf, @pdf_env
-      @pdf_env['PATH_INFO'].should == "file"
-    end
-
-    it "should replace .pdf in REQUEST_URI when the extname is .pdf" do
-      @pdf.send :set_request_to_render_as_pdf, @pdf_env
-      @pdf_env['REQUEST_URI'].should == "file"
-    end    
-  end
-end
+end