[IDP-1391] Validate and apply nullable on nested schema properties (#12)

* [IDP-1391] Validate and apply nullable on nested schema properties * Add test covering regular nested class definition * Added comments and github issue explaining the patch * fix comment * Added comment concerning NullabilityInfoContext * Improve naming * fix naming * improve example names * update expected yaml
workleap · May 8, 2024 · ef3e043 · ef3e043
1 parent 5398351
commit ef3e043
Show file tree

Hide file tree

Showing 4 changed files with 281 additions and 27 deletions.
diff --git a/src/Workleap.Extensions.OpenAPI/RequiredType/ExtractRequiredAttributeFromNullableType.cs b/src/Workleap.Extensions.OpenAPI/RequiredType/ExtractRequiredAttributeFromNullableType.cs
@@ -1,3 +1,4 @@
+using System.Reflection;
 using Microsoft.OpenApi.Models;
 using Swashbuckle.AspNetCore.SwaggerGen;
 
@@ -14,6 +15,8 @@ public void Apply(OpenApiSchema schema, SchemaFilterContext context)
             return;
         }
 
+        PatchNonNullableReferenceTypesOnNestedSchema(schema, context);
+
         // This is used in conjunction with the SupportNonNullableReferenceTypes extension which uses the C# nullable feature to set properties as nullable.
         var notNullableProperties = schema
             .Properties
@@ -25,4 +28,37 @@ public void Apply(OpenApiSchema schema, SchemaFilterContext context)
             schema.Required.Add(property.Key);
         }
     }
+
+    // There is a bug on where SupportNonNullableReferenceTypes does not work for nested record types. This method is a workaround to fix the issue.
+    // https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2758
+    private static void PatchNonNullableReferenceTypesOnNestedSchema(OpenApiSchema schema, SchemaFilterContext context)
+    {
+        // NullabilityInfoContext is used to analyze the nullability of properties. It uses reflection to inspect the type of the member and determine if it is nullable.
+        var nullabilityInfoContext = new NullabilityInfoContext();
+        var contextProperties = context.Type.GetProperties();
+
+        foreach (var (name, property) in schema.Properties)
+        {
+            var contextProperty = contextProperties.FirstOrDefault(x => string.Equals(name, x.Name, StringComparison.OrdinalIgnoreCase));
+            if (contextProperty is null)
+            {
+                continue;
+            }
+
+            var nullabilityInfo = nullabilityInfoContext.Create(contextProperty);
+            // If nullability is unknown or ambiguous, we continue.
+            if (nullabilityInfo is { ReadState: NullabilityState.Unknown, WriteState: NullabilityState.Unknown } || nullabilityInfo.ReadState != nullabilityInfo.WriteState)
+            {
+                continue;
+            }
+
+            // If there is a mismatch between the OpenApiSchema nullability and the context reflected nullability, we defer to the context nullability.
+            var detectedNullability = property.Nullable;
+            var reflectedNullability = nullabilityInfo.ReadState == NullabilityState.Nullable;
+            if (detectedNullability != reflectedNullability)
+            {
+                property.Nullable = reflectedNullability;
+            }
+        }
+    }
 }
diff --git a/src/tests/WebApi.OpenAPI.SystemTest/RequiredType/RequiredTypeController.cs b/src/tests/WebApi.OpenAPI.SystemTest/RequiredType/RequiredTypeController.cs
@@ -8,11 +8,51 @@ namespace WebApi.OpenAPI.SystemTest.OperationId;
 public class RequiredTypeController : ControllerBase
 {
     [HttpGet("/recordClassRequiredType", Name = "GetRecordClassRequiredType")]
-    [ProducesResponseType(typeof(RequiredExample), StatusCodes.Status200OK)]
+    [ProducesResponseType(typeof(RequiredExampleGrandParentRecord), StatusCodes.Status200OK)]
     public IActionResult RecordClassRequiredType()
     {
-        return this.Ok(new RequiredExample(0, "example", null, 0));
+        return this.Ok();
     }
+
+    [HttpGet("/classRequiredType", Name = "GetClassRequiredType")]
+    [ProducesResponseType(typeof(RequiredExampleGrandParentClass), StatusCodes.Status200OK)]
+    public IActionResult ClassRequiredType()
+    {
+        return this.Ok();
+    }
+}
+
+public sealed record RequiredExampleGrandParentRecord(RequiredExampleGrandParentRecord.RequiredExampleParentRecord RequiredExampleParent, string? OptionalGrandParentProperty)
+{
+    public sealed record RequiredExampleParentRecord(IReadOnlyCollection<RequiredExampleParentRecord.RequiredExampleRecord> RequiredExamples, string? OptionalParentProperty)
+    {
+        public sealed record RequiredExampleRecord(IReadOnlyCollection<RequiredExampleRecord.ResultRecord> Results, string ExampleProperty)
+        {
+            public sealed record ResultRecord(string ResultId, string? OptionalResultProperty);
+        }
+    }
+}
+
+public sealed class RequiredExampleGrandParentClass(RequiredExampleParentClass requiredExampleParent, string? optionalGrandParentProperty)
+{
+    public RequiredExampleParentClass RequiredExampleParent { get; } = requiredExampleParent;
+    public string? OptionalGrandParentProperty { get; } = optionalGrandParentProperty;
+}
+
+public sealed class RequiredExampleParentClass(IReadOnlyCollection<RequiredExampleClass> requiredExamples, string? optionalParentProperty)
+{
+    public IReadOnlyCollection<RequiredExampleClass> RequiredExamples { get; } = requiredExamples;
+    public string? OptionalParentProperty { get; } = optionalParentProperty;
 }
 
-public sealed record RequiredExample(int RequiredIntProperty, string RequiredStringProperty, string? OptionalStringProperty, int? OptionalIntProperty);
+public sealed class RequiredExampleClass(IReadOnlyCollection<ResultClass> results, string exampleProperty)
+{
+    public IReadOnlyCollection<ResultClass> Results { get; } = results;
+    public string ExampleProperty { get; } = exampleProperty;
+}
+
+public sealed class ResultClass(string resultId, string? optionalResultProperty)
+{
+    public string ResultId { get; } = resultId;
+    public string? OptionalResultProperty { get; } = optionalResultProperty;
+}
diff --git a/src/tests/WebApi.OpenAPI.SystemTest/openapi-v1.yaml b/src/tests/WebApi.OpenAPI.SystemTest/openapi-v1.yaml
@@ -53,7 +53,19 @@ paths:
           content:
             application/json:
               schema:
-                $ref: '#/components/schemas/RequiredExample'
+                $ref: '#/components/schemas/RequiredExampleGrandParentRecord'
+  /classRequiredType:
+    get:
+      tags:
+        - RequiredType
+      operationId: GetClassRequiredType
+      responses:
+        '200':
+          description: Success
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/RequiredExampleGrandParentClass'
   '/minimal-endpoint-with-typed-result-no-produces/{id}':
     get:
       tags:
@@ -288,23 +300,100 @@ components:
           type: string
           nullable: true
       additionalProperties: { }
-    RequiredExample:
+    RequiredExampleClass:
       required:
-        - requiredIntProperty
-        - requiredStringProperty
+        - exampleProperty
+        - results
       type: object
       properties:
-        requiredIntProperty:
-          type: integer
-          format: int32
-        requiredStringProperty:
+        results:
+          type: array
+          items:
+            $ref: '#/components/schemas/ResultClass'
+        exampleProperty:
           type: string
-        optionalStringProperty:
+      additionalProperties: false
+    RequiredExampleGrandParentClass:
+      required:
+        - requiredExampleParent
+      type: object
+      properties:
+        requiredExampleParent:
+          $ref: '#/components/schemas/RequiredExampleParentClass'
+        optionalGrandParentProperty:
           type: string
           nullable: true
-        optionalIntProperty:
-          type: integer
-          format: int32
+      additionalProperties: false
+    RequiredExampleGrandParentRecord:
+      required:
+        - requiredExampleParent
+      type: object
+      properties:
+        requiredExampleParent:
+          $ref: '#/components/schemas/RequiredExampleParentRecord'
+        optionalGrandParentProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    RequiredExampleParentClass:
+      required:
+        - requiredExamples
+      type: object
+      properties:
+        requiredExamples:
+          type: array
+          items:
+            $ref: '#/components/schemas/RequiredExampleClass'
+        optionalParentProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    RequiredExampleParentRecord:
+      required:
+        - requiredExamples
+      type: object
+      properties:
+        requiredExamples:
+          type: array
+          items:
+            $ref: '#/components/schemas/RequiredExampleRecord'
+        optionalParentProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    RequiredExampleRecord:
+      required:
+        - exampleProperty
+        - results
+      type: object
+      properties:
+        results:
+          type: array
+          items:
+            $ref: '#/components/schemas/ResultRecord'
+        exampleProperty:
+          type: string
+      additionalProperties: false
+    ResultClass:
+      required:
+        - resultId
+      type: object
+      properties:
+        resultId:
+          type: string
+        optionalResultProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    ResultRecord:
+      required:
+        - resultId
+      type: object
+      properties:
+        resultId:
+          type: string
+        optionalResultProperty:
+          type: string
           nullable: true
       additionalProperties: false
     TypedResultExample:

diff --git a/src/tests/expected-openapi-document.yaml b/src/tests/expected-openapi-document.yaml
@@ -53,7 +53,19 @@ paths:
           content:
             application/json:
               schema:
-                $ref: '#/components/schemas/RequiredExample'
+                $ref: '#/components/schemas/RequiredExampleGrandParentRecord'
+  /classRequiredType:
+    get:
+      tags:
+        - RequiredType
+      operationId: GetClassRequiredType
+      responses:
+        '200':
+          description: Success
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/RequiredExampleGrandParentClass'
   '/minimal-endpoint-with-typed-result-no-produces/{id}':
     get:
       tags:
@@ -288,23 +300,100 @@ components:
           type: string
           nullable: true
       additionalProperties: { }
-    RequiredExample:
+    RequiredExampleClass:
       required:
-        - requiredIntProperty
-        - requiredStringProperty
+        - exampleProperty
+        - results
       type: object
       properties:
-        requiredIntProperty:
-          type: integer
-          format: int32
-        requiredStringProperty:
+        results:
+          type: array
+          items:
+            $ref: '#/components/schemas/ResultClass'
+        exampleProperty:
           type: string
-        optionalStringProperty:
+      additionalProperties: false
+    RequiredExampleGrandParentClass:
+      required:
+        - requiredExampleParent
+      type: object
+      properties:
+        requiredExampleParent:
+          $ref: '#/components/schemas/RequiredExampleParentClass'
+        optionalGrandParentProperty:
           type: string
           nullable: true
-        optionalIntProperty:
-          type: integer
-          format: int32
+      additionalProperties: false
+    RequiredExampleGrandParentRecord:
+      required:
+        - requiredExampleParent
+      type: object
+      properties:
+        requiredExampleParent:
+          $ref: '#/components/schemas/RequiredExampleParentRecord'
+        optionalGrandParentProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    RequiredExampleParentClass:
+      required:
+        - requiredExamples
+      type: object
+      properties:
+        requiredExamples:
+          type: array
+          items:
+            $ref: '#/components/schemas/RequiredExampleClass'
+        optionalParentProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    RequiredExampleParentRecord:
+      required:
+        - requiredExamples
+      type: object
+      properties:
+        requiredExamples:
+          type: array
+          items:
+            $ref: '#/components/schemas/RequiredExampleRecord'
+        optionalParentProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    RequiredExampleRecord:
+      required:
+        - exampleProperty
+        - results
+      type: object
+      properties:
+        results:
+          type: array
+          items:
+            $ref: '#/components/schemas/ResultRecord'
+        exampleProperty:
+          type: string
+      additionalProperties: false
+    ResultClass:
+      required:
+        - resultId
+      type: object
+      properties:
+        resultId:
+          type: string
+        optionalResultProperty:
+          type: string
+          nullable: true
+      additionalProperties: false
+    ResultRecord:
+      required:
+        - resultId
+      type: object
+      properties:
+        resultId:
+          type: string
+        optionalResultProperty:
+          type: string
           nullable: true
       additionalProperties: false
     TypedResultExample: