Skip to content

Use an internal Grape::Validations::SharedOptions value object#2721

Open
ericproulx wants to merge 1 commit into
masterfrom
refactor/shared-options-data
Open

Use an internal Grape::Validations::SharedOptions value object#2721
ericproulx wants to merge 1 commit into
masterfrom
refactor/shared-options-data

Conversation

@ericproulx
Copy link
Copy Markdown
Contributor

@ericproulx ericproulx commented May 15, 2026

Summary

Validators::Base#initialize consumed the shared opts via opts.values_at(:fail_fast, :allow_blank) and copied both into per-validator ivars (@fail_fast / @allow_blank), with ValuesValidator reaching into @allow_blank directly.

Introduce Grape::Validations::SharedOptionsData.define(:allow_blank, :fail_fast) with defaults mirroring the prior values_at behaviour — as a purely internal representation:

@opts = SharedOptions.new(**opts.slice(:allow_blank, :fail_fast))

and expose the fields via extend Forwardable; def_delegators :@opts, :allow_blank, :fail_fast. #fail_fast? delegates; ValuesValidator switches to the allow_blank reader.

No contract change

The public 5th argument of Validators::Base#initialize stays a plain Hash. The Hash → value-object conversion happens inside Base, so:

  • Custom validators that subclass and super the args through — unaffected.
  • Custom validators / user specs that hand-construct with a literal Hash 5th arg — unaffected (the original concern from the earlier revision of this PR is gone).
  • opts.slice(:allow_blank, :fail_fast) preserves the historical tolerance of silently ignoring any other keys (matching the old values_at).
  • ValidationsSpec#shared_opts keeps emitting the frozen { allow_blank:, fail_fast: } Hash — its (internal) surface is unchanged.

No UPGRADING.md entry needed — there is no behaviour or contract change, just an internal cleanup that removes the scattered ivars in favour of one delegated value object.

Test plan

  • bundle exec rspec — 2307 examples, 0 failures
  • RuboCop clean on lib/grape/validations/
  • CI green across Gemfile variants

🤖 Generated with Claude Code

@ericproulx ericproulx force-pushed the refactor/shared-options-data branch from 304266f to 28395d2 Compare May 15, 2026 08:59
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

Danger Report

No issues found.

View run

@github-actions
Copy link
Copy Markdown

Danger Report

No issues found.

View run

@ericproulx ericproulx force-pushed the refactor/shared-options-data branch from 28395d2 to d70bf23 Compare May 15, 2026 09:02
`Validators::Base#initialize` consumed the shared `opts` via
`opts.values_at(:fail_fast, :allow_blank)` and copied the two values
into per-validator ivars.

Introduce `Grape::Validations::SharedOptions`
(`Data.define(:allow_blank, :fail_fast)` with defaults mirroring the
prior `values_at` behaviour) as an *internal* representation: `Base`
builds it from the `opts` Hash —
`SharedOptions.new(**opts.slice(:allow_blank, :fail_fast))` — and
exposes the two fields via `Forwardable.def_delegators :@opts,
:allow_blank, :fail_fast`. `#fail_fast?` calls the delegated
`fail_fast`; `ValuesValidator` (the one subclass reading `@allow_blank`
directly) uses the `allow_blank` reader.

The public 5th-argument contract of `Validators::Base#initialize`
stays a plain Hash, so custom validators and any code that
hand-constructs a validator with a Hash are unaffected. `slice` keeps
the historical tolerance of ignoring unknown keys. `ValidationsSpec`
keeps emitting the `{ allow_blank:, fail_fast: }.freeze` Hash; nothing
about its public surface changes — no UPGRADING entry needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ericproulx ericproulx force-pushed the refactor/shared-options-data branch from d70bf23 to ae11328 Compare May 15, 2026 09:07
@ericproulx ericproulx changed the title Introduce Grape::Validations::SharedOptions value object Use an internal Grape::Validations::SharedOptions value object May 15, 2026
@ericproulx ericproulx requested a review from dblock May 15, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant