diff --git a/CHANGES.rst b/CHANGES.rst index 22b3b00e..38ba5c4e 100644 --- a/CHANGES.rst +++ b/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 diff --git a/coaster/app.py b/coaster/app.py index f0f55f2d..28c3e430 100644 --- a/coaster/app.py +++ b/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) diff --git a/coaster/sqlalchemy/comparators.py b/coaster/sqlalchemy/comparators.py index b973bca0..b60da415 100644 --- a/coaster/sqlalchemy/comparators.py +++ b/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): """ diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py index 6456af4a..cb78833c 100644 --- a/coaster/sqlalchemy/mixins.py +++ b/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 diff --git a/coaster/sqlalchemy/statemanager.py b/coaster/sqlalchemy/statemanager.py index eb79e8ef..de2f75e5 100644 --- a/coaster/sqlalchemy/statemanager.py +++ b/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) diff --git a/coaster/utils/classes.py b/coaster/utils/classes.py index 5e75c17e..2fecd0df 100644 --- a/coaster/utils/classes.py +++ b/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): diff --git a/coaster/views/__init__.py b/coaster/views/__init__.py index 8b241dc8..820cc082 100644 --- a/coaster/views/__init__.py +++ b/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 diff --git a/coaster/views/classview.py b/coaster/views/classview.py new file mode 100644 index 00000000..cee7a7c1 --- /dev/null +++ b/coaster/views/classview.py @@ -0,0 +1,519 @@ +# -*- coding: utf-8 -*- + +""" +Class-based views +----------------- + +Group related views into a class for easier management. +""" + +from __future__ import unicode_literals +from functools import wraps, update_wrapper +from sqlalchemy.orm.attributes import InstrumentedAttribute +from sqlalchemy.orm.mapper import Mapper +from sqlalchemy.orm.properties import RelationshipProperty +from werkzeug.routing import parse_rule +from werkzeug.local import LocalProxy +from flask import _request_ctx_stack, has_request_context, request, redirect, make_response +from ..auth import current_auth, add_auth_attribute +from ..sqlalchemy import PermissionMixin + +__all__ = [ + 'route', 'rulejoin', 'current_view', # Functions + 'ClassView', 'ModelView', # View base classes + 'url_change_check', # View decorators + 'UrlForView', 'InstanceLoader', 'UrlChangeCheck' # Mixin classes + ] + +#: A proxy object that holds the currently executing :class:`ClassView` instance, +#: for use in templates as context. Exposed to templates by :func:`coaster.app.init_app`. +current_view = LocalProxy(lambda: has_request_context() and getattr(_request_ctx_stack.top, 'current_view', None)) + + +# :func:`route` wraps :class:`ViewDecorator` so that it can have an independent __doc__ +def route(rule, **options): + """ + Decorator for defining routes on a :class:`ClassView` and its methods. + Accepts the same parameters that Flask's ``app.``:meth:`~flask.Flask.route` + accepts. See :class:`ClassView` for usage notes. + """ + return ViewDecorator(rule, **options) + + +def rulejoin(class_rule, method_rule): + """ + Join class and method rules. Used internally by :class:`ClassView` to + combine rules from the :func:`route` decorators on the class and on the + individual view handler methods:: + + >>> rulejoin('/', '') + '/' + >>> rulejoin('/', 'first') + '/first' + >>> rulejoin('/first', '/second') + '/second' + >>> rulejoin('/first', 'second') + '/first/second' + >>> rulejoin('/first/', 'second') + '/first/second' + >>> rulejoin('/first/', '') + '/first/' + >>> rulejoin('/first/', 'third') + '/first//third' + """ + if method_rule.startswith('/'): + return method_rule + else: + return class_rule + ('' if class_rule.endswith('/') or not method_rule else '/') + method_rule + + +class ViewDecorator(object): + """ + Internal object for :func:`route` decorated view methods. + """ + def __init__(self, rule, **options): + self.routes = [(rule, options)] + + def reroute(self, f): + # Use type(self) instead of ViewDecorator so this works for (future) subclasses of ViewDecorator + r = type(self)('') + r.routes = self.routes + return r.__call__(f) + + def __call__(self, decorated): + # Are we decorating a ClassView? If so, annotate the ClassView and return it + if type(decorated) is type and issubclass(decorated, ClassView): + if '__routes__' not in decorated.__dict__: + decorated.__routes__ = [] + decorated.__routes__.extend(self.routes) + return decorated + + # Are we decorating another ViewDecorator? If so, copy routes and + # wrapped method from it. + elif isinstance(decorated, (ViewDecorator, ViewDecoratorWrapper)): + self.routes.extend(decorated.routes) + self.func = decorated.func + + # If neither ClassView nor ViewDecorator, assume it's a callable method + else: + self.func = decorated + + self.name = self.__name__ = self.func.__name__ + self.endpoint = self.name # This will change once init_app calls __set_name__ + self.__doc__ = self.func.__doc__ + return self + + # Normally Python 3.6+, but called manually by :meth:`ClassView.init_app` + def __set_name__(self, owner, name): + self.name = name + + def __get__(self, obj, cls=None): + return ViewDecoratorWrapper(self, obj, cls) + + def init_app(self, app, cls, callback=None): + """ + Register routes for a given app and ClassView subclass + """ + def view_func(*args, **kwargs): + # Instantiate the view class. We depend on its __init__ requiring no parameters + viewinst = view_func.view_class() + # Place it on the request stack for :obj:`current_view` to discover + _request_ctx_stack.top.current_view = viewinst + # Call the instance's before_request method + viewinst.before_request(view_func.__name__, kwargs) + # Finally, call the view handler method + return viewinst.after_request(make_response(view_func.wrapped_func(viewinst, *args, **kwargs))) + + # Decorate the wrapped view function with the class's desired decorators. + # Mixin classes may provide their own decorators, and all of them will be applied. + wrapped_func = self.func + for base in cls.__mro__: + if '__decorators__' in base.__dict__: + for decorator in base.__dict__['__decorators__']: + wrapped_func = decorator(wrapped_func) + + # Make view_func resemble the underlying view handler method... + view_func = update_wrapper(view_func, wrapped_func) + # ...but give view_func the name of the method in the class (self.name), + # as this is important to the before_request method. self.name will + # differ from __name__ only if the view handler method was defined + # outside the class and then added to the class. + view_func.__name__ = self.name + # Note: While this fixes for external inspection, the wrappers we just + # applied will continue to have self.func.__name__, which will break them + # if they depend on it and if the method was defined outside the class + # with a different name from what it has inside the class. Ideally, + # the programmer should correct for this by always correcting __name__ + # before adding it to the class. See url_change_check below for an + # exampe of how this may happen + + # Stick `wrapped_func` and `cls` into view_func to avoid creating a closure. + view_func.wrapped_func = wrapped_func + view_func.view_class = cls + + # Revisit endpoint to account for subclasses + endpoint = cls.__name__ + '_' + self.name + + for class_rule, class_options in cls.__routes__: + for method_rule, method_options in self.routes: + use_options = dict(method_options) + use_options.update(class_options) + endpoint = use_options.pop('endpoint', endpoint) + use_rule = rulejoin(class_rule, method_rule) + app.add_url_rule(use_rule, endpoint, view_func, **use_options) + if callback: + callback(use_rule, endpoint, view_func, **use_options) + + +class ViewDecoratorWrapper(object): + """Wrapper for a view at runtime""" + def __init__(self, viewd, obj, cls=None): + self.__viewd = viewd + self.__obj = obj + self.__cls = cls + + def __call__(self, *args, **kwargs): + """Treat this like a call to the method (and not to the view)""" + return self.__viewd.func(self.__obj, *args, **kwargs) + + def __getattr__(self, name): + return getattr(self.__viewd, name) + + +class ClassView(object): + """ + Base class for defining a collection of views that are related to each + other. Subclasses may define methods decorated with :func:`route`. When + :meth:`init_app` is called, these will be added as routes to the app. + + Typical use:: + + @route('/') + class IndexView(ClassView): + @route('') + def index(): + return render_template('index.html.jinja2') + + @route('about') + def about(): + return render_template('about.html.jinja2') + + IndexView.init_app(app) + + The :func:`route` decorator on the class specifies the base rule, which is + prefixed to the rule specified on each view method. This example produces + two view handlers, for ``/`` and ``/about``. Multiple :func:`route` + decorators may be used in both places. + + A rudimentary CRUD view collection can be assembled like this:: + + @route('/doc/') + class DocumentView(ClassView): + @route('') + @render_with('mydocument.html.jinja2', json=True) + def view(self, name): + document = MyDocument.query.filter_by(name=name).first_or_404() + return document.current_access() + + @route('edit', methods=['POST']) + @requestform('title', 'content') + def edit(self, name, title, content): + document = MyDocument.query.filter_by(name=name).first_or_404() + document.title = title + document.content = content + return 'edited!' + + DocumentView.init_app(app) + + See :class:`ModelView` for a better way to build views around a model. + """ + # If the class did not get a @route decorator, provide a fallback route + __routes__ = [('', {})] + #: Subclasses may define decorators here. These will be applied to every + #: view handler in the class, but only when called as a view and not + #: as a Python method call. + __decorators__ = [] + + def before_request(self, view, kwargs): + """ + This method is called after the app's ``before_request`` handlers, and + before the class's view method. It receives the name of the view + method, and all keyword arguments that will be sent to the view method. + Subclasses and mixin classes may define their own + :meth:`before_request` to pre-process requests. + + :param str view: The name of the view handler that will be called. The + view itself can be retrieved as ``getattr(self, view)`` + :param dict kwargs: Parameters that will be sent to the view handler. + These are typically parameters from the URL rule + """ + pass + + def after_request(self, response): + """ + This method is called with the response from the view handler method. + It must return a valid response object. Subclasses and mixin classes + may override this to perform any necessary post-processing:: + + class MyView(ClassView): + ... + def after_request(self, response): + response = super(MyView, self).after_request(response) + ... # Process here + return response + + :param response: Response from the view handler method + :return: Response object + """ + return response + + @classmethod + def __get_raw_attr(cls, name): + for base in cls.__mro__: + if name in base.__dict__: + return base.__dict__[name] + raise AttributeError(name) + + @classmethod + def add_route_for(cls, _name, rule, **options): + """ + Add a route for an existing method or view. Useful for modifying routes + that a subclass inherits from a base class:: + + class BaseView(ClassView): + def latent_view(self): + return 'latent-view' + + @route('other') + def other_view(self): + return 'other-view' + + @route('/path') + class SubView(BaseView): + pass + + SubView.add_route_for('latent_view', 'latent') + SubView.add_route_for('other_view', 'another') + SubView.init_app(app) + + # Created routes: + # /path/latent -> SubView.latent (added) + # /path/other -> SubView.other (inherited) + # /path/another -> SubView.other (added) + + :param _name: Name of the method or view on the class + :param rule: URL rule to be added + :param options: Additional options for :meth:`~flask.Flask.add_url_rule` + """ + setattr(cls, _name, route(rule, **options)(cls.__get_raw_attr(_name))) + + @classmethod + def init_app(cls, app, callback=None): + """ + Register views on an app. If :attr:`callback` is specified, it will + be called after ``app.``:meth:`~flask.Flask.add_url_rule`, with the same + parameters. + """ + processed = set() + for base in cls.__mro__: + for name, attr in base.__dict__.items(): + if name in processed: + continue + processed.add(name) + if isinstance(attr, ViewDecorator): + attr.__set_name__(base, name) # Required for Python < 3.6 + attr.init_app(app, cls, callback=callback) + + +class ModelView(ClassView): + """ + Base class for constructing views around a model. Functionality is provided + via mixin classes that must precede :class:`ModelView` in base class order. + Two mixins are provided: :class:`UrlForView` and :class:`InstanceLoader`. + Sample use:: + + @route('/doc/') + class DocumentView(UrlForView, InstanceLoader, ModelView): + model = Document + route_model_map = { + 'document': 'name' + } + + @route('') + @render_with(json=True) + def view(self, **kwargs): + return self.obj.current_access() + + DocumentView.init_app(app) + + Views will receive keyword arguments as in :class:`ClassView`, but + typically will have no use for them if :meth:`loader` works as expected. + """ + + #: The model that this view class represents, to be specified by subclasses. + model = None + #: A base query to use if the model needs special handling. + query = None + + #: A mapping of URL rule variables to attributes on the model. For example, + #: if the URL rule is ``//``, the attribute map can be:: + #: + #: model = MyModel + #: route_model_map = { + #: 'document': 'name', # Map 'document' in URL to MyModel.name + #: 'parent': 'parent.name', # Map 'parent' to MyModel.parent.name + #: } + #: + #: The :class:`InstanceLoader` mixin class will convert this mapping into + #: SQLAlchemy attribute references to load the instance object. + route_model_map = {} + + def loader(self, view, kwargs): # pragma: no cover + """ + Subclasses or mixin classes may override this method to provide a model + instance loader. The return value of this method will be placed at + ``self.obj``. + + :return: Object instance loaded from database + """ + pass + + def before_request(self, view, kwargs): + """ + :class:`ModelView` overrides :meth:`~ClassView.before_request` to call + :meth:`loader`. Subclasses overriding this method must use + :func:`super` to ensure :meth:`loader` is called. + """ + super(ModelView, self).before_request(view, kwargs) + self.obj = self.loader(view, kwargs) + + +class UrlForView(object): + """ + Mixin class for :class:`ModelView` that registers view handler methods with + :class:`~coaster.sqlalchemy.mixins.UrlForMixin`'s + :meth:`~coaster.sqlalchemy.mixins.UrlForMixin.is_url_for`. + """ + @classmethod + def init_app(cls, app, callback=None): + def register_view_on_model(rule, endpoint, view_func, **options): + # Only pass in the attrs that are included in the rule. + # 1. Extract list of variables from the rule + rulevars = (v for c, a, v in parse_rule(rule)) + # Make a subset of cls.route_model_map with the required variables + params = {v: cls.route_model_map[v] for v in rulevars if v in cls.route_model_map} + # Hook up is_url_for with the view function's name, endpoint name and parameters + cls.model.is_url_for(view_func.__name__, endpoint, **params)(view_func) + if callback: # pragma: no cover + callback(rule, endpoint, view_func, **options) + + super(ModelView, cls).init_app(app, callback=register_view_on_model) + + +def url_change_check(f): + """ + View method decorator that checks the URL of the loaded object in + ``self.obj`` against the URL in the request (using + ``self.obj.url_for(__name__)``). If the URLs do not match, + and the request is a ``GET``, it issues a redirect to the correct URL. + Usage:: + + @route('/doc/') + class MyModelView(UrlForView, InstanceLoader, ModelView): + model = MyModel + route_model_map = {'document': 'url_id_name'} + + @route('') + @url_change_check + @render_with(json=True) + def view(self, **kwargs): + return self.obj.current_access() + + If the decorator is required for all view handlers in the class, use + :class:`UrlChangeCheck`. + """ + @wraps(f) + def wrapper(self, *args, **kwargs): + if request.method == 'GET' and self.obj is not None: + correct_url = self.obj.url_for(f.__name__, _external=True) + if correct_url != request.base_url: + if request.query_string: + correct_url = correct_url + '?' + request.query_string.decode() + return redirect(correct_url) # TODO: Decide if this should be 302 (default) or 301 + return f(self, *args, **kwargs) + return wrapper + + +class UrlChangeCheck(UrlForView): + """ + Mixin class for :class:`ModelView` and + :class:`~coaster.sqlalchemy.mixins.UrlForMixin` that applies the + :func:`url_change_check` decorator to all view handler methods. Subclasses + :class:`UrlForView`, which it depends on to register the view with the + model so that URLs can be generated. Usage:: + + @route('/doc/') + class MyModelView(UrlChangeCheck, InstanceLoader, ModelView): + model = MyModel + route_model_map = {'document': 'url_id_name'} + + @route('') + @render_with(json=True) + def view(self, **kwargs): + return self.obj.current_access() + """ + __decorators__ = [url_change_check] + + +class InstanceLoader(object): + """ + Mixin class for :class:`ModelView` that provides a :meth:`loader` that + attempts to load an instance of the model based on attributes in the + :attr:`~ModelView.route_model_map` dictionary. + + :class:`InstanceLoader` will traverse relationships (many-to-one or + one-to-one) and perform a SQL ``JOIN`` with the target class. + """ + def loader(self, view, kwargs): + if any((name in self.route_model_map for name in kwargs)): + # We have a URL route attribute that matches one of the model's attributes. + # Attempt to load the model instance + filters = {self.route_model_map[key]: value + for key, value in kwargs.items() + if key in self.route_model_map} + + query = self.query or self.model.query + joined_models = set() + for name, value in filters.items(): + if '.' in name: + # Did we get something like `parent.name`? + # Dig into it to find the source column + source = self.model + for subname in name.split('.'): + source = getattr(source, subname) + # Did we get to something like 'parent'? If it's a relationship, find + # the source class, join it to the query, and then continue looking for + # attributes over there + if isinstance(source, InstrumentedAttribute): + if isinstance(source.property, RelationshipProperty): + if isinstance(source.property.argument, Mapper): + source = source.property.argument.class_ # Unlikely to be used. pragma: no cover + else: + source = source.property.argument + if source not in joined_models: + # SQL JOIN the other model + query = query.join(source) + # But ensure we don't JOIN twice + joined_models.add(source) + query = query.filter(source == value) + else: + query = query.filter(getattr(self.model, name) == value) + obj = query.one_or_404() + # Determine permissions available on the object for the current actor, + # but only if the view method has a requires_permission decorator + if hasattr(getattr(self, view).func, 'requires_permission') and isinstance(obj, PermissionMixin): + perms = obj.current_permissions + if hasattr(current_auth, 'permissions'): + perms = perms | current_auth.permissions + add_auth_attribute('permissions', perms) + return obj diff --git a/coaster/views/decorators.py b/coaster/views/decorators.py index 9751de83..2db33b6a 100644 --- a/coaster/views/decorators.py +++ b/coaster/views/decorators.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,15 +406,12 @@ 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 = {} @@ -409,6 +419,8 @@ def myview(): 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") @@ -416,7 +428,7 @@ def myview(): 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 diff --git a/docs/conf.py b/docs/conf.py index adfaa982..ae971288 100644 --- a/docs/conf.py +++ b/docs/conf.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, } diff --git a/docs/views/classview.rst b/docs/views/classview.rst new file mode 100644 index 00000000..0e81f7c5 --- /dev/null +++ b/docs/views/classview.rst @@ -0,0 +1,2 @@ +.. automodule:: coaster.views.classview + :members: diff --git a/docs/views/index.rst b/docs/views/index.rst index 770e0c93..227fc3b9 100644 --- a/docs/views/index.rst +++ b/docs/views/index.rst @@ -5,3 +5,4 @@ misc decorators + classview diff --git a/tests/test_models.py b/tests/test_models.py index b0810185..86ea4075 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -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') diff --git a/tests/test_views.py b/tests/test_views.py index 11eb7338..6d4e8f30 100644 --- a/tests/test_views.py +++ b/tests/test_views.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' diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py new file mode 100644 index 00000000..9f7da289 --- /dev/null +++ b/tests/test_views_classview.py @@ -0,0 +1,427 @@ +# -*- coding: utf-8 -*- + +from __future__ import absolute_import, unicode_literals + +import unittest +from flask import Flask, json +from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin, BaseIdNameMixin +from coaster.auth import add_auth_attribute +from coaster.utils import InspectableSet +from coaster.db import SQLAlchemy +from coaster.views import (ClassView, ModelView, UrlForView, UrlChangeCheck, InstanceLoader, + route, requestargs, requestform, render_with, current_view, requires_permission) + + +app = Flask(__name__) +app.testing = True +app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite://' +app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False +db = SQLAlchemy(app) + + +# --- Models ------------------------------------------------------------------ + +class ViewDocument(BaseNameMixin, db.Model): + __tablename__ = 'view_document' + __roles__ = { + 'all': { + 'read': {'name', 'title'} + } + } + + def permissions(self, user, inherited=()): + perms = super(ViewDocument, self).permissions(user, inherited) + if user == 'this-is-the-owner': # Our hack of a user object, for testing + perms.add('edit') + perms.add('delete') + return perms + + +class ScopedViewDocument(BaseScopedNameMixin, db.Model): + __tablename__ = 'scoped_view_document' + parent_id = db.Column(None, db.ForeignKey('view_document.id'), nullable=False) + parent = db.relationship(ViewDocument, backref=db.backref('children', cascade='all, delete-orphan')) + + __roles__ = { + 'all': { + 'read': {'name', 'title', 'doctype'} + } + } + + @property + def doctype(self): + return 'scoped-doc' + + +class RenameableDocument(BaseIdNameMixin, db.Model): + __tablename__ = 'renameable_document' + __uuid_primary_key__ = False # So that we can get consistent `1-` url_name in tests + __roles__ = { + 'all': { + 'read': {'name', 'title'} + } + } + + +# --- Views ------------------------------------------------------------------- + +@route('/') +class IndexView(ClassView): + @route('') + def index(self): + return 'index' + + @route('page') + def page(self): + return 'page' + + @route('current_view') + def current_view_is_self(self): + return str(current_view == self) + +IndexView.init_app(app) + + +@route('/doc/') +class DocumentView(ClassView): + @route('') + @render_with(json=True) + def view(self, name): + document = ViewDocument.query.filter_by(name=name).first_or_404() + return document.current_access() + + @route('edit', methods=['POST']) # Maps to /doc//edit + @route('/edit/', methods=['POST']) # Maps to /edit/ + @route('', methods=['POST']) # Maps to /doc/ + @requestform('title') + def edit(self, name, title): + document = ViewDocument.query.filter_by(name=name).first_or_404() + document.title = title + return 'edited!' + +DocumentView.init_app(app) + + +class BaseView(ClassView): + @route('') + def first(self): + return 'first' + + @route('second') + def second(self): + return 'second' + + @route('third') + def third(self): + return 'third' + + @route('inherited') + def inherited(self): + return 'inherited' + + @route('also-inherited') + def also_inherited(self): + return 'also_inherited' + + def latent_route(self): + return 'latent-route' + + +@route('/subclasstest') +class SubView(BaseView): + @BaseView.first.reroute + def first(self): + return 'rerouted-first' + + @route('2') + @BaseView.second.reroute + def second(self): + return 'rerouted-second' + + def third(self): + return 'removed-third' + +SubView.add_route_for('also_inherited', '/inherited') +SubView.add_route_for('also_inherited', 'inherited2') +SubView.add_route_for('latent_route', 'latent') +SubView.init_app(app) + + +@route('/secondsub') +class AnotherSubView(BaseView): + @route('2-2') + @BaseView.second.reroute + def second(self): + return 'also-rerouted-second' + +AnotherSubView.init_app(app) + + +@route('/model/') +class ModelDocumentView(UrlForView, InstanceLoader, ModelView): + model = ViewDocument + route_model_map = { + 'document': 'name', + } + + @requestargs('access_token') + def before_request(self, view, kwargs, access_token=None): + if access_token == 'owner-admin-secret': + add_auth_attribute('permissions', InspectableSet({'siteadmin'})) + add_auth_attribute('user', 'this-is-the-owner') # See ViewDocument.permissions + if access_token == 'owner-secret': + add_auth_attribute('user', 'this-is-the-owner') # See ViewDocument.permissions + return super(ModelDocumentView, self).before_request(view, kwargs) + + @route('') + @render_with(json=True) + def view(self, **kwargs): + return self.obj.current_access() + + @route('edit', methods=['GET', 'POST']) + @route('', methods=['PUT']) + @requires_permission('edit') + def edit(self, **kwargs): + return 'edit-called' + + +ModelDocumentView.init_app(app) + + +@route('/model//') +class ScopedDocumentView(ModelDocumentView): + model = ScopedViewDocument + route_model_map = { + 'document': 'name', + 'parent': 'parent.name', + } + +ScopedDocumentView.init_app(app) + + +@route('/rename/') +class RenameableDocumentView(UrlChangeCheck, InstanceLoader, ModelView): + model = RenameableDocument + route_model_map = { + 'document': 'url_name', + } + + @route('') + @render_with(json=True) + def view(self, **kwargs): + return self.obj.current_access() + +RenameableDocumentView.init_app(app) + + +# --- Tests ------------------------------------------------------------------- + +class TestClassView(unittest.TestCase): + app = app + + def setUp(self): + self.ctx = self.app.test_request_context() + self.ctx.push() + db.create_all() + self.session = db.session + self.client = self.app.test_client() + + def tearDown(self): + self.session.rollback() + db.drop_all() + self.ctx.pop() + + def test_index(self): + """Test index view (/)""" + rv = self.client.get('/') + assert rv.data == b'index' + + def test_page(self): + """Test page view (/page)""" + rv = self.client.get('/page') + assert rv.data == b'page' + + def test_current_view(self): + rv = self.client.get('/current_view') + assert rv.data == b'True' + + def test_document_404(self): + """Test 404 response from within a view""" + rv = self.client.get('/doc/this-doc-does-not-exist') + assert rv.status_code == 404 # This 404 came from DocumentView.view + + def test_document_view(self): + """Test document view (loaded from database)""" + doc = ViewDocument(name='test1', title="Test") + self.session.add(doc) + self.session.commit() + + rv = self.client.get('/doc/test1') + assert rv.status_code == 200 + data = json.loads(rv.data) + assert data['name'] == 'test1' + assert data['title'] == "Test" + + def test_document_edit(self): + """POST handler shares URL with GET handler but is routed to correctly""" + doc = ViewDocument(name='test1', title="Test") + self.session.add(doc) + self.session.commit() + + self.client.post('/doc/test1/edit', data={'title': "Edit 1"}) + assert doc.title == "Edit 1" + self.client.post('/edit/test1', data={'title': "Edit 2"}) + assert doc.title == "Edit 2" + self.client.post('/doc/test1', data={'title': "Edit 3"}) + assert doc.title == "Edit 3" + + def test_callable_view(self): + """View handlers are callable as regular methods""" + doc = ViewDocument(name='test1', title="Test") + self.session.add(doc) + self.session.commit() + + rv = DocumentView().view('test1') + assert rv.status_code == 200 + data = json.loads(rv.data) + assert data['name'] == 'test1' + assert data['title'] == "Test" + + rv = DocumentView().edit('test1', "Edited") + assert rv == 'edited!' + assert doc.title == "Edited" + + def test_rerouted(self): + """Subclass replaces view handler""" + rv = self.client.get('/subclasstest') + assert rv.data != b'first' + assert rv.data == b'rerouted-first' + assert rv.status_code == 200 + + def test_rerouted_with_new_routes(self): + """Subclass replaces view handler and adds new routes""" + rv = self.client.get('/subclasstest/second') + assert rv.data != b'second' + assert rv.data == b'rerouted-second' + assert rv.status_code == 200 + rv = self.client.get('/subclasstest/2') + assert rv.data != b'second' + assert rv.data == b'rerouted-second' + assert rv.status_code == 200 + + def test_unrouted(self): + """Subclass removes a route from base class""" + rv = self.client.get('/subclasstest/third') + assert rv.data != b'third' + assert rv.data != b'unrouted-third' + assert rv.status_code == 404 + + def test_inherited(self): + """Subclass inherits a view from the base class without modifying it""" + rv = self.client.get('/subclasstest/inherited') + assert rv.data == b'inherited' + assert rv.status_code == 200 + + def test_added_routes(self): + """Subclass adds more routes to a base class's view handler""" + rv = self.client.get('/subclasstest/also-inherited') # From base class + assert rv.data == b'also_inherited' + rv = self.client.get('/subclasstest/inherited2') # Added in sub class + assert rv.data == b'also_inherited' + rv = self.client.get('/inherited') # Added in sub class + assert rv.data == b'also_inherited' + rv = self.client.get('/subclasstest/latent') + assert rv.data == b'latent-route' + + def test_cant_route_missing_method(self): + """Routes can't be added for missing attributes""" + with self.assertRaises(AttributeError): + SubView.add_route_for('this_method_does_not_exist', '/missing') + + def test_second_subview_reroute(self): + """Using reroute does not mutate the base class""" + rv = self.client.get('/secondsub/second') + assert rv.data != b'second' + assert rv.data == b'also-rerouted-second' + assert rv.status_code == 200 + rv = self.client.get('/secondsub/2-2') + assert rv.data != b'second' + assert rv.data == b'also-rerouted-second' + assert rv.status_code == 200 + # Confirm we did not accidentally acquire this from SubView's use of reroute + rv = self.client.get('/secondsub/2') + assert rv.status_code == 404 + + def test_modelview_instanceloader_view(self): + """Test document view in ModelView with InstanceLoader""" + doc = ViewDocument(name='test1', title="Test") + self.session.add(doc) + self.session.commit() + + rv = self.client.get('/model/test1') + assert rv.status_code == 200 + data = json.loads(rv.data) + assert data['name'] == 'test1' + assert data['title'] == "Test" + + def test_modelview_instanceloader_requires_permission_edit(self): + """Test document edit in ModelView with InstanceLoader and requires_permission""" + doc = ViewDocument(name='test1', title="Test") + self.session.add(doc) + self.session.commit() + + rv = self.client.post('/model/test1/edit') + assert rv.status_code == 403 + rv = self.client.post('/model/test1/edit?access_token=owner-secret') + assert rv.status_code == 200 + assert rv.data == b'edit-called' + rv = self.client.post('/model/test1/edit?access_token=owner-admin-secret') + assert rv.status_code == 200 + assert rv.data == b'edit-called' + + def test_modelview_url_for(self): + """Test that ModelView provides model.is_url_for with appropriate parameters""" + doc1 = ViewDocument(name='test1', title="Test 1") + doc2 = ViewDocument(name='test2', title="Test 2") + + assert doc1.url_for('view') == '/model/test1' + assert doc2.url_for('view') == '/model/test2' + + def test_scopedmodelview_view(self): + """Test that InstanceLoader in a scoped model correctly loads parent""" + doc = ViewDocument(name='test1', title="Test 1") + sdoc = ScopedViewDocument(name='test2', title="Test 2", parent=doc) + self.session.add_all([doc, sdoc]) + self.session.commit() + + rv = self.client.get('/model/test1/test2') + assert rv.status_code == 200 + data = json.loads(rv.data) + assert data['name'] == 'test2' + assert data['doctype'] == 'scoped-doc' + + # The joined load actually worked + rv = self.client.get('/model/this-doc-does-not-exist/test2') + assert rv.status_code == 404 + + def test_redirectablemodel_view(self): + doc = RenameableDocument(name='test1', title="Test 1") + self.session.add(doc) + self.session.commit() + + rv = self.client.get('/rename/1-test1') + assert rv.status_code == 200 + data = json.loads(rv.data) + assert data['name'] == 'test1' + + doc.name = 'renamed' + self.session.commit() + + rv = self.client.get('/rename/1-test1?preserve=this') + assert rv.status_code == 302 + assert rv.location == 'http://localhost/rename/1-renamed?preserve=this' + + rv = self.client.get('/rename/1-renamed') + assert rv.status_code == 200 + data = json.loads(rv.data) + assert data['name'] == 'renamed' diff --git a/tests/test_loadmodels.py b/tests/test_views_loadmodels.py similarity index 100% rename from tests/test_loadmodels.py rename to tests/test_views_loadmodels.py diff --git a/tests/test_render_with.py b/tests/test_views_renderwith.py similarity index 100% rename from tests/test_render_with.py rename to tests/test_views_renderwith.py