Validate generated JSON schemas

[harry-pledge-io]

Hi there, I wanted to bring up an issue I encountered with Portman that might be worth looking into.

In my OpenAPI spec, I had a field using "types": ["object", "null"] (note the use of "types" instead of "type") along with "nullable": true. Portman generated the contract tests successfully, but the resulting JSON schema was invalid.

It might be helpful if Portman could catch this kind of inconsistency earlier in the process. For example, it could fail during the Postman collection generation with a clear JSON schema validation error. This change could make it easier for users to spot and fix these issues in their OpenAPI specs before they cause problems down the line.

I'm happy to provide more details or a specific example if that would be helpful. Thanks for considering this feedback!

[thim81]

hi @harry-pledge-io

After re-reading your issue, I had to correct my previous post.

So your request, is to detect invalid OpenAPI properties before it gets converted? Or prevent the generation of invalid JSON schema tests?

Follow-up question:

• Would this be "fatal" error and stop Portman conversion OR would it trigger a "warning" and skip the JSON schema validation injection?

[harry-pledge-io]

I was thinking more of the latter as it should be quicker to implement, but the former would be ideal too perhaps as a separate feature.

I would expect any tests the library generates to be valid, therefore the generated JSON schema injected by the library should be valid and, ideally, I should not have to worry about checking this.

To address your follow up question: Since I opted into schemaValidation tests in the Portman config, I would expect this to be a fatal error and stop the conversion. Skipping the JSON schema validation injection with a warning could be easy to miss and produce unexpected behaviour.

[thim81]

I understand your reasoning to leverage contract testing with JSON schema validation, so the schema should be valid.

Typically, we see workflows, where the OpenAPI is validated using Spectral or Vacuum (or any other of the OSS OpenAPI linting tools), before it is handed over to Portman. So that is why we never really got this request before.

Do you have a workflow for your OpenAPI specs with linting, filtering, ..., or just pass the raw OpenAPI file to Portman?

@nicklloyd What do you think of this feature request?

Technically it is straightforward, since AJV is already included in Portman, so we could throw a error when injecting the JSON schema validation.

[harry-pledge-io]

Yeah, I do think it's fair that you expect a valid OpenAPI spec before hand. In my case I am actually using Spectral to validate the spec before passing it to Portman, but perhaps they have an issue on their library because it does not raise any validation errors in this case. Anyway that's another issue.

Here's some reproduction steps using a modified Swagger Petstore example:

{
  "openapi": "3.1.0",
  "info": {
    "version": "1.0.0",
    "title": "Swagger Petstore",
    "license": {
      "name": "MIT"
    }
  },
  "servers": [
    {
      "url": "http://petstore.swagger.io/v1"
    }
  ],
  "paths": {
    "/pets": {
      "get": {
        "summary": "List all pets",
        "operationId": "listPets",
        "tags": ["pets"],
        "parameters": [
          {
            "name": "limit",
            "in": "query",
            "description": "How many items to return at one time (max 100)",
            "required": false,
            "schema": {
              "type": "integer",
              "maximum": 100,
              "format": "int32"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "A paged array of pets",
            "headers": {
              "x-next": {
                "description": "A link to the next page of responses",
                "schema": {
                  "type": "string"
                }
              }
            },
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Pets"
                }
              }
            }
          },
          "default": {
            "description": "unexpected error",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Error"
                }
              }
            }
          }
        }
      },
      "post": {
        "summary": "Create a pet",
        "operationId": "createPets",
        "tags": ["pets"],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/Pet"
              }
            }
          },
          "required": true
        },
        "responses": {
          "201": {
            "description": "Null response"
          },
          "default": {
            "description": "unexpected error",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Error"
                }
              }
            }
          }
        }
      }
    },
    "/pets/{petId}": {
      "get": {
        "summary": "Info for a specific pet",
        "operationId": "showPetById",
        "tags": ["pets"],
        "parameters": [
          {
            "name": "petId",
            "in": "path",
            "required": true,
            "description": "The id of the pet to retrieve",
            "schema": {
              "type": "string"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "Expected response to a valid request",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Pet"
                }
              }
            }
          },
          "default": {
            "description": "unexpected error",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Error"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Pet": {
        "types": ["object", "null"],
        "nullable": true,
        "required": ["id", "name"],
        "properties": {
          "id": {
            "type": "integer",
            "format": "int64"
          },
          "name": {
            "type": "string"
          },
          "tag": {
            "type": "string"
          }
        }
      },
      "Pets": {
        "type": "array",
        "maxItems": 100,
        "items": {
          "$ref": "#/components/schemas/Pet"
        }
      },
      "Error": {
        "type": "object",
        "required": ["code", "message"],
        "properties": {
          "code": {
            "type": "integer",
            "format": "int32"
          },
          "message": {
            "type": "string"
          }
        }
      }
    }
  }
}

Note that the components.schemas.Pet object is missing type and instead using types with a value ["object", "null"] and "nullable": true is added.

1. Run this through spectral linter and it passes (Optional)
2. Add a contract test for the GET pets operation to the Portman config

{
        "openApiOperation": "*::/pets",
        "statusCode": {
          "enabled": true
        },
        "contentType": {
          "enabled": true
        },
        "jsonBody": {
          "enabled": true
        },
        "schemaValidation": {
          "enabled": true,
          "additionalProperties": true
        },
        "headersPresent": {
          "enabled": true
        }
      }

3. Generate the postman collection with Portman
4. Get the generated response JSON schema

   // Response Validation
   const schema = {"type":"array","maxItems":100,"items":{"types":["object","null"],"required":["id","name"],"properties":{"id":{"type":"integer","format":"int64"},"name":{"type":"string"},"tag":{"type":"string"}},"type":[null,"null"]}}

5. Validate this JSON schema, I used https://www.jsonschemavalidator.net/
6. It should fail to parse the schema.

[thim81]

Thanks for all the info, I already spotted that 3.1.0 OpenAPI version, which might be a factor in the incorrect JSON schema used.

I'll look into it in the next week and come back to you.

[harry-pledge-io]

Good spot. I just did a quick check with 3.0.0 and it produces the same result.

Thanks for your time!

[thim81]

hi @harry-pledge-io

Thanks to your example, I'm able to reproduce it.

I'll see if we can add a schema validation and/or convert the schema into a valid schema.

[thim81]

@harry-pledge-io We were able to find and fix the cause, and add a validation step during the conversion. Have a look at the PR for more details.

REMARK: we did not stop the Portman conversion but added a warning and skipped the JSON schema check injection in the Postman collection.

[harry-pledge-io]

LGTM, thanks for looking into it!

Is there a particular reason you prefer to warn in this case? I ask because I do the Portman conversion during a step in a CI pipeline and I would miss any instances of this warning unless I decide to manually look at each pipeline (or write some extra logic to look for warning logs in the pipeline). If you don't want to effect existing behaviour, then it would be ideal If there were a way to opt into fatal errors instead (perhaps with some kind of strict mode flag).

[thim81]

The introduction of a "strict mode" could be useful, we are always cautious with the introduction of too many flags to handle edge cases.

Out of curiosity, what else would you consider strict (fatal)?

[thim81]

hi @harry-pledge-io

While we discuss the "strict mode", I can already share the news that we just released Portman 1.30.1, which contains the enhancements for the OpenAPI to JSON schema conversion, including the validation and alerting of the generated JSON schema.

[harry-pledge-io]

The introduction of a "strict mode" could be useful, we are always cautious with the introduction of too many flags to handle edge cases.

Out of curiosity, what else would you consider strict (fatal)?

I'm not sure what all the current instances of warnings/errors are in Portman but I would say likely anything that's currently considered a warning/error but does not stop the process. Also anything that prevents Portman from outputting what is defined in the Portman config.

[thim81]

@nicklloyd what is your opinion on the "strict" parameter, and which errors would you consider a hard exit of the conversion?

[thim81]

@harry-pledge-io

Besides the "strict" discussion, did the last version of Portman solved the original issue?