diff --git a/ckan/controllers/related.py b/ckan/controllers/related.py index ed24857aed1..ec067e0043e 100644 --- a/ckan/controllers/related.py +++ b/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) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 68c5eb4e7f3..fa2a34d570a 100644 --- a/ckan/lib/helpers.py +++ b/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 diff --git a/ckan/lib/navl/dictization_functions.py b/ckan/lib/navl/dictization_functions.py index f21eaf89764..bae614adfb7 100644 --- a/ckan/lib/navl/dictization_functions.py +++ b/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 diff --git a/ckan/lib/uploader.py b/ckan/lib/uploader.py index 8456e24f420..fcd8bdb19f1 100644 --- a/ckan/lib/uploader.py +++ b/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) diff --git a/ckan/logic/__init__.py b/ckan/logic/__init__.py index 4d7c49d1a56..c759b3e0081 100644 --- a/ckan/logic/__init__.py +++ b/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': diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 0cbae7ea10f..76e38cc7f55 100644 --- a/ckan/logic/action/create.py +++ b/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 diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index 88690a409c9..bcc4e9602e1 100644 --- a/ckan/logic/action/delete.py +++ b/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() diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 4020e9384aa..bb71675c2d7 100644 --- a/ckan/logic/action/get.py +++ b/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(): diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 6975034212a..45c4035a2c8 100644 --- a/ckan/logic/action/update.py +++ b/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): diff --git a/ckan/logic/converters.py b/ckan/logic/converters.py index b2a7f45209e..1388bb6e42b 100644 --- a/ckan/logic/converters.py +++ b/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): diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index d32eb8de647..841c8762319 100644 --- a/ckan/logic/schema.py +++ b/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], diff --git a/ckan/logic/validators.py b/ckan/logic/validators.py index 9c649f33a3f..cc07f7e7ab5 100644 --- a/ckan/logic/validators.py +++ b/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')) diff --git a/ckan/model/modification.py b/ckan/model/modification.py index 29ac624c200..585d771198b 100644 --- a/ckan/model/modification.py +++ b/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 diff --git a/ckan/new_tests/lib/navl/test_dictization_functions.py b/ckan/new_tests/lib/navl/test_dictization_functions.py new file mode 100644 index 00000000000..226d6b3ef38 --- /dev/null +++ b/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) diff --git a/ckan/new_tests/logic/action/test_get.py b/ckan/new_tests/logic/action/test_get.py index 143dc64c4d8..1692ff3494e 100644 --- a/ckan/new_tests/logic/action/test_get.py +++ b/ckan/new_tests/logic/action/test_get.py @@ -459,3 +459,302 @@ def test_package_search_facet_field_is_json(self): nose.tools.assert_raises( logic.ValidationError, helpers.call_action, 'package_search', **kwargs) + + +class TestOrganizationListForUser(object): + '''Functional tests for the organization_list_for_user() action function.''' + + def setup(self): + helpers.reset_db() + search.clear() + + def test_when_user_is_not_a_member_of_any_organizations(self): + """ + + When the user isn't a member of any organizations (in any capacity) + organization_list_for_user() should return an empty list. + + """ + user = factories.User() + context = {'user': user['name']} + + # Create an organization so we can test that it does not get returned. + factories.Organization() + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert organizations == [] + + def test_when_user_is_an_admin_of_one_organization(self): + """ + + When the user is an admin of one organization + organization_list_for_user() should return a list of just that one + organization. + + """ + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + + # Create a second organization just so we can test that it does not get + # returned. + factories.Organization() + + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='admin') + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert len(organizations) == 1 + assert organizations[0]['id'] == organization['id'] + + def test_when_user_is_an_admin_of_three_organizations(self): + """ + + When the user is an admin of three organizations + organization_list_for_user() should return a list of all three + organizations. + + """ + user = factories.User() + context = {'user': user['name']} + organization_1 = factories.Organization() + organization_2 = factories.Organization() + organization_3 = factories.Organization() + + # Create a second organization just so we can test that it does not get + # returned. + factories.Organization() + + # Make the user an admin of all three organizations: + for organization in (organization_1, organization_2, organization_3): + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='admin') + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert len(organizations) == 3 + ids = [organization['id'] for organization in organizations] + for organization in (organization_1, organization_2, organization_3): + assert organization['id'] in ids + + def test_does_not_return_members(self): + """ + + By default organization_list_for_user() should not return organizations + that the user is just a member (not an admin) of. + + """ + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='member') + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert organizations == [] + + def test_does_not_return_editors(self): + """ + + By default organization_list_for_user() should not return organizations + that the user is just an editor (not an admin) of. + + """ + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='editor') + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert organizations == [] + + def test_editor_permission(self): + """ + + organization_list_for_user() should return organizations that the user + is an editor of if passed a permission that belongs to the editor role. + + """ + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='editor') + + organizations = helpers.call_action('organization_list_for_user', + permission='create_dataset', + context=context) + + assert [org['id'] for org in organizations] == [organization['id']] + + def test_member_permission(self): + """ + + organization_list_for_user() should return organizations that the user + is a member of if passed a permission that belongs to the member role. + + """ + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='member') + + organizations = helpers.call_action('organization_list_for_user', + permission='read', + context=context) + + assert [org['id'] for org in organizations] == [organization['id']] + + def test_invalid_permission(self): + ''' + + organization_list_for_user() should return an empty list if passed a + non-existent or invalid permission. + + Note that we test this with a user who is an editor of one organization. + If the user was an admin of the organization then it would return that + organization - admins have all permissions, including permissions that + don't exist. + + ''' + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + factories.Organization() + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='editor') + + for permission in ('', ' ', 'foo', 27.3, 5, True, False, None): + organizations = helpers.call_action('organization_list_for_user', + permission=permission, + context=context) + + assert organizations == [] + + def test_that_it_does_not_return_groups(self): + """ + + organization_list_for_user() should not return groups that the user is + a member, editor or admin of. + + """ + user = factories.User() + context = {'user': user['name']} + group_1 = factories.Group() + group_2 = factories.Group() + group_3 = factories.Group() + helpers.call_action('member_create', id=group_1['id'], + object=user['id'], object_type='user', + capacity='member') + helpers.call_action('member_create', id=group_2['id'], + object=user['id'], object_type='user', + capacity='editor') + helpers.call_action('member_create', id=group_3['id'], + object=user['id'], object_type='user', + capacity='admin') + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert organizations == [] + + def test_that_it_does_not_return_previous_memberships(self): + """ + + organization_list_for_user() should return organizations that the user + was previously an admin of. + + """ + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + + # Make the user an admin of the organization. + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='admin') + + # Remove the user from the organization. + helpers.call_action('member_delete', id=organization['id'], + object=user['id'], object_type='user') + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert organizations == [] + + def test_when_user_is_sysadmin(self): + """ + + When the user is a sysadmin organization_list_for_user() should just + return all organizations, even if the user is not a member of them. + + """ + user = factories.Sysadmin() + context = {'user': user['name']} + organization = factories.Organization() + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert [org['id'] for org in organizations] == [organization['id']] + + def test_that_it_does_not_return_deleted_organizations(self): + """ + + organization_list_for_user() should not return deleted organizations + that the user was an admin of. + + """ + user = factories.User() + context = {'user': user['name']} + organization = factories.Organization() + + # Make the user an admin of the organization. + helpers.call_action('member_create', id=organization['id'], + object=user['id'], object_type='user', + capacity='admin') + + # Delete the organization. + helpers.call_action('organization_delete', id=organization['id']) + + organizations = helpers.call_action('organization_list_for_user', + context=context) + + assert organizations == [] + + def test_with_no_authorized_user(self): + """ + + organization_list_for_user() should return an empty list if there's no + authorized user. Users who aren't logged-in don't have any permissions. + + """ + # Create an organization so we can test that it doesn't get returned. + organization = factories.Organization() + + organizations = helpers.call_action('organization_list_for_user') + + assert organizations == [] diff --git a/ckan/new_tests/logic/test_conversion.py b/ckan/new_tests/logic/test_conversion.py new file mode 100644 index 00000000000..fad4c58da63 --- /dev/null +++ b/ckan/new_tests/logic/test_conversion.py @@ -0,0 +1,152 @@ +'''Functional tests for converters in ckan/logic/converters.py. + +''' +import nose + +from ckan import model +import ckan.plugins as p +import ckan.lib.plugins as lib_plugins +from ckan.lib.navl.dictization_functions import validate +from ckan.logic.schema import default_extras_schema +from ckan.logic.converters import convert_to_extras + + +eq_ = nose.tools.eq_ + + +class TestConvertToExtras(object): + + def test_convert_to_extras_field_gets_stored_as_extra(self): + + data_dict = { + 'custom_text': 'Hi', + } + + context = { + 'model': model, + 'session': model.Session, + } + + schema = { + 'custom_text': [convert_to_extras], + 'extras': default_extras_schema(), + } + + data, errors = validate(data_dict, schema, context) + + assert 'extras' in data + eq_(len(data['extras']), 1) + eq_(data['extras'][0]['key'], 'custom_text') + eq_(data['extras'][0]['value'], 'Hi') + + def test_convert_to_extras_field_can_be_combined_with_a_proper_extra(self): + + data_dict = { + 'custom_text': 'Hi', + 'extras': [ + {'key': 'proper_extra', 'value': 'Bye'}, + + ] + } + + schema = { + 'custom_text': [convert_to_extras], + 'extras': default_extras_schema(), + } + + context = { + 'model': model, + 'session': model.Session, + } + + data, errors = validate(data_dict, schema, context) + + assert 'extras' in data + eq_(len(data['extras']), 2) + eq_(sorted([e['key'] for e in data['extras']]), + ['custom_text', 'proper_extra']) + eq_(sorted([e['value'] for e in data['extras']]), + ['Bye', 'Hi']) + + def test_convert_to_extras_field_can_be_combined_with_more_extras(self): + + data_dict = { + 'custom_text': 'Hi', + 'extras': [ + {'key': 'proper_extra', 'value': 'Bye'}, + {'key': 'proper_extra2', 'value': 'Bye2'}, + ] + } + + schema = { + 'custom_text': [convert_to_extras], + 'extras': default_extras_schema(), + } + + context = { + 'model': model, + 'session': model.Session, + } + + data, errors = validate(data_dict, schema, context) + + assert 'extras' in data + eq_(len(data['extras']), 3) + eq_(sorted([e['key'] for e in data['extras']]), + ['custom_text', 'proper_extra', 'proper_extra2']) + eq_(sorted([e['value'] for e in data['extras']]), + ['Bye', 'Bye2', 'Hi']) + + def test_convert_to_extras_field_can_be_combined_with_extras_deleted(self): + + data_dict = { + 'custom_text': 'Hi', + 'extras': [ + {'key': 'proper_extra', 'value': 'Bye', 'deleted': True}, + {'key': 'proper_extra2', 'value': 'Bye2'}, + ] + } + + schema = { + 'custom_text': [convert_to_extras], + 'extras': default_extras_schema(), + } + + context = { + 'model': model, + 'session': model.Session, + } + + data, errors = validate(data_dict, schema, context) + + assert 'extras' in data + eq_(len(data['extras']), 3) + eq_(sorted([e['key'] for e in data['extras']]), + ['custom_text', 'proper_extra', 'proper_extra2']) + eq_(sorted([e['value'] for e in data['extras']]), + ['Bye', 'Bye2', 'Hi']) + + def test_convert_to_extras_free_extra_can_not_have_the_same_key(self): + + data_dict = { + 'custom_text': 'Hi', + 'extras': [ + {'key': 'custom_text', 'value': 'Bye'}, + ] + } + + schema = { + 'custom_text': [convert_to_extras], + 'extras': default_extras_schema(), + } + + context = { + 'model': model, + 'session': model.Session, + } + + data, errors = validate(data_dict, schema, context) + + assert 'extras' in errors + eq_(errors['extras'], + [{'key': [u'There is a schema field with the same name']}]) diff --git a/ckan/new_tests/logic/test_converters.py b/ckan/new_tests/logic/test_converters.py index 49a6df7775e..7ea86c4c1dd 100644 --- a/ckan/new_tests/logic/test_converters.py +++ b/ckan/new_tests/logic/test_converters.py @@ -2,10 +2,14 @@ '''Unit tests for ckan/logic/converters.py. ''' +import nose import unittest import ckan.logic.converters as converters +eq_ = nose.tools.eq_ + + class TestRemoveWhitespaceConverter(unittest.TestCase): def test_leading_space(self): string = ' http://example.com' @@ -27,6 +31,52 @@ def test_space_between(self): def test_not_a_string(self): string = 12345 - expected = 12345 converted = converters.remove_whitespace(string, {}) self.assertEqual(string, converted) + + +class TestConvertToExtras(unittest.TestCase): + + def test_convert_to_extras_output_unflattened(self): + + key = ('test_field',) + data = { + ('test_field',): 'test_value', + } + errors = {} + context = {} + + converters.convert_to_extras(key, data, errors, context) + + eq_(data[('extras', 0, 'key')], 'test_field') + eq_(data[('extras', 0, 'value')], 'test_value') + + assert not ('extras',) in data + + eq_(errors, {}) + + def test_convert_to_extras_output_unflattened_with_correct_index(self): + + key = ('test_field',) + data = { + ('test_field',): 'test_value', + ('extras', 0, 'deleted'): '', + ('extras', 0, 'id'): '', + ('extras', 0, 'key'): 'proper_extra', + ('extras', 0, 'revision_timestamp'): '', + ('extras', 0, 'state'): '', + ('extras', 0, 'value'): 'proper_extra_value', + } + errors = {} + context = {} + + converters.convert_to_extras(key, data, errors, context) + + eq_(data[('extras', 0, 'key')], 'proper_extra') + eq_(data[('extras', 0, 'value')], 'proper_extra_value') + eq_(data[('extras', 1, 'key')], 'test_field') + eq_(data[('extras', 1, 'value')], 'test_value') + + assert not ('extras',) in data + + eq_(errors, {}) diff --git a/ckan/plugins/interfaces.py b/ckan/plugins/interfaces.py index aa0e6af0c07..761f3ce79c7 100644 --- a/ckan/plugins/interfaces.py +++ b/ckan/plugins/interfaces.py @@ -192,6 +192,9 @@ class IDomainObjectModification(Interface): def notify(self, entity, operation): pass + def notify_after_commit(self, entity, operation): + pass + class IResourceUrlChange(Interface): """ @@ -520,10 +523,107 @@ class IResourceController(Interface): Hook into the resource controller. """ + def before_create(self, context, resource): + """ + Extensions will receive this before a resource is created. + + :param context: The context object of the current request, this + includes for example access to the ``model`` and the ``user``. + :type context: dictionary + :param resource: An object representing the resource to be added + to the dataset (the one that is about to be created). + :type resource: dictionary + """ + pass + + def after_create(self, context, resource): + """ + Extensions will receive this after a resource is created. + + :param context: The context object of the current request, this + includes for example access to the ``model`` and the ``user``. + :type context: dictionary + :param resource: An object representing the latest resource added + to the dataset (the one that was just created). A key in the + resource dictionary worth mentioning is ``url_type`` which is + set to ``upload`` when the resource file is uploaded instead + of linked. + :type resource: dictionary + """ + pass + + def before_update(self, context, current, resource): + """ + Extensions will receive this before a resource is updated. + + :param context: The context object of the current request, this + includes for example access to the ``model`` and the ``user``. + :type context: dictionary + :param current: The current resource which is about to be updated + :type current: dictionary + :param resource: An object representing the updated resource which + will replace the ``current`` one. + :type resource: dictionary + """ + pass + + def after_update(self, context, resource): + """ + Extensions will receive this after a resource is updated. + + :param context: The context object of the current request, this + includes for example access to the ``model`` and the ``user``. + :type context: dictionary + :param resource: An object representing the updated resource in + the dataset (the one that was just updated). As with + ``after_create``, a noteworthy key in the resource dictionary + ``url_type`` which is set to ``upload`` when the resource file + is uploaded instead of linked. + :type resource: dictionary + """ + pass + + def before_delete(self, context, resource, resources): + """ + Extensions will receive this before a previously created resource is + deleted. + + :param context: The context object of the current request, this + includes for example access to the ``model`` and the ``user``. + :type context: dictionary + :param resource: An object representing the resource that is about + to be deleted. This is a dictionary with one key: ``id`` which + holds the id ``string`` of the resource that should be deleted. + :type resource: dictionary + :param resources: The list of resources from which the resource will + be deleted (including the resource to be deleted if it existed + in the package). + :type resources: list + """ + pass + + def after_delete(self, context, resources): + """ + Extensions will receive this after a previously created resource is + deleted. + + :param context: The context object of the current request, this + includes for example access to the ``model`` and the ``user``. + :type context: dictionary + :param resources: A list of objects representing the remaining + resources after a resource has been removed. + :type resource: list + """ + pass + def before_show(self, resource_dict): ''' - Extensions will receive the validated data dict before the resource - is ready for display. + Extensions will receive the validated data dict before the resource + is ready for display. + + Be aware that this method is not only called for UI display, but also + in other methods like when a resource is deleted because showing a + package is used to get access to the resources in a package. ''' return resource_dict diff --git a/ckan/plugins/toolkit.py b/ckan/plugins/toolkit.py index 1a76a6edee1..d9826368122 100644 --- a/ckan/plugins/toolkit.py +++ b/ckan/plugins/toolkit.py @@ -32,6 +32,7 @@ class _Toolkit(object): 'get_validator', # get navl schema validator 'check_access', # check logic function authorisation 'navl_validate', # implements validate method with navl schema + 'missing', # placeholder for missing values for validation 'ObjectNotFound', # action not found exception # (ckan.logic.NotFound) 'NotAuthorized', # action not authorized exception @@ -160,6 +161,7 @@ def _initialize(self): t['get_validator'] = logic.get_validator t['check_access'] = logic.check_access t['navl_validate'] = dictization_functions.validate + t['missing'] = dictization_functions.missing t['ObjectNotFound'] = logic.NotFound # Name change intentional t['NotAuthorized'] = logic.NotAuthorized t['ValidationError'] = logic.ValidationError diff --git a/ckan/templates/snippets/organization.html b/ckan/templates/snippets/organization.html index 398c5073cdd..8dcdebe9bc8 100644 --- a/ckan/templates/snippets/organization.html +++ b/ckan/templates/snippets/organization.html @@ -14,7 +14,9 @@ #} -{% with truncate=truncate or 0, url=h.url_for(controller='organization', action='read', id=organization.name) %} +{% set truncate = truncate or 0 %} +{% set url = h.url_for(controller='organization', action='read', id=organization.name) %} + {% block info %}
{% if has_context_title %} @@ -65,4 +67,3 @@

{{ organization.title or organization.name }}

{% endblock %} -{% endwith %} diff --git a/ckan/templates/user/login.html b/ckan/templates/user/login.html index 06fd18a577a..62a8d4b1a25 100644 --- a/ckan/templates/user/login.html +++ b/ckan/templates/user/login.html @@ -24,7 +24,7 @@

{% block page_heading %}{{ _('Login') }}{% endblock %}< {% block help_register_inner %}

{{ _('Need an Account?') }}

-

{% trans %}Then sign right up, it only takes a minute.{% endtrans %}

ยง +

{% trans %}Then sign right up, it only takes a minute.{% endtrans %}

{% block help_register_button %} {{ _('Create an Account') }} diff --git a/ckan/tests/functional/test_related.py b/ckan/tests/functional/test_related.py index 9e65917fb2a..15b34dd2809 100644 --- a/ckan/tests/functional/test_related.py +++ b/ckan/tests/functional/test_related.py @@ -346,6 +346,33 @@ def test_non_sysadmin_can_update_related_item(self): result = logic.get_action('related_update')(context, result) assert_equal(result['title'], 'New Title') + def test_update_related_item_check_owner_status(self): + '''After edit of a related item by a sysadmin, check that the owner id is unchanged + ''' + offset = h.url_for(controller='related', + action='new', id='warandpeace') + data = { + "title": "testing_create", + "url": u"http://ckan.org/feed/", + } + user = model.User.by_name('tester') + admin = model.User.by_name('testsysadmin') + + #create related item + context = dict(model=model, user=user.name, session=model.Session) + data_dict = dict(title="testing_create",description="description", + url="http://ckan.org/feed/",image_url="",type="visualization") + res = logic.get_action("related_create")( context, data_dict ) + + #edit related item + data_dict = dict(id=res['id'],title="testing_update",description="description", + url="http://ckan.org/feed/",image_url="",type="visualization") + + context = dict(model=model, user=admin.name, session=model.Session) + result = logic.get_action('related_update')(context,data_dict) + #Confirm related item owner status + assert result['owner_id'] == user.id + def test_related_show(self): rel = self._related_create("Title", "Description", "visualization", diff --git a/ckanext/datastore/plugin.py b/ckanext/datastore/plugin.py index 09000c92d2d..aae4ee70d1f 100644 --- a/ckanext/datastore/plugin.py +++ b/ckanext/datastore/plugin.py @@ -23,6 +23,22 @@ ValidationError = p.toolkit.ValidationError +def _is_legacy_mode(config): + ''' + Decides if the DataStore should run on legacy mode + + Returns True if `ckan.datastore.read_url` is not set in the provided + config object or CKAN is running on Postgres < 9.x + ''' + write_url = config.get('ckan.datastore.write_url') + + engine = db._get_engine({'connection_url': write_url}) + connection = engine.connect() + + return (not config.get('ckan.datastore.read_url') or + not db._pg_version_is_at_least(connection, '9.0')) + + class DatastoreException(Exception): pass @@ -62,7 +78,7 @@ def configure(self, config): # Legacy mode means that we have no read url. Consequently sql search is not # available and permissions do not have to be changed. In legacy mode, the # datastore runs on PG prior to 9.0 (for example 8.4). - self.legacy_mode = 'ckan.datastore.read_url' not in self.config + self.legacy_mode = _is_legacy_mode(self.config) datapusher_formats = config.get('datapusher.formats', '').split() self.datapusher_formats = datapusher_formats or DEFAULT_FORMATS diff --git a/ckanext/datastore/tests/test_unit.py b/ckanext/datastore/tests/test_unit.py index ad2fad16773..3789ccc8503 100644 --- a/ckanext/datastore/tests/test_unit.py +++ b/ckanext/datastore/tests/test_unit.py @@ -1,9 +1,13 @@ import unittest import pylons import nose +import mock + +from pylons import config import ckan.tests as tests import ckanext.datastore.db as db +import ckanext.datastore.plugin as plugin class TestTypeGetters(unittest.TestCase): @@ -35,3 +39,52 @@ def test_pg_version_check(self): connection = engine.connect() assert db._pg_version_is_at_least(connection, '8.0') assert not db._pg_version_is_at_least(connection, '10.0') + + +class TestLegacyModeSetting(): + + @mock.patch('ckanext.datastore.db._pg_version_is_at_least') + def test_legacy_mode_set_if_no_read_url_and_pg_9(self, pgv): + + pgv.return_value = True + + test_config = { + 'ckan.datastore.write_url': config['ckan.datastore.write_url'], + } + + assert plugin._is_legacy_mode(test_config) + + @mock.patch('ckanext.datastore.db._pg_version_is_at_least') + def test_legacy_mode_set_if_no_read_url_and_pg_8(self, pgv): + + pgv.return_value = False + + test_config = { + 'ckan.datastore.write_url': config['ckan.datastore.write_url'], + } + + assert plugin._is_legacy_mode(test_config) + + @mock.patch('ckanext.datastore.db._pg_version_is_at_least') + def test_legacy_mode_set_if_read_url_and_pg_8(self, pgv): + + pgv.return_value = False + + test_config = { + 'ckan.datastore.write_url': config['ckan.datastore.write_url'], + 'ckan.datastore.read_url': 'some_test_read_url', + } + + assert plugin._is_legacy_mode(test_config) + + @mock.patch('ckanext.datastore.db._pg_version_is_at_least') + def test_legacy_mode_not_set_if_read_url_and_pg_9(self, pgv): + + pgv.return_value = True + + test_config = { + 'ckan.datastore.write_url': config['ckan.datastore.write_url'], + 'ckan.datastore.read_url': 'some_test_read_url', + } + + assert not plugin._is_legacy_mode(test_config) diff --git a/ckanext/example_idatasetform/templates/package/snippets/package_metadata_fields.html b/ckanext/example_idatasetform/templates/package/snippets/package_metadata_fields.html index 50271bd638d..80614c3c590 100644 --- a/ckanext/example_idatasetform/templates/package/snippets/package_metadata_fields.html +++ b/ckanext/example_idatasetform/templates/package/snippets/package_metadata_fields.html @@ -1,9 +1,7 @@ {% ckan_extends %} -{# Remove 'free extras' from the package form. If you're using -convert_to/from_extras() as we are with our 'custom_text' field below then -you need to remove free extras from the form, or editing your custom field -won't work. #} +{# Remove 'free extras' from the package form, as we don't want to support + them on our example. #} {% block custom_fields %} {% endblock %} diff --git a/ckanext/example_iresourcecontroller/__init__.py b/ckanext/example_iresourcecontroller/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ckanext/example_iresourcecontroller/plugin.py b/ckanext/example_iresourcecontroller/plugin.py new file mode 100644 index 00000000000..1330f23887b --- /dev/null +++ b/ckanext/example_iresourcecontroller/plugin.py @@ -0,0 +1,31 @@ +from collections import defaultdict +import ckan.plugins as plugins + + +class ExampleIResourceControllerPlugin(plugins.SingletonPlugin): + + plugins.implements(plugins.IResourceController) + + def __init__(self, *args, **kwargs): + self.counter = defaultdict(int) + + def before_create(self, context, resource): + self.counter['before_create'] += 1 + + def after_create(self, context, resource): + self.counter['after_create'] += 1 + + def before_update(self, context, current, resource): + self.counter['before_update'] += 1 + + def after_update(self, context, resource): + self.counter['after_update'] += 1 + + def before_delete(self, context, resource, resources): + self.counter['before_delete'] += 1 + + def after_delete(self, context, resources): + self.counter['after_delete'] += 1 + + def before_show(self, resource): + self.counter['before_show'] += 1 diff --git a/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py b/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py new file mode 100644 index 00000000000..c254331faf0 --- /dev/null +++ b/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py @@ -0,0 +1,118 @@ +'''Tests for the ckanext.example_iauthfunctions extension. + +''' +import pylons.config as config +import webtest + +import ckan.model as model +import ckan.tests as tests + +import ckan.plugins +import ckan.new_tests.factories as factories +import ckan.new_tests.helpers as helpers +from ckanext.example_iresourcecontroller import plugin + + +class TestExampleIResourceController(object): + '''Tests for the plugin that uses IResourceController. + + ''' + + def setup(self): + # Set up the test app + self.app = ckan.config.middleware.make_app( + config['global_conf'], **config) + self.app = webtest.TestApp(self.app) + + def teardown(self): + # Unload the plugin + ckan.plugins.unload('example_iresourcecontroller') + model.repo.rebuild_db() + + def test_resource_controller_plugin_create(self): + user = factories.Sysadmin() + package = factories.Dataset(user=user) + + # Set up the plugin + ckan.plugins.load('example_iresourcecontroller') + plugin = ckan.plugins.get_plugin('example_iresourcecontroller') + + res = helpers.call_action('resource_create', + package_id=package['id'], + name='test-resource', + url='http://resource.create/', + apikey=user['apikey']) + + assert plugin.counter['before_create'] == 1, plugin.counter + assert plugin.counter['after_create'] == 1, plugin.counter + assert plugin.counter['before_update'] == 0, plugin.counter + assert plugin.counter['after_update'] == 0, plugin.counter + assert plugin.counter['before_delete'] == 0, plugin.counter + assert plugin.counter['after_delete'] == 0, plugin.counter + + def test_resource_controller_plugin_update(self): + user = factories.Sysadmin() + resource = factories.Resource(user=user) + + # Set up the plugin here because we don't want the resource creation + # to affect it (because we will only check for changes to update) + ckan.plugins.load('example_iresourcecontroller') + plugin = ckan.plugins.get_plugin('example_iresourcecontroller') + + res = helpers.call_action('resource_update', + id=resource['id'], + url='http://resource.updated/', + apikey=user['apikey']) + + assert plugin.counter['before_create'] == 0, plugin.counter + assert plugin.counter['after_create'] == 0, plugin.counter + assert plugin.counter['before_update'] == 1, plugin.counter + assert plugin.counter['after_update'] == 1, plugin.counter + assert plugin.counter['before_delete'] == 0, plugin.counter + assert plugin.counter['after_delete'] == 0, plugin.counter + + def test_resource_controller_plugin_delete(self): + user = factories.Sysadmin() + resource = factories.Resource(user=user) + + # Set up the plugin here because we don't want the resource creation + # to affect it (because we will only check for changes to delete) + ckan.plugins.load('example_iresourcecontroller') + plugin = ckan.plugins.get_plugin('example_iresourcecontroller') + + res = helpers.call_action('resource_delete', + id=resource['id'], + apikey=user['apikey']) + + assert plugin.counter['before_create'] == 0, plugin.counter + assert plugin.counter['after_create'] == 0, plugin.counter + assert plugin.counter['before_update'] == 0, plugin.counter + assert plugin.counter['after_update'] == 0, plugin.counter + assert plugin.counter['before_delete'] == 1, plugin.counter + assert plugin.counter['after_delete'] == 1, plugin.counter + + def test_resource_controller_plugin_show(self): + """ + Before show gets called by the other methods but we test it + separately here and make sure that it doesn't call the other + methods. + """ + user = factories.Sysadmin() + package = factories.Dataset(user=user) + resource = factories.Resource(user=user, package_id=package['id']) + + # Set up the plugin here because we don't want the resource creation + # to affect it (because we will only check for changes to delete) + ckan.plugins.load('example_iresourcecontroller') + plugin = ckan.plugins.get_plugin('example_iresourcecontroller') + + res = helpers.call_action('package_show', + name_or_id=package['id']) + + assert plugin.counter['before_create'] == 0, plugin.counter + assert plugin.counter['after_create'] == 0, plugin.counter + assert plugin.counter['before_update'] == 0, plugin.counter + assert plugin.counter['after_update'] == 0, plugin.counter + assert plugin.counter['before_delete'] == 0, plugin.counter + assert plugin.counter['after_delete'] == 0, plugin.counter + assert plugin.counter['before_show'] == 1, plugin.counter diff --git a/ckanext/multilingual/plugin.py b/ckanext/multilingual/plugin.py index a4ee31e3b48..f754b10af04 100644 --- a/ckanext/multilingual/plugin.py +++ b/ckanext/multilingual/plugin.py @@ -3,7 +3,7 @@ from ckan.plugins import SingletonPlugin, implements, IPackageController from ckan.plugins import IGroupController, IOrganizationController, ITagController import pylons -import ckan.logic.action.get as action_get +from ckan.logic import get_action from pylons import config LANGS = ['en', 'fr', 'de', 'es', 'it', 'nl', 'ro', 'pt', 'pl'] @@ -35,7 +35,7 @@ def translate_data_dict(data_dict): terms.add(item) # Get the translations of all the terms (as a list of dictionaries). - translations = ckan.logic.action.get.term_translation_show( + translations = get_action('term_translation_show')( {'model': ckan.model}, {'terms': terms, 'lang_codes': (desired_lang_code, fallback_lang_code)}) @@ -110,7 +110,7 @@ def before_index(self, search_data): ## translate title title = search_data.get('title') search_data['title_' + default_lang] = title - title_translations = action_get.term_translation_show( + title_translations = get_action('term_translation_show')( {'model': ckan.model}, {'terms': [title], 'lang_codes': LANGS}) @@ -130,7 +130,7 @@ def before_index(self, search_data): if isinstance(item, basestring): all_terms.append(item) - field_translations = action_get.term_translation_show( + field_translations = get_action('term_translation_show')( {'model': ckan.model}, {'terms': all_terms, 'lang_codes': LANGS}) @@ -185,7 +185,7 @@ def after_search(self, search_results, search_params): for facet in facets.values(): for item in facet['items']: terms.add(item['display_name']) - translations = ckan.logic.action.get.term_translation_show( + translations = get_action('term_translation_show')( {'model': ckan.model}, {'terms': terms, 'lang_codes': (desired_lang_code, fallback_lang_code)}) @@ -220,7 +220,7 @@ def before_view(self, dataset_dict): desired_lang_code = pylons.request.environ['CKAN_LANG'] fallback_lang_code = pylons.config.get('ckan.locale_default', 'en') terms = [value for param, value in c.fields] - translations = ckan.logic.action.get.term_translation_show( + translations = get_action('term_translation_show')( {'model': ckan.model}, {'terms': terms, 'lang_codes': (desired_lang_code, fallback_lang_code)}) diff --git a/doc/extensions/adding-custom-fields.rst b/doc/extensions/adding-custom-fields.rst index 35d894d68fe..6da4c2cfa04 100644 --- a/doc/extensions/adding-custom-fields.rst +++ b/doc/extensions/adding-custom-fields.rst @@ -143,7 +143,8 @@ the templates. Create a directory called ``ckanext-extrafields/ckanext/extrafields/templates/package/snippets/``. We need to override a few templates in order to get our custom field rendered. -Firstly we need to remove the default custom field handling. Create a template +A common option when using a custom schema is to remove the default custom +field handling that allows arbitrary key/value pairs. Create a template file in our templates directory called ``package/snippets/package_metadata_fields.html`` containing @@ -153,7 +154,16 @@ file in our templates directory called :end-before: {% block package_metadata_fields %} This overrides the custom_fields block with an empty block so the default CKAN -custom fields form does not render. Next add a template in our template +custom fields form does not render. + + +.. versionadded:: 2.2.1 + + Starting from CKAN 2.2.1 you can combine free extras with custom fields + handled with ``convert_to_extras`` and ``convert_from_extras``. On prior + versions you'll always need to remove the free extras handling. + +Next add a template in our template directory called ``package/snippets/package_basic_fields.html`` containing .. literalinclude:: ../../ckanext/example_idatasetform/templates/package/snippets/package_basic_fields.html diff --git a/setup.py b/setup.py index 757cc17a661..6061814c19c 100644 --- a/setup.py +++ b/setup.py @@ -120,6 +120,7 @@ 'example_theme_v20_pubsub = ckanext.example_theme.v20_pubsub.plugin:ExampleThemePlugin', 'example_theme_v21_custom_jquery_plugin = ckanext.example_theme.v21_custom_jquery_plugin.plugin:ExampleThemePlugin', 'example_theme_custom_config_setting = ckanext.example_theme.custom_config_setting.plugin:ExampleThemePlugin', + 'example_iresourcecontroller = ckanext.example_iresourcecontroller.plugin:ExampleIResourceControllerPlugin', 'example_ivalidators = ckanext.example_ivalidators.plugin:ExampleIValidatorsPlugin', ], 'ckan.system_plugins': [