diff --git a/CHANGELOG.md b/CHANGELOG.md index dabfb17a1..4df0a62b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ * [#3071](https://github.com/ruby-grape/grape/pull/3071): Skip the query parse when building the params of a request with no query string - [@ericproulx](https://github.com/ericproulx). * [#3072](https://github.com/ruby-grape/grape/pull/3072): Answer the `Accept` lists axios, jQuery and HTTPie send without negotiating them on every request - [@ericproulx](https://github.com/ericproulx). * [#3073](https://github.com/ruby-grape/grape/pull/3073): Let a `rescue_from :all` in a namespace or mounted API outrank the enclosing scope's handlers - [@ericproulx](https://github.com/ericproulx). +* [#3074](https://github.com/ruby-grape/grape/pull/3074): Speed up `declared` by working out each param's key and empty value when the endpoint compiles - [@ericproulx](https://github.com/ericproulx). * [#3075](https://github.com/ruby-grape/grape/pull/3075): Validate the params of an optional Hash scope without the attributes iterator - [@ericproulx](https://github.com/ericproulx). * [#3076](https://github.com/ruby-grape/grape/pull/3076): Skip the query merge when building the params of a body sent without a query string - [@ericproulx](https://github.com/ericproulx). * Your contribution here. diff --git a/lib/grape/declared_params.rb b/lib/grape/declared_params.rb new file mode 100644 index 000000000..5a9a4c89c --- /dev/null +++ b/lib/grape/declared_params.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +module Grape + # The params an endpoint declares, as +declared+ filters a Hash by them. + # + # What +declared+ answers for a param depends on the request only for its + # value. The key it is filed under -- its name, or the one +as:+ renamed it + # to -- and the empty value it stands for when it was not passed -- an + # Array, a Set or a Hash, after its documented type -- are settled by the + # declaration. Both were worked out again on every call, for every param: + # the path to the param built to look its new name up, its documented name + # spelled out to look its type up, that type matched against the ones that + # have an empty value. They are worked out here once, when the endpoint + # compiles, as Validations::UndeclaredParams is. + class DeclaredParams + extend Grape::Util::FreezeOnNew + + # A declared param: the +name+ it is read by, the +key+ and +string_key+ + # it is filed under, the params declared under it (+children+, nil for a + # param declared without a block), the +empty_value+ it stands for when + # it was not passed (nil when its type has none), and the +scope+ that + # declared it, whose +given+ +evaluate_given+ asks about. + Param = Data.define(:name, :key, :string_key, :children, :empty_value, :scope) + + EMPTY_HASH = {}.freeze + EMPTY_ARRAY = [].freeze + EMPTY_SET = Set.new.freeze + private_constant :EMPTY_HASH, :EMPTY_ARRAY, :EMPTY_SET + + # +all+ are the params of the endpoint and of its namespaces, +own+ the + # ones of the endpoint's own +params+ block, which is what + # +include_parent_namespaces: false+ keeps. + attr_reader :all, :own, :contract_key_maps + + # @param declared_params [Array] the + # endpoint's declared params, its namespaces' included + # @param own_declared_params [Array] the + # ones its own +params+ block declares + # @param route_params [Hash] the endpoint's documented params, by full name + # @param renamed_params [Hash] the names +as:+ gave, by the path of the + # param they rename + # @param contract_key_maps [Array] the keys of the + # endpoint's contracts + def initialize(declared_params:, own_declared_params:, route_params:, renamed_params:, contract_key_maps:) + @route_params = route_params + @renamed_params = renamed_params + @contract_key_maps = contract_key_maps + # An endpoint's own params are among all of them, and compile once. + compiled = {}.compare_by_identity + @all = declared_params.flat_map { |attr| compiled[attr] ||= compile(attr, nil, nil) }.freeze + @own = own_declared_params.flat_map { |attr| compiled[attr] ||= compile(attr, nil, nil) }.freeze + end + + private + + # The params of a ParamsScope::Attr, whose key is a name, or a Hash of + # names to the Attrs declared under them. +parent_name+ is the enclosing + # param as the route documents it (+"a[b]"+) and +parent_path+ the names + # leading to it (+%w[a b]+), both nil at the root. + def compile(attr, parent_name, parent_path) + declared = attr.key + return [param(declared, nil, attr.scope, parent_name, parent_path)] unless declared.is_a?(Hash) + + declared.map { |name, children| param(name, children, attr.scope, parent_name, parent_path) } + end + + def param(name, children, scope, parent_name, parent_path) + path = [*parent_path, name.to_s] + full_name = parent_name ? "#{parent_name}[#{name}]" : name.to_s + key = @renamed_params[path] || name + Param.new( + name:, + # A name read from +configuration+ is nil until the API is mounted + # with one, and an API compiles unmounted too. + key: key.respond_to?(:to_sym) ? key.to_sym : key, + string_key: -key.to_s, + children: children&.flat_map { |child| compile(child, full_name, path) }.freeze, + empty_value: empty_value_for(full_name), + scope: + ) + end + + # A Hash is only empty when no param is declared under it: one that + # declares some stands for a Hash of them instead, each nil or empty in + # turn. + def empty_value_for(full_name) + type = documented_type(full_name) + return unless type + return EMPTY_HASH if type == 'Hash' && no_child_params?("#{full_name}[") + return EMPTY_ARRAY if array_type?(type) + + EMPTY_SET if type.start_with?('#] the params + # to keep of them + # # An element of an Array scope that is not a Hash has no keys to filter, # so it is returned as it came in, the way a leaf value is. Validation # lets one through wherever nothing in the scope is required of it: an @@ -27,15 +20,13 @@ def call(passed_params, declared_params, route_params, renamed_params) # all optional checks nothing on an element that is not a Hash. Filtering # it anyway meant building an empty copy of it, which raised for +nil+, # +false+ and numbers and indexed a String with a Symbol. - def recursive_declared(passed_params, declared_params:, route_params:, renamed_params:, params_nested_path: []) + def call(passed_params, declared_params) return passed_params unless passed_params.is_a?(Array) || passed_params.respond_to?(:key?) res = if passed_params.is_a?(Array) - passed_params.map do |passed_param| - recursive_declared(passed_param, declared_params:, params_nested_path:, renamed_params:, route_params:) - end + passed_params.map { |passed_param| call(passed_param, declared_params) } else - declared_hash(passed_params, declared_params:, params_nested_path:, renamed_params:, route_params:) + declared_hash(passed_params, declared_params) end @contract_key_map.each { |key_map| key_map.write(passed_params, res) } @@ -43,102 +34,38 @@ def recursive_declared(passed_params, declared_params:, route_params:, renamed_p res end - def declared_hash(passed_params, declared_params:, params_nested_path:, renamed_params:, route_params:) - declared_params.each_with_object(passed_params.class.new) do |declared_param_attr, memo| - next if @evaluate_given && !declared_param_attr.scope.attr_meets_dependency?(passed_params) - - declared_hash_attr( - passed_params, - declared_param: declared_param_attr.key, - params_nested_path:, - memo:, - renamed_params:, - route_params: - ) - end - end - - def declared_hash_attr(passed_params, declared_param:, params_nested_path:, memo:, renamed_params:, route_params:) - return declare_leaf(passed_params, declared_param:, params_nested_path:, memo:, renamed_params:, route_params:) unless declared_param.is_a?(Hash) - - declared_param.each_pair do |parent, children| - declare_nested(passed_params, parent:, children:, params_nested_path:, memo:, renamed_params:, route_params:) - end - end - - def declare_nested(passed_params, parent:, children:, params_nested_path:, memo:, renamed_params:, route_params:) - return unless @include_missing || passed_params.key?(parent) - - memo_key = build_memo_key(params_nested_path, parent, renamed_params) - passed_children = passed_params[parent] || passed_params.class.new - nested_path = nested_path_for(params_nested_path, parent) - # A value that is neither a Hash nor an Array where children are declared - # -- left unvalidated by a +given+ whose dependency is not met, say -- is - # returned as it came in, as #recursive_declared returns an element like - # it. - has_passed_children = !passed_children.respond_to?(:any?) || passed_children.any? - - memo[memo_key] = handle_passed_param(nested_path, route_params:, has_passed_children:) do - recursive_declared( - passed_children, - declared_params: children, - params_nested_path: nested_path, - renamed_params:, - route_params: - ) - end - end + private - # The declared param has no children. Find its value or set it to nil. - def declare_leaf(passed_params, declared_param:, params_nested_path:, memo:, renamed_params:, route_params:) - return unless @include_missing || (passed_params.respond_to?(:key?) && passed_params.key?(declared_param)) + def declared_hash(passed_params, declared_params) + declared_params.each_with_object(passed_params.class.new) do |declared_param, memo| + next if @evaluate_given && !declared_param.scope.attr_meets_dependency?(passed_params) - memo_key = build_memo_key(params_nested_path, declared_param, renamed_params) - passed_param = passed_params[declared_param] + name = declared_param.name + next unless @include_missing || passed_params.key?(name) - memo[memo_key] = passed_param || handle_passed_param(nested_path_for(params_nested_path, declared_param), route_params:) do - passed_param + memo[@stringify ? declared_param.string_key : declared_param.key] = declared_value(passed_params[name], declared_param, passed_params) end end - # The lookup key is the param's whole path, built fresh -- two Arrays and a - # String per declared param -- and only an API using +as:+ has anything to - # find with it, so the common empty table is not asked. - def build_memo_key(params_nested_path, declared_param, renamed_params) - renamed_param_name = renamed_params[nested_path_for(params_nested_path, declared_param)] unless renamed_params.empty? - param = renamed_param_name || declared_param - @stringify ? param.to_s : param.to_sym - end - - def nested_path_for(parent_path, key) - parent_path + [key.to_s] - end - - def handle_passed_param(params_nested_path, route_params:, has_passed_children: false) - return yield if has_passed_children - - key = params_nested_path.first - key += "[#{params_nested_path[1..].join('][')}]" if params_nested_path.size > 1 + # What a declared param is filed as: its value, or the empty value its + # type has when that is nil, a fresh one every time since the caller may + # fill it. + def declared_value(passed_param, declared_param, passed_params) + children = declared_param.children + return declared_children(passed_param || passed_params.class.new, declared_param, children) if children - type = route_params.dig(key, :type) - return yield if type.nil? - - empty_value_for(type, key, route_params) || yield + passed_param || declared_param.empty_value&.dup || passed_param end - # The empty value a param of +type+ declares when it was not passed, or - # nil when +type+ has none. A fresh one every time: the caller may fill it. - def empty_value_for(type, key, route_params) - return {} if type == 'Hash' && no_child_params?(route_params, "#{key}[") - return [] if type == 'Array' || (type.start_with?('[') && !type.include?(',')) - - Set.new if type.start_with?('# nil, 'address' => 'somewhere') end end + + describe 'a param a desc documents in its own way' do + let(:app) do + Class.new(Grape::API) do + format :json + + desc 'documented by hand', params: { 'limit' => { type: Integer }, 'note' => 'free text', 'ids' => { type: 'Array' } } + params do + optional :limit, type: Integer + optional :note, type: String + optional :ids, type: String + end + get('/') { declared(params) } + end + end + + # The type decides the empty value a param that was not passed stands + # for, and only a type spelled as a String names one. + it 'has the empty value of the type the desc names, and none when it names no type as a String' do + get '/' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)).to eq('limit' => nil, 'note' => nil, 'ids' => []) + end + end + + describe 'a param named by the configuration' do + let(:api) do + Class.new(Grape::API) do + format :json + + params do + requires configuration[:name], type: String + optional :tags, type: Array + end + get('/') { declared(params) } + end + end + let(:app) do + mounted = api + Class.new(Grape::API) { mount mounted, with: { name: :title } } + end + + it 'compiles before the API is mounted, when it has no name yet' do + expect { api.compile! }.not_to raise_error + end + + it 'is filed under the name the API is mounted with' do + get '/', title: 'grape', junk: 1 + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)).to eq('title' => 'grape', 'tags' => []) + end + end + + describe 'an empty value' do + let(:app) do + Class.new(Grape::API) do + format :json + + params do + optional :tags, type: Array + optional :meta, type: Hash + optional :ids, type: Set + end + get '/' do + first = declared(params) + first[:tags] << 'filled' + first[:meta][:filled] = true + first[:ids] << 1 + declared(params).transform_values(&:size) + end + end + end + + it 'is a new one for every call' do + 2.times do + get '/' + expect(JSON.parse(last_response.body)).to eq('tags' => 0, 'meta' => 0, 'ids' => 0) + end + end + end end