Skip to content

Commit

Permalink
Specify ClassVar for all mixin classvars
Browse files Browse the repository at this point in the history
  • Loading branch information
jace committed May 28, 2023
1 parent 7746a36 commit 7024fe4
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 39 deletions.
1 change: 1 addition & 0 deletions src/coaster/sqlalchemy/annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def _configure_annotations(_mapper: t.Any, cls: t.Type) -> None:
):
data = attr.column._coaster_annotations
elif hasattr(attr, '_coaster_annotations'):
# pylint: disable=protected-access
data = attr._coaster_annotations
elif isinstance(
attr, (QueryableAttribute, RelationshipProperty, MapperProperty)
Expand Down
67 changes: 34 additions & 33 deletions src/coaster/sqlalchemy/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class MyModel(BaseMixin, db.Model):
import typing as t

from flask import Flask, current_app, url_for
from sqlalchemy import event
from sqlalchemy.ext.hybrid import hybrid_property
from sqlalchemy.orm import Mapped, declarative_mixin, declared_attr, synonym
from sqlalchemy.sql import func, select
Expand Down Expand Up @@ -98,10 +99,9 @@ class MyModel(IdMixin, db.Model):

query_class: t.ClassVar[t.Type[Query]] = Query
query: t.ClassVar[QueryProperty]
__column_annotations__: dict
#: Use UUID primary key? If yes, UUIDs are automatically generated without
#: the need to commit to the database
__uuid_primary_key__ = False
__uuid_primary_key__: t.ClassVar[bool] = False

@immutable
@declared_attr
Expand Down Expand Up @@ -172,6 +172,8 @@ class UuidMixin:
code.
"""

__uuid_primary_key__: t.ClassVar[bool]

@immutable
@with_roles(read={'all'})
@declared_attr
Expand Down Expand Up @@ -240,7 +242,6 @@ class TimestampMixin:
query_class: t.ClassVar[t.Type[Query]] = Query
query: t.ClassVar[QueryProperty]
__with_timezone__: t.ClassVar[bool] = False
__column_annotations__: t.ClassVar[dict]

@immutable
@declared_attr
Expand Down Expand Up @@ -375,11 +376,13 @@ class UrlForMixin:
#: strings. The same action can point to different endpoints in different apps. The
#: app may also be None as fallback. Each subclass will get its own dictionary.
#: This particular dictionary is only used as an inherited fallback.
url_for_endpoints: t.Dict[t.Optional[Flask], t.Dict[str, UrlEndpointData]] = {
None: {}
}
url_for_endpoints: t.ClassVar[
t.Dict[t.Optional[Flask], t.Dict[str, UrlEndpointData]]
] = {None: {}}
#: Mapping of {app: {action: (classview, attr)}}
view_for_endpoints: t.Dict[t.Optional[Flask], t.Dict[str, t.Tuple[t.Any, str]]] = {}
view_for_endpoints: t.ClassVar[
t.Dict[t.Optional[Flask], t.Dict[str, t.Tuple[t.Any, str]]]
] = {}

#: Dictionary of URLs available on this object
urls = UrlDictStub()
Expand Down Expand Up @@ -578,12 +581,12 @@ class BaseNameMixin(BaseMixin):
"""

#: Prevent use of these reserved names
reserved_names: t.Collection[str] = []
reserved_names: t.ClassVar[t.Collection[str]] = []
#: Allow blank names after all?
__name_blank_allowed__ = False
__name_blank_allowed__: t.ClassVar[bool] = False
#: How long are names and title allowed to be? `None` for unlimited length
__name_length__: t.Optional[int] = 250
__title_length__: t.Optional[int] = 250
__name_length__: t.ClassVar[t.Optional[int]] = 250
__title_length__: t.ClassVar[t.Optional[int]] = 250

@declared_attr
def name(cls) -> Mapped[str]:
Expand Down Expand Up @@ -718,15 +721,15 @@ class Event(BaseScopedNameMixin, db.Model):
"""

#: Prevent use of these reserved names
reserved_names: t.Collection[str] = []
reserved_names: t.ClassVar[t.Collection[str]] = []
#: Allow blank names after all?
__name_blank_allowed__ = False
__name_blank_allowed__: t.ClassVar[bool] = False
#: How long are names and title allowed to be? `None` for unlimited length
__name_length__: t.Optional[int] = 250
__title_length__: t.Optional[int] = 250
__name_length__: t.ClassVar[t.Optional[int]] = 250
__title_length__: t.ClassVar[t.Optional[int]] = 250

#: Specify expected type for a 'parent' attr
parent: Mapped[t.Any]
parent: t.Any

@declared_attr
def name(cls) -> Mapped[str]:
Expand Down Expand Up @@ -880,10 +883,10 @@ class BaseIdNameMixin(BaseMixin):
"""

#: Allow blank names after all?
__name_blank_allowed__ = False
__name_blank_allowed__: t.ClassVar[bool] = False
#: How long are names and title allowed to be? `None` for unlimited length
__name_length__: t.Optional[int] = 250
__title_length__: t.Optional[int] = 250
__name_length__: t.ClassVar[t.Optional[int]] = 250
__title_length__: t.ClassVar[t.Optional[int]] = 250

@declared_attr
def name(cls) -> Mapped[str]:
Expand Down Expand Up @@ -986,7 +989,7 @@ class Issue(BaseScopedIdMixin, db.Model):
"""

#: Specify expected type for a 'parent' attr
parent: Mapped[t.Any]
parent: t.Any

# FIXME: Rename this to `scoped_id` and provide a migration guide.
@with_roles(read={'all'})
Expand Down Expand Up @@ -1068,10 +1071,10 @@ class Event(BaseScopedIdNameMixin, db.Model):
"""

#: Allow blank names after all?
__name_blank_allowed__ = False
__name_blank_allowed__: t.ClassVar[bool] = False
#: How long are names and title allowed to be? `None` for unlimited length
__name_length__: t.Optional[int] = 250
__title_length__: t.Optional[int] = 250
__name_length__: t.ClassVar[t.Optional[int]] = 250
__title_length__: t.ClassVar[t.Optional[int]] = 250

@declared_attr
def name(cls) -> Mapped[str]:
Expand Down Expand Up @@ -1220,10 +1223,8 @@ def _configure_uuid_listener(mapper: t.Any, class_: UuidMixin) -> None:
auto_init_default(mapper.column_attrs.uuid)


sa.event.listen(IdMixin, 'mapper_configured', _configure_id_listener, propagate=True)
sa.event.listen(
UuidMixin, 'mapper_configured', _configure_uuid_listener, propagate=True
)
event.listen(IdMixin, 'mapper_configured', _configure_id_listener, propagate=True)
event.listen(UuidMixin, 'mapper_configured', _configure_uuid_listener, propagate=True)


# Populate name and url_id columns
Expand All @@ -1246,9 +1247,9 @@ def _make_scoped_id(
target.make_id() # type: ignore[unreachable]


sa.event.listen(BaseNameMixin, 'before_insert', _make_name, propagate=True)
sa.event.listen(BaseIdNameMixin, 'before_insert', _make_name, propagate=True)
sa.event.listen(BaseScopedIdMixin, 'before_insert', _make_scoped_id, propagate=True)
sa.event.listen(BaseScopedNameMixin, 'before_insert', _make_scoped_name, propagate=True)
sa.event.listen(BaseScopedIdNameMixin, 'before_insert', _make_scoped_id, propagate=True)
sa.event.listen(BaseScopedIdNameMixin, 'before_insert', _make_name, propagate=True)
event.listen(BaseNameMixin, 'before_insert', _make_name, propagate=True)
event.listen(BaseIdNameMixin, 'before_insert', _make_name, propagate=True)
event.listen(BaseScopedIdMixin, 'before_insert', _make_scoped_id, propagate=True)
event.listen(BaseScopedNameMixin, 'before_insert', _make_scoped_name, propagate=True)
event.listen(BaseScopedIdNameMixin, 'before_insert', _make_scoped_id, propagate=True)
event.listen(BaseScopedIdNameMixin, 'before_insert', _make_name, propagate=True)
5 changes: 3 additions & 2 deletions src/coaster/sqlalchemy/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,10 @@ class ModelBase:
metadata: t.ClassVar[sa.MetaData]
query_class: t.ClassVar[type[Query]] = Query
query: t.ClassVar[QueryProperty] = QueryProperty()
# Added by Coaster annotations
__column_annotations__: t.ClassVar[t.Dict[str, t.List[str]]]
__column_annotations_by_attr__: t.ClassVar[t.Dict[str, t.List[str]]]

# FIXME: Drop bind_key arg, Mypy cannot understand it when there's a missing import,
# including in pre-commit checks within this repository itself
def __init_subclass__(cls) -> None:
"""Configure a declarative base class."""
if ModelBase in cls.__bases__:
Expand Down
26 changes: 22 additions & 4 deletions tests/coaster_tests/sqlalchemy_annotations_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from coaster.sqlalchemy import (
BaseMixin,
ImmutableColumnError,
ModelBase,
UuidMixin,
cached,
immutable,
Expand Down Expand Up @@ -134,22 +135,39 @@ def test_has_annotations() -> None:
def test_annotation_in_annotations() -> None:
"""Annotations were discovered."""
for model in (IdOnly, IdUuid, UuidOnly):
assert issubclass(model, ModelBase)
for annotation in (immutable, cached):
assert annotation.__name__ in model.__column_annotations__
assert (
annotation.__name__
in model.__column_annotations__ # type: ignore[attr-defined]
)


def test_attr_in_annotations() -> None:
"""Annotated attributes were discovered and documented."""
for model in (IdOnly, IdUuid, UuidOnly):
assert 'is_immutable' in model.__column_annotations__['immutable']
assert 'is_cached' in model.__column_annotations__['cached']
assert issubclass(model, ModelBase)
assert (
'is_immutable'
in model.__column_annotations__['immutable'] # type: ignore[attr-defined]
)
assert (
'is_cached'
in model.__column_annotations__['cached'] # type: ignore[attr-defined]
)


def test_base_attrs_in_annotations() -> None:
"""Annotations in the base class were also discovered and added to subclass."""
for model in (IdOnly, IdUuid, UuidOnly):
assert issubclass(model, ModelBase)
for attr in ('created_at', 'id'):
assert attr in model.__column_annotations__['immutable']
assert (
attr
in model.__column_annotations__[ # type: ignore[attr-defined]
'immutable'
]
)
assert 'uuid' in IdUuid.__column_annotations__['immutable']


Expand Down

0 comments on commit 7024fe4

Please sign in to comment.