From f9775a186465cecc9bf0f8cae47313dfbf913967 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 15:40:37 +0100 Subject: [PATCH 01/21] [#4801] Configure Flask app to serve public folders Credit to @smotornyuk This adds the Flask MultiStatic plugin to be able to add static assets form extensions. --- ckan/config/middleware/flask_app.py | 14 +- ckan/plugins/toolkit.py | 4 +- ckan/plugins/toolkit.py.orig | 528 ++++++++++++++++++++++++++++ requirements-py2.in | 1 + requirements-py2.txt | 1 + requirements.in | 1 + requirements.txt | 1 + 7 files changed, 546 insertions(+), 4 deletions(-) create mode 100644 ckan/plugins/toolkit.py.orig diff --git a/ckan/config/middleware/flask_app.py b/ckan/config/middleware/flask_app.py index 1d64913575e..61ff20d4524 100644 --- a/ckan/config/middleware/flask_app.py +++ b/ckan/config/middleware/flask_app.py @@ -8,9 +8,10 @@ import itertools import pkgutil -from flask import Flask, Blueprint, send_from_directory +from flask import Blueprint, send_from_directory from flask.ctx import _AppCtxGlobals from flask.sessions import SessionInterface +from flask_multistatic import MultiStaticFlask import six from werkzeug.exceptions import default_exceptions, HTTPException @@ -82,7 +83,14 @@ def make_flask_stack(conf, **app_conf): debug = asbool(conf.get('debug', conf.get('DEBUG', False))) testing = asbool(app_conf.get('testing', app_conf.get('TESTING', False))) - app = flask_app = CKANFlask(__name__) + app = flask_app = CKANFlask(__name__, static_url_path='') + + # Static files folders (core and extensions) + public_folder = config.get(u'ckan.base_public_folder') + app.static_folder = config.get( + 'extra_public_paths', '' + ).split(',') + [os.path.join(root, public_folder)] + app.jinja_options = jinja_extensions.get_jinja_env_options() app.debug = debug @@ -374,7 +382,7 @@ def __getattr__(self, name): return getattr(app_globals.app_globals, name) -class CKANFlask(Flask): +class CKANFlask(MultiStaticFlask): '''Extend the Flask class with a special method called on incoming requests by AskAppDispatcherMiddleware. diff --git a/ckan/plugins/toolkit.py b/ckan/plugins/toolkit.py index 350070f31b4..64148ad5ed4 100644 --- a/ckan/plugins/toolkit.py +++ b/ckan/plugins/toolkit.py @@ -507,7 +507,9 @@ def _get_endpoint(cls): return common.c.controller, common.c.action except AttributeError: return (None, None) - + # service routes, like `static` + if len(endpoint) == 1: + return endpoint + ('index', ) return endpoint def __getattr__(self, name): diff --git a/ckan/plugins/toolkit.py.orig b/ckan/plugins/toolkit.py.orig new file mode 100644 index 00000000000..a949202ef6c --- /dev/null +++ b/ckan/plugins/toolkit.py.orig @@ -0,0 +1,528 @@ +# encoding: utf-8 + +import sys + + +class _Toolkit(object): + '''This class is intended to make functions/objects consistently + available to plugins, whilst giving core CKAN developers the ability move + code around or change underlying frameworks etc. This object allows + us to avoid circular imports while making functions/objects + available to plugins. + + It should not be used internally within ckan - only by extensions. + + Functions/objects should only be removed after reasonable + deprecation notice has been given.''' + + # contents should describe the available functions/objects. We check + # that this list matches the actual availables in the initialisation + contents = [ + # Global CKAN configuration object + 'config', + # i18n translation + '_', + # i18n translation (plural form) + 'ungettext', + # template context + 'c', + # template helpers + 'h', + # http request object + 'request', + # template render function + 'render', + # snippet render function + 'render_snippet', + # converts a string to a boolean + 'asbool', + # converts a string to an integer + 'asint', + # converts a string to a list + 'aslist', + # stop tags in a string being escaped + 'literal', + # get logic action function + 'get_action', + # get flask/pylons endpoint fragments + 'get_endpoint', + # decorator for chained action + 'chained_action', + # get navl schema converter + 'get_converter', + # get navl schema validator + 'get_validator', + # check logic function authorisation + 'check_access', + # decorator for chained authentication functions + 'chained_auth_function', + # implements validate method with navl schema + 'navl_validate', + # placeholder for missing values for validation + 'missing', + # action not found exception (ckan.logic.NotFound) + 'ObjectNotFound', + # action not authorized exception + 'NotAuthorized', + # validator not found exception + 'UnknownValidator', + # model update validation error + 'ValidationError', + # validation exception to stop further validators from being called + 'StopOnError', + # validation invalid exception + 'Invalid', + # old class for providing CLI interfaces + 'CkanCommand', + # function for initializing CLI interfaces + 'load_config', + # base class for IDatasetForm plugins + 'DefaultDatasetForm', + # base class for IGroupForm plugins + 'DefaultGroupForm', + # base class for IGroupForm plugins for orgs + 'DefaultOrganizationForm', + # response object for cookies etc + 'response', + # Allow controllers to be created + 'BaseController', + # abort actions + 'abort', + # allow redirections + 'redirect_to', + # create urls + 'url_for', + # helpful for actions + 'get_or_bust', + # actions can be accessed via api + 'side_effect_free', + # allow auth functions to be checked for sysadmins + 'auth_sysadmins_check', + # allow anonymous access to an auth function + 'auth_allow_anonymous_access', + # disallow anonymous access to an auth function + 'auth_disallow_anonymous_access', + # Helper not found error. + 'HelperError', + # Enqueue background job + 'enqueue_job', + + # Fully defined in this file ## + 'add_template_directory', + 'add_resource', + 'add_public_directory', + 'add_ckan_admin_tab', + 'requires_ckan_version', + 'check_ckan_version', + 'CkanVersionException', + ] + + def __init__(self): + self._toolkit = {} + + # For some members in the the toolkit (e.g. that are exported from + # third-party libraries) we override their docstrings by putting our + # own docstrings into this dict. The Sphinx plugin that documents this + # plugins toolkit will use these docstring overrides instead of the + # object's actual docstring, when present. + self.docstring_overrides = {} + + def _initialize(self): + ''' get the required functions/objects, store them for later + access and check that they match the contents dict. ''' + import six + import ckan + import ckan.logic as logic + + import ckan.lib.base as base + import ckan.logic.validators as logic_validators + import ckan.lib.navl.dictization_functions as dictization_functions + import ckan.lib.helpers as h + import ckan.lib.plugins as lib_plugins + import ckan.common as common + from ckan.exceptions import ( + CkanVersionException, + HelperError + ) + from ckan.lib.jobs import enqueue as enqueue_job + + import ckan.common as converters + if six.PY2: + import ckan.lib.cli as cli + import pylons + + # Allow class access to these modules + self.__class__.ckan = ckan + self.__class__.base = base + + t = self._toolkit + + # imported functions + t['config'] = common.config + self.docstring_overrides['config'] = '''The CKAN configuration object. + +It stores the configuration values defined in the :ref:`config_file`, eg:: + + title = toolkit.config.get("ckan.site_title") + +''' + t['_'] = common._ + self.docstring_overrides['_'] = '''Translates a string to the +current locale. + +The ``_()`` function is a reference to the ``ugettext()`` function. +Everywhere in your code where you want strings to be internationalized +(made available for translation into different languages), wrap them in the +``_()`` function, eg.:: + + msg = toolkit._("Hello") + +Returns the localized unicode string. +''' + t['ungettext'] = common.ungettext + self.docstring_overrides['ungettext'] = '''Translates a string with +plural forms to the current locale. + +Mark a string for translation that has pural forms in the format +``ungettext(singular, plural, n)``. Returns the localized unicode string of +the pluralized value. + +Mark a string to be localized as follows:: + + msg = toolkit.ungettext("Mouse", "Mice", len(mouses)) + +''' + t['c'] = common.c + self.docstring_overrides['c'] = '''The Pylons template context object. + +This object is used to pass request-specific information to different parts of +the code in a thread-safe way (so that variables from different requests being +executed at the same time don't get confused with each other). + +Any attributes assigned to :py:attr:`~ckan.plugins.toolkit.c` are +available throughout the template and application code, and are local to the +current request. + +''' + t['h'] = h.helper_functions + t['request'] = common.request + self.docstring_overrides['request'] = '''The Pylons request object. + +A new request object is created for each HTTP request. It has methods and +attributes for getting things like the request headers, query-string variables, +request body variables, cookies, the request URL, etc. + +''' + t['render'] = base.render + t['abort'] = base.abort + t['asbool'] = converters.asbool + self.docstring_overrides['asbool'] = '''Convert a string (e.g. 1, +true, True) from the config file into a boolean. + +For example: ``if toolkit.asbool(config.get('ckan.legacy_templates', False)):`` + +''' + t['asint'] = converters.asint + self.docstring_overrides['asint'] = '''Convert a string from the config +file into an int. + +For example: ``bar = toolkit.asint(config.get('ckan.foo.bar', 0))`` + +''' + t['aslist'] = converters.aslist + self.docstring_overrides['aslist'] = '''Convert a space-separated +string from the config file into a list. + +For example: ``bar = toolkit.aslist(config.get('ckan.foo.bar', []))`` + +''' + t['literal'] = h.literal + t['get_action'] = logic.get_action + t['chained_action'] = logic.chained_action + t['get_converter'] = logic.get_validator # For backwards compatibility + t['get_validator'] = logic.get_validator + t['check_access'] = logic.check_access + t['chained_auth_function'] = logic.chained_auth_function + t['navl_validate'] = dictization_functions.validate + t['missing'] = dictization_functions.missing + t['ObjectNotFound'] = logic.NotFound # Name change intentional + t['NotAuthorized'] = logic.NotAuthorized + t['ValidationError'] = logic.ValidationError + t['StopOnError'] = dictization_functions.StopOnError + t['UnknownValidator'] = logic.UnknownValidator + t['Invalid'] = logic_validators.Invalid + + t['DefaultDatasetForm'] = lib_plugins.DefaultDatasetForm + t['DefaultGroupForm'] = lib_plugins.DefaultGroupForm + t['DefaultOrganizationForm'] = lib_plugins.DefaultOrganizationForm + + t['redirect_to'] = h.redirect_to + t['url_for'] = h.url_for + t['get_or_bust'] = logic.get_or_bust + t['side_effect_free'] = logic.side_effect_free + t['auth_sysadmins_check'] = logic.auth_sysadmins_check + t['auth_allow_anonymous_access'] = logic.auth_allow_anonymous_access + t['auth_disallow_anonymous_access'] = ( + logic.auth_disallow_anonymous_access + ) + + # class functions + t['render_snippet'] = self._render_snippet + t['add_template_directory'] = self._add_template_directory + t['add_public_directory'] = self._add_public_directory + t['add_resource'] = self._add_resource + t['add_ckan_admin_tab'] = self._add_ckan_admin_tabs + t['requires_ckan_version'] = self._requires_ckan_version + t['check_ckan_version'] = self._check_ckan_version + t['get_endpoint'] = self._get_endpoint + t['CkanVersionException'] = CkanVersionException + t['HelperError'] = HelperError + t['enqueue_job'] = enqueue_job + + if six.PY2: + + t['literal'] = webhelpers.html.tags.literal + t['response'] = pylons.response + self.docstring_overrides['response'] = ''' +The Pylons response object. + +Pylons uses this object to generate the HTTP response it returns to the web +browser. It has attributes like the HTTP status code, the response headers, +content type, cookies, etc. + +''' + t['BaseController'] = base.BaseController + # TODO: Sort these out + t['CkanCommand'] = cli.CkanCommand + t['load_config'] = cli.load_config + + # check contents list correct + errors = set(t).symmetric_difference(set(self.contents)) + if errors: + raise Exception('Plugin toolkit error %s not matching' % errors) + + # wrappers + # Wrapper for the render_snippet function as it uses keywords rather than + # dict to pass data. + @classmethod + def _render_snippet(cls, template, data=None): + '''Render a template snippet and return the output. + + See :doc:`/theming/index`. + + ''' + data = data or {} + return cls.base.render_snippet(template, **data) + + # new functions + @classmethod + def _add_template_directory(cls, config, relative_path): + '''Add a path to the :ref:`extra_template_paths` config setting. + + The path is relative to the file calling this function. + + ''' + cls._add_served_directory(config, relative_path, + 'extra_template_paths') + + @classmethod + def _add_public_directory(cls, config, relative_path): + '''Add a path to the :ref:`extra_public_paths` config setting. + + The path is relative to the file calling this function. + + Webassets addition: append directory to webassets load paths + in order to correctly rewrite relative css paths and resolve + public urls. + + ''' + import ckan.lib.helpers as h + from ckan.lib.webassets_tools import add_public_path + path = cls._add_served_directory( + config, + relative_path, + 'extra_public_paths' + ) + add_public_path(path, h.url_for_static('/')) + + @classmethod + def _add_served_directory(cls, config, relative_path, config_var): + ''' Add extra public/template directories to config. ''' + import inspect + import os + + assert config_var in ('extra_template_paths', 'extra_public_paths') + # we want the filename that of the function caller but they will + # have used one of the available helper functions + # TODO: starting from python 3.5, `inspect.stack` returns list + # of named tuples `FrameInfo`. Don't forget to remove + # `getframeinfo` wrapper after migration. + filename = inspect.getframeinfo(inspect.stack()[2][0]).filename + + this_dir = os.path.dirname(filename) + absolute_path = os.path.join(this_dir, relative_path) + if absolute_path not in config.get(config_var, ''): + if config.get(config_var): + config[config_var] += ',' + absolute_path + else: + config[config_var] = absolute_path + return absolute_path + + @classmethod + def _add_resource(cls, path, name): + '''Add a WebAssets library to CKAN. + + WebAssets libraries are directories containing static resource + files (e.g. CSS, JavaScript or image files) that can be + compiled into WebAsset Bundles. + + See :doc:`/theming/index` for more details. + + ''' + import inspect + import os + from ckan.lib.webassets_tools import create_library + + # we want the filename that of the function caller but they + # will have used one of the available helper functions + # TODO: starting from python 3.5, `inspect.stack` returns list + # of named tuples `FrameInfo`. Don't forget to remove + # `getframeinfo` wrapper after migration. + filename = inspect.getframeinfo(inspect.stack()[1][0]).filename + + this_dir = os.path.dirname(filename) + absolute_path = os.path.join(this_dir, path) + create_library(name, absolute_path) + + # TODO: remove next two lines after dropping Fanstatic support + import ckan.lib.fanstatic_resources + ckan.lib.fanstatic_resources.create_library(name, absolute_path) + + @classmethod + def _add_ckan_admin_tabs(cls, config, route_name, tab_label, + config_var='ckan.admin_tabs', icon=None): + ''' + Update 'ckan.admin_tabs' dict the passed config dict. + ''' + # get the admin_tabs dict from the config, or an empty dict. + admin_tabs_dict = config.get(config_var, {}) + # update the admin_tabs dict with the new values + admin_tabs_dict.update({ + route_name: { + 'label': tab_label, + 'icon': icon + } + }) + # update the config with the updated admin_tabs dict + config.update({config_var: admin_tabs_dict}) + + @classmethod + def _version_str_2_list(cls, v_str): + ''' convert a version string into a list of ints + eg 1.6.1b --> [1, 6, 1] ''' + import re + v_str = re.sub(r'[^0-9.]', '', v_str) + return [int(part) for part in v_str.split('.')] + + @classmethod + def _check_ckan_version(cls, min_version=None, max_version=None): + '''Return ``True`` if the CKAN version is greater than or equal to + ``min_version`` and less than or equal to ``max_version``, + return ``False`` otherwise. + + If no ``min_version`` is given, just check whether the CKAN version is + less than or equal to ``max_version``. + + If no ``max_version`` is given, just check whether the CKAN version is + greater than or equal to ``min_version``. + + :param min_version: the minimum acceptable CKAN version, + eg. ``'2.1'`` + :type min_version: string + + :param max_version: the maximum acceptable CKAN version, + eg. ``'2.3'`` + :type max_version: string + + ''' + current = cls._version_str_2_list(cls.ckan.__version__) + + if min_version: + min_required = cls._version_str_2_list(min_version) + if current < min_required: + return False + if max_version: + max_required = cls._version_str_2_list(max_version) + if current > max_required: + return False + return True + + @classmethod + def _requires_ckan_version(cls, min_version, max_version=None): + '''Raise :py:exc:`~ckan.plugins.toolkit.CkanVersionException` if the + CKAN version is not greater than or equal to ``min_version`` and + less then or equal to ``max_version``. + + If no ``max_version`` is given, just check whether the CKAN version is + greater than or equal to ``min_version``. + + Plugins can call this function if they require a certain CKAN version, + other versions of CKAN will crash if a user tries to use the plugin + with them. + + :param min_version: the minimum acceptable CKAN version, + eg. ``'2.1'`` + :type min_version: string + + :param max_version: the maximum acceptable CKAN version, + eg. ``'2.3'`` + :type max_version: string + + ''' + from ckan.exceptions import CkanVersionException + if not cls._check_ckan_version(min_version=min_version, + max_version=max_version): + if not max_version: + error = 'Requires ckan version %s or higher' % min_version + else: + error = 'Requires ckan version between {0} and {1}'.format( + min_version, + max_version + ) + raise CkanVersionException(error) + + @classmethod + def _get_endpoint(cls): + """Returns tuple in format: (controller|blueprint, action|view). + """ + import ckan.common as common + try: + # CKAN >= 2.8 + endpoint = tuple(common.request.endpoint.split('.')) + except AttributeError: + try: + return common.c.controller, common.c.action + except AttributeError: + return (None, None) + + return endpoint + + def __getattr__(self, name): + ''' return the function/object requested ''' + if not self._toolkit: + self._initialize() + if name in self._toolkit: + return self._toolkit[name] + else: + if name == '__bases__': + return self.__class__.__bases__ + raise AttributeError('`%s` not found in plugins toolkit' % name) + + def __dir__(self): + if not self._toolkit: + self._initialize() + return sorted(self._toolkit.keys()) + + +# https://mail.python.org/pipermail/python-ideas/2012-May/014969.html +sys.modules[__name__] = _Toolkit() diff --git a/requirements-py2.in b/requirements-py2.in index a6f1e5b5fd4..4b7c9d03861 100644 --- a/requirements-py2.in +++ b/requirements-py2.in @@ -9,6 +9,7 @@ fanstatic==0.12 feedgen==0.8.0 Flask==1.1.1 Flask-Babel==0.11.2 +flask-multistatic==1.0 Jinja2==2.10.1 Markdown==2.6.7 passlib==1.6.5 diff --git a/requirements-py2.txt b/requirements-py2.txt index dd78b3a342b..650bd935c26 100644 --- a/requirements-py2.txt +++ b/requirements-py2.txt @@ -16,6 +16,7 @@ dominate==2.4.0 fanstatic==0.12 flask-babel==0.11.2 Flask==1.1.1 +flask-multistatic==1.0 feedgen==0.8.0 formencode==1.3.1 # via pylons funcsigs==1.0.2 # via beaker diff --git a/requirements.in b/requirements.in index 7c963651275..921620f10fa 100644 --- a/requirements.in +++ b/requirements.in @@ -10,6 +10,7 @@ fanstatic==1.1 feedgen==0.8.0 Flask==1.1.1 Flask-Babel==0.11.2 +flask-multistatic==1.0 Jinja2==2.10.1 Markdown==2.6.7 passlib==1.6.5 diff --git a/requirements.txt b/requirements.txt index a14598d7e9f..86feeecc357 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,6 +17,7 @@ fanstatic==1.1 feedgen==0.8.0 flask-babel==0.11.2 flask==1.1.1 +flask-multistatic==1.0 funcsigs==1.0.2 # via beaker idna==2.8 # via requests itsdangerous==1.1.0 # via flask From 6916b7a3cf209daf2fb2fd62baf36bddf0f56e4c Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 15:46:35 +0100 Subject: [PATCH 02/21] [#4801] Fix multidict handling in friendly form plugin --- ckan/lib/repoze_plugins/friendly_form.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ckan/lib/repoze_plugins/friendly_form.py b/ckan/lib/repoze_plugins/friendly_form.py index 50f1f3b5f0d..572a753e84b 100644 --- a/ckan/lib/repoze_plugins/friendly_form.py +++ b/ckan/lib/repoze_plugins/friendly_form.py @@ -30,9 +30,9 @@ from webob import Request try: - from webob import UnicodeMultiDict as multidict + from webob.multidict import MultiDict except ImportError: - from webob import multidict + from webob import UnicodeMultiDict as MultiDict from webob.exc import HTTPFound, HTTPUnauthorized from zope.interface import implementer @@ -187,7 +187,7 @@ def identify(self, environ): # We are on the URL where repoze.who logs the user out. # r = Request(environ) params = dict(list(r.GET.items()) + list(r.POST.items())) - form = multidict(params) + form = MultiDict(params) form.update(query) referer = environ.get(u'HTTP_REFERER', script_name) came_from = form.get(u'came_from', referer) From 01481d973721a9818b387d349b5f124abd536c0a Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 23:16:06 +0100 Subject: [PATCH 03/21] [#4801] Align format of flask exceptions with Pylons/webob ones --- ckan/config/middleware/flask_app.py | 8 +++++--- ckan/controllers/error.py | 2 +- ckan/templates/error_document_template.html | 5 ++++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/ckan/config/middleware/flask_app.py b/ckan/config/middleware/flask_app.py index 61ff20d4524..329eda3d70b 100644 --- a/ckan/config/middleware/flask_app.py +++ b/ckan/config/middleware/flask_app.py @@ -464,9 +464,11 @@ def _register_error_handler(app): def error_handler(e): log.error(e, exc_info=sys.exc_info) if isinstance(e, HTTPException): - extra_vars = {u'code': [e.code], u'content': e.description} - # TODO: Remove - g.code = [e.code] + extra_vars = { + u'code': e.code, + u'content': e.description, + u'name': e.name + } return base.render( u'error_document_template.html', extra_vars), e.code diff --git a/ckan/controllers/error.py b/ckan/controllers/error.py index 5bca6ab20fe..244f42ffebc 100644 --- a/ckan/controllers/error.py +++ b/ckan/controllers/error.py @@ -45,7 +45,7 @@ def document(self): cgi.escape(request.GET.get('message', '')) prefix = request.environ.get('SCRIPT_NAME', ''), code = cgi.escape(request.GET.get('code', - str(original_response.status_int))), + str(original_response.status_int))) extra_vars = {'code': code, 'content': content, 'prefix': prefix} return render('error_document_template.html', extra_vars=extra_vars) diff --git a/ckan/templates/error_document_template.html b/ckan/templates/error_document_template.html index 0baecba15bf..9ed973f92ad 100644 --- a/ckan/templates/error_document_template.html +++ b/ckan/templates/error_document_template.html @@ -1,10 +1,13 @@ {% extends "page.html" %} -{% block subtitle %}{{ gettext('Error %(error_code)s', error_code=code[0]) }}{% endblock %} +{% block subtitle %}{{ gettext('Error %(error_code)s', error_code=code) }}{% endblock %} {% block primary %}
+ {% if name %} +

{{ code }} {{ name }}

+ {% endif %} {{ content}}
From 5c9e38bdabc8cdca8d8f31a79390601ef2077bd3 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 23:34:26 +0100 Subject: [PATCH 04/21] [#4801] Remove no longer relevant legacy tests These were not testing what was intended, and were masked by the catch-all template controller --- ckan/tests/legacy/functional/api/test_util.py | 41 ------------------- ckan/tests/legacy/functional/test_error.py | 7 ---- 2 files changed, 48 deletions(-) delete mode 100644 ckan/tests/legacy/functional/api/test_util.py delete mode 100644 ckan/tests/legacy/functional/test_error.py diff --git a/ckan/tests/legacy/functional/api/test_util.py b/ckan/tests/legacy/functional/api/test_util.py deleted file mode 100644 index eb99c38335a..00000000000 --- a/ckan/tests/legacy/functional/api/test_util.py +++ /dev/null @@ -1,41 +0,0 @@ -# encoding: utf-8 - -import pytest -from ckan import model -from ckan.lib.create_test_data import CreateTestData -from ckan.tests.legacy import TestController as ControllerTestCase -from ckan.tests.legacy import url_for - - -@pytest.fixture(autouse=True) -def initial_data(clean_db): - CreateTestData.create() - - -def test_munge_package_name(app): - response = app.get( - url=url_for(controller="api", action="munge_package_name", ver=2), - params={"name": "test name"}, - status=200, - ) - assert response.body == '"test-name"' - - -def test_munge_title_to_package_name(app): - response = app.get( - url=url_for( - controller="api", action="munge_title_to_package_name", ver=2 - ), - params={"name": "Test title"}, - status=200, - ) - assert response.body == '"test-title"' - - -def test_munge_tag(app): - response = app.get( - url=url_for(controller="api", action="munge_tag", ver=2), - params={"name": "Test subject"}, - status=200, - ) - assert response.body == '"test-subject"' diff --git a/ckan/tests/legacy/functional/test_error.py b/ckan/tests/legacy/functional/test_error.py deleted file mode 100644 index 5fc3b38b61b..00000000000 --- a/ckan/tests/legacy/functional/test_error.py +++ /dev/null @@ -1,7 +0,0 @@ -# encoding: utf-8 - - -def test_without_redirect(app): - # this is what a web bot might do - res = app.get("/error/document") - assert "There is no error." in str(res) From 8ee21484d3ceb6c0f0d9eb8d239a3a97baf2e5ac Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 23:41:08 +0100 Subject: [PATCH 05/21] [#4801] Serve robots.txt from the public folder (not templates) --- ckan/config/routing.py | 3 --- ckan/{templates => public}/robots.txt | 0 2 files changed, 3 deletions(-) rename ckan/{templates => public}/robots.txt (100%) diff --git a/ckan/config/routing.py b/ckan/config/routing.py index 97338269eec..f08b4a440ae 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -134,9 +134,6 @@ def make_map(): # users map.redirect('/users/{url:.*}', '/user/{url}') - # robots.txt - map.connect('/(robots.txt)', controller='template', action='view') - # Mark all unmarked routes added up until now as core routes for route in map.matchlist: if not hasattr(route, '_ckan_core'): diff --git a/ckan/templates/robots.txt b/ckan/public/robots.txt similarity index 100% rename from ckan/templates/robots.txt rename to ckan/public/robots.txt From e4d523e06bfbe6cfd17957671f2b995e7d9d2a40 Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 23:42:32 +0100 Subject: [PATCH 06/21] [#4801] Update non-root test using weird URL and fix related bug in example theme plugin --- ckan/tests/test_none_root.py | 2 +- ckanext/example_theme_docs/custom_config_setting/plugin.py | 2 +- ckanext/example_theme_docs/v12_extra_public_dir/plugin.py | 2 +- ckanext/example_theme_docs/v15_fanstatic/plugin.py | 2 +- .../example_theme_docs/v22_fanstatic_and_webassets/plugin.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ckan/tests/test_none_root.py b/ckan/tests/test_none_root.py index de90736d9b0..0aaa2bee347 100644 --- a/ckan/tests/test_none_root.py +++ b/ckan/tests/test_none_root.py @@ -7,7 +7,7 @@ @pytest.mark.ckan_config(u"ckan.plugins", u"example_theme_v15_fanstatic") @pytest.mark.usefixtures(u"with_plugins") def test_resource_url(app): - content = app.get(u"/en/base.html") + content = app.get(u"/") if u"example_theme.css" not in content: assert u"example_theme.min.css" in content assert u'href="/data/webassets/example_theme' in content diff --git a/ckanext/example_theme_docs/custom_config_setting/plugin.py b/ckanext/example_theme_docs/custom_config_setting/plugin.py index 84779692844..28951739fef 100644 --- a/ckanext/example_theme_docs/custom_config_setting/plugin.py +++ b/ckanext/example_theme_docs/custom_config_setting/plugin.py @@ -30,7 +30,7 @@ def most_popular_groups(): # Get a list of all the site's groups from CKAN, sorted by number of # datasets. groups = toolkit.get_action('group_list')( - data_dict={'sort': 'packages desc', 'all_fields': True}) + data_dict={'sort': 'package_count desc', 'all_fields': True}) # Truncate the list to the 10 most popular groups only. groups = groups[:10] diff --git a/ckanext/example_theme_docs/v12_extra_public_dir/plugin.py b/ckanext/example_theme_docs/v12_extra_public_dir/plugin.py index ec3ef074a1e..2311d48fb5c 100644 --- a/ckanext/example_theme_docs/v12_extra_public_dir/plugin.py +++ b/ckanext/example_theme_docs/v12_extra_public_dir/plugin.py @@ -10,7 +10,7 @@ def most_popular_groups(): # Get a list of all the site's groups from CKAN, sorted by number of # datasets. groups = toolkit.get_action('group_list')( - data_dict={'sort': 'packages desc', 'all_fields': True}) + data_dict={'sort': 'package_count desc', 'all_fields': True}) # Truncate the list to the 10 most popular groups only. groups = groups[:10] diff --git a/ckanext/example_theme_docs/v15_fanstatic/plugin.py b/ckanext/example_theme_docs/v15_fanstatic/plugin.py index e952bcb47c9..7fde6fc9d66 100644 --- a/ckanext/example_theme_docs/v15_fanstatic/plugin.py +++ b/ckanext/example_theme_docs/v15_fanstatic/plugin.py @@ -10,7 +10,7 @@ def most_popular_groups(): # Get a list of all the site's groups from CKAN, sorted by number of # datasets. groups = toolkit.get_action('group_list')( - data_dict={'sort': 'packages desc', 'all_fields': True}) + data_dict={'sort': 'package_count desc', 'all_fields': True}) # Truncate the list to the 10 most popular groups only. groups = groups[:10] diff --git a/ckanext/example_theme_docs/v22_fanstatic_and_webassets/plugin.py b/ckanext/example_theme_docs/v22_fanstatic_and_webassets/plugin.py index ff7d1e7588b..e8871878c24 100644 --- a/ckanext/example_theme_docs/v22_fanstatic_and_webassets/plugin.py +++ b/ckanext/example_theme_docs/v22_fanstatic_and_webassets/plugin.py @@ -10,7 +10,7 @@ def most_popular_groups(): # Get a list of all the site's groups from CKAN, sorted by number of # datasets. groups = toolkit.get_action(u'group_list')( - data_dict={u'sort': u'packages desc', u'all_fields': True}) + data_dict={u'sort': u'package_count desc', u'all_fields': True}) # Truncate the list to the 10 most popular groups only. groups = groups[:10] From edeb59e2002640365d05d24b2fcaae47f614f2ad Mon Sep 17 00:00:00 2001 From: amercader Date: Thu, 19 Dec 2019 23:44:24 +0100 Subject: [PATCH 07/21] [#4801] Remove catch-all template controller Pylons used to have a catch-all route defined that tried to look for the path in the provided URL in the templates folder. With changed in the multiple static folders introduced in f9775a1 this route is never reached, as we now set the catch-all route to look for the path in the public folder(s) (which makes more sense to be honest). Modified the middleware tests to reflect this. --- ckan/config/routing.py | 1 - ckan/controllers/template.py | 45 ------------------------- ckan/tests/config/test_middleware.py | 42 ++++++++--------------- ckan/tests/controllers/test_template.py | 16 --------- 4 files changed, 14 insertions(+), 90 deletions(-) delete mode 100644 ckan/controllers/template.py delete mode 100644 ckan/tests/controllers/test_template.py diff --git a/ckan/config/routing.py b/ckan/config/routing.py index f08b4a440ae..f4632a2643a 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -153,6 +153,5 @@ def make_map(): map.redirect('/favicon.ico', config.get('ckan.favicon')) map.redirect('/*(url)/', '/{url}', _redirect_code='301 Moved Permanently') - map.connect('/*url', controller='template', action='view', ckan_core=True) return map diff --git a/ckan/controllers/template.py b/ckan/controllers/template.py deleted file mode 100644 index d51f891d446..00000000000 --- a/ckan/controllers/template.py +++ /dev/null @@ -1,45 +0,0 @@ -# encoding: utf-8 - -import ckan.lib.base as base -import ckan.lib.render -from ckan.common import response - - -class TemplateController(base.BaseController): - - def view(self, url): - u"""By default, the final controller tried to fulfill the request - when no other routes match. It may be used to display a template - when all else fails, e.g.:: - - def view(self, url): - return render('/%s' % url) - - Or if you're using Mako and want to explicitly send a 404 (Not - Found) response code when the requested template doesn't exist:: - - import mako.exceptions - - def view(self, url): - try: - return render('/%s' % url) - except mako.exceptions.TopLevelLookupException: - abort(404) - - By default this controller aborts the request with a 404 (Not - Found) - """ - if url.endswith(u'.txt'): - response.headers[b'Content-Type'] = b'text/plain; charset=utf-8' - # Default content-type is text/html - try: - return base.render(url) - except ckan.lib.render.TemplateNotFound: - if url.endswith(u'.html'): - base.abort(404) - url += u'.html' - response.headers[b'Content-Type'] = b'text/html; charset=utf-8' - try: - return base.render(url) - except ckan.lib.render.TemplateNotFound: - base.abort(404) diff --git a/ckan/tests/config/test_middleware.py b/ckan/tests/config/test_middleware.py index 1f2c7effdf4..28cbf97b206 100644 --- a/ckan/tests/config/test_middleware.py +++ b/ckan/tests/config/test_middleware.py @@ -137,24 +137,6 @@ def test_view(): return app -def test_ask_around_pylons_core_route_get(patched_app): - environ = {u"PATH_INFO": u"/tag", u"REQUEST_METHOD": u"GET"} - wsgiref.util.setup_testing_defaults(environ) - - answers = patched_app.app.ask_around(environ) - - assert answers == [(False, u"flask_app"), (True, u"pylons_app", u"core")] - - -def test_ask_around_pylons_core_route_post(patched_app): - environ = {u"PATH_INFO": u"/tag", u"REQUEST_METHOD": u"POST"} - wsgiref.util.setup_testing_defaults(environ) - - answers = patched_app.app.ask_around(environ) - - assert answers == [(False, u"flask_app"), (True, u"pylons_app", u"core")] - - def test_flask_core_route_is_served_by_flask(patched_app): res = patched_app.get(u"/") @@ -184,8 +166,11 @@ def test_ask_around_pylons_extension_route_get_before_map( answers = patched_app.app.ask_around(environ) + # Even though this route is defined in Pylons, there is catch all route + # in Flask for all requests to serve static files with the same name, + # so we get two positive answers assert answers == [ - (False, u"flask_app"), + (True, u"flask_app", u"core"), (True, u"pylons_app", u"extension"), ] @@ -214,11 +199,12 @@ def test_ask_around_pylons_extension_route_post_using_get( answers = patched_app.app.ask_around(environ) - # We are going to get an answer from Pylons, but just because it will - # match the catch-all template route, hence the `core` origin. + # Even though this route is defined in Pylons, there is catch all route + # in Flask for all requests to serve static files with the same name, + # so we get two positive answers assert answers == [ - (False, u"flask_app"), - (True, u"pylons_app", u"core"), + (True, u"flask_app", u"core"), + (False, u"pylons_app"), ] def test_ask_around_pylons_extension_route_get_after_map( @@ -232,8 +218,11 @@ def test_ask_around_pylons_extension_route_get_after_map( answers = patched_app.app.ask_around(environ) + # Even though this route is defined in Pylons, there is catch all route + # in Flask for all requests to serve static files with the same name, + # so we get two positive answers assert answers == [ - (False, u"flask_app"), + (True, u"flask_app", u"core"), (True, u"pylons_app", u"extension"), ] @@ -453,10 +442,7 @@ def test_ask_around_flask_core_route_post(app): answers = ckan_app.ask_around(environ) - # Even though this route is defined in Flask, there is catch all route - # in Pylons for all requests to point arbitrary urls to templates with - # the same name, so we get two positive answers assert answers == [ (True, u"flask_app", u"core"), - (True, u"pylons_app", u"core"), + (False, u"pylons_app"), ] diff --git a/ckan/tests/controllers/test_template.py b/ckan/tests/controllers/test_template.py deleted file mode 100644 index eb17c607b14..00000000000 --- a/ckan/tests/controllers/test_template.py +++ /dev/null @@ -1,16 +0,0 @@ -# encoding: utf-8 - -import pytest - - -@pytest.mark.parametrize( - u"url,expected", - [ - (u"/robots.txt", u"text/plain; charset=utf-8"), - (u"/page", u"text/html; charset=utf-8"), - (u"/page.html", u"text/html; charset=utf-8"), - ], -) -def test_content_type(url, expected, app): - response = app.get(url, status=200) - assert response.headers.get(u"Content-Type") == expected From fe2e6ea3d085b8ee8096dc33d42b679e94b50aa6 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 12:40:05 +0100 Subject: [PATCH 08/21] Consolidate PY2 var name --- ckan/lib/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 07a035be59b..6c61416f856 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -277,7 +277,7 @@ def _get_auto_flask_context(): if _internal_test_request_context: return _internal_test_request_context - if six.Py2: + if six.PY2: from ckan.lib.cli import _cli_test_request_context From 4e015d93a56a7fd95a5daa6ad2b60038e9f1558e Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 12:40:32 +0100 Subject: [PATCH 09/21] [#4801] Use load_config from new CLI, support expanding user name --- ckan/cli/__init__.py | 9 ++++++--- ckan/tests/pytest_ckan/ckan_setup.py | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ckan/cli/__init__.py b/ckan/cli/__init__.py index 281f52b6ac4..b792fe0f023 100644 --- a/ckan/cli/__init__.py +++ b/ckan/cli/__init__.py @@ -22,10 +22,13 @@ def error_shout(exception): ) -def load_config(config=None): +def load_config(ini_path=None): from paste.deploy import appconfig - if config: - filename = os.path.abspath(config) + + if ini_path: + if ini_path.startswith('~'): + ini_path = os.path.expanduser(ini_path) + filename = os.path.abspath(ini_path) config_source = u'-c parameter' elif os.environ.get(u'CKAN_INI'): filename = os.environ.get(u'CKAN_INI') diff --git a/ckan/tests/pytest_ckan/ckan_setup.py b/ckan/tests/pytest_ckan/ckan_setup.py index 6cd29a12887..1bddb9dc269 100644 --- a/ckan/tests/pytest_ckan/ckan_setup.py +++ b/ckan/tests/pytest_ckan/ckan_setup.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from ckan.lib.cli import load_config +from ckan.cli import load_config def pytest_addoption(parser): From 7935b5477cdd3fd4b0e73ca146448068705a8606 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 13:42:51 +0100 Subject: [PATCH 10/21] [#4801] Default to Flask's uggettext (_) in all cases --- ckan/common.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ckan/common.py b/ckan/common.py index 1143439bcf1..e46ad4c8edc 100644 --- a/ckan/common.py +++ b/ckan/common.py @@ -71,10 +71,7 @@ def streaming_response( def ugettext(*args, **kwargs): - if is_flask_request(): - return flask_ugettext(*args, **kwargs) - else: - return pylons_ugettext(*args, **kwargs) + return flask_ugettext(*args, **kwargs) _ = ugettext From 5f7d60fc98f42f833af439ea70b64dd0c0952b5c Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 13:43:10 +0100 Subject: [PATCH 11/21] [#4801] Properly initialize environment in pytest I wrobgly assumed that the `load_config` method on the new cli did the same as the one in the old, but it just parses the config file. Added a call to `make_app` (which calls `load_environment`) to be able to generate a test request context in order to be able to generate URLs outside the context of an actual web request. --- ckan/lib/helpers.py | 4 ++++ ckan/tests/pytest_ckan/ckan_setup.py | 14 +++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 6c61416f856..de36659802c 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -277,6 +277,10 @@ def _get_auto_flask_context(): if _internal_test_request_context: return _internal_test_request_context + from ckan.tests.pytest_ckan.ckan_setup import _tests_test_request_context + if _tests_test_request_context: + return _tests_test_request_context + if six.PY2: from ckan.lib.cli import _cli_test_request_context diff --git a/ckan/tests/pytest_ckan/ckan_setup.py b/ckan/tests/pytest_ckan/ckan_setup.py index 1bddb9dc269..a9ac91caa4f 100644 --- a/ckan/tests/pytest_ckan/ckan_setup.py +++ b/ckan/tests/pytest_ckan/ckan_setup.py @@ -1,7 +1,12 @@ # -*- coding: utf-8 -*- +from ckan.config.middleware import make_app from ckan.cli import load_config +# This is a test Flask request context to be used internally. +# Do not use it! +_tests_test_request_context = None + def pytest_addoption(parser): """Allow using custom config file during tests. @@ -12,7 +17,14 @@ def pytest_addoption(parser): def pytest_sessionstart(session): """Initialize CKAN environment. """ - load_config(session.config.option.ckan_ini) + conf = load_config(session.config.option.ckan_ini) + # Set this internal test request context with the configured environment so + # it can be used when calling url_for from the cli. + global _tests_test_request_context + + app = make_app(conf.global_conf, **conf.local_conf) + flask_app = app.apps['flask_app']._wsgi_app + _tests_test_request_context = flask_app.test_request_context() def pytest_runtest_setup(item): From f72da33e71d9b4972875a08981dbcd2eabd6bfec Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 14:21:55 +0100 Subject: [PATCH 12/21] [#4801] Refactor augment_data so it does not change dicts on loops --- ckan/lib/navl/dictization_functions.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/ckan/lib/navl/dictization_functions.py b/ckan/lib/navl/dictization_functions.py index a1f274c20a5..e9a79b1d927 100644 --- a/ckan/lib/navl/dictization_functions.py +++ b/ckan/lib/navl/dictization_functions.py @@ -176,8 +176,10 @@ def augment_data(data, schema): new_data = copy.copy(data) + keys_to_remove = [] + junk = {} + extras_keys = {} # fill junk and extras - for key, value in new_data.items(): if key in full_schema: continue @@ -190,16 +192,21 @@ def augment_data(data, schema): raise DataError('Only lists of dicts can be placed against ' 'subschema %s, not %s' % (key, type(data[key]))) - if key[:-1] in key_combinations: extras_key = key[:-1] + ('__extras',) - extras = new_data.get(extras_key, {}) + extras = extras_keys.get(extras_key, {}) extras[key[-1]] = value - new_data[extras_key] = extras + extras_keys[extras_key] = extras else: - junk = new_data.get(("__junk",), {}) junk[key] = value - new_data[("__junk",)] = junk + keys_to_remove.append(key) + + if junk: + new_data[("__junk",)] = junk + for extra_key in extras_keys: + new_data[extra_key] = extras_keys[extra_key] + + for key in keys_to_remove: new_data.pop(key) # add missing From 8cce17b00dda4011f82f659e6398a21a8e8a9cfa Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 14:47:02 +0100 Subject: [PATCH 13/21] Coding standards --- ckan/cli/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/cli/__init__.py b/ckan/cli/__init__.py index b792fe0f023..08ca0ead3e4 100644 --- a/ckan/cli/__init__.py +++ b/ckan/cli/__init__.py @@ -26,7 +26,7 @@ def load_config(ini_path=None): from paste.deploy import appconfig if ini_path: - if ini_path.startswith('~'): + if ini_path.startswith(u'~'): ini_path = os.path.expanduser(ini_path) filename = os.path.abspath(ini_path) config_source = u'-c parameter' From c2ba9410a76b5b068c22d03f17a2d46a8643ba9b Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 14:48:12 +0100 Subject: [PATCH 14/21] [#4801] Remove deprecated helper function --- ckan/lib/helpers.py | 16 ---------------- ckan/tests/legacy/lib/test_helpers.py | 6 ------ 2 files changed, 22 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index de36659802c..ddbaa9b3274 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -1724,22 +1724,6 @@ def tzname(self, dt): return None -@core_helper -@maintain.deprecated('h.time_ago_in_words_from_str is deprecated in 2.2 ' - 'and will be removed. Please use ' - 'h.time_ago_from_timestamp instead') -def time_ago_in_words_from_str(date_str, granularity='month'): - '''Deprecated in 2.2 use time_ago_from_timestamp''' - if date_str: - try: - return formatters.localised_nice_date( - date_str_to_datetime(date_str), show_date=False - ) - except ValueError: - pass - return _('Unknown') - - @core_helper def time_ago_from_timestamp(timestamp): ''' Returns a string like `5 months ago` for a datetime relative to now diff --git a/ckan/tests/legacy/lib/test_helpers.py b/ckan/tests/legacy/lib/test_helpers.py index 259248b9222..c71524fdb9c 100644 --- a/ckan/tests/legacy/lib/test_helpers.py +++ b/ckan/tests/legacy/lib/test_helpers.py @@ -53,12 +53,6 @@ def test_date_str_to_datetime_with_ambiguous_microseconds(self): with pytest.raises(ValueError): h.date_str_to_datetime("2008-04-13T20:40:20.500") - def test_time_ago_in_words_from_str(self): - two_months_ago = datetime.datetime.now() - datetime.timedelta(days=65) - two_months_ago_str = two_months_ago.isoformat() - res = h.time_ago_in_words_from_str(two_months_ago_str) - assert res == "2 months ago" - def test_gravatar(self): email = "zephod@gmail.com" expected = [ From 8c913e2be1e2a6014337cd1dde2a6b59b8c1353b Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 14:49:04 +0100 Subject: [PATCH 15/21] [#4801] Don't use pylons to get language outside a request --- ckan/lib/i18n.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/ckan/lib/i18n.py b/ckan/lib/i18n.py index caa40252f6f..70a0fb1d4bd 100644 --- a/ckan/lib/i18n.py +++ b/ckan/lib/i18n.py @@ -49,7 +49,6 @@ UnknownLocaleError) from babel.support import Translations import polib -import six from ckan.common import config, is_flask_request, aslist import ckan.i18n @@ -57,7 +56,7 @@ from ckan.plugins.interfaces import ITranslation if six.PY2: - from pylons import i18n + from pylons import i18n as pylons_i18n import pylons @@ -228,9 +227,9 @@ def _set_lang(lang): if config.get('ckan.i18n_directory'): fake_config = {'pylons.paths': {'root': config['ckan.i18n_directory']}, 'pylons.package': config['pylons.package']} - i18n.set_lang(lang, pylons_config=fake_config, class_=Translations) + pylons_i18n.set_lang(lang, pylons_config=fake_config, class_=Translations) else: - i18n.set_lang(lang, class_=Translations) + pylons_i18n.set_lang(lang, class_=Translations) def handle_request(request, tmpl_context): @@ -279,10 +278,6 @@ def get_lang(): if is_flask_request(): from ckan.config.middleware.flask_app import get_locale return get_locale() - else: - langs = i18n.get_lang() - if langs: - return langs[0] return 'en' From 1167dac9b3c81c907ad8e0da926496f56e3583fb Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 14:57:02 +0100 Subject: [PATCH 16/21] [#4801] Consolidate usage of OrderedDict Always import from collections. Remove from ckan.common --- ckan/authz.py | 4 ++-- ckan/common.py | 5 ----- ckan/model/__init__.py | 3 --- ckan/model/domain_object.py | 2 +- ckan/model/resource.py | 2 +- ckan/views/group.py | 6 +++--- ckanext/datastore/backend/postgres.py | 3 ++- 7 files changed, 9 insertions(+), 16 deletions(-) diff --git a/ckan/authz.py b/ckan/authz.py index f96c86c30c8..4d607eb3275 100644 --- a/ckan/authz.py +++ b/ckan/authz.py @@ -3,7 +3,7 @@ import functools import sys -from collections import defaultdict +from collections import defaultdict, OrderedDict from logging import getLogger import six @@ -13,7 +13,7 @@ import ckan.plugins as p import ckan.model as model -from ckan.common import OrderedDict, _, c +from ckan.common import _, c import ckan.lib.maintain as maintain diff --git a/ckan/common.py b/ckan/common.py index e46ad4c8edc..28963f3890c 100644 --- a/ckan/common.py +++ b/ckan/common.py @@ -28,11 +28,6 @@ current_app = flask.current_app -try: - from collections import OrderedDict # from python 2.7 -except ImportError: - from sqlalchemy.util import OrderedDict - def is_flask_request(): u''' diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index c4c42ec4e49..5e55d0b041d 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -3,13 +3,10 @@ import warnings import logging import re -from datetime import datetime from time import sleep from os.path import splitext -from six import text_type from sqlalchemy import MetaData, __version__ as sqav, Table -from sqlalchemy.util import OrderedDict from sqlalchemy.exc import ProgrammingError from alembic.command import ( diff --git a/ckan/model/domain_object.py b/ckan/model/domain_object.py index 47fdd363f97..d91fec4a5b5 100644 --- a/ckan/model/domain_object.py +++ b/ckan/model/domain_object.py @@ -1,10 +1,10 @@ # encoding: utf-8 import datetime +from collections import OrderedDict import sqlalchemy as sa from sqlalchemy import orm -from sqlalchemy.util import OrderedDict from ckan.model import meta, core diff --git a/ckan/model/resource.py b/ckan/model/resource.py index 11bf74f023d..8d4b5e0eb3a 100644 --- a/ckan/model/resource.py +++ b/ckan/model/resource.py @@ -3,7 +3,7 @@ import datetime from six import text_type -from sqlalchemy.util import OrderedDict +from collections import OrderedDict from sqlalchemy.ext.orderinglist import ordering_list from sqlalchemy import orm from ckan.common import config diff --git a/ckan/views/group.py b/ckan/views/group.py index 04cd8467c12..8fa552eb68e 100644 --- a/ckan/views/group.py +++ b/ckan/views/group.py @@ -2,11 +2,11 @@ import logging import re -from six.moves.urllib.parse import urlencode - +from collections import OrderedDict import six from six import string_types +from six.moves.urllib.parse import urlencode import ckan.lib.base as base import ckan.lib.helpers as h @@ -17,7 +17,7 @@ import ckan.authz as authz import ckan.lib.plugins as lib_plugins import ckan.plugins as plugins -from ckan.common import OrderedDict, g, config, request, _ +from ckan.common import g, config, request, _ from flask import Blueprint from flask.views import MethodView diff --git a/ckanext/datastore/backend/postgres.py b/ckanext/datastore/backend/postgres.py index 202fdce7d18..7b5716481ce 100644 --- a/ckanext/datastore/backend/postgres.py +++ b/ckanext/datastore/backend/postgres.py @@ -10,6 +10,7 @@ import datetime import hashlib import json +from collections import OrderedDict import six from six.moves.urllib.parse import ( @@ -31,7 +32,7 @@ import ckan.model as model import ckan.plugins as plugins -from ckan.common import config, OrderedDict +from ckan.common import config from ckanext.datastore.backend import ( DatastoreBackend, From a44af3ef9aa2762b4b130d2afbb67ff5146c106d Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 14:58:34 +0100 Subject: [PATCH 17/21] [#4801] OrderedDict.keys() returns an odict_keys object in py3 --- ckan/cli/sysadmin.py | 2 +- ckan/views/dataset.py | 2 +- ckan/views/group.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ckan/cli/sysadmin.py b/ckan/cli/sysadmin.py index d30e6959876..2d959e3edfa 100644 --- a/ckan/cli/sysadmin.py +++ b/ckan/cli/sysadmin.py @@ -5,7 +5,7 @@ import ckan.model as model from ckan.cli import error_shout -from ckan.lib.cli import user_add +#from ckan.lib.cli import user_add @click.group( diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index 31fb4daaffa..dfe5dce04f1 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -276,7 +276,7 @@ def search(package_type): data_dict = { u'q': q, u'fq': fq.strip(), - u'facet.field': facets.keys(), + u'facet.field': list(facets.keys()), u'rows': limit, u'start': (page - 1) * limit, u'sort': sort_by, diff --git a/ckan/views/group.py b/ckan/views/group.py index 8fa552eb68e..46aeba23b13 100644 --- a/ckan/views/group.py +++ b/ckan/views/group.py @@ -361,7 +361,7 @@ def pager_url(q=None, page=None): u'q': q, u'fq': fq, u'include_private': True, - u'facet.field': facets.keys(), + u'facet.field': list(facets.keys()), u'rows': limit, u'sort': sort_by, u'start': (page - 1) * limit, From cb9b03ee921b8a1f9dcb46241ca925b739d6d4cb Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 15:11:55 +0100 Subject: [PATCH 18/21] [#4801] Use ints in paginator Otherwise we get the following in py3: TypeError: 'float' object cannot be interpreted as an integer --- ckan/lib/pagination.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ckan/lib/pagination.py b/ckan/lib/pagination.py index 0385466a974..795530cde74 100644 --- a/ckan/lib/pagination.py +++ b/ckan/lib/pagination.py @@ -176,7 +176,8 @@ def __init__( # Compute the number of the first and last available page if self.item_count > 0: self.first_page = 1 - self.page_count = ((self.item_count - 1) / self.items_per_page) + 1 + self.page_count = int( + ((self.item_count - 1) / self.items_per_page) + 1) self.last_page = self.first_page + self.page_count - 1 # Make sure that the requested page number is the range of From fe2c5e154915bfe500d03564fc878853881123f1 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 15:13:28 +0100 Subject: [PATCH 19/21] Pep 8 --- ckan/cli/sysadmin.py | 2 +- ckan/lib/i18n.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ckan/cli/sysadmin.py b/ckan/cli/sysadmin.py index 2d959e3edfa..d30e6959876 100644 --- a/ckan/cli/sysadmin.py +++ b/ckan/cli/sysadmin.py @@ -5,7 +5,7 @@ import ckan.model as model from ckan.cli import error_shout -#from ckan.lib.cli import user_add +from ckan.lib.cli import user_add @click.group( diff --git a/ckan/lib/i18n.py b/ckan/lib/i18n.py index 70a0fb1d4bd..32adda8d3ec 100644 --- a/ckan/lib/i18n.py +++ b/ckan/lib/i18n.py @@ -227,7 +227,8 @@ def _set_lang(lang): if config.get('ckan.i18n_directory'): fake_config = {'pylons.paths': {'root': config['ckan.i18n_directory']}, 'pylons.package': config['pylons.package']} - pylons_i18n.set_lang(lang, pylons_config=fake_config, class_=Translations) + pylons_i18n.set_lang( + lang, pylons_config=fake_config, class_=Translations) else: pylons_i18n.set_lang(lang, class_=Translations) From 3a8f00e4f59f95186f956579952d7b8633cce594 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 15:40:21 +0100 Subject: [PATCH 20/21] [#4801] Do not fall back to Pylons on url_for on py3 --- ckan/lib/helpers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index ddbaa9b3274..6ffe84d43c2 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -344,7 +344,6 @@ def url_for(*args, **kw): raise Exception('API URLs must specify the version (eg ver=3)') _auto_flask_context = _get_auto_flask_context() - try: if _auto_flask_context: _auto_flask_context.push() @@ -356,9 +355,11 @@ def url_for(*args, **kw): my_url = _url_for_flask(*args, **kw) except FlaskRouteBuildError: - - # If it doesn't succeed, fallback to the Pylons router - my_url = _url_for_pylons(*args, **kw) + if six.PY2: + # If it doesn't succeed, fallback to the Pylons router + my_url = _url_for_pylons(*args, **kw) + else: + raise finally: if _auto_flask_context: _auto_flask_context.pop() From a35902f1af876ad2e825a990f13d2e857b2eaec7 Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 20 Dec 2019 16:10:56 +0100 Subject: [PATCH 21/21] [#4801] Don't use controller based routes on url_for --- ckan/templates/snippets/follow_button.html | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ckan/templates/snippets/follow_button.html b/ckan/templates/snippets/follow_button.html index f0e3e654dc6..a3b6f06d220 100644 --- a/ckan/templates/snippets/follow_button.html +++ b/ckan/templates/snippets/follow_button.html @@ -1,15 +1,10 @@ -{% set controller = obj_type %} -{% if controller == 'dataset' %} - {% set controller = 'package' %} -{% endif %} - {% if following %} - + {{ _('Unfollow') }} {% else %} - + {{ _('Follow') }}