Skip to content

Commit

Permalink
Redo unused reroute and add_route_for
Browse files Browse the repository at this point in the history
* `reroute` is renamed to `replace`
* `ClassView.add_route_for` is removed in favour of `ViewMethod.with_route` for better type hinting support and to allow the entire class to be within one block
  • Loading branch information
jace committed Dec 12, 2023
1 parent e2d416c commit e2c58c0
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 80 deletions.
105 changes: 52 additions & 53 deletions src/coaster/views/classview.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import typing_extensions as te
import warnings
from functools import partial, update_wrapper, wraps
from inspect import getattr_static, iscoroutinefunction
from inspect import iscoroutinefunction
from typing import cast, overload

from flask import abort, make_response, redirect, request
Expand Down Expand Up @@ -314,26 +314,67 @@ def __init__(
# self.__name__ and self.default_endpoint will change in __set_name__
self.__name__ = self.default_endpoint = func.__name__

def reroute(
def replace(
self,
__f: t.Union[
ViewMethod[_P2, _R2_co], MethodProtocol[te.Concatenate[t.Any, _P2], _R2_co]
],
) -> ViewMethod[_P2, _R2_co]:
"""Replace a view handler in a subclass while keeping its URL route rules."""
"""
Replace a view method in a subclass while keeping its URL routes.
To be used when a subclass needs a custom implementation::
class CrudView:
@route('delete', methods=['GET', 'POST'])
def delete(self): ...
@route('/<doc>')
class MyModelView(CrudView, ModelView[MyModel]):
@route('remove', methods=['GET', 'POST']) # Add another route
@CrudView.delete.replace # Keep the existing 'delete' route
def delete(self):
super().delete() # Call into base class's implementation if needed
"""
cls = cast(t.Type[ViewMethod], self.__class__)
r: ViewMethod[_P2, _R2_co] = cls(__f, data=self.data)
r.routes = self.routes
return r

def copy_for_subclass(self) -> te.Self:
def copy(self) -> te.Self:
"""Make a copy of this ViewMethod, for use in a subclass."""
# Like reroute, but a copy without a new function body
r = self.__class__(self.__func__, data=self.data)
r.routes = self.routes
# Don't copy decorated_func, as it will be re-wrapped by init_app / __set_name__
r.default_endpoint = self.default_endpoint
return r
return self.__class__(self)

def with_route(self, rule: str, **options: t.Any) -> te.Self:
"""
Make a copy of this ViewMethod with an additional route.
To be used when a subclass needs an additional route, but doesn't need a new
implementation. This can also be used to provide a base implementation with no
routes::
class CrudView:
@route('delete', methods=['GET', 'POST'])
def delete(self): ...
@viewdata() # This creates a ViewMethod with no routes
def latent(self): ...
@route('/<doc>')
class MyModelView(CrudView, ModelView[MyModel]):
delete = CrudView.delete.with_route('remove', methods=['GET', 'POST'])
latent = CrudView.latent.with_route('latent')
"""
return self.__class__(self, rule=rule, rule_options=options)

def with_data(self, **data: t.Any) -> te.Self:
"""
Make a copy of this ViewMethod with additional data.
See :meth:`with_route` for usage notes. This method adds or replaces data
instead of adding a URL route.
"""
return self.__class__(self, data=data)

def __set_name__(self, owner: t.Type, name: str) -> None:
# `name` is almost always the existing value acquired from decorated.__name__,
Expand Down Expand Up @@ -730,48 +771,6 @@ def is_available(self) -> bool:
getattr(self, _v).is_available() for _v in self.__views__
)

@classmethod
def add_route_for(cls, __name: str, /, rule: str, **options) -> None:
"""
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`
"""
attr = getattr_static(cls, __name)
# getattr_static raises AttributeError if no default is provided, but the
# documentation is unclear about this:
# https://docs.python.org/3/library/inspect.html#inspect.getattr_static
view_method = ViewMethod(attr, rule=rule, rule_options=options)
setattr(cls, __name, view_method)
view_method.__set_name__(cls, __name)
if __name not in cls.__views__:
cls.__views__ = frozenset(cls.__views__) | {__name}

def __init_subclass__(cls) -> None:
"""Copy views from base classes into the subclass."""
view_names = set()
Expand All @@ -790,7 +789,7 @@ def __init_subclass__(cls) -> None:
# base class is not a mixin, it may be a conflicting route.
# init_app on both classes will be called in the future, so it
# needs a guard condition there. How will the guard work?
attr = attr.copy_for_subclass()
attr = attr.copy()
setattr(cls, name, attr)
attr.__set_name__(cls, name)
view_names.add(name)
Expand Down
51 changes: 24 additions & 27 deletions tests/coaster_tests/views_classview_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ def inherited(self) -> str:
def also_inherited(self) -> str:
return 'also_inherited'

@viewdata()
def latent_route(self) -> str:
return 'latent-route'

Expand All @@ -204,24 +205,26 @@ class SubView(BaseView):
"""Test subclass of a ClassView."""

@viewdata(title="Still first")
@BaseView.first.reroute
@BaseView.first.replace
def first(self):
return 'rerouted-first'
return 'replaced-first'

# Mypy can't process this, but Pyright can. Appears to be a Mypy bug
@route('2') # type: ignore[arg-type]
@BaseView.second.reroute
@BaseView.second.replace
@viewdata(title="Not still second")
def second(self):
return 'rerouted-second'
return 'replaced-second'

def third(self):
return 'removed-third'

also_inherited = BaseView.also_inherited.with_route('/inherited').with_route(
'inherited2', endpoint='just_also_inherited'
)
latent_route = BaseView.latent_route.with_route('latent')


SubView.add_route_for('also_inherited', '/inherited')
SubView.add_route_for('also_inherited', 'inherited2', endpoint='just_also_inherited')
SubView.add_route_for('latent_route', 'latent')
SubView.init_app(app)


Expand All @@ -230,9 +233,9 @@ class AnotherSubView(BaseView):
"""Test second subclass of a ClassView."""

@route('2-2')
@BaseView.second.reroute
@BaseView.second.replace
def second(self):
return 'also-rerouted-second'
return 'also-replaced-second'


AnotherSubView.init_app(app)
Expand Down Expand Up @@ -487,22 +490,22 @@ def test_callable_view(self) -> None:
assert rv == 'edited!'
assert doc.title == "Edited"

def test_rerouted(self) -> None:
def test_replaced(self) -> None:
"""Subclass replaces view method."""
rv = self.client.get('/subclasstest')
assert rv.data != b'first'
assert rv.data == b'rerouted-first'
assert rv.data == b'replaced-first'
assert rv.status_code == 200

def test_rerouted_with_new_routes(self) -> None:
def test_replaced_with_new_routes(self) -> None:
"""Subclass replaces view method and adds new routes."""
rv = self.client.get('/subclasstest/second')
assert rv.data != b'second'
assert rv.data == b'rerouted-second'
assert rv.data == b'replaced-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.data == b'replaced-second'
assert rv.status_code == 200

def test_unrouted(self) -> None:
Expand All @@ -529,22 +532,17 @@ def test_added_routes(self) -> None:
rv = self.client.get('/subclasstest/latent')
assert rv.data == b'latent-route'

def test_cant_route_missing_method(self) -> None:
"""Routes can't be added for missing attributes."""
with pytest.raises(AttributeError):
SubView.add_route_for('this_method_does_not_exist', '/missing')

def test_second_subview_reroute(self) -> None:
"""Using reroute does not mutate the base class."""
def test_second_subview_replace(self) -> None:
"""Using replace 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.data == b'also-replaced-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.data == b'also-replaced-second'
assert rv.status_code == 200
# Confirm we did not accidentally acquire this from SubView's use of reroute
# Confirm we did not accidentally acquire this from SubView's use of replace
rv = self.client.get('/secondsub/2')
assert rv.status_code == 404

Expand Down Expand Up @@ -572,9 +570,8 @@ def test_viewdata(self) -> None:
assert BaseView.first.data['title'] == "First"
assert BaseView.second.data['title'] == "Second"
assert SubView.first.data['title'] == "Still first"
assert (
SubView.second.data['title'] != "Not still second"
) # Reroute took priority
# Replacement took priority
assert SubView.second.data['title'] != "Not still second"
assert SubView.second.data['title'] == "Second"

def test_viewlist(self) -> None:
Expand Down

0 comments on commit e2c58c0

Please sign in to comment.