andrewlock / StronglyTypedId

A Rosyln-powered generator for strongly-typed IDs
MIT License
1.52k stars 80 forks source link

StronglyTypedId generates incorrect OpenApi specification #58

Open nZeus opened 2 years ago

nZeus commented 2 years ago

Hello,

I have a Strongly Typed Id struct ConsentCollectionId

[StronglyTypedId(backingType: StronglyTypedIdBackingType.Guid, StronglyTypedIdConverter.SystemTextJson)]
public partial struct ConsentCollectionId { }

This type is used as a parameter in one of my controllers. Unfortunately, the generated open-api specification file contains the wrong type description: image

Instead of

{
  "value": "3fa85f64-5717-4562-b3fc-2c963f66afa6"
}

it should be just

"3fa85f64-5717-4562-b3fc-2c963f66afa6"

Do you know if there is a way to change this? As a work-around, I changed the type of the parameter back to Guid and do the mapping manually.

Kind regards, Ilia

andrewlock commented 2 years ago

I haven't tried it, but I think you can use the approach in this blog post: https://blog.magnusmontin.net/2020/04/03/custom-data-types-in-asp-net-core-web-apis/

e.g.:

services.AddSwaggerGen(c =>
{
    c.SwaggerDoc("v1", new OpenApiInfo { Title = "My API", Version = "v1" });
    c.MapType<ConsentCollectionId>(() => new OpenApiSchema { Type = "string", Pattern = "^[a-f0-9]{32}$",  });
});

It's something I need to look into further though!

nZeus commented 2 years ago

Oh that's a great idea 👍 I modified it a bit, it works really well. Thank you!

c.MapType<ConsentCollectionId>(() => new OpenApiSchema { Type = "string", Format = "uuid" });
andrewlock commented 2 years ago

Awesome, thanks for the update!

hankovich commented 2 years ago

@nZeus there are two cons I see in the provided solution

  1. TId? isn't covered. If TId is registered, swashbuckle will not add a schema for TId? by default, we have to do it manually
  2. MapType isn't idemponent and will throw if you will call it twice for the same TId

We have the following extension to properly map ids. It addresses both cons mentioned

public static SwaggerGenOptions MapStronglyTypedId<TId>(this SwaggerGenOptions options)
    where TId : struct
{
    return options.MapValueObjectInternal<TId>("Value");
}

private static SwaggerGenOptions MapValueObjectInternal<TId>(this SwaggerGenOptions options, string propertyName)
    where TId : struct
{
    var (schemaType, format) = GetValueObjectInfo(typeof(TId), propertyName);

    var schema = new OpenApiSchema
    {
        Type = schemaType,
        Format = format,
    };

    var nullableSchema = new OpenApiSchema
    {
        Type = schemaType,
        Format = format,
        Nullable = true,
    };

    options.SchemaGeneratorOptions.CustomTypeMappings[typeof(TId)] = () => schema;
    options.SchemaGeneratorOptions.CustomTypeMappings[typeof(TId?)] = () => nullableSchema;

    return options;
}

private static (string Type, string? Format) GetValueObjectInfo(Type type, string propertyName)
{
    var property = type.GetProperty(propertyName);

    if (property == null)
    {
        throw new InvalidOperationException($"{type} is not a valid type for value objects. Missing `{propertyName}` property.");
    }

    var propertyType = property.PropertyType;

    if (propertyType == typeof(Guid))
    {
        return ("string", "uuid");
    }

    if (propertyType == typeof(string))
    {
        return ("string", null);
    }

    if (propertyType == typeof(long))
    {
        return ("integer", "int64");
    }

    if (propertyType == typeof(int))
    {
        return ("integer", "int32");
    }

    throw new InvalidOperationException($"{propertyType} is not a valid property type for value objects.");
}
lucasteles commented 1 year ago

I ended up writing a schema filter to get the sample write automatically (not the best, but gets the job done)


builder.Services
.AddSwaggerGen(c =>
{
    c.SchemaFilter<StrongIdSwaggerExamples>();
});

public class StrongIdSwaggerExamples : ISchemaFilter
{
    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        if (!context.Type.Name.EndsWith("Id") || !context.Type.IsValueType ||
            context.Type.GetCustomAttribute(typeof(TypeConverterAttribute)) is not TypeConverterAttribute attr
            || Type.GetType(attr.ConverterTypeName) is not { } type)
            return;
        if (Activator.CreateInstance(type) is not TypeConverter converter) return;

        if (converter.CanConvertTo(typeof(Guid)) || converter.CanConvertTo(typeof(string)))        
        {
            schema.Type = "string";
            schema.Example = new OpenApiString(Guid.NewGuid().ToString());
        }
        else if (converter.CanConvertTo(typeof(int)) || converter.CanConvertTo(typeof(long)))
        {
            schema.Type = "integer";
            schema.Example = new OpenApiInteger(0);
        }
    }
}
lucasteles commented 1 year ago

@hankovich this PR would provide a self-contained way to have this

hankovich commented 1 year ago

@lucasteles thanks, looks nice.

Unfortunately we don't want to reference additional packages (except Newtonsoft.Json) from projects with contracts where all strongly typed ids live. That's why we don't use dapper type handlers and probably will not use schema filters as well.