Always ref request body and response schemas and fix nested types (do…

…tnet#56513) * Always ref request body and response schemas and fix nested types * Address feedback
ladeak · Jul 2, 2024 · 13f3713 · 13f3713
1 parent e6a0cc8
commit 13f3713
Show file tree

Hide file tree

Showing 15 changed files with 324 additions and 231 deletions.
diff --git a/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs b/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs
@@ -53,7 +53,8 @@ internal static class JsonTypeInfoExtensions
     internal static string? GetSchemaReferenceId(this JsonTypeInfo jsonTypeInfo, bool isTopLevel = true)
     {
         var type = jsonTypeInfo.Type;
-        if (isTopLevel && OpenApiConstants.PrimitiveTypes.Contains(type))
+        var underlyingType = Nullable.GetUnderlyingType(type);
+        if (isTopLevel && OpenApiConstants.PrimitiveTypes.Contains(underlyingType ?? type))
         {
             return null;
         }

diff --git a/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs b/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs
@@ -313,6 +313,10 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 schema.Enum = [ReadOpenApiAny(ref reader, out var constType)];
                 schema.Type = constType;
                 break;
+            case OpenApiSchemaKeywords.RefKeyword:
+                reader.Read();
+                schema.Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = reader.GetString() };
+                break;
             default:
                 reader.Skip();
                 break;

diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs
@@ -228,7 +228,7 @@ private async Task<OpenApiResponse> GetResponseAsync(ApiDescription apiDescripti
             .Select(responseFormat => responseFormat.MediaType);
         foreach (var contentType in apiResponseFormatContentTypes)
         {
-            var schema = apiResponseType.Type is { } type ? await _componentService.GetOrCreateSchemaAsync(type, null, cancellationToken) : new OpenApiSchema();
+            var schema = apiResponseType.Type is { } type ? await _componentService.GetOrCreateSchemaAsync(type, null, captureSchemaByRef: true, cancellationToken) : new OpenApiSchema();
             response.Content[contentType] = new OpenApiMediaType { Schema = schema };
         }
 
@@ -269,7 +269,7 @@ private async Task<OpenApiResponse> GetResponseAsync(ApiDescription apiDescripti
                     _ => throw new InvalidOperationException($"Unsupported parameter source: {parameter.Source.Id}")
                 },
                 Required = IsRequired(parameter),
-                Schema = await _componentService.GetOrCreateSchemaAsync(parameter.Type, parameter, cancellationToken),
+                Schema = await _componentService.GetOrCreateSchemaAsync(parameter.Type, parameter, cancellationToken: cancellationToken),
                 Description = GetParameterDescriptionFromAttribute(parameter)
             };
 
@@ -347,7 +347,7 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(IList<ApiRequestFormat
             if (parameter.All(parameter => parameter.ModelMetadata.ContainerType is null))
             {
                 var description = parameter.Single();
-                var parameterSchema = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken);
+                var parameterSchema = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken);
                 // Form files are keyed by their parameter name so we must capture the parameter name
                 // as a property in the schema.
                 if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection))
@@ -410,15 +410,15 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(IList<ApiRequestFormat
                     var propertySchema = new OpenApiSchema { Type = "object", Properties = new Dictionary<string, OpenApiSchema>() };
                     foreach (var description in parameter)
                     {
-                        propertySchema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken);
+                        propertySchema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken);
                     }
                     schema.AllOf.Add(propertySchema);
                 }
                 else
                 {
                     foreach (var description in parameter)
                     {
-                        schema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken);
+                        schema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken);
                     }
                 }
             }
@@ -465,7 +465,7 @@ private async Task<OpenApiRequestBody> GetJsonRequestBody(IList<ApiRequestFormat
         foreach (var requestForm in supportedRequestFormats)
         {
             var contentType = requestForm.MediaType;
-            requestBody.Content[contentType] = new OpenApiMediaType { Schema = await _componentService.GetOrCreateSchemaAsync(bodyParameter.Type, bodyParameter, cancellationToken) };
+            requestBody.Content[contentType] = new OpenApiMediaType { Schema = await _componentService.GetOrCreateSchemaAsync(bodyParameter.Type, bodyParameter, captureSchemaByRef: true, cancellationToken: cancellationToken) };
         }
 
         return requestBody;

diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs
@@ -93,6 +93,14 @@ internal sealed class OpenApiSchemaService(
             schema.ApplyPolymorphismOptions(context);
             if (context.PropertyInfo is { AttributeProvider: { } attributeProvider } jsonPropertyInfo)
             {
+                // Short-circuit STJ's handling of nested properties, which uses a reference to the
+                // properties type schema with a schema that uses a document level reference.
+                // For example, if the property is a `public NestedTyped Nested { get; set; }` property,
+                // "nested": "#/properties/nested" becomes "nested": "#/components/schemas/NestedType"
+                if (jsonPropertyInfo.PropertyType == jsonPropertyInfo.DeclaringType)
+                {
+                    return new JsonObject { [OpenApiSchemaKeywords.RefKeyword] = context.TypeInfo.GetSchemaReferenceId() };
+                }
                 schema.ApplyNullabilityContextInfo(jsonPropertyInfo);
                 if (attributeProvider.GetCustomAttributes(inherit: false).OfType<ValidationAttribute>() is { } validationAttributes)
                 {
@@ -112,7 +120,7 @@ internal sealed class OpenApiSchemaService(
         }
     };
 
-    internal async Task<OpenApiSchema> GetOrCreateSchemaAsync(Type type, ApiParameterDescription? parameterDescription = null, CancellationToken cancellationToken = default)
+    internal async Task<OpenApiSchema> GetOrCreateSchemaAsync(Type type, ApiParameterDescription? parameterDescription = null, bool captureSchemaByRef = false, CancellationToken cancellationToken = default)
     {
         var key = parameterDescription?.ParameterDescriptor is IParameterInfoParameterDescriptor parameterInfoDescription
             && parameterDescription.ModelMetadata.PropertyName is null
@@ -126,7 +134,7 @@ internal async Task<OpenApiSchema> GetOrCreateSchemaAsync(Type type, ApiParamete
         Debug.Assert(deserializedSchema != null, "The schema should have been deserialized successfully and materialize a non-null value.");
         var schema = deserializedSchema.Schema;
         await ApplySchemaTransformersAsync(schema, type, parameterDescription, cancellationToken);
-        _schemaStore.PopulateSchemaIntoReferenceCache(schema);
+        _schemaStore.PopulateSchemaIntoReferenceCache(schema, captureSchemaByRef);
         return schema;
     }
 

diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs
@@ -79,9 +79,12 @@ public JsonNode GetOrAdd(OpenApiSchemaKey key, Func<OpenApiSchemaKey, JsonNode>
     /// schemas into the top-level document.
     /// </summary>
     /// <param name="schema">The <see cref="OpenApiSchema"/> to add to the schemas-with-references cache.</param>
-    public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema)
+    /// <param name="captureSchemaByRef"><see langword="true"/> if schema should always be referenced instead of inlined.</param>
+    public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema, bool captureSchemaByRef)
     {
-        AddOrUpdateSchemaByReference(schema);
+        // Only capture top-level schemas by ref. Nested schemas will follow the
+        // reference by duplicate rules.
+        AddOrUpdateSchemaByReference(schema, captureSchemaByRef: captureSchemaByRef);
         if (schema.AdditionalProperties is not null)
         {
             AddOrUpdateSchemaByReference(schema.AdditionalProperties);
@@ -119,10 +122,10 @@ public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema)
         }
     }
 
-    private void AddOrUpdateSchemaByReference(OpenApiSchema schema, string? baseTypeSchemaId = null)
+    private void AddOrUpdateSchemaByReference(OpenApiSchema schema, string? baseTypeSchemaId = null, bool captureSchemaByRef = false)
     {
         var targetReferenceId = baseTypeSchemaId is not null ? $"{baseTypeSchemaId}{GetSchemaReferenceId(schema)}" : GetSchemaReferenceId(schema);
-        if (SchemasByReference.TryGetValue(schema, out var referenceId))
+        if (SchemasByReference.TryGetValue(schema, out var referenceId) || captureSchemaByRef)
         {
             // If we've already used this reference ID else where in the document, increment a counter value to the reference
             // ID to avoid name collisions. These collisions are most likely to occur when the same .NET type produces a different

diff --git a/...OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt b/...OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt
@@ -59,20 +59,7 @@
             "content": {
               "application/json": {
                 "schema": {
-                  "type": "object",
-                  "properties": {
-                    "hypotenuse": {
-                      "type": "number",
-                      "format": "double"
-                    },
-                    "color": {
-                      "type": "string"
-                    },
-                    "sides": {
-                      "type": "integer",
-                      "format": "int32"
-                    }
-                  }
+                  "$ref": "#/components/schemas/Triangle"
                 }
               }
             }
@@ -91,25 +78,7 @@
             "content": {
               "application/json": {
                 "schema": {
-                  "required": [
-                    "$type"
-                  ],
-                  "type": "object",
-                  "anyOf": [
-                    {
-                      "$ref": "#/components/schemas/ShapeTriangle"
-                    },
-                    {
-                      "$ref": "#/components/schemas/ShapeSquare"
-                    }
-                  ],
-                  "discriminator": {
-                    "propertyName": "$type",
-                    "mapping": {
-                      "triangle": "#/components/schemas/ShapeTriangle",
-                      "square": "#/components/schemas/ShapeSquare"
-                    }
-                  }
+                  "$ref": "#/components/schemas/Shape"
                 }
               }
             }
@@ -120,6 +89,27 @@
   },
   "components": {
     "schemas": {
+      "Shape": {
+        "required": [
+          "$type"
+        ],
+        "type": "object",
+        "anyOf": [
+          {
+            "$ref": "#/components/schemas/ShapeTriangle"
+          },
+          {
+            "$ref": "#/components/schemas/ShapeSquare"
+          }
+        ],
+        "discriminator": {
+          "propertyName": "$type",
+          "mapping": {
+            "triangle": "#/components/schemas/ShapeTriangle",
+            "square": "#/components/schemas/ShapeSquare"
+          }
+        }
+      },
       "ShapeSquare": {
         "properties": {
           "$type": {
@@ -186,6 +176,22 @@
             "format": "date-time"
           }
         }
+      },
+      "Triangle": {
+        "type": "object",
+        "properties": {
+          "hypotenuse": {
+            "type": "number",
+            "format": "double"
+          },
+          "color": {
+            "type": "string"
+          },
+          "sides": {
+            "type": "integer",
+            "format": "int32"
+          }
+        }
       }
     }
   },