Incorrect nullable annotation for array of nullable items

[dvlasyuk]

I have a need in my project to fine-tune the validation of collection items. And I would like to reflect information about these constraints in the generated OpenAPI schema. However, I ran into one problem that doesn't seem to have a workaround.

Since the built-in validation attributes do not support validation of collection items, I use custom validation attributes and custom schema filters (which use these attributes) to enrich the generated schema with appropriate metadata. Filters check if a member has a matching attribute and override the corresponding schema metadata using that attribute. Below are examples of a model and one such filter that should override the nullable annotation (examples are simplified for brevity).

public class TestModel
{
    [NonNullableItems]
    public List<int?> Data1 { get; set; }
    public List<int?> Data2 { get; set; }
}

public class NonNullableItemsSchemaFilter : ISchemaFilter
{
    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        var attribute = context.MemberInfo
            ?.GetCustomAttributes<NonNullableItemsAttribute>()
            .FirstOrDefault();

        if (attribute != null)
        {
            schema.Items.Nullable = false;
        }
    }
}

I expect that for the described model using the described filter, the OpenAPI schema shown below will be generated.

"TestModel": {
  "type": "object",
  "properties": {
    "data1": {
      "type": "array",
      "nullable": true,
      "items": {
        "type": "string",
        "nullable": false
      }
    },
    "data2": {
      "type": "array",
      "nullable": true,
      "items": {
        "type": "string",
        "nullable": true
      }
    },
  },
  "additionalProperties": false
}

In fact, a schema is generated that does not contain nullable annotations for array items, i.e. the default value is assumed (false, according to the OpenAPI specification). This scheme incorrectly describes the behavior of the API, since its implementation allows null values as items of the data1 array.

I checked the source code of the library and found out that this occurs because nullable annotation is explicitly populated only for object property schemas. In all other cases, including schemas for array items, the default value is retained.

I could get around this limitation by explicitly populating nullable in my filter depending on the attribute presence (and not just overwriting it if the attribute is present). However, this requires determining whether the corresponding member is nullable, which implies analysis of the underlying data type, a nullable context, a serializer behavior. This problem is difficult to solve within a schema filter due to lack of data. Moreover, it is impractical to solve it within a schema filter, since this will lead to duplication of a logic already implemented within the library.

The described behavior is actual for version 6.1.

[dvlasyuk]

I checked the repository for similar issues and found that #1849 seems to be caused by the same implementation detail.

[Eneuman]

@domaindrivendev I have a feeling that this also has to do with nullable attributes and generics. I'm gonna take a look at this and #1849.

[Eneuman]

Got it figured out. PR coming this weekend.

[Eneuman]

@domaindrivendev To get generics to work properly with non nullable references it's gonna take a bit of rewriting. We should probobly discuss this before I make any changes. Do you want me to do a "Do not merge" PR and we can discuss it there or how do you want to handle this?

[Eneuman]

@domaindrivendev Let me know if you want me to proceed with this. Non-nullable handling is complex and I would need to change some code around to get it to work.

[dnperfors]

I am seeing similar issues where I use a int?[] (or effectively an Nullable<int>[] and I'd expect to get nullable in the items. Looking in the code I suspect the following might be (part of) the issue:

private DataContract GetDataContractFor(Type modelType)
{
    var effectiveType = Nullable.GetUnderlyingType(modelType) ?? modelType;
    return _serializerDataContractResolver.GetDataContractForType(effectiveType);
}

Because the Nullability is effectively removed completely. Would it be possible to fix this? or is there a known workaround that I might be able to use?

[kristinn-is]

Any update on this issue or workarounds ?

[Eneuman]

PR fixing problems with nullable reference types in dictionary just got merged so I'll take a look at this next week :)

[kristinn-is]

@Eneuman did you manage to take a look at this ?

[Eneuman]

@kristinn-is Yes I did and the problem is that List is treated like a Object and the nullabillity information for the contained type is not used.

There are several ways to fix this so I need to discuss this with @domaindrivendev before making the PR.

I browsed the Open API specification and I couldn't find that it was allowed to set the nullable attribute on the items type attribute, so I think that the result from this would be something like this:

"TestModel": {
  "type": "object",
  "properties": {
    "data1": {
      "type": "array",
      "nullable": true,
      "items": {
        $ref: '#/components/schemas/Pet'
      }
    },
  },
  "additionalProperties": false
}

And then the nullability is set on the ref object.
Ref would only be used if the List<> contains a nullable reference type, otherwise it would look the same as today.

@domaindrivendev
I'm thinking about modifying
public DataContract GetDataContractForType(Type type)
and introduce a new check for "GenericList" as a new DataType.
Then adding a new DataContract.ForGenericList, and finally modifying private OpenApiSchema GenerateConcreteSchema(DataContract dataContract, SchemaRepository schemaRepository) to hande the new DataType and to set returnAsReference = true if the List<> has a nullable type.
Let me know if you want me to proceed.

[kristinn-is]

Currently I'm modifying the items list and adding the nullable value there and the code gets generated correctly in VS2022 at least.
"items": {
"type": "boolean",
"nullable": true <- added
},

so maybe it is allowed but you only didn't find it in the documentation ?

This workaround is bad as I need to edit the file every time the core code changes that the swagger doc is based on.

Another problem is that the swagger doc then removes this again when the api is deployed.

[Qluxzz]

I have a schema filter which fixes this. I have only tested it for [FromQuery] arguments so I'm not sure if it works the same for [FromBody] but here it is:

internal class NullableListItem : ISchemaFilter
{
  public void Apply(OpenApiSchema schema, SchemaFilterContext context)
  {
    var type = context.Type;

    var isEnumerable = typeof(System.Collections.IEnumerable).IsAssignableFrom(type);

    if (!isEnumerable)
      return;

    var listItemIsNullable = type
      .GetGenericArguments()
      .Any(type =>
        Nullable.GetUnderlyingType(type) != null
      );

    if (!listItemIsNullable)
      return;

    schema.Items.Nullable = true;
  }
}

[304NotModified]

I have a schema filter which fixes this. I have only tested it for [FromQuery] arguments so I'm not sure if it works the same for [FromBody] but here it is:

internal class NullableListItem : ISchemaFilter
{
  public void Apply(OpenApiSchema schema, SchemaFilterContext context)
  {
    var type = context.Type;

    var isEnumerable = typeof(System.Collections.IEnumerable).IsAssignableFrom(type);

    if (!isEnumerable)
      return;

    var listItemIsNullable = type
      .GetGenericArguments()
      .Any(type =>
        Nullable.GetUnderlyingType(type) != null
      );

    if (!listItemIsNullable)
      return;

    schema.Items.Nullable = true;
  }
}

Thanks! This works, but too bad partialy.

There is no validation error in the GUI, but it sends 0 and not null:

Someone an idea how to fix this?

[martincostello]

To make issue tracking a bit less overwhelming for the new maintainers (see #2778), I've created a new tracking issue to roll-up various nullability issues here: #2793.

We'll refer back to this issue from there and include it as part of resolving that issue, but I'm going to close this one to help prune the backlog.