Skip to content

jsonSchema composition for inheritance and includeRelations #5584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
InvictusMB wants to merge 2 commits into from
Closed

jsonSchema composition for inheritance and includeRelations #5584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f42e71e
feat!: building jsonSchema with relations produces composition
InvictusMB May 26, 2020
f069b5b
feat!: model inheritance produces jsonSchema composition
InvictusMB May 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 78 additions & 26 deletions packages/repository-json-schema/src/__tests__/integration/build-schema.integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,27 +1019,54 @@ describe('build-schema', () => {

const expectedSchema: JsonSchema = {
definitions: {
ProductWithRelations: {
title: 'ProductWithRelations',
Category: {
Copy link
Member

@bajtos bajtos Jul 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it very difficult to review this part of the changeset. Is there a way how to preserve the old order of definitions entries, to keep the amount of changed lines as small as possible please?

Copy link
Contributor Author

@InvictusMB InvictusMB Aug 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bajtos I don't know how to proceed with this PR tbh.
The original goal was to eliminate all duplication that blows up taxonomy in OpenAPI definitions. The reason is that each model creates 4+ definitions (Thing, ThingPartial, ThingWithRelations, NewThing, etc.) and this causes issues in the client-side generated code as TypeScript does not consider all those incarnations of the same model compatible.

This code seems to work fine on my projects but I've observed more cases where extra definitions are created. Other variations on JsonSchemaOptions (e.g. excluding properties) create new definitions. I'm not sure if this code is future-proof. A proper fix would be to construct an inheritance chain/tree in terms of OpenAPI schema but I don't have enough experience with lb4 to account for all scenarios.

Also I'm not sure if the isInherited check and getParentModel implemented here are sufficient.

My suggestion for moving this forward would be to expose modelToJsonSchema to userland customization/override instead. So that we can explore the advanced scenarios and come up with patterns that work before writing them in stone.

additionalProperties: false,
description:
`(tsType: ProductWithRelations, ` +
`schemaOptions: { includeRelations: true })`,
'(tsType: Category, schemaOptions: { includeRelations: false })',
properties: {
id: {
type: 'number',
},
},
title: 'Category',
},
Product: {
title: 'Product',
description:
`(tsType: Product, ` +
`schemaOptions: { includeRelations: false })`,
properties: {
id: {type: 'number'},
categoryId: {type: 'number'},
category: {$ref: '#/definitions/CategoryWithRelations'},
},
additionalProperties: false,
},
},
properties: {
id: {type: 'number'},
products: {
type: 'array',
items: {$ref: '#/definitions/ProductWithRelations'},
ProductWithRelations: {
title: 'ProductWithRelations',
description:
`(tsType: ProductWithRelations, ` +
`schemaOptions: { includeRelations: true })`,
allOf: [
{$ref: '#/definitions/Product'},
{
properties: {
category: {$ref: '#/definitions/CategoryWithRelations'},
},
},
],
},
},
additionalProperties: false,
allOf: [
{$ref: '#/definitions/Category'},
{
properties: {
products: {
type: 'array',
items: {$ref: '#/definitions/ProductWithRelations'},
},
},
},
],
title: 'CategoryWithRelations',
description:
`(tsType: CategoryWithRelations, ` +
Expand All @@ -1066,28 +1093,53 @@ describe('build-schema', () => {
}
const expectedSchema: JsonSchema = {
definitions: {
ProductWithRelations: {
title: 'ProductWithRelations',
CategoryWithoutProp: {
additionalProperties: false,
description:
`(tsType: ProductWithRelations, ` +
`schemaOptions: { includeRelations: true })`,
'(tsType: CategoryWithoutProp, schemaOptions: { includeRelations: false })',
title: 'CategoryWithoutProp',
},
Product: {
title: 'Product',
description:
'(tsType: Product, schemaOptions: { includeRelations: false })',
properties: {
id: {type: 'number'},
categoryId: {type: 'number'},
category: {
$ref: '#/definitions/CategoryWithoutPropWithRelations',
categoryId: {
type: 'number',
},
id: {
type: 'number',
},
},
additionalProperties: false,
},
},
properties: {
products: {
type: 'array',
items: {$ref: '#/definitions/ProductWithRelations'},
ProductWithRelations: {
allOf: [
{$ref: '#/definitions/Product'},
{
properties: {
category: {
$ref: '#/definitions/CategoryWithoutPropWithRelations',
},
},
},
],
description:
'(tsType: ProductWithRelations, schemaOptions: { includeRelations: true })',
title: 'ProductWithRelations',
},
},
additionalProperties: false,
allOf: [
{$ref: '#/definitions/CategoryWithoutProp'},
{
properties: {
products: {
type: 'array',
items: {$ref: '#/definitions/ProductWithRelations'},
},
},
},
],
title: 'CategoryWithoutPropWithRelations',
description:
`(tsType: CategoryWithoutPropWithRelations, ` +
Expand Down
37 changes: 21 additions & 16 deletions packages/repository-json-schema/src/__tests__/unit/build-schema.unit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,15 +313,15 @@ describe('build-schema', () => {
title: 'ParentWithItsChildren',
includeRelations: true,
});
expect(schema.properties).to.containEql({
children: {
type: 'array',
// The reference here should be `ChildWithRelations`,
// instead of `ParentWithItsChildren`
items: {$ref: '#/definitions/ChildWithRelations'},
expect(schema.allOf).to.containEql({
properties: {
children: {
type: 'array',
// The reference here should be `ChildWithRelations`,
// instead of `ParentWithItsChildren`
items: {$ref: '#/definitions/ChildWithRelations'},
},
},
benchmarkId: {type: 'string'},
color: {type: 'string'},
});
// The recursive calls should NOT inherit
// `title` from the previous call's `options`.
Expand All @@ -332,8 +332,7 @@ describe('build-schema', () => {
title: 'ChildWithRelations',
description:
'(tsType: ChildWithRelations, schemaOptions: { includeRelations: true })',
properties: {name: {type: 'string'}},
additionalProperties: false,
allOf: [{$ref: '#/definitions/Child'}, {properties: {}}],
},
});
});
Expand Down Expand Up @@ -423,13 +422,19 @@ describe('build-schema', () => {
const newUserSchema = modelToJsonSchema(NewUser, {});
expect(newUserSchema).to.eql({
title: 'NewUser',
properties: {
id: {type: 'string'},
name: {type: 'string'},
password: {type: 'string'},
allOf: [
{$ref: `#/definitions/User`},
{
properties: {
password: {type: 'string'},
},
required: ['password'],
additionalProperties: false,
},
],
definitions: {
User: userSchema,
},
required: ['name', 'password'],
additionalProperties: false,
});
});

Expand Down
125 changes: 94 additions & 31 deletions packages/repository-json-schema/src/build-schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import {MetadataInspector} from '@loopback/context';
import {
isBuiltinType,
Model,
ModelDefinition,
ModelMetadataHelper,
Null,
Expand Down Expand Up @@ -458,7 +459,55 @@ export function modelToJsonSchema<T extends object>(
result.description = descriptionSuffix;
}

if (options.includeRelations) {
const parentSchema = modelToJsonSchema(ctor, {
...options,
includeRelations: false,
});
const properties: typeof result.properties = {};

for (const r in meta.relations) {
const relMeta = meta.relations[r];
const targetType = resolveType(relMeta.target);

// `title` is the unique identity of a schema,
// it should be removed from the `options`
// when generating the relation or property schemas
const targetOptions = {...options};
delete targetOptions.title;

const targetSchema = getJsonSchema(targetType, targetOptions);
const targetRef = {$ref: `#/definitions/${targetSchema.title}`};
const propDef = getNavigationalPropertyForRelation(relMeta, targetRef);

properties[relMeta.name] = properties[relMeta.name] || propDef;
includeReferencedSchema(targetSchema.title!, targetSchema);
}
result.allOf = [
{$ref: `#/definitions/${parentSchema.title}`},
{
properties,
},
];
includeReferencedSchema(parentSchema.title!, parentSchema);
return result;
}

const ownPropsSchema: Pick<
typeof result,
'required' | 'properties' | 'additionalProperties'
> = {};
const parentModel = getParentModel(ctor);
const parentMeta = parentModel
? (ModelMetadataHelper.getModelMetadata(parentModel) as ModelDefinition)
: null;

for (const p in meta.properties) {
if (isInherited(parentMeta, meta, p)) {
debug('Property % is excluded by inheritance', p);
continue;
}

if (options.exclude && options.exclude.includes(p as keyof T)) {
debug('Property % is excluded by %s', p, options.exclude);
continue;
Expand All @@ -471,20 +520,20 @@ export function modelToJsonSchema<T extends object>(
);
}

result.properties = result.properties ?? {};
result.properties[p] = result.properties[p] || {};
ownPropsSchema.properties = ownPropsSchema.properties ?? {};
ownPropsSchema.properties[p] = ownPropsSchema.properties[p] || {};

const metaProperty = Object.assign({}, meta.properties[p]);

// populating "properties" key
result.properties[p] = metaToJsonProperty(metaProperty);
ownPropsSchema.properties[p] = metaToJsonProperty(metaProperty);

// handling 'required' metadata
const optional = options.optional.includes(p as keyof T);

if (metaProperty.required && !(partial || optional)) {
result.required = result.required ?? [];
result.required.push(p);
ownPropsSchema.required = ownPropsSchema.required ?? [];
ownPropsSchema.required.push(p);
}

// populating JSON Schema 'definitions'
Expand Down Expand Up @@ -514,44 +563,36 @@ export function modelToJsonSchema<T extends object>(
const propSchema = getJsonSchema(referenceType, propOptions);

// JSONSchema6Definition allows both boolean and JSONSchema6 types
if (typeof result.properties[p] !== 'boolean') {
const prop = result.properties[p] as JsonSchema;
if (typeof ownPropsSchema.properties[p] !== 'boolean') {
const prop = ownPropsSchema.properties[p] as JsonSchema;
const propTitle = propSchema.title ?? referenceType.name;
const targetRef = {$ref: `#/definitions/${propTitle}`};

if (prop.type === 'array' && prop.items) {
// Update $ref for array type
prop.items = targetRef;
} else {
result.properties[p] = targetRef;
ownPropsSchema.properties[p] = targetRef;
}
includeReferencedSchema(propTitle, propSchema);
}
}

result.additionalProperties = meta.settings.strict === false;
debug(' additionalProperties?', result.additionalProperties);

if (options.includeRelations) {
for (const r in meta.relations) {
result.properties = result.properties ?? {};
const relMeta = meta.relations[r];
const targetType = resolveType(relMeta.target);

// `title` is the unique identity of a schema,
// it should be removed from the `options`
// when generating the relation or property schemas
const targetOptions = {...options};
delete targetOptions.title;

const targetSchema = getJsonSchema(targetType, targetOptions);
const targetRef = {$ref: `#/definitions/${targetSchema.title}`};
const propDef = getNavigationalPropertyForRelation(relMeta, targetRef);

result.properties[relMeta.name] =
result.properties[relMeta.name] || propDef;
includeReferencedSchema(targetSchema.title!, targetSchema);
}
ownPropsSchema.additionalProperties = meta.settings.strict === false;
debug(' additionalProperties?', ownPropsSchema.additionalProperties);

const hasProperties = !!ownPropsSchema.properties;
if (parentModel) {
const parentOptions = {...options};
delete parentOptions.title;
const parentSchema = modelToJsonSchema(parentModel, parentOptions);
result.allOf = [
{$ref: `#/definitions/${parentSchema.title}`},
hasProperties && ownPropsSchema,
].filter(Boolean);
includeReferencedSchema(parentSchema.title!, parentSchema);
} else {
Object.assign(result, ownPropsSchema);
}

function includeReferencedSchema(name: string, schema: JsonSchema) {
Expand All @@ -578,3 +619,25 @@ export function modelToJsonSchema<T extends object>(
}
return result;
}

function getParentModel(ctor: Function) {
const proto = Object.getPrototypeOf(ctor) as Function;
const isBaseType = proto.name === 'Model' || proto.name === 'Entity';
const isModel = proto instanceof Model.constructor;
return !isBaseType && isModel ? proto : null;
}

function isInherited(
parentModel: ModelDefinition | null,
model: ModelDefinition,
propertyName: string,
) {
if (!parentModel) {
return false;
}
// TODO(InvictusMB): deep equality check here?
return (
model.properties[propertyName]?.type ===
parentModel.properties[propertyName]?.type
);
}