Skip to content

Commit

Permalink
fix!: additionalItems being applied for regular arrays (#1140)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonaslagoni authored Mar 8, 2023
1 parent cd77ed1 commit 2cb820f
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 73 deletions.
25 changes: 25 additions & 0 deletions docs/migrations/version-1-to-2.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,31 @@ components:
The _type_ property in the _CloudEvent_ schema will in this case have an _anonymous_schema_ id. If another schema in the _allOf_ list has the same property and an id other than _anonymous_schema_, it will now use that id. Meaning, in this example, it will be _DogType_ and _CatType_.
## Accurate array types
For JSON Schema inputs (indirectly for AsyncAPI and OpenAPI), `additionalItems` was applied to regular array types, when it should only be applied for tuples.

This means that a schema such as:
```
"tags": {
"type": "array",
"items": {
"$ref": "http://asyncapi.com/definitions/2.6.0/tag.json"
},
"additionalItems": true
},
```

Would generate a type such as, in TypeScript:
```
private _tags?: (Tag | any)[];
```

Where it now generates:
```
private _tags?: Tag[];
```

## Creates union type for operation message oneOf

In the example above, where `operation.message.oneOf` is set, Modelina will now generate a union type for it. Previously, Modelina ignored this union type, and only generated models for the content of `operation.message.oneOf`. In the example above, that meant models for `Dog` and `Cat`. Now, Modelina will generate a union type of `Pet` in addition to `Dog` and `Cat`.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ exports[`Should be able to render auto-implemented properties in CSharp and shou
Array [
"public class Root
{
public dynamic[]? Emails { get; set; }
public string[]? Emails { get; set; }
public string? StringProp { get; set; }
public double? NumberProp { get; set; }
}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ exports[`Should be able to render collections in C# as IEnumerable and should lo
Array [
"public class Root
{
private IEnumerable<dynamic>? email;
private IEnumerable<string>? email;
public IEnumerable<dynamic>? Email
public IEnumerable<string>? Email
{
get { return email; }
set { email = value; }
Expand Down
25 changes: 10 additions & 15 deletions src/helpers/CommonModelToMetaModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,15 +445,9 @@ export function convertToArrayModel(
return undefined;
}

const isNormalArray =
!Array.isArray(jsonSchemaModel.items) &&
jsonSchemaModel.additionalItems === undefined &&
jsonSchemaModel.items !== undefined;
//item multiple types + additionalItems sat = both count, as normal array
//item single type + additionalItems sat = contradicting, only items count, as normal array
//item not sat + additionalItems sat = anything is allowed, as normal array
//item single type + additionalItems not sat = normal array
//item not sat + additionalItems not sat = normal array, any type
const isNormalArray = !Array.isArray(jsonSchemaModel.items);
//items single type = normal array
//items not sat = normal array, any type
if (isNormalArray) {
const placeholderModel = new AnyModel('', undefined);
const metaModel = new ArrayModel(
Expand All @@ -462,12 +456,13 @@ export function convertToArrayModel(
placeholderModel
);
alreadySeenModels.set(jsonSchemaModel, metaModel);

const valueModel = convertToMetaModel(
jsonSchemaModel.items as CommonModel,
alreadySeenModels
);
metaModel.valueModel = valueModel;
if (jsonSchemaModel.items !== undefined) {
const valueModel = convertToMetaModel(
jsonSchemaModel.items as CommonModel,
alreadySeenModels
);
metaModel.valueModel = valueModel;
}
return metaModel;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ exports[`CSharpGenerator should render \`class\` type 1`] = `
private bool? marriage;
private dynamic? members;
private dynamic[]? tupleType;
private dynamic[] arrayType;
private string[] arrayType;
private Dictionary<string, dynamic>? additionalProperties;
public string StreetName
Expand Down Expand Up @@ -87,7 +87,7 @@ exports[`CSharpGenerator should render \`class\` type 1`] = `
set { tupleType = value; }
}
public dynamic[] ArrayType
public string[] ArrayType
{
get { return arrayType; }
set { arrayType = value; }
Expand Down
2 changes: 1 addition & 1 deletion test/generators/go/__snapshots__/GoGenerator.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ type Address struct {
Marriage bool
Members interface{}
TupleType []interface{}
ArrayType []interface{}
ArrayType []string
AdditionalProperties map[string]interface{}
}"
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ exports[`JAVA_COMMON_PRESET should render accurately when there is no additional
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
public Boolean getRequiredProp() { return this.requiredProp; }
public void setRequiredProp(Boolean requiredProp) { this.requiredProp = requiredProp; }
Expand All @@ -20,8 +20,8 @@ exports[`JAVA_COMMON_PRESET should render accurately when there is no additional
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
@Override
public boolean equals(Object o) {
Expand Down Expand Up @@ -75,7 +75,7 @@ exports[`JAVA_COMMON_PRESET should render common function in class by common pre
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
private Map<String, Object> additionalProperties;
public Boolean getRequiredProp() { return this.requiredProp; }
Expand All @@ -90,8 +90,8 @@ exports[`JAVA_COMMON_PRESET should render common function in class by common pre
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
Expand Down Expand Up @@ -150,7 +150,7 @@ exports[`JAVA_COMMON_PRESET with option should not render any functions when all
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
private Map<String, Object> additionalProperties;
public Boolean getRequiredProp() { return this.requiredProp; }
Expand All @@ -165,8 +165,8 @@ exports[`JAVA_COMMON_PRESET with option should not render any functions when all
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
Expand All @@ -179,7 +179,7 @@ exports[`JAVA_COMMON_PRESET with option should render all functions 1`] = `
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
private Map<String, Object> additionalProperties;
public Boolean getRequiredProp() { return this.requiredProp; }
Expand All @@ -194,8 +194,8 @@ exports[`JAVA_COMMON_PRESET with option should render all functions 1`] = `
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
Expand Down Expand Up @@ -254,7 +254,7 @@ exports[`JAVA_COMMON_PRESET with option should render classToString 1`] = `
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
private Map<String, Object> additionalProperties;
public Boolean getRequiredProp() { return this.requiredProp; }
Expand All @@ -269,8 +269,8 @@ exports[`JAVA_COMMON_PRESET with option should render classToString 1`] = `
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
Expand Down Expand Up @@ -306,7 +306,7 @@ exports[`JAVA_COMMON_PRESET with option should render equals 1`] = `
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
private Map<String, Object> additionalProperties;
public Boolean getRequiredProp() { return this.requiredProp; }
Expand All @@ -321,8 +321,8 @@ exports[`JAVA_COMMON_PRESET with option should render equals 1`] = `
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
Expand Down Expand Up @@ -353,7 +353,7 @@ exports[`JAVA_COMMON_PRESET with option should render hashCode 1`] = `
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
private Map<String, Object> additionalProperties;
public Boolean getRequiredProp() { return this.requiredProp; }
Expand All @@ -368,8 +368,8 @@ exports[`JAVA_COMMON_PRESET with option should render hashCode 1`] = `
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
Expand All @@ -387,7 +387,7 @@ exports[`JAVA_COMMON_PRESET with option should render un/marshal 1`] = `
private String stringProp;
private Double numberProp;
private Boolean booleanProp;
private Object[] arrayProp;
private String[] arrayProp;
private Map<String, Object> additionalProperties;
public Boolean getRequiredProp() { return this.requiredProp; }
Expand All @@ -402,8 +402,8 @@ exports[`JAVA_COMMON_PRESET with option should render un/marshal 1`] = `
public Boolean getBooleanProp() { return this.booleanProp; }
public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; }
public Object[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; }
public String[] getArrayProp() { return this.arrayProp; }
public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; }
public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
Expand Down Expand Up @@ -447,7 +447,7 @@ exports[`JAVA_COMMON_PRESET with option should render un/marshal 1`] = `
result.setBooleanProp(jsonObject.getBoolean(\\"booleanProp\\"));
}
if(jsonObject.has(\\"arrayProp\\")) {
result.setArrayProp(jsonObject.getObject[](\\"arrayProp\\"));
result.setArrayProp(jsonObject.getString[](\\"arrayProp\\"));
}
if(jsonObject.has(\\"additionalProperties\\")) {
result.setAdditionalProperties(jsonObject.getMap<String, Object>(\\"additionalProperties\\"));
Expand Down
3 changes: 1 addition & 2 deletions test/generators/rust/presets/CommonPreset.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,9 @@ describe('RUST_COMMON_PRESET', () => {
};

const models = await generator.generate(doc);
expect(models).toHaveLength(3);
expect(models).toHaveLength(2);
expect(models[0].result).toMatchSnapshot();
expect(models[1].result).toMatchSnapshot();
expect(models[2].result).toMatchSnapshot();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,13 @@ exports[`RUST_COMMON_PRESET Enum should render reserved union for dict array 1`]
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct Class {
#[serde(rename=\\"students\\")]
pub students: Vec<crate::Union>,
pub students: Vec<crate::Student>,
#[serde(rename=\\"additionalProperties\\", skip_serializing_if = \\"Option::is_none\\")]
pub additional_properties: Option<std::collections::HashMap<String, serde_json::Value>>,
}
impl Class {
pub fn new(students: Vec<crate::Union>, additional_properties: Option<std::collections::HashMap<String, serde_json::Value>>) -> Class {
pub fn new(students: Vec<crate::Student>, additional_properties: Option<std::collections::HashMap<String, serde_json::Value>>) -> Class {
Class {
students,
additional_properties,
Expand All @@ -214,20 +214,6 @@ impl Class {
`;

exports[`RUST_COMMON_PRESET Enum should render reserved union for dict array 2`] = `
"// Union represents a union of types: Student, serde_json::Value
#[derive(Clone, Debug, Deserialize, Serialize)]
#[serde(untagged)]
pub enum Union {
#[serde(rename=\\"Student\\")]
Student(crate::Student),
#[serde(rename=\\"Undefined\\")]
Undefined(serde_json::Value),
}
"
`;

exports[`RUST_COMMON_PRESET Enum should render reserved union for dict array 3`] = `
"// Student represents a Student model.
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct Student {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ exports[`TypeScriptGenerator should render \`class\` type 1`] = `
private _members?: string | number | boolean;
private _tupleType?: [string, number];
private _tupleTypeWithAdditionalItems?: (string | number | any)[];
private _arrayType: (string | any)[];
private _arrayType: string[];
private _additionalProperties?: Map<string, any | string>;
constructor(input: {
Expand All @@ -380,7 +380,7 @@ exports[`TypeScriptGenerator should render \`class\` type 1`] = `
members?: string | number | boolean,
tupleType?: [string, number],
tupleTypeWithAdditionalItems?: (string | number | any)[],
arrayType: (string | any)[],
arrayType: string[],
additionalProperties?: Map<string, any | string>,
}) {
this._streetName = input.streetName;
Expand Down Expand Up @@ -419,8 +419,8 @@ exports[`TypeScriptGenerator should render \`class\` type 1`] = `
get tupleTypeWithAdditionalItems(): (string | number | any)[] | undefined { return this._tupleTypeWithAdditionalItems; }
set tupleTypeWithAdditionalItems(tupleTypeWithAdditionalItems: (string | number | any)[] | undefined) { this._tupleTypeWithAdditionalItems = tupleTypeWithAdditionalItems; }
get arrayType(): (string | any)[] { return this._arrayType; }
set arrayType(arrayType: (string | any)[]) { this._arrayType = arrayType; }
get arrayType(): string[] { return this._arrayType; }
set arrayType(arrayType: string[]) { this._arrayType = arrayType; }
get additionalProperties(): Map<string, any | string> | undefined { return this._additionalProperties; }
set additionalProperties(additionalProperties: Map<string, any | string> | undefined) { this._additionalProperties = additionalProperties; }
Expand All @@ -447,14 +447,14 @@ exports[`TypeScriptGenerator should render \`interface\` type 1`] = `
members?: string | number | boolean;
tupleType?: [string, number];
tupleTypeWithAdditionalItems?: (string | number | any)[];
arrayType: (string | any)[];
arrayType: string[];
additionalProperties?: Map<string, any | string>;
}"
`;

exports[`TypeScriptGenerator should render \`type\` type - array of primitive type 1`] = `"type TypeArray = (string | any)[];"`;
exports[`TypeScriptGenerator should render \`type\` type - array of primitive type 1`] = `"type TypeArray = string[];"`;

exports[`TypeScriptGenerator should render \`type\` type - array of union type 1`] = `"type TypeArray = (string | number | boolean | any)[];"`;
exports[`TypeScriptGenerator should render \`type\` type - array of union type 1`] = `"type TypeArray = (string | number | boolean)[];"`;

exports[`TypeScriptGenerator should render \`type\` type - enum 1`] = `
"enum TypeEnum {
Expand Down
Loading

0 comments on commit 2cb820f

Please sign in to comment.