Permalink
Browse files

Merge branch 'bugfix/stop_excessive_escaping'

  • Loading branch information...
2 parents 674d7b5 + 33c2072 commit e995c6a218cc0aeaed877391e47e0d437cb0e6a0 @jiblits jiblits committed Jun 4, 2012
Showing with 36 additions and 23 deletions.
  1. +11 −7 lib/active_fulfillment/fulfillment/services/amazon_mws.rb
  2. +25 −16 test/unit/services/amazon_mws_test.rb
@@ -174,10 +174,14 @@ def test_mode?
false
end
+ def build_full_query(verb, uri, params)
+ signature = sign(verb, uri, params)
+ build_query(params) + "&Signature=#{signature}"
+ end
+
def commit(verb, service, op, params)
uri = URI.parse("https://#{endpoint}/#{ACTIONS[service]}/#{VERSION}")
- signature = sign(verb, uri, params)
- query = build_query(params) + "&Signature=#{signature}"
+ query = build_full_query(verb, uri, params)
headers = build_headers(query)
data = ssl_post(uri.to_s, query, headers)
@@ -395,7 +399,7 @@ def build_tracking_request(order_id, options)
def build_address(address)
requires!(address, :name, :address1, :city, :state, :country, :zip)
- ary = address.map{ |key, value| [escape(LOOKUPS[:destination_address][key]), escape(value)] if value.length > 0 }
+ ary = address.map{ |key, value| [LOOKUPS[:destination_address][key], value] if value.length > 0 }
Hash[ary.compact]
end
@@ -408,13 +412,13 @@ def build_items(line_items)
entry = value % counter
case key
when :sku
- items[entry] = escape(line_item[:sku] || "SKU-#{counter}")
+ items[entry] = line_item[:sku] || "SKU-#{counter}"
when :order_id
- items[entry] = escape(line_item[:sku] || "FULFILLMENT-ITEM-ID-#{counter}")
+ items[entry] = line_item[:sku] || "FULFILLMENT-ITEM-ID-#{counter}"
when :quantity
- items[entry] = escape(line_item[:quantity] || 1)
+ items[entry] = line_item[:quantity] || 1
else
- items[entry] = escape(line_item[key]) if line_item.include? key
+ items[entry] = line_item[key] if line_item.include? key
end
end
items
@@ -98,27 +98,27 @@ def test_verify_amazon_response
def test_build_address
expected_items = {
- "DestinationAddress.Name" => @address[:name].gsub(' ', '%20'),
- "DestinationAddress.Line1" => @address[:address1].gsub(' ', '%20'),
- "DestinationAddress.Line2" => @address[:address2].gsub(' ', '%20'),
- "DestinationAddress.City" => @address[:city].gsub(' ', '%20'),
- "DestinationAddress.StateOrProvinceCode" => @address[:state].gsub(' ', '%20'),
- "DestinationAddress.CountryCode" => @address[:country].gsub(' ', '%20'),
- "DestinationAddress.PostalCode" => @address[:zip].gsub(' ', '%20'),
- "DestinationAddress.PhoneNumber" => CGI.escape(@address[:phone])
+ "DestinationAddress.Name" => @address[:name],
+ "DestinationAddress.Line1" => @address[:address1],
+ "DestinationAddress.Line2" => @address[:address2],
+ "DestinationAddress.City" => @address[:city],
+ "DestinationAddress.StateOrProvinceCode" => @address[:state],
+ "DestinationAddress.CountryCode" => @address[:country],
+ "DestinationAddress.PostalCode" => @address[:zip],
+ "DestinationAddress.PhoneNumber" => @address[:phone]
}
assert_equal expected_items, @service.build_address(@address)
end
def test_build_address_with_missing_fields
expected_items = {
- "DestinationAddress.Name" => @address[:name].gsub(' ', '%20'),
- "DestinationAddress.Line1" => @address[:address1].gsub(' ', '%20'),
- "DestinationAddress.City" => @address[:city].gsub(' ', '%20'),
- "DestinationAddress.StateOrProvinceCode" => @address[:state].gsub(' ', '%20'),
- "DestinationAddress.CountryCode" => @address[:country].gsub(' ', '%20'),
- "DestinationAddress.PostalCode" => @address[:zip].gsub(' ', '%20'),
- "DestinationAddress.PhoneNumber" => CGI.escape(@address[:phone])
+ "DestinationAddress.Name" => @address[:name],
+ "DestinationAddress.Line1" => @address[:address1],
+ "DestinationAddress.City" => @address[:city],
+ "DestinationAddress.StateOrProvinceCode" => @address[:state],
+ "DestinationAddress.CountryCode" => @address[:country],
+ "DestinationAddress.PostalCode" => @address[:zip],
+ "DestinationAddress.PhoneNumber" => @address[:phone]
}
@address[:address2] = ""
@@ -137,10 +137,12 @@ def test_integrated_registration_url_creation
def test_build_items
expected_items = {
"Items.member.1.DisplayableComment" => "Awesome",
- "Items.member.1.Quantity" => "1",
+ "Items.member.1.Quantity" => 1,
"Items.member.1.SellerFulfillmentOrderItemId" => "SETTLERS1",
"Items.member.1.SellerSKU" => "SETTLERS1"
}
+
+ actual_items = @service.build_items(@line_items)
assert_equal expected_items, @service.build_items(@line_items)
end
@@ -210,6 +212,13 @@ def test_fetch_tracking_numbers
assert_nil response.tracking_numbers['extern_id_1154539615777']
end
+ def test_that_generated_requests_do_not_double_escape_spaces
+ fulfillment_request = @service.send(:build_fulfillment_request, "12345", @address, @line_items, @options)
+ result = @service.build_full_query(:post, URI.parse("http://example.com/someservice/2011"), fulfillment_request)
+
+ assert !result.include?('%2520')
+ end
+
def test_fetch_tracking_numbers_ignores_not_found
response = mock('response')
response.stubs(:code).returns(500)

0 comments on commit e995c6a

Please sign in to comment.