Speed up group validators inside large Array scopes

[braktar]

mutually_exclusive, exactly_one_of, at_least_one_of and all_or_none_of all go through MultipleParamsBase#keys_in_common (or the same full_name-over-every-key pattern in AtLeastOneOfValidator).

Previously, for every element of an Array scope, the validator:

1. called scope.full_name on every key of the element hash
2. intersected that set with the (small) list of declared group attrs

That is O(request_keys × nesting) per element, even when the group only declares 2–3 attrs. On large payloads this dominates validation time; the cost grows with unrelated keys on each element, not with the exclusivity check itself.

Change

Only inspect the declared group attrs, and call full_name for those that are present (still needed for error path names):

attrs.filter_map do |attr|
  scope.full_name(attr) if resource_params.key?(attr)
end

Same idea for AtLeastOneOfValidator (attrs.any? { params.key?(attr) }).

Behavior and error messages are unchanged.

Motivation

Hit in production on the API on which is based the large_model benchmark: nested mutually_exclusive under services / quantities / timewindows (hundreds–thousands of elements). Profiling showed MutuallyExclusiveValidator as the top Grape validator cost (~0.19s of ~0.34s validators on a ~429-service payload), largely from full_name work inside keys_in_common.

Benchmark

Fixture: POST JSON with requires :items, type: Array, each element ~15 optional keys, and 3 mutually_exclusive rules (beer/wine, pickup/value, delivery/value). Happy-path body (no validation error). Warmup 3 calls, then average of 5.

N (array size)	OLD (full_name on all keys)	NEW (attrs only)	Speedup	Saved
830	0.155 s	0.040 s	3.8×	0.11 s
2000	0.381 s	0.047 s	8.2×	0.33 s

Repro sketch:

# mode = 'old' | 'new'  - for 'old', reopen MultipleParamsBase#keys_in_common
# to the previous intersect-all-keys implementation before defining the API.
#
# params do
#   requires :items, type: Array do
#     optional :beer, :wine, :pickup, :delivery, :a, :b, ... # many keys
#     mutually_exclusive :beer, :wine
#     mutually_exclusive :pickup, :value
#     mutually_exclusive :delivery, :value
#   end
# end

Gains scale with (elements × keys_per_element × group_rules); small flat params blocks see little difference.

[github-actions]

Danger Report

No issues found.

View run

[ericproulx]

Thanks for this — the premise is right and worth landing. full_name on every request key is genuinely the wrong shape for a large Array scope.

But keys_in_common was doing two things at once, and only one of them was the slow one. Mapping both sides through full_name was also what made the comparison key-type agnostic and what deduped the group (Array#&). Swapping in resource_params.key?(attr) + filter_map drops both, so "behavior and error messages are unchanged" doesn't quite hold. Two behaviour changes, both reproduced against master at c69a0cb:

1. The lookup is now key-type sensitive

known_keys & resource_params.keys.map! { |attr| scope.full_name(attr) } ran the request keys and the declared attrs through full_name, which stringifies, so :beer and 'beer' compared equal. key? is exact.

                                                  master      this PR
string attrs + build_with :hash (both set)     => 400      => 201   <-- rule silently dropped
string attrs + default builder (both set)      => 400      => 400
symbol attrs + build_with :hash (both set)     => 400      => 400

build_with :hash is deep_symbolize_keys, so a group declared as mutually_exclusive 'beer', 'wine' stops matching entirely — no error, no warning, the request just passes. The default :hash_with_indifferent_access and :hashie_mash builders normalize on key?, which is why the suite doesn't catch it; any custom builder returning a plain Hash has the same exposure. at_least_one_of fails the other way (a valid request 400s), and exactly_one_of rejects every request.

2. filter_map doesn't dedup, Array#& did

exactly_one_of(*%i[beer beer wine]), beer only    => master 201, PR 400 "beer, beer are mutually exclusive"
all_or_none_of(*%i[beer beer wine]), beer+wine    => master 400, PR 201

Realistic when the group is splatted from a computed list. .uniq on the attrs at construction fixes this and the doubled name in the message.

Regression specs

These four pass on master and fail on this branch (4 examples, 4 failures):

# frozen_string_literal: true

describe 'group validator regressions' do
  subject(:validate) { post path, params }

  let(:path) { '/' }

  describe Grape::Validations::Validators::MutuallyExclusiveValidator do
    let(:app) do
      Class.new(Grape::API) do
        build_with :hash

        rescue_from Grape::Exceptions::ValidationErrors do |e|
          error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
        end

        params do
          optional :beer
          optional :wine
          mutually_exclusive 'beer', 'wine'
        end
        post do
        end
      end
    end

    context 'when the group names its attrs as Strings under a symbolizing params builder' do
      let(:params) { { beer: true, wine: true } }

      it 'returns a validation error' do
        validate
        expect(last_response.status).to eq 400
        expect(JSON.parse(last_response.body)).to eq('beer,wine' => ['are mutually exclusive'])
      end
    end
  end

  describe Grape::Validations::Validators::AtLeastOneOfValidator do
    let(:app) do
      Class.new(Grape::API) do
        build_with :hash

        rescue_from Grape::Exceptions::ValidationErrors do |e|
          error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
        end

        params do
          optional :beer
          optional :wine
          at_least_one_of 'beer', 'wine'
        end
        post do
        end
      end
    end

    context 'when the group names its attrs as Strings under a symbolizing params builder' do
      let(:params) { { beer: true } }

      it 'does not return a validation error' do
        validate
        expect(last_response.status).to eq 201
      end
    end
  end

  describe Grape::Validations::Validators::ExactlyOneOfValidator do
    let(:app) do
      Class.new(Grape::API) do
        rescue_from Grape::Exceptions::ValidationErrors do |e|
          error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
        end

        params do
          optional :beer
          optional :wine
          exactly_one_of(*%i[beer beer wine])
        end
        post do
        end
      end
    end

    context 'when the same attr is named twice in the group' do
      let(:params) { { beer: true } }

      it 'does not return a validation error' do
        validate
        expect(last_response.status).to eq 201
      end
    end
  end

  describe Grape::Validations::Validators::AllOrNoneOfValidator do
    let(:app) do
      Class.new(Grape::API) do
        rescue_from Grape::Exceptions::ValidationErrors do |e|
          error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
        end

        params do
          optional :beer
          optional :wine
          all_or_none_of(*%i[beer beer wine])
        end
        post do
        end
      end
    end

    context 'when the same attr is named twice in the group' do
      let(:params) { { beer: true, wine: true } }

      it 'returns a validation error' do
        validate
        expect(last_response.status).to eq 400
        expect(JSON.parse(last_response.body)).to eq(
          'beer,beer,wine' => ['provide all or none of parameters']
        )
      end
    end
  end
end

They want splitting across the four existing spec/grape/validations/validators/*_spec.rb files rather than landing as one file — they're written together here so you can drop them in and watch them go red.

The optimization can go further

None of the four callers need a full name unless they are about to raise: mutually_exclusive wants a count, exactly_one_of == 1, all_or_none_of count == attrs.length, at_least_one_of a boolean. Full names are error-message formatting. Measured on this PR's own shape (2000-element Array scope, one present attr per element, happy path):

	ms/validate
keys_in_common as written here	2.33
attrs.select { |a| resource_params.key?(a) }, .map { scope.full_name(_1) } moved into validation_error!	0.87

62% of what's left is full_name calls whose result is discarded — and it isn't cheap: a fiber-local ParamScopeTracker.current, a recursive walk up the parent scopes, and a string interpolation per call.

That restructure also makes the two bugs above one-line fixes instead of two, because the key lookup and the name formatting stop being the same expression. Right now AtLeastOneOfValidator re-implements the lookup inline (attrs.any? { |attr| params.key?(attr) }) rather than calling the shared helper, so the key-matching rule lives in two files and fix #1 has to be written twice. A shared present_attrs / any_attr_present? pair would keep one definition.

Smaller things

• exactly_one_of_validator.rb:17 and all_or_none_of_validator.rb:13 now recompute all_keys on the error path for attrs keys_in_common just built names for. known_keys used to be computed once and reused. This disappears for free under the restructure above.
• keys_in_common's arity changed from (resource_params, known_keys = all_keys) to (resource_params). It's private, so no UPGRADING entry is needed, but subclassing a validator base is the documented way to add a group validator, and an out-of-tree one passing the second argument now raises ArgumentError at request time. known_keys = nil falling through to the new path would cost nothing.
• The CHANGELOG entry goes at the bottom of #### Fixes, directly above * Your contribution here. — every other 4.1.0 entry is appended, and putting it at the top will conflict with the other open PRs.
• The whitespace fix at CHANGELOG.md:350 (the 3.x section) is unrelated to group validators — worth dropping from this PR.
• The doc comment on keys_in_common restates the method name and then describes the implementation it replaced. Per AGENTS.md, that's PR-description material; comments are for why something non-obvious is happening. Same for the comment at mutually_exclusive_spec.rb:264.

On the added specs

Worth knowing: copying this PR's mutually_exclusive_spec.rb onto master gives 12 examples, 0 failures — both new examples pass without the change, so they don't currently guard it. The first one asserts 201, which a completely broken mutually_exclusive also satisfies. And at_least_one_of, exactly_one_of and all_or_none_of are all rewritten here with no new coverage; exactly_one_of in particular moved from known_keys.intersect? semantics to a raw count.

🤖 Generated with Claude Code

[braktar]

Thanks for the review, all points landed.

Addressed in the follow-up commit:

1. Key-type matching — attr_present? tries both Symbol and String spellings, so mutually_exclusive 'beer', 'wine' still works under build_with :hash.
2. Dedup — present_attrs ends with .uniq, matching the old Array#& behaviour (duplicate names in a splatted group).
3. full_name only on error — happy path is presence/count via present_attrs / any_attr_present?; names are built only when calling validation_error!.
4. Shared helpers — lookup lives once on MultipleParamsBase; AtLeastOneOfValidator no longer inlines its own key?.
5. keys_in_common arity — second arg kept as _known_keys = nil for out-of-tree subclasses.
6. CHANGELOG — entry moved to the bottom of #### Fixes (#2981); unrelated 3.x whitespace dropped.
7. Specs — your four regressions split across the existing validator spec files; the weak “assert 201 on a large array” example was replaced with a collision case that actually fails if the rule is dropped.

Benchmark impact (isolated processes, Ruby 4.0.5):

Synthetic (Array + 3× mutually_exclusive, happy path):

N	master	first PR	after review
830	0.092 s	0.020 s	0.016 s
2000	0.219 s	0.055 s	0.028 s

benchmark/large_model.rb + vrp_example.json (~5k timewindows with at_least_one_of): master 0.433 s → after review 0.257 s (~1.7×).

Deferring full_name accounts for the extra cut on the happy path, in line with your measurements.

Happy to tweak further if anything still looks off.