Skip to content

Commit 2a39a4d

Browse files
committed
Instantiate validators at definition time
Validators are now instantiated once at route definition time rather than per-request, eliminating repeated allocation overhead. Instances are frozen to make them safe for sharing across requests. Freezing strategy: inputs (attrs, options, opts) are frozen at the DSL boundary before entering the validator, so subclass ivars derived from them are frozen by construction. Base.new reduces to super.freeze. Remove freeze_state! and ValidatorFactory. ParamsScope: precompute full_path via build_full_path before instance_eval so child scopes can read the parent path immediately. Simplify meets_hash_dependency? with all? and dependency.first. Validators::Base: add validation_error! helper to replace repeated Grape::Exceptions::Validation.new calls across single-attr validators. Fix DefaultValidator to always dup duplicable default values regardless of frozen state, preserving per-request isolation. Add DeepFreeze utility (freezes Hash/Array/String recursively, skips Procs, coercers, and classes). Add specs for DeepFreeze, SameAsValidator, and ExceptValuesValidator.
1 parent 1d8e5e9 commit 2a39a4d

28 files changed

Lines changed: 481 additions & 231 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* [#2663](https://github.com/ruby-grape/grape/pull/2663): Refactor `ParamsScope` and `Parameters` DSL to use named kwargs - [@ericproulx](https://github.com/ericproulx).
99
* [#2664](https://github.com/ruby-grape/grape/pull/2664): Drop `test-prof` dependency - [@ericproulx](https://github.com/ericproulx).
1010
* [#2665](https://github.com/ruby-grape/grape/pull/2665): Pass `attrs` directly to `AttributesIterator` instead of `validator` - [@ericproulx](https://github.com/ericproulx).
11+
* [#2657](https://github.com/ruby-grape/grape/pull/2657): Instantiate validators at definition time - [@ericproulx](https://github.com/ericproulx).
1112
* Your contribution here.
1213

1314
#### Fixes

UPGRADING.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,30 @@ Upgrading Grape
33

44
### Upgrading to >= 3.2
55

6+
#### Custom validators must not mutate instance variables at request time
7+
8+
Validator instances are now instantiated once at route definition time and frozen. Any custom validator that assigns or mutates instance variables inside `validate_param!` or `validate!` will raise `FrozenError` at request time.
9+
10+
```ruby
11+
# Before — broken in >= 3.2
12+
def validate_param!(attr_name, params)
13+
@computed = expensive_computation(params[attr_name]) # FrozenError
14+
raise Grape::Exceptions::Validation.new(...) unless @computed.valid?
15+
end
16+
17+
# After — compute in initialize, read at request time
18+
def initialize(attrs, options, required, scope, opts)
19+
super
20+
@computed = expensive_computation(@option)
21+
end
22+
23+
def validate_param!(attr_name, params)
24+
raise Grape::Exceptions::Validation.new(...) unless @computed.valid_for?(params[attr_name])
25+
end
26+
```
27+
28+
See [#2657](https://github.com/ruby-grape/grape/pull/2657) for more information.
29+
630
#### `with` now uses keyword arguments
731

832
The `with` DSL method now uses `**opts` instead of a positional hash. Calls using bare keyword syntax are unaffected:

lib/grape/endpoint.rb

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ def run
176176
status 204
177177
else
178178
run_filters before_validations, :before_validation
179-
run_validators validations, request
179+
run_validators request: request
180180
run_filters after_validations, :after_validation
181181
response_object = execute
182182
end
@@ -205,11 +205,14 @@ def execute
205205
end
206206
end
207207

208-
def run_validators(validators, request)
208+
def run_validators(request:)
209+
validators = inheritable_setting.route[:saved_validations]
210+
return if validators.empty?
211+
209212
validation_errors = []
210213

211214
Grape::Validations::ParamScopeTracker.track do
212-
ActiveSupport::Notifications.instrument('endpoint_run_validators.grape', endpoint: self, validators: validators, request: request) do
215+
ActiveSupport::Notifications.instrument('endpoint_run_validators.grape', endpoint: self, validators: validators, request:) do
213216
validators.each do |validator|
214217
validator.validate(request)
215218
rescue Grape::Exceptions::Validation => e
@@ -222,7 +225,7 @@ def run_validators(validators, request)
222225
end
223226
end
224227

225-
validation_errors.any? && raise(Grape::Exceptions::ValidationErrors.new(errors: validation_errors, headers: header))
228+
raise(Grape::Exceptions::ValidationErrors.new(errors: validation_errors, headers: header)) if validation_errors.any?
226229
end
227230

228231
def run_filters(filters, type = :other)
@@ -239,16 +242,6 @@ def run_filters(filters, type = :other)
239242
end
240243
end
241244

242-
def validations
243-
saved_validations = inheritable_setting.route[:saved_validations]
244-
return if saved_validations.nil?
245-
return enum_for(:validations) unless block_given?
246-
247-
saved_validations.each do |saved_validation|
248-
yield Grape::Validations::ValidatorFactory.create_validator(saved_validation)
249-
end
250-
end
251-
252245
def options?
253246
options[:options_route_enabled] &&
254247
env[Rack::REQUEST_METHOD] == Rack::OPTIONS

lib/grape/util/deep_freeze.rb

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# frozen_string_literal: true
2+
3+
module Grape
4+
module Util
5+
module DeepFreeze
6+
module_function
7+
8+
# Recursively freezes Hash (keys and values), Array (elements), and String
9+
# objects. All other types are returned as-is.
10+
#
11+
# Already-frozen objects (including Symbols, Integers, true/false/nil, and
12+
# any object that was previously frozen) are returned immediately via the
13+
# +obj.frozen?+ guard.
14+
#
15+
# Intentionally left unfrozen:
16+
# - Procs / lambdas — may be deferred DB-backed callables
17+
# - Coercers (e.g. ArrayCoercer) — use lazy ivar memoization at request time
18+
# - Classes / Modules — shared constants that must remain open
19+
# - ParamsScope — self-freezes at the end of its own initialize
20+
def deep_freeze(obj)
21+
return obj if obj.frozen?
22+
23+
case obj
24+
when Hash
25+
obj.each do |k, v|
26+
deep_freeze(k)
27+
deep_freeze(v)
28+
end
29+
obj.freeze
30+
when Array
31+
obj.each { |v| deep_freeze(v) }
32+
obj.freeze
33+
when String
34+
obj.freeze
35+
else
36+
obj
37+
end
38+
end
39+
end
40+
end
41+
end

lib/grape/validations/contract_scope.rb

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,7 @@ def initialize(api, contract = nil, &block)
2121
end
2222

2323
api.inheritable_setting.namespace_stackable[:contract_key_map] = key_map
24-
25-
validator_options = {
26-
validator_class: Grape::Validations.require_validator(:contract_scope),
27-
opts: { schema: contract, fail_fast: false }
28-
}
29-
30-
api.inheritable_setting.namespace_stackable[:validations] = validator_options
24+
api.inheritable_setting.namespace_stackable[:validations] = Validators::ContractScopeValidator.new(schema: contract)
3125
end
3226
end
3327
end

lib/grape/validations/params_scope.rb

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
module Grape
44
module Validations
55
class ParamsScope
6-
attr_accessor :element, :parent
7-
attr_reader :type, :nearest_array_ancestor
6+
attr_reader :parent, :type, :nearest_array_ancestor, :full_path
87

98
def qualifying_params
109
ParamScopeTracker.current&.qualifying_params(self)
@@ -21,7 +20,7 @@ def qualifying_params
2120
SPECIAL_JSON = [JSON, Array[JSON]].freeze
2221

2322
class Attr
24-
attr_accessor :key, :scope
23+
attr_reader :key, :scope
2524

2625
# Open up a new ParamsScope::Attr
2726
# @param key [Hash, Symbol] key of attr
@@ -77,11 +76,13 @@ def initialize(api:, element: nil, element_renamed: nil, parent: nil, optional:
7776
# instance_eval, so local variables from initialize are unreachable.
7877
# configure_declared_params consumes it and clears @declared_params to nil.
7978
@declared_params = []
79+
@full_path = build_full_path
8080

8181
instance_eval(&block) if block
8282

8383
configure_declared_params
8484
@nearest_array_ancestor = find_nearest_array_ancestor
85+
freeze
8586
end
8687

8788
def configuration
@@ -95,9 +96,9 @@ def should_validate?(parameters)
9596

9697
return false if @optional && (scoped_params.blank? || all_element_blank?(scoped_params))
9798
return false unless meets_dependency?(scoped_params, parameters)
98-
return true if parent.nil?
99+
return true if @parent.nil?
99100

100-
parent.should_validate?(parameters)
101+
@parent.should_validate?(parameters)
101102
end
102103

103104
def meets_dependency?(params, request_params)
@@ -124,17 +125,14 @@ def meets_hash_dependency?(params)
124125
# params might be anything what looks like a hash, so it must implement a `key?` method
125126
return false unless params.respond_to?(:key?)
126127

127-
@dependent_on.each do |dependency|
128+
@dependent_on.all? do |dependency|
128129
if dependency.is_a?(Hash)
129-
dependency_key = dependency.keys[0]
130-
proc = dependency.values[0]
131-
return false unless proc.call(params[dependency_key])
132-
elsif params[dependency].blank?
133-
return false
130+
key, callable = dependency.first
131+
callable.call(params[key])
132+
else
133+
params[dependency].present?
134134
end
135135
end
136-
137-
true
138136
end
139137

140138
# @return [String] the proper attribute name, with nesting considered.
@@ -194,21 +192,18 @@ def push_declared_params(attrs, **opts)
194192
@declared_params.concat(attrs.map { |attr| ::Grape::Validations::ParamsScope::Attr.new(attr, opts[:declared_params_scope]) })
195193
end
196194

197-
# Get the full path of the parameter scope in the hierarchy.
198-
#
199-
# @return [Array<Symbol>] the nesting/path of the current parameter scope
200-
def full_path
195+
private
196+
197+
def build_full_path
201198
if nested?
202-
(@parent.full_path + [@element])
199+
@parent.full_path + [@element]
203200
elsif lateral?
204201
@parent.full_path
205202
else
206203
[]
207204
end
208205
end
209206

210-
private
211-
212207
# Add a new parameter which should be renamed when using the +#declared+
213208
# method.
214209
#
@@ -314,17 +309,19 @@ def new_group_scope(group, &)
314309
self.class.new(api: @api, parent: self, group: group, &)
315310
end
316311

317-
# Pushes declared params to parent or settings
312+
# Pushes declared params to parent or settings, then clears @declared_params.
313+
# Clearing here (rather than in initialize) keeps the lifecycle ownership in
314+
# one place: this method both consumes and invalidates the ivar so that
315+
# push_declared_params cannot be called on the frozen scope later.
318316
def configure_declared_params
319317
push_renamed_param(full_path, @element_renamed) if @element_renamed
320318

321319
if nested?
322-
@parent.push_declared_params [element => @declared_params]
320+
@parent.push_declared_params [@element => @declared_params]
323321
else
324322
@api.inheritable_setting.namespace_stackable[:declared_params] = @declared_params
325323
end
326-
327-
# params were stored in settings, it can be cleaned from the params scope
324+
ensure
328325
@declared_params = nil
329326
end
330327

@@ -354,15 +351,15 @@ def validates(attrs, validations)
354351

355352
document_params attrs, validations, coerce_type, values, except_values
356353

357-
opts = derive_validator_options(validations)
354+
opts = derive_validator_options(validations).freeze
358355

359356
# Validate for presence before any other validators
360357
validates_presence(validations, attrs, opts)
361358

362359
# Before we run the rest of the validators, let's handle
363360
# whatever coercion so that we are working with correctly
364361
# type casted values
365-
coerce_type validations, attrs, required, opts
362+
coerce_type validations.extract!(:coerce, :coerce_with, :coerce_message), attrs, required, opts
366363

367364
validations.each do |type, options|
368365
# Don't try to look up validators for documentation params that don't have one.
@@ -435,17 +432,19 @@ def check_coerce_with(validations)
435432
def coerce_type(validations, attrs, required, opts)
436433
check_coerce_with(validations)
437434

438-
return unless validations.key?(:coerce)
435+
# Falsy check (not key?) is intentional: when a remountable API is first
436+
# evaluated on its base instance (no configuration supplied yet),
437+
# configuration[:some_type] evaluates to nil. Skipping instantiation
438+
# here is correct — the real mounted instance will replay this step with
439+
# the actual type value.
440+
return unless validations[:coerce]
439441

440442
coerce_options = {
441443
type: validations[:coerce],
442444
method: validations[:coerce_with],
443445
message: validations[:coerce_message]
444446
}
445447
validate('coerce', coerce_options, attrs, required, opts)
446-
validations.delete(:coerce_with)
447-
validations.delete(:coerce)
448-
validations.delete(:coerce_message)
449448
end
450449

451450
def guess_coerce_type(coerce_type, *values_list)
@@ -469,15 +468,15 @@ def check_incompatible_option_values(default, values, except_values)
469468
end
470469

471470
def validate(type, options, attrs, required, opts)
472-
validator_options = {
473-
attributes: attrs,
474-
options: options,
475-
required: required,
476-
params_scope: self,
477-
opts: opts,
478-
validator_class: Validations.require_validator(type)
479-
}
480-
@api.inheritable_setting.namespace_stackable[:validations] = validator_options
471+
validator_class = Validations.require_validator(type)
472+
validator_instance = validator_class.new(
473+
attrs,
474+
options,
475+
required,
476+
self,
477+
opts
478+
)
479+
@api.inheritable_setting.namespace_stackable[:validations] = validator_instance
481480
end
482481

483482
def validate_value_coercion(coerce_type, *values_list)

lib/grape/validations/validator_factory.rb

Lines changed: 0 additions & 15 deletions
This file was deleted.

lib/grape/validations/validators/all_or_none_of_validator.rb

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,16 @@ module Grape
44
module Validations
55
module Validators
66
class AllOrNoneOfValidator < MultipleParamsBase
7+
def initialize(attrs, options, required, scope, opts)
8+
super
9+
@exception_message = message(:all_or_none)
10+
end
11+
712
def validate_params!(params)
813
keys = keys_in_common(params)
9-
return if keys.empty? || keys.length == all_keys.length
14+
return if keys.empty? || keys.length == @attrs.length
1015

11-
raise Grape::Exceptions::Validation.new(params: all_keys, message: message(:all_or_none))
16+
raise Grape::Exceptions::Validation.new(params: all_keys, message: @exception_message)
1217
end
1318
end
1419
end

lib/grape/validations/validators/allow_blank_validator.rb

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,20 @@ module Grape
44
module Validations
55
module Validators
66
class AllowBlankValidator < Base
7-
def validate_param!(attr_name, params)
8-
return if (options_key?(:value) ? @option[:value] : @option) || !params.is_a?(Hash)
7+
def initialize(attrs, options, required, scope, opts)
8+
super
9+
10+
@value = option_value
11+
@exception_message = message(:blank)
12+
end
913

10-
value = params[attr_name]
11-
value = value.scrub if value.respond_to?(:valid_encoding?) && !value.valid_encoding?
14+
def validate_param!(attr_name, params)
15+
return if @value || !hash_like?(params)
1216

17+
value = scrub(params[attr_name])
1318
return if value == false || value.present?
1419

15-
raise Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: message(:blank))
20+
validation_error!(attr_name)
1621
end
1722
end
1823
end

0 commit comments

Comments
 (0)