diff --git a/tildes/requirements-dev.txt b/tildes/requirements-dev.txt index f191be2..9be1033 100644 --- a/tildes/requirements-dev.txt +++ b/tildes/requirements-dev.txt @@ -30,7 +30,7 @@ jinja2==2.10.3 # via pyramid-jinja2 lazy-object-proxy==1.4.3 # via astroid mako==1.1.0 # via alembic, pyramid-mako markupsafe==1.1.1 # via jinja2, mako, pyramid-jinja2 -marshmallow==2.20.5 +marshmallow==3.3.0 mccabe==0.6.1 # via prospector, pylint more-itertools==8.0.2 # via pytest, zipp mypy-extensions==0.4.3 # via mypy @@ -108,7 +108,7 @@ urllib3==1.25.7 # via requests, sentry-sdk venusian==3.0.0 # via cornice, pyramid waitress==1.3.1 # via webtest wcwidth==0.1.7 # via prompt-toolkit, pytest -webargs==4.4.1 +webargs==5.5.2 webassets==0.12.1 # via pyramid-webassets webencodings==0.5.1 # via bleach, html5lib webob==1.8.5 # via pyramid, webtest diff --git a/tildes/requirements.in b/tildes/requirements.in index 590d341..3bfa8a4 100644 --- a/tildes/requirements.in +++ b/tildes/requirements.in @@ -9,7 +9,7 @@ cornice gunicorn html5lib ipython -marshmallow<3.0 # 3.0+ requires significant updates +marshmallow Pillow pip-tools prometheus-client @@ -35,6 +35,6 @@ SQLAlchemy SQLAlchemy-Utils stripe titlecase -webargs<5.0 # 5.0.0 breaks many views, will require significant updates +webargs wrapt zope.sqlalchemy diff --git a/tildes/requirements.txt b/tildes/requirements.txt index 7b7759f..09fb105 100644 --- a/tildes/requirements.txt +++ b/tildes/requirements.txt @@ -21,7 +21,7 @@ jedi==0.15.1 # via ipython jinja2==2.10.3 # via pyramid-jinja2 mako==1.1.0 # via alembic markupsafe==1.1.1 # via jinja2, mako, pyramid-jinja2 -marshmallow==2.20.5 +marshmallow==3.3.0 parso==0.5.1 # via jedi pastedeploy==2.0.1 # via plaster-pastedeploy pexpect==4.7.0 # via ipython @@ -65,7 +65,7 @@ translationstring==1.3 # via pyramid urllib3==1.25.7 # via requests, sentry-sdk venusian==3.0.0 # via cornice, pyramid wcwidth==0.1.7 # via prompt-toolkit -webargs==4.4.1 +webargs==5.5.2 webassets==0.12.1 # via pyramid-webassets webencodings==0.5.1 # via bleach, html5lib webob==1.8.5 # via pyramid diff --git a/tildes/tests/test_markdown_field.py b/tildes/tests/test_markdown_field.py index bfaf34d..753f84c 100644 --- a/tildes/tests/test_markdown_field.py +++ b/tildes/tests/test_markdown_field.py @@ -15,7 +15,7 @@ class MarkdownFieldTestSchema(Schema): def validate_string(string): """Validate a string against a standard Markdown field.""" - MarkdownFieldTestSchema(strict=True).validate({"markdown": string}) + MarkdownFieldTestSchema().load({"markdown": string}) def test_normal_text_validates(): @@ -61,7 +61,7 @@ def test_carriage_returns_stripped(): """Ensure loading a value strips out carriage returns from the string.""" test_string = "some\r\nreturns\r\nin\nhere" - schema = MarkdownFieldTestSchema(strict=True) + schema = MarkdownFieldTestSchema() result = schema.load({"markdown": test_string}) - assert "\r" not in result.data["markdown"] + assert "\r" not in result["markdown"] diff --git a/tildes/tests/test_simplestring_field.py b/tildes/tests/test_simplestring_field.py index 734339d..bdf7e7b 100644 --- a/tildes/tests/test_simplestring_field.py +++ b/tildes/tests/test_simplestring_field.py @@ -19,10 +19,10 @@ def process_string(string): This also works for testing validation since .load() will raise a ValidationError if an invalid string is attempted. """ - schema = SimpleStringTestSchema(strict=True) + schema = SimpleStringTestSchema() result = schema.load({"subject": string}) - return result.data["subject"] + return result["subject"] def test_changing_max_length(): diff --git a/tildes/tests/test_title.py b/tildes/tests/test_title.py index a1db5bf..02303be 100644 --- a/tildes/tests/test_title.py +++ b/tildes/tests/test_title.py @@ -1,7 +1,7 @@ # Copyright (c) 2018 Tildes contributors # SPDX-License-Identifier: AGPL-3.0-or-later -from marshmallow.exceptions import ValidationError +from marshmallow import ValidationError from pytest import fixture, raises from tildes.schemas.topic import TITLE_MAX_LENGTH, TopicSchema @@ -23,44 +23,44 @@ def test_too_long_title_invalid(title_schema): """Ensure a too-long title is invalid.""" title = "x" * (TITLE_MAX_LENGTH + 1) with raises(ValidationError): - title_schema.validate({"title": title}) + title_schema.load({"title": title}) def test_empty_title_invalid(title_schema): """Ensure an empty title is invalid.""" with raises(ValidationError): - title_schema.validate({"title": ""}) + title_schema.load({"title": ""}) def test_whitespace_only_title_invalid(title_schema): """Ensure a whitespace-only title is invalid.""" with raises(ValidationError): - title_schema.validate({"title": " \n "}) + title_schema.load({"title": " \n "}) def test_whitespace_trimmed(title_schema): """Ensure leading/trailing whitespace on a title is removed.""" title = " actual title " result = title_schema.load({"title": title}) - assert result.data["title"] == "actual title" + assert result["title"] == "actual title" def test_consecutive_whitespace_removed(title_schema): """Ensure consecutive whitespace in a title is compressed.""" title = "sure are \n a lot of spaces" result = title_schema.load({"title": title}) - assert result.data["title"] == "sure are a lot of spaces" + assert result["title"] == "sure are a lot of spaces" def test_unicode_spaces_normalized(title_schema): """Test that some unicode space characters are converted to normal ones.""" title = "some\u2009weird\u00a0spaces\u205fin\u00a0here" result = title_schema.load({"title": title}) - assert result.data["title"] == "some weird spaces in here" + assert result["title"] == "some weird spaces in here" def test_unicode_control_chars_removed(title_schema): """Test that some unicode control characters are stripped from titles.""" title = "nothing\u0000strange\u0085going\u009con\u007fhere" result = title_schema.load({"title": title}) - assert result.data["title"] == "nothingstrangegoingonhere" + assert result["title"] == "nothingstrangegoingonhere" diff --git a/tildes/tests/test_user.py b/tildes/tests/test_user.py index 8c77eef..dafd1d7 100644 --- a/tildes/tests/test_user.py +++ b/tildes/tests/test_user.py @@ -11,10 +11,10 @@ from tildes.schemas.user import PASSWORD_MIN_LENGTH, UserSchema def test_creation_validates_schema(mocker): - """Ensure that model creation goes through schema validation.""" - mocker.spy(UserSchema, "validate") + """Ensure that model creation goes through schema validation (via load()).""" + mocker.spy(UserSchema, "load") User("testing", "testpassword") - call_args = [call[0] for call in UserSchema.validate.call_args_list] + call_args = [call[0] for call in UserSchema.load.call_args_list] expected_args = {"username": "testing", "password": "testpassword"} assert any(expected_args in call for call in call_args) diff --git a/tildes/tildes/models/database_model.py b/tildes/tildes/models/database_model.py index f413a45..3b6662b 100644 --- a/tildes/tildes/models/database_model.py +++ b/tildes/tildes/models/database_model.py @@ -140,7 +140,7 @@ class DatabaseModelBase: return value result = self.schema.load({attribute: value}) - return result.data[attribute] + return result[attribute] DatabaseModel = declarative_base( diff --git a/tildes/tildes/models/user/user.py b/tildes/tildes/models/user/user.py index 40879fe..15ef8fe 100644 --- a/tildes/tildes/models/user/user.py +++ b/tildes/tildes/models/user/user.py @@ -222,7 +222,7 @@ class User(DatabaseModel): def password(self, value: str) -> None: # need to do manual validation since some password checks depend on checking the # username at the same time (for similarity) - self.schema.validate({"username": self.username, "password": value}) + self.schema.load({"username": self.username, "password": value}) self.password_hash = hash_string(value) diff --git a/tildes/tildes/resources/group.py b/tildes/tildes/resources/group.py index 3bde070..13b2698 100644 --- a/tildes/tildes/resources/group.py +++ b/tildes/tildes/resources/group.py @@ -19,13 +19,13 @@ from tildes.schemas.group import GroupSchema locations=("matchdict",), ) def group_by_path(request: Request, path: str) -> Group: - """Get a group specified by {group_path} in the route (or 404).""" + """Get a group specified by {path} in the route (or 404).""" # If loading the specified group path into the GroupSchema changed it, do a 301 # redirect to the resulting group path. This will happen in cases like the original # url including capital letters in the group path, where we want to redirect to the # proper all-lowercase path instead. - if path != request.matchdict["group_path"]: - request.matchdict["group_path"] = path + if path != request.matchdict["path"]: + request.matchdict["path"] = path proper_url = request.route_url(request.matched_route.name, **request.matchdict) raise HTTPMovedPermanently(location=proper_url) diff --git a/tildes/tildes/resources/topic.py b/tildes/tildes/resources/topic.py index e53f5df..541ba10 100644 --- a/tildes/tildes/resources/topic.py +++ b/tildes/tildes/resources/topic.py @@ -35,8 +35,8 @@ def topic_by_id36(request: Request, topic_id36: str) -> Topic: # if there's also a group specified in the route, check that it's the same group as # the topic was posted in, otherwise redirect to correct group - if "group_path" in request.matchdict: - path_from_route = request.matchdict["group_path"].lower() + if "path" in request.matchdict: + path_from_route = request.matchdict["path"].lower() if path_from_route != topic.group.path: raise HTTPFound(topic.permalink) diff --git a/tildes/tildes/routes.py b/tildes/tildes/routes.py index 85aed87..5ff8299 100644 --- a/tildes/tildes/routes.py +++ b/tildes/tildes/routes.py @@ -32,8 +32,8 @@ def includeme(config: Configurator) -> None: config.add_route("register", "/register") - config.add_route("group", "/~{group_path}", factory=group_by_path) - with config.route_prefix_context("/~{group_path}"): + config.add_route("group", "/~{path}", factory=group_by_path) + with config.route_prefix_context("/~{path}"): config.add_route("new_topic", "/new_topic", factory=group_by_path) config.add_route("group_topics", "/topics", factory=group_by_path) @@ -123,7 +123,7 @@ def includeme(config: Configurator) -> None: # Add routes for the link-shortener under the /shortener path with config.route_prefix_context("/shortener"): - config.add_route("shortener_group", "/~{group_path}", factory=group_by_path) + config.add_route("shortener_group", "/~{path}", factory=group_by_path) config.add_route("shortener_topic", "/{topic_id36}", factory=topic_by_id36) @@ -135,7 +135,7 @@ def add_intercooler_routes(config: Configurator) -> None: name = "ic_" + name config.add_route(name, path, header="X-IC-Request:true", **kwargs) - with config.route_prefix_context("/group/{group_path}"): + with config.route_prefix_context("/group/{path}"): add_ic_route("group_subscribe", "/subscribe", factory=group_by_path) add_ic_route("group_user_settings", "/user_settings", factory=group_by_path) diff --git a/tildes/tildes/schemas/comment.py b/tildes/tildes/schemas/comment.py index 0562276..01cfdd0 100644 --- a/tildes/tildes/schemas/comment.py +++ b/tildes/tildes/schemas/comment.py @@ -12,22 +12,13 @@ from tildes.schemas.fields import Enum, ID36, Markdown, SimpleString class CommentSchema(Schema): """Marshmallow schema for comments.""" + comment_id36 = ID36() markdown = Markdown() parent_comment_id36 = ID36() - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True - class CommentLabelSchema(Schema): """Marshmallow schema for comment labels.""" name = Enum(CommentLabelOption) reason = SimpleString(max_length=1000, missing=None) - - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True diff --git a/tildes/tildes/schemas/fields.py b/tildes/tildes/schemas/fields.py index 9785742..8cde3ea 100644 --- a/tildes/tildes/schemas/fields.py +++ b/tildes/tildes/schemas/fields.py @@ -4,7 +4,7 @@ """Custom schema field definitions.""" import enum -from typing import Any, Optional, Type +from typing import Any, Mapping, Optional, Type import sqlalchemy_utils from marshmallow.exceptions import ValidationError @@ -16,6 +16,10 @@ from tildes.lib.id import ID36_REGEX from tildes.lib.string import simplify_string +# type alias for the data argument passed to _deserialize methods +DataType = Optional[Mapping[str, Any]] + + class Enum(Field): """Field for a native Python Enum (or subclasses).""" @@ -25,11 +29,15 @@ class Enum(Field): super().__init__(*args, **kwargs) self._enum_class = enum_class - def _serialize(self, value: enum.Enum, attr: str, obj: object) -> str: + def _serialize( + self, value: enum.Enum, attr: str, obj: object, **kwargs: Any + ) -> str: """Serialize the enum value - lowercase version of its name.""" return value.name.lower() - def _deserialize(self, value: str, attr: str, data: dict) -> enum.Enum: + def _deserialize( + self, value: str, attr: Optional[str], data: DataType, **kwargs: Any, + ) -> enum.Enum: """Deserialize a string to the enum member with that name.""" if not self._enum_class: raise ValidationError("Cannot deserialize with no enum class.") @@ -43,9 +51,9 @@ class Enum(Field): class ID36(String): """Field for a base-36 ID.""" - def __init__(self) -> None: + def __init__(self, **kwargs: Any) -> None: """Initialize the field with a regex validator.""" - super().__init__(validate=Regexp(ID36_REGEX)) + super().__init__(validate=Regexp(ID36_REGEX), **kwargs) class ShortTimePeriod(Field): @@ -55,7 +63,7 @@ class ShortTimePeriod(Field): """ def _deserialize( - self, value: str, attr: str, data: dict + self, value: str, attr: Optional[str], data: DataType, **kwargs: Any, ) -> Optional[SimpleHoursPeriod]: """Deserialize to a SimpleHoursPeriod object.""" if value == "all": @@ -67,7 +75,7 @@ class ShortTimePeriod(Field): raise ValidationError("Invalid time period") def _serialize( - self, value: Optional[SimpleHoursPeriod], attr: str, obj: object + self, value: Optional[SimpleHoursPeriod], attr: str, obj: object, **kwargs: Any, ) -> Optional[str]: """Serialize the value to the "short form" string.""" if not value: @@ -95,13 +103,15 @@ class Markdown(Field): if value.isspace(): raise ValidationError("Cannot be entirely whitespace.") - def _deserialize(self, value: str, attr: str, data: dict) -> str: + def _deserialize( + self, value: str, attr: Optional[str], data: DataType, **kwargs: Any, + ) -> str: """Deserialize the string, removing carriage returns in the process.""" value = value.replace("\r", "") return value - def _serialize(self, value: str, attr: str, obj: object) -> str: + def _serialize(self, value: str, attr: str, obj: object, **kwargs: Any) -> str: """Serialize the value (no-op in this case).""" return value @@ -126,11 +136,13 @@ class SimpleString(Field): super().__init__(validate=Length(min=1, max=max_length), **kwargs) - def _deserialize(self, value: str, attr: str, data: dict) -> str: + def _deserialize( + self, value: str, attr: Optional[str], data: DataType, **kwargs: Any, + ) -> str: """Deserialize the string, removing/replacing as necessary.""" return simplify_string(value) - def _serialize(self, value: str, attr: str, obj: object) -> str: + def _serialize(self, value: str, attr: str, obj: object, **kwargs: Any) -> str: """Serialize the value (no-op in this case).""" return value @@ -138,11 +150,15 @@ class SimpleString(Field): class Ltree(Field): """Field for postgresql ltree type.""" - def _serialize(self, value: sqlalchemy_utils.Ltree, attr: str, obj: object) -> str: + def _serialize( + self, value: sqlalchemy_utils.Ltree, attr: str, obj: object, **kwargs: Any + ) -> str: """Serialize the Ltree value - use the (string) path.""" return value.path - def _deserialize(self, value: str, attr: str, data: dict) -> sqlalchemy_utils.Ltree: + def _deserialize( + self, value: str, attr: Optional[str], data: DataType, **kwargs: Any, + ) -> sqlalchemy_utils.Ltree: """Deserialize a string path to an Ltree object.""" # convert to lowercase and replace spaces with underscores value = value.lower().replace(" ", "_") diff --git a/tildes/tildes/schemas/group.py b/tildes/tildes/schemas/group.py index 32e033f..2b51824 100644 --- a/tildes/tildes/schemas/group.py +++ b/tildes/tildes/schemas/group.py @@ -4,6 +4,7 @@ """Validation/dumping schema for groups.""" import re +from typing import Any import sqlalchemy_utils from marshmallow import pre_load, Schema, validates @@ -32,7 +33,7 @@ SHORT_DESCRIPTION_MAX_LENGTH = 200 class GroupSchema(Schema): """Marshmallow schema for groups.""" - path = Ltree(required=True, load_from="group_path") + path = Ltree(required=True) created_time = DateTime(dump_only=True) short_description = SimpleString( max_length=SHORT_DESCRIPTION_MAX_LENGTH, allow_none=True @@ -40,17 +41,14 @@ class GroupSchema(Schema): sidebar_markdown = Markdown(allow_none=True) @pre_load - def prepare_path(self, data: dict) -> dict: + def prepare_path(self, data: dict, many: bool, partial: Any) -> dict: """Prepare the path value before it's validated.""" + # pylint: disable=unused-argument if not self.context.get("fix_path_capitalization"): return data - # path can also be loaded from group_path, so we need to check both - keys = ("path", "group_path") - - for key in keys: - if key in data and isinstance(data[key], str): - data[key] = data[key].lower() + if "path" in data and isinstance(data["path"], str): + data["path"] = data["path"].lower() return data @@ -67,8 +65,9 @@ class GroupSchema(Schema): raise ValidationError("Path element %s is invalid" % element) @pre_load - def prepare_sidebar_markdown(self, data: dict) -> dict: + def prepare_sidebar_markdown(self, data: dict, many: bool, partial: Any) -> dict: """Prepare the sidebar_markdown value before it's validated.""" + # pylint: disable=unused-argument if "sidebar_markdown" not in data: return data @@ -78,18 +77,9 @@ class GroupSchema(Schema): return data - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True - def is_valid_group_path(path: str) -> bool: """Return whether the group path is valid or not.""" schema = GroupSchema(partial=True) - try: - schema.validate({"path": path}) - except ValidationError: - return False - - return True + errors = schema.validate({"path": path}) + return not errors diff --git a/tildes/tildes/schemas/group_wiki_page.py b/tildes/tildes/schemas/group_wiki_page.py index b152726..ea4229f 100644 --- a/tildes/tildes/schemas/group_wiki_page.py +++ b/tildes/tildes/schemas/group_wiki_page.py @@ -16,8 +16,3 @@ class GroupWikiPageSchema(Schema): page_name = SimpleString(max_length=PAGE_NAME_MAX_LENGTH) markdown = Markdown(max_length=1_000_000) - - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True diff --git a/tildes/tildes/schemas/listing.py b/tildes/tildes/schemas/listing.py index 0326f70..5a797e0 100644 --- a/tildes/tildes/schemas/listing.py +++ b/tildes/tildes/schemas/listing.py @@ -3,6 +3,8 @@ """Validation schema for topic listing views.""" +from typing import Any + from marshmallow import pre_load, Schema, validates_schema, ValidationError from marshmallow.fields import Boolean, Integer from marshmallow.validate import Range @@ -14,36 +16,38 @@ from tildes.schemas.fields import Enum, ID36, Ltree, PostType, ShortTimePeriod class PaginatedListingSchema(Schema): """Marshmallow schema to validate arguments for a paginated listing page.""" - after = ID36() - before = ID36() + after = ID36(missing=None) + before = ID36(missing=None) per_page = Integer(validate=Range(min=1, max=100), missing=50) @validates_schema - def either_after_or_before(self, data: dict) -> None: + def either_after_or_before(self, data: dict, many: bool, partial: Any) -> None: """Fail validation if both after and before were specified.""" + # pylint: disable=unused-argument if data.get("after") and data.get("before"): raise ValidationError("Can't specify both after and before.") - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True - class TopicListingSchema(PaginatedListingSchema): """Marshmallow schema to validate arguments for a topic listing page.""" period = ShortTimePeriod(allow_none=True) - order = Enum(TopicSortOption) + order = Enum(TopicSortOption, missing=None) tag = Ltree(missing=None) unfiltered = Boolean(missing=False) - rank_start = Integer(load_from="n", validate=Range(min=1), missing=None) + rank_start = Integer(data_key="n", validate=Range(min=1), missing=None) @pre_load - def reset_rank_start_on_first_page(self, data: dict) -> dict: + def reset_rank_start_on_first_page( + self, data: dict, many: bool, partial: Any + ) -> dict: """Reset rank_start to 1 if this is a first page (no before/after).""" + # pylint: disable=unused-argument + if "rank_start" not in self.fields: + return data + if not (data.get("before") or data.get("after")): - data["rank_start"] = 1 + data["n"] = 1 return data @@ -55,15 +59,18 @@ class MixedListingSchema(PaginatedListingSchema): of just one or the other. """ - anchor_type = PostType() + anchor_type = PostType(missing=None) @pre_load - def set_anchor_type_from_before_or_after(self, data: dict) -> dict: + def set_anchor_type_from_before_or_after( + self, data: dict, many: bool, partial: Any + ) -> dict: """Set the anchor_type if before or after has a special value indicating type. For example, if after or before looks like "t-123" that means it is referring to the topic with ID36 "123". "c-123" also works, for comments. """ + # pylint: disable=unused-argument keys = ("after", "before") for key in keys: diff --git a/tildes/tildes/schemas/message.py b/tildes/tildes/schemas/message.py index ac9c168..2f16075 100644 --- a/tildes/tildes/schemas/message.py +++ b/tildes/tildes/schemas/message.py @@ -21,11 +21,6 @@ class MessageConversationSchema(Schema): rendered_html = String(dump_only=True) created_time = DateTime(dump_only=True) - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True - class MessageReplySchema(Schema): """Marshmallow schema for message replies.""" @@ -34,8 +29,3 @@ class MessageReplySchema(Schema): markdown = Markdown() rendered_html = String(dump_only=True) created_time = DateTime(dump_only=True) - - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True diff --git a/tildes/tildes/schemas/topic.py b/tildes/tildes/schemas/topic.py index f287562..10c5bde 100644 --- a/tildes/tildes/schemas/topic.py +++ b/tildes/tildes/schemas/topic.py @@ -5,6 +5,7 @@ import re import typing +from typing import Any from urllib.parse import urlparse from marshmallow import pre_load, Schema, validates, validates_schema, ValidationError @@ -36,8 +37,9 @@ class TopicSchema(Schema): group = Nested(GroupSchema, dump_only=True) @pre_load - def prepare_tags(self, data: dict) -> dict: + def prepare_tags(self, data: dict, many: bool, partial: Any) -> dict: """Prepare the tags before they're validated.""" + # pylint: disable=unused-argument if "tags" not in data: return data @@ -87,13 +89,14 @@ class TopicSchema(Schema): group_schema = GroupSchema(partial=True) for tag in value: try: - group_schema.validate({"path": tag}) + group_schema.load({"path": tag}) except ValidationError: raise ValidationError("Tag %s is invalid" % tag) @pre_load - def prepare_markdown(self, data: dict) -> dict: + def prepare_markdown(self, data: dict, many: bool, partial: Any) -> dict: """Prepare the markdown value before it's validated.""" + # pylint: disable=unused-argument if "markdown" not in data: return data @@ -104,8 +107,9 @@ class TopicSchema(Schema): return data @pre_load - def prepare_link(self, data: dict) -> dict: + def prepare_link(self, data: dict, many: bool, partial: Any) -> dict: """Prepare the link value before it's validated.""" + # pylint: disable=unused-argument if "link" not in data: return data @@ -125,8 +129,9 @@ class TopicSchema(Schema): return data @validates_schema - def link_or_markdown(self, data: dict) -> None: + def link_or_markdown(self, data: dict, many: bool, partial: Any) -> None: """Fail validation unless at least one of link or markdown were set.""" + # pylint: disable=unused-argument if "link" not in data and "markdown" not in data: return @@ -135,8 +140,3 @@ class TopicSchema(Schema): if not (markdown or link): raise ValidationError("Topics must have either markdown or a link.") - - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True diff --git a/tildes/tildes/schemas/user.py b/tildes/tildes/schemas/user.py index 3b27284..53a4826 100644 --- a/tildes/tildes/schemas/user.py +++ b/tildes/tildes/schemas/user.py @@ -4,6 +4,7 @@ """Validation/dumping schema for users.""" import re +from typing import Any from marshmallow import post_dump, pre_load, Schema, validates, validates_schema from marshmallow.exceptions import ValidationError @@ -60,16 +61,20 @@ class UserSchema(Schema): bio_markdown = Markdown(max_length=BIO_MAX_LENGTH, allow_none=True) @post_dump - def anonymize_username(self, data: dict) -> dict: + def anonymize_username(self, data: dict, many: bool) -> dict: """Hide the username if the dumping context specifies to do so.""" + # pylint: disable=unused-argument if "username" in data and self.context.get("hide_username"): data["username"] = "" return data @validates_schema - def username_pass_not_substrings(self, data: dict) -> None: + def username_pass_not_substrings( + self, data: dict, many: bool, partial: Any + ) -> None: """Ensure the username isn't in the password and vice versa.""" + # pylint: disable=unused-argument username = data.get("username") password = data.get("password") if not (username and password): @@ -97,11 +102,12 @@ class UserSchema(Schema): raise ValidationError("That password exists in a data breach (see sidebar)") @pre_load - def username_trim_whitespace(self, data: dict) -> dict: + def username_trim_whitespace(self, data: dict, many: bool, partial: Any) -> dict: """Trim leading/trailing whitespace around the username. Requires username_trim_whitespace be True in the schema's context. """ + # pylint: disable=unused-argument if not self.context.get("username_trim_whitespace"): return data @@ -113,8 +119,9 @@ class UserSchema(Schema): return data @pre_load - def prepare_email_address(self, data: dict) -> dict: + def prepare_email_address(self, data: dict, many: bool, partial: Any) -> dict: """Prepare the email address value before it's validated.""" + # pylint: disable=unused-argument if "email_address" not in data: return data @@ -128,8 +135,9 @@ class UserSchema(Schema): return data @pre_load - def prepare_bio_markdown(self, data: dict) -> dict: + def prepare_bio_markdown(self, data: dict, many: bool, partial: Any) -> dict: """Prepare the bio_markdown value before it's validated.""" + # pylint: disable=unused-argument if "bio_markdown" not in data: return data @@ -139,11 +147,6 @@ class UserSchema(Schema): return data - class Meta: - """Always use strict checking so error handlers are invoked.""" - - strict = True - def is_valid_username(username: str) -> bool: """Return whether the username is valid or not. @@ -153,9 +156,5 @@ def is_valid_username(username: str) -> bool: specific reason for invalidity. """ schema = UserSchema(partial=True) - try: - schema.validate({"username": username}) - except ValidationError: - return False - - return True + errors = schema.validate({"username": username}) + return not errors diff --git a/tildes/tildes/templates/group_wiki.jinja2 b/tildes/tildes/templates/group_wiki.jinja2 index 7cc5a3c..819dfff 100644 --- a/tildes/tildes/templates/group_wiki.jinja2 +++ b/tildes/tildes/templates/group_wiki.jinja2 @@ -18,7 +18,7 @@ diff --git a/tildes/tildes/views/api/v0/group.py b/tildes/tildes/views/api/v0/group.py index 2cf0e10..0cb1666 100644 --- a/tildes/tildes/views/api/v0/group.py +++ b/tildes/tildes/views/api/v0/group.py @@ -9,7 +9,7 @@ from tildes.api import APIv0 from tildes.resources.group import group_by_path -ONE = APIv0(name="group", path="/groups/{group_path}", factory=group_by_path) +ONE = APIv0(name="group", path="/groups/{path}", factory=group_by_path) @ONE.get() diff --git a/tildes/tildes/views/api/v0/topic.py b/tildes/tildes/views/api/v0/topic.py index f74c6df..5e1536c 100644 --- a/tildes/tildes/views/api/v0/topic.py +++ b/tildes/tildes/views/api/v0/topic.py @@ -10,7 +10,7 @@ from tildes.resources.topic import topic_by_id36 ONE = APIv0( - name="topic", path="/groups/{group_path}/topics/{topic_id36}", factory=topic_by_id36 + name="topic", path="/groups/{path}/topics/{topic_id36}", factory=topic_by_id36 ) diff --git a/tildes/tildes/views/api/web/comment.py b/tildes/tildes/views/api/web/comment.py index a212926..9ed5ed6 100644 --- a/tildes/tildes/views/api/web/comment.py +++ b/tildes/tildes/views/api/web/comment.py @@ -277,7 +277,8 @@ def delete_vote_comment(request: Request) -> dict: renderer="comment_contents.jinja2", ) @use_kwargs(CommentLabelSchema(only=("name",)), locations=("matchdict",)) -@use_kwargs(CommentLabelSchema(only=("reason",))) +# need to specify only "form" location for reason, or it will crash by looking for JSON +@use_kwargs(CommentLabelSchema(only=("reason",)), locations=("form",)) def put_label_comment( request: Request, name: CommentLabelOption, reason: str ) -> Response: @@ -352,7 +353,7 @@ def delete_label_comment(request: Request, name: CommentLabelOption) -> Response @ic_view_config( route_name="comment_mark_read", request_method="PUT", permission="mark_read" ) -@use_kwargs({"mark_all_previous": Boolean(missing=False)}) +@use_kwargs({"mark_all_previous": Boolean(missing=False)}, locations=("query",)) def put_mark_comments_read(request: Request, mark_all_previous: bool) -> Response: """Mark comment(s) read, clearing notifications. diff --git a/tildes/tildes/views/api/web/group.py b/tildes/tildes/views/api/web/group.py index c48ac60..20f16c4 100644 --- a/tildes/tildes/views/api/web/group.py +++ b/tildes/tildes/views/api/web/group.py @@ -12,6 +12,7 @@ from webargs.pyramidparser import use_kwargs from zope.sqlalchemy import mark_changed from tildes.enums import TopicSortOption +from tildes.lib.datetime import SimpleHoursPeriod from tildes.models.group import Group, GroupSubscription from tildes.models.user import UserGroupSettings from tildes.schemas.fields import Enum, ShortTimePeriod @@ -84,10 +85,14 @@ def delete_subscribe_group(request: Request) -> dict: @ic_view_config(route_name="group_user_settings", request_method="PATCH") @use_kwargs( - {"order": Enum(TopicSortOption), "period": ShortTimePeriod(allow_none=True)} + { + "order": Enum(TopicSortOption), + "period": ShortTimePeriod(allow_none=True, missing=None), + }, + locations=("form",), # will crash due to trying to find JSON data without this ) def patch_group_user_settings( - request: Request, order: TopicSortOption, period: Optional[ShortTimePeriod] + request: Request, order: TopicSortOption, period: Optional[SimpleHoursPeriod] ) -> dict: """Set the user's default listing options.""" if period: diff --git a/tildes/tildes/views/api/web/topic.py b/tildes/tildes/views/api/web/topic.py index bdbcdc7..3ed661e 100644 --- a/tildes/tildes/views/api/web/topic.py +++ b/tildes/tildes/views/api/web/topic.py @@ -68,7 +68,7 @@ def delete_topic(request: Request) -> Response: response = Response() response.headers["X-IC-Redirect"] = request.route_url( - "group", group_path=topic.group.path + "group", path=topic.group.path ) return response @@ -158,7 +158,7 @@ def get_topic_tags(request: Request) -> dict: renderer="topic_tags.jinja2", permission="tag", ) -@use_kwargs({"tags": String(), "conflict_check": String()}) +@use_kwargs({"tags": String(missing=""), "conflict_check": String(missing="")}) def put_tag_topic(request: Request, tags: str, conflict_check: str) -> dict: """Apply tags to a topic with Intercooler.""" topic = request.context diff --git a/tildes/tildes/views/api/web/user.py b/tildes/tildes/views/api/web/user.py index 6a6faf4..16d3e8a 100644 --- a/tildes/tildes/views/api/web/user.py +++ b/tildes/tildes/views/api/web/user.py @@ -20,6 +20,7 @@ from sqlalchemy.exc import IntegrityError from webargs.pyramidparser import use_kwargs from tildes.enums import LogEventType, TopicSortOption +from tildes.lib.datetime import SimpleHoursPeriod from tildes.lib.string import separate_string from tildes.models.log import Log from tildes.models.user import User, UserInviteCode @@ -321,10 +322,14 @@ def get_invite_code(request: Request) -> dict: permission="edit_default_listing_options", ) @use_kwargs( - {"order": Enum(TopicSortOption), "period": ShortTimePeriod(allow_none=True)} + { + "order": Enum(TopicSortOption), + "period": ShortTimePeriod(allow_none=True, missing=None), + }, + locations=("form",), # will crash due to trying to find JSON data without this ) def put_default_listing_options( - request: Request, order: TopicSortOption, period: Optional[ShortTimePeriod] + request: Request, order: TopicSortOption, period: Optional[SimpleHoursPeriod] ) -> dict: """Set the user's default listing options.""" user = request.context @@ -358,7 +363,7 @@ def put_filtered_topic_tags(request: Request, tags: str) -> dict: except ValidationError: raise ValidationError({"tags": ["Invalid tags"]}) - request.user.filtered_topic_tags = result.data["tags"] + request.user.filtered_topic_tags = result["tags"] return IC_NOOP diff --git a/tildes/tildes/views/bookmarks.py b/tildes/tildes/views/bookmarks.py index 3c4f313..64d81be 100644 --- a/tildes/tildes/views/bookmarks.py +++ b/tildes/tildes/views/bookmarks.py @@ -1,6 +1,6 @@ """Views relating to bookmarks.""" -from typing import Type, Union +from typing import Optional, Type, Union from pyramid.request import Request from pyramid.view import view_config @@ -15,9 +15,13 @@ from tildes.schemas.listing import PaginatedListingSchema @view_config(route_name="bookmarks", renderer="bookmarks.jinja2") @use_kwargs(PaginatedListingSchema) -@use_kwargs({"post_type": PostType(load_from="type", missing="topic")}) +@use_kwargs({"post_type": PostType(data_key="type", missing="topic")}) def get_bookmarks( - request: Request, after: str, before: str, per_page: int, post_type: str + request: Request, + after: Optional[str], + before: Optional[str], + per_page: int, + post_type: str, ) -> dict: """Generate the bookmarks page.""" # pylint: disable=unused-argument diff --git a/tildes/tildes/views/exceptions.py b/tildes/tildes/views/exceptions.py index 7562a16..9a4ca06 100644 --- a/tildes/tildes/views/exceptions.py +++ b/tildes/tildes/views/exceptions.py @@ -28,7 +28,7 @@ from tildes.models.group import Group def errors_from_validationerror(validation_error: ValidationError) -> Sequence[str]: """Extract errors from a marshmallow ValidationError into a displayable format.""" - errors_by_field = validation_error.messages + errors_by_field = validation_error.normalized_messages() error_strings = [] for field, errors in errors_by_field.items(): @@ -47,7 +47,7 @@ def errors_from_validationerror(validation_error: ValidationError) -> Sequence[s def group_not_found(request: Request) -> dict: """Show the user a customized 404 page for group names.""" request.response.status_int = 404 - supplied_name = request.matchdict.get("group_path") + supplied_name = request.matchdict.get("path") # the 'word_similarity' function here is from the 'pg_trgm' extension group_suggestions = ( request.query(Group) diff --git a/tildes/tildes/views/group_wiki_page.py b/tildes/tildes/views/group_wiki_page.py index a11014d..35937f8 100644 --- a/tildes/tildes/views/group_wiki_page.py +++ b/tildes/tildes/views/group_wiki_page.py @@ -76,7 +76,7 @@ def post_group_wiki(request: Request, page_name: str, markdown: str) -> HTTPFoun raise HTTPFound( location=request.route_url( - "group_wiki_page", group_path=group.path, wiki_page_path=new_page.path + "group_wiki_page", path=group.path, wiki_page_path=new_page.path ) ) @@ -104,6 +104,6 @@ def post_group_wiki_page(request: Request, markdown: str, edit_message: str) -> raise HTTPFound( location=request.route_url( - "group_wiki_page", group_path=page.group.path, wiki_page_path=page.path + "group_wiki_page", path=page.group.path, wiki_page_path=page.path ) ) diff --git a/tildes/tildes/views/login.py b/tildes/tildes/views/login.py index fd8cba6..63b30a4 100644 --- a/tildes/tildes/views/login.py +++ b/tildes/tildes/views/login.py @@ -115,7 +115,7 @@ def post_login( ) @not_logged_in @rate_limit_view("login_two_factor") -@use_kwargs({"code": String(), "from_url": String(missing="")}) +@use_kwargs({"code": String(missing=""), "from_url": String(missing="")}) def post_login_two_factor(request: Request, code: str, from_url: str) -> NoReturn: """Process a log in request with 2FA.""" # Look up the user for the supplied username diff --git a/tildes/tildes/views/notifications.py b/tildes/tildes/views/notifications.py index 96da5d9..563be53 100644 --- a/tildes/tildes/views/notifications.py +++ b/tildes/tildes/views/notifications.py @@ -3,6 +3,8 @@ """Views related to notifications.""" +from typing import Optional + from pyramid.request import Request from pyramid.view import view_config from sqlalchemy.sql.expression import desc @@ -39,7 +41,7 @@ def get_user_unread_notifications(request: Request) -> dict: @view_config(route_name="notifications", renderer="notifications.jinja2") @use_kwargs(PaginatedListingSchema()) def get_user_notifications( - request: Request, after: str, before: str, per_page: int + request: Request, after: Optional[str], before: Optional[str], per_page: int ) -> dict: """Show the logged-in user's previously-read notifications.""" query = ( diff --git a/tildes/tildes/views/topic.py b/tildes/tildes/views/topic.py index 9e6358b..e6f019e 100644 --- a/tildes/tildes/views/topic.py +++ b/tildes/tildes/views/topic.py @@ -49,7 +49,10 @@ DefaultSettings = namedtuple("DefaultSettings", ["order", "period"]) @view_config(route_name="group_topics", request_method="POST", permission="post_topic") @use_kwargs(TopicSchema(only=("title", "markdown", "link"))) -@use_kwargs({"tags": String(missing=""), "confirm_repost": Boolean(missing=False)}) +@use_kwargs( + {"tags": String(missing=""), "confirm_repost": Boolean(missing=False)}, + locations=("form",), # will crash due to trying to find JSON data without this +) def post_group_topics( request: Request, title: str, @@ -142,16 +145,19 @@ def post_group_topics( @use_kwargs(TopicListingSchema()) def get_group_topics( request: Request, - order: Any, # more specific would be better, but missing isn't typed - period: Any, # more specific would be better, but missing isn't typed - after: str, - before: str, + after: Optional[str], + before: Optional[str], + order: Optional[TopicSortOption], per_page: int, rank_start: Optional[int], tag: Optional[Ltree], unfiltered: bool, + **kwargs: Any ) -> dict: """Get a listing of topics in the group.""" + # period needs special treatment so we can distinguish between missing and None + period = kwargs.get("period", missing) + is_home_page = request.matched_route.name == "home" if is_home_page: @@ -179,7 +185,7 @@ def get_group_topics( default_settings = _get_default_settings(request, order) - if order is missing: + if not order: order = default_settings.order if period is missing: @@ -297,22 +303,25 @@ def get_group_topics( @view_config(route_name="search", renderer="search.jinja2") @view_config(route_name="group_search", renderer="search.jinja2") @use_kwargs(TopicListingSchema(only=("after", "before", "order", "per_page", "period"))) -@use_kwargs({"search": String(load_from="q", missing="")}) +@use_kwargs({"search": String(data_key="q", missing="")}) def get_search( request: Request, - order: Any, - period: Any, - after: str, - before: str, + order: Optional[TopicSortOption], + after: Optional[str], + before: Optional[str], per_page: int, search: str, + **kwargs: Any ) -> dict: """Get a list of search results.""" + # period needs special treatment so we can distinguish between missing and None + period = kwargs.get("period", missing) + group = None if isinstance(request.context, Group): group = request.context - if order is missing: + if not order: order = TopicSortOption.NEW if period is missing: @@ -372,7 +381,13 @@ def get_new_topic_form(request: Request) -> dict: @view_config(route_name="topic", renderer="topic.jinja2") @view_config(route_name="topic_no_title", renderer="topic.jinja2") -@use_kwargs({"comment_order": Enum(CommentTreeSortOption, missing="relevance")}) +@use_kwargs( + { + "comment_order": Enum( + CommentTreeSortOption, missing=CommentTreeSortOption.RELEVANCE + ) + } +) def get_topic(request: Request, comment_order: CommentTreeSortOption) -> dict: """View a single topic.""" topic = request.context @@ -470,7 +485,9 @@ def post_comment_on_topic(request: Request, markdown: str) -> HTTPFound: raise HTTPFound(location=topic.permalink) -def _get_default_settings(request: Request, order: Any) -> DefaultSettings: # noqa +def _get_default_settings( + request: Request, order: Optional[TopicSortOption] +) -> DefaultSettings: if isinstance(request.context, Group) and request.user: user_settings = ( request.query(UserGroupSettings) @@ -492,7 +509,7 @@ def _get_default_settings(request: Request, order: Any) -> DefaultSettings: # n # the default period depends on what the order is, so we need to see if we're going # to end up using the default order here as well - if order is missing: + if not order: order = default_order if user_settings and user_settings.default_period: diff --git a/tildes/tildes/views/user.py b/tildes/tildes/views/user.py index 6a455a0..e08e285 100644 --- a/tildes/tildes/views/user.py +++ b/tildes/tildes/views/user.py @@ -25,8 +25,8 @@ from tildes.schemas.listing import MixedListingSchema @use_kwargs(MixedListingSchema()) @use_kwargs( { - "post_type": PostType(load_from="type"), - "order_name": String(load_from="order", missing="new"), + "post_type": PostType(data_key="type", missing=None), + "order_name": String(data_key="order", missing="new"), } ) def get_user( @@ -36,7 +36,7 @@ def get_user( per_page: int, anchor_type: Optional[str], order_name: str, - post_type: Optional[str] = None, + post_type: Optional[str], ) -> dict: """Generate the main user history page.""" user = request.context @@ -93,9 +93,9 @@ def get_user( @use_kwargs(MixedListingSchema()) @use_kwargs( { - "post_type": PostType(load_from="type", required=True), - "order_name": String(load_from="order", missing="new"), - "search": String(load_from="q", missing=""), + "post_type": PostType(data_key="type", required=True), + "order_name": String(data_key="order", missing="new"), + "search": String(data_key="q", missing=""), } ) def get_user_search( diff --git a/tildes/tildes/views/votes.py b/tildes/tildes/views/votes.py index 7221499..7472da9 100644 --- a/tildes/tildes/views/votes.py +++ b/tildes/tildes/views/votes.py @@ -1,6 +1,6 @@ """Views relating to voted posts.""" -from typing import Type, Union +from typing import Optional, Type, Union from pyramid.request import Request from pyramid.view import view_config @@ -15,9 +15,13 @@ from tildes.schemas.listing import PaginatedListingSchema @view_config(route_name="votes", renderer="votes.jinja2") @use_kwargs(PaginatedListingSchema) -@use_kwargs({"post_type": PostType(load_from="type", missing="topic")}) +@use_kwargs({"post_type": PostType(data_key="type", missing="topic")}) def get_voted_posts( - request: Request, after: str, before: str, per_page: int, post_type: str + request: Request, + after: Optional[str], + before: Optional[str], + per_page: int, + post_type: str, ) -> dict: """Generate the voted posts page.""" # pylint: disable=unused-argument