Skip to content

Commit ef749b3

Browse files
cogwirrelcogwirrel
authored
fix(type-safe-api): fix model serialisation for nested collections in typescript (#882)
Previously, arrays of arrays or dictionaries of dictionaries (or any combination) were not serialising/deserialising correctly. There were 2 main problems: 1. Nested arrays/dictionaries of models would be treated as if they were a single array/dictionary 2. Nested dates (even under single level arrays) weren't being serialised/deserialised at all which could lead to runtime errors
1 parent 0e532c0 commit ef749b3

File tree

7 files changed

+4952
-1307
lines changed

7 files changed

+4952
-1307
lines changed

‎packages/type-safe-api/scripts/type-safe-api/generators/generate-next.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import _orderBy from "lodash/orderBy";
1717
import _uniq from "lodash/uniq";
1818
import _uniqBy from "lodash/uniqBy";
1919
import _isEqual from "lodash/isEqual";
20+
import _cloneDeepWith from "lodash/cloneDeepWith"
2021
import { OpenAPIV3 } from "openapi-types";
2122
import * as parseOpenapi from "parse-openapi";
2223
import { getOperationResponses } from "parse-openapi/dist/parser/getOperationResponses";
@@ -302,6 +303,7 @@ const splitAndWriteFiles = (renderedFileContents: string[], outputPath: string)
302303

303304
// Model types which indicate it is composed (ie inherits/mixin's another schema)
304305
const COMPOSED_SCHEMA_TYPES = new Set(["one-of", "any-of", "all-of"]);
306+
const COLLECTION_TYPES = new Set(["array", "dictionary"]);
305307
const PRIMITIVE_TYPES = new Set(["string", "integer", "number", "boolean", "null", "any", "binary", "void"]);
306308

307309
/**
@@ -496,7 +498,7 @@ const mutateModelWithAdditionalTypes = (model: parseOpenapi.Model) => {
496498
(model as any).javaType = toJavaType(model);
497499
(model as any).pythonName = toPythonName('property', model.name);
498500
(model as any).pythonType = toPythonType(model);
499-
(model as any).isPrimitive = PRIMITIVE_TYPES.has(model.type);
501+
(model as any).isPrimitive = PRIMITIVE_TYPES.has(model.type) && !COMPOSED_SCHEMA_TYPES.has(model.export) && !COLLECTION_TYPES.has(model.export);
500502
};
501503

502504
interface MockDataContext {
@@ -615,13 +617,17 @@ const _ensureModelLinks = (spec: OpenAPIV3.Document, modelsByName: {[name: strin
615617
if (modelsByName[name] && !model.link) {
616618
model.link = modelsByName[name];
617619
}
620+
} else if (model.link && typeof schema.additionalProperties !== 'boolean') {
621+
_ensureModelLinks(spec, modelsByName, model.link, schema.additionalProperties, visited);
618622
}
619623
} else if (model.export === "array" && 'items' in schema && schema.items) {
620624
if (isRef(schema.items)) {
621625
const name = splitRef(schema.items.$ref)[2];
622626
if (modelsByName[name] && !model.link) {
623627
model.link = modelsByName[name];
624628
}
629+
} else if (model.link) {
630+
_ensureModelLinks(spec, modelsByName, model.link, schema.items, visited);
625631
}
626632
}
627633

@@ -713,13 +719,12 @@ const buildData = async (inSpec: OpenAPIV3.Document, metadata: any) => {
713719
// In order for the new generator not to be breaking, we apply the same logic here, however this can be removed
714720
// in future since we have control to avoid the duplicate handlers while allowing an operation to be part of
715721
// multiple "services".
716-
let spec = JSON.parse(JSON.stringify(inSpec, (key, value) => {
722+
let spec = _cloneDeepWith(inSpec, (value, key) => {
717723
// Keep only the first tag where we find a tag
718724
if (key === "tags" && value && value.length > 0 && typeof value[0] === "string") {
719725
return [value[0]];
720726
}
721-
return value;
722-
})) as OpenAPIV3.Document;
727+
}) as OpenAPIV3.Document;
723728

724729
// Ensure spec has schemas set
725730
if (!spec?.components?.schemas) {
@@ -775,16 +780,15 @@ const buildData = async (inSpec: OpenAPIV3.Document, metadata: any) => {
775780

776781
// "Inline" any refs to non objects/enums
777782
const inlinedRefs: Set<string> = new Set();
778-
spec = JSON.parse(JSON.stringify(spec, (k, v) => {
783+
spec = _cloneDeepWith(spec, (v) => {
779784
if (v && typeof v === "object" && v.$ref) {
780785
const resolved = resolveRef(spec, v.$ref);
781786
if (resolved && resolved.type && resolved.type !== "object" && !(resolved.type === "string" && resolved.enum)) {
782787
inlinedRefs.add(v.$ref);
783788
return resolved;
784789
}
785790
}
786-
return v;
787-
}));
791+
});
788792

789793
// Delete the non object schemas that were inlined
790794
[...inlinedRefs].forEach(ref => {
@@ -808,7 +812,7 @@ const buildData = async (inSpec: OpenAPIV3.Document, metadata: any) => {
808812
faker.setDefaultRefDate(new Date("2021-06-10"));
809813
const mockDataContext: MockDataContext = {
810814
faker,
811-
dereferencedSpec: await SwaggerParser.dereference(structuredClone(spec), { dereference: { circular: 'ignore' }}) as OpenAPIV3.Document,
815+
dereferencedSpec: await SwaggerParser.dereference(structuredClone(spec), { dereference: { circular: 'ignore' } }) as OpenAPIV3.Document,
812816
};
813817

814818
// Augment operations with additional data

‎packages/type-safe-api/scripts/type-safe-api/generators/typescript/templates/client/models/models.ejs‎

Lines changed: 90 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,16 @@ import {
3333
} from './<%= importName %>';
3434
<%_ }); _%>
3535
<%_ const isComposite = model.export === "one-of" || model.export === "any-of" || model.export === "all-of"; _%>
36+
<%_
37+
// Nested arrays of primitives (besides dates) don't need to have .map(...) called to convert them as the base case will be a noop
38+
// eg. an array of arrays of strings doesn't need to be rendered as `value.map(item0 => item0.map(item1 => item1))`
39+
const canShortCircuitConversion = (property) => {
40+
if (["array", "dictionary"].includes(property.export)) {
41+
return canShortCircuitConversion(property.link);
42+
}
43+
return property.isPrimitive && !["date", "date-time"].includes(property.format);
44+
};
45+
_%>
3646
3747
<%_ if (model.export === "enum") { _%>
3848
/**
@@ -126,19 +136,45 @@ export function <%= model.name %>FromJSONTyped(json: any, ignoreDiscriminator: b
126136
<%_ } else { _%>
127137
return {
128138
139+
<%_
140+
// Renders the appropriate nested function for .map() or mapValues() for arrays and dictionaries for the given type
141+
const renderNestedFromJsonValue = (type, depth = 0) => {
142+
const itemIdentifier = `item${depth}`;
143+
if (type.isPrimitive) {
144+
return `(${itemIdentifier}) => ${["date", "date-time"].includes(type.format) ? `new Date(${itemIdentifier})` : itemIdentifier}`;
145+
} else if (type.export === "array") {
146+
return `(${itemIdentifier}) => ${itemIdentifier}.map(${renderNestedFromJsonValue(type.link, depth + 1)})`;
147+
} else if (type.export === "dictionary") {
148+
return `(${itemIdentifier}) => mapValues(${itemIdentifier}, ${renderNestedFromJsonValue(type.link, depth + 1)})`;
149+
}
150+
return `${type.name || type.type}FromJSON`;
151+
};
152+
// Renders the code to transform a property of the model from its json representation into the model types
153+
const renderFromJsonValue = (property) => {
154+
const value = `json['${property.name}']`;
155+
let rendered = '';
156+
if (canShortCircuitConversion(property)) {
157+
rendered = value;
158+
} else if (property.isPrimitive) {
159+
rendered = ["date", "date-time"].includes(property.format) ? `(new Date(${value}))` : value;
160+
} else if (property.export === "array") {
161+
rendered = `((${value} as Array<any>).map(${renderNestedFromJsonValue(property.link)}))`;
162+
rendered = property.uniqueItems ? `new Set(${rendered})` : rendered;
163+
} else if (property.export === "dictionary") {
164+
rendered = `(mapValues(${value}, ${renderNestedFromJsonValue(property.link)}))`;
165+
} else {
166+
rendered = `${property.type}FromJSON(${value})`;
167+
}
168+
rendered = property.isNullable ? `${value} === null ? null : ${rendered}` : rendered;
169+
rendered = !property.isRequired ? `!exists(json, '${property.name}') ? undefined : ${rendered}` : rendered;
170+
return rendered;
171+
};
172+
_%>
129173
<%_ if (model.export === "dictionary") { _%>
130174
...json,
131175
<%_ } _%>
132176
<%_ model.properties.forEach((property) => { _%>
133-
<%_ if (property.isPrimitive) { _%>
134-
'<%= 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 %>']<% } %>,
135-
<%_ } else if (property.export === 'array') { _%>
136-
'<%= property.typescriptName %>': <% if (!property.isRequired) { %>!exists(json, '<%- property.name %>') ? undefined : <% } %><% if (property.isNullable) { %>json['<%- property.name %>'] === null ? null : <% } %><%= property.uniqueItems ? 'new Set(' : '' %>((json['<%= property.name %>'] as Array<any>).map(<%= property.type %>FromJSON))<%= property.uniqueItems ? ')' : '' %>,
137-
<%_ } else if (property.export === 'dictionary') { _%>
138-
'<%= property.typescriptName %>': <% if (!property.isRequired) { %>!exists(json, '<%- property.name %>') ? undefined : <% } %><% if (property.isNullable) { %>json['<%- property.name %>'] === null ? null : <% } %>(mapValues(json['<%= property.name %>'], <%= property.type %>FromJSON)),
139-
<%_ } else { _%>
140-
'<%= property.typescriptName %>': <% if (!property.isRequired) { %>!exists(json, '<%- property.name %>') ? undefined : <% } %><% if (property.isNullable) { %>json['<%- property.name %>'] === null ? null : <% } %><%= property.type %>FromJSON(json['<%= property.name %>']),
141-
<%_ } _%>
177+
'<%= property.typescriptName %>': <%- renderFromJsonValue(property) %>,
142178
<%_ }); _%>
143179
};
144180
<%_ } _%>
@@ -173,24 +209,56 @@ export function <%= model.name %>ToJSON(value?: <%= model.name %> | null): any {
173209
<%_ } else { _%>
174210
return {
175211
212+
<%_
213+
// Render code to convert a date to its string representation
214+
const renderToJsonDateValue = (identifier, format) => {
215+
return `${identifier}.toISOString()${format === 'date' ? '.substr(0,10)' : ''}`;
216+
};
217+
// Renders the appropriate nested function for .map() or mapValues() for arrays and dictionaries for the given type
218+
const renderNestedToJsonValue = (type, depth = 0) => {
219+
const itemIdentifier = `item${depth}`;
220+
if (type.isPrimitive) {
221+
return `(${itemIdentifier}) => ${["date", "date-time"].includes(type.format) ? renderToJsonDateValue(itemIdentifier, type.format) : itemIdentifier}`;
222+
} else if (type.export === "array") {
223+
return `(${itemIdentifier}) => ${itemIdentifier}.map(${renderNestedToJsonValue(type.link, depth + 1)})`;
224+
} else if (type.export === "dictionary") {
225+
return `(${itemIdentifier}) => mapValues(${itemIdentifier}, ${renderNestedToJsonValue(type.link, depth + 1)})`;
226+
}
227+
return `${type.name || type.type}ToJSON`;
228+
};
229+
// Renders the code to transform a property of the model to its json representation from the model types
230+
const renderToJsonValue = (property) => {
231+
const value = `value.${property.typescriptName}`;
232+
let rendered = '';
233+
234+
if (canShortCircuitConversion(property)) {
235+
rendered = value;
236+
} else if (property.isPrimitive) {
237+
rendered = ["date", "date-time"].includes(property.format) ? `(${renderToJsonDateValue(value, property.format)})` : value;
238+
} else if (property.export === "array") {
239+
const prefix = property.uniqueItems ? `Array.from(${value} as Array<any>)` : `(${value} as Array<any>)`;
240+
rendered = `(${prefix}.map(${renderNestedToJsonValue(property.link)}))`;
241+
} else if (property.export === "dictionary") {
242+
rendered = `(mapValues(${value}, ${renderNestedToJsonValue(property.link)}))`;
243+
} else if (property.type !== "any") {
244+
rendered = `${property.type}ToJSON(${value})`;
245+
} else {
246+
rendered = value;
247+
}
248+
249+
if ((property.isPrimitive && ["date", "date-time"].includes(property.format)) || (!property.isPrimitive && ["array", "dictionary"].includes(property.export))) {
250+
rendered = property.isNullable ? `${value} === null ? null : ${rendered}` : rendered;
251+
rendered = !property.isRequired ? `${value} === undefined ? undefined : ${rendered}` : rendered;
252+
}
253+
return rendered;
254+
};
255+
_%>
176256
<%_ if (model.export === "dictionary") { _%>
177257
...value,
178258
<%_ } _%>
179259
<%_ model.properties.forEach((property) => { _%>
180260
<%_ if (!property.isReadOnly) { _%>
181-
<%_ if (property.isPrimitive && ["date", "date-time"].includes(property.format)) { _%>
182-
'<%= 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)<% } %>),
183-
<%_ } else if (property.isPrimitive) { _%>
184-
'<%= property.name %>': value.<%- property.typescriptName %>,
185-
<%_ } else if (property.export === 'array') { _%>
186-
'<%= property.name %>': <% if (!property.isRequired) { %>value.<%- property.typescriptName %> === undefined ? undefined : <% } %>(<% if (property.isNullable) { %>value.<%- property.typescriptName %> === null ? null : <% } %><% if (property.uniqueItems) { %>Array.from(value.<%- property.typescriptName %> as Set<any>)<% } else { %>(value.<%- property.typescriptName %> as Array<any>)<% } %>.map(<%- property.type %>ToJSON)),
187-
<%_ } else if (property.export === 'dictionary') { _%>
188-
'<%= property.name %>': <% if (!property.isRequired) { %>value.<%- property.typescriptName %> === undefined ? undefined : <% } %>(<% if (property.isNullable) { %>value.<%- property.typescriptName %> === null ? null : <% } %>mapValues(value.<%- property.typescriptName %>, <%- property.type %>ToJSON)),
189-
<%_ } else if (property.type !== 'any') { _%>
190-
'<%= property.name %>': <%- property.type %>ToJSON(value.<%- property.typescriptName %>),
191-
<%_ } else { _%>
192-
'<%= property.name %>': value.<%- property.typescriptName %>,
193-
<%_ } _%>
261+
'<%= property.name %>': <%- renderToJsonValue(property) %>,
194262
<%_ } _%>
195263
<%_ }); _%>
196264
};

‎packages/type-safe-api/test/resources/specs/edge-cases.yaml‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,16 @@ paths:
138138
application/json:
139139
schema:
140140
$ref: "#/components/schemas/ArrayOfOneOfs"
141+
/nested-collections:
142+
post:
143+
operationId: nestedCollections
144+
responses:
145+
200:
146+
description: ok
147+
content:
148+
application/json:
149+
schema:
150+
$ref: "#/components/schemas/NestedCollections"
141151
/additional-properties:
142152
post:
143153
operationId: dictionary
@@ -175,6 +185,96 @@ components:
175185
type: array
176186
items:
177187
$ref: "#/components/schemas/NamedOneOfUnion"
188+
NestedCollections:
189+
type: object
190+
properties:
191+
nestedArrayOfStrings:
192+
type: array
193+
items:
194+
type: array
195+
items:
196+
type: string
197+
nestedArrayOfDates:
198+
type: array
199+
items:
200+
type: array
201+
items:
202+
type: string
203+
format: date
204+
nestedArrayOfObjects:
205+
type: array
206+
items:
207+
type: array
208+
items:
209+
$ref: "#/components/schemas/SomeObject"
210+
fourDimensionalNestedArrayOfObjects:
211+
type: array
212+
items:
213+
type: array
214+
items:
215+
type: array
216+
items:
217+
type: array
218+
items:
219+
$ref: "#/components/schemas/SomeObject"
220+
nestedDictionaryOfStrings:
221+
type: object
222+
additionalProperties:
223+
type: object
224+
additionalProperties:
225+
type: string
226+
nestedDictionaryOfObjects:
227+
type: object
228+
additionalProperties:
229+
type: object
230+
additionalProperties:
231+
$ref: "#/components/schemas/SomeObject"
232+
fourDimensionalNestedDictionaryOfObjects:
233+
type: object
234+
additionalProperties:
235+
type: object
236+
additionalProperties:
237+
type: object
238+
additionalProperties:
239+
type: object
240+
additionalProperties:
241+
$ref: "#/components/schemas/SomeObject"
242+
nestedMixOfDictionariesAndArrays:
243+
type: array
244+
items:
245+
type: object
246+
additionalProperties:
247+
type: array
248+
items:
249+
type: array
250+
items:
251+
type: object
252+
additionalProperties:
253+
type: array
254+
items:
255+
$ref: "#/components/schemas/SomeObject"
256+
cycleArray:
257+
$ref: "#/components/schemas/CycleArray"
258+
cycleDictionary:
259+
$ref: "#/components/schemas/CycleDictionary"
260+
CycleArray:
261+
type: array
262+
items:
263+
$ref: "#/components/schemas/CycleArrayNode"
264+
CycleArrayNode:
265+
type: object
266+
properties:
267+
nodes:
268+
$ref: "#/components/schemas/CycleArray"
269+
CycleDictionary:
270+
type: object
271+
additionalProperties:
272+
$ref: "#/components/schemas/CycleDictionaryNode"
273+
CycleDictionaryNode:
274+
type: object
275+
properties:
276+
nodes:
277+
$ref: "#/components/schemas/CycleDictionary"
178278
AdditionalPropertiesResponse:
179279
type: object
180280
properties:

0 commit comments

Comments
 (0)