Skip to content

Commit

Permalink
[#3196] Common request object for Flask and Pylons
Browse files Browse the repository at this point in the history
A new CKAN Request object based on Werkzeug's LocalProxy that will forward
to Flask or Pylons own request objects depending on the output of
`_get_request` (which essentially calls `is_flask_request`), and at the
same time provide all objects methods to be able to interact with them
transparently.

We don't use LocalProxy directly so we can handle the special case of
accessing query string parameters via `request.params` on Flask (which
uses `request.args`). All new Flask-only code should use `args` but for
backwards compatibility we will support `params` for a while.
  • Loading branch information
amercader committed Aug 8, 2016
1 parent 60134e8 commit 2ca6171
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 4 deletions.
59 changes: 57 additions & 2 deletions ckan/common.py
Expand Up @@ -13,10 +13,10 @@
import flask
import pylons

from werkzeug.local import Local
from werkzeug.local import Local, LocalProxy

from pylons.i18n import _, ungettext
from pylons import g, c, request, session, response
from pylons import g, c, session, response
import simplejson as json

try:
Expand All @@ -25,6 +25,25 @@
from sqlalchemy.util import OrderedDict


def is_flask_request():
u'''
A centralized way to determine whether we are in the context of a
request being served by Flask or Pylons
'''
try:
pylons.request.environ
pylons_request_available = True
except TypeError:
pylons_request_available = False

if (flask.request and
(flask.request.environ.get(u'ckan.app') == u'flask_app' or
not pylons_request_available)):
return True
else:
return False


class CKANConfig(MutableMapping):
u'''Main CKAN configuration object
Expand Down Expand Up @@ -91,10 +110,46 @@ def __delitem__(self, key):
except TypeError:
pass


def _get_request():
if is_flask_request():
return flask.request
else:
return pylons.request


class CKANRequest(LocalProxy):
u'''Common request object
This is just a wrapper around LocalProxy so we can handle some special
cases for backwards compatibility.
LocalProxy will forward to Flask or Pylons own request objects depending
on the output of `_get_request` (which essentially calls
`is_flask_request`) and at the same time provide all objects methods to be
able to interact with them transparently.
'''

def __getattr__(self, name):
u''' Special case as Pylons' request.params is used all over the place.
All new code meant to be run just in Flask (eg views) should always
use request.args
'''
try:
return super(CKANRequest, self).__getattr__(name)
except AttributeError:
if name == u'params':
return super(CKANRequest, self).__getattr__(u'args')
else:
raise

local = Local()

# This a proxy to the bounded config object
local(u'config')

# Thread-local safe objects
config = local.config = CKANConfig()

# Proxies to already thread-local safe objects
request = CKANRequest(_get_request)
25 changes: 23 additions & 2 deletions ckan/tests/test_common.py
Expand Up @@ -3,10 +3,11 @@
import flask
import pylons

from nose.tools import eq_, assert_not_equal as neq_
from nose.tools import eq_, assert_not_equal as neq_, assert_raises

from ckan.tests import helpers
from ckan.common import CKANConfig, config as ckan_config
from ckan.common import (CKANConfig, config as ckan_config,
request as ckan_request)


class TestConfigObject(object):
Expand Down Expand Up @@ -212,3 +213,23 @@ def test_config_option_update_action_works_on_flask(self):
helpers.call_action(u'config_option_update', {}, **params)

eq_(pylons.config[u'ckan.site_title'], u'Example title action')


class TestCommonRequest(object):

def test_params_also_works_on_flask_request(self):

app = helpers._get_test_app()

with app.flask_app.test_request_context(u'/hello?a=1'):

assert u'a' in ckan_request.args
assert u'a' in ckan_request.params

def test_other_missing_attributes_raise_attributeerror_exceptions(self):

app = helpers._get_test_app()

with app.flask_app.test_request_context(u'/hello?a=1'):

assert_raises(AttributeError, getattr, ckan_request, u'not_here')

0 comments on commit 2ca6171

Please sign in to comment.