Skip to content

Commit

Permalink
fix: validate enum values in nested models (#163)
Browse files Browse the repository at this point in the history
The Java generator for `openapi-generator` completely skips over enum
validation in the `validateJsonElement` function that is generated for
each model class. As a result, enum values are validated inconsistently;
if I have a model `X` that has a property that is an enum, the enum
values will be validated if I am deserializing a JSON blob in which the
top-level object is an instance of `X`, but the enum values will _not_
be validated if I am deserializing a JSON blob that has an instance of
`X` nested in it.

```js
// Let's say I have a schema component `X` that has a `this_is_an` property that is a string and a `status` property that is an enum of `"active"` or `"failed"`.
// Given that component, the below is not a valid `X` and would cause an error in metal-java <= v0.10.0
{
   "this_is_an": "X",
   "status": "not-a-status"
}

// The below is an object that has a property, `x_list` that is a list of `X` instances.  The `X` in this case is still not valid, but this would not cause an error in metal-java <= v0.10.0
{
  "x_list": [{
     "this_is_an": "X",
     "status": "not-a-status"
  }]
}
```

This PR introduces custom templates so that we can add enum validation
to `validateJsonElement` and ensure that enum values are validated even
for nested models.

We needed to modify two templates:
- `libraries/okhttp-gson/pojo.mustache` had to be updated to generate
code that validates model properties that are enums
- `modelEnum.mustache` and `modelInnerEnum.mustache` had to be updated
to generate a `validateJsonElement` function for all enums

Template changes happen in this commit:
ddc37db
Code is updated by the bot in this commit:
b0f102a

This PR also includes an `example` that I used to replicate the
validation issue and confirm that my tests fixed it; I intended to
remove it before merge, since it's really not an example of how the SDK
should be used, but I'm open to keeping it. In the meantime, at least,
this PR can be poked at by checking it out locally and running these
commands in the project root:

```sh
# if you check out the first commit on this PR, you will need to regenerate the code so the example uses the old code
make generate
# recompile the code so that the example is using the code that was generated above
make build_client
# tell Java where to find the compiled code
export ExampleClassPath="equinix-openapi-metal/target/equinix-openapi-metal-0.10.0.jar:equinix-openapi-metal/target/lib/*"
# run the example (doesn't hit an API, so doesn't need a token)
java -classpath $ExampleClassPath examples/VirtualCircuit.java
```

---------

Co-authored-by: equinix-labs@auto-commit-workflow <[email protected]>
  • Loading branch information
ctreatma and equinix-labs@auto-commit-workflow authored Oct 18, 2023
1 parent 2672e4f commit 974a694
Show file tree
Hide file tree
Showing 55 changed files with 1,530 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ public TypeEnum read(final JsonReader jsonReader) throws IOException {
return TypeEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
TypeEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_TYPE = "type";
Expand Down Expand Up @@ -880,6 +885,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("type") != null && !jsonObj.get("type").isJsonNull()) && !jsonObj.get("type").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `type` to be a primitive type in the JSON string but got `%s`", jsonObj.get("type").toString()));
}
// validate the optional field `type`
if (jsonObj.get("type") != null && !jsonObj.get("type").isJsonNull()) {
TypeEnum.validateJsonElement(jsonObj.get("type"));
}
// ensure the optional json data is an array if present
if (jsonObj.get("tags") != null && !jsonObj.get("tags").isJsonNull() && !jsonObj.get("tags").isJsonArray()) {
throw new IllegalArgumentException(String.format("Expected the field `tags` to be an array in the JSON string but got `%s`", jsonObj.get("tags").toString()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ public AddressFamilyEnum read(final JsonReader jsonReader) throws IOException {
return AddressFamilyEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
AddressFamilyEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_ADDRESS_FAMILY = "address_family";
Expand Down Expand Up @@ -268,6 +273,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("address_family") != null && !jsonObj.get("address_family").isJsonNull()) && !jsonObj.get("address_family").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `address_family` to be a primitive type in the JSON string but got `%s`", jsonObj.get("address_family").toString()));
}
// validate the optional field `address_family`
if (jsonObj.get("address_family") != null && !jsonObj.get("address_family").isJsonNull()) {
AddressFamilyEnum.validateJsonElement(jsonObj.get("address_family"));
}
}

public static class CustomTypeAdapterFactory implements TypeAdapterFactory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ public DeploymentTypeEnum read(final JsonReader jsonReader) throws IOException {
return DeploymentTypeEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
DeploymentTypeEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_DEPLOYMENT_TYPE = "deployment_type";
Expand Down Expand Up @@ -202,6 +207,11 @@ public StatusEnum read(final JsonReader jsonReader) throws IOException {
return StatusEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
StatusEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_STATUS = "status";
Expand Down Expand Up @@ -660,6 +670,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("deployment_type") != null && !jsonObj.get("deployment_type").isJsonNull()) && !jsonObj.get("deployment_type").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `deployment_type` to be a primitive type in the JSON string but got `%s`", jsonObj.get("deployment_type").toString()));
}
// validate the optional field `deployment_type`
if (jsonObj.get("deployment_type") != null && !jsonObj.get("deployment_type").isJsonNull()) {
DeploymentTypeEnum.validateJsonElement(jsonObj.get("deployment_type"));
}
if ((jsonObj.get("href") != null && !jsonObj.get("href").isJsonNull()) && !jsonObj.get("href").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `href` to be a primitive type in the JSON string but got `%s`", jsonObj.get("href").toString()));
}
Expand Down Expand Up @@ -707,6 +721,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("status") != null && !jsonObj.get("status").isJsonNull()) && !jsonObj.get("status").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `status` to be a primitive type in the JSON string but got `%s`", jsonObj.get("status").toString()));
}
// validate the optional field `status`
if (jsonObj.get("status") != null && !jsonObj.get("status").isJsonNull()) {
StatusEnum.validateJsonElement(jsonObj.get("status"));
}
}

public static class CustomTypeAdapterFactory implements TypeAdapterFactory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ public DeploymentTypeEnum read(final JsonReader jsonReader) throws IOException {
return DeploymentTypeEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
DeploymentTypeEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_DEPLOYMENT_TYPE = "deployment_type";
Expand Down Expand Up @@ -335,6 +340,8 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if (!jsonObj.get("deployment_type").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `deployment_type` to be a primitive type in the JSON string but got `%s`", jsonObj.get("deployment_type").toString()));
}
// validate the required field `deployment_type`
DeploymentTypeEnum.validateJsonElement(jsonObj.get("deployment_type"));
if ((jsonObj.get("md5") != null && !jsonObj.get("md5").isJsonNull()) && !jsonObj.get("md5").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `md5` to be a primitive type in the JSON string but got `%s`", jsonObj.get("md5").toString()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ public StateEnum read(final JsonReader jsonReader) throws IOException {
return StateEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
StateEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_STATE = "state";
Expand Down Expand Up @@ -491,6 +496,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("state") != null && !jsonObj.get("state").isJsonNull()) && !jsonObj.get("state").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `state` to be a primitive type in the JSON string but got `%s`", jsonObj.get("state").toString()));
}
// validate the optional field `state`
if (jsonObj.get("state") != null && !jsonObj.get("state").isJsonNull()) {
StateEnum.validateJsonElement(jsonObj.get("state"));
}
if ((jsonObj.get("href") != null && !jsonObj.get("href").isJsonNull()) && !jsonObj.get("href").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `href` to be a primitive type in the JSON string but got `%s`", jsonObj.get("href").toString()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ public AddressFamilyEnum read(final JsonReader jsonReader) throws IOException {
return AddressFamilyEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
AddressFamilyEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_ADDRESS_FAMILY = "address_family";
Expand Down Expand Up @@ -179,6 +184,11 @@ public StatusEnum read(final JsonReader jsonReader) throws IOException {
return StatusEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
StatusEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_STATUS = "status";
Expand Down Expand Up @@ -534,6 +544,8 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if (!jsonObj.get("address_family").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `address_family` to be a primitive type in the JSON string but got `%s`", jsonObj.get("address_family").toString()));
}
// validate the required field `address_family`
AddressFamilyEnum.validateJsonElement(jsonObj.get("address_family"));
// validate the optional field `device`
if (jsonObj.get("device") != null && !jsonObj.get("device").isJsonNull()) {
Href.validateJsonElement(jsonObj.get("device"));
Expand All @@ -551,6 +563,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("status") != null && !jsonObj.get("status").isJsonNull()) && !jsonObj.get("status").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `status` to be a primitive type in the JSON string but got `%s`", jsonObj.get("status").toString()));
}
// validate the optional field `status`
if (jsonObj.get("status") != null && !jsonObj.get("status").isJsonNull()) {
StatusEnum.validateJsonElement(jsonObj.get("status"));
}
}

public static class CustomTypeAdapterFactory implements TypeAdapterFactory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ public CountEnum read(final JsonReader jsonReader) throws IOException {
return CountEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
Integer value = jsonElement.getAsInt();
CountEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_COUNT = "count";
Expand Down Expand Up @@ -146,6 +151,11 @@ public UnitEnum read(final JsonReader jsonReader) throws IOException {
return UnitEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
UnitEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_UNIT = "unit";
Expand Down Expand Up @@ -310,9 +320,17 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
}
}
JsonObject jsonObj = jsonElement.getAsJsonObject();
// validate the optional field `count`
if (jsonObj.get("count") != null && !jsonObj.get("count").isJsonNull()) {
CountEnum.validateJsonElement(jsonObj.get("count"));
}
if ((jsonObj.get("unit") != null && !jsonObj.get("unit").isJsonNull()) && !jsonObj.get("unit").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `unit` to be a primitive type in the JSON string but got `%s`", jsonObj.get("unit").toString()));
}
// validate the optional field `unit`
if (jsonObj.get("unit") != null && !jsonObj.get("unit").isJsonNull()) {
UnitEnum.validateJsonElement(jsonObj.get("unit"));
}
}

public static class CustomTypeAdapterFactory implements TypeAdapterFactory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ public ModeEnum read(final JsonReader jsonReader) throws IOException {
return ModeEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
ModeEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_MODE = "mode";
Expand Down Expand Up @@ -184,6 +189,11 @@ public TypeEnum read(final JsonReader jsonReader) throws IOException {
return TypeEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
TypeEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_TYPE = "type";
Expand Down Expand Up @@ -626,6 +636,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("mode") != null && !jsonObj.get("mode").isJsonNull()) && !jsonObj.get("mode").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `mode` to be a primitive type in the JSON string but got `%s`", jsonObj.get("mode").toString()));
}
// validate the optional field `mode`
if (jsonObj.get("mode") != null && !jsonObj.get("mode").isJsonNull()) {
ModeEnum.validateJsonElement(jsonObj.get("mode"));
}
if (!jsonObj.get("name").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `name` to be a primitive type in the JSON string but got `%s`", jsonObj.get("name").toString()));
}
Expand All @@ -642,6 +656,8 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if (!jsonObj.get("type").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `type` to be a primitive type in the JSON string but got `%s`", jsonObj.get("type").toString()));
}
// validate the required field `type`
TypeEnum.validateJsonElement(jsonObj.get("type"));
if ((jsonObj.get("use_case") != null && !jsonObj.get("use_case").isJsonNull()) && !jsonObj.get("use_case").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `use_case` to be a primitive type in the JSON string but got `%s`", jsonObj.get("use_case").toString()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@ public StateEnum read(final JsonReader jsonReader) throws IOException {
return StateEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
StateEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_STATE = "state";
Expand Down Expand Up @@ -1651,6 +1656,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("state") != null && !jsonObj.get("state").isJsonNull()) && !jsonObj.get("state").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `state` to be a primitive type in the JSON string but got `%s`", jsonObj.get("state").toString()));
}
// validate the optional field `state`
if (jsonObj.get("state") != null && !jsonObj.get("state").isJsonNull()) {
StateEnum.validateJsonElement(jsonObj.get("state"));
}
// validate the optional field `storage`
if (jsonObj.get("storage") != null && !jsonObj.get("storage").isJsonNull()) {
Storage.validateJsonElement(jsonObj.get("storage"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ public TypeEnum read(final JsonReader jsonReader) throws IOException {
return TypeEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
TypeEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_TYPE = "type";
Expand Down Expand Up @@ -394,6 +399,8 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if (!jsonObj.get("type").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `type` to be a primitive type in the JSON string but got `%s`", jsonObj.get("type").toString()));
}
// validate the required field `type`
TypeEnum.validateJsonElement(jsonObj.get("type"));
if ((jsonObj.get("operating_system") != null && !jsonObj.get("operating_system").isJsonNull()) && !jsonObj.get("operating_system").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `operating_system` to be a primitive type in the JSON string but got `%s`", jsonObj.get("operating_system").toString()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ public BillingCycleEnum read(final JsonReader jsonReader) throws IOException {
return BillingCycleEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
BillingCycleEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_BILLING_CYCLE = "billing_cycle";
Expand Down Expand Up @@ -1038,6 +1043,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("billing_cycle") != null && !jsonObj.get("billing_cycle").isJsonNull()) && !jsonObj.get("billing_cycle").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `billing_cycle` to be a primitive type in the JSON string but got `%s`", jsonObj.get("billing_cycle").toString()));
}
// validate the optional field `billing_cycle`
if (jsonObj.get("billing_cycle") != null && !jsonObj.get("billing_cycle").isJsonNull()) {
BillingCycleEnum.validateJsonElement(jsonObj.get("billing_cycle"));
}
if ((jsonObj.get("description") != null && !jsonObj.get("description").isJsonNull()) && !jsonObj.get("description").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `description` to be a primitive type in the JSON string but got `%s`", jsonObj.get("description").toString()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ public BillingCycleEnum read(final JsonReader jsonReader) throws IOException {
return BillingCycleEnum.fromValue(value);
}
}

public static void validateJsonElement(JsonElement jsonElement) throws IOException {
String value = jsonElement.getAsString();
BillingCycleEnum.fromValue(value);
}
}

public static final String SERIALIZED_NAME_BILLING_CYCLE = "billing_cycle";
Expand Down Expand Up @@ -1022,6 +1027,10 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
if ((jsonObj.get("billing_cycle") != null && !jsonObj.get("billing_cycle").isJsonNull()) && !jsonObj.get("billing_cycle").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `billing_cycle` to be a primitive type in the JSON string but got `%s`", jsonObj.get("billing_cycle").toString()));
}
// validate the optional field `billing_cycle`
if (jsonObj.get("billing_cycle") != null && !jsonObj.get("billing_cycle").isJsonNull()) {
BillingCycleEnum.validateJsonElement(jsonObj.get("billing_cycle"));
}
if ((jsonObj.get("description") != null && !jsonObj.get("description").isJsonNull()) && !jsonObj.get("description").isJsonPrimitive()) {
throw new IllegalArgumentException(String.format("Expected the field `description` to be a primitive type in the JSON string but got `%s`", jsonObj.get("description").toString()));
}
Expand Down
Loading

0 comments on commit 974a694

Please sign in to comment.