From ab0463bf2a8bbdd456b14d76979d23701ce1967f Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Fri, 3 Mar 2023 08:28:56 -0800 Subject: [PATCH] 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 Co-authored-by: Alan Rominger --- awx/main/fields.py | 23 ++++++---- .../migrations/0182_constructed_inventory.py | 18 +++++++- awx/main/models/__init__.py | 1 + awx/main/models/inventory.py | 14 ++++++- .../test_instance_group_ordering.py | 42 ++++++++++++++++++- awx/main/tests/functional/test_instances.py | 3 +- 6 files changed, 90 insertions(+), 11 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 5d3710ed51..ce548fb58c 100644 --- a/awx/main/fields.py +++ b/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): - if kwargs.get('action') in ('post_add', 'post_remove'): - order_with_respect_to = None - for field in sender._meta.local_fields: - 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: kwargs['instance'].pk})): + def _update_m2m_position(self, sender, instance, action, **kwargs): + if action in ('post_add', 'post_remove'): + descriptor = getattr(instance, self.name) + order_with_respect_to = descriptor.source_field_name + + for i, ig in enumerate(sender.objects.filter(**{order_with_respect_to: instance.pk})): if ig.position != i: ig.position = i ig.save() diff --git a/awx/main/migrations/0182_constructed_inventory.py b/awx/main/migrations/0182_constructed_inventory.py index a41e303597..10ece7b2f4 100644 --- a/awx/main/migrations/0182_constructed_inventory.py +++ b/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', ), ), diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index 8a608aeead..19a422740c 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -18,6 +18,7 @@ from awx.main.models.inventory import ( # noqa HostMetric, HostMetricSummaryMonthly, Inventory, + InventoryConstructedInventoryMembership, InventorySource, InventoryUpdate, SmartInventoryMembership, diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 75609b91b6..978e520278 100644 --- a/awx/main/models/inventory.py +++ b/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', diff --git a/awx/main/tests/functional/test_instance_group_ordering.py b/awx/main/tests/functional/test_instance_group_ordering.py index 42c69ffc7f..fb8c0db168 100644 --- a/awx/main/tests/functional/test_instance_group_ordering.py +++ b/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 diff --git a/awx/main/tests/functional/test_instances.py b/awx/main/tests/functional/test_instances.py index 9db370225c..4ed652179d 100644 --- a/awx/main/tests/functional/test_instances.py +++ b/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]