Uploaded image for project: 'Mongoid'
  1. Mongoid
  2. MONGOID-3552

after_remove callback called at incorrect time on has_and_belongs_to_many associations

XMLWordPrintableJSON

    • Minor Change

      In the class ManyToMany, the delete method looks as follows:

      class ManyToMany < Many
...
def delete(document)
  doc = super
    if doc && persistable?
      base.pull(foreign_key => doc.send(__metadata.primary_key))
      target._unloaded = criteria
      unsynced(base, foreign_key)
    end
  doc
...
end

      The call to super on the first line of the method invokes the delete method in class Many, which looks as follows:

      Class Many < Relations::Many
...
def delete(document)
  execute_callback :before_remove, document
  target.delete(document) do |doc|
    if doc
      unbind_one(doc)
      cascade!(doc) if !_assigning?
    end
    execute_callback :after_remove, doc
  end
...
end

      I have included these for reference.

      Here are three models, Foo, Thing, and Bar:

      class Foo
  ...
  has_and_belongs_to_many :bars, after_add: :after_add_callback, 
    after_remove: :after_remove_callback, dependent: :nullify

  has_many :things

  def after_add_callback(added_bar)
    # deletes all things
    # creates new things using Thing.collection.insert([lots of thing documents])    
    puts "self.bars.length: #{self.bars.length}"
  end

  def after_remove_callback(removed_bar)
    # deletes all things
    # creates new things using Thing.collection.insert([lots of thing documents])
    puts "self.bars.length: #{self.bars.length}"
  end
  ...
end

class Things
  ...
  belongs_to :foo
  ...
end

class Bar
  ...
  has_and_belongs_to_many :foos, dependent: :nullify
  ...
end

      If I perform the following rspec test, I get behavior that is not anticipated:

      require 'spec_helper'

describe Foo do

  describe 'failing to work as expected' do

    it 'does not keep track of bars' do
      foo = Foo.create()
      bar1 = Bar.create()
      bar2 = Bar.create()
      bar3 = Bar.create()

      foo.bars.length.should==0
      foo.bars.count.should==0

      foo.bars << bar1
      # callback puts self.bars.length: 1 # expected 1

      foo.bars.length.should==1
      foo.bars.count.should==1

      foo.reload

      foo.bars.length.should==1
      foo.bars.count.should==1

      foo.bars << bar2
      # callback puts self.bars.length: 1 # expected 2

      foo.bars.length.should==1 
      foo.bars.count.should==2

      foo.reload

      foo.bars.length.should==2
      foo.bars.count.should==2
      
      foo.bars << bar3
      # callback puts self.bars.length: 2 # expected 3

      foo.bars.length.should==2 
      foo.bars.count.should==3

      foo.reload

      foo.bars.length.should==3
      foo.bars.count.should==3
      
      foo.bars.delete(bar3)
      # callback puts self.bars.length: 3 #expected 2

      foo.bars.length.should==2
      foo.bars.count.should==2

      foo.reload

      foo.bars.length.should==2
      foo.bars.count.should==2
    end
  end

  describe 'works as expected' do

    it 'fails to display changes to things' do
      foo = Foo.create()
      bar1 = Bar.create()
      bar2 = Bar.create()
      bar3 = Bar.create()

      foo.bars.length.should==0
      foo.bars.count.should==0

      foo.bars << bar1
      # callback puts self.bars.length: 1 #expected 1

      foo.bars.length.should==1
      foo.bars.count.should==1

      foo.bars.length.should==1
      foo.bars.count.should==1
 
      foo.bars << bar2
      # callback puts self.bars.length: 2 # expected 2

      foo.bars.length.should==2 # get 2, excepted 2
      foo.bars.count.should==2

      foo.bars.length.should==2
      foo.bars.count.should==2

      foo.bars << bar3
      # callback puts self.bars.length: 3 # expected 3

      foo.bars.length.should==3 # get 3, expected 3
      foo.bars.count.should==3

      foo.bars.length.should==3
      foo.bars.count.should==3

      foo.bars.delete(bar3)
      # callback puts self.bars.length: 2 # expected 2

      foo.bars.length.should==2
      foo.bars.count.should==2
    end
  end

end

      On the failing to work as expected test, the problem is that, within the callback methods after_add_bar and after_remove_bar in the class Foo, I make atomic changes that are dependent on each of the bars. In addition, I create documents using Moped's Model.collection.insert() method, which is also dependent on the bars relation in Foo. If I check the length of bars within either callback, except for after the first push operation, the callback lengths are consistently 1 count off.

      If I were to run the same second test, works as expected, the values for the length of bars in both the callbacks and the test are all what I would expect. However, because I do not reload foo, the relationship to things is incorrect and returns 0 documents (because the previously existing documents were deleted in the callback). There are in fact many newly created things that should be related to the current foo object.

      (Note: I did include foreign_key information in each thing :foo_id => Foo's id. This is not the problem.)

      The problem appears to directly relate the the time when the :after_remove callback is called in the class ManyToMany. The problem is not present if I rewrite the delete method in the Many and ManyToMany classes as follows:

      class Many < Relations::Many
  ...
  def delete(document, opts={})
    execute_after_remove = opts[:execute_after_remove] || true
    execute_callback :before_remove, document
    target.delete(document) do |doc|
      if doc
        unbind_one(doc)
        cascade!(doc) if !_assigning?
      end
      if execute_after_remove
        execute_callback :after_remove, doc
      end
      end
  end
  ...
end

class ManyToMany < Many
  ...
  def delete(document)
    doc = super(document, execute_after_remove: false)
    if doc && persistable?
      base.pull(foreign_key => doc.send(__metadata.primary_key))
      target._unloaded = criteria
       unsynced(base, foreign_key)
    end
    execute_callback :after_remove, doc
    doc
  end
  ...
end

      In the above implementation, the methods in Foo called by :after_add and :after_remove correctly perceive the number of bar documents in bars in both tests.

      By instead moving the call to execute_callback, for the remove callback, to the end of the delete method in the ManyToMany class, the methods supplied for the after_remove and after_add callbacks both work as expected.

      I am not suggesting that the hacked delete methods I used above to fix the problem should be used, but I am merely showing them to highlight the issue.

      I hope this issue was clear enough. If not, I would be more than willing to go into more detail.

            Assignee:
            neil.shweky@mongodb.com Neil Shweky (Inactive)
            Reporter:
            peterjkirby Peter Kirby
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: