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 96e7cf9
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 38 deletions.
30 changes: 15 additions & 15 deletions lib/koala/api/graph_batch_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,15 @@ def execute(http_options = {})
end

original_api.graph_call("/", args, "post", http_options) do |response|
raise bad_response("Facebook returned an empty body") if response.nil?
raise bad_response('Facebook returned an empty body') if response.nil?

# 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 @@ -91,7 +92,7 @@ def generate_results(response, batch)

def bad_response(message)
# Facebook sometimes reportedly returns an empty body at times
BadFacebookResponse.new(200, "", message)
BadFacebookResponse.new(200, '', message)
end

def result_from_response(response, options)
Expand Down Expand Up @@ -131,26 +132,25 @@ 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:)
result = Koala::HTTPService::Response.new(response['status'], response['body'], headers)
result = Koala::HTTPService::Response.new(response['code'], response['body'], headers)

# Get the HTTP component they want
case component
when :status then response["code"].to_i
# 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
86 changes: 63 additions & 23 deletions spec/cases/graph_api_batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -383,45 +383,85 @@
end
end

describe "processing the request" do
it "returns the result headers as a hash if http_component is headers" do
describe 'processing the request' do
let(:response_status) { 203 }
let(:response_body) { '{\"id\":\"1234\"}'.gsub('\\', '') }
let(:response_headers) { { 'Content-Type' => 'text/javascript; charset=UTF-8' } }

it 'returns the result headers as a hash if http_component is headers' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '[{"code":203,"headers":[{"name":"Content-Type","value":"text/javascript; charset=UTF-8"}],"body":"{\"id\":\"1234\"}"}]', {}))
result = @api.batch do |batch_api|
batch_api.get_object(KoalaTest.user1, {}, :http_component => :headers)
end
expect(result[0]).to eq({"Content-Type" => "text/javascript; charset=UTF-8"})
expect(response_headers).to eq(result[0])
end

it "returns the complete response if http_component is response" do
it 'returns the complete response if http_component is response' do
allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '[{"code":203,"headers":[{"name":"Content-Type","value":"text/javascript; charset=UTF-8"}],"body":"{\"id\":\"1234\"}"}]', {}))
result = @api.batch do |batch_api|
batch_api.get_object(KoalaTest.user1, {}, :http_component => :response)
end
expect result[0].status == 203
expect result[0].body == "{\"id\":\"1234\"}"
expect result[0].headers == {"Content-Type"=>"text/javascript; charset=UTF-8"}

expect(response_status).to eq(result[0].status)
expect(response_body).to eq(result[0].body)
expect(response_headers).to eq(result[0].headers)
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, "", {}))
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", {}))
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

%i[headers status].each do |component|
describe "with http_component set to #{component}" 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: component) { |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: component) 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
end

context "with error info" do
Expand Down

0 comments on commit 96e7cf9

Please sign in to comment.