Merge branch 'master' of github.com:ckan/ckan into 1908-remove-iconve…

…rters

Conflicts:
	setup.py

ckan/controllers/related.py

@@ -177,7 +177,7 @@ def _edit_or_new(self, id, related_id, is_edit):
                     data['id'] = related_id
                 else:
                     data['dataset_id'] = id
-                data['owner_id'] = c.userobj.id
+                    data['owner_id'] = c.userobj.id
 
                 related = logic.get_action(action_name)(context, data)
 

ckan/lib/helpers.py

@@ -598,7 +598,8 @@ def get_facet_items_dict(facet, limit=None, exclude_active=False):
     facets = sorted(facets, key=lambda item: item['count'], reverse=True)
     if c.search_facets_limits and limit is None:
         limit = c.search_facets_limits.get(facet)
-    if limit is not None:
+    # zero treated as infinite for hysterical raisins
+    if limit is not None and limit > 0:
         return facets[:limit]
     return facets
 

ckan/lib/navl/dictization_functions.py

@@ -230,8 +230,12 @@ def validate(data, schema, context=None):
     # empty fields and missing fields when doing partial updates.
     empty_lists = [key for key, value in data.items() if value == []]
 
+    # create a copy of the context which also includes the schema keys so
+    # they can be used by the validators
+    validators_context = dict(context, schema_keys=schema.keys())
+
     flattened = flatten_dict(data)
-    converted_data, errors = _validate(flattened, schema, context)
+    converted_data, errors = _validate(flattened, schema, validators_context)
     converted_data = unflatten(converted_data)
 
     # check config for partial update fix option

ckan/lib/uploader.py

@@ -188,10 +188,27 @@ def get_path(self, id):
         return filepath
 
     def upload(self, id, max_size=10):
+        '''Actually upload the file.
+
+        :returns: ``'file uploaded'`` if a new file was successfully uploaded
+            (whether it overwrote a previously uploaded file or not),
+            ``'file deleted'`` if an existing uploaded file was deleted,
+            or ``None`` if nothing changed
+        :rtype: ``string`` or ``None``
+
+        '''
         if not self.storage_path:
             return
+
+        # Get directory and filepath on the system
+        # where the file for this resource will be stored
         directory = self.get_directory(id)
         filepath = self.get_path(id)
+
+        # If a filename has been provided (a file is being uploaded)
+        # we write it to the filepath (and overwrite it if it already
+        # exists). This way the uploaded file will always be stored
+        # in the same location
         if self.filename:
             try:
                 os.makedirs(directory)
@@ -217,7 +234,13 @@ def upload(self, id, max_size=10):
                     )
             output_file.close()
             os.rename(tmp_filepath, filepath)
+            return
 
+        # The resource form only sets self.clear (via the input clear_upload)
+        # to True when an uploaded file is not replaced by another uploaded
+        # file, only if it is replaced by a link to file.
+        # If the uploaded file is replaced by a link, we should remove the
+        # previously uploaded file to clean up the file system.
         if self.clear:
             try:
                 os.remove(filepath)

ckan/logic/__init__.py

@@ -98,11 +98,17 @@ def prettify(field_name):
                 return _(field_name.replace('_', ' '))
 
             summary = {}
+
             for key, error in error_dict.iteritems():
                 if key == 'resources':
                     summary[_('Resources')] = _('Package resource(s) invalid')
                 elif key == 'extras':
-                    summary[_('Extras')] = _('Missing Value')
+                    errors_extras = []
+                    for item in error:
+                        if (item.get('key')
+                                and item['key'][0] not in errors_extras):
+                            errors_extras.append(item.get('key')[0])
+                    summary[_('Extras')] = ', '.join(errors_extras)
                 elif key == 'extras_validation':
                     summary[_('Extras')] = error[0]
                 elif key == 'tags':

ckan/logic/action/create.py

@@ -268,6 +268,9 @@ def resource_create(context, data_dict):
 
     _check_access('resource_create', context, data_dict)
 
+    for plugin in plugins.PluginImplementations(plugins.IResourceController):
+        plugin.before_create(context, data_dict)
+
     if not 'resources' in pkg_dict:
         pkg_dict['resources'] = []
 
@@ -294,6 +297,9 @@ def resource_create(context, data_dict):
     pkg_dict = _get_action('package_show')(context, {'id': package_id})
     resource = pkg_dict['resources'][-1]
 
+    for plugin in plugins.PluginImplementations(plugins.IResourceController):
+        plugin.after_create(context, resource)
+
     return resource
 
 

ckan/logic/action/delete.py

@@ -97,6 +97,10 @@ def resource_delete(context, data_dict):
     package_id = entity.get_package_id()
 
     pkg_dict = _get_action('package_show')(context, {'id': package_id})
+
+    for plugin in plugins.PluginImplementations(plugins.IResourceController):
+        plugin.before_delete(context, data_dict,
+                             pkg_dict.get('resources', []))
 
     if pkg_dict.get('resources'):
         pkg_dict['resources'] = [r for r in pkg_dict['resources'] if not
@@ -107,6 +111,9 @@ def resource_delete(context, data_dict):
         errors = e.error_dict['resources'][-1]
         raise ValidationError(errors)
 
+    for plugin in plugins.PluginImplementations(plugins.IResourceController):
+        plugin.after_delete(context, pkg_dict.get('resources', []))
+
     model.repo.commit()
 
 

ckan/logic/action/get.py

@@ -577,15 +577,38 @@ def group_list_authz(context, data_dict):
 
 
 def organization_list_for_user(context, data_dict):
-    '''Return the list of organizations that the user is a member of.
+    '''Return the organizations that the user has a given permission for.
+
+    By default this returns the list of organizations that the currently
+    authorized user can edit, i.e. the list of organizations that the user is an
+    admin of.
+
+    Specifically it returns the list of organizations that the currently
+    authorized user has a given permission (for example: "edit_group") against.
+
+    When a user becomes a member of an organization in CKAN they're given a
+    "capacity" (sometimes called a "role"), for example "member", "editor" or
+    "admin".
+
+    Each of these roles has certain permissions associated with it. For example
+    the admin role has the "admin" permission (which means they have permission
+    to do anything). The editor role has permissions like "create_dataset",
+    "update_dataset" and "delete_dataset".  The member role has the "read"
+    permission.
+
+    This function returns the list of organizations that the authorized user has
+    a given permission for. For example the list of organizations that the user
+    is an admin of, or the list of organizations that the user can create
+    datasets in.
 
     :param permission: the permission the user has against the
-        returned organizations (optional, default: ``edit_group``)
+        returned organizations, for example ``"read"`` or ``"create_dataset"``
+        (optional, default: ``"edit_group"``)
     :type permission: string
 
-    :returns: list of dictized organizations that the user is
-        authorized to edit
+    :returns: list of organizations that the user has the given permission for
     :rtype: list of dicts
+
     '''
     model = context['model']
     user = context['user']
@@ -613,7 +636,8 @@ def organization_list_for_user(context, data_dict):
         q = model.Session.query(model.Member) \
             .filter(model.Member.table_name == 'user') \
             .filter(model.Member.capacity.in_(roles)) \
-            .filter(model.Member.table_id == user_id)
+            .filter(model.Member.table_id == user_id) \
+            .filter(model.Member.state == 'active')
 
         group_ids = []
         for row in q.all():

ckan/logic/action/update.py

@@ -225,6 +225,9 @@ def resource_update(context, data_dict):
         logging.error('Could not find resource ' + id)
         raise NotFound(_('Resource was not found.'))
 
+    for plugin in plugins.PluginImplementations(plugins.IResourceController):
+        plugin.before_update(context, pkg_dict['resources'][n], data_dict)
+
     upload = uploader.ResourceUpload(data_dict)
 
     pkg_dict['resources'][n] = data_dict
@@ -240,7 +243,13 @@ def resource_update(context, data_dict):
 
     upload.upload(id, uploader.get_max_resource_size())
     model.repo.commit()
-    return _get_action('resource_show')(context, {'id': id})
+
+    resource = _get_action('resource_show')(context, {'id': id})
+
+    for plugin in plugins.PluginImplementations(plugins.IResourceController):
+        plugin.after_update(context, resource)
+
+    return resource
 
 
 def resource_view_update(context, data_dict):

ckan/logic/converters.py

@@ -7,11 +7,18 @@
 
 from ckan.common import _
 
+
 def convert_to_extras(key, data, errors, context):
-    extras = data.get(('extras',), [])
-    if not extras:
-        data[('extras',)] = extras
-    extras.append({'key': key[-1], 'value': data[key]})
+
+    # Get the current extras index
+    current_indexes = [k[1] for k in data.keys()
+                       if len(k) > 1 and k[0] == 'extras']
+
+    new_index = max(current_indexes) + 1 if current_indexes else 0
+
+    data[('extras', new_index, 'key')] = key[-1]
+    data[('extras', new_index, 'value')] = data[key]
+
 
 def convert_from_extras(key, data, errors, context):
 

ckan/logic/schema.py

@@ -54,6 +54,7 @@
                                    if_empty_guess_format,
                                    clean_format,
                                    no_loops_in_hierarchy,
+                                   extra_key_not_in_root_schema,
                                    )
 from ckan.logic.converters import (convert_user_name_or_id_to_id,
                                    convert_package_name_or_id_to_id,
@@ -373,7 +374,7 @@ def default_extras_schema():
 
     schema = {
         'id': [ignore],
-        'key': [not_empty, unicode],
+        'key': [not_empty, extra_key_not_in_root_schema, unicode],
         'value': [not_missing],
         'state': [ignore],
         'deleted': [ignore_missing],

ckan/logic/validators.py

@@ -784,3 +784,9 @@ def no_loops_in_hierarchy(key, data, errors, context):
             raise Invalid(_('This parent would create a loop in the '
                             'hierarchy'))
 
+
+def extra_key_not_in_root_schema(key, data, errors, context):
+
+    for schema_key in context.get('schema_keys', []):
+        if schema_key == data[key]:
+            raise Invalid(_('There is a schema field with the same name'))

ckan/model/modification.py

@@ -32,7 +32,12 @@ def notify_observers(self, func):
 
 
     def before_commit(self, session):
+        self.notify_observers(session, self.notify)
 
+    def after_commit(self, session):
+        self.notify_observers(session, self.notify_after_commit)
+
+    def notify_observers(self, session, method):
         session.flush()
         if not hasattr(session, '_object_cache'):
             return
@@ -44,13 +49,13 @@ def before_commit(self, session):
 
         for obj in set(new):
             if isinstance(obj, (_package.Package, resource.Resource)):
-                self.notify(obj, domain_object.DomainObjectOperation.new)
+                method(obj, domain_object.DomainObjectOperation.new)
         for obj in set(deleted):
             if isinstance(obj, (_package.Package, resource.Resource)):
-                self.notify(obj, domain_object.DomainObjectOperation.deleted)
+                method(obj, domain_object.DomainObjectOperation.deleted)
         for obj in set(changed):
             if isinstance(obj, resource.Resource):
-                self.notify(obj, domain_object.DomainObjectOperation.changed)
+                method(obj, domain_object.DomainObjectOperation.changed)
             if getattr(obj, 'url_changed', False):
                 for item in plugins.PluginImplementations(plugins.IResourceUrlChange):
                     item.notify(obj)
@@ -69,7 +74,7 @@ def before_commit(self, session):
                     if package and package not in deleted | new:
                         changed_pkgs.add(package)
         for obj in changed_pkgs:
-            self.notify(obj, domain_object.DomainObjectOperation.changed)
+            method(obj, domain_object.DomainObjectOperation.changed)
 
 
     def notify(self, entity, operation):
@@ -82,3 +87,14 @@ def notify(self, entity, operation):
                 # We reraise all exceptions so they are obvious there
                 # is something wrong
                 raise
+
+    def notify_after_commit(self, entity, operation):
+        for observer in plugins.PluginImplementations(
+                plugins.IDomainObjectModification):
+            try:
+                observer.notify_after_commit(entity, operation)
+            except Exception, ex:
+                log.exception(ex)
+                # We reraise all exceptions so they are obvious there
+                # is something wrong
+                raise

ckan/new_tests/lib/navl/test_dictization_functions.py

@@ -0,0 +1,49 @@
+import nose
+from ckan.lib.navl.dictization_functions import validate
+
+
+eq_ = nose.tools.eq_
+
+
+class TestValidate(object):
+
+    def test_validate_passes_a_copy_of_the_context_to_validators(self):
+
+        # We need to pass some keys on the context, otherwise validate
+        # will do context = context || {}, which creates a new one, defeating
+        # the purpose of this test
+        context = {'foo': 'bar'}
+
+        def my_validator(key, data, errors, context_in_validator):
+
+            assert not id(context) == id(context_in_validator)
+
+        data_dict = {
+            'my_field': 'test',
+        }
+
+        schema = {
+            'my_field': [my_validator],
+        }
+
+        data, errors = validate(data_dict, schema, context)
+
+    def test_validate_adds_schema_keys_to_context(self):
+
+        def my_validator(key, data, errors, context):
+
+            assert 'schema_keys' in context
+
+            eq_(context['schema_keys'], ['my_field'])
+
+        data_dict = {
+            'my_field': 'test',
+        }
+
+        schema = {
+            'my_field': [my_validator],
+        }
+
+        context = {}
+
+        data, errors = validate(data_dict, schema, context)