From 2ca6171a0610b71e63ea76f1dae2f45df04f548a Mon Sep 17 00:00:00 2001
From: amercader <amercadero@gmail.com>
Date: Mon, 8 Aug 2016 16:14:06 +0100
Subject: [PATCH] [#3196] Common request object for Flask and Pylons

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.
---
 ckan/common.py            | 59 +++++++++++++++++++++++++++++++++++++--
 ckan/tests/test_common.py | 25 +++++++++++++++--
 2 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/ckan/common.py b/ckan/common.py
index cfb7409004c..d29fcc63ee3 100644
--- a/ckan/common.py
+++ b/ckan/common.py
@@ -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:
@@ -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
 
@@ -91,6 +110,39 @@ 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
@@ -98,3 +150,6 @@ def __delitem__(self, key):
 
 # Thread-local safe objects
 config = local.config = CKANConfig()
+
+# Proxies to already thread-local safe objects
+request = CKANRequest(_get_request)
diff --git a/ckan/tests/test_common.py b/ckan/tests/test_common.py
index cf3dda812a2..d498dd62292 100644
--- a/ckan/tests/test_common.py
+++ b/ckan/tests/test_common.py
@@ -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):
@@ -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')