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

How to map Optional<T> #2359

Open
KillerBoogie opened this issue Feb 27, 2022 · 18 comments
Open

How to map Optional<T> #2359

KillerBoogie opened this issue Feb 27, 2022 · 18 comments
Labels
help-wanted A change up for grabs for contributions from the community

Comments

@KillerBoogie
Copy link

KillerBoogie commented Feb 27, 2022

Please excuse, if this is not the right spot to post. It is unclear if I just don't know how to use the api correctly, if swashbuckle is missing a feature, or if it is a bug.

I'm using Optional<T> in my DTOs to implement Json Patch (details). The type T is stored in the 'Value' field of the Optional<T> class. The Optional class allows to differentiate between null and not provided.

public readonly struct Optional<T>
    {
        public Optional(T? value)
        {
            this.HasValue = true;
            this.Value = value;
        }

        public bool HasValue { get; }
        public T? Value { get; }
        public static implicit operator Optional<T>(T value) => new Optional<T>(value);
        public override string ToString() => this.HasValue ? (this.Value?.ToString() ?? "null") : "unspecified";
    }

The first DTOs I worked with held only primitive types, e.g.

public class PatchGroupDTO
    {
        public Optional<Guid?> SalesGroupId { get; init; }       
        public Optional<string?> Name { get; init; }
    }

The example that Swagger UI displays is wrong, because it displays the property "value":

{
  "salesGroupId": {
    "value": "3fa85f64-5717-4562-b3fc-2c963f66afa6"
  },
  "name": {
    "value": "string"
  }
}

Our solution was to map the types in Startup.cs.

services.AddSwaggerGen(c =>
  {
   c.SwaggerDoc("v1", new OpenApiInfo { Title = "xxx.API", Version = "v1" });
   c.MapType<Optional<Guid?>>(() => new OpenApiSchema { Type = "string", Format = "uuid" });
   c.MapType<Optional<string?>>(() => new OpenApiSchema { Type = "string" });
   c.MapType<Optional<bool?>>(() => new OpenApiSchema { Type = "boolean" });
   c.MapType<Optional<int?>>(() => new OpenApiSchema { Type = "integer" });
})

The example is then correctly displayed as:

{
  "salesGroupId": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
  "name":"string"
}

In my latest DTO I needed nested objects. The simplified DTO looks like this:

public record UpdateCartDTO
{
   public Optional<AddressDTO?> InvoicingAddress { get; init; }
   public Optional<List<CommentDTO>?> Comments { get; init; }
   public Optional<List<string>?> ReservationCodes { get; init; }
};

The example will wrongly show the nested value property again:

{
  "invoicingAddress": {
    "value": {
      "type": "string",
      "receipient": "string",
      "deliveryInstructio": "string",
      "street": "string",
      "streetNumber": "string",
      "streetAffix": "string",
      "zip": "string",
      "city": "string",
      "state": "string",
      "isO3": "string"
    }
  },
"comments": {
    "value": [
      {
        "language": "string",
        "comment": "string"
      }
    ]
  },
  "reservationCodes": {
    "value": [
      "string"
    ]   
  }
}

List<string> can be mapped with:

c.MapType<Optional<List<string>?>>(
 () => new OpenApiSchema { Type = "array", Items = new OpenApiSchema { Type="string" } });

I have problems to map the nested objects. I tried:

c.MapType<Optional<AddressDTO?>>(() => new OpenApiSchema { Type = "object" });

But this only shows empty braces {}. The way I understand the api I would need to manually define all the types of the AddressDTO with Item = new OpenApiSchema{...}. Is there any solution that can just refer to the AddressDTO?

It is in general cumbersome to define a MapType for every T that is used inside Optional. Is there any way to have a generic mapping from Optional to T? Something like:

c.MapType<Optional<T?>>(() => new OpenApiSchema { Object= "T"});
@KillerBoogie
Copy link
Author

Is there anyone who can help?

@momvart
Copy link

momvart commented Apr 17, 2022

Same problem here for a custom PaginatedList<T> class.

I think if type was passed to the mapper function (schemaFactory), we could use open generics.

@momvart
Copy link

momvart commented Apr 17, 2022

Is there anyone who can help?

I managed to handle it using a ISchemaFilter. Implementing one should be straightforward. Also, this question on StackOverflow may be helpful for your implementation.

@maganuk
Copy link

maganuk commented Apr 29, 2022

@KillerBoogie were you able to resolve this? I'm trying to use Optional for a patch request too.

@KillerBoogie
Copy link
Author

not, yet. I haven't had time to try out the hint from momt99.

@maganuk
Copy link

maganuk commented Apr 29, 2022

@momt99 any chance you could share the snippet here? thanks

@momvart
Copy link

momvart commented Apr 30, 2022

@maganuk
This is a simplified version of what I have done for my PaginatedList<T>. (Please note that the main definition of the generic class is somewhere else and the private class included in this code is used to benefit from the schema generation for the properties.)

public class PaginatedListSchemaFilter : ISchemaFilter
{
    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        var type = context.Type;
        if (!(type.IsGenericType
            && type.GetGenericTypeDefinition() == typeof(PaginatedList<>)))
        {
            return;
        }

        schema.Type = string.Empty;
        schema.Items = null;

        // For properties: PageIndex, PageSize, ...
        var paginatedListSchema = context.SchemaGenerator.GenerateSchema(
            typeof(PaginatedList), context.SchemaRepository);
        schema.AllOf.Add(paginatedListSchema);

        var itemType = type.GetGenericArguments()[0];
        var genericPartType = new OpenApiSchema();
        genericPartType.Properties.Add(
            ToCamelCase(nameof(PaginatedList.Items)),
            new OpenApiSchema
            {
                Type = "array",
                Items = context.SchemaGenerator.GenerateSchema(
                    itemType, context.SchemaRepository),
            });
        schema.AllOf.Add(genericPartType);
    }

    private static string ToCamelCase(string name) =>
        name.Length > 1
        ? char.ToLower(name[0]) + name[1..]
        : name.ToLower();

    private interface PaginatedList
    {
        IReadOnlyList<object> Items { get; }

        int PageIndex { get; }

        int PageSize { get; }

        int TotalCount { get; }

        int TotalPages { get; }

        bool HasPreviousPage { get; }

        bool HasNextPage { get; }
    }
}

@maganuk
Copy link

maganuk commented Apr 30, 2022

@momt99 thanks very much for this!

@harvzor
Copy link

harvzor commented Jul 28, 2023

@maganuk or @KillerBoogie, did you ever figure out a solution for Optional<T>?

The closes I've gotten to is here: https://github.com/harvzor/harvzor-optional/pull/3/files#diff-5ab2141080f58ecf05c182fbed6f6531819aeaab1c5034af92d5336dfe146fb9R86

But the code is a real mess and is likely full of bugs.

I'm trying to create a library which will solve this Optional issue everywhere (turns out there's lots of caveats with Newtonsoft/STJ if you're not careful).

@maganuk
Copy link

maganuk commented Jul 28, 2023

@harvzor we ended up creating separate models without the Optional generics and using those for Swagger. Irritating but works. In fact I tried again recently to map the optionals but the problem is that reflection does not reveal if the inner type is nullable or not and we the nullability status to be present in our Swagger doc.

I think the best way to solve this is using code generators. But we don't have the bandwidth to work on that. Please do let me know if you make any progress on this.

@macwier
Copy link

macwier commented Aug 14, 2023

We have something like this:

internal class OptionalDocumentFilter : IDocumentFilter
    {
        public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context)
        {
            foreach (var schema in swaggerDoc.Components.Schemas)
            {
                RemoveOptionalLevelFromProperties(schema.Value, context);
            }

            RemoveOptionalSchemas(swaggerDoc);
        }

        private void RemoveOptionalLevelFromProperties(OpenApiSchema schema, DocumentFilterContext context)
        {
            if (schema.Properties == null)
            {
                return;
            }

            foreach (var property in schema.Properties.ToList())
            {
                if (property.Value.AllOf != null)
                {
                    // swashbuckle uses allOf for references, so that custom coments can be included.
                    for (int i = 0; i < property.Value.AllOf.Count; i++)
                    {
                        var currentSchema = property.Value.AllOf[i];
                        if (currentSchema.IsReferenceToOptional(context.SchemaRepository))
                        {
                            var optionalSchema = context.SchemaRepository.Schemas[currentSchema.Reference.Id];

                            if (!optionalSchema.TryGetValuePropertySchema(out var valueSchema))
                            {
                                throw new InvalidOperationException("Optional schema must have a value property.");
                            }

                            if (valueSchema.Reference != null)
                            {
                                // if the value of optional is a reference (i.e. a complex type), then just use it as all off.
                                property.Value.AllOf[i] = valueSchema;
                            }
                            else
                            {
                                // this is e.g. Optional<string>. We can't use AllOf here, so we must replace the whole property.
                                schema.Properties[property.Key] = valueSchema;
                            }


                        }
                    }
                }
            }
        }

        private void RemoveOptionalSchemas(OpenApiDocument swaggerDoc)
        {
            var schemasToRemove = swaggerDoc.Components.Schemas
                .Where(e => e.IsOptional())
                .ToList();

            foreach (var schema in schemasToRemove)
            {
                swaggerDoc.Components.Schemas.Remove(schema);
            }
        }
    }

And enable like this:

            options.UseAllOfToExtendReferenceSchemas();
            options.DocumentFilter<OptionalDocumentFilter>();

@sujith-k-s
Copy link

@macwier we're just trying this out. Could you possibly share what you did for IsReferenceToOptional and TryGetValuePropertySchema?

@macwier
Copy link

macwier commented Aug 14, 2023

Both of this depend on your implementation of the Optional class/struct.
The IsReferenceToOptional is bascially checking if the schema is the one generated by swashbuckle for the Optional class (it asserts that name contains 'opiontal' and that it has two properties, IsDefined: bool and value: T.

The TryGetValuePropertySchema is basicially schema.Properties.TryGetValue("value", out propertySchema).
I.e. it get's the schema of the T.

In essence what the whole thing does is it replaces any occurence of Optional<T> with T by brute force.

@alesdvorakcz
Copy link

alesdvorakcz commented Aug 18, 2023

I used @macwier's code and adjusted it to work with Optional<T> from above. I think it works great.

public class OptionalDocumentFilter : IDocumentFilter
{
    public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context)
    {
        foreach (var schema in swaggerDoc.Components.Schemas)
        {
            RemoveOptionalLevelFromProperties(schema.Value, context);
        }

        RemoveOptionalSchemas(swaggerDoc);
    }

    private void RemoveOptionalLevelFromProperties(OpenApiSchema schema, DocumentFilterContext context)
    {
        if (schema.Properties == null)
        {
            return;
        }

        foreach (var property in schema.Properties.ToList())
        {
            if (property.Value.AllOf != null)
            {
                // swashbuckle uses allOf for references, so that custom coments can be included.
                for (int i = 0; i < property.Value.AllOf.Count; i++)
                {
                    var currentSchema = property.Value.AllOf[i];
                    if (IsReferenceToOptional(currentSchema.Reference.Id, context.SchemaRepository))
                    {
                        var optionalSchema = context.SchemaRepository.Schemas[currentSchema.Reference.Id];

                        if (!optionalSchema.Properties.TryGetValue("value", out var valueSchema))
                        {
                            throw new InvalidOperationException("Optional schema must have a value property.");
                        }

                        if (valueSchema.Reference != null)
                        {
                            // if the value of optional is a reference (i.e. a complex type), then just use it as all off.
                            property.Value.AllOf[i] = valueSchema;
                        }
                        else
                        {
                            // this is e.g. Optional<string>. We can't use AllOf here, so we must replace the whole property.
                            schema.Properties[property.Key] = valueSchema;
                        }
                    }
                }
            }
        }
    }

    private static bool IsReferenceToOptional(string referenceId, SchemaRepository schemaRepository)
    {
        var referencedSchema = schemaRepository.Schemas.First(x => x.Key == referenceId);
        return IsOptionalSchema(referencedSchema);
    }

    private static bool IsOptionalSchema(KeyValuePair<string, OpenApiSchema> referencedSchema)
    {
        return referencedSchema.Key.EndsWith("Optional") &&
            referencedSchema.Value.Properties.Count() == 2 &&
            referencedSchema.Value.Properties.Any(x => x.Key == "value") &&
            referencedSchema.Value.Properties.Any(x => x.Key == "hasValue");
    }

    private void RemoveOptionalSchemas(OpenApiDocument swaggerDoc)
    {
        swaggerDoc.Components.Schemas
            .Where(IsOptionalSchema)
            .ToList()
            .ForEach(schema => swaggerDoc.Components.Schemas.Remove(schema));
    }
}

harvzor added a commit to harvzor/harvzor-optional that referenced this issue Sep 7, 2023
@harvzor
Copy link

harvzor commented Sep 7, 2023

I used @macwier's code and adjusted it to work with Optional<T> from above. I think it works great.

public class OptionalDocumentFilter : IDocumentFilter
{
    public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context)
    {
        foreach (var schema in swaggerDoc.Components.Schemas)
        {
            RemoveOptionalLevelFromProperties(schema.Value, context);
        }

        RemoveOptionalSchemas(swaggerDoc);
    }

    private void RemoveOptionalLevelFromProperties(OpenApiSchema schema, DocumentFilterContext context)
    {
        if (schema.Properties == null)
        {
            return;
        }

        foreach (var property in schema.Properties.ToList())
        {
            if (property.Value.AllOf != null)
            {
                // swashbuckle uses allOf for references, so that custom coments can be included.
                for (int i = 0; i < property.Value.AllOf.Count; i++)
                {
                    var currentSchema = property.Value.AllOf[i];
                    if (IsReferenceToOptional(currentSchema.Reference.Id, context.SchemaRepository))
                    {
                        var optionalSchema = context.SchemaRepository.Schemas[currentSchema.Reference.Id];

                        if (!optionalSchema.Properties.TryGetValue("value", out var valueSchema))
                        {
                            throw new InvalidOperationException("Optional schema must have a value property.");
                        }

                        if (valueSchema.Reference != null)
                        {
                            // if the value of optional is a reference (i.e. a complex type), then just use it as all off.
                            property.Value.AllOf[i] = valueSchema;
                        }
                        else
                        {
                            // this is e.g. Optional<string>. We can't use AllOf here, so we must replace the whole property.
                            schema.Properties[property.Key] = valueSchema;
                        }
                    }
                }
            }
        }
    }

    private static bool IsReferenceToOptional(string referenceId, SchemaRepository schemaRepository)
    {
        var referencedSchema = schemaRepository.Schemas.First(x => x.Key == referenceId);
        return IsOptionalSchema(referencedSchema);
    }

    private static bool IsOptionalSchema(KeyValuePair<string, OpenApiSchema> referencedSchema)
    {
        return referencedSchema.Key.EndsWith("Optional") &&
            referencedSchema.Value.Properties.Count() == 2 &&
            referencedSchema.Value.Properties.Any(x => x.Key == "value") &&
            referencedSchema.Value.Properties.Any(x => x.Key == "hasValue");
    }

    private void RemoveOptionalSchemas(OpenApiDocument swaggerDoc)
    {
        swaggerDoc.Components.Schemas
            .Where(IsOptionalSchema)
            .ToList()
            .ForEach(schema => swaggerDoc.Components.Schemas.Remove(schema));
    }
}

I've given this a quick test and it misses some pain points in the generated schema:

Doesn't work with Optional query params

This wouldn't work on query params, like if you had a method like: public int Get([FromQuery] Optional<int> foo);. This appears to be because you only search within objects in the request for Optionals.

Only removes Optional schemas, and not other schemas if they shouldn't be generated

This filter only removes any schemas with the name ending in Optional, if you have an Optional with an interface like:

public interface IOptional
{
    bool IsDefined { get; }
    
    object Value { get; set; }
    
    Type GenericType { get; }
}

SwashBuckle will generate a schema for the Type property, and will also generate schemas for all of the properties in Type, which really adds a lot of basically useless schemas to the Swagger doc

And even then, doesn't seem to actually remove Optional schemas?

I tried a fairly simple example:

public class Foo
{
    public Optional<string> String { get; set; }
}

[Route("/")]
[ApiController]
public class IndexController : Controller
{
    [HttpPost]
    public IActionResult Post([FromBody] Foo foo)
    {
        return Ok(new Optional<Foo>());
    }
}

And it never actually removed the StringOptional from the schemas as this line never returns as true:

if (IsReferenceToOptional(currentSchema.Reference.Id, context.SchemaRepository))

Maybe because it's searching for properties on schemas?

Probably lots of other issues

Likely it's good enough for basic use cases, but you'll find it a bit buggy if you try many other things, so this isn't a perfect workaround.

My tests

I implemented this and ran this against the project test I've written for my fix. You can see the failing tests here: https://github.com/harvzor/harvzor-optional/actions/runs/6110092503/job/16582434597#step:4:142

And here's the related PR where I implemented your code: harvzor/harvzor-optional#5

If you run the Harvzor.Optional.SystemTextJson.WebExample, you can try it out to see the Swagger in your browser.

@harvzor
Copy link

harvzor commented Sep 8, 2023

I've created a PR to better support open generics: #2704

Would be really nice if @alesdvorakcz, @macwier and @maganuk (and anyone else) could take a look to see if it solves the issue of open generics 🙏

@thorhj
Copy link

thorhj commented Mar 9, 2024

@harvzor I think what you are suggesting in the PR is exactly what I would need. I am currently working with an API that uses Option<T> for nullable types. These are just serialized to T or null, but I cannot currently express that in Swagger.

With your PR I believe I could make a mapping like this:

c.MapType(typeof(Option<>), (ctx) =>
{
  var type = ctx.UnderlyingType.GetGenericArguments[0];
  var schema = ctx.SchemaGenerator.GenerateSchema(type, ctx.SchemaRepository);
  schema.Nullable = true;
  return schema;
}

I can see the issue and PR has been idle since September. Is there a hold-up? When do you think this could be added?

@adamjones2
Copy link

I also need a way to provide a custom mapping for a type in terms of some other types. Without this feature MapType is severely limited in what it covers - generics are out, and practically so are all non-generic types of any complexity unless you fancy redefining all the smaller contained types, duplicating Swashbuckle's definitions and introducing diversion risk. Custom mappings of types IMO is the most important extension point of Swashbuckle but it doesn't seem to really be there at the moment.

Would love for this PR to make some progress (willing to help fwiw).

@martincostello martincostello added the help-wanted A change up for grabs for contributions from the community label Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help-wanted A change up for grabs for contributions from the community
Projects
None yet
Development

No branches or pull requests

10 participants