From fa881a88c873f42ada86effa2419c111a5d2042b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Gil?= Date: Fri, 2 Apr 2010 22:14:46 -0300 Subject: [PATCH 1/4] build querystring with all form elements and then parse to get query params * Basically Field#to_param was replaced for Field#to_query_string and some methods related to build params were moved to Form class. Before this commit the params hash was made by parsing each element querystring to param and then merge, now we build the whole querystring first and then parse it to get params with Rack or Rails depending of the configure mode. --- lib/webrat/core/elements/field.rb | 129 ++++++------------ lib/webrat/core/elements/form.rb | 96 ++++++++----- ...nil_to_param.rb => nil_to_query_string.rb} | 2 +- spec/private/core/field_spec.rb | 2 +- spec/private/core/form_spec.rb | 51 +++++++ 5 files changed, 154 insertions(+), 126 deletions(-) rename lib/webrat/core_extensions/{nil_to_param.rb => nil_to_query_string.rb} (65%) create mode 100644 spec/private/core/form_spec.rb diff --git a/lib/webrat/core/elements/field.rb b/lib/webrat/core/elements/field.rb index 4d74f6e..815575b 100644 --- a/lib/webrat/core/elements/field.rb +++ b/lib/webrat/core/elements/field.rb @@ -1,6 +1,7 @@ require "cgi" +require "digest/md5" require "webrat/core_extensions/blank" -require "webrat/core_extensions/nil_to_param" +require "webrat/core_extensions/nil_to_query_string" require "webrat/core/elements/element" @@ -13,7 +14,7 @@ module Webrat attr_reader :value def self.xpath_search - [".//button", ".//input", ".//textarea", ".//select"] + ".//button|.//input|.//textarea|.//select" end def self.xpath_search_excluding_hidden @@ -84,19 +85,17 @@ module Webrat raise DisabledFieldError.new("Cannot interact with disabled form element (#{self})") end - def to_param + def to_query_string return nil if disabled? - params = case Webrat.configuration.mode - when :rails - parse_rails_request_params("#{name}=#{escaped_value}") - when :merb - ::Merb::Parse.query("#{name}=#{escaped_value}") - else - { name => [*@value].first.to_s } + query_string = case Webrat.configuration.mode + when :rails, :merb, :rack, :sinatra + build_query_string + when :mechanize + build_query_string(false) end - unescape_params(params) + query_string end def set(value) @@ -109,18 +108,6 @@ module Webrat protected - def parse_rails_request_params(params) - if defined?(ActionController::AbstractRequest) - ActionController::AbstractRequest.parse_query_parameters(params) - elsif defined?(ActionController::UrlEncodedPairParser) - # For Rails > 2.2 - ActionController::UrlEncodedPairParser.parse_query_parameters(params) - else - # For Rails > 2.3 - Rack::Utils.parse_nested_query(params) - end - end - def form Form.load(@session, form_element) end @@ -138,23 +125,20 @@ module Webrat @element["name"] end - def escaped_value - CGI.escape(@value.to_s) + def build_query_string(escape_value=true) + if @value.is_a?(Array) + @value.collect {|value| "#{name}=#{ escape_value ? escape(value) : value }" }.join("&") + else + "#{name}=#{ escape_value ? escape(value) : value }" + end end - # Because we have to escape it before sending it to the above case statement, - # we have to make sure we unescape each value when it gets back so assertions - # involving characters like <, >, and & work as expected - def unescape_params(params) - case params.class.name - when 'Hash', 'Mash' - params.each { |key,value| params[key] = unescape_params(value) } - params - when 'Array' - params.collect { |value| unescape_params(value) } - else - CGI.unescapeHTML(params) - end + def escape(value) + CGI.escape(value.to_s) + end + + def escaped_value + CGI.escape(@value.to_s) end def labels @@ -186,22 +170,6 @@ module Webrat def default_value @element["value"] end - - def replace_param_value(params, oval, nval) - output = Hash.new - params.each do |key, value| - case value - when Hash - value = replace_param_value(value, oval, nval) - when Array - value = value.map { |o| o == oval ? nval : oval } - when oval - value = nval - end - output[key] = value - end - output - end end class ButtonField < Field #:nodoc: @@ -210,7 +178,7 @@ module Webrat [".//button", ".//input[@type = 'submit']", ".//input[@type = 'button']", ".//input[@type = 'image']"] end - def to_param + def to_query_string return nil if @value.nil? super end @@ -233,13 +201,13 @@ module Webrat ".//input[@type = 'hidden']" end - def to_param + def to_query_string if collection_name? super else checkbox_with_same_name = form.field_named(name, CheckboxField) - if checkbox_with_same_name.to_param.blank? + if checkbox_with_same_name.to_query_string.blank? super else nil @@ -261,7 +229,7 @@ module Webrat ".//input[@type = 'checkbox']" end - def to_param + def to_query_string return nil if @value.nil? super end @@ -306,7 +274,7 @@ module Webrat ".//input[@type = 'radio']" end - def to_param + def to_query_string return nil if @value.nil? super end @@ -363,30 +331,32 @@ module Webrat attr_accessor :content_type def set(value, content_type = nil) + @original_value = @value + @content_type ||= content_type super(value) - @content_type = content_type end - def to_param - if @value.nil? - super - else - replace_param_value(super, @value, test_uploaded_file) - end + def digest_value + @value ? Digest::MD5.hexdigest(self.object_id.to_s) : "" end - protected + def to_query_string + @value.nil? ? set("") : set(digest_value) + super + end def test_uploaded_file + return "" if @original_value.blank? + case Webrat.configuration.mode when :rails if content_type - ActionController::TestUploadedFile.new(@value, content_type) + ActionController::TestUploadedFile.new(@original_value, content_type) else - ActionController::TestUploadedFile.new(@value) + ActionController::TestUploadedFile.new(@original_value) end when :rack, :merb - Rack::Test::UploadedFile.new(@value, content_type) + Rack::Test::UploadedFile.new(@original_value, content_type) end end @@ -450,25 +420,6 @@ module Webrat @value.delete(value) end - # We have to overide how the uri string is formed when dealing with multiples - # Where normally a select field might produce name=value with a multiple, - # we need to form something like name[]=value1&name[]=value2 - def to_param - return nil if disabled? - - uri_string = @value.collect {|value| "#{name}=#{CGI.escape(value)}"}.join("&") - params = case Webrat.configuration.mode - when :rails - parse_rails_request_params(uri_string) - when :merb - ::Merb::Parse.query(uri_string) - else - { name => @value } - end - - unescape_params(params) - end - protected # Overwrite SelectField definition because we don't want to select the first option diff --git a/lib/webrat/core/elements/form.rb b/lib/webrat/core/elements/form.rb index 21d710b..ef627ce 100644 --- a/lib/webrat/core/elements/form.rb +++ b/lib/webrat/core/elements/form.rb @@ -38,15 +38,24 @@ module Webrat end end + # iterate over all form fields to build a request querystring to get params from it, + # for file_field we made a work around to pass a digest as value to later replace it + # in params hash with the real file. def params - all_params = {} + query_string = [] + replaces = {} fields.each do |field| - next if field.to_param.nil? - merge(all_params, field.to_param) + next if field.to_query_string.nil? + replaces.merge!({field.digest_value => field.test_uploaded_file}) if field.is_a?(FileField) + query_string << field.to_query_string end - all_params + query_params = Form.query_string_to_params(query_string.join('&')) + + query_params = Form.replace_params_values(query_params, replaces) + + Form.unescape_params(query_params) end def form_method @@ -57,47 +66,64 @@ module Webrat @element["action"].blank? ? @session.current_url : @element["action"] end - def merge(all_params, new_param) - new_param.each do |key, value| - case all_params[key] - when *hash_classes - merge_hash_values(all_params[key], value) + protected + + def self.replace_param_value(params, oval, nval) + output = Hash.new + params.each do |key, value| + case value + when Hash + value = replace_param_value(value, oval, nval) when Array - all_params[key] += value - else - all_params[key] = value + value = value.map { |o| o == oval ? nval : o } + when oval + value = nval end + output[key] = value + end + output + end + + def self.replace_params_values(params, values) + values.each do |key, value| + params = replace_param_value(params, key, value) + end + params + end + + def self.unescape_params(params) + case params.class.name + when 'Hash', 'Mash' + params.each { |key,value| params[key] = unescape_params(value) } + params + when 'Array' + params.collect { |value| unescape_params(value) } + else + params.is_a?(String) ? CGI.unescapeHTML(params) : params end end - def merge_hash_values(a, b) # :nodoc: - a.keys.each do |k| - if b.has_key?(k) - case [a[k], b[k]].map{|value| value.class} - when *hash_classes.zip(hash_classes) - a[k] = merge_hash_values(a[k], b[k]) - b.delete(k) - when [Array, Array] - a[k] += b[k] - b.delete(k) - end - end - end - a.merge!(b) - end - - def hash_classes - klasses = [Hash] - + def self.query_string_to_params(query_string) case Webrat.configuration.mode when :rails - klasses << HashWithIndifferentAccess + parse_rails_request_params(query_string) when :merb - klasses << Mash + ::Merb::Parse.query(query_string) + when :rack, :sinatra + Rack::Utils.parse_nested_query(query_string) + else + query_string.split('&').map {|query| { query.split('=').first => query.split('=').last }} end - - klasses end + def self.parse_rails_request_params(query_string) + if defined?(ActionController::AbstractRequest) + ActionController::AbstractRequest.parse_query_parameters(query_string) + elsif defined?(ActionController::UrlEncodedPairParser) + ActionController::UrlEncodedPairParser.parse_query_parameters(query_string) + else + Rack::Utils.parse_nested_query(query_string) + end + end end end diff --git a/lib/webrat/core_extensions/nil_to_param.rb b/lib/webrat/core_extensions/nil_to_query_string.rb similarity index 65% rename from lib/webrat/core_extensions/nil_to_param.rb rename to lib/webrat/core_extensions/nil_to_query_string.rb index 019290b..d609058 100644 --- a/lib/webrat/core_extensions/nil_to_param.rb +++ b/lib/webrat/core_extensions/nil_to_query_string.rb @@ -1,5 +1,5 @@ class NilClass #:nodoc: - def to_param + def to_query_string nil end end diff --git a/spec/private/core/field_spec.rb b/spec/private/core/field_spec.rb index 0a7e255..a629368 100644 --- a/spec/private/core/field_spec.rb +++ b/spec/private/core/field_spec.rb @@ -77,7 +77,7 @@ module Webrat element = Webrat::XML.document(html).css('input').first text_field = TextField.new(nil, element) - text_field.to_param.should == { 'email' => 'user@example.com' } + text_field.to_query_string.should == 'email=user@example.com' end end end diff --git a/spec/private/core/form_spec.rb b/spec/private/core/form_spec.rb new file mode 100644 index 0000000..aa28fa1 --- /dev/null +++ b/spec/private/core/form_spec.rb @@ -0,0 +1,51 @@ +require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') + +describe "Multiple nested params" do + it "should be corretly posted" do + Webrat.configuration.mode = :rails + + with_html <<-HTML + +
+
+
+ + +
+
+ + +
+
+
+
+ + +
+
+ +
+ + HTML + + params = { "user" => { "family" => { "parents" => { + "0" => [ {"name" => "Alice", "gender"=>"Mother"}, {"name" => "Michael", "gender"=>"Father"} ], + "1" => [ {"name" => "Jenny", "gender"=>"Mother"} ] + } + } + } + } + + webrat_session.should_receive(:post).with("/family", params) + click_button + end +end From 15102bd60d206f3a7a903f2c899eed3551e0a93a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Gil?= Date: Mon, 5 Apr 2010 11:07:35 -0300 Subject: [PATCH 2/4] !trivial: define and call class methods in From appropriate --- lib/webrat/core/elements/form.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/webrat/core/elements/form.rb b/lib/webrat/core/elements/form.rb index ef627ce..1a9b5e6 100644 --- a/lib/webrat/core/elements/form.rb +++ b/lib/webrat/core/elements/form.rb @@ -51,11 +51,11 @@ module Webrat query_string << field.to_query_string end - query_params = Form.query_string_to_params(query_string.join('&')) + query_params = self.class.query_string_to_params(query_string.join('&')) - query_params = Form.replace_params_values(query_params, replaces) + query_params = self.class.replace_params_values(query_params, replaces) - Form.unescape_params(query_params) + self.class.unescape_params(query_params) end def form_method @@ -66,8 +66,6 @@ module Webrat @element["action"].blank? ? @session.current_url : @element["action"] end - protected - def self.replace_param_value(params, oval, nval) output = Hash.new params.each do |key, value| From c49e23d81d365705e6b821ae3bc4e60729a04662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Gil?= Date: Mon, 5 Apr 2010 11:31:38 -0300 Subject: [PATCH 3/4] fixes #341 attach_file with nested attributes --- lib/webrat/core/elements/form.rb | 2 +- spec/private/rails/attaches_file_spec.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/webrat/core/elements/form.rb b/lib/webrat/core/elements/form.rb index 1a9b5e6..5ab345c 100644 --- a/lib/webrat/core/elements/form.rb +++ b/lib/webrat/core/elements/form.rb @@ -73,7 +73,7 @@ module Webrat when Hash value = replace_param_value(value, oval, nval) when Array - value = value.map { |o| o == oval ? nval : o } + value = value.map { |o| o == oval ? nval : ( o.is_a?(Hash) ? replace_param_value(o, oval, nval) : o) } when oval value = nval end diff --git a/spec/private/rails/attaches_file_spec.rb b/spec/private/rails/attaches_file_spec.rb index e044db0..8d1445c 100644 --- a/spec/private/rails/attaches_file_spec.rb +++ b/spec/private/rails/attaches_file_spec.rb @@ -78,4 +78,19 @@ describe "attach_file" do attach_file "Picture", @filename, "image/png" click_button end + + it "should support nested attributes" do + with_html <<-HTML + +
+ + + +
+ + HTML + webrat_session.should_receive(:post).with("/albums", { "album" => { "photos_attributes" => [{"image" => @uploaded_file}] } }) + attach_file "Photo", @filename + click_button + end end From e36487458fe9e2dd03802c0cc1b94597f4941bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Gil?= Date: Fri, 9 Apr 2010 11:38:33 -0300 Subject: [PATCH 4/4] test added for nested params files upload --- spec/private/rails/attaches_file_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spec/private/rails/attaches_file_spec.rb b/spec/private/rails/attaches_file_spec.rb index 8d1445c..c7e0211 100644 --- a/spec/private/rails/attaches_file_spec.rb +++ b/spec/private/rails/attaches_file_spec.rb @@ -93,4 +93,22 @@ describe "attach_file" do attach_file "Photo", @filename click_button end + + it "should support nested attributes with multiple files" do + with_html <<-HTML + +
+ + + + + +
+ + HTML + webrat_session.should_receive(:post).with("/albums", { "album" => { "photos_attributes" => [{"image" => @uploaded_file}, {"image" => @uploaded_file}] } }) + attach_file "Photo 1", @filename + attach_file "Photo 2", @filename + click_button + end end