Skip to content

Commit

Permalink
http-client-java, fix expandable enum equals (#5283)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@XiaofeiCao
XiaofeiCao authored Dec 6, 2024
1 parent 252b3df commit 6fc2484
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 22 deletions.
9 changes: 5 additions & 4 deletions ...rc/main/java/com/microsoft/typespec/http/client/generator/core/template/EnumTemplate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 5 additions & 6 deletions ...client-generator-test/src/main/java/tsptest/armresourceprovider/models/PriorityModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

/**
* Defines values for PriorityModel.
*/
public final class PriorityModel implements ExpandableEnum<Integer> {
private static final Map<Integer, PriorityModel> VALUES = new ConcurrentHashMap<>();

private static final Function<Integer, PriorityModel> NEW_INSTANCE = PriorityModel::new;

/**
* Static value 0 for PriorityModel.
*/
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -76,7 +75,7 @@ public String toString() {

@Override
public boolean equals(Object obj) {
return Objects.equals(this.value, obj);
return this == obj;
}

@Override
Expand Down
11 changes: 5 additions & 6 deletions ...tp-client-generator-test/src/main/java/tsptest/enumservice/models/OlympicRecordModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@
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<Double> {
private static final Map<Double, OlympicRecordModel> VALUES = new ConcurrentHashMap<>();

private static final Function<Double, OlympicRecordModel> NEW_INSTANCE = OlympicRecordModel::new;

/**
* Static value 9.58 for OlympicRecordModel.
*/
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -82,7 +81,7 @@ public String toString() {
@Generated
@Override
public boolean equals(Object obj) {
return Objects.equals(this.value, obj);
return this == obj;
}

@Generated
Expand Down
11 changes: 5 additions & 6 deletions ...or/http-client-generator-test/src/main/java/tsptest/enumservice/models/PriorityModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

/**
* Defines values for PriorityModel.
*/
public final class PriorityModel implements ExpandableEnum<Integer> {
private static final Map<Integer, PriorityModel> VALUES = new ConcurrentHashMap<>();

private static final Function<Integer, PriorityModel> NEW_INSTANCE = PriorityModel::new;

/**
* Static value 100 for PriorityModel.
*/
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -82,7 +81,7 @@ public String toString() {
@Generated
@Override
public boolean equals(Object obj) {
return Objects.equals(this.value, obj);
return this == obj;
}

@Generated
Expand Down
10 changes: 10 additions & 0 deletions ...ava/generator/http-client-generator-test/src/test/java/tsptest/enumservice/EnumTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit 6fc2484

Please sign in to comment.