Fix #309: Added XML support for entities. An XML API will return an e…

…rror instead of returning a string representation of a response. Failing formatters will get reported with a 500.

CHANGELOG.markdown

@@ -1,5 +1,8 @@
 Next Release
 ============
+* [#309](https://github.com/intridea/grape/pull/309): An XML format API will return an error instead of returning a string representation of the response if the latter cannot be converted to XML - [@dblock](http://github.com/dblock).
+* [#309](https://github.com/intridea/grape/pull/309): Added XML support to entities - [@johnnyiller](https://github.com/johnnyiller), [@dblock](http://github.com/dblock).
+* A formatter that raises an exception will cause the API to return a 500 error - [@dblock](http://github.com/dblock).
 * Your contribution here.
 
 0.2.6 (01/11/2013)

lib/grape.rb

@@ -1,5 +1,20 @@
+require 'logger'
 require 'rack'
+require 'rack/mount'
 require 'rack/builder'
+require 'rack/accept'
+require 'rack/auth/basic'
+require 'rack/auth/digest/md5'
+require 'hashie'
+require 'active_support/all'
+require 'grape/util/deep_merge'
+require 'grape/util/content_types'
+require 'multi_json'
+require 'multi_xml'
+require 'virtus'
+require 'i18n'
+
+I18n.load_path << File.expand_path('../grape/locale/en.yml', __FILE__)
 
 module Grape
   autoload :API,                 'grape/api'

lib/grape/api.rb

@@ -1,10 +1,3 @@
-require 'rack/mount'
-require 'rack/auth/basic'
-require 'rack/auth/digest/md5'
-require 'logger'
-require 'grape/util/deep_merge'
-require 'grape/util/content_types'
-
 module Grape
   # The API class is the primary entry point for
   # creating Grape APIs.Users should subclass this

lib/grape/endpoint.rb

@@ -1,7 +1,3 @@
-require 'rack'
-require 'grape'
-require 'hashie'
-
 module Grape
   # An Endpoint is the proxy scope in which all routing
   # blocks are executed. In other words, any methods

lib/grape/entity.rb

@@ -1,5 +1,3 @@
-require 'hashie'
-
 module Grape
   # An Entity is a lightweight structure that allows you to easily
   # represent data from your application in a consistent and abstracted
@@ -335,6 +333,11 @@ def to_json(options = {})
       MultiJson.dump(serializable_hash(options))
     end
 
+    def to_xml(options = {})
+      options = options.to_h if options && options.respond_to?(:to_h)
+      serializable_hash(options).to_xml(options)
+    end
+
     protected
 
     def key_for(attribute)

lib/grape/formatter/xml.rb

@@ -4,7 +4,8 @@ module Xml
       class << self
 
         def call(object, env)
-          object.respond_to?(:to_xml) ? object.to_xml : object.to_s
+          return object.to_xml if object.respond_to?(:to_xml)
+          raise "cannot convert #{object.class} to xml"
         end
 
       end

lib/grape/middleware/base.rb

@@ -1,8 +1,3 @@
-require 'active_support/core_ext/hash/indifferent_access'
-require 'grape/util/content_types'
-require 'multi_json'
-require 'multi_xml'
-
 module Grape
   module Middleware
     class Base

lib/grape/middleware/error.rb

@@ -1,5 +1,4 @@
 require 'grape/middleware/base'
-require 'multi_json'
 
 module Grape
   module Middleware

lib/grape/middleware/formatter.rb

@@ -24,8 +24,12 @@ def before
       def after
         status, headers, bodies = *@app_response
         formatter = Grape::Formatter::Base.formatter_for env['api.format'], options
-        bodymap = bodies.collect do |body|
-          formatter.call body, env
+        begin
+          bodymap = bodies.collect do |body|
+            formatter.call body, env
+          end
+        rescue Exception => e
+          throw :error, :status => 500, :message => e.message
         end
         headers['Content-Type'] = content_type_for(env['api.format']) unless headers['Content-Type']
         Rack::Response.new(bodymap, status, headers).to_a

lib/grape/middleware/versioner/header.rb

@@ -1,5 +1,4 @@
 require 'grape/middleware/base'
-require 'rack/accept'
 
 module Grape
   module Middleware

lib/grape/util/content_types.rb

@@ -1,5 +1,3 @@
-require 'active_support/ordered_hash'
-
 module Grape
   module ContentTypes
     # Content types are listed in order of preference.

lib/grape/validations.rb

@@ -1,7 +1,3 @@
-require 'virtus'
-require 'i18n'
-
-I18n.load_path << File.expand_path('../locale/en.yml', __FILE__)
 module Grape
 
   module Validations

spec/grape/api_spec.rb

@@ -1690,6 +1690,52 @@ def serializable_hash
         last_response.body.should == '[{"abc":"def"},{"abc":"def"}]'
       end
     end
+    context ":xml" do
+      before(:each) do
+        subject.format :xml
+      end
+      it 'string' do
+        subject.get "/example" do
+          "example"
+        end
+        get '/example'
+        last_response.status.should == 500
+        last_response.body.should == <<-XML
+<?xml version="1.0" encoding="UTF-8"?>
+<error>
+  <message>cannot convert String to xml</message>
+</error>
+XML
+      end
+      it 'hash' do
+        subject.get "/example" do
+          { :example1 => "example1", :example2 => "example2" }
+        end
+        get '/example'
+        last_response.status.should == 200
+        last_response.body.should == <<-XML
+<?xml version="1.0" encoding="UTF-8"?>
+<hash>
+  <example1>example1</example1>
+  <example2>example2</example2>
+</hash>
+XML
+      end
+      it 'array' do
+        subject.get "/example" do
+          [ "example1", "example2" ]
+        end
+        get '/example'
+        last_response.status.should == 200
+        last_response.body.should == <<-XML
+<?xml version="1.0" encoding="UTF-8"?>
+<strings type="array">
+  <string>example1</string>
+  <string>example2</string>
+</strings>
+XML
+      end
+    end
   end
 
   context "catch-all" do

spec/grape/endpoint_spec.rb

@@ -552,6 +552,57 @@ def initialize(id)
       end
 
     end
+
+    it 'presents with xml' do 
+      entity = Class.new(Grape::Entity)
+      entity.root "examples", "example"
+      entity.expose :name
+
+      subject.format :xml
+
+      subject.get '/example' do 
+        c = Class.new do 
+          attr_reader :name
+          def initialize(args)
+            @name = args[:name] || "no name set"
+          end
+        end
+        present c.new({:name => "johnnyiller"}), :with => entity
+      end
+      get '/example'
+      last_response.status.should == 200
+      last_response.headers['Content-type'].should == "application/xml"
+      last_response.body.should == <<-XML
+<?xml version="1.0" encoding="UTF-8"?>
+<hash>
+  <example>
+    <name>johnnyiller</name>
+  </example>
+</hash>
+XML
+    end
+
+    it 'presents with json' do 
+      entity = Class.new(Grape::Entity)
+      entity.root "examples", "example"
+      entity.expose :name
+
+      subject.format :json
+
+      subject.get '/example' do 
+        c = Class.new do 
+          attr_reader :name
+          def initialize(args)
+            @name = args[:name] || "no name set"
+          end
+        end
+        present c.new({:name => "johnnyiller"}), :with => entity
+      end
+      get '/example'
+      last_response.status.should == 200
+      last_response.headers['Content-type'].should == "application/json"
+      last_response.body.should == '{"example":{"name":"johnnyiller"}}'
+    end
   end
 
   context 'filters' do

spec/grape/middleware/formatter_spec.rb

@@ -4,7 +4,7 @@
   subject{ Grape::Middleware::Formatter.new(app) }
   before{ subject.stub!(:dup).and_return(subject) }
 
-  let(:app){ lambda{|env| [200, {}, [@body]]} }
+  let(:app){ lambda{|env| [200, {}, [@body || { "foo" => "bar" }]]} }
 
   context 'serialization' do
     it 'looks at the bodies for possibly serializable data' do
@@ -37,6 +37,7 @@ def to_xml
   end
 
   context 'detection' do
+
     it 'uses the extension if one is provided' do
       subject.call({'PATH_INFO' => '/info.xml'})
       subject.env['api.format'].should == :xml
@@ -45,9 +46,9 @@ def to_xml
     end
 
     it 'uses the format parameter if one is provided' do
-      subject.call({'PATH_INFO' => '/somewhere','QUERY_STRING' => 'format=json'})
+      subject.call({'PATH_INFO' => '/info','QUERY_STRING' => 'format=json'})
       subject.env['api.format'].should == :json
-      subject.call({'PATH_INFO' => '/somewhere','QUERY_STRING' => 'format=xml'})
+      subject.call({'PATH_INFO' => '/info','QUERY_STRING' => 'format=xml'})
       subject.env['api.format'].should == :xml
     end
 

spec/spec_helper.rb

@@ -12,11 +12,8 @@
 
 require 'rack/test'
 require 'pry'
-
 require 'base64'
 
-require 'hashie/hash'
-
 Dir["#{File.dirname(__FILE__)}/support/*.rb"].each do |file|
   require file
 end