Discourse openapi.json generate issue

[Kyle-Ye]

See original issue and context here https://github.com/apple/swift-openapi-generator/issues/72

For the following legal input, OpenAPIKit will throw an error here.

With message saying

failed: caught error: "dataCorrupted(Swift.DecodingError.Context(codingPath: [], debugDescription: "The given data was not valid YAML.", underlyingError: Optional(Expected `items` value in .content['application/json'].schema.properties.form_template_ids for the request body of the **POST** endpoint under `/categories.json` to be parsable as Mapping but it was not.)))"

https://github.com/mattpolzin/OpenAPIKit/blob/84f6702500fb00476d74420c1a0df64cddd3993e/Sources/OpenAPIKit/Schema%20Object/JSONSchemaContext.swift#L926

items = try container.decodeIfPresent(JSONSchema.self, forKey: .items) // ❌ Throw Error

{
  "components": {
    "schemas": {}
  },
  "info": {
    "description": "TODO",
    "license": {
      "name": "MIT",
      "url": "https://docs.discourse.org/LICENSE.txt"
    },
    "title": "Discourse API Documentation",
    "version": "latest",
    "x-logo": {
      "url": "https://docs.discourse.org/logo.svg"
    }
  },
  "openapi": "3.1.0",
  "paths": {
    "/categories.json": {
      "post": {
        "operationId": "createCategory",
        "parameters": [],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "additionalProperties": false,
                "properties": {
                  "form_template_ids": {
                    "items": [],
                    "type": "array"
                  }
                },
                "required": []
              }
            }
          }
        }
    }
  }
}

[Kyle-Ye]

If you can't reproduce the issue, you can try https://github.com/Kyle-Ye/swift-openapi-generator/tree/bugfix/openapikit_parse and run YamsParserOpenAPITest/testDiscourseOpenAPI to reproduce.

[Kyle-Ye]

I see it, the issue is in the OpenAPI doc for discourse. The items key expects an object value, but there's an array wrapping that object value. Removing the array should fix it.

Might also be worth pasting the doc into an OpenAPI linter first, in case there are other issues.

According to the code, the items key expects an object value, but there's an empty array actually.

I can't find this "items" rule for https://spec.openapis.org/oas/v3.1.0. And when I run a OpenAPI linter for the latest Discourse openapi.json, it passed successfully. So it does not look to be a upstream issue for discourse_api_docs

"it passed successfully": with an online linter https://apitools.dev/swagger-parser/online/

cc @czechboy0

[mattpolzin]

That schema does not look valid to me. The items key for an array schema "MUST be a valid JSON Schema" (https://json-schema.org/draft/2020-12/json-schema-core.html#section-10.3.1.2-1) and a JSON Schema "MUST be an object or a boolean" (https://json-schema.org/draft/2020-12/json-schema-core.html#section-4.3-2). Because this part of the OpenAPI Document is governed by JSON Schema rules, they likely don't bother mentioning too many of those rules inside the OpenAPI specification itself.

OpenAPIKit could arguably be more lenient here, treating the empty array the same as an empty object since that is quite probably the intention the authors of the document had, but there are a lot of places where such leniency might be desirable so OpenAPIKit is strict by default until a good reason for leniency is provided (and an obvious definite intended result for the document even if it isn't proper OpenAPI).

I'm on the fence about this one -- I hate that OpenAPIKit makes life hard for someone ingesting an API that happens to have relatively minor bugs, but I also want to make sure that OpenAPIKit is part of the solution, not the problem (where I see the problem as "tooling is too relaxed, so people release OpenAPI Documents that they think mean one thing but really parts of the document are just being ignored by tooling as not conforming to the spec"). Again, this particular situation is borderline for me because I think I am comfortable saying "anyone who uses an empty array here probably meant to use an empty object." -- but what about the case where the array is populated with multiple entries, that becomes something that simply cannot be interpreted as OpenAPI so I would not want a tool to ignore that entry but rather let someone know they made a mistake.

[Kyle-Ye]

I'll read the material you provide. And once conformed, I think we should close this issue and open a issue to the discourse_api_docs upstream.

[mattpolzin]

I think we should close this issue and open a issue to the discourse_api_docs upstream.

I agree about opening the issue against the OpenAPI Document; OpenAPIKit's strictness is a double edged sword, but I've found issues in numerous high profile OpenAPI Documents (e.g. GitHub's https://github.com/github/rest-api-description/issues/69) and the document authors are generally very happy to have the help identifying such subtle details.

[Kyle-Ye]

The related Github repos do not open Issues tab. So I just submit a feedback via their Discourse Community.

https://meta.discourse.org/t/discourse-openapi-json-docs-does-not-follow-openapi-3-1-0-spec/268932

[Kyle-Ye]

Close and tracked via https://github.com/rswag/rswag/issues/647

[Kyle-Ye]

Close and tracked via rswag/rswag#647

Considering the bug will first be solved in rswag and then discourse_api_docs. There is a high probability that it will block me for a long time.

For anyone interested, I just make a fork and some commits to temporary workaround the bug.

https://github.com/Kyle-Ye/OpenAPIKit/tree/discourse_3.1.0

[mattpolzin]

@Kyle-Ye FYI I am not sure but I suspect this problem actually does originate with the Discourse project, even if the rswag project could do a better job of calling out the mistake.

I believe the problem needs to be fixed here: https://github.com/discourse/discourse/blob/main/spec/requests/api/schemas/json/category_create_request.json#L45

There are a few other files that have similar problems (type: array paired with items: []), so if I am right about that being the problem, it would be worth it to whoever fixes this to fix all locations where the same spec bug occurs.

[mattpolzin]

I located a prior commit that fixed a lot of the same problem elsewhere in their spec: https://github.com/discourse/discourse/commit/95a6268c458c83375225b7d1b25db5366ebfaef0

[mattpolzin]

Actually, the commit I just linked to fixes exactly the problem you found. Perhaps the problem will soon be fixed with an upcoming release of their OpenAPI Document?

[EDIT] Nope, it fixes the category create response spec but not the same problem with the request spec. A separate PR against the Discourse repo will still be needed to fix the remaining problems.

[Kyle-Ye]

Actually, the commit I just linked to fixes exactly the problem you found. Perhaps the problem will soon be fixed with an upcoming release of their OpenAPI Document?

[EDIT] Nope, it fixes the category create response spec but not the same problem with the request spec. A separate PR against the Discourse repo will still be needed to fix the remaining problems.

Thanks for identifying this. I'll close the rswag issue and create a PR in discourse to fix it.

https://github.com/discourse/discourse/pull/22256