Skip to content

Commit

Permalink
fix(type-safe-api): more robust support for composed schemas in types…
Browse files Browse the repository at this point in the history 
…cript (#856)

Revamped models generated from composed schemas (ie oneOf, anyOf, allOf). Rather than inlining
properties, use intersection and union types to represent allOf and anyOf. oneOf is difficult to
model in typescript, so we opt for the same behaviour as anyOf for now.

OpenAPI generator never worked for anyOf/oneOf with a mixture of object and primitive types, whereas
this change generates valid code for this case.
  • Loading branch information
@cogwirrel
cogwirrel authored Oct 14, 2024
1 parent 39b2b71 commit 4fb92b5
Show file tree
Hide file tree
Showing 16 changed files with 24,381 additions and 1,659 deletions.
2 changes: 1 addition & 1 deletion packages/type-safe-api/scripts/type-safe-api/generators/docs/templates/markdown/model.ejs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
| Name | Type | Description | Notes |
|------------ | ------------- | ------------- | -------------|
<%_ model.resolvedProperties.forEach(property => { _%>
<%_ model.properties.forEach(property => { _%>
| **<%- property.name %>** | <% if (property.isPrimitive || property.export === "array" || property.export === "dictionary") { %>**<%- property.javaType %>**<% } else { %>[**<%- property.type %>**](<%- property.type %>.md)<% } %> | <%- property.description || '' %> | <% if (!property.isRequired) { %>[optional] <% } %><% if (property.isReadOnly) { %>[readonly] <% } %><% if (property.defaultValue) { %>[default to <%- property.defaultValue %>]<% } %> |
<%_ }); _%>
Expand Down
4 changes: 2 additions & 2 deletions packages/type-safe-api/scripts/type-safe-api/generators/docs/templates/plantuml/schemas.ejs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ title <%- info.title %> Schemas Diagram

<%_ models.forEach((model) => { _%>
entity <%- model.name %> {
<%_ model.resolvedProperties.forEach(property => { _%>
<%_ model.properties.forEach(property => { _%>
<% if (property.isRequired) { %>* <% } %><%- property.name %>: <%- property.javaType %>
<%_ }); _%>
}
<%_ }); _%>

<%_ models.forEach((model) => { _%>
<%_ model.resolvedProperties.forEach((property) => { _%>
<%_ model.properties.forEach((property) => { _%>
<%_ if (!property.isPrimitive) { _%>
<%- model.name %> -- <% if (["array", "dictionary"].includes(property.export)) { %>"0..*" <% } %><%- property.type %> : <%- property.name %>
<%_ } _%>
Expand Down
125 changes: 87 additions & 38 deletions packages/type-safe-api/scripts/type-safe-api/generators/generate-next.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,23 +201,39 @@ const splitAndWriteFiles = (renderedFileContents: string[], outputPath: string)
const COMPOSED_SCHEMA_TYPES = new Set(["one-of", "any-of", "all-of"]);
const PRIMITIVE_TYPES = new Set(["string", "integer", "number", "boolean", "null", "any", "binary", "void"]);

const resolveComposedProperties = (data: parseOpenapi.ParsedSpecification, model: parseOpenapi.Model): parseOpenapi.Model[] => {
if (COMPOSED_SCHEMA_TYPES.has(model.export)) {
// Find the composed model, if any
const parentModelProperty = model.properties.find(p => !p.name && p.export === "reference");
const composedModel = data.models.find(m => m.name === parentModelProperty?.type);

// The actual properties for a composed model are nested under a special property called "properties"
return [
...(model.properties.find(p => p.name === "properties")?.properties ?? []),
...(composedModel ? resolveComposedProperties(data, composedModel).map((p => {
(p as any)._composedFrom = composedModel.name;
return p;
})) : []),
];
}
/**
* Mutates the given data to ensure composite models (ie allOf, oneOf, anyOf) have the necessary
* properties for representing them in generated code. Adds `composedModels` and `composedPrimitives`
* which contain the models and primitive types that each model is composed of.
*/
const ensureCompositeModels = (data: parseOpenapi.ParsedSpecification) => {
const visited = new Set<parseOpenapi.Model>();
data.models.forEach(model => mutateModelWithCompositeProperties(data, model, visited));
}

return model.properties ?? [];
const mutateModelWithCompositeProperties = (data: parseOpenapi.ParsedSpecification, model: parseOpenapi.Model, visited: Set<parseOpenapi.Model>) => {
if (COMPOSED_SCHEMA_TYPES.has(model.export) && !visited.has(model)) {
visited.add(model);

// Find the models/primitives which this is composed from
const composedModelReferences = model.properties.filter(p => !p.name && p.export === "reference");
const composedPrimitives = model.properties.filter(p => !p.name && p.export !== "reference");

const modelsByName = Object.fromEntries(data.models.map(m => [m.name, m]));
const composedModels = composedModelReferences.flatMap(r => modelsByName[r.type] ? [modelsByName[r.type]] : []);
// Recursively resolve composed properties of properties, to ensure mixins for all-of include all recursive all-of properties
composedModels.forEach(m => mutateModelWithCompositeProperties(data, m, visited));

// For all-of models, we include all composed model properties.
if (model.export === "all-of") {
if (composedPrimitives.length > 0) {
throw new Error(`Schema "${model.name}" defines allOf with non-object types. allOf may only compose object types in the OpenAPI specification.`);
}
}

(model as any).composedModels = composedModels;
(model as any).composedPrimitives = composedPrimitives;
}
};

const toTypescriptPrimitive = (property: parseOpenapi.Model): string => {
Expand Down Expand Up @@ -340,24 +356,26 @@ const toPythonType = (property: parseOpenapi.Model): string => {
* Mutates the given model to add language specific types and names
*/
const mutateModelWithAdditionalTypes = (model: parseOpenapi.Model) => {
// Trim any surrounding quotes from name
model.name = _trim(model.name, `"'`);

(model as any).typescriptName = model.name;
(model as any).typescriptType = toTypeScriptType(model);
(model as any).javaName = model.name;
(model as any).javaType = toJavaType(model);
(model as any).pythonName = _snakeCase(model.name);
(model as any).pythonType = toPythonType(model);
(model as any).isPrimitive = PRIMITIVE_TYPES.has(model.type);

// Trim any surrounding quotes from name
model.name = _trim(model.name, `"'`);
};

const mutateWithOpenapiSchemaProperties = (spec: OpenAPIV3.Document, model: parseOpenapi.Model, schema: OpenAPIV3.SchemaObject, visited: Set<parseOpenapi.Model> = new Set()) => {
(model as any).format = schema.format;
(model as any).oapiType = schema.type;
(model as any).isInteger = schema.type === "integer";
(model as any).isShort = schema.format === "int32";
(model as any).isLong = schema.format === "int64";
(model as any).deprecated = !!schema.deprecated;
(model as any).openapiType = schema.type;
(model as any).isNotSchema = !!schema.not;

visited.add(model);

Expand All @@ -366,6 +384,19 @@ const mutateWithOpenapiSchemaProperties = (spec: OpenAPIV3.Document, model: pars
const subSchema = resolveIfRef(spec, schema.items);
mutateWithOpenapiSchemaProperties(spec, model.link, subSchema, visited);
}

// Also apply to object properties recursively
if (model.export === "dictionary" && model.link && 'additionalProperties' in schema && schema.additionalProperties && !visited.has(model.link)) {
const subSchema = resolveIfRef(spec, schema.additionalProperties);
// Additional properties can be "true" rather than a type
if (subSchema !== true) {
mutateWithOpenapiSchemaProperties(spec, model.link, subSchema, visited);
}
}
model.properties.filter(p => !visited.has(p) && schema.properties?.[p.name]).forEach(property => {
const subSchema = resolveIfRef(spec, schema.properties![property.name]);
mutateWithOpenapiSchemaProperties(spec, property, subSchema, visited);
});
};

interface SubSchema {
Expand All @@ -380,25 +411,41 @@ interface SubSchemaRef {
readonly schema: OpenAPIV3.SchemaObject;
}

const isInlineObjectOrArraySubSchema = (schema?: OpenAPIV3.SchemaObject | OpenAPIV3.ReferenceObject): schema is OpenAPIV3.SchemaObject => !!schema && !isRef(schema) && ["object", "array"].includes(schema.type as any);
const isCompositeSchema = (schema: OpenAPIV3.SchemaObject) =>
!!schema.allOf || !!schema.anyOf || !!schema.oneOf;

const hasSubSchemasToVisit = (schema?: OpenAPIV3.SchemaObject | OpenAPIV3.ReferenceObject): schema is OpenAPIV3.SchemaObject =>
!!schema && !isRef(schema) && (["object", "array"].includes(schema.type as any) || isCompositeSchema(schema) || !!schema.not);

const filterInlineCompositeSchemas = (schemas: (OpenAPIV3.SchemaObject | OpenAPIV3.ReferenceObject)[], nameParts: string[], namePartPrefix: string, prop: string): SubSchema[] => {
let inlineSchemaIndex = 0;
return schemas.flatMap((s, i) => {
if (hasSubSchemasToVisit(s)) {
const subSchema: SubSchema = { nameParts: [...nameParts, `${namePartPrefix}${inlineSchemaIndex === 0 ? '' : inlineSchemaIndex}`], schema: s, prop: `${prop}.[${i}]` };
inlineSchemaIndex++;
return [subSchema];
}
return [];
});
}

const hoistInlineObjectSubSchemas = (nameParts: string[], schema: OpenAPIV3.SchemaObject): SubSchemaRef[] => {
// Find all the inline subschemas
// Find all the inline subschemas we should visit
const inlineSubSchemas: SubSchema[] = [
...(isInlineObjectOrArraySubSchema(schema.not) ? [{ nameParts: [...nameParts, 'Not'], schema: schema.not, prop: 'not' }] : []),
...(schema.anyOf ? schema.anyOf.filter(isInlineObjectOrArraySubSchema).map((s, i) => ({ nameParts: [...nameParts, `${i}`], schema: s, prop: `anyOf.[${i}]` })) : []),
...(schema.allOf ? schema.allOf.filter(isInlineObjectOrArraySubSchema).map((s, i) => ({ nameParts: [...nameParts, `${i}`], schema: s, prop: `allOf.[${i}]` })) : []),
...(schema.oneOf ? schema.oneOf.filter(isInlineObjectOrArraySubSchema).map((s, i) => ({ nameParts: [...nameParts, `${i}`], schema: s, prop: `oneOf.[${i}]` })) : []),
...('items' in schema && isInlineObjectOrArraySubSchema(schema.items) ? [{ nameParts: [...nameParts, 'Inner'], schema: schema.items, prop: 'items' }] : []),
...(Object.entries(schema.properties ?? {}).filter(([, s]) => isInlineObjectOrArraySubSchema(s)).map(([name, s]) => ({ nameParts: [...nameParts, name], schema: s as OpenAPIV3.SchemaObject, prop: `properties.${name}` }))),
...((typeof schema.additionalProperties !== "boolean" && isInlineObjectOrArraySubSchema(schema.additionalProperties)) ? [{ nameParts: [...nameParts, 'Value'], schema: schema.additionalProperties, prop: `additionalProperties` }] : []),
...(hasSubSchemasToVisit(schema.not) ? [{ nameParts: [...nameParts, 'Not'], schema: schema.not, prop: 'not' }] : []),
...(schema.anyOf ? filterInlineCompositeSchemas(schema.anyOf, nameParts, 'AnyOf', 'anyOf') : []),
...(schema.allOf ? filterInlineCompositeSchemas(schema.allOf, nameParts, 'AllOf', 'allOf') : []),
...(schema.oneOf ? filterInlineCompositeSchemas(schema.oneOf, nameParts, 'OneOf', 'oneOf') : []),
...('items' in schema && hasSubSchemasToVisit(schema.items) ? [{ nameParts: [...nameParts, 'Inner'], schema: schema.items, prop: 'items' }] : []),
...(Object.entries(schema.properties ?? {}).filter(([, s]) => hasSubSchemasToVisit(s)).map(([name, s]) => ({ nameParts: [...nameParts, name], schema: s as OpenAPIV3.SchemaObject, prop: `properties.${name}` }))),
...((typeof schema.additionalProperties !== "boolean" && hasSubSchemasToVisit(schema.additionalProperties)) ? [{ nameParts: [...nameParts, 'Value'], schema: schema.additionalProperties, prop: `additionalProperties` }] : []),
];

// Hoist these recursively first (ie depth first search) so that we don't miss refs
const recursiveRefs = inlineSubSchemas.flatMap((s) => hoistInlineObjectSubSchemas(s.nameParts, s.schema));

// Clone the object subschemas to build the refs
const refs = inlineSubSchemas.filter(s => s.schema.type === "object" && ["items", "additionalProperties"].includes(s.prop)).map(s => {
// Clone the object subschemas to build the refs. Note that only objects with "properties" are hoisted as these are non-dictionary types
const refs = inlineSubSchemas.filter(s => (s.schema.type === "object" && s.schema.properties) || isCompositeSchema(s.schema)).map(s => {
const name = s.nameParts.map(_upperFirst).join('');
const $ref = `#/components/schemas/${name}`;
const ref = {
Expand Down Expand Up @@ -472,6 +519,9 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {
// Start with the data from https://github.com/webpro/parse-openapi which extracts most of what we need
const data = { ...parseOpenapi.parse(spec), metadata };

// Mutate the models with enough data to render composite models in the templates
ensureCompositeModels(data);

// Augment operations with additional data
data.services.forEach((service) => {

Expand Down Expand Up @@ -614,20 +664,19 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {

// Augment models with additional data
data.models.forEach((model) => {
// Add a snake_case name
(model as any).nameSnakeCase = _snakeCase(model.name);

const matchingSpecModel = spec?.components?.schemas?.[model.name]

if (matchingSpecModel) {
const specModel = isRef(matchingSpecModel) ? resolveRef(spec, matchingSpecModel.$ref) as OpenAPIV3.SchemaObject : matchingSpecModel;

// Resolve properties inherited from composed schemas
const composedProperties = resolveComposedProperties(data, model);
(model as any).resolvedProperties = composedProperties;

// Add unique imports
(model as any).uniqueImports = _orderBy(_uniq([
...model.imports,
// Include composed property imports, if any
...composedProperties.filter(p => p.export === "reference").map(p => p.type),
// Include property imports, if any
...model.properties.filter(p => p.export === "reference").map(p => p.type),
]));

// Add deprecated flag if present
Expand All @@ -639,7 +688,7 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {
}

// Augment properties with additional data
[...model.properties, ...((model as any).resolvedProperties as parseOpenapi.Model[])].forEach((property) => {
model.properties.forEach((property) => {
const matchingSpecProperty = specModel.properties?.[property.name];

if (matchingSpecProperty) {
Expand Down
65 changes: 58 additions & 7 deletions ...e-safe-api/scripts/type-safe-api/generators/typescript/templates/client/models/models.ejs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,19 @@
* Do not edit the class manually.
*/
import { exists, mapValues } from './model-utils';
<%_ const modelsByName = Object.fromEntries(models.map(m => [m.name, m])); _%>
<%_ model.uniqueImports.forEach((importName) => { _%>
import type { <%= importName %> } from './<%= importName %>';
import {
<%= importName %>FromJSON,
<%= importName %>FromJSONTyped,
<%= importName %>ToJSON,
<%_ if (!(modelsByName[importName] && modelsByName[importName].export === "enum")) { _%>
instanceOf<%= importName %>,
<%_ } _%>
} from './<%= importName %>';
<%_ }); _%>
<%_ const isComposite = model.export === "one-of" || model.export === "any-of" || model.export === "all-of"; _%>
<%_ if (model.export === "enum") { _%>
/**
Expand All @@ -40,6 +45,13 @@ export type <%= model.name %> =
<%- enumMember.value %><% if (i < model.enum.length - 1) { %> | <% } %>
<%_ }); _%>
<%_ } else if (isComposite) { _%>
/**
* @type <%= model.name %>
* <%- model.description || '' %>
* @export
*/
export type <%- model.name %> =<% model.properties.forEach((composedType, i) => { %><% if (i > 0) { %><% if (model.export === "all-of") { %> &<% } else { %> |<% } %><% } %> <%- composedType.typescriptType %><% }); %>;
<%_ } else { _%>
/**
* <%- model.description || '' %>
Expand All @@ -53,7 +65,7 @@ export interface <%= model.name %> {
<%_ if (model.additionalProperties) { _%>
[key: string]: <%- model.additionalProperties.typescriptType %>;
<%_ } _%>
<%_ model.resolvedProperties.forEach((property) => { _%>
<%_ model.properties.forEach((property) => { _%>
/**
* <%- property.description || '' %>
* @type {<%- property.typescriptType %>}
Expand All @@ -65,20 +77,25 @@ export interface <%= model.name %> {
<%= property.isReadOnly ? 'readonly ' : '' %><%= property.typescriptName %><%= property.isRequired ? '' : '?' %>: <%- property.typescriptType %><%= (property.isNullable || property.type === "any") ? ' | null' : '' %>;
<%_ }); _%>
}
<%_ } _%>
<%_ if (model.export !== "enum") { _%>
/**
* Check if a given object implements the <%= model.name %> interface.
*/
export function instanceOf<%= model.name %>(value: object): boolean {
<%_ if (isComposite) { _%>
return <% model.properties.forEach((property, i) => { %><% if (i > 0) { %> <% if (model.export === "all-of") { %>&&<% } else { %>||<% } %> <% } %><% if (property.isPrimitive) { %>typeof value === "<%- property.typescriptType %>"<% } else { %>instanceOf<%- property.type %>(value)<% } %><% }); %>
<%_ } else { _%>
let isInstance = true;
<%_ model.resolvedProperties.forEach((property) => { _%>
<%_ model.properties.forEach((property) => { _%>
<%_ if (property.isRequired) { _%>
isInstance = isInstance && "<%= property.name %>" in value;
<%_ } _%>
<%_ }); _%>
return isInstance;
<%_ } _%>
}
<%_ } _%>
Expand All @@ -87,16 +104,32 @@ export function <%= model.name %>FromJSON(json: any): <%= model.name %> {
}
export function <%= model.name %>FromJSONTyped(json: any, ignoreDiscriminator: boolean): <%= model.name %> {
<%_ if (model.resolvedProperties.length > 0) { _%>
<%_ if (model.properties.length > 0) { _%>
if ((json === undefined) || (json === null)) {
return json;
}
<%_ if (isComposite) { _%>
<%_ model.composedPrimitives.forEach((primitive) => { _%>
if (typeof json === "<%- primitive.typescriptType %>") {
return json;
}
<%_ }); _%>
<%_ if (model.composedModels.length > 0) { _%>
return {
<%_ model.properties.filter(p => !p.isPrimitive).forEach((composedType) => { _%>
...<%- composedType.typescriptType %>FromJSONTyped(json, true),
<%_ }); _%>
};
<%_ } else { _%>
return json;
<%_ } _%>
<%_ } else { _%>
return {
<%_ if (model.additionalProperties) { _%>
...json,
<%_ } _%>
<%_ model.resolvedProperties.forEach((property) => { _%>
<%_ model.properties.forEach((property) => { _%>
<%_ if (property.isPrimitive) { _%>
'<%= property.typescriptName %>': <% if (!property.isRequired) { %>!exists(json, '<%- property.name %>') ? undefined : <% } %><% if (["date", "date-time"].includes(property.format) && property.isNullable) { %>json['<%- property.name %>'] === null ? null : <% } %><% if (["date", "date-time"].includes(property.format)) { %>(new Date(json['<%= property.name %>']))<% } else { %>json['<%= property.name %>']<% } %>,
<%_ } else if (property.export === 'array') { _%>
Expand All @@ -108,25 +141,42 @@ export function <%= model.name %>FromJSONTyped(json: any, ignoreDiscriminator: b
<%_ } _%>
<%_ }); _%>
};
<%_ } _%>
<%_ } else { _%>
return json;
<%_ } _%>
}
export function <%= model.name %>ToJSON(value?: <%= model.name %> | null): any {
<%_ if (model.resolvedProperties.length > 0) { _%>
<%_ if (model.properties.length > 0) { _%>
if (value === undefined) {
return undefined;
}
if (value === null) {
return null;
}
<%_ if (isComposite) { _%>
<%_ model.composedPrimitives.forEach((primitive) => { _%>
if (typeof value === "<%- primitive.typescriptType %>") {
return value;
}
<%_ }); _%>
<%_ if (model.composedModels.length > 0) { _%>
return {
<%_ model.properties.filter(p => !p.isPrimitive).forEach((property, i) => { _%>
...<%- property.typescriptType %>ToJSON(value as any),
<%_ }); _%>
};
<%_ } else { _%>
return value;
<%_ } _%>
<%_ } else { _%>
return {
<%_ if (model.additionalProperties) { _%>
...value,
<%_ } _%>
<%_ model.resolvedProperties.forEach((property) => { _%>
<%_ model.properties.forEach((property) => { _%>
<%_ if (!property.isReadOnly) { _%>
<%_ if (property.isPrimitive && ["date", "date-time"].includes(property.format)) { _%>
'<%= property.name %>': <% if (!property.isRequired) { %>value.<%- property.typescriptName %> === undefined ? undefined : <% } %>(<% if (property.isNullable) { %>value.<%- property.typescriptName %> === null ? null : <% } %>value.<%- property.typescriptName %>.toISOString()<% if (property.format === 'date') { %>.substr(0,10)<% } %>),
Expand All @@ -144,6 +194,7 @@ export function <%= model.name %>ToJSON(value?: <%= model.name %> | null): any {
<%_ } _%>
<%_ }); _%>
};
<%_ } _%>
<%_ } else { _%>
return value;
<%_ } _%>
Expand Down
Loading

0 comments on commit 4fb92b5

Please sign in to comment.