From df5a8854d22b73a6ac67a73f7f5da475631d9ca3 Mon Sep 17 00:00:00 2001 From: Xiaofei Cao <92354331+XiaofeiCao@users.noreply.github.com> Date: Thu, 12 Dec 2024 17:31:49 +0800 Subject: [PATCH] http-client-java, unify branded/unbranded ExpandableEnum (#5334) ### Issue - Last item of https://github.com/Azure/autorest.java/issues/2841 - Previous unbranded implementation has two major issues - No `equals()` override - Non-string implementation can't compile ### This PR - Apply current branded ExpandableEnum interface implementation to unbranded - `fromString` will be unified as `fromValue`, and will throw if parameter is null(previously will return null) ### Test Tested with openai, no compilation issue found: ```java// Code generated by Microsoft (R) TypeSpec Code Generator. package com.openai; import io.clientcore.core.annotation.Metadata; import io.clientcore.core.util.ExpandableEnum; import java.util.ArrayList; import java.util.Collection; import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; /** * Defines values for OlympicRecordModel. */ public final class OlympicRecordModel implements ExpandableEnum { private static final Map VALUES = new ConcurrentHashMap<>(); private static final Function NEW_INSTANCE = OlympicRecordModel::new; /** * Static value 9.58 for OlympicRecordModel. */ @Metadata(generated = true) public static final OlympicRecordModel OLYMPIC_100_METERS = fromValue(9.58); /** * Static value 19.3 for OlympicRecordModel. */ @Metadata(generated = true) public static final OlympicRecordModel OLYMPIC_200_METERS = fromValue(19.3); private final Double value; private OlympicRecordModel(Double value) { this.value = value; } /** * Creates or finds a OlympicRecordModel. * * @param value a value to look for. * @return the corresponding OlympicRecordModel. * @throws IllegalArgumentException if value is null. */ @Metadata(generated = true) public static OlympicRecordModel fromValue(Double value) { if (value == null) { throw new IllegalArgumentException("'value' cannot be null."); } return VALUES.computeIfAbsent(value, NEW_INSTANCE); } /** * Gets known OlympicRecordModel values. * * @return Known OlympicRecordModel values. */ @Metadata(generated = true) public static Collection values() { return new ArrayList<>(VALUES.values()); } /** * Gets the value of the OlympicRecordModel instance. * * @return the value of the OlympicRecordModel instance. */ @Metadata(generated = true) @Override public Double getValue() { return this.value; } @Metadata(generated = true) @Override public String toString() { return Objects.toString(this.value); } @Metadata(generated = true) @Override public boolean equals(Object obj) { return this == obj; } @Metadata(generated = true) @Override public int hashCode() { return Objects.hashCode(this.value); } } ``` --- .../generator/core/mapper/ChoiceMapper.java | 6 +- .../core/model/clientmodel/ClassType.java | 1 + .../core/model/clientmodel/EnumType.java | 3 +- .../generator/core/template/EnumTemplate.java | 285 +++++++----------- .../models/PriorityModel.java | 5 +- .../models/OlympicRecordModel.java | 5 +- .../enumservice/models/PriorityModel.java | 5 +- 7 files changed, 125 insertions(+), 185 deletions(-) diff --git a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/mapper/ChoiceMapper.java b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/mapper/ChoiceMapper.java index 5433e6f68b..c8f34e3d46 100644 --- a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/mapper/ChoiceMapper.java +++ b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/mapper/ChoiceMapper.java @@ -4,6 +4,7 @@ package com.microsoft.typespec.http.client.generator.core.mapper; import com.microsoft.typespec.http.client.generator.core.extension.model.codemodel.ChoiceSchema; +import com.microsoft.typespec.http.client.generator.core.extension.plugin.JavaSettings; import com.microsoft.typespec.http.client.generator.core.model.clientmodel.ClassType; import com.microsoft.typespec.http.client.generator.core.model.clientmodel.EnumType; import com.microsoft.typespec.http.client.generator.core.model.clientmodel.IType; @@ -53,9 +54,12 @@ protected boolean useCodeModelNameForEnumMember() { private IType createChoiceType(ChoiceSchema enumType) { IType elementType = Mappers.getSchemaMapper().map(enumType.getChoiceType()); boolean isStringEnum = elementType == ClassType.STRING; - if (isStringEnum) { + JavaSettings javaSettings = JavaSettings.getInstance(); + if (isStringEnum && javaSettings.isBranded()) { + // for branded string enum, will generate ExpandableStringEnum subclass return MapperUtils.createEnumType(enumType, true, useCodeModelNameForEnumMember()); } else { + // other cases, will generate ExpandableEnum interface implementation return MapperUtils.createEnumType(enumType, true, useCodeModelNameForEnumMember(), "getValue", "fromValue"); } } diff --git a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java index 0477cfebbd..37e8ae191e 100644 --- a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java +++ b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java @@ -143,6 +143,7 @@ public String getGenericClass() { put(SimpleResponse.class, new ClassDetails(SimpleResponse.class, "io.clientcore.core.http.SimpleResponse")); put(ExpandableStringEnum.class, new ClassDetails(ExpandableStringEnum.class, "io.clientcore.core.util.ExpandableEnum")); + put(ExpandableEnum.class, new ClassDetails(ExpandableEnum.class, "io.clientcore.core.util.ExpandableEnum")); put(HttpResponseException.class, new ClassDetails(HttpResponseException.class, "io.clientcore.core.http.exception.HttpResponseException")); put(HttpTrait.class, new ClassDetails(HttpTrait.class, "io.clientcore.core.models.traits.HttpTrait")); diff --git a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/EnumType.java b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/EnumType.java index b4728d0413..ab4fff4375 100644 --- a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/EnumType.java +++ b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/EnumType.java @@ -120,8 +120,7 @@ public final String defaultValueExpression(String sourceExpression) { return getName() + "." + enumValue.getName(); } } - return String.format("%1$s.from%2$s(%3$s)", getName(), - CodeNamer.toPascalCase(this.getElementType().toString()), + return String.format("%1$s.%2$s(%3$s)", getName(), getFromMethodName(), this.getElementType().defaultValueExpression(sourceExpression)); } else { for (ClientEnumValue enumValue : this.getValues()) { diff --git a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/template/EnumTemplate.java b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/template/EnumTemplate.java index f75dd15152..8b1b3567c9 100644 --- a/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/template/EnumTemplate.java +++ b/packages/http-client-java/generator/http-client-generator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/template/EnumTemplate.java @@ -41,7 +41,7 @@ public final void write(EnumType enumType, JavaFile javaFile) { if (settings.isBranded()) { writeBrandedExpandableEnum(enumType, javaFile, settings); } else { - writeExpandableStringEnumInterface(enumType, javaFile, settings); + writeExpandableEnumInterface(enumType, javaFile, settings); } } else { writeEnum(enumType, javaFile, settings); @@ -59,185 +59,8 @@ protected void writeBrandedExpandableEnum(EnumType enumType, JavaFile javaFile, if (enumType.getElementType() == ClassType.STRING) { writeExpandableStringEnum(enumType, javaFile, settings); } else { - Set imports = new HashSet<>(); - imports.add("java.util.Collection"); - imports.add("java.lang.IllegalArgumentException"); - imports.add("java.util.Map"); - imports.add("java.util.concurrent.ConcurrentHashMap"); - imports.add("java.util.ArrayList"); - imports.add("java.util.Objects"); - imports.add(ClassType.EXPANDABLE_ENUM.getFullName()); - imports.add("java.util.function.Function"); - if (!settings.isStreamStyleSerialization()) { - imports.add("com.fasterxml.jackson.annotation.JsonCreator"); - } - - addGeneratedImport(imports); - - javaFile.declareImport(imports); - javaFile.javadocComment(comment -> comment.description(enumType.getDescription())); - - String enumName = enumType.getName(); - IType elementType = enumType.getElementType(); - String typeName = elementType.getClientType().asNullable().toString(); - String pascalTypeName = CodeNamer.toPascalCase(typeName); - String declaration = enumName + " implements ExpandableEnum<" + pascalTypeName + ">"; - javaFile.publicFinalClass(declaration, classBlock -> { - classBlock.privateStaticFinalVariable( - String.format("Map<%1$s, %2$s> VALUES = new ConcurrentHashMap<>()", pascalTypeName, enumName)); - classBlock.privateStaticFinalVariable( - String.format("Function<%1$s, %2$s> NEW_INSTANCE = %2$s::new", pascalTypeName, enumName)); - - for (ClientEnumValue enumValue : enumType.getValues()) { - String value = enumValue.getValue(); - classBlock.javadocComment(CoreUtils.isNullOrEmpty(enumValue.getDescription()) - ? "Static value " + value + " for " + enumName + "." - : enumValue.getDescription()); - addGeneratedAnnotation(classBlock); - classBlock.publicStaticFinalVariable(String.format("%1$s %2$s = fromValue(%3$s)", enumName, - enumValue.getName(), elementType.defaultValueExpression(value))); - } - - classBlock.variable(pascalTypeName + " value", JavaVisibility.Private, JavaModifier.Final); - classBlock.privateConstructor(enumName + "(" + pascalTypeName + " value)", ctor -> { - ctor.line("this.value = value;"); - }); - - // fromValue(typeName) - classBlock.javadocComment(comment -> { - comment.description("Creates or finds a " + enumName); - comment.param("value", "a value to look for"); - comment.methodReturns("the corresponding " + enumName); - }); - - addGeneratedAnnotation(classBlock); - if (!settings.isStreamStyleSerialization()) { - classBlock.annotation("JsonCreator"); - } - - classBlock.publicStaticMethod(String.format("%1$s fromValue(%2$s value)", enumName, pascalTypeName), - function -> { - function.line("Objects.requireNonNull(value, \"'value' cannot be null.\");"); - function.methodReturn("VALUES.computeIfAbsent(value, NEW_INSTANCE)"); - }); - - // values - classBlock.javadocComment(comment -> { - comment.description("Gets known " + enumName + " values."); - comment.methodReturns("Known " + enumName + " values."); - }); - addGeneratedAnnotation(classBlock); - classBlock.publicStaticMethod(String.format("Collection<%s> values()", enumName), - function -> function.methodReturn("new ArrayList<>(VALUES.values())")); - - // getValue - classBlock.javadocComment(comment -> { - comment.description("Gets the value of the " + enumName + " instance."); - comment.methodReturns("the value of the " + enumName + " instance."); - }); - - addGeneratedAnnotation(classBlock); - classBlock.annotation("Override"); - classBlock.publicMethod(pascalTypeName + " getValue()", - function -> function.methodReturn("this.value")); - - // toString - addGeneratedAnnotation(classBlock); - classBlock.annotation("Override"); - classBlock.method(JavaVisibility.Public, null, "String toString()", - function -> function.methodReturn("Objects.toString(this.value)")); - - // equals - addGeneratedAnnotation(classBlock); - classBlock.annotation("Override"); - classBlock.method(JavaVisibility.Public, null, "boolean equals(Object obj)", - function -> function.methodReturn("this == obj")); - - // hashcode - addGeneratedAnnotation(classBlock); - classBlock.annotation("Override"); - classBlock.method(JavaVisibility.Public, null, "int hashCode()", - function -> function.methodReturn("Objects.hashCode(this.value)")); - }); - } - } - - private void writeExpandableStringEnumInterface(EnumType enumType, JavaFile javaFile, JavaSettings settings) { - Set imports = new HashSet<>(); - imports.add("java.util.Collection"); - imports.add("java.util.concurrent.ConcurrentHashMap"); - imports.add("java.util.Map"); - imports.add(getStringEnumImport()); - if (!settings.isStreamStyleSerialization()) { - imports.add("com.fasterxml.jackson.annotation.JsonCreator"); + writeExpandableEnumInterface(enumType, javaFile, settings); } - - addGeneratedImport(imports); - - javaFile.declareImport(imports); - javaFile.javadocComment(comment -> comment.description(enumType.getDescription())); - - String enumName = enumType.getName(); - IType elementType = enumType.getElementType(); - String typeName = elementType.getClientType().toString(); - String pascalTypeName = CodeNamer.toPascalCase(typeName); - String declaration = enumName + " implements ExpandableEnum<" + pascalTypeName + ">"; - - javaFile.publicFinalClass(declaration, classBlock -> { - classBlock.privateStaticFinalVariable("Map VALUES = new ConcurrentHashMap<>()"); - - for (ClientEnumValue enumValue : enumType.getValues()) { - String value = enumValue.getValue(); - classBlock.javadocComment(CoreUtils.isNullOrEmpty(enumValue.getDescription()) - ? "Static value " + value + " for " + enumName + "." - : enumValue.getDescription()); - addGeneratedAnnotation(classBlock); - classBlock.publicStaticFinalVariable(String.format("%1$s %2$s = from%3$s(%4$s)", enumName, - enumValue.getName(), pascalTypeName, elementType.defaultValueExpression(value))); - } - - classBlock.variable("String name", JavaVisibility.Private, JavaModifier.Final); - classBlock.privateConstructor(enumName + "(String name)", ctor -> { - ctor.line("this.name = name;"); - }); - - // fromString(typeName) - classBlock.javadocComment(comment -> { - comment.description("Creates or finds a " + enumName); - comment.param("name", "a name to look for"); - comment.methodReturns("the corresponding " + enumName); - }); - - addGeneratedAnnotation(classBlock); - if (!settings.isStreamStyleSerialization()) { - classBlock.annotation("JsonCreator"); - } - - classBlock.publicStaticMethod(String.format("%1$s from%2$s(%3$s name)", enumName, pascalTypeName, typeName), - function -> { - function.ifBlock("name == null", ifAction -> ifAction.methodReturn("null")); - function.line(enumName + " value = VALUES.get(name);"); - function.ifBlock("value != null", ifAction -> { - ifAction.line("return value;"); - }); - function.methodReturn("VALUES.computeIfAbsent(name, key -> new " + enumName + "(key))"); - }); - - // getValue - classBlock.javadocComment(comment -> { - comment.description("Gets the value of the " + enumName + " instance."); - comment.methodReturns("the value of the " + enumName + " instance."); - }); - - addGeneratedAnnotation(classBlock); - classBlock.annotation("Override"); - classBlock.publicMethod(pascalTypeName + " getValue()", function -> function.methodReturn("this.name")); - - addGeneratedAnnotation(classBlock); - classBlock.annotation("Override"); - classBlock.method(JavaVisibility.Public, null, "String toString()", - function -> function.methodReturn("name")); - }); } private void writeExpandableStringEnum(EnumType enumType, JavaFile javaFile, JavaSettings settings) { @@ -385,6 +208,110 @@ private void writeEnum(EnumType enumType, JavaFile javaFile, JavaSettings settin }); } + private void writeExpandableEnumInterface(EnumType enumType, JavaFile javaFile, JavaSettings settings) { + Set imports = new HashSet<>(); + imports.add("java.util.Collection"); + imports.add("java.lang.IllegalArgumentException"); + imports.add("java.util.Map"); + imports.add("java.util.concurrent.ConcurrentHashMap"); + imports.add("java.util.ArrayList"); + imports.add("java.util.Objects"); + imports.add(ClassType.EXPANDABLE_ENUM.getFullName()); + imports.add("java.util.function.Function"); + if (!settings.isStreamStyleSerialization()) { + imports.add("com.fasterxml.jackson.annotation.JsonCreator"); + } + + addGeneratedImport(imports); + + javaFile.declareImport(imports); + javaFile.javadocComment(comment -> comment.description(enumType.getDescription())); + + String enumName = enumType.getName(); + IType elementType = enumType.getElementType(); + String typeName = elementType.getClientType().asNullable().toString(); + String pascalTypeName = CodeNamer.toPascalCase(typeName); + String declaration = enumName + " implements ExpandableEnum<" + pascalTypeName + ">"; + javaFile.publicFinalClass(declaration, classBlock -> { + classBlock.privateStaticFinalVariable( + String.format("Map<%1$s, %2$s> VALUES = new ConcurrentHashMap<>()", pascalTypeName, enumName)); + classBlock.privateStaticFinalVariable( + String.format("Function<%1$s, %2$s> NEW_INSTANCE = %2$s::new", pascalTypeName, enumName)); + + for (ClientEnumValue enumValue : enumType.getValues()) { + String value = enumValue.getValue(); + classBlock.javadocComment(CoreUtils.isNullOrEmpty(enumValue.getDescription()) + ? "Static value " + value + " for " + enumName + "." + : enumValue.getDescription()); + addGeneratedAnnotation(classBlock); + classBlock.publicStaticFinalVariable(String.format("%1$s %2$s = fromValue(%3$s)", enumName, + enumValue.getName(), elementType.defaultValueExpression(value))); + } + + classBlock.variable(pascalTypeName + " value", JavaVisibility.Private, JavaModifier.Final); + classBlock.privateConstructor(enumName + "(" + pascalTypeName + " value)", ctor -> { + ctor.line("this.value = value;"); + }); + + // fromValue(typeName) + classBlock.javadocComment(comment -> { + comment.description("Creates or finds a " + enumName); + comment.param("value", "a value to look for"); + comment.methodReturns("the corresponding " + enumName); + comment.methodThrows("IllegalArgumentException", "if value is null"); + }); + + addGeneratedAnnotation(classBlock); + if (!settings.isStreamStyleSerialization()) { + classBlock.annotation("JsonCreator"); + } + + classBlock.publicStaticMethod(String.format("%1$s fromValue(%2$s value)", enumName, pascalTypeName), + function -> { + function.ifBlock("value == null", + ifBlock -> ifBlock.line("throw new IllegalArgumentException(\"'value' cannot be null.\");")); + function.methodReturn("VALUES.computeIfAbsent(value, NEW_INSTANCE)"); + }); + + // values + classBlock.javadocComment(comment -> { + comment.description("Gets known " + enumName + " values."); + comment.methodReturns("Known " + enumName + " values."); + }); + addGeneratedAnnotation(classBlock); + classBlock.publicStaticMethod(String.format("Collection<%s> values()", enumName), + function -> function.methodReturn("new ArrayList<>(VALUES.values())")); + + // getValue + classBlock.javadocComment(comment -> { + comment.description("Gets the value of the " + enumName + " instance."); + comment.methodReturns("the value of the " + enumName + " instance."); + }); + + addGeneratedAnnotation(classBlock); + classBlock.annotation("Override"); + classBlock.publicMethod(pascalTypeName + " getValue()", function -> function.methodReturn("this.value")); + + // toString + addGeneratedAnnotation(classBlock); + classBlock.annotation("Override"); + classBlock.method(JavaVisibility.Public, null, "String toString()", + function -> function.methodReturn("Objects.toString(this.value)")); + + // equals + addGeneratedAnnotation(classBlock); + classBlock.annotation("Override"); + classBlock.method(JavaVisibility.Public, null, "boolean equals(Object obj)", + function -> function.methodReturn("this == obj")); + + // hashcode + addGeneratedAnnotation(classBlock); + classBlock.annotation("Override"); + classBlock.method(JavaVisibility.Public, null, "int hashCode()", + function -> function.methodReturn("Objects.hashCode(this.value)")); + }); + } + protected String getStringEnumImport() { return ClassType.EXPANDABLE_STRING_ENUM.getFullName(); } diff --git a/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armresourceprovider/models/PriorityModel.java b/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armresourceprovider/models/PriorityModel.java index 33536bff25..cf1fbdf017 100644 --- a/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armresourceprovider/models/PriorityModel.java +++ b/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/armresourceprovider/models/PriorityModel.java @@ -42,10 +42,13 @@ private PriorityModel(Integer value) { * * @param value a value to look for. * @return the corresponding PriorityModel. + * @throws IllegalArgumentException if value is null. */ @JsonCreator public static PriorityModel fromValue(Integer value) { - Objects.requireNonNull(value, "'value' cannot be null."); + if (value == null) { + throw new IllegalArgumentException("'value' cannot be null."); + } return VALUES.computeIfAbsent(value, NEW_INSTANCE); } diff --git a/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/OlympicRecordModel.java b/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/OlympicRecordModel.java index c46f716d2b..1b12c43962 100644 --- a/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/OlympicRecordModel.java +++ b/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/OlympicRecordModel.java @@ -44,10 +44,13 @@ private OlympicRecordModel(Double value) { * * @param value a value to look for. * @return the corresponding OlympicRecordModel. + * @throws IllegalArgumentException if value is null. */ @Generated public static OlympicRecordModel fromValue(Double value) { - Objects.requireNonNull(value, "'value' cannot be null."); + if (value == null) { + throw new IllegalArgumentException("'value' cannot be null."); + } return VALUES.computeIfAbsent(value, NEW_INSTANCE); } diff --git a/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/PriorityModel.java b/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/PriorityModel.java index 43b1221128..c5c75d6d73 100644 --- a/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/PriorityModel.java +++ b/packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/enumservice/models/PriorityModel.java @@ -44,10 +44,13 @@ private PriorityModel(Integer value) { * * @param value a value to look for. * @return the corresponding PriorityModel. + * @throws IllegalArgumentException if value is null. */ @Generated public static PriorityModel fromValue(Integer value) { - Objects.requireNonNull(value, "'value' cannot be null."); + if (value == null) { + throw new IllegalArgumentException("'value' cannot be null."); + } return VALUES.computeIfAbsent(value, NEW_INSTANCE); }