Prevention of double calls in callbacks

[Greg-Myers-SB]

Hi Mongoid team,

I was looking at the PR that originally added the run_callbacks method and I noticed it mentions:
#1058

• This does not include multi-level nested documents.
• This does not include prevention of double calls on new child save.

However mongoid does now include multi-level nested documents so that later got resolved :)

The reason I was reading this code is because I noticed that if you have a grandparent relation and you use around_save on the grandchild and cascade callbacks, it will call the around_save twice.

Does anyone have any thoughts on preventing the hooks being fired multiple times per document so that I could do something like a side-effect in a callback?

[Greg-Myers-SB]

To add the modern-day code, I believe this is an effect of:
run_callbacks using cascadable_callbacks which is recursive leading to the grandparent calling hooks on grandchildren, then the parent ALSO calling hooks on grandchild.

[Greg-Myers-SB]

We discovered that this is partially fixed in Mongoid 8 with the introduction of :persist_parent hook that can be used for uniqueness, although it's currently marked private. Can that be backported to Mongoid 7.x and made public?

[alexbevi]

Hi @Greg-Myers-SB, thank you for starting this discussion. I've moved the details you've shared to https://jira.mongodb.org/browse/MONGOID-5542 as this may be a bug, and if it was already addressed in Mongoid 8 we can use this ticket to track the backporting.

[Greg-Myers-SB]

I was running in to two issues, one was the callback ordering which is now fixed in Mongoid 8. The other is that if you embed documents A -> B -> C, and you make the after_save ->() { puts "#{class}"} in all three classes, then you get C multiple times as output.

[alexbevi]

Thanks for clarifying. To simplify our investigation do you have a self-contained reproduction that showcases the result you described above? It sounds pretty straightforward but this would ensure we don't overlook something obvious.

[Greg-Myers-SB]

require 'mongoid'

Mongoid.configure do |config|
  config.clients.default = {
    hosts: ['localhost:27017'],
    database: 'my_db',
  }

  config.log_level = :warn
end

$logger = []

class Root
  include Mongoid::Document
  embeds_many :embedded_once, cascade_callbacks: true
  after_save :trace

  def trace
    $logger << "A"
  end
end

class EmbeddedOnce
  include Mongoid::Document
  embeds_many :embedded_twice, cascade_callbacks: true
  embedded_in :root
  after_save :trace

  def trace
    $logger << "B"
  end
end

class EmbeddedTwice
  include Mongoid::Document
  embedded_in :embedded_once
  after_save :trace

  def trace
    $logger << "C"
  end
end

require 'minitest/autorun'

class EmbedHookTest < MiniTest::Unit::TestCase
  def setup
    @root = Root.new(
      embedded_once: [
        EmbeddedOnce.new(
          embedded_twice: [EmbeddedTwice.new]
        )
      ]
    )
  end

  def test_hooks
    @root.save
    assert_equal(["A", "B", "C"], $logger)
    # Actual is C, C, B, A
  end
end

[alexbevi]

Thank you for the excellent reproduction. I've added this to MONGOID-5542 which our team will triage next week :)