Skip to content

Commit

Permalink
Incorporate the review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
saikambaiyyagari committed May 7, 2024
1 parent 5652df0 commit d9cd078
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 28 deletions.
24 changes: 12 additions & 12 deletions lib/koala/api/graph_batch_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ def execute(http_options = {})
# when http_component is set we receive Koala::Http_service response object
# from graph_call so this needs to be parsed
# as generate_results method handles only JSON response
if http_options[:http_component]
response = JSON.load(response.body)
raise bad_response("Facebook returned an invalid body") unless response.is_a?(Array)
if http_options[:http_component] && http_options[:http_component] == :response
response = json_body(response.body)

raise bad_response('Facebook returned an invalid body') unless response.is_a?(Array)
end

batch_results += generate_results(response, batch)
Expand Down Expand Up @@ -131,10 +132,13 @@ def batch_args(calls_for_batch)
JSON.dump calls
end

def json_body(response)
# quirks_mode is needed because Facebook sometimes returns a raw true or false value --
# in Ruby 2.4 we can drop that.
JSON.parse(response.fetch("body"), quirks_mode: true)
def json_body(body)
return if body.nil?

JSON.parse(body)
rescue JSON::ParserError => e
Koala::Utils.logger.error("#{e.class}: #{e.message} while parsing #{body}")
nil
end

def desired_component(component:, response:, headers:)
Expand All @@ -146,11 +150,7 @@ def desired_component(component:, response:, headers:)
# facebook returns the headers as an array of k/v pairs, but we want a regular hash
when :headers then headers
# (see note in regular api method about JSON parsing)
when :response
Koala::HTTPService::Response.new(
response["code"].to_i,
response["body"],
headers)
when :response then result
else GraphCollection.evaluate(result, original_api)
end
end
Expand Down
83 changes: 67 additions & 16 deletions spec/cases/graph_api_batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -401,27 +401,78 @@
expect result[0].body == "{\"id\":\"1234\"}"
expect result[0].headers == {"Content-Type"=>"text/javascript; charset=UTF-8"}
end
describe "if it errors" do
it "raises an APIError if the response is not 200" do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(500, "[]", {}))

describe 'if it errors' do
it 'raises an APIError if the response is not 200' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(500, '[]', {}))
expect {
Koala::Facebook::API.new("foo").batch {|batch_api| batch_api.get_object('me') }
Koala::Facebook::API.new('foo').batch { |batch_api| batch_api.get_object('me') }
}.to raise_exception(Koala::Facebook::APIError)
end

it "raises a BadFacebookResponse if the body is empty" do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, "", {}))
expect {
Koala::Facebook::API.new("foo").batch {|batch_api| batch_api.get_object('me') }
}.to raise_exception(Koala::Facebook::BadFacebookResponse)
end

it "raises a BadFacebookResponse if the body is non-empty, non-array" do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, "200", {}))
it 'raises a BadFacebookResponse if the body is empty' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '', {}))
expect {
Koala::Facebook::API.new("foo").batch(http_component: :response) {|batch_api| batch_api.get_object('me') }
}.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
Koala::Facebook::API.new('foo').batch { |batch_api| batch_api.get_object('me') }
}.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an empty body \[HTTP 200\]/)
end

describe 'handle invalid body errors' do
describe 'with http_component set to :response' do
it 'raises a BadFacebookResponse if the body is non-empty, non-array' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '200', {}))
expect {
Koala::Facebook::API.new('foo').batch(http_component: :response) do |batch_api|
batch_api.get_object('me')
end
}.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
end

it 'raises a BadFacebookResponse if the body is invalid JSON' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '{"\"id\":\1234\"}"}', {}))
expect {
Koala::Facebook::API.new('foo').batch(http_component: :response) do |batch_api|
batch_api.get_object('me')
end
}.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
end
end

describe 'with http_component set to :headers' do
it 'should not raise a BadFacebookResponse if the body is non-empty, non-array' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '200', {}))
expect {
Koala::Facebook::API.new('foo').batch(http_component: :headers) { |batch_api| batch_api.get_object('me') }
}.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
end

it 'should not raise a BadFacebookResponse if the body is invalid JSON' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '{"\"id\":\1234\"}"}', {}))
expect {
Koala::Facebook::API.new('foo').batch(http_component: :headers) do |batch_api|
batch_api.get_object('me')
end
}.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
end
end

describe 'with http_component set to :status' do
it 'should not raise a BadFacebookResponse if the body is non-empty, non-array' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '200', {}))
expect {
Koala::Facebook::API.new('foo').batch(http_component: :status) { |batch_api| batch_api.get_object('me') }
}.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
end

it 'should not raise a BadFacebookResponse if the body is invalid JSON' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '{"\"id\":\1234\"}"}', {}))
expect {
Koala::Facebook::API.new('foo').batch(http_component: :status) do |batch_api|
batch_api.get_object('me')
end
}.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
end
end
end

context "with error info" do
Expand Down

0 comments on commit d9cd078

Please sign in to comment.