jonaslagoni
commented
3 years ago WIP, starting to write up possible solution to fix the above problems
Possible solution, gonna use examples from the TypeScript generator to show the changes and how they solve the problems. All the TS changes with the new system can be found here: https://github.com/jonaslagoni/generator-model-sdk/tree/feature/better_generator_types/src/generators/typescript
Tight coupling with JSON Schema draft 7
To remove the tight coupling entirely we need to analyze what kind of models we might encounter in the real world regardless of JSON Schema or not.
Introducing new specific model types, that replaces the CommonModel:
- Model: The base model for all other models, used to contain standard information, such as
originalInput - CoreModel: Is the core collection of models, all but PropertyModel
- EnumModel: A primary data model, that is generated to separate enum structure in most languages.
- ObjectModel: Primary data model, that generates classes/interfaces/structs, etc. Which contains properties of types:
- UnionModel: When more than one model is represented.
- BooleanModel: When the data model is a boolean representation.
- NumberModel: When a data model is a number in any form.
- RefModel: When other models are referenced, usually for other generated models
- DictionaryModel: When the data model should be interpreted as a dictionary/map, this is important for still representing
additionalPropertiesandpatternProperties. - ArrayModel: When the data model should be an array of a specific type.
- TupleModel: When the data model should be a tuple of multiple types.
- PropertyModel: I decided to wrap any properties into a
PropertyModelfor easier access to functions such asisRequiredfunction, for better developer experience. - StringModel: When the data model should be of string type.
It solves the following issues:
Solving iteration of properties
Solving the following:
- Iterating pattern properties are cumbersome and require knowledge of what the name for the property is
- Additional properties property are hard to access
- Iterating all properties for a model is though
Normal iteration
Current implementation on how we iterate properties (CLICK ME)
https://github.com/asyncapi/modelina/blob/d888adc86d89b989e4f2c2df5282b384a0b72cfe/src/generators/typescript/TypeScriptRenderer.ts#L123-L147
Changed implementation with new type system, becomes straight forward cause `patternProperties` and `additionalProperties` are already resolved directly as **ObjectModel** properties using the new `DictionaryModel` (CLICK ME)
```ts
async renderProperties(): Promise
Serialization functionality
And for de/serialization functionality I introduced to DictionaryModel a serializationType: 'unwrap' | 'normal' to support something like unwrapping a dictionary in that process, which means we go from:
To something like the following (CLICK ME)
```ts function renderMarshalProperty(modelInstanceVariable: string, model: Model) { if (model instanceof RefModel) { const resolvedModel = model.referencedModel; if (resolvedModel instanceof ObjectModel) { return `$\{${modelInstanceVariable}.marshal()}`; } } return realizePropertyFactory(modelInstanceVariable); } function renderMarshalProperties(model: ObjectModel, renderer: TypeScriptRenderer) { const properties = model.properties || {}; const propertyKeys = [...Object.entries(properties)]; const marshalProperties = propertyKeys.map(([prop, propModel]) => { const formattedPropertyName = renderer.nameProperty(prop, propModel); if (propModel.propertyModel instanceof DictionaryModel && propModel.propertyModel.serializationType === 'unwrap') { const modelInstanceVariable = 'value'; const patternPropertyMarshalCode = renderMarshalProperty(modelInstanceVariable, propModel.propertyModel.keyType); const marshalCode = `json += \`"$\{key}": ${patternPropertyMarshalCode},\`;`; return `if(this.${prop} !== undefined) { for (const [key, value] of this.${prop}.entries()) { //Only unwrap dictionary entries which are not already part of the properties if(Object.keys(this).includes(String(key))) continue; ${marshalCode} } }`; } const modelInstanceVariable = `this.${formattedPropertyName}`; const propMarshalCode = renderMarshalProperty(modelInstanceVariable, propModel.propertyModel); const marshalCode = `json += \`"${prop}": ${propMarshalCode},\`;`; return \`if(${modelInstanceVariable} !== undefined) { ${marshalCode} }`; }); return marshalProperties.join('\n'); } ```
Figuring out a referenced model's type
Current implementation (CLICK ME)
https://github.com/asyncapi/modelina/blob/d888adc86d89b989e4f2c2df5282b384a0b72cfe/src/generators/typescript/presets/CommonPreset.ts#L15-L25
Changed implemenation, makes it easier as we have access to the `referencedModel` directly through the new **RefModel** (CLICK ME)
```ts function renderMarshalProperty(modelInstanceVariable: string, model: Model) { if (model instanceof RefModel) { const resolvedModel = model.referencedModel; if (resolvedModel instanceof ObjectModel) { return `$\{${modelInstanceVariable}.marshal()}`; } } return realizePropertyFactory(modelInstanceVariable); } ```
Figuring out a property's type that should be generated can be cluttered
Current implemenation: https://github.com/asyncapi/modelina/blob/d888adc86d89b989e4f2c2df5282b384a0b72cfe/src/generators/typescript/TypeScriptRenderer.ts#L51-L93
Changed implementation, makes it much clear what is what, while you do check more types in one go, it is clear what they are and what is rendered:
renderType(model: Model): string {
if (model instanceof UnionModel) {
return model.unionModels.map(t => this.renderType(t)).join(' | ');
} else if (model instanceof EnumModel) {
return model.values.map(value => typeof value === 'string' ? `"${value}"` : value).join(' | ');
} else if (model instanceof RefModel) {
return this.nameType(model.referencedModel);
} else if (model instanceof TupleModel) {
const types = model.tupleItems.map((item) => {
return this.renderType(item);
});
return `[${types.join(', ')}]`;
} else if (model instanceof ArrayModel) {
const arrayType = model.item ? this.renderType(model.item) : 'unknown';
return `Array<${arrayType}>`;
} else if (model instanceof NumberModel || model instanceof IntegerModel) {
return 'number';
} else if (model instanceof StringModel) {
return 'String';
} else if (model instanceof DictionaryModel) {
const keyType = this.renderType(model.keyType);
const valueType = this.renderType(model.valueType);
return `Map<${keyType}, ${valueType}>`;
} else if (model instanceof BooleanModel) {
return 'boolean';
}
return 'unknown';
}
github-actions[bot]
Reason/Context
This is one big braindump, that will continue to be iterated until the CommonModel enables a better behavior in the following use-cases.
Issues
Currently, many languages uses the following syntax to iterate pattern properties that may be present:
Somewhat the same ordeal as pattern properties.
The CommonModel is tightly coupled with JSON Schema draft 7 as we wanted to see how it all played out with the interpretation of schema files, as well as other inputs before making a change.
We need to remember that different inputs have different effects on the output. For example to enable a property for
additionalPropertiesin the model from a JSON Schema draft 7 input, for serialization, it needs to NOT be rendered as JSON property, but unwrapped.This goes back to the 3 aforementioned issues, we have no way to say
for (const [propertyName, propertyModel] of model.properties) {and then receive all the expected properties.To access any referenced models you need to have access to the
CommonInputModeland then look it up, for then to determine the type of referenced model, such as in this case, where we need to do something specific for referenced Enums.For example for the TypeScript renderer you can see how the type is determined here, which is hard to distinguish between what is what: https://github.com/asyncapi/modelina/blob/d888adc86d89b989e4f2c2df5282b384a0b72cfe/src/generators/typescript/TypeScriptRenderer.ts#L51-L93