azure-sdk-for-java icon indicating copy to clipboard operation
azure-sdk-for-java copied to clipboard

[FEATURE REQ] azure-core support for extensible enum of type Integer and Float

Open weidongxu-microsoft opened this issue 1 year ago • 4 comments

Is your feature request related to a problem? Please describe.

azure-core support for extensible enum of type Integer and Float.

Similar to String type https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/core/azure-core/src/main/java/com/azure/core/util/ExpandableStringEnum.java

Describe the solution you'd like

Possibly a generic class ExpandableEnum<T> (or ExtensibleEnum<>?).

Describe alternatives you've considered

Class generated via codegen without azure-core class.

Additional context

Information Checklist Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • [x] Description Added
  • [x] Expected solution specified

weidongxu-microsoft avatar Feb 20 '24 07:02 weidongxu-microsoft

Do you have usecases for where Integer and Float are needed?

srnagar avatar Feb 20 '24 20:02 srnagar

I believe @billwert has a case where long is being used in this scenario

alzimmermsft avatar Feb 20 '24 21:02 alzimmermsft

Yeah, we have an enum that looks like this in typespec: https://github.com/Azure/azure-rest-api-specs/blob/c7b8df506c79231f08e1d878b1c4917a5abb1113/specification/eventgrid/Azure.Messaging.EventGrid/main.tsp#L132

  @doc("Supported delays for release operation.")
  enum ReleaseDelay {
    @doc("Release the event after 0 seconds.")
    By0Seconds: 0,

    @doc("Release the event after 10 seconds.")
    By10Seconds: 10,

    @doc("Release the event after 60 seconds.")
    By60Seconds: 60,

    @doc("Release the event after 600 seconds.")
    By600Seconds: 600,

    @doc("Release the event after 3600 seconds.")
    By3600Seconds: 3600,
  }

The codegen for this becomes an ExpandableStringEnum with a fromLong method on it that then initializes thus: https://github.com/billwert/azure-sdk-for-java/blob/12fb25c810ab61180d66c315d4e3a57b548d08ae/sdk/eventgrid/azure-messaging-eventgrid-namespaces-http/src/main/java/com/azure/messaging/eventgrid/namespaces/http/models/ReleaseDelay.java#L20

public final class ReleaseDelay extends ExpandableStringEnum<ReleaseDelay> {
    /**
     * Release the event after 0 seconds.
     */
    @Generated
    public static final ReleaseDelay BY0_SECONDS = fromLong(0L);

    /**
     * Release the event after 10 seconds.
     */
    @Generated
    public static final ReleaseDelay BY10_SECONDS = fromLong(10L);
<snip>

Also of interest is the code generator emitted this when the type was used:

requestOptions.addQueryParam("releaseDelayInSeconds", String.valueOf(releaseDelayInSeconds.toLong()),
                false)

However the type didn't define a toLong() so it became a build error. It turns out that we didn't want a long, really, anyway. I mention it here as there's seemingly already some confusion about what to do with numeric enums from typespec. That's maybe just an issue I should go open in the autorest.java repo but wanted to call it out here in the context of "what does support for numeric enums" look like. (Also why did it default to long and not int? so many questions..)

billwert avatar Feb 20 '24 23:02 billwert

Hi Bill, thanks very much on the use case. I wasn't expecting to see the usage so soon (issue was mainly for parity with other codegen).

The half baked class is mostly due to the "fixed enum" as such https://github.com/Azure/autorest.java/blob/ff03ca0c69955f5490da2a61e947ff91a563e0b5/typespec-tests/src/main/java/com/cadl/enumservice/models/Priority.java (you can see it has fromLong and toLong)

Long is because the there is no way in typespec source specifying the data type (typespec only have number). It could even become a Double if add a non-integer value :-( e.g.

  @doc("Supported delays for release operation.")
  enum ReleaseDelay {
    @doc("Release the event after 0 seconds.")
    By0Seconds: 0,

    @doc("Release the event after 10 seconds.")
    By10Seconds: 10,

    @doc("Release the event after 60 seconds.")
    By60Seconds: 60,

    @doc("Release the event after 600 seconds.")
    By600Seconds: 600,

    @doc("Release the event after 3600 seconds.")
    By3600Seconds: 3600,

    @doc("Release the event after 0.5 seconds.")
    By05Seconds: 0.5,
  }

Though in future, the extensible enum would be written as

  @doc("Supported delays for release operation.")
  union ReleaseDelay {
    int32,

    @doc("Release the event after 0 seconds.")
    By0Seconds: 0,

    @doc("Release the event after 10 seconds.")
    By10Seconds: 10,

    @doc("Release the event after 60 seconds.")
    By60Seconds: 60,

    @doc("Release the event after 600 seconds.")
    By600Seconds: 600,

    @doc("Release the event after 3600 seconds.")
    By3600Seconds: 3600,
  }

where we would know the type on that int32 (basically it means, the value can be these specified value, or just an int32 -- an extensible enum of integer). But still no such on enum.


So far, we don't have a azure-core class for extensible enum as int/float. A formal support would depend on this issue about the decision on azure-core.

Meanwhile, since the lib is still preview, we may use this class for the EventGrid. And replace the class when azure-core be ready.

Maybe customize the API in client to be

requestOptions.addQueryParam("releaseDelayInSeconds", releaseDelayInSeconds.toString(), false)

It happens that we do not really need a Long as this is on query parameter, which could only be a string. But if this ReleaseDelay was used in a model, it had to serialize to / de-serialize from a JSON number. And then this ExpandableStringEnum would not work at all.


@srnagar Do let us know what you think we can do, before we had the azure-core class.

weidongxu-microsoft avatar Feb 22 '24 12:02 weidongxu-microsoft