[ckan-changes] commit/ckan: dread: Added failing test for the problem. Improving related tests and docs to work out what is not quite right.

Bitbucket commits-noreply at bitbucket.org
Thu Nov 10 13:30:10 UTC 2011


1 new commit in ckan:


https://bitbucket.org/okfn/ckan/changeset/8b73510e1eb1/
changeset:   8b73510e1eb1
branch:      defect-1394-resource-validation
user:        dread
date:        2011-11-10 14:29:08
summary:     Added failing test for the problem. Improving related tests and docs to work out what is not quite right.
affected #:  5 files

diff -r 427a990a608e2bc26df3b6477b27d59f1b122e79 -r 8b73510e1eb18712e05a82805d4738dd43ddc3de ckan/lib/navl/dictization_functions.py
--- a/ckan/lib/navl/dictization_functions.py
+++ b/ckan/lib/navl/dictization_functions.py
@@ -206,7 +206,7 @@
     return schema
 
 def validate(data, schema, context=None):
-    '''validate an unflattened nested dict agiast a schema'''
+    '''validate an unflattened nested dict against a schema'''
 
     context = context or {}
 


diff -r 427a990a608e2bc26df3b6477b27d59f1b122e79 -r 8b73510e1eb18712e05a82805d4738dd43ddc3de ckan/logic/action/create.py
--- a/ckan/logic/action/create.py
+++ b/ckan/logic/action/create.py
@@ -55,6 +55,7 @@
     else:
         rev.message = _(u'REST API: Create object %s') % data.get("name")
 
+    import pdb; pdb.set_trace()
     pkg = package_dict_save(data, context)
     admins = []
     if user:


diff -r 427a990a608e2bc26df3b6477b27d59f1b122e79 -r 8b73510e1eb18712e05a82805d4738dd43ddc3de ckan/logic/action/update.py
--- a/ckan/logic/action/update.py
+++ b/ckan/logic/action/update.py
@@ -38,7 +38,7 @@
     error_summary = {}
     for key, error in error_dict.iteritems():
         if key == 'resources':
-            error_summary[_('Resources')] = _('Package resource(s) incomplete')
+            error_summary[_('Resources')] = _('Resource error')
         elif key == 'extras':
             error_summary[_('Extras')] = _('Missing Value')
         elif key == 'extras_validation':


diff -r 427a990a608e2bc26df3b6477b27d59f1b122e79 -r 8b73510e1eb18712e05a82805d4738dd43ddc3de ckan/tests/functional/api/__init__.py
--- a/ckan/tests/functional/api/__init__.py
+++ b/ckan/tests/functional/api/__init__.py
@@ -1,34 +1,118 @@
+from pprint import pformat
+import copy
+import datetime
+
 from nose.tools import assert_equal
-import copy
 
-def change_lists_to_sets(iterable):
+def flatten_resource_extras(pkg_dict):
+    '''Takes a package dictionary with resource extras under the
+    resource key 'extras' and moves them to be under the 'resource'
+    key itself. The latter format is how packages are expressed when
+    you show a package.
+    '''
+    pkg = copy.deepcopy(pkg_dict)
+    for res in pkg['resources']:
+        for extra_key, extra_value in res['extras'].items():
+            res[extra_key] = extra_value
+        del res['extras']
+    return pkg
+
+def change_lists_to_sets(iterable, recurse=True):
     '''recursive method to drill down into iterables to
     convert any list or tuples into sets. Does not work
     though for dictionaries in lists.'''
     if isinstance(iterable, dict):
-        for key in iterable:
-            if isinstance(iterable[key], (list, tuple)):
-                try:
-                    iterable[key] = set(iterable[key])
-                except TypeError:
-                    # e.g. unhashable
-                    pass
-            elif getattr(iterable[key], '__iter__', False):
-                change_lists_to_sets(iterable[key])
+        if recurse:
+            for key in iterable:
+                if isinstance(iterable[key], (list, tuple)):
+                    try:
+                        iterable[key] = set(iterable[key])
+                    except TypeError:
+                        # e.g. unhashable
+                        pass
+                elif getattr(iterable[key], '__iter__', False):
+                    change_lists_to_sets(iterable[key])
     elif isinstance(iterable, (list, tuple)):
         for item in iterable:
             if isinstance(item, (list, tuple)):
                 iterable.pop(item)
                 iterable.append(set(item))
             elif getattr(item, '__iter__', False):
-                change_lists_to_sets(item)
+                if recurse:
+                    change_lists_to_sets(item)
     else:
         raise NotImplementedError
 
-def assert_dicts_equal_ignoring_ordering(dict1, dict2):
-    '''Asserts dicts are equal, assuming that the ordering of
-    any lists is unimportant.'''                
-    dicts = [copy.deepcopy(dict1), copy.deepcopy(dict2)]
-    for d in dicts:
-        d = change_lists_to_sets(d)
-    assert_equal(dicts[0], dicts[1])
+def assert_iterables_equal(obj1, obj2,
+                           ignore_list_order=False,
+                           keys_to_ignore=[],
+                           _recursion_path=[],
+                           _recursion_errors=''):
+    '''Asserts two iterable structures are equal
+    (or close enough to equal - see options).
+    Iterable structures can be made up of dictionary, lists, tuples. 
+    NB _recursion_path and _recursion_errors are for internal use only.
+    '''
+    objs = [copy.deepcopy(obj1), copy.deepcopy(obj2)]
+    if ignore_list_order:
+        for obj_ in objs:
+            if isinstance(obj_, list):
+                obj_ = change_lists_to_sets(obj_, recurse=False)
+    if objs[0] != objs[1]:
+        if not _recursion_path:
+            # i.e. not recursed at this point
+            _recursion_errors += 'Error - dictionaries do not match'
+            _recursion_errors += '\n%s\n!=\n%s' % (pformat(objs[0]),
+                                                    pformat(objs[1]))
+        # Find the problem
+        context = '\n* (%s):\n    ' % ' / '.join(_recursion_path) if _recursion_path else ''
+
+        if type(objs[0]) != type(objs[1]):
+            _recursion_errors += context + 'Types do not match: %s != %s' % (type(objs[0]), type(objs[1]))
+        elif isinstance(objs[0], dict):
+            # keys
+            ignore = set(keys_to_ignore)
+            keys_missing_in_0 = set(objs[1].keys()) - set(objs[0].keys()) - ignore
+            keys_missing_in_1 = set(objs[0].keys()) - set(objs[1].keys()) - ignore
+            if keys_missing_in_0:
+                _recursion_errors += context + 'Key "%s" only present in second obj' % keys_missing_in_0
+            if keys_missing_in_1:
+                _recursion_errors += context + 'Key "%s" only present in first obj' % keys_missing_in_1
+            if keys_missing_in_0 or keys_missing_in_1:
+                raise AssertionError(_recursion_errors)
+
+            # values
+            for key in set(objs[0].keys()) - ignore:
+                assert_iterables_equal(objs[0][key], objs[1][key],
+                                  ignore_list_order=ignore_list_order,
+                                  keys_to_ignore=keys_to_ignore,
+                                  _recursion_path=_recursion_path + ['Dictionary key "%s"' % key],
+                                  _recursion_errors=_recursion_errors)
+        elif isinstance(objs[0], set):
+            items_missing_in_0 = objs[1] - objs[0]
+            items_missing_in_1 = objs[0] - objs[1]
+            if items_missing_in_0:
+                _recursion_errors += context + 'Item "%s" only present in second set' % items_missing_in_0
+            if items_missing_in_1:
+                _recursion_errors += context + 'Item "%s" only present in first set' % items_missing_in_1
+            raise AssertionError(_recursion_errors)
+        elif isinstance(objs[0], (list, tuple)):
+            if len(objs[0]) != len(objs[1]):
+                _recursion_errors += context + 'List length not equal\n%i != %i' % \
+                                    (len(objs[0]), len(objs[1]))
+                raise AssertionError(_recursion_errors)
+            if objs[0] != objs[1]:
+                for i in range(len(objs[0])):
+                    # recurse
+                    assert_iterables_equal(objs[0][i], objs[1][i],
+                                      ignore_list_order=ignore_list_order,
+                                      keys_to_ignore=keys_to_ignore,
+                                      _recursion_path=_recursion_path + ['List item %i' % i],
+                                      _recursion_errors=_recursion_errors)
+        elif isinstance(objs[0], (basestring, datetime)) or objs[0] is None:
+            if objs[0] != objs[1]:
+                _recursion_errors += context + '%r != %r' % \
+                                    (values[0], values[1])
+                raise AssertionError(_recursion_errors)
+        else:
+            raise NotImplementedError


diff -r 427a990a608e2bc26df3b6477b27d59f1b122e79 -r 8b73510e1eb18712e05a82805d4738dd43ddc3de ckan/tests/functional/api/test_action.py
--- a/ckan/tests/functional/api/test_action.py
+++ b/ckan/tests/functional/api/test_action.py
@@ -1,13 +1,26 @@
 import re
 import json
-from pprint import pprint, pformat
+from pprint import pprint
+import copy
+
 from nose.tools import assert_equal
 
 import ckan
 from ckan.lib.create_test_data import CreateTestData
 import ckan.model as model
 from ckan.tests import WsgiAppCase
-from ckan.tests.functional.api import assert_dicts_equal_ignoring_ordering, change_lists_to_sets
+from ckan.tests.functional.api import flatten_resource_extras, assert_iterables_equal, change_lists_to_sets
+
+# package dict keys that are created when automatically a package
+# is created.
+auto_generated_keys = set(
+    ('revision_id', 'revision_timestamp', 'relationships_as_object',
+     'state', 'id', 'groups', 'relationships_as_subject',
+     u'mimetype', u'resource_group_id', u'name', u'cache_last_updated',
+     u'cache_url', u'webstore_url', u'mimetype_inner',
+     u'webstore_last_updated', u'last_modified', u'resource_type',
+     'package_id'))
+    
 
 class TestAction(WsgiAppCase):
 
@@ -35,11 +48,12 @@
     def test_01_package_list(self):
         postparams = '%s=1' % json.dumps({})
         res = self.app.post('/api/action/package_list', params=postparams)
-        assert_dicts_equal_ignoring_ordering(
+        assert_iterables_equal(
             json.loads(res.body),
             {"help": "Lists packages by name or id",
              "success": True,
-             "result": ["annakarenina", "warandpeace"]})
+             "result": ["annakarenina", "warandpeace"]},
+            ignore_list_order=True)
 
     def test_01_package_show(self):
         anna_id = model.Package.by_name(u'annakarenina').id
@@ -50,8 +64,31 @@
         assert_equal(res_dict['help'], None)
         pkg = res_dict['result']
         assert_equal(pkg['name'], 'annakarenina')
-        missing_keys = set(('title', 'groups')) - set(pkg.keys())
+        missing_keys = set(('title', 'groups', 'extras')) - set(pkg.keys())
         assert not missing_keys, missing_keys
+        assert_iterables_equal(pkg['extras'],
+                               [{'key': 'genre', 'value': 'romantic novel'},
+                                {'key': 'original media', 'value': 'book'}],
+                               ignore_list_order=True,
+                               keys_to_ignore=('id', 'revision_timestamp',
+                                               'revision_id', 'state', 'package_id'))
+        resources = model.Package.by_name(u'annakarenina').resources
+        for resource in resources:
+            if resource.format==u'plain text':
+                full_text_resource = resource
+                break
+        else:
+            assert 0, 'Could not find plain text resource for anna'
+        expected_size = resource.extras['size_extra']
+                             
+        for resource in pkg['resources']:
+            if resource['format'] == u'plain text':
+                assert resource['extras'].has_key('size_extra')
+                assert_equal(resource['extras']['size_extra'], expected_size)
+                break
+        else:
+            assert 0, 'Could not find plain text resource for anna'
+                
 
     def test_01_package_show_with_jsonp(self):
         anna_id = model.Package.by_name(u'annakarenina').id
@@ -78,11 +115,10 @@
         assert res_obj['result'][0]['name'] == 'warandpeace'
 
     def test_03_create_update_package(self):
-
         package = {
             'author': None,
             'author_email': None,
-            'extras': [{'key': u'original media','value': u'"book"'}],
+            'extras': [{'key': u'original media', 'value': u'"book"'}],
             'license_id': u'other-open',
             'maintainer': None,
             'maintainer_email': None,
@@ -108,23 +144,25 @@
             'version': u'0.7a'
         }
 
-        wee = json.dumps(package)
         postparams = '%s=1' % json.dumps(package)
         res = self.app.post('/api/action/package_create', params=postparams,
                             extra_environ={'Authorization': 'tester'})
         package_created = json.loads(res.body)['result']
-        print package_created
+        expected_package = flatten_resource_extras(package)
+        assert_iterables_equal(package_created, expected_package,
+                               ignore_list_order=True,
+                               keys_to_ignore=auto_generated_keys)
+                               
         package_created['name'] = 'moo'
         postparams = '%s=1' % json.dumps(package_created)
         res = self.app.post('/api/action/package_update', params=postparams,
                             extra_environ={'Authorization': 'tester'})
 
         package_updated = json.loads(res.body)['result']
-        package_updated.pop('revision_id')
-        package_updated.pop('revision_timestamp')
-        package_created.pop('revision_id')
-        package_created.pop('revision_timestamp')
-        assert package_updated == package_created#, (pformat(json.loads(res.body)), pformat(package_created['result']))
+        assert_iterables_equal(package_updated, package_created,
+                           ignore_list_order=True,
+                           keys_to_ignore=('revision_id',
+                                           'revision_timestamp'))
 
     def test_18_create_package_not_authorized(self):
 
@@ -140,7 +178,6 @@
             'url': u'http://www.annakarenina.com',
         }
 
-        wee = json.dumps(package)
         postparams = '%s=1' % json.dumps(package)
         res = self.app.post('/api/action/package_create', params=postparams,
                                      status=self.STATUS_403_ACCESS_DENIED)
@@ -218,11 +255,13 @@
     def test_06_tag_list(self):
         postparams = '%s=1' % json.dumps({})
         res = self.app.post('/api/action/tag_list', params=postparams)
-        assert_dicts_equal_ignoring_ordering(
+        assert_iterables_equal(
             json.loads(res.body),
             {'help': 'Returns a list of tags',
              'success': True,
-             'result': ['russian', 'tolstoy']})
+             'result': ['russian', 'tolstoy']},
+            ignore_list_order=True
+            )
         #Get all fields
         postparams = '%s=1' % json.dumps({'all_fields':True})
         res = self.app.post('/api/action/tag_list', params=postparams)
@@ -423,7 +462,7 @@
         postparams = '%s=1' % json.dumps({})
         res = self.app.post('/api/action/group_list', params=postparams)
         res_obj = json.loads(res.body)
-        assert_dicts_equal_ignoring_ordering(
+        assert_iterables_equal(
             res_obj,
             {
                 'result': [
@@ -432,7 +471,8 @@
                     ],
                 'help': 'Returns a list of groups',
                 'success': True
-            })
+            },
+            ignore_list_order=True)
         
         #Get all fields
         postparams = '%s=1' % json.dumps({'all_fields':True})
@@ -578,13 +618,32 @@
         resource_updated = json.loads(res.body)['result']
         assert resource_updated['url'] == new_resource_url, resource_updated
 
-        resource_updated.pop('url')
-        resource_updated.pop('revision_id')
-        resource_updated.pop('revision_timestamp')
-        resource_created.pop('url')
-        resource_created.pop('revision_id')
-        resource_created.pop('revision_timestamp')
-        assert resource_updated == resource_created
+        assert_iterables_equal(resource_updated, resource_created,
+                           ignore_list_order=True,
+                           keys_to_ignore=('url', 'revision_id',
+                                           'revision_timestamp'))
+
+    def test_19_update_resource_validation_error(self):
+        package = {
+            'name': u'validation_error',
+            'resources': [{
+                'description': u'Main sheet.',
+                'extras': {u'last_modified': u'wednesday', # should be iso date
+                           u'testkey': u'testvalue'}, 
+                'format': u'csv',
+                'url': u'http://test.com/sheet.csv'
+            }],
+            'title': u'test19a',
+        }
+
+        postparams = '%s=1' % json.dumps(package)
+        res = self.app.post('/api/action/package_create', params=postparams,
+                            extra_environ={'Authorization': 'tester'})
+        res = json.loads(res.body)
+#        import pdb; pdb.set_trace()
+        assert_equal(res['success'], False)
+        import pdb; pdb.set_trace()
+
 
     def test_20_status_show(self):
         postparams = '%s=1' % json.dumps({})

Repository URL: https://bitbucket.org/okfn/ckan/

--

This is a commit notification from bitbucket.org. You are receiving
this because you have the service enabled, addressing the recipient of
this email.




More information about the ckan-changes mailing list