Autorest: x-ms-enum: we should revise enum deserialization to exclude a need of breaking change in case of a new value

[hovsepm]

This sounds like a good and immutable candidate for enum:

        "dayOfWeek": {
          "type": "string",
          "description": "Day of the week when a cache can be patched.",
          "enum": [
            "Monday",
            "Tuesday",
            "Wednesday",
            "Thursday",
            "Friday",
            "Saturday",
            "Sunday",
          ],
          "x-ms-enum": {
            "name": "DayOfWeek",
            "modelAsString": false
          }
        },

but owner team decided to introduce 2 more values: "Everyday" and "Weekend". And it will be a breaking change in the generated SDK (at least in C# and Java) with unnecessary major version bump of the package. We need to have better handling of enum enhancements in our generated code to avoid this kind of "silly" version breaks in client libraries (Server API devs does not consider it as a breaking change and they will not change an API version for a new enum value).

For example we can introduce "Unknown" value as the first/default one in the generated enums and have a special handling/intermediate step in the deserialization component that will set Server returned values to Unknown if they are not in the current set of enum values.

[brjohnstmsft]

I have a proposed solution to this that we use in the Azure Search .NET SDK. We call it "extensible enums". We have several data types that are open-ended, but that we don't want to model as strings due to the lack of good Intellisense support.

Here is an example: https://github.com/Azure/azure-sdk-for-net/blob/AutoRest/src/Search/Microsoft.Azure.Search/Customizations/Indexes/Models/DataType.cs

And here is the base class from which all extensible enums derive: https://github.com/Azure/azure-sdk-for-net/blob/AutoRest/src/Search/Microsoft.Azure.Search/Customizations/Models/ExtensibleEnum.cs

And finally, here is a JSON converter that holds it all together: https://github.com/Azure/azure-sdk-for-net/blob/AutoRest/src/Search/Microsoft.Azure.Search/Customizations/Serialization/ExtensibleEnumConverter.cs https://github.com/Azure/azure-sdk-for-net/blob/AutoRest/src/Search/Microsoft.Azure.Search/Customizations/Serialization/ConverterBase.cs

This is the third time this issue has come up: https://github.com/Azure/autorest/issues/290 https://github.com/Azure/autorest/issues/733

But I think it hasn't gained traction because we haven't been able to agree on an approach and everyone has been pretty busy.

I'd be happy to contribute our infrastructure to AutoRest and the ClientRuntime if we can reach consensus on the approach.

[olydis]

@brjohnstmsft:
Note that the example @hovsepm mentions generates an actual C# enum due to "modelAsString": false. Modulo noise:

C# public enum DayOfWeek { Monday, Tuesday, Wednesday, Thursday, Friday, Saturday, Sunday }

[brjohnstmsft]

I know, and that is precisely the source of the versioning problem. How else could you solve it while still using an enum type?

[olydis]

Not sure about the details, but basically by adding an Unknown option to the enum as proposed. But I see the drawbacks of that compared to your generic solution. On the other hand, actual enums are "beautiful" (after all we want to generate code that looks handwritten).

[brjohnstmsft]

Unknown is a lot less usable than getting a strongly-typed object that you can identify by looking at its underlying string value. That's why we implemented ExtensibleEnum in the first place.

[matthchr]

I agree with @brjohnstmsft that an ExtensibleEnum would be great. In C# at least you can make it appear/behave almost identical to an enum

My service has 2 types of enum it deals with:

• 1 is easily mapped to enum on the client because the server guarantees backwards compatibility (the server will always map a new enum value to an old one if the request is going to an older versioned client -- we do this with resource states -- things like Idle, Running etc -- the server does the mapping)
• The second is things that we want to be enum's on the client but the server doesn't have a contract that says it will never change them - for example CertificateFormat is one of these for us -- we may support new formats in the future and there's no way for the server to map a new format to an old one. It would be great to have an ExtensibleEnum here.

[matthchr]

See 733 and 290 as well

[ro-joowan]

Folks from the Azure Data Lake team would also love to have this feature. Our current Swagger design was to set the enums with modelAsString: false. Therefore, moving to modelAsString: true now causes a couple of major issues: (1) The breaking change of moving enum type to string type for the customer and (2) The current modelAsString: true model creates a static class of the enum type, which prevents instantiation.

[dsgouda]

@olydis closing this for now, please reopen if something is incomplete here

[ro-joowan]

@fearthecowboy -- Does "modelAsString : true" use this new behavior now by default?