Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Provide a fix for many associations not yielding to each in callbacks. #462

Merged
merged 1 commit into from

2 participants

@gaffneyc

:warning: Feedback / review wanted

Ran into an issue when iterating over a many association in a
before_save callback on a document. For an association named foos,
foos.inspect would show the items but foos.each would never yield to
the given block since it appears to be empty.

The original issue I was tracking down was that instead of not yielding
it was yielding instances of Mongo::Cursor instead of the association
object. I haven't been able to recreate that case outside of the app.

I don't know if this is the best way to fix the issue. We started seeing the above issue in our app starting at b965105.

@gaffneyc gaffneyc Provide a fix for many associations not yielding to each in callbacks.
Ran into an issue when iterating over a many association in a
before_save callback on a document. For an association named foos,
`foos.inspect` would show the items but `foos.each` would never yield to
the given block since it appears to be empty.

The original issue I was tracking down was that instead of not yielding
it was yielding instances of Mongo::Cursor instead of the association
object. I haven't been able to recreate that case.
9844b2a
@jnunemaker
Owner

I'm not a huge fan of the commit that you referenced that started the issues. Automatic is generally a bad thing and that commit moves to automatic.

Thanks for digging in. I'll pull this so at least your issue is fixed until I can whip things back into shape regarding the commit you referenced.

@jnunemaker jnunemaker merged commit d1eb6bc into master

1 check failed

Details default The Travis build failed
@gaffneyc

I got the feeling that there is a weird boundary between MongoMapper and Plucky, that commit made me feel like Plucky knew too much about MongoMapper's implementation. Should be a cleaner division.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 2, 2012
  1. @gaffneyc

    Provide a fix for many associations not yielding to each in callbacks.

    gaffneyc authored
    Ran into an issue when iterating over a many association in a
    before_save callback on a document. For an association named foos,
    `foos.inspect` would show the items but `foos.each` would never yield to
    the given block since it appears to be empty.
    
    The original issue I was tracking down was that instead of not yielding
    it was yielding instances of Mongo::Cursor instead of the association
    object. I haven't been able to recreate that case.
This page is out of date. Refresh to see the latest.
View
6 lib/mongo_mapper/plugins/associations/many_documents_proxy.rb
@@ -5,7 +5,7 @@ module Associations
class ManyDocumentsProxy < Collection
include DynamicQuerying::ClassMethods
- def_delegators :query, *(Querying::Methods - [:to_a, :size, :empty?])
+ def_delegators :query, *(Querying::Methods - [:each, :to_a, :size, :empty?])
def replace(docs)
load_target
@@ -71,6 +71,10 @@ def save_to_collection(options={})
@target.each { |doc| doc.save(options) } if @target
end
+ def each(&block)
+ load_target.each(&block)
+ end
+
protected
def query(options={})
klass.
View
29 test/functional/associations/test_many_documents_proxy.rb
@@ -35,6 +35,35 @@ def pets
instance.pets.should_not be_empty
end
+ should "be able to iterate associated documents in a callback" do
+ @owner_class.class_eval do
+ before_save :search_pets
+
+ def search_pets
+ pets.each { |p| p.name = "Animal" }
+ end
+ end
+
+ owner = @owner_class.new
+ sophie = owner.pets.build(:name => "Sophie")
+ pippa = owner.pets.build(:name => "Pippa")
+
+ owner.save
+ owner.reload
+ owner.pets.reload
+
+ pets = []
+ owner.pets.each { |p| pets << p }
+
+ assert_equal(2, pets.size)
+ assert(pets.include?(sophie))
+ assert(pets.include?(pippa))
+
+ # Weird way of testing that the callback actually interated
+ assert_equal("Animal", sophie.reload.name)
+ assert_equal("Animal", pippa.reload.name)
+ end
+
should "allow assignment of many associated documents using a hash" do
person_attributes = {
'name' => 'Mr. Pet Lover',
Something went wrong with that request. Please try again.