diff --git a/awx/main/migrations/0038_v320_release.py b/awx/main/migrations/0038_v320_release.py index ec9f7a24b7..4ef84465e6 100644 --- a/awx/main/migrations/0038_v320_release.py +++ b/awx/main/migrations/0038_v320_release.py @@ -20,6 +20,12 @@ class Migration(migrations.Migration): ] operations = [ + # Release UJT unique_together constraint + migrations.AlterUniqueTogether( + name='unifiedjobtemplate', + unique_together=set([]), + ), + # Inventory Refresh migrations.RenameField( 'InventorySource', diff --git a/awx/main/models/base.py b/awx/main/models/base.py index d2f374b5a0..ea70070770 100644 --- a/awx/main/models/base.py +++ b/awx/main/models/base.py @@ -10,7 +10,7 @@ import yaml # Django from django.db import models -from django.core.exceptions import ValidationError +from django.core.exceptions import ValidationError, ObjectDoesNotExist from django.utils.translation import ugettext_lazy as _ from django.utils.timezone import now @@ -287,6 +287,27 @@ class PrimordialModel(CreatedModifiedModel): # Description should always be empty string, never null. return self.description or '' + def validate_unique(self, exclude=None): + super(PrimordialModel, self).validate_unique(exclude=exclude) + model = type(self) + if not hasattr(model, 'SOFT_UNIQUE_TOGETHER'): + return + errors = [] + for ut in model.SOFT_UNIQUE_TOGETHER: + kwargs = {} + for field_name in ut: + kwargs[field_name] = getattr(self, field_name, None) + try: + obj = model.objects.get(**kwargs) + except ObjectDoesNotExist: + continue + if not (self.pk and self.pk == obj.pk): + errors.append( + '%s with this (%s) combination already exists.' % (model.__name__, ', '.join(ut)) + ) + if errors: + raise ValidationError(errors) + class CommonModel(PrimordialModel): ''' a base model where the name is unique ''' diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 192b22902f..f88f838087 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -1183,6 +1183,8 @@ class InventorySourceOptions(BaseModel): class InventorySource(UnifiedJobTemplate, InventorySourceOptions): + SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name', 'inventory')] + class Meta: app_label = 'main' diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index f469809647..f4ae545f2e 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -224,6 +224,7 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour A job template is a reusable job definition for applying a project (with playbook) to an inventory source with a given credential. ''' + SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name')] class Meta: app_label = 'main' @@ -1433,4 +1434,3 @@ class SystemJob(UnifiedJob, SystemJobOptions, JobNotificationMixin): def get_notification_friendly_name(self): return "System Job" - diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index ee9f2c09aa..4275661437 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -223,6 +223,8 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin): A project represents a playbook git repo that can access a set of inventories ''' + SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name', 'organization')] + class Meta: app_label = 'main' ordering = ('id',) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 37a97ffa26..c0fbbecc20 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -92,7 +92,9 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio class Meta: app_label = 'main' - unique_together = [('polymorphic_ctype', 'name')] + # unique_together here is intentionally commented out. Please make sure sub-classes of this model + # contain at least this uniqueness restriction: SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name')] + #unique_together = [('polymorphic_ctype', 'name')] old_pk = models.PositiveIntegerField( null=True, diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index 52d58717a0..5b4e8c2869 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -328,6 +328,8 @@ class WorkflowJobOptions(BaseModel): class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTemplateMixin, ResourceMixin): + SOFT_UNIQUE_TOGETHER = [('polymorphic_ctype', 'name', 'organization')] + class Meta: app_label = 'main' diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index 831c7afc2d..c74d751ea2 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -1,6 +1,8 @@ import pytest import mock +from django.core.exceptions import ValidationError + from awx.api.versioning import reverse from awx.main.models import InventorySource, Project, ProjectUpdate @@ -34,6 +36,19 @@ def test_inventory_source_notification_on_cloud_only(get, post, inventory_source assert response.status_code == 400 +@pytest.mark.django_db +def test_inventory_source_unique_together_with_inv(inventory_factory): + inv1 = inventory_factory('foo') + inv2 = inventory_factory('bar') + is1 = InventorySource(name='foo', source='file', inventory=inv1) + is1.save() + is2 = InventorySource(name='foo', source='file', inventory=inv1) + with pytest.raises(ValidationError): + is2.validate_unique() + is2 = InventorySource(name='foo', source='file', inventory=inv2) + is2.validate_unique() + + @pytest.mark.parametrize("role_field,expected_status_code", [ (None, 403), ('admin_role', 200), @@ -347,4 +362,3 @@ class TestInsightsCredential: patch(insights_inventory.get_absolute_url(), {'insights_credential': scm_credential.id}, admin_user, expect=400) - diff --git a/awx/main/tests/functional/models/test_workflow.py b/awx/main/tests/functional/models/test_workflow.py index 0cb278826f..5cae3008e0 100644 --- a/awx/main/tests/functional/models/test_workflow.py +++ b/awx/main/tests/functional/models/test_workflow.py @@ -3,13 +3,14 @@ import pytest # AWX -from awx.main.models.workflow import WorkflowJob, WorkflowJobNode, WorkflowJobTemplateNode +from awx.main.models.workflow import WorkflowJob, WorkflowJobNode, WorkflowJobTemplateNode, WorkflowJobTemplate from awx.main.models.jobs import Job from awx.main.models.projects import ProjectUpdate from awx.main.scheduler.dag_workflow import WorkflowDAG # Django from django.test import TransactionTestCase +from django.core.exceptions import ValidationError @pytest.mark.django_db @@ -155,6 +156,15 @@ class TestWorkflowJobTemplate: assert nodes[1].unified_job_template == job_template assert nodes[2].inventory == inventory + def test_wfjt_unique_together_with_org(self, organization): + wfjt1 = WorkflowJobTemplate(name='foo', organization=organization) + wfjt1.save() + wfjt2 = WorkflowJobTemplate(name='foo', organization=organization) + with pytest.raises(ValidationError): + wfjt2.validate_unique() + wfjt2 = WorkflowJobTemplate(name='foo', organization=None) + wfjt2.validate_unique() + @pytest.mark.django_db class TestWorkflowJobFailure: diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py index e8b86502f5..69c5fdcf65 100644 --- a/awx/main/tests/functional/test_projects.py +++ b/awx/main/tests/functional/test_projects.py @@ -6,6 +6,8 @@ import pytest from awx.api.versioning import reverse from awx.main.models import Project +from django.core.exceptions import ValidationError + # # Project listing and visibility tests @@ -238,3 +240,14 @@ def test_cannot_schedule_manual_project(project, admin_user, post): }, admin_user, expect=400 ) assert 'Manual' in response.data['unified_job_template'][0] + + +@pytest.mark.django_db +def test_project_unique_together_with_org(organization): + proj1 = Project(name='foo', organization=organization) + proj1.save() + proj2 = Project(name='foo', organization=organization) + with pytest.raises(ValidationError): + proj2.validate_unique() + proj2 = Project(name='foo', organization=None) + proj2.validate_unique() diff --git a/awx/main/tests/unit/utils/test_named_url_graph.py b/awx/main/tests/unit/utils/test_named_url_graph.py index d6b0917fd9..2ca9d9badc 100644 --- a/awx/main/tests/unit/utils/test_named_url_graph.py +++ b/awx/main/tests/unit/utils/test_named_url_graph.py @@ -24,9 +24,11 @@ def common_model_class_mock(): @pytest.fixture def common_model_name_not_unique_class_mock(): - def class_generator(ut, fk_a_obj, fk_b_obj, plural): + def class_generator(ut, fk_a_obj, fk_b_obj, plural, soft_ut=[]): class ModelClass(CommonModelNameNotUnique): + SOFT_UNIQUE_TOGETHER = soft_ut + class Meta: unique_together = ut verbose_name_plural = plural @@ -92,6 +94,33 @@ def test_invalid_generation(common_model_name_not_unique_class_mock, assert not settings_mock.NAMED_URL_FORMATS +def test_soft_unique_together_being_included(common_model_name_not_unique_class_mock, + common_model_class_mock, settings_mock): + models = [] + model_1 = common_model_class_mock('model_1') + models.append(model_1) + model_2 = common_model_name_not_unique_class_mock( + (), + model_1, + model_1, + 'model_2', + soft_ut=[('name', 'fk_a')] + ) + models.append(model_2) + + random.shuffle(models) + with mock.patch('awx.main.utils.named_url_graph.settings', settings_mock): + generate_graph(models) + assert settings_mock.NAMED_URL_GRAPH[model_1].model == model_1 + assert settings_mock.NAMED_URL_GRAPH[model_1].fields == ('name',) + assert settings_mock.NAMED_URL_GRAPH[model_1].adj_list == [] + + assert settings_mock.NAMED_URL_GRAPH[model_2].model == model_2 + assert settings_mock.NAMED_URL_GRAPH[model_2].fields == ('name',) + assert zip(*settings_mock.NAMED_URL_GRAPH[model_2].adj_list)[0] == ('fk_a',) + assert [x.model for x in zip(*settings_mock.NAMED_URL_GRAPH[model_2].adj_list)[1]] == [model_1] + + def test_chain_generation(common_model_class_mock, common_model_name_not_unique_class_mock, settings_mock): """ Graph topology: diff --git a/awx/main/utils/named_url_graph.py b/awx/main/utils/named_url_graph.py index 9df37194c8..1f813d8269 100644 --- a/awx/main/utils/named_url_graph.py +++ b/awx/main/utils/named_url_graph.py @@ -187,6 +187,8 @@ def _get_all_unique_togethers(model): ret.append(uts) else: ret.extend(uts) + soft_uts = getattr(model_to_backtrack, 'SOFT_UNIQUE_TOGETHER', []) + ret.extend(soft_uts) for parent_class in model_to_backtrack.__bases__: if issubclass(parent_class, models.Model) and\ hasattr(parent_class, '_meta') and\