From bd7d9db1ceaf647fa0cd2484bedd509234438e9b Mon Sep 17 00:00:00 2001 From: chris meyers Date: Thu, 5 Apr 2018 17:23:09 -0400 Subject: [PATCH] correctly cascade set null * It's problematic to delete an instance that is referenced by a foreign key; where the referening model is one that has a Polymorphic parent. * Specifically, when Django goes to nullify the relationship it relies on the related instances[0] class type to issue a query to decide what to nullify. So if the foreignkey references multiple different types (i.e. ProjectUpdate, Job) then only 1 of those class types will get nullified. The end result is an IntegrityError when delete() is called. * This changeset ensures that the parent Polymorphic class is queried so that all the foreignkey entries are nullified * Also remove old Django "hack" that doesn't work with Django 1.11 --- .../0030_v330_polymorphic_delete.py | 21 +++++++++++++++++++ awx/main/models/unified_jobs.py | 9 ++------ awx/main/utils/polymorphic.py | 5 +++++ 3 files changed, 28 insertions(+), 7 deletions(-) create mode 100644 awx/main/migrations/0030_v330_polymorphic_delete.py diff --git a/awx/main/migrations/0030_v330_polymorphic_delete.py b/awx/main/migrations/0030_v330_polymorphic_delete.py new file mode 100644 index 0000000000..ad041718ee --- /dev/null +++ b/awx/main/migrations/0030_v330_polymorphic_delete.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.11 on 2018-04-06 13:44 +from __future__ import unicode_literals + +import awx.main.utils.polymorphic +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0029_v330_encrypt_oauth2_secret'), + ] + + operations = [ + migrations.AlterField( + model_name='unifiedjob', + name='instance_group', + field=models.ForeignKey(blank=True, default=None, help_text='The Rampart/Instance group the job was run under', null=True, on_delete=awx.main.utils.polymorphic.SET_NULL, to='main.InstanceGroup'), + ), + ] diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 08e5cecb1c..540d5ee2e7 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -38,6 +38,7 @@ from awx.main.utils import ( copy_model_by_class, copy_m2m_relationships, get_type_for_model, parse_yaml_or_json ) +from awx.main.utils import polymorphic 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 @@ -89,9 +90,6 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio ALL_STATUS_CHOICES = OrderedDict(PROJECT_STATUS_CHOICES + INVENTORY_SOURCE_STATUS_CHOICES + JOB_TEMPLATE_STATUS_CHOICES + DEPRECATED_STATUS_CHOICES).items() - # NOTE: Working around a django-polymorphic issue: https://github.com/django-polymorphic/django-polymorphic/issues/229 - base_manager_name = 'base_objects' - class Meta: app_label = 'main' # unique_together here is intentionally commented out. Please make sure sub-classes of this model @@ -536,9 +534,6 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique PASSWORD_FIELDS = ('start_args',) - # NOTE: Working around a django-polymorphic issue: https://github.com/django-polymorphic/django-polymorphic/issues/229 - base_manager_name = 'base_objects' - class Meta: app_label = 'main' @@ -669,7 +664,7 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique blank=True, null=True, default=None, - on_delete=models.SET_NULL, + on_delete=polymorphic.SET_NULL, help_text=_('The Rampart/Instance group the job was run under'), ) credentials = models.ManyToManyField( diff --git a/awx/main/utils/polymorphic.py b/awx/main/utils/polymorphic.py index 4eabba213e..f8ed1bc160 100644 --- a/awx/main/utils/polymorphic.py +++ b/awx/main/utils/polymorphic.py @@ -1,5 +1,6 @@ from django.contrib.contenttypes.models import ContentType +from django.db import models def build_polymorphic_ctypes_map(cls): @@ -10,3 +11,7 @@ def build_polymorphic_ctypes_map(cls): if ct_model_class and issubclass(ct_model_class, cls): mapping[ct.id] = ct_model_class._camel_to_underscore(ct_model_class.__name__) return mapping + + +def SET_NULL(collector, field, sub_objs, using): + return models.SET_NULL(collector, field, sub_objs.non_polymorphic(), using)