Merge pull request #885 from cchurch/more-serializer-fixes

More DRF 3.3 Serializer Fixes
This commit is contained in:
Chris Church 2016-02-11 23:15:17 -05:00
commit f68329bd58
8 changed files with 222 additions and 96 deletions

89
awx/api/fields.py Normal file
View File

@ -0,0 +1,89 @@
# Copyright (c) 2016 Ansible, Inc.
# All Rights Reserved.
# Django
from django.utils.encoding import force_text
# Django REST Framework
from rest_framework import serializers
__all__ = ['BooleanNullField', 'CharNullField', 'ChoiceNullField', 'EncryptedPasswordField', 'VerbatimField']
class NullFieldMixin(object):
'''
Mixin to prevent shortcutting validation when we want to allow null input,
but coerce the resulting value to another type.
'''
def validate_empty_values(self, data):
(is_empty_value, data) = super(NullFieldMixin, self).validate_empty_values(data)
if is_empty_value and data is None:
return (False, data)
return (is_empty_value, data)
class BooleanNullField(NullFieldMixin, serializers.NullBooleanField):
'''
Custom boolean field that allows null and empty string as False values.
'''
def to_internal_value(self, data):
return bool(super(BooleanNullField, self).to_internal_value(data))
class CharNullField(NullFieldMixin, serializers.CharField):
'''
Custom char field that allows null as input and coerces to an empty string.
'''
def __init__(self, **kwargs):
kwargs['allow_null'] = True
super(CharNullField, self).__init__(**kwargs)
def to_internal_value(self, data):
return super(CharNullField, self).to_internal_value(data or u'')
class ChoiceNullField(NullFieldMixin, serializers.ChoiceField):
'''
Custom choice field that allows null as input and coerces to an empty string.
'''
def __init__(self, **kwargs):
kwargs['allow_null'] = True
super(ChoiceNullField, self).__init__(**kwargs)
def to_internal_value(self, data):
return super(ChoiceNullField, self).to_internal_value(data or u'')
class EncryptedPasswordField(CharNullField):
'''
Custom field to handle encrypted password values (on credentials).
'''
def to_internal_value(self, data):
value = super(EncryptedPasswordField, self).to_internal_value(data or u'')
# If user submits a value starting with $encrypted$, ignore it.
if force_text(value).startswith('$encrypted$'):
raise serializers.SkipField
return value
def to_representation(self, value):
# Replace the actual encrypted value with the string $encrypted$.
if force_text(value).startswith('$encrypted$'):
return '$encrypted$'
return value
class VerbatimField(serializers.Field):
'''
Custom field that passes the value through without changes.
'''
def to_internal_value(self, data):
return data
def to_representation(self, value):
return value

View File

@ -24,11 +24,13 @@ from django.core.exceptions import ObjectDoesNotExist, ValidationError as Django
from django.db import models
# from django.utils.translation import ugettext_lazy as _
from django.utils.encoding import force_text, smart_text
from django.utils.text import capfirst
# Django REST Framework
from rest_framework.exceptions import ValidationError
from rest_framework import fields
from rest_framework import serializers
from rest_framework import validators
from rest_framework.utils.serializer_helpers import ReturnList
# Django-Polymorphic
@ -42,6 +44,7 @@ from awx.main.redact import REPLACE_STR
from awx.main.conf import tower_settings
from awx.api.license import feature_enabled
from awx.api.fields import BooleanNullField, CharNullField, ChoiceNullField, EncryptedPasswordField, VerbatimField
from awx.fact.models import * # noqa
@ -202,11 +205,6 @@ class BaseSerializer(serializers.ModelSerializer):
'modified', 'name', 'description')
summary_fields = () # FIXME: List of field names from this serializer that should be used when included as part of another's summary_fields.
summarizable_fields = () # FIXME: List of field names on this serializer that should be included in summary_fields.
extra_kwargs = {
'description': {
'allow_null': True,
},
}
# add the URL and related resources
type = serializers.SerializerMethodField()
@ -358,24 +356,50 @@ class BaseSerializer(serializers.ModelSerializer):
# Pass model field default onto the serializer field if field is not read-only.
if model_field.has_default() and not field_kwargs.get('read_only', False):
field_kwargs['default'] = model_field.get_default()
field_kwargs['default'] = field_kwargs['initial'] = model_field.get_default()
# Enforce minimum value of 0 for PositiveIntegerFields.
if isinstance(model_field, (models.PositiveIntegerField, models.PositiveSmallIntegerField)) and 'choices' not in field_kwargs:
field_kwargs['min_value'] = 0
# Use custom boolean field that allows null and empty string as False values.
if isinstance(model_field, models.BooleanField) and not field_kwargs.get('read_only', False):
field_class = BooleanNullField
# Use custom char or choice field that coerces null to an empty string.
if isinstance(model_field, (models.CharField, models.TextField)) and not field_kwargs.get('read_only', False):
if 'choices' in field_kwargs:
field_class = ChoiceNullField
else:
field_class = CharNullField
# Update verbosity choices from settings (for job templates, jobs, ad hoc commands).
if field_name == 'verbosity' and 'choices' in field_kwargs:
field_kwargs['choices'] = getattr(settings, 'VERBOSITY_CHOICES', field_kwargs['choices'])
# Update the message used for the unique validator to use capitalized
# verbose name; keeps unique message the same as with DRF 2.x.
for validator in field_kwargs.get('validators', []):
if isinstance(validator, validators.UniqueValidator):
unique_error_message = model_field.error_messages.get('unique', None)
if unique_error_message:
unique_error_message = unique_error_message % {
'model_name': capfirst(opts.verbose_name),
'field_label': capfirst(model_field.verbose_name),
}
validator.message = unique_error_message
return field_class, field_kwargs
def build_relational_field(self, field_name, relation_info):
field_class, field_kwargs = super(BaseSerializer, self).build_relational_field(field_name, relation_info)
# Don't include choicse for foreign key fields.
# Don't include choices for foreign key fields.
field_kwargs.pop('choices', None)
return field_class, field_kwargs
def validate_description(self, value):
# Description should always be empty string, never null.
return value or u''
def get_unique_together_validators(self):
# Allow the model's full_clean method to handle the unique together validation.
return []
def run_validation(self, data=fields.empty):
try:
@ -653,7 +677,6 @@ class UserSerializer(BaseSerializer):
fields = ('*', '-name', '-description', '-modified',
'-summary_fields', 'username', 'first_name', 'last_name',
'email', 'is_superuser', 'password', 'ldap_dn')
def to_representation(self, obj):
ret = super(UserSerializer, self).to_representation(obj)
@ -772,17 +795,9 @@ class OrganizationSerializer(BaseSerializer):
class ProjectOptionsSerializer(BaseSerializer):
scm_clean = serializers.NullBooleanField(default=False)
scm_delete_on_update = serializers.NullBooleanField(default=False)
class Meta:
fields = ('*', 'local_path', 'scm_type', 'scm_url', 'scm_branch',
'scm_clean', 'scm_delete_on_update', 'credential')
extra_kwargs = {
'scm_type': {
'allow_null': True,
},
}
def get_related(self, obj):
res = super(ProjectOptionsSerializer, self).get_related(obj)
@ -791,15 +806,6 @@ class ProjectOptionsSerializer(BaseSerializer):
args=(obj.credential.pk,))
return res
def validate_scm_type(self, value):
return value or u''
def validate_scm_clean(self, value):
return bool(value)
def validate_scm_delete_on_update(self, value):
return bool(value)
def validate(self, attrs):
errors = {}
@ -831,9 +837,7 @@ class ProjectOptionsSerializer(BaseSerializer):
class ProjectSerializer(UnifiedJobTemplateSerializer, ProjectOptionsSerializer):
scm_delete_on_next_update = serializers.BooleanField(read_only=True)
scm_update_on_launch = serializers.NullBooleanField(default=False)
status = serializers.ChoiceField(choices=Project.PROJECT_STATUS_CHOICES, read_only=True, required=False)
status = serializers.ChoiceField(choices=Project.PROJECT_STATUS_CHOICES, read_only=True)
last_update_failed = serializers.BooleanField(read_only=True)
last_updated = serializers.DateTimeField(read_only=True)
@ -842,9 +846,7 @@ class ProjectSerializer(UnifiedJobTemplateSerializer, ProjectOptionsSerializer):
fields = ('*', 'scm_delete_on_next_update', 'scm_update_on_launch',
'scm_update_cache_timeout') + \
('last_update_failed', 'last_updated') # Backwards compatibility
def clean_scm_update_on_launch(self, value):
return bool(value)
read_only_fields = ('scm_delete_on_next_update',)
def get_related(self, obj):
res = super(ProjectSerializer, self).get_related(obj)
@ -1216,14 +1218,6 @@ class InventorySourceOptionsSerializer(BaseSerializer):
class Meta:
fields = ('*', 'source', 'source_path', 'source_script', 'source_vars', 'credential',
'source_regions', 'instance_filters', 'group_by', 'overwrite', 'overwrite_vars')
extra_kwargs = {
'instance_filters': {
'allow_null': True,
},
'group_by': {
'allow_null': True,
},
}
def get_related(self, obj):
res = super(InventorySourceOptionsSerializer, self).get_related(obj)
@ -1281,7 +1275,7 @@ class InventorySourceOptionsSerializer(BaseSerializer):
class InventorySourceSerializer(UnifiedJobTemplateSerializer, InventorySourceOptionsSerializer):
status = serializers.ChoiceField(choices=InventorySource.INVENTORY_SOURCE_STATUS_CHOICES, read_only=True, required=False)
status = serializers.ChoiceField(choices=InventorySource.INVENTORY_SOURCE_STATUS_CHOICES, read_only=True)
last_update_failed = serializers.BooleanField(read_only=True)
last_updated = serializers.DateTimeField(read_only=True)
@ -1440,14 +1434,7 @@ class PermissionSerializer(BaseSerializer):
class CredentialSerializer(BaseSerializer):
# FIXME: may want to make some of these filtered based on user accessing
password = serializers.CharField(required=False, default='')
security_token = serializers.CharField(required=False, default='')
ssh_key_data = serializers.CharField(required=False, default='')
ssh_key_unlock = serializers.CharField(required=False, default='')
become_password = serializers.CharField(required=False, default='')
vault_password = serializers.CharField(required=False, default='')
# FIXME: may want to make some fields filtered based on user accessing
class Meta:
model = Credential
@ -1456,24 +1443,23 @@ class CredentialSerializer(BaseSerializer):
'become_method', 'become_username', 'become_password',
'vault_password')
def build_standard_field(self, field_name, model_field):
field_class, field_kwargs = super(CredentialSerializer, self).build_standard_field(field_name, model_field)
if field_name in Credential.PASSWORD_FIELDS:
field_class = EncryptedPasswordField
field_kwargs['required'] = False
field_kwargs['default'] = ''
return field_class, field_kwargs
def to_representation(self, obj):
ret = super(CredentialSerializer, self).to_representation(obj)
if obj is not None and 'user' in ret and (not obj.user or not obj.user.is_active):
ret['user'] = None
if obj is not None and 'team' in ret and (not obj.team or not obj.team.active):
ret['team'] = None
# Replace the actual encrypted value with the string $encrypted$.
for field in Credential.PASSWORD_FIELDS:
if field in ret and force_text(ret[field]).startswith('$encrypted$'):
ret[field] = '$encrypted$'
return ret
def validate(self, attrs):
# If the value sent to the API startswith $encrypted$, ignore it.
for field in Credential.PASSWORD_FIELDS:
if force_text(attrs.get(field, '')).startswith('$encrypted$'):
attrs.pop(field, None)
# If creating a credential from a view that automatically sets the
# parent_key (user or team), set the other value to None.
view = self.context.get('view', None)
@ -2230,6 +2216,8 @@ class ActivityStreamSerializer(BaseSerializer):
class TowerSettingsSerializer(BaseSerializer):
value = VerbatimField()
class Meta:
model = TowerSettings
fields = ('key', 'description', 'category', 'value', 'value_type', 'user')
@ -2263,14 +2251,20 @@ class TowerSettingsSerializer(BaseSerializer):
manifest = settings.TOWER_SETTINGS_MANIFEST
if attrs['key'] not in manifest:
raise serializers.ValidationError(dict(key=["Key {0} is not a valid settings key".format(attrs['key'])]))
# TODO: Type checking/coercion, contextual validation
return super(TowerSettingsSerializer, self).validate(attrs)
def _create(self, validated_data):
current_val = TowerSettings.objects.filter(key=validated_data['key'])
if current_val.exists():
return self.update(current_val[0], validated_data)
return super(TowerSettingsSerializer, self).create(validated_data)
if attrs['value_type'] == 'json':
attrs['value'] = json.dumps(attrs['value'])
elif attrs['value_type'] == 'list':
try:
attrs['value'] = ','.join(map(force_text, attrs['value']))
except TypeError:
attrs['value'] = force_text(attrs['value'])
elif attrs['value_type'] == 'bool':
attrs['value'] = force_text(bool(attrs['value']))
else:
attrs['value'] = force_text(attrs['value'])
return super(TowerSettingsSerializer, self).validate(attrs)
class AuthTokenSerializer(serializers.Serializer):

View File

@ -6,7 +6,7 @@ import json
# Django
from django.db import models
from django.utils.encoding import smart_text
from django.utils.encoding import force_text
from django.utils.translation import ugettext_lazy as _
# Tower
@ -34,7 +34,9 @@ class TowerSettings(CreatedModifiedModel):
)
description = models.TextField()
category = models.CharField(max_length=128)
value = models.TextField()
value = models.TextField(
blank=True,
)
value_type = models.CharField(
max_length=12,
choices=SETTINGS_TYPE_CHOICES
@ -54,9 +56,12 @@ class TowerSettings(CreatedModifiedModel):
elif self.value_type == 'password':
converted_type = self.value
elif self.value_type == 'list':
converted_type = [x.strip() for x in self.value.split(',')]
if self.value:
converted_type = [x.strip() for x in self.value.split(',')]
else:
converted_type = []
elif self.value_type == 'bool':
converted_type = smart_text(self.value).lower() in ('true', 'yes', '1')
converted_type = force_text(self.value).lower() in ('true', 'yes', '1')
elif self.value_type == 'string':
converted_type = self.value
else:
@ -69,8 +74,11 @@ class TowerSettings(CreatedModifiedModel):
if self.value_type == 'json':
self.value = json.dumps(value)
elif self.value_type == 'list':
self.value = ','.join(value)
try:
self.value = ','.join(map(force_text, value))
except TypeError:
self.value = force_text(value)
elif self.value_type == 'bool':
self.value = smart_text(bool(value))
self.value = force_text(bool(value))
else:
self.value = smart_text(value)
self.value = force_text(value)

View File

@ -262,7 +262,9 @@ class OrganizationsTest(BaseTest):
data1 = self.post(self.collection(), new_org, expect=201, auth=self.get_super_credentials())
# duplicate post results in 400
self.post(self.collection(), new_org, expect=400, auth=self.get_super_credentials())
response = self.post(self.collection(), new_org, expect=400, auth=self.get_super_credentials())
self.assertTrue('name' in response, response)
self.assertTrue('Name' in response['name'][0], response)
# look at what we got back from the post, make sure we added an org
last_org = Organization.objects.order_by('-pk')[0]

View File

@ -231,12 +231,19 @@ class ProjectsTest(BaseTransactionTest):
'description': 'Does amazing things',
'local_path': os.path.basename(project_dir),
'scm_type': None,
'scm_update_on_launch': '',
'scm_delete_on_update': None,
'scm_clean': False,
}
# Adding a project with scm_type=None should work, but scm_type will be
# changed to an empty string.
# changed to an empty string. Other boolean fields should accept null
# or an empty string for False, but save the value as a boolean.
response = self.post(projects, project_data, expect=201,
auth=self.get_super_credentials())
self.assertEqual(response['scm_type'], u'')
self.assertEqual(response['scm_update_on_launch'], False)
self.assertEqual(response['scm_delete_on_update'], False)
self.assertEqual(response['scm_clean'], False)
# can edit project using same local path.
project_detail = reverse('api:project_detail', args=(response['id'],))
@ -494,7 +501,9 @@ class ProjectsTest(BaseTransactionTest):
ssh_key_data = TEST_SSH_KEY_DATA_LOCKED,
ssh_key_unlock = TEST_SSH_KEY_DATA_UNLOCK,
ssh_password = 'narf',
sudo_password = 'troz'
sudo_password = 'troz',
security_token = '',
vault_password = None,
)
# can add credentials to a user (if user or org admin or super user)
@ -561,13 +570,17 @@ class ProjectsTest(BaseTransactionTest):
# Repeating the same POST should violate a unique constraint.
with self.current_user(self.super_django_user):
data = dict(name='xyz', user=self.super_django_user.pk)
self.post(url, data, expect=400)
response = self.post(url, data, expect=400)
self.assertTrue('__all__' in response, response)
self.assertTrue('already exists' in response['__all__'][0], response)
# Test with null where we expect a string value.
# Test with null where we expect a string value. Value will be coerced
# to an empty string.
with self.current_user(self.super_django_user):
data = dict(name='zyx', user=self.super_django_user.pk, kind='ssh',
become_username=None)
self.post(url, data, expect=400)
response = self.post(url, data, expect=201)
self.assertEqual(response['become_username'], '')
# Test with encrypted ssh key and no unlock password.
with self.current_user(self.super_django_user):
@ -698,14 +711,15 @@ class ProjectsTest(BaseTransactionTest):
# user=user.pk, # no need to specify, this will be automatically filled in
inventory=inventory.pk,
project=project.pk,
permission_type=PERM_INVENTORY_DEPLOY
permission_type=PERM_INVENTORY_DEPLOY,
run_ad_hoc_commands=None,
)
team_permission = dict(
name='team can deploy a certain project to a certain inventory',
# team=team.pk, # no need to specify, this will be automatically filled in
inventory=inventory.pk,
project=project.pk,
permission_type=PERM_INVENTORY_DEPLOY
permission_type=PERM_INVENTORY_DEPLOY,
)
url = reverse('api:user_permissions_list', args=(user.pk,))

View File

@ -35,6 +35,13 @@ TEST_TOWER_SETTINGS_MANIFEST = {
"default": ["A", "Simple", "List"],
"type": "list",
"category": "test"
},
"TEST_SETTING_JSON": {
"name": "A JSON Field",
"description": "A JSON Field",
"default": {"key": "value", "otherkey": ["list", "of", "things"]},
"type": "json",
"category": "test"
}
}
@ -46,7 +53,7 @@ class SettingsTest(BaseTest):
self.setup_instances()
self.setup_users()
def get_settings(self, expected_count=4):
def get_settings(self, expected_count=5):
result = self.get(reverse('api:settings_list'), expect=200)
self.assertEqual(result['count'], expected_count)
return result['results']
@ -74,21 +81,29 @@ class SettingsTest(BaseTest):
with self.current_user(self.super_django_user):
self.get_settings(expected_count=len(TEST_TOWER_SETTINGS_MANIFEST))
def test_set_and_reset_settings(self):
def set_and_reset_setting(self, key, values, expected_values=()):
settings_reset = reverse('api:settings_reset')
setting = self.get_individual_setting(key)
self.assertEqual(setting['value'], TEST_TOWER_SETTINGS_MANIFEST[key]['default'])
for n, value in enumerate(values):
self.set_setting(key, value)
setting = self.get_individual_setting(key)
if len(expected_values) > n:
self.assertEqual(setting['value'], expected_values[n])
else:
self.assertEqual(setting['value'], value)
self.post(settings_reset, data={"key": key}, expect=204)
setting = self.get_individual_setting(key)
self.assertEqual(setting['value'], TEST_TOWER_SETTINGS_MANIFEST[key]['default'])
def test_set_and_reset_settings(self):
with self.current_user(self.super_django_user):
# Set and reset a single setting
setting_int = self.get_individual_setting('TEST_SETTING_INT')
self.assertEqual(setting_int['value'], TEST_TOWER_SETTINGS_MANIFEST['TEST_SETTING_INT']['default'])
self.set_setting('TEST_SETTING_INT', 2)
setting_int = self.get_individual_setting('TEST_SETTING_INT')
self.assertEqual(setting_int['value'], 2)
self.set_setting('TEST_SETTING_INT', 3)
setting_int = self.get_individual_setting('TEST_SETTING_INT')
self.assertEqual(setting_int['value'], 3)
self.post(settings_reset, data={"key": 'TEST_SETTING_INT'}, expect=204)
setting_int = self.get_individual_setting('TEST_SETTING_INT')
self.assertEqual(setting_int['value'], TEST_TOWER_SETTINGS_MANIFEST['TEST_SETTING_INT']['default'])
self.set_and_reset_setting('TEST_SETTING_INT', (2, 0))
self.set_and_reset_setting('TEST_SETTING_STRING', ('blah', '', u'\u2620'))
self.set_and_reset_setting('TEST_SETTING_BOOL', (True, False))
# List values are always saved as strings.
self.set_and_reset_setting('TEST_SETTING_LIST', ([4, 5, 6], [], [2]), (['4', '5', '6'], [], ['2']))
self.set_and_reset_setting('TEST_SETTING_JSON', ({"k": "v"}, {}, [], [7, 8], 'str'))
def test_clear_all_settings(self):
settings_list = reverse('api:settings_list')
@ -97,6 +112,7 @@ class SettingsTest(BaseTest):
self.set_setting('TEST_SETTING_STRING', "foo")
self.set_setting('TEST_SETTING_BOOL', False)
self.set_setting('TEST_SETTING_LIST', [1,2,3])
self.set_setting('TEST_SETTING_JSON', '{"key": "new value"}')
all_settings = self.get_settings()
for setting_entry in all_settings:
self.assertNotEqual(setting_entry['value'],

View File

@ -207,6 +207,8 @@ class UsersTest(BaseTest):
self.post(url, expect=403, data=new_super_user, auth=self.get_other_credentials())
self.post(url, expect=403, data=new_super_user, auth=self.get_normal_credentials())
self.post(url, expect=201, data=new_super_user, auth=self.get_super_credentials())
new_super_user2 = dict(username='nommy2', password='cookie', is_superuser=None)
self.post(url, expect=201, data=new_super_user2, auth=self.get_super_credentials())
def test_auth_token_login(self):
auth_token_url = reverse('api:auth_token_view')

View File

@ -226,6 +226,7 @@ REST_FRAMEWORK = {
'EXCEPTION_HANDLER': 'awx.api.views.api_exception_handler',
'VIEW_NAME_FUNCTION': 'awx.api.generics.get_view_name',
'VIEW_DESCRIPTION_FUNCTION': 'awx.api.generics.get_view_description',
'NON_FIELD_ERRORS_KEY': '__all__',
}
AUTHENTICATION_BACKENDS = (