From 36c866e46e932d9b756663c7cebd65190c96dc14 Mon Sep 17 00:00:00 2001 From: Muhammad Sufyan Date: Thu, 4 Jul 2024 18:02:22 +0500 Subject: [PATCH 1/3] fix(empty-response): handle empty response in json deserialize This commit fixes an issue where ApiHelper::json_deserialize and response_handler returned the same empty payload (nil or empty string) or default values in case of scalar types instead of nil when the API response was missing. The change ensures the utility and response handler behaves as expected and returns nil for empty responses. closes #42 --- apimatic_core.gemspec | 2 +- lib/apimatic-core/response_handler.rb | 2 + lib/apimatic-core/utilities/api_helper.rb | 2 +- .../response_handler_test.rb | 86 +++++++++++++++++++ .../utilities/api_helper_test.rb | 8 +- 5 files changed, 95 insertions(+), 5 deletions(-) diff --git a/apimatic_core.gemspec b/apimatic_core.gemspec index 07970be..aa612cd 100644 --- a/apimatic_core.gemspec +++ b/apimatic_core.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = 'apimatic_core' - s.version = '0.3.8' + s.version = '0.3.9' s.summary = 'A library that contains apimatic-apimatic-core logic and utilities for consuming REST APIs using Python SDKs generated '\ 'by APIMatic.' s.description = 'The APIMatic Core libraries provide a stable runtime that powers all the functionality of SDKs.'\ diff --git a/lib/apimatic-core/response_handler.rb b/lib/apimatic-core/response_handler.rb index 5397fbc..ec5d5e7 100644 --- a/lib/apimatic-core/response_handler.rb +++ b/lib/apimatic-core/response_handler.rb @@ -191,6 +191,8 @@ def apply_xml_deserializer(response) # Applies deserializer to the response. # @param [Boolean] should_symbolize_hash Flag to symbolize the hash during response deserialization. def apply_deserializer(response, should_symbolize_hash) + return if response.raw_body.nil? or response.raw_body.to_s.strip.empty? + return apply_xml_deserializer(response) if @is_xml_response return response.raw_body if @deserializer.nil? diff --git a/lib/apimatic-core/utilities/api_helper.rb b/lib/apimatic-core/utilities/api_helper.rb index 8a276db..3b6df73 100644 --- a/lib/apimatic-core/utilities/api_helper.rb +++ b/lib/apimatic-core/utilities/api_helper.rb @@ -290,7 +290,7 @@ def self.valid_type?(value, type_callable, is_model_hash: false, is_inner_model_ # @return [Hash, Array, nil] The parsed JSON object, or nil if the input string is nil. # @raise [TypeError] if the server responds with invalid JSON and primitive type parsing is not allowed. def self.json_deserialize(json, should_symbolize = false, allow_primitive_type_parsing = false) - return if json.nil? + return if json.nil? or json.to_s.strip.empty? begin JSON.parse(json, symbolize_names: should_symbolize) diff --git a/test/test-apimatic-core/response_handler_test.rb b/test/test-apimatic-core/response_handler_test.rb index 9e18bbc..e2b37a8 100644 --- a/test/test-apimatic-core/response_handler_test.rb +++ b/test/test-apimatic-core/response_handler_test.rb @@ -321,6 +321,46 @@ def test_primitive_response_body assert_equal expected_response, actual_response end + def test_empty_response_body + response_body_mock = '' + response_mock = MockHelper.create_response status_code: 200, + raw_body: response_body_mock + actual_response = @response_handler.deserializer(ApiHelper.method(:deserialize_primitive_types)) + .is_primitive_response(true) + .deserialize_into(proc do |response_body| + response_body.to_i + end) + .handle(response_mock, MockHelper.get_global_errors) + + assert_nil actual_response + + actual_response = @response_handler + .deserializer(ApiHelper.method(:custom_type_deserializer)) + .deserialize_into(Validate.method(:from_hash)) + .handle(response_mock, MockHelper.get_global_errors) + + assert_nil actual_response + + response_body_mock = ' ' + response_mock = MockHelper.create_response status_code: 200, + raw_body: response_body_mock + actual_response = @response_handler.deserializer(ApiHelper.method(:deserialize_primitive_types)) + .is_primitive_response(true) + .deserialize_into(proc do |response_body| + response_body.to_i + end) + .handle(response_mock, MockHelper.get_global_errors) + + assert_nil actual_response + + actual_response = @response_handler + .deserializer(ApiHelper.method(:custom_type_deserializer)) + .deserialize_into(Validate.method(:from_hash)) + .handle(response_mock, MockHelper.get_global_errors) + + assert_nil actual_response + end + def test_primitive_response_body_oaf response_body_mock = '"Sunday"' response_mock = MockHelper.create_response status_code: 200, @@ -439,6 +479,30 @@ def test_converted_sdk_api_response_with_custom_fields assert_equal expected_response.data.to_s, actual_response.data.to_s assert_equal expected_response.body.to_s, actual_response.body.to_s assert_equal expected_response.cursor, actual_response.cursor + + response_body_mock = '' + response_mock = MockHelper.create_response status_code: 200, + raw_body: response_body_mock + actual_response = @response_handler + .deserializer(ApiHelper.method(:json_deserialize)) + .is_api_response(true) + .convertor(SdkApiResponseWithCustomFields.method(:create)) + .handle(response_mock, MockHelper.get_global_errors, true) + expected_response = SdkApiResponseWithCustomFields.new(response_mock, + data: nil, + errors: nil) + + refute_nil(actual_response) + + assert_instance_of SdkApiResponseWithCustomFields, actual_response + assert_nil actual_response.errors + assert !actual_response.error? + assert actual_response.success? + assert_equal expected_response.status_code, actual_response.status_code + assert_equal expected_response.raw_body, actual_response.raw_body + assert_nil actual_response.data + assert_nil actual_response.body + assert_nil actual_response.cursor end def test_converted_sdk_api_response @@ -463,6 +527,28 @@ def test_converted_sdk_api_response assert_equal expected_response.status_code, actual_response.status_code assert_equal expected_response.raw_body, actual_response.raw_body assert_equal expected_response.data, actual_response.data + + response_body_mock = ' ' + response_mock = MockHelper.create_response status_code: 200, + raw_body: response_body_mock + actual_response = @response_handler + .deserializer(ApiHelper.method(:json_deserialize)) + .is_api_response(true) + .convertor(SdkApiResponse.method(:create)) + .handle(response_mock, MockHelper.get_global_errors) + expected_response = SdkApiResponse.new(response_mock, + data: nil, + errors: nil) + + refute_nil(actual_response) + + assert_instance_of SdkApiResponse, actual_response + assert !actual_response.error? + assert actual_response.success? + assert_nil actual_response.errors + assert_equal expected_response.status_code, actual_response.status_code + assert_equal expected_response.raw_body, actual_response.raw_body + assert_nil actual_response.data end def test_converted_sdk_api_response_errors diff --git a/test/test-apimatic-core/utilities/api_helper_test.rb b/test/test-apimatic-core/utilities/api_helper_test.rb index cbe4419..bbad8bd 100644 --- a/test/test-apimatic-core/utilities/api_helper_test.rb +++ b/test/test-apimatic-core/utilities/api_helper_test.rb @@ -236,16 +236,18 @@ def test_clean_url end def test_json_deserialize - assert_nil(ApiHelper.json_deserialize(nil, false)) + assert_nil(ApiHelper.json_deserialize(nil, )) + assert_nil(ApiHelper.json_deserialize('', )) + assert_nil(ApiHelper.json_deserialize(' ', )) assert_equal(ApiHelper.json_deserialize( '{"name":"Jone","age":23,"address":"H # 531, S # 20","uid":"1234","birthday":"2016-03-13",'\ -'"birthtime":"2016-03-13T12:52:32.123Z"}', + '"birthtime":"2016-03-13T12:52:32.123Z"}', false), { "name" => "Jone", "age" => 23, "address" => "H # 531, S # 20", "uid" => "1234", "birthday" => "2016-03-13", "birthtime" => "2016-03-13T12:52:32.123Z" }) assert_equal(ApiHelper.json_deserialize( '{"name":"Jone","age":23,"address":"H # 531, S # 20","uid":"1234",'\ -'"birthday":"2016-03-13","birthtime":"2016-03-13T12:52:32.123Z"}', + '"birthday":"2016-03-13","birthtime":"2016-03-13T12:52:32.123Z"}', true), { :name => "Jone", :age => 23, :address => "H # 531, S # 20", :uid => "1234", :birthday => "2016-03-13", :birthtime => "2016-03-13T12:52:32.123Z" }) From 844ff10b042166399f9da1be50f4347b739d6713 Mon Sep 17 00:00:00 2001 From: Muhammad Sufyan Date: Thu, 4 Jul 2024 18:06:25 +0500 Subject: [PATCH 2/3] fixed linting issues --- lib/apimatic-core/response_handler.rb | 2 +- lib/apimatic-core/utilities/api_helper.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/apimatic-core/response_handler.rb b/lib/apimatic-core/response_handler.rb index ec5d5e7..3e80594 100644 --- a/lib/apimatic-core/response_handler.rb +++ b/lib/apimatic-core/response_handler.rb @@ -191,7 +191,7 @@ def apply_xml_deserializer(response) # Applies deserializer to the response. # @param [Boolean] should_symbolize_hash Flag to symbolize the hash during response deserialization. def apply_deserializer(response, should_symbolize_hash) - return if response.raw_body.nil? or response.raw_body.to_s.strip.empty? + return if response.raw_body.nil? || response.raw_body.to_s.strip.empty? return apply_xml_deserializer(response) if @is_xml_response return response.raw_body if @deserializer.nil? diff --git a/lib/apimatic-core/utilities/api_helper.rb b/lib/apimatic-core/utilities/api_helper.rb index 3b6df73..5d727a5 100644 --- a/lib/apimatic-core/utilities/api_helper.rb +++ b/lib/apimatic-core/utilities/api_helper.rb @@ -290,7 +290,7 @@ def self.valid_type?(value, type_callable, is_model_hash: false, is_inner_model_ # @return [Hash, Array, nil] The parsed JSON object, or nil if the input string is nil. # @raise [TypeError] if the server responds with invalid JSON and primitive type parsing is not allowed. def self.json_deserialize(json, should_symbolize = false, allow_primitive_type_parsing = false) - return if json.nil? or json.to_s.strip.empty? + return if json.nil? || json.to_s.strip.empty? begin JSON.parse(json, symbolize_names: should_symbolize) From 1a09c6e5f66ae5779eec98bfb76476966e2c13fd Mon Sep 17 00:00:00 2001 From: Muhammad Sufyan Date: Fri, 5 Jul 2024 16:15:17 +0500 Subject: [PATCH 3/3] adds support for is_nullable_response flag in response handler --- lib/apimatic-core/response_handler.rb | 11 ++++- .../response_handler_test.rb | 47 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/lib/apimatic-core/response_handler.rb b/lib/apimatic-core/response_handler.rb index 3e80594..2989b83 100644 --- a/lib/apimatic-core/response_handler.rb +++ b/lib/apimatic-core/response_handler.rb @@ -16,6 +16,7 @@ def initialize @is_date_response = false @is_response_array = false @is_response_void = false + @is_nullable_response = false end # Sets deserializer for the response. @@ -138,6 +139,14 @@ def is_response_void(is_response_void) @is_response_void = is_response_void self end + + # Sets the is_nullable_response property. + # @param [Boolean] is_nullable_response Flag to return early in case of empty response payload. + # @return [ResponseHandler] An updated instance of ResponseHandler. + def is_nullable_response(is_nullable_response) + @is_nullable_response = is_nullable_response + self + end # rubocop:enable Naming/PredicateName # Main method to handle the response with all the set properties. @@ -191,7 +200,7 @@ def apply_xml_deserializer(response) # Applies deserializer to the response. # @param [Boolean] should_symbolize_hash Flag to symbolize the hash during response deserialization. def apply_deserializer(response, should_symbolize_hash) - return if response.raw_body.nil? || response.raw_body.to_s.strip.empty? + return if @is_nullable_response && (response.raw_body.nil? || response.raw_body.to_s.strip.empty?) return apply_xml_deserializer(response) if @is_xml_response return response.raw_body if @deserializer.nil? diff --git a/test/test-apimatic-core/response_handler_test.rb b/test/test-apimatic-core/response_handler_test.rb index e2b37a8..2046266 100644 --- a/test/test-apimatic-core/response_handler_test.rb +++ b/test/test-apimatic-core/response_handler_test.rb @@ -331,10 +331,55 @@ def test_empty_response_body response_body.to_i end) .handle(response_mock, MockHelper.get_global_errors) + expected_response = 0 + + refute_nil actual_response + assert_equal expected_response, actual_response + + actual_response = @response_handler + .deserializer(ApiHelper.method(:custom_type_deserializer)) + .deserialize_into(Validate.method(:from_hash)) + .handle(response_mock, MockHelper.get_global_errors) + + assert_nil actual_response + + response_body_mock = ' ' + response_mock = MockHelper.create_response status_code: 200, + raw_body: response_body_mock + actual_response = @response_handler.deserializer(ApiHelper.method(:deserialize_primitive_types)) + .is_primitive_response(true) + .deserialize_into(proc do |response_body| + response_body.to_i + end) + .handle(response_mock, MockHelper.get_global_errors) + + refute_nil actual_response + assert_equal expected_response, actual_response + + actual_response = @response_handler + .deserializer(ApiHelper.method(:custom_type_deserializer)) + .deserialize_into(Validate.method(:from_hash)) + .handle(response_mock, MockHelper.get_global_errors) + + assert_nil actual_response + end + + def test_empty_response_body_with_nullable_response_flag + response_body_mock = '' + response_mock = MockHelper.create_response status_code: 200, + raw_body: response_body_mock + actual_response = @response_handler.deserializer(ApiHelper.method(:deserialize_primitive_types)) + .is_primitive_response(true) + .is_nullable_response(true) + .deserialize_into(proc do |response_body| + response_body.to_i + end) + .handle(response_mock, MockHelper.get_global_errors) assert_nil actual_response actual_response = @response_handler + .is_nullable_response(true) .deserializer(ApiHelper.method(:custom_type_deserializer)) .deserialize_into(Validate.method(:from_hash)) .handle(response_mock, MockHelper.get_global_errors) @@ -345,6 +390,7 @@ def test_empty_response_body response_mock = MockHelper.create_response status_code: 200, raw_body: response_body_mock actual_response = @response_handler.deserializer(ApiHelper.method(:deserialize_primitive_types)) + .is_nullable_response(true) .is_primitive_response(true) .deserialize_into(proc do |response_body| response_body.to_i @@ -354,6 +400,7 @@ def test_empty_response_body assert_nil actual_response actual_response = @response_handler + .is_nullable_response(true) .deserializer(ApiHelper.method(:custom_type_deserializer)) .deserialize_into(Validate.method(:from_hash)) .handle(response_mock, MockHelper.get_global_errors)