From cc236698893e79e655efdbc1165c011416259875 Mon Sep 17 00:00:00 2001 From: Yen-Nan Lin Date: Thu, 14 Sep 2017 22:48:01 +0800 Subject: [PATCH] Let decorator_class infer anonymous class' decorator from superclass (#820) * Let decorator_class infer anonymous class' decorator from superclass * Replace constantize with safe_constantize on decorator's private methods * Don't raise NameError on decorator private methods. To prevent rescuing and checking if NameError is from internal decorator private methods, we change to return nil on these private methods. * Revise decorator inferred_object_class and collection_decorator_name based on code review comments --- lib/draper/decoratable.rb | 6 +++--- lib/draper/decorator.rb | 21 +++++++++++---------- spec/draper/decoratable_spec.rb | 14 +++++++++++++- spec/draper/decorator_spec.rb | 5 +++-- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/lib/draper/decoratable.rb b/lib/draper/decoratable.rb index 73dc07e..6c99ced 100644 --- a/lib/draper/decoratable.rb +++ b/lib/draper/decoratable.rb @@ -72,9 +72,9 @@ module Draper def decorator_class prefix = respond_to?(:model_name) ? model_name : name decorator_name = "#{prefix}Decorator" - decorator_name.constantize - rescue NameError => error - raise unless error.missing_name?(decorator_name) + decorator_name_constant = decorator_name.safe_constantize + return decorator_name_constant unless decorator_name_constant.nil? + if superclass.respond_to?(:decorator_class) superclass.decorator_class else diff --git a/lib/draper/decorator.rb b/lib/draper/decorator.rb index 384ad0e..1401595 100644 --- a/lib/draper/decorator.rb +++ b/lib/draper/decorator.rb @@ -225,9 +225,9 @@ module Draper # @return [Class] the class created by {decorate_collection}. def self.collection_decorator_class name = collection_decorator_name - name.constantize - rescue NameError - Draper::CollectionDecorator + name_constant = name && name.safe_constantize + + name_constant || Draper::CollectionDecorator end private @@ -242,22 +242,23 @@ module Draper end def self.object_class_name - raise NameError if name.nil? || name.demodulize !~ /.+Decorator$/ + return nil if name.nil? || name.demodulize !~ /.+Decorator$/ name.chomp("Decorator") end def self.inferred_object_class name = object_class_name - name.constantize - rescue NameError => error - raise if name && !error.missing_name?(name) + name_constant = name && name.safe_constantize + return name_constant unless name_constant.nil? + raise Draper::UninferrableObjectError.new(self) end def self.collection_decorator_name - plural = object_class_name.pluralize - raise NameError if plural == object_class_name - "#{plural}Decorator" + singular = object_class_name + plural = singular && singular.pluralize + + "#{plural}Decorator" unless plural == singular end def handle_multiple_decoration(options) diff --git a/spec/draper/decoratable_spec.rb b/spec/draper/decoratable_spec.rb index ecabee6..df05580 100644 --- a/spec/draper/decoratable_spec.rb +++ b/spec/draper/decoratable_spec.rb @@ -205,10 +205,22 @@ module Draper context "when an unrelated NameError is thrown" do it "re-raises that error" do - allow_any_instance_of(String).to receive(:constantize) { Draper::Base } + # Not related to safe_constantize behavior, we just want to raise a NameError inside the function + allow_any_instance_of(String).to receive(:safe_constantize) { Draper::Base } expect{Product.decorator_class}.to raise_error NameError, /Draper::Base/ end end + + context "when an anonymous class is given" do + it "infers the decorator from a superclass" do + anonymous_class = Class.new(Product) do + def self.name + to_s + end + end + expect(anonymous_class.decorator_class).to be ProductDecorator + end + end end end diff --git a/spec/draper/decorator_spec.rb b/spec/draper/decorator_spec.rb index c188ce1..a354211 100644 --- a/spec/draper/decorator_spec.rb +++ b/spec/draper/decorator_spec.rb @@ -202,7 +202,8 @@ module Draper context "when an unrelated NameError is thrown" do it "re-raises that error" do - allow_any_instance_of(String).to receive(:constantize) { SomethingThatDoesntExist } + # Not related to safe_constantize behavior, we just want to raise a NameError inside the function + allow_any_instance_of(String).to receive(:safe_constantize) { SomethingThatDoesntExist } expect{ProductDecorator.object_class}.to raise_error NameError, /SomethingThatDoesntExist/ end end @@ -225,7 +226,7 @@ module Draper describe '.collection_decorator_class' do it 'defaults to CollectionDecorator' do - allow_any_instance_of(String).to receive(:constantize) { SomethingThatDoesntExist } + allow_any_instance_of(String).to receive(:safe_constantize) { nil } expect(ProductDecorator.collection_decorator_class).to be Draper::CollectionDecorator end