Refactor redirect support out of RailsSession & SinatraSession and into Session#request_page

lib/webrat/core/session.rb

@@ -8,7 +8,7 @@ module Webrat
   # A page load or form submission returned an unsuccessful response code (500-599)
   class PageLoadError < WebratError
   end
-  
+
   def self.session_class
     case Webrat.configuration.mode
     when :rails
@@ -37,29 +37,29 @@ For example:
       STR
     end
   end
-  
+
   class Session
     extend Forwardable
     include Logging
     include SaveAndOpenPage
-    
+
     attr_reader :current_url
     attr_reader :elements
-    
+
     def initialize(context = nil) #:nodoc:
       @http_method     = :get
       @data            = {}
       @default_headers = {}
       @custom_headers  = {}
       @context         = context
-      
+
       reset
     end
-    
+
     def current_dom #:nodoc:
       current_scope.dom
     end
-    
+
     # For backwards compatibility -- removing in 1.0
     def current_page #:nodoc:
       page = OpenStruct.new
@@ -68,11 +68,11 @@ For example:
       page.data = @data
       page
     end
-    
+
     def doc_root #:nodoc:
       nil
     end
-    
+
     def header(key, value)
       @custom_headers[key] = value
     end
@@ -80,7 +80,7 @@ For example:
     def http_accept(mime_type)
       header('Accept', Webrat::MIME.mime_type(mime_type))
     end
-    
+
     def basic_auth(user, pass)
       encoded_login = ["#{user}:#{pass}"].pack("m*")
       header('HTTP_AUTHORIZATION', "Basic #{encoded_login}")
@@ -103,28 +103,30 @@ For example:
 
       save_and_open_page if exception_caught? && Webrat.configuration.open_error_files?
       raise PageLoadError.new("Page load was not successful (Code: #{response_code.inspect}):\n#{formatted_error}") unless success_code?
-      
+
       reset
-      
+
       @current_url  = url
       @http_method  = http_method
       @data         = data
-      
+
+      request_page(response.location, :get, data) if response.redirect?
+
       return response
     end
-    
+
     def success_code? #:nodoc:
       (200..499).include?(response_code)
     end
-    
+
     def exception_caught? #:nodoc:
       response_body =~ /Exception caught/
     end
-    
+
     def current_scope #:nodoc:
       scopes.last || page_scope
     end
-    
+
     # Reloads the last page requested. Note that this will resubmit forms
     # and their data.
     def reloads
@@ -132,10 +134,10 @@ For example:
     end
 
     webrat_deprecate :reload, :reloads
-      
+
-    
+
     # Works like click_link, but only looks for the link text within a given selector
-    # 
+    #
     # Example:
     #   click_link_within "#user_12", "Vote"
     def click_link_within(selector, link_text)
@@ -145,14 +147,14 @@ For example:
     end
 
     webrat_deprecate :clicks_link_within, :click_link_within
-    
+
     def within(selector)
       scopes.push(Scope.from_scope(self, current_scope, selector))
       ret = yield(current_scope)
       scopes.pop
       return ret
     end
-    
+
     # Issues a GET request for a page, follows any redirects, and verifies the final page
     # load was successful.
     #
@@ -161,7 +163,7 @@ For example:
     def visit(url = nil, http_method = :get, data = {})
       request_page(url, http_method, data)
     end
-    
+
     webrat_deprecate :visits, :visit
 
     # Subclasses can override this to show error messages without html
@@ -176,25 +178,25 @@ For example:
     def page_scope #:nodoc:
       @_page_scope ||= Scope.from_page(self, response, response_body)
     end
-    
+
     def dom
       page_scope.dom
     end
-    
+
     def xml_content_type?
       false
     end
-    
+
     def simulate
       return if Webrat.configuration.mode == :selenium
       yield
     end
-    
+
     def automate
       return unless Webrat.configuration.mode == :selenium
       yield
     end
-    
+
     def_delegators :current_scope, :fill_in,            :fills_in
     def_delegators :current_scope, :set_hidden_field
     def_delegators :current_scope, :submit_form
@@ -213,14 +215,14 @@ For example:
     def_delegators :current_scope, :field_by_xpath
     def_delegators :current_scope, :field_with_id
     def_delegators :current_scope, :select_option
-    
+
   private
-    
+
     def reset
       @elements     = {}
       @_scopes      = nil
       @_page_scope  = nil
     end
-    
+
   end
 end

lib/webrat/rails.rb

@@ -50,10 +50,7 @@ module Webrat
 
     def do_request(http_method, url, data, headers) #:nodoc:
       update_protocol(url)
-
       integration_session.send(http_method, normalize_url(url), data, headers)
-      integration_session.follow_redirect_with_headers(headers) while integration_session.internal_redirect?
-      integration_session.status
     end
 
     # remove protocol, host and anchor
@@ -82,27 +79,6 @@ module Webrat
 end
 
 module ActionController #:nodoc:
-  module Integration #:nodoc:
-    class Session #:nodoc:
-      def internal_redirect?
-        redirect? && response.redirect_url_match?(host)
-      end
-
-      def follow_redirect_with_headers(h = {})
-        raise "Not a redirect! #{@status} #{@status_message}" unless redirect?
-
-        h = Hash.new if h.nil?
-        h['HTTP_REFERER'] = request.url
-
-        location = headers["location"]
-        location = location.first if location.is_a?(Array)
-        
-        get(location, {}, h)
-        status
-      end
-    end
-  end
-
   IntegrationTest.class_eval do
     include Webrat::Methods
     include Webrat::Matchers

lib/webrat/sinatra.rb

@@ -11,7 +11,6 @@ module Webrat
         path, data, headers = *args
         params = data.merge({:env => headers || {}})
         self.__send__("#{verb}_it", path, params)
-        get_it(@response.location, params) while @response.redirect?
       end
     end
   end

spec/fakes/test_session.rb

@@ -2,33 +2,39 @@ module Webrat #:nodoc:
   def self.session_class #:nodoc:
     TestSession
   end
-  
+
   class TestSession < Session #:nodoc:
     attr_accessor :response_body
     attr_writer :response_code
-    
+
     def doc_root
       File.expand_path(File.join(".", "public"))
     end
-    
+
     def response
-      @response ||= Object.new
+      @response ||= TestResponse.new
     end
-    
+
     def response_code
       @response_code || 200
     end
-    
+
-    def get(url, data)
+    def get(url, data, headers = nil)
     end
-    
+
-    def post(url, data)
+    def post(url, data, headers = nil)
     end
-    
+
-    def put(url, data)
+    def put(url, data, headers = nil)
     end
-    
+
-    def delete(url, data)
+    def delete(url, data, headers = nil)
+    end
+  end
+  
+  class TestResponse #:nodoc:
+    def redirect?
+      false
     end
   end
 end

spec/private/core/session_spec.rb

@@ -1,47 +1,47 @@
 require File.expand_path(File.dirname(__FILE__) + "/../../spec_helper")
 
 describe Webrat::Session do
-  
+
   it "should not have a doc_root" do
     session = Webrat::Session.new
     session.doc_root.should be_nil
   end
-  
+
   it "should expose the current_dom" do
     session = Webrat::Session.new
-    
+
     def session.response
       Object.new
     end
-    
+
     def session.response_body
       "<html></html>"
     end
-    
+
     session.should respond_to(:current_dom)
   end
-  
+
   it "should open the page in the browser in MacOSX" do
     session = Webrat::Session.new
     session.stub!(:ruby_platform => 'darwin')
     session.should_receive(:`).with("open path")
     session.open_in_browser("path")
   end
-  
+
   it "should open the page in the browser in cygwin" do
     session = Webrat::Session.new
     session.stub!(:ruby_platform => 'i386-cygwin')
     session.should_receive(:`).with("rundll32 url.dll,FileProtocolHandler path\\to\\file")
     session.open_in_browser("path/to/file")
   end
-  
+
   it "should open the page in the browser in Win32" do
     session = Webrat::Session.new
     session.stub!(:ruby_platform => 'win32')
     session.should_receive(:`).with("rundll32 url.dll,FileProtocolHandler path\\to\\file")
     session.open_in_browser("path/to/file")
   end
-  
+
   it "should provide a current_page for backwards compatibility" do
     session = Webrat::Session.new
     current_page = session.current_page
@@ -58,7 +58,7 @@ describe Webrat::Session do
 
   it "should return a copy of the headers to be sent" do
     session = Webrat::Session.new
-    session.instance_eval { 
+    session.instance_eval {
       @default_headers = {'HTTP_X_FORWARDED_FOR' => '192.168.1.1'}
       @custom_headers = {'Accept' => 'application/xml'}
     }
@@ -89,17 +89,17 @@ describe Webrat::Session do
     before(:each) do
       webrat_session = Webrat::Session.new
     end
-  
+
     it "should raise an error if the request is not a success" do
       webrat_session.stub!(:get)
       webrat_session.stub!(:response_body => "Exception caught")
       webrat_session.stub!(:response_code => 500)
       webrat_session.stub!(:formatted_error => "application error")
       webrat_session.stub!(:save_and_open_page)
-  
+
       lambda { webrat_session.request_page('some url', :get, {}) }.should raise_error(Webrat::PageLoadError)
     end
-    
+
     it "should raise an error but not open if the request is not a success and config quashes save_and_open" do
       Webrat.configure do |config|
         config.open_error_files = false
@@ -109,8 +109,17 @@ describe Webrat::Session do
       webrat_session.stub!(:response_code => 500)
       webrat_session.stub!(:formatted_error => "application error")
       webrat_session.should_not_receive(:save_and_open_page)
-  
+
       lambda { webrat_session.request_page('some url', :get, {}) }.should raise_error(Webrat::PageLoadError)
     end
+
+    it "should follow redirects" do
+      webrat_session.response.should_receive(:redirect?).twice.and_return(true, false)
+      webrat_session.response.should_receive(:location).once.and_return("/newurl")
+
+      webrat_session.request_page("/oldurl", :get, {})
+
+      webrat_session.current_url.should == "/newurl"
+    end
   end
-end
+end

spec/private/rails/rails_session_spec.rb

@@ -6,8 +6,6 @@ describe Webrat::RailsSession do
   before :each do
     Webrat.configuration.mode = :rails
     @integration_session = mock("integration_session")
-    @integration_session.stub!(:internal_redirect?)
-    @integration_session.stub!(:status)
   end
 
   it "should delegate response_body to the session response body" do
@@ -80,44 +78,6 @@ describe Webrat::RailsSession do
     end
   end
 
-  context "following redirects" do
-    it "should use forward headers when following redirects" do
-      @integration_session.stub!(:post)
-      @integration_session.stub!(:host)
-      @integration_session.stub!(:status)
-
-      @integration_session.should_receive(:internal_redirect?).twice.and_return(true, false)
-      @integration_session.should_receive(:follow_redirect_with_headers).with("headers")
-
-      rails_session = Webrat::RailsSession.new(@integration_session)
-      rails_session.post("url", "data", "headers")
-    end
-
-    it "should follow internal redirects" do
-      @integration_session.stub!(:get)
-      @integration_session.stub!(:host)
-      @integration_session.stub!(:status)
-
-      @integration_session.should_receive(:internal_redirect?).twice.and_return(true, false)
-      @integration_session.should_receive(:follow_redirect_with_headers)
-
-      rails_session = Webrat::RailsSession.new(@integration_session)
-      rails_session.get("url", "data", "headers")
-    end
-
-    it "should not follow external redirects" do
-      @integration_session.stub!(:get)
-      @integration_session.stub!(:host)
-      @integration_session.stub!(:status)
-
-      @integration_session.should_receive(:internal_redirect?).and_return(false)
-      @integration_session.should_not_receive(:follow_redirect_with_headers)
-
-      rails_session = Webrat::RailsSession.new(@integration_session)
-      rails_session.get("url", "data", "headers")
-    end
-  end
-
   it "should provide a saved_page_dir" do
     Webrat::RailsSession.new(mock("integration session")).should respond_to(:saved_page_dir)
   end
@@ -125,66 +85,4 @@ describe Webrat::RailsSession do
   it "should provide a doc_root" do
     Webrat::RailsSession.new(mock("integration session")).should respond_to(:doc_root)
   end
-end
-
-describe ActionController::Integration::Session do
-  before :each do
-    Webrat.configuration.mode = :rails
-    @integration_session = ActionController::Integration::Session.new
-    @integration_session.stub!(:request => mock("request", :url => "http://source.url/"))
-    @integration_session.stub!(:response => mock("response"))
-  end
-
-  describe "internal_redirect?" do
-    it "should return false if the response is not a redirect" do
-      @integration_session.should_receive(:redirect?).and_return(false)
-      @integration_session.internal_redirect?.should == false
-    end
-
-    it "should return false if the response was a redirect but the response location does not match the request host" do
-      @integration_session.should_receive(:redirect?).and_return(true)
-      @integration_session.response.should_receive(:redirect_url_match?).and_return(false)
-      @integration_session.internal_redirect?.should == false
-    end
-
-    it "should return true if the response is a redirect and the response location matches the request host" do
-      @integration_session.should_receive(:redirect?).and_return(true)
-      @integration_session.response.should_receive(:redirect_url_match?).and_return(true)
-      @integration_session.internal_redirect?.should == true
-    end
-  end
-
-  describe "follow_redirect_with_headers" do
-    before do
-      Webrat.configuration.mode = :rails
-      @integration_session.stub!(:headers).and_return({ 'location' => ["/"]})
-      @integration_session.stub!(:redirect?).and_return true
-      @integration_session.stub!(:get)
-    end
-
-    it "should raise an exception if response wasn't a redirect" do
-      @integration_session.stub!(:redirect?).and_return false
-      lambda { @integration_session.follow_redirect_with_headers }.should raise_error
-    end
-
-    it "should set the HTTP referer header" do
-      headers = {}
-
-      @integration_session.follow_redirect_with_headers(headers)
-      headers["HTTP_REFERER"].should == "http://source.url/"
-    end
-
-    it "should GET the first location header" do
-      @integration_session.stub!("headers").and_return({ 'location' => ['/target'] })
-
-      @integration_session.should_receive(:get).with("/target", {}, hash_including("headers" => "foo"))
-
-      @integration_session.follow_redirect_with_headers({"headers" => "foo"})
-    end
-
-    it "should return the status" do
-      @integration_session.stub!(:status).and_return "202"
-      @integration_session.follow_redirect_with_headers.should == "202"
-    end
-  end
 end

spec/private/sinatra/sinatra_spec.rb

@@ -4,11 +4,6 @@ describe Webrat::SinatraSession do
   before :each do
     Webrat.configuration.mode = :sinatra
     @sinatra_session = Webrat::SinatraSession.new
-
-    @response = mock(:response)
-    @response.stub!(:redirect?)
-
-    @sinatra_session.instance_variable_set("@response", @response)
   end
 
   it "should delegate get to get_it" do
@@ -30,14 +25,4 @@ describe Webrat::SinatraSession do
     @sinatra_session.should_receive(:delete_it).with("url", { :env => "headers" })
     @sinatra_session.delete("url", {}, "headers")
   end
-
-  it "should forward headers when following redirects" do
-    @response.should_receive(:redirect?).twice.and_return(true, false)
-    @response.should_receive(:location).and_return("redirect url")
-
-    @sinatra_session.should_receive(:get_it).with("original url", { :env => "headers" })
-    @sinatra_session.should_receive(:get_it).with("redirect url", { :env => "headers" })
-
-    @sinatra_session.get("original url", {}, "headers")
-  end
 end

spec/public/visit_spec.rb

@@ -13,22 +13,31 @@ describe "visit" do
     webrat_session.should_receive(:get).with("/", {})
     visit("/")
   end
-  
+
   it "should assert valid response" do
     webrat_session.response_code = 501
     lambda { visit("/") }.should raise_error(Webrat::PageLoadError)
   end
-  
+
   [200, 300, 400, 499].each do |status|
     it "should consider the #{status} status code as success" do
       webrat_session.response_code = status
       lambda { visit("/") }.should_not raise_error
     end
   end
-  
+
   it "should require a visit before manipulating page" do
     lambda { fill_in "foo", :with => "blah" }.should raise_error(Webrat::WebratError)
   end
+
+  it "should follow redirects" do
+    webrat_session.response.should_receive(:redirect?).twice.and_return(true, false)
+    webrat_session.response.should_receive(:location).once.and_return("/newurl")
+
+    visit("/oldurl")
+
+    current_url.should == "/newurl"
+  end
 end
 
 describe "visit with referer" do
@@ -45,5 +54,5 @@ describe "visit with referer" do
     webrat_session.should_receive(:get).with("/", {}, {"HTTP_REFERER" => "/old_url"})
     visit("/")
   end
-  
+
 end