From caabe228bc6c6e920043334d717e72093559e118 Mon Sep 17 00:00:00 2001 From: Rasik Pandey Date: Sat, 30 Aug 2008 04:19:18 +0300 Subject: [PATCH] Format related patches to support serializing data out in the correct format with correct http request headers per http method type [#450 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarmo Tänav Signed-off-by: Jeremy Kemper --- activeresource/lib/active_resource/base.rb | 13 ++++--- .../lib/active_resource/connection.rb | 25 ++++++++++---- .../lib/active_resource/custom_methods.rb | 19 ++++++----- .../active_resource/formats/json_format.rb | 12 +++---- .../lib/active_resource/formats/xml_format.rb | 18 +++++----- .../lib/active_resource/http_mock.rb | 2 +- .../test/base/custom_methods_test.rb | 3 +- activeresource/test/base_test.rb | 2 +- activeresource/test/format_test.rb | 34 +++++++++++++------ 9 files changed, 78 insertions(+), 50 deletions(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 4080a3ebe6c75..17fbb4b6d0850 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -843,8 +843,13 @@ def exists? # # my_group.to_xml(:skip_instruct => true) # # => [...] - def to_xml(options={}) - attributes.to_xml({:root => self.class.element_name}.merge(options)) + def encode(options={}) + case self.class.format + when ActiveResource::Formats[:xml] + self.class.format.encode(attributes, {:root => self.class.element_name}.merge(options)) + else + self.class.format.encode(attributes, options) + end end # A method to reload the attributes of this object from the remote web service. @@ -929,14 +934,14 @@ def connection(refresh = false) # Update the resource on the remote service. def update - returning connection.put(element_path(prefix_options), to_xml, self.class.headers) do |response| + returning connection.put(element_path(prefix_options), encode, self.class.headers) do |response| load_attributes_from_response(response) end end # Create (i.e., save to the remote service) the new resource. def create - returning connection.post(collection_path, to_xml, self.class.headers) do |response| + returning connection.post(collection_path, encode, self.class.headers) do |response| self.id = id_from_response(response) load_attributes_from_response(response) end diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index 0c4ea432d730a..2a1411867822c 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -63,6 +63,13 @@ def allowed_methods # This class is used by ActiveResource::Base to interface with REST # services. class Connection + + HTTP_FORMAT_HEADER_NAMES = { :get => 'Accept', + :put => 'Content-Type', + :post => 'Content-Type', + :delete => 'Accept' + } + attr_reader :site, :user, :password, :timeout attr_accessor :format @@ -106,25 +113,25 @@ def timeout=(timeout) # Execute a GET request. # Used to get (find) resources. def get(path, headers = {}) - format.decode(request(:get, path, build_request_headers(headers)).body) + format.decode(request(:get, path, build_request_headers(headers, :get)).body) end # Execute a DELETE request (see HTTP protocol documentation if unfamiliar). # Used to delete resources. def delete(path, headers = {}) - request(:delete, path, build_request_headers(headers)) + request(:delete, path, build_request_headers(headers, :delete)) end # Execute a PUT request (see HTTP protocol documentation if unfamiliar). # Used to update resources. def put(path, body = '', headers = {}) - request(:put, path, body.to_s, build_request_headers(headers)) + request(:put, path, body.to_s, build_request_headers(headers, :put)) end # Execute a POST request. # Used to create new resources. def post(path, body = '', headers = {}) - request(:post, path, body.to_s, build_request_headers(headers)) + request(:post, path, body.to_s, build_request_headers(headers, :post)) end # Execute a HEAD request. @@ -187,12 +194,12 @@ def http end def default_header - @default_header ||= { 'Content-Type' => format.mime_type } + @default_header ||= {} end # Builds headers for request to remote service. - def build_request_headers(headers) - authorization_header.update(default_header).update(headers) + def build_request_headers(headers, http_method=nil) + authorization_header.update(default_header).update(headers).update(http_format_header(http_method)) end # Sets authorization header @@ -200,6 +207,10 @@ def authorization_header (@user || @password ? { 'Authorization' => 'Basic ' + ["#{@user}:#{ @password}"].pack('m').delete("\r\n") } : {}) end + def http_format_header(http_method) + {HTTP_FORMAT_HEADER_NAMES[http_method] => format.mime_type} + end + def logger #:nodoc: ActiveResource::Base.logger end diff --git a/activeresource/lib/active_resource/custom_methods.rb b/activeresource/lib/active_resource/custom_methods.rb index 4c8699288c719..91924e7fec1c5 100644 --- a/activeresource/lib/active_resource/custom_methods.rb +++ b/activeresource/lib/active_resource/custom_methods.rb @@ -30,7 +30,7 @@ module ActiveResource # Person.get(:active) # GET /people/active.xml # # => [{:id => 1, :name => 'Ryan'}, {:id => 2, :name => 'Joe'}] # - module CustomMethods + module CustomMethods def self.included(base) base.class_eval do extend ActiveResource::CustomMethods::ClassMethods @@ -83,24 +83,25 @@ def custom_method_collection_url(method_name, options = {}) "#{prefix(prefix_options)}#{collection_name}/#{method_name}.#{format.extension}#{query_string(query_options)}" end end - + module InstanceMethods def get(method_name, options = {}) connection.get(custom_method_element_url(method_name, options), self.class.headers) end - - def post(method_name, options = {}, body = '') + + def post(method_name, options = {}, body = nil) + request_body = body.nil? ? encode : body if new? - connection.post(custom_method_new_element_url(method_name, options), (body.nil? ? to_xml : body), self.class.headers) + connection.post(custom_method_new_element_url(method_name, options), request_body, self.class.headers) else - connection.post(custom_method_element_url(method_name, options), body, self.class.headers) + connection.post(custom_method_element_url(method_name, options), request_body, self.class.headers) end end - + def put(method_name, options = {}, body = '') connection.put(custom_method_element_url(method_name, options), body, self.class.headers) end - + def delete(method_name, options = {}) connection.delete(custom_method_element_url(method_name, options), self.class.headers) end @@ -110,7 +111,7 @@ def delete(method_name, options = {}) def custom_method_element_url(method_name, options = {}) "#{self.class.prefix(prefix_options)}#{self.class.collection_name}/#{id}/#{method_name}.#{self.class.format.extension}#{self.class.send!(:query_string, options)}" end - + def custom_method_new_element_url(method_name, options = {}) "#{self.class.prefix(prefix_options)}#{self.class.collection_name}/new/#{method_name}.#{self.class.format.extension}#{self.class.send!(:query_string, options)}" end diff --git a/activeresource/lib/active_resource/formats/json_format.rb b/activeresource/lib/active_resource/formats/json_format.rb index df0d6ca372b9a..9e269d4dede1d 100644 --- a/activeresource/lib/active_resource/formats/json_format.rb +++ b/activeresource/lib/active_resource/formats/json_format.rb @@ -2,22 +2,22 @@ module ActiveResource module Formats module JsonFormat extend self - + def extension "json" end - + def mime_type "application/json" end - - def encode(hash) + + def encode(hash, options={}) hash.to_json end - + def decode(json) ActiveSupport::JSON.decode(json) end end end -end \ No newline at end of file +end diff --git a/activeresource/lib/active_resource/formats/xml_format.rb b/activeresource/lib/active_resource/formats/xml_format.rb index 5e97ffa77645d..86c6cec745d9c 100644 --- a/activeresource/lib/active_resource/formats/xml_format.rb +++ b/activeresource/lib/active_resource/formats/xml_format.rb @@ -2,23 +2,23 @@ module ActiveResource module Formats module XmlFormat extend self - + def extension "xml" end - + def mime_type "application/xml" end - - def encode(hash) - hash.to_xml + + def encode(hash, options={}) + hash.to_xml(options) end - + def decode(xml) from_xml_data(Hash.from_xml(xml)) end - + private # Manipulate from_xml Hash, because xml_simple is not exactly what we # want for Active Resource. @@ -28,7 +28,7 @@ def from_xml_data(data) else data end - end + end end end -end \ No newline at end of file +end diff --git a/activeresource/lib/active_resource/http_mock.rb b/activeresource/lib/active_resource/http_mock.rb index 22f83ae910661..7b6e110ef4537 100644 --- a/activeresource/lib/active_resource/http_mock.rb +++ b/activeresource/lib/active_resource/http_mock.rb @@ -146,7 +146,7 @@ class Request attr_accessor :path, :method, :body, :headers def initialize(method, path, body = nil, headers = {}) - @method, @path, @body, @headers = method, path, body, headers.reverse_merge('Content-Type' => 'application/xml') + @method, @path, @body, @headers = method, path, body, headers.merge(ActiveResource::Connection::HTTP_FORMAT_HEADER_NAMES[method] => 'application/xml') end def ==(other_request) diff --git a/activeresource/test/base/custom_methods_test.rb b/activeresource/test/base/custom_methods_test.rb index 62c33ef9bce84..ba5799edfbbf0 100644 --- a/activeresource/test/base/custom_methods_test.rb +++ b/activeresource/test/base/custom_methods_test.rb @@ -10,8 +10,7 @@ def setup @ryan = { :name => 'Ryan' }.to_xml(:root => 'person') @addy = { :id => 1, :street => '12345 Street' }.to_xml(:root => 'address') @addy_deep = { :id => 1, :street => '12345 Street', :zip => "27519" }.to_xml(:root => 'address') - @default_request_headers = { 'Content-Type' => 'application/xml' } - + ActiveResource::HttpMock.respond_to do |mock| mock.get "/people/1.xml", {}, @matz mock.get "/people/1/shallow.xml", {}, @matz diff --git a/activeresource/test/base_test.rb b/activeresource/test/base_test.rb index 6b57ca3a8f7a5..b4dc71b42378f 100644 --- a/activeresource/test/base_test.rb +++ b/activeresource/test/base_test.rb @@ -819,7 +819,7 @@ def to_param def test_to_xml matz = Person.find(1) - xml = matz.to_xml + xml = matz.encode assert xml.starts_with?('') assert xml.include?('Matz') assert xml.include?('1') diff --git a/activeresource/test/format_test.rb b/activeresource/test/format_test.rb index 3c81803fce06e..365576a092be3 100644 --- a/activeresource/test/format_test.rb +++ b/activeresource/test/format_test.rb @@ -5,14 +5,22 @@ class FormatTest < Test::Unit::TestCase def setup @matz = { :id => 1, :name => 'Matz' } @david = { :id => 2, :name => 'David' } - + @programmers = [ @matz, @david ] end - + + def test_http_format_header_name + header_name = ActiveResource::Connection::HTTP_FORMAT_HEADER_NAMES[:get] + assert_equal 'Accept', header_name + + headers_names = [ActiveResource::Connection::HTTP_FORMAT_HEADER_NAMES[:put], ActiveResource::Connection::HTTP_FORMAT_HEADER_NAMES[:post]] + headers_names.each{|header_name| assert_equal 'Content-Type', header_name} + end + def test_formats_on_single_element for format in [ :json, :xml ] using_format(Person, format) do - ActiveResource::HttpMock.respond_to.get "/people/1.#{format}", {}, ActiveResource::Formats[format].encode(@david) + ActiveResource::HttpMock.respond_to.get "/people/1.#{format}", {'Accept' => ActiveResource::Formats[format].mime_type}, ActiveResource::Formats[format].encode(@david) assert_equal @david[:name], Person.find(1).name end end @@ -21,7 +29,7 @@ def test_formats_on_single_element def test_formats_on_collection for format in [ :json, :xml ] using_format(Person, format) do - ActiveResource::HttpMock.respond_to.get "/people.#{format}", {}, ActiveResource::Formats[format].encode(@programmers) + ActiveResource::HttpMock.respond_to.get "/people.#{format}", {'Accept' => ActiveResource::Formats[format].mime_type}, ActiveResource::Formats[format].encode(@programmers) remote_programmers = Person.find(:all) assert_equal 2, remote_programmers.size assert remote_programmers.select { |p| p.name == 'David' } @@ -32,7 +40,7 @@ def test_formats_on_collection def test_formats_on_custom_collection_method for format in [ :json, :xml ] using_format(Person, format) do - ActiveResource::HttpMock.respond_to.get "/people/retrieve.#{format}?name=David", {}, ActiveResource::Formats[format].encode([@david]) + ActiveResource::HttpMock.respond_to.get "/people/retrieve.#{format}?name=David", {'Accept' => ActiveResource::Formats[format].mime_type}, ActiveResource::Formats[format].encode([@david]) remote_programmers = Person.get(:retrieve, :name => 'David') assert_equal 1, remote_programmers.size assert_equal @david[:id], remote_programmers[0]['id'] @@ -40,13 +48,13 @@ def test_formats_on_custom_collection_method end end end - + def test_formats_on_custom_element_method for format in [ :json, :xml ] using_format(Person, format) do ActiveResource::HttpMock.respond_to do |mock| - mock.get "/people/2.#{format}", {}, ActiveResource::Formats[format].encode(@david) - mock.get "/people/2/shallow.#{format}", {}, ActiveResource::Formats[format].encode(@david) + mock.get "/people/2.#{format}", {'Accept' => ActiveResource::Formats[format].mime_type}, ActiveResource::Formats[format].encode(@david) + mock.get "/people/2/shallow.#{format}", {'Accept' => ActiveResource::Formats[format].mime_type}, ActiveResource::Formats[format].encode(@david) end remote_programmer = Person.find(2).get(:shallow) assert_equal @david[:id], remote_programmer['id'] @@ -57,20 +65,24 @@ def test_formats_on_custom_element_method for format in [ :json, :xml ] ryan = ActiveResource::Formats[format].encode({ :name => 'Ryan' }) using_format(Person, format) do - ActiveResource::HttpMock.respond_to.post "/people/new/register.#{format}", {}, ryan, 201, 'Location' => "/people/5.#{format}" remote_ryan = Person.new(:name => 'Ryan') + ActiveResource::HttpMock.respond_to.post "/people.#{format}", {'Content-Type' => ActiveResource::Formats[format].mime_type}, ryan, 201, {'Location' => "/people/5.#{format}"} + remote_ryan.save + + remote_ryan = Person.new(:name => 'Ryan') + ActiveResource::HttpMock.respond_to.post "/people/new/register.#{format}", {'Content-Type' => ActiveResource::Formats[format].mime_type}, ryan, 201, {'Location' => "/people/5.#{format}"} assert_equal ActiveResource::Response.new(ryan, 201, {'Location' => "/people/5.#{format}"}), remote_ryan.post(:register) end end end - + def test_setting_format_before_site resource = Class.new(ActiveResource::Base) resource.format = :json resource.site = 'http://37s.sunrise.i:3000' assert_equal ActiveResource::Formats[:json], resource.connection.format end - + private def using_format(klass, mime_type_reference) previous_format = klass.format