[Django] #33410: captureOnCommitCallbacks executes callbacks multiple times
Django
-------------------------------------+-------------------------------------
Reporter: flaeppe | Owner: flaeppe
Type: Bug | Status: assigned
Component: Testing | Version: dev
framework | Keywords:
Severity: Normal | captureOnCommitCallbacks
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
When recursively adding multiple on commit callbacks,
`captureOnCommitCallbacks` executes some callbacks multiple times. Below
is a test case to reproduce the error.
{{{
#!div style="font-size: 80%"
{{{#!python
def test_execute_tree(self):
"""
A visualisation of the callback tree tested. Each node is expected
to be visited
only once. The child count of a node symbolises the amount of on
commit
callbacks.
root
└── branch_1
├── branch_2
│ ├── leaf_1
│ └── leaf_2
└── leaf_3
"""
(
branch_1_call_counter,
branch_2_call_counter,
leaf_1_call_counter,
leaf_2_call_counter,
leaf_3_call_counter,
) = [itertools.count(start=1) for _ in range(5)]
def leaf_3():
next(leaf_3_call_counter)
def leaf_2():
next(leaf_2_call_counter)
def leaf_1():
next(leaf_1_call_counter)
def branch_2():
next(branch_2_call_counter)
transaction.on_commit(leaf_1)
transaction.on_commit(leaf_2)
def branch_1():
next(branch_1_call_counter)
transaction.on_commit(branch_2)
transaction.on_commit(leaf_3)
with self.captureOnCommitCallbacks(execute=True) as callbacks:
with transaction.atomic():
transaction.on_commit(branch_1)
# Every counter shall only have been called once, starting at 1;
next value then
# has to be 2
self.assertEqual(next(branch_1_call_counter), 2)
self.assertEqual(next(branch_2_call_counter), 2)
self.assertEqual(next(leaf_1_call_counter), 2)
self.assertEqual(next(leaf_2_call_counter), 2)
self.assertEqual(next(leaf_3_call_counter), 2)
# Make sure all calls can be seen and the execution order is
correct
self.assertEqual(
[(id(callback), callback.__name__) for callback in callbacks],
[
(id(branch_1), "branch_1"),
(id(branch_2), "branch_2"),
(id(leaf_3), "leaf_3"),
(id(leaf_1), "leaf_1"),
(id(leaf_2), "leaf_2"),
],
)
}}}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33410>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: flaeppe | Owner: flaeppe
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage:
captureOnCommitCallbacks | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: flaeppe | Owner: flaeppe
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage:
captureOnCommitCallbacks | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
When recursively adding multiple on commit callbacks,
`captureOnCommitCallbacks` executes some callbacks multiple times. Below
is a test case to reproduce the error.
Patch: https://github.com/django/django/pull/15285
def leaf_3():
next(leaf_3_call_counter)
def leaf_2():
next(leaf_2_call_counter)
def leaf_1():
next(leaf_1_call_counter)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Petter Friberg | Owner: Petter
| Friberg
Type: Bug | Status: assigned
Component: Testing framework | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
captureOnCommitCallbacks |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Eugene Morozov, Adam Johnson (added)
* severity: Normal => Release blocker
* version: dev => 4.0
* stage: Unreviewed => Accepted
Comment:
Thanks for the report!
Marking as a release blocker since it's a bug in the new feature added in
d89f976bddb49fb168334960acc8979c3de991fa (#33054).
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Petter Friberg | Owner: Petter
| Friberg
Component: Testing framework | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
captureOnCommitCallbacks |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Petter Friberg | Owner: Petter
| Friberg
Component: Testing framework | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
captureOnCommitCallbacks |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Petter Friberg | Owner: Petter
| Friberg
Component: Testing framework | Version: 4.0
Severity: Release blocker | Resolution:
captureOnCommitCallbacks | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Petter Friberg | Owner: Petter
| Friberg
Component: Testing framework | Version: 4.0
Keywords: | Triage Stage: Ready for
captureOnCommitCallbacks | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"bc174e6ea0ce676c5a7f467bda9739e6ef6b6186" bc174e6]:
{{{
#!CommitTicketReference repository=""
revision="bc174e6ea0ce676c5a7f467bda9739e6ef6b6186"
Fixed #33410 -- Fixed recursive capturing of callbacks by
TestCase.captureOnCommitCallbacks().
Regression in d89f976bddb49fb168334960acc8979c3de991fa.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Petter Friberg | Owner: Petter
| Friberg
Type: Bug | Status: closed
Component: Testing framework | Version: 4.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
captureOnCommitCallbacks | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"11475958f6c1aca61638c476cc314ec0410161dc" 11475958]:
{{{
#!CommitTicketReference repository=""
revision="11475958f6c1aca61638c476cc314ec0410161dc"
[4.0.x] Fixed #33410 -- Fixed recursive capturing of callbacks by
TestCase.captureOnCommitCallbacks().
Regression in d89f976bddb49fb168334960acc8979c3de991fa.
Backport of bc174e6ea0ce676c5a7f467bda9739e6ef6b6186 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33410#comment:8>