From 6fc24841fba1e1f9b63bfe7e30a3a79d109f91ef Mon Sep 17 00:00:00 2001 From: Xiaofei Cao <92354331+XiaofeiCao@users.noreply.github.com> Date: Fri, 6 Dec 2024 17:47:31 +0800 Subject: [PATCH] http-client-java, fix expandable enum `equals` (#5283) Bug found in deserialization mock test: https://dev.azure.com/azure-sdk/public/_build/results?buildId=4390469&view=logs&jobId=1ddb3322-d991-5251-324c-57ed6980c710&j=1ddb3322-d991-5251-324c-57ed6980c710&t=0da78e2d-17fd-5004-a494-748a34fd2ecc Fluent lite packages will experience generation failure, thus already released packages should be good. Premium packages are generated with `ExpandableStringEnum`, thus not affected as well. Not sure about DPG packages. I've not yet seen any released packages with ExpandableEnum. --- .../client/generator/core/template/EnumTemplate.java | 9 +++++---- .../armresourceprovider/models/PriorityModel.java | 11 +++++------ .../enumservice/models/OlympicRecordModel.java | 11 +++++------ .../tsptest/enumservice/models/PriorityModel.java | 11 +++++------ .../src/test/java/tsptest/enumservice/EnumTests.java | 10 ++++++++++ 5 files changed, 30 insertions(+), 22 deletions(-) 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 d7fbac8ba4..f75dd15152 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 @@ -67,6 +67,7 @@ protected void writeBrandedExpandableEnum(EnumType enumType, JavaFile javaFile, 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"); } @@ -84,6 +85,8 @@ protected void writeBrandedExpandableEnum(EnumType enumType, JavaFile javaFile, 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(); @@ -115,9 +118,7 @@ protected void writeBrandedExpandableEnum(EnumType enumType, JavaFile javaFile, classBlock.publicStaticMethod(String.format("%1$s fromValue(%2$s value)", enumName, pascalTypeName), function -> { function.line("Objects.requireNonNull(value, \"'value' cannot be null.\");"); - function.line(enumName + " member = VALUES.get(value);"); - function.ifBlock("member != null", ifAction -> ifAction.line("return member;")); - function.methodReturn("VALUES.computeIfAbsent(value, key -> new " + enumName + "(key))"); + function.methodReturn("VALUES.computeIfAbsent(value, NEW_INSTANCE)"); }); // values @@ -150,7 +151,7 @@ protected void writeBrandedExpandableEnum(EnumType enumType, JavaFile javaFile, addGeneratedAnnotation(classBlock); classBlock.annotation("Override"); classBlock.method(JavaVisibility.Public, null, "boolean equals(Object obj)", - function -> function.methodReturn("Objects.equals(this.value, obj)")); + function -> function.methodReturn("this == obj")); // hashcode addGeneratedAnnotation(classBlock); 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 80db9412da..33536bff25 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 @@ -11,6 +11,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; /** * Defines values for PriorityModel. @@ -18,6 +19,8 @@ public final class PriorityModel implements ExpandableEnum { private static final Map VALUES = new ConcurrentHashMap<>(); + private static final Function NEW_INSTANCE = PriorityModel::new; + /** * Static value 0 for PriorityModel. */ @@ -43,11 +46,7 @@ private PriorityModel(Integer value) { @JsonCreator public static PriorityModel fromValue(Integer value) { Objects.requireNonNull(value, "'value' cannot be null."); - PriorityModel member = VALUES.get(value); - if (member != null) { - return member; - } - return VALUES.computeIfAbsent(value, key -> new PriorityModel(key)); + return VALUES.computeIfAbsent(value, NEW_INSTANCE); } /** @@ -76,7 +75,7 @@ public String toString() { @Override public boolean equals(Object obj) { - return Objects.equals(this.value, obj); + return this == obj; } @Override 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 dce6221287..c46f716d2b 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 @@ -11,6 +11,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; /** * Defines values for OlympicRecordModel. @@ -18,6 +19,8 @@ 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. */ @@ -45,11 +48,7 @@ private OlympicRecordModel(Double value) { @Generated public static OlympicRecordModel fromValue(Double value) { Objects.requireNonNull(value, "'value' cannot be null."); - OlympicRecordModel member = VALUES.get(value); - if (member != null) { - return member; - } - return VALUES.computeIfAbsent(value, key -> new OlympicRecordModel(key)); + return VALUES.computeIfAbsent(value, NEW_INSTANCE); } /** @@ -82,7 +81,7 @@ public String toString() { @Generated @Override public boolean equals(Object obj) { - return Objects.equals(this.value, obj); + return this == obj; } @Generated 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 99432a3cf0..43b1221128 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 @@ -11,6 +11,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; /** * Defines values for PriorityModel. @@ -18,6 +19,8 @@ public final class PriorityModel implements ExpandableEnum { private static final Map VALUES = new ConcurrentHashMap<>(); + private static final Function NEW_INSTANCE = PriorityModel::new; + /** * Static value 100 for PriorityModel. */ @@ -45,11 +48,7 @@ private PriorityModel(Integer value) { @Generated public static PriorityModel fromValue(Integer value) { Objects.requireNonNull(value, "'value' cannot be null."); - PriorityModel member = VALUES.get(value); - if (member != null) { - return member; - } - return VALUES.computeIfAbsent(value, key -> new PriorityModel(key)); + return VALUES.computeIfAbsent(value, NEW_INSTANCE); } /** @@ -82,7 +81,7 @@ public String toString() { @Generated @Override public boolean equals(Object obj) { - return Objects.equals(this.value, obj); + return this == obj; } @Generated diff --git a/packages/http-client-java/generator/http-client-generator-test/src/test/java/tsptest/enumservice/EnumTests.java b/packages/http-client-java/generator/http-client-generator-test/src/test/java/tsptest/enumservice/EnumTests.java index 05bf2aa4df..fd3ba7b708 100644 --- a/packages/http-client-java/generator/http-client-generator-test/src/test/java/tsptest/enumservice/EnumTests.java +++ b/packages/http-client-java/generator/http-client-generator-test/src/test/java/tsptest/enumservice/EnumTests.java @@ -23,6 +23,7 @@ import tsptest.enumservice.implementation.EnumServiceClientImpl; import tsptest.enumservice.models.ColorModel; import tsptest.enumservice.models.Priority; +import tsptest.enumservice.models.PriorityModel; public class EnumTests { @@ -129,6 +130,15 @@ public void testStringArrayAsMulti() throws Exception { Assertions.assertEquals("colorArrayOpt=Green&colorArrayOpt=Red", request.getUrl().getQuery()); } + @Test + public void testExpandableEnum() { + Assertions.assertEquals(PriorityModel.HIGH, PriorityModel.fromValue(100)); + Assertions.assertNotEquals(PriorityModel.HIGH, PriorityModel.LOW); + Assertions.assertNotEquals(PriorityModel.HIGH, PriorityModel.fromValue(200)); + + Assertions.assertEquals(100, PriorityModel.HIGH.getValue()); + } + private static void verifyQuery(String query, String key, String value) { Assertions.assertEquals( URLEncoder.encode(key, StandardCharsets.UTF_8) + "=" + URLEncoder.encode(value, StandardCharsets.UTF_8),