Swashbuckle.aspnetcore: [5.0.0] AllOf in Properties Breaking Back Compat

[matthawley]

Swagger Version: v2

While I realize that having AllOf for properties is valid for swagger, making this change from RC5 -> official release did not provide time for people to comment and it's causing backwards compatibility issues with multiple of our components that do not handle AllOf appropriately. This is blocking us from moving from RC5.

Can I request that this be opt-in (or now that 5.0.0 is released, opt-out)? All other new references of AllOf were added behind the Polymorphic check, which is disabled by default.

Specifically, I'm referring to this line:

https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/f987e3613fa9d3a34695d70a40c85f3c7d65f294/src/Swashbuckle.AspNetCore.Newtonsoft/SchemaGenerator/NewtonsoftObjectHandler.cs#L146

[matthawley]

I wrote a filter to reverse this change.

public class ReverseAllOfPropertiesFilter : ISchemaFilter
{
    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        (string key, OpenApiSchema typeSchema)[] allOfProperties = schema.Properties
            .Where(x => null != x.Value.AllOf && 1 == x.Value.AllOf.Count)
            .Select(x => (x.Key, x.Value.AllOf.First()))
            .ToArray();

        foreach ((string key, OpenApiSchema typeSchema) item in allOfProperties)
        {
            schema.Properties[item.key] = item.typeSchema;
        }
    }
}

[domaindrivendev]

While I think it's reasonable to introduce breaking changes for folks who are depending on pre-release versions (it's one of the caveats to using pre-release software), I do agree that this should be an optional setting.

This change was introduced to address the Swagger/OpenAPI specification issue described here, specifically that contextual metadata (e.g. a property description, whether a property can have a null value etc.) cannot be assigned to referenced schema's because the same schema may be used in different contexts where the contextual metadata may not always apply.

While the introduction of an inline schema that "extends" a shared schema (via allOf) for certain properties is a little ugly, it is perfectly valid and it's also the recommended approach to workaround this glaring constraint in the specification. So, based on that, I'm leaning toward an opt-out setting. Does that seem like the right call to you?

[matthawley]

Totally reasonable to have breaking changes, it was just unexpected between the last RC and the release.

I'm good with the opt-out approach.

[kant2002]

@domaindrivendev the additional problem here is that AutoRest breaks when using allOf. I understand that strictly speaking this is not issue of Swashbuckle, but it create inconvenience for these who use Swashbuclke + Autorest. I'm also fine with current solution, given that it is presented here. If you know where in the docs that can be highlighted, I can submit PR to docs.

[scp-mb]

This has also hit us for one of our APIs, it's no longer specifying the correct classes on properties, or even nested properties which causes Autorest to create it's own intermediate classes. Using the SchemaFilter above from @matthawley does restore the expected behaviour.

5.0.0-rc5:

5.0.0:

[markusschaber]

This also hit us, our app build broke. :-/

[tmarkovski]

This completely breaks pipelines depending on Swashbuckle + Autorest to generate service clients
https://github.com/Azure/autorest/issues/3417

[domaindrivendev]

As of 5.1.0, the allOf behavior has been made opt-in. This should resolve backcompat issues. For anyone wanting this behavior going forward, please use the following setting UseAllOfToExtendReferenceSchemas.

[SkinnySackboy]

@markusschaber @tmarkovski @kant2002 have any of you managed to find a suitable solution to this allOf issue between Swashbuckle and Autorest? I've been trying for a few days, using workarounds to suppress/remove the allOf etc. from the json but I simply cannot get it to work, having recently upgraded from ASP.NET Core 2.2 to 3.1 and from Swashbuckle 4.0.1 to 5.1.2

Thanks!

[matthawley]

@SkinnySackboy Per @domaindrivendev an opt-out was added in 5.1.0.

[kant2002]

@SkinnySackboy I just use class posted here https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/1488#issuecomment-575327707

[SkinnySackboy]

Thanks @matthawley do you know how I can opt-out?

@kant2002 Thanks, I tried this but it doesn't seem to help unfortunately, I still have json created with allOf which AutoRest doesn't like.

[matthawley]

@SkinnySackboy We haven't taken a newer version yet, so I can't really help other than saying my original filter does work for us. Just make sure you're registering it.

[SkinnySackboy]

Thanks @matthawley I'll try again perhaps with older versions and see if some other combination than what I'm trying works.

[markusschaber]

@SkinnySackboy we're using some post processing/filtering of the Json. I don't know the exact details, as another team member implemented it, but I guess it's along the lines of what has been posted above.

[SkinnySackboy]

@markusschaber Many thanks, I ended up doing something similar.