Fix up nullability and primitive type handing in schema generation (#…

…56372) * Fix up nullability and primitive type handing in schema generation * Guard against unsupported NIC and fix object schemas * Address feedback * Move primitive types to constants
dotnet · Jun 26, 2024 · 4c8b5fe · 4c8b5fe
1 parent ee4a0e9
commit 4c8b5fe
Show file tree

Hide file tree

Showing 12 changed files with 188 additions and 165 deletions.
diff --git a/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs b/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs
@@ -24,8 +24,6 @@ namespace Microsoft.AspNetCore.OpenApi;
 /// </summary>
 internal static class JsonNodeSchemaExtensions
 {
-    private static readonly NullabilityInfoContext _nullabilityInfoContext = new();
-
     private static readonly Dictionary<Type, OpenApiSchema> _simpleTypeToOpenApiSchema = new()
     {
         [typeof(bool)] = new() { Type = "boolean" },
@@ -350,7 +348,10 @@ internal static void ApplyPolymorphismOptions(this JsonNode schema, JsonSchemaEx
     /// <param name="context">The <see cref="JsonSchemaExporterContext"/> associated with the current type.</param>
     internal static void ApplySchemaReferenceId(this JsonNode schema, JsonSchemaExporterContext context)
     {
-        schema[OpenApiConstants.SchemaId] = context.TypeInfo.GetSchemaReferenceId();
+        if (context.TypeInfo.GetSchemaReferenceId() is { } schemaReferenceId)
+        {
+            schema[OpenApiConstants.SchemaId] = schemaReferenceId;
+        }
     }
 
     /// <summary>
@@ -365,7 +366,8 @@ internal static void ApplyNullabilityContextInfo(this JsonNode schema, Parameter
             return;
         }
 
-        var nullabilityInfo = _nullabilityInfoContext.Create(parameterInfo);
+        var nullabilityInfoContext = new NullabilityInfoContext();
+        var nullabilityInfo = nullabilityInfoContext.Create(parameterInfo);
         if (nullabilityInfo.WriteState == NullabilityState.Nullable)
         {
             schema[OpenApiSchemaKeywords.NullableKeyword] = true;
@@ -376,16 +378,12 @@ internal static void ApplyNullabilityContextInfo(this JsonNode schema, Parameter
     /// Support applying nullability status for reference types provided as a property or field.
     /// </summary>
     /// <param name="schema">The <see cref="JsonNode"/> produced by the underlying schema generator.</param>
-    /// <param name="attributeProvider">The <see cref="PropertyInfo" /> or <see cref="FieldInfo"/> associated with the schema.</param>
-    internal static void ApplyNullabilityContextInfo(this JsonNode schema, ICustomAttributeProvider attributeProvider)
+    /// <param name="propertyInfo">The <see cref="JsonPropertyInfo" /> associated with the schema.</param>
+    internal static void ApplyNullabilityContextInfo(this JsonNode schema, JsonPropertyInfo propertyInfo)
     {
-        var nullabilityInfo = attributeProvider switch
-        {
-            PropertyInfo propertyInfo => !propertyInfo.PropertyType.IsValueType ? _nullabilityInfoContext.Create(propertyInfo) : null,
-            FieldInfo fieldInfo => !fieldInfo.FieldType.IsValueType ? _nullabilityInfoContext.Create(fieldInfo) : null,
-            _ => null
-        };
-        if (nullabilityInfo is { WriteState: NullabilityState.Nullable } or { ReadState: NullabilityState.Nullable })
+        // Avoid setting explicit nullability annotations for `object` types so they continue to match on the catch
+        // all schema (no type, no format, no constraints).
+        if (propertyInfo.PropertyType != typeof(object) && (propertyInfo.IsGetNullable || propertyInfo.IsSetNullable))
         {
             schema[OpenApiSchemaKeywords.NullableKeyword] = true;
         }

diff --git a/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs b/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs
@@ -45,10 +45,19 @@ internal static class JsonTypeInfoExtensions
     /// generated reference ID.
     /// </summary>
     /// <param name="jsonTypeInfo">The <see cref="JsonTypeInfo"/> associated with the target schema.</param>
+    /// <param name="isTopLevel">
+    /// When <see langword="false" />, returns schema name for primitive
+    /// types to support use in list/dictionary types.
+    /// </param>
     /// <returns>The schema reference ID represented as a string name.</returns>
-    internal static string GetSchemaReferenceId(this JsonTypeInfo jsonTypeInfo)
+    internal static string? GetSchemaReferenceId(this JsonTypeInfo jsonTypeInfo, bool isTopLevel = true)
     {
         var type = jsonTypeInfo.Type;
+        if (isTopLevel && OpenApiConstants.PrimitiveTypes.Contains(type))
+        {
+            return null;
+        }
+
         // Short-hand if the type we're generating a schema reference ID for is
         // one of the simple types defined above.
         if (_simpleTypeToName.TryGetValue(type, out var simpleName))
@@ -59,14 +68,14 @@ internal static string GetSchemaReferenceId(this JsonTypeInfo jsonTypeInfo)
         if (jsonTypeInfo is JsonTypeInfo { Kind: JsonTypeInfoKind.Enumerable, ElementType: { } elementType })
         {
             var elementTypeInfo = jsonTypeInfo.Options.GetTypeInfo(elementType);
-            return $"ArrayOf{elementTypeInfo.GetSchemaReferenceId()}";
+            return $"ArrayOf{elementTypeInfo.GetSchemaReferenceId(isTopLevel: false)}";
         }
 
         if (jsonTypeInfo is JsonTypeInfo { Kind: JsonTypeInfoKind.Dictionary, KeyType: { } keyType, ElementType: { } valueType })
         {
             var keyTypeInfo = jsonTypeInfo.Options.GetTypeInfo(keyType);
             var valueTypeInfo = jsonTypeInfo.Options.GetTypeInfo(valueType);
-            return $"DictionaryOf{keyTypeInfo.GetSchemaReferenceId()}And{valueTypeInfo.GetSchemaReferenceId()}";
+            return $"DictionaryOf{keyTypeInfo.GetSchemaReferenceId(isTopLevel: false)}And{valueTypeInfo.GetSchemaReferenceId(isTopLevel: false)}";
         }
 
         return type.GetSchemaReferenceId(jsonTypeInfo.Options);
@@ -86,7 +95,7 @@ internal static string GetSchemaReferenceId(this Type type, JsonSerializerOption
         if (type.IsArray && type.GetElementType() is { } elementType)
         {
             var elementTypeInfo = options.GetTypeInfo(elementType);
-            return $"ArrayOf{elementTypeInfo.GetSchemaReferenceId()}";
+            return $"ArrayOf{elementTypeInfo.GetSchemaReferenceId(isTopLevel: false)}";
         }
 
         // Special handling for anonymous types
@@ -98,23 +107,14 @@ internal static string GetSchemaReferenceId(this Type type, JsonSerializerOption
             return $"{typeName}Of{propertyNames}";
         }
 
+        // Special handling for generic types that are collections
+        // Generic types become a concatenation of the generic type name and the type arguments
         if (type.IsGenericType)
         {
-            // Nullable types are suffixed with `?` (e.g. `Todo?`)
-            if (type.GetGenericTypeDefinition() == typeof(Nullable<>)
-                && Nullable.GetUnderlyingType(type) is { } underlyingType)
-            {
-                return $"{underlyingType.GetSchemaReferenceId(options)}?";
-            }
-            // Special handling for generic types that are collections
-            // Generic types become a concatenation of the generic type name and the type arguments
-            else
-            {
-                var genericTypeName = type.Name[..type.Name.LastIndexOf('`')];
-                var genericArguments = type.GetGenericArguments();
-                var argumentNames = string.Join("And", genericArguments.Select(arg => arg.GetSchemaReferenceId(options)));
-                return $"{genericTypeName}Of{argumentNames}";
-            }
+            var genericTypeName = type.Name[..type.Name.LastIndexOf('`')];
+            var genericArguments = type.GetGenericArguments();
+            var argumentNames = string.Join("And", genericArguments.Select(arg => arg.GetSchemaReferenceId(options)));
+            return $"{genericTypeName}Of{argumentNames}";
         }
         return type.Name;
     }

diff --git a/src/OpenApi/src/Services/OpenApiConstants.cs b/src/OpenApi/src/Services/OpenApiConstants.cs
@@ -27,4 +27,39 @@ internal static class OpenApiConstants
         OperationType.Patch,
         OperationType.Trace
     ];
+    // Represents primitive types that should never be represented as
+    // a schema reference and always inlined.
+    internal static readonly List<Type> PrimitiveTypes =
+    [
+        typeof(bool),
+        typeof(byte),
+        typeof(sbyte),
+        typeof(byte[]),
+        typeof(string),
+        typeof(int),
+        typeof(uint),
+        typeof(nint),
+        typeof(nuint),
+        typeof(Int128),
+        typeof(UInt128),
+        typeof(long),
+        typeof(ulong),
+        typeof(float),
+        typeof(double),
+        typeof(decimal),
+        typeof(Half),
+        typeof(ulong),
+        typeof(short),
+        typeof(ushort),
+        typeof(char),
+        typeof(object),
+        typeof(DateTime),
+        typeof(DateTimeOffset),
+        typeof(TimeOnly),
+        typeof(DateOnly),
+        typeof(TimeSpan),
+        typeof(Guid),
+        typeof(Uri),
+        typeof(Version)
+    ];
 }
diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs
@@ -81,11 +81,18 @@ internal sealed class OpenApiSchemaService(
                     }
                 };
             }
+            // STJ uses `true` in place of an empty object to represent a schema that matches
+            // anything. We override this default behavior here to match the style traditionally
+            // expected in OpenAPI documents.
+            if (type == typeof(object))
+            {
+                schema = new JsonObject();
+            }
             schema.ApplyPrimitiveTypesAndFormats(context);
             schema.ApplySchemaReferenceId(context);
-            if (context.PropertyInfo is { AttributeProvider: { } attributeProvider })
+            if (context.PropertyInfo is { AttributeProvider: { } attributeProvider } jsonPropertyInfo)
             {
-                schema.ApplyNullabilityContextInfo(attributeProvider);
+                schema.ApplyNullabilityContextInfo(jsonPropertyInfo);
                 if (attributeProvider.GetCustomAttributes(inherit: false).OfType<ValidationAttribute>() is { } validationAttributes)
                 {
                     schema.ApplyValidationAttributes(validationAttributes);

diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs
@@ -144,9 +144,8 @@ private void AddOrUpdateSchemaByReference(OpenApiSchema schema)
             // }
             // In this case, although the reference ID  based on the .NET type we would use is `string`, the
             // two schemas are distinct.
-            if (referenceId == null)
+            if (referenceId == null && GetSchemaReferenceId(schema) is { } targetReferenceId)
             {
-                var targetReferenceId = GetSchemaReferenceId(schema);
                 if (_referenceIdCounter.TryGetValue(targetReferenceId, out var counter))
                 {
                     counter++;
@@ -166,14 +165,14 @@ private void AddOrUpdateSchemaByReference(OpenApiSchema schema)
         }
     }
 
-    private static string GetSchemaReferenceId(OpenApiSchema schema)
+    private static string? GetSchemaReferenceId(OpenApiSchema schema)
     {
         if (schema.Extensions.TryGetValue(OpenApiConstants.SchemaId, out var referenceIdAny)
             && referenceIdAny is OpenApiString { Value: string referenceId })
         {
             return referenceId;
         }
 
-        throw new InvalidOperationException("The schema reference ID must be set on the schema.");
+        return null;
     }
 }
diff --git a/...enApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=controllers.verified.txt b/...enApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=controllers.verified.txt
@@ -25,7 +25,8 @@
             "in": "path",
             "required": true,
             "schema": {
-              "$ref": "#/components/schemas/string2"
+              "minLength": 5,
+              "type": "string"
             }
           }
         ],
@@ -35,17 +36,17 @@
             "content": {
               "text/plain": {
                 "schema": {
-                  "$ref": "#/components/schemas/string"
+                  "type": "string"
                 }
               },
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/string"
+                  "type": "string"
                 }
               },
               "text/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/string"
+                  "type": "string"
                 }
               }
             }
@@ -65,10 +66,12 @@
                 "type": "object",
                 "properties": {
                   "Title": {
-                    "$ref": "#/components/schemas/string2"
+                    "minLength": 5,
+                    "type": "string"
                   },
                   "Description": {
-                    "$ref": "#/components/schemas/string2"
+                    "minLength": 5,
+                    "type": "string"
                   },
                   "IsCompleted": {
                     "type": "boolean"
@@ -92,17 +95,7 @@
       }
     }
   },
-  "components": {
-    "schemas": {
-      "string": {
-        "type": "string"
-      },
-      "string2": {
-        "minLength": 5,
-        "type": "string"
-      }
-    }
-  },
+  "components": { },
   "tags": [
     {
       "name": "Test"

diff --git a/...ots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=forms.verified.txt b/...ots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=forms.verified.txt
@@ -200,13 +200,6 @@
   },
   "components": {
     "schemas": {
-      "boolean": {
-        "type": "boolean"
-      },
-      "DateTime": {
-        "type": "string",
-        "format": "date-time"
-      },
       "IFormFile": {
         "type": "string",
         "format": "binary"
@@ -217,13 +210,6 @@
           "$ref": "#/components/schemas/IFormFile"
         }
       },
-      "int": {
-        "type": "integer",
-        "format": "int32"
-      },
-      "string": {
-        "type": "string"
-      },
       "Todo": {
         "required": [
           "id",
@@ -234,16 +220,18 @@
         "type": "object",
         "properties": {
           "id": {
-            "$ref": "#/components/schemas/int"
+            "type": "integer",
+            "format": "int32"
           },
           "title": {
-            "$ref": "#/components/schemas/string"
+            "type": "string"
           },
           "completed": {
-            "$ref": "#/components/schemas/boolean"
+            "type": "boolean"
           },
           "createdAt": {
-            "$ref": "#/components/schemas/DateTime"
+            "type": "string",
+            "format": "date-time"
           }
         }
       }

diff --git a/...OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt b/...OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt
@@ -89,20 +89,6 @@
   },
   "components": {
     "schemas": {
-      "boolean": {
-        "type": "boolean"
-      },
-      "DateTime": {
-        "type": "string",
-        "format": "date-time"
-      },
-      "int": {
-        "type": "integer",
-        "format": "int32"
-      },
-      "string": {
-        "type": "string"
-      },
       "Todo": {
         "required": [
           "id",
@@ -113,16 +99,18 @@
         "type": "object",
         "properties": {
           "id": {
-            "$ref": "#/components/schemas/int"
+            "type": "integer",
+            "format": "int32"
           },
           "title": {
-            "$ref": "#/components/schemas/string"
+            "type": "string"
           },
           "completed": {
-            "$ref": "#/components/schemas/boolean"
+            "type": "boolean"
           },
           "createdAt": {
-            "$ref": "#/components/schemas/DateTime"
+            "type": "string",
+            "format": "date-time"
           }
         }
       }