diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index 96235542c97..26bb2fd3b95 100644 --- a/ckan/logic/validators.py +++ b/ckan/logic/validators.py @@ -62,14 +62,34 @@ def package_id_not_changed(value, context): return value def int_validator(value, context): - if isinstance(value, int): - return value + ''' + Return an integer for value, which may be a string in base 10 or + a numeric type (e.g. int, long, float, Decimal, Fraction). Return + None for None or empty/all-whitespace string values. + + :raises: ckan.lib.navl.dictization_functions.Invalid for other + inputs or non-whole values + ''' + if value is None: + return None + if hasattr(value, 'strip') and not value.strip(): + return None + try: - if value.strip() == '': - return None - return int(value) - except (AttributeError, ValueError), e: - raise Invalid(_('Invalid integer')) + whole, part = divmod(value, 1) + except TypeError: + try: + return int(value) + except ValueError: + pass + else: + if not part: + try: + return int(whole) + except TypeError: + pass # complex number: fail like int(complex) does + + raise Invalid(_('Invalid integer')) def natural_number_validator(value, context): value = int_validator(value, context) @@ -154,10 +174,10 @@ def package_id_or_name_exists(package_id_or_name, context): return package_id_or_name def user_id_exists(user_id, context): - """Raises Invalid if the given user_id does not exist in the model given + '''Raises Invalid if the given user_id does not exist in the model given in the context, otherwise returns the given user_id. - """ + ''' model = context['model'] session = context['session'] @@ -184,10 +204,10 @@ def user_id_or_name_exists(user_id_or_name, context): return user_id_or_name def group_id_exists(group_id, context): - """Raises Invalid if the given group_id does not exist in the model given + '''Raises Invalid if the given group_id does not exist in the model given in the context, otherwise returns the given group_id. - """ + ''' model = context['model'] session = context['session'] @@ -198,10 +218,10 @@ def group_id_exists(group_id, context): def related_id_exists(related_id, context): - """Raises Invalid if the given related_id does not exist in the model + '''Raises Invalid if the given related_id does not exist in the model given in the context, otherwise returns the given related_id. - """ + ''' model = context['model'] session = context['session'] @@ -211,9 +231,9 @@ def related_id_exists(related_id, context): return related_id def group_id_or_name_exists(reference, context): - """ + ''' Raises Invalid if a group identified by the name or id cannot be found. - """ + ''' model = context['model'] result = model.Group.get(reference) if not result: @@ -221,13 +241,13 @@ def group_id_or_name_exists(reference, context): return reference def activity_type_exists(activity_type): - """Raises Invalid if there is no registered activity renderer for the + '''Raises Invalid if there is no registered activity renderer for the given activity_type. Otherwise returns the given activity_type. This just uses object_id_validators as a lookup. very safe. - """ + ''' if activity_type in object_id_validators: return activity_type else: @@ -266,7 +286,7 @@ def resource_id_exists(value, context): } def object_id_validator(key, activity_dict, errors, context): - """Validate the 'object_id' value of an activity_dict. + '''Validate the 'object_id' value of an activity_dict. Uses the object_id_validators dict (above) to find and call an 'object_id' validator function for the given activity_dict's 'activity_type' value. @@ -278,7 +298,7 @@ def object_id_validator(key, activity_dict, errors, context): Raises Invalid if there is no object_id_validator for the activity_dict's 'activity_type' value. - """ + ''' activity_type = activity_dict[('activity_type',)] if object_id_validators.has_key(activity_type): object_id = activity_dict[('object_id',)] @@ -328,15 +348,15 @@ def name_validator(value, context): return value def package_name_validator(key, data, errors, context): - model = context["model"] - session = context["session"] - package = context.get("package") + model = context['model'] + session = context['session'] + package = context.get('package') query = session.query(model.Package.name).filter_by(name=data[key]) if package: package_id = package.id else: - package_id = data.get(key[:-1] + ("id",)) + package_id = data.get(key[:-1] + ('id',)) if package_id and package_id is not missing: query = query.filter(model.Package.id <> package_id) result = query.first() @@ -656,7 +676,7 @@ def tag_not_in_vocabulary(key, tag_dict, errors, context): return def url_validator(key, data, errors, context): - """ Checks that the provided value (if it is present) is a valid URL """ + ''' Checks that the provided value (if it is present) is a valid URL ''' import urlparse import string diff --git a/ckan/new_tests/logic/test_validators.py b/ckan/new_tests/logic/test_validators.py index 42d8013112e..2020afc06c7 100644 --- a/ckan/new_tests/logic/test_validators.py +++ b/ckan/new_tests/logic/test_validators.py @@ -3,6 +3,9 @@ ''' import copy +import fractions +import decimal +import warnings import mock import nose.tools @@ -14,6 +17,11 @@ # different places in the code) we have to either do this or introduce a shared # test helper functions module (which we also don't want to do). import ckan.new_tests.lib.navl.test_validators as t +import ckan.lib.navl.dictization_functions as df +import ckan.logic.validators as validators +import ckan.model as model + +assert_equals = nose.tools.assert_equals def returns_arg(function): @@ -55,7 +63,6 @@ def call_validator(*args, **kwargs): ''' def call_and_assert(*args, **kwargs): - import ckan.lib.navl.dictization_functions as df nose.tools.assert_raises(df.Invalid, function, *args, **kwargs) return call_and_assert @@ -144,9 +151,6 @@ def test_name_validator_with_invalid_value(self): '''If given an invalid value name_validator() should do raise Invalid. ''' - import ckan.logic.validators as validators - import ckan.model as model - invalid_values = [ # Non-string names aren't allowed as names. 13, @@ -198,9 +202,6 @@ def test_name_validator_with_valid_value(self): return the string. ''' - import ckan.logic.validators as validators - import ckan.model as model - valid_names = [ 'fred', 'fred-flintstone', @@ -229,8 +230,6 @@ def test_user_name_validator_with_non_string_value(self): value. ''' - import ckan.logic.validators as validators - non_string_values = [ 13, 23.7, @@ -270,8 +269,6 @@ def test_user_name_validator_with_a_name_that_already_exists(self): user name that already exists. ''' - import ckan.logic.validators as validators - # Mock ckan.model. model.User.get('user_name') will return another mock # object rather than None, which will simulate an existing user with # the same user name in the database. @@ -293,9 +290,6 @@ def call_validator(*args, **kwargs): def test_user_name_validator_successful(self): '''user_name_validator() should do nothing if given a valid name.''' - - import ckan.logic.validators as validators - data = factories.validator_data_dict() key = ('name',) data[key] = 'new_user_name' @@ -319,19 +313,15 @@ def call_validator(*args, **kwargs): # the context dict. def test_if_empty_guess_format(self): - - import ckan.logic.validators as validators - import ckan.lib.navl.dictization_functions as dictization_functions - data = {'name': 'package_name', 'resources': [ {'url': 'http://fakedomain/my.csv', 'format': ''}, {'url': 'http://fakedomain/my.pdf', - 'format': dictization_functions.Missing}, + 'format': df.Missing}, {'url': 'http://fakedomain/my.pdf', 'format': 'pdf'}, {'url': 'http://fakedomain/my.pdf', 'id': 'fake_resource_id', 'format': ''} ]} - data = dictization_functions.flatten_dict(data) + data = df.flatten_dict(data) @t.does_not_modify_errors_dict def call_validator(*args, **kwargs): @@ -358,8 +348,6 @@ def call_validator(*args, **kwargs): assert new_data[('resources', 3, 'format')] == '' def test_clean_format(self): - import ckan.logic.validators as validators - format = validators.clean_format('csv') assert format == 'CSV' @@ -373,9 +361,6 @@ def test_clean_format(self): assert format == '' def test_datasets_with_org_can_be_private_when_creating(self): - - import ckan.logic.validators as validators - data = factories.validator_data_dict() errors = factories.validator_errors_dict() @@ -397,9 +382,6 @@ def call_validator(*args, **kwargs): call_validator(key, data, errors, context={'model': mock_model}) def test_datasets_with_no_org_cannot_be_private_when_creating(self): - - import ckan.logic.validators as validators - data = factories.validator_data_dict() errors = factories.validator_errors_dict() @@ -420,9 +402,6 @@ def call_validator(*args, **kwargs): call_validator(key, data, errors, context={'model': mock_model}) def test_datasets_with_org_can_be_private_when_updating(self): - - import ckan.logic.validators as validators - data = factories.validator_data_dict() errors = factories.validator_errors_dict() @@ -444,5 +423,82 @@ def call_validator(*args, **kwargs): *args, **kwargs) call_validator(key, data, errors, context={'model': mock_model}) - #TODO: Need to test when you are not providing owner_org and the validator - # queries for the dataset with package_show + +class TestIntValidator(object): + + def test_int_unchanged(self): + returns_arg(validators.int_validator)(42) + + def test_zero_unchanged(self): + returns_arg(validators.int_validator)(0) + + def test_long_unchanged(self): + returns_arg(validators.int_validator)(3948756923874659827346598) + + def test_None_unchanged(self): + returns_arg(validators.int_validator)(None) + + def test_float_converted(self): + assert_equals(validators.int_validator(42.0, None), 42) + + def test_fraction_converted(self): + assert_equals(validators.int_validator( + fractions.Fraction(2, 1), {}), 2) + + def test_decimal_converted(self): + assert_equals(validators.int_validator( + decimal.Decimal('19.00'), {}), 19) + + def test_long_int_string_converted(self): + assert_equals(validators.int_validator( + '528735648764587235684376', {}), 528735648764587235684376) + + def test_negative_int_string_converted(self): + assert_equals(validators.int_validator('-2', {}), -2) + + def test_positive_int_string_converted(self): + assert_equals(validators.int_validator('+3', {}), 3) + + def test_zero_prefixed_int_string_converted_as_decimal(self): + assert_equals(validators.int_validator('0123', {}), 123) + + def test_string_with_whitespace_converted(self): + assert_equals(validators.int_validator('\t 98\n', {}), 98) + + def test_empty_string_becomes_None(self): + assert_equals(validators.int_validator('', {}), None) + + def test_whitespace_string_becomes_None(self): + assert_equals(validators.int_validator('\n\n \t', {}), None) + + def test_float_with_decimal_raises_Invalid(self): + raises_Invalid(validators.int_validator)(42.5, {}) + + def test_float_string_raises_Invalid(self): + raises_Invalid(validators.int_validator)('42.0', {}) + + def test_exponent_string_raises_Invalid(self): + raises_Invalid(validators.int_validator)('1e6', {}) + + def test_non_numeric_string_raises_Invalid(self): + raises_Invalid(validators.int_validator)('text', {}) + + def test_non_whole_fraction_raises_Invalid(self): + raises_Invalid(validators.int_validator)(fractions.Fraction(3, 2), {}) + + def test_non_whole_decimal_raises_Invalid(self): + raises_Invalid(validators.int_validator)(decimal.Decimal('19.99'), {}) + + def test_complex_with_imaginary_component_raises_Invalid(self): + with warnings.catch_warnings(): # divmod() issues warning for complex + warnings.filterwarnings('ignore', category=DeprecationWarning) + raises_Invalid(validators.int_validator)(1 + 1j, {}) + + def test_complex_without_imaginary_component_raises_Invalid(self): + with warnings.catch_warnings(): # divmod() issues warning for complex + warnings.filterwarnings('ignore', category=DeprecationWarning) + raises_Invalid(validators.int_validator)(1 + 0j, {}) + + +#TODO: Need to test when you are not providing owner_org and the validator +# queries for the dataset with package_show