Allow fallback to use formatter for default format (#34)

chrisbloom7 · dblock · commit f808c48c53d2 · 2019-11-18T09:03:08.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 * Your contribution here.
 
+#### v0.5.0
+
+* [#34](https://github.com/ruby-grape/grape-rabl/pulls/34): If no RABL template is specified, fallback to the default response format as determined by Grape - [@chrisbloom7](https://github.com/chrisbloom7).
+
 #### v0.4.3
 
 * [#44](https://github.com/ruby-grape/grape-rabl/issues/44): Don't require unused hashie - [@tsuwatch](https://github.com/tsuwatch).
diff --git a/README.md b/README.md
@@ -20,6 +20,10 @@ And then execute:
 
     $ bundle
 
+## Upgrading
+
+See [UPGRADING](UPGRADING.md).
+
 ## Usage
 
 ### Setup view root directory
@@ -134,7 +138,7 @@ class UserAPI < Grape::API
     @history = User.find(params[:id]).history
   end
 
-  # do not use rabl, fallback to the defalt Grape JSON formatter
+  # do not use rabl, fallback to the default Grape response formatter
   get '/users' do
     User.all
   end
diff --git a/UPGRADING.md b/UPGRADING.md
@@ -0,0 +1,32 @@
+# Upgrading Grape::Rabl
+
+## Upgrading to >= 0.5
+
+### Fallback rendering when no RABL template is defined
+
+Prior to v0.5.0, Grape::Rabl would always render content as JSON when no Rabl template was specified for a request. Beginning in v0.5.0 Grape::Rabl will now fallback to using the default response format [as determined by Grape](https://github.com/ruby-grape/grape#api-formats)
+
+```ruby
+class SampleApi < Grape::API
+  format :xml
+  formatter :xml, Grape::Formatter::Rabl
+
+  get 'list' do
+    %w[thing]
+  end
+end
+```
+
+#### Former behavior
+
+```ruby
+response.body # => ["thing"]
+```
+
+#### Current behavior
+
+```ruby
+response.body # => <?xml version="1.0" encoding="UTF-8"?>\n<strings type="array">\n  <string>thing</string>\n</strings>
+```
+
+See [#34](https://github.com/ruby-grape/grape-rabl/pull/34) for more information.
diff --git a/lib/grape-rabl/formatter.rb b/lib/grape-rabl/formatter.rb
@@ -29,12 +29,18 @@ def render
             end
           end
         else
-          Grape::Formatter::Json.call object, env
+          fallback_formatter.call object, env
         end
       end
 
       private
 
+      # Find a formatter to fallback to. `env[Grape::Env::API_FORMAT]` will always be a
+      # valid formatter, otherwise a HTTP 406 error would have already have been thrown
+      def fallback_formatter
+        Grape::Formatter.formatter_for(env[Grape::Env::API_FORMAT])
+      end
+
       def view_path(template)
         if template.split('.')[-1] == 'rabl'
           File.join(env['api.tilt.root'], template)
diff --git a/lib/grape-rabl/version.rb b/lib/grape-rabl/version.rb
@@ -1,5 +1,5 @@
 module Grape
   module Rabl
-    VERSION = '0.4.4'.freeze
+    VERSION = '0.5.0'.freeze
   end
 end
diff --git a/spec/grape_rabl_formatter_spec.rb b/spec/grape_rabl_formatter_spec.rb
@@ -0,0 +1,84 @@
+require 'spec_helper'
+
+describe 'Grape::Rabl formatter' do
+  subject do
+    Class.new(Grape::API)
+  end
+
+  let(:xml_render) do
+    %(<?xml version="1.0" encoding="UTF-8"?>
+<hash>
+  <errors type="array">
+    <error>bad</error>
+    <error>things</error>
+    <error>happened</error>
+  </errors>
+</hash>
+)
+  end
+
+  def app
+    subject
+  end
+
+  context 'rendering' do
+    context 'when no rabl template is specified' do
+      before do
+        # Grape::API defaults to the following declarations:
+        # content_type :xml, 'application/xml'
+        # content_type :json, 'application/json'
+        # content_type :binary, 'application/octet-stream'
+        # content_type :txt, 'text/plain'
+        # default_format :txt
+        subject.formatter :xml, Grape::Formatter::Rabl
+        subject.formatter :txt, Grape::Formatter::Rabl
+        subject.get('/oops') { { errors: %w[bad things happened] } }
+        expect_any_instance_of(Grape::Rabl::Formatter).to receive(:render).and_call_original
+      end
+
+      it 'falls back to :txt given no other format information' do
+        get '/oops'
+        expect(last_response.body).to eq('{:errors=>["bad", "things", "happened"]}')
+        expect(last_response.headers['Content-Type']).to eq('text/plain')
+      end
+
+      it 'falls back to the file extension if it is a valid format' do
+        get '/oops.xml'
+        expect(last_response.body).to eq(xml_render)
+        expect(last_response.headers['Content-Type']).to eq('application/xml')
+      end
+
+      it 'falls back to the value of the `format` parameter in the query string if it is provided' do
+        get '/oops?format=xml'
+        expect(last_response.body).to eq(xml_render)
+        expect(last_response.headers['Content-Type']).to eq('application/xml')
+      end
+
+      it 'falls back to the format set by the `format` option if it is a valid format' do
+        # `format` option must be declared before endpoint
+        subject.format :xml
+        subject.get('/oops/2') { { errors: %w[bad things happened] } }
+
+        get '/oops/2'
+        expect(last_response.body).to eq(xml_render)
+        expect(last_response.headers['Content-Type']).to eq('application/xml')
+      end
+
+      it 'falls back to the `Accept` header if it is a valid format' do
+        get '/oops', {}, 'HTTP_ACCEPT' => 'application/xml'
+        expect(last_response.body).to eq(xml_render)
+        expect(last_response.headers['Content-Type']).to eq('application/xml')
+      end
+
+      it 'falls back to the default_format option if it is a valid format' do
+        # `default_format` option must be declared before endpoint
+        subject.default_format :xml
+        subject.get('/oops/2') { { errors: %w[bad things happened] } }
+
+        get '/oops/2'
+        expect(last_response.body).to eq(xml_render)
+        expect(last_response.headers['Content-Type']).to eq('application/xml')
+      end
+    end
+  end
+end
