Improved conditional response code from Dan Kubb. Handles ETag on 304 responses better.

git-svn-id: svn+ssh://rubyforge.org/var/svn/mongrel/trunk@173 19e92222-5c0b-0410-8929-a290d50e31e9

3 files changed, 14 insertions, 7 deletions

diff --git a/lib/mongrel/command.rb b/lib/mongrel/command.rb
index d538010..2277063 100644
--- a/lib/mongrel/command.rb
+++ b/lib/mongrel/command.rb
@@ -48,11 +48,11 @@ module Mongrel
           puts @opt
         end
         
-        # I need to add my own -v definition to prevent the -h from exiting by default as well.
+        # I need to add my own -v definition to prevent the -v from exiting by default as well.
         @opt.on_tail("--version", "Show version") do
           @done_validating = true
           if VERSION
-            puts "Version #{MONGREL_VERSION}"
+            puts "Version #{Mongrel::Const::MONGREL_VERSION}"
           end
         end
         
diff --git a/lib/mongrel/handlers.rb b/lib/mongrel/handlers.rb
index de328b7..d71e151 100644
--- a/lib/mongrel/handlers.rb
+++ b/lib/mongrel/handlers.rb
@@ -179,14 +179,15 @@ module Mongrel
         else unmodified_since || none_match  # validation successful if we get this far and at least one of the header exists
       end
 
+      header = response.header
+      header[Const::ETAG] = etag
+
       if same_response
         response.start(304) {}
       else
         # first we setup the headers and status then we do a very fast send on the socket directly
         response.status = 200
-        header = response.header
         header[Const::LAST_MODIFIED] = mtime.httpdate
-        header[Const::ETAG] = etag
 
         # set the mime type from our map based on the ending
         dot_at = req_path.rindex('.')
diff --git a/test/test_conditional.rb b/test/test_conditional.rb
index cccf976..42791ba 100644
--- a/test/test_conditional.rb
+++ b/test/test_conditional.rb
@@ -15,8 +15,8 @@ class HttpParserTest < Test::Unit::TestCase
     # get the ETag and Last-Modified headers
     @path = '/README'
     res = @http.start { |http| http.get(@path) }
-    @etag = res['ETag']
-    @last_modified = res['Last-Modified']
+    assert_not_nil @etag = res['ETag']
+    assert_not_nil @last_modified = res['Last-Modified']
   end
 
   def teardown
@@ -94,7 +94,13 @@ class HttpParserTest < Test::Unit::TestCase
     # assert the response status is correct for GET and HEAD
     def assert_status_for_get_and_head(status_class, headers = {})
       %w{ get head }.each do |method|
-        assert_kind_of status_class, @http.send(method, @path, headers)
+        res = @http.send(method, @path, headers)
+        assert_kind_of status_class, res
+        assert_equal @etag, res['ETag']
+        case status_class
+          when Net::HTTPNotModified : assert_nil res['Last-Modified']
+          when Net::HTTPOK          : assert_equal @last_modified, res['Last-Modified']
+        end
       end
     end
 end