microsoft / OpenAPI.NET

The OpenAPI.NET SDK contains a useful object model for OpenAPI documents in .NET along with common serializers to extract raw OpenAPI JSON and YAML documents from the model.
MIT License
1.42k stars 239 forks source link

v2 - type upgrade from string to flag enum - easy catch for consumers #1924

Open baywet opened 3 days ago

baywet commented 3 days ago

While I support the use of a flaggable enum for performance reasons, I believe this is going to lead to a regression that's going to be hard to catch for most people. And which I think our own code is a victim of.

v1 Type is a string v2 Type is a flaggable enum (currently)

It's super easy to not notice the change in some scenarios, especially the ones involving string interpolation, implicit operators. In that case, overlooking the fact that multiple values are present will lead to cases with multiple entries falling off a cliff.

I think to start with, our to identifier method should throw if multiple values are present, instead of returning null to signal something was wrong in the logic of the application https://github.com/microsoft/OpenAPI.NET/blob/cb5bb2797a4a4aa61c96d4fd453ac5257474848d/src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs#L32

darrelmiller commented 2 days ago

@baywet We should talk.