From 248bba5ffea23bd942b88f1cf613e292db816315 Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Sat, 8 Aug 2020 15:18:56 -0300 Subject: [PATCH 1/8] Add spec to cover the case when an invalid Blueprinter is passed as the Blueprinter for a given association. Add spec to cover the case when an object that does not have any ancestors is passed as the blueprint. --- spec/integrations/base_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/integrations/base_spec.rb b/spec/integrations/base_spec.rb index 110daa03..b71e8a81 100644 --- a/spec/integrations/base_spec.rb +++ b/spec/integrations/base_spec.rb @@ -87,6 +87,26 @@ end it('returns json with association') { should eq(result) } end + context 'Given associated blueprint does not inherit from Blueprinter::Base' do + let(:blueprint) do + vehicle_invalid_blueprint = Class.new + Class.new(Blueprinter::Base) do + identifier :id + association :vehicles, blueprint: vehicle_invalid_blueprint + end + end + it { expect { subject }.to raise_error(Blueprinter::BlueprinterError) } + end + context 'Given associated blueprint does not have any ancestors' do + let(:blueprint) do + vehicle_invalid_blueprint = {} + Class.new(Blueprinter::Base) do + identifier :id + association :vehicles, blueprint: vehicle_invalid_blueprint + end + end + it { expect { subject }.to raise_error(Blueprinter::BlueprinterError) } + end context "Given association with dynamic blueprint" do class UserBlueprint < Blueprinter::Base fields :id From a9ce9c5ac63fb125b550c633db6cb75d43f1f640 Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Sat, 8 Aug 2020 15:20:34 -0300 Subject: [PATCH 2/8] Add logic to validate a given Blueprint class provided as the blueprint for an association has ancestors and inherits from Blueprinter::Base. Raise errors in case a given Blueprint is not valid --- .../extractors/association_extractor.rb | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/blueprinter/extractors/association_extractor.rb b/lib/blueprinter/extractors/association_extractor.rb index d6cd7f6a..3ebdc9d0 100644 --- a/lib/blueprinter/extractors/association_extractor.rb +++ b/lib/blueprinter/extractors/association_extractor.rb @@ -14,7 +14,7 @@ def extract(association_name, object, local_options, options={}) value = @extractor.extract(association_name, object, local_options, options_without_default) return default_value(options) if use_default_value?(value, options[:default_if]) view = options[:view] || :default - blueprint = association_blueprint(options[:blueprint], value) + blueprint = association_blueprint(options[:blueprint], value, association_name) blueprint.prepare(value, view_name: view, local_options: local_options) end @@ -24,8 +24,33 @@ def default_value(association_options) association_options.key?(:default) ? association_options.fetch(:default) : Blueprinter.configuration.association_default end - def association_blueprint(blueprint, value) - blueprint.is_a?(Proc) ? blueprint.call(value) : blueprint + def association_blueprint(blueprint, value, association_name) + if blueprint.is_a?(Proc) + blueprint.call(value) + else + validate_blueprint(blueprint, association_name) + blueprint + end + end + + def validate_blueprint(blueprint, association_name) + # If the class passed as a blueprint does not respond to ancestors + # it means it, at the very least, does not have Blueprinter::Base as + # one of its ancestor classes (e.g: Hash) and thus an error should + # be raised. + unless blueprint.respond_to?(:ancestors) + raise BlueprinterError, "Blueprint provided for #{association_name} "\ + 'association is not valid.' + end + + # Guard clause in case Blueprinter::Base is present in the ancestor list + # for the blueprint class provided. + return if blueprint.ancestors.include? Blueprinter::Base + + # Raise error describing what's wrong. + raise BlueprinterError, "Class #{blueprint.name} does not inherit from "\ + 'Blueprinter::Base and is not a valid Blueprinter '\ + "for #{association_name} association." end end -end +end \ No newline at end of file From 22ed826db22786be181d0402d97c5db2522e421d Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Sat, 8 Aug 2020 15:42:29 -0300 Subject: [PATCH 3/8] Split validation logic into two methods. --- lib/blueprinter/extractors/association_extractor.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/blueprinter/extractors/association_extractor.rb b/lib/blueprinter/extractors/association_extractor.rb index 3ebdc9d0..e6186206 100644 --- a/lib/blueprinter/extractors/association_extractor.rb +++ b/lib/blueprinter/extractors/association_extractor.rb @@ -7,8 +7,8 @@ def initialize @extractor = Blueprinter.configuration.extractor_default.new end - def extract(association_name, object, local_options, options={}) - options_without_default = options.reject { |k,_| k == :default || k == :default_if } + def extract(association_name, object, local_options, options = {}) + options_without_default = options.reject { |k, _| k == :default || k == :default_if } # Merge in assocation options hash local_options = local_options.merge(options[:options]) if options[:options].is_a?(Hash) value = @extractor.extract(association_name, object, local_options, options_without_default) @@ -28,12 +28,13 @@ def association_blueprint(blueprint, value, association_name) if blueprint.is_a?(Proc) blueprint.call(value) else - validate_blueprint(blueprint, association_name) + validate_blueprint_has_ancestors(blueprint, association_name) + validate_blueprinter_has_correct_ancestor(blueprint, association_name) blueprint end end - def validate_blueprint(blueprint, association_name) + def validate_blueprint_has_ancestors(blueprint, association_name) # If the class passed as a blueprint does not respond to ancestors # it means it, at the very least, does not have Blueprinter::Base as # one of its ancestor classes (e.g: Hash) and thus an error should @@ -42,7 +43,9 @@ def validate_blueprint(blueprint, association_name) raise BlueprinterError, "Blueprint provided for #{association_name} "\ 'association is not valid.' end + end + def validate_blueprinter_has_correct_ancestor(blueprint, association_name) # Guard clause in case Blueprinter::Base is present in the ancestor list # for the blueprint class provided. return if blueprint.ancestors.include? Blueprinter::Base From 088ba007129f6bf3d272b06a871cf1caf529c593 Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Tue, 11 Aug 2020 14:40:44 -0300 Subject: [PATCH 4/8] Add line to the end of the file. --- lib/blueprinter/extractors/association_extractor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/blueprinter/extractors/association_extractor.rb b/lib/blueprinter/extractors/association_extractor.rb index e6186206..a9b1f07d 100644 --- a/lib/blueprinter/extractors/association_extractor.rb +++ b/lib/blueprinter/extractors/association_extractor.rb @@ -56,4 +56,4 @@ def validate_blueprinter_has_correct_ancestor(blueprint, association_name) "for #{association_name} association." end end -end \ No newline at end of file +end From 0398d0be5292744a8b91237fb8ffd5fb5a0ad6c9 Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Tue, 11 Aug 2020 14:44:39 -0300 Subject: [PATCH 5/8] Move validations into base_helpers and leave association_extractor as it was. --- .../extractors/association_extractor.rb | 38 +++---------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/lib/blueprinter/extractors/association_extractor.rb b/lib/blueprinter/extractors/association_extractor.rb index a9b1f07d..d6cd7f6a 100644 --- a/lib/blueprinter/extractors/association_extractor.rb +++ b/lib/blueprinter/extractors/association_extractor.rb @@ -7,14 +7,14 @@ def initialize @extractor = Blueprinter.configuration.extractor_default.new end - def extract(association_name, object, local_options, options = {}) - options_without_default = options.reject { |k, _| k == :default || k == :default_if } + def extract(association_name, object, local_options, options={}) + options_without_default = options.reject { |k,_| k == :default || k == :default_if } # Merge in assocation options hash local_options = local_options.merge(options[:options]) if options[:options].is_a?(Hash) value = @extractor.extract(association_name, object, local_options, options_without_default) return default_value(options) if use_default_value?(value, options[:default_if]) view = options[:view] || :default - blueprint = association_blueprint(options[:blueprint], value, association_name) + blueprint = association_blueprint(options[:blueprint], value) blueprint.prepare(value, view_name: view, local_options: local_options) end @@ -24,36 +24,8 @@ def default_value(association_options) association_options.key?(:default) ? association_options.fetch(:default) : Blueprinter.configuration.association_default end - def association_blueprint(blueprint, value, association_name) - if blueprint.is_a?(Proc) - blueprint.call(value) - else - validate_blueprint_has_ancestors(blueprint, association_name) - validate_blueprinter_has_correct_ancestor(blueprint, association_name) - blueprint - end - end - - def validate_blueprint_has_ancestors(blueprint, association_name) - # If the class passed as a blueprint does not respond to ancestors - # it means it, at the very least, does not have Blueprinter::Base as - # one of its ancestor classes (e.g: Hash) and thus an error should - # be raised. - unless blueprint.respond_to?(:ancestors) - raise BlueprinterError, "Blueprint provided for #{association_name} "\ - 'association is not valid.' - end - end - - def validate_blueprinter_has_correct_ancestor(blueprint, association_name) - # Guard clause in case Blueprinter::Base is present in the ancestor list - # for the blueprint class provided. - return if blueprint.ancestors.include? Blueprinter::Base - - # Raise error describing what's wrong. - raise BlueprinterError, "Class #{blueprint.name} does not inherit from "\ - 'Blueprinter::Base and is not a valid Blueprinter '\ - "for #{association_name} association." + def association_blueprint(blueprint, value) + blueprint.is_a?(Proc) ? blueprint.call(value) : blueprint end end end From 03a44f19035b62890acc1d8e68f37644ef2e9cd9 Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Tue, 11 Aug 2020 14:58:18 -0300 Subject: [PATCH 6/8] Move validations into base_helper. Add method to check if the blueprinter is a Proc. --- lib/blueprinter/base.rb | 11 +++--- lib/blueprinter/helpers/base_helpers.rb | 45 +++++++++++++++++++++---- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/lib/blueprinter/base.rb b/lib/blueprinter/base.rb index 5d24281f..0498e567 100644 --- a/lib/blueprinter/base.rb +++ b/lib/blueprinter/base.rb @@ -156,7 +156,11 @@ def self.field(method, options = {}, &block) # # @return [Field] A Field object def self.association(method, options = {}, &block) - raise BlueprinterError, 'blueprint required' unless options[:blueprint] + validate_presence_of_blueprint options[:blueprint] + unless dynamic_blueprint?(options[:blueprint]) + validate_blueprint_has_ancestors(options[:blueprint], method) + validate_blueprint_has_blueprinter_base_ancestor(options[:blueprint], method) + end field( method, @@ -214,7 +218,7 @@ def self.render(object, options = {}) # # => [{id:1, title: Hello},{id:2, title: My Day}] # # @return [Hash] - def self.render_as_hash(object, options= {}) + def self.render_as_hash(object, options = {}) prepare_for_render(object, options) end @@ -239,7 +243,7 @@ def self.render_as_hash(object, options= {}) # # => [{"id" => "1", "title" => "Hello"},{"id" => "2", "title" => "My Day"}] # # @return [Hash] - def self.render_as_json(object, options= {}) + def self.render_as_json(object, options = {}) prepare_for_render(object, options).as_json end @@ -279,7 +283,6 @@ def self.fields(*field_names) end - # Specify one transformer to be included for serialization. # Takes a class which extends Blueprinter::Transformer # diff --git a/lib/blueprinter/helpers/base_helpers.rb b/lib/blueprinter/helpers/base_helpers.rb index 393e61ab..76226f88 100644 --- a/lib/blueprinter/helpers/base_helpers.rb +++ b/lib/blueprinter/helpers/base_helpers.rb @@ -8,6 +8,7 @@ module SingletonMethods include TypeHelpers private + def prepare_for_render(object, options) view_name = options.delete(:view) || :default root = options.delete(:root) @@ -20,19 +21,19 @@ def prepare_data(object, view_name, local_options) if array_like?(object) object.map do |obj| object_to_hash(obj, - view_name: view_name, - local_options: local_options) + view_name: view_name, + local_options: local_options) end else object_to_hash(object, - view_name: view_name, - local_options: local_options) + view_name: view_name, + local_options: local_options) end end def prepend_root_and_meta(data, root, meta) return data unless root - ret = { root => data } + ret = {root => data} meta ? ret.merge!(meta: meta) : ret end @@ -43,10 +44,10 @@ def inherited(subclass) def object_to_hash(object, view_name:, local_options:) result_hash = view_collection.fields_for(view_name).each_with_object({}) do |field, hash| next if field.skip?(field.name, object, local_options) - hash[field.name] = field.extract(object, local_options) + hash[field.name] = field.extract(object, local_options) end view_collection.transformers(view_name).each do |transformer| - transformer.transform(result_hash,object,local_options) + transformer.transform(result_hash, object, local_options) end result_hash end @@ -62,6 +63,36 @@ def validate_root_and_meta(root, meta) end end + def dynamic_blueprint?(blueprint) + blueprint.is_a?(Proc) + end + + def validate_presence_of_blueprint(blueprint) + raise BlueprinterError, 'Blueprint required' unless blueprint + end + + def validate_blueprint_has_ancestors(blueprint, association_name) + # If the class passed as a blueprint does not respond to ancestors + # it means it, at the very least, does not have Blueprinter::Base as + # one of its ancestor classes (e.g: Hash) and thus an error should + # be raised. + unless blueprint.respond_to?(:ancestors) + raise BlueprinterError, "Blueprint provided for #{association_name} "\ + 'association is not valid.' + end + end + + def validate_blueprint_has_blueprinter_base_ancestor(blueprint, association_name) + # Guard clause in case Blueprinter::Base is present in the ancestor list + # for the blueprint class provided. + return if blueprint.ancestors.include? Blueprinter::Base + + # Raise error describing what's wrong. + raise BlueprinterError, "Class #{blueprint.name} does not inherit from "\ + 'Blueprinter::Base and is not a valid Blueprinter '\ + "for #{association_name} association." + end + def jsonify(blob) Blueprinter.configuration.jsonify(blob) end From 5f48b5c1d1eae99c027370eb44de19d983f29212 Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Tue, 11 Aug 2020 15:02:35 -0300 Subject: [PATCH 7/8] Add base validation method to invoke other validations in a single place. --- lib/blueprinter/base.rb | 6 +----- lib/blueprinter/helpers/base_helpers.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/blueprinter/base.rb b/lib/blueprinter/base.rb index 0498e567..9cee675d 100644 --- a/lib/blueprinter/base.rb +++ b/lib/blueprinter/base.rb @@ -156,11 +156,7 @@ def self.field(method, options = {}, &block) # # @return [Field] A Field object def self.association(method, options = {}, &block) - validate_presence_of_blueprint options[:blueprint] - unless dynamic_blueprint?(options[:blueprint]) - validate_blueprint_has_ancestors(options[:blueprint], method) - validate_blueprint_has_blueprinter_base_ancestor(options[:blueprint], method) - end + validate_blueprint(options[:blueprint], method) field( method, diff --git a/lib/blueprinter/helpers/base_helpers.rb b/lib/blueprinter/helpers/base_helpers.rb index 76226f88..49b8bb0f 100644 --- a/lib/blueprinter/helpers/base_helpers.rb +++ b/lib/blueprinter/helpers/base_helpers.rb @@ -67,6 +67,14 @@ def dynamic_blueprint?(blueprint) blueprint.is_a?(Proc) end + def validate_blueprint(blueprint, method) + validate_presence_of_blueprint(blueprint) + unless dynamic_blueprint?(blueprint) + validate_blueprint_has_ancestors(blueprint, method) + validate_blueprint_has_blueprinter_base_ancestor(blueprint, method) + end + end + def validate_presence_of_blueprint(blueprint) raise BlueprinterError, 'Blueprint required' unless blueprint end From dab36ce70fc9f5a67874ecffb983ea1cd66b82ce Mon Sep 17 00:00:00 2001 From: Charles Washington Date: Tue, 11 Aug 2020 17:33:10 -0300 Subject: [PATCH 8/8] Add bang to methods that might throw errors. --- lib/blueprinter/base.rb | 2 +- lib/blueprinter/helpers/base_helpers.rb | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/blueprinter/base.rb b/lib/blueprinter/base.rb index 9cee675d..34e02dd2 100644 --- a/lib/blueprinter/base.rb +++ b/lib/blueprinter/base.rb @@ -156,7 +156,7 @@ def self.field(method, options = {}, &block) # # @return [Field] A Field object def self.association(method, options = {}, &block) - validate_blueprint(options[:blueprint], method) + validate_blueprint!(options[:blueprint], method) field( method, diff --git a/lib/blueprinter/helpers/base_helpers.rb b/lib/blueprinter/helpers/base_helpers.rb index 49b8bb0f..99b40845 100644 --- a/lib/blueprinter/helpers/base_helpers.rb +++ b/lib/blueprinter/helpers/base_helpers.rb @@ -13,7 +13,7 @@ def prepare_for_render(object, options) view_name = options.delete(:view) || :default root = options.delete(:root) meta = options.delete(:meta) - validate_root_and_meta(root, meta) + validate_root_and_meta!(root, meta) prepare(object, view_name: view_name, local_options: options, root: root, meta: meta) end @@ -52,7 +52,7 @@ def object_to_hash(object, view_name:, local_options:) result_hash end - def validate_root_and_meta(root, meta) + def validate_root_and_meta!(root, meta) case root when String, Symbol # no-op @@ -67,19 +67,19 @@ def dynamic_blueprint?(blueprint) blueprint.is_a?(Proc) end - def validate_blueprint(blueprint, method) - validate_presence_of_blueprint(blueprint) + def validate_blueprint!(blueprint, method) + validate_presence_of_blueprint!(blueprint) unless dynamic_blueprint?(blueprint) - validate_blueprint_has_ancestors(blueprint, method) - validate_blueprint_has_blueprinter_base_ancestor(blueprint, method) + validate_blueprint_has_ancestors!(blueprint, method) + validate_blueprint_has_blueprinter_base_ancestor!(blueprint, method) end end - def validate_presence_of_blueprint(blueprint) + def validate_presence_of_blueprint!(blueprint) raise BlueprinterError, 'Blueprint required' unless blueprint end - def validate_blueprint_has_ancestors(blueprint, association_name) + def validate_blueprint_has_ancestors!(blueprint, association_name) # If the class passed as a blueprint does not respond to ancestors # it means it, at the very least, does not have Blueprinter::Base as # one of its ancestor classes (e.g: Hash) and thus an error should @@ -90,7 +90,7 @@ def validate_blueprint_has_ancestors(blueprint, association_name) end end - def validate_blueprint_has_blueprinter_base_ancestor(blueprint, association_name) + def validate_blueprint_has_blueprinter_base_ancestor!(blueprint, association_name) # Guard clause in case Blueprinter::Base is present in the ancestor list # for the blueprint class provided. return if blueprint.ancestors.include? Blueprinter::Base