Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StronglyTypedId generates incorrect OpenApi specification #58

Open
nZeus opened this issue Mar 8, 2022 · 7 comments · May be fixed by #86
Open

StronglyTypedId generates incorrect OpenApi specification #58

nZeus opened this issue Mar 8, 2022 · 7 comments · May be fixed by #86

Comments

@nZeus
Copy link

nZeus commented Mar 8, 2022

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
Copy link
Owner

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
Copy link
Author

nZeus commented Mar 11, 2022

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
Copy link
Owner

Awesome, thanks for the update!

@hankovich
Copy link

@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
Copy link

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 lucasteles linked a pull request Oct 21, 2022 that will close this issue
@lucasteles
Copy link

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

@hankovich
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants