Ordered m2m for Inventory/Inventory relationship (#13602)

Including changes to our custom Ordered m2m field which previously broke
if the source and target model was the same.

Signed-off-by: Rick Elrod <rick@elrod.me>
Co-authored-by: Alan Rominger <arominge@redhat.com>

awx/main/fields.py

@@ -954,6 +954,16 @@ class OrderedManyToManyDescriptor(ManyToManyDescriptor):
                 def get_queryset(self):
                     return super(OrderedManyRelatedManager, self).get_queryset().order_by('%s__position' % self.through._meta.model_name)
 
+                def add(self, *objects):
+                    if len(objects) > 1:
+                        raise RuntimeError('Ordered many-to-many fields do not support multiple objects')
+                    return super().add(*objects)
+
+                def remove(self, *objects):
+                    if len(objects) > 1:
+                        raise RuntimeError('Ordered many-to-many fields do not support multiple objects')
+                    return super().remove(*objects)
+
             return OrderedManyRelatedManager
 
         return add_custom_queryset_to_many_related_manager(
@@ -971,13 +981,12 @@ class OrderedManyToManyField(models.ManyToManyField):
     by a special `position` column on the M2M table
     """
 
-    def _update_m2m_position(self, sender, **kwargs):
+    def _update_m2m_position(self, sender, instance, action, **kwargs):
-        if kwargs.get('action') in ('post_add', 'post_remove'):
+        if action in ('post_add', 'post_remove'):
-            order_with_respect_to = None
+            descriptor = getattr(instance, self.name)
-            for field in sender._meta.local_fields:
+            order_with_respect_to = descriptor.source_field_name
-                if isinstance(field, models.ForeignKey) and isinstance(kwargs['instance'], field.related_model):
+
-                    order_with_respect_to = field.name
+            for i, ig in enumerate(sender.objects.filter(**{order_with_respect_to: instance.pk})):
-            for i, ig in enumerate(sender.objects.filter(**{order_with_respect_to: kwargs['instance'].pk})):
                 if ig.position != i:
                     ig.position = i
                     ig.save()

awx/main/migrations/0182_constructed_inventory.py

@@ -1,6 +1,8 @@
 # Generated by Django 3.2.16 on 2022-12-07 14:20
 
+import awx.main.fields
 from django.db import migrations, models
+import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
@@ -9,13 +11,27 @@ class Migration(migrations.Migration):
     ]
 
     operations = [
+        migrations.CreateModel(
+            name='InventoryConstructedInventoryMembership',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('position', models.PositiveIntegerField(db_index=True, default=None, null=True)),
+                (
+                    'constructed_inventory',
+                    models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.inventory', related_name='constructed_inventory_memberships'),
+                ),
+                ('input_inventory', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.inventory')),
+            ],
+        ),
         migrations.AddField(
             model_name='inventory',
             name='input_inventories',
-            field=models.ManyToManyField(
+            field=awx.main.fields.OrderedManyToManyField(
                 blank=True,
+                through_fields=('constructed_inventory', 'input_inventory'),
                 help_text='Only valid for constructed inventories, this links to the inventories that will be used.',
                 related_name='destination_inventories',
+                through='main.InventoryConstructedInventoryMembership',
                 to='main.Inventory',
             ),
         ),

awx/main/models/__init__.py

@@ -18,6 +18,7 @@ from awx.main.models.inventory import (  # noqa
     HostMetric,
     HostMetricSummaryMonthly,
     Inventory,
+    InventoryConstructedInventoryMembership,
     InventorySource,
     InventoryUpdate,
     SmartInventoryMembership,

awx/main/models/inventory.py

@@ -58,6 +58,16 @@ __all__ = ['Inventory', 'Host', 'Group', 'InventorySource', 'InventoryUpdate', '
 logger = logging.getLogger('awx.main.models.inventory')
 
 
+class InventoryConstructedInventoryMembership(models.Model):
+    constructed_inventory = models.ForeignKey('Inventory', on_delete=models.CASCADE, related_name='constructed_inventory_memberships')
+    input_inventory = models.ForeignKey('Inventory', on_delete=models.CASCADE)
+    position = models.PositiveIntegerField(
+        null=True,
+        default=None,
+        db_index=True,
+    )
+
+
 class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin):
     """
     an inventory source contains lists and hosts.
@@ -140,11 +150,13 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin):
         default=None,
         help_text=_('Filter that will be applied to the hosts of this inventory.'),
     )
-    input_inventories = models.ManyToManyField(
+    input_inventories = OrderedManyToManyField(
         'Inventory',
         blank=True,
+        through_fields=('constructed_inventory', 'input_inventory'),
         related_name='destination_inventories',
         help_text=_('Only valid for constructed inventories, this links to the inventories that will be used.'),
+        through='InventoryConstructedInventoryMembership',
     )
     instance_groups = OrderedManyToManyField(
         'InstanceGroup',

awx/main/tests/functional/test_instance_group_ordering.py

@@ -1,6 +1,6 @@
 import pytest
 
-from awx.main.models import InstanceGroup
+from awx.main.models import InstanceGroup, Inventory
 
 
 @pytest.fixture(scope='function')
@@ -38,6 +38,16 @@ def test_instance_group_ordering(source_model):
     assert source_model.instance_groups.through.objects.count() == 0
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize('source_model', ['job_template', 'inventory', 'organization'], indirect=True)
+def test_instance_group_bulk_add(source_model):
+    groups = [InstanceGroup.objects.create(name='host-%d' % i) for i in range(5)]
+    groups.reverse()
+    with pytest.raises(RuntimeError) as err:
+        source_model.instance_groups.add(*groups)
+    assert 'Ordered many-to-many fields do not support multiple objects' in str(err)
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize('source_model', ['job_template', 'inventory', 'organization'], indirect=True)
 def test_instance_group_middle_deletion(source_model):
@@ -66,3 +76,33 @@ def test_explicit_ordering(source_model):
 
     assert [g.name for g in source_model.instance_groups.all()] == ['host-4', 'host-3', 'host-2', 'host-1', 'host-0']
     assert [g.name for g in source_model.instance_groups.order_by('name').all()] == ['host-0', 'host-1', 'host-2', 'host-3', 'host-4']
+
+
+@pytest.mark.django_db
+def test_input_inventories_ordering():
+    constructed_inventory = Inventory.objects.create(name='my_constructed', kind='constructed')
+    input_inventories = [Inventory.objects.create(name='inv-%d' % i) for i in range(5)]
+    input_inventories.reverse()
+    for inv in input_inventories:
+        constructed_inventory.input_inventories.add(inv)
+
+    assert [g.name for g in constructed_inventory.input_inventories.all()] == ['inv-4', 'inv-3', 'inv-2', 'inv-1', 'inv-0']
+    assert [(row.position, row.input_inventory.name) for row in constructed_inventory.input_inventories.through.objects.all()] == [
+        (0, 'inv-4'),
+        (1, 'inv-3'),
+        (2, 'inv-2'),
+        (3, 'inv-1'),
+        (4, 'inv-0'),
+    ]
+
+    constructed_inventory.input_inventories.remove(input_inventories[0])
+    assert [g.name for g in constructed_inventory.input_inventories.all()] == ['inv-3', 'inv-2', 'inv-1', 'inv-0']
+    assert [(row.position, row.input_inventory.name) for row in constructed_inventory.input_inventories.through.objects.all()] == [
+        (0, 'inv-3'),
+        (1, 'inv-2'),
+        (2, 'inv-1'),
+        (3, 'inv-0'),
+    ]
+
+    constructed_inventory.input_inventories.clear()
+    assert constructed_inventory.input_inventories.through.objects.count() == 0

awx/main/tests/functional/test_instances.py

@@ -94,7 +94,8 @@ def test_instance_dup(org_admin, organization, project, instance_factory, instan
 
     ig_all = instance_group_factory("all", instances=[i1, i2, i3])
     ig_dup = instance_group_factory("duplicates", instances=[i1])
-    project.organization.instance_groups.add(ig_all, ig_dup)
+    project.organization.instance_groups.add(ig_all)
+    project.organization.instance_groups.add(ig_dup)
     actual_num_instances = Instance.objects.count()
     list_response = get(reverse('api:instance_list'), user=system_auditor)
     api_num_instances_auditor = list(list_response.data.items())[0][1]