Swashbuckle.AspNetCore icon indicating copy to clipboard operation
Swashbuckle.AspNetCore copied to clipboard

Records (positional) and DataAnnotations

Open CaringDev opened this issue 3 years ago • 16 comments

I'd like to combine C# 9 positional records, System.ComponentModel.DataAnnotations, model validation and Swashbuckle.AspNetCore (current latest 6.2.1) in a concise and convenient way:

Ideally, this record

public record Foo([StringLength(2)] string Bar);

would produce

"Foo": {
    "type": "object",
    "properties": {
      "bar": {
        "maxLength": 2,
        "minLength": 0,
        "type": "string"
      }
    },
    "additionalProperties": false
  }

but currently, I get

"Foo": {
    "type": "object",
    "properties": {
      "bar": {
        "type": "string"
      }
    },
    "additionalProperties": false
  }

Targeting the attribute to the underlying property (by adding property: to the attribute) fixes the OpenAPI definition but breaks model validation (i.e. we get a 200 for models that should return a 400).

Known "workaround"s:

  • ~duplicate the attributes (using property and param)~ does not work in .NET 9 (Preview 7) anymore
  • use 'traditional' (optionally init) property-bag classes (read-only properties in classes with constructors do not show up in the generated OpenAPI definition).

Is this something you would consider adding / having added (any blockers / difficulties you see) to Swashbuckle?

CaringDev avatar Sep 03 '21 07:09 CaringDev

Def a useful feature that will become more relevant with the adoption of records for API types. It's doable but non-trivial and I don't have the bandwidth to look into it at the moment. However, I would accept a PR if you're interested? But, if it's looking like a big refactor, I would recommend discussing and aligning on the approach before knocking out the PR.

Off the top of my head, you would need to update the SchemaGenerator.GenerateSchemaForMember method here: https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/v6.2.1/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs#L62

When retrieving custom attributes, you could do an additional check to determine if the member has a matching constructor parameter and if so include attributes on that parameter. Actually, we already do something similar but specifically for honoring Newtonsoft support for deserializing property values via the constructor. https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/v6.2.1/src/Swashbuckle.AspNetCore.Newtonsoft/SchemaGenerator/NewtonsoftDataContractResolver.cs#L147

domaindrivendev avatar Sep 08 '21 14:09 domaindrivendev

That Newtonsoft logic seems very promising... ctor support has been added to STJ in 5.x I guess this would allow cleaning up the Newtonsoft special casing.

  • Is there already a way to determine which STJ version is used?
  • Would it be acceptable to add a direct STJ >= 5.x dependency? Currently for netstandard2.0 there's a transitive one >= 4.6 (through Swashbuckle.AspNetCore.SwaggerGen and Swashbuckle.AspNetCore.SwaggerUI).
  • Or would we need to #if TFM? Note that this is not safe (e.g. using STJ 5 on netcore3.1 is possible).

As I'm currently very busy as well, I will have a go starting mid-October. I hope you don't mind having an issue open that long?

CaringDev avatar Sep 10 '21 05:09 CaringDev

Here's how I solved another issue with getting the StringLength attribute values from records for a specific property. Maybe something like this could be of use in Swashbuckle if it was a bit more generic?

    public static int MaxLength<T>(Expression<Func<T, string?>> outExpr) {
        var expr = (MemberExpression)outExpr.Body;
        var prop = (PropertyInfo)expr.Member;
        var length = prop.GetCustomAttribute<StringLengthAttribute>()?.MaximumLength;

        if (length != null) {
            return length.Value;
        }

        //for records we need to dig the attributes out of the constructor
        var constructors = typeof(T).GetConstructors();
        foreach (var constructorInfo in constructors) {
            foreach (var parames in constructorInfo.GetParameters().Where(p => p.Name == prop.Name)) {
                length = parames.GetCustomAttribute<StringLengthAttribute>()?.MaximumLength;
                if (length != null) {
                    return length.Value;
                }
            }
        }
        throw new ArgumentException($"Attribute {nameof(StringLengthAttribute)} not found in property {prop.Name} of type {typeof(T)}");
    }

spaasis avatar Jan 20 '22 10:01 spaasis

In relation to this issue, created fix for positional record example/summary handling https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/2546

pixellos avatar Nov 17 '22 16:11 pixellos

#2546

martincostello avatar May 04 '24 15:05 martincostello

Should this issue have been closed? #2546 does not seem to solve the data annotation issue with C# records.

jacobilsoe avatar May 14 '24 09:05 jacobilsoe

My understanding was that it did - if it doesn't, please open a new issue and we can look into it.

martincostello avatar May 14 '24 10:05 martincostello

Not possible to just reopen?

jacobilsoe avatar May 15 '24 07:05 jacobilsoe

Either way, we'd like a repro so we can see exactly what doesn't seem to be working.

martincostello avatar May 15 '24 08:05 martincostello

The repro is already stated in this issue.

jacobilsoe avatar May 15 '24 08:05 jacobilsoe

I needed this even thought about contributing a fix. However after digging deeper it seems to me that this issue should be fixed in ASP.NET Core's OpenAPI support and not in Swashbuckle. In the meantime I use the following filter.

using System.ComponentModel.DataAnnotations;
using System.Reflection;
using Microsoft.OpenApi.Models;
using Swashbuckle.AspNetCore.SwaggerGen;

public class PositionalRecordsSchemaFilter : ISchemaFilter
{
    public void Apply(OpenApiSchema model, SchemaFilterContext context)
    {
        ConstructorInfo[] constructors = context.Type.GetConstructors();

        if (constructors.Length == 1)
        {
            ParameterInfo[] parameters = constructors[0].GetParameters();

            foreach (KeyValuePair<string, OpenApiSchema> modelProperty in model.Properties)
            {
                ParameterInfo? constructorParameter = parameters.FirstOrDefault(p => String.Equals(p.Name, modelProperty.Key, StringComparison.OrdinalIgnoreCase));

                if (constructorParameter is not null)
                {
                    if (constructorParameter.ParameterType == typeof(string))
                    {
                        foreach (Attribute attribute in constructorParameter.GetCustomAttributes())
                        {
                            if (attribute is MinLengthAttribute minLength)
                            {
                                modelProperty.Value.MinLength = minLength.Length;
                            }
                            else if (attribute is MaxLengthAttribute maxLength)
                            {
                                modelProperty.Value.MaxLength = maxLength.Length;
                            }
                            else if (attribute is StringLengthAttribute stringLength)
                            {
                                modelProperty.Value.MinLength = stringLength.MinimumLength;
                                modelProperty.Value.MaxLength = stringLength.MaximumLength;
                            }
                            else if (attribute is RegularExpressionAttribute regex)
                            {
                                modelProperty.Value.Pattern = regex.Pattern;
                            }
                        }
                    }

                    if (IsNumeric(constructorParameter.ParameterType))
                    {
                        RangeAttribute? rangeAttribute = constructorParameter.GetCustomAttribute<RangeAttribute>();

                        if (rangeAttribute is not null)
                        {
                            decimal? minValue = GetDecimalValue(rangeAttribute.Minimum);
                            decimal? maxValue = GetDecimalValue(rangeAttribute.Maximum);

                            if (minValue is not null)
                            {
                                modelProperty.Value.ExclusiveMinimum = rangeAttribute.MinimumIsExclusive;
                                modelProperty.Value.Minimum = minValue;
                            }

                            if (maxValue is not null)
                            {
                                modelProperty.Value.ExclusiveMaximum = rangeAttribute.MaximumIsExclusive;
                                modelProperty.Value.Maximum = maxValue;
                            }
                        }
                    }
                }
            }
        }
    }

    private static bool IsNumeric(Type? type)
    {
        if (type is null)
        {
            return false;
        }

        return Type.GetTypeCode(type) switch
        {
            TypeCode.Byte => true,
            TypeCode.SByte => true,
            TypeCode.UInt16 => true,
            TypeCode.UInt32 => true,
            TypeCode.UInt64 => true,
            TypeCode.Int16 => true,
            TypeCode.Int32 => true,
            TypeCode.Int64 => true,
            TypeCode.Decimal => true,
            TypeCode.Double => true,
            TypeCode.Single => true,
            //Support for int?, double?, decimal? etc.
            TypeCode.Object => type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>) && IsNumeric(Nullable.GetUnderlyingType(type)),
            _ => false,
        };
    }

    private static decimal? GetDecimalValue(object? value)
    {
        if (value is null)
        {
            return null;
        }

        return Convert.ToDecimal(value);
    }
}

Note that it works for my API, it might need fixing for some cases and it lacks support for some things for example BigInteger properties, AllowedValues and DeniedValues and probably more

Eirenarch avatar Jul 15 '24 12:07 Eirenarch