Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In toJson, it called non-exist toInt method on extensible enum whose value is Integer. #2841

Open
3 tasks
Tracked by #2823 ...
XiaofeiCao opened this issue Jun 27, 2024 · 4 comments
Open
3 tasks
Tracked by #2823 ...

Comments

@XiaofeiCao
Copy link
Contributor

XiaofeiCao commented Jun 27, 2024

Swagger:
https://github.com/Azure/azure-rest-api-specs/blob/1cb8cb0a95c20513c5d767614888f415be99245d/specification/storagemover/resource-manager/Microsoft.StorageMover/stable/2024-07-01/storagemover.json#L1853-L1856

"minute": {
  "description": "The minute element of the time. Allowed values are 0 and 30. If not specified, its value defaults to 0.",
  "type": "integer",
  "format": "int32",
  "enum": [
    0,
    30
  ],
  "default": 0,
  "x-ms-enum": {
    "name": "Minute",
    "modelAsString": true
  }
}

In toJson, we are doing:

jsonWriter.writeNumberField("minute", this.minute == null ? null : this.minute.toInt());

Tasks

@XiaofeiCao XiaofeiCao changed the title **cdn**: In toJson, it called non-exist toInt method on extensible enum whose value is Integer. In toJson, it called non-exist toInt method on extensible enum whose value is Integer. Jun 27, 2024
@XiaofeiCao
Copy link
Contributor Author

Discussed with srikanta, will wait for azure-core support(currently on non-branding).
https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/clientcore/core/src/main/java/io/clientcore/core/util/ExpandableEnum.java

@XiaofeiCao
Copy link
Contributor Author

XiaofeiCao commented Sep 11, 2024

TypeSpec

@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,
}

GeneratedCode

public final class ReleaseDelay implements ExpandableEnum<Integer> {
    private static final Map<Integer, ReleaseDelay> VALUES = new ConcurrentHashMap<>();

    private final Integer value;

    private ReleaseDelay(Integer value) {
        this.value= value;
    }

    @Override
    public Integer getValue() {
        return value;
    }

    /**
     * Gets all known {@link ReleaseDelay} values.
     *
     * @return The known {@link ReleaseDelay} values.
     */
    public static Collection<ReleaseDelay> values() {
        return VALUES.values();
    }

    /**
     * Creates or finds a {@link ReleaseDelay} for the passed {@code value}.
     *
     * <p>{@code null} will be returned if {@code value} is {@code null}.</p>
     *
     * @param value A value to look for.
     *
     * @return The corresponding {@link ReleaseDelay} of the provided value, or {@code null} if {@code value} was
     * {@code null}.
     */
    public static ReleaseDelay fromInt(Integer value) {
        if (value == null) {
            return null;
        }

        ReleaseDelay member = VALUES.get(value);

        if (member!= null) {
            return member;
        }

        return VALUES.computeIfAbsent(value, ReleaseDelay::new);
    }

    @Override
    public String toString() {
        return Objects.toString(value);
    }

    @Override
    public int hashCode() {
        return Objects.hashCode(value);
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        }

        if (!(obj instanceof ReleaseDelay)) {
            return false;
        }

        ReleaseDelay other = (ReleaseDelay) obj;

        return Objects.equals(value, other.value);
    }

    /**
     * Release the event after 0 seconds.
     */
    public static final ReleaseDelay BY0SECONDS= fromInt(0);

    /**
     * Release the event after 10 seconds.
     */
    public static final ReleaseDelay BY10SECONDS = fromInt(10);

    /**
     * Release the event after 60 seconds.
     */
    public static final ReleaseDelay BY60SECONDS = fromInt(60);

    /**
     * Release the event after 600 seconds.
     */
    public static final ReleaseDelay BY600SECONDS = fromInt(600);

    /**
     * Release the event after 3600 seconds.
     */
    public static final ReleaseDelay BY3600SECONDS = fromInt(3600);
}

Discussion

fromInteger(Integer value)/fromInt(Integer value) vs fromInt(int value)?

1. Integer:

Pros:

  • seems better with getValue() since it also uses Integer
  • easier implementation(null check is always there, and we can use enumType.getNullableType everywhere)

Cons:

  • ClientType is primitive, though actual interface is boxed type.

2. int

Pros:

  • ClientType is int

Cons:

  • not align with getValue(), as getValue returns boxed type.

I prefer fromInt(Integer value) so that codegen won't have to deal with fixed vs expandable difference when doing deserialization.
Fixed enum example: https://github.com/Azure/autorest.java/blob/main/typespec-tests/src/main/java/com/cadl/enumservice/models/Unit.java

@XiaofeiCao
Copy link
Contributor Author

XiaofeiCao commented Sep 12, 2024

9-12 discussion conclusion:

  1. Use fromValue for the static deserialize method and boxed type as parameter, to align with getValue that we have in ExpandableEnum
  2. In fromValue, we should throw instead of directly returning null. This will differ from current ExpandableStringEnum, which returns null, though I believe String can be seen as a special case.

@XiaofeiCao
Copy link
Contributor Author

We should also deal with unbranded.

9-26 discussion:

  1. For Swagger, we should also generate ExpandableEnum for non-string, and do customization for those existing ExpandableStringEnum in cdn, monitor and sql.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant