From 47ccfd27eb7b5e112f3cf9e69f13d9444237a22e Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Tue, 10 Nov 2026 03:04:45 +0100 Subject: [PATCH] Work out each declared param's key and empty value when the endpoint compiles `declared` answers, for each declared param, the key it is filed under (its name, or the one `as:` gave it) and the empty value it stands for when it was not passed (an Array, a Set or a Hash, after its documented type). Neither depends on the request, and both were worked out again on every call: 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, and for a Hash every documented param scanned for a child. `Grape::DeclaredParams` holds them per endpoint, built by `Endpoint#compile!` as `Validations::UndeclaredParams` is, and `DeclaredParamsHandler` walks its params. A param a `desc` documents by hand with a type that is not a String, or with no Hash of details, has no empty value. `declared` raised on it when it was not passed. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 1 + lib/grape/declared_params.rb | 114 +++++++++++++++++++++++ lib/grape/declared_params_handler.rb | 133 ++++++--------------------- lib/grape/dsl/declared.rb | 10 +- lib/grape/endpoint.rb | 20 +++- lib/grape/endpoint/state.rb | 9 +- spec/grape/endpoint/declared_spec.rb | 80 ++++++++++++++++ 7 files changed, 252 insertions(+), 115 deletions(-) create mode 100644 lib/grape/declared_params.rb 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