Merge pull request #3900 from AlanCoding/fewer_type_methods

Remove duplicated type methods and old Django logic

Reviewed-by: https://github.com/softwarefactory-project-zuul[bot]

awx/main/models/activity_stream.py

@@ -97,12 +97,4 @@ class ActivityStream(models.Model):
             if 'update_fields' in kwargs and 'deleted_actor' not in kwargs['update_fields']:
                 kwargs['update_fields'].append('deleted_actor')
 
-        # For compatibility with Django 1.4.x, attempt to handle any calls to
-        # save that pass update_fields.
-        try:
-            super(ActivityStream, self).save(*args, **kwargs)
-        except TypeError:
-            if 'update_fields' not in kwargs:
-                raise
-            kwargs.pop('update_fields')
         super(ActivityStream, self).save(*args, **kwargs)

awx/main/models/unified_jobs.py

@@ -20,7 +20,6 @@ from django.core.exceptions import NON_FIELD_ERRORS
 from django.utils.translation import ugettext_lazy as _
 from django.utils.timezone import now
 from django.utils.encoding import smart_text
-from django.apps import apps
 from django.contrib.contenttypes.models import ContentType
 
 # REST Framework
@@ -40,6 +39,7 @@ from awx.main.dispatch.control import Control as ControlDispatcher
 from awx.main.registrar import activity_stream_registrar
 from awx.main.models.mixins import ResourceMixin, TaskManagerUnifiedJobMixin
 from awx.main.utils import (
+    camelcase_to_underscore, get_model_for_type,
     encrypt_dict, decrypt_field, _inventory_updates,
     copy_model_by_class, copy_m2m_relationships,
     get_type_for_model, parse_yaml_or_json, getattr_dne
@@ -487,34 +487,15 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio
 
 
 class UnifiedJobTypeStringMixin(object):
-    @classmethod
-    def _underscore_to_camel(cls, word):
-        return ''.join(x.capitalize() or '_' for x in word.split('_'))
-
-    @classmethod
-    def _camel_to_underscore(cls, word):
-        return re.sub('(?!^)([A-Z]+)', r'_\1', word).lower()
-
-    @classmethod
-    def _model_type(cls, job_type):
-        # Django >= 1.9
-        #app = apps.get_app_config('main')
-        model_str = cls._underscore_to_camel(job_type)
-        try:
-            return apps.get_model('main', model_str)
-        except LookupError:
-            print("Lookup model error")
-            return None
-
     @classmethod
     def get_instance_by_type(cls, job_type, job_id):
-        model = cls._model_type(job_type)
+        model = get_model_for_type(job_type)
         if not model:
             return None
         return model.objects.get(id=job_id)
 
     def model_to_str(self):
-        return UnifiedJobTypeStringMixin._camel_to_underscore(self.__class__.__name__)
+        return camelcase_to_underscore(self.__class__.__name__)
 
 
 class UnifiedJobDeprecatedStdout(models.Model):

awx/main/tests/functional/utils/test_common.py

@@ -3,14 +3,10 @@ import pytest
 import copy
 import json
 
-from django.db import DatabaseError
-
 from awx.main.utils.common import (
     model_instance_diff,
-    model_to_dict,
-    get_model_for_type
+    model_to_dict
 )
-from awx.main import models
 
 
 @pytest.mark.django_db
@@ -62,20 +58,3 @@ def test_model_instance_diff(alice, bob):
     assert hasattr(alice, 'is_superuser')
     assert hasattr(bob, 'is_superuser')
     assert 'is_superuser' not in output_dict
-
-
-@pytest.mark.django_db
-def test_get_model_for_invalid_type():
-    with pytest.raises(DatabaseError) as exc:
-        get_model_for_type('foobar')
-    assert 'not a valid AWX model' in str(exc)
-
-
-@pytest.mark.django_db
-@pytest.mark.parametrize("model_type,model_class", [
-    ('inventory', models.Inventory),
-    ('job_template', models.JobTemplate),
-    ('unified_job_template', models.UnifiedJobTemplate)
-])
-def test_get_model_for_valid_type(model_type, model_class):
-    assert get_model_for_type(model_type) == model_class

awx/main/tests/unit/utils/test_common.py

@@ -22,7 +22,11 @@ from awx.main.models import (
     InventoryUpdate,
     ProjectUpdate,
     SystemJob,
-    WorkflowJob
+    WorkflowJob,
+    Inventory,
+    JobTemplate,
+    UnifiedJobTemplate,
+    UnifiedJob
 )
 
 
@@ -92,19 +96,40 @@ def test_set_environ():
     assert key not in os.environ
 
 
-# Cases relied on for scheduler dependent jobs list
-@pytest.mark.parametrize('model,name', [
+TEST_MODELS = [
     (Job, 'job'),
     (AdHocCommand, 'ad_hoc_command'),
     (InventoryUpdate, 'inventory_update'),
     (ProjectUpdate, 'project_update'),
     (SystemJob, 'system_job'),
-    (WorkflowJob, 'workflow_job')
-])
+    (WorkflowJob, 'workflow_job'),
+    (UnifiedJob, 'unified_job'),
+    (Inventory, 'inventory'),
+    (JobTemplate, 'job_template'),
+    (UnifiedJobTemplate, 'unified_job_template')
+]
+
+
+# Cases relied on for scheduler dependent jobs list
+@pytest.mark.parametrize('model,name', TEST_MODELS)
 def test_get_type_for_model(model, name):
     assert common.get_type_for_model(model) == name
 
 
+@pytest.mark.django_db
+def test_get_model_for_invalid_type():
+    with pytest.raises(LookupError):
+        common.get_model_for_type('foobar')
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize("model_type,model_class", [
+    (name, cls) for cls, name in TEST_MODELS
+])
+def test_get_model_for_valid_type(model_type, model_class):
+    assert common.get_model_for_type(model_type) == model_class
+
+
 @pytest.fixture
 def memoized_function(mocker, mock_cache):
     with mock.patch('awx.main.utils.common.get_memoize_cache', return_value=mock_cache):

awx/main/utils/common.py

@@ -20,7 +20,6 @@ from decimal import Decimal
 
 # Django
 from django.core.exceptions import ObjectDoesNotExist
-from django.db import DatabaseError
 from django.utils.translation import ugettext_lazy as _
 from django.db.models.fields.related import ForeignObjectRel, ManyToManyField
 from django.db.models.query import QuerySet
@@ -34,7 +33,7 @@ from django.apps import apps
 
 logger = logging.getLogger('awx.main.utils')
 
-__all__ = ['get_object_or_400', 'camelcase_to_underscore', 'memoize', 'memoize_delete',
+__all__ = ['get_object_or_400', 'camelcase_to_underscore', 'underscore_to_camelcase', 'memoize', 'memoize_delete',
            'get_ansible_version', 'get_ssh_version', 'get_licenser', 'get_awx_version', 'update_scm_url',
            'get_type_for_model', 'get_model_for_type', 'copy_model_by_class', 'region_sorting',
            'copy_m2m_relationships', 'prefetch_page_capabilities', 'to_python_boolean',
@@ -92,6 +91,14 @@ def camelcase_to_underscore(s):
     return s.lower().strip('_')
 
 
+def underscore_to_camelcase(s):
+    '''
+    Convert lowercase_with_underscore names to CamelCase.
+    '''
+    return ''.join(x.capitalize() or '_' for x in s.split('_'))
+
+
+
 class RequireDebugTrueOrTest(logging.Filter):
     '''
     Logging filter to output when in DEBUG mode or running tests.
@@ -497,20 +504,16 @@ def get_type_for_model(model):
     return camelcase_to_underscore(opts.object_name)
 
 
-def get_model_for_type(type):
+def get_model_for_type(type_name):
     '''
     Return model class for a given type name.
     '''
-    from django.contrib.contenttypes.models import ContentType
-    for ct in ContentType.objects.filter(Q(app_label='main') | Q(app_label='auth', model='user')):
-        ct_model = ct.model_class()
-        if not ct_model:
-            continue
-        ct_type = get_type_for_model(ct_model)
-        if type == ct_type:
-            return ct_model
+    model_str = underscore_to_camelcase(type_name)
+    if model_str == 'User':
+        use_app = 'auth'
     else:
-        raise DatabaseError('"{}" is not a valid AWX model.'.format(type))
+        use_app = 'main'
+    return apps.get_model(use_app, model_str)
 
 
 def prefetch_page_capabilities(model, page, prefetch_list, user):

awx/main/utils/polymorphic.py

@@ -2,6 +2,8 @@
 from django.contrib.contenttypes.models import ContentType
 from django.db import models
 
+from awx.main.utils.common import camelcase_to_underscore
+
 
 def build_polymorphic_ctypes_map(cls):
     # {'1': 'unified_job', '2': 'Job', '3': 'project_update', ...}
@@ -9,7 +11,7 @@ def build_polymorphic_ctypes_map(cls):
     for ct in ContentType.objects.filter(app_label='main'):
         ct_model_class = ct.model_class()
         if ct_model_class and issubclass(ct_model_class, cls):
-            mapping[ct.id] = ct_model_class._camel_to_underscore(ct_model_class.__name__)
+            mapping[ct.id] = camelcase_to_underscore(ct_model_class.__name__)
     return mapping