From 85f6d58f4826dc3195535f3be775ca66a2f285b8 Mon Sep 17 00:00:00 2001 From: Tim DiLauro Date: Fri, 30 Aug 2024 08:01:27 -0400 Subject: [PATCH] Stop using `flask-pydantic-spec`. (#2014) --- poetry.lock | 31 +-------- pyproject.toml | 2 - .../api/admin/controller/custom_lists.py | 46 ++++++------- src/palace/manager/api/admin/routes.py | 69 +------------------ src/palace/manager/api/app.py | 7 -- src/palace/manager/api/routes.py | 14 +--- src/palace/manager/core/app_server.py | 18 ----- src/palace/manager/util/flask_util.py | 19 +++++ tests/mocks/flask.py | 27 ++++---- 9 files changed, 59 insertions(+), 174 deletions(-) diff --git a/poetry.lock b/poetry.lock index b504d5d78b..0b8b37b54a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1469,24 +1469,6 @@ files = [ [package.dependencies] Flask = ">=0.9" -[[package]] -name = "flask-pydantic-spec" -version = "0.6.0" -description = "generate OpenAPI document and validate request & response with Python annotations." -optional = false -python-versions = ">=3.8" -files = [ - {file = "flask_pydantic_spec-0.6.0-py3-none-any.whl", hash = "sha256:480689102eed43900b2164d07af930221a2379253ed06337d280a47224990035"}, - {file = "flask_pydantic_spec-0.6.0.tar.gz", hash = "sha256:f20d63cba821dfaaa92fa19c1e1975e0acf544ea6a3cc9eb5cbac62a81117790"}, -] - -[package.dependencies] -inflection = ">=0.5.0,<1" -pydantic = ">=1.2,<2" - -[package.extras] -flask = ["flask"] - [[package]] name = "freezegun" version = "1.5.1" @@ -2020,17 +2002,6 @@ files = [ {file = "idna-3.7.tar.gz", hash = "sha256:028ff3aadf0609c1fd278d8ea3089299412a7a8b9bd005dd08b9f8285bcb5cfc"}, ] -[[package]] -name = "inflection" -version = "0.5.1" -description = "A port of Ruby on Rails inflector to Python" -optional = false -python-versions = ">=3.5" -files = [ - {file = "inflection-0.5.1-py2.py3-none-any.whl", hash = "sha256:f38b2b640938a4f35ade69ac3d053042959b62a0f1076a5bbaa1b9526605a8a2"}, - {file = "inflection-0.5.1.tar.gz", hash = "sha256:1a29730d366e996aaacffb2f1f1cb9593dc38e2ddd30c91250c6dde09ea9b417"}, -] - [[package]] name = "iniconfig" version = "2.0.0" @@ -5124,4 +5095,4 @@ lxml = ">=3.8" [metadata] lock-version = "2.0" python-versions = ">=3.10,<4" -content-hash = "b422e5d0530d0abdcf7714a2983e565f263f247622fc8a5ce018c0b6c63465b3" +content-hash = "516641d4707fd026e58036e379fd66853dc84e4af7620e5ce6eb0f2cfe872fcb" diff --git a/pyproject.toml b/pyproject.toml index 25b599a03e..a57480a18d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -177,7 +177,6 @@ module = [ "feedparser", "firebase_admin.*", "flask_babel", - "flask_pydantic_spec.*", "fuzzywuzzy", "google.auth", "greenlet", @@ -234,7 +233,6 @@ firebase-admin = "^6.0.1" Flask = "^3.0" Flask-Babel = "^4.0" Flask-Cors = "4.0.1" -flask-pydantic-spec = "^0.6.0" fuzzywuzzy = "0.18.0" # fuzzywuzzy is for author name manipulations html-sanitizer = "^2.1.0" isbnlib = "^3.10.14" diff --git a/src/palace/manager/api/admin/controller/custom_lists.py b/src/palace/manager/api/admin/controller/custom_lists.py index 7bafb28fbb..2e88419dca 100644 --- a/src/palace/manager/api/admin/controller/custom_lists.py +++ b/src/palace/manager/api/admin/controller/custom_lists.py @@ -7,7 +7,6 @@ import flask from flask import Response, url_for from flask_babel import lazy_gettext as _ -from flask_pydantic_spec.flask_backend import Context from pydantic import BaseModel from palace.manager.api.admin.controller.base import AdminPermissionsControllerMixin @@ -38,6 +37,7 @@ from palace.manager.sqlalchemy.model.licensing import LicensePool from palace.manager.sqlalchemy.model.work import Work from palace.manager.sqlalchemy.util import create, get_one +from palace.manager.util.flask_util import parse_multi_dict from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException @@ -98,28 +98,22 @@ def custom_lists(self) -> dict | ProblemDetail | Response | None: return dict(custom_lists=custom_lists) if flask.request.method == "POST": - ctx: Context = flask.request.context.body # type: ignore + list_ = self.CustomListPostRequest.parse_obj( + parse_multi_dict(flask.request.form) + ) return self._create_or_update_list( library, - ctx.name, - ctx.entries, - ctx.collections, - id=ctx.id, - auto_update=ctx.auto_update, - auto_update_facets=ctx.auto_update_facets, - auto_update_query=ctx.auto_update_query, + list_.name, + list_.entries, + list_.collections, + id=list_.id, + auto_update=list_.auto_update, + auto_update_facets=list_.auto_update_facets, + auto_update_query=list_.auto_update_query, ) return None - def _getJSONFromRequest(self, values: str | None) -> list: - if values: - return_values = json.loads(values) - else: - return_values = [] - - return return_values - def _get_work_from_urn(self, library: Library, urn: str | None) -> Work | None: identifier, ignore = Identifier.parse_urn(self._db, urn) @@ -365,17 +359,19 @@ def custom_list(self, list_id: int) -> Response | dict | ProblemDetail | None: ) elif flask.request.method == "POST": - ctx: Context = flask.request.context.body # type: ignore + list_ = self.CustomListPostRequest.parse_obj( + parse_multi_dict(flask.request.form) + ) return self._create_or_update_list( library, - ctx.name, - ctx.entries, - ctx.collections, - deleted_entries=ctx.deletedEntries, + list_.name, + list_.entries, + list_.collections, + deleted_entries=list_.deletedEntries, id=list_id, - auto_update=ctx.auto_update, - auto_update_query=ctx.auto_update_query, - auto_update_facets=ctx.auto_update_facets, + auto_update=list_.auto_update, + auto_update_query=list_.auto_update_query, + auto_update_facets=list_.auto_update_facets, ) elif flask.request.method == "DELETE": diff --git a/src/palace/manager/api/admin/routes.py b/src/palace/manager/api/admin/routes.py index d0d9d01dd0..48483b782a 100644 --- a/src/palace/manager/api/admin/routes.py +++ b/src/palace/manager/api/admin/routes.py @@ -3,36 +3,19 @@ import flask from flask import Response, make_response, redirect, url_for -from flask_pydantic_spec import FileResponse as SpecFileResponse -from flask_pydantic_spec import Request as SpecRequest -from flask_pydantic_spec import Response as SpecResponse from palace.manager.api.admin.config import Configuration as AdminClientConfig from palace.manager.api.admin.config import OperationalMode -from palace.manager.api.admin.controller.custom_lists import CustomListsController from palace.manager.api.admin.dashboard_stats import generate_statistics from palace.manager.api.admin.model.dashboard_statistics import StatisticsResponse -from palace.manager.api.admin.model.inventory_report import InventoryReportInfo -from palace.manager.api.admin.model.quicksight import ( - QuicksightDashboardNamesResponse, - QuicksightGenerateUrlRequest, - QuicksightGenerateUrlResponse, -) from palace.manager.api.admin.templates import ( admin_sign_in_again as sign_in_again_template, ) -from palace.manager.api.app import api_spec, app +from palace.manager.api.app import app from palace.manager.api.controller.static_file import StaticFileController from palace.manager.api.routes import allows_library, has_library, library_route -from palace.manager.core.app_server import ( - ensure_pydantic_after_problem_detail, - returns_problem_detail, -) -from palace.manager.util.problem_detail import ( - BaseProblemDetailException, - ProblemDetail, - ProblemDetailModel, -) +from palace.manager.core.app_server import returns_problem_detail +from palace.manager.util.problem_detail import BaseProblemDetailException, ProblemDetail # An admin's session will expire after this amount of time and # the admin will have to log in again. @@ -95,8 +78,6 @@ def decorated(*args, **kwargs): def returns_json_or_response_or_problem_detail(f): - ensure_pydantic_after_problem_detail(f) - @wraps(f) def decorated(*args, **kwargs): try: @@ -348,7 +329,6 @@ def circulation_events(): @app.route("/admin/stats") -@api_spec.validate(resp=SpecResponse(HTTP_200=StatisticsResponse), tags=["admin.stats"]) @returns_json_or_response_or_problem_detail @requires_admin def stats(): @@ -359,11 +339,6 @@ def stats(): @app.route("/admin/quicksight_embed/") -@api_spec.validate( - resp=SpecResponse(HTTP_200=QuicksightGenerateUrlResponse), - tags=["admin.quicksight"], - query=QuicksightGenerateUrlRequest, -) @returns_json_or_response_or_problem_detail @requires_admin def generate_quicksight_url(dashboard_name: str): @@ -373,10 +348,6 @@ def generate_quicksight_url(dashboard_name: str): @app.route("/admin/quicksight_embed/names") -@api_spec.validate( - resp=SpecResponse(HTTP_200=QuicksightDashboardNamesResponse), - tags=["admin.quicksight"], -) @returns_json_or_response_or_problem_detail @requires_admin def get_quicksight_names(): @@ -573,11 +544,6 @@ def discovery_service_library_registrations(): @library_route("/admin/custom_lists", methods=["POST"]) -@api_spec.validate( - resp=SpecFileResponse(content_type="application/atom+xml"), - body=SpecRequest(CustomListsController.CustomListPostRequest), - tags=["admin.customlists"], -) @has_library @returns_json_or_response_or_problem_detail @requires_admin @@ -587,10 +553,6 @@ def custom_lists_post(): @library_route("/admin/custom_lists", methods=["GET"]) -@api_spec.validate( - resp=SpecFileResponse(content_type="application/atom+xml"), - tags=["admin.customlists"], -) @has_library @returns_json_or_response_or_problem_detail @requires_admin @@ -600,10 +562,6 @@ def custom_lists_get(): @library_route("/admin/custom_list/", methods=["GET"]) -@api_spec.validate( - resp=SpecFileResponse(content_type="application/atom+xml"), - tags=["admin.customlists"], -) @has_library @returns_json_or_response_or_problem_detail @requires_admin @@ -613,11 +571,6 @@ def custom_list_get(list_id: int): @library_route("/admin/custom_list/", methods=["POST"]) -@api_spec.validate( - resp=SpecFileResponse(content_type="application/atom+xml"), - body=SpecRequest(CustomListsController.CustomListPostRequest), - tags=["admin.customlists"], -) @has_library @returns_json_or_response_or_problem_detail @requires_admin @@ -636,13 +589,6 @@ def custom_list_delete(list_id): @library_route("/admin/custom_list//share", methods=["POST"]) -@api_spec.validate( - resp=SpecResponse( - HTTP_200=CustomListsController.CustomListSharePostResponse, - HTTP_403=ProblemDetailModel, - ), - tags=["admin.customlists"], -) @has_library @returns_json_or_response_or_problem_detail @requires_admin @@ -653,7 +599,6 @@ def custom_list_share(list_id: int): @library_route("/admin/custom_list//share", methods=["DELETE"]) -@api_spec.validate(resp=SpecResponse(HTTP_204=None), tags=["admin.customlists"]) @has_library @returns_json_or_response_or_problem_detail @requires_admin @@ -736,14 +681,6 @@ def diagnostics(): "/admin/reports/inventory_report/", methods=["GET"], ) -@api_spec.validate( - resp=SpecResponse( - HTTP_200=InventoryReportInfo, - HTTP_403=ProblemDetailModel, - HTTP_404=ProblemDetailModel, - ), - tags=["admin.inventory"], -) @allows_library @returns_json_or_response_or_problem_detail @requires_admin diff --git a/src/palace/manager/api/app.py b/src/palace/manager/api/app.py index d32d2071f4..5e1807cbcd 100644 --- a/src/palace/manager/api/app.py +++ b/src/palace/manager/api/app.py @@ -3,7 +3,6 @@ import flask_babel from flask import request from flask_babel import Babel -from flask_pydantic_spec import FlaskPydanticSpec from sqlalchemy.orm import Session from palace.manager.api.admin.controller import setup_admin_controllers @@ -39,12 +38,6 @@ def get_locale(): app.config["BABEL_TRANSLATION_DIRECTORIES"] = "../translations" babel = Babel(app, locale_selector=get_locale) -# The autodoc spec, can be accessed at "/apidoc/swagger" -api_spec = FlaskPydanticSpec( - "Palace Manager", mode="strict", title="Palace Manager API" -) -api_spec.register(app) - # We use URIs as identifiers throughout the application, meaning that # we never want werkzeug's merge_slashes feature. app.url_map.merge_slashes = False diff --git a/src/palace/manager/api/routes.py b/src/palace/manager/api/routes.py index b04e0baecd..7429e05562 100644 --- a/src/palace/manager/api/routes.py +++ b/src/palace/manager/api/routes.py @@ -3,14 +3,8 @@ import flask from flask import Response, make_response, request from flask_cors.core import get_cors_options, set_cors_headers -from flask_pydantic_spec import Response as SpecResponse -from palace.manager.api.app import api_spec, app -from palace.manager.api.model.patron_auth import PatronAuthAccessToken -from palace.manager.api.model.time_tracking import ( - PlaytimeEntriesPost, - PlaytimeEntriesPostResponse, -) +from palace.manager.api.app import app from palace.manager.core.app_server import compressible, returns_problem_detail from palace.manager.sqlalchemy.hassessioncache import HasSessionCache from palace.manager.util.problem_detail import ProblemDetail @@ -362,7 +356,6 @@ def delete_patron_devices(): @library_dir_route("/patrons/me/token", methods=["POST"]) -@api_spec.validate(resp=SpecResponse(HTTP_200=PatronAuthAccessToken), tags=["patron"]) @has_library @requires_auth @returns_problem_detail @@ -540,11 +533,6 @@ def track_analytics_event(identifier_type, identifier, event_type): ) @has_library @requires_auth -@api_spec.validate( - resp=SpecResponse(HTTP_200=PlaytimeEntriesPostResponse), - body=PlaytimeEntriesPost, - tags=["analytics"], -) @returns_problem_detail def track_playtime_events(collection_id, identifier_type, identifier): """The actual response type is 207, but due to a bug in flask-pydantic-spec we must document it as a 200""" diff --git a/src/palace/manager/core/app_server.py b/src/palace/manager/core/app_server.py index f538099f2b..630b697440 100644 --- a/src/palace/manager/core/app_server.py +++ b/src/palace/manager/core/app_server.py @@ -8,7 +8,6 @@ import flask from flask import Response, make_response, url_for -from flask_pydantic_spec import FlaskPydanticSpec from psycopg2 import OperationalError from werkzeug.exceptions import HTTPException @@ -80,24 +79,7 @@ def load_pagination_from_request( return base_class.from_request(get_arg, default_size, **kwargs) -def ensure_pydantic_after_problem_detail(func): - """We must ensure the problem_detail decorators are always placed below the - `spec.validate` decorator because the `spec.validate` decorator will always expect a - tuple or dict-like response, not a ProblemDetail like response. - The problem_detail decorators will convert the ProblemDetail before the response gets validated. - """ - spec = getattr(func, "_decorator", None) - if spec and isinstance(spec, FlaskPydanticSpec): - raise RuntimeError( - "FlaskPydanticSpec MUST be decorated above the problem_detail decorator" - + ", else problem details will throw errors during response validation" - + f": {func}" - ) - - def returns_problem_detail(f): - ensure_pydantic_after_problem_detail(f) - @wraps(f) def decorated(*args, **kwargs): v = f(*args, **kwargs) diff --git a/src/palace/manager/util/flask_util.py b/src/palace/manager/util/flask_util.py index cbf52dbbcb..99bd809aa3 100644 --- a/src/palace/manager/util/flask_util.py +++ b/src/palace/manager/util/flask_util.py @@ -1,12 +1,15 @@ """Utilities for Flask applications.""" import datetime +import json import time +from json import JSONDecodeError from typing import Any from wsgiref.handlers import format_date_time from flask import Response as FlaskResponse from lxml import etree from pydantic import BaseModel, Extra +from werkzeug.datastructures import MultiDict from palace.manager.util import problem_detail from palace.manager.util.datetime_helpers import utc_now @@ -216,3 +219,19 @@ def str_comma_list_validator(value): raise TypeError("string required") return value.split(",") + + +# This code is borrowed from `flask-pydantic-spec +# - https://github.com/turner-townsend/flask-pydantic-spec/blob/2d29e45f428b7e7bee60c1bc3657e95ee1f3a866/flask_pydantic_spec/utils.py#L200-L211 +def parse_multi_dict(input: MultiDict) -> dict[str, Any]: + result = {} + for key, value in input.to_dict(flat=False).items(): + if len(value) == 1: + try: + value_to_use = json.loads(value[0]) + except (TypeError, JSONDecodeError): + value_to_use = value[0] + else: + value_to_use = value + result[key] = value_to_use + return result diff --git a/tests/mocks/flask.py b/tests/mocks/flask.py index 805a806394..7b3aaa8a4c 100644 --- a/tests/mocks/flask.py +++ b/tests/mocks/flask.py @@ -1,19 +1,20 @@ -from flask_pydantic_spec.flask_backend import Context -from flask_pydantic_spec.utils import parse_multi_dict +from pydantic import BaseModel +from werkzeug.datastructures import MultiDict +from palace.manager.util.flask_util import parse_multi_dict -def add_request_context(request, model, form=None) -> None: - """Add a flask pydantic model into the request context - :param model: The pydantic model + +def add_request_context( + request, model: type[BaseModel], form: MultiDict | None = None +) -> None: + """Add form data into the request context. + + Before doing so, we verify that it can be parsed into the Pydantic model. + + :param model: A pydantic model :param form: A form multidict - TODO: - - query params - - json post requests """ - body = None - query = None + if form is not None: + model.parse_obj(parse_multi_dict(form)) request.form = form - body = model.parse_obj(parse_multi_dict(form)) - - request.context = Context(query, body, None, None)