Merge pull request #167 from hasgeek/classview

Class-based views

Author: jace

CHANGES.rst

@@ -18,6 +18,9 @@
 * New: ``coaster.auth`` module with a ``current_auth`` proxy that provides
   a standardised API for login managers to use
 * New: ``is_collection`` util for testing if an item is a collection data type
+* New: ``coaster.views.requires_permission`` decorator
+* New: ``coaster.views.classview`` provides a new take on organising views
+  into a class
 
 
 0.6.0

coaster/app.py

@@ -17,6 +17,7 @@
     from flask.json import tojson_filter as _tojson_filter
 from . import logger
 from .auth import current_auth
+from .views import current_view
 
 __all__ = ['SandboxedFlask', 'init_app']
 
@@ -103,6 +104,8 @@ def init_app(app, env=None):
     """
     # Make current_auth available to app templates
     app.jinja_env.globals['current_auth'] = current_auth
+    # Make the current view available to app templates
+    app.jinja_env.globals['current_view'] = current_view
     # Disable Flask-SQLAlchemy events.
     # Apps that want it can turn it back on in their config
     app.config.setdefault('SQLALCHEMY_TRACK_MODIFICATIONS', False)

coaster/sqlalchemy/comparators.py

@@ -8,6 +8,7 @@
 from __future__ import absolute_import
 import uuid as uuid_
 from sqlalchemy.ext.hybrid import Comparator
+from flask import abort
 from flask_sqlalchemy import BaseQuery
 import six
 from ..utils import buid2uuid, suuid2uuid
@@ -38,6 +39,18 @@ def isempty(self):
         """
         return not self.session.query(self.exists()).scalar()
 
+    def one_or_404(self):
+        """
+        Extends :meth:`~sqlalchemy.orm.query.Query.one_or_none` to raise a 404
+        if no result is found. This method offers a safety net over
+        :meth:`~flask_sqlalchemy.BaseQuery.first_or_404` as it helps identify
+        poorly specified queries that could have returned more than one result.
+        """
+        result = self.one_or_none()
+        if not result:
+            abort(404)
+        return result
+
 
 class SplitIndexComparator(Comparator):
     """

coaster/sqlalchemy/mixins.py

@@ -31,7 +31,8 @@ class MyModel(BaseMixin, db.Model):
 from sqlalchemy_utils.types import UUIDType
 from flask import url_for
 import six
-from ..utils import make_name, uuid2suuid, uuid2buid, buid2uuid, suuid2uuid
+from ..utils import make_name, uuid2suuid, uuid2buid, buid2uuid, suuid2uuid, InspectableSet
+from ..auth import current_auth
 from .immutable_annotation import immutable
 from .roles import RoleMixin, with_roles
 from .comparators import Query, SqlSplitIdComparator, SqlHexUuidComparator, SqlBuidComparator, SqlSuuidComparator
@@ -225,6 +226,15 @@ def permissions(self, user, inherited=None):
         else:
             return set()
 
+    @property
+    def current_permissions(self):
+        """
+        :class:`~coaster.utils.classes.InspectableSet` containing currently
+        available permissions from this object, using
+        :obj:`~coaster.auth.current_auth`.
+        """
+        return InspectableSet(self.permissions(current_auth.actor))
+
 
 class UrlForMixin(object):
     """
@@ -262,6 +272,9 @@ def url_for(self, action='view', **kwargs):
 
     @classmethod
     def is_url_for(cls, _action, _endpoint=None, _external=None, **paramattrs):
+        """
+        View decorator that registers the view as a :meth:`url_for` target.
+        """
         def decorator(f):
             if 'url_for_endpoints' not in cls.__dict__:
                 cls.url_for_endpoints = {}  # Stick it into the class with the first endpoint
@@ -536,7 +549,7 @@ def __init__(self, *args, **kw):
             self.make_name()
 
     def __repr__(self):
-        return '<%s %s "%s">' % (self.__class__.__name__, self.url_name, self.title)
+        return '<%s %s "%s">' % (self.__class__.__name__, self.url_id_name, self.title)
 
     def make_name(self):
         """Autogenerates a :attr:`name` from the :attr:`title`"""
@@ -571,11 +584,19 @@ def url_name_suuid(self):
         Returns a URL name combining :attr:`name` and :attr:`suuid` in name-suuid syntax.
         To use this, the class must derive from :class:`UuidMixin`.
         """
-        return '%s-%s' % (self.name, self.suuid)
+        if isinstance(self, UuidMixin):
+            return '%s-%s' % (self.name, self.suuid)
+        else:
+            return '%s-%s' % (self.name, self.url_id)
 
     @url_name_suuid.comparator
     def url_name_suuid(cls):
-        return SqlSuuidComparator(cls.uuid, splitindex=-1)
+        if issubclass(cls, UuidMixin):
+            return SqlSuuidComparator(cls.uuid, splitindex=-1)
+        elif cls.__uuid_primary_key__:
+            return SqlHexUuidComparator(cls.id, splitindex=-1)
+        else:
+            return SqlSplitIdComparator(cls.id, splitindex=-1)
 
 
 class BaseScopedIdMixin(BaseMixin):
@@ -624,8 +645,10 @@ def permissions(self, user, inherited=None):
         """
         if inherited is not None:
             return inherited | super(BaseScopedIdMixin, self).permissions(user)
-        else:
+        elif self.parent is not None and isinstance(self.parent, PermissionMixin):
             return self.parent.permissions(user) | super(BaseScopedIdMixin, self).permissions(user)
+        else:
+            return super(BaseScopedIdMixin, self).permissions(user)
 
 
 class BaseScopedIdNameMixin(BaseScopedIdMixin):
@@ -682,7 +705,7 @@ def __init__(self, *args, **kw):
             self.make_name()
 
     def __repr__(self):
-        return '<%s %s "%s" of %s>' % (self.__class__.__name__, self.url_name, self.title,
+        return '<%s %s "%s" of %s>' % (self.__class__.__name__, self.url_id_name, self.title,
             repr(self.parent)[1:-1] if self.parent else None)
 
     @classmethod

coaster/sqlalchemy/statemanager.py

@@ -456,6 +456,9 @@ def add_transition(self, statemanager, from_, to, if_=None, data=None):
             'if': if_,             # Additional conditions that must ALL pass
             }
 
+    def __set_name__(self, owner, name):  # pragma: no cover
+        self.name = name
+
     # Make the transition a non-data descriptor
     def __get__(self, obj, cls=None):
         if obj is None:
@@ -501,6 +504,9 @@ def is_available(self):
         """
         return not self._state_invalid()
 
+    def __getattr__(self, name):
+        return getattr(self.statetransition, name)
+
     def __call__(self, *args, **kwargs):
         """Call the transition"""
         # Validate that each of the state managers is in the correct state
@@ -827,17 +833,17 @@ def group(self, items, keep_empty=False):
                     del groups[key]
         return groups
 
-    def __getattr__(self, attr):
+    def __getattr__(self, name):
         """
         Given the name of a state, returns:
 
-        1. If called on an instance, a boolean indicating if the state is active
+        1. If called on an instance, a ManagedStateWrapper, which implements __bool__
         2. If called on a class, a query filter
 
         Returns the default value or raises :exc:`AttributeError` on anything else.
         """
-        if hasattr(self.statemanager, attr):
-            mstate = getattr(self.statemanager, attr)
+        if hasattr(self.statemanager, name):
+            mstate = getattr(self.statemanager, name)
             if isinstance(mstate, (ManagedState, ManagedStateGroup)):
                 return mstate(self.obj, self.cls)
-        raise AttributeError("Not a state: %s" % attr)
+        raise AttributeError("Not a state: %s" % name)

coaster/utils/classes.py

@@ -214,9 +214,10 @@ def value_for(cls, name):
 
 class InspectableSet(Set):
     """
-    Given a set, mimics a dictionary where the items are keys and have a value
-    of ``True``, and any other key has a value of ``False``. Also supports
-    attribute access. Useful in templates to simplify membership inspection::
+    Given a set, mimics a read-only dictionary where the items are keys and
+    have a value of ``True``, and any other key has a value of ``False``. Also
+    supports attribute access. Useful in templates to simplify membership
+    inspection::
 
         >>> myset = InspectableSet({'member', 'other'})
         >>> 'member' in myset
@@ -231,6 +232,20 @@ class InspectableSet(Set):
         True
         >>> myset['random']
         False
+        >>> joinset = myset | {'added'}
+        >>> isinstance(joinset, InspectableSet)
+        True
+        >>> joinset = joinset | InspectableSet({'inspectable'})
+        >>> isinstance(joinset, InspectableSet)
+        True
+        >>> 'member' in joinset
+        True
+        >>> 'other' in joinset
+        True
+        >>> 'added' in joinset
+        True
+        >>> 'inspectable' in joinset
+        True
     """
     def __init__(self, members):
         if not isinstance(members, set):

coaster/views/__init__.py

@@ -10,3 +10,4 @@
 from __future__ import absolute_import
 from .misc import *  # NOQA
 from .decorators import *  # NOQA
+from .classview import *  # NOQA

coaster/views/classview.py

@@ -19,13 +19,13 @@
     request, Response, url_for)
 from ..utils import is_collection
 from ..auth import current_auth, add_auth_attribute
-from .misc import jsonp as render_jsonp
+from .misc import jsonp
 
 __all__ = [
     'RequestTypeError', 'RequestValueError',
     'requestargs', 'requestform', 'requestquery',
     'load_model', 'load_models',
-    'render_with', 'cors',
+    'render_with', 'cors', 'requires_permission',
     ]
 
 
@@ -108,7 +108,7 @@ def datasource():
                 return request.values if request else {}
 
         @wraps(f)
-        def decorated_function(**kw):
+        def decorated_function(*args, **kw):
             values = datasource()
             for name, filt, is_list in namefilt:
                 # Process name if
@@ -123,7 +123,7 @@ def decorated_function(**kw):
                     except ValueError as e:
                         raise RequestValueError(e)
             try:
-                return f(**kw)
+                return f(*args, **kw)
             except TypeError as e:
                 raise RequestTypeError(e)
         return decorated_function
@@ -236,7 +236,7 @@ def show_page(folder, page):
     """
     def inner(f):
         @wraps(f)
-        def decorated_function(**kw):
+        def decorated_function(*args, **kw):
             permissions = None
             permission_required = kwargs.get('permission')
             url_check_attributes = kwargs.get('urlcheck', [])
@@ -316,14 +316,13 @@ def decorated_function(**kw):
             if permission_required and not (permission_required & permissions):
                 abort(403)
             if kwargs.get('kwargs'):
-                return f(kwargs=kw, **result)
+                return f(*args, kwargs=kw, **result)
             else:
-                return f(**result)
+                return f(*args, **result)
         return decorated_function
     return inner
 
 
-
 def _best_mimetype_match(available_list, accept_mimetypes, default=None):
     for use_mimetype, quality in accept_mimetypes:
         for mimetype in available_list:
@@ -332,7 +331,21 @@ def _best_mimetype_match(available_list, accept_mimetypes, default=None):
     return default
 
 
-def render_with(template, json=False, jsonp=False):
+def dict_jsonify(param):
+    """Convert the parameter into a dictionary before calling jsonify, if it's not already one"""
+    if not isinstance(param, dict):
+        param = dict(param)
+    return jsonify(param)
+
+
+def dict_jsonp(param):
+    """Convert the parameter into a dictionary before calling jsonp, if it's not already one"""
+    if not isinstance(param, dict):
+        param = dict(param)
+    return jsonp(param)
+
+
+def render_with(template=None, json=False, jsonp=False):
     """
     Decorator to render the wrapped function with the given template (or dictionary
     of mimetype keys to templates, where the template is a string name of a template
@@ -393,30 +406,29 @@ def myview():
     """
     if jsonp:
         templates = {
-            'application/json': render_jsonp,
-            'text/json': render_jsonp,
-            'text/x-json': render_jsonp,
+            'application/json': dict_jsonp,
+            'application/javascript': dict_jsonp,
             }
     elif json:
         templates = {
-            'application/json': jsonify,
-            'text/json': jsonify,
-            'text/x-json': jsonify,
+            'application/json': dict_jsonify,
             }
     else:
         templates = {}
     if isinstance(template, six.string_types):
         templates['text/html'] = template
     elif isinstance(template, dict):
         templates.update(template)
+    elif template is None and (json or jsonp):
+        pass
     else:  # pragma: no cover
         raise ValueError("Expected string or dict for template")
 
     default_mimetype = '*/*'
     if '*/*' not in templates:
         templates['*/*'] = six.text_type
         default_mimetype = 'text/plain'
-        for mimetype in ('text/html', 'text/plain', 'application/json', 'text/json', 'text/x-json'):
+        for mimetype in ('text/html', 'text/plain', 'application/json'):
             if mimetype in templates:
                 templates['*/*'] = templates[mimetype]
                 default_mimetype = mimetype  # Remember which mimetype's handler is serving for */*
@@ -582,3 +594,31 @@ def wrapper(*args, **kwargs):
             return resp
         return wrapper
     return inner
+
+
+def requires_permission(permission):
+    """
+    View decorator that requires a certain permission to be present in
+    ``current_auth.permissions`` before the view is allowed to proceed.
+    Aborts with ``403 Forbidden`` if the permission is not present.
+
+    :param permission: Permission that is required. If an iterable is provided,
+        any one permission must be available
+    """
+    def inner(f):
+        @wraps(f)
+        def wrapper(*args, **kwargs):
+            add_auth_attribute('login_required', True)
+            if not hasattr(current_auth, 'permissions'):
+                test = False
+            elif is_collection(permission):
+                test = bool(current_auth.permissions.intersection(permission))
+            else:
+                test = permission in current_auth.permissions
+            if not test:
+                abort(403)
+            return f(*args, **kwargs)
+
+        wrapper.requires_permission = permission
+        return wrapper
+    return inner

coaster/views/decorators.py

@@ -248,5 +248,7 @@
 # Example configuration for intersphinx: refer to the Python standard library.
 intersphinx_mapping = {
     'https://docs.python.org/2/': None,
-    'http://flask-sqlalchemy.pocoo.org/2.2/': None,
+    'http://flask.pocoo.org/docs/': None,
+    'http://docs.sqlalchemy.org/en/latest/': None,
+    'http://flask-sqlalchemy.pocoo.org/2.3/': None,
     }

docs/conf.py

@@ -0,0 +1,2 @@
+.. automodule:: coaster.views.classview
+   :members:

docs/views/classview.rst

@@ -5,3 +5,4 @@
 
    misc
    decorators
+   classview

docs/views/index.rst

@@ -837,9 +837,8 @@ def test_uuid_url_id_name_suuid(self):
 
         self.assertEqual(u1.url_id, '74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u1.url_id_name, '74d588574a7611e78c27c38403d0935c-test')
-        with self.assertRaises(AttributeError):
-            # No UuidMixin == No suuid or url_name_suuid attributes
-            self.assertEqual(u1.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN')
+        # No UuidMixin means no suuid, so fallback to hex UUID
+        self.assertEqual(u1.url_name_suuid, 'test-74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u2.url_id, '74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u2.url_id_name, '74d588574a7611e78c27c38403d0935c-test')
         self.assertEqual(u2.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN')

tests/test_models.py

@@ -1,10 +1,12 @@
 # -*- coding: utf-8 -*-
 
 import unittest
-from werkzeug.exceptions import BadRequest
+from werkzeug.exceptions import BadRequest, Forbidden
 from flask import Flask, session, json
 from coaster.app import load_config_from_file
-from coaster.views import get_current_url, get_next_url, jsonp, requestargs, requestquery, requestform
+from coaster.auth import current_auth, add_auth_attribute
+from coaster.views import (get_current_url, get_next_url, jsonp, requestargs, requestquery, requestform,
+    requires_permission)
 
 
 def index():
@@ -45,6 +47,18 @@ def requestcombo_test(query1, form1):
     return query1, form1
 
 
+@requires_permission('allow-this')
+def permission1():
+    return 'allowed1'
+
+
+@requires_permission({'allow-this', 'allow-that'})
+def permission2():
+    return 'allowed2'
+
+
+# --- Tests -------------------------------------------------------------------
+
 class TestCoasterViews(unittest.TestCase):
     def setUp(self):
         self.app = Flask(__name__)
@@ -134,3 +148,26 @@ def test_requestargs(self):
 
         # Calling without a request context works as well
         self.assertEqual(requestargs_test1(p1='1', p2=3, p3=[1, 2]), ('1', 3, [1, 2]))
+
+    def test_requires_permission(self):
+        with self.app.test_request_context():
+            with self.assertRaises(Forbidden):
+                permission1()
+            with self.assertRaises(Forbidden):
+                permission2()
+
+            add_auth_attribute('permissions', set())
+
+            with self.assertRaises(Forbidden):
+                permission1()
+            with self.assertRaises(Forbidden):
+                permission2()
+
+            current_auth.permissions.add('allow-that')  # FIXME! Shouldn't this be a frozenset?
+            with self.assertRaises(Forbidden):
+                permission1()
+            assert permission2() == 'allowed2'
+
+            current_auth.permissions.add('allow-this')
+            assert permission1() == 'allowed1'
+            assert permission2() == 'allowed2'