From d4c072aa82f360b68288bbf805315eb35a3b8087 Mon Sep 17 00:00:00 2001 From: Nora Date: Thu, 12 Sep 2019 21:26:02 -0400 Subject: [PATCH 1/3] feat(repository): add keyFrom to resolved relation metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Miroslav Bajtoš --- .../resolve-has-many-metadata.integration.ts | 164 ++++++++++++++++++ .../resolve-has-one-metadata.integration.ts | 164 ++++++++++++++++++ .../relations/has-many/has-many.helpers.ts | 14 +- .../src/relations/has-one/has-one.helpers.ts | 14 +- 4 files changed, 352 insertions(+), 4 deletions(-) create mode 100644 packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts create mode 100644 packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts diff --git a/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts b/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts new file mode 100644 index 000000000000..2da5e5b19f6b --- /dev/null +++ b/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts @@ -0,0 +1,164 @@ +// Copyright IBM Corp. 2019. All Rights Reserved. +// Node module: @loopback/repository +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {expect} from '@loopback/testlab'; +import { + Entity, + HasManyDefinition, + ModelDefinition, + RelationType, +} from '../../..'; +import {resolveHasManyMetadata} from '../../../relations/has-many/has-many.helpers'; + +describe('keyTo and keyFrom with resolveHasManyMetadata', () => { + it('resolves metadata using keyTo and keyFrom', () => { + const meta = resolveHasManyMetadata(Category.definition.relations[ + 'products' + ] as HasManyDefinition); + + expect(meta).to.eql({ + name: 'products', + type: 'hasMany', + targetsMany: true, + source: Category, + keyFrom: 'id', + target: () => Product, + keyTo: 'categoryId', + }); + }); + + it('resolves metadata using keyTo, but not keyFrom', () => { + const meta = resolveHasManyMetadata(Category.definition.relations[ + 'items' + ] as HasManyDefinition); + + expect(meta).to.not.have.property('keyFrom'); + + expect(meta).to.eql({ + name: 'items', + type: 'hasMany', + targetsMany: true, + source: Category, + target: () => Item, + keyTo: 'categoryId', + }); + }); + + it('infers keyTo if is it not provided', () => { + const meta = resolveHasManyMetadata(Category.definition.relations[ + 'things' + ] as HasManyDefinition); + + expect(meta).to.eql({ + name: 'things', + type: 'hasMany', + targetsMany: true, + source: Category, + keyFrom: 'id', + target: () => Thing, + keyTo: 'categoryId', + }); + }); + + it('throws if both keyFrom and keyTo are not provided', async () => { + let error; + + try { + resolveHasManyMetadata(Category.definition.relations[ + 'categories' + ] as HasManyDefinition); + } catch (err) { + error = err; + } + + expect(error.message).to.eql( + 'Invalid hasMany definition for Category#categories: target model ' + + 'Category is missing definition of foreign key categoryId', + ); + + expect(error.code).to.eql('INVALID_RELATION_DEFINITION'); + }); + + /****** HELPERS *******/ + + class Category extends Entity {} + + Category.definition = new ModelDefinition('Category') + .addProperty('id', {type: 'number', id: true, required: true}) + .addRelation({ + name: 'products', + type: RelationType.hasMany, + targetsMany: true, + + source: Category, + keyFrom: 'id', + + target: () => Product, + keyTo: 'categoryId', + }) + .addRelation({ + name: 'items', + type: RelationType.hasMany, + targetsMany: true, + + source: Category, + // no keyFrom + + target: () => Item, + keyTo: 'categoryId', + }) + .addRelation({ + name: 'things', + type: RelationType.hasMany, + targetsMany: true, + + source: Category, + keyFrom: 'id', + + target: () => Thing, + // no keyTo + }) + .addRelation({ + name: 'categories', + type: RelationType.hasMany, + targetsMany: true, + + source: Category, + // no keyFrom + + target: () => Category, + // no keyTo + }); + + class Product extends Entity {} + + Product.definition = new ModelDefinition('Product') + .addProperty('id', { + type: 'number', + id: true, + required: true, + }) + .addProperty('categoryId', {type: 'number'}); + + class Item extends Entity {} + + Item.definition = new ModelDefinition('Item') + .addProperty('id', { + type: 'number', + id: true, + required: true, + }) + .addProperty('categoryId', {type: 'number'}); + + class Thing extends Entity {} + + Thing.definition = new ModelDefinition('Thing') + .addProperty('id', { + type: 'number', + id: true, + required: true, + }) + .addProperty('categoryId', {type: 'number'}); +}); diff --git a/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts b/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts new file mode 100644 index 000000000000..71d79a6e71ba --- /dev/null +++ b/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts @@ -0,0 +1,164 @@ +// Copyright IBM Corp. 2019. All Rights Reserved. +// Node module: @loopback/repository +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {expect} from '@loopback/testlab'; +import { + Entity, + HasOneDefinition, + ModelDefinition, + RelationType, +} from '../../..'; +import {resolveHasOneMetadata} from '../../../relations/has-one/has-one.helpers'; + +describe('keyTo and keyFrom with resolveHasOneMetadata', () => { + it('resolves metadata using keyTo and keyFrom', () => { + const meta = resolveHasOneMetadata(Category.definition.relations[ + 'product' + ] as HasOneDefinition); + + expect(meta).to.eql({ + name: 'product', + type: 'hasOne', + targetsMany: false, + source: Category, + keyFrom: 'id', + target: () => Product, + keyTo: 'categoryId', + }); + }); + + it('resolves metadata using keyTo, but not keyFrom', () => { + const meta = resolveHasOneMetadata(Category.definition.relations[ + 'item' + ] as HasOneDefinition); + + expect(meta).to.not.have.property('keyFrom'); + + expect(meta).to.eql({ + name: 'item', + type: 'hasOne', + targetsMany: false, + source: Category, + target: () => Item, + keyTo: 'categoryId', + }); + }); + + it('infers keyTo if is it not provided', () => { + const meta = resolveHasOneMetadata(Category.definition.relations[ + 'thing' + ] as HasOneDefinition); + + expect(meta).to.eql({ + name: 'thing', + type: 'hasOne', + targetsMany: false, + source: Category, + keyFrom: 'id', + target: () => Thing, + keyTo: 'categoryId', + }); + }); + + it('throws if both keyFrom and keyTo are not provided', async () => { + let error; + + try { + resolveHasOneMetadata(Category.definition.relations[ + 'category' + ] as HasOneDefinition); + } catch (err) { + error = err; + } + + expect(error.message).to.eql( + 'Invalid hasOne definition for Category#category: target model Category' + + ' is missing definition of foreign key categoryId', + ); + + expect(error.code).to.eql('INVALID_RELATION_DEFINITION'); + }); + + /****** HELPERS *******/ + + class Category extends Entity {} + + Category.definition = new ModelDefinition('Category') + .addProperty('id', {type: 'number', id: true, required: true}) + .addRelation({ + name: 'product', + type: RelationType.hasOne, + targetsMany: false, + + source: Category, + keyFrom: 'id', + + target: () => Product, + keyTo: 'categoryId', + }) + .addRelation({ + name: 'item', + type: RelationType.hasOne, + targetsMany: false, + + source: Category, + // no keyFrom + + target: () => Item, + keyTo: 'categoryId', + }) + .addRelation({ + name: 'thing', + type: RelationType.hasOne, + targetsMany: false, + + source: Category, + keyFrom: 'id', + + target: () => Thing, + // no keyTo + }) + .addRelation({ + name: 'category', + type: RelationType.hasOne, + targetsMany: false, + + source: Category, + // no keyFrom + + target: () => Category, + // no keyTo + }); + + class Product extends Entity {} + + Product.definition = new ModelDefinition('Product') + .addProperty('id', { + type: 'number', + id: true, + required: true, + }) + .addProperty('categoryId', {type: 'number'}); + + class Item extends Entity {} + + Item.definition = new ModelDefinition('Item') + .addProperty('id', { + type: 'number', + id: true, + required: true, + }) + .addProperty('categoryId', {type: 'number'}); + + class Thing extends Entity {} + + Thing.definition = new ModelDefinition('Thing') + .addProperty('id', { + type: 'number', + id: true, + required: true, + }) + .addProperty('categoryId', {type: 'number'}); +}); diff --git a/packages/repository/src/relations/has-many/has-many.helpers.ts b/packages/repository/src/relations/has-many/has-many.helpers.ts index dd09141dff0d..5e27fb321546 100644 --- a/packages/repository/src/relations/has-many/has-many.helpers.ts +++ b/packages/repository/src/relations/has-many/has-many.helpers.ts @@ -15,7 +15,10 @@ const debug = debugFactory('loopback:repository:has-many-helpers'); * Relation definition with optional metadata (e.g. `keyTo`) filled in. * @internal */ -export type HasManyResolvedDefinition = HasManyDefinition & {keyTo: string}; +export type HasManyResolvedDefinition = HasManyDefinition & { + keyFrom: string; + keyTo: string; +}; /** * Resolves given hasMany metadata if target is specified to be a resolver. @@ -49,6 +52,13 @@ export function resolveHasManyMetadata( throw new InvalidRelationError(reason, relationMeta); } + const keyFrom = sourceModel.getIdProperties()[0]; + + if (relationMeta.keyTo) { + // The explict cast is needed because of a limitation of type inference + return Object.assign(relationMeta, {keyFrom}) as HasManyResolvedDefinition; + } + debug( 'Resolved model %s from given metadata: %o', targetModel.modelName, @@ -62,5 +72,5 @@ export function resolveHasManyMetadata( throw new InvalidRelationError(reason, relationMeta); } - return Object.assign(relationMeta, {keyTo: defaultFkName}); + return Object.assign(relationMeta, {keyFrom, keyTo: defaultFkName}); } diff --git a/packages/repository/src/relations/has-one/has-one.helpers.ts b/packages/repository/src/relations/has-one/has-one.helpers.ts index debc1cf1fea2..c880fd4efbf8 100644 --- a/packages/repository/src/relations/has-one/has-one.helpers.ts +++ b/packages/repository/src/relations/has-one/has-one.helpers.ts @@ -15,7 +15,10 @@ const debug = debugFactory('loopback:repository:has-one-helpers'); * Relation definition with optional metadata (e.g. `keyTo`) filled in. * @internal */ -export type HasOneResolvedDefinition = HasOneDefinition & {keyTo: string}; +export type HasOneResolvedDefinition = HasOneDefinition & { + keyFrom: string; + keyTo: string; +}; /** * Resolves given hasOne metadata if target is specified to be a resolver. @@ -49,6 +52,13 @@ export function resolveHasOneMetadata( throw new InvalidRelationError(reason, relationMeta); } + const keyFrom = sourceModel.getIdProperties()[0]; + + if (relationMeta.keyTo) { + // The explict cast is needed because of a limitation of type inference + return Object.assign(relationMeta, {keyFrom}) as HasOneResolvedDefinition; + } + debug( 'Resolved model %s from given metadata: %o', targetModel.modelName, @@ -62,5 +72,5 @@ export function resolveHasOneMetadata( throw new InvalidRelationError(reason, relationMeta); } - return Object.assign(relationMeta, {keyTo: defaultFkName}); + return Object.assign(relationMeta, {keyFrom, keyTo: defaultFkName}); } From 10150d1761ff66383de08abf303b8b7ef0247c6a Mon Sep 17 00:00:00 2001 From: Nora Date: Mon, 16 Sep 2019 13:56:49 -0400 Subject: [PATCH 2/3] fixup! apply feedback --- .../resolve-has-many-metadata.integration.ts | 22 +++++++++++++++++-- .../resolve-has-one-metadata.integration.ts | 22 +++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts b/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts index 2da5e5b19f6b..40f244388377 100644 --- a/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts +++ b/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts @@ -46,7 +46,7 @@ describe('keyTo and keyFrom with resolveHasManyMetadata', () => { }); }); - it('infers keyTo if is it not provided', () => { + it('infers keyTo if it is not provided', () => { const meta = resolveHasManyMetadata(Category.definition.relations[ 'things' ] as HasManyDefinition); @@ -62,7 +62,7 @@ describe('keyTo and keyFrom with resolveHasManyMetadata', () => { }); }); - it('throws if both keyFrom and keyTo are not provided', async () => { + it('throws if keyFrom, keyTo, and default foreign key name are not provided', async () => { let error; try { @@ -81,6 +81,24 @@ describe('keyTo and keyFrom with resolveHasManyMetadata', () => { expect(error.code).to.eql('INVALID_RELATION_DEFINITION'); }); + it('resolves metadata if keyTo and keyFrom are not provided, but default foreign key is', async () => { + Category.definition.addProperty('categoryId', {type: 'number'}); + + const meta = resolveHasManyMetadata(Category.definition.relations[ + 'categories' + ] as HasManyDefinition); + + expect(meta).to.eql({ + name: 'categories', + type: 'hasMany', + targetsMany: true, + source: Category, + keyFrom: 'id', + target: () => Category, + keyTo: 'categoryId', + }); + }); + /****** HELPERS *******/ class Category extends Entity {} diff --git a/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts b/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts index 71d79a6e71ba..60d4b974fc09 100644 --- a/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts +++ b/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts @@ -46,7 +46,7 @@ describe('keyTo and keyFrom with resolveHasOneMetadata', () => { }); }); - it('infers keyTo if is it not provided', () => { + it('infers keyTo if it is not provided', () => { const meta = resolveHasOneMetadata(Category.definition.relations[ 'thing' ] as HasOneDefinition); @@ -62,7 +62,7 @@ describe('keyTo and keyFrom with resolveHasOneMetadata', () => { }); }); - it('throws if both keyFrom and keyTo are not provided', async () => { + it('throws if keyFrom, keyTo, and default foreign key name are not provided', async () => { let error; try { @@ -81,6 +81,24 @@ describe('keyTo and keyFrom with resolveHasOneMetadata', () => { expect(error.code).to.eql('INVALID_RELATION_DEFINITION'); }); + it('resolves metadata if keyTo and keyFrom are not provided, but default foreign key is', async () => { + Category.definition.addProperty('categoryId', {type: 'number'}); + + const meta = resolveHasOneMetadata(Category.definition.relations[ + 'category' + ] as HasOneDefinition); + + expect(meta).to.eql({ + name: 'category', + type: 'hasOne', + targetsMany: false, + source: Category, + keyFrom: 'id', + target: () => Category, + keyTo: 'categoryId', + }); + }); + /****** HELPERS *******/ class Category extends Entity {} From cdede00e6f8238c75522abadbc08ec04073a645d Mon Sep 17 00:00:00 2001 From: Nora Date: Tue, 17 Sep 2019 09:13:30 -0400 Subject: [PATCH 3/3] fixup! add keyFrom to existing if statement --- .../resolve-has-many-metadata.integration.ts | 5 ++--- .../resolve-has-one-metadata.integration.ts | 5 ++--- .../src/relations/has-many/has-many.helpers.ts | 13 ++++--------- .../src/relations/has-one/has-one.helpers.ts | 13 ++++--------- 4 files changed, 12 insertions(+), 24 deletions(-) diff --git a/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts b/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts index 40f244388377..bcf494ca11ac 100644 --- a/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts +++ b/packages/repository/src/__tests__/integration/repositories/resolve-has-many-metadata.integration.ts @@ -29,18 +29,17 @@ describe('keyTo and keyFrom with resolveHasManyMetadata', () => { }); }); - it('resolves metadata using keyTo, but not keyFrom', () => { + it('infers keyFrom if it is not provided', () => { const meta = resolveHasManyMetadata(Category.definition.relations[ 'items' ] as HasManyDefinition); - expect(meta).to.not.have.property('keyFrom'); - expect(meta).to.eql({ name: 'items', type: 'hasMany', targetsMany: true, source: Category, + keyFrom: 'id', target: () => Item, keyTo: 'categoryId', }); diff --git a/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts b/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts index 60d4b974fc09..3db13b0d66b4 100644 --- a/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts +++ b/packages/repository/src/__tests__/integration/repositories/resolve-has-one-metadata.integration.ts @@ -29,18 +29,17 @@ describe('keyTo and keyFrom with resolveHasOneMetadata', () => { }); }); - it('resolves metadata using keyTo, but not keyFrom', () => { + it('infers keyFrom if it is not provided', () => { const meta = resolveHasOneMetadata(Category.definition.relations[ 'item' ] as HasOneDefinition); - expect(meta).to.not.have.property('keyFrom'); - expect(meta).to.eql({ name: 'item', type: 'hasOne', targetsMany: false, source: Category, + keyFrom: 'id', target: () => Item, keyTo: 'categoryId', }); diff --git a/packages/repository/src/relations/has-many/has-many.helpers.ts b/packages/repository/src/relations/has-many/has-many.helpers.ts index 5e27fb321546..702f44721e43 100644 --- a/packages/repository/src/relations/has-many/has-many.helpers.ts +++ b/packages/repository/src/relations/has-many/has-many.helpers.ts @@ -39,13 +39,6 @@ export function resolveHasManyMetadata( const targetModelProperties = targetModel.definition && targetModel.definition.properties; - // Make sure that if it already keys to the foreign key property, - // the key exists in the target model - if (relationMeta.keyTo && targetModelProperties[relationMeta.keyTo]) { - // The explicit cast is needed because of a limitation of type inference - return relationMeta as HasManyResolvedDefinition; - } - const sourceModel = relationMeta.source; if (!sourceModel || !sourceModel.modelName) { const reason = 'source model must be defined'; @@ -54,8 +47,10 @@ export function resolveHasManyMetadata( const keyFrom = sourceModel.getIdProperties()[0]; - if (relationMeta.keyTo) { - // The explict cast is needed because of a limitation of type inference + // Make sure that if it already keys to the foreign key property, + // the key exists in the target model + if (relationMeta.keyTo && targetModelProperties[relationMeta.keyTo]) { + // The explicit cast is needed because of a limitation of type inference return Object.assign(relationMeta, {keyFrom}) as HasManyResolvedDefinition; } diff --git a/packages/repository/src/relations/has-one/has-one.helpers.ts b/packages/repository/src/relations/has-one/has-one.helpers.ts index c880fd4efbf8..b91f1643d67a 100644 --- a/packages/repository/src/relations/has-one/has-one.helpers.ts +++ b/packages/repository/src/relations/has-one/has-one.helpers.ts @@ -39,13 +39,6 @@ export function resolveHasOneMetadata( const targetModelProperties = targetModel.definition && targetModel.definition.properties; - // Make sure that if it already keys to the foreign key property, - // the key exists in the target model - if (relationMeta.keyTo && targetModelProperties[relationMeta.keyTo]) { - // The explicit cast is needed because of a limitation of type inference - return relationMeta as HasOneResolvedDefinition; - } - const sourceModel = relationMeta.source; if (!sourceModel || !sourceModel.modelName) { const reason = 'source model must be defined'; @@ -54,8 +47,10 @@ export function resolveHasOneMetadata( const keyFrom = sourceModel.getIdProperties()[0]; - if (relationMeta.keyTo) { - // The explict cast is needed because of a limitation of type inference + // Make sure that if it already keys to the foreign key property, + // the key exists in the target model + if (relationMeta.keyTo && targetModelProperties[relationMeta.keyTo]) { + // The explicit cast is needed because of a limitation of type inference return Object.assign(relationMeta, {keyFrom}) as HasOneResolvedDefinition; }