send inv computed tasks *after* commit to avoid a race condition

awx/main/signals.py

@@ -10,6 +10,7 @@ import pkg_resources
 import sys
 
 # Django
+from django.db import connection
 from django.conf import settings
 from django.db.models.signals import (
     pre_save,
@@ -124,7 +125,9 @@ def emit_update_inventory_on_created_or_deleted(sender, **kwargs):
         pass
     else:
         if inventory is not None:
-            update_inventory_computed_fields.delay(inventory.id)
+            connection.on_commit(
+                lambda: update_inventory_computed_fields.delay(inventory.id)
+            )
 
 
 def rebuild_role_ancestor_list(reverse, model, instance, pk_set, action, **kwargs):

awx/main/tests/functional/__init__.py

@@ -2,6 +2,9 @@ from django.db import connection
 from django.db.models.signals import post_migrate
 from django.apps import apps
 from django.conf import settings
+from unittest import mock
+
+import contextlib
 
 
 def app_post_migration(sender, app_config, **kwargs):
@@ -23,3 +26,13 @@ if settings.DATABASES['default']['ENGINE'] == 'django.db.backends.sqlite3':
 
 
 
+@contextlib.contextmanager
+def immediate_on_commit():
+    """
+    Context manager executing transaction.on_commit() hooks immediately as
+    if the connection was in auto-commit mode.
+    """
+    def on_commit(func):
+        func()
+    with mock.patch('django.db.connection.on_commit', side_effect=on_commit) as patch:
+        yield patch

awx/main/tests/functional/api/test_oauth.py

@@ -1,8 +1,6 @@
 import pytest
 import base64
-import contextlib
 import json
-from unittest import mock
 
 from django.db import connection
 from django.test.utils import override_settings
@@ -12,22 +10,11 @@ from awx.main.utils.encryption import decrypt_value, get_encryption_key
 from awx.api.versioning import reverse, drf_reverse
 from awx.main.models.oauth import (OAuth2Application as Application, 
                                    OAuth2AccessToken as AccessToken)
+from awx.main.tests.functional import immediate_on_commit
 from awx.sso.models import UserEnterpriseAuth
 from oauth2_provider.models import RefreshToken
 
 
-@contextlib.contextmanager
-def immediate_on_commit():
-    """
-    Context manager executing transaction.on_commit() hooks immediately as
-    if the connection was in auto-commit mode.
-    """
-    def on_commit(func):
-        func()
-    with mock.patch('django.db.connection.on_commit', side_effect=on_commit) as patch:
-        yield patch
-
-
 @pytest.mark.django_db
 def test_personal_access_token_creation(oauth_application, post, alice):
     url = drf_reverse('api:oauth_authorization_root_view') + 'token/'

awx/main/tests/functional/models/test_context_managers.py

@@ -11,6 +11,7 @@ from awx.main.signals import (
 # AWX models
 from awx.main.models.organization import Organization
 from awx.main.models import ActivityStream, Job
+from awx.main.tests.functional import immediate_on_commit
 
 
 @pytest.mark.django_db
@@ -34,9 +35,10 @@ class TestComputedFields:
 
     def test_computed_fields_normal_use(self, mocker, inventory):
         job = Job.objects.create(name='fake-job', inventory=inventory)
-        with mocker.patch.object(update_inventory_computed_fields, 'delay'):
+        with immediate_on_commit():
-            job.delete()
+            with mocker.patch.object(update_inventory_computed_fields, 'delay'):
-            update_inventory_computed_fields.delay.assert_called_once_with(inventory.id)
+                job.delete()
+                update_inventory_computed_fields.delay.assert_called_once_with(inventory.id)
 
     def test_disable_computed_fields(self, mocker, inventory):
         job = Job.objects.create(name='fake-job', inventory=inventory)

awx/main/tests/functional/models/test_unified_job.py

@@ -283,13 +283,13 @@ class TestTaskImpact:
 
     def test_limit_task_impact(self, job_host_limit, run_computed_fields_right_away):
         job = job_host_limit(5, 2)
-        job.inventory.refresh_from_db()  # FIXME: computed fields operates on reloaded inventory
+        job.inventory.update_computed_fields()
         assert job.inventory.total_hosts == 5
         assert job.task_impact == 2 + 1  # forks becomes constraint
 
     def test_host_task_impact(self, job_host_limit, run_computed_fields_right_away):
         job = job_host_limit(3, 5)
-        job.inventory.refresh_from_db()  # FIXME: computed fields operates on reloaded inventory
+        job.inventory.update_computed_fields()
         assert job.task_impact == 3 + 1  # hosts becomes constraint
 
     def test_shard_task_impact(self, slice_job_factory, run_computed_fields_right_away):
@@ -304,6 +304,7 @@ class TestTaskImpact:
             len(jobs[0].inventory.get_script_data(slice_number=i + 1, slice_count=3)['all']['hosts'])
             for i in range(3)
         ] == [1, 1, 1]
+        jobs[0].inventory.update_computed_fields()
         assert [job.task_impact for job in jobs] == [2, 2, 2]  # plus one base task impact
         # Uneven distribution - first job takes the extra host
         jobs[0].inventory.hosts.create(name='remainder_foo')
@@ -311,5 +312,5 @@ class TestTaskImpact:
             len(jobs[0].inventory.get_script_data(slice_number=i + 1, slice_count=3)['all']['hosts'])
             for i in range(3)
         ] == [2, 1, 1]
-        jobs[0].inventory.refresh_from_db()  # FIXME: computed fields operates on reloaded inventory
+        jobs[0].inventory.update_computed_fields()
         assert [job.task_impact for job in jobs] == [3, 2, 2]