From 2f46147f8b83f656c37b95ed74cf601400b141f4 Mon Sep 17 00:00:00 2001 From: iMacTia Date: Mon, 4 Mar 2019 14:16:16 +0000 Subject: [PATCH 1/4] Fixes Rubocop Metrics/BlockLength --- .rubocop_todo.yml | 6 - lib/faraday/encoders/nested_params_encoder.rb | 129 +++++++++--------- 2 files changed, 66 insertions(+), 69 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0d726656a..549025df8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -10,12 +10,6 @@ Metrics/AbcSize: Max: 79 -# Offense count: 2 -# Configuration parameters: CountComments, ExcludedMethods. -# ExcludedMethods: refine -Metrics/BlockLength: - Max: 27 - # Offense count: 5 # Configuration parameters: CountComments. Metrics/ClassLength: diff --git a/lib/faraday/encoders/nested_params_encoder.rb b/lib/faraday/encoders/nested_params_encoder.rb index e51273b1c..1ee8f3f65 100644 --- a/lib/faraday/encoders/nested_params_encoder.rb +++ b/lib/faraday/encoders/nested_params_encoder.rb @@ -10,12 +10,14 @@ class << self def_delegators :'Faraday::Utils', :escape, :unescape end + extend self + # @param params [nil, Array, #to_hash] parameters to be encoded # # @return [String] the encoded params # # @raise [TypeError] if params can not be converted to a Hash - def self.encode(params) + def encode(params) return nil if params.nil? unless params.is_a?(Array) @@ -33,42 +35,11 @@ def self.encode(params) params.sort! end - # Helper lambda - to_query = lambda do |parent, value| - if value.is_a?(Hash) - value = value.map do |key, val| - key = escape(key) - [key, val] - end - value.sort! - buffer = +'' - value.each do |key, val| - new_parent = "#{parent}%5B#{key}%5D" - buffer << "#{to_query.call(new_parent, val)}&" - end - return buffer.chop - elsif value.is_a?(Array) - new_parent = "#{parent}%5B%5D" - return new_parent if value.empty? - - buffer = +'' - value.each do |val| - buffer << "#{to_query.call(new_parent, val)}&" - end - return buffer.chop - elsif value.nil? - return parent - else - encoded_value = escape(value) - return "#{parent}=#{encoded_value}" - end - end - # The params have form [['key1', 'value1'], ['key2', 'value2']]. buffer = +'' params.each do |parent, value| encoded_parent = escape(parent) - buffer << "#{to_query.call(encoded_parent, value)}&" + buffer << "#{encode_pair(encoded_parent, value)}&" end buffer.chop end @@ -78,7 +49,7 @@ def self.encode(params) # @return [Array] the decoded params # # @raise [TypeError] if the nesting is incorrect - def self.decode(query) + def decode(query) return nil if query.nil? params = {} @@ -88,43 +59,18 @@ def self.decode(query) key, value = pair.split('=', 2) key = unescape(key) value = unescape(value.tr('+', ' ')) if value - - subkeys = key.scan(/[^\[\]]+(?:\]?\[\])?/) - context = params - subkeys.each_with_index do |subkey, i| - is_array = subkey =~ /[\[\]]+\Z/ - subkey = $` if is_array - last_subkey = i == subkeys.length - 1 - - if !last_subkey || is_array - value_type = is_array ? Array : Hash - raise TypeError, format("expected #{value_type.name} (got #{context[subkey].class.name}) for param `#{subkey}'") if context[subkey] && !context[subkey].is_a?(value_type) - - context = (context[subkey] ||= value_type.new) - end - - if context.is_a?(Array) && !is_array - context << {} if !context.last.is_a?(Hash) || context.last.key?(subkey) - context = context.last - end - - if last_subkey - if is_array - context << value - else - context[subkey] = value - end - end - end + decode_pair(key, value, params) end dehash(params, 0) end + private + # Internal: convert a nested hash with purely numeric keys into an array. # FIXME: this is not compatible with Rack::Utils.parse_nested_query # @!visibility private - def self.dehash(hash, depth) + def dehash(hash, depth) hash.each do |key, value| hash[key] = dehash(value, depth + 1) if value.is_a?(Hash) end @@ -135,5 +81,62 @@ def self.dehash(hash, depth) hash end end + + def encode_pair(parent, value) + if value.is_a?(Hash) + value = value.map do |key, val| + key = escape(key) + [key, val] + end + value.sort! + buffer = +'' + value.each do |key, val| + new_parent = "#{parent}%5B#{key}%5D" + buffer << "#{encode_pair(new_parent, val)}&" + end + buffer.chop + elsif value.is_a?(Array) + new_parent = "#{parent}%5B%5D" + return new_parent if value.empty? + + buffer = +'' + value.each { |val| buffer << "#{encode_pair(new_parent, val)}&" } + buffer.chop + elsif value.nil? + parent + else + encoded_value = escape(value) + "#{parent}=#{encoded_value}" + end + end + + def decode_pair(key, value, context) + subkeys = key.scan(/[^\[\]]+(?:\]?\[\])?/) + subkeys.each_with_index do |subkey, i| + is_array = subkey =~ /[\[\]]+\Z/ + subkey = $` if is_array + last_subkey = i == subkeys.length - 1 + + if !last_subkey || is_array + value_type = is_array ? Array : Hash + raise TypeError, format("expected #{value_type.name} (got #{context[subkey].class.name}) for param `#{subkey}'") if context[subkey] && !context[subkey].is_a?(value_type) + + context = (context[subkey] ||= value_type.new) + end + + if context.is_a?(Array) && !is_array + context << {} if !context.last.is_a?(Hash) || context.last.key?(subkey) + context = context.last + end + + if last_subkey + if is_array + context << value + else + context[subkey] = value + end + end + end + end end end From 56d6dd8f85936e7f2e77dac182e4a561c79687ce Mon Sep 17 00:00:00 2001 From: iMacTia Date: Mon, 4 Mar 2019 15:44:17 +0000 Subject: [PATCH 2/4] Reduces #encode_pair complexity --- lib/faraday/encoders/nested_params_encoder.rb | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/faraday/encoders/nested_params_encoder.rb b/lib/faraday/encoders/nested_params_encoder.rb index 1ee8f3f65..2783b0360 100644 --- a/lib/faraday/encoders/nested_params_encoder.rb +++ b/lib/faraday/encoders/nested_params_encoder.rb @@ -67,13 +67,13 @@ def decode(query) private + SUBKEYS_REGEX = /[^\[\]]+(?:\]?\[\])?/.freeze + # Internal: convert a nested hash with purely numeric keys into an array. # FIXME: this is not compatible with Rack::Utils.parse_nested_query # @!visibility private def dehash(hash, depth) - hash.each do |key, value| - hash[key] = dehash(value, depth + 1) if value.is_a?(Hash) - end + hash.each { |key, value| hash[key] = dehash(value, depth + 1) if value.is_a?(Hash) } if depth.positive? && !hash.empty? && hash.keys.all? { |k| k =~ /^\d+$/ } hash.keys.sort.inject([]) { |all, key| all << hash[key] } @@ -84,24 +84,9 @@ def dehash(hash, depth) def encode_pair(parent, value) if value.is_a?(Hash) - value = value.map do |key, val| - key = escape(key) - [key, val] - end - value.sort! - buffer = +'' - value.each do |key, val| - new_parent = "#{parent}%5B#{key}%5D" - buffer << "#{encode_pair(new_parent, val)}&" - end - buffer.chop + encode_hash(parent, value) elsif value.is_a?(Array) - new_parent = "#{parent}%5B%5D" - return new_parent if value.empty? - - buffer = +'' - value.each { |val| buffer << "#{encode_pair(new_parent, val)}&" } - buffer.chop + encode_array(parent, value) elsif value.nil? parent else @@ -110,8 +95,28 @@ def encode_pair(parent, value) end end + def encode_hash(parent, value) + value = value.map { |key, val| [escape(key), val] }.sort + + buffer = +'' + value.each do |key, val| + new_parent = "#{parent}%5B#{key}%5D" + buffer << "#{encode_pair(new_parent, val)}&" + end + buffer.chop + end + + def encode_array(parent, value) + new_parent = "#{parent}%5B%5D" + return new_parent if value.empty? + + buffer = +'' + value.each { |val| buffer << "#{encode_pair(new_parent, val)}&" } + buffer.chop + end + def decode_pair(key, value, context) - subkeys = key.scan(/[^\[\]]+(?:\]?\[\])?/) + subkeys = key.scan(SUBKEYS_REGEX) subkeys.each_with_index do |subkey, i| is_array = subkey =~ /[\[\]]+\Z/ subkey = $` if is_array From c90ac32b900d1f64f3ad5759b540e48ab4f53980 Mon Sep 17 00:00:00 2001 From: iMacTia Date: Mon, 4 Mar 2019 16:28:36 +0000 Subject: [PATCH 3/4] Reduces #decode_pair complexity --- lib/faraday/encoders/nested_params_encoder.rb | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/lib/faraday/encoders/nested_params_encoder.rb b/lib/faraday/encoders/nested_params_encoder.rb index 2783b0360..0e132fdfe 100644 --- a/lib/faraday/encoders/nested_params_encoder.rb +++ b/lib/faraday/encoders/nested_params_encoder.rb @@ -21,10 +21,8 @@ def encode(params) return nil if params.nil? unless params.is_a?(Array) - unless params.respond_to?(:to_hash) - raise TypeError, - "Can't convert #{params.class} into Hash." - end + raise TypeError, "Can't convert #{params.class} into Hash." unless params.respond_to?(:to_hash) + params = params.to_hash params = params.map do |key, value| key = key.to_s if key.is_a?(Symbol) @@ -122,26 +120,31 @@ def decode_pair(key, value, context) subkey = $` if is_array last_subkey = i == subkeys.length - 1 - if !last_subkey || is_array - value_type = is_array ? Array : Hash - raise TypeError, format("expected #{value_type.name} (got #{context[subkey].class.name}) for param `#{subkey}'") if context[subkey] && !context[subkey].is_a?(value_type) + context = prepare_context(context, subkey, is_array, last_subkey) + add_to_context(is_array, context, value, subkey) if last_subkey + end + end - context = (context[subkey] ||= value_type.new) - end + def prepare_context(context, subkey, is_array, last_subkey) + context = new_context(subkey, is_array, context) if !last_subkey || is_array + context = match_context(context, subkey) if context.is_a?(Array) && !is_array + context + end - if context.is_a?(Array) && !is_array - context << {} if !context.last.is_a?(Hash) || context.last.key?(subkey) - context = context.last - end + def new_context(subkey, is_array, context) + value_type = is_array ? Array : Hash + raise TypeError, "expected #{value_type.name} (got #{context[subkey].class.name}) for param `#{subkey}'" if context[subkey] && !context[subkey].is_a?(value_type) - if last_subkey - if is_array - context << value - else - context[subkey] = value - end - end - end + context[subkey] ||= value_type.new + end + + def match_context(context, subkey) + context << {} if !context.last.is_a?(Hash) || context.last.key?(subkey) + context.last + end + + def add_to_context(is_array, context, value, subkey) + is_array ? context << value : context[subkey] = value end end end From 4ad3c609695178dc71ad9fa870043d3200fa82de Mon Sep 17 00:00:00 2001 From: iMacTia Date: Mon, 4 Mar 2019 16:37:07 +0000 Subject: [PATCH 4/4] Adds cool refactoring of hash values sorted by hash key. Thanks to Olle collegue --- lib/faraday/encoders/nested_params_encoder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/faraday/encoders/nested_params_encoder.rb b/lib/faraday/encoders/nested_params_encoder.rb index 0e132fdfe..b1e200df4 100644 --- a/lib/faraday/encoders/nested_params_encoder.rb +++ b/lib/faraday/encoders/nested_params_encoder.rb @@ -74,7 +74,7 @@ def dehash(hash, depth) hash.each { |key, value| hash[key] = dehash(value, depth + 1) if value.is_a?(Hash) } if depth.positive? && !hash.empty? && hash.keys.all? { |k| k =~ /^\d+$/ } - hash.keys.sort.inject([]) { |all, key| all << hash[key] } + hash.sort.map(&:last) else hash end