Commit f41bd1ea authored by Deimos's avatar Deimos

Upgrade webargs to 6.1.0

This was not a fun upgrade. webargs made some major changes to its
approaches in 6.0, which are mostly covered here:
https://webargs.readthedocs.io/en/latest/upgrading.html

To keep using it on Tildes, this commit had to make the following
changes:

  - Write my own wrapper for use_kwargs that changes some of the default
    behavior. Specifically, we want the location that data is being
    loaded from to default to "query" (the query string) instead of
    webargs' default of "json". We also needed to set the "unknown"
    behavior on every schema to "exclude" so that the schemas would
    ignore any data fields they didn't need, since the default behavior
    is to throw an error, which happens almost everywhere because of
    Intercooler variables and/or multiple use_kwargs calls for different
    subsets of the data.

  - All @pre_load hooks in schemas needed to be rewritten so that they
    weren't modifying data in-place (copy to a new data dict first).
    Because webargs is now passing all data through all schemas,
    modifying in-place could result in an earlier schema modifying data
    that would then be passed in modified form to the later ones.
    Specifically, this caused an issue with tags on posting a new topic,
    where we just wanted to treat the tags as a string, but TopicSchema
    would convert it to a list in @pre_load.

  - use_kwargs on every endpoint using non-query data needed to be
    updated to support the new single-location approach, either replacing
    an existing locations= with location=, or adding location="form",
    since form data was no longer used by default.

  - The code that parsed the errors returned by webargs/Marshmallow
    ValidationErrors needed to update to handle the additional "level"
    in the dict of errors, where errors are now split out by location
    and then field, instead of only by field.

  - A few other minor updates, like always passing a schema object
    instead of a class, and never passing a callable (mostly just for
    simplicity in the wrapper).
parent 44a86996
......@@ -109,7 +109,7 @@ urllib3==1.25.10 # via requests, sentry-sdk
venusian==3.0.0 # via cornice, pyramid
waitress==1.4.4 # via webtest
wcwidth==0.2.5 # via prompt-toolkit
webargs==5.5.3
webargs==6.1.0
webassets==2.0 # via pyramid-webassets
webencodings==0.5.1 # via bleach, html5lib
webob==1.8.6 # via pyramid, webtest
......
......@@ -34,6 +34,6 @@ SQLAlchemy
SQLAlchemy-Utils
stripe
titlecase
webargs==5.5.3
webargs
wrapt
zope.sqlalchemy
......@@ -67,7 +67,7 @@ translationstring==1.4 # via pyramid
urllib3==1.25.10 # via requests, sentry-sdk
venusian==3.0.0 # via cornice, pyramid
wcwidth==0.2.5 # via prompt-toolkit
webargs==5.5.3
webargs==6.1.0
webassets==2.0 # via pyramid-webassets
webencodings==0.5.1 # via bleach, html5lib
webob==1.8.6 # via pyramid
......
......@@ -5,15 +5,15 @@
from pyramid.httpexceptions import HTTPForbidden, HTTPNotFound
from pyramid.request import Request
from webargs.pyramidparser import use_kwargs
from tildes.lib.id import id36_to_id
from tildes.models.comment import Comment, CommentNotification
from tildes.resources import get_resource
from tildes.schemas.comment import CommentSchema
from tildes.views.decorators import use_kwargs
@use_kwargs(CommentSchema(only=("comment_id36",)), locations=("matchdict",))
@use_kwargs(CommentSchema(only=("comment_id36",)), location="matchdict")
def comment_by_id36(request: Request, comment_id36: str) -> Comment:
"""Get a comment specified by {comment_id36} in the route (or 404)."""
query = (
......@@ -28,7 +28,7 @@ def comment_by_id36(request: Request, comment_id36: str) -> Comment:
raise HTTPNotFound("Comment not found (or it was deleted)")
@use_kwargs(CommentSchema(only=("comment_id36",)), locations=("matchdict",))
@use_kwargs(CommentSchema(only=("comment_id36",)), location="matchdict")
def notification_by_comment_id36(
request: Request, comment_id36: str
) -> CommentNotification:
......
......@@ -7,16 +7,16 @@ from marshmallow.fields import String
from pyramid.httpexceptions import HTTPMovedPermanently, HTTPNotFound
from pyramid.request import Request
from sqlalchemy_utils import Ltree
from webargs.pyramidparser import use_kwargs
from tildes.models.group import Group, GroupWikiPage
from tildes.resources import get_resource
from tildes.schemas.group import GroupSchema
from tildes.views.decorators import use_kwargs
@use_kwargs(
GroupSchema(only=("path",), context={"fix_path_capitalization": True}),
locations=("matchdict",),
location="matchdict",
)
def group_by_path(request: Request, path: str) -> Group:
"""Get a group specified by {path} in the route (or 404)."""
......@@ -35,7 +35,7 @@ def group_by_path(request: Request, path: str) -> Group:
return get_resource(request, query)
@use_kwargs({"wiki_page_path": String()}, locations=("matchdict",))
@use_kwargs({"wiki_page_path": String()}, location="matchdict")
def group_wiki_page_by_path(request: Request, wiki_page_path: str) -> GroupWikiPage:
"""Get a group's wiki page by its path (or 404)."""
group = group_by_path(request) # pylint: disable=no-value-for-parameter
......
......@@ -4,16 +4,16 @@
"""Root factories for messages."""
from pyramid.request import Request
from webargs.pyramidparser import use_kwargs
from tildes.lib.id import id36_to_id
from tildes.models.message import MessageConversation
from tildes.resources import get_resource
from tildes.schemas.message import MessageConversationSchema
from tildes.views.decorators import use_kwargs
@use_kwargs(
MessageConversationSchema(only=("conversation_id36",)), locations=("matchdict",)
MessageConversationSchema(only=("conversation_id36",)), location="matchdict"
)
def message_conversation_by_id36(
request: Request, conversation_id36: str
......
......@@ -5,15 +5,15 @@
from pyramid.httpexceptions import HTTPFound, HTTPNotFound
from pyramid.request import Request
from webargs.pyramidparser import use_kwargs
from tildes.lib.id import id36_to_id
from tildes.models.topic import Topic
from tildes.resources import get_resource
from tildes.schemas.topic import TopicSchema
from tildes.views.decorators import use_kwargs
@use_kwargs(TopicSchema(only=("topic_id36",)), locations=("matchdict",))
@use_kwargs(TopicSchema(only=("topic_id36",)), location="matchdict")
def topic_by_id36(request: Request, topic_id36: str) -> Topic:
"""Get a topic specified by {topic_id36} in the route (or 404)."""
try:
......
......@@ -4,14 +4,14 @@
"""Root factories for users."""
from pyramid.request import Request
from webargs.pyramidparser import use_kwargs
from tildes.models.user import User
from tildes.resources import get_resource
from tildes.schemas.user import UserSchema
from tildes.views.decorators import use_kwargs
@use_kwargs(UserSchema(only=("username",)), locations=("matchdict",))
@use_kwargs(UserSchema(only=("username",)), location="matchdict")
def user_by_username(request: Request, username: str) -> User:
"""Get a user specified by {username} in the route or 404 if not found."""
query = request.query(User).include_deleted().filter(User.username == username)
......
......@@ -47,10 +47,14 @@ class GroupSchema(Schema):
if not self.context.get("fix_path_capitalization"):
return data
if "path" in data and isinstance(data["path"], str):
data["path"] = data["path"].lower()
if "path" not in data or not isinstance(data["path"], str):
return data
new_data = data.copy()
return data
new_data["path"] = new_data["path"].lower()
return new_data
@validates("path")
def validate_path(self, value: sqlalchemy_utils.Ltree) -> None:
......@@ -71,11 +75,13 @@ class GroupSchema(Schema):
if "sidebar_markdown" not in data:
return data
new_data = data.copy()
# if the value is empty, convert it to None
if not data["sidebar_markdown"] or data["sidebar_markdown"].isspace():
data["sidebar_markdown"] = None
if not new_data["sidebar_markdown"] or new_data["sidebar_markdown"].isspace():
new_data["sidebar_markdown"] = None
return data
return new_data
def is_valid_group_path(path: str) -> bool:
......
......@@ -46,10 +46,12 @@ class TopicListingSchema(PaginatedListingSchema):
if "rank_start" not in self.fields:
return data
if not (data.get("before") or data.get("after")):
data["n"] = 1
new_data = data.copy()
return data
if not (new_data.get("before") or new_data.get("after")):
new_data["n"] = 1
return new_data
class MixedListingSchema(PaginatedListingSchema):
......@@ -71,10 +73,12 @@ class MixedListingSchema(PaginatedListingSchema):
to the topic with ID36 "123". "c-123" also works, for comments.
"""
# pylint: disable=unused-argument
new_data = data.copy()
keys = ("after", "before")
for key in keys:
value = data.get(key)
value = new_data.get(key)
if not value:
continue
......@@ -83,12 +87,12 @@ class MixedListingSchema(PaginatedListingSchema):
continue
if type_char == "c":
data["anchor_type"] = "comment"
new_data["anchor_type"] = "comment"
elif type_char == "t":
data["anchor_type"] = "topic"
new_data["anchor_type"] = "topic"
else:
continue
data[key] = id36
new_data[key] = id36
return data
return new_data
......@@ -43,10 +43,12 @@ class TopicSchema(Schema):
if "title" not in data:
return data
new_data = data.copy()
# strip any trailing periods
data["title"] = data["title"].rstrip(".")
new_data["title"] = new_data["title"].rstrip(".")
return data
return new_data
@pre_load
def prepare_tags(self, data: dict, many: bool, partial: Any) -> dict:
......@@ -55,9 +57,11 @@ class TopicSchema(Schema):
if "tags" not in data:
return data
new_data = data.copy()
tags: typing.List[str] = []
for tag in data["tags"]:
for tag in new_data["tags"]:
tag = tag.lower()
# replace underscores with spaces
......@@ -84,9 +88,9 @@ class TopicSchema(Schema):
tags.append(tag)
data["tags"] = tags
new_data["tags"] = tags
return data
return new_data
@validates("tags")
def validate_tags(self, value: typing.List[str]) -> None:
......@@ -112,11 +116,13 @@ class TopicSchema(Schema):
if "markdown" not in data:
return data
new_data = data.copy()
# if the value is empty, convert it to None
if not data["markdown"] or data["markdown"].isspace():
data["markdown"] = None
if not new_data["markdown"] or new_data["markdown"].isspace():
new_data["markdown"] = None
return data
return new_data
@pre_load
def prepare_link(self, data: dict, many: bool, partial: Any) -> dict:
......@@ -125,23 +131,25 @@ class TopicSchema(Schema):
if "link" not in data:
return data
new_data = data.copy()
# remove leading/trailing whitespace
data["link"] = data["link"].strip()
new_data["link"] = new_data["link"].strip()
# if the value is empty, convert it to None
if not data["link"]:
data["link"] = None
return data
if not new_data["link"]:
new_data["link"] = None
return new_data
# prepend http:// to the link if it doesn't have a scheme
parsed = urlparse(data["link"])
parsed = urlparse(new_data["link"])
if not parsed.scheme:
data["link"] = "http://" + data["link"]
new_data["link"] = "http://" + new_data["link"]
# run the link through the url-transformation process
data["link"] = apply_url_transformations(data["link"])
new_data["link"] = apply_url_transformations(new_data["link"])
return data
return new_data
@validates_schema
def link_or_markdown(self, data: dict, many: bool, partial: Any) -> None:
......
......@@ -63,10 +63,17 @@ class UserSchema(Schema):
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"] = "<unknown>"
if not self.context.get("hide_username"):
return data
if "username" not in data:
return data
new_data = data.copy()
return data
new_data["username"] = "<unknown>"
return new_data
@validates_schema
def username_pass_not_substrings(
......@@ -116,9 +123,11 @@ class UserSchema(Schema):
if "username" not in data:
return data
data["username"] = data["username"].strip()
new_data = data.copy()
new_data["username"] = new_data["username"].strip()
return data
return new_data
@pre_load
def prepare_email_address(self, data: dict, many: bool, partial: Any) -> dict:
......@@ -127,14 +136,16 @@ class UserSchema(Schema):
if "email_address" not in data:
return data
new_data = data.copy()
# remove any leading/trailing whitespace
data["email_address"] = data["email_address"].strip()
new_data["email_address"] = new_data["email_address"].strip()
# if the value is empty, convert it to None
if not data["email_address"] or data["email_address"].isspace():
data["email_address"] = None
if not new_data["email_address"] or new_data["email_address"].isspace():
new_data["email_address"] = None
return data
return new_data
@pre_load
def prepare_bio_markdown(self, data: dict, many: bool, partial: Any) -> dict:
......@@ -143,11 +154,13 @@ class UserSchema(Schema):
if "bio_markdown" not in data:
return data
new_data = data.copy()
# if the value is empty, convert it to None
if not data["bio_markdown"] or data["bio_markdown"].isspace():
data["bio_markdown"] = None
if not new_data["bio_markdown"] or new_data["bio_markdown"].isspace():
new_data["bio_markdown"] = None
return data
return new_data
def is_valid_username(username: str) -> bool:
......
......@@ -9,7 +9,6 @@ from pyramid.request import Request
from pyramid.response import Response
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm.exc import FlushError
from webargs.pyramidparser import use_kwargs
from tildes.enums import CommentLabelOption, CommentNotificationType, LogEventType
from tildes.models.comment import (
......@@ -22,7 +21,7 @@ from tildes.models.comment import (
from tildes.models.log import LogComment
from tildes.schemas.comment import CommentLabelSchema, CommentSchema
from tildes.views import IC_NOOP
from tildes.views.decorators import ic_view_config, rate_limit_view
from tildes.views.decorators import ic_view_config, rate_limit_view, use_kwargs
def _mark_comment_read_from_interaction(request: Request, comment: Comment) -> None:
......@@ -47,7 +46,7 @@ def _mark_comment_read_from_interaction(request: Request, comment: Comment) -> N
renderer="single_comment.jinja2",
permission="comment",
)
@use_kwargs(CommentSchema(only=("markdown",)))
@use_kwargs(CommentSchema(only=("markdown",)), location="form")
@rate_limit_view("comment_post")
def post_toplevel_comment(request: Request, markdown: str) -> dict:
"""Post a new top-level comment on a topic with Intercooler."""
......@@ -83,7 +82,7 @@ def post_toplevel_comment(request: Request, markdown: str) -> dict:
renderer="single_comment.jinja2",
permission="reply",
)
@use_kwargs(CommentSchema(only=("markdown",)))
@use_kwargs(CommentSchema(only=("markdown",)), location="form")
@rate_limit_view("comment_post")
def post_comment_reply(request: Request, markdown: str) -> dict:
"""Post a reply to a comment with Intercooler."""
......@@ -159,7 +158,7 @@ def get_comment_edit(request: Request) -> dict:
renderer="comment_contents.jinja2",
permission="edit",
)
@use_kwargs(CommentSchema(only=("markdown",)))
@use_kwargs(CommentSchema(only=("markdown",)), location="form")
def patch_comment(request: Request, markdown: str) -> dict:
"""Update a comment with Intercooler."""
comment = request.context
......@@ -260,9 +259,8 @@ def delete_vote_comment(request: Request) -> dict:
permission="label",
renderer="comment_contents.jinja2",
)
@use_kwargs(CommentLabelSchema(only=("name",)), locations=("matchdict",))
# need to specify only "form" location for reason, or it will crash by looking for JSON
@use_kwargs(CommentLabelSchema(only=("reason",)), locations=("form",))
@use_kwargs(CommentLabelSchema(only=("name",)), location="matchdict")
@use_kwargs(CommentLabelSchema(only=("reason",)), location="form")
def put_label_comment(
request: Request, name: CommentLabelOption, reason: str
) -> Response:
......@@ -308,7 +306,7 @@ def put_label_comment(
permission="label",
renderer="comment_contents.jinja2",
)
@use_kwargs(CommentLabelSchema(only=("name",)), locations=("matchdict",))
@use_kwargs(CommentLabelSchema(only=("name",)), location="matchdict")
def delete_label_comment(request: Request, name: CommentLabelOption) -> Response:
"""Remove a label (that the user previously added) from a comment."""
comment = request.context
......@@ -337,7 +335,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)}, locations=("query",))
@use_kwargs({"mark_all_previous": Boolean(missing=False)}, location="query")
def put_mark_comments_read(request: Request, mark_all_previous: bool) -> Response:
"""Mark comment(s) read, clearing notifications.
......
......@@ -8,7 +8,6 @@ from typing import Optional
from pyramid.request import Request
from sqlalchemy.dialects.postgresql import insert
from sqlalchemy.exc import IntegrityError
from webargs.pyramidparser import use_kwargs
from zope.sqlalchemy import mark_changed
from tildes.enums import TopicSortOption
......@@ -17,7 +16,7 @@ from tildes.models.group import Group, GroupSubscription
from tildes.models.user import UserGroupSettings
from tildes.schemas.fields import Enum, ShortTimePeriod
from tildes.views import IC_NOOP
from tildes.views.decorators import ic_view_config
from tildes.views.decorators import ic_view_config, use_kwargs
@ic_view_config(
......@@ -89,7 +88,7 @@ def delete_subscribe_group(request: Request) -> dict:
"order": Enum(TopicSortOption),
"period": ShortTimePeriod(allow_none=True, missing=None),
},
locations=("form",), # will crash due to trying to find JSON data without this
location="form",
)
def patch_group_user_settings(
request: Request, order: TopicSortOption, period: Optional[SimpleHoursPeriod]
......
......@@ -4,11 +4,10 @@
"""Web API endpoint for previewing Markdown."""
from pyramid.request import Request
from webargs.pyramidparser import use_kwargs
from tildes.lib.markdown import convert_markdown_to_safe_html
from tildes.schemas.group_wiki_page import GroupWikiPageSchema
from tildes.views.decorators import ic_view_config
from tildes.views.decorators import ic_view_config, use_kwargs
@ic_view_config(
......@@ -17,7 +16,7 @@ from tildes.views.decorators import ic_view_config
renderer="markdown_preview.jinja2",
)
# uses GroupWikiPageSchema because it should always have the highest max_length
@use_kwargs(GroupWikiPageSchema(only=("markdown",)))
@use_kwargs(GroupWikiPageSchema(only=("markdown",)), location="form")
def markdown_preview(request: Request, markdown: str) -> dict:
"""Render the provided text as Markdown."""
# pylint: disable=unused-argument
......
......@@ -4,11 +4,10 @@
"""Web API endpoints related to messages."""
from pyramid.request import Request
from webargs.pyramidparser import use_kwargs
from tildes.models.message import MessageReply
from tildes.schemas.message import MessageReplySchema
from tildes.views.decorators import ic_view_config
from tildes.views.decorators import ic_view_config, use_kwargs
@ic_view_config(
......@@ -17,7 +16,7 @@ from tildes.views.decorators import ic_view_config
renderer="single_message.jinja2",
permission="reply",
)
@use_kwargs(MessageReplySchema(only=("markdown",)))
@use_kwargs(MessageReplySchema(only=("markdown",)), location="form")
def post_message_reply(request: Request, markdown: str) -> dict:
"""Post a reply to a message conversation with Intercooler."""
conversation = request.context
......
......@@ -9,7 +9,6 @@ from pyramid.httpexceptions import HTTPNotFound
from pyramid.request import Request
from pyramid.response import Response
from sqlalchemy.exc import IntegrityError
from webargs.pyramidparser import use_kwargs
from tildes.enums import LogEventType
from tildes.models.group import Group
......@@ -18,7 +17,7 @@ from tildes.models.topic import Topic, TopicBookmark, TopicIgnore, TopicVote
from tildes.schemas.group import GroupSchema
from tildes.schemas.topic import TopicSchema
from tildes.views import IC_NOOP
from tildes.views.decorators import ic_view_config
from tildes.views.decorators import ic_view_config, use_kwargs
@ic_view_config(
......@@ -50,7 +49,7 @@ def get_topic_contents(request: Request) -> dict:
renderer="topic_contents.jinja2",
permission="edit",
)
@use_kwargs(TopicSchema(only=("markdown",)))
@use_kwargs(TopicSchema(only=("markdown",)), location="form")
def patch_topic(request: Request, markdown: str) -> dict:
"""Update a topic with Intercooler."""
topic = request.context
......@@ -158,7 +157,9 @@ def get_topic_tags(request: Request) -> dict:
renderer="topic_tags.jinja2",
permission="tag",
)
@use_kwargs({"tags": String(missing=""), "conflict_check": String(missing="")})
@use_kwargs(
{"tags": String(missing=""), "conflict_check": String(missing="")}, location="form"
)
def put_tag_topic(request: Request, tags: str, conflict_check: str) -> dict:
"""Apply tags to a topic with Intercooler."""
topic = request.context
......@@ -225,7 +226,7 @@ def get_topic_group(request: Request) -> dict:
request_method="PATCH",
permission="move",
)
@use_kwargs(GroupSchema(only=("path",)))
@use_kwargs(GroupSchema(only=("path",)), location="form")
def patch_move_topic(request: Request, path: str) -> dict:
"""Move a topic to a different group with Intercooler."""
topic = request.context
......@@ -334,7 +335,7 @@ def get_topic_title(request: Request) -> dict:
request_method="PATCH",
permission="edit_title",
)
@use_kwargs(TopicSchema(only=("title",)))
@use_kwargs(TopicSchema(only=("title",)), location="form")
def patch_topic_title(request: Request, title: str) -> dict:
"""Edit a topic's title with Intercooler."""
topic = request.context
......@@ -373,7 +374,7 @@ def get_topic_link(request: Request) -> dict:
request_method="PATCH",
permission="edit_link",
)
@use_kwargs(TopicSchema(only=("link",)))
@use_kwargs(TopicSchema(only=("link",)), location="form")
def patch_topic_link(request: Request, link: str) -> dict:
"""Edit a topic's link with Intercooler."""
topic = request.context
......
......@@ -17,7 +17,6 @@ from pyramid.httpexceptions import (
from pyramid.request import Request
from pyramid.response import Response
from sqlalchemy.exc import IntegrityError
from webargs.pyramidparser import use_kwargs
from tildes.enums import LogEventType, TopicSortOption
from tildes.lib.datetime import SimpleHoursPeriod
......@@ -28,7 +27,7 @@ from tildes.schemas.fields import Enum, ShortTimePeriod
from tildes.schemas.topic import TopicSchema
from tildes.schemas.user import UserSchema