From 613d8d198d034f1c3fe5cce0cb60f7421fbe1824 Mon Sep 17 00:00:00 2001 From: Michael Chouinard <46358556+chouinar@users.noreply.github.com> Date: Fri, 7 Jun 2024 10:20:47 -0400 Subject: [PATCH] [Issue #2066] Remove the BASE_RESPONSE_SCHEMA (navapbc/simpler-grants-gov#70) Fixes #2066 Remove the `BASE_RESPONSE_SCHEMA` configuration. Adjust the healthcheck endpoint to be consistent in defining the schema / responses with the other endpoints. APIFlask allows you to set a `BASE_RESPONSE_SCHEMA` - the idea is that its the shared schema that every API endpoint will return, and they will only differ in the `data` object within. This sounds great on paper as it prevents you from needing to define most of the response schema for many endpoints, but in practice, its clunky to use. If we want to modify anything in the response schema outside of the `data` object, it will affect every single endpoint. This means when we add something like the pagination info to our search endpoint, a pagination object appears on the healthcheck response. As we intend to make these docs something the public will use, this is going to be confusing. There is also a "bug" (unsure if it is as it was an intended change a few months ago in APIFlask/apispec) that the error response objects end up nested within themselves in the examples. For example, currently the error response for the healthcheck endpoint (which can literally only return a 5xx error) has an example response of: ```json { "data": { "data": "string", "errors": [ { "field": "string", "message": "string", "type": "string" } ], "message": "string", "status_code": 0 }, "message": "string", "pagination_info": { "order_by": "id", "page_offset": 1, "page_size": 25, "sort_direction": "ascending", "total_pages": 2, "total_records": 42 }, "status_code": 0, "warnings": [ { "field": "string", "message": "string", "type": "string" } ] } ``` When in reality, the error response it actually returns looks like: ```json { "data": {}, "errors": [], "message": "Service Unavailable", "status_code": 503 } ``` This set of changes works around all of these confusing issues and just requires us to define specific response schemas for each endpoint with some small set of details. I've kept some base schema classes to derive from that we can use in most cases. Before & After Examples in OpenAPI
EndpointBeforeAfterActual Response (before change)
GET /health (200) ```json { "data": { "message": "string" }, "message": "string", "pagination_info": { "order_by": "id", "page_offset": 1, "page_size": 25, "sort_direction": "ascending", "total_pages": 2, "total_records": 42 }, "status_code": 0, "warnings": [ { "field": "string", "message": "string", "type": "string" } ] } ``` ```json { "data": null, "message": "Success", "status_code": 200 } ``` ```json { "data": {}, "message": "Service healthy", "pagination_info": null, "status_code": 200, "warnings": [] } ```
GET /health (503) ```json { "data": { "data": "string", "errors": [ { "field": "string", "message": "string", "type": "string" } ], "message": "string", "status_code": 0 }, "message": "string", "pagination_info": { "order_by": "id", "page_offset": 1, "page_size": 25, "sort_direction": "ascending", "total_pages": 2, "total_records": 42 }, "status_code": 0, "warnings": [ { "field": "string", "message": "string", "type": "string" } ] } ``` ```json { "data": {}, "errors": [], "message": "Error", "status_code": 0 } ``` ```json { "data": {}, "message": "Service unavailable", "pagination_info": null, "status_code": 200, "warnings": [] } ```
POST /v0.1/opportunities/search (200) ```json { "data": [ {.. Excluding for brevity } ], "message": "string", "pagination_info": { "order_by": "id", "page_offset": 1, "page_size": 25, "sort_direction": "ascending", "total_pages": 2, "total_records": 42 }, "status_code": 0, "warnings": [ { "field": "string", "message": "string", "type": "string" } ] } ``` ```json { "data": [ {.. Excluding for brevity } ], "message": "Success", "pagination_info": { "order_by": "id", "page_offset": 1, "page_size": 25, "sort_direction": "ascending", "total_pages": 2, "total_records": 42 }, "status_code": 200 } ``` ```json { "data": [ {}, {}, {} // excluded for brevity ], "message": "Success", "pagination_info": { "order_by": "opportunity_id", "page_offset": 1, "page_size": 3, "sort_direction": "ascending", "total_pages": 1010, "total_records": 3030 }, "status_code": 200, "warnings": [] } ```
POST /v0.1/opportunities/search (401 or 422) ```json { "data": { "data": "string", "errors": [ { "field": "string", "message": "string", "type": "string" } ], "message": "string", "status_code": 0 }, "message": "string", "pagination_info": { "order_by": "id", "page_offset": 1, "page_size": 25, "sort_direction": "ascending", "total_pages": 2, "total_records": 42 }, "status_code": 0, "warnings": [ { "field": "string", "message": "string", "type": "string" } ] } ``` ```json { "data": {}, "errors": [], "message": "Error", "status_code": 0 } ``` ```json { "data": {}, "errors": [], "message": "The server could not verify that you are authorized to access the URL requested", "status_code": 401 } ```
--------- Co-authored-by: nava-platform-bot --- api/openapi.generated.yml | 656 +++++------------- api/src/api/healthcheck.py | 28 +- .../opportunities_v0/opportunity_routes.py | 4 +- .../opportunities_v0/opportunity_schemas.py | 9 + .../opportunities_v0_1/opportunity_routes.py | 4 +- .../opportunities_v0_1/opportunity_schemas.py | 9 + .../opportunities_v1/opportunity_routes.py | 4 +- .../opportunities_v1/opportunity_schemas.py | 9 + api/src/api/schemas/response_schema.py | 47 +- api/src/app.py | 2 +- api/tests/src/api/test_healthcheck.py | 6 +- api/tests/src/api/test_route_error_format.py | 8 +- documentation/api/api-details.md | 10 +- 13 files changed, 285 insertions(+), 511 deletions(-) diff --git a/api/openapi.generated.yml b/api/openapi.generated.yml index 7302ded44..b7fa2ffa0 100644 --- a/api/openapi.generated.yml +++ b/api/openapi.generated.yml @@ -34,55 +34,13 @@ paths: content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/Healthcheck' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: &id001 - - object - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: &id002 - - object - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/HealthcheckResponse' description: Successful response '503': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id001 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id002 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Service Unavailable tags: - Health @@ -101,83 +59,19 @@ paths: content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - type: array - items: - $ref: '#/components/schemas/OpportunityV0' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: &id003 - - object - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: &id004 - - object - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/OpportunitySearchResponseV0' description: Successful response '422': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id003 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id004 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Validation error '401': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id003 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id004 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Authentication error tags: - Opportunity v0 @@ -213,83 +107,19 @@ paths: content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - type: array - items: - $ref: '#/components/schemas/OpportunityV1' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: &id005 - - object - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: &id006 - - object - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/OpportunitySearchResponseV1' description: Successful response '422': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id005 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id006 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Validation error '401': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id005 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id006 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Authentication error tags: - Opportunity v1 @@ -325,83 +155,19 @@ paths: content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - type: array - items: - $ref: '#/components/schemas/OpportunityV01' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: &id007 - - object - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: &id008 - - object - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/OpportunitySearchResponseV01' description: Successful response '422': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id007 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id008 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Validation error '401': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id007 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id008 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Authentication error tags: - Opportunity v0.1 @@ -483,81 +249,19 @@ paths: content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/OpportunityV0' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: &id009 - - object - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: &id010 - - object - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/OpportunityGetResponseV0' description: Successful response '401': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id009 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id010 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Authentication error '404': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id009 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id010 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Not found tags: - Opportunity v0 @@ -593,81 +297,19 @@ paths: content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/OpportunityV1' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: &id011 - - object - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: &id012 - - object - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/OpportunityGetResponseV1' description: Successful response '401': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id011 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id012 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Authentication error '404': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id011 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id012 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Not found tags: - Opportunity v1 @@ -703,81 +345,19 @@ paths: content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/OpportunityV01' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: &id013 - - object - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: &id014 - - object - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/OpportunityGetResponseV01' description: Successful response '401': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id013 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id014 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Authentication error '404': content: application/json: schema: - type: object - properties: - message: - type: string - description: The message to return - data: - $ref: '#/components/schemas/ErrorResponse' - status_code: - type: integer - description: The HTTP status code - pagination_info: - description: The pagination information for paginated endpoints - type: *id013 - allOf: - - $ref: '#/components/schemas/PaginationInfo' - warnings: - type: array - items: - type: *id014 - allOf: - - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ErrorResponse' description: Not found tags: - Opportunity v0.1 @@ -803,66 +383,51 @@ paths: openapi: 3.1.0 components: schemas: - PaginationInfo: + HealthcheckResponse: type: object properties: - page_offset: - type: integer - description: The page number that was fetched - example: 1 - page_size: - type: integer - description: The size of the page fetched - example: 25 - total_records: - type: integer - description: The total number of records fetchable - example: 42 - total_pages: - type: integer - description: The total number of pages that can be fetched - example: 2 - order_by: + message: type: string - description: The field that the records were sorted by - example: id - sort_direction: - description: The direction the records are sorted - enum: - - ascending - - descending - type: - - string + description: The message to return + example: Success + data: + example: null + status_code: + type: integer + description: The HTTP status code + example: 200 ValidationIssue: type: object properties: type: type: string description: The type of error + example: invalid message: type: string description: The message to return + example: Not a valid string. field: type: string description: The field that failed - Healthcheck: - type: object - properties: - message: - type: string + example: summary.summary_description ErrorResponse: type: object properties: + data: + description: Additional data that might be useful in resolving an error + (see specific endpoints for details, this is used infrequently) + example: {} message: type: string - description: The message to return - data: - description: The REST resource object + description: General description of the error + example: Error status_code: type: integer - description: The HTTP status code + description: The HTTP status code of the error errors: type: array + example: [] items: type: - object @@ -938,6 +503,36 @@ components: required: - paging - sorting + PaginationInfo: + type: object + properties: + page_offset: + type: integer + description: The page number that was fetched + example: 1 + page_size: + type: integer + description: The size of the page fetched + example: 25 + total_records: + type: integer + description: The total number of records fetchable + example: 42 + total_pages: + type: integer + description: The total number of pages that can be fetched + example: 2 + order_by: + type: string + description: The field that the records were sorted by + example: id + sort_direction: + description: The direction the records are sorted + enum: + - ascending + - descending + type: + - string OpportunityV0: type: object properties: @@ -991,6 +586,27 @@ components: type: string format: date-time readOnly: true + OpportunitySearchResponseV0: + type: object + properties: + pagination_info: + description: The pagination information for paginated endpoints + type: &id001 + - object + allOf: + - $ref: '#/components/schemas/PaginationInfo' + message: + type: string + description: The message to return + example: Success + data: + type: array + items: + $ref: '#/components/schemas/OpportunityV0' + status_code: + type: integer + description: The HTTP status code + example: 200 FundingInstrumentFilterV1: type: object properties: @@ -1440,6 +1056,26 @@ components: type: string format: date-time readOnly: true + OpportunitySearchResponseV1: + type: object + properties: + pagination_info: + description: The pagination information for paginated endpoints + type: *id001 + allOf: + - $ref: '#/components/schemas/PaginationInfo' + message: + type: string + description: The message to return + example: Success + data: + type: array + items: + $ref: '#/components/schemas/OpportunityV1' + status_code: + type: integer + description: The HTTP status code + example: 200 FundingInstrumentFilterV01: type: object properties: @@ -1889,6 +1525,74 @@ components: type: string format: date-time readOnly: true + OpportunitySearchResponseV01: + type: object + properties: + pagination_info: + description: The pagination information for paginated endpoints + type: *id001 + allOf: + - $ref: '#/components/schemas/PaginationInfo' + message: + type: string + description: The message to return + example: Success + data: + type: array + items: + $ref: '#/components/schemas/OpportunityV01' + status_code: + type: integer + description: The HTTP status code + example: 200 + OpportunityGetResponseV0: + type: object + properties: + message: + type: string + description: The message to return + example: Success + data: + type: + - object + allOf: + - $ref: '#/components/schemas/OpportunityV0' + status_code: + type: integer + description: The HTTP status code + example: 200 + OpportunityGetResponseV1: + type: object + properties: + message: + type: string + description: The message to return + example: Success + data: + type: + - object + allOf: + - $ref: '#/components/schemas/OpportunityV1' + status_code: + type: integer + description: The HTTP status code + example: 200 + OpportunityGetResponseV01: + type: object + properties: + message: + type: string + description: The message to return + example: Success + data: + type: + - object + allOf: + - $ref: '#/components/schemas/OpportunityV01' + status_code: + type: integer + description: The HTTP status code + example: 200 securitySchemes: ApiKeyAuth: type: apiKey diff --git a/api/src/api/healthcheck.py b/api/src/api/healthcheck.py index 22f944273..3740a5757 100644 --- a/api/src/api/healthcheck.py +++ b/api/src/api/healthcheck.py @@ -1,33 +1,39 @@ import logging -from typing import Tuple from apiflask import APIBlueprint -from flask import current_app from sqlalchemy import text from werkzeug.exceptions import ServiceUnavailable +import src.adapters.db as db import src.adapters.db.flask_db as flask_db from src.api import response -from src.api.schemas.extension import Schema, fields +from src.api.route_utils import raise_flask_error +from src.api.schemas.extension import fields +from src.api.schemas.response_schema import AbstractResponseSchema logger = logging.getLogger(__name__) -class HealthcheckSchema(Schema): - message = fields.String() +class HealthcheckResponseSchema(AbstractResponseSchema): + # We don't have any data to return with the healthcheck endpoint + data = fields.MixinField(metadata={"example": None}) healthcheck_blueprint = APIBlueprint("healthcheck", __name__, tag="Health") @healthcheck_blueprint.get("/health") -@healthcheck_blueprint.output(HealthcheckSchema) +@healthcheck_blueprint.output(HealthcheckResponseSchema) @healthcheck_blueprint.doc(responses=[200, ServiceUnavailable.code]) -def health() -> Tuple[response.ApiResponse, int]: +@flask_db.with_db_session() +def health(db_session: db.Session) -> response.ApiResponse: try: - with flask_db.get_db(current_app).get_connection() as conn: - assert conn.scalar(text("SELECT 1 AS healthy")) == 1 - return response.ApiResponse(message="Service healthy"), 200 + with db_session.begin(): + if db_session.scalar(text("SELECT 1 AS healthy")) != 1: + raise Exception("Connection to Postgres DB failure") + except Exception: logger.exception("Connection to DB failure") - return response.ApiResponse(message="Service unavailable"), ServiceUnavailable.code + raise_flask_error(ServiceUnavailable.code, message="Service Unavailable") + + return response.ApiResponse(message="Service healthy") diff --git a/api/src/api/opportunities_v0/opportunity_routes.py b/api/src/api/opportunities_v0/opportunity_routes.py index 07d46049d..675b77de8 100644 --- a/api/src/api/opportunities_v0/opportunity_routes.py +++ b/api/src/api/opportunities_v0/opportunity_routes.py @@ -34,7 +34,7 @@ arg_name="feature_flag_config", ) # many=True allows us to return a list of opportunity objects -@opportunity_blueprint.output(opportunity_schemas.OpportunityV0Schema(many=True)) +@opportunity_blueprint.output(opportunity_schemas.OpportunitySearchResponseV0Schema) @opportunity_blueprint.auth_required(api_key_auth) @opportunity_blueprint.doc(description=SHARED_ALPHA_DESCRIPTION) @flask_db.with_db_session() @@ -65,7 +65,7 @@ def opportunity_search( @opportunity_blueprint.get("/opportunities/") -@opportunity_blueprint.output(opportunity_schemas.OpportunityV0Schema) +@opportunity_blueprint.output(opportunity_schemas.OpportunityGetResponseV0Schema) @opportunity_blueprint.auth_required(api_key_auth) @opportunity_blueprint.doc(description=SHARED_ALPHA_DESCRIPTION) @flask_db.with_db_session() diff --git a/api/src/api/opportunities_v0/opportunity_schemas.py b/api/src/api/opportunities_v0/opportunity_schemas.py index 4a9aa6cbe..fd03e80dd 100644 --- a/api/src/api/opportunities_v0/opportunity_schemas.py +++ b/api/src/api/opportunities_v0/opportunity_schemas.py @@ -5,6 +5,7 @@ from src.api.feature_flags.feature_flag import FeatureFlag from src.api.feature_flags.feature_flag_config import FeatureFlagConfig, get_feature_flag_config from src.api.schemas.extension import Schema, fields +from src.api.schemas.response_schema import AbstractResponseSchema, PaginationMixinSchema from src.constants.lookup_constants import OpportunityCategoryLegacy from src.pagination.pagination_schema import PaginationSchema, generate_sorting_schema @@ -113,3 +114,11 @@ def post_load(self, data: dict, **kwargs: Any) -> FeatureFlagConfig: feature_flag_config.enable_opportunity_log_msg = enable_opportunity_log_msg return feature_flag_config + + +class OpportunityGetResponseV0Schema(AbstractResponseSchema): + data = fields.Nested(OpportunityV0Schema()) + + +class OpportunitySearchResponseV0Schema(AbstractResponseSchema, PaginationMixinSchema): + data = fields.Nested(OpportunityV0Schema(many=True)) diff --git a/api/src/api/opportunities_v0_1/opportunity_routes.py b/api/src/api/opportunities_v0_1/opportunity_routes.py index 6ae77d6d0..9006c9455 100644 --- a/api/src/api/opportunities_v0_1/opportunity_routes.py +++ b/api/src/api/opportunities_v0_1/opportunity_routes.py @@ -67,7 +67,7 @@ examples=examples, ) # many=True allows us to return a list of opportunity objects -@opportunity_blueprint.output(opportunity_schemas.OpportunityV01Schema(many=True)) +@opportunity_blueprint.output(opportunity_schemas.OpportunitySearchResponseV01Schema) @opportunity_blueprint.auth_required(api_key_auth) @opportunity_blueprint.doc(description=SHARED_ALPHA_DESCRIPTION) @flask_db.with_db_session() @@ -92,7 +92,7 @@ def opportunity_search(db_session: db.Session, search_params: dict) -> response. @opportunity_blueprint.get("/opportunities/") -@opportunity_blueprint.output(opportunity_schemas.OpportunityV01Schema) +@opportunity_blueprint.output(opportunity_schemas.OpportunityGetResponseV01Schema) @opportunity_blueprint.auth_required(api_key_auth) @opportunity_blueprint.doc(description=SHARED_ALPHA_DESCRIPTION) @flask_db.with_db_session() diff --git a/api/src/api/opportunities_v0_1/opportunity_schemas.py b/api/src/api/opportunities_v0_1/opportunity_schemas.py index a54384660..3e3c6c2fc 100644 --- a/api/src/api/opportunities_v0_1/opportunity_schemas.py +++ b/api/src/api/opportunities_v0_1/opportunity_schemas.py @@ -1,4 +1,5 @@ from src.api.schemas.extension import Schema, fields, validators +from src.api.schemas.response_schema import AbstractResponseSchema, PaginationMixinSchema from src.api.schemas.search_schema import StrSearchSchemaBuilder from src.constants.lookup_constants import ( ApplicantType, @@ -296,3 +297,11 @@ class OpportunitySearchRequestV01Schema(Schema): ), required=True, ) + + +class OpportunityGetResponseV01Schema(AbstractResponseSchema): + data = fields.Nested(OpportunityV01Schema()) + + +class OpportunitySearchResponseV01Schema(AbstractResponseSchema, PaginationMixinSchema): + data = fields.Nested(OpportunityV01Schema(many=True)) diff --git a/api/src/api/opportunities_v1/opportunity_routes.py b/api/src/api/opportunities_v1/opportunity_routes.py index 0d94996b0..9f3e1b319 100644 --- a/api/src/api/opportunities_v1/opportunity_routes.py +++ b/api/src/api/opportunities_v1/opportunity_routes.py @@ -30,7 +30,7 @@ opportunity_schemas.OpportunitySearchRequestV1Schema, arg_name="search_params" ) # many=True allows us to return a list of opportunity objects -@opportunity_blueprint.output(opportunity_schemas.OpportunityV1Schema(many=True)) +@opportunity_blueprint.output(opportunity_schemas.OpportunitySearchResponseV1Schema) @opportunity_blueprint.auth_required(api_key_auth) @opportunity_blueprint.doc(description=SHARED_ALPHA_DESCRIPTION) def opportunity_search(search_params: dict) -> response.ApiResponse: @@ -53,7 +53,7 @@ def opportunity_search(search_params: dict) -> response.ApiResponse: @opportunity_blueprint.get("/opportunities/") -@opportunity_blueprint.output(opportunity_schemas.OpportunityV1Schema) +@opportunity_blueprint.output(opportunity_schemas.OpportunityGetResponseV1Schema) @opportunity_blueprint.auth_required(api_key_auth) @opportunity_blueprint.doc(description=SHARED_ALPHA_DESCRIPTION) @flask_db.with_db_session() diff --git a/api/src/api/opportunities_v1/opportunity_schemas.py b/api/src/api/opportunities_v1/opportunity_schemas.py index 5f72c7958..ebf47ebef 100644 --- a/api/src/api/opportunities_v1/opportunity_schemas.py +++ b/api/src/api/opportunities_v1/opportunity_schemas.py @@ -1,4 +1,5 @@ from src.api.schemas.extension import Schema, fields, validators +from src.api.schemas.response_schema import AbstractResponseSchema, PaginationMixinSchema from src.api.schemas.search_schema import StrSearchSchemaBuilder from src.constants.lookup_constants import ( ApplicantType, @@ -296,3 +297,11 @@ class OpportunitySearchRequestV1Schema(Schema): ), required=True, ) + + +class OpportunityGetResponseV1Schema(AbstractResponseSchema): + data = fields.Nested(OpportunityV1Schema()) + + +class OpportunitySearchResponseV1Schema(AbstractResponseSchema, PaginationMixinSchema): + data = fields.Nested(OpportunityV1Schema(many=True)) diff --git a/api/src/api/schemas/response_schema.py b/api/src/api/schemas/response_schema.py index b6509699b..928ef19ab 100644 --- a/api/src/api/schemas/response_schema.py +++ b/api/src/api/schemas/response_schema.py @@ -3,25 +3,52 @@ class ValidationIssueSchema(Schema): - type = fields.String(metadata={"description": "The type of error"}) - message = fields.String(metadata={"description": "The message to return"}) - field = fields.String(metadata={"description": "The field that failed"}) + type = fields.String(metadata={"description": "The type of error", "example": "invalid"}) + message = fields.String( + metadata={"description": "The message to return", "example": "Not a valid string."} + ) + field = fields.String( + metadata={"description": "The field that failed", "example": "summary.summary_description"} + ) -class BaseResponseSchema(Schema): - message = fields.String(metadata={"description": "The message to return"}) +class AbstractResponseSchema(Schema): + message = fields.String(metadata={"description": "The message to return", "example": "Success"}) data = fields.MixinField(metadata={"description": "The REST resource object"}, dump_default={}) - status_code = fields.Integer(metadata={"description": "The HTTP status code"}, dump_default=200) + status_code = fields.Integer( + metadata={"description": "The HTTP status code", "example": 200}, dump_default=200 + ) -class ErrorResponseSchema(BaseResponseSchema): - errors = fields.List(fields.Nested(ValidationIssueSchema()), dump_default=[]) +class WarningMixinSchema(Schema): + warnings = fields.List( + fields.Nested(ValidationIssueSchema()), + metadata={ + "description": "A list of warnings - indicating something you may want to be aware of, but did not prevent handling of the request" + }, + dump_default=[], + ) -class ResponseSchema(BaseResponseSchema): +class PaginationMixinSchema(Schema): pagination_info = fields.Nested( PaginationInfoSchema(), metadata={"description": "The pagination information for paginated endpoints"}, ) - warnings = fields.List(fields.Nested(ValidationIssueSchema()), dump_default=[]) + +class ErrorResponseSchema(Schema): + data = fields.MixinField( + metadata={ + "description": "Additional data that might be useful in resolving an error (see specific endpoints for details, this is used infrequently)", + "example": {}, + }, + dump_default={}, + ) + message = fields.String( + metadata={"description": "General description of the error", "example": "Error"} + ) + status_code = fields.Integer(metadata={"description": "The HTTP status code of the error"}) + errors = fields.List( + fields.Nested(ValidationIssueSchema()), metadata={"example": []}, dump_default=[] + ) diff --git a/api/src/app.py b/api/src/app.py index b32322faa..98fea1607 100644 --- a/api/src/app.py +++ b/api/src/app.py @@ -67,7 +67,7 @@ def configure_app(app: APIFlask) -> None: # Modify the response schema to instead use the format of our ApiResponse class # which adds additional details to the object. # https://apiflask.com/schema/#base-response-schema-customization - app.config["BASE_RESPONSE_SCHEMA"] = response_schema.ResponseSchema + # app.config["BASE_RESPONSE_SCHEMA"] = response_schema.ResponseSchema app.config["HTTP_ERROR_SCHEMA"] = response_schema.ErrorResponseSchema app.config["VALIDATION_ERROR_SCHEMA"] = response_schema.ErrorResponseSchema app.config["SWAGGER_UI_CSS"] = "/static/swagger-ui.min.css" diff --git a/api/tests/src/api/test_healthcheck.py b/api/tests/src/api/test_healthcheck.py index 1fe7fd533..70e699b99 100644 --- a/api/tests/src/api/test_healthcheck.py +++ b/api/tests/src/api/test_healthcheck.py @@ -4,6 +4,7 @@ def test_get_healthcheck_200(client): response = client.get("/health") assert response.status_code == 200 + assert response.get_json()["message"] == "Service healthy" def test_get_healthcheck_503_db_bad_state(client, monkeypatch): @@ -11,8 +12,9 @@ def test_get_healthcheck_503_db_bad_state(client, monkeypatch): def err_method(*args): raise Exception("Fake Error") - # Mock db.DB.get_session method to fail - monkeypatch.setattr(db.DBClient, "get_connection", err_method) + # Mock db_session.Scalar to fail + monkeypatch.setattr(db.Session, "scalar", err_method) response = client.get("/health") assert response.status_code == 503 + assert response.get_json()["message"] == "Service Unavailable" diff --git a/api/tests/src/api/test_route_error_format.py b/api/tests/src/api/test_route_error_format.py index 0b95f5529..faa025476 100644 --- a/api/tests/src/api/test_route_error_format.py +++ b/api/tests/src/api/test_route_error_format.py @@ -17,6 +17,7 @@ from src.api.response import ApiResponse, ValidationErrorDetail from src.api.route_utils import raise_flask_error from src.api.schemas.extension import Schema, fields +from src.api.schemas.response_schema import AbstractResponseSchema, WarningMixinSchema from src.auth.api_key_auth import api_key_auth from src.util.dict_util import flatten_dict from tests.src.schemas.schema_validation_utils import ( @@ -35,10 +36,14 @@ def header(api_auth_token): return {"X-Auth": api_auth_token} -class OutputSchema(Schema): +class OutputData(Schema): output_val = fields.String() +class OutputSchema(AbstractResponseSchema, WarningMixinSchema): + data = fields.Nested(OutputData()) + + test_blueprint = APIBlueprint("test", __name__, tag="test") @@ -111,7 +116,6 @@ def override(self): assert resp.status_code == 500 resp_json = resp.get_json() - assert resp_json["data"] == {} assert resp_json["errors"] == [] assert resp_json["message"] == "Internal Server Error" diff --git a/documentation/api/api-details.md b/documentation/api/api-details.md index 14ee18ad2..d0cb4f62a 100644 --- a/documentation/api/api-details.md +++ b/documentation/api/api-details.md @@ -42,6 +42,7 @@ We would define the Marshmallow schema in-python like so: ```py from enum import StrEnum from src.api.schemas.extension import Schema, fields, validators +from src.api.schemas.response_schema import AbstractResponseSchema class Suffix(StrEnum): SENIOR = "SR" @@ -65,6 +66,10 @@ class NameSchema(Schema): class ExampleSchema(Schema): name = fields.Nested(NameSchema()) birth_date = fields.Date(metadata={"description": "Their birth date"}) + +class ExampleResponseSchema(AbstractResponseSchema): + # Note that AbstractResponseSchema defines a message and status_code field as well + data = fields.Nested(ExampleSchema()) ``` Anything specified in the metadata field is passed to the OpenAPI file that backs the swagger endpoint. The values @@ -77,9 +82,8 @@ but it's recommended you try to populate the following: You can specify validators that will be run when the request is being serialized by APIFlask Defining a response works the exact same way however field validation does not occur on response, only formatting. -The response schema only dictates the data portion of the response, the rest of the response is defined in -[ResponseSchema](../../api/src/api/schemas/response_schema.py) which is connected to APIFlask via the `BASE_RESPONSE_SCHEMA` config. - +To keep our response schema following a consistent pattern, we have a few base schema classes like [AbstractResponseSchema](../../api/src/api/schemas/response_schema.py) +that you can derive from for shared values like the message. ### Schema tips