From fd944cfe11e6a5b1bd4c43bb0b6bf16126007898 Mon Sep 17 00:00:00 2001 From: Andrew Haines Date: Mon, 24 Dec 2012 07:30:05 +0000 Subject: [PATCH 1/2] Freeze collections Closes #387 --- lib/draper/collection_decorator.rb | 4 +- spec/draper/collection_decorator_spec.rb | 52 ++++++++++++----------- spec/draper/decoratable_spec.rb | 2 +- spec/draper/decorated_association_spec.rb | 2 +- spec/draper/decorator_spec.rb | 2 +- spec/draper/finders_spec.rb | 4 +- 6 files changed, 34 insertions(+), 32 deletions(-) diff --git a/lib/draper/collection_decorator.rb b/lib/draper/collection_decorator.rb index f482d11..e96c5dd 100644 --- a/lib/draper/collection_decorator.rb +++ b/lib/draper/collection_decorator.rb @@ -16,7 +16,7 @@ module Draper # @option options [Hash] :context context available to each item's decorator def initialize(source, options = {}) options.assert_valid_keys(:with, :context) - @source = source + @source = source.dup.freeze @decorator_class = options[:with] @context = options.fetch(:context, {}) end @@ -26,7 +26,7 @@ module Draper end def decorated_collection - @decorated_collection ||= source.collect {|item| decorate_item(item) } + @decorated_collection ||= source.map{|item| decorate_item(item)}.freeze end def find(*args, &block) diff --git a/spec/draper/collection_decorator_spec.rb b/spec/draper/collection_decorator_spec.rb index 00dd6aa..c43f37d 100644 --- a/spec/draper/collection_decorator_spec.rb +++ b/spec/draper/collection_decorator_spec.rb @@ -16,6 +16,27 @@ describe Draper::CollectionDecorator do subject.map{|item| item.source}.should == source end + describe "#source" do + it "duplicates the source collection" do + subject.source.should == source + subject.source.should_not be source + end + + it "is frozen" do + subject.source.should be_frozen + end + + it "is aliased to #to_source" do + subject.to_source.should == source + end + end + + describe "#decorated_collection" do + it "is frozen" do + subject.decorated_collection.should be_frozen + end + end + context "with context" do subject { Draper::CollectionDecorator.new(source, with: ProductDecorator, context: {some: 'context'}) } @@ -81,16 +102,6 @@ describe Draper::CollectionDecorator do end end - describe "#source" do - it "returns the source collection" do - subject.source.should be source - end - - it "is aliased to #to_source" do - subject.to_source.should be source - end - end - describe "item decoration" do subject { subject_class.new(source, options) } let(:decorator_classes) { subject.decorated_collection.map(&:class) } @@ -140,6 +151,7 @@ describe Draper::CollectionDecorator do describe "#find" do context "with a block" do it "decorates Enumerable#find" do + subject.stub decorated_collection: [] subject.decorated_collection.should_receive(:find) subject.find {|p| p.title == "title" } end @@ -196,7 +208,7 @@ describe Draper::CollectionDecorator do describe "#to_ary" do # required for `render @collection` in Rails it "delegates to the decorated collection" do - subject.decorated_collection.should_receive(:to_ary).and_return(:an_array) + subject.stub decorated_collection: double(to_ary: :an_array) subject.to_ary.should == :an_array end end @@ -212,20 +224,10 @@ describe Draper::CollectionDecorator do end end - context "Array methods" do - describe "#include?" do - it "delegates to the decorated collection" do - subject.decorated_collection.should_receive(:include?).with(:something).and_return(true) - subject.should include :something - end - end - - describe "#[]" do - it "delegates to the decorated collection" do - subject.decorated_collection.should_receive(:[]).with(42).and_return(:something) - subject[42].should == :something - end - end + it "delegates array methods to the decorated collection" do + subject.stub decorated_collection: [] + subject.decorated_collection.should_receive(:[]).with(42).and_return(:the_answer) + subject[42].should == :the_answer end describe "#==" do diff --git a/spec/draper/decoratable_spec.rb b/spec/draper/decoratable_spec.rb index efc3140..4f863be 100644 --- a/spec/draper/decoratable_spec.rb +++ b/spec/draper/decoratable_spec.rb @@ -150,7 +150,7 @@ describe Draper::Decoratable do decorator.should be_a Draper::CollectionDecorator decorator.decorator_class.should be WidgetDecorator - decorator.source.should be Product.scoped + decorator.source.should == Product.scoped end it "accepts context" do diff --git a/spec/draper/decorated_association_spec.rb b/spec/draper/decorated_association_spec.rb index 88a1fcd..66604cb 100644 --- a/spec/draper/decorated_association_spec.rb +++ b/spec/draper/decorated_association_spec.rb @@ -88,7 +88,7 @@ describe Draper::DecoratedAssociation do it "applies the scope before decoration" do scoped = [:scoped] associated.should_receive(:foo).and_return(scoped) - decorated_association.call.source.should be scoped + decorated_association.call.source.should == scoped end end end diff --git a/spec/draper/decorator_spec.rb b/spec/draper/decorator_spec.rb index 89e03b3..f0ab2d7 100755 --- a/spec/draper/decorator_spec.rb +++ b/spec/draper/decorator_spec.rb @@ -94,7 +94,7 @@ describe Draper::Decorator do it "returns a collection decorator" do subject.should be_a Draper::CollectionDecorator - subject.source.should be source + subject.source.should == source end it "uses itself as the item decorator by default" do diff --git a/spec/draper/finders_spec.rb b/spec/draper/finders_spec.rb index 63db8ae..e338f64 100644 --- a/spec/draper/finders_spec.rb +++ b/spec/draper/finders_spec.rb @@ -64,7 +64,7 @@ describe Draper::Finders do describe ".find_all_by_" do it "proxies to the model class" do - Product.should_receive(:find_all_by_name_and_size).with("apples", "large") + Product.should_receive(:find_all_by_name_and_size).with("apples", "large").and_return([]) ProductDecorator.find_all_by_name_and_size("apples", "large") end @@ -73,7 +73,7 @@ describe Draper::Finders do Product.stub(:find_all_by_name).and_return(found) decorator = ProductDecorator.find_all_by_name("apples") decorator.should be_a Draper::CollectionDecorator - decorator.source.should be found + decorator.source.should == found end end From 4960d49c931c04ea66f5441681e542dc88278156 Mon Sep 17 00:00:00 2001 From: Andrew Haines Date: Mon, 24 Dec 2012 07:33:47 +0000 Subject: [PATCH 2/2] Stop delegating to the source collection Closes #389 --- lib/draper/collection_decorator.rb | 13 --------- spec/draper/collection_decorator_spec.rb | 35 ------------------------ 2 files changed, 48 deletions(-) diff --git a/lib/draper/collection_decorator.rb b/lib/draper/collection_decorator.rb index e96c5dd..abf012e 100644 --- a/lib/draper/collection_decorator.rb +++ b/lib/draper/collection_decorator.rb @@ -37,19 +37,6 @@ module Draper end end - def method_missing(method, *args, &block) - source.send(method, *args, &block) - end - - def respond_to?(method, include_private = false) - super || source.respond_to?(method, include_private) - end - - def kind_of?(klass) - super || source.kind_of?(klass) - end - alias_method :is_a?, :kind_of? - def ==(other) source == (other.respond_to?(:source) ? other.source : other) end diff --git a/spec/draper/collection_decorator_spec.rb b/spec/draper/collection_decorator_spec.rb index c43f37d..ce21437 100644 --- a/spec/draper/collection_decorator_spec.rb +++ b/spec/draper/collection_decorator_spec.rb @@ -213,17 +213,6 @@ describe Draper::CollectionDecorator do end end - describe "#respond_to?" do - it "returns true for its own methods" do - subject.should respond_to :decorated_collection - end - - it "returns true for the wrapped collection's methods" do - source.stub(:respond_to?).with(:whatever, true).and_return(true) - subject.respond_to?(:whatever, true).should be_true - end - end - it "delegates array methods to the decorated collection" do subject.stub decorated_collection: [] subject.decorated_collection.should_receive(:[]).with(42).and_return(:the_answer) @@ -264,30 +253,6 @@ describe Draper::CollectionDecorator do end end - it "pretends to be the source class" do - subject.kind_of?(source.class).should be_true - subject.is_a?(source.class).should be_true - end - - it "is still its own class" do - subject.kind_of?(subject.class).should be_true - subject.is_a?(subject.class).should be_true - end - - describe "#method_missing" do - before do - class << source - def page_number - 42 - end - end - end - - it "proxies unknown methods to the source collection" do - subject.page_number.should == 42 - end - end - describe "#to_s" do subject { Draper::CollectionDecorator.new(source, options) } let(:source) { ["a", "b", "c"] }