Skip to content

Commit

Permalink
Reverted class spoofing
Browse files Browse the repository at this point in the history
Decorators shouldn't pretend to be instances of the underlying model 
classes for the following reasons:

1. It's terribly wrong to pretend being a class without implementing 
it's interface. It can break third party code.
2. Hacking Ruby core methods can lead to unexpected and hard to 
understand behavior. It should be avoided without a strong reason 
behind.
3. I'd prefer compatibility patches to be narrow-scoped and live in 
there own modules.

Resolves #859.
Reverts  #72,
         #110,
         #417,
         #497.
  • Loading branch information
Alexander-Senko committed Sep 13, 2024
1 parent c7e37c1 commit 93a9bc2
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 75 deletions.
6 changes: 0 additions & 6 deletions lib/draper/collection_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,6 @@ def decorated?

alias :decorated_with? :instance_of?

def kind_of?(klass)
decorated_collection.kind_of?(klass) || super
end

alias_method :is_a?, :kind_of?

def replace(other)
decorated_collection.replace(other)
self
Expand Down
20 changes: 2 additions & 18 deletions lib/draper/decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def initialize(object, options = {})
options.assert_valid_keys(:context)
@object = object
@context = options.fetch(:context, {})
handle_multiple_decoration(options) if object.instance_of?(self.class)
handle_multiple_decoration(options) if object.is_a?(Draper::Decorator)
end

class << self
Expand Down Expand Up @@ -185,22 +185,6 @@ def hash
self.class.hash ^ object.hash
end

# Checks if `self.kind_of?(klass)` or `object.kind_of?(klass)`
#
# @param [Class] klass
def kind_of?(klass)
super || object.kind_of?(klass)
end

alias :is_a? :kind_of?

# Checks if `self.instance_of?(klass)` or `object.instance_of?(klass)`
#
# @param [Class] klass
def instance_of?(klass)
super || object.instance_of?(klass)
end

delegate :to_s

# In case object is nil
Expand Down Expand Up @@ -267,7 +251,7 @@ def handle_multiple_decoration(options)
if object.applied_decorators.last == self.class
@context = object.context unless options.has_key?(:context)
@object = object.object
else
elsif object.applied_decorators.include?(self.class)
warn "Reapplying #{self.class} decorator to target that is already decorated with it. Call stack:\n#{caller(1).join("\n")}"
end
end
Expand Down
23 changes: 0 additions & 23 deletions spec/draper/collection_decorator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,29 +249,6 @@ module Draper
end
end

describe '#kind_of?' do
it 'asks the kind of its decorated collection' do
decorator = ProductsDecorator.new([])
expect(decorator.decorated_collection).to receive(:kind_of?).with(Array).and_return("true")
expect(decorator.kind_of?(Array)).to eq "true"
end

context 'when asking the underlying collection returns false' do
it 'asks the CollectionDecorator instance itself' do
decorator = ProductsDecorator.new([])
allow(decorator.decorated_collection).to receive(:kind_of?).with(::Draper::CollectionDecorator).and_return(false)
expect(decorator.kind_of?(::Draper::CollectionDecorator)).to be true
end
end
end

describe '#is_a?' do
it 'aliases to #kind_of?' do
decorator = ProductsDecorator.new([])
expect(decorator.method(:kind_of?)).to eq decorator.method(:is_a?)
end
end

describe "#replace" do
it "replaces the decorated collection" do
decorator = CollectionDecorator.new([Product.new])
Expand Down
28 changes: 0 additions & 28 deletions spec/draper/decorator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -800,34 +800,6 @@ def hello_world
end
end

describe "class spoofing" do
it "pretends to be a kind of the object class" do
decorator = Decorator.new(Model.new)

expect(decorator.kind_of?(Model)).to be_truthy
expect(decorator.is_a?(Model)).to be_truthy
end

it "is still a kind of its own class" do
decorator = Decorator.new(Model.new)

expect(decorator.kind_of?(Decorator)).to be_truthy
expect(decorator.is_a?(Decorator)).to be_truthy
end

it "pretends to be an instance of the object class" do
decorator = Decorator.new(Model.new)

expect(decorator.instance_of?(Model)).to be_truthy
end

it "is still an instance of its own class" do
decorator = Decorator.new(Model.new)

expect(decorator.instance_of?(Decorator)).to be_truthy
end
end

describe ".decorates_finders" do
protect_class Decorator

Expand Down

0 comments on commit 93a9bc2

Please sign in to comment.