From d4e2b6cdb4094efb0393348d3ee2a17544a7a33d Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Wed, 2 Jul 2014 08:37:24 -0500 Subject: [PATCH 1/4] Prevent a get request returning an class instance instead of a collection from throwing an exception. --- lib/her/model/associations/has_many_association.rb | 9 ++++++++- spec/model/associations_spec.rb | 9 +++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/her/model/associations/has_many_association.rb b/lib/her/model/associations/has_many_association.rb index 63cef796..9930f6c6 100644 --- a/lib/her/model/associations/has_many_association.rb +++ b/lib/her/model/associations/has_many_association.rb @@ -83,7 +83,14 @@ def create(attributes = {}) # @private def fetch - super.tap do |o| + collection = super + # super can return an instance rather than a collection when + # the response is nil. + unless collection.is_a?(Her::Collection) + for_collection = collection.attributes.empty? ? [] : collection + collection = Her::Collection.new(for_collection) + end + collection.tap do |o| inverse_of = @opts[:inverse_of] || @parent.singularized_resource_name o.each { |entry| entry.send("#{inverse_of}=", @parent) } end diff --git a/spec/model/associations_spec.rb b/spec/model/associations_spec.rb index 8d293403..8fa4f49b 100644 --- a/spec/model/associations_spec.rb +++ b/spec/model/associations_spec.rb @@ -89,6 +89,7 @@ builder.adapter :test do |stub| stub.get("/users/1") { |env| [200, {}, { :id => 1, :name => "Tobias Fünke", :comments => [{ :comment => { :id => 2, :body => "Tobias, you blow hard!", :user_id => 1 } }, { :comment => { :id => 3, :body => "I wouldn't mind kissing that man between the cheeks, so to speak", :user_id => 1 } }], :role => { :id => 1, :body => "Admin" }, :organization => { :id => 1, :name => "Bluth Company" }, :organization_id => 1 }.to_json] } stub.get("/users/2") { |env| [200, {}, { :id => 2, :name => "Lindsay Fünke", :organization_id => 2 }.to_json] } + stub.get("/users/3/comments") { |env| [200, {}, nil]} stub.get("/users/1/comments") { |env| [200, {}, [{ :comment => { :id => 4, :body => "They're having a FIRESALE?" } }].to_json] } stub.get("/users/2/comments") { |env| [200, {}, [{ :comment => { :id => 4, :body => "They're having a FIRESALE?" } }, { :comment => { :id => 5, :body => "Is this the tiny town from Footloose?" } }].to_json] } stub.get("/users/2/comments/5") { |env| [200, {}, { :comment => { :id => 5, :body => "Is this the tiny town from Footloose?" } }.to_json] } @@ -137,6 +138,7 @@ let(:user_with_included_data_after_create) { Foo::User.create } let(:user_with_included_data_after_save_existing) { Foo::User.save_existing(5, :name => "Clancy Brown") } let(:user_with_included_data_after_destroy) { Foo::User.new(:id => 5).destroy } + let(:user_with_no_comments) { Foo::User.new(id: 3)} it "maps an array of included data through has_many" do @user_with_included_data.comments.first.should be_a(Foo::Comment) @@ -228,6 +230,13 @@ params[:comments].length.should eq(2) end + it 'responds properly to blank?' do + @user_with_included_data.comments.should_not be_blank + #debugger + user_with_no_comments.comments.should be_blank + end + + [:create, :save_existing, :destroy].each do |type| context "after #{type}" do let(:subject) { self.send("user_with_included_data_after_#{type}")} From f6a205ad0328148ef88ca6e3331e6097c944d794 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Wed, 2 Jul 2014 08:48:53 -0500 Subject: [PATCH 2/4] wrap in an array --- lib/her/model/associations/has_many_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/her/model/associations/has_many_association.rb b/lib/her/model/associations/has_many_association.rb index 9930f6c6..de547b2d 100644 --- a/lib/her/model/associations/has_many_association.rb +++ b/lib/her/model/associations/has_many_association.rb @@ -88,7 +88,7 @@ def fetch # the response is nil. unless collection.is_a?(Her::Collection) for_collection = collection.attributes.empty? ? [] : collection - collection = Her::Collection.new(for_collection) + collection = Her::Collection.new(Array(for_collection)) end collection.tap do |o| inverse_of = @opts[:inverse_of] || @parent.singularized_resource_name From b649becd90c94cdfe120c78f8c52cf0898eddeaf Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Wed, 2 Jul 2014 08:50:19 -0500 Subject: [PATCH 3/4] remove stray debugger statement --- spec/model/associations_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/model/associations_spec.rb b/spec/model/associations_spec.rb index 8fa4f49b..a2ed5a68 100644 --- a/spec/model/associations_spec.rb +++ b/spec/model/associations_spec.rb @@ -232,7 +232,6 @@ it 'responds properly to blank?' do @user_with_included_data.comments.should_not be_blank - #debugger user_with_no_comments.comments.should be_blank end From 21099ecc5a1a7b9ec16d67ba6fcb09134f7a5b80 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Mon, 7 Jul 2014 08:56:17 -0500 Subject: [PATCH 4/4] PR feedback: Have has_many_association's fetch hint to association's fetch that the response should be a collection, instead of turning the response into a collection. --- lib/her/model/associations/association.rb | 2 +- lib/her/model/associations/has_many_association.rb | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/her/model/associations/association.rb b/lib/her/model/associations/association.rb index 6fc96a87..cf87a5c3 100644 --- a/lib/her/model/associations/association.rb +++ b/lib/her/model/associations/association.rb @@ -49,7 +49,7 @@ def fetch(opts = {}) if @parent.attributes[@name].blank? || @params.any? path = build_association_path lambda { "#{@parent.request_path(@params)}#{@opts[:path]}" } - @klass.get(path, @params) + opts[:as] == :collection ? @klass.get_collection(path, @params) : @klass.get(path, @params) else @parent.attributes[@name] end diff --git a/lib/her/model/associations/has_many_association.rb b/lib/her/model/associations/has_many_association.rb index de547b2d..f846ee5a 100644 --- a/lib/her/model/associations/has_many_association.rb +++ b/lib/her/model/associations/has_many_association.rb @@ -83,14 +83,7 @@ def create(attributes = {}) # @private def fetch - collection = super - # super can return an instance rather than a collection when - # the response is nil. - unless collection.is_a?(Her::Collection) - for_collection = collection.attributes.empty? ? [] : collection - collection = Her::Collection.new(Array(for_collection)) - end - collection.tap do |o| + super(as: :collection).tap do |o| inverse_of = @opts[:inverse_of] || @parent.singularized_resource_name o.each { |entry| entry.send("#{inverse_of}=", @parent) } end