Schemas for arrays created because properties have descriptions

martincostello commented 1 month ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

Using the latest bits out for Microsoft.AspNetCore.OpenApi (9.0.0-preview.7.24369.10), there's some odd looking behaviour for array properties of schemas when their property has an associated description that causes them to be created as their own schemas when they are otherwise only used once and could (should?) in inlined to the schema they are present in.

For comparison, below is a diff of the existing NSwag schema and the OpenAPI version for the components of the document (I've hand edited for the sake of clarity to remove unimportant differences):

"components": {
  "schemas": {
+   "ArrayOfstring": {
+     "type": "array",
+     "items": {
+       "type": "string"
+     },
+     "description": "The Ids of the user's favorite lines, if any."
+   },
+   "ArrayOfstring2": {
+     "type": "array",
+     "items": {
+       "type": "string"
+     },
+     "description": "The error details, if any."
+   },
    "PreferencesResponse": {
      "type": "object",
      "description": "Represents the API response for a user's preferences.",
      "required": [
        "favoriteLines",
        "userId"
      ],
      "properties": {
        "favoriteLines": {
          "type": "array",
-         "description": "The Ids of the user's favorite lines, if any.",
-         "items": {
-           "type": "string"
-         }
+        "$ref": "#/components/schemas/ArrayOfstring"
        },
        "userId": {
          "type": "string",
          "description": "The user's Id."
        }
      }
    },
    "ErrorResponse": {
      "type": "object",
      "description": "Represents an error from an API resource.",
      "required": [
        "statusCode",
        "message",
        "requestId",
        "details"
      ],
      "properties": {
        "statusCode": {
          "type": "integer",
          "description": "The HTTP status code.",
          "format": "int32"
        },
        "message": {
          "type": "string",
          "description": "The error message."
        },
        "requestId": {
          "type": "string",
          "description": "The request Id."
        },
        "details": {
-         "type": "array",
-         "description": "The error details, if any.",
-         "items": {
-           "type": "string"
-         }
+         "$ref": "#/components/schemas/ArrayOfstring2"
        }
      }
    }
  }
}

The two array properties are essentially the same (a simple array of strings), but because they have different descriptions, they've been lifted out to an inline array. This behaviour seems strange to me, it seems more intuitive for them to have instead be left defined inline in their associated schemas.

Expected Behavior

Either:

The array properties are inlined to the schemas that use them, rather than made their own individual schemas.
There is only one ArrayOfstring schema that is reused, with the descriptions left next to the references (I'm not sure if this is valid OpenAPI v3 though).

Steps To Reproduce

Clone https://github.com/martincostello/alexa-london-travel-site/commit/743e2113a2a277763685d59c2e55a1eff1c31bad
Build and run the application LondonTravel.Site project
View the OpenAPI document at https://localhost:50001/openapi/api.json

Exceptions (if any)

None.

.NET Version

9.0.100-preview.7.24371.2

Anything else?

No response

martincostello commented 1 month ago

Having tests things out with another app, I think actually this is more of a general issue with how arrays are handled.

I've tried out the latest bits with an internal app, and seeing a similar effect with objects that contain an array of objects. The schemas are included below.

Essentially, instead of each schema containing a property that is an array of another schema, instead each schema contains a property containing another schema, which is an array of an inlined schema.

NSwag

```json { "components": { "schemas": { "DotNetRuntimes": { "type": "object", "description": "Represents a collection of .NET runtimes.", "additionalProperties": false, "properties": { "runtimes": { "type": "array", "description": "The known .NET runtimes.", "items": { "$ref": "#/components/schemas/DotNetRuntime" } } } }, "DotNetRuntime": { "type": "object", "description": "Represents information about a .NET runtime.", "additionalProperties": false, "properties": { "name": { "type": "string", "description": "The name of the .NET product." }, "channel": { "type": "string", "description": "The channel of the .NET runtime." }, "isSupported": { "type": "boolean", "description": "Whether the .NET runtime is supported." }, "endOfLife": { "type": "string", "description": "The end-of-life date of the runtime, if known.", "format": "date", "nullable": true }, "latestRuntime": { "type": "string", "description": "The latest runtime version of the .NET channel version." }, "latestSdk": { "type": "string", "description": "The latest SDK version of the .NET channel version." } } } "LambdaRuntimes": { "type": "object", "description": "Represents a collection of AWS Lambda runtimes.", "additionalProperties": false, "properties": { "runtimes": { "type": "array", "description": "The known AWS Lambda runtimes.", "items": { "$ref": "#/components/schemas/LambdaRuntime" } } } }, "LambdaRuntime": { "type": "object", "description": "Represents information about an AWS Lambda runtime.", "additionalProperties": false, "properties": { "id": { "type": "string", "description": "The Id of the AWS Lambda runtime." }, "name": { "type": "string", "description": "The name of the AWS Lambda runtime." }, "deprecation": { "type": "string", "description": "The date the runtime will be deprecated, if known.", "format": "date", "nullable": true }, "isSupported": { "type": "boolean", "description": "Whether the AWS Lambda runtime is supported." } } } } } } ```

OpenAPI

```json { "components": { "schemas": { "ArrayOfDotNetRuntime": { "type": "array", "items": { "type": "object", "properties": { "name": { "type": "string", "description": "The name of the .NET product." }, "channel": { "type": "string", "description": "The channel of the .NET runtime." }, "isSupported": { "type": "boolean", "description": "A value indicating whether the .NET runtime is supported." }, "endOfLife": { "type": "string", "description": "The end-of-life date of the runtime, if known.", "format": "date", "nullable": true }, "latestRuntime": { "type": "string", "description": "The latest runtime version of the .NET channel version." }, "latestSdk": { "type": "string", "description": "The latest SDK version of the .NET channel version." } }, "description": "Represents information about a .NET runtime." }, "description": "The known .NET runtimes." }, "ArrayOfLambdaRuntime": { "type": "array", "items": { "type": "object", "properties": { "id": { "type": "string", "description": "The Id of the AWS Lambda runtime." }, "name": { "type": "string", "description": "The name of the AWS Lambda runtime." }, "deprecation": { "type": "string", "description": "The date the runtime will be deprecated, if known.", "format": "date", "nullable": true }, "isSupported": { "type": "boolean", "description": "A value indicating whether the AWS Lambda runtime is supported." } }, "description": "Represents information about an AWS Lambda runtime." }, "description": "The known AWS Lambda runtimes." }, "DotNetRuntimes": { "type": "object", "properties": { "runtimes": { "$ref": "#/components/schemas/ArrayOfDotNetRuntime" } }, "description": "Represents a collection of .NET runtimes." }, "LambdaRuntimes": { "type": "object", "properties": { "runtimes": { "$ref": "#/components/schemas/ArrayOfLambdaRuntime" } }, "description": "Represents a collection of AWS Lambda runtimes." } } } } ```

martincostello commented 1 month ago

It might be that actually this has some overlap #56990.

Having worked around #56975 I got the result I expected without this issue for my snapshot test, but then when run with other tests, the schema started adding the Arrayofstring* schemas, causing the snapshot test to fail.

However with this app, the same wasn't true: https://github.com/dotnet/aspnetcore/issues/56919#issuecomment-2242692828

captainsafia commented 1 month ago

@martincostello I assume this doesn't repro following https://github.com/dotnet/aspnetcore/pull/56980?

martincostello commented 1 month ago

I'll manually update something to RC1 and check it out - my automated process targets the next preview due, so it won't be pulling in any fixes that haven't been backported until after preview 7 ships.

martincostello commented 1 month ago

@captainsafia Yep, looks good now!

Here's the diff for the snapshot from preview 6: https://github.com/martincostello/alexa-london-travel-site/commit/c5706031a183ea991265e709b51d90913d9f907a

dotnet / aspnetcore