OpenAPI - Distinguish between stored procedure parameters and result set columns

[seantleonard]

Why make this change?

• Closes [Bug]: Inconsistent Example Schema for Request Body of Stored-Procedure in Swagger UI #1509

What is this change?

• The OpenApiDocumentor service now correctly distinguishes between input parameters and result set columns when documenting a stored procedures REST endpoint. Previously, only the result set columns were resolved. As a result, the OpenApi document that is generated now shows input parameters as fields to be used in the request bodies of POST, PUT, and PATCH. (GET and DELETE have no request bodies).

How was this tested?

• Integration Tests - Added tests with additional stored procedures in our sql deployment script which demonstrates how parameters and output result set columns (and their JSON data types) are resolved and included in the open api doc.

OpenAPI document example

An example OpenAPI document which helps visualize how the parsing works. Irrelevant fields were removed.

{
    "openapi": "3.0.1",
    "paths": {
        "/sp1": {
            "get": {"..."},
            "post": {
                "tags": [
                    "sp1"
                ],
                "description": "Executes a stored procedure.",
                "requestBody": {
                    "content": {
                        "application/json": {
                            "schema": {
                                "$ref": "#/components/schemas/sp1_sp_request"
                            }
                        }
                    },
                    "required": true
                },
                "responses": {
                    "201": {
                        "description": "Created",
                        "content": {
                            "application/json": {
                                "schema": {
                                    "properties": {
                                        "value": {
                                            "type": "array",
                                            "items": {
                                                "$ref": "#/components/schemas/sp1_sp_response"
                                            }
                                        }
                                    }
                                }
                            }
                        }
                    },
                    "400": {
                        "description": "BadRequest"
                    }
                }
            }
		}
    },
    "components": {
        "schemas": {
            "sp1_sp_request": {
                "type": "object",
                "properties": {
                    "inputName": {
                        "type": "string",
                        "format": ""
                    }
                }
            },
            "sp1_sp_response": {
                "type": "object",
                "properties": {
                    "outputName": {
                        "type": "string",
                        "format": ""
                    }
                }
            }
        }
    }
}

[Aniruddh25]

Looks good! Left few minor suggestions.

[abhishekkumams]

you mentioned in PR description that GET and DELETE won't have request bodies, but they can have input params, right? So, why not request bodies?

[seantleonard]

you mentioned in PR description that GET and DELETE won't have request bodies, but they can have input params, right? So, why not request bodies?

Request bodies in those two request types are not convention per HTTP spec:

1. GET https://www.rfc-editor.org/rfc/rfc9110.html#section-9.3.1-6
2. DELETE https://www.rfc-editor.org/rfc/rfc9110.html#section-9.3.5-6

Relevant text with my annotation in square brackets:

A client SHOULD NOT generate content in a [GET/DELETE] request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported.

[severussundar]

LGTM

[ayush3797]

Nit: The comment here:

data-api-builder/src/Core/Services/OpenAPI/OpenApiDocumentor.cs

Line 169 in 73716e7

// Entities which disable their REST endpoint must not be included in

is repeated at line 178.

[ayush3797]

It might not be relevant for this PR but I think we default stored procedures to atleast support POST operation if no method is defined. This doesn't seem to be the case here:

data-api-builder/src/Core/Services/OpenAPI/OpenApiDocumentor.cs

Line 456 in 73716e7

if (spRestMethods is null)

[seantleonard]

It might not be relevant for this PR but I think we default stored procedures to atleast support POST operation if no method is defined. This doesn't seem to be the case here:

data-api-builder/src/Core/Services/OpenAPI/OpenApiDocumentor.cs

Line 456 in 73716e7

if (spRestMethods is null)

Good catch. The sp default methods are added (POST) when RuntimeEntities is created and ProcessRestDefaults() is executed. This also looks to be artifacts of old config system where the collection may be null. The nullable annotation and null check are not necessary.