From ce364d16638c6d6906b5c00ffdcb0374caed7f5b Mon Sep 17 00:00:00 2001 From: Josh Knowles Date: Mon, 29 Dec 2008 21:19:13 -0500 Subject: [PATCH] Refactor redirect support out of RailsSession & SinatraSession and into Session#request_page --- lib/webrat/core/session.rb | 64 +++++++------- lib/webrat/rails.rb | 24 ------ lib/webrat/sinatra.rb | 1 - spec/fakes/test_session.rb | 32 ++++--- spec/private/core/session_spec.rb | 39 +++++---- spec/private/rails/rails_session_spec.rb | 102 ----------------------- spec/private/sinatra/sinatra_spec.rb | 15 ---- spec/public/visit_spec.rb | 17 +++- 8 files changed, 89 insertions(+), 205 deletions(-) diff --git a/lib/webrat/core/session.rb b/lib/webrat/core/session.rb index b7b05ca..5cefee9 100644 --- a/lib/webrat/core/session.rb +++ b/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 diff --git a/lib/webrat/rails.rb b/lib/webrat/rails.rb index fd1020d..555ca09 100644 --- a/lib/webrat/rails.rb +++ b/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 diff --git a/lib/webrat/sinatra.rb b/lib/webrat/sinatra.rb index c365387..30e41a5 100644 --- a/lib/webrat/sinatra.rb +++ b/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 diff --git a/spec/fakes/test_session.rb b/spec/fakes/test_session.rb index 005e714..7533fe0 100644 --- a/spec/fakes/test_session.rb +++ b/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 \ No newline at end of file diff --git a/spec/private/core/session_spec.rb b/spec/private/core/session_spec.rb index 013668a..31d0fb6 100644 --- a/spec/private/core/session_spec.rb +++ b/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 "" 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 \ No newline at end of file diff --git a/spec/private/rails/rails_session_spec.rb b/spec/private/rails/rails_session_spec.rb index 90c7cc3..54ff1bf 100644 --- a/spec/private/rails/rails_session_spec.rb +++ b/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 \ No newline at end of file diff --git a/spec/private/sinatra/sinatra_spec.rb b/spec/private/sinatra/sinatra_spec.rb index 64bccc9..73ab674 100644 --- a/spec/private/sinatra/sinatra_spec.rb +++ b/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 \ No newline at end of file diff --git a/spec/public/visit_spec.rb b/spec/public/visit_spec.rb index a0e823d..ce37384 100644 --- a/spec/public/visit_spec.rb +++ b/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