diff --git a/awx/main/fields.py b/awx/main/fields.py index bc524b01c6..ecd2211711 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -11,6 +11,7 @@ from jinja2 import Environment, StrictUndefined from jinja2.exceptions import UndefinedError, TemplateSyntaxError # Django +import django from django.core import exceptions as django_exceptions from django.db.models.signals import ( post_save, @@ -24,8 +25,10 @@ from django.db.models.fields.related_descriptors import ( ForwardManyToOneDescriptor, ManyToManyDescriptor, ReverseManyToOneDescriptor, + create_forward_many_to_many_manager ) from django.utils.encoding import smart_text +from django.utils.functional import cached_property from django.utils.translation import ugettext_lazy as _ # jsonschema @@ -52,8 +55,8 @@ from awx.main import utils __all__ = ['AutoOneToOneField', 'ImplicitRoleField', 'JSONField', - 'SmartFilterField', 'update_role_parentage_for_instance', - 'is_implicit_parent'] + 'SmartFilterField', 'OrderedManyToManyField', + 'update_role_parentage_for_instance', 'is_implicit_parent'] # Provide a (better) custom error message for enum jsonschema validation @@ -987,3 +990,115 @@ class OAuth2ClientSecretField(models.CharField): if value and value.startswith('$encrypted$'): return decrypt_value(get_encryption_key('value', pk=None), value) return value + + +class OrderedManyToManyDescriptor(ManyToManyDescriptor): + """ + Django doesn't seem to support: + + class Meta: + ordering = [...] + + ...on custom through= relations for ManyToMany fields. + + Meaning, queries made _through_ the intermediary table will _not_ apply an + ORDER_BY clause based on the `Meta.ordering` of the intermediary M2M class + (which is the behavior we want for "ordered" many to many relations): + + https://github.com/django/django/blob/stable/1.11.x/django/db/models/fields/related_descriptors.py#L593 + + This descriptor automatically sorts all queries through this relation + using the `position` column on the M2M table. + """ + + @cached_property + def related_manager_cls(self): + model = self.rel.related_model if self.reverse else self.rel.model + + def add_custom_queryset_to_many_related_manager(many_related_manage_cls): + class OrderedManyRelatedManager(many_related_manage_cls): + def get_queryset(self): + return super(OrderedManyRelatedManager, self).get_queryset().order_by( + '%s__position' % self.through._meta.model_name + ) + + def add(self, *objs): + # Django < 2 doesn't support this method on + # ManyToManyFields w/ an intermediary model + # We should be able to remove this code snippet when we + # upgrade Django. + # see: https://github.com/django/django/blob/stable/1.11.x/django/db/models/fields/related_descriptors.py#L926 + if not django.__version__.startswith('1.'): + raise RuntimeError( + 'This method is no longer necessary in Django>=2' + ) + try: + self.through._meta.auto_created = True + super(OrderedManyRelatedManager, self).add(*objs) + finally: + self.through._meta.auto_created = False + + def remove(self, *objs): + # Django < 2 doesn't support this method on + # ManyToManyFields w/ an intermediary model + # We should be able to remove this code snippet when we + # upgrade Django. + # see: https://github.com/django/django/blob/stable/1.11.x/django/db/models/fields/related_descriptors.py#L944 + if not django.__version__.startswith('1.'): + raise RuntimeError( + 'This method is no longer necessary in Django>=2' + ) + try: + self.through._meta.auto_created = True + super(OrderedManyRelatedManager, self).remove(*objs) + finally: + self.through._meta.auto_created = False + + return OrderedManyRelatedManager + + return add_custom_queryset_to_many_related_manager( + create_forward_many_to_many_manager( + model._default_manager.__class__, + self.rel, + reverse=self.reverse, + ) + ) + + +class OrderedManyToManyField(models.ManyToManyField): + """ + A ManyToManyField that automatically sorts all querysets + 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} + )): + if ig.position != i: + ig.position = i + ig.save() + + def contribute_to_class(self, cls, name, **kwargs): + super(OrderedManyToManyField, self).contribute_to_class(cls, name, **kwargs) + setattr( + cls, name, + OrderedManyToManyDescriptor(self.remote_field, reverse=False) + ) + + through = getattr(cls, name).through + if isinstance(through, str) and "." not in through: + # support lazy loading of string model names + through = '.'.join([cls._meta.app_label, through]) + m2m_changed.connect( + self._update_m2m_position, + sender=through + ) diff --git a/awx/main/migrations/0073_v360_create_instance_group_m2m.py b/awx/main/migrations/0073_v360_create_instance_group_m2m.py new file mode 100644 index 0000000000..93283634a8 --- /dev/null +++ b/awx/main/migrations/0073_v360_create_instance_group_m2m.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-05-06 13:48 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0072_v350_deprecate_fields'), + ] + + operations = [ + migrations.CreateModel( + name='InventoryInstanceGroupMembership', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('position', models.PositiveIntegerField(default=None, null=True, db_index=True)), + ('instancegroup', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.InstanceGroup')), + ('inventory', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.Inventory')), + ], + ), + migrations.CreateModel( + name='OrganizationInstanceGroupMembership', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('position', models.PositiveIntegerField(default=None, null=True, db_index=True)), + ('instancegroup', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.InstanceGroup')), + ('organization', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.Organization')), + ], + ), + migrations.CreateModel( + name='UnifiedJobTemplateInstanceGroupMembership', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('position', models.PositiveIntegerField(default=None, null=True, db_index=True)), + ('instancegroup', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.InstanceGroup')), + ('unifiedjobtemplate', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.UnifiedJobTemplate')), + ], + ), + ] diff --git a/awx/main/migrations/0074_v360_migrate_instance_group_relations.py b/awx/main/migrations/0074_v360_migrate_instance_group_relations.py new file mode 100644 index 0000000000..aeaf0c7fe6 --- /dev/null +++ b/awx/main/migrations/0074_v360_migrate_instance_group_relations.py @@ -0,0 +1,42 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-05-06 13:49 +from __future__ import unicode_literals + +from django.db import migrations + + +def create_through_relations(apps, schema_editor): + relations = [ + [ + 'UnifiedJobTemplate', + apps.get_model('main', 'UnifiedJobTemplateInstanceGroupMembership'), + ], + [ + 'Organization', + apps.get_model('main', 'OrganizationInstanceGroupMembership'), + ], + [ + 'Inventory', + apps.get_model('main', 'InventoryInstanceGroupMembership'), + ], + ] + for cls, Membership in relations: + Target = apps.get_model('main', cls) + for x in Target.objects.iterator(): + for i, instance_group in enumerate(x.instance_groups.all()): + Membership( + instancegroup=instance_group, + position=i, + **{'%s' % cls.lower(): x} + ).save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0073_v360_create_instance_group_m2m'), + ] + + operations = [ + migrations.RunPython(create_through_relations) + ] diff --git a/awx/main/migrations/0075_v360_remove_old_instance_group_relations.py b/awx/main/migrations/0075_v360_remove_old_instance_group_relations.py new file mode 100644 index 0000000000..f684df3d8a --- /dev/null +++ b/awx/main/migrations/0075_v360_remove_old_instance_group_relations.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-05-06 15:15 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0074_v360_migrate_instance_group_relations'), + ] + + operations = [ + migrations.RemoveField( + model_name='inventory', + name='instance_groups', + ), + migrations.RemoveField( + model_name='organization', + name='instance_groups', + ), + migrations.RemoveField( + model_name='unifiedjobtemplate', + name='instance_groups', + ), + ] diff --git a/awx/main/migrations/0076_v360_add_new_instance_group_relations.py b/awx/main/migrations/0076_v360_add_new_instance_group_relations.py new file mode 100644 index 0000000000..c1dc29f79c --- /dev/null +++ b/awx/main/migrations/0076_v360_add_new_instance_group_relations.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-05-06 15:20 +from __future__ import unicode_literals + +from django.db import migrations, models +from awx.main.fields import OrderedManyToManyField + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0075_v360_remove_old_instance_group_relations'), + ] + + operations = [ + migrations.AddField( + model_name='inventory', + name='instance_groups', + field=OrderedManyToManyField(blank=True, through='main.InventoryInstanceGroupMembership', to='main.InstanceGroup'), + ), + migrations.AddField( + model_name='organization', + name='instance_groups', + field=OrderedManyToManyField(blank=True, through='main.OrganizationInstanceGroupMembership', to='main.InstanceGroup'), + ), + migrations.AddField( + model_name='unifiedjobtemplate', + name='instance_groups', + field=OrderedManyToManyField(blank=True, through='main.UnifiedJobTemplateInstanceGroupMembership', to='main.InstanceGroup'), + ), + ] diff --git a/awx/main/models/ha.py b/awx/main/models/ha.py index fbd52f2a1e..e5437f42b2 100644 --- a/awx/main/models/ha.py +++ b/awx/main/models/ha.py @@ -332,3 +332,54 @@ def on_job_create(sender, instance, created=False, raw=False, **kwargs): instance=Instance.objects.me(), unified_job=instance, ) + + +class UnifiedJobTemplateInstanceGroupMembership(models.Model): + + unifiedjobtemplate = models.ForeignKey( + 'UnifiedJobTemplate', + on_delete=models.CASCADE + ) + instancegroup = models.ForeignKey( + 'InstanceGroup', + on_delete=models.CASCADE + ) + position = models.PositiveIntegerField( + null=True, + default=None, + db_index=True, + ) + + +class OrganizationInstanceGroupMembership(models.Model): + + organization = models.ForeignKey( + 'Organization', + on_delete=models.CASCADE + ) + instancegroup = models.ForeignKey( + 'InstanceGroup', + on_delete=models.CASCADE + ) + position = models.PositiveIntegerField( + null=True, + default=None, + db_index=True, + ) + + +class InventoryInstanceGroupMembership(models.Model): + + inventory = models.ForeignKey( + 'Inventory', + on_delete=models.CASCADE + ) + instancegroup = models.ForeignKey( + 'InstanceGroup', + on_delete=models.CASCADE + ) + position = models.PositiveIntegerField( + null=True, + default=None, + db_index=True, + ) diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 82b1455fa2..a4ec912a00 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -38,6 +38,7 @@ from awx.main.fields import ( ImplicitRoleField, JSONBField, SmartFilterField, + OrderedManyToManyField, ) from awx.main.managers import HostManager from awx.main.models.base import ( @@ -157,9 +158,10 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin): default=None, help_text=_('Filter that will be applied to the hosts of this inventory.'), ) - instance_groups = models.ManyToManyField( + instance_groups = OrderedManyToManyField( 'InstanceGroup', blank=True, + through='InventoryInstanceGroupMembership', ) admin_role = ImplicitRoleField( parent_role='organization.inventory_admin_role', diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 54e8d7ad08..220dae0781 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -15,7 +15,9 @@ from django.utils.translation import ugettext_lazy as _ # AWX from awx.api.versioning import reverse -from awx.main.fields import AutoOneToOneField, ImplicitRoleField +from awx.main.fields import ( + AutoOneToOneField, ImplicitRoleField, OrderedManyToManyField +) from awx.main.models.base import ( BaseModel, CommonModel, CommonModelNameNotUnique, CreatedModifiedModel, NotificationFieldsModel @@ -39,9 +41,10 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi app_label = 'main' ordering = ('name',) - instance_groups = models.ManyToManyField( + instance_groups = OrderedManyToManyField( 'InstanceGroup', blank=True, + through='OrganizationInstanceGroupMembership' ) max_hosts = models.PositiveIntegerField( blank=True, diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 1b341a5476..ed63cb6dde 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -48,7 +48,7 @@ from awx.main.utils import polymorphic, schedule_task_manager from awx.main.constants import ACTIVE_STATES, CAN_CANCEL from awx.main.redact import UriCleaner, REPLACE_STR from awx.main.consumers import emit_channel_notification -from awx.main.fields import JSONField, AskForField +from awx.main.fields import JSONField, AskForField, OrderedManyToManyField __all__ = ['UnifiedJobTemplate', 'UnifiedJob', 'StdoutMaxBytesExceeded'] @@ -164,9 +164,10 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio blank=True, related_name='%(class)s_labels' ) - instance_groups = models.ManyToManyField( + instance_groups = OrderedManyToManyField( 'InstanceGroup', blank=True, + through='UnifiedJobTemplateInstanceGroupMembership' ) def get_absolute_url(self, request=None): diff --git a/awx/main/tests/functional/test_instance_group_ordering.py b/awx/main/tests/functional/test_instance_group_ordering.py new file mode 100644 index 0000000000..aaed0779eb --- /dev/null +++ b/awx/main/tests/functional/test_instance_group_ordering.py @@ -0,0 +1,102 @@ +import pytest + +from awx.main.models import InstanceGroup + + +@pytest.fixture(scope='function') +def source_model(request): + return request.getfixturevalue(request.param) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + 'source_model', ['job_template', 'inventory', 'organization'], indirect=True +) +def test_instance_group_ordering(source_model): + groups = [ + InstanceGroup.objects.create(name='host-%d' % i) + for i in range(5) + ] + groups.reverse() + for group in groups: + source_model.instance_groups.add(group) + + assert [g.name for g in source_model.instance_groups.all()] == [ + 'host-4', 'host-3', 'host-2', 'host-1', 'host-0' + ] + assert [ + (row.position, row.instancegroup.name) + for row in source_model.instance_groups.through.objects.all() + ] == [ + (0, 'host-4'), + (1, 'host-3'), + (2, 'host-2'), + (3, 'host-1'), + (4, 'host-0'), + ] + + source_model.instance_groups.remove(groups[0]) + assert [g.name for g in source_model.instance_groups.all()] == [ + 'host-3', 'host-2', 'host-1', 'host-0' + ] + assert [ + (row.position, row.instancegroup.name) + for row in source_model.instance_groups.through.objects.all() + ] == [ + (0, 'host-3'), + (1, 'host-2'), + (2, 'host-1'), + (3, 'host-0'), + ] + + source_model.instance_groups.clear() + 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_middle_deletion(source_model): + groups = [ + InstanceGroup.objects.create(name='host-%d' % i) + for i in range(5) + ] + groups.reverse() + for group in groups: + source_model.instance_groups.add(group) + + source_model.instance_groups.remove(groups[2]) + assert [g.name for g in source_model.instance_groups.all()] == [ + 'host-4', 'host-3', 'host-1', 'host-0' + ] + assert [ + (row.position, row.instancegroup.name) + for row in source_model.instance_groups.through.objects.all() + ] == [ + (0, 'host-4'), + (1, 'host-3'), + (2, 'host-1'), + (3, 'host-0'), + ] + + +@pytest.mark.django_db +@pytest.mark.parametrize( + 'source_model', ['job_template', 'inventory', 'organization'], indirect=True +) +def test_explicit_ordering(source_model): + groups = [ + InstanceGroup.objects.create(name='host-%d' % i) + for i in range(5) + ] + groups.reverse() + for group in groups: + source_model.instance_groups.add(group) + + 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' + ]