Return 404 unless we can find API builder for resource
In order to protect us from rendering a resource simply converted to json, without processing it with API data class, this commit changes JSON responder behavior to render 404 if we can't find associated data class. The only exception to that rule is when resource is already a Hash, meaning that it was processed before - we sometimes return for example simple Hash responses like { result: true }. The Hash exception could allow to accidentally pass resource.as_json to responder, but in travis-ci/travis-support@124b8b6 I disabled default as_json method on AR::Base classes, so the risk of such mistake is lowered.
This commit is contained in:
parent
33463fe042
commit
61142c7cf6
|
@ -8,7 +8,7 @@ class Travis::Api::App
|
|||
end
|
||||
|
||||
def apply
|
||||
halt result.to_json
|
||||
halt result.to_json if result
|
||||
end
|
||||
|
||||
private
|
||||
|
@ -21,11 +21,15 @@ class Travis::Api::App
|
|||
end
|
||||
|
||||
def result
|
||||
builder ? builder.new(resource, params).data : resource
|
||||
builder ? builder.new(resource, params).data : basic_type_resource
|
||||
end
|
||||
|
||||
def builder
|
||||
@builder ||= Travis::Api.builder(resource, { :version => version }.merge(options))
|
||||
if defined?(@builder)
|
||||
@builder
|
||||
else
|
||||
@builder = Travis::Api.builder(resource, { :version => version }.merge(options))
|
||||
end
|
||||
end
|
||||
|
||||
def accept_params
|
||||
|
@ -39,6 +43,10 @@ class Travis::Api::App
|
|||
def params
|
||||
(request.params || {}).merge(accept_params)
|
||||
end
|
||||
|
||||
def basic_type_resource
|
||||
resource if resource.is_a?(Hash)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
54
spec/unit/responders/json_spec.rb
Normal file
54
spec/unit/responders/json_spec.rb
Normal file
|
@ -0,0 +1,54 @@
|
|||
require 'spec_helper'
|
||||
|
||||
module Travis::Api::App::Responders
|
||||
describe Json do
|
||||
class MyJson < Json
|
||||
end
|
||||
|
||||
let(:request) { stub 'request', params: {} }
|
||||
let(:endpoint) { stub 'endpoint', request: request }
|
||||
let(:resource) { stub 'resource' }
|
||||
let(:accept) { stub 'accept entry', version: '2', params: {} }
|
||||
let(:options) { { :accept => accept} }
|
||||
let(:json) { MyJson.new(endpoint, resource, options) }
|
||||
|
||||
context 'with resource not associated with Api data class' do
|
||||
it 'returns nil result' do
|
||||
json.apply.should be_false
|
||||
end
|
||||
end
|
||||
|
||||
context 'with resource being' do
|
||||
context 'a Hash instance' do
|
||||
let(:resource) { { foo: 'bar' } }
|
||||
|
||||
it 'returns resource converted to_json' do
|
||||
json.expects(:halt).with({ foo: 'bar' }.to_json)
|
||||
json.apply
|
||||
end
|
||||
end
|
||||
|
||||
context 'nil' do
|
||||
let(:resource) { nil }
|
||||
|
||||
it 'responds with 404' do
|
||||
json.apply?.should be_false
|
||||
json.apply.should be_false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'with resource associated with Api data class' do
|
||||
let(:builder) { stub 'builder', data: { foo: 'bar' } }
|
||||
let(:builder_class) { stub 'builder class', new: builder }
|
||||
before do
|
||||
json.stubs :builder => builder_class
|
||||
end
|
||||
|
||||
it 'returns proper data converted to json' do
|
||||
json.expects(:halt).with({ foo: 'bar' }.to_json)
|
||||
json.apply
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue
Block a user