From 9bd050af0778578ecb96b506323498db0138fb06 Mon Sep 17 00:00:00 2001 From: Rob Hanlon Date: Tue, 3 Mar 2020 08:46:04 -0800 Subject: [PATCH] Consistently use Template#key when evaluating templates in backends (#254) * Consistently use Template#path when evaluating templates in backends * Move key check into template * Pass key and options instead of entire template object --- lib/dry/schema/messages/abstract.rb | 20 +++++- lib/dry/schema/messages/i18n.rb | 14 ++-- lib/dry/schema/messages/namespaced.rb | 8 +-- lib/dry/schema/messages/template.rb | 19 ++++-- lib/dry/schema/messages/yaml.rb | 36 +++++----- spec/integration/messages/i18n_spec.rb | 38 +++++++---- spec/unit/dry/schema/messages/i18n_spec.rb | 8 +-- .../unit/dry/schema/messages/template_spec.rb | 68 +++++++++++++++++++ spec/unit/dry/schema/messages/yaml_spec.rb | 3 +- 9 files changed, 162 insertions(+), 52 deletions(-) create mode 100644 spec/unit/dry/schema/messages/template_spec.rb diff --git a/lib/dry/schema/messages/abstract.rb b/lib/dry/schema/messages/abstract.rb index 271f6bb9..f83870f3 100644 --- a/lib/dry/schema/messages/abstract.rb +++ b/lib/dry/schema/messages/abstract.rb @@ -99,7 +99,6 @@ def call(predicate, options) Template.new( messages: self, key: path, - text: result[:text], options: opts ), result[:meta] @@ -108,6 +107,15 @@ def call(predicate, options) alias_method :[], :call + # Check if given key is defined + # + # @return [Boolean] + # + # @api public + def key?(_key, _options = EMPTY_HASH) + raise NotImplementedError + end + # Retrieve an array of looked up paths # # @param [Symbol] predicate @@ -160,6 +168,16 @@ def default_locale config.default_locale end + # @api private + def interpolatable_data(_key, _options, **_data) + raise NotImplementedError + end + + # @api private + def interpolate(_key, _options, **_data) + raise NotImplementedError + end + private # @api private diff --git a/lib/dry/schema/messages/i18n.rb b/lib/dry/schema/messages/i18n.rb index 679dcf13..d9a542aa 100644 --- a/lib/dry/schema/messages/i18n.rb +++ b/lib/dry/schema/messages/i18n.rb @@ -95,23 +95,23 @@ def prepare(paths = config.load_paths) end # @api private - def pruned_data(_template, **input) - input + def interpolatable_data(_key, _options, **data) + data end # @api private - def interpolate(template, **data) - text_key = "#{template.key}.text" + def interpolate(key, options, **data) + text_key = "#{key}.text" opts = { locale: default_locale, - **template.options, + **options, **data } - key = key?(text_key, opts) ? text_key : template.key + resolved_key = key?(text_key, opts) ? text_key : key - t.(key, **opts) + t.(resolved_key, **opts) end private diff --git a/lib/dry/schema/messages/namespaced.rb b/lib/dry/schema/messages/namespaced.rb index 46c63bcd..861485f5 100644 --- a/lib/dry/schema/messages/namespaced.rb +++ b/lib/dry/schema/messages/namespaced.rb @@ -71,13 +71,13 @@ def cache_key(predicate, options) end # @api private - def pruned_data(template, **input) - messages.pruned_data(template, **input) + def interpolatable_data(key, options, **data) + messages.interpolatable_data(key, options, **data) end # @api private - def interpolate(template, **data) - messages.interpolate(template, **data) + def interpolate(key, options, **data) + messages.interpolate(key, options, **data) end end end diff --git a/lib/dry/schema/messages/template.rb b/lib/dry/schema/messages/template.rb index 6de20b1b..4a0d8937 100644 --- a/lib/dry/schema/messages/template.rb +++ b/lib/dry/schema/messages/template.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'dry/initializer' +require 'dry/equalizer' require 'dry/schema/constants' @@ -10,22 +11,32 @@ module Messages # @api private class Template extend Dry::Initializer + include Dry::Equalizer(:messages, :key, :options) option :messages option :key - option :text option :options # @api private - def data(input = EMPTY_HASH) - messages.pruned_data(self, **options, **input) + def data(data = EMPTY_HASH) + ensure_message! + messages.interpolatable_data(key, options, **options, **data) end # @api private def call(data = EMPTY_HASH) - messages.interpolate(self, **data) + ensure_message! + messages.interpolate(key, options, **data) end alias_method :[], :call + + private + + def ensure_message! + return if messages.key?(key, options) + + raise KeyError, "No message found for template, template=#{inspect}" + end end end end diff --git a/lib/dry/schema/messages/yaml.rb b/lib/dry/schema/messages/yaml.rb index a3141cc5..f0bef173 100644 --- a/lib/dry/schema/messages/yaml.rb +++ b/lib/dry/schema/messages/yaml.rb @@ -133,11 +133,6 @@ def merge(overrides) end end - # @api private - def cache - @cache ||= self.class.cache[self] - end - # @api private def prepare @data = config.load_paths.map { |path| load_translations(path) }.reduce({}, :merge) @@ -145,34 +140,43 @@ def prepare end # @api private - def pruned_data(template, **input) - tokens, = evaluation_context(template) - input.select { |k,| tokens.include?(k) } + def interpolatable_data(key, options, **data) + tokens = evaluation_context(key, options).fetch(:tokens) + data.select { |k,| tokens.include?(k) } end # @api private - def interpolate(template, **data) - _, evaluator = evaluation_context(template) + def interpolate(key, options, **data) + evaluator = evaluation_context(key, options).fetch(:evaluator) data.empty? ? evaluator.() : evaluator.(**data) end private - def evaluation_context(template) - cache.fetch_or_store(template.text) do - tokens = template.text.scan(TOKEN_REGEXP).flatten(1).map(&:to_sym).to_set - text = template.text.gsub('%', '#') + # @api private + def evaluation_context(key, options) + cache.fetch_or_store(get(key, options).fetch(:text)) do |input| + tokens = input.scan(TOKEN_REGEXP).flatten(1).map(&:to_sym).to_set + text = input.gsub('%', '#') # rubocop:disable Security/Eval - evaluator = eval(<<~RUBY, EMPTY_CONTEXT, __FILE__, __LINE__) + evaluator = eval(<<~RUBY, EMPTY_CONTEXT, __FILE__, __LINE__ + 1) -> (#{tokens.map { |token| "#{token}:" }.join(', ')}) { "#{text}" } RUBY # rubocop:enable Security/Eval - [tokens, evaluator] + { + tokens: tokens, + evaluator: evaluator + } end end + # @api private + def cache + @cache ||= self.class.cache[self] + end + # @api private def load_translations(path) data = self.class.flat_hash(YAML.load_file(path)) diff --git a/spec/integration/messages/i18n_spec.rb b/spec/integration/messages/i18n_spec.rb index 2b933956..10f24301 100644 --- a/spec/integration/messages/i18n_spec.rb +++ b/spec/integration/messages/i18n_spec.rb @@ -33,11 +33,19 @@ def store_errors(**errors) { default_locale: :pl } end - it 'returns a message' do - template, meta = messages[:size?, path: :age, size: 10] - - expect(template.()).to eql('wielkość musi być równa 10') - expect(meta).to eql({}) + it 'returns a template' do + template, = messages[:size?, path: :age, size: 10] + + expect(template).to eq( + Dry::Schema::Messages::Template.new( + messages: messages, + key: 'dry_schema.errors.size?.arg.default', + options: { + locale: :pl, + size: 10 + } + ) + ) end end @@ -50,35 +58,35 @@ def store_errors(**errors) expect(messages[:not_here, path: :srsly]).to be(nil) end - it 'returns a message for a predicate' do + it 'returns a template for a predicate' do template, meta = messages[:filled?, path: :name] expect(template.()).to eql('nie może być pusty') expect(meta).to eql({}) end - it 'returns a message for a specific rule' do + it 'returns a template for a specific rule' do template, meta = messages[:filled?, path: :email] expect(template.()).to eql('Proszę podać adres email') expect(meta).to eql({}) end - it 'returns a message for a specific val type' do + it 'returns a template for a specific val type' do template, meta = messages[:size?, path: :pages, val_type: String] expect(template.(size: 2)).to eql('wielkość musi być równa 2') expect(meta).to eql({}) end - it 'returns a message for a specific rule and its default arg type' do + it 'returns a template for a specific rule and its default arg type' do template, meta = messages[:size?, path: :pages] expect(template.(size: 2)).to eql('wielkość musi być równa 2') expect(meta).to eql({}) end - it 'returns a message for a specific rule and its arg type' do + it 'returns a template for a specific rule and its arg type' do template, meta = messages[:size?, path: :pages, arg_type: Range] expect(template.(size_left: 1, size_right: 2)).to eql('wielkość musi być między 1 a 2') @@ -130,7 +138,7 @@ def store_errors(**errors) I18n.fallbacks = I18n::Locale::Fallbacks.new(:en) end - it 'returns a message for a predicate in the default_locale' do + it 'returns a template for a predicate in the default_locale' do template, meta = messages[:even?, path: :some_number] expect(I18n.locale).to eql(:pl) @@ -141,28 +149,28 @@ def store_errors(**errors) end context 'with a different locale' do - it 'returns a message for a predicate' do + it 'returns a template for a predicate' do template, meta = messages[:filled?, path: :name, locale: :en] expect(template.()).to eql('must be filled') expect(meta).to eql({}) end - it 'returns a message for a specific rule' do + it 'returns a template for a specific rule' do template, meta = messages[:filled?, path: :email, locale: :en] expect(template.()).to eql('Please provide your email') expect(meta).to eql({}) end - it 'returns a message for a specific rule and its default arg type' do + it 'returns a template for a specific rule and its default arg type' do template, meta = messages[:size?, path: :pages, locale: :en] expect(template.(size: 2)).to eql('size must be 2') expect(meta).to eql({}) end - it 'returns a message for a specific rule and its arg type' do + it 'returns a template for a specific rule and its arg type' do template, meta = messages[:size?, path: :pages, arg_type: Range, locale: :en] expect(template.(size_left: 1, size_right: 2)).to eql('size must be within 1 - 2') diff --git a/spec/unit/dry/schema/messages/i18n_spec.rb b/spec/unit/dry/schema/messages/i18n_spec.rb index 6f78a7ce..98bd2dcc 100644 --- a/spec/unit/dry/schema/messages/i18n_spec.rb +++ b/spec/unit/dry/schema/messages/i18n_spec.rb @@ -8,8 +8,8 @@ Dry::Schema::Messages::I18n.build end - describe '#lookup' do - it 'returns lookup result' do + describe '#[]' do + it 'returns template result' do template, meta = messages[:filled?, path: [:name]] expect(template.()).to eql('must be filled') @@ -29,8 +29,8 @@ Dry::Schema::Messages::I18n.build(top_namespace: 'my_app') end - describe '#lookup' do - it 'returns lookup result' do + describe '#[]' do + it 'returns template result' do template, meta = messages[:filled?, path: [:name]] expect(template.()).to eql('must be filled') diff --git a/spec/unit/dry/schema/messages/template_spec.rb b/spec/unit/dry/schema/messages/template_spec.rb new file mode 100644 index 00000000..896b28dc --- /dev/null +++ b/spec/unit/dry/schema/messages/template_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'dry/schema/messages/template' +require 'dry/schema/messages/abstract' + +RSpec.describe Dry::Schema::Messages::Template do + let(:messages) { instance_double('Dry::Schema::Messages::Abstract') } + let(:key) { 'key' } + let(:options) { { name: 'Alice', locale: :en } } + + subject(:template) do + Dry::Schema::Messages::Template.new( + messages: messages, + key: key, + options: options + ) + end + + describe '#data' do + it 'delegates to the message backend' do + input = { adjective: 'rad', ignored: 'param' } + data = { adjective: 'rad', name: 'Alice' } + + allow(messages).to receive(:key?) + .with(key, options) + .and_return(true) + + allow(messages).to receive(:interpolatable_data) + .with(key, options, **options, **input) + .and_return(data) + + expect(template.data(input)).to eq(data) + end + + it 'raises a KeyError when the key does not exist' do + allow(messages).to receive(:key?) + .with(key, options) + .and_return(false) + + expect { template.data }.to raise_error(KeyError) + end + end + + describe '#call' do + it 'delegates to the message backend' do + data = { adjective: 'rad', name: 'Alice' } + message = 'Alice is awesome and rad' + + allow(messages).to receive(:key?) + .with(key, options) + .and_return(true) + + allow(messages).to receive(:interpolate) + .with(key, options, **data) + .and_return(message) + + expect(template.(data)).to eq(message) + end + + it 'raises a KeyError when the key does not exist' do + allow(messages).to receive(:key?) + .with(key, options) + .and_return(false) + + expect { template.() }.to raise_error(KeyError) + end + end +end diff --git a/spec/unit/dry/schema/messages/yaml_spec.rb b/spec/unit/dry/schema/messages/yaml_spec.rb index b920595f..e62662ff 100644 --- a/spec/unit/dry/schema/messages/yaml_spec.rb +++ b/spec/unit/dry/schema/messages/yaml_spec.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true require 'dry/schema/messages/yaml' +require 'dry/schema/messages/template' RSpec.describe Dry::Schema::Messages::YAML do subject(:messages) do Dry::Schema::Messages::YAML.build end - describe '#lookup' do + describe '#[]' do context 'with default config' do it 'returns text and optional meta' do template, meta = messages[:filled?, path: [:name]]