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

Commit & rollback callbacks defined in mongoid 8.1 before code to run them is added

    • Type: Icon: Bug Bug
    • Resolution: Fixed
    • Priority: Icon: Minor - P4 Minor - P4
    • 8.1.4
    • Affects Version/s: 8.1.3
    • Component/s: None
    • None

      MONGOID-5531 adds commit and rollback transactions to Mongoid, targeting mongoid 9.0. As a result, the corresponding PR was not merged onto the 8.1-stable branch. However, a recent commit to address MONGOID-5658 contains code that defines commit and rollback callbacks. This code was merged onto the 8.1-stable branch. The code seems to originate from MONGOID-5531's PR (#5519).

      I'm not sure why this was included or whether it's relevant to the fix related to embedded callbacks, but I believe it was erroneous to define these callbacks when the corresponding code that actually executes the callbacks was not added. For our use case, we use a gem that runs callbacks on an after_commit callback if it's defined, and if not, runs callbacks on the after_save callback. Previously, it always ran on the after_save callback because the after_commit callback was not defined on Mongoid models. When we upgraded to mongoid 8.1.3, the callback stopped running, because when the gem saw that after_commit callbacks were available, it set up its callback to trigger on the after_commit callback, which is never actually run by mongoid 8.1.3 (since it's lacking the rest of MONGOID-5531's code that actually enables the callbacks).

      I recognize that this may not be especially problematic for many people, but as I said above, I feel that it doesn't really make sense to define callback registration methods for callbacks you don't run, so I would consider this a bug.

            Assignee:
            dmitry.rybakov@mongodb.com Dmitry Rybakov
            Reporter:
            dan@cacheventures.com Daniel Arnold
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: