From 3beb20bb6295008e1e9d9361dd909ccce613747d Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 5 Mar 2012 13:13:16 +0000 Subject: [PATCH 01/26] reduce function calls for x_to_api dictiztions --- ckan/lib/dictization/model_dictize.py | 70 +++++++++++++++++---------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index ee3d268250c..40cf8541d56 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -315,31 +315,46 @@ def task_status_dictize(task_status, context): ## conversion to api -def group_to_api1(group, context): - +def group_to_api(group, context): + api_version = context.get('api_version') + assert api_version, 'No api_version supplied in context' dictized = group_dictize(group, context) - dictized["extras"] = dict((extra["key"], json.loads(extra["value"])) + dictized["extras"] = dict((extra["key"], json.loads(extra["value"])) for extra in dictized["extras"]) - dictized["packages"] = sorted([package["name"] for package in dictized["packages"]]) + if api_version == 1: + dictized["packages"] = sorted([pkg["name"] for pkg in dictized["packages"]]) + else: + dictized["packages"] = sorted([pkg["id"] for pkg in dictized["packages"]]) return dictized +def group_to_api1(group, context): + # DEPRICIATED set api_version in context and use group_to_api() + context['api_version'] = 1 + return group_to_api(group, context) + def group_to_api2(group, context): - - dictized = group_dictize(group, context) - dictized["extras"] = dict((extra["key"], json.loads(extra["value"])) - for extra in dictized["extras"]) - dictized["packages"] = sorted([package["id"] for package in dictized["packages"]]) - return dictized + # DEPRICIATED set api_version in context and use group_to_api() + context['api_version'] = 2 + return group_to_api(group, context) -def tag_to_api1(tag, context): - +def tag_to_api(tag, context): + api_version = context.get('api_version') + assert api_version, 'No api_version supplied in context' dictized = tag_dictize(tag, context) - return sorted([package["name"] for package in dictized["packages"]]) + if api_version == 1: + return sorted([package["name"] for package in dictized["packages"]]) + else: + return sorted([package["id"] for package in dictized["packages"]]) -def tag_to_api2(tag, context): +def tag_to_api1(tag, context): + # DEPRICIATED set api_version in context and use tag_to_api() + context['api_version'] = 1 + return tag_to_api(tag, context) - dictized = tag_dictize(tag, context) - return sorted([package["id"] for package in dictized["packages"]]) +def tag_to_api2(tag, context): + # DEPRICIATED set api_version in context and use tag_to_api() + context['api_version'] = 1 + return tag_to_api(tag, context) def resource_dict_to_api(res_dict, package_id, context): res_dict.pop("revision_id") @@ -348,7 +363,9 @@ def resource_dict_to_api(res_dict, package_id, context): res_dict["package_id"] = package_id -def _package_to_api_base(pkg, context, api_version): +def package_to_api(pkg, context): + api_version = context.get('api_version') + assert api_version, 'No api_version supplied in context' # shared package_to_api code dictized = package_dictize(pkg, context) dictized.pop("revision_timestamp") @@ -385,8 +402,13 @@ def make_api_2(package_id): if api_version == 1: api_fn = make_api_1 + dictized["groups"] = [group["name"] for group in dictized["groups"]] + # FIXME why is this just for version 1? + if pkg.resources: + dictized['download_url'] = pkg.resources[0].url else: api_fn = make_api_2 + dictized["groups"] = [group["id"] for group in dictized["groups"]] subjects = dictized.pop("relationships_as_subject") objects = dictized.pop("relationships_as_object") @@ -411,16 +433,14 @@ def make_api_2(package_id): return dictized def package_to_api1(pkg, context): - dictized = _package_to_api_base(pkg, context, 1) - dictized["groups"] = [group["name"] for group in dictized["groups"]] - if pkg.resources: - dictized['download_url'] = pkg.resources[0].url - return dictized + # DEPRICIATED set api_version in context and use package_to_api() + context['api_version'] = 1 + return package_to_api(tag, context) def package_to_api2(pkg, context): - dictized = _package_to_api_base(pkg, context, 2) - dictized["groups"] = [group["id"] for group in dictized["groups"]] - return dictized + # DEPRICIATED set api_version in context and use package_to_api() + context['api_version'] = 2 + return package_to_api(tag, context) def vocabulary_dictize(vocabulary, context): vocabulary_dict = table_dictize(vocabulary, context) From 493c62ad0ec36884deca98f06906e1ec228e44dc Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 5 Mar 2012 13:14:01 +0000 Subject: [PATCH 02/26] move depriciated function to base of file --- ckan/lib/dictization/model_dictize.py | 61 ++++++++++++++------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 40cf8541d56..57d69ef0d15 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -327,16 +327,6 @@ def group_to_api(group, context): dictized["packages"] = sorted([pkg["id"] for pkg in dictized["packages"]]) return dictized -def group_to_api1(group, context): - # DEPRICIATED set api_version in context and use group_to_api() - context['api_version'] = 1 - return group_to_api(group, context) - -def group_to_api2(group, context): - # DEPRICIATED set api_version in context and use group_to_api() - context['api_version'] = 2 - return group_to_api(group, context) - def tag_to_api(tag, context): api_version = context.get('api_version') assert api_version, 'No api_version supplied in context' @@ -346,15 +336,6 @@ def tag_to_api(tag, context): else: return sorted([package["id"] for package in dictized["packages"]]) -def tag_to_api1(tag, context): - # DEPRICIATED set api_version in context and use tag_to_api() - context['api_version'] = 1 - return tag_to_api(tag, context) - -def tag_to_api2(tag, context): - # DEPRICIATED set api_version in context and use tag_to_api() - context['api_version'] = 1 - return tag_to_api(tag, context) def resource_dict_to_api(res_dict, package_id, context): res_dict.pop("revision_id") @@ -366,7 +347,6 @@ def resource_dict_to_api(res_dict, package_id, context): def package_to_api(pkg, context): api_version = context.get('api_version') assert api_version, 'No api_version supplied in context' - # shared package_to_api code dictized = package_dictize(pkg, context) dictized.pop("revision_timestamp") @@ -432,16 +412,6 @@ def make_api_2(package_id): return dictized -def package_to_api1(pkg, context): - # DEPRICIATED set api_version in context and use package_to_api() - context['api_version'] = 1 - return package_to_api(tag, context) - -def package_to_api2(pkg, context): - # DEPRICIATED set api_version in context and use package_to_api() - context['api_version'] = 2 - return package_to_api(tag, context) - def vocabulary_dictize(vocabulary, context): vocabulary_dict = table_dictize(vocabulary, context) assert not vocabulary_dict.has_key('tags') @@ -466,3 +436,34 @@ def activity_detail_dictize(activity_detail, context): def activity_detail_list_dictize(activity_detail_list, context): return [activity_detail_dictize(activity_detail, context) for activity_detail in activity_detail_list] + + +def package_to_api1(pkg, context): + # DEPRICIATED set api_version in context and use package_to_api() + context['api_version'] = 1 + return package_to_api(pkg, context) + +def package_to_api2(pkg, context): + # DEPRICIATED set api_version in context and use package_to_api() + context['api_version'] = 2 + return package_to_api(pkg, context) + +def group_to_api1(group, context): + # DEPRICIATED set api_version in context and use group_to_api() + context['api_version'] = 1 + return group_to_api(group, context) + +def group_to_api2(group, context): + # DEPRICIATED set api_version in context and use group_to_api() + context['api_version'] = 2 + return group_to_api(group, context) + +def tag_to_api1(tag, context): + # DEPRICIATED set api_version in context and use tag_to_api() + context['api_version'] = 1 + return tag_to_api(tag, context) + +def tag_to_api2(tag, context): + # DEPRICIATED set api_version in context and use tag_to_api() + context['api_version'] = 1 + return tag_to_api(tag, context) From a4755718cb7dcbc2183fd092280c80b28ab44e76 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 5 Mar 2012 13:19:29 +0000 Subject: [PATCH 03/26] use common api dictiz functions --- ckan/logic/action/create.py | 10 ++-------- ckan/logic/action/get.py | 15 +++------------ ckan/logic/action/update.py | 11 ++--------- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index d88d14cbb1f..585ed78203f 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -316,10 +316,7 @@ def package_create_rest(context, data_dict): pkg = context['package'] - if api == '1': - package_dict = model_dictize.package_to_api1(pkg, context) - else: - package_dict = model_dictize.package_to_api2(pkg, context) + package_dict = model_dictize.package_to_api(pkg, context) data_dict['id'] = pkg.id @@ -336,10 +333,7 @@ def group_create_rest(context, data_dict): group = context['group'] - if api == '1': - group_dict = model_dictize.group_to_api1(group, context) - else: - group_dict = model_dictize.group_to_api2(group, context) + group_dict = model_dictize.group_to_api(group, context) data_dict['id'] = group.id diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index a148b6a8071..de08279da8f 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -512,10 +512,7 @@ def package_show_rest(context, data_dict): api = context.get('api_version') or '1' pkg = context['package'] - if api == '1': - package_dict = model_dictize.package_to_api1(pkg, context) - else: - package_dict = model_dictize.package_to_api2(pkg, context) + package_dict = model_dictize.package_to_api(pkg, context) return package_dict @@ -527,10 +524,7 @@ def group_show_rest(context, data_dict): api = context.get('api_version') or '1' group = context['group'] - if api == '2': - group_dict = model_dictize.group_to_api2(group, context) - else: - group_dict = model_dictize.group_to_api1(group, context) + group_dict = model_dictize.group_to_api(group, context) return group_dict @@ -542,10 +536,7 @@ def tag_show_rest(context, data_dict): api = context.get('api_version') or '1' tag = context['tag'] - if api == '2': - tag_dict = model_dictize.tag_to_api2(tag, context) - else: - tag_dict = model_dictize.tag_to_api1(tag, context) + tag_dict = model_dictize.tag_to_api(tag, context) return tag_dict diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 2fc23f2328f..7a27dc8b1ca 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -534,10 +534,7 @@ def package_update_rest(context, data_dict): pkg = context['package'] - if api == '1': - package_dict = model_dictize.package_to_api1(pkg, context) - else: - package_dict = model_dictize.package_to_api2(pkg, context) + package_dict = model_dictize.package_to_api(pkg, context) return package_dict @@ -557,11 +554,7 @@ def group_update_rest(context, data_dict): group = context['group'] - - if api == '1': - group_dict = model_dictize.group_to_api1(group, context) - else: - group_dict = model_dictize.group_to_api2(group, context) + group_dict = model_dictize.group_to_api(group, context) return group_dict From 509062507368ecda0e6f363110970944dfa227cd Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 5 Mar 2012 13:22:15 +0000 Subject: [PATCH 04/26] remove unused api variables --- ckan/logic/action/create.py | 4 ---- ckan/logic/action/get.py | 3 --- ckan/logic/action/update.py | 2 -- 3 files changed, 9 deletions(-) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 585ed78203f..5e76e0f2e82 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -307,8 +307,6 @@ def user_create(context, data_dict): def package_create_rest(context, data_dict): - api = context.get('api_version') or '1' - check_access('package_create_rest', context, data_dict) dictized_package = model_save.package_api_to_dict(data_dict, context) @@ -324,8 +322,6 @@ def package_create_rest(context, data_dict): def group_create_rest(context, data_dict): - api = context.get('api_version') or '1' - check_access('group_create_rest', context, data_dict) dictized_group = model_save.group_api_to_dict(data_dict, context) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index de08279da8f..a7eb32c2e03 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -509,7 +509,6 @@ def package_show_rest(context, data_dict): logic.get_action('package_show')(context, data_dict) - api = context.get('api_version') or '1' pkg = context['package'] package_dict = model_dictize.package_to_api(pkg, context) @@ -521,7 +520,6 @@ def group_show_rest(context, data_dict): check_access('group_show_rest',context, data_dict) group_show(context, data_dict) - api = context.get('api_version') or '1' group = context['group'] group_dict = model_dictize.group_to_api(group, context) @@ -533,7 +531,6 @@ def tag_show_rest(context, data_dict): check_access('tag_show_rest',context, data_dict) tag_show(context, data_dict) - api = context.get('api_version') or '1' tag = context['tag'] tag_dict = model_dictize.tag_to_api(tag, context) diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 7a27dc8b1ca..32fc159d0ee 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -511,7 +511,6 @@ def package_update_rest(context, data_dict): model = context['model'] id = data_dict.get("id") request_id = context['id'] - api = context.get('api_version') or '1' pkg = model.Package.get(request_id) if not pkg: @@ -542,7 +541,6 @@ def group_update_rest(context, data_dict): model = context['model'] id = data_dict["id"] - api = context.get('api_version') or '1' group = model.Group.get(id) context["group"] = group context["allow_partial_update"] = True From f7b36699141f5d35186438e28c5f6ddc7e11df70 Mon Sep 17 00:00:00 2001 From: Toby Date: Tue, 6 Mar 2012 11:56:46 +0000 Subject: [PATCH 05/26] internally api_version now int not str --- ckan/controllers/api.py | 15 ++++++--------- ckan/lib/dictization/model_dictize.py | 2 +- ckan/lib/dictization/model_save.py | 6 ++++-- ckan/lib/search/__init__.py | 6 +++--- ckan/logic/action/create.py | 4 ++-- ckan/logic/action/get.py | 19 ++++++++----------- ckan/logic/action/update.py | 6 ++---- ckan/tests/functional/api/base.py | 10 +++++----- .../functional/api/model/test_package.py | 6 +++--- ckan/tests/lib/test_dictization.py | 1 + 10 files changed, 35 insertions(+), 40 deletions(-) diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py index 5d3f97c9520..315333fb45a 100644 --- a/ckan/controllers/api.py +++ b/ckan/controllers/api.py @@ -38,7 +38,7 @@ def __call__(self, environ, start_response): api_version = routes_dict.get('ver') if api_version: api_version = api_version[1:] - routes_dict['ver'] = api_version + routes_dict['ver'] = int(api_version) self._identify_user() try: @@ -131,12 +131,10 @@ def _set_response_header(self, name, value): def get_api(self, ver=None): response_data = {} - response_data['version'] = ver or '1' + response_data['version'] = ver return self._finish_ok(response_data) - def action(self, logic_function): - # FIXME this is a hack till ver gets passed - api_version = 3 + def action(self, logic_function, ver=None): function = get_action(logic_function) if not function: log.error('Can\'t find logic function: %s' % logic_function) @@ -144,7 +142,7 @@ def action(self, logic_function): gettext('Action name not known: %s') % str(logic_function)) context = {'model': model, 'session': model.Session, 'user': c.user, - 'api_version':api_version} + 'api_version':ver} model.Session()._context = context return_dict = {'help': function.__doc__} try: @@ -419,7 +417,6 @@ def delete(self, ver=None, register=None, subregister=None, id=None, id2=None): def search(self, ver=None, register=None): log.debug('search %s params: %r' % (register, request.params)) - ver = ver or '1' # i.e. default to v1 if register == 'revision': since_time = None if request.params.has_key('since_id'): @@ -453,7 +450,7 @@ def search(self, ver=None, register=None): # if using API v2, default to returning the package ID if # no field list is specified if register in ['dataset', 'package'] and not params.get('fl'): - params['fl'] = 'id' if ver == '2' else 'name' + params['fl'] = 'id' if ver == 2 else 'name' try: if register == 'resource': @@ -486,7 +483,7 @@ def search(self, ver=None, register=None): else: # For package searches in API v3 and higher, we can pass # parameters straight to Solr. - if ver in u'12': + if ver in [1, 2]: # Otherwise, put all unrecognised ones into the q parameter params = convert_legacy_parameters_to_solr(params) query = query_for(model.Package) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 57d69ef0d15..dc2bd407019 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -465,5 +465,5 @@ def tag_to_api1(tag, context): def tag_to_api2(tag, context): # DEPRICIATED set api_version in context and use tag_to_api() - context['api_version'] = 1 + context['api_version'] = 2 return tag_to_api(tag, context) diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index f4ee8a0c9e1..065869821c1 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -392,7 +392,9 @@ def user_dict_save(user_dict, context): def package_api_to_dict(api1_dict, context): package = context.get("package") - api_version = context.get('api_version') or '1' + api_version = context.get('api_version') + assert api_version, 'No api_version supplied in context' + dictized = {} for key, value in api1_dict.iteritems(): @@ -418,7 +420,7 @@ def package_api_to_dict(api1_dict, context): new_value.append({"key": extras_key, "value": None}) if key == 'groups' and len(value): - if api_version == '1': + if api_version == 1: new_value = [{'name': item} for item in value] else: new_value = [{'id': item} for item in value] diff --git a/ckan/lib/search/__init__.py b/ckan/lib/search/__init__.py index 2c432b98acc..7137a18a1ba 100644 --- a/ckan/lib/search/__init__.py +++ b/ckan/lib/search/__init__.py @@ -100,7 +100,7 @@ def notify(self, entity, operation): dispatch_by_operation( entity.__class__.__name__, get_action('package_show_rest')( - {'model': model, 'ignore_auth': True}, + {'model': model, 'ignore_auth': True, 'api_version':1}, {'id': entity.id} ), operation @@ -119,7 +119,7 @@ def rebuild(package=None): if package: pkg_dict = get_action('package_show_rest')( - {'model': model, 'ignore_auth': True}, + {'model': model, 'ignore_auth': True, 'api_version':1}, {'id': package} ) package_index.remove_dict(pkg_dict) @@ -130,7 +130,7 @@ def rebuild(package=None): for pkg in model.Session.query(model.Package).filter(model.Package.state == 'active').all(): package_index.insert_dict( get_action('package_show_rest')( - {'model': model, 'ignore_auth': True}, + {'model': model, 'ignore_auth': True, 'api_version':1}, {'id': pkg.id} ) ) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 5e76e0f2e82..de71b8f14cf 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -116,8 +116,8 @@ def package_relationship_create(context, data_dict): model = context['model'] user = context['user'] schema = context.get('schema') or ckan.logic.schema.default_create_relationship_schema() - api = context.get('api_version') or '1' - ref_package_by = 'id' if api == '2' else 'name' + api = context.get('api_version') + ref_package_by = 'id' if api == 2 else 'name' id = data_dict['subject'] id2 = data_dict['object'] diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index a7eb32c2e03..00162c69196 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -46,8 +46,8 @@ def package_list(context, data_dict): model = context["model"] user = context["user"] - api = context.get("api_version", '1') - ref_package_by = 'id' if api == '2' else 'name' + api = context.get("api_version", 1) + ref_package_by = 'id' if api == 2 else 'name' check_access('package_list', context, data_dict) @@ -105,8 +105,8 @@ def group_list(context, data_dict): model = context['model'] user = context['user'] - api = context.get('api_version') or '1' - ref_group_by = 'id' if api == '2' else 'name'; + api = context.get('api_version') + ref_group_by = 'id' if api == 2 else 'name'; order_by = data_dict.get('order_by', 'name') if order_by not in set(('name', 'packages')): raise logic.ParameterError('"order_by" value %r not implemented.' % order_by) @@ -286,12 +286,12 @@ def package_relationships_list(context, data_dict): ##TODO needs to work with dictization layer model = context['model'] user = context['user'] - api = context.get('api_version') or '1' + api = context.get('api_version') id = data_dict["id"] id2 = data_dict.get("id2") rel = data_dict.get("rel") - ref_package_by = 'id' if api == '2' else 'name'; + ref_package_by = 'id' if api == 2 else 'name'; pkg1 = model.Package.get(id) pkg2 = None if not pkg1: @@ -343,7 +343,6 @@ def package_show(context, data_dict): def resource_show(context, data_dict): model = context['model'] - api = context.get('api_version') or '1' id = data_dict['id'] resource = model.Resource.get(id) @@ -358,9 +357,9 @@ def resource_show(context, data_dict): def revision_show(context, data_dict): model = context['model'] - api = context.get('api_version') or '1' + api = context.get('api_version') id = data_dict['id'] - ref_package_by = 'id' if api == '2' else 'name' + ref_package_by = 'id' if api == 2 else 'name' rev = model.Session.query(model.Revision).get(id) if rev is None: @@ -374,8 +373,6 @@ def group_show(context, data_dict): model = context['model'] id = data_dict['id'] - api = context.get('api_version') or '1' - group = model.Group.get(id) context['group'] = group diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 32fc159d0ee..32d6db93cdf 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -223,8 +223,8 @@ def package_update_validate(context, data_dict): def _update_package_relationship(relationship, comment, context): model = context['model'] - api = context.get('api_version') or '1' - ref_package_by = 'id' if api == '2' else 'name' + api = context.get('api_version') + ref_package_by = 'id' if api == 2 else 'name' is_changed = relationship.comment != comment if is_changed: rev = model.repo.new_revision() @@ -243,12 +243,10 @@ def package_relationship_update(context, data_dict): model = context['model'] user = context['user'] schema = context.get('schema') or ckan.logic.schema.default_update_relationship_schema() - api = context.get('api_version') or '1' id = data_dict['subject'] id2 = data_dict['object'] rel = data_dict['type'] - ref_package_by = 'id' if api == '2' else 'name' pkg1 = model.Package.get(id) pkg2 = model.Package.get(id2) diff --git a/ckan/tests/functional/api/base.py b/ckan/tests/functional/api/base.py index f079ce87f7f..d1267643ea2 100644 --- a/ckan/tests/functional/api/base.py +++ b/ckan/tests/functional/api/base.py @@ -81,7 +81,7 @@ def offset(self, path): assert self.api_version != None, "API version is missing." base = '/api' if self.api_version: - base += '/' + self.api_version + base += '/%s' % self.api_version utf8_encoded = (u'%s%s' % (base, path)).encode('utf8') url_encoded = urllib.quote(utf8_encoded) return url_encoded @@ -281,7 +281,7 @@ def _ref_group(cls, group): class Api1TestCase(Api1and2TestCase): - api_version = '1' + api_version = 1 ref_package_by = 'name' ref_group_by = 'name' ref_tag_by = 'name' @@ -293,7 +293,7 @@ def assert_msg_represents_anna(self, msg): class Api2TestCase(Api1and2TestCase): - api_version = '2' + api_version = 2 ref_package_by = 'id' ref_group_by = 'id' ref_tag_by = 'id' @@ -305,7 +305,7 @@ def assert_msg_represents_anna(self, msg): class Api3TestCase(ApiTestCase): - api_version = '3' + api_version = 3 ref_package_by = 'name' ref_group_by = 'name' ref_tag_by = 'name' @@ -317,7 +317,7 @@ def assert_msg_represents_anna(self, msg): class ApiUnversionedTestCase(Api1TestCase): api_version = '' - oldest_api_version = '1' + oldest_api_version = 1 def get_expected_api_version(self): return self.oldest_api_version diff --git a/ckan/tests/functional/api/model/test_package.py b/ckan/tests/functional/api/model/test_package.py index eed3e1a2cf6..5ef0cf93ca5 100644 --- a/ckan/tests/functional/api/model/test_package.py +++ b/ckan/tests/functional/api/model/test_package.py @@ -34,7 +34,7 @@ def get_groups_identifiers(self, test_groups, users=[]): groups = [] for grp in test_groups: group = model.Group.get(grp) - if self.get_expected_api_version() == '1': + if self.get_expected_api_version() == 1: groups.append(group.name) else: groups.append(group.id) @@ -144,7 +144,7 @@ def test_register_post_with_group(self): pkg_groups = model.Session.query(model.Group).\ join(model.Member, model.Member.group_id == model.Group.id).\ filter(model.Member.table_id == package.id).all() - if self.get_expected_api_version() == '1': + if self.get_expected_api_version() == 1: self.assert_equal([g.name for g in pkg_groups], groups) else: self.assert_equal([g.id for g in pkg_groups], groups) @@ -197,7 +197,7 @@ def test_register_post_with_group_sysadmin(self): pkg_groups = model.Session.query(model.Group).\ join(model.Member, model.Member.group_id == model.Group.id).\ filter(model.Member.table_id == package.id).all() - if self.get_expected_api_version() == '1': + if self.get_expected_api_version() == 1: self.assert_equal([g.name for g in pkg_groups], groups) else: self.assert_equal([g.id for g in pkg_groups], groups) diff --git a/ckan/tests/lib/test_dictization.py b/ckan/tests/lib/test_dictization.py index 18cf40e9db9..07d8dbc6055 100644 --- a/ckan/tests/lib/test_dictization.py +++ b/ckan/tests/lib/test_dictization.py @@ -755,6 +755,7 @@ def test_14_resource_no_id(self): def test_15_api_to_dictize(self): context = {"model": model, + 'api_version': 1, "session": model.Session} api_data = { From 8d0e1883bbf300e4379e27163372e983746003df Mon Sep 17 00:00:00 2001 From: Toby Date: Tue, 6 Mar 2012 11:57:20 +0000 Subject: [PATCH 06/26] make calls into logic.actions using get_action --- ckan/logic/action/get.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 00162c69196..c6144c35f66 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -516,7 +516,7 @@ def group_show_rest(context, data_dict): check_access('group_show_rest',context, data_dict) - group_show(context, data_dict) + logic.get_action('group_show')(context, data_dict) group = context['group'] group_dict = model_dictize.group_to_api(group, context) @@ -527,7 +527,7 @@ def tag_show_rest(context, data_dict): check_access('tag_show_rest',context, data_dict) - tag_show(context, data_dict) + logic.get_action('tag_show')(context, data_dict) tag = context['tag'] tag_dict = model_dictize.tag_to_api(tag, context) From e097dc564956afcac837b0833c807f762324a2b6 Mon Sep 17 00:00:00 2001 From: Toby Date: Tue, 6 Mar 2012 15:04:17 +0000 Subject: [PATCH 07/26] controllers.api nicer imports --- ckan/controllers/api.py | 87 ++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py index 315333fb45a..4a138600000 100644 --- a/ckan/controllers/api.py +++ b/ckan/controllers/api.py @@ -1,33 +1,41 @@ import logging import cgi +import datetime +import glob +from pylons import c, request, response +from pylons.i18n import _, gettext from paste.util.multidict import MultiDict from webob.multidict import UnicodeMultiDict -from ckan.lib.base import BaseController, response, c, _, gettext, request -from ckan.lib.helpers import json, date_str_to_datetime -import ckan.model as model import ckan.rating -from ckan.lib.search import (query_for, QueryOptions, SearchIndexError, SearchError, - SearchQueryError, DEFAULT_OPTIONS, - convert_legacy_parameters_to_solr) -from ckan.lib.navl.dictization_functions import DataError -from ckan.lib.munge import munge_name, munge_title_to_name, munge_tag -from ckan.logic import get_action, check_access -from ckan.logic import NotFound, NotAuthorized, ValidationError, ParameterError -from ckan.lib.jsonp import jsonpify -from ckan.forms.common import package_exists, group_exists +import ckan.model as model +import ckan.logic as logic +import ckan.lib.base as base +import ckan.lib.helpers as h +import ckan.lib.search as search +import ckan.lib.navl.dictization_functions +import ckan.lib.jsonp as jsonp +import ckan.lib.munge as munge +import ckan.forms.common as common log = logging.getLogger(__name__) +# shortcuts +get_action = logic.get_action +NotAuthorized = logic.NotAuthorized +NotFound = logic.NotFound +ValidationError = logic.ValidationError +DataError = ckan.lib.navl.dictization_functions.DataError + IGNORE_FIELDS = ['q'] CONTENT_TYPES = { 'text': 'text/plain;charset=utf-8', 'html': 'text/html;charset=utf-8', 'json': 'application/json;charset=utf-8', } -class ApiController(BaseController): +class ApiController(base.BaseController): _actions = {} @@ -43,7 +51,7 @@ def __call__(self, environ, start_response): self._identify_user() try: context = {'model':model,'user': c.user or c.author} - check_access('site_read',context) + logic.check_access('site_read',context) except NotAuthorized: response_msg = self._finish(403, _('Not authorized to see this page')) # Call start_response manually instead of the parent __call__ @@ -55,7 +63,7 @@ def __call__(self, environ, start_response): # avoid status_code_redirect intercepting error responses environ['pylons.status_code_redirect'] = True - return BaseController.__call__(self, environ, start_response) + return base.BaseController.__call__(self, environ, start_response) def _finish(self, status_int, response_data=None, content_type='text'): @@ -71,7 +79,7 @@ def _finish(self, status_int, response_data=None, if response_data is not None: response.headers['Content-Type'] = CONTENT_TYPES[content_type] if content_type == 'json': - response_msg = json.dumps(response_data) + response_msg = h.json.dumps(response_data) else: response_msg = response_data # Support "JSONP" callback. @@ -186,19 +194,19 @@ def action(self, logic_function, ver=None): return_dict['success'] = False log.error('Validation error: %r' % str(e.error_dict)) return self._finish(409, return_dict, content_type='json') - except ParameterError, e: + except logic.ParameterError, e: return_dict['error'] = {'__type': 'Parameter Error', 'message': '%s: %s' % \ (_('Parameter Error'), e.extra_msg)} return_dict['success'] = False log.error('Parameter error: %r' % e.extra_msg) return self._finish(409, return_dict, content_type='json') - except SearchQueryError, e: + except search.SearchQueryError, e: return_dict['error'] = {'__type': 'Search Query Error', 'message': 'Search Query is invalid: %r' % e.args } return_dict['success'] = False return self._finish(400, return_dict, content_type='json') - except SearchError, e: + except search.SearchError, e: return_dict['error'] = {'__type': 'Search Error', 'message': 'Search error: %r' % e.args } return_dict['success'] = False @@ -329,7 +337,7 @@ def create(self, ver=None, register=None, subregister=None, id=None, id2=None): #TODO make better error message return self._finish(400, _(u'Integrity Error') + \ ': %s - %s' % (e.error, request_data)) - except SearchIndexError: + except search.SearchIndexError: log.error('Unable to add package to search index: %s' % request_data) return self._finish(500, _(u'Unable to add package to search index') % request_data) except: @@ -377,7 +385,7 @@ def update(self, ver=None, register=None, subregister=None, id=None, id2=None): #TODO make better error message return self._finish(400, _(u'Integrity Error') + \ ': %s - %s' % (e.error, request_data)) - except SearchIndexError: + except search.SearchIndexError: log.error('Unable to update search index: %s' % request_data) return self._finish(500, _(u'Unable to update search index') % request_data) @@ -432,7 +440,7 @@ def search(self, ver=None, register=None): elif request.params.has_key('since_time'): since_time_str = request.params['since_time'] try: - since_time = date_str_to_datetime(since_time_str) + since_time = h.date_str_to_datetime(since_time_str) except ValueError, inst: return self._finish_bad_request('ValueError: %s' % inst) else: @@ -454,12 +462,12 @@ def search(self, ver=None, register=None): try: if register == 'resource': - query = query_for(model.Resource) + query = search.query_for(model.Resource) # resource search still uses ckan query parser - options = QueryOptions() + options = search.QueryOptions() for k, v in params.items(): - if (k in DEFAULT_OPTIONS.keys()): + if (k in search.DEFAULT_OPTIONS.keys()): options[k] = v options.update(params) options.username = c.user @@ -468,7 +476,7 @@ def search(self, ver=None, register=None): query_fields = MultiDict() for field, value in params.items(): field = field.strip() - if field in DEFAULT_OPTIONS.keys() or \ + if field in search.DEFAULT_OPTIONS.keys() or \ field in IGNORE_FIELDS: continue values = [value] @@ -485,11 +493,11 @@ def search(self, ver=None, register=None): # parameters straight to Solr. if ver in [1, 2]: # Otherwise, put all unrecognised ones into the q parameter - params = convert_legacy_parameters_to_solr(params) - query = query_for(model.Package) + params = search.convert_legacy_parameters_to_solr(params) + query = search.query_for(model.Package) results = query.run(params) return self._finish_ok(results) - except SearchError, e: + except search.SearchError, e: log.exception(e) return self._finish_bad_request( gettext('Bad search option: %s') % e) @@ -501,14 +509,14 @@ def search(self, ver=None, register=None): def _get_search_params(cls, request_params): if request_params.has_key('qjson'): try: - params = json.loads(request_params['qjson'], encoding='utf8') + params = h.json.loads(request_params['qjson'], encoding='utf8') except ValueError, e: raise ValueError, gettext('Malformed qjson value') + ': %r' % e elif len(request_params) == 1 and \ len(request_params.values()[0]) < 2 and \ request_params.keys()[0].startswith('{'): # e.g. {some-json}='1' or {some-json}='' - params = json.loads(request_params.keys()[0], encoding='utf8') + params = h.json.loads(request_params.keys()[0], encoding='utf8') else: params = request_params if not isinstance(params, (UnicodeMultiDict, dict)): @@ -545,7 +553,6 @@ def _calc_throughput(self, ver=None): period = 10 # Seconds. timing_cache_path = self._get_timing_cache_path() call_count = 0 - import datetime, glob for t in range(0, period): expr = '%s/%s*' % ( timing_cache_path, @@ -555,7 +562,7 @@ def _calc_throughput(self, ver=None): # Todo: Clear old records. return float(call_count) / period - @jsonpify + @jsonp.jsonpify def user_autocomplete(self): q = request.params.get('q', '') limit = request.params.get('limit', 20) @@ -569,7 +576,7 @@ def user_autocomplete(self): user_list = get_action('user_autocomplete')(context,data_dict) return user_list - @jsonpify + @jsonp.jsonpify def group_autocomplete(self): q = request.params.get('q', '') t = request.params.get('type', None) @@ -591,7 +598,7 @@ def convert_to_dict(user): return out - @jsonpify + @jsonp.jsonpify def authorizationgroup_autocomplete(self): q = request.params.get('q', '') limit = request.params.get('limit', 20) @@ -615,10 +622,10 @@ def is_slug_valid(self): slug = request.params.get('slug') or '' slugtype = request.params.get('type') or '' if slugtype==u'package': - response_data = dict(valid=not bool(package_exists(slug))) + response_data = dict(valid=not bool(common.package_exists(slug))) return self._finish_ok(response_data) if slugtype==u'group': - response_data = dict(valid=not bool(group_exists(slug))) + response_data = dict(valid=not bool(common.group_exists(slug))) return self._finish_ok(response_data) return self._finish_bad_request('Bad slug type: %s' % slugtype) @@ -676,17 +683,17 @@ def format_autocomplete(self): def munge_package_name(self): name = request.params.get('name') - munged_name = munge_name(name) + munged_name = munge.munge_name(name) return self._finish_ok(munged_name) def munge_title_to_package_name(self): name = request.params.get('title') or request.params.get('name') - munged_name = munge_title_to_name(name) + munged_name = munge.munge_title_to_name(name) return self._finish_ok(munged_name) def munge_tag(self): tag = request.params.get('tag') or request.params.get('name') - munged_tag = munge_tag(tag) + munged_tag = munge.munge_tag(tag) return self._finish_ok(munged_tag) def status(self): From 90ddb4521f83b5e14664ed55345ac9f29b66eb40 Mon Sep 17 00:00:00 2001 From: Toby Date: Tue, 6 Mar 2012 15:04:54 +0000 Subject: [PATCH 08/26] model_dictize.py nicer imports --- ckan/lib/dictization/model_dictize.py | 75 +++++++++++++-------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index dc2bd407019..95838406ccb 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -1,14 +1,13 @@ +import datetime from pylons import config from sqlalchemy.sql import select -from ckan.plugins import PluginImplementations, IPackageController, IGroupController -import datetime -from ckan.model import PackageRevision -from ckan.lib.dictization import (obj_list_dictize, - table_dictize) -from ckan.logic import NotFound +import ckan.model import ckan.misc -from ckan.lib.helpers import json +import ckan.logic as logic +import ckan.plugins as plugins +import ckan.lib.helpers as h +import ckan.lib.dictization as d ## package save @@ -22,9 +21,9 @@ def group_list_dictize(obj_list, context, for obj in obj_list: if context.get('with_capacity'): obj, capacity = obj - group_dict = table_dictize(obj, context, capacity=capacity) + group_dict = d.table_dictize(obj, context, capacity=capacity) else: - group_dict = table_dictize(obj, context) + group_dict = d.table_dictize(obj, context) group_dict.pop('created') if active and obj.state not in ('active', 'pending'): continue @@ -50,14 +49,14 @@ def resource_list_dictize(res_list, context): def extras_dict_dictize(extras_dict, context): result_list = [] for name, extra in extras_dict.iteritems(): - dictized = table_dictize(extra, context) + dictized = d.table_dictize(extra, context) if not extra.state == 'active': continue value = dictized["value"] ## This is to make sure the frontend does not show a plain string ## as json with brackets. if not(context.get("extras_as_string") and isinstance(value, basestring)): - dictized["value"] = json.dumps(value) + dictized["value"] = h.json.dumps(value) result_list.append(dictized) return sorted(result_list, key=lambda x: x["key"]) @@ -68,16 +67,16 @@ def extras_list_dictize(extras_list, context): for extra in extras_list: if active and extra.state not in ('active', 'pending'): continue - dictized = table_dictize(extra, context) + dictized = d.table_dictize(extra, context) value = dictized["value"] if not(context.get("extras_as_string") and isinstance(value, basestring)): - dictized["value"] = json.dumps(value) + dictized["value"] = h.json.dumps(value) result_list.append(dictized) return sorted(result_list, key=lambda x: x["key"]) def resource_dictize(res, context): - resource = table_dictize(res, context) + resource = d.table_dictize(res, context) extras = resource.pop("extras", None) if extras: resource.update(extras) @@ -111,7 +110,7 @@ def _execute_with_revision(q, rev_table, context): revision = session.query(context['model'].Revision).filter_by( id=revision_id).first() if not revision: - raise NotFound + raise logic.NotFound revision_date = revision.timestamp if revision_date: @@ -143,8 +142,8 @@ def package_dictize(pkg, context): q = select([package_rev]).where(package_rev.c.id == pkg.id) result = _execute_with_revision(q, package_rev, context).first() if not result: - raise NotFound - result_dict = table_dictize(result, context) + raise logic.NotFound + result_dict = d.table_dictize(result, context) #resources res_rev = model.resource_revision_table resource_group = model.resource_group_table @@ -160,7 +159,7 @@ def package_dictize(pkg, context): from_obj=tag_rev.join(tag, tag.c.id == tag_rev.c.tag_id) ).where(tag_rev.c.package_id == pkg.id) result = _execute_with_revision(q, tag_rev, context) - result_dict["tags"] = obj_list_dictize(result, context, lambda x: x["name"]) + result_dict["tags"] = d.obj_list_dictize(result, context, lambda x: x["name"]) #extras extra_rev = model.extra_revision_table q = select([extra_rev]).where(extra_rev.c.package_id == pkg.id) @@ -173,19 +172,19 @@ def package_dictize(pkg, context): from_obj=member_rev.join(group, group.c.id == member_rev.c.group_id) ).where(member_rev.c.table_id == pkg.id) result = _execute_with_revision(q, member_rev, context) - result_dict["groups"] = obj_list_dictize(result, context) + result_dict["groups"] = d.obj_list_dictize(result, context) #relations rel_rev = model.package_relationship_revision_table q = select([rel_rev]).where(rel_rev.c.subject_package_id == pkg.id) result = _execute_with_revision(q, rel_rev, context) - result_dict["relationships_as_subject"] = obj_list_dictize(result, context) + result_dict["relationships_as_subject"] = d.obj_list_dictize(result, context) q = select([rel_rev]).where(rel_rev.c.object_package_id == pkg.id) result = _execute_with_revision(q, rel_rev, context) - result_dict["relationships_as_object"] = obj_list_dictize(result, context) + result_dict["relationships_as_object"] = d.obj_list_dictize(result, context) # Extra properties from the domain object # We need an actual Package object for this, not a PackageRevision - if isinstance(pkg,PackageRevision): + if isinstance(pkg, ckan.model.PackageRevision): pkg = model.Package.get(pkg.id) # isopen @@ -210,7 +209,7 @@ def package_dictize(pkg, context): if pkg.metadata_created else None if context.get('for_view'): - for item in PluginImplementations(IPackageController): + for item in plugins.PluginImplementations(plugins.IPackageController): result_dict = item.before_view(result_dict) @@ -229,7 +228,7 @@ def _get_members(context, group, member_type): def group_dictize(group, context): model = context['model'] - result_dict = table_dictize(group, context) + result_dict = d.table_dictize(group, context) result_dict['display_name'] = group.display_name @@ -238,7 +237,7 @@ def group_dictize(group, context): context['with_capacity'] = True - result_dict['packages'] = obj_list_dictize( + result_dict['packages'] = d.obj_list_dictize( _get_members(context, group, 'packages'), context) @@ -257,7 +256,7 @@ def group_dictize(group, context): context['with_capacity'] = False if context.get('for_view'): - for item in PluginImplementations(IGroupController): + for item in plugins.PluginImplementations(plugins.IGroupController): result_dict = item.before_view(result_dict) return result_dict @@ -268,17 +267,17 @@ def tag_list_dictize(tag_list, context): for tag in tag_list: if context.get('with_capacity'): tag, capacity = tag - dictized = table_dictize(tag, context, capacity=capacity) + dictized = d.table_dictize(tag, context, capacity=capacity) else: - dictized = table_dictize(tag, context) + dictized = d.table_dictize(tag, context) result_list.append(dictized) return result_list def tag_dictize(tag, context): - result_dict = table_dictize(tag, context) - result_dict["packages"] = obj_list_dictize(tag.packages, context) + result_dict = d.table_dictize(tag, context) + result_dict["packages"] = d.obj_list_dictize(tag.packages, context) return result_dict def user_list_dictize(obj_list, context, @@ -297,9 +296,9 @@ def user_dictize(user, context): if context.get('with_capacity'): user, capacity = user - result_dict = table_dictize(user, context, capacity=capacity) + result_dict = d.table_dictize(user, context, capacity=capacity) else: - result_dict = table_dictize(user, context) + result_dict = d.table_dictize(user, context) del result_dict['password'] @@ -311,7 +310,7 @@ def user_dictize(user, context): return result_dict def task_status_dictize(task_status, context): - return table_dictize(task_status, context) + return d.table_dictize(task_status, context) ## conversion to api @@ -319,7 +318,7 @@ def group_to_api(group, context): api_version = context.get('api_version') assert api_version, 'No api_version supplied in context' dictized = group_dictize(group, context) - dictized["extras"] = dict((extra["key"], json.loads(extra["value"])) + dictized["extras"] = dict((extra["key"], h.json.loads(extra["value"])) for extra in dictized["extras"]) if api_version == 1: dictized["packages"] = sorted([pkg["name"] for pkg in dictized["packages"]]) @@ -352,7 +351,7 @@ def package_to_api(pkg, context): dictized["tags"] = [tag["name"] for tag in dictized["tags"] \ if not tag.get('vocabulary_id')] - dictized["extras"] = dict((extra["key"], json.loads(extra["value"])) + dictized["extras"] = dict((extra["key"], h.json.loads(extra["value"])) for extra in dictized["extras"]) dictized['license'] = pkg.license.title if pkg.license else None dictized['ratings_average'] = pkg.get_average_rating() @@ -413,7 +412,7 @@ def make_api_2(package_id): return dictized def vocabulary_dictize(vocabulary, context): - vocabulary_dict = table_dictize(vocabulary, context) + vocabulary_dict = d.table_dictize(vocabulary, context) assert not vocabulary_dict.has_key('tags') vocabulary_dict['tags'] = [tag_dictize(tag, context) for tag in vocabulary.tags] @@ -424,14 +423,14 @@ def vocabulary_list_dictize(vocabulary_list, context): for vocabulary in vocabulary_list] def activity_dictize(activity, context): - activity_dict = table_dictize(activity, context) + activity_dict = d.table_dictize(activity, context) return activity_dict def activity_list_dictize(activity_list, context): return [activity_dictize(activity, context) for activity in activity_list] def activity_detail_dictize(activity_detail, context): - return table_dictize(activity_detail, context) + return d.table_dictize(activity_detail, context) def activity_detail_list_dictize(activity_detail_list, context): return [activity_detail_dictize(activity_detail, context) From 0e3ee9e959c193432f85d756c76347049e6d91fa Mon Sep 17 00:00:00 2001 From: Toby Date: Tue, 6 Mar 2012 15:05:09 +0000 Subject: [PATCH 09/26] model_save.py nicer imports --- ckan/lib/dictization/model_save.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index 065869821c1..184c8da9a2c 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -1,6 +1,7 @@ -from ckan.lib.dictization import table_dict_save +import uuid from sqlalchemy.orm import class_mapper -from ckan.lib.helpers import json +import ckan.lib.dictization as d +import ckan.lib.helpers as h ##package saving @@ -95,7 +96,7 @@ def package_extras_save(extra_dicts, obj, context): elif extras_as_string: new_extras[extra_dict["key"]] = extra_dict["value"] else: - new_extras[extra_dict["key"]] = json.loads(extra_dict["value"]) + new_extras[extra_dict["key"]] = h.json.loads(extra_dict["value"]) #new for key in set(new_extras.keys()) - set(old_extras.keys()): state = 'pending' if context.get('pending') else 'active' @@ -133,7 +134,7 @@ def group_extras_save(extras_dicts, context): if extras_as_string: result_dict[extra_dict["key"]] = extra_dict["value"] else: - result_dict[extra_dict["key"]] = json.loads(extra_dict["value"]) + result_dict[extra_dict["key"]] = h.json.loads(extra_dict["value"]) return result_dict @@ -159,7 +160,7 @@ def package_tag_list_save(tag_dicts, package, context): tags = set() for tag_dict in tag_dicts: if (tag_dict.get('name'), tag_dict.get('vocabulary_id')) not in tag_name_vocab: - tag_obj = table_dict_save(tag_dict, model.Tag, context) + tag_obj = d.table_dict_save(tag_dict, model.Tag, context) tags.add(tag_obj) tag_name_vocab.add((tag_obj.name, tag_obj.vocabulary_id)) @@ -248,7 +249,7 @@ def relationship_list_save(relationship_dicts, package, attr, context): relationships = [] for relationship_dict in relationship_dicts: - obj = table_dict_save(relationship_dict, + obj = d.table_dict_save(relationship_dict, model.PackageRelationship, context) relationships.append(obj) @@ -262,7 +263,6 @@ def relationship_list_save(relationship_dicts, package, attr, context): relationship_list.append(relationship) def package_dict_save(pkg_dict, context): - import uuid model = context["model"] package = context.get("package") @@ -276,7 +276,7 @@ def package_dict_save(pkg_dict, context): if 'metadata_modified' in pkg_dict: del pkg_dict['metadata_modified'] - pkg = table_dict_save(pkg_dict, Package, context) + pkg = d.table_dict_save(pkg_dict, Package, context) if not pkg.id: pkg.id = str(uuid.uuid4()) @@ -337,7 +337,6 @@ def group_member_save(context, group_dict, member_table_name): def group_dict_save(group_dict, context): - import uuid model = context["model"] session = context["session"] @@ -348,7 +347,7 @@ def group_dict_save(group_dict, context): if group: group_dict["id"] = group.id - group = table_dict_save(group_dict, Group, context) + group = d.table_dict_save(group_dict, Group, context) if not group.id: group.id = str(uuid.uuid4()) @@ -385,7 +384,7 @@ def user_dict_save(user_dict, context): if 'password' in user_dict and not len(user_dict['password']): del user_dict['password'] - user = table_dict_save(user_dict, User, context) + user = d.table_dict_save(user_dict, User, context) return user @@ -415,7 +414,7 @@ def package_api_to_dict(api1_dict, context): for extras_key, extras_value in updated_extras.iteritems(): if extras_value is not None: new_value.append({"key": extras_key, - "value": json.dumps(extras_value)}) + "value": h.json.dumps(extras_value)}) else: new_value.append({"key": extras_key, "value": None}) @@ -457,7 +456,7 @@ def task_status_dict_save(task_status_dict, context): if task_status: task_status_dict["id"] = task_status.id - task_status = table_dict_save(task_status_dict, model.TaskStatus, context) + task_status = d.table_dict_save(task_status_dict, model.TaskStatus, context) return task_status def activity_dict_save(activity_dict, context): @@ -532,5 +531,5 @@ def tag_dict_save(tag_dict, context): tag = context.get('tag') if tag: tag_dict['id'] = tag.id - tag = table_dict_save(tag_dict, model.Tag, context) + tag = d.table_dict_save(tag_dict, model.Tag, context) return tag From f82f3b9acd8a4dc82813a163511e87cf706681ac Mon Sep 17 00:00:00 2001 From: Toby Date: Tue, 6 Mar 2012 17:02:28 +0000 Subject: [PATCH 10/26] clean routes.py --- ckan/config/routing.py | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/ckan/config/routing.py b/ckan/config/routing.py index 2ed0d3f4939..99f0cd48984 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -25,6 +25,7 @@ def make_map(): DELETE = dict(method=['DELETE']) GET_POST = dict(method=['GET', 'POST']) PUT_POST = dict(method=['PUT','POST']) + GET_POST_DELETE = dict(method=['GET', 'POST', 'DELETE']) OPTIONS = dict(method=['OPTIONS']) from lib.plugins import register_package_plugins @@ -104,8 +105,8 @@ def make_map(): m.connect('/rest/{register}/{id}/:subregister/{id2}', action='delete', conditions=DELETE) - # /api/2/util - with SubMapper(map, controller='api', path_prefix='/api{ver:/2}', ver='/2') as m: + # /api/util ver 1, 2 or none + with SubMapper(map, controller='api', path_prefix='/api{ver:/1|/2|}', ver='/1') as m: m.connect('/util/user/autocomplete', action='user_autocomplete') m.connect('/util/is_slug_valid', action='is_slug_valid', conditions=GET) @@ -118,9 +119,6 @@ def make_map(): m.connect('/util/authorizationgroup/autocomplete', action='authorizationgroup_autocomplete') m.connect('/util/group/autocomplete', action='group_autocomplete') - - # /api/util - with SubMapper(map, controller='api', path_prefix='/api') as m: m.connect('/util/markdown', action='markdown') m.connect('/util/dataset/munge_name', action='munge_package_name') m.connect('/util/dataset/munge_title_to_name', @@ -128,22 +126,20 @@ def make_map(): m.connect('/util/tag/munge', action='munge_tag') m.connect('/util/status', action='status') - ## Webstore - if config.get('ckan.datastore.enabled', False): - map.connect('datastore_read', '/api/data/{id}{url:(/.*)?}', - controller='datastore', action='read', url='', - conditions={'method': ['GET']} - ) - map.connect('datastore_write', '/api/data/{id}{url:(/.*)?}', - controller='datastore', action='write', url='', - conditions={'method': ['PUT','POST', 'DELETE']} - ) - - ########### ## /END API ########### + + ## Webstore + if config.get('ckan.datastore.enabled', False): + with SubMapper(map, controller='datastore') as m: + m.connect('datastore_read', '/api/data/{id}{url:(/.*)?}', + action='read', url='', conditions=GET) + m.connect('datastore_write', '/api/data/{id}{url:(/.*)?}', + action='write', url='', conditions=GET_POST_DELETE) + + map.redirect('/packages', '/dataset') map.redirect('/packages/{url:.*}', '/dataset/{url}') map.redirect('/package', '/dataset') From 5a1734a4fed955deda27242f12e401806624852e Mon Sep 17 00:00:00 2001 From: Toby Date: Thu, 8 Mar 2012 18:01:00 +0000 Subject: [PATCH 11/26] kill the cookies :) --- ckan/config/middleware.py | 23 +----------- ckan/config/routing.py | 2 + ckan/controllers/package.py | 4 -- ckan/controllers/user.py | 50 ++++++++++++++++--------- ckan/exceptions.py | 2 + ckan/lib/base.py | 16 ++++++-- ckan/lib/helpers.py | 15 +++++++- ckan/lib/i18n.py | 9 ----- ckan/public/scripts/application.js | 15 ++++++++ ckan/templates/layout_base.html | 1 + ckan/templates/user/edit_user_form.html | 2 +- ckan/templates/user/layout.html | 4 +- 12 files changed, 84 insertions(+), 59 deletions(-) diff --git a/ckan/config/middleware.py b/ckan/config/middleware.py index 15c092877e2..647d33652ef 100644 --- a/ckan/config/middleware.py +++ b/ckan/config/middleware.py @@ -134,17 +134,6 @@ def __init__(self, app, config): self.default_locale = config.get('ckan.locale_default', 'en') self.local_list = get_locales() - def get_cookie_lang(self, environ): - # get the lang from cookie if present - cookie = environ.get('HTTP_COOKIE') - if cookie: - cookies = [c.strip() for c in cookie.split(';')] - lang = [c.split('=')[1] for c in cookies \ - if c.startswith('ckan_lang')] - if lang and lang[0] in self.local_list: - return lang[0] - return None - def __call__(self, environ, start_response): # strip the language selector from the requested url # and set environ variables for the language selected @@ -165,16 +154,8 @@ def __call__(self, environ, start_response): else: environ['PATH_INFO'] = '/' else: - # use cookie lang or default language from config - cookie_lang = self.get_cookie_lang(environ) - if cookie_lang: - environ['CKAN_LANG'] = cookie_lang - default = (cookie_lang == self.default_locale) - environ['CKAN_LANG_IS_DEFAULT'] = default - else: - environ['CKAN_LANG'] = self.default_locale - environ['CKAN_LANG_IS_DEFAULT'] = True - + environ['CKAN_LANG'] = self.default_locale + environ['CKAN_LANG_IS_DEFAULT'] = True # Current application url path_info = environ['PATH_INFO'] diff --git a/ckan/config/routing.py b/ckan/config/routing.py index 99f0cd48984..6d094d5a680 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -237,8 +237,10 @@ def make_map(): m.connect('/user/login', action='login') m.connect('/user/logged_in', action='logged_in') m.connect('/user/logged_out', action='logged_out') + m.connect('/user/logged_out_redirect', action='logged_out_page') m.connect('/user/reset', action='request_reset') m.connect('/user/me', action='me') + m.connect('/user/set_lang/{lang}', action='set_lang') m.connect('/user/{id:.*}', action='read') m.connect('/user', action='index') diff --git a/ckan/controllers/package.py b/ckan/controllers/package.py index 9504f765d90..5435742c8ff 100644 --- a/ckan/controllers/package.py +++ b/ckan/controllers/package.py @@ -181,10 +181,6 @@ def read(self, id): abort(404, _('Dataset not found')) except NotAuthorized: abort(401, _('Unauthorized to read package %s') % id) - - #set a cookie so we know whether to display the welcome message - c.hide_welcome_message = bool(request.cookies.get('hide_welcome_message', False)) - response.set_cookie('hide_welcome_message', '1', max_age=3600) #(make cross-site?) # used by disqus plugin c.current_package_id = c.pkg.id diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index b7f74533e7b..a422e20fa3a 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -1,4 +1,5 @@ import logging +from pylons import session import genshi from urllib import quote @@ -102,11 +103,11 @@ def read(self, id=None): {'id':c.user_dict['id']}) return render('user/read.html') - def me(self): + def me(self, locale=None): if not c.user: - h.redirect_to(controller='user', action='login', id=None) + h.redirect_to(locale=locale, controller='user', action='login', id=None) user_ref = c.userobj.get_reference_preferred_for_uri() - h.redirect_to(controller='user', action='read', id=user_ref) + h.redirect_to(locale=locale, controller='user', action='read', id=user_ref) def register(self, data=None, errors=None, error_summary=None): return self.new(data, errors, error_summary) @@ -243,6 +244,10 @@ def _save_edit(self, id, context): def login(self): + lang = session.get('lang') + if lang: + session.delete() + return h.redirect_to(locale=str(lang), controller='user', action='login') if 'error' in request.params: h.flash_error(request.params['error']) @@ -257,6 +262,8 @@ def login(self): return render('user/logout_first.html') def logged_in(self): + # we need to set the language via a redirect + lang = session.get('lang') if c.user: context = {'model': model, 'user': c.user} @@ -265,26 +272,35 @@ def logged_in(self): user_dict = get_action('user_show')(context,data_dict) - # Max age of cookies: 50 years. Matches time set in templates/user/login.html - cookie_timeout=50*365*24*60*60 - - response.set_cookie("ckan_user", user_dict['name'], max_age=cookie_timeout) - response.set_cookie("ckan_display_name", user_dict['display_name'], max_age=cookie_timeout) - response.set_cookie("ckan_apikey", user_dict['apikey'], max_age=cookie_timeout) h.flash_success(_("%s is now logged in") % user_dict['display_name']) - return self.me() + return self.me(locale=lang) else: - h.flash_error('Login failed. Bad username or password.' + \ - ' (Or if using OpenID, it hasn\'t been associated with a user account.)') - h.redirect_to(controller='user', action='login') + h.flash_error(_('Login failed. Bad username or password.' + \ + ' (Or if using OpenID, it hasn\'t been associated with a user account.)')) + h.redirect_to(locale=lang, controller='user', action='login') + def logout(self): + # save our language in the session so we don't loose it + session['lang'] = request.environ.get('CKAN_LANG') + session.save() + h.redirect_to('/user/logout_generic') + + def set_lang(self, lang): + # this allows us to set the lang in session. Used for logging + # in/out to prevent being lost when repoze.who redirects things + session['lang'] = str(lang) + session.save() + def logged_out(self): + # we need to get our language info back and the show the correct page + lang = session.get('lang') c.user = None - response.delete_cookie("ckan_user") - response.delete_cookie("ckan_display_name") - response.delete_cookie("ckan_apikey") + session.delete() + h.redirect_to(locale=lang, controller='user', action='logged_out_page') + + def logged_out_page(self): return render('user/logout.html') - + def request_reset(self): if request.method == 'POST': id = request.params.get('user') diff --git a/ckan/exceptions.py b/ckan/exceptions.py index f1dc047a404..40bea2e8c29 100644 --- a/ckan/exceptions.py +++ b/ckan/exceptions.py @@ -4,3 +4,5 @@ class CkanException(Exception): class EmptyRevisionException(CkanException): pass +class CkanUrlException(Exception): + pass diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 2c346d8acf8..3aae3717de9 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -18,6 +18,7 @@ from genshi.template import MarkupTemplate from webhelpers.html import literal +import ckan.exceptions import ckan from ckan import authz from ckan.lib import i18n @@ -74,9 +75,12 @@ def render_template(): if cache_expire is not None: response.headers["Cache-Control"] = "max-age=%s, must-revalidate" % cache_expire - return cached_template(template_name, render_template, cache_key=cache_key, + try: + return cached_template(template_name, render_template, cache_key=cache_key, cache_type=cache_type, cache_expire=cache_expire) - #, ns_options=('method'), method=method) + except ckan.exceptions.CkanUrlException, e: + raise ckan.exceptions.CkanUrlException('\nAn Exception has been raised for template %s\n%s' + % (template_name, e.message)) class ValidationException(Exception): @@ -136,7 +140,13 @@ def __call__(self, environ, start_response): """Invoke the Controller""" # WSGIController.__call__ dispatches to the Controller method # the request is routed to. This routing information is - # available in environ['pylons.routes_dict'] + # available in environ['pylons.routes_dict'] + + # clean out any old cookies as they may contain api keys etc + for cookie in request.cookies: + if cookie.startswith('ckan') and cookie not in ['ckan', 'ckan_killtopbar']: + response.delete_cookie(cookie) + try: return WSGIController.__call__(self, environ, start_response) finally: diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 1e7004f8521..cbcf64d5163 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -23,6 +23,7 @@ from alphabet_paginate import AlphaPage from lxml.html import fromstring import i18n +import ckan.exceptions get_available_locales = i18n.get_available_locales get_locales_dict = i18n.get_locales_dict @@ -50,7 +51,6 @@ def url(*args, **kw): return _add_i18n_to_url(my_url, locale=locale, **kw) def url_for(*args, **kw): - """Create url adding i18n information if selected wrapper for routes.url_for""" locale = kw.pop('locale', None) @@ -112,9 +112,18 @@ def _add_i18n_to_url(url_to_amend, **kw): # stop the root being added twice in redirects if no_root: url = url_to_amend[len(root):] + if not default_locale: + url = '/%s%s' % (locale, url) + + if url == '/packages': + raise ckan.exceptions.CkanUrlException('There is a broken url being created %s' % kw) return url +def lang(): + from pylons import request + return request.environ.get('CKAN_LANG') + class Message(object): """A message returned by ``Flash.pop_messages()``. @@ -183,7 +192,9 @@ def __call__(self, message, category=None, ignore_duplicate=False, allow_html=Fa def pop_messages(self): from pylons import session messages = session.pop(self.session_key, []) - session.save() + # only save session if it has changed + if messages: + session.save() return [Message(*m) for m in messages] def are_there_messages(self): diff --git a/ckan/lib/i18n.py b/ckan/lib/i18n.py index 3cc1fea1da7..7a1dae4fc0e 100644 --- a/ckan/lib/i18n.py +++ b/ckan/lib/i18n.py @@ -95,15 +95,6 @@ def handle_request(request, tmpl_context): if lang != 'en': i18n.set_lang(lang) tmpl_context.language = lang - - # set ckan_lang cookie if we have changed the language. We need to - # remember this because repoze.who does it's own redirect. - try: - if request.cookies.get('ckan_lang') != lang: - response.set_cookie('ckan_lang', lang) - except AttributeError: - # when testing FakeRequest does not have cookies - pass return lang def get_lang(): diff --git a/ckan/public/scripts/application.js b/ckan/public/scripts/application.js index e8e40859cfe..e629ea65daa 100644 --- a/ckan/public/scripts/application.js +++ b/ckan/public/scripts/application.js @@ -99,6 +99,21 @@ if (isGroupEdit) { CKAN.Utils.setupUrlEditor('group',readOnly=true); } + // OpenID hack + // We need to remember the language we are using whilst logging in + // we set this in the user session so we don't forget then + // carry on as before. + if (window.openid && openid.signin){ + openid._signin = openid.signin; + openid.signin = function (arg) { + $.get('/user/set_lang/' + CKAN.LANG, function (){openid._signin(arg);}) + } + } + if ($('#login').length){ + $('#login').submit( function () { + $.ajax('/user/set_lang/' + CKAN.LANG, {async:false}); + }); + } }); }(jQuery)); diff --git a/ckan/templates/layout_base.html b/ckan/templates/layout_base.html index 22ea24cd58b..0fdb261e106 100644 --- a/ckan/templates/layout_base.html +++ b/ckan/templates/layout_base.html @@ -243,6 +243,7 @@

Meta

CKAN.plugins.push('storage'); CKAN.SITE_URL = '${h.url('/')}'; + CKAN.LANG = '${h.lang()}'; // later use will add offsets with leading '/' so ensure no trailing slash CKAN.SITE_URL = CKAN.SITE_URL.replace(/\/$/, ''); $(document).ready(function() { diff --git a/ckan/templates/user/edit_user_form.html b/ckan/templates/user/edit_user_form.html index ffe56cb9f1c..b697a6d0572 100644 --- a/ckan/templates/user/edit_user_form.html +++ b/ckan/templates/user/edit_user_form.html @@ -56,7 +56,7 @@

Errors in form

- +
diff --git a/ckan/templates/user/layout.html b/ckan/templates/user/layout.html index e474d9ad949..b4e898547ae 100644 --- a/ckan/templates/user/layout.html +++ b/ckan/templates/user/layout.html @@ -8,7 +8,7 @@ @@ -16,7 +16,7 @@ From c805ea0046a1c607ad12120614f7ee39dd9337f9 Mon Sep 17 00:00:00 2001 From: Toby Date: Fri, 9 Mar 2012 12:44:55 +0000 Subject: [PATCH 12/26] fix tests for cookie changes --- ckan/tests/functional/test_user.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ckan/tests/functional/test_user.py b/ckan/tests/functional/test_user.py index cb2a65b86cb..20f582e24a8 100644 --- a/ckan/tests/functional/test_user.py +++ b/ckan/tests/functional/test_user.py @@ -135,6 +135,7 @@ def test_user_login_page(self): def test_logout(self): res = self.app.get('/user/logout') res2 = res.follow() + res2 = res2.follow() assert 'You have logged out successfully.' in res2, res2 def _get_cookie_headers(self, res): @@ -188,9 +189,6 @@ def test_login(self): # check cookie created cookie = res.request.environ['HTTP_COOKIE'] - # I think some versions of webob do not produce quotes, hence the 'or' - assert 'ckan_display_name="testlogin"' in cookie or \ - 'ckan_display_name=testlogin' in cookie, cookie assert 'auth_tkt=' in cookie, cookie assert 'testlogin!userid_type:unicode' in cookie, cookie @@ -347,6 +345,7 @@ def test_register_whilst_logged_in(self): # logout and login as user B res = self.app.get('/user/logout') res2 = res.follow() + res2 = res2.follow() assert 'You have logged out successfully.' in res2, res2 offset = url_for(controller='user', action='login') res = self.app.get(offset) @@ -432,9 +431,6 @@ def test_user_create(self): # check cookies created cookie = res.request.environ['HTTP_COOKIE'] - # I think some versions of webob do not produce quotes, hence the 'or' - assert 'ckan_display_name="Test Create"' in cookie or\ - 'ckan_display_name=Test Create' in cookie, cookie assert 'auth_tkt=' in cookie, cookie assert 'testcreate!userid_type:unicode' in cookie, cookie From 3ff6b198906fd98f6d1e1ed6c227157349a17dba Mon Sep 17 00:00:00 2001 From: Toby Date: Fri, 9 Mar 2012 12:45:32 +0000 Subject: [PATCH 13/26] improve imports in lib.helpers --- ckan/lib/helpers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index cbcf64d5163..2c7a9c3e19f 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -24,6 +24,7 @@ from lxml.html import fromstring import i18n import ckan.exceptions +from pylons import request get_available_locales = i18n.get_available_locales get_locales_dict = i18n.get_locales_dict @@ -80,7 +81,6 @@ def _add_i18n_to_url(url_to_amend, **kw): # (as part of the language changing feature). # A locale of default will not add locale info to the url. - from pylons import request default_locale = False locale = kw.pop('locale', None) @@ -121,7 +121,6 @@ def _add_i18n_to_url(url_to_amend, **kw): return url def lang(): - from pylons import request return request.environ.get('CKAN_LANG') class Message(object): @@ -244,7 +243,6 @@ def subnav_named_route(c, text, routename,**kwargs): ) def facet_items(c, name, limit=10): - from pylons import request if not c.facets or not c.facets.get(name): return [] facets = [] From 5c25750a920a6e3f86942c5d72ae4dce2f65d83a Mon Sep 17 00:00:00 2001 From: Toby Date: Fri, 9 Mar 2012 18:45:03 +0000 Subject: [PATCH 14/26] remove unused import --- ckan/lib/i18n.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ckan/lib/i18n.py b/ckan/lib/i18n.py index 7a1dae4fc0e..87e7312e6a0 100644 --- a/ckan/lib/i18n.py +++ b/ckan/lib/i18n.py @@ -3,7 +3,6 @@ from babel import Locale, localedata from babel.core import LOCALE_ALIASES from pylons import config -from pylons import response from pylons import i18n import ckan.i18n From a1cc10eb039092ebfe37f7b336efdb8e3c59c3ee Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 12 Mar 2012 12:34:56 +0000 Subject: [PATCH 15/26] add page_cache middleware --- ckan/config/middleware.py | 63 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/ckan/config/middleware.py b/ckan/config/middleware.py index 647d33652ef..aae3d26dd7f 100644 --- a/ckan/config/middleware.py +++ b/ckan/config/middleware.py @@ -3,6 +3,7 @@ import logging from beaker.middleware import CacheMiddleware, SessionMiddleware +from beaker.container import MemoryNamespaceManager from paste.cascade import Cascade from paste.registry import RegistryManager from paste.urlparser import StaticURLParser @@ -101,10 +102,11 @@ def make_app(global_conf, full_stack=True, static_files=True, **app_conf): who_parser.remote_user_key, ) - app = I18nMiddleware(app, config) # Establish the Registry for this application app = RegistryManager(app) + app = I18nMiddleware(app, config) + if asbool(static_files): # Serve static files static_max_age = None if not asbool(config.get('ckan.cache_enabled')) \ @@ -124,6 +126,10 @@ def make_app(global_conf, full_stack=True, static_files=True, **app_conf): ) app = Cascade(extra_static_parsers+static_parsers) + # page cache + if asbool(config.get('ckan.page_cache_enabled')): + app = PageCacheMiddleware(app, config) + return app class I18nMiddleware(object): @@ -172,3 +178,58 @@ def __call__(self, environ, start_response): environ['CKAN_CURRENT_URL'] = path_info return self.app(environ, start_response) + + +class PageCacheMiddleware(object): + ''' A simple page cache that can store and serve pages. ''' + + def __init__(self, app, config): + self.app = app + self.cache = MemoryNamespaceManager('page-cache') + self.eager_caching = asbool(config.get('ckan.page_cache_eager')) + self.expires_time = int(config.get('ckan.page_cache_expire', 300)) + + def __call__(self, environ, start_response): + + def _start_response(status, response_headers, exc_info=None): + # allows us to get the status and headers + environ['CKAN_PAGE_STATUS'] = status + environ['CKAN_PAGE_HEADERS'] = response_headers + return start_response(status, response_headers, exc_info) + + # only use cache for GET requests + # If there is a cookie we avoid the cache + if environ['REQUEST_METHOD'] != 'GET' or environ.get('HTTP_COOKIE'): + return self.app(environ, start_response) + + # get our cache key + key = '%s?%s' % (environ['PATH_INFO'], environ['QUERY_STRING']) + + # if cached return cached result + try: + result = self.cache[key] + start_response(result['status'], result['headers']) + return result['content'] + except KeyError: + pass + + # generate the response from our application + page = self.app(environ, _start_response) + + cachable = False + if environ.get('CKAN_PAGE_CACHABLE'): + cachable = True + elif self.eager_caching: + for header, value in environ['CKAN_PAGE_HEADERS']: + if header == 'Cache-Control' and 'public' in value: + cachable = True + break + # cache things if cachable + if cachable: + # make sure we consume any file handles etc + page = list(page) + data = {'headers': environ['CKAN_PAGE_HEADERS'], + 'status': environ['CKAN_PAGE_STATUS'], + 'content': page} + self.cache.set_value(key, data, expiretime=self.expires_time) + return page From 5b4ecd29285127aa97ba956c232eca646f9e1a55 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 12 Mar 2012 12:36:24 +0000 Subject: [PATCH 16/26] allow caching in base.render() --- ckan/lib/base.py | 67 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 3aae3717de9..9523531655a 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -45,9 +45,10 @@ def abort(status_code=None, detail='', headers=None, comment=None): headers=headers, comment=comment) -def render(template_name, extra_vars=None, cache_key=None, cache_type=None, - cache_expire=None, method='xhtml', loader_class=MarkupTemplate): - +def render(template_name, extra_vars=None, cache_key=None, cache_type=None, + cache_expire=None, method='xhtml', loader_class=MarkupTemplate, + cache_force = None): + def render_template(): globs = extra_vars or {} globs.update(pylons_globals()) @@ -61,20 +62,62 @@ def render_template(): template = globs['app_globals'].genshi_loader.load(template_name, cls=loader_class) stream = template.generate(**globs) - + for item in PluginImplementations(IGenshiStreamFilter): stream = item.filter(stream) - + return literal(stream.render(method=method, encoding=None, strip_whitespace=False)) - + if 'Pragma' in response.headers: del response.headers["Pragma"] - if cache_key is not None or cache_type is not None: - response.headers["Cache-Control"] = "public" - - if cache_expire is not None: - response.headers["Cache-Control"] = "max-age=%s, must-revalidate" % cache_expire - + + if asbool(config.get('ckan.template_cache_enabled')): + allow_cache = True + # force cache or not if explicit + if cache_force is not None: + allow_cache = cache_force + # do not allow caching of pages for logged in users/flash messages etc + elif session.last_accessed: + allow_cache = False + # don't cache if based on a non-cachable template used in this + elif request.environ.get('__no_cache__'): + allow_cache = False + # don't cache if we have set the __no_cache__ param in the query string + elif request.params.get('__no_cache__'): + allow_cache = False + # don't cache if we have extra vars containing data + elif extra_vars: + for k, v in extra_vars.iteritems(): + allow_cache = False + break + else: + allow_cache = False + + if allow_cache: + request.environ['CKAN_PAGE_CACHABLE'] = True + response.headers["Cache-Control"] = "public" + + if cache_expire is None: + try: + cache_expire = int(config.get('ckan.cache_expires')) + cache_type = config.get('ckan.cache_type', 'memory') + except ValueError: + pass + if cache_expire is not None: + response.headers["Cache-Control"] += ",max-age=%s, must-revalidate" % cache_expire + if cache_expire or cache_type or cache_key: + if cache_key: + cache_key += '_' + h.lang() + else: + cache_key = h.lang() + cache_key += '_?_' + request.environ.get('QUERY_STRING', '') + else: + # we do not want caching + response.headers["Cache-Control"] = ["private"] + cache_key = cache_type = cache_expire = None + request.environ['__no_cache__'] = True + + # render time :) try: return cached_template(template_name, render_template, cache_key=cache_key, cache_type=cache_type, cache_expire=cache_expire) From a931c2a3e5d0cb41dd539e3a4b27f98080392a01 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 12 Mar 2012 12:38:09 +0000 Subject: [PATCH 17/26] better cookie lang logic --- ckan/controllers/user.py | 7 ++++--- ckan/lib/base.py | 6 ++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index a422e20fa3a..e4778e51ec2 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -244,9 +244,9 @@ def _save_edit(self, id, context): def login(self): - lang = session.get('lang') + lang = session.pop('lang', None) if lang: - session.delete() + session.save() return h.redirect_to(locale=str(lang), controller='user', action='login') if 'error' in request.params: h.flash_error(request.params['error']) @@ -263,7 +263,8 @@ def login(self): def logged_in(self): # we need to set the language via a redirect - lang = session.get('lang') + lang = session.pop('lang', None) + session.save() if c.user: context = {'model': model, 'user': c.user} diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 9523531655a..c693a8ebe59 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -190,6 +190,12 @@ def __call__(self, environ, start_response): if cookie.startswith('ckan') and cookie not in ['ckan', 'ckan_killtopbar']: response.delete_cookie(cookie) + if cookie == 'ckan' and not c.user and not h.are_there_flash_messages(): + if session.id: + if not session.get('lang'): + session.delete() + else: + response.delete_cookie(cookie) try: return WSGIController.__call__(self, environ, start_response) finally: From a15af4f6f446add69c66b44ab811108ad9460659 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 12 Mar 2012 12:38:41 +0000 Subject: [PATCH 18/26] allow caching of home page --- ckan/controllers/home.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/controllers/home.py b/ckan/controllers/home.py index 3a068208cb8..a675826af44 100644 --- a/ckan/controllers/home.py +++ b/ckan/controllers/home.py @@ -82,7 +82,7 @@ def index(self): ckan.logic.action.get.recently_changed_packages_activity_list_html( context, {}) - return render('home/index.html') + return render('home/index.html', cache_force=True) def license(self): return render('home/license.html') From 5228dd1171309dde68d5ddcc5de9341d5a4c7403 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 12 Mar 2012 12:39:13 +0000 Subject: [PATCH 19/26] better comment --- ckan/lib/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index c693a8ebe59..d9626af7c5a 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -54,7 +54,7 @@ def render_template(): globs.update(pylons_globals()) globs['actions'] = model.Action - # Using pylons.url() or pylons.url_for() directly destroys the + # Using pylons.url() directly destroys the # localisation stuff so we remove it so any bad templates crash # and burn del globs['url'] From 44812132f0837bf196a03a6414452d57077935a5 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 12 Mar 2012 12:39:39 +0000 Subject: [PATCH 20/26] do not cache some redirects --- ckan/lib/helpers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 2c7a9c3e19f..5d0acef5e2b 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -42,6 +42,8 @@ def redirect_to(*args, **kw): '''A routes.redirect_to wrapper to retain the i18n settings''' kw['__ckan_no_root'] = True + if are_there_flash_messages(): + kw['__no_cache__'] = True return _redirect_to(url_for(*args, **kw)) def url(*args, **kw): From ebab39c707e53c5f4ab0655066e9c84d1c16196b Mon Sep 17 00:00:00 2001 From: Toby Date: Fri, 16 Mar 2012 16:30:06 +0000 Subject: [PATCH 21/26] fixes for tests --- ckan/controllers/admin.py | 2 +- ckan/lib/base.py | 2 +- ckan/tests/functional/test_user.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ckan/controllers/admin.py b/ckan/controllers/admin.py index c7b67e7eeac..b3da73f6713 100644 --- a/ckan/controllers/admin.py +++ b/ckan/controllers/admin.py @@ -241,7 +241,7 @@ def trash(self): model.Revision).filter_by(state=model.State.DELETED) c.deleted_packages = model.Session.query( model.Package).filter_by(state=model.State.DELETED) - if not request.params: + if not request.params or (len(request.params) == 1 and '__no_cache__' in request.params): return render('admin/trash.html') else: # NB: we repeat retrieval of of revisions diff --git a/ckan/lib/base.py b/ckan/lib/base.py index d9626af7c5a..7ace0c79ba8 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -113,7 +113,7 @@ def render_template(): cache_key += '_?_' + request.environ.get('QUERY_STRING', '') else: # we do not want caching - response.headers["Cache-Control"] = ["private"] + response.headers["Cache-Control"] = "private" cache_key = cache_type = cache_expire = None request.environ['__no_cache__'] = True diff --git a/ckan/tests/functional/test_user.py b/ckan/tests/functional/test_user.py index 20f582e24a8..f7339c075bd 100644 --- a/ckan/tests/functional/test_user.py +++ b/ckan/tests/functional/test_user.py @@ -174,8 +174,8 @@ def test_login(self): # then get redirected to user page res = res.follow() assert_equal(res.status, 302) - assert res.header('Location') in ('http://localhost/user/testlogin', - '/user/testlogin') + assert res.header('Location').startswith('http://localhost/user/testlogin') or \ + res.header('Location').startswith('/user/testlogin') res = res.follow() assert_equal(res.status, 200) assert 'testlogin is now logged in' in res.body @@ -869,7 +869,7 @@ def test_request_reset_user_password_using_search(self): fv['user'] = 'kittens' res = fv.submit() assert_equal(res.status, 302) - assert_equal(res.header_dict['Location'], 'http://localhost/') + assert_equal(res.header_dict['Location'], 'http://localhost/?__no_cache__=True') CreateTestData.create_user(name='larry2', fullname='kittens') res = self.app.get(offset) From 33a754401e44863e235e78af37f7f87d093b83e7 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 19 Mar 2012 15:00:56 +0000 Subject: [PATCH 22/26] page cache uses redis --- ckan/config/middleware.py | 98 ++++++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 31 deletions(-) diff --git a/ckan/config/middleware.py b/ckan/config/middleware.py index aae3d26dd7f..8ed5ec0f47b 100644 --- a/ckan/config/middleware.py +++ b/ckan/config/middleware.py @@ -1,9 +1,9 @@ """Pylons middleware initialization""" import urllib import logging +import json from beaker.middleware import CacheMiddleware, SessionMiddleware -from beaker.container import MemoryNamespaceManager from paste.cascade import Cascade from paste.registry import RegistryManager from paste.urlparser import StaticURLParser @@ -126,7 +126,7 @@ def make_app(global_conf, full_stack=True, static_files=True, **app_conf): ) app = Cascade(extra_static_parsers+static_parsers) - # page cache + # Page cache if asbool(config.get('ckan.page_cache_enabled')): app = PageCacheMiddleware(app, config) @@ -181,55 +181,91 @@ def __call__(self, environ, start_response): class PageCacheMiddleware(object): - ''' A simple page cache that can store and serve pages. ''' + ''' A simple page cache that can store and serve pages. It uses + Redis as storage. It caches pages that have a http status code of + 200, use the GET method. Only non-logged in users receive cached + pages. + Cachable pages are indicated by a environ CKAN_PAGE_CACHABLE + variable.''' def __init__(self, app, config): self.app = app - self.cache = MemoryNamespaceManager('page-cache') - self.eager_caching = asbool(config.get('ckan.page_cache_eager')) - self.expires_time = int(config.get('ckan.page_cache_expire', 300)) + import redis # only import if used + self.redis = redis # we need to reference this within the class + self.redis_exception = redis.exceptions.ConnectionError + self.redis_connection = None def __call__(self, environ, start_response): def _start_response(status, response_headers, exc_info=None): - # allows us to get the status and headers + # This wrapper allows us to get the status and headers. environ['CKAN_PAGE_STATUS'] = status environ['CKAN_PAGE_HEADERS'] = response_headers return start_response(status, response_headers, exc_info) - # only use cache for GET requests - # If there is a cookie we avoid the cache - if environ['REQUEST_METHOD'] != 'GET' or environ.get('HTTP_COOKIE'): + # Only use cache for GET requests + # If there is a cookie we avoid the cache. + # REMOTE_USER is used by some tests. + if environ['REQUEST_METHOD'] != 'GET' or environ.get('HTTP_COOKIE') or \ + environ.get('REMOTE_USER'): return self.app(environ, start_response) - # get our cache key - key = '%s?%s' % (environ['PATH_INFO'], environ['QUERY_STRING']) + # Make our cache key + key = 'page:%s?%s' % (environ['PATH_INFO'], environ['QUERY_STRING']) - # if cached return cached result + # Try to connect if we don't have a connection. Doing this here + # allows the redis server to be unavailable at times. + if self.redis_connection is None: + try: + self.redis_connection = self.redis.StrictRedis() + self.redis_connection.flushdb() + except self.redis_exception: + return self.app(environ, start_response) + + # If cached return cached result try: - result = self.cache[key] - start_response(result['status'], result['headers']) - return result['content'] - except KeyError: - pass + result = self.redis_connection.lrange(key, 0, 2) + except self.redis_exception: + # Connection failed so clear it and return the page as normal. + self.redis_connection = None + return self.app(environ, start_response) - # generate the response from our application + if result: + headers = json.loads(result[1]) + # Convert headers from list to tuples. + headers = [(str(key), str(value)) for key, value in headers] + start_response(str(result[0]), headers) + # Returning a huge string slows down the server. Therefore we + # cut it up into more usable chunks. + page = result[2] + out = [] + total = len(page) + position = 0 + size = 4096 + while position < total: + out.append(page[position:position + size]) + position += size + return out + + # Generate the response from our application. page = self.app(environ, _start_response) + # Only cache http status 200 pages + if not environ['CKAN_PAGE_STATUS'].startswith('200'): + return page + cachable = False if environ.get('CKAN_PAGE_CACHABLE'): cachable = True - elif self.eager_caching: - for header, value in environ['CKAN_PAGE_HEADERS']: - if header == 'Cache-Control' and 'public' in value: - cachable = True - break - # cache things if cachable + + # Cache things if cachable. if cachable: - # make sure we consume any file handles etc - page = list(page) - data = {'headers': environ['CKAN_PAGE_HEADERS'], - 'status': environ['CKAN_PAGE_STATUS'], - 'content': page} - self.cache.set_value(key, data, expiretime=self.expires_time) + # Make sure we consume any file handles etc. + page_string = ''.join(list(page)) + # Use a pipe to add page in a transaction. + pipe = self.redis_connection.pipeline() + pipe.rpush(key, environ['CKAN_PAGE_STATUS']) + pipe.rpush(key, json.dumps(environ['CKAN_PAGE_HEADERS'])) + pipe.rpush(key, page_string) + pipe.execute() return page From 0a0b595a9a099de11a56b2e96604a9856b944e1a Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 19 Mar 2012 15:02:07 +0000 Subject: [PATCH 23/26] lib.base.render() cleanups and remove template caching --- ckan/lib/base.py | 78 +++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 44 deletions(-) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index 7ace0c79ba8..d2a253419ce 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -54,9 +54,8 @@ def render_template(): globs.update(pylons_globals()) globs['actions'] = model.Action - # Using pylons.url() directly destroys the - # localisation stuff so we remove it so any bad templates crash - # and burn + # Using pylons.url() directly destroys the localisation stuff so + # we remove it so any bad templates crash and burn del globs['url'] template = globs['app_globals'].genshi_loader.load(template_name, @@ -71,56 +70,47 @@ def render_template(): if 'Pragma' in response.headers: del response.headers["Pragma"] - if asbool(config.get('ckan.template_cache_enabled')): - allow_cache = True - # force cache or not if explicit - if cache_force is not None: - allow_cache = cache_force - # do not allow caching of pages for logged in users/flash messages etc - elif session.last_accessed: - allow_cache = False - # don't cache if based on a non-cachable template used in this - elif request.environ.get('__no_cache__'): - allow_cache = False - # don't cache if we have set the __no_cache__ param in the query string - elif request.params.get('__no_cache__'): - allow_cache = False - # don't cache if we have extra vars containing data - elif extra_vars: - for k, v in extra_vars.iteritems(): - allow_cache = False - break - else: + ## Caching Logic + allow_cache = True + # Force cache or not if explicit. + if cache_force is not None: + allow_cache = cache_force + # Do not allow caching of pages for logged in users/flash messages etc. + elif session.last_accessed: + allow_cache = False + # Tests etc. + elif 'REMOTE_USER' in request.environ: + allow_cache = False + # Don't cache if based on a non-cachable template used in this. + elif request.environ.get('__no_cache__'): allow_cache = False + # Don't cache if we have set the __no_cache__ param in the query string. + elif request.params.get('__no_cache__'): + allow_cache = False + # Don't cache if we have extra vars containing data. + elif extra_vars: + for k, v in extra_vars.iteritems(): + allow_cache = False + break + # Record cachability for the page cache if enabled + request.environ['CKAN_PAGE_CACHABLE'] = allow_cache if allow_cache: - request.environ['CKAN_PAGE_CACHABLE'] = True response.headers["Cache-Control"] = "public" - - if cache_expire is None: - try: - cache_expire = int(config.get('ckan.cache_expires')) - cache_type = config.get('ckan.cache_type', 'memory') - except ValueError: - pass - if cache_expire is not None: - response.headers["Cache-Control"] += ",max-age=%s, must-revalidate" % cache_expire - if cache_expire or cache_type or cache_key: - if cache_key: - cache_key += '_' + h.lang() - else: - cache_key = h.lang() - cache_key += '_?_' + request.environ.get('QUERY_STRING', '') + try: + cache_expire = int(config.get('ckan.cache_expires', 0)) + response.headers["Cache-Control"] += ", max-age=%s, must-revalidate" % cache_expire + except ValueError: + pass else: - # we do not want caching + # We do not want caching. response.headers["Cache-Control"] = "private" - cache_key = cache_type = cache_expire = None + # Prevent any further rendering from being cached. request.environ['__no_cache__'] = True - # render time :) + # Render Time :) try: - return cached_template(template_name, render_template, cache_key=cache_key, - cache_type=cache_type, cache_expire=cache_expire) + return cached_template(template_name, render_template) except ckan.exceptions.CkanUrlException, e: raise ckan.exceptions.CkanUrlException('\nAn Exception has been raised for template %s\n%s' % (template_name, e.message)) From 2e449400c422b0c5885d7f21fa54a508c9f6c304 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 19 Mar 2012 15:03:01 +0000 Subject: [PATCH 24/26] remove welcome bar --- ckan/public/scripts/application.js | 19 ------------------- ckan/templates/layout_base.html | 4 ---- 2 files changed, 23 deletions(-) diff --git a/ckan/public/scripts/application.js b/ckan/public/scripts/application.js index 2e7b6ea1211..64d09e8804a 100644 --- a/ckan/public/scripts/application.js +++ b/ckan/public/scripts/application.js @@ -33,11 +33,6 @@ var CKAN = CKAN || {}; window.location = ($(e.target).attr('action')); }); - var isFrontPage = $('body.index.home').length > 0; - if (isFrontPage) { - CKAN.Utils.setupWelcomeBanner($('.js-welcome-banner')); - } - var isGroupView = $('body.group.read').length > 0; if (isGroupView) { // Show extract of notes field @@ -838,20 +833,6 @@ CKAN.Utils = function($, my) { input.change(callback); }; - my.setupWelcomeBanner = function(banner) { - - var cookieName = 'ckan_killtopbar'; - var isKilled = ($.cookie(cookieName)!=null); - if (!isKilled) { - banner.show(); - // Bind to the close button - banner.find('.js-kill-button').live('click', function() { - $.cookie(cookieName, 'true', { expires: 365 }); - banner.hide(); - }); - } - }; - my.setupDatasetDeleteButton = function() { var select = $('select.dataset-delete'); select.attr('disabled','disabled'); diff --git a/ckan/templates/layout_base.html b/ckan/templates/layout_base.html index 80c728a866a..032d2573992 100644 --- a/ckan/templates/layout_base.html +++ b/ckan/templates/layout_base.html @@ -51,10 +51,6 @@ ${defined('body_class') and body_class()} "> -
From e056cb8bad1e9691a4c596512c03f885cc2b20a5 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 19 Mar 2012 16:41:39 +0000 Subject: [PATCH 25/26] add cache clearing on database changes --- ckan/model/meta.py | 52 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/ckan/model/meta.py b/ckan/model/meta.py index 81d471e528d..38f8ec24d64 100644 --- a/ckan/model/meta.py +++ b/ckan/model/meta.py @@ -1,4 +1,6 @@ import datetime +from paste.deploy.converters import asbool +from pylons import config """SQLAlchemy Metadata and Session object""" from sqlalchemy import MetaData, __version__ as sqav from sqlalchemy.orm import class_mapper @@ -17,6 +19,44 @@ from ckan.lib.activity import DatasetActivitySessionExtension +class CkanCacheExtension(SessionExtension): + ''' This extension checks what tables have been affected by + database access and allows us to act on them. Currently this is + used by the page cache to flush the cache when data in the database + is altered. ''' + + def __init__(self, *args, **kw): + super(CkanCacheExtension, self).__init__(*args, **kw) + # Setup Redis support if needed. + self.use_redis = asbool(config.get('ckan.page_cache_enabled')) + if self.use_redis: + import redis + self.redis = redis + self.redis_connection is None + self.redis_exception = redis.exceptions.ConnectionError + + def after_commit(self, session): + if hasattr(session, '_object_cache'): + oc = session._object_cache + oc_list = oc['new'] + oc_list.update(oc['changed']) + oc_list.update(oc['deleted']) + objs = set() + for item in oc_list: + objs.add(item.__class__.__name__) + + # Flush Redis + if self.use_redis: + if self.redis_connection is None: + try: + self.redis_connection = self.redis.StrictRedis() + except self.redis_exception: + pass + try: + self.redis_connection.flushdb() + except self.redis_exception: + pass + class CkanSessionExtension(SessionExtension): def before_flush(self, session, flush_context, instances): @@ -96,16 +136,20 @@ def after_rollback(self, session): autoflush=False, autocommit=False, expire_on_commit=False, - extension=[CkanSessionExtension(), extension.PluginSessionExtension(), - DatasetActivitySessionExtension()], + extension=[CkanCacheExtension(), + CkanSessionExtension(), + extension.PluginSessionExtension(), + DatasetActivitySessionExtension()], )) create_local_session = sessionmaker( autoflush=False, autocommit=False, expire_on_commit=False, - extension=[CkanSessionExtension(), extension.PluginSessionExtension(), - DatasetActivitySessionExtension()], + extension=[CkanCacheExtension(), + CkanSessionExtension(), + extension.PluginSessionExtension(), + DatasetActivitySessionExtension()], ) #mapper = Session.mapper From 88400b7a0fceb0e37d7f8481489373889d8e7449 Mon Sep 17 00:00:00 2001 From: Toby Date: Mon, 19 Mar 2012 16:54:19 +0000 Subject: [PATCH 26/26] clean up whitespace in template rendering --- ckan/lib/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/base.py b/ckan/lib/base.py index b2e5b7adeb8..ae984abb92e 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -69,7 +69,7 @@ def render_template(): if loader_class == NewTextTemplate: return literal(stream.render(method="text", encoding=None)) - return literal(stream.render(method=method, encoding=None, strip_whitespace=False)) + return literal(stream.render(method=method, encoding=None, strip_whitespace=True)) if 'Pragma' in response.headers: