fix(UNTICKETED): Dont parse pdf as json (#44)

Co-authored-by: Harry Stebbins <[email protected]> Co-authored-by: jgordo04 <[email protected]> Co-authored-by: Harry Stebbins <[email protected]>
dailypay · Nov 14, 2024 · fd4c13c · fd4c13c
1 parent c4b9614
commit fd4c13c
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 15 deletions.
diff --git a/.github/workflows/reviewdog.yaml b/.github/workflows/reviewdog.yaml
@@ -25,5 +25,5 @@ jobs:
       - name: Auto Commit
         uses: stefanzweifel/git-auto-commit-action@v5
         with:
-          commit_message: Rubocop Auto Corrections
+          commit_message: chore: Rubocop Auto Corrections
           commit_user_name: Rubocop
diff --git a/lib/httpigeon/response.rb b/lib/httpigeon/response.rb
@@ -5,7 +5,7 @@ class Response
     attr_reader :request, :parsed_response, :raw_response
 
     delegate :each, :to_h, :to_json, :with_indifferent_access, to: :parsed_response
-    delegate :status, :body, :env, to: :raw_response
+    delegate :status, :body, :env, :headers, to: :raw_response
 
     def initialize(request, raw_response)
       @request = request
@@ -25,7 +25,8 @@ def [](key)
     private
 
     def parse_response
-      parsed_body = body.is_a?(String) ? JSON.parse(body) : body
+      parsable_content_type = headers.blank? || headers['content-type'].blank? || headers['content-type'].include?('application/json')
+      parsed_body = body.is_a?(String) && parsable_content_type ? JSON.parse(body) : body
       @parsed_response = deep_with_indifferent_access(parsed_body)
     rescue JSON::ParserError
       @parsed_response = body.presence || {}

diff --git a/spec/httpigeon/request_spec.rb b/spec/httpigeon/request_spec.rb
@@ -12,7 +12,7 @@
       response = described_class.get(endpoint, query, headers, event_type)
 
       expect(response).to be_a(HTTPigeon::Response)
-      expect(response.parsed_response).to eq({ "message" => "read-that", "status" => "200" })
+      expect(response.parsed_response).to eq({ "message" => "read-that", "status" => 200 })
       expect(response.raw_response).to be_a(Faraday::Response)
       expect(described_class).to have_received(:new).with(base_url: endpoint, headers: headers, event_type: event_type, log_filters: [])
     end
@@ -29,7 +29,7 @@
       response = described_class.post(endpoint, payload, headers, event_type)
 
       expect(response).to be_a(HTTPigeon::Response)
-      expect(response.parsed_response).to eq({ "message" => "wrote-this", "status" => "202" })
+      expect(response.parsed_response).to eq({ "message" => "wrote-this", "status" => 202 })
       expect(response.raw_response).to be_a(Faraday::Response)
       expect(described_class).to have_received(:new).with(base_url: endpoint, headers: headers, event_type: event_type, log_filters: [])
     end
@@ -46,7 +46,7 @@
       response = described_class.put(endpoint, payload, headers, event_type)
 
       expect(response).to be_a(HTTPigeon::Response)
-      expect(response.parsed_response).to eq({ "message" => "changed-this", "status" => "202" })
+      expect(response.parsed_response).to eq({ "message" => "changed-this", "status" => 202 })
       expect(response.raw_response).to be_a(Faraday::Response)
       expect(described_class).to have_received(:new).with(base_url: endpoint, headers: headers, event_type: event_type, log_filters: [])
     end
@@ -63,7 +63,7 @@
       response = described_class.delete(endpoint, query, headers, event_type)
 
       expect(response).to be_a(HTTPigeon::Response)
-      expect(response.parsed_response).to eq({ "message" => "deleted-that", "status" => "200" })
+      expect(response.parsed_response).to eq({ "message" => "deleted-that", "status" => 200 })
       expect(response.raw_response).to be_a(Faraday::Response)
       expect(described_class).to have_received(:new).with(base_url: endpoint, headers: headers, event_type: event_type, log_filters: [])
     end
@@ -190,12 +190,14 @@
       allow(SecureRandom).to receive(:uuid).and_return('request-uuid')
       allow(HTTPigeon::Logger).to receive(:new).and_return(logger_double)
       allow_any_instance_of(Faraday::Response).to receive(:body).and_return(response_body)
+      allow_any_instance_of(Faraday::Response).to receive(:headers).and_return(response_headers)
       allow(request.fuse).to receive(:execute).and_call_original
     end
 
     context 'when circuit breaker is disabled' do
       let(:method) { :post }
       let(:response_body) { { response: 'body' }.to_json }
+      let(:response_headers) { { 'content-type' => 'application/json' } }
 
       before { run_request }
 
@@ -241,6 +243,7 @@
     context 'when circuit breaker is enabled' do
       let(:response_body) { { response: 'body' }.to_json }
       let(:method) { :get }
+      let(:response_headers) { { 'content-type' => 'application/json' } }
 
       before do
         allow(HTTPigeon).to receive(:mount_circuit_breaker).and_return(true)
@@ -269,48 +272,68 @@
         {
           description: 'when the response is a json object',
           response_body: '{ "response": "body" }',
-          expected_parsed_response: { response: 'body' }.with_indifferent_access
+          expected_parsed_response: { response: 'body' }.with_indifferent_access,
+          headers: { 'content-type' => 'application/json' }
         },
         {
           description: 'when the response is an array',
           response_body: '["foo"]',
-          expected_parsed_response: ['foo']
+          expected_parsed_response: ['foo'],
+          headers: { 'content-type' => 'application/json' }
         },
         {
           description: 'when the response is nested array',
           response_body: '[["foo"], ["bar"]]',
-          expected_parsed_response: [['foo'], ['bar']]
+          expected_parsed_response: [['foo'], ['bar']],
+          headers: { 'content-type' => 'application/json' }
         },
         {
           description: 'when the response is nested json objects',
           response_body: '{ "response": { "inner": "object" } }',
-          expected_parsed_response: { response: { inner: 'object' }.with_indifferent_access }.with_indifferent_access
+          expected_parsed_response: { response: { inner: 'object' }.with_indifferent_access }.with_indifferent_access,
+          headers: { 'content-type' => 'application/json' }
         },
         {
           description: 'when the response is json objects inside an array',
           response_body: '[{ "foo": "bar" }, { "baz": "qux" }]',
-          expected_parsed_response: [{ foo: 'bar' }.with_indifferent_access, { baz: 'qux' }.with_indifferent_access]
+          expected_parsed_response: [{ foo: 'bar' }.with_indifferent_access, { baz: 'qux' }.with_indifferent_access],
+          headers: { 'content-type' => 'application/json' }
         },
         {
           description: 'when the response is arrays inside a json object',
           response_body: '{ "response": ["foo", "bar"] }',
-          expected_parsed_response: { response: ['foo', 'bar'] }.with_indifferent_access
+          expected_parsed_response: { response: ['foo', 'bar'] }.with_indifferent_access,
+          headers: { 'content-type' => 'application/json' }
         },
         {
           description: 'when the response is a truly absurd json object',
           response_body: '[{"foo":"bar"},1,"foobar",true,null,[{"inner":"object"},1,null,[]]]',
-          expected_parsed_response: [{ foo: "bar" }.with_indifferent_access, 1, "foobar", true, nil, [{ inner: "object" }.with_indifferent_access, 1, nil, []]]
+          expected_parsed_response: [{ foo: "bar" }.with_indifferent_access, 1, "foobar", true, nil, [{ inner: "object" }.with_indifferent_access, 1, nil, []]],
+          headers: { 'content-type' => 'application/json' }
         },
         {
           description: 'when the response is invalid json',
           response_body: 'invalid json',
-          expected_parsed_response: 'invalid json'
+          expected_parsed_response: 'invalid json',
+          headers: { 'content-type' => 'application/json' }
+        },
+        {
+          description: 'when the response is a pdf',
+          response_body: File.binread('spec/test-image.pdf'),
+          expected_parsed_response: File.binread('spec/test-image.pdf'),
+          headers: { 'content-type' => 'application/pdf' }
+        },
+        {
+          description: 'when no headers are set, json is assumed',
+          response_body: '{ "response": "body" }',
+          expected_parsed_response: { response: 'body' }.with_indifferent_access
         }
       ]
 
       test_cases.each do |test_case|
         context test_case[:description] do
           let(:response_body) { test_case[:response_body] }
+          let(:response_headers) { test_case[:headers] }
 
           it 'parses the response appropriately' do
             expect(run_request).to eq(test_case[:expected_parsed_response])

diff --git a/spec/test-image.pdf b/spec/test-image.pdf