From 72d3036ac461dac08e81877107b87feb45d26e54 Mon Sep 17 00:00:00 2001 From: tobes Date: Tue, 22 Jan 2013 09:33:05 +0000 Subject: [PATCH 01/35] [#285] Remove code in base controller that is not used in the ckan codebase --- ckan/lib/base.py | 222 ----------------------------------------------- 1 file changed, 222 deletions(-) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 4fd2953ed21..3fb56b3bbd7 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -340,119 +340,6 @@ def _set_cors(self): response.headers['Access-Control-Allow-Headers'] = \ "X-CKAN-API-KEY, Authorization, Content-Type" - def _get_user(self, reference): - return model.User.by_name(reference) - - def _get_pkg(self, reference): - return model.Package.get(reference) - - def _get_group(self, reference): - return model.Group.get(reference) - - def _get_tag(self, reference): - return model.Tag.get(reference) - - @classmethod - def _get_request_data(cls, try_url_params=False): - '''Returns a dictionary, extracted from a request. - - If there is no data, None or "" is returned. - ValueError will be raised if the data is not a JSON-formatted dict. - - The data is retrieved as a JSON-encoded dictionary from the request - body. Or, if the `try_url_params` argument is True and the request is - a GET request, then an attempt is made to read the data from the url - parameters of the request. - - try_url_params - If try_url_params is False, then the data_dict is read from the - request body. - - If try_url_params is True and the request is a GET request then the - data is read from the url parameters. The resulting dict will only - be 1 level deep, with the url-param fields being the keys. If a - single key has more than one value specified, then the value will - be a list of strings, otherwise just a string. - - This function is only used by the API, so no strings need to be - translated. - - TODO: If this is only used by the API, then perhaps it should be - moved to the api controller class? - ''' - cls.log.debug('Retrieving request params: %r' % request.params) - cls.log.debug('Retrieving request POST: %r' % request.POST) - cls.log.debug('Retrieving request GET: %r' % request.GET) - request_data = None - if request.POST: - try: - keys = request.POST.keys() - # Parsing breaks if there is a = in the value, so for now - # we will check if the data is actually all in a single key - if keys and request.POST[keys[0]] in [u'1', u'']: - request_data = keys[0] - else: - request_data = urllib.unquote_plus(request.body) - except Exception, inst: - msg = "Could not find the POST data: %r : %s" % \ - (request.POST, inst) - raise ValueError(msg) - - elif try_url_params and request.GET: - return request.GET.mixed() - - else: - try: - if request.method in ['POST', 'PUT']: - request_data = request.body - else: - request_data = None - except Exception, inst: - msg = "Could not extract request body data: %s" % \ - (inst) - raise ValueError(msg) - cls.log.debug('Retrieved request body: %r' % request.body) - if not request_data: - msg = "No request body data" - raise ValueError(msg) - if request_data: - try: - request_data = json.loads(request_data, encoding='utf8') - except ValueError, e: - raise ValueError('Error decoding JSON data. ' - 'Error: %r ' - 'JSON data extracted from the request: %r' % - (e, request_data)) - if not isinstance(request_data, dict): - raise ValueError('Request data JSON decoded to %r but ' - 'it needs to be a dictionary.' % request_data) - # ensure unicode values - for key, val in request_data.items(): - # if val is str then assume it is ascii, since json converts - # utf8 encoded JSON to unicode - request_data[key] = cls._make_unicode(val) - cls.log.debug('Request data extracted: %r' % request_data) - return request_data - - @classmethod - def _make_unicode(cls, entity): - """Cast bare strings and strings in lists or dicts to Unicode - """ - if isinstance(entity, str): - return unicode(entity) - elif isinstance(entity, list): - new_items = [] - for item in entity: - new_items.append(cls._make_unicode(item)) - return new_items - elif isinstance(entity, dict): - new_dict = {} - for key, val in entity.items(): - new_dict[key] = cls._make_unicode(val) - return new_dict - else: - return entity - def _get_user_for_apikey(self): apikey_header_name = config.get(APIKEY_HEADER_NAME_KEY, APIKEY_HEADER_NAME_DEFAULT) @@ -475,115 +362,6 @@ def _get_user_for_apikey(self): user = query.filter_by(apikey=apikey).first() return user - def _get_timing_cache_path(self): - - return path - - def _handle_update_of_authz(self, domain_object): - '''In the event of a post request to a domain object\'s authz form, - work out which of the four possible actions is to be done, - and do it before displaying the page. - - Returns the updated roles for the domain_object. - ''' - from ckan.logic import NotFound, get_action - - context = {'model': model, 'session': model.Session, - 'user': c.user or c.author} - data_dict = {'domain_object': domain_object.id} - - # Work out actions needed, depending on which button was pressed - update_type = 'user' - if 'save' in request.POST: - update_or_add = 'update' - elif 'add' in request.POST: - update_or_add = 'add' - else: - update_type = None - update_or_add = None - - # Work out what role checkboxes are checked or unchecked - checked_roles = [box_id for (box_id, value) in request.params.items() - if (value == u'on')] - unchecked_roles = [box_id for (box_id, value) in request.params.items() - if (value == u'submitted')] - - action = None - if update_or_add is 'update': - # Get user_roles by decoding the checkbox grid - user$role strings - user_roles = {} - for checked_role in checked_roles: - obj_id, role = checked_role.split('$') - if obj_id not in user_roles: - user_roles[obj_id] = [] - user_roles[obj_id].append(role) - # Users without roles need adding to the user_roles too to make - # their roles be deleted - for unchecked_role in unchecked_roles: - obj_id, role = unchecked_role.split('$') - if obj_id not in user_roles: - user_roles[obj_id] = [] - # Convert user_roles to role dictionaries - role_dicts = [] - for user, roles in user_roles.items(): - role_dicts.append({update_type: user, 'roles': roles}) - data_dict['user_roles'] = role_dicts - - action = 'user_role_bulk_update' - success_message = _('Updated') - elif update_or_add is 'add': - # Roles for this new user is a simple list from the checkbox row - data_dict['roles'] = checked_roles - - # User comes from the input box. - new_user = request.params.get('new_user_name') - if new_user: - data_dict[update_type] = new_user - - action = 'user_role_update' - success_message = _('User role(s) added') - else: - h.flash_error(_('Please supply a user name')) - - if action: - try: - roles = get_action(action)(context, data_dict) - except NotFound, e: - h.flash_error(_('Not found') + (': %s' % e if str(e) else '')) - else: - h.flash_success(success_message) - - # Return roles for all users on this domain object - if update_or_add is 'add': - if update_type in data_dict: - del data_dict[update_type] - return get_action('roles_show')(context, data_dict) - - def _prepare_authz_info_for_render(self, user_object_roles): - # ================= - # Display the page - - # Find out all the possible roles. At the moment, any role can be - # associated with any object, so that's easy: - possible_roles = model.Role.get_all() - - # uniquify and sort - users = sorted(list(set([uor['user_id'] - for uor in user_object_roles['roles'] - if uor['user_id']]))) - - # make a dictionary from (user, role) to True, False - users_roles = [(uor['user_id'], uor['role']) - for uor in user_object_roles['roles'] - if uor['user_id']] - user_role_dict = {} - for u in users: - for r in possible_roles: - user_role_dict[(u, r)] = (u, r) in users_roles - - c.roles = possible_roles - c.users = users - c.user_role_dict = user_role_dict # Include the '_' function in the public names __all__ = [__name for __name in locals().keys() if not __name.startswith('_') From 8f91b3ce702dc55d1b68c4542ccea95572b97a5a Mon Sep 17 00:00:00 2001 From: tobes Date: Wed, 23 Jan 2013 08:09:31 +0000 Subject: [PATCH 02/35] [#285] Re add function mis-used in api controller --- ckan/lib/base.py | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 3fb56b3bbd7..ce06951c2b6 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -363,6 +363,88 @@ def _get_user_for_apikey(self): return user + @classmethod + def _get_request_data(cls, try_url_params=False): + '''Returns a dictionary, extracted from a request. + + If there is no data, None or "" is returned. + ValueError will be raised if the data is not a JSON-formatted dict. + + The data is retrieved as a JSON-encoded dictionary from the request + body. Or, if the `try_url_params` argument is True and the request is + a GET request, then an attempt is made to read the data from the url + parameters of the request. + + try_url_params + If try_url_params is False, then the data_dict is read from the + request body. + + If try_url_params is True and the request is a GET request then the + data is read from the url parameters. The resulting dict will only + be 1 level deep, with the url-param fields being the keys. If a + single key has more than one value specified, then the value will + be a list of strings, otherwise just a string. + + This function is only used by the API, so no strings need to be + translated. + + TODO: If this is only used by the API, then perhaps it should be + moved to the api controller class? + ''' + cls.log.debug('Retrieving request params: %r' % request.params) + cls.log.debug('Retrieving request POST: %r' % request.POST) + cls.log.debug('Retrieving request GET: %r' % request.GET) + request_data = None + if request.POST: + try: + keys = request.POST.keys() + # Parsing breaks if there is a = in the value, so for now + # we will check if the data is actually all in a single key + if keys and request.POST[keys[0]] in [u'1', u'']: + request_data = keys[0] + else: + request_data = urllib.unquote_plus(request.body) + except Exception, inst: + msg = "Could not find the POST data: %r : %s" % \ + (request.POST, inst) + raise ValueError(msg) + + elif try_url_params and request.GET: + return request.GET.mixed() + + else: + try: + if request.method in ['POST', 'PUT']: + request_data = request.body + else: + request_data = None + except Exception, inst: + msg = "Could not extract request body data: %s" % \ + (inst) + raise ValueError(msg) + cls.log.debug('Retrieved request body: %r' % request.body) + if not request_data: + msg = "No request body data" + raise ValueError(msg) + if request_data: + try: + request_data = json.loads(request_data, encoding='utf8') + except ValueError, e: + raise ValueError('Error decoding JSON data. ' + 'Error: %r ' + 'JSON data extracted from the request: %r' % + (e, request_data)) + if not isinstance(request_data, dict): + raise ValueError('Request data JSON decoded to %r but ' + 'it needs to be a dictionary.' % request_data) + # ensure unicode values + for key, val in request_data.items(): + # if val is str then assume it is ascii, since json converts + # utf8 encoded JSON to unicode + request_data[key] = cls._make_unicode(val) + cls.log.debug('Request data extracted: %r' % request_data) + return request_data + # Include the '_' function in the public names __all__ = [__name for __name in locals().keys() if not __name.startswith('_') or __name == '_'] From cdce0fcd538a1ada46fc3f4b9344f1bb32a249e7 Mon Sep 17 00:00:00 2001 From: tobes Date: Wed, 23 Jan 2013 12:07:17 +0000 Subject: [PATCH 03/35] [#285] _make_unicode() still has user --- ckan/lib/base.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index ce06951c2b6..eb38918b02c 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -445,6 +445,24 @@ def _get_request_data(cls, try_url_params=False): cls.log.debug('Request data extracted: %r' % request_data) return request_data + @classmethod + def _make_unicode(cls, entity): + """Cast bare strings and strings in lists or dicts to Unicode + """ + if isinstance(entity, str): + return unicode(entity) + elif isinstance(entity, list): + new_items = [] + for item in entity: + new_items.append(cls._make_unicode(item)) + return new_items + elif isinstance(entity, dict): + new_dict = {} + for key, val in entity.items(): + new_dict[key] = cls._make_unicode(val) + return new_dict + else: + return entity # Include the '_' function in the public names __all__ = [__name for __name in locals().keys() if not __name.startswith('_') or __name == '_'] From 2edc0a875270aad655c0cb66b02f5864c5920582 Mon Sep 17 00:00:00 2001 From: tobes Date: Thu, 24 Jan 2013 12:25:59 +0000 Subject: [PATCH 04/35] [#285] Move function into only function using it --- ckan/lib/base.py | 43 +++++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index eb38918b02c..a280fd36245 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -385,12 +385,24 @@ def _get_request_data(cls, try_url_params=False): single key has more than one value specified, then the value will be a list of strings, otherwise just a string. - This function is only used by the API, so no strings need to be - translated. - - TODO: If this is only used by the API, then perhaps it should be - moved to the api controller class? ''' + def make_unicode(entity): + '''Cast bare strings and strings in lists or dicts to Unicode. ''' + if isinstance(entity, str): + return unicode(entity) + elif isinstance(entity, list): + new_items = [] + for item in entity: + new_items.append(make_unicode(item)) + return new_items + elif isinstance(entity, dict): + new_dict = {} + for key, val in entity.items(): + new_dict[key] = make_unicode(val) + return new_dict + else: + return entity + cls.log.debug('Retrieving request params: %r' % request.params) cls.log.debug('Retrieving request POST: %r' % request.POST) cls.log.debug('Retrieving request GET: %r' % request.GET) @@ -441,28 +453,11 @@ def _get_request_data(cls, try_url_params=False): for key, val in request_data.items(): # if val is str then assume it is ascii, since json converts # utf8 encoded JSON to unicode - request_data[key] = cls._make_unicode(val) + request_data[key] = make_unicode(val) cls.log.debug('Request data extracted: %r' % request_data) return request_data - @classmethod - def _make_unicode(cls, entity): - """Cast bare strings and strings in lists or dicts to Unicode - """ - if isinstance(entity, str): - return unicode(entity) - elif isinstance(entity, list): - new_items = [] - for item in entity: - new_items.append(cls._make_unicode(item)) - return new_items - elif isinstance(entity, dict): - new_dict = {} - for key, val in entity.items(): - new_dict[key] = cls._make_unicode(val) - return new_dict - else: - return entity + # Include the '_' function in the public names __all__ = [__name for __name in locals().keys() if not __name.startswith('_') or __name == '_'] From 4e4aea6c95b4ad280d4713d3b90fe3574cfad815 Mon Sep 17 00:00:00 2001 From: tobes Date: Thu, 24 Jan 2013 12:29:45 +0000 Subject: [PATCH 05/35] [#285] Move function into API controller as only user --- ckan/controllers/api.py | 94 ++++++++++++++++++++++++++++++++++++++++ ckan/lib/base.py | 95 ----------------------------------------- 2 files changed, 94 insertions(+), 95 deletions(-) diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py index cf22ba9ad78..c092d6e9533 100644 --- a/ckan/controllers/api.py +++ b/ckan/controllers/api.py @@ -758,3 +758,97 @@ def i18n_js_translations(self, lang): return '{}' f = open(source, 'r') return(f) + + @classmethod + def _get_request_data(cls, try_url_params=False): + '''Returns a dictionary, extracted from a request. + + If there is no data, None or "" is returned. + ValueError will be raised if the data is not a JSON-formatted dict. + + The data is retrieved as a JSON-encoded dictionary from the request + body. Or, if the `try_url_params` argument is True and the request is + a GET request, then an attempt is made to read the data from the url + parameters of the request. + + try_url_params + If try_url_params is False, then the data_dict is read from the + request body. + + If try_url_params is True and the request is a GET request then the + data is read from the url parameters. The resulting dict will only + be 1 level deep, with the url-param fields being the keys. If a + single key has more than one value specified, then the value will + be a list of strings, otherwise just a string. + + ''' + def make_unicode(entity): + '''Cast bare strings and strings in lists or dicts to Unicode. ''' + if isinstance(entity, str): + return unicode(entity) + elif isinstance(entity, list): + new_items = [] + for item in entity: + new_items.append(make_unicode(item)) + return new_items + elif isinstance(entity, dict): + new_dict = {} + for key, val in entity.items(): + new_dict[key] = make_unicode(val) + return new_dict + else: + return entity + + cls.log.debug('Retrieving request params: %r' % request.params) + cls.log.debug('Retrieving request POST: %r' % request.POST) + cls.log.debug('Retrieving request GET: %r' % request.GET) + request_data = None + if request.POST: + try: + keys = request.POST.keys() + # Parsing breaks if there is a = in the value, so for now + # we will check if the data is actually all in a single key + if keys and request.POST[keys[0]] in [u'1', u'']: + request_data = keys[0] + else: + request_data = urllib.unquote_plus(request.body) + except Exception, inst: + msg = "Could not find the POST data: %r : %s" % \ + (request.POST, inst) + raise ValueError(msg) + + elif try_url_params and request.GET: + return request.GET.mixed() + + else: + try: + if request.method in ['POST', 'PUT']: + request_data = request.body + else: + request_data = None + except Exception, inst: + msg = "Could not extract request body data: %s" % \ + (inst) + raise ValueError(msg) + cls.log.debug('Retrieved request body: %r' % request.body) + if not request_data: + msg = "No request body data" + raise ValueError(msg) + if request_data: + try: + request_data = json.loads(request_data, encoding='utf8') + except ValueError, e: + raise ValueError('Error decoding JSON data. ' + 'Error: %r ' + 'JSON data extracted from the request: %r' % + (e, request_data)) + if not isinstance(request_data, dict): + raise ValueError('Request data JSON decoded to %r but ' + 'it needs to be a dictionary.' % request_data) + # ensure unicode values + for key, val in request_data.items(): + # if val is str then assume it is ascii, since json converts + # utf8 encoded JSON to unicode + request_data[key] = make_unicode(val) + cls.log.debug('Request data extracted: %r' % request_data) + return request_data diff --git a/ckan/lib/base.py b/ckan/lib/base.py index a280fd36245..3fb56b3bbd7 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -363,101 +363,6 @@ def _get_user_for_apikey(self): return user - @classmethod - def _get_request_data(cls, try_url_params=False): - '''Returns a dictionary, extracted from a request. - - If there is no data, None or "" is returned. - ValueError will be raised if the data is not a JSON-formatted dict. - - The data is retrieved as a JSON-encoded dictionary from the request - body. Or, if the `try_url_params` argument is True and the request is - a GET request, then an attempt is made to read the data from the url - parameters of the request. - - try_url_params - If try_url_params is False, then the data_dict is read from the - request body. - - If try_url_params is True and the request is a GET request then the - data is read from the url parameters. The resulting dict will only - be 1 level deep, with the url-param fields being the keys. If a - single key has more than one value specified, then the value will - be a list of strings, otherwise just a string. - - ''' - def make_unicode(entity): - '''Cast bare strings and strings in lists or dicts to Unicode. ''' - if isinstance(entity, str): - return unicode(entity) - elif isinstance(entity, list): - new_items = [] - for item in entity: - new_items.append(make_unicode(item)) - return new_items - elif isinstance(entity, dict): - new_dict = {} - for key, val in entity.items(): - new_dict[key] = make_unicode(val) - return new_dict - else: - return entity - - cls.log.debug('Retrieving request params: %r' % request.params) - cls.log.debug('Retrieving request POST: %r' % request.POST) - cls.log.debug('Retrieving request GET: %r' % request.GET) - request_data = None - if request.POST: - try: - keys = request.POST.keys() - # Parsing breaks if there is a = in the value, so for now - # we will check if the data is actually all in a single key - if keys and request.POST[keys[0]] in [u'1', u'']: - request_data = keys[0] - else: - request_data = urllib.unquote_plus(request.body) - except Exception, inst: - msg = "Could not find the POST data: %r : %s" % \ - (request.POST, inst) - raise ValueError(msg) - - elif try_url_params and request.GET: - return request.GET.mixed() - - else: - try: - if request.method in ['POST', 'PUT']: - request_data = request.body - else: - request_data = None - except Exception, inst: - msg = "Could not extract request body data: %s" % \ - (inst) - raise ValueError(msg) - cls.log.debug('Retrieved request body: %r' % request.body) - if not request_data: - msg = "No request body data" - raise ValueError(msg) - if request_data: - try: - request_data = json.loads(request_data, encoding='utf8') - except ValueError, e: - raise ValueError('Error decoding JSON data. ' - 'Error: %r ' - 'JSON data extracted from the request: %r' % - (e, request_data)) - if not isinstance(request_data, dict): - raise ValueError('Request data JSON decoded to %r but ' - 'it needs to be a dictionary.' % request_data) - # ensure unicode values - for key, val in request_data.items(): - # if val is str then assume it is ascii, since json converts - # utf8 encoded JSON to unicode - request_data[key] = make_unicode(val) - cls.log.debug('Request data extracted: %r' % request_data) - return request_data - - # Include the '_' function in the public names __all__ = [__name for __name in locals().keys() if not __name.startswith('_') or __name == '_'] From 7e51ec6bc5fcaa80610804738ec66e416e46ab1d Mon Sep 17 00:00:00 2001 From: tobes Date: Thu, 24 Jan 2013 13:18:15 +0000 Subject: [PATCH 06/35] [#285] Fix missing imports --- ckan/controllers/api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py index c092d6e9533..cb9653241d6 100644 --- a/ckan/controllers/api.py +++ b/ckan/controllers/api.py @@ -3,6 +3,8 @@ import cgi import datetime import glob +import urllib +import simplejson as json from pylons import c, request, response from pylons.i18n import _, gettext From 1d5d602eb28723c3c261f9a8a0aa14e815a1af8a Mon Sep 17 00:00:00 2001 From: tobes Date: Thu, 24 Jan 2013 18:21:37 +0000 Subject: [PATCH 07/35] [#285] Use json defined in h --- ckan/controllers/api.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py index cb9653241d6..db87ceda02b 100644 --- a/ckan/controllers/api.py +++ b/ckan/controllers/api.py @@ -4,7 +4,6 @@ import datetime import glob import urllib -import simplejson as json from pylons import c, request, response from pylons.i18n import _, gettext @@ -838,7 +837,7 @@ def make_unicode(entity): raise ValueError(msg) if request_data: try: - request_data = json.loads(request_data, encoding='utf8') + request_data = h.json.loads(request_data, encoding='utf8') except ValueError, e: raise ValueError('Error decoding JSON data. ' 'Error: %r ' From 22ec8c182e58b3b27c5f1e19b55209337aac1754 Mon Sep 17 00:00:00 2001 From: tobes Date: Fri, 25 Jan 2013 02:41:38 +0000 Subject: [PATCH 08/35] [#306] Fix error where icon passed to qs in links --- ckan/lib/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 7fc9bf12214..c0f63d0c143 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -322,11 +322,11 @@ def _create_link_text(text, **kwargs): kwargs ''' if kwargs.pop('inner_span', None): text = literal('') + text + literal('') - icon = kwargs.pop('icon', None) if icon: text = literal(' ' % icon) + text return text + icon = kwargs.pop('icon', None) class_ = _link_class(kwargs) return link_to( _create_link_text(text, **kwargs), From fa213d94e5a7d887813d71dffba086fb0540eb41 Mon Sep 17 00:00:00 2001 From: kindly Date: Fri, 25 Jan 2013 23:05:53 +0000 Subject: [PATCH 09/35] 309 no private datasets in activity streams, revisions, group listing and tag listings --- ckan/controllers/revision.py | 7 +++++-- ckan/lib/activity_streams_session_extension.py | 9 +++++++++ ckan/lib/dictization/model_dictize.py | 17 ++++++++++++++--- ckan/logic/action/get.py | 11 +---------- ckan/model/__init__.py | 3 ++- ckan/tests/functional/api/model/test_package.py | 1 - ckan/tests/functional/api/model/test_tag.py | 3 +++ ckan/tests/logic/test_tag.py | 2 ++ 8 files changed, 36 insertions(+), 17 deletions(-) diff --git a/ckan/controllers/revision.py b/ckan/controllers/revision.py index b3e6a5dfdf9..84683c1efc9 100644 --- a/ckan/controllers/revision.py +++ b/ckan/controllers/revision.py @@ -69,6 +69,8 @@ def list(self): # but in the meantime while that is fixed, # avoid an exception here continue + if package.private: + continue number = len(package.all_revisions) package_revision = None count = 0 @@ -142,10 +144,11 @@ def read(self, id=None): pkgs = model.Session.query(model.PackageRevision).\ filter_by(revision=c.revision) - c.packages = [pkg.continuity for pkg in pkgs] + c.packages = [pkg.continuity for pkg in pkgs if not pkg.private] pkgtags = model.Session.query(model.PackageTagRevision).\ filter_by(revision=c.revision) - c.pkgtags = [pkgtag.continuity for pkgtag in pkgtags] + c.pkgtags = [pkgtag.continuity for pkgtag in pkgtags + if not pkgtag.package.private] grps = model.Session.query(model.GroupRevision).\ filter_by(revision=c.revision) c.groups = [grp.continuity for grp in grps] diff --git a/ckan/lib/activity_streams_session_extension.py b/ckan/lib/activity_streams_session_extension.py index 9fbc1a67e7f..c2d9713fff2 100644 --- a/ckan/lib/activity_streams_session_extension.py +++ b/ckan/lib/activity_streams_session_extension.py @@ -79,6 +79,11 @@ def before_commit(self, session): # object is a package. logger.debug("Looks like this object is a package") logger.debug("activity: %s" % activity) + ## Ignore private datasets for 2.0, + ## fix later to have privicy against streams + if obj.private: + continue + activities[obj.id] = activity activity_detail = activity_stream_detail(obj, activity.id, "new") @@ -113,6 +118,10 @@ def before_commit(self, session): for package in related_packages: if package is None: continue + ## Ignore private datasets for 2.0, + ## fix later to have privicy against streams + if package.private: + continue if package.id in activities: activity = activities[package.id] diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 40cc566624d..1c7865e9516 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -9,6 +9,7 @@ import ckan.lib.helpers as h import ckan.lib.dictization as d import ckan.new_authz as new_authz +import ckan.lib.search as search ## package save @@ -302,11 +303,15 @@ def _get_members(context, group, member_type): model = context['model'] Entity = getattr(model, member_type[:-1].capitalize()) - return model.Session.query(Entity, model.Member.capacity).\ + q = model.Session.query(Entity, model.Member.capacity).\ join(model.Member, model.Member.table_id == Entity.id).\ filter(model.Member.group_id == group.id).\ filter(model.Member.state == 'active').\ - filter(model.Member.table_name == member_type[:-1]).all() + filter(model.Member.table_name == member_type[:-1]) + if member_type == 'packages': + q = q.filter(Entity.private==False) + return q.all() + def group_dictize(group, context): model = context['model'] @@ -376,8 +381,14 @@ def tag_list_dictize(tag_list, context): def tag_dictize(tag, context): result_dict = d.table_dictize(tag, context) - result_dict["packages"] = d.obj_list_dictize(tag.packages, context) + query = search.PackageSearchQuery() + + q = {'q': '+tags:"%s" +capacity:public' % tag.name, 'fl': 'data_dict', + 'wt': 'json', 'rows': 1000} + result_dict["packages"] = [ + h.json.loads(result['data_dict']) for result in query.run(q)['results'] + ] # Add display_names to tags. At first a tag's display_name is just the # same as its name, but the display_name might get changed later (e.g. # translated into another language by the multilingual extension). diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 5a6525dfa58..331b8be3c72 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -929,16 +929,7 @@ def tag_show(context, data_dict): _check_access('tag_show',context, data_dict) - tag_dict = model_dictize.tag_dictize(tag,context) - - extended_packages = [] - for package in tag_dict['packages']: - pkg = model.Package.get(package['id']) - extended_packages.append(model_dictize.package_dictize(pkg,context)) - - tag_dict['packages'] = extended_packages - - return tag_dict + return model_dictize.tag_dictize(tag,context) def user_show(context, data_dict): '''Return a user account. diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index 94d30d9c0be..6cc0914f09e 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -460,7 +460,8 @@ def revision_as_dict(revision, include_packages=True, include_groups=True, )) if include_packages: revision_dict['packages'] = [getattr(pkg, ref_package_by) \ - for pkg in revision.packages if pkg] + for pkg in revision.packages + if (pkg and not pkg.private)] if include_groups: revision_dict['groups'] = [getattr(grp, ref_package_by) \ for grp in revision.groups if grp] diff --git a/ckan/tests/functional/api/model/test_package.py b/ckan/tests/functional/api/model/test_package.py index e5a22bf0c7b..84563eb94ae 100644 --- a/ckan/tests/functional/api/model/test_package.py +++ b/ckan/tests/functional/api/model/test_package.py @@ -279,7 +279,6 @@ def test_register_post_indexerror(self): self.post_json(offset, data, status=500, extra_environ=self.admin_extra_environ) model.Session.remove() finally: - plugins.unload('synchronous_search') SolrSettings.init(original_settings) def test_register_post_tag_too_long(self): diff --git a/ckan/tests/functional/api/model/test_tag.py b/ckan/tests/functional/api/model/test_tag.py index 365d24df594..f9f0afdaa2a 100644 --- a/ckan/tests/functional/api/model/test_tag.py +++ b/ckan/tests/functional/api/model/test_tag.py @@ -4,6 +4,7 @@ from ckan import model from ckan.lib.create_test_data import CreateTestData +import ckan.lib.search as search from ckan.tests.functional.api.base import BaseModelApiTestCase from ckan.tests.functional.api.base import Api1TestCase as Version1TestCase @@ -14,6 +15,7 @@ class TagsTestCase(BaseModelApiTestCase): @classmethod def setup_class(cls): + search.clear() CreateTestData.create() cls.testsysadmin = model.User.by_name(u'testsysadmin') cls.comment = u'Comment umlaut: \xfc.' @@ -22,6 +24,7 @@ def setup_class(cls): @classmethod def teardown_class(cls): + search.clear() model.repo.rebuild_db() def test_register_get_ok(self): diff --git a/ckan/tests/logic/test_tag.py b/ckan/tests/logic/test_tag.py index 1f710ab6563..82d0cdd1e1e 100644 --- a/ckan/tests/logic/test_tag.py +++ b/ckan/tests/logic/test_tag.py @@ -1,6 +1,7 @@ import json from pprint import pprint from nose.tools import assert_equal, assert_raises +import ckan.lib.search as search import ckan.model as model from ckan.lib.create_test_data import CreateTestData @@ -10,6 +11,7 @@ class TestAction(WsgiAppCase): @classmethod def setup_class(cls): + search.clear() CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') cls.normal_user = model.User.get('annafan') From 556c218588699694763fd8acd32b23219b256d17 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Sat, 26 Jan 2013 13:52:28 +0100 Subject: [PATCH 10/35] Fix IRC channel link in README Link to #ckan channel on webchat.freenode.net --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 50b11d82170..fce4eeea262 100644 --- a/README.rst +++ b/README.rst @@ -22,7 +22,7 @@ Community --------- * Developer mailing list: `ckan-dev@lists.okfn.org `_ -* Developer IRC channel: #ckan on `irc.freenode.net `_ +* Developer IRC channel: `#ckan on irc.freenode.net `_ * Issue tracker: `trac.ckan.org `_ * `CKAN tag on StackOverflow `_ From c14558a1b4113a07365300469975bc6338ec00cd Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Sat, 26 Jan 2013 19:11:21 +0100 Subject: [PATCH 11/35] Update issue tracker link in README --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index fce4eeea262..0a2562fced5 100644 --- a/README.rst +++ b/README.rst @@ -23,7 +23,7 @@ Community * Developer mailing list: `ckan-dev@lists.okfn.org `_ * Developer IRC channel: `#ckan on irc.freenode.net `_ -* Issue tracker: `trac.ckan.org `_ +* `Issue tracker `_ * `CKAN tag on StackOverflow `_ From 90b04f17615b28569ab6e33c3a48994bcbf4cc9d Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Sat, 26 Jan 2013 19:14:42 +0100 Subject: [PATCH 12/35] Update contributing link in README --- README.rst | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/README.rst b/README.rst index 0a2562fced5..17fce2e20d0 100644 --- a/README.rst +++ b/README.rst @@ -25,17 +25,14 @@ Community * Developer IRC channel: `#ckan on irc.freenode.net `_ * `Issue tracker `_ * `CKAN tag on StackOverflow `_ +* `Wiki `_ Contributing to CKAN -------------------- -CKAN is a free software project and code contributions are welcome. -The `For CKAN Developers `_ -section of the documentation explains how to contribute to CKAN or its documentation, -including our **coding standards**. - -The `CKAN Wiki `_ is also open fo contributions. +For contributing to CKAN or its documentation, see +`CONTRIBUTING `_. Copying and License From d294d0dcc9e6c95fdee4ac6aab1068777f6c4fd9 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Sun, 27 Jan 2013 00:32:52 +0100 Subject: [PATCH 13/35] [#287] Add is_datastore_supported method to tests and engine_is_pg to model --- ckan/model/__init__.py | 1 + ckan/model/meta.py | 8 +++++++- ckan/tests/__init__.py | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index 94d30d9c0be..f6373130cec 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -13,6 +13,7 @@ from meta import ( Session, engine_is_sqlite, + engine_is_pg, ) from core import ( System, diff --git a/ckan/model/meta.py b/ckan/model/meta.py index 41d67541a1b..13dcf1cb1fc 100644 --- a/ckan/model/meta.py +++ b/ckan/model/meta.py @@ -10,7 +10,7 @@ import extension import ckan.lib.activity_streams_session_extension as activity -__all__ = ['Session', 'engine_is_sqlite'] +__all__ = ['Session', 'engine_is_sqlite', 'engine_is_pg'] class CkanCacheExtension(SessionExtension): @@ -158,3 +158,9 @@ def engine_is_sqlite(): Returns true iff the engine is connected to a sqlite database. """ return engine.url.drivername == 'sqlite' + +def engine_is_pg(): + """ + Returns true iff the engine is connected to a postgresql database. + """ + return engine.url.drivername == 'psycopg2' diff --git a/ckan/tests/__init__.py b/ckan/tests/__init__.py index 0d4be8b4e49..1a0aa934cc1 100644 --- a/ckan/tests/__init__.py +++ b/ckan/tests/__init__.py @@ -330,6 +330,10 @@ def is_migration_supported(): is_supported_db = not model.engine_is_sqlite() return is_supported_db +def is_datastore_supported(): + is_supported_db = model.engine_is_pg() + return is_supported_db + def search_related(test): def skip_test(*args): raise SkipTest("Search not supported") From 23c2bcaf93e9f607b57e5421d6274579a0314c4e Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Sun, 27 Jan 2013 00:59:24 +0100 Subject: [PATCH 14/35] [#287] Log warning if the database is not a pg database. Skip datastore tests, if there is no support for them. --- ckanext/datastore/plugin.py | 32 +++++++++++++++----------- ckanext/datastore/tests/test_create.py | 3 +++ ckanext/datastore/tests/test_delete.py | 3 +++ ckanext/datastore/tests/test_search.py | 7 ++++++ ckanext/datastore/tests/test_upsert.py | 7 ++++++ 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/ckanext/datastore/plugin.py b/ckanext/datastore/plugin.py index 98f8cd92af1..412ba37f48c 100644 --- a/ckanext/datastore/plugin.py +++ b/ckanext/datastore/plugin.py @@ -1,11 +1,13 @@ import logging import pylons from sqlalchemy.exc import ProgrammingError + import ckan.plugins as p import ckanext.datastore.logic.action as action import ckanext.datastore.logic.auth as auth import ckanext.datastore.db as db import ckan.logic as logic +import ckan.model as model log = logging.getLogger(__name__) _get_or_bust = logic.get_or_bust @@ -52,21 +54,25 @@ def configure(self, config): else: self.read_url = self.config['ckan.datastore.read_url'] - if not self._is_read_only_database(): - # Make sure that the right permissions are set - # so that no harmful queries can be made - if not ('debug' in config and config['debug']): - self._check_separate_db() - if self.legacy_mode: - log.warn('Legacy mode active. The sql search will not be available.') - else: - self._check_read_permissions() + if model.engine_is_pg(): + if not self._is_read_only_database(): + # Make sure that the right permissions are set + # so that no harmful queries can be made + if not ('debug' in config and config['debug']): + self._check_separate_db() + if self.legacy_mode: + log.warn('Legacy mode active. The sql search will not be available.') + else: + self._check_read_permissions() - self._create_alias_table() + self._create_alias_table() + else: + log.warn("We detected that CKAN is running on a read only database. " + "Permission checks and _table_metadata creation are skipped." + "Make sure that replication is properly set-up.") else: - log.warn("We detected that CKAN is running on a read only database. " - "Permission checks and _table_metadata creation are skipped." - "Make sure that replication is properly set-up.") + log.warn("We detected that you do not use a PostgreSQL database. " + "The DataStore will NOT work and datastore tests will be skipped.") ## Do light wrapping around action function to add datastore_active ## to resource dict. Not using IAction extension as this prevents other plugins diff --git a/ckanext/datastore/tests/test_create.py b/ckanext/datastore/tests/test_create.py index 4fd7da6b48c..d4afaf01fd4 100644 --- a/ckanext/datastore/tests/test_create.py +++ b/ckanext/datastore/tests/test_create.py @@ -1,4 +1,5 @@ import json +import nose import sqlalchemy.orm as orm @@ -18,6 +19,8 @@ class TestDatastoreCreate(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') diff --git a/ckanext/datastore/tests/test_delete.py b/ckanext/datastore/tests/test_delete.py index 41a43431d5e..01333142585 100644 --- a/ckanext/datastore/tests/test_delete.py +++ b/ckanext/datastore/tests/test_delete.py @@ -1,4 +1,5 @@ import json +import nose import sqlalchemy import sqlalchemy.orm as orm @@ -19,6 +20,8 @@ class TestDatastoreDelete(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') diff --git a/ckanext/datastore/tests/test_search.py b/ckanext/datastore/tests/test_search.py index 36fe8556a31..44e6a4f5ac8 100644 --- a/ckanext/datastore/tests/test_search.py +++ b/ckanext/datastore/tests/test_search.py @@ -1,4 +1,5 @@ import json +import nose import pprint import sqlalchemy.orm as orm @@ -18,6 +19,8 @@ class TestDatastoreSearch(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') @@ -331,6 +334,8 @@ def test_search_table_metadata(self): class TestDatastoreFullTextSearch(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') @@ -397,6 +402,8 @@ class TestDatastoreSQL(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') diff --git a/ckanext/datastore/tests/test_upsert.py b/ckanext/datastore/tests/test_upsert.py index fa8016a6d3c..de32c376f4c 100644 --- a/ckanext/datastore/tests/test_upsert.py +++ b/ckanext/datastore/tests/test_upsert.py @@ -1,4 +1,5 @@ import json +import nose import datetime import sqlalchemy.orm as orm @@ -19,6 +20,8 @@ class TestDatastoreUpsert(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') @@ -243,6 +246,8 @@ class TestDatastoreInsert(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') @@ -344,6 +349,8 @@ class TestDatastoreUpdate(tests.WsgiAppCase): @classmethod def setup_class(cls): + if not tests.is_datastore_supported(): + raise nose.SkipTest("Datastore not supported") p.load('datastore') ctd.CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') From 6e33f76b18db06e6f1b643cf7f9645daba059ac9 Mon Sep 17 00:00:00 2001 From: kindly Date: Mon, 28 Jan 2013 10:16:30 +0000 Subject: [PATCH 15/35] make group_list_dictize reasonably fast by going through search index --- ckan/lib/dictization/model_dictize.py | 12 ++++++++++-- ckan/tests/functional/test_group.py | 2 ++ ckan/tests/logic/test_action.py | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 1c7865e9516..ab6a4f107d0 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -18,6 +18,15 @@ def group_list_dictize(obj_list, context, active = context.get('active', True) with_private = context.get('include_private_packages', False) + + query = search.PackageSearchQuery() + + q = {'q': '+capacity:public' if not with_private else '*:*', + 'fl': 'groups', 'facet.field': ['groups'], + 'facet.limit': -1, 'rows': 1} + + query.run(q) + result_list = [] for obj in obj_list: @@ -32,8 +41,7 @@ def group_list_dictize(obj_list, context, group_dict['display_name'] = obj.display_name - group_dict['packages'] = \ - len(obj.packages(with_private=with_private, context=context)) + group_dict['packages'] = query.facets['groups'].get(obj.name, 0) if context.get('for_view'): if group_dict['is_organization']: diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index cbe1fb52f13..c45090a6a7d 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -8,6 +8,7 @@ import ckan.model as model from ckan.lib.create_test_data import CreateTestData from ckan.logic import check_access, NotAuthorized, get_action +import ckan.lib.search as search from pylons import config @@ -51,6 +52,7 @@ class TestGroup(FunctionalTestCase): @classmethod def setup_class(self): + search.clear() model.Session.remove() CreateTestData.create() diff --git a/ckan/tests/logic/test_action.py b/ckan/tests/logic/test_action.py index 6f418edfce3..1e115b24374 100644 --- a/ckan/tests/logic/test_action.py +++ b/ckan/tests/logic/test_action.py @@ -16,6 +16,7 @@ from ckan.logic import get_action, NotAuthorized from ckan.logic.action import get_domain_object from ckan.tests import TestRoles +import ckan.lib.search as search from ckan import plugins from ckan.plugins import SingletonPlugin, implements, IPackageController @@ -28,6 +29,7 @@ class TestAction(WsgiAppCase): @classmethod def setup_class(cls): + search.clear() CreateTestData.create() cls.sysadmin_user = model.User.get('testsysadmin') cls.normal_user = model.User.get('annafan') From 2a74c8dc16900ce364a86ddeed746b3f5ac90661 Mon Sep 17 00:00:00 2001 From: kindly Date: Mon, 28 Jan 2013 23:15:48 +0000 Subject: [PATCH 16/35] [#309] add package count using search index --- ckan/controllers/home.py | 1 + ckan/lib/dictization/model_dictize.py | 6 ++++++ ckan/logic/action/get.py | 1 + ckan/templates/snippets/organization.html | 2 +- ckan/tests/lib/test_dictization.py | 7 ++++++- 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ckan/controllers/home.py b/ckan/controllers/home.py index 1fe85d47b56..a766771cd39 100644 --- a/ckan/controllers/home.py +++ b/ckan/controllers/home.py @@ -125,6 +125,7 @@ def db_to_form_schema(group_type=None): 'ignore_auth': True, 'user': c.user or c.author, 'schema': db_to_form_schema(group_type=group_type), + 'limits': {'packages': 2}, 'for_view': True} data_dict = {'id': id} diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index ab6a4f107d0..490b5601fc5 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -318,6 +318,8 @@ def _get_members(context, group, member_type): filter(model.Member.table_name == member_type[:-1]) if member_type == 'packages': q = q.filter(Entity.private==False) + if 'limits' in context and member_type in context['limits']: + return q[:context['limits'][member_type]] return q.all() @@ -336,6 +338,10 @@ def group_dictize(group, context): _get_members(context, group, 'packages'), context) + query = search.PackageSearchQuery() + q = {'q': 'groups:"%s" +capacity:public' % group.name, 'rows': 1} + result_dict['package_count'] = query.run(q)['count'] + result_dict['tags'] = tag_list_dictize( _get_members(context, group, 'tags'), context) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 331b8be3c72..99dac9f194c 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -824,6 +824,7 @@ def _group_or_org_show(context, data_dict, is_org=False): _check_access('organization_show',context, data_dict) else: _check_access('group_show',context, data_dict) + group_dict = model_dictize.group_dictize(group, context) diff --git a/ckan/templates/snippets/organization.html b/ckan/templates/snippets/organization.html index 8988a4c1338..d0fddf914bd 100644 --- a/ckan/templates/snippets/organization.html +++ b/ckan/templates/snippets/organization.html @@ -35,7 +35,7 @@

{{ organization.title or organization.name }}

{{ _('Datasets') }}
-
{{ h.SI_number_span(organization.packages|length) }}
+
{{ h.SI_number_span(organization.package_count) }}
diff --git a/ckan/tests/lib/test_dictization.py b/ckan/tests/lib/test_dictization.py index c78c9365dc0..61828f0a27d 100644 --- a/ckan/tests/lib/test_dictization.py +++ b/ckan/tests/lib/test_dictization.py @@ -1,6 +1,7 @@ from ckan.tests import assert_equal, assert_not_in, assert_in from pprint import pprint, pformat from difflib import unified_diff +import ckan.lib.search as search from ckan.lib.create_test_data import CreateTestData from ckan import model @@ -32,6 +33,7 @@ class TestBasicDictize: def setup_class(cls): # clean the db so we can run these tests on their own model.repo.rebuild_db() + search.clear() CreateTestData.create() cls.package_expected = { @@ -928,6 +930,7 @@ def test_16_group_dictized(self): 'name': u'help', 'display_name': u'help', 'image_url': u'', + 'package_count': 2, 'is_organization': False, 'packages': [{'author': None, 'author_email': None, @@ -969,10 +972,12 @@ def test_16_group_dictized(self): result['packages'] = sorted(result['packages'], key=lambda x: x['name']) assert_equal(sorted(result.keys()), sorted(expected.keys())) + for key in result: - if key == 'is_organization': + if key in ('is_organization', 'package_count'): continue assert_equal(sorted(result[key]), sorted(expected[key])) + assert_equal(result['package_count'], expected['package_count']) def test_17_group_apis_to_dict(self): From 1fa0b1a464a44019fc5c65277f0cd7e95222b178 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Wed, 30 Jan 2013 10:12:17 +0000 Subject: [PATCH 17/35] [#257] Set timeout to 10 seconds in recline source --- ckanext/reclinepreview/theme/public/vendor/recline/recline.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckanext/reclinepreview/theme/public/vendor/recline/recline.js b/ckanext/reclinepreview/theme/public/vendor/recline/recline.js index 083279fc2a3..912773291b8 100644 --- a/ckanext/reclinepreview/theme/public/vendor/recline/recline.js +++ b/ckanext/reclinepreview/theme/public/vendor/recline/recline.js @@ -423,7 +423,7 @@ this.recline.Backend.DataProxy = this.recline.Backend.DataProxy || {}; my.dataproxy_url = 'http://jsonpdataproxy.appspot.com'; // Timeout for dataproxy (after this time if no response we error) // Needed because use JSONP so do not receive e.g. 500 errors - my.timeout = 5000; + my.timeout = 10000; // ## load // From e024873f596d481137845951af57988b077969fb Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 12:15:40 +0100 Subject: [PATCH 18/35] [#316] Disable follower lists Disable follower lists for normal users Make it so that only sysadmins can see a dataset, user or group's list of followers. Add auth functions to user, dataset and group follower_list functions. user controller, group controller, package controller: /followers pages send 401 if not authorized Remove followers tabs from user, dataset and group pages. Some tests need to be fixed now --- ckan/controllers/group.py | 6 +++++- ckan/controllers/user.py | 5 ++++- ckan/logic/action/get.py | 3 +++ ckan/logic/auth/get.py | 12 ++++++++++++ ckan/templates/group/read_base.html | 3 --- ckan/templates/package/read_base.html | 1 - ckan/templates/user/read_base.html | 1 - 7 files changed, 24 insertions(+), 7 deletions(-) diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index f9e21baaf38..57395125e2c 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -673,7 +673,11 @@ def followers(self, id): context = {'model': model, 'session': model.Session, 'user': c.user or c.author} c.group_dict = self._get_group_dict(id) - c.followers = get_action('group_follower_list')(context, {'id': id}) + try: + c.followers = get_action('group_follower_list')(context, + {'id': id}) + except NotAuthorized: + abort(401, _('Unauthorized to view followers %s') % '') return render('group/followers.html') def admins(self, id): diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index d4eea07fcc1..ca5f4899577 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -479,7 +479,10 @@ def followers(self, id=None): data_dict = {'id': id, 'user_obj': c.userobj} self._setup_template_variables(context, data_dict) f = get_action('user_follower_list') - c.followers = f(context, {'id': c.user_dict['id']}) + try: + c.followers = f(context, {'id': c.user_dict['id']}) + except NotAuthorized: + abort(401, _('Unauthorized to view followers %s') % '') return render('user/followers.html') def activity(self, id, offset=0): diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 5a6525dfa58..9d69f537613 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -2228,6 +2228,7 @@ def user_follower_list(context, data_dict): :rtype: list of dictionaries ''' + _check_access('user_follower_list', context, data_dict) return _follower_list(context, data_dict, ckan.logic.schema.default_follow_user_schema(), context['model'].UserFollowingUser) @@ -2242,6 +2243,7 @@ def dataset_follower_list(context, data_dict): :rtype: list of dictionaries ''' + _check_access('dataset_follower_list', context, data_dict) return _follower_list(context, data_dict, ckan.logic.schema.default_follow_dataset_schema(), context['model'].UserFollowingDataset) @@ -2256,6 +2258,7 @@ def group_follower_list(context, data_dict): :rtype: list of dictionaries ''' + _check_access('group_follower_list', context, data_dict) return _follower_list(context, data_dict, ckan.logic.schema.default_follow_group_schema(), context['model'].UserFollowingGroup) diff --git a/ckan/logic/auth/get.py b/ckan/logic/auth/get.py index 95a75b0d1af..f79e1b3b54a 100644 --- a/ckan/logic/auth/get.py +++ b/ckan/logic/auth/get.py @@ -217,3 +217,15 @@ def dashboard_new_activities_count(context, data_dict): # This is so a better not authourized message can be sent. return new_authz.is_authorized('dashboard_activity_list', context, data_dict) + + +def user_follower_list(context, data_dict): + return sysadmin(context, data_dict) + + +def dataset_follower_list(context, data_dict): + return sysadmin(context, data_dict) + + +def group_follower_list(context, data_dict): + return sysadmin(context, data_dict) diff --git a/ckan/templates/group/read_base.html b/ckan/templates/group/read_base.html index f476bce7a6b..e41cb7ebe7d 100644 --- a/ckan/templates/group/read_base.html +++ b/ckan/templates/group/read_base.html @@ -26,9 +26,6 @@ {% link_for _('Activity Stream'), controller='group', action='activity', id=c.group_dict.name, icon='time' %} - - {% link_for _('Followers'), controller='group', action='followers', id=c.group_dict.name, icon='group' %} - {% link_for _('Administrators'), controller='group', action='admins', id=c.group_dict.name, icon='cog' %} diff --git a/ckan/templates/package/read_base.html b/ckan/templates/package/read_base.html index 9a3d838ceb8..14c90e7cbf9 100644 --- a/ckan/templates/package/read_base.html +++ b/ckan/templates/package/read_base.html @@ -43,7 +43,6 @@ {% snippet 'snippets/page_header.html', items=[ h.build_nav_icon('dataset_read', _('Dataset'), id=pkg.name), h.build_nav_icon('dataset_activity', _('Activity Stream'), id=pkg.name), - h.build_nav_icon('dataset_followers', _('Followers'), id=pkg.name), h.build_nav_icon('related_list', _('Related'), id=pkg.name), ] %} {% endblock %} diff --git a/ckan/templates/user/read_base.html b/ckan/templates/user/read_base.html index cf287f33170..6baf84d3aa5 100644 --- a/ckan/templates/user/read_base.html +++ b/ckan/templates/user/read_base.html @@ -24,7 +24,6 @@ {% snippet 'snippets/page_header.html', items=[ h.build_nav_icon('user_datasets', _('Datasets'), id=user.name), h.build_nav_icon('user_activity_stream', _('Activity Stream'), id=user.name), - h.build_nav_icon('user_followers', _('Followers'), id=user.name), ] %} {% endblock %}
From 44a1fd16956341331e9eb534911a9d08acc6a758 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 13:02:51 +0100 Subject: [PATCH 19/35] [#316] Update follower api tests Make it pass a sysadmin's API key whenever calling *_follower_list, since only sysadmins are authorised to call that now. --- ckan/tests/functional/api/test_follow.py | 116 ++++++++++++++--------- 1 file changed, 72 insertions(+), 44 deletions(-) diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index 90bc4a23101..d03d20e7714 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -25,7 +25,8 @@ def datetime_from_string(s): ''' return datetime.datetime.strptime(s, '%Y-%m-%dT%H:%M:%S.%f') -def follow_user(app, follower_id, apikey, object_id, object_arg): +def follow_user(app, follower_id, apikey, object_id, object_arg, + sysadmin_apikey): '''Test a user starting to follow another user via the API. :param follower_id: id of the user that will be following something. @@ -65,7 +66,7 @@ def follow_user(app, follower_id, apikey, object_id, object_arg): # Check that the follower appears in the object's list of followers. followers = ckan.tests.call_action_api(app, 'user_follower_list', - id=object_id) + id=object_id, apikey=sysadmin_apikey) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 @@ -85,7 +86,8 @@ def follow_user(app, follower_id, apikey, object_id, object_arg): 'user_followee_count', id=follower_id) assert followee_count_after == followee_count_before + 1 -def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): +def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg, + sysadmin_apikey): '''Test a user starting to follow a dataset via the API. :param follower_id: id of the user. @@ -125,7 +127,7 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): # Check that the follower appears in the dataset's list of followers. followers = ckan.tests.call_action_api(app, 'dataset_follower_list', - id=dataset_id) + id=dataset_id, apikey=sysadmin_apikey) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 @@ -145,7 +147,7 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): 'dataset_followee_count', id=follower_id) assert followee_count_after == followee_count_before + 1 -def follow_group(app, user_id, apikey, group_id, group_arg): +def follow_group(app, user_id, apikey, group_id, group_arg, sysadmin_apikey): '''Test a user starting to follow a group via the API. :param user_id: id of the user @@ -185,7 +187,7 @@ def follow_group(app, user_id, apikey, group_id, group_arg): # Check that the user appears in the group's list of followers. followers = ckan.tests.call_action_api(app, 'group_follower_list', - id=group_id) + id=group_id, apikey=sysadmin_apikey) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == user_id]) == 1 @@ -316,27 +318,33 @@ def test_01_follow_missing_object_id(self): def test_02_user_follow_user_by_id(self): follow_user(self.app, self.annafan['id'], self.annafan['apikey'], - self.russianfan['id'], self.russianfan['id']) + self.russianfan['id'], self.russianfan['id'], + self.testsysadmin['apikey']) def test_02_user_follow_dataset_by_id(self): follow_dataset(self.app, self.annafan['id'], self.annafan['apikey'], - self.warandpeace['id'], self.warandpeace['id']) + self.warandpeace['id'], self.warandpeace['id'], + self.testsysadmin['apikey']) def test_02_user_follow_group_by_id(self): follow_group(self.app, self.annafan['id'], self.annafan['apikey'], - self.rogers_group['id'], self.rogers_group['id']) + self.rogers_group['id'], self.rogers_group['id'], + self.testsysadmin['apikey']) def test_02_user_follow_user_by_name(self): follow_user(self.app, self.annafan['id'], self.annafan['apikey'], - self.testsysadmin['id'], self.testsysadmin['name']) + self.testsysadmin['id'], self.testsysadmin['name'], + self.testsysadmin['apikey']) def test_02_user_follow_dataset_by_name(self): follow_dataset(self.app, self.joeadmin['id'], self.joeadmin['apikey'], - self.warandpeace['id'], self.warandpeace['name']) + self.warandpeace['id'], self.warandpeace['name'], + self.testsysadmin['apikey']) def test_02_user_follow_group_by_name(self): follow_group(self.app, self.joeadmin['id'], self.joeadmin['apikey'], - self.rogers_group['id'], self.rogers_group['name']) + self.rogers_group['id'], self.rogers_group['name'], + self.testsysadmin['apikey']) def test_03_user_follow_user_already_following(self): for object_id in (self.russianfan['id'], self.russianfan['name'], @@ -400,28 +408,31 @@ def test_04_follower_list_bad_id(self): 'group_follower_list'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): error = ckan.tests.call_action_api(self.app, action, - status=409, id=object_id) + status=409, id=object_id, + apikey=self.testsysadmin['apikey']) assert error['id'] def test_04_follower_list_missing_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): - error = ckan.tests.call_action_api(self.app, action, status=409) + error = ckan.tests.call_action_api(self.app, action, status=409, + apikey=self.testsysadmin['apikey']) assert error['id'] == ['Missing value'] def test_04_user_follower_list_no_followers(self): followers = ckan.tests.call_action_api(self.app, 'user_follower_list', - id=self.annafan['id']) + id=self.annafan['id'], apikey=self.testsysadmin['apikey']) assert followers == [] def test_04_dataset_follower_list_no_followers(self): followers = ckan.tests.call_action_api(self.app, - 'dataset_follower_list', id=self.annakarenina['id']) + 'dataset_follower_list', id=self.annakarenina['id'], + apikey=self.testsysadmin['apikey']) assert followers == [] def test_04_group_follower_list_no_followers(self): followers = ckan.tests.call_action_api(self.app, 'group_follower_list', - id=self.davids_group['id']) + id=self.davids_group['id'], apikey=self.testsysadmin['apikey']) assert followers == [] def test_04_am_following_bad_id(self): @@ -529,26 +540,34 @@ def setup_class(self): self.app = paste.fixture.TestApp(pylons.test.pylonsapp) follow_user(self.app, self.testsysadmin['id'], self.testsysadmin['apikey'], self.joeadmin['id'], - self.joeadmin['id']) + self.joeadmin['id'], self.testsysadmin['apikey']) follow_user(self.app, self.tester['id'], self.tester['apikey'], - self.joeadmin['id'], self.joeadmin['id']) + self.joeadmin['id'], self.joeadmin['id'], + self.testsysadmin['apikey']) follow_user(self.app, self.russianfan['id'], self.russianfan['apikey'], - self.joeadmin['id'], self.joeadmin['id']) + self.joeadmin['id'], self.joeadmin['id'], + self.testsysadmin['apikey']) follow_user(self.app, self.annafan['id'], self.annafan['apikey'], - self.joeadmin['id'], self.joeadmin['id']) + self.joeadmin['id'], self.joeadmin['id'], + self.testsysadmin['apikey']) follow_user(self.app, self.annafan['id'], self.annafan['apikey'], - self.tester['id'], self.tester['id']) + self.tester['id'], self.tester['id'], + self.testsysadmin['apikey']) follow_dataset(self.app, self.testsysadmin['id'], self.testsysadmin['apikey'], self.warandpeace['id'], - self.warandpeace['id']) + self.warandpeace['id'], self.testsysadmin['apikey']) follow_dataset(self.app, self.tester['id'], self.tester['apikey'], - self.warandpeace['id'], self.warandpeace['id']) + self.warandpeace['id'], self.warandpeace['id'], + self.testsysadmin['apikey']) follow_dataset(self.app, self.russianfan['id'], self.russianfan['apikey'], - self.warandpeace['id'], self.warandpeace['id']) + self.warandpeace['id'], self.warandpeace['id'], + self.testsysadmin['apikey']) follow_dataset(self.app, self.annafan['id'], self.annafan['apikey'], - self.warandpeace['id'], self.warandpeace['id']) + self.warandpeace['id'], self.warandpeace['id'], + self.testsysadmin['apikey']) follow_group(self.app, self.annafan['id'], self.annafan['apikey'], - self.davids_group['id'], self.davids_group['id']) + self.davids_group['id'], self.davids_group['id'], + self.testsysadmin['apikey']) @classmethod def teardown_class(self): @@ -653,7 +672,7 @@ def _unfollow_user(self, follower_id, apikey, object_id, object_arg): # Check that the user doesn't appear in the object's list of followers. followers = ckan.tests.call_action_api(self.app, 'user_follower_list', - id=object_id) + id=object_id, apikey=self.testsysadmin['apikey']) assert len([follower for follower in followers if follower['id'] == follower_id]) == 0 @@ -693,7 +712,8 @@ def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): # Check that the user doesn't appear in the dataset's list of # followers. followers = ckan.tests.call_action_api(self.app, - 'dataset_follower_list', id=dataset_id) + 'dataset_follower_list', id=dataset_id, + apikey=self.testsysadmin['apikey']) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 @@ -733,7 +753,7 @@ def _unfollow_group(self, user_id, apikey, group_id, group_arg): # Check that the user doesn't appear in the group's list of # followers. followers = ckan.tests.call_action_api(self.app, 'group_follower_list', - id=group_id) + id=group_id, apikey=self.testsysadmin['apikey']) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 @@ -800,31 +820,38 @@ def setup_class(self): self.app = paste.fixture.TestApp(pylons.test.pylonsapp) follow_user(self.app, self.joeadmin['id'], self.joeadmin['apikey'], - self.testsysadmin['id'], self.testsysadmin['id']) + self.testsysadmin['id'], self.testsysadmin['id'], + self.testsysadmin['apikey']) follow_user(self.app, self.annafan['id'], self.annafan['apikey'], - self.testsysadmin['id'], self.testsysadmin['id']) + self.testsysadmin['id'], self.testsysadmin['id'], + self.testsysadmin['apikey']) follow_user(self.app, self.russianfan['id'], self.russianfan['apikey'], - self.testsysadmin['id'], self.testsysadmin['id']) + self.testsysadmin['id'], self.testsysadmin['id'], + self.testsysadmin['apikey']) follow_dataset(self.app, self.joeadmin['id'], self.joeadmin['apikey'], - self.annakarenina['id'], self.annakarenina['id']) + self.annakarenina['id'], self.annakarenina['id'], + self.testsysadmin['apikey']) follow_dataset(self.app, self.annafan['id'], self.annafan['apikey'], - self.annakarenina['id'], self.annakarenina['id']) + self.annakarenina['id'], self.annakarenina['id'], + self.testsysadmin['apikey']) follow_dataset(self.app, self.russianfan['id'], self.russianfan['apikey'], - self.annakarenina['id'], self.annakarenina['id']) + self.annakarenina['id'], self.annakarenina['id'], + self.testsysadmin['apikey']) follow_user(self.app, self.tester['id'], self.tester['apikey'], - self.joeadmin['id'], self.joeadmin['id']) + self.joeadmin['id'], self.joeadmin['id'], + self.testsysadmin['apikey']) follow_dataset(self.app, self.testsysadmin['id'], self.testsysadmin['apikey'], self.warandpeace['id'], - self.warandpeace['id']) + self.warandpeace['id'], self.testsysadmin['apikey']) follow_group(self.app, self.testsysadmin['id'], self.testsysadmin['apikey'], self.davids_group['id'], - self.davids_group['id']) + self.davids_group['id'], self.testsysadmin['apikey']) session = ckan.model.Session() session.delete(ckan.model.User.get('joeadmin')) @@ -848,17 +875,17 @@ def test_01_on_delete_cascade_api(self): ''' # It should no longer be possible to get joeadmin's follower list. error = ckan.tests.call_action_api(self.app, 'user_follower_list', - status=409, id='joeadmin') + status=409, id='joeadmin', apikey=self.testsysadmin['apikey']) assert 'id' in error # It should no longer be possible to get warandpeace's follower list. error = ckan.tests.call_action_api(self.app, 'dataset_follower_list', - status=409, id='warandpeace') + status=409, id='warandpeace', apikey=self.testsysadmin['apikey']) assert 'id' in error # It should no longer be possible to get david's follower list. error = ckan.tests.call_action_api(self.app, 'group_follower_list', - status=409, id='david') + status=409, id='david', apikey=self.testsysadmin['apikey']) assert 'id' in error # It should no longer be possible to get joeadmin's follower count. @@ -926,13 +953,14 @@ def test_01_on_delete_cascade_api(self): # Users who joeadmin was following should no longer have him in their # follower list. followers = ckan.tests.call_action_api(self.app, 'user_follower_list', - id=self.testsysadmin['id']) + id=self.testsysadmin['id'], apikey=self.testsysadmin['apikey']) assert 'joeadmin' not in [follower['name'] for follower in followers] # Datasets who joeadmin was following should no longer have him in # their follower list. followers = ckan.tests.call_action_api(self.app, - 'dataset_follower_list', id=self.annakarenina['id']) + 'dataset_follower_list', id=self.annakarenina['id'], + apikey=self.testsysadmin['apikey']) assert 'joeadmin' not in [follower['name'] for follower in followers] def test_02_on_delete_cascade_db(self): From eab929e4842179dc09583cd67f58e9677d2c1b6f Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Wed, 30 Jan 2013 14:03:29 +0000 Subject: [PATCH 20/35] [#257] Set the timeout outside the recline source so that it can be updated without patching the source again --- ckanext/reclinepreview/theme/public/preview_recline.js | 2 ++ ckanext/reclinepreview/theme/public/vendor/recline/recline.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ckanext/reclinepreview/theme/public/preview_recline.js b/ckanext/reclinepreview/theme/public/preview_recline.js index 91c5063e816..bf56d89b66c 100644 --- a/ckanext/reclinepreview/theme/public/preview_recline.js +++ b/ckanext/reclinepreview/theme/public/preview_recline.js @@ -39,6 +39,8 @@ this.ckan.module('reclinepreview', function (jQuery, _) { .html(msg); } + recline.Backend.DataProxy.timeout = 10000; + // 2 situations // a) something was posted to the datastore - need to check for this // b) csv or xls (but not datastore) diff --git a/ckanext/reclinepreview/theme/public/vendor/recline/recline.js b/ckanext/reclinepreview/theme/public/vendor/recline/recline.js index 912773291b8..083279fc2a3 100644 --- a/ckanext/reclinepreview/theme/public/vendor/recline/recline.js +++ b/ckanext/reclinepreview/theme/public/vendor/recline/recline.js @@ -423,7 +423,7 @@ this.recline.Backend.DataProxy = this.recline.Backend.DataProxy || {}; my.dataproxy_url = 'http://jsonpdataproxy.appspot.com'; // Timeout for dataproxy (after this time if no response we error) // Needed because use JSONP so do not receive e.g. 500 errors - my.timeout = 10000; + my.timeout = 5000; // ## load // From 40e4630d24a81b3294d31d93d5b942f02713acff Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 15:47:46 +0100 Subject: [PATCH 21/35] [#316] Update follower frontend tests --- ckan/tests/functional/test_follow.py | 82 +++++----------------------- 1 file changed, 14 insertions(+), 68 deletions(-) diff --git a/ckan/tests/functional/test_follow.py b/ckan/tests/functional/test_follow.py index 268fb823706..acb97deb0ac 100644 --- a/ckan/tests/functional/test_follow.py +++ b/ckan/tests/functional/test_follow.py @@ -68,24 +68,12 @@ def test_dataset_read_not_logged_in(self): assert 'id="dataset_follow_button"' not in result def test_dataset_followers_not_logged_in(self): + '''Not-logged-in users cannot see /dataset/followers/ pages.''' offset = url_for(controller='package', action='followers', id='warandpeace') result = self.app.get(offset) - assert 'href="/dataset/followers/warandpeace"' in result - assert 'Followers (0)' in result - assert 'id="dataset_follow_button"' not in result - assert '
  • ' not in result - - offset = url_for(controller='package', action='followers', - id='annakarenina') - result = self.app.get(offset) - assert 'href="/dataset/followers/annakarenina"' in result - assert 'Followers (3)' in result - assert 'id="dataset_follow_button"' not in result - assert str(result).count('
  • ') == 3 - assert self.joeadmin.display_name in result - assert self.annafan.display_name in result - assert self.russianfan.display_name in result + assert result.status == 302 + assert '/user/login' in result.header_dict['location'] def test_user_read_not_logged_in(self): offset = url_for(controller='user', action='read', @@ -106,20 +94,8 @@ def test_user_followers_not_logged_in(self): offset = url_for(controller='user', action='followers', id='joeadmin') result = self.app.get(offset) - assert 'href="/user/followers/joeadmin"' in result - assert 'Followers (0)' in result - assert '
  • ' not in result - assert 'id="user_follow_button"' not in result - - offset = url_for(controller='user', action='followers', - id='annafan') - result = self.app.get(offset) - assert 'href="/user/followers/annafan"' in result - assert 'Followers (2)' in result - assert 'id="user_follow_button"' not in result - assert str(result).count('
  • ') == 2 - assert self.tester.display_name in result - assert self.russianfan.display_name in result + assert result.status == 302 + assert '/user/login' in result.header_dict['location'] def test_own_user_read_logged_in(self): offset = url_for(controller='user', action='read', @@ -141,28 +117,17 @@ def test_own_user_read_logged_in(self): def test_own_user_followers_logged_in(self): offset = url_for(controller='user', action='followers', id='joeadmin') - extra_environ = {'Authorization': str(self.joeadmin.apikey)} + extra_environ = {'Authorization': str(self.testsysadmin.apikey)} result = self.app.get(offset, extra_environ=extra_environ) assert 'href="/user/followers/joeadmin"' in result - assert 'My Followers (0)' in result - assert 'id="user_follow_button"' not in result + assert 'Followers (0)' in result + assert 'id="user_follow_button"' in result assert '
  • ' not in result - offset = url_for(controller='user', action='followers', - id='annafan') - extra_environ = {'Authorization': str(self.annafan.apikey)} - result = self.app.get(offset, extra_environ=extra_environ) - assert 'href="/user/followers/annafan"' in result - assert 'My Followers (2)' in result - assert 'id="user_follow_button"' not in result - assert str(result).count('
  • ') == 2 - assert self.tester.display_name in result - assert self.russianfan.display_name in result - def test_dataset_read_logged_in(self): offset = url_for(controller='package', action='read', id='warandpeace') - extra_environ = {'Authorization': str(self.tester.apikey)} + extra_environ = {'Authorization': str(self.testsysadmin.apikey)} result = self.app.get(offset, extra_environ=extra_environ) assert 'href="/dataset/followers/warandpeace"' in result assert 'Followers (0)' in result @@ -179,16 +144,16 @@ def test_dataset_read_logged_in(self): def test_dataset_follow_logged_in(self): offset = url_for(controller='package', action='followers', id='warandpeace') - extra_environ = {'Authorization': str(self.tester.apikey)} + extra_environ = {'Authorization': str(self.testsysadmin.apikey)} result = self.app.get(offset, extra_environ=extra_environ) - assert 'href="/dataset/followers/warandpeace"' in result + assert 'id="dataset_follow_button"' in result assert 'Followers (0)' in result assert 'id="dataset_follow_button"' in result assert '
  • ' not in result offset = url_for(controller='package', action='followers', id='annakarenina') - extra_environ = {'Authorization': str(self.tester.apikey)} + extra_environ = {'Authorization': str(self.testsysadmin.apikey)} result = self.app.get(offset, extra_environ=extra_environ) assert 'href="/dataset/followers/annakarenina"' in result assert 'Followers (3)' in result @@ -202,7 +167,7 @@ def test_dataset_follow_logged_in(self): # button. offset = url_for(controller='package', action='followers', id='annakarenina') - extra_environ = {'Authorization': str(self.joeadmin.apikey)} + extra_environ = {'Authorization': str(self.testsysadmin.apikey)} result = self.app.get(offset, extra_environ=extra_environ) assert 'Unfollow' in result @@ -226,28 +191,9 @@ def test_user_read_logged_in(self): def test_user_follow_logged_in(self): offset = url_for(controller='user', action='followers', id='joeadmin') - extra_environ = {'Authorization': str(self.tester.apikey)} + extra_environ = {'Authorization': str(self.testsysadmin.apikey)} result = self.app.get(offset, extra_environ=extra_environ) assert 'href="/user/followers/joeadmin"' in result assert 'Followers (0)' in result assert '
  • ' not in result assert 'id="user_follow_button"' in result - - offset = url_for(controller='user', action='followers', - id='annafan') - extra_environ = {'Authorization': str(self.tester.apikey)} - result = self.app.get(offset, extra_environ=extra_environ) - assert 'href="/user/followers/annafan"' in result - assert 'Followers (2)' in result - assert 'id="user_follow_button"' in result - assert str(result).count('
  • ') == 2 - assert self.tester.display_name in result - assert self.russianfan.display_name in result - - # russianfan is following annafan so he should see an Unfollow - # button. - offset = url_for(controller='user', action='followers', - id='annafan') - extra_environ = {'Authorization': str(self.russianfan.apikey)} - result = self.app.get(offset, extra_environ=extra_environ) - assert 'Unfollow' in result From 825117eca63105ac582b7708f3dde242499c30db Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 16:03:52 +0100 Subject: [PATCH 22/35] [#316] Add tests for new follower list auth --- ckan/tests/functional/api/test_follow.py | 39 ++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index d03d20e7714..e3cfdd74994 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -258,6 +258,45 @@ def setup_class(self): def teardown_class(self): ckan.model.repo.rebuild_db() + def test_00_visitor_cannot_get_user_follower_list(self): + ckan.tests.call_action_api(self.app, 'user_follower_list', + id=self.russianfan['id'], status=403) + + def test_00_user_cannot_get_user_follower_list(self): + ckan.tests.call_action_api(self.app, 'user_follower_list', + id=self.russianfan['id'], status=403, + apikey=self.annafan['apikey']) + + def test_00_sysadmin_can_get_user_follower_list(self): + ckan.tests.call_action_api(self.app, 'user_follower_list', + id=self.russianfan['id'], status=200, + apikey=self.testsysadmin['apikey']) + + def test_00_visitor_cannot_get_dataset_follower_list(self): + ckan.tests.call_action_api(self.app, 'dataset_follower_list', + id='warandpeace', status=403) + + def test_00_user_cannot_get_dataset_follower_list(self): + ckan.tests.call_action_api(self.app, 'dataset_follower_list', + id='warandpeace', status=403, apikey=self.annafan['apikey']) + + def test_00_sysadmin_can_get_dataset_follower_list(self): + ckan.tests.call_action_api(self.app, 'dataset_follower_list', + id='warandpeace', status=200, + apikey=self.testsysadmin['apikey']) + + def test_00_visitor_cannot_get_group_follower_list(self): + ckan.tests.call_action_api(self.app, 'group_follower_list', + id='roger', status=403) + + def test_00_user_cannot_get_group_follower_list(self): + ckan.tests.call_action_api(self.app, 'group_follower_list', + id='roger', status=403, apikey=self.annafan['apikey']) + + def test_00_sysadmin_can_get_group_follower_list(self): + ckan.tests.call_action_api(self.app, 'group_follower_list', + id='roger', status=200, apikey=self.testsysadmin['apikey']) + def test_01_user_follow_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): error = ckan.tests.call_action_api(self.app, 'follow_user', From 5c7e022a09798d02b1fd30356517a9ef071d0577 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 16:08:13 +0100 Subject: [PATCH 23/35] [#316] Remove "started following" activities --- ckan/logic/action/create.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index b9f1aca8679..5f551125920 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -1028,24 +1028,6 @@ def follow_user(context, data_dict): follower = model_save.follower_dict_save(validated_data_dict, context, model.UserFollowingUser) - activity_dict = { - 'user_id': userobj.id, - 'object_id': validated_data_dict['id'], - 'activity_type': 'follow user', - } - activity_dict['data'] = { - 'user': ckan.lib.dictization.table_dictize( - model.User.get(validated_data_dict['id']), context), - } - activity_create_context = { - 'model': model, - 'user': userobj, - 'defer_commit': True, - 'session': session - } - logic.get_action('activity_create')(activity_create_context, - activity_dict, ignore_auth=True) - if not context.get('defer_commit'): model.repo.commit() From a20d0b3f9aa5cf65d1f9780a98a7faaff90f6ab7 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 16:17:38 +0100 Subject: [PATCH 24/35] [#316] Update activity streams API tests --- ckan/tests/functional/api/test_activity.py | 38 ++++++++++++---------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index 4fa1ed45f72..c0daf7b17ee 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -2063,24 +2063,28 @@ def test_follow_user(self): # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( before['user activity stream'], after['user activity stream'])) - assert len(user_new_activities) == 1, ("There should be 1 new " - " activity in the user's activity stream, but found %i" % + assert len(user_new_activities) == 0, ("There should be 0 new " + " activities in the user's activity stream, but found %i" % len(user_new_activities)) - activity = user_new_activities[0] + + # The rest of this test is commented out because follow_user activities + # are disabled, uncomment it if they're enabled again. + + #activity = user_new_activities[0] # Check that the new activity has the right attributes. - assert activity['object_id'] == self.sysadmin_user['id'], \ - str(activity['object_id']) - assert activity['user_id'] == user['id'], str(activity['user_id']) - assert activity['activity_type'] == 'follow user', \ - str(activity['activity_type']) - if 'id' not in activity: - assert False, "activity object should have an id value" + #assert activity['object_id'] == self.sysadmin_user['id'], \ + # str(activity['object_id']) + #assert activity['user_id'] == user['id'], str(activity['user_id']) + #assert activity['activity_type'] == 'follow user', \ + # str(activity['activity_type']) + #if 'id' not in activity: + # assert False, "activity object should have an id value" # TODO: Test for the _correct_ revision_id value. - if 'revision_id' not in activity: - assert False, "activity object should have a revision_id value" - timestamp = datetime_from_string(activity['timestamp']) - assert timestamp >= before['time'] and timestamp <= \ - after['time'], str(activity['timestamp']) - - assert len(self.activity_details(activity)) == 0 + #if 'revision_id' not in activity: + # assert False, "activity object should have a revision_id value" + #timestamp = datetime_from_string(activity['timestamp']) + #assert timestamp >= before['time'] and timestamp <= \ + # after['time'], str(activity['timestamp']) + # + #assert len(self.activity_details(activity)) == 0 From b722815983378201dba41a4b2adc36c4430aafac Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 16:20:15 +0100 Subject: [PATCH 25/35] [#316] Update activity streams frontend tests --- ckan/tests/functional/test_activity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/tests/functional/test_activity.py b/ckan/tests/functional/test_activity.py index 7e4f0d89a3d..a8d9e0a9588 100644 --- a/ckan/tests/functional/test_activity.py +++ b/ckan/tests/functional/test_activity.py @@ -127,7 +127,7 @@ def test_user_activity(self): result = self.app.get(offset, status=200) stripped = self.strip_tags(result) assert '%s started following %s' % (user['fullname'], - 'joeadmin') in stripped, stripped + 'joeadmin') not in stripped, stripped # Create a new group. group = { From ef89d80549214276f5c3f7ee08c50ce28597a44e Mon Sep 17 00:00:00 2001 From: John Glover Date: Wed, 30 Jan 2013 15:55:39 +0000 Subject: [PATCH 26/35] Remove theme folder from gitignore --- .gitignore | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.gitignore b/.gitignore index daf2c2e2007..b765bfee61f 100644 --- a/.gitignore +++ b/.gitignore @@ -6,8 +6,6 @@ syntax: glob .DS_Store ckan.egg-info/* sandbox/* -theme/* -theme dist # pylons @@ -28,9 +26,6 @@ fl_notes.txt .noseids *~ -# local symlinks -ckan/public/scripts/ckanjs.js - # custom style ckan/public/base/less/custom.less From 7f4f0def48560951d0d171e2665dfeefaad80e82 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 18:19:07 +0100 Subject: [PATCH 27/35] [#316] Add auth for followee_list APIs Don't let visitors or other users see what a user is following. --- ckan/logic/action/get.py | 3 + ckan/logic/auth/get.py | 29 ++++++++++ ckan/tests/functional/api/test_follow.py | 73 +++++++++++++++++++++++- 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 9d69f537613..8ce5a442a3c 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -2383,6 +2383,7 @@ def user_followee_list(context, data_dict): :rtype: list of dictionaries ''' + _check_access('user_followee_list', context, data_dict) schema = context.get('schema') or ( ckan.logic.schema.default_follow_user_schema()) data_dict, errors = _validate(data_dict, schema, context) @@ -2410,6 +2411,7 @@ def dataset_followee_list(context, data_dict): :rtype: list of dictionaries ''' + _check_access('dataset_followee_list', context, data_dict) schema = context.get('schema') or ( ckan.logic.schema.default_follow_user_schema()) data_dict, errors = _validate(data_dict, schema, context) @@ -2438,6 +2440,7 @@ def group_followee_list(context, data_dict): :rtype: list of dictionaries ''' + _check_access('group_followee_list', context, data_dict) schema = context.get('schema', ckan.logic.schema.default_follow_user_schema()) data_dict, errors = _validate(data_dict, schema, context) diff --git a/ckan/logic/auth/get.py b/ckan/logic/auth/get.py index f79e1b3b54a..c6412bc27c9 100644 --- a/ckan/logic/auth/get.py +++ b/ckan/logic/auth/get.py @@ -229,3 +229,32 @@ def dataset_follower_list(context, data_dict): def group_follower_list(context, data_dict): return sysadmin(context, data_dict) + + +def _followee_list(context, data_dict): + model = context['model'] + + # Visitors cannot see what users are following. + authorized_user = model.User.get(context.get('user')) + if not authorized_user: + return {'success': False, 'msg': _('Not authorized')} + + # Any user is authorized to see what she herself is following. + requested_user = model.User.get(data_dict.get('id')) + if authorized_user == requested_user: + return {'success': True} + + # Sysadmins are authorized to see what anyone is following. + return sysadmin(context, data_dict) + + +def user_followee_list(context, data_dict): + return _followee_list(context, data_dict) + + +def dataset_followee_list(context, data_dict): + return _followee_list(context, data_dict) + + +def group_followee_list(context, data_dict): + return _followee_list(context, data_dict) diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index e3cfdd74994..eb4530e3cea 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -72,7 +72,7 @@ def follow_user(app, follower_id, apikey, object_id, object_arg, # Check that the object appears in the follower's list of followees. followees = ckan.tests.call_action_api(app, 'user_followee_list', - id=follower_id) + apikey=sysadmin_apikey, id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == object_id]) == 1 @@ -133,7 +133,7 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg, # Check that the dataset appears in the follower's list of followees. followees = ckan.tests.call_action_api(app, 'dataset_followee_list', - id=follower_id) + apikey=sysadmin_apikey, id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == dataset_id]) == 1 @@ -194,7 +194,7 @@ def follow_group(app, user_id, apikey, group_id, group_arg, sysadmin_apikey): # Check that the group appears in the user's list of followees. followees = ckan.tests.call_action_api(app, 'group_followee_list', - id=user_id) + apikey=sysadmin_apikey, id=user_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == group_id]) == 1 @@ -297,6 +297,73 @@ def test_00_sysadmin_can_get_group_follower_list(self): ckan.tests.call_action_api(self.app, 'group_follower_list', id='roger', status=200, apikey=self.testsysadmin['apikey']) + def test_00_visitor_cannot_get_user_followee_list(self): + '''A visitor cannot see what users a user is following.''' + ckan.tests.call_action_api(self.app, 'user_followee_list', + id=self.russianfan['id'], status=403) + + def test_00_user_cannot_get_user_followee_list(self): + '''A user cannot see what users another user is following.''' + ckan.tests.call_action_api(self.app, 'user_followee_list', + id=self.russianfan['id'], status=403, + apikey=self.annafan['apikey']) + + def test_00_sysadmin_can_get_user_followee_list(self): + '''A sysadmin can see what users another user is following.''' + ckan.tests.call_action_api(self.app, 'user_followee_list', + id=self.russianfan['id'], status=200, + apikey=self.testsysadmin['apikey']) + + def test_00_user_can_get_own_user_followee_list(self): + '''A user can see what users she herself is following.''' + ckan.tests.call_action_api(self.app, 'user_followee_list', + id=self.russianfan['id'], status=200, + apikey=self.russianfan['apikey']) + + def test_00_visitor_cannot_get_dataset_followee_list(self): + '''A visitor cannot see what datasets a user is following.''' + ckan.tests.call_action_api(self.app, 'dataset_followee_list', + id=self.russianfan['id'], status=403) + + def test_00_user_cannot_get_dataset_followee_list(self): + '''A user cannot see what datasets another user is following.''' + ckan.tests.call_action_api(self.app, 'dataset_followee_list', + id='russianfan', status=403, apikey=self.annafan['apikey']) + + def test_00_sysadmin_can_get_dataset_followee_list(self): + '''A sysadmin can see what datasets another user is following.''' + ckan.tests.call_action_api(self.app, 'dataset_followee_list', + id='russianfan', status=200, + apikey=self.testsysadmin['apikey']) + + def test_00_user_can_get_own_dataset_followee_list(self): + '''A user can see what datasets she herself is following.''' + ckan.tests.call_action_api(self.app, 'dataset_followee_list', + id=self.russianfan['id'], status=200, + apikey=self.russianfan['apikey']) + + def test_00_visitor_cannot_get_group_followee_list(self): + '''A visitor cannot see what groups a user is following.''' + ckan.tests.call_action_api(self.app, 'group_followee_list', + id='roger', status=403) + + def test_00_user_cannot_get_group_followee_list(self): + '''A user cannot see what groups another user is following.''' + ckan.tests.call_action_api(self.app, 'group_followee_list', + id='roger', status=403, apikey=self.annafan['apikey']) + + def test_00_sysadmin_can_get_group_followee_list(self): + '''A sysadmin can see what groups another user is following.''' + ckan.tests.call_action_api(self.app, 'group_followee_list', + id=self.annafan['id'], status=200, + apikey=self.testsysadmin['apikey']) + + def test_00_user_can_get_own_group_followee_list(self): + '''A user can see what groups she herself is following.''' + ckan.tests.call_action_api(self.app, 'group_followee_list', + id=self.russianfan['id'], status=200, + apikey=self.russianfan['apikey']) + def test_01_user_follow_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): error = ckan.tests.call_action_api(self.app, 'follow_user', From 6adc06072ad1535e5671ad3142a16408cae60cb5 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 18:38:42 +0100 Subject: [PATCH 28/35] [#316] Removed 'follow dataset' and 'follow group' activities Forgot to remove these in earlier commit 5c7e022a09798d02b1fd30356517a9ef071d0577 --- ckan/logic/action/create.py | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 5f551125920..13495a0d4df 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -1081,24 +1081,6 @@ def follow_dataset(context, data_dict): follower = model_save.follower_dict_save(validated_data_dict, context, model.UserFollowingDataset) - activity_dict = { - 'user_id': userobj.id, - 'object_id': validated_data_dict['id'], - 'activity_type': 'follow dataset', - } - activity_dict['data'] = { - 'dataset': ckan.lib.dictization.table_dictize( - model.Package.get(validated_data_dict['id']), context), - } - activity_create_context = { - 'model': model, - 'user': userobj, - 'defer_commit':True, - 'session': session - } - logic.get_action('activity_create')(activity_create_context, - activity_dict, ignore_auth=True) - if not context.get('defer_commit'): model.repo.commit() @@ -1191,24 +1173,6 @@ def follow_group(context, data_dict): follower = model_save.follower_dict_save(validated_data_dict, context, model.UserFollowingGroup) - activity_dict = { - 'user_id': userobj.id, - 'object_id': validated_data_dict['id'], - 'activity_type': 'follow group', - } - activity_dict['data'] = { - 'group': ckan.lib.dictization.table_dictize( - model.Group.get(validated_data_dict['id']), context), - } - activity_create_context = { - 'model': model, - 'user': userobj, - 'defer_commit': True, - 'session': session - } - logic.get_action('activity_create')(activity_create_context, - activity_dict, ignore_auth=True) - if not context.get('defer_commit'): model.repo.commit() From 99920836a6ee3ed4a4d8f4812ae28918d0278ee7 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 18:39:41 +0100 Subject: [PATCH 29/35] [#316] Update user dashboard tests --- ckan/tests/functional/api/test_dashboard.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index adf9953ce9a..f44e163e293 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -172,23 +172,23 @@ def test_03_dashboard_activity_list_own_activities(self): '''Test that a user's own activities appear in her dashboard.''' activities = self.dashboard_activity_list(self.new_user) - # FIXME: There should actually be 6 activities here, but when you + # FIXME: There should actually be 3 activities here, but when you # follow something it's old activities (from before you followed it) # appear in your activity stream. So here we get more activities than # expected. - assert len(activities) == 8 + assert len(activities) == 5, len(activities) assert activities[0]['activity_type'] == 'changed package' - assert activities[1]['activity_type'] == 'follow group' - assert activities[2]['activity_type'] == 'follow user' - assert activities[3]['activity_type'] == 'follow dataset' - assert activities[4]['activity_type'] == 'new package' - assert activities[5]['activity_type'] == 'new user' + #assert activities[1]['activity_type'] == 'follow group' + #assert activities[2]['activity_type'] == 'follow user' + #assert activities[3]['activity_type'] == 'follow dataset' + assert activities[1]['activity_type'] == 'new package' + assert activities[2]['activity_type'] == 'new user' - # FIXME: Shouldn't need the [:6] here, it's because when you follow + # FIXME: Shouldn't need the [:3] here, it's because when you follow # something its old activities (from before you started following it) # appear in your dashboard. - for activity in activities[:6]: + for activity in activities[:3]: assert activity['user_id'] == self.new_user['id'] def test_03_own_activities_not_marked_as_new(self): From 7ea77f068fff68e669b3dafc69d31515ff535c55 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 30 Jan 2013 18:53:11 +0100 Subject: [PATCH 30/35] [#316] Update some more activity streams tests --- ckan/tests/functional/api/test_activity.py | 40 ++++++++++++---------- ckan/tests/functional/test_activity.py | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index c0daf7b17ee..51bee6de429 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -2017,32 +2017,36 @@ def test_follow_dataset(self): # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( before['user activity stream'], after['user activity stream'])) - assert len(user_new_activities) == 1, ("There should be 1 new " + assert len(user_new_activities) == 0, ("There should be 0 new " " activity in the user's activity stream, but found %i" % len(user_new_activities)) - activity = user_new_activities[0] + + # The rest of this test is commented out because 'follow dataset' + # activities are disabled, even they are reenabled then uncomment it. + + #activity = user_new_activities[0] # The same new activity should appear in the package's activity stream. - pkg_new_activities = after['package activity stream'] - for activity in user_new_activities: - assert activity in pkg_new_activities + #pkg_new_activities = after['package activity stream'] + #for activity in user_new_activities: + # assert activity in pkg_new_activities # Check that the new activity has the right attributes. - assert activity['object_id'] == self.warandpeace['id'], \ - str(activity['object_id']) - assert activity['user_id'] == user['id'], str(activity['user_id']) - assert activity['activity_type'] == 'follow dataset', \ - str(activity['activity_type']) - if 'id' not in activity: - assert False, "activity object should have an id value" + #assert activity['object_id'] == self.warandpeace['id'], \ + # str(activity['object_id']) + #assert activity['user_id'] == user['id'], str(activity['user_id']) + #assert activity['activity_type'] == 'follow dataset', \ + # str(activity['activity_type']) + #if 'id' not in activity: + # assert False, "activity object should have an id value" # TODO: Test for the _correct_ revision_id value. - if 'revision_id' not in activity: - assert False, "activity object should have a revision_id value" - timestamp = datetime_from_string(activity['timestamp']) - assert timestamp >= before['time'] and timestamp <= \ - after['time'], str(activity['timestamp']) + #if 'revision_id' not in activity: + # assert False, "activity object should have a revision_id value" + #timestamp = datetime_from_string(activity['timestamp']) + #assert timestamp >= before['time'] and timestamp <= \ + # after['time'], str(activity['timestamp']) - assert len(self.activity_details(activity)) == 0 + #assert len(self.activity_details(activity)) == 0 def test_follow_user(self): user = self.normal_user diff --git a/ckan/tests/functional/test_activity.py b/ckan/tests/functional/test_activity.py index a8d9e0a9588..5634171c12d 100644 --- a/ckan/tests/functional/test_activity.py +++ b/ckan/tests/functional/test_activity.py @@ -120,7 +120,7 @@ def test_user_activity(self): result = self.app.get(offset, status=200) stripped = self.strip_tags(result) assert '%s started following %s' % (user['fullname'], - package['title']) in stripped, stripped + package['title']) not in stripped, stripped # Follow another user. follow_user(context, {'id': 'joeadmin'}) From 01ccbbac5d97d3077cdc471bb39716c1ea03f91f Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Sun, 3 Feb 2013 19:06:57 +0100 Subject: [PATCH 31/35] [#345] Also recognize driver `postgres` in `engine_is_pg` --- ckan/model/meta.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ckan/model/meta.py b/ckan/model/meta.py index 13dcf1cb1fc..b8d6b42cd65 100644 --- a/ckan/model/meta.py +++ b/ckan/model/meta.py @@ -153,14 +153,12 @@ def after_rollback(self, session): # names, you'll need a metadata for each database metadata = MetaData() + def engine_is_sqlite(): - """ - Returns true iff the engine is connected to a sqlite database. - """ + # Returns true iff the engine is connected to a sqlite database. return engine.url.drivername == 'sqlite' + def engine_is_pg(): - """ - Returns true iff the engine is connected to a postgresql database. - """ - return engine.url.drivername == 'psycopg2' + # Returns true iff the engine is connected to a postgresql database. + return engine.url.drivername in ['psycopg2', 'postgres'] From a915602a6a2c2be580ffddcf7012076a04ff5810 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Mon, 4 Feb 2013 17:50:56 +0100 Subject: [PATCH 32/35] [#309] Tidy up a couple of comments No point in putting "TODO" comments in the source code no one will remember to go back and do them and the comment may become outdated/misleading. --- ckan/lib/activity_streams_session_extension.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ckan/lib/activity_streams_session_extension.py b/ckan/lib/activity_streams_session_extension.py index c2d9713fff2..08a6e37756c 100644 --- a/ckan/lib/activity_streams_session_extension.py +++ b/ckan/lib/activity_streams_session_extension.py @@ -79,8 +79,8 @@ def before_commit(self, session): # object is a package. logger.debug("Looks like this object is a package") logger.debug("activity: %s" % activity) - ## Ignore private datasets for 2.0, - ## fix later to have privicy against streams + + # Don't create activities for private datasets. if obj.private: continue @@ -118,8 +118,8 @@ def before_commit(self, session): for package in related_packages: if package is None: continue - ## Ignore private datasets for 2.0, - ## fix later to have privicy against streams + + # Don't create activities for private datasets. if package.private: continue From 680c58fffde62c6834cfb006f94d7eb1ab592845 Mon Sep 17 00:00:00 2001 From: John Glover Date: Mon, 4 Feb 2013 17:58:16 +0100 Subject: [PATCH 33/35] [#285] PEP8 --- ckan/controllers/api.py | 7 +++---- ckan/lib/base.py | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py index db87ceda02b..f13e8eb7e14 100644 --- a/ckan/controllers/api.py +++ b/ckan/controllers/api.py @@ -546,7 +546,7 @@ def search(self, ver=None, register=None): def _get_search_params(cls, request_params): if 'qjson' in request_params: try: - qjson_param = request_params['qjson'].replace('\\\\u','\\u') + qjson_param = request_params['qjson'].replace('\\\\u', '\\u') params = h.json.loads(qjson_param, encoding='utf8') except ValueError, e: raise ValueError(gettext('Malformed qjson value') + ': %r' @@ -669,9 +669,8 @@ def group_exists(val): def dataset_autocomplete(self): q = request.params.get('incomplete', '') - q_lower = q.lower() limit = request.params.get('limit', 10) - tag_names = [] + package_dicts = [] if q: context = {'model': model, 'session': model.Session, 'user': c.user or c.author} @@ -753,7 +752,7 @@ def i18n_js_translations(self, lang): ''' translation strings for front end ''' ckan_path = os.path.join(os.path.dirname(__file__), '..') source = os.path.abspath(os.path.join(ckan_path, 'public', - 'base', 'i18n', '%s.js' % lang)) + 'base', 'i18n', '%s.js' % lang)) response.headers['Content-Type'] = CONTENT_TYPES['json'] if not os.path.exists(source): return '{}' diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 3fb56b3bbd7..33253d6e0d0 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -86,7 +86,7 @@ def render_jinja2(template_name, extra_vars): def render(template_name, extra_vars=None, cache_key=None, cache_type=None, cache_expire=None, method='xhtml', loader_class=MarkupTemplate, - cache_force = None, renderer=None): + cache_force=None, renderer=None): ''' Main template rendering function. ''' def render_template(): @@ -101,12 +101,12 @@ def render_template(): try: template_path, template_type = lib.render.template_info(template_name) except lib.render.TemplateNotFound: - template_type = 'genshi' + template_type = 'genshi' template_path = '' # snippets should not pass the context # but allow for legacy genshi templates - if renderer == 'snippet' and template_type != 'genshi': + if renderer == 'snippet' and template_type != 'genshi': del globs['c'] del globs['tmpl_context'] @@ -115,12 +115,12 @@ def render_template(): context_vars = globs.get('c') if context_vars: context_vars = dir(context_vars) - debug_info = {'template_name' : template_name, - 'template_path' : template_path, - 'template_type' : template_type, - 'vars' : globs, + debug_info = {'template_name': template_name, + 'template_path': template_path, + 'template_type': template_type, + 'vars': globs, 'c_vars': context_vars, - 'renderer' : renderer,} + 'renderer': renderer} if 'CKAN_DEBUG_INFO' not in request.environ: request.environ['CKAN_DEBUG_INFO'] = [] request.environ['CKAN_DEBUG_INFO'].append(debug_info) @@ -221,9 +221,9 @@ def __before__(self, action, **params): if c.userobj: from ckan.logic import get_action new_activities_count = get_action( - 'dashboard_new_activities_count') + 'dashboard_new_activities_count') context = {'model': model, 'session': model.Session, - 'user': c.user or c.author} + 'user': c.user or c.author} c.new_activities = new_activities_count(context, {}) def _identify_user(self): From db2b4627fb08652e476047474129cdb3d2631085 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Tue, 5 Feb 2013 19:39:56 +0100 Subject: [PATCH 34/35] [#345] Remove confusing warning if the datastore is running on a read only db --- ckanext/datastore/plugin.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ckanext/datastore/plugin.py b/ckanext/datastore/plugin.py index 412ba37f48c..cdb547a2211 100644 --- a/ckanext/datastore/plugin.py +++ b/ckanext/datastore/plugin.py @@ -68,8 +68,7 @@ def configure(self, config): self._create_alias_table() else: log.warn("We detected that CKAN is running on a read only database. " - "Permission checks and _table_metadata creation are skipped." - "Make sure that replication is properly set-up.") + "Permission checks and the creation of _table_metadata are skipped.") else: log.warn("We detected that you do not use a PostgreSQL database. " "The DataStore will NOT work and datastore tests will be skipped.") From 42e2a197768947da883233cb97c6a2db02041892 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Tue, 5 Feb 2013 19:41:35 +0100 Subject: [PATCH 35/35] [#345] Fix detection in engine_is_pg --- ckan/model/meta.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ckan/model/meta.py b/ckan/model/meta.py index b8d6b42cd65..ce175d309d7 100644 --- a/ckan/model/meta.py +++ b/ckan/model/meta.py @@ -161,4 +161,6 @@ def engine_is_sqlite(): def engine_is_pg(): # Returns true iff the engine is connected to a postgresql database. - return engine.url.drivername in ['psycopg2', 'postgres'] + # According to http://docs.sqlalchemy.org/en/latest/core/engines.html#postgresql + # all Postgres driver names start with `postgresql` + return engine.url.drivername.startswith('postgresql')