diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py index ad6d8410e19..723d4bbf6cc 100644 --- a/ckan/controllers/api.py +++ b/ckan/controllers/api.py @@ -34,7 +34,9 @@ 'text': 'text/plain;charset=utf-8', 'html': 'text/html;charset=utf-8', 'json': 'application/json;charset=utf-8', - } +} + + class ApiController(base.BaseController): _actions = {} @@ -50,10 +52,11 @@ def __call__(self, environ, start_response): self._identify_user() try: - context = {'model':model,'user': c.user or c.author} - logic.check_access('site_read',context) + context = {'model': model, 'user': c.user or c.author} + logic.check_access('site_read', context) except NotAuthorized: - response_msg = self._finish(403, _('Not authorized to see this page')) + response_msg = self._finish(403, + _('Not authorized to see this page')) # Call start_response manually instead of the parent __call__ # because we want to end the request instead of continuing. response_msg = response_msg.encode('utf8') @@ -83,9 +86,9 @@ def _finish(self, status_int, response_data=None, else: response_msg = response_data # Support "JSONP" callback. - if status_int==200 and request.params.has_key('callback') and \ - (request.method == 'GET' or \ - c.logic_function and request.method == 'POST'): + if status_int == 200 and 'callback' in request.params and \ + (request.method == 'GET' or + c.logic_function and request.method == 'POST'): # escape callback to remove '<', '&', '>' chars callback = cgi.escape(request.params['callback']) response_msg = self._wrap_jsonp(callback, response_msg) @@ -133,8 +136,9 @@ def _set_response_header(self, name, value): try: value = str(value) except Exception, inst: - msg = "Couldn't convert '%s' header value '%s' to string: %s" % (name, value, inst) - raise Exception, msg + msg = "Couldn't convert '%s' header value '%s' to string: %s" % \ + (name, value, inst) + raise Exception(msg) response.headers[name] = value def get_api(self, ver=None): @@ -143,19 +147,21 @@ def get_api(self, ver=None): return self._finish_ok(response_data) def action(self, logic_function, ver=None): - function = get_action(logic_function) - if not function: + try: + function = get_action(logic_function) + except KeyError: log.error('Can\'t find logic function: %s' % logic_function) return self._finish_bad_request( gettext('Action name not known: %s') % str(logic_function)) context = {'model': model, 'session': model.Session, 'user': c.user, - 'api_version':ver} + 'api_version': ver} model.Session()._context = context return_dict = {'help': function.__doc__} try: side_effect_free = getattr(function, 'side_effect_free', False) - request_data = self._get_request_data(try_url_params=side_effect_free) + request_data = self._get_request_data(try_url_params= + side_effect_free) except ValueError, inst: log.error('Bad request data: %s' % str(inst)) return self._finish_bad_request( @@ -164,8 +170,8 @@ def action(self, logic_function, ver=None): # this occurs if request_data is blank log.error('Bad request data - not dict: %r' % request_data) return self._finish_bad_request( - gettext('Bad request data: %s') % \ - 'Request data JSON decoded to %r but ' \ + gettext('Bad request data: %s') % + 'Request data JSON decoded to %r but ' 'it needs to be a dictionary.' % request_data) try: result = function(context, request_data) @@ -174,8 +180,8 @@ def action(self, logic_function, ver=None): except DataError, e: log.error('Format incorrect: %s - %s' % (e.error, request_data)) #TODO make better error message - return self._finish(400, _(u'Integrity Error') + \ - ': %s - %s' % (e.error, request_data)) + return self._finish(400, _(u'Integrity Error') + + ': %s - %s' % (e.error, request_data)) except NotAuthorized: return_dict['error'] = {'__type': 'Authorization Error', 'message': _('Access denied')} @@ -197,25 +203,27 @@ def action(self, logic_function, ver=None): return self._finish(409, return_dict, content_type='json') except logic.ParameterError, e: return_dict['error'] = {'__type': 'Parameter Error', - 'message': '%s: %s' % \ + 'message': '%s: %s' % (_('Parameter Error'), e.extra_msg)} return_dict['success'] = False log.error('Parameter error: %r' % e.extra_msg) return self._finish(409, return_dict, content_type='json') except search.SearchQueryError, e: return_dict['error'] = {'__type': 'Search Query Error', - 'message': 'Search Query is invalid: %r' % e.args } + 'message': 'Search Query is invalid: %r' % + e.args} return_dict['success'] = False return self._finish(400, return_dict, content_type='json') except search.SearchError, e: return_dict['error'] = {'__type': 'Search Error', - 'message': 'Search error: %r' % e.args } + 'message': 'Search error: %r' % e.args} return_dict['success'] = False return self._finish(409, return_dict, content_type='json') return self._finish_ok(return_dict) def _get_action_from_map(self, action_map, register, subregister): - # Helper function to get the action function specified in the action map + ''' Helper function to get the action function specified in + the action map''' # translate old package calls to use dataset if register == 'package': @@ -258,7 +266,8 @@ def list(self, ver=None, register=None, subregister=None, id=None): except NotAuthorized: return self._finish_not_authz() - def show(self, ver=None, register=None, subregister=None, id=None, id2=None): + def show(self, ver=None, register=None, subregister=None, + id=None, id2=None): action_map = { 'revision': 'revision_show', 'group': 'group_show_rest', @@ -289,15 +298,17 @@ def show(self, ver=None, register=None, subregister=None, id=None, id2=None): return self._finish_not_authz() def _represent_package(self, package): - return package.as_dict(ref_package_by=self.ref_package_by, ref_group_by=self.ref_group_by) + return package.as_dict(ref_package_by=self.ref_package_by, + ref_group_by=self.ref_group_by) - def create(self, ver=None, register=None, subregister=None, id=None, id2=None): + def create(self, ver=None, register=None, subregister=None, + id=None, id2=None): action_map = { - 'group': 'group_create_rest', - 'dataset': 'package_create_rest', - 'rating': 'rating_create', - 'related': 'related_create', + 'group': 'group_create_rest', + 'dataset': 'package_create_rest', + 'rating': 'rating_create', + 'related': 'related_create', ('dataset', 'relationships'): 'package_relationship_create_rest', } for type in model.PackageRelationship.get_all_types(): @@ -317,14 +328,15 @@ def create(self, ver=None, register=None, subregister=None, id=None, id2=None): action = self._get_action_from_map(action_map, register, subregister) if not action: return self._finish_bad_request( - gettext('Cannot create new entity of this type: %s %s') % \ + gettext('Cannot create new entity of this type: %s %s') % (register, subregister)) try: response_data = action(context, data_dict) location = None if "id" in data_dict: - location = str('%s/%s' % (request.path.replace('package', 'dataset'), + location = str('%s/%s' % (request.path.replace('package', + 'dataset'), data_dict.get("id"))) return self._finish_ok(response_data, resource_location=location) @@ -339,19 +351,23 @@ def create(self, ver=None, register=None, subregister=None, id=None, id2=None): except DataError, e: log.error('Format incorrect: %s - %s' % (e.error, request_data)) #TODO make better error message - return self._finish(400, _(u'Integrity Error') + \ - ': %s - %s' % (e.error, request_data)) + return self._finish(400, _(u'Integrity Error') + + ': %s - %s' % (e.error, request_data)) except search.SearchIndexError: - log.error('Unable to add package to search index: %s' % request_data) - return self._finish(500, _(u'Unable to add package to search index') % request_data) + log.error('Unable to add package to search index: %s' % + request_data) + return self._finish(500, + _(u'Unable to add package to search index') % + request_data) except: model.Session.rollback() raise - def update(self, ver=None, register=None, subregister=None, id=None, id2=None): + def update(self, ver=None, register=None, subregister=None, + id=None, id2=None): action_map = { - 'dataset': 'package_update_rest', - 'group': 'group_update_rest', + 'dataset': 'package_update_rest', + 'group': 'group_update_rest', ('dataset', 'relationships'): 'package_relationship_update_rest', } for type in model.PackageRelationship.get_all_types(): @@ -371,8 +387,8 @@ def update(self, ver=None, register=None, subregister=None, id=None, id2=None): action = self._get_action_from_map(action_map, register, subregister) if not action: return self._finish_bad_request( - gettext('Cannot update entity of this type: %s') % \ - register.encode('utf-8')) + gettext('Cannot update entity of this type: %s') % + register.encode('utf-8')) try: response_data = action(context, data_dict) return self._finish_ok(response_data) @@ -387,17 +403,19 @@ def update(self, ver=None, register=None, subregister=None, id=None, id2=None): except DataError, e: log.error('Format incorrect: %s - %s' % (e.error, request_data)) #TODO make better error message - return self._finish(400, _(u'Integrity Error') + \ - ': %s - %s' % (e.error, request_data)) + return self._finish(400, _(u'Integrity Error') + + ': %s - %s' % (e.error, request_data)) except search.SearchIndexError: log.error('Unable to update search index: %s' % request_data) - return self._finish(500, _(u'Unable to update search index') % request_data) + return self._finish(500, _(u'Unable to update search index') % + request_data) - def delete(self, ver=None, register=None, subregister=None, id=None, id2=None): + def delete(self, ver=None, register=None, subregister=None, + id=None, id2=None): action_map = { - 'group': 'group_delete', - 'dataset': 'package_delete', - 'related': 'related_delete', + 'group': 'group_delete', + 'dataset': 'package_delete', + 'related': 'related_delete', ('dataset', 'relationships'): 'package_relationship_delete_rest', } for type in model.PackageRelationship.get_all_types(): @@ -413,7 +431,7 @@ def delete(self, ver=None, register=None, subregister=None, id=None, id2=None): action = self._get_action_from_map(action_map, register, subregister) if not action: return self._finish_bad_request( - gettext('Cannot delete entity of this type: %s %s') %\ + gettext('Cannot delete entity of this type: %s %s') % (register, subregister or '')) try: response_data = action(context, data_dict) @@ -432,7 +450,7 @@ def search(self, ver=None, register=None): log.debug('search %s params: %r' % (register, request.params)) if register == 'revision': since_time = None - if request.params.has_key('since_id'): + if 'since_id' in request.params: id = request.params['since_id'] if not id: return self._finish_bad_request( @@ -442,7 +460,7 @@ def search(self, ver=None, register=None): return self._finish_not_found( gettext(u'There is no revision with id: %s') % id) since_time = rev.timestamp - elif request.params.has_key('since_time'): + elif 'since_time' in request.params: since_time_str = request.params['since_time'] try: since_time = h.date_str_to_datetime(since_time_str) @@ -450,8 +468,10 @@ def search(self, ver=None, register=None): return self._finish_bad_request('ValueError: %s' % inst) else: return self._finish_bad_request( - gettext("Missing search term ('since_id=UUID' or 'since_time=TIMESTAMP')")) - revs = model.Session.query(model.Revision).filter(model.Revision.timestamp>since_time) + gettext("Missing search term ('since_id=UUID' or " + + " 'since_time=TIMESTAMP')")) + revs = model.Session.query(model.Revision).\ + filter(model.Revision.timestamp > since_time) return self._finish_ok([rev.id for rev in revs]) elif register in ['dataset', 'package', 'resource']: try: @@ -482,7 +502,7 @@ def search(self, ver=None, register=None): for field, value in params.items(): field = field.strip() if field in search.DEFAULT_OPTIONS.keys() or \ - field in IGNORE_FIELDS: + field in IGNORE_FIELDS: continue values = [value] if isinstance(value, list): @@ -491,14 +511,18 @@ def search(self, ver=None, register=None): query_fields.add(field, v) results = query.run( - query=params.get('q'), fields=query_fields, options=options + query=params.get('q'), + fields=query_fields, + options=options ) else: # For package searches in API v3 and higher, we can pass # parameters straight to Solr. if ver in [1, 2]: - # Otherwise, put all unrecognised ones into the q parameter - params = search.convert_legacy_parameters_to_solr(params) + # Otherwise, put all unrecognised ones into the q + # parameter + params = search.\ + convert_legacy_parameters_to_solr(params) query = search.query_for(model.Package) results = query.run(params) return self._finish_ok(results) @@ -512,20 +536,23 @@ def search(self, ver=None, register=None): @classmethod def _get_search_params(cls, request_params): - if request_params.has_key('qjson'): + if 'qjson' in request_params: try: params = h.json.loads(request_params['qjson'], encoding='utf8') except ValueError, e: - raise ValueError, gettext('Malformed qjson value') + ': %r' % e + raise ValueError(gettext('Malformed qjson value') + ': %r' + % e) elif len(request_params) == 1 and \ - len(request_params.values()[0]) < 2 and \ - request_params.keys()[0].startswith('{'): + len(request_params.values()[0]) < 2 and \ + request_params.keys()[0].startswith('{'): # e.g. {some-json}='1' or {some-json}='' params = h.json.loads(request_params.keys()[0], encoding='utf8') else: params = request_params if not isinstance(params, (UnicodeMultiDict, dict)): - raise ValueError, _('Request params must be in form of a json encoded dictionary.') + msg = _('Request params must be in form ' + + 'of a json encoded dictionary.') + raise ValueError(msg) return params def markdown(self, ver=None): @@ -559,7 +586,8 @@ def _calc_throughput(self, ver=None): for t in range(0, period): expr = '%s/%s*' % ( timing_cache_path, - (datetime.datetime.now() - datetime.timedelta(0,t)).isoformat()[0:19], + (datetime.datetime.now() - + datetime.timedelta(0, t)).isoformat()[0:19], ) call_count += len(glob.glob(expr)) # Todo: Clear old records. @@ -574,9 +602,9 @@ def user_autocomplete(self): context = {'model': model, 'session': model.Session, 'user': c.user or c.author} - data_dict = {'q':q,'limit':limit} + data_dict = {'q': q, 'limit': limit} - user_list = get_action('user_autocomplete')(context,data_dict) + user_list = get_action('user_autocomplete')(context, data_dict) return user_list @jsonp.jsonpify @@ -591,16 +619,17 @@ def group_autocomplete(self): limit = min(50, limit) query = model.Group.search_by_name_or_title(q, t) + def convert_to_dict(user): out = {} for k in ['id', 'name', 'title']: out[k] = getattr(user, k) return out + query = query.limit(limit) out = map(convert_to_dict, query.all()) return out - @jsonp.jsonpify def authorizationgroup_autocomplete(self): q = request.params.get('q', '') @@ -612,11 +641,13 @@ def authorizationgroup_autocomplete(self): limit = min(50, limit) query = model.AuthorizationGroup.search(q) + def convert_to_dict(user): out = {} for k in ['id', 'name']: out[k] = getattr(user, k) return out + query = query.limit(limit) out = map(convert_to_dict, query.all()) return out @@ -626,13 +657,13 @@ def is_slug_valid(self): slugtype = request.params.get('type') or '' # TODO: We need plugins to be able to register new disallowed names disallowed = ['new', 'edit', 'search'] - if slugtype==u'package': + if slugtype == u'package': response_data = dict(valid=not bool(common.package_exists(slug) - or slug in disallowed )) + or slug in disallowed)) return self._finish_ok(response_data) - if slugtype==u'group': + if slugtype == u'group': response_data = dict(valid=not bool(common.group_exists(slug) or - slug in disallowed )) + slug in disallowed)) return self._finish_ok(response_data) return self._finish_bad_request('Bad slug type: %s' % slugtype) @@ -647,7 +678,8 @@ def dataset_autocomplete(self): data_dict = {'q': q, 'limit': limit} - package_dicts = get_action('package_autocomplete')(context, data_dict) + package_dicts = get_action('package_autocomplete')(context, + data_dict) resultSet = {'ResultSet': {'Result': package_dicts}} return self._finish_ok(resultSet) @@ -706,9 +738,9 @@ def munge_tag(self): def format_icon(self): f = request.params.get('format') out = { - 'format' : f, - 'icon' : h.icon_url(h.format_icon(f)) - } + 'format': f, + 'icon': h.icon_url(h.format_icon(f)) + } return self._finish_ok(out) def status(self): diff --git a/ckan/logic/__init__.py b/ckan/logic/__init__.py index 098b7edae04..bb7d94b4b7c 100644 --- a/ckan/logic/__init__.py +++ b/ckan/logic/__init__.py @@ -11,12 +11,13 @@ log = logging.getLogger(__name__) + class AttributeDict(dict): def __getattr__(self, name): try: return self[name] except KeyError: - raise AttributeError('No such attribute %r'%name) + raise AttributeError('No such attribute %r' % name) def __setattr__(self, name, value): raise AttributeError( @@ -33,12 +34,15 @@ def __str__(self): self.extra_msg) return ' - '.join([str(err_msg) for err_msg in err_msgs if err_msg]) + class NotFound(ActionError): pass + class NotAuthorized(ActionError): pass + class ParameterError(ActionError): pass @@ -56,6 +60,7 @@ def __str__(self): log = logging.getLogger(__name__) + def parse_params(params, ignore_keys=None): '''Takes a dict and returns it with some values standardised. This is done on a dict before calling tuplize_dict on it. @@ -109,6 +114,7 @@ def clean_dict(data_dict): clean_dict(inner_dict) return data_dict + def tuplize_dict(data_dict): '''Takes a dict with keys of the form 'table__0__key' and converts them to a tuple like ('table', 0, 'key'). @@ -130,6 +136,7 @@ def tuplize_dict(data_dict): tuplized_dict[tuple(key_list)] = value return tuplized_dict + def untuplize_dict(tuplized_dict): data_dict = {} @@ -138,30 +145,33 @@ def untuplize_dict(tuplized_dict): data_dict[new_key] = value return data_dict + def flatten_to_string_key(dict): flattented = flatten_dict(dict) return untuplize_dict(flattented) + def check_access(action, context, data_dict=None): model = context['model'] user = context.get('user') - log.debug('check access - user %r, action %s' % (user,action)) - + log.debug('check access - user %r, action %s' % (user, action)) + if action: - #if action != model.Action.READ and user in (model.PSEUDO_USER__VISITOR, ''): + #if action != model.Action.READ and user in + # (model.PSEUDO_USER__VISITOR, ''): # # TODO Check the API key is valid at some point too! # log.debug('Valid API key needed to make changes') # raise NotAuthorized logic_authorization = is_authorized(action, context, data_dict) if not logic_authorization['success']: - msg = logic_authorization.get('msg','') + msg = logic_authorization.get('msg', '') raise NotAuthorized(msg) elif not user: msg = _('No valid API key provided.') log.debug(msg) - raise NotAuthorized(msg) + raise NotAuthorized(msg) log.debug('Access OK.') return True @@ -172,7 +182,7 @@ def check_access_old(entity, action, context): user = context.get('user') if context.get('ignore_auth'): return True - log.debug('check access - user %r, action %s' % (user,action)) + log.debug('check access - user %r, action %s' % (user, action)) if action and entity and not isinstance(entity, model.PackageRelationship): if action != model.Action.READ and user == '': log.debug('Valid API key needed to make changes') @@ -189,20 +199,23 @@ def check_access_old(entity, action, context): #raise NotAuthorized log.debug('Access OK.') - return True + return True _actions = {} + def get_action(action): if _actions: + if not action in _actions: + raise KeyError("Action '%s' not found" % action) return _actions.get(action) # Otherwise look in all the plugins to resolve all possible # First get the default ones in the ckan/logic/action directory # Rather than writing them out in full will use __import__ # to load anything from ckan.logic.action that looks like it might # be an action - for action_module_name in ['get', 'create', 'update','delete']: - module_path = 'ckan.logic.action.'+action_module_name + for action_module_name in ['get', 'create', 'update', 'delete']: + module_path = 'ckan.logic.action.' + action_module_name module = __import__(module_path) for part in module_path.split('.')[1:]: module = getattr(module, part) @@ -214,8 +227,8 @@ def get_action(action): # Whitelist all actions defined in logic/action/get.py as # being side-effect free. - v.side_effect_free = getattr(v, 'side_effect_free', True) and \ - action_module_name == 'get' + v.side_effect_free = getattr(v, 'side_effect_free', True)\ + and action_module_name == 'get' # Then overwrite them with any specific ones in the plugins: resolved_action_plugins = {} @@ -236,6 +249,7 @@ def get_action(action): _actions.update(fetched_actions) return _actions.get(action) + def get_or_bust(data_dict, keys): '''Try and get values from dictionary and if they are not there raise a validation error. @@ -264,6 +278,7 @@ def get_or_bust(data_dict, keys): return values[0] return tuple(values) + def side_effect_free(action): '''A decorator that marks the given action as side-effect-free. diff --git a/ckan/tests/logic/test_action.py b/ckan/tests/logic/test_action.py index f22858ee1ab..d104c5d3e94 100644 --- a/ckan/tests/logic/test_action.py +++ b/ckan/tests/logic/test_action.py @@ -870,16 +870,16 @@ def test_34_roles_show_for_authgroup_on_authgroup(self): annafan = model.User.by_name(u'annafan') authgroup = model.AuthorizationGroup.by_name(u'anauthzgroup') authgroup2 = model.AuthorizationGroup.by_name(u'anotherauthzgroup') - + model.add_authorization_group_to_role(authgroup2, 'editor', authgroup) model.repo.commit_and_remove() - + postparams = '%s=1' % json.dumps({'domain_object': authgroup.id, 'authorization_group': authgroup2.id}) res = self.app.post('/api/action/roles_show', params=postparams, extra_environ={'Authorization': str(annafan.apikey)}, status=200) - + authgroup_roles = TestRoles.get_roles(authgroup.id, authgroup_ref=authgroup2.name) assert_equal(authgroup_roles, ['"anotherauthzgroup" is "editor" on "anauthzgroup"']) @@ -904,7 +904,7 @@ def test_35_user_role_update(self): assert_equal(results['roles'][0]['role'], 'reader') assert_equal(results['roles'][0]['package_id'], anna.id) assert_equal(results['roles'][0]['user_id'], tester.id) - + roles_after = get_action('roles_show') \ ({'model': model, 'session': model.Session}, \ {'domain_object': anna.id, @@ -934,12 +934,12 @@ def test_36_user_role_update_for_auth_group(self): assert_equal(results['roles'][0]['role'], 'editor') assert_equal(results['roles'][0]['package_id'], anna.id) assert_equal(results['roles'][0]['authorized_group_id'], authgroup.id) - + all_roles_after = TestRoles.get_roles(anna.id) authgroup_roles_after = TestRoles.get_roles(anna.id, authgroup_ref=authgroup.name) assert_equal(set(all_roles_before) ^ set(all_roles_after), set([u'"anauthzgroup" is "editor" on "annakarenina"'])) - + roles_after = get_action('roles_show') \ ({'model': model, 'session': model.Session}, \ {'domain_object': anna.id, @@ -1016,6 +1016,12 @@ def test_40_task_resource_status(self): assert res['success'] is True assert res['result'] == [{"status": "FAILURE", "entity_id": "749cdcf2-3fc8-44ae-aed0-5eff8cc5032c", "task_type": "qa", "last_updated": "2012-04-20T21:32:45.553986", "date_done": "2012-04-20T21:33:01.622557", "entity_type": "resource", "traceback": "Traceback", "value": "51f2105d-85b1-4393-b821-ac11475919d9", "state": None, "key": "celery_task_id", "error": "", "id": "5753adae-cd0d-4327-915d-edd832d1c9a3"}] + def test_41_missing_action(self): + try: + get_action('unicorns') + assert False, "We found a non-existent action" + except KeyError: + assert True class TestActionTermTranslation(WsgiAppCase): @@ -1240,7 +1246,7 @@ def after_search(self, search_results, search_params): return search_results def before_view(self, data_dict): - + data_dict['title'] = 'string_not_found_in_rest_of_template' return data_dict @@ -1306,7 +1312,7 @@ def test_before_index(self): res = self.app.post('/api/action/package_search', params=search_params) res_dict = json.loads(res.body)['result'] - assert res_dict['count'] == 0 + assert res_dict['count'] == 0 assert len(res_dict['results']) == 0 # all datasets should get abcabcabc @@ -1323,8 +1329,8 @@ def test_before_view(self): res = self.app.get('/dataset/annakarenina') assert 'string_not_found_in_rest_of_template' in res.body - + res = self.app.get('/dataset?q=') assert res.body.count('string_not_found_in_rest_of_template') == 2 - +