api-guidelines
api-guidelines copied to clipboard
microsoft
Polymorphism, Versioning, Extensible Enums
We want a production workload/client to not break because of something “new” happening that the client had never seen before. This applies even if the “new” behavior was technically already allowed/something the client should have taken into account according to the service documentation.
This is why we say (and enforce unless you can get an exemption after explaining your specific situation to the breaking change review board) that we don’t allow for new fields being added without an API version bump while also saying that clients should not crash if it sees a field it doesn’t know about.
We have similar concern and many discussions around what it means for new enum values (thus our extensible enum story) where we have tried to thread the needle on usability and version resilience.
Polymorphic types is yet another class of APIs where we have difficult trade-offs to make. Should we, at the expense of a “well” written client, restrict what data we return to an older client? And, if we do, how many of the “not-well implemented” clients do we save from running into problems? I would venture that there will still be clients that does a switch on all the “known” discriminators with a default that assumes the last kind, or a default that raises an “unknown thing” exception.
From #331
Add guidance on how to patch a polymorphic type (it is a bit tricky).
Clipped from email stream:
@bexxx says:
We discussed to change the ScaleSettings type of a deployment from a flat type to one using polymorphism to be future proof (in case we add a second type like “TrafficBased”…) and use the ScaleType as the type discriminator. I did the changes and for GET and POST it looks ok.
Add guidance on how to patch a polymorphic type (it is a bit tricky).
Clipped from email stream:
@bexxx says:
We discussed to change the ScaleSettings type of a deployment from a flat type to one using polymorphism to be future proof (in case we add a second type like “TrafficBased”…) and use the ScaleType as the type discriminator. I did the changes and for GET and POST it looks ok. Now we also have a PATCH with content-type Application/merge-patch+json. Here all properties used to be optional to allow the request to only submit the properties whose values change. To enable the use of polymorphism here as well I need to make the scale_type (discriminator) required to know what type the user wants to patch, is this the accepted way? Or should we do smarter validation and deserialization on the service to keep everything optional as well.
@johanste response
This is the prevailing guidance in this scenario has been:
Discriminator/kind is not required – and in fact, usually not allowed to change. This is the default answer. If you do allow for change of kind, it is indeed service specific, and only properties that map 1:1 between the old and new kind/type are migrated. You may fail the request with a 409 response if one or more properties of the existing resource cannot be migrated to the new kind. The principle of least surprise is applied on a case-by-case basis. If you are changing to a kind that is effectively a superset of the existing kind, this can make sense. If you do not allow for kind, and you get a request to modify properties not available in the current kind, you return 409 Conflict indicating that the resource cannot be updated like this.
… but as always, feedback is welcome…
On a side note, this problem exists whenever you have interdependency between property values in a resource. Discriminated polymorphic types is one instance where this happens. A common other case is that only certain SKUs of a resource allow for specific configurations to be set.
