From e0c075d819b3a8c6c677502327490e10aed97849 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 7 Mar 2026 16:54:37 +0100 Subject: [PATCH] Extract Grape::Util::Translation for shared I18n logic Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 1 + README.md | 25 +++++++ UPGRADING.md | 25 +++++++ lib/grape/exceptions/base.rb | 62 +++++---------- lib/grape/exceptions/validation.rb | 9 ++- lib/grape/exceptions/validation_errors.rb | 18 +++-- lib/grape/util/translation.rb | 42 +++++++++++ lib/grape/validations/validators/base.rb | 2 + .../validators/length_validator.rb | 8 +- .../validators/same_as_validator.rb | 2 +- spec/grape/exceptions/base_spec.rb | 48 +++++++----- spec/grape/exceptions/validation_spec.rb | 42 ++++++++++- spec/grape/util/translation_spec.rb | 28 +++++++ .../validations/validators/base_i18n_spec.rb | 75 +++++++++++++++++++ .../validators/coerce_validator_spec.rb | 43 ++++++----- spec/grape/validations/validators/zh-CN.yml | 10 --- 16 files changed, 332 insertions(+), 108 deletions(-) create mode 100644 lib/grape/util/translation.rb create mode 100644 spec/grape/util/translation_spec.rb create mode 100644 spec/grape/validations/validators/base_i18n_spec.rb delete mode 100644 spec/grape/validations/validators/zh-CN.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 8886a7009..f07b4454c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +* [#2662](https://github.com/ruby-grape/grape/pull/2662): Extract `Grape::Util::Translation` for shared I18n fallback logic - [@ericproulx](https://github.com/ericproulx). * [#2656](https://github.com/ruby-grape/grape/pull/2656): Remove useless instance_variable_defined? checks - [@ericproulx](https://github.com/ericproulx). * [#2619](https://github.com/ruby-grape/grape/pull/2619): Remove TOC from README.md and danger-toc check - [@alexanderadam](https://github.com/alexanderadam). * [#2663](https://github.com/ruby-grape/grape/pull/2663): Refactor `ParamsScope` and `Parameters` DSL to use named kwargs - [@ericproulx](https://github.com/ericproulx). diff --git a/README.md b/README.md index f9a0edf06..e06af7077 100644 --- a/README.md +++ b/README.md @@ -1896,6 +1896,31 @@ Grape supports I18n for parameter-related error messages, but will fallback to E In case your app enforces available locales only and :en is not included in your available locales, Grape cannot fall back to English and will return the translation key for the error message. To avoid this behaviour, either provide a translation for your default locale or add :en to your available locales. +Custom validators that inherit from `Grape::Validations::Validators::Base` have access to a `translate` helper (see `Grape::Util::Translation`) and should use it instead of calling `I18n` directly. It applies the same `:en` fallback as built-in validators, defaults `scope` to `'grape.errors.messages'`, and handles interpolation without needing `format`: + +```ruby +# Good — scope defaults to 'grape.errors.messages', interpolation forwarded automatically +translate(:special, min: 2, max: 10) + +# Bad — format is unnecessary and risks conflicting with I18n reserved keys +format I18n.t(:special, scope: 'grape.errors.messages'), min: 2, max: 10 +``` + +Example custom validator: + +```ruby +class SpecialValidator < Grape::Validations::Validators::Base + def validate_param!(attr_name, params) + return if valid?(params[attr_name]) + + raise Grape::Exceptions::Validation.new( + params: [@scope.full_name(attr_name)], + message: translate(:special, min: 2, max: 10) + ) + end +end +``` + ### Custom Validation messages Grape supports custom validation messages for parameter-related and coerce-related error messages. diff --git a/UPGRADING.md b/UPGRADING.md index f365b49c3..50d6584ba 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -21,6 +21,31 @@ with({ type: String }) { ... } See [#2663](https://github.com/ruby-grape/grape/pull/2663) for more information. +#### Custom validators: use `translate` instead of `I18n` directly + +`Grape::Util::Translation` is now included in `Grape::Validations::Validators::Base`. Custom validators that previously called `I18n.t` or `I18n.translate` directly should switch to the `translate`, which provides the same `:en` fallback logic used by all built-in validators. + +Key points: +- `scope` defaults to `'grape.errors.messages'` — no need to specify it for standard error message keys. +- Interpolation variables are passed directly to I18n. +- `format` is no longer needed — `translate` returns the fully interpolated string. + +```ruby +# Before +raise Grape::Exceptions::Validation.new( + params: [@scope.full_name(attr_name)], + message: format(I18n.t(:my_key, scope: 'grape.errors.messages'), min: 2, max: 10) +) + +# After +raise Grape::Exceptions::Validation.new( + params: [@scope.full_name(attr_name)], + message: translate(:my_key, min: 2, max: 10) +) +``` + +See [#2662](https://github.com/ruby-grape/grape/pull/2662) for more information. + ### Upgrading to >= 3.1 #### Explicit kwargs for `namespace` and `route_param` diff --git a/lib/grape/exceptions/base.rb b/lib/grape/exceptions/base.rb index be24e43ed..afe9fd4ed 100644 --- a/lib/grape/exceptions/base.rb +++ b/lib/grape/exceptions/base.rb @@ -3,9 +3,9 @@ module Grape module Exceptions class Base < StandardError - BASE_MESSAGES_KEY = 'grape.errors.messages' - BASE_ATTRIBUTES_KEY = 'grape.errors.attributes' - FALLBACK_LOCALE = :en + include Grape::Util::Translation + + MESSAGE_STEPS = %w[problem summary resolution].to_h { |s| [s, s.capitalize] }.freeze attr_reader :status, :headers @@ -20,55 +20,29 @@ def [](index) __send__ index end - protected + private - # TODO: translate attribute first - # if BASE_ATTRIBUTES_KEY.key respond to a string message, then short_message is returned - # if BASE_ATTRIBUTES_KEY.key respond to a Hash, means it may have problem , summary and resolution - def compose_message(key, **attributes) - short_message = translate_message(key, attributes) + def compose_message(key, **) + short_message = translate_message(key, **) return short_message unless short_message.is_a?(Hash) - each_steps(key, attributes).with_object(+'') do |detail_array, message| - message << "\n#{detail_array[0]}:\n #{detail_array[1]}" unless detail_array[1].blank? - end - end - - def each_steps(key, attributes) - return enum_for(:each_steps, key, attributes) unless block_given? - - yield 'Problem', translate_message(:"#{key}.problem", attributes) - yield 'Summary', translate_message(:"#{key}.summary", attributes) - yield 'Resolution', translate_message(:"#{key}.resolution", attributes) - end - - def translate_attributes(keys, options = {}) - keys.map do |key| - translate("#{BASE_ATTRIBUTES_KEY}.#{key}", options.merge(default: key.to_s)) - end.join(', ') + MESSAGE_STEPS.filter_map do |step, label| + detail = translate_message(:"#{key}.#{step}", **) + "\n#{label}:\n #{detail}" if detail.present? + end.join end - def translate_message(key, options = {}) - case key + def translate_message(translation_key, **) + case translation_key when Symbol - translate("#{BASE_MESSAGES_KEY}.#{key}", options.merge(default: '')) + translate(translation_key, **) + when Hash + translation_key => { key:, **opts } + translate(key, **opts) when Proc - key.call - else - key - end - end - - def translate(key, options) - message = ::I18n.translate(key, **options) - message.presence || fallback_message(key, options) - end - - def fallback_message(key, options) - if ::I18n.enforce_available_locales && !::I18n.available_locales.include?(FALLBACK_LOCALE) - key + translation_key.call else - ::I18n.translate(key, locale: FALLBACK_LOCALE, **options) + translation_key end end end diff --git a/lib/grape/exceptions/validation.rb b/lib/grape/exceptions/validation.rb index 0a9e9c5dd..8599c94e7 100644 --- a/lib/grape/exceptions/validation.rb +++ b/lib/grape/exceptions/validation.rb @@ -3,12 +3,15 @@ module Grape module Exceptions class Validation < Base - attr_accessor :params, :message_key + attr_reader :params, :message_key def initialize(params:, message: nil, status: nil, headers: nil) - @params = params + @params = Array(params) if message - @message_key = message if message.is_a?(Symbol) + @message_key = case message + when Symbol then message + when Hash then message[:key] + end message = translate_message(message) end diff --git a/lib/grape/exceptions/validation_errors.rb b/lib/grape/exceptions/validation_errors.rb index 8859d579b..0fbe70c83 100644 --- a/lib/grape/exceptions/validation_errors.rb +++ b/lib/grape/exceptions/validation_errors.rb @@ -3,9 +3,6 @@ module Grape module Exceptions class ValidationErrors < Base - ERRORS_FORMAT_KEY = 'grape.errors.format' - DEFAULT_ERRORS_FORMAT = '%s %s' - include Enumerable attr_reader :errors @@ -38,9 +35,10 @@ def to_json(*_opts) def full_messages messages = map do |attributes, error| - I18n.t( - ERRORS_FORMAT_KEY, - default: DEFAULT_ERRORS_FORMAT, + translate( + :format, + scope: 'grape.errors', + default: '%s %s', attributes: translate_attributes(attributes), message: error.message ) @@ -48,6 +46,14 @@ def full_messages messages.uniq! messages end + + private + + def translate_attributes(keys) + keys.map do |key| + translate(key, scope: 'grape.errors.attributes', default: key.to_s) + end.join(', ') + end end end end diff --git a/lib/grape/util/translation.rb b/lib/grape/util/translation.rb new file mode 100644 index 000000000..be94782e2 --- /dev/null +++ b/lib/grape/util/translation.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Grape + module Util + module Translation + FALLBACK_LOCALE = :en + private_constant :FALLBACK_LOCALE + # Sentinel returned by I18n when a key is missing (passed as the default: + # value). Using a named class rather than plain Object.new makes it + # identifiable in debug output and immune to backends that call .to_s on + # the default before returning it. + MISSING = Class.new { def inspect = 'Grape::Util::Translation::MISSING' }.new.freeze + private_constant :MISSING + + private + + # Extra keyword args (**) are forwarded verbatim to I18n as interpolation + # variables (e.g. +min:+, +max:+ from LengthValidator's Hash message). + # Callers must not pass unintended keyword arguments — any extra keyword + # will silently become an I18n interpolation variable. + def translate(key, default: MISSING, scope: 'grape.errors.messages', locale: nil, **) + i18n_opts = { default:, scope:, ** } + i18n_opts[:locale] = locale if locale + message = ::I18n.translate(key, **i18n_opts) + return message unless message.equal?(MISSING) + + effective_default = default.equal?(MISSING) ? [*Array(scope), key].join('.') : default + return effective_default if fallback_locale?(locale) || fallback_locale_unavailable? + + ::I18n.translate(key, default: effective_default, scope:, locale: FALLBACK_LOCALE, **) + end + + def fallback_locale?(locale) + (locale || ::I18n.locale) == FALLBACK_LOCALE + end + + def fallback_locale_unavailable? + ::I18n.enforce_available_locales && !::I18n.available_locales.include?(FALLBACK_LOCALE) + end + end + end +end diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index f4788afb2..4fd64acef 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -4,6 +4,8 @@ module Grape module Validations module Validators class Base + include Grape::Util::Translation + attr_reader :attrs # Creates a new Validator from options specified diff --git a/lib/grape/validations/validators/length_validator.rb b/lib/grape/validations/validators/length_validator.rb index c84b4c096..ad85a2a7b 100644 --- a/lib/grape/validations/validators/length_validator.rb +++ b/lib/grape/validations/validators/length_validator.rb @@ -34,13 +34,13 @@ def build_message if options_key?(:message) @option[:message] elsif @min && @max - format I18n.t(:length, scope: 'grape.errors.messages'), min: @min, max: @max + translate(:length, min: @min, max: @max) elsif @min - format I18n.t(:length_min, scope: 'grape.errors.messages'), min: @min + translate(:length_min, min: @min) elsif @max - format I18n.t(:length_max, scope: 'grape.errors.messages'), max: @max + translate(:length_max, max: @max) else - format I18n.t(:length_is, scope: 'grape.errors.messages'), is: @is + translate(:length_is, is: @is) end end end diff --git a/lib/grape/validations/validators/same_as_validator.rb b/lib/grape/validations/validators/same_as_validator.rb index 5a65afa60..b91c03be3 100644 --- a/lib/grape/validations/validators/same_as_validator.rb +++ b/lib/grape/validations/validators/same_as_validator.rb @@ -20,7 +20,7 @@ def build_message if options_key?(:message) @option[:message] else - format I18n.t(:same_as, scope: 'grape.errors.messages'), parameter: @option + translate(:same_as, parameter: @option) end end end diff --git a/spec/grape/exceptions/base_spec.rb b/spec/grape/exceptions/base_spec.rb index 3a6a7849d..16b07ac43 100644 --- a/spec/grape/exceptions/base_spec.rb +++ b/spec/grape/exceptions/base_spec.rb @@ -23,21 +23,15 @@ let(:key) { :invalid_formatter } let(:attributes) { { klass: String, to_format: 'xml' } } - after do - I18n.enforce_available_locales = true - I18n.available_locales = %i[en] - I18n.locale = :en - I18n.default_locale = :en - I18n.reload! - end + after { I18n.reload! } context 'when I18n enforces available locales' do - before { I18n.enforce_available_locales = true } - context 'when the fallback locale is available' do - before do + around do |example| I18n.available_locales = %i[de en] - I18n.default_locale = :de + I18n.with_locale(:de) { example.run } + ensure + I18n.available_locales = %i[en] end it 'returns the translated message' do @@ -46,23 +40,36 @@ end context 'when the fallback locale is not available' do - before do + around do |example| I18n.available_locales = %i[de jp] - I18n.locale = :de - I18n.default_locale = :de + I18n.with_locale(:de) do + example.run + ensure + I18n.available_locales = %i[en] + end end - it 'returns the translation string' do + it 'returns the scoped translation key as a string' do expect(subject).to eq("grape.errors.messages.#{key}") end end end context 'when I18n does not enforce available locales' do - before { I18n.enforce_available_locales = false } + around do |example| + I18n.enforce_available_locales = false + example.run + ensure + I18n.enforce_available_locales = true + end context 'when the fallback locale is available' do - before { I18n.available_locales = %i[de en] } + around do |example| + I18n.available_locales = %i[de en] + I18n.with_locale(:de) { example.run } + ensure + I18n.available_locales = %i[en] + end it 'returns the translated message' do expect(subject).to eq('cannot convert String to xml') @@ -70,7 +77,12 @@ end context 'when the fallback locale is not available' do - before { I18n.available_locales = %i[de jp] } + around do |example| + I18n.available_locales = %i[de jp] + I18n.with_locale(:de) { example.run } + ensure + I18n.available_locales = %i[en] + end it 'returns the translated message' do expect(subject).to eq('cannot convert String to xml') diff --git a/spec/grape/exceptions/validation_spec.rb b/spec/grape/exceptions/validation_spec.rb index 767941122..c36f3244f 100644 --- a/spec/grape/exceptions/validation_spec.rb +++ b/spec/grape/exceptions/validation_spec.rb @@ -5,15 +5,51 @@ expect { described_class.new(message: 'presence') }.to raise_error(ArgumentError, /missing keyword:.+?params/) end - context 'when message is a symbol' do + context 'when message is a Symbol' do + subject(:error) { described_class.new(params: ['id'], message: :presence) } + it 'stores message_key' do - expect(described_class.new(params: ['id'], message: :presence).message_key).to eq(:presence) + expect(error.message_key).to eq(:presence) + end + + it 'translates the message' do + expect(error.message).to eq('is missing') + end + end + + context 'when message is a Hash' do + subject(:error) { described_class.new(params: ['id'], message: { key: :between, min: 2, max: 10 }) } + + before do + I18n.backend.store_translations(:en, grape: { errors: { messages: { between: 'must be between %s and %s' } } }) + end + + after { I18n.reload! } + + it 'stores the :key entry as message_key' do + expect(error.message_key).to eq(:between) + end + + it 'translates the message with interpolation params' do + expect(error.message).to eq('must be between 2 and 10') + end + end + + context 'when message is a Proc' do + it 'calls the proc to produce the message' do + expect(described_class.new(params: ['id'], message: -> { 'computed' }).message).to eq('computed') end end context 'when message is a String' do + subject(:error) { described_class.new(params: ['id'], message: 'raw message') } + it 'does not store the message_key' do - expect(described_class.new(params: ['id'], message: 'presence').message_key).to be_nil + expect(error.message_key).to be_nil + end + + it 'returns the string as-is' do + expect(error.message).to eq('raw message') end end end diff --git a/spec/grape/util/translation_spec.rb b/spec/grape/util/translation_spec.rb new file mode 100644 index 000000000..bded528e0 --- /dev/null +++ b/spec/grape/util/translation_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +describe Grape::Util::Translation do + subject(:translator) do + Class.new do + include Grape::Util::Translation + + def translate_message(key, **opts) + translate(key, **opts) + end + end.new + end + + describe '#translate_message' do + context 'when the translation value uses a reserved I18n interpolation key' do + around do |example| + I18n.backend.store_translations(:en, grape: { errors: { messages: { reserved_key_test: 'value %{scope}' } } }) # rubocop:disable Style/FormatStringToken + example.run + ensure + I18n.reload! + end + + it 'raises I18n::ReservedInterpolationKey' do + expect { translator.translate_message(:reserved_key_test) }.to raise_error(I18n::ReservedInterpolationKey) + end + end + end +end diff --git a/spec/grape/validations/validators/base_i18n_spec.rb b/spec/grape/validations/validators/base_i18n_spec.rb new file mode 100644 index 000000000..b99fdbef7 --- /dev/null +++ b/spec/grape/validations/validators/base_i18n_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +describe Grape::Validations::Validators::Base do + describe 'i18n' do + subject { Class.new(Grape::API) } + + let(:app) { subject } + + let(:custom_i18n_validator) do + Class.new(Grape::Validations::Validators::Base) do + def validate_param!(attr_name, params) + return if params.respond_to?(:key?) && params[attr_name] == 'accept' + + raise Grape::Exceptions::Validation.new( + params: @scope.full_name(attr_name), + message: message(:custom_i18n_test) + ) + end + end + end + + around do |example| + I18n.available_locales = %i[en zh-CN] + I18n.backend.store_translations(:en, grape: { errors: { messages: { custom_i18n_test: 'custom validation failed (en)' } } }) + I18n.backend.store_translations(:'zh-CN', grape: { errors: { format: '%s %s', + messages: { custom_i18n_test: '自定义校验失败 (zh-CN)' } } }) + example.run + ensure + I18n.available_locales = %i[en] + I18n.reload! + end + + before do + stub_const('CustomI18nValidator', custom_i18n_validator) + Grape::Validations.register(CustomI18nValidator) + end + + after { Grape::Validations.deregister('custom_i18n') } + + it 'uses the request-time locale regardless of the locale active at definition time' do + # Define the API while zh-CN is the active locale + I18n.with_locale(:'zh-CN') do + subject.params do + requires :token, custom_i18n: true + end + subject.post do + end + end + # Switch to English before making the request + I18n.with_locale(:en) do + post '/', token: 'reject' + end + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('token custom validation failed (en)') + end + + it 'uses zh-CN message when request is made with zh-CN locale' do + I18n.with_locale(:en) do + subject.params do + requires :token, custom_i18n: true + end + subject.post do + end + end + + I18n.with_locale(:'zh-CN') do + post '/', token: 'reject' + end + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('token 自定义校验失败 (zh-CN)') + end + end +end diff --git a/spec/grape/validations/validators/coerce_validator_spec.rb b/spec/grape/validations/validators/coerce_validator_spec.rb index 126154a79..a9fae7280 100644 --- a/spec/grape/validations/validators/coerce_validator_spec.rb +++ b/spec/grape/validations/validators/coerce_validator_spec.rb @@ -19,17 +19,20 @@ def self.parsed?(value) end context 'i18n' do + before do + I18n.available_locales = %i[en zh-CN] + zh_cn = { grape: { errors: { format: '%s%s', + attributes: { age: '年龄' }, + messages: { coerce: '格式不正确' } } } } + I18n.backend.store_translations(:'zh-CN', zh_cn) + end + after do I18n.available_locales = %i[en] - I18n.locale = :en - I18n.default_locale = :en + I18n.reload! end it 'i18n error on malformed input' do - I18n.available_locales = %i[en zh-CN] - I18n.load_path << File.expand_path('zh-CN.yml', __dir__) - I18n.reload! - I18n.locale = :'zh-CN' subject.params do requires :age, type: Integer end @@ -37,24 +40,26 @@ def self.parsed?(value) 'int works' end - get '/single', age: '43a' + I18n.with_locale(:'zh-CN') { get '/single', age: '43a' } expect(last_response).to be_bad_request expect(last_response.body).to eq('年龄格式不正确') end - it 'gives an english fallback error when default locale message is blank' do - I18n.available_locales = %i[en pt-BR] - I18n.locale = :'pt-BR' - subject.params do - requires :age, type: Integer - end - subject.get '/single' do - 'int works' - end + context 'when the locale has no translation for the message' do + before { I18n.available_locales = %i[en pt-BR] } - get '/single', age: '43a' - expect(last_response).to be_bad_request - expect(last_response.body).to eq('age is invalid') + it 'gives an english fallback error' do + subject.params do + requires :age, type: Integer + end + subject.get '/single' do + 'int works' + end + + I18n.with_locale(:'pt-BR') { get '/single', age: '43a' } + expect(last_response).to be_bad_request + expect(last_response.body).to eq('age is invalid') + end end end diff --git a/spec/grape/validations/validators/zh-CN.yml b/spec/grape/validations/validators/zh-CN.yml deleted file mode 100644 index 39f6bbd7f..000000000 --- a/spec/grape/validations/validators/zh-CN.yml +++ /dev/null @@ -1,10 +0,0 @@ -zh-CN: - grape: - errors: - format: ! '%{attributes}%{message}' - attributes: - age: 年龄 - messages: - coerce: '格式不正确' - presence: '请填写' - regexp: '格式不正确'