From 78777cf6fbad14ab94399480c706949fd1addf3b Mon Sep 17 00:00:00 2001 From: shimks Date: Fri, 17 Aug 2018 11:56:08 -0400 Subject: [PATCH 1/6] fixup! init commit --- package.json | 2 +- .../src/build-schema.ts | 9 +- .../integration/build-schema.integration.ts | 86 +++++++++++++++- .../test/unit/build-schema.unit.ts | 20 +++- .../repository/examples/models/order.model.ts | 2 +- packages/repository/package.json | 2 +- .../src/decorators/model.decorator.ts | 16 ++- .../src/decorators/relation.decorator.ts | 69 ++++++++----- packages/repository/src/model.ts | 16 ++- .../src/repositories/relation.factory.ts | 68 +++++++++++-- .../has-many.relation.acceptance.ts | 22 ++--- .../fixtures/models/bad/cyclic-x.model.ts | 12 +++ .../fixtures/models/bad/cyclic-y.model.ts | 12 +++ .../relation.factory.integration.ts | 62 ++++++++++-- .../model-and-relation.decorator.unit.ts | 46 ++++++++- .../unit/decorator/relation.decorator.unit.ts | 85 +++++++++++++++- .../repository/test/unit/model/model.unit.ts | 15 ++- .../repositories/relation.repository.unit.ts | 99 +++++++++++++++++++ 18 files changed, 564 insertions(+), 79 deletions(-) create mode 100644 packages/repository/test/fixtures/models/bad/cyclic-x.model.ts create mode 100644 packages/repository/test/fixtures/models/bad/cyclic-y.model.ts diff --git a/package.json b/package.json index 059ba30804a9..24031c3ecdb4 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "test:ci": "node packages/build/bin/run-nyc npm run mocha --scripts-prepend-node-path", "verify:docs": "npm run build:site -- --verify", "build:site": "./bin/build-docs-site.sh", - "mocha": "node packages/build/bin/run-mocha \"packages/*/DIST/test/**/*.js\" \"examples/*/DIST/test/**/*.js\" \"packages/cli/test/**/*.js\" \"packages/build/test/*/*.js\"", + "mocha": "node packages/build/bin/run-mocha \"packages/*/DIST/test/**/*.js\" \"examples/*/DIST/test/**/*.js\" \"packages/cli/test/**/*.js\" \"packages/build/test/*/*.js\" --exclude packages/*/DIST/test/fixtures/**/*.js", "posttest": "npm run lint", "commitmsg": "commitlint -E GIT_PARAMS" }, diff --git a/packages/repository-json-schema/src/build-schema.ts b/packages/repository-json-schema/src/build-schema.ts index 2ce6fa7b8006..948d42256d63 100644 --- a/packages/repository-json-schema/src/build-schema.ts +++ b/packages/repository-json-schema/src/build-schema.ts @@ -7,6 +7,7 @@ import { ModelMetadataHelper, PropertyDefinition, ModelDefinition, + isModelResolver, } from '@loopback/repository'; import {MetadataInspector} from '@loopback/context'; import { @@ -124,6 +125,9 @@ export function metaToJsonProperty(meta: PropertyDefinition): JSONSchema { propertyType = stringTypeToWrapper(propertyType); if (isComplexType(propertyType)) { + propertyType = isModelResolver(propertyType) + ? propertyType() + : propertyType; Object.assign(propDef, {$ref: `#/definitions/${propertyType.name}`}); } else { Object.assign(propDef, { @@ -171,12 +175,15 @@ export function modelToJsonSchema(ctor: Function): JSONSchema { result.properties[p] = metaToJsonProperty(metaProperty); // populating JSON Schema 'definitions' - const referenceType = isArrayType(metaProperty.type as string | Function) + let referenceType = isArrayType(metaProperty.type as string | Function) ? // shimks: ugly type casting; this should be replaced by logic to throw // error if itemType/type is not a string or a function (metaProperty.itemType as string | Function) : (metaProperty.type as string | Function); if (typeof referenceType === 'function' && isComplexType(referenceType)) { + referenceType = isModelResolver(referenceType) + ? referenceType() + : referenceType; const propSchema = getJsonSchema(referenceType); if (propSchema && Object.keys(propSchema).length > 0) { diff --git a/packages/repository-json-schema/test/integration/build-schema.integration.ts b/packages/repository-json-schema/test/integration/build-schema.integration.ts index f4d35bf45721..a8fd9137d022 100644 --- a/packages/repository-json-schema/test/integration/build-schema.integration.ts +++ b/packages/repository-json-schema/test/integration/build-schema.integration.ts @@ -3,7 +3,13 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {model, property} from '@loopback/repository'; +import { + model, + property, + belongsTo, + hasMany, + Entity, +} from '@loopback/repository'; import { modelToJsonSchema, JSON_SCHEMA_KEY, @@ -370,6 +376,84 @@ describe('build-schema', () => { expectValidJsonSchema(jsonSchema); }); + it('properly converts decorated custom array type with a resolver', () => { + @model() + class CustomType { + @property() + prop: string; + } + + @model() + class TestModel { + @property.array(() => CustomType) + cusType: CustomType[]; + } + + const jsonSchema = modelToJsonSchema(TestModel); + expect(jsonSchema.properties).to.deepEqual({ + cusType: { + type: 'array', + items: {$ref: '#/definitions/CustomType'}, + }, + }); + expect(jsonSchema.definitions).to.deepEqual({ + CustomType: { + title: 'CustomType', + properties: { + prop: { + type: 'string', + }, + }, + }, + }); + expectValidJsonSchema(jsonSchema); + }); + + it('properly converts decorated models with hasMany and belongsTo', () => { + @model() + class Order extends Entity { + @property({id: true}) + id: number; + @belongsTo(() => Customer) + customerId: number; + } + + @model() + class Customer extends Entity { + @property({id: true}) + id: number; + @hasMany(() => Order) + orders: Order[]; + } + + const orderSchema = modelToJsonSchema(Order); + const customerSchema = modelToJsonSchema(Customer); + expect(orderSchema.properties).to.deepEqual({ + id: {type: 'number'}, + customerId: {type: 'number'}, + }); + expect(customerSchema.properties).to.deepEqual({ + id: {type: 'number'}, + orders: { + type: 'array', + items: {$ref: '#/definitions/Order'}, + }, + }); + expect(customerSchema.definitions).to.deepEqual({ + Order: { + title: 'Order', + properties: { + id: { + type: 'number', + }, + customerId: {type: 'number'}, + }, + }, + }); + + expectValidJsonSchema(orderSchema); + }); + it('creates definitions only at the root level of the schema', () => { @model() class CustomTypeFoo { diff --git a/packages/repository-json-schema/test/unit/build-schema.unit.ts b/packages/repository-json-schema/test/unit/build-schema.unit.ts index 5d3ecf898264..08e4df2c7737 100644 --- a/packages/repository-json-schema/test/unit/build-schema.unit.ts +++ b/packages/repository-json-schema/test/unit/build-schema.unit.ts @@ -7,6 +7,8 @@ import {expect} from '@loopback/testlab'; import {isComplexType, stringTypeToWrapper, metaToJsonProperty} from '../..'; describe('build-schema', () => { + class CustomType {} + describe('stringTypeToWrapper', () => { context('when given primitive types in string', () => { it('returns String for "string"', () => { @@ -76,7 +78,6 @@ describe('build-schema', () => { }); it('returns true if any other wrappers are passed in', () => { - class CustomType {} expect(isComplexType(CustomType)).to.eql(true); }); }); @@ -107,12 +108,17 @@ describe('build-schema', () => { }); it('converts complex types', () => { - class CustomType {} expect(metaToJsonProperty({type: CustomType})).to.eql({ $ref: '#/definitions/CustomType', }); }); + it('converts complex types with resolver', () => { + expect(metaToJsonProperty({type: () => CustomType})).to.eql({ + $ref: '#/definitions/CustomType', + }); + }); + it('converts primitive arrays', () => { expect(metaToJsonProperty({type: Array, itemType: Number})).to.eql({ type: 'array', @@ -121,11 +127,19 @@ describe('build-schema', () => { }); it('converts arrays of custom types', () => { - class CustomType {} expect(metaToJsonProperty({type: Array, itemType: CustomType})).to.eql({ type: 'array', items: {$ref: '#/definitions/CustomType'}, }); }); + + it('converts array types with resolver', () => { + expect( + metaToJsonProperty({type: Array, itemType: () => CustomType}), + ).to.eql({ + type: 'array', + items: {$ref: '#/definitions/CustomType'}, + }); + }); }); }); diff --git a/packages/repository/examples/models/order.model.ts b/packages/repository/examples/models/order.model.ts index 5dea92626ded..2a2fbf03a2fa 100644 --- a/packages/repository/examples/models/order.model.ts +++ b/packages/repository/examples/models/order.model.ts @@ -26,6 +26,6 @@ class Order extends Entity { id: string; customerId: string; - @belongsTo() + @belongsTo(() => Customer) customer: Customer; } diff --git a/packages/repository/package.json b/packages/repository/package.json index f47d3052fed2..7431dfc4a551 100644 --- a/packages/repository/package.json +++ b/packages/repository/package.json @@ -15,7 +15,7 @@ "build:dist10": "lb-tsc es2018", "clean": "lb-clean loopback-repository*.tgz dist* package api-docs", "pretest": "npm run build", - "test": "lb-mocha \"DIST/test/**/*.js\"", + "test": "lb-mocha \"DIST/test/**/*.js\" --exclude DIST/test/fixtures/models/bad/*.js", "verify": "npm pack && tar xf loopback-repository*.tgz && tree package && npm run clean" }, "author": "IBM", diff --git a/packages/repository/src/decorators/model.decorator.ts b/packages/repository/src/decorators/model.decorator.ts index 21be80f0c937..674de1acae4d 100644 --- a/packages/repository/src/decorators/model.decorator.ts +++ b/packages/repository/src/decorators/model.decorator.ts @@ -14,6 +14,7 @@ import { ModelDefinition, ModelDefinitionSyntax, PropertyDefinition, + ModelResolver, } from '../model'; import {RELATIONS_KEY, RelationDefinitionBase} from './relation.decorator'; @@ -91,6 +92,16 @@ export function model(definition?: Partial) { * @returns {(target:any, key:string)} */ export function property(definition?: Partial) { + if ( + definition && + (definition.type === Array || definition.type === 'array') && + (definition as object).hasOwnProperty('itemType') && + !definition.itemType + ) { + // this path is taken when cyclic dependency is detected + // in that case, a ModelResolver should be used instead + throw new Error('target model is undefined'); + } return PropertyDecoratorFactory.createDecorator( MODEL_PROPERTIES_KEY, Object.assign({}, definition), @@ -100,7 +111,6 @@ export function property(definition?: Partial) { export namespace property { export const ERR_PROP_NOT_ARRAY = '@property.array can only decorate array properties!'; - export const ERR_NO_ARGS = 'decorator received less than two parameters'; /** * @@ -108,8 +118,8 @@ export namespace property { * @param definition Optional PropertyDefinition object for additional * metadata */ - export function array( - itemType: Function, + export function array( + itemType: T | ModelResolver, definition?: Partial, ) { return function(target: Object, propertyName: string) { diff --git a/packages/repository/src/decorators/relation.decorator.ts b/packages/repository/src/decorators/relation.decorator.ts index e683765fd77b..f5cdf8f83c00 100644 --- a/packages/repository/src/decorators/relation.decorator.ts +++ b/packages/repository/src/decorators/relation.decorator.ts @@ -4,8 +4,8 @@ // License text available at https://opensource.org/licenses/MIT import {Class} from '../common-types'; -import {Entity} from '../model'; -import {PropertyDecoratorFactory} from '@loopback/context'; +import {Entity, ModelResolver, isModelResolver} from '../model'; +import {PropertyDecoratorFactory, MetadataInspector} from '@loopback/context'; import {property} from './model.decorator'; import {camelCase} from 'lodash'; @@ -31,11 +31,12 @@ export class RelationMetadata { export interface RelationDefinitionBase { type: RelationType; + target: typeof Entity | ModelResolver; } export interface HasManyDefinition extends RelationDefinitionBase { type: RelationType.hasMany; - keyTo: string; + keyTo?: string; } /** @@ -53,10 +54,22 @@ export function relation(definition?: Object) { * @param definition * @returns {(target:any, key:string)} */ -export function belongsTo(definition?: Object) { - // Apply model definition to the model class - const rel = Object.assign({type: RelationType.belongsTo}, definition); - return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel); +export function belongsTo( + targetModel: T | ModelResolver, + definition?: Partial, +) { + return function(target: Object, key: string) { + const propMeta = { + type: MetadataInspector.getDesignTypeForProperty(target, key), + }; + property(propMeta)(target, key); + // Apply model definition to the model class + const rel = Object.assign( + {type: RelationType.belongsTo, target: targetModel}, + definition, + ); + PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel)(target, key); + }; } /** @@ -78,7 +91,7 @@ export function hasOne(definition?: Object) { * @returns {(target:any, key:string)} */ export function hasMany( - targetModel: T, + targetModel: T | ModelResolver, definition?: Partial, ) { // todo(shimks): extract out common logic (such as @property.array) to @@ -86,28 +99,30 @@ export function hasMany( return function(target: Object, key: string) { property.array(targetModel)(target, key); - const defaultFkName = camelCase(target.constructor.name + '_id'); - const hasKeyTo = definition && definition.keyTo; - const hasDefaultFkProperty = - targetModel.definition && - targetModel.definition.properties && - targetModel.definition.properties[defaultFkName]; - if (!(hasKeyTo || hasDefaultFkProperty)) { - // note(shimks): should we also check for the existence of explicitly - // given foreign key name on the juggler definition? - throw new Error( - `foreign key ${defaultFkName} not found on ${ - targetModel.name - } model's juggler definition`, - ); + const meta: Partial = {target: targetModel}; + + if (!isModelResolver(targetModel)) { + const defaultFkName = camelCase(target.constructor.name + '_id'); + const hasKeyTo = definition && definition.keyTo; + const hasDefaultFkProperty = + targetModel.definition && + targetModel.definition.properties && + targetModel.definition.properties[defaultFkName]; + if (!(hasKeyTo || hasDefaultFkProperty)) { + // note(shimks): should we also check for the existence of explicitly + // given foreign key name on the juggler definition? + throw new Error( + `foreign key ${defaultFkName} not found on ${ + targetModel.name + } model's juggler definition`, + ); + } + Object.assign(meta, {keyTo: defaultFkName}); } - const meta = {keyTo: defaultFkName}; + Object.assign(meta, definition, {type: RelationType.hasMany}); - PropertyDecoratorFactory.createDecorator( - RELATIONS_KEY, - meta as HasManyDefinition, - )(target, key); + PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, meta)(target, key); }; } diff --git a/packages/repository/src/model.ts b/packages/repository/src/model.ts index 10acd085baee..372c04f9e782 100644 --- a/packages/repository/src/model.ts +++ b/packages/repository/src/model.ts @@ -21,11 +21,11 @@ export type PropertyType = string | Function | Object | Type; * Property definition for a model */ export interface PropertyDefinition { - type: PropertyType; // For example, 'string', String, or {} + type: PropertyType | ModelResolver; // For example, 'string', String, or {} id?: boolean; json?: PropertyForm; store?: PropertyForm; - itemType?: PropertyType; // type of array + itemType?: PropertyType | ModelResolver; // type of array [attribute: string]: any; // Other attributes } @@ -263,6 +263,18 @@ export abstract class Entity extends Model implements Persistable { } } +/** + * DONT FORGET TO WRITE THE DOCS HERE + * REVIEWERS REMIND ME IF THIS IS STILL HERE + */ +export type ModelResolver = () => T; + +export function isModelResolver( + fn: ModelResolver | T, +): fn is ModelResolver { + return !/^class/.test(fn.toString()); +} + /** * Domain events */ diff --git a/packages/repository/src/repositories/relation.factory.ts b/packages/repository/src/repositories/relation.factory.ts index 1b89d8d4b227..74fb5ffd34ef 100644 --- a/packages/repository/src/repositories/relation.factory.ts +++ b/packages/repository/src/repositories/relation.factory.ts @@ -4,13 +4,23 @@ // License text available at https://opensource.org/licenses/MIT import {EntityCrudRepository} from './repository'; -import {HasManyDefinition} from '../decorators/relation.decorator'; -import {Entity} from '../model'; +import { + HasManyDefinition, + RelationType, + RELATIONS_KEY, +} from '../decorators/relation.decorator'; +import {Entity, isModelResolver} from '../model'; import { HasManyRepository, DefaultHasManyEntityCrudRepository, } from './relation.repository'; -import {DataObject} from '..'; +import {DataObject} from '../common-types'; +import {MetadataInspector} from '@loopback/context'; +import {RelationMap} from '../decorators/model.decorator'; + +const debug = require('debug')('loopback:repository:relation:factory'); + +const ERR_NO_BELONGSTO_META = 'no belongsTo metadata found'; export type HasManyRepositoryFactory = ( fkValue: ForeignKeyType, @@ -37,13 +47,13 @@ export function createHasManyRepositoryFactory< relationMetadata: HasManyDefinition, targetRepository: EntityCrudRepository, ): HasManyRepositoryFactory { + resolveHasManyMetadata(relationMetadata); + debug('resolved relation metadata: %o', relationMetadata); + const fkName = relationMetadata.keyTo; + if (!fkName) { + throw new Error('The foreign key property name (keyTo) must be specified'); + } return function(fkValue: ForeignKeyType) { - const fkName = relationMetadata.keyTo; - if (!fkName) { - throw new Error( - 'The foreign key property name (keyTo) must be specified', - ); - } // tslint:disable-next-line:no-any const constraint: any = {[fkName]: fkValue}; return new DefaultHasManyEntityCrudRepository< @@ -53,3 +63,43 @@ export function createHasManyRepositoryFactory< >(targetRepository, constraint as DataObject); }; } + +export function resolveHasManyMetadata(relationMeta: HasManyDefinition) { + if ( + relationMeta.target && + isModelResolver(relationMeta.target) && + !relationMeta.keyTo + ) { + const resolvedModel = relationMeta.target(); + + debug('resolved model from given metadata: %o', resolvedModel); + + const targetRelationMeta: + | RelationMap + | undefined = MetadataInspector.getAllPropertyMetadata( + RELATIONS_KEY, + resolvedModel.prototype, + ); + + debug('relation metadata from %o: %o', resolvedModel, targetRelationMeta); + + if (!targetRelationMeta) { + throw new Error(ERR_NO_BELONGSTO_META); + } + + let belongsToMetaExists = false; + + for (const key in targetRelationMeta) { + if (targetRelationMeta[key].type === RelationType.belongsTo) { + relationMeta.keyTo = key; + belongsToMetaExists = true; + break; + } + } + + if (!belongsToMetaExists) { + throw new Error(ERR_NO_BELONGSTO_META); + } + } + return relationMeta; +} diff --git a/packages/repository/test/acceptance/has-many.relation.acceptance.ts b/packages/repository/test/acceptance/has-many.relation.acceptance.ts index 5da5cd482767..e2c4030c0f72 100644 --- a/packages/repository/test/acceptance/has-many.relation.acceptance.ts +++ b/packages/repository/test/acceptance/has-many.relation.acceptance.ts @@ -10,6 +10,7 @@ import { DefaultCrudRepository, juggler, hasMany, + belongsTo, repository, RepositoryMixin, AppWithRepository, @@ -76,13 +77,13 @@ describe('HasMany relation', () => { it('can patch many instances', async () => { await controller.createCustomerOrders(existingCustomerId, { description: 'order 1', - isDelivered: false, + isShipped: false, }); await controller.createCustomerOrders(existingCustomerId, { description: 'order 2', - isDelivered: false, + isShipped: false, }); - const patchObject = {isDelivered: true}; + const patchObject = {isShipped: true}; const arePatched = await controller.patchCustomerOrders( existingCustomerId, patchObject, @@ -90,18 +91,18 @@ describe('HasMany relation', () => { expect(arePatched).to.equal(2); const patchedData = _.map( await controller.findCustomerOrders(existingCustomerId), - d => _.pick(d, ['customerId', 'description', 'isDelivered']), + d => _.pick(d, ['customerId', 'description', 'isShipped']), ); expect(patchedData).to.eql([ { customerId: existingCustomerId, description: 'order 1', - isDelivered: true, + isShipped: true, }, { customerId: existingCustomerId, description: 'order 2', - isDelivered: true, + isShipped: true, }, ]); }); @@ -166,12 +167,9 @@ describe('HasMany relation', () => { type: 'boolean', required: false, }) - isDelivered: boolean; + isShipped: boolean; - @property({ - type: 'number', - required: true, - }) + @belongsTo(() => Customer) customerId: number; } @@ -188,7 +186,7 @@ describe('HasMany relation', () => { }) name: string; - @hasMany(Order) + @hasMany(() => Order) orders: Order[]; } diff --git a/packages/repository/test/fixtures/models/bad/cyclic-x.model.ts b/packages/repository/test/fixtures/models/bad/cyclic-x.model.ts new file mode 100644 index 000000000000..7336b04ac50c --- /dev/null +++ b/packages/repository/test/fixtures/models/bad/cyclic-x.model.ts @@ -0,0 +1,12 @@ +// Copyright IBM Corp. 2018. 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 {property} from '../../../..'; +import {BadCyclicY} from './cyclic-y.model'; + +export class BadCyclicX { + @property.array(BadCyclicY) + cyclicProp: BadCyclicY[]; +} diff --git a/packages/repository/test/fixtures/models/bad/cyclic-y.model.ts b/packages/repository/test/fixtures/models/bad/cyclic-y.model.ts new file mode 100644 index 000000000000..34c7dbb52ecb --- /dev/null +++ b/packages/repository/test/fixtures/models/bad/cyclic-y.model.ts @@ -0,0 +1,12 @@ +// Copyright IBM Corp. 2018. 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 {property} from '../../../..'; +import {BadCyclicX} from './cyclic-x.model'; + +export class BadCyclicY { + @property.array(BadCyclicX) + cyclicProp: BadCyclicX[]; +} diff --git a/packages/repository/test/integration/repositories/relation.factory.integration.ts b/packages/repository/test/integration/repositories/relation.factory.integration.ts index b144d88c9dab..2d7f8e765e3e 100644 --- a/packages/repository/test/integration/repositories/relation.factory.integration.ts +++ b/packages/repository/test/integration/repositories/relation.factory.integration.ts @@ -14,6 +14,10 @@ import { createHasManyRepositoryFactory, HasManyDefinition, HasManyRepositoryFactory, + hasMany, + belongsTo, + model, + property, } from '../../..'; import {expect} from '@loopback/testlab'; @@ -116,16 +120,53 @@ describe('HasMany relation', () => { ); }); - it('errors when keyTo is not available hasMany metadata', () => { - const keytolessMeta = { - type: RelationType.hasMany, - }; - expect( + context('createHasManyRepositoryFactory', () => { + it('errors when keyTo is not available hasMany metadata', () => { + class SomeClass extends Entity {} + const keytolessMeta: HasManyDefinition = { + type: RelationType.hasMany, + target: SomeClass, + }; + expect(() => + createHasManyRepositoryFactory(keytolessMeta, reviewRepo), + ).to.throw(/The foreign key property name \(keyTo\) must be specified/); + }); + + it('resolves belongsTo metadata', () => { + @model() + class Card extends Entity { + @property({id: true}) + id: number; + @belongsTo(() => Suite) + suiteId: string; + } + + @model() + class Suite extends Entity { + @property({id: true}) + id: string; + @hasMany(() => Card) + cards: Card[]; + } + + const hasManyMeta = Suite.definition.relations.cards as HasManyDefinition; + expect(hasManyMeta).to.eql({ + type: RelationType.hasMany, + target: () => Card, + }); createHasManyRepositoryFactory( - keytolessMeta as HasManyDefinition, - reviewRepo, - ), - ).to.throw(/The foreign key property name \(keyTo\) must be specified/); + hasManyMeta, + new DefaultCrudRepository( + Suite, + new juggler.DataSource({connector: 'memory'}), + ), + ); + expect(hasManyMeta).to.eql({ + type: RelationType.hasMany, + target: () => Card, + keyTo: 'suiteId', + }); + }); }); //--- HELPERS ---// @@ -181,14 +222,17 @@ describe('HasMany relation', () => { relations: { orders: { type: RelationType.hasMany, + target: () => Order, keyTo: 'customerId', }, reviewsAuthored: { type: RelationType.hasMany, + target: () => Review, keyTo: 'authorId', }, reviewsApproved: { type: RelationType.hasMany, + target: () => Review, keyTo: 'approvedId', }, }, diff --git a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts index 17f8c10c127d..69e09da3fb90 100644 --- a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts +++ b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts @@ -96,8 +96,8 @@ describe('model decorator', () => { @property() customerId: string; - @belongsTo({target: 'Customer'}) // TypeScript does not allow me to reference Customer here + @belongsTo(() => Customer) customer: ICustomer; // Validates that property no longer requires a parameter @@ -259,6 +259,7 @@ describe('model decorator', () => { ) || /* istanbul ignore next */ {}; expect(meta.orders).to.eql({ type: RelationType.hasMany, + target: Order, keyTo: 'customerId', }); }); @@ -271,7 +272,7 @@ describe('model decorator', () => { ) || /* istanbul ignore next */ {}; expect(meta.customer).to.eql({ type: RelationType.belongsTo, - target: 'Customer', + target: () => Customer, }); }); @@ -319,7 +320,7 @@ describe('model decorator', () => { describe('property namespace', () => { describe('array', () => { - it('"@property.array" adds array metadata', () => { + it('adds array metadata', () => { @model() class TestModel { @property.array(Product) @@ -334,7 +335,33 @@ describe('model decorator', () => { expect(meta.items).to.eql({type: Array, itemType: Product}); }); - it('throws when @property.array is used on a non-array property', () => { + it('adds model resolver metadata', () => { + class CyclicX { + @property.array(() => CyclicY) + cyclicProp: CyclicY[]; + } + class CyclicY { + @property.array(() => CyclicX) + cyclicProp: CyclicX[]; + } + const cyclicXMeta = MetadataInspector.getAllPropertyMetadata( + MODEL_PROPERTIES_KEY, + CyclicX.prototype, + ); + const cyclicYMeta = MetadataInspector.getAllPropertyMetadata( + MODEL_PROPERTIES_KEY, + CyclicY.prototype, + ); + + expect(cyclicXMeta) + .to.have.property('cyclicProp') + .which.eql({type: Array, itemType: () => CyclicY}); + expect(cyclicYMeta) + .to.have.property('cyclicProp') + .which.eql({type: Array, itemType: () => CyclicX}); + }); + + it('throws when used on a non-array property', () => { expect.throws( () => { // tslint:disable-next-line:no-unused-variable @@ -347,6 +374,17 @@ describe('model decorator', () => { property.ERR_PROP_NOT_ARRAY, ); }); + + it('throws when properties are cyclic', () => { + expect.throws( + () => { + require('../../fixtures/models/bad/cyclic-x.model'); + require('../../fixtures/models/bad/cyclic-y.model'); + }, + Error, + 'model is undefined', + ); + }); }); }); }); diff --git a/packages/repository/test/unit/decorator/relation.decorator.unit.ts b/packages/repository/test/unit/decorator/relation.decorator.unit.ts index d186edf0d314..e4dd344a2ab4 100644 --- a/packages/repository/test/unit/decorator/relation.decorator.unit.ts +++ b/packages/repository/test/unit/decorator/relation.decorator.unit.ts @@ -4,9 +4,17 @@ // License text available at https://opensource.org/licenses/MIT import {expect} from '@loopback/testlab'; -import {Entity, hasMany, RELATIONS_KEY, RelationType, property} from '../../..'; +import { + Entity, + hasMany, + RELATIONS_KEY, + RelationType, + property, + MODEL_PROPERTIES_KEY, + model, + belongsTo, +} from '../../..'; import {MetadataInspector} from '@loopback/context'; -import {MODEL_PROPERTIES_KEY, model} from '../../../src'; describe('relation decorator', () => { context('hasMany', () => { @@ -58,6 +66,7 @@ describe('relation decorator', () => { ); expect(meta).to.eql({ type: RelationType.hasMany, + target: Address, keyTo: 'addressBookId', }); expect(jugglerMeta).to.eql({ @@ -92,6 +101,7 @@ describe('relation decorator', () => { ); expect(meta).to.eql({ type: RelationType.hasMany, + target: Address, keyTo: 'someForeignKey', }); expect(jugglerMeta).to.eql({ @@ -100,8 +110,58 @@ describe('relation decorator', () => { }); }); + context('when given resolver', () => { + it('assigns it to target key', () => { + class Address extends Entity { + addressId: number; + street: string; + province: string; + } + + class AddressBook extends Entity { + id: number; + + @hasMany(() => Address) + addresses: Address[]; + } + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + AddressBook.prototype, + 'addresses', + ); + expect(meta).to.eql({ + type: RelationType.hasMany, + target: () => Address, + }); + }); + + it('accepts explicit keyTo property', () => { + class Address extends Entity { + addressId: number; + street: string; + province: string; + } + + class AddressBook extends Entity { + id: number; + + @hasMany(() => Address, {keyTo: 'someForeignKey'}) + addresses: Address[]; + } + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + AddressBook.prototype, + 'addresses', + ); + expect(meta).to.eql({ + type: RelationType.hasMany, + target: () => Address, + keyTo: 'someForeignKey', + }); + }); + }); + context('when interacting with @property.array', () => { - // Do you think this test case is necessary? it('does not get its property metadata overwritten by @property.array', () => { expect(() => { class Address extends Entity { @@ -123,4 +183,23 @@ describe('relation decorator', () => { }); }); }); + + context('belongsTo', () => { + it('creates juggler property metadata', () => { + class AddressBook extends Entity { + id: number; + } + class Address extends Entity { + @belongsTo(AddressBook) + addressBookId: number; + } + const jugglerMeta = MetadataInspector.getAllPropertyMetadata( + MODEL_PROPERTIES_KEY, + Address.prototype, + ); + expect(jugglerMeta).to.eql({ + addressBookId: {type: Number}, + }); + }); + }); }); diff --git a/packages/repository/test/unit/model/model.unit.ts b/packages/repository/test/unit/model/model.unit.ts index fd952dc47ddc..17baba7369b9 100644 --- a/packages/repository/test/unit/model/model.unit.ts +++ b/packages/repository/test/unit/model/model.unit.ts @@ -4,8 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {expect} from '@loopback/testlab'; -import {STRING} from '../../../'; -import {Entity, ModelDefinition} from '../../../'; +import {STRING, Entity, ModelDefinition, isModelResolver} from '../../../'; describe('model', () => { const customerDef = new ModelDefinition('Customer'); @@ -186,4 +185,16 @@ describe('model', () => { const instance = new NoId(); expect(() => instance.getId()).to.throw(/missing.*id/); }); + + context('isModelResolver', () => { + class SomeModel {} + + it('returns true if given arg is a resolver', () => { + expect(isModelResolver(() => SomeModel)).to.be.true(); + }); + + it('returns false if given arg is not a resolver', () => { + expect(isModelResolver(SomeModel)).to.be.false(); + }); + }); }); diff --git a/packages/repository/test/unit/repositories/relation.repository.unit.ts b/packages/repository/test/unit/repositories/relation.repository.unit.ts index 5dec448cd672..84a081a5e4d7 100644 --- a/packages/repository/test/unit/repositories/relation.repository.unit.ts +++ b/packages/repository/test/unit/repositories/relation.repository.unit.ts @@ -16,7 +16,13 @@ import { Options, DataObject, Where, + resolveHasManyMetadata, + HasManyDefinition, + RelationType, + RELATIONS_KEY, + RelationMap, } from '../../..'; +import {MetadataInspector} from '@loopback/context'; describe('relation repository', () => { context('HasManyRepository interface', () => { @@ -108,6 +114,99 @@ describe('relation repository', () => { }); }); + context('resolveHasManyMetadata', () => { + it('retains non-resolver type', () => { + class TargetModel extends Entity {} + const meta: HasManyDefinition = { + type: RelationType.hasMany, + target: TargetModel, + }; + const result = resolveHasManyMetadata(meta); + + expect(result).to.eql(meta); + }); + + it('throws if no target relation metadata is found', () => { + class TargetModel extends Entity {} + const meta: HasManyDefinition = { + type: RelationType.hasMany, + target: () => TargetModel, + }; + expect(() => resolveHasManyMetadata(meta)).to.throw( + /no belongsTo metadata found/, + ); + }); + + it('throws if no belongsTo metadata is found', () => { + class SourceModel extends Entity {} + class TargetModel extends Entity {} + const targetRelationMeta: RelationMap = { + foreignId: {type: RelationType.hasMany, target: () => SourceModel}, + }; + MetadataInspector.defineMetadata( + RELATIONS_KEY, + targetRelationMeta, + TargetModel.prototype, + ); + const meta: HasManyDefinition = { + type: RelationType.hasMany, + target: () => TargetModel, + }; + expect(() => resolveHasManyMetadata(meta)).to.throw( + /no belongsTo metadata found/, + ); + }); + + it('retains predefined keyTo property', () => { + class SourceModel extends Entity {} + class TargetModel extends Entity {} + const targetRelationMeta: RelationMap = { + foreignId: {type: RelationType.belongsTo, target: () => SourceModel}, + }; + MetadataInspector.defineMetadata( + RELATIONS_KEY, + targetRelationMeta, + TargetModel.prototype, + ); + const meta: HasManyDefinition = { + type: RelationType.hasMany, + target: () => TargetModel, + keyTo: 'someOtherForeignId', + }; + const result = resolveHasManyMetadata(meta); + const expected: HasManyDefinition = { + type: RelationType.hasMany, + target: () => TargetModel, + keyTo: 'someOtherForeignId', + }; + expect(result).to.eql(expected); + }); + + it('infers keyTo from property decorated with @belongsTo on target model', () => { + class SourceModel extends Entity {} + class TargetModel extends Entity {} + const targetRelationMeta: RelationMap = { + foreignId: {type: RelationType.belongsTo, target: () => SourceModel}, + }; + MetadataInspector.defineMetadata( + RELATIONS_KEY, + targetRelationMeta, + TargetModel.prototype, + ); + const meta: HasManyDefinition = { + type: RelationType.hasMany, + target: () => TargetModel, + }; + const result = resolveHasManyMetadata(meta); + const expected: HasManyDefinition = { + type: RelationType.hasMany, + target: () => TargetModel, + keyTo: 'foreignId', + }; + expect(result).to.eql(expected); + }); + }); + /*------------- HELPERS ---------------*/ class Customer extends Entity { From 17000375ec5b32a5424164b68643b7a8c8830b01 Mon Sep 17 00:00:00 2001 From: shimks Date: Fri, 17 Aug 2018 13:34:31 -0400 Subject: [PATCH 2/6] fixup! apply TypeResolver feedback --- .../src/build-schema.ts | 10 +--- .../src/decorators/model.decorator.ts | 4 +- .../src/decorators/relation.decorator.ts | 10 ++-- packages/repository/src/model.ts | 29 +++++++--- .../src/repositories/relation.factory.ts | 20 +++---- .../repository/test/unit/model/model.unit.ts | 29 ++++++++-- .../repositories/relation.repository.unit.ts | 56 +++++++++---------- 7 files changed, 88 insertions(+), 70 deletions(-) diff --git a/packages/repository-json-schema/src/build-schema.ts b/packages/repository-json-schema/src/build-schema.ts index 948d42256d63..db55a7dd1e34 100644 --- a/packages/repository-json-schema/src/build-schema.ts +++ b/packages/repository-json-schema/src/build-schema.ts @@ -7,7 +7,7 @@ import { ModelMetadataHelper, PropertyDefinition, ModelDefinition, - isModelResolver, + resolveType, } from '@loopback/repository'; import {MetadataInspector} from '@loopback/context'; import { @@ -125,9 +125,7 @@ export function metaToJsonProperty(meta: PropertyDefinition): JSONSchema { propertyType = stringTypeToWrapper(propertyType); if (isComplexType(propertyType)) { - propertyType = isModelResolver(propertyType) - ? propertyType() - : propertyType; + propertyType = resolveType(propertyType); Object.assign(propDef, {$ref: `#/definitions/${propertyType.name}`}); } else { Object.assign(propDef, { @@ -181,9 +179,7 @@ export function modelToJsonSchema(ctor: Function): JSONSchema { (metaProperty.itemType as string | Function) : (metaProperty.type as string | Function); if (typeof referenceType === 'function' && isComplexType(referenceType)) { - referenceType = isModelResolver(referenceType) - ? referenceType() - : referenceType; + referenceType = resolveType(referenceType); const propSchema = getJsonSchema(referenceType); if (propSchema && Object.keys(propSchema).length > 0) { diff --git a/packages/repository/src/decorators/model.decorator.ts b/packages/repository/src/decorators/model.decorator.ts index 674de1acae4d..77981836e5c1 100644 --- a/packages/repository/src/decorators/model.decorator.ts +++ b/packages/repository/src/decorators/model.decorator.ts @@ -14,7 +14,7 @@ import { ModelDefinition, ModelDefinitionSyntax, PropertyDefinition, - ModelResolver, + TypeResolver, } from '../model'; import {RELATIONS_KEY, RelationDefinitionBase} from './relation.decorator'; @@ -119,7 +119,7 @@ export namespace property { * metadata */ export function array( - itemType: T | ModelResolver, + itemType: T | TypeResolver, definition?: Partial, ) { return function(target: Object, propertyName: string) { diff --git a/packages/repository/src/decorators/relation.decorator.ts b/packages/repository/src/decorators/relation.decorator.ts index f5cdf8f83c00..e37242d13584 100644 --- a/packages/repository/src/decorators/relation.decorator.ts +++ b/packages/repository/src/decorators/relation.decorator.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {Class} from '../common-types'; -import {Entity, ModelResolver, isModelResolver} from '../model'; +import {Entity, TypeResolver, isTypeResolver} from '../model'; import {PropertyDecoratorFactory, MetadataInspector} from '@loopback/context'; import {property} from './model.decorator'; import {camelCase} from 'lodash'; @@ -31,7 +31,7 @@ export class RelationMetadata { export interface RelationDefinitionBase { type: RelationType; - target: typeof Entity | ModelResolver; + target: typeof Entity | TypeResolver; } export interface HasManyDefinition extends RelationDefinitionBase { @@ -55,7 +55,7 @@ export function relation(definition?: Object) { * @returns {(target:any, key:string)} */ export function belongsTo( - targetModel: T | ModelResolver, + targetModel: T | TypeResolver, definition?: Partial, ) { return function(target: Object, key: string) { @@ -91,7 +91,7 @@ export function hasOne(definition?: Object) { * @returns {(target:any, key:string)} */ export function hasMany( - targetModel: T | ModelResolver, + targetModel: T | TypeResolver, definition?: Partial, ) { // todo(shimks): extract out common logic (such as @property.array) to @@ -101,7 +101,7 @@ export function hasMany( const meta: Partial = {target: targetModel}; - if (!isModelResolver(targetModel)) { + if (!isTypeResolver(targetModel)) { const defaultFkName = camelCase(target.constructor.name + '_id'); const hasKeyTo = definition && definition.keyTo; const hasDefaultFkProperty = diff --git a/packages/repository/src/model.ts b/packages/repository/src/model.ts index 372c04f9e782..0c3aaf03c336 100644 --- a/packages/repository/src/model.ts +++ b/packages/repository/src/model.ts @@ -21,11 +21,11 @@ export type PropertyType = string | Function | Object | Type; * Property definition for a model */ export interface PropertyDefinition { - type: PropertyType | ModelResolver; // For example, 'string', String, or {} + type: PropertyType | TypeResolver; // For example, 'string', String, or {} id?: boolean; json?: PropertyForm; store?: PropertyForm; - itemType?: PropertyType | ModelResolver; // type of array + itemType?: PropertyType | TypeResolver; // type of array [attribute: string]: any; // Other attributes } @@ -264,17 +264,30 @@ export abstract class Entity extends Model implements Persistable { } /** - * DONT FORGET TO WRITE THE DOCS HERE - * REVIEWERS REMIND ME IF THIS IS STILL HERE + * An anonymous function that resolves to a class/entity + * Intended to be used for cases when the JS engine is unable to fully define + * a given type */ -export type ModelResolver = () => T; +export type TypeResolver = () => T; -export function isModelResolver( - fn: ModelResolver | T, -): fn is ModelResolver { +/** + * A function that checks whether given element is a TypeResolver or not + * @param fn + */ +export function isTypeResolver( + fn: TypeResolver | T, +): fn is TypeResolver { return !/^class/.test(fn.toString()); } +/** + * If given class/function is a TypeResolver, the resolved class is returned + * @param fn + */ +export function resolveType(fn: TypeResolver | T) { + return isTypeResolver(fn) ? fn() : fn; +} + /** * Domain events */ diff --git a/packages/repository/src/repositories/relation.factory.ts b/packages/repository/src/repositories/relation.factory.ts index 74fb5ffd34ef..536596100eac 100644 --- a/packages/repository/src/repositories/relation.factory.ts +++ b/packages/repository/src/repositories/relation.factory.ts @@ -7,16 +7,13 @@ import {EntityCrudRepository} from './repository'; import { HasManyDefinition, RelationType, - RELATIONS_KEY, } from '../decorators/relation.decorator'; -import {Entity, isModelResolver} from '../model'; +import {Entity, isTypeResolver} from '../model'; import { HasManyRepository, DefaultHasManyEntityCrudRepository, } from './relation.repository'; import {DataObject} from '../common-types'; -import {MetadataInspector} from '@loopback/context'; -import {RelationMap} from '../decorators/model.decorator'; const debug = require('debug')('loopback:repository:relation:factory'); @@ -64,22 +61,23 @@ export function createHasManyRepositoryFactory< }; } +/** + * Resolves given hasMany metadata if target is specified to be a resolver. + * Mainly used to infer what the `keyTo` property should be from the target's + * belongsTo metadata + * @param relationMeta hasMany metadata to resolve + */ export function resolveHasManyMetadata(relationMeta: HasManyDefinition) { if ( relationMeta.target && - isModelResolver(relationMeta.target) && + isTypeResolver(relationMeta.target) && !relationMeta.keyTo ) { const resolvedModel = relationMeta.target(); debug('resolved model from given metadata: %o', resolvedModel); - const targetRelationMeta: - | RelationMap - | undefined = MetadataInspector.getAllPropertyMetadata( - RELATIONS_KEY, - resolvedModel.prototype, - ); + const targetRelationMeta = resolvedModel.definition.relations; debug('relation metadata from %o: %o', resolvedModel, targetRelationMeta); diff --git a/packages/repository/test/unit/model/model.unit.ts b/packages/repository/test/unit/model/model.unit.ts index 17baba7369b9..9c3d1d8b436b 100644 --- a/packages/repository/test/unit/model/model.unit.ts +++ b/packages/repository/test/unit/model/model.unit.ts @@ -4,7 +4,13 @@ // License text available at https://opensource.org/licenses/MIT import {expect} from '@loopback/testlab'; -import {STRING, Entity, ModelDefinition, isModelResolver} from '../../../'; +import { + STRING, + Entity, + ModelDefinition, + isTypeResolver, + resolveType, +} from '../../../'; describe('model', () => { const customerDef = new ModelDefinition('Customer'); @@ -186,15 +192,26 @@ describe('model', () => { expect(() => instance.getId()).to.throw(/missing.*id/); }); - context('isModelResolver', () => { + context('TypeResolver', () => { class SomeModel {} - it('returns true if given arg is a resolver', () => { - expect(isModelResolver(() => SomeModel)).to.be.true(); + context('isTypeResolver', () => { + it('returns true if given arg is a resolver', () => { + expect(isTypeResolver(() => SomeModel)).to.be.true(); + }); + + it('returns false if given arg is not a resolver', () => { + expect(isTypeResolver(SomeModel)).to.be.false(); + }); }); - it('returns false if given arg is not a resolver', () => { - expect(isModelResolver(SomeModel)).to.be.false(); + context('resolveType', () => { + it('resolves given TypeResolver', () => { + expect(resolveType(() => SomeModel)).to.eql(SomeModel); + }); + it('returns given arg if not a TypeResolver', () => { + expect(resolveType(SomeModel)).to.eql(SomeModel); + }); }); }); }); diff --git a/packages/repository/test/unit/repositories/relation.repository.unit.ts b/packages/repository/test/unit/repositories/relation.repository.unit.ts index 84a081a5e4d7..c25d0fce5470 100644 --- a/packages/repository/test/unit/repositories/relation.repository.unit.ts +++ b/packages/repository/test/unit/repositories/relation.repository.unit.ts @@ -19,10 +19,8 @@ import { resolveHasManyMetadata, HasManyDefinition, RelationType, - RELATIONS_KEY, - RelationMap, + ModelDefinition, } from '../../..'; -import {MetadataInspector} from '@loopback/context'; describe('relation repository', () => { context('HasManyRepository interface', () => { @@ -127,7 +125,11 @@ describe('relation repository', () => { }); it('throws if no target relation metadata is found', () => { - class TargetModel extends Entity {} + class TargetModel extends Entity { + static definition = new ModelDefinition({ + name: 'TargetModel', + }); + } const meta: HasManyDefinition = { type: RelationType.hasMany, target: () => TargetModel, @@ -139,15 +141,14 @@ describe('relation repository', () => { it('throws if no belongsTo metadata is found', () => { class SourceModel extends Entity {} - class TargetModel extends Entity {} - const targetRelationMeta: RelationMap = { - foreignId: {type: RelationType.hasMany, target: () => SourceModel}, - }; - MetadataInspector.defineMetadata( - RELATIONS_KEY, - targetRelationMeta, - TargetModel.prototype, - ); + class TargetModel extends Entity { + static definition = new ModelDefinition({ + name: 'TargetModel', + relations: { + foreignId: {type: RelationType.hasMany, target: () => SourceModel}, + }, + }); + } const meta: HasManyDefinition = { type: RelationType.hasMany, target: () => TargetModel, @@ -158,16 +159,7 @@ describe('relation repository', () => { }); it('retains predefined keyTo property', () => { - class SourceModel extends Entity {} class TargetModel extends Entity {} - const targetRelationMeta: RelationMap = { - foreignId: {type: RelationType.belongsTo, target: () => SourceModel}, - }; - MetadataInspector.defineMetadata( - RELATIONS_KEY, - targetRelationMeta, - TargetModel.prototype, - ); const meta: HasManyDefinition = { type: RelationType.hasMany, target: () => TargetModel, @@ -184,15 +176,17 @@ describe('relation repository', () => { it('infers keyTo from property decorated with @belongsTo on target model', () => { class SourceModel extends Entity {} - class TargetModel extends Entity {} - const targetRelationMeta: RelationMap = { - foreignId: {type: RelationType.belongsTo, target: () => SourceModel}, - }; - MetadataInspector.defineMetadata( - RELATIONS_KEY, - targetRelationMeta, - TargetModel.prototype, - ); + class TargetModel extends Entity { + static definition = new ModelDefinition({ + name: 'TargetModel', + relations: { + foreignId: { + type: RelationType.belongsTo, + target: () => SourceModel, + }, + }, + }); + } const meta: HasManyDefinition = { type: RelationType.hasMany, target: () => TargetModel, From 25cb5d863d7f6da15f4186c2bd83e48c5a338a7f Mon Sep 17 00:00:00 2001 From: shimks Date: Tue, 21 Aug 2018 17:03:39 -0400 Subject: [PATCH 3/6] feat: add belongsTo repository --- .../src/decorators/relation.decorator.ts | 41 ++- .../src/repositories/legacy-juggler-bridge.ts | 30 +- .../src/repositories/relation.factory.ts | 80 ++++- .../src/repositories/relation.repository.ts | 54 ++- .../belongs-to.relation.acceptance.ts | 67 ++++ .../has-many.relation.acceptance.ts | 87 +---- .../test/fixtures/models/customer.model.ts | 19 ++ .../repository/test/fixtures/models/index.ts | 2 + .../test/fixtures/models/order.model.ts | 26 ++ .../repositories/customer.repository.ts | 26 ++ .../test/fixtures/repositories/index.ts | 2 + .../fixtures/repositories/order.repository.ts | 22 ++ .../relation.factory.integration.ts | 320 +++++++++++------- .../model-and-relation.decorator.unit.ts | 1 + .../unit/decorator/relation.decorator.unit.ts | 154 +++++++++ .../repositories/constraint-utils.unit.ts | 9 + .../repositories/relation.repository.unit.ts | 103 ++++++ 17 files changed, 827 insertions(+), 216 deletions(-) create mode 100644 packages/repository/test/acceptance/belongs-to.relation.acceptance.ts create mode 100644 packages/repository/test/fixtures/models/customer.model.ts create mode 100644 packages/repository/test/fixtures/models/index.ts create mode 100644 packages/repository/test/fixtures/models/order.model.ts create mode 100644 packages/repository/test/fixtures/repositories/customer.repository.ts create mode 100644 packages/repository/test/fixtures/repositories/index.ts create mode 100644 packages/repository/test/fixtures/repositories/order.repository.ts diff --git a/packages/repository/src/decorators/relation.decorator.ts b/packages/repository/src/decorators/relation.decorator.ts index e37242d13584..82394b741864 100644 --- a/packages/repository/src/decorators/relation.decorator.ts +++ b/packages/repository/src/decorators/relation.decorator.ts @@ -39,6 +39,12 @@ export interface HasManyDefinition extends RelationDefinitionBase { keyTo?: string; } +export interface BelongsToDefinition extends RelationDefinitionBase { + type: RelationType.belongsTo; + keyTo?: string; + keyFrom?: string; +} + /** * Decorator for relations * @param definition @@ -56,18 +62,43 @@ export function relation(definition?: Object) { */ export function belongsTo( targetModel: T | TypeResolver, - definition?: Partial, + definition?: Partial, ) { return function(target: Object, key: string) { const propMeta = { type: MetadataInspector.getDesignTypeForProperty(target, key), }; property(propMeta)(target, key); + + const rel: BelongsToDefinition = { + type: RelationType.belongsTo, + target: targetModel, + keyFrom: key, + }; + + if (!isTypeResolver(targetModel)) { + const hasKeyTo = definition && definition.keyTo; + let hasPrimaryKey = false; + + for (const propertyKey in targetModel.definition.properties) { + if (targetModel.definition.properties[propertyKey].id === true) { + rel.keyTo = propertyKey; + hasPrimaryKey = true; + break; + } + } + + if (!hasPrimaryKey && !hasKeyTo) { + throw new Error( + `primary key not found on ${ + targetModel.name + } model's juggler definition`, + ); + } + } + // Apply model definition to the model class - const rel = Object.assign( - {type: RelationType.belongsTo, target: targetModel}, - definition, - ); + Object.assign(rel, definition); PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel)(target, key); }; } diff --git a/packages/repository/src/repositories/legacy-juggler-bridge.ts b/packages/repository/src/repositories/legacy-juggler-bridge.ts index c862cbcb4e48..b537df1c3224 100644 --- a/packages/repository/src/repositories/legacy-juggler-bridge.ts +++ b/packages/repository/src/repositories/legacy-juggler-bridge.ts @@ -6,7 +6,7 @@ import * as legacy from 'loopback-datasource-juggler'; import * as assert from 'assert'; -import {isPromiseLike} from '@loopback/context'; +import {isPromiseLike, Getter} from '@loopback/context'; import { Options, AnyObject, @@ -21,8 +21,16 @@ import {EntityCrudRepository} from './repository'; import { createHasManyRepositoryFactory, HasManyRepositoryFactory, + BelongsToFactory, + createBelongsToFactory, } from './relation.factory'; -import {HasManyDefinition} from '../decorators/relation.decorator'; +import { + HasManyDefinition, + BelongsToDefinition, +} from '../decorators/relation.decorator'; +// need the import for exporting of a return type +// tslint:disable-next-line:no-unused-variable +import {HasManyRepository} from './relation.repository'; export namespace juggler { export import DataSource = legacy.DataSource; @@ -162,7 +170,9 @@ export class DefaultCrudRepository ForeignKeyType >( relationName: string, - targetRepo: EntityCrudRepository, + targetRepo: + | EntityCrudRepository + | Getter>, ): HasManyRepositoryFactory { const meta = this.entityClass.definition.relations[relationName]; return createHasManyRepositoryFactory( @@ -171,6 +181,20 @@ export class DefaultCrudRepository ); } + protected _createBelongsToFactoryFor< + Target extends Entity, + TargetId, + Source extends Entity + >( + relationName: string, + targetRepo: + | EntityCrudRepository + | Getter>, + ): BelongsToFactory { + const meta = this.entityClass.definition.relations[relationName]; + return createBelongsToFactory(meta as BelongsToDefinition, targetRepo); + } + async create(entity: DataObject, options?: Options): Promise { const model = await ensurePromise(this.modelClass.create(entity, options)); return this.toEntity(model); diff --git a/packages/repository/src/repositories/relation.factory.ts b/packages/repository/src/repositories/relation.factory.ts index 536596100eac..c6a623f3e600 100644 --- a/packages/repository/src/repositories/relation.factory.ts +++ b/packages/repository/src/repositories/relation.factory.ts @@ -7,22 +7,30 @@ import {EntityCrudRepository} from './repository'; import { HasManyDefinition, RelationType, + BelongsToDefinition, } from '../decorators/relation.decorator'; import {Entity, isTypeResolver} from '../model'; import { HasManyRepository, DefaultHasManyEntityCrudRepository, + DefaultBelongsToEntityCrudRepository, } from './relation.repository'; import {DataObject} from '../common-types'; +import {Getter} from '@loopback/context'; const debug = require('debug')('loopback:repository:relation:factory'); const ERR_NO_BELONGSTO_META = 'no belongsTo metadata found'; +const ERR_NO_ID_META = 'no id metadata found'; export type HasManyRepositoryFactory = ( fkValue: ForeignKeyType, ) => HasManyRepository; +export type BelongsToFactory = ( + sourceModel: Source, +) => Promise; + /** * Enforces a constraint on a repository based on a relationship contract * between models. For example, if a Customer model is related to an Order model @@ -42,7 +50,9 @@ export function createHasManyRepositoryFactory< ForeignKeyType >( relationMetadata: HasManyDefinition, - targetRepository: EntityCrudRepository, + targetRepository: + | EntityCrudRepository + | Getter>, ): HasManyRepositoryFactory { resolveHasManyMetadata(relationMetadata); debug('resolved relation metadata: %o', relationMetadata); @@ -61,6 +71,39 @@ export function createHasManyRepositoryFactory< }; } +export function createBelongsToFactory< + Target extends Entity, + TargetId, + Source extends Entity +>( + belongsToMetadata: BelongsToDefinition, + targetRepository: + | EntityCrudRepository + | Getter>, +): BelongsToFactory { + resolveBelongsToMetadata(belongsToMetadata); + const foreignKey = belongsToMetadata.keyFrom; + const primaryKey = belongsToMetadata.keyTo; + if (!foreignKey) { + throw new Error( + 'The foreign key property name (keyFrom) must be specified', + ); + } + if (!primaryKey) { + throw new Error('The primary key property name (keyTo) must be specified'); + } + return async function(sourceModel: Source) { + const foreignKeyValue = sourceModel[foreignKey as keyof Source]; + // tslint:disable-next-line:no-any + const constraint: any = {[primaryKey]: foreignKeyValue}; + const constrainedRepo = new DefaultBelongsToEntityCrudRepository( + targetRepository, + constraint as DataObject, + ); + return constrainedRepo.find(); + }; +} + /** * Resolves given hasMany metadata if target is specified to be a resolver. * Mainly used to infer what the `keyTo` property should be from the target's @@ -101,3 +144,38 @@ export function resolveHasManyMetadata(relationMeta: HasManyDefinition) { } return relationMeta; } + +export function resolveBelongsToMetadata(relationMeta: BelongsToDefinition) { + if ( + relationMeta.target && + isTypeResolver(relationMeta.target) && + !relationMeta.keyTo + ) { + const resolvedModel = relationMeta.target(); + + debug('resolved model from given metadata: %o', resolvedModel); + + const targetPropertiesMeta = resolvedModel.definition.properties; + + debug('relation metadata from %o: %o', resolvedModel, targetPropertiesMeta); + + if (!targetPropertiesMeta) { + throw new Error(ERR_NO_ID_META); + } + + let idMetaExists = false; + + for (const key in targetPropertiesMeta) { + if (targetPropertiesMeta[key].id === true) { + relationMeta.keyTo = key; + idMetaExists = true; + break; + } + } + + if (!idMetaExists) { + throw new Error(ERR_NO_ID_META); + } + } + return relationMeta; +} diff --git a/packages/repository/src/repositories/relation.repository.ts b/packages/repository/src/repositories/relation.repository.ts index b9e255dc5552..541567f4319a 100644 --- a/packages/repository/src/repositories/relation.repository.ts +++ b/packages/repository/src/repositories/relation.repository.ts @@ -12,6 +12,7 @@ import { import {DataObject, Options} from '../common-types'; import {Entity} from '../model'; import {Filter, Where} from '../query'; +import {Getter} from '@loopback/context'; /** * CRUD operations for a target repository of a HasMany relation @@ -55,6 +56,10 @@ export interface HasManyRepository { ): Promise; } +export interface BelongsToRepository { + find(options?: Options): Promise; +} + export class DefaultHasManyEntityCrudRepository< TargetEntity extends Entity, TargetID, @@ -62,34 +67,44 @@ export class DefaultHasManyEntityCrudRepository< > implements HasManyRepository { /** * Constructor of DefaultHasManyEntityCrudRepository - * @param targetRepository the related target model repository instance + * @param targetRepositoryGetter the related target model repository instance * @param constraint the key value pair representing foreign key name to constrain * the target repository instance */ + + public targetRepositoryGetter: Getter; + constructor( - public targetRepository: TargetRepository, + targetRepository: TargetRepository | Getter, public constraint: DataObject, - ) {} + ) { + this.targetRepositoryGetter = + typeof targetRepository === 'object' + ? ((() => Promise.resolve(targetRepository)) as Getter< + TargetRepository + >) + : targetRepository; + } async create( targetModelData: DataObject, options?: Options, ): Promise { - return await this.targetRepository.create( + return (await this.targetRepositoryGetter()).create( constrainDataObject(targetModelData, this.constraint), options, ); } async find(filter?: Filter, options?: Options): Promise { - return await this.targetRepository.find( + return (await this.targetRepositoryGetter()).find( constrainFilter(filter, this.constraint), options, ); } async delete(where?: Where, options?: Options): Promise { - return await this.targetRepository.deleteAll( + return (await this.targetRepositoryGetter()).deleteAll( constrainWhere(where, this.constraint), options, ); @@ -100,10 +115,35 @@ export class DefaultHasManyEntityCrudRepository< where?: Where, options?: Options, ): Promise { - return await this.targetRepository.updateAll( + return (await this.targetRepositoryGetter()).updateAll( constrainDataObject(dataObject, this.constraint), constrainWhere(where, this.constraint), options, ); } } + +export class DefaultBelongsToEntityCrudRepository< + TargetEntity extends Entity, + TargetId, + TargetRepository extends EntityCrudRepository +> { + public targetRepositoryGetter: Getter; + + constructor( + targetRepository: TargetRepository | Getter, + public constraint: DataObject, + ) { + this.targetRepositoryGetter = + typeof targetRepository === 'object' + ? () => Promise.resolve(targetRepository) + : targetRepository; + } + + async find(options?: Options): Promise { + return (await (await this.targetRepositoryGetter()).find( + constrainFilter(undefined, this.constraint), + options, + ))[0]; + } +} diff --git a/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts new file mode 100644 index 000000000000..f94a9a88f587 --- /dev/null +++ b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts @@ -0,0 +1,67 @@ +// Copyright IBM Corp. 2017,2018. 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 {juggler, repository, RepositoryMixin, AppWithRepository} from '../..'; +import {CustomerRepository, OrderRepository} from '../fixtures/repositories'; +import {expect} from '@loopback/testlab'; +import {Application} from '@loopback/core'; + +describe('HasMany relation', () => { + // Given a Customer and Order models - see definitions at the bottom + + let app: AppWithRepository; + let controller: OrderController; + let customerRepo: CustomerRepository; + let orderRepo: OrderRepository; + + before(givenApplicationWithMemoryDB); + before(givenBoundCrudRepositoriesForCustomerAndOrder); + before(givenOrderController); + + afterEach(async () => { + await orderRepo.deleteAll(); + }); + + it('can find customer of order', async () => { + const customer = await customerRepo.create({name: 'Order McForder'}); + const order = await orderRepo.create({ + customerId: customer.id, + description: 'Order from Order McForder, the hoarder of Mordor', + }); + const result = await controller.findOwnerOfOrder(order.id); + expect(result).to.deepEqual(customer); + }); + + //--- HELPERS ---// + + class OrderController { + constructor( + @repository(OrderRepository) protected orderRepository: OrderRepository, + ) {} + + async findOwnerOfOrder(orderId: number) { + const order = await this.orderRepository.findById(orderId); + return await this.orderRepository.customer(order); + } + } + + function givenApplicationWithMemoryDB() { + class TestApp extends RepositoryMixin(Application) {} + app = new TestApp(); + app.dataSource(new juggler.DataSource({name: 'db', connector: 'memory'})); + } + + async function givenBoundCrudRepositoriesForCustomerAndOrder() { + app.repository(CustomerRepository); + app.repository(OrderRepository); + customerRepo = await app.getRepository(CustomerRepository); + orderRepo = await app.getRepository(OrderRepository); + } + + async function givenOrderController() { + app.controller(OrderController); + controller = await app.get('controllers.OrderController'); + } +}); diff --git a/packages/repository/test/acceptance/has-many.relation.acceptance.ts b/packages/repository/test/acceptance/has-many.relation.acceptance.ts index e2c4030c0f72..c996ce2d08a1 100644 --- a/packages/repository/test/acceptance/has-many.relation.acceptance.ts +++ b/packages/repository/test/acceptance/has-many.relation.acceptance.ts @@ -3,22 +3,11 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import { - model, - property, - Entity, - DefaultCrudRepository, - juggler, - hasMany, - belongsTo, - repository, - RepositoryMixin, - AppWithRepository, - HasManyRepositoryFactory, -} from '../..'; +import {juggler, repository, RepositoryMixin, AppWithRepository} from '../..'; +import {Order} from '../fixtures/models'; +import {CustomerRepository, OrderRepository} from '../fixtures/repositories'; import {expect} from '@loopback/testlab'; import * as _ from 'lodash'; -import {inject} from '@loopback/context'; import {Application} from '@loopback/core'; describe('HasMany relation', () => { @@ -149,76 +138,6 @@ describe('HasMany relation', () => { //--- HELPERS ---// - @model() - class Order extends Entity { - @property({ - type: 'number', - id: true, - }) - id: number; - - @property({ - type: 'string', - required: true, - }) - description: string; - - @property({ - type: 'boolean', - required: false, - }) - isShipped: boolean; - - @belongsTo(() => Customer) - customerId: number; - } - - @model() - class Customer extends Entity { - @property({ - type: 'number', - id: true, - }) - id: number; - - @property({ - type: 'string', - }) - name: string; - - @hasMany(() => Order) - orders: Order[]; - } - - class OrderRepository extends DefaultCrudRepository< - Order, - typeof Order.prototype.id - > { - constructor(@inject('datasources.db') protected db: juggler.DataSource) { - super(Order, db); - } - } - - class CustomerRepository extends DefaultCrudRepository< - Customer, - typeof Customer.prototype.id - > { - public orders: HasManyRepositoryFactory< - Order, - typeof Customer.prototype.id - >; - constructor( - @inject('datasources.db') protected db: juggler.DataSource, - @repository(OrderRepository) orderRepository: OrderRepository, - ) { - super(Customer, db); - this.orders = this._createHasManyRepositoryFactoryFor( - 'orders', - orderRepository, - ); - } - } - class CustomerController { constructor( @repository(CustomerRepository) diff --git a/packages/repository/test/fixtures/models/customer.model.ts b/packages/repository/test/fixtures/models/customer.model.ts new file mode 100644 index 000000000000..8c49456ad3d3 --- /dev/null +++ b/packages/repository/test/fixtures/models/customer.model.ts @@ -0,0 +1,19 @@ +import {model, property, hasMany, Entity} from '../../..'; +import {Order} from './order.model'; + +@model() +export class Customer extends Entity { + @property({ + type: 'number', + id: true, + }) + id: number; + + @property({ + type: 'string', + }) + name: string; + + @hasMany(() => Order) + orders: Order[]; +} diff --git a/packages/repository/test/fixtures/models/index.ts b/packages/repository/test/fixtures/models/index.ts new file mode 100644 index 000000000000..f7706bb383b9 --- /dev/null +++ b/packages/repository/test/fixtures/models/index.ts @@ -0,0 +1,2 @@ +export * from './customer.model'; +export * from './order.model'; diff --git a/packages/repository/test/fixtures/models/order.model.ts b/packages/repository/test/fixtures/models/order.model.ts new file mode 100644 index 000000000000..55bfa6960e97 --- /dev/null +++ b/packages/repository/test/fixtures/models/order.model.ts @@ -0,0 +1,26 @@ +import {model, property, belongsTo, Entity} from '../../..'; +import {Customer} from './customer.model'; + +@model() +export class Order extends Entity { + @property({ + type: 'number', + id: true, + }) + id: number; + + @property({ + type: 'string', + required: true, + }) + description: string; + + @property({ + type: 'boolean', + required: false, + }) + isShipped: boolean; + + @belongsTo(() => Customer) + customerId: number; +} diff --git a/packages/repository/test/fixtures/repositories/customer.repository.ts b/packages/repository/test/fixtures/repositories/customer.repository.ts new file mode 100644 index 000000000000..f7ae4afaa351 --- /dev/null +++ b/packages/repository/test/fixtures/repositories/customer.repository.ts @@ -0,0 +1,26 @@ +import {Customer, Order} from '../models'; +import {OrderRepository} from './order.repository'; +import { + DefaultCrudRepository, + HasManyRepositoryFactory, + juggler, +} from '../../..'; +import {inject, Getter} from '@loopback/context'; + +export class CustomerRepository extends DefaultCrudRepository< + Customer, + typeof Customer.prototype.id +> { + public orders: HasManyRepositoryFactory; + constructor( + @inject('datasources.db') protected db: juggler.DataSource, + @inject.getter('repositories.OrderRepository') + orderRepositoryGetter: Getter, + ) { + super(Customer, db); + this.orders = this._createHasManyRepositoryFactoryFor( + 'orders', + orderRepositoryGetter, + ); + } +} diff --git a/packages/repository/test/fixtures/repositories/index.ts b/packages/repository/test/fixtures/repositories/index.ts new file mode 100644 index 000000000000..4c0e725c50a1 --- /dev/null +++ b/packages/repository/test/fixtures/repositories/index.ts @@ -0,0 +1,2 @@ +export * from './customer.repository'; +export * from './order.repository'; diff --git a/packages/repository/test/fixtures/repositories/order.repository.ts b/packages/repository/test/fixtures/repositories/order.repository.ts new file mode 100644 index 000000000000..449de8797f3b --- /dev/null +++ b/packages/repository/test/fixtures/repositories/order.repository.ts @@ -0,0 +1,22 @@ +import {Order, Customer} from '../models'; +import {CustomerRepository} from '../repositories'; +import {DefaultCrudRepository, juggler, BelongsToFactory} from '../../..'; +import {inject, Getter} from '@loopback/context'; + +export class OrderRepository extends DefaultCrudRepository< + Order, + typeof Order.prototype.id +> { + public customer: BelongsToFactory; + constructor( + @inject('datasources.db') protected db: juggler.DataSource, + @inject.getter('repositories.CustomerRepository') + customerRepositoryGetter: Getter, + ) { + super(Order, db); + this.customer = this._createBelongsToFactoryFor( + 'customerId', + customerRepositoryGetter, + ); + } +} diff --git a/packages/repository/test/integration/repositories/relation.factory.integration.ts b/packages/repository/test/integration/repositories/relation.factory.integration.ts index 2d7f8e765e3e..4c1c68f811d2 100644 --- a/packages/repository/test/integration/repositories/relation.factory.integration.ts +++ b/packages/repository/test/integration/repositories/relation.factory.integration.ts @@ -18,29 +18,28 @@ import { belongsTo, model, property, + createBelongsToFactory, + BelongsToDefinition, } from '../../..'; import {expect} from '@loopback/testlab'; -describe('HasMany relation', () => { - // Given a Customer and Order models - see definitions at the bottom - let db: juggler.DataSource; - let customerRepo: EntityCrudRepository< - Customer, - typeof Customer.prototype.id - >; - let orderRepo: EntityCrudRepository; - let reviewRepo: EntityCrudRepository; - let customerOrderRepo: HasManyRepository; - let customerAuthoredReviewFactoryFn: HasManyRepositoryFactory< - Review, - typeof Customer.prototype.id - >; - let customerApprovedReviewFactoryFn: HasManyRepositoryFactory< - Review, - typeof Customer.prototype.id - >; - let existingCustomerId: number; +// Given a Customer and Order models - see definitions at the bottom +let db: juggler.DataSource; +let customerRepo: EntityCrudRepository; +let orderRepo: EntityCrudRepository; +let reviewRepo: EntityCrudRepository; +let customerOrderRepo: HasManyRepository; +let customerAuthoredReviewFactoryFn: HasManyRepositoryFactory< + Review, + typeof Customer.prototype.id +>; +let customerApprovedReviewFactoryFn: HasManyRepositoryFactory< + Review, + typeof Customer.prototype.id +>; +let existingCustomerId: number; +describe('HasMany relation', () => { before(givenCrudRepositories); before(givenPersistedCustomerInstance); before(givenConstrainedRepositories); @@ -168,107 +167,196 @@ describe('HasMany relation', () => { }); }); }); +}); - //--- HELPERS ---// +describe('belongsTo relation', () => { + it('can find an instance of the related model', async () => { + const findCustomerOfOrder = createBelongsToFactory( + Order.definition.relations.customerId as BelongsToDefinition, + customerRepo, + ); - class Order extends Entity { - id: number; - description: string; - customerId: number; + const customer = await customerRepo.create({name: 'Order McForder'}); + const order = await orderRepo.create({ + customerId: customer.id, + description: 'Order from Order McForder, the hoarder of Mordor', + }); + const result = await findCustomerOfOrder(order); + expect(result).to.deepEqual(customer); + }); - static definition = new ModelDefinition({ - name: 'Order', - properties: { - id: {type: 'number', id: true}, - description: {type: 'string', required: true}, - customerId: {type: 'number', required: true}, - }, + context('createBelongsToFactory', () => { + it('errors when keyFrom is not available from belongsTo metadata', () => { + class SomeClass extends Entity {} + const keyFromLessMeta: BelongsToDefinition = { + type: RelationType.belongsTo, + target: SomeClass, + keyTo: 'someKey', + }; + expect(() => + createBelongsToFactory(keyFromLessMeta, reviewRepo), + ).to.throw(/The foreign key property name \(keyFrom\) must be specified/); }); - } - - class Review extends Entity { - id: number; - description: string; - authorId: number; - approvedId: number; - - static definition = new ModelDefinition({ - name: 'Review', - properties: { - id: {type: 'number', id: true}, - description: {type: 'string', required: true}, - authorId: {type: 'number', required: false}, - approvedId: {type: 'number', required: false}, - }, + + it('errors when keyTo is not available from belongsTo metadata', () => { + class SomeClass extends Entity {} + const keyToLessMeta: BelongsToDefinition = { + type: RelationType.belongsTo, + target: SomeClass, + keyFrom: 'someKey', + }; + expect(() => createBelongsToFactory(keyToLessMeta, reviewRepo)).to.throw( + /The primary key property name \(keyTo\) must be specified/, + ); }); - } - - class Customer extends Entity { - id: number; - name: string; - orders: Order[]; - reviewsAuthored: Review[]; - reviewsApproved: Review[]; - - static definition = new ModelDefinition({ - name: 'Customer', - properties: { - id: {type: 'number', id: true}, - name: {type: 'string', required: true}, - orders: {type: Order, array: true}, - reviewsAuthored: {type: Review, array: true}, - reviewsApproved: {type: Review, array: true}, - }, - relations: { - orders: { - type: RelationType.hasMany, - target: () => Order, - keyTo: 'customerId', - }, - reviewsAuthored: { - type: RelationType.hasMany, - target: () => Review, - keyTo: 'authorId', - }, - reviewsApproved: { - type: RelationType.hasMany, - target: () => Review, - keyTo: 'approvedId', - }, - }, + + it('resolves property id metadata', () => { + @model() + class Card extends Entity { + @property({id: true}) + id: number; + @belongsTo(() => Suite) + suiteId: string; + } + + @model() + class Suite extends Entity { + @property({id: true}) + id: string; + cards: Card[]; + } + + const belongsToMeta = Card.definition.relations + .suiteId as BelongsToDefinition; + expect(belongsToMeta).to.eql({ + type: RelationType.belongsTo, + target: () => Suite, + keyFrom: 'suiteId', + }); + createBelongsToFactory( + belongsToMeta, + new DefaultCrudRepository( + Suite, + new juggler.DataSource({connector: 'memory'}), + ), + ); + expect(belongsToMeta).to.eql({ + type: RelationType.belongsTo, + target: () => Suite, + keyFrom: 'suiteId', + keyTo: 'id', + }); }); - } - - function givenCrudRepositories() { - db = new juggler.DataSource({connector: 'memory'}); - - customerRepo = new DefaultCrudRepository(Customer, db); - orderRepo = new DefaultCrudRepository(Order, db); - reviewRepo = new DefaultCrudRepository(Review, db); - } - - async function givenPersistedCustomerInstance() { - existingCustomerId = (await customerRepo.create({name: 'a customer'})).id; - } - - function givenConstrainedRepositories() { - const orderFactoryFn = createHasManyRepositoryFactory< - Order, - typeof Order.prototype.id, - typeof Customer.prototype.id - >(Customer.definition.relations.orders as HasManyDefinition, orderRepo); - - customerOrderRepo = orderFactoryFn(existingCustomerId); - } - - function givenRepositoryFactoryFunctions() { - customerAuthoredReviewFactoryFn = createHasManyRepositoryFactory( - Customer.definition.relations.reviewsAuthored as HasManyDefinition, - reviewRepo, - ); - customerApprovedReviewFactoryFn = createHasManyRepositoryFactory( - Customer.definition.relations.reviewsApproved as HasManyDefinition, - reviewRepo, - ); - } + }); }); + +//--- HELPERS ---// + +class Order extends Entity { + id: number; + description: string; + customerId: number; + + static definition: ModelDefinition = new ModelDefinition({ + name: 'Order', + properties: { + id: {type: 'number', id: true}, + description: {type: 'string', required: true}, + customerId: {type: 'number', required: true}, + }, + relations: { + customerId: { + type: RelationType.belongsTo, + target: () => Customer, + keyFrom: 'customerId', + keyTo: 'id', + }, + }, + }); +} + +class Review extends Entity { + id: number; + description: string; + authorId: number; + approvedId: number; + + static definition = new ModelDefinition({ + name: 'Review', + properties: { + id: {type: 'number', id: true}, + description: {type: 'string', required: true}, + authorId: {type: 'number', required: false}, + approvedId: {type: 'number', required: false}, + }, + }); +} + +class Customer extends Entity { + id: number; + name: string; + orders: Order[]; + reviewsAuthored: Review[]; + reviewsApproved: Review[]; + + static definition: ModelDefinition = new ModelDefinition({ + name: 'Customer', + properties: { + id: {type: 'number', id: true}, + name: {type: 'string', required: true}, + orders: {type: Order, array: true}, + reviewsAuthored: {type: Review, array: true}, + reviewsApproved: {type: Review, array: true}, + }, + relations: { + orders: { + type: RelationType.hasMany, + target: () => Order, + keyTo: 'customerId', + }, + reviewsAuthored: { + type: RelationType.hasMany, + target: () => Review, + keyTo: 'authorId', + }, + reviewsApproved: { + type: RelationType.hasMany, + target: () => Review, + keyTo: 'approvedId', + }, + }, + }); +} + +function givenCrudRepositories() { + db = new juggler.DataSource({connector: 'memory'}); + + customerRepo = new DefaultCrudRepository(Customer, db); + orderRepo = new DefaultCrudRepository(Order, db); + reviewRepo = new DefaultCrudRepository(Review, db); +} + +async function givenPersistedCustomerInstance() { + existingCustomerId = (await customerRepo.create({name: 'a customer'})).id; +} + +function givenConstrainedRepositories() { + const orderFactoryFn = createHasManyRepositoryFactory< + Order, + typeof Order.prototype.id, + typeof Customer.prototype.id + >(Customer.definition.relations.orders as HasManyDefinition, orderRepo); + + customerOrderRepo = orderFactoryFn(existingCustomerId); +} + +function givenRepositoryFactoryFunctions() { + customerAuthoredReviewFactoryFn = createHasManyRepositoryFactory( + Customer.definition.relations.reviewsAuthored as HasManyDefinition, + reviewRepo, + ); + customerApprovedReviewFactoryFn = createHasManyRepositoryFactory( + Customer.definition.relations.reviewsApproved as HasManyDefinition, + reviewRepo, + ); +} diff --git a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts index 69e09da3fb90..8ad477ec10bf 100644 --- a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts +++ b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts @@ -273,6 +273,7 @@ describe('model decorator', () => { expect(meta.customer).to.eql({ type: RelationType.belongsTo, target: () => Customer, + keyFrom: 'customer', }); }); diff --git a/packages/repository/test/unit/decorator/relation.decorator.unit.ts b/packages/repository/test/unit/decorator/relation.decorator.unit.ts index e4dd344a2ab4..671ab1b20517 100644 --- a/packages/repository/test/unit/decorator/relation.decorator.unit.ts +++ b/packages/repository/test/unit/decorator/relation.decorator.unit.ts @@ -186,7 +186,9 @@ describe('relation decorator', () => { context('belongsTo', () => { it('creates juggler property metadata', () => { + @model() class AddressBook extends Entity { + @property({id: true}) id: number; } class Address extends Entity { @@ -201,5 +203,157 @@ describe('relation decorator', () => { addressBookId: {type: Number}, }); }); + + context('when given model', () => { + it('throws when id is not defined in the target TypeScript model', () => { + expect(() => { + @model() + class AddressBook extends Entity { + addresses: Address[]; + } + + class Address extends Entity { + addressId: number; + street: string; + province: string; + @belongsTo(AddressBook) + addressBookId: number; + } + }).throw(/primary key not found on AddressBook/); + }); + + it('infers foreign key', () => { + @model() + class AddressBook extends Entity { + @property({id: true}) + id: number; + } + class Address extends Entity { + @belongsTo(AddressBook) + addressBookId: number; + @property() + someOtherKey: string; + } + const meta = MetadataInspector.getAllPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + ); + expect(meta) + .to.have.property('addressBookId') + .which.containEql({keyFrom: 'addressBookId'}); + }); + + it('takes in complex property type and defines infers primary key name', () => { + @model() + class AddressBook extends Entity { + @property({id: true}) + yellowPageId: number; + addresses: Address[]; + } + + @model() + class Address extends Entity { + addressId: number; + street: string; + province: string; + @belongsTo(AddressBook) + addressBookId: number; + } + + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + 'addressBookId', + ); + expect(meta).to.eql({ + type: RelationType.belongsTo, + target: AddressBook, + keyFrom: 'addressBookId', + keyTo: 'yellowPageId', + }); + }); + + it('takes in both complex property type and belongsTo metadata', () => { + @model() + class AddressBook extends Entity { + id: number; + addresses: Address[]; + } + + class Address extends Entity { + addressId: number; + street: string; + province: string; + @belongsTo(AddressBook, {keyTo: 'somePrimaryKey'}) + addressBookId: number; + } + + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + 'addressBookId', + ); + expect(meta).to.eql({ + type: RelationType.belongsTo, + target: AddressBook, + keyFrom: 'addressBookId', + keyTo: 'somePrimaryKey', + }); + }); + }); + + context('when given resolver', () => { + it('assigns it to target key', () => { + class Address extends Entity { + addressId: number; + street: string; + province: string; + @belongsTo(() => AddressBook) + addressBookId: number; + } + + class AddressBook extends Entity { + id: number; + addresses: Address[]; + } + + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + 'addressBookId', + ); + expect(meta).to.eql({ + type: RelationType.belongsTo, + target: () => AddressBook, + keyFrom: 'addressBookId', + }); + }); + + it('accepts explicit keyTo property', () => { + class Address extends Entity { + addressId: number; + street: string; + province: string; + @belongsTo(() => AddressBook, {keyTo: 'somePrimaryKey'}) + addressBookId: number; + } + + class AddressBook extends Entity { + id: number; + addresses: Address[]; + } + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + 'addressBookId', + ); + expect(meta).to.eql({ + type: RelationType.belongsTo, + target: () => AddressBook, + keyFrom: 'addressBookId', + keyTo: 'somePrimaryKey', + }); + }); + }); }); }); diff --git a/packages/repository/test/unit/repositories/constraint-utils.unit.ts b/packages/repository/test/unit/repositories/constraint-utils.unit.ts index 1a970c2aa50c..51b6e9629625 100644 --- a/packages/repository/test/unit/repositories/constraint-utils.unit.ts +++ b/packages/repository/test/unit/repositories/constraint-utils.unit.ts @@ -31,6 +31,15 @@ describe('constraint utility functions', () => { where: Object.assign({}, inputFilter.where, constraint), }); }); + + it('applies a where constraint without a filter to start from', () => { + const constraint = {id: 'a wonderful id'}; + const result = constrainFilter(undefined, constraint); + expect(result).to.containEql({ + where: constraint, + }); + }); + it('applies a filter constraint with where object', () => { const constraint: Filter = {where: {id: '10'}}; const result = constrainFilter(inputFilter, constraint); diff --git a/packages/repository/test/unit/repositories/relation.repository.unit.ts b/packages/repository/test/unit/repositories/relation.repository.unit.ts index c25d0fce5470..080e46a3342f 100644 --- a/packages/repository/test/unit/repositories/relation.repository.unit.ts +++ b/packages/repository/test/unit/repositories/relation.repository.unit.ts @@ -20,6 +20,8 @@ import { HasManyDefinition, RelationType, ModelDefinition, + BelongsToDefinition, + resolveBelongsToMetadata, } from '../../..'; describe('relation repository', () => { @@ -65,6 +67,25 @@ describe('relation repository', () => { }); context('DefaultHasManyEntityCrudRepository', () => { + it('can take in a repository instance as an argument', () => { + const HasManyCrudInstance = givenDefaultHasManyCrudInstance({}); + expect(HasManyCrudInstance.targetRepositoryGetter()).to.eql( + Promise.resolve(repo), + ); + }); + + it('can take in a repository getter as an argument', () => { + repo = sinon.createStubInstance(CustomerRepository); + const repoGetter = () => Promise.resolve(repo); + const HasManyCrudInstance = new DefaultHasManyEntityCrudRepository( + repoGetter, + {}, + ); + expect(HasManyCrudInstance) + .to.have.property('targetRepositoryGetter') + .which.eql(repoGetter); + }); + it('can create related model instance', async () => { const constraint: Partial = {age: 25}; const HasManyCrudInstance = givenDefaultHasManyCrudInstance(constraint); @@ -201,6 +222,88 @@ describe('relation repository', () => { }); }); + context('resolveBelongsToMetadata', () => { + it('retains non-resolver type', () => { + class TargetModel extends Entity {} + const meta: BelongsToDefinition = { + type: RelationType.belongsTo, + target: TargetModel, + }; + const result = resolveBelongsToMetadata(meta); + + expect(result).to.eql(meta); + }); + + it('throws if no target definition metadata is found', () => { + class TargetModel extends Entity { + static definition = new ModelDefinition({ + name: 'TargetModel', + }); + } + const meta: BelongsToDefinition = { + type: RelationType.belongsTo, + target: () => TargetModel, + }; + expect(() => resolveBelongsToMetadata(meta)).to.throw( + /no id metadata found/, + ); + }); + + it('throws if no belongsTo metadata is found', () => { + class TargetModel extends Entity { + static definition = new ModelDefinition({ + name: 'TargetModel', + properties: { + propertyThatIsNotId: {type: 'number'}, + }, + }); + } + const meta: BelongsToDefinition = { + type: RelationType.belongsTo, + target: () => TargetModel, + }; + expect(() => resolveBelongsToMetadata(meta)).to.throw( + /no id metadata found/, + ); + }); + + it('retains predefined keyTo property', () => { + class TargetModel extends Entity {} + const meta: BelongsToDefinition = { + type: RelationType.belongsTo, + target: () => TargetModel, + keyTo: 'someOtherForeignId', + }; + const result = resolveBelongsToMetadata(meta); + const expected: BelongsToDefinition = { + type: RelationType.belongsTo, + target: () => TargetModel, + keyTo: 'someOtherForeignId', + }; + expect(result).to.eql(expected); + }); + + it('infers keyTo from property decorated with @property({id: true}) on target model', () => { + class TargetModel extends Entity { + static definition = new ModelDefinition({ + name: 'TargetModel', + properties: {anId: {type: 'number', id: true}}, + }); + } + const meta: BelongsToDefinition = { + type: RelationType.belongsTo, + target: () => TargetModel, + }; + const result = resolveBelongsToMetadata(meta); + const expected: BelongsToDefinition = { + type: RelationType.belongsTo, + target: () => TargetModel, + keyTo: 'anId', + }; + expect(result).to.eql(expected); + }); + }); + /*------------- HELPERS ---------------*/ class Customer extends Entity { From f0230ced89200cc3d511f767c3f17fd423b2bdd2 Mon Sep 17 00:00:00 2001 From: shimks Date: Thu, 23 Aug 2018 23:43:53 -0400 Subject: [PATCH 4/6] fixup! apply feedback --- docs/site/HasMany-relation.md | 8 +-- docs/site/todo-list-tutorial-repository.md | 7 +-- .../src/repositories/todo-list.repository.ts | 7 +-- .../src/decorators/model.decorator.ts | 16 ++++-- .../src/decorators/relation.decorator.ts | 20 ++++++-- .../src/decorators/repository.decorator.ts | 9 +++- packages/repository/src/model.ts | 3 ++ .../src/repositories/legacy-juggler-bridge.ts | 8 +-- .../src/repositories/relation.factory.ts | 9 ++-- .../src/repositories/relation.repository.ts | 49 +++++++------------ .../belongs-to.relation.acceptance.ts | 2 +- ...has-many-without-di.relation.acceptance.ts | 9 ++-- .../has-many.relation.acceptance.ts | 6 +-- .../{cyclic-x.model.ts => category.model.ts} | 10 ++-- .../{cyclic-y.model.ts => product.model.ts} | 10 ++-- .../repositories/customer.repository.ts | 3 +- .../fixtures/repositories/order.repository.ts | 9 +++- .../relation.factory.integration.ts | 40 +++++++++------ packages/repository/test/test-utils.ts | 3 ++ .../model-and-relation.decorator.unit.ts | 39 +++++++-------- .../repositories/relation.repository.unit.ts | 23 ++------- 21 files changed, 147 insertions(+), 143 deletions(-) rename packages/repository/test/fixtures/models/bad/{cyclic-x.model.ts => category.model.ts} (54%) rename packages/repository/test/fixtures/models/bad/{cyclic-y.model.ts => product.model.ts} (54%) create mode 100644 packages/repository/test/test-utils.ts diff --git a/docs/site/HasMany-relation.md b/docs/site/HasMany-relation.md index 8fecec727def..23abf5631a62 100644 --- a/docs/site/HasMany-relation.md +++ b/docs/site/HasMany-relation.md @@ -99,6 +99,7 @@ repository, the following are required: - Use [Dependency Injection](Dependency-injection.md) to inject an instance of the target repository in the constructor of your source repository class. + - Declare a property with the factory function type `HasManyRepositoryFactory` on the source repository class. @@ -121,7 +122,7 @@ import { HasManyRepositoryFactory, repository, } from '@loopback/repository'; -import {inject} from '@loopback/core'; +import {inject, Getter} from '@loopback/core'; class CustomerRepository extends DefaultCrudRepository< Customer, @@ -130,12 +131,13 @@ class CustomerRepository extends DefaultCrudRepository< public orders: HasManyRepositoryFactory; constructor( @inject('datasources.db') protected db: juggler.DataSource, - @repository(OrderRepository) orderRepository: OrderRepository, + @repository.getter('repositories.OrderRepository') + getOrderRepository: Getter, ) { super(Customer, db); this.orders = this._createHasManyRepositoryFactoryFor( 'orders', - orderRepository, + getOrderRepository, ); } } diff --git a/docs/site/todo-list-tutorial-repository.md b/docs/site/todo-list-tutorial-repository.md index c9e6769bdafe..da733a8e65a3 100644 --- a/docs/site/todo-list-tutorial-repository.md +++ b/docs/site/todo-list-tutorial-repository.md @@ -49,7 +49,7 @@ import { repository, } from '@loopback/repository'; import {TodoList, Todo} from '../models'; -import {inject} from '@loopback/core'; +import {inject, Getter} from '@loopback/core'; import {TodoRepository} from './todo.repository'; export class TodoListRepository extends DefaultCrudRepository< @@ -60,12 +60,13 @@ export class TodoListRepository extends DefaultCrudRepository< constructor( @inject('datasources.db') protected datasource: juggler.DataSource, - @repository(TodoRepository) protected todoRepository: TodoRepository, + @repository.getter('repositories.TodoRepository') + protected getTodoRepository: Getter, ) { super(TodoList, datasource); this.todos = this._createHasManyRepositoryFactoryFor( 'todos', - todoRepository, + getTodoRepository, ); } } diff --git a/examples/todo-list/src/repositories/todo-list.repository.ts b/examples/todo-list/src/repositories/todo-list.repository.ts index 7fb84eda2623..1df05aee2f4d 100644 --- a/examples/todo-list/src/repositories/todo-list.repository.ts +++ b/examples/todo-list/src/repositories/todo-list.repository.ts @@ -10,7 +10,7 @@ import { repository, } from '@loopback/repository'; import {TodoList, Todo} from '../models'; -import {inject} from '@loopback/core'; +import {inject, Getter} from '@loopback/core'; import {TodoRepository} from './todo.repository'; export class TodoListRepository extends DefaultCrudRepository< @@ -21,12 +21,13 @@ export class TodoListRepository extends DefaultCrudRepository< constructor( @inject('datasources.db') protected datasource: juggler.DataSource, - @repository(TodoRepository) protected todoRepository: TodoRepository, + @repository.getter('repositories.TodoRepository') + protected getTodoRepository: Getter, ) { super(TodoList, datasource); this.todos = this._createHasManyRepositoryFactoryFor( 'todos', - todoRepository, + getTodoRepository, ); } } diff --git a/packages/repository/src/decorators/model.decorator.ts b/packages/repository/src/decorators/model.decorator.ts index 77981836e5c1..79c74fc18fc4 100644 --- a/packages/repository/src/decorators/model.decorator.ts +++ b/packages/repository/src/decorators/model.decorator.ts @@ -15,6 +15,7 @@ import { ModelDefinitionSyntax, PropertyDefinition, TypeResolver, + ERR_TARGET_UNDEFINED, } from '../model'; import {RELATIONS_KEY, RelationDefinitionBase} from './relation.decorator'; @@ -92,15 +93,20 @@ export function model(definition?: Partial) { * @returns {(target:any, key:string)} */ export function property(definition?: Partial) { - if ( + const isCyclic = + definition && + (definition as Object).hasOwnProperty('type') && + !definition.type; + const isCyclicArray = definition && (definition.type === Array || definition.type === 'array') && (definition as object).hasOwnProperty('itemType') && - !definition.itemType - ) { + !definition.itemType; + + if (isCyclic || isCyclicArray) { // this path is taken when cyclic dependency is detected - // in that case, a ModelResolver should be used instead - throw new Error('target model is undefined'); + // in that case, a TypeResolver should be used instead + throw new Error(ERR_TARGET_UNDEFINED); } return PropertyDecoratorFactory.createDecorator( MODEL_PROPERTIES_KEY, diff --git a/packages/repository/src/decorators/relation.decorator.ts b/packages/repository/src/decorators/relation.decorator.ts index 82394b741864..77c6f7afac1c 100644 --- a/packages/repository/src/decorators/relation.decorator.ts +++ b/packages/repository/src/decorators/relation.decorator.ts @@ -4,7 +4,12 @@ // License text available at https://opensource.org/licenses/MIT import {Class} from '../common-types'; -import {Entity, TypeResolver, isTypeResolver} from '../model'; +import { + Entity, + TypeResolver, + isTypeResolver, + ERR_TARGET_UNDEFINED, +} from '../model'; import {PropertyDecoratorFactory, MetadataInspector} from '@loopback/context'; import {property} from './model.decorator'; import {camelCase} from 'lodash'; @@ -64,6 +69,13 @@ export function belongsTo( targetModel: T | TypeResolver, definition?: Partial, ) { + const defIsCyclic = + definition && + (definition as Object).hasOwnProperty('target') && + !definition.target; + if (!targetModel || defIsCyclic) { + throw new Error(ERR_TARGET_UNDEFINED); + } return function(target: Object, key: string) { const propMeta = { type: MetadataInspector.getDesignTypeForProperty(target, key), @@ -90,9 +102,7 @@ export function belongsTo( if (!hasPrimaryKey && !hasKeyTo) { throw new Error( - `primary key not found on ${ - targetModel.name - } model's juggler definition`, + `primary key not found on ${targetModel.name} model's definition`, ); } } @@ -145,7 +155,7 @@ export function hasMany( throw new Error( `foreign key ${defaultFkName} not found on ${ targetModel.name - } model's juggler definition`, + } model's definition`, ); } Object.assign(meta, {keyTo: defaultFkName}); diff --git a/packages/repository/src/decorators/repository.decorator.ts b/packages/repository/src/decorators/repository.decorator.ts index e2496a66214f..3a0b152a30b4 100644 --- a/packages/repository/src/decorators/repository.decorator.ts +++ b/packages/repository/src/decorators/repository.decorator.ts @@ -8,7 +8,7 @@ import {Model, Entity} from '../model'; import {Repository, DefaultCrudRepository} from '../repositories'; import {DataSource} from '../datasource'; import {juggler} from '../repositories/legacy-juggler-bridge'; -import {inject, Context, Injection} from '@loopback/context'; +import {inject, Context, Injection, BindingAddress} from '@loopback/context'; import {Class} from '../common-types'; /** @@ -172,6 +172,13 @@ export function repository( }; } +export namespace repository { + // tslint:disable-next-line:no-any + export function getter(bindingKey: BindingAddress) { + return inject.getter(bindingKey); + } +} + /** * Resolve the @repository injection * @param ctx Context diff --git a/packages/repository/src/model.ts b/packages/repository/src/model.ts index 0c3aaf03c336..9c3740797e42 100644 --- a/packages/repository/src/model.ts +++ b/packages/repository/src/model.ts @@ -288,6 +288,9 @@ export function resolveType(fn: TypeResolver | T) { return isTypeResolver(fn) ? fn() : fn; } +export const ERR_TARGET_UNDEFINED = + 'Target model is undefined. Please consider using TypeResolver (() => TargetModel)'; + /** * Domain events */ diff --git a/packages/repository/src/repositories/legacy-juggler-bridge.ts b/packages/repository/src/repositories/legacy-juggler-bridge.ts index b537df1c3224..8d0ea4fad69b 100644 --- a/packages/repository/src/repositories/legacy-juggler-bridge.ts +++ b/packages/repository/src/repositories/legacy-juggler-bridge.ts @@ -170,9 +170,7 @@ export class DefaultCrudRepository ForeignKeyType >( relationName: string, - targetRepo: - | EntityCrudRepository - | Getter>, + targetRepo: Getter>, ): HasManyRepositoryFactory { const meta = this.entityClass.definition.relations[relationName]; return createHasManyRepositoryFactory( @@ -187,9 +185,7 @@ export class DefaultCrudRepository Source extends Entity >( relationName: string, - targetRepo: - | EntityCrudRepository - | Getter>, + targetRepo: Getter>, ): BelongsToFactory { const meta = this.entityClass.definition.relations[relationName]; return createBelongsToFactory(meta as BelongsToDefinition, targetRepo); diff --git a/packages/repository/src/repositories/relation.factory.ts b/packages/repository/src/repositories/relation.factory.ts index c6a623f3e600..be8f46ae069e 100644 --- a/packages/repository/src/repositories/relation.factory.ts +++ b/packages/repository/src/repositories/relation.factory.ts @@ -50,9 +50,7 @@ export function createHasManyRepositoryFactory< ForeignKeyType >( relationMetadata: HasManyDefinition, - targetRepository: - | EntityCrudRepository - | Getter>, + targetRepository: Getter>, ): HasManyRepositoryFactory { resolveHasManyMetadata(relationMetadata); debug('resolved relation metadata: %o', relationMetadata); @@ -77,9 +75,7 @@ export function createBelongsToFactory< Source extends Entity >( belongsToMetadata: BelongsToDefinition, - targetRepository: - | EntityCrudRepository - | Getter>, + targetRepository: Getter>, ): BelongsToFactory { resolveBelongsToMetadata(belongsToMetadata); const foreignKey = belongsToMetadata.keyFrom; @@ -92,6 +88,7 @@ export function createBelongsToFactory< if (!primaryKey) { throw new Error('The primary key property name (keyTo) must be specified'); } + // NOTE change this to accept ID return async function(sourceModel: Source) { const foreignKeyValue = sourceModel[foreignKey as keyof Source]; // tslint:disable-next-line:no-any diff --git a/packages/repository/src/repositories/relation.repository.ts b/packages/repository/src/repositories/relation.repository.ts index 541567f4319a..54753da34266 100644 --- a/packages/repository/src/repositories/relation.repository.ts +++ b/packages/repository/src/repositories/relation.repository.ts @@ -72,39 +72,29 @@ export class DefaultHasManyEntityCrudRepository< * the target repository instance */ - public targetRepositoryGetter: Getter; - constructor( - targetRepository: TargetRepository | Getter, + public getTargetRepository: Getter, public constraint: DataObject, - ) { - this.targetRepositoryGetter = - typeof targetRepository === 'object' - ? ((() => Promise.resolve(targetRepository)) as Getter< - TargetRepository - >) - : targetRepository; - } - + ) {} async create( targetModelData: DataObject, options?: Options, ): Promise { - return (await this.targetRepositoryGetter()).create( + const targetRepo = await this.getTargetRepository(); + return targetRepo.create( constrainDataObject(targetModelData, this.constraint), options, ); } async find(filter?: Filter, options?: Options): Promise { - return (await this.targetRepositoryGetter()).find( - constrainFilter(filter, this.constraint), - options, - ); + const targetRepo = await this.getTargetRepository(); + return targetRepo.find(constrainFilter(filter, this.constraint), options); } async delete(where?: Where, options?: Options): Promise { - return (await this.targetRepositoryGetter()).deleteAll( + const targetRepo = await this.getTargetRepository(); + return targetRepo.deleteAll( constrainWhere(where, this.constraint), options, ); @@ -115,7 +105,8 @@ export class DefaultHasManyEntityCrudRepository< where?: Where, options?: Options, ): Promise { - return (await this.targetRepositoryGetter()).updateAll( + const targetRepo = await this.getTargetRepository(); + return targetRepo.updateAll( constrainDataObject(dataObject, this.constraint), constrainWhere(where, this.constraint), options, @@ -127,23 +118,17 @@ export class DefaultBelongsToEntityCrudRepository< TargetEntity extends Entity, TargetId, TargetRepository extends EntityCrudRepository -> { - public targetRepositoryGetter: Getter; - +> implements BelongsToRepository { constructor( - targetRepository: TargetRepository | Getter, + public getTargetRepository: Getter, public constraint: DataObject, - ) { - this.targetRepositoryGetter = - typeof targetRepository === 'object' - ? () => Promise.resolve(targetRepository) - : targetRepository; - } - + ) {} async find(options?: Options): Promise { - return (await (await this.targetRepositoryGetter()).find( + const targetRepo = await this.getTargetRepository(); + const result = await targetRepo.find( constrainFilter(undefined, this.constraint), options, - ))[0]; + ); + return result[0]; } } diff --git a/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts index f94a9a88f587..a2ec93ac1d4e 100644 --- a/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts +++ b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts @@ -20,7 +20,7 @@ describe('HasMany relation', () => { before(givenBoundCrudRepositoriesForCustomerAndOrder); before(givenOrderController); - afterEach(async () => { + beforeEach(async () => { await orderRepo.deleteAll(); }); diff --git a/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts b/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts index a5e05aa77657..6ddc5f52239c 100644 --- a/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts +++ b/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts @@ -14,6 +14,7 @@ import { EntityCrudRepository, } from '../..'; import {expect} from '@loopback/testlab'; +import {createGetter} from '../test-utils'; describe('HasMany relation', () => { // Given a Customer and Order models - see definitions at the bottom @@ -27,11 +28,11 @@ describe('HasMany relation', () => { before(givenOrderRepository); before(givenCustomerRepository); beforeEach(async () => { - existingCustomerId = (await givenPersistedCustomerInstance()).id; - }); - afterEach(async () => { await orderRepo.deleteAll(); }); + beforeEach(async () => { + existingCustomerId = (await givenPersistedCustomerInstance()).id; + }); it('can create an instance of the related model', async () => { async function createCustomerOrders( @@ -145,7 +146,7 @@ describe('HasMany relation', () => { super(Customer, db); this.orders = this._createHasManyRepositoryFactoryFor( 'orders', - orderRepository, + createGetter(orderRepository), ); } } diff --git a/packages/repository/test/acceptance/has-many.relation.acceptance.ts b/packages/repository/test/acceptance/has-many.relation.acceptance.ts index c996ce2d08a1..d9faf9f721a5 100644 --- a/packages/repository/test/acceptance/has-many.relation.acceptance.ts +++ b/packages/repository/test/acceptance/has-many.relation.acceptance.ts @@ -24,11 +24,11 @@ describe('HasMany relation', () => { before(givenCustomerController); beforeEach(async () => { - existingCustomerId = (await givenPersistedCustomerInstance()).id; - }); - afterEach(async () => { await orderRepo.deleteAll(); }); + beforeEach(async () => { + existingCustomerId = (await givenPersistedCustomerInstance()).id; + }); it('can create an instance of the related model', async () => { const order = await controller.createCustomerOrders(existingCustomerId, { diff --git a/packages/repository/test/fixtures/models/bad/cyclic-x.model.ts b/packages/repository/test/fixtures/models/bad/category.model.ts similarity index 54% rename from packages/repository/test/fixtures/models/bad/cyclic-x.model.ts rename to packages/repository/test/fixtures/models/bad/category.model.ts index 7336b04ac50c..82e065a3bbc4 100644 --- a/packages/repository/test/fixtures/models/bad/cyclic-x.model.ts +++ b/packages/repository/test/fixtures/models/bad/category.model.ts @@ -3,10 +3,10 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {property} from '../../../..'; -import {BadCyclicY} from './cyclic-y.model'; +import {hasMany, Entity} from '../../../..'; +import {Product} from './product.model'; -export class BadCyclicX { - @property.array(BadCyclicY) - cyclicProp: BadCyclicY[]; +export class Category extends Entity { + @hasMany(Product) + products: Product[]; } diff --git a/packages/repository/test/fixtures/models/bad/cyclic-y.model.ts b/packages/repository/test/fixtures/models/bad/product.model.ts similarity index 54% rename from packages/repository/test/fixtures/models/bad/cyclic-y.model.ts rename to packages/repository/test/fixtures/models/bad/product.model.ts index 34c7dbb52ecb..eabe231c37a8 100644 --- a/packages/repository/test/fixtures/models/bad/cyclic-y.model.ts +++ b/packages/repository/test/fixtures/models/bad/product.model.ts @@ -3,10 +3,10 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {property} from '../../../..'; -import {BadCyclicX} from './cyclic-x.model'; +import {belongsTo, Entity} from '../../../..'; +import {Category} from './category.model'; -export class BadCyclicY { - @property.array(BadCyclicX) - cyclicProp: BadCyclicX[]; +export class Product extends Entity { + @belongsTo(Category) + categoryId: number; } diff --git a/packages/repository/test/fixtures/repositories/customer.repository.ts b/packages/repository/test/fixtures/repositories/customer.repository.ts index f7ae4afaa351..4dfa9df6a359 100644 --- a/packages/repository/test/fixtures/repositories/customer.repository.ts +++ b/packages/repository/test/fixtures/repositories/customer.repository.ts @@ -4,6 +4,7 @@ import { DefaultCrudRepository, HasManyRepositoryFactory, juggler, + repository, } from '../../..'; import {inject, Getter} from '@loopback/context'; @@ -14,7 +15,7 @@ export class CustomerRepository extends DefaultCrudRepository< public orders: HasManyRepositoryFactory; constructor( @inject('datasources.db') protected db: juggler.DataSource, - @inject.getter('repositories.OrderRepository') + @repository.getter('repositories.OrderRepository') orderRepositoryGetter: Getter, ) { super(Customer, db); diff --git a/packages/repository/test/fixtures/repositories/order.repository.ts b/packages/repository/test/fixtures/repositories/order.repository.ts index 449de8797f3b..d024916d4861 100644 --- a/packages/repository/test/fixtures/repositories/order.repository.ts +++ b/packages/repository/test/fixtures/repositories/order.repository.ts @@ -1,6 +1,11 @@ import {Order, Customer} from '../models'; import {CustomerRepository} from '../repositories'; -import {DefaultCrudRepository, juggler, BelongsToFactory} from '../../..'; +import { + DefaultCrudRepository, + juggler, + BelongsToFactory, + repository, +} from '../../..'; import {inject, Getter} from '@loopback/context'; export class OrderRepository extends DefaultCrudRepository< @@ -10,7 +15,7 @@ export class OrderRepository extends DefaultCrudRepository< public customer: BelongsToFactory; constructor( @inject('datasources.db') protected db: juggler.DataSource, - @inject.getter('repositories.CustomerRepository') + @repository.getter('repositories.CustomerRepository') customerRepositoryGetter: Getter, ) { super(Order, db); diff --git a/packages/repository/test/integration/repositories/relation.factory.integration.ts b/packages/repository/test/integration/repositories/relation.factory.integration.ts index 4c1c68f811d2..e6b7ecb48ef0 100644 --- a/packages/repository/test/integration/repositories/relation.factory.integration.ts +++ b/packages/repository/test/integration/repositories/relation.factory.integration.ts @@ -22,6 +22,7 @@ import { BelongsToDefinition, } from '../../..'; import {expect} from '@loopback/testlab'; +import {createGetter} from '../../test-utils'; // Given a Customer and Order models - see definitions at the bottom let db: juggler.DataSource; @@ -120,14 +121,14 @@ describe('HasMany relation', () => { }); context('createHasManyRepositoryFactory', () => { - it('errors when keyTo is not available hasMany metadata', () => { + it('errors when keyTo is not available in hasMany metadata', () => { class SomeClass extends Entity {} const keytolessMeta: HasManyDefinition = { type: RelationType.hasMany, target: SomeClass, }; expect(() => - createHasManyRepositoryFactory(keytolessMeta, reviewRepo), + createHasManyRepositoryFactory(keytolessMeta, createGetter(reviewRepo)), ).to.throw(/The foreign key property name \(keyTo\) must be specified/); }); @@ -155,9 +156,11 @@ describe('HasMany relation', () => { }); createHasManyRepositoryFactory( hasManyMeta, - new DefaultCrudRepository( - Suite, - new juggler.DataSource({connector: 'memory'}), + createGetter( + new DefaultCrudRepository( + Suite, + new juggler.DataSource({connector: 'memory'}), + ), ), ); expect(hasManyMeta).to.eql({ @@ -173,7 +176,7 @@ describe('belongsTo relation', () => { it('can find an instance of the related model', async () => { const findCustomerOfOrder = createBelongsToFactory( Order.definition.relations.customerId as BelongsToDefinition, - customerRepo, + createGetter(customerRepo), ); const customer = await customerRepo.create({name: 'Order McForder'}); @@ -194,7 +197,7 @@ describe('belongsTo relation', () => { keyTo: 'someKey', }; expect(() => - createBelongsToFactory(keyFromLessMeta, reviewRepo), + createBelongsToFactory(keyFromLessMeta, createGetter(reviewRepo)), ).to.throw(/The foreign key property name \(keyFrom\) must be specified/); }); @@ -205,9 +208,9 @@ describe('belongsTo relation', () => { target: SomeClass, keyFrom: 'someKey', }; - expect(() => createBelongsToFactory(keyToLessMeta, reviewRepo)).to.throw( - /The primary key property name \(keyTo\) must be specified/, - ); + expect(() => + createBelongsToFactory(keyToLessMeta, createGetter(reviewRepo)), + ).to.throw(/The primary key property name \(keyTo\) must be specified/); }); it('resolves property id metadata', () => { @@ -235,9 +238,11 @@ describe('belongsTo relation', () => { }); createBelongsToFactory( belongsToMeta, - new DefaultCrudRepository( - Suite, - new juggler.DataSource({connector: 'memory'}), + createGetter( + new DefaultCrudRepository( + Suite, + new juggler.DataSource({connector: 'memory'}), + ), ), ); expect(belongsToMeta).to.eql({ @@ -345,7 +350,10 @@ function givenConstrainedRepositories() { Order, typeof Order.prototype.id, typeof Customer.prototype.id - >(Customer.definition.relations.orders as HasManyDefinition, orderRepo); + >( + Customer.definition.relations.orders as HasManyDefinition, + createGetter(orderRepo), + ); customerOrderRepo = orderFactoryFn(existingCustomerId); } @@ -353,10 +361,10 @@ function givenConstrainedRepositories() { function givenRepositoryFactoryFunctions() { customerAuthoredReviewFactoryFn = createHasManyRepositoryFactory( Customer.definition.relations.reviewsAuthored as HasManyDefinition, - reviewRepo, + createGetter(reviewRepo), ); customerApprovedReviewFactoryFn = createHasManyRepositoryFactory( Customer.definition.relations.reviewsApproved as HasManyDefinition, - reviewRepo, + createGetter(reviewRepo), ); } diff --git a/packages/repository/test/test-utils.ts b/packages/repository/test/test-utils.ts new file mode 100644 index 000000000000..ff38ee0c7704 --- /dev/null +++ b/packages/repository/test/test-utils.ts @@ -0,0 +1,3 @@ +export function createGetter(value: T) { + return () => Promise.resolve(value); +} diff --git a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts index 8ad477ec10bf..5f0bc6352ed8 100644 --- a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts +++ b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts @@ -21,6 +21,7 @@ import { RelationType, Entity, ValueObject, + ERR_TARGET_UNDEFINED, } from '../../../'; import {MetadataInspector} from '@loopback/context'; @@ -319,6 +320,15 @@ describe('model decorator', () => { expect(House.definition.relations).to.containEql({person: relationMeta}); }); + it('throws when hasMany and belongsTo relations are cyclic', () => { + expect(() => { + require('../../fixtures/models/bad/category.model'); + }).to.throw(ERR_TARGET_UNDEFINED); + expect(() => { + require('../../fixtures/models/bad/product.model'); + }).to.throw(ERR_TARGET_UNDEFINED); + }); + describe('property namespace', () => { describe('array', () => { it('adds array metadata', () => { @@ -363,28 +373,13 @@ describe('model decorator', () => { }); it('throws when used on a non-array property', () => { - expect.throws( - () => { - // tslint:disable-next-line:no-unused-variable - class Oops { - @property.array(Product) - product: Product; - } - }, - Error, - property.ERR_PROP_NOT_ARRAY, - ); - }); - - it('throws when properties are cyclic', () => { - expect.throws( - () => { - require('../../fixtures/models/bad/cyclic-x.model'); - require('../../fixtures/models/bad/cyclic-y.model'); - }, - Error, - 'model is undefined', - ); + expect(() => { + // tslint:disable-next-line:no-unused-variable + class Oops { + @property.array(Product) + product: Product; + } + }).to.throw(property.ERR_PROP_NOT_ARRAY); }); }); }); diff --git a/packages/repository/test/unit/repositories/relation.repository.unit.ts b/packages/repository/test/unit/repositories/relation.repository.unit.ts index 080e46a3342f..38a7d1a92d0b 100644 --- a/packages/repository/test/unit/repositories/relation.repository.unit.ts +++ b/packages/repository/test/unit/repositories/relation.repository.unit.ts @@ -23,6 +23,7 @@ import { BelongsToDefinition, resolveBelongsToMetadata, } from '../../..'; +import {createGetter} from '../../test-utils'; describe('relation repository', () => { context('HasManyRepository interface', () => { @@ -67,25 +68,6 @@ describe('relation repository', () => { }); context('DefaultHasManyEntityCrudRepository', () => { - it('can take in a repository instance as an argument', () => { - const HasManyCrudInstance = givenDefaultHasManyCrudInstance({}); - expect(HasManyCrudInstance.targetRepositoryGetter()).to.eql( - Promise.resolve(repo), - ); - }); - - it('can take in a repository getter as an argument', () => { - repo = sinon.createStubInstance(CustomerRepository); - const repoGetter = () => Promise.resolve(repo); - const HasManyCrudInstance = new DefaultHasManyEntityCrudRepository( - repoGetter, - {}, - ); - expect(HasManyCrudInstance) - .to.have.property('targetRepositoryGetter') - .which.eql(repoGetter); - }); - it('can create related model instance', async () => { const constraint: Partial = {age: 25}; const HasManyCrudInstance = givenDefaultHasManyCrudInstance(constraint); @@ -326,10 +308,11 @@ describe('relation repository', () => { function givenDefaultHasManyCrudInstance(constraint: DataObject) { repo = sinon.createStubInstance(CustomerRepository); + const repoGetter = createGetter(repo); return new DefaultHasManyEntityCrudRepository< Customer, typeof Customer.prototype.id, CustomerRepository - >(repo, constraint); + >(repoGetter, constraint); } }); From ec84a0da56b59f8c2d954f685836db79a8790264 Mon Sep 17 00:00:00 2001 From: shimks Date: Sun, 26 Aug 2018 22:23:24 -0400 Subject: [PATCH 5/6] fixup! change belongs factory function to take in the source id --- .../src/decorators/relation.decorator.ts | 4 ++-- .../src/decorators/repository.decorator.ts | 6 +++++ .../src/repositories/legacy-juggler-bridge.ts | 23 +++++++++--------- .../src/repositories/relation.factory.ts | 24 ++++++++++++++----- .../src/repositories/relation.repository.ts | 20 ++++++++++++---- .../belongs-to.relation.acceptance.ts | 3 +-- .../test/fixtures/models/customer.model.ts | 5 ++++ .../repository/test/fixtures/models/index.ts | 5 ++++ .../test/fixtures/models/order.model.ts | 5 ++++ .../repositories/customer.repository.ts | 5 ++++ .../test/fixtures/repositories/index.ts | 5 ++++ .../fixtures/repositories/order.repository.ts | 9 +++++-- .../relation.factory.integration.ts | 19 ++++++++++++--- packages/repository/test/test-utils.ts | 5 ++++ 14 files changed, 107 insertions(+), 31 deletions(-) diff --git a/packages/repository/src/decorators/relation.decorator.ts b/packages/repository/src/decorators/relation.decorator.ts index 77c6f7afac1c..12c348f69170 100644 --- a/packages/repository/src/decorators/relation.decorator.ts +++ b/packages/repository/src/decorators/relation.decorator.ts @@ -109,7 +109,7 @@ export function belongsTo( // Apply model definition to the model class Object.assign(rel, definition); - PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel)(target, key); + relation(rel)(target, key); }; } @@ -163,7 +163,7 @@ export function hasMany( Object.assign(meta, definition, {type: RelationType.hasMany}); - PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, meta)(target, key); + relation(meta)(target, key); }; } diff --git a/packages/repository/src/decorators/repository.decorator.ts b/packages/repository/src/decorators/repository.decorator.ts index 3a0b152a30b4..8d5daaff88ea 100644 --- a/packages/repository/src/decorators/repository.decorator.ts +++ b/packages/repository/src/decorators/repository.decorator.ts @@ -173,6 +173,12 @@ export function repository( } export namespace repository { + /** + * Decorator used to inject a Getter for a repository + * Mainly intended for usage with repository injections on relation repository + * factory + * @param bindingKey + */ // tslint:disable-next-line:no-any export function getter(bindingKey: BindingAddress) { return inject.getter(bindingKey); diff --git a/packages/repository/src/repositories/legacy-juggler-bridge.ts b/packages/repository/src/repositories/legacy-juggler-bridge.ts index 8d0ea4fad69b..49812450fbff 100644 --- a/packages/repository/src/repositories/legacy-juggler-bridge.ts +++ b/packages/repository/src/repositories/legacy-juggler-bridge.ts @@ -164,31 +164,30 @@ export class DefaultCrudRepository * @param relationName Name of the relation defined on the source model * @param targetRepo Target repository instance */ - protected _createHasManyRepositoryFactoryFor< - Target extends Entity, - TargetID, - ForeignKeyType - >( + protected _createHasManyRepositoryFactoryFor( relationName: string, targetRepo: Getter>, - ): HasManyRepositoryFactory { + ): HasManyRepositoryFactory { const meta = this.entityClass.definition.relations[relationName]; - return createHasManyRepositoryFactory( + return createHasManyRepositoryFactory( meta as HasManyDefinition, targetRepo, ); } - protected _createBelongsToFactoryFor< + protected _createBelongsToRepositoryFactoryFor< Target extends Entity, - TargetId, - Source extends Entity + TargetId >( relationName: string, targetRepo: Getter>, - ): BelongsToFactory { + ): BelongsToFactory { const meta = this.entityClass.definition.relations[relationName]; - return createBelongsToFactory(meta as BelongsToDefinition, targetRepo); + return createBelongsToFactory( + meta as BelongsToDefinition, + targetRepo, + this, + ); } async create(entity: DataObject, options?: Options): Promise { diff --git a/packages/repository/src/repositories/relation.factory.ts b/packages/repository/src/repositories/relation.factory.ts index be8f46ae069e..799e794d179a 100644 --- a/packages/repository/src/repositories/relation.factory.ts +++ b/packages/repository/src/repositories/relation.factory.ts @@ -27,8 +27,8 @@ export type HasManyRepositoryFactory = ( fkValue: ForeignKeyType, ) => HasManyRepository; -export type BelongsToFactory = ( - sourceModel: Source, +export type BelongsToFactory = ( + sourceId: SourceId, ) => Promise; /** @@ -69,14 +69,19 @@ export function createHasManyRepositoryFactory< }; } +/** + * Enforces a BelongsTo constraint on a repository + */ export function createBelongsToFactory< Target extends Entity, TargetId, - Source extends Entity + Source extends Entity, + SourceId >( belongsToMetadata: BelongsToDefinition, targetRepository: Getter>, -): BelongsToFactory { + sourceRepository: EntityCrudRepository, +): BelongsToFactory { resolveBelongsToMetadata(belongsToMetadata); const foreignKey = belongsToMetadata.keyFrom; const primaryKey = belongsToMetadata.keyTo; @@ -89,7 +94,8 @@ export function createBelongsToFactory< throw new Error('The primary key property name (keyTo) must be specified'); } // NOTE change this to accept ID - return async function(sourceModel: Source) { + return async function(sourceId: SourceId) { + const sourceModel = await sourceRepository.findById(sourceId); const foreignKeyValue = sourceModel[foreignKey as keyof Source]; // tslint:disable-next-line:no-any const constraint: any = {[primaryKey]: foreignKeyValue}; @@ -97,7 +103,7 @@ export function createBelongsToFactory< targetRepository, constraint as DataObject, ); - return constrainedRepo.find(); + return constrainedRepo.get(); }; } @@ -142,6 +148,12 @@ export function resolveHasManyMetadata(relationMeta: HasManyDefinition) { return relationMeta; } +/** + * Resolves given belongsTo metadata if target is specified to be a resolver. + * Mainly used to infer what the `keyTo` property should be from the target's + * property id metadata + * @param relationMeta belongsTo metadata to resolve + */ export function resolveBelongsToMetadata(relationMeta: BelongsToDefinition) { if ( relationMeta.target && diff --git a/packages/repository/src/repositories/relation.repository.ts b/packages/repository/src/repositories/relation.repository.ts index 54753da34266..f3ca65f03c1d 100644 --- a/packages/repository/src/repositories/relation.repository.ts +++ b/packages/repository/src/repositories/relation.repository.ts @@ -56,8 +56,15 @@ export interface HasManyRepository { ): Promise; } +/** + * CRUD operations for a target repository of a BelongsTo relation + */ export interface BelongsToRepository { - find(options?: Options): Promise; + /** + * Gets the target model instance + * @param options + */ + get(options?: Options): Promise; } export class DefaultHasManyEntityCrudRepository< @@ -67,11 +74,10 @@ export class DefaultHasManyEntityCrudRepository< > implements HasManyRepository { /** * Constructor of DefaultHasManyEntityCrudRepository - * @param targetRepositoryGetter the related target model repository instance + * @param getTargetRepository the related target model repository instance * @param constraint the key value pair representing foreign key name to constrain * the target repository instance */ - constructor( public getTargetRepository: Getter, public constraint: DataObject, @@ -119,11 +125,17 @@ export class DefaultBelongsToEntityCrudRepository< TargetId, TargetRepository extends EntityCrudRepository > implements BelongsToRepository { + /** + * Constructor of DefaultBelongsToEntityCrudRepository + * @param getTargetRepository the related target model repository instance + * @param constraint the key value pair representing foreign key name to constrain + * the target repository instance + */ constructor( public getTargetRepository: Getter, public constraint: DataObject, ) {} - async find(options?: Options): Promise { + async get(options?: Options): Promise { const targetRepo = await this.getTargetRepository(); const result = await targetRepo.find( constrainFilter(undefined, this.constraint), diff --git a/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts index a2ec93ac1d4e..ca53d5e65338 100644 --- a/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts +++ b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts @@ -42,8 +42,7 @@ describe('HasMany relation', () => { ) {} async findOwnerOfOrder(orderId: number) { - const order = await this.orderRepository.findById(orderId); - return await this.orderRepository.customer(order); + return await this.orderRepository.customer(orderId); } } diff --git a/packages/repository/test/fixtures/models/customer.model.ts b/packages/repository/test/fixtures/models/customer.model.ts index 8c49456ad3d3..2523e210219f 100644 --- a/packages/repository/test/fixtures/models/customer.model.ts +++ b/packages/repository/test/fixtures/models/customer.model.ts @@ -1,3 +1,8 @@ +// Copyright IBM Corp. 2018. 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 {model, property, hasMany, Entity} from '../../..'; import {Order} from './order.model'; diff --git a/packages/repository/test/fixtures/models/index.ts b/packages/repository/test/fixtures/models/index.ts index f7706bb383b9..b08915009cef 100644 --- a/packages/repository/test/fixtures/models/index.ts +++ b/packages/repository/test/fixtures/models/index.ts @@ -1,2 +1,7 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/repository +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + export * from './customer.model'; export * from './order.model'; diff --git a/packages/repository/test/fixtures/models/order.model.ts b/packages/repository/test/fixtures/models/order.model.ts index 55bfa6960e97..df8e20fe33cf 100644 --- a/packages/repository/test/fixtures/models/order.model.ts +++ b/packages/repository/test/fixtures/models/order.model.ts @@ -1,3 +1,8 @@ +// Copyright IBM Corp. 2018. 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 {model, property, belongsTo, Entity} from '../../..'; import {Customer} from './customer.model'; diff --git a/packages/repository/test/fixtures/repositories/customer.repository.ts b/packages/repository/test/fixtures/repositories/customer.repository.ts index 4dfa9df6a359..bd7e545bd5f3 100644 --- a/packages/repository/test/fixtures/repositories/customer.repository.ts +++ b/packages/repository/test/fixtures/repositories/customer.repository.ts @@ -1,3 +1,8 @@ +// Copyright IBM Corp. 2018. 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 {Customer, Order} from '../models'; import {OrderRepository} from './order.repository'; import { diff --git a/packages/repository/test/fixtures/repositories/index.ts b/packages/repository/test/fixtures/repositories/index.ts index 4c0e725c50a1..e77360ff004c 100644 --- a/packages/repository/test/fixtures/repositories/index.ts +++ b/packages/repository/test/fixtures/repositories/index.ts @@ -1,2 +1,7 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/repository +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + export * from './customer.repository'; export * from './order.repository'; diff --git a/packages/repository/test/fixtures/repositories/order.repository.ts b/packages/repository/test/fixtures/repositories/order.repository.ts index d024916d4861..4eb8079228e4 100644 --- a/packages/repository/test/fixtures/repositories/order.repository.ts +++ b/packages/repository/test/fixtures/repositories/order.repository.ts @@ -1,3 +1,8 @@ +// Copyright IBM Corp. 2018. 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 {Order, Customer} from '../models'; import {CustomerRepository} from '../repositories'; import { @@ -12,14 +17,14 @@ export class OrderRepository extends DefaultCrudRepository< Order, typeof Order.prototype.id > { - public customer: BelongsToFactory; + public customer: BelongsToFactory; constructor( @inject('datasources.db') protected db: juggler.DataSource, @repository.getter('repositories.CustomerRepository') customerRepositoryGetter: Getter, ) { super(Order, db); - this.customer = this._createBelongsToFactoryFor( + this.customer = this._createBelongsToRepositoryFactoryFor( 'customerId', customerRepositoryGetter, ); diff --git a/packages/repository/test/integration/repositories/relation.factory.integration.ts b/packages/repository/test/integration/repositories/relation.factory.integration.ts index e6b7ecb48ef0..1ac1580060b3 100644 --- a/packages/repository/test/integration/repositories/relation.factory.integration.ts +++ b/packages/repository/test/integration/repositories/relation.factory.integration.ts @@ -177,6 +177,7 @@ describe('belongsTo relation', () => { const findCustomerOfOrder = createBelongsToFactory( Order.definition.relations.customerId as BelongsToDefinition, createGetter(customerRepo), + orderRepo, ); const customer = await customerRepo.create({name: 'Order McForder'}); @@ -184,7 +185,7 @@ describe('belongsTo relation', () => { customerId: customer.id, description: 'Order from Order McForder, the hoarder of Mordor', }); - const result = await findCustomerOfOrder(order); + const result = await findCustomerOfOrder(order.id); expect(result).to.deepEqual(customer); }); @@ -197,7 +198,11 @@ describe('belongsTo relation', () => { keyTo: 'someKey', }; expect(() => - createBelongsToFactory(keyFromLessMeta, createGetter(reviewRepo)), + createBelongsToFactory( + keyFromLessMeta, + createGetter(reviewRepo), + orderRepo, + ), ).to.throw(/The foreign key property name \(keyFrom\) must be specified/); }); @@ -209,7 +214,11 @@ describe('belongsTo relation', () => { keyFrom: 'someKey', }; expect(() => - createBelongsToFactory(keyToLessMeta, createGetter(reviewRepo)), + createBelongsToFactory( + keyToLessMeta, + createGetter(reviewRepo), + orderRepo, + ), ).to.throw(/The primary key property name \(keyTo\) must be specified/); }); @@ -244,6 +253,10 @@ describe('belongsTo relation', () => { new juggler.DataSource({connector: 'memory'}), ), ), + new DefaultCrudRepository( + Card, + new juggler.DataSource({connector: 'memory'}), + ), ); expect(belongsToMeta).to.eql({ type: RelationType.belongsTo, diff --git a/packages/repository/test/test-utils.ts b/packages/repository/test/test-utils.ts index ff38ee0c7704..496f1bed4bef 100644 --- a/packages/repository/test/test-utils.ts +++ b/packages/repository/test/test-utils.ts @@ -1,3 +1,8 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/repository +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + export function createGetter(value: T) { return () => Promise.resolve(value); } From bcd7ee98cc4958ccde595cf26e35e6f53f236401 Mon Sep 17 00:00:00 2001 From: shimks Date: Tue, 28 Aug 2018 02:22:53 -0400 Subject: [PATCH 6/6] fixup! apply some of the feedback --- docs/site/HasMany-relation.md | 27 +- docs/site/todo-list-tutorial-model.md | 2 +- docs/site/todo-list-tutorial-repository.md | 15 +- .../todo-list/src/models/todo-list.model.ts | 2 +- .../src/repositories/todo-list.repository.ts | 9 +- package.json | 2 +- packages/repository/package.json | 2 +- .../src/decorators/relation.decorator.ts | 44 +-- .../src/repositories/legacy-juggler-bridge.ts | 30 +- .../src/repositories/relation.factory.ts | 19 +- .../src/repositories/relation.repository.ts | 4 +- .../belongs-to.relation.acceptance.ts | 2 +- ...has-many-without-di.relation.acceptance.ts | 11 +- .../fixtures/models/bad/category.model.ts | 12 - .../test/fixtures/models/bad/product.model.ts | 12 - .../test/fixtures/models/order.model.ts | 4 +- .../repositories/customer.repository.ts | 6 +- .../fixtures/repositories/order.repository.ts | 6 +- .../relation.factory.integration.ts | 35 +- .../model-and-relation.decorator.unit.ts | 16 +- .../unit/decorator/relation.decorator.unit.ts | 315 ++++-------------- .../repositories/relation.repository.unit.ts | 22 -- 22 files changed, 142 insertions(+), 455 deletions(-) delete mode 100644 packages/repository/test/fixtures/models/bad/category.model.ts delete mode 100644 packages/repository/test/fixtures/models/bad/product.model.ts diff --git a/docs/site/HasMany-relation.md b/docs/site/HasMany-relation.md index 23abf5631a62..4d0790562b33 100644 --- a/docs/site/HasMany-relation.md +++ b/docs/site/HasMany-relation.md @@ -56,7 +56,7 @@ export class Customer extends Entity { }) name: string; - @hasMany(Order) + @hasMany(() => Order) orders?: Order[]; constructor(data: Partial) { @@ -85,7 +85,7 @@ as follows: // import statements class Customer extends Entity { // constructor, properties, etc. - @hasMany(Order, {keyTo: 'custId'}) + @hasMany(() => Order, {keyTo: 'custId'}) orders?: Order[]; } ``` @@ -101,17 +101,17 @@ repository, the following are required: the target repository in the constructor of your source repository class. - Declare a property with the factory function type - `HasManyRepositoryFactory` on - the source repository class. -- call the `_createHasManyRepositoryFactoryFor` function in the constructor of - the source repository class with the relation name (decorated relation - property on the source model) and target repository instance and assign it the - property mentioned above. + `HasManyAccessor` on the source + repository class. +- call the `_createHasManyAccessorFor` function in the constructor of the source + repository class with the relation name (decorated relation property on the + source model) and target repository instance and assign it the property + mentioned above. The following code snippet shows how it would look like: {% include code-caption.html -content="/src/repositories/customer.repository.ts.ts" %} +content="/src/repositories/customer.repository.ts" %} ```ts import {Order, Customer} from '../models'; @@ -119,7 +119,7 @@ import {OrderRepository} from './order.repository.ts'; import { DefaultCrudRepository, juggler, - HasManyRepositoryFactory, + HasManyAccessor, repository, } from '@loopback/repository'; import {inject, Getter} from '@loopback/core'; @@ -128,17 +128,14 @@ class CustomerRepository extends DefaultCrudRepository< Customer, typeof Customer.prototype.id > { - public orders: HasManyRepositoryFactory; + public orders: HasManyAccessor; constructor( @inject('datasources.db') protected db: juggler.DataSource, @repository.getter('repositories.OrderRepository') getOrderRepository: Getter, ) { super(Customer, db); - this.orders = this._createHasManyRepositoryFactoryFor( - 'orders', - getOrderRepository, - ); + this.orders = this._createHasManyAccessorFor_('orders', getOrderRepository); } } ``` diff --git a/docs/site/todo-list-tutorial-model.md b/docs/site/todo-list-tutorial-model.md index 18066eab237a..2f3b1f6131ed 100644 --- a/docs/site/todo-list-tutorial-model.md +++ b/docs/site/todo-list-tutorial-model.md @@ -78,7 +78,7 @@ model. To `TodoList` model, add in the following property: export class TodoList extends Entity { // ...properties defined by the CLI... - @hasMany(Todo) + @hasMany(() => Todo, {keyTo: 'todoListId'}) todos?: Todo[]; // ...constructor def... diff --git a/docs/site/todo-list-tutorial-repository.md b/docs/site/todo-list-tutorial-repository.md index da733a8e65a3..758d5524f2dc 100644 --- a/docs/site/todo-list-tutorial-repository.md +++ b/docs/site/todo-list-tutorial-repository.md @@ -35,9 +35,9 @@ we'll need to make two more additions: - inject `TodoRepository` instance Once the property type for `todos` have been defined, use -`this._createHasManyRepositoryFactoryFor` to assign it a repository contraining -factory function. Pass in the name of the relationship (`todos`) and the Todo -repository instance to constrain as the arguments for the function. +`this._createHasManyAccessorFor` to assign it a repository constraining factory +function. Pass in the name of the relationship (`todos`) and the Todo repository +instance to constrain as the arguments for the function. #### src/repositories/todo-list.repository.ts @@ -45,7 +45,7 @@ repository instance to constrain as the arguments for the function. import { DefaultCrudRepository, juggler, - HasManyRepositoryFactory, + HasManyAccessor, repository, } from '@loopback/repository'; import {TodoList, Todo} from '../models'; @@ -56,7 +56,7 @@ export class TodoListRepository extends DefaultCrudRepository< TodoList, typeof TodoList.prototype.id > { - public todos: HasManyRepositoryFactory; + public todos: HasManyAccessor; constructor( @inject('datasources.db') protected datasource: juggler.DataSource, @@ -64,10 +64,7 @@ export class TodoListRepository extends DefaultCrudRepository< protected getTodoRepository: Getter, ) { super(TodoList, datasource); - this.todos = this._createHasManyRepositoryFactoryFor( - 'todos', - getTodoRepository, - ); + this.todos = this._createHasManyAccessorFor('todos', getTodoRepository); } } ``` diff --git a/examples/todo-list/src/models/todo-list.model.ts b/examples/todo-list/src/models/todo-list.model.ts index 2fe4a29c2a1c..229d2dca7136 100644 --- a/examples/todo-list/src/models/todo-list.model.ts +++ b/examples/todo-list/src/models/todo-list.model.ts @@ -25,7 +25,7 @@ export class TodoList extends Entity { }) color?: string; - @hasMany(Todo) + @hasMany(() => Todo, {keyTo: 'todoListId'}) todos: Todo[]; constructor(data?: Partial) { diff --git a/examples/todo-list/src/repositories/todo-list.repository.ts b/examples/todo-list/src/repositories/todo-list.repository.ts index 1df05aee2f4d..f1eb213f5112 100644 --- a/examples/todo-list/src/repositories/todo-list.repository.ts +++ b/examples/todo-list/src/repositories/todo-list.repository.ts @@ -6,7 +6,7 @@ import { DefaultCrudRepository, juggler, - HasManyRepositoryFactory, + HasManyAccessor, repository, } from '@loopback/repository'; import {TodoList, Todo} from '../models'; @@ -17,7 +17,7 @@ export class TodoListRepository extends DefaultCrudRepository< TodoList, typeof TodoList.prototype.id > { - public todos: HasManyRepositoryFactory; + public todos: HasManyAccessor; constructor( @inject('datasources.db') protected datasource: juggler.DataSource, @@ -25,9 +25,6 @@ export class TodoListRepository extends DefaultCrudRepository< protected getTodoRepository: Getter, ) { super(TodoList, datasource); - this.todos = this._createHasManyRepositoryFactoryFor( - 'todos', - getTodoRepository, - ); + this.todos = this._createHasManyAccessorFor('todos', getTodoRepository); } } diff --git a/package.json b/package.json index 24031c3ecdb4..059ba30804a9 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "test:ci": "node packages/build/bin/run-nyc npm run mocha --scripts-prepend-node-path", "verify:docs": "npm run build:site -- --verify", "build:site": "./bin/build-docs-site.sh", - "mocha": "node packages/build/bin/run-mocha \"packages/*/DIST/test/**/*.js\" \"examples/*/DIST/test/**/*.js\" \"packages/cli/test/**/*.js\" \"packages/build/test/*/*.js\" --exclude packages/*/DIST/test/fixtures/**/*.js", + "mocha": "node packages/build/bin/run-mocha \"packages/*/DIST/test/**/*.js\" \"examples/*/DIST/test/**/*.js\" \"packages/cli/test/**/*.js\" \"packages/build/test/*/*.js\"", "posttest": "npm run lint", "commitmsg": "commitlint -E GIT_PARAMS" }, diff --git a/packages/repository/package.json b/packages/repository/package.json index 7431dfc4a551..f47d3052fed2 100644 --- a/packages/repository/package.json +++ b/packages/repository/package.json @@ -15,7 +15,7 @@ "build:dist10": "lb-tsc es2018", "clean": "lb-clean loopback-repository*.tgz dist* package api-docs", "pretest": "npm run build", - "test": "lb-mocha \"DIST/test/**/*.js\" --exclude DIST/test/fixtures/models/bad/*.js", + "test": "lb-mocha \"DIST/test/**/*.js\"", "verify": "npm pack && tar xf loopback-repository*.tgz && tree package && npm run clean" }, "author": "IBM", diff --git a/packages/repository/src/decorators/relation.decorator.ts b/packages/repository/src/decorators/relation.decorator.ts index 12c348f69170..2cb01e13e972 100644 --- a/packages/repository/src/decorators/relation.decorator.ts +++ b/packages/repository/src/decorators/relation.decorator.ts @@ -36,7 +36,7 @@ export class RelationMetadata { export interface RelationDefinitionBase { type: RelationType; - target: typeof Entity | TypeResolver; + target: TypeResolver; } export interface HasManyDefinition extends RelationDefinitionBase { @@ -66,7 +66,7 @@ export function relation(definition?: Object) { * @returns {(target:any, key:string)} */ export function belongsTo( - targetModel: T | TypeResolver, + targetModel: TypeResolver, definition?: Partial, ) { const defIsCyclic = @@ -88,25 +88,6 @@ export function belongsTo( keyFrom: key, }; - if (!isTypeResolver(targetModel)) { - const hasKeyTo = definition && definition.keyTo; - let hasPrimaryKey = false; - - for (const propertyKey in targetModel.definition.properties) { - if (targetModel.definition.properties[propertyKey].id === true) { - rel.keyTo = propertyKey; - hasPrimaryKey = true; - break; - } - } - - if (!hasPrimaryKey && !hasKeyTo) { - throw new Error( - `primary key not found on ${targetModel.name} model's definition`, - ); - } - } - // Apply model definition to the model class Object.assign(rel, definition); relation(rel)(target, key); @@ -132,7 +113,7 @@ export function hasOne(definition?: Object) { * @returns {(target:any, key:string)} */ export function hasMany( - targetModel: T | TypeResolver, + targetModel: TypeResolver, definition?: Partial, ) { // todo(shimks): extract out common logic (such as @property.array) to @@ -142,25 +123,6 @@ export function hasMany( const meta: Partial = {target: targetModel}; - if (!isTypeResolver(targetModel)) { - const defaultFkName = camelCase(target.constructor.name + '_id'); - const hasKeyTo = definition && definition.keyTo; - const hasDefaultFkProperty = - targetModel.definition && - targetModel.definition.properties && - targetModel.definition.properties[defaultFkName]; - if (!(hasKeyTo || hasDefaultFkProperty)) { - // note(shimks): should we also check for the existence of explicitly - // given foreign key name on the juggler definition? - throw new Error( - `foreign key ${defaultFkName} not found on ${ - targetModel.name - } model's definition`, - ); - } - Object.assign(meta, {keyTo: defaultFkName}); - } - Object.assign(meta, definition, {type: RelationType.hasMany}); relation(meta)(target, key); diff --git a/packages/repository/src/repositories/legacy-juggler-bridge.ts b/packages/repository/src/repositories/legacy-juggler-bridge.ts index 49812450fbff..773867cac9bd 100644 --- a/packages/repository/src/repositories/legacy-juggler-bridge.ts +++ b/packages/repository/src/repositories/legacy-juggler-bridge.ts @@ -20,17 +20,14 @@ import {Filter, Where} from '../query'; import {EntityCrudRepository} from './repository'; import { createHasManyRepositoryFactory, - HasManyRepositoryFactory, - BelongsToFactory, + HasManyAccessor, + BelongsToAccessor, createBelongsToFactory, } from './relation.factory'; import { HasManyDefinition, BelongsToDefinition, } from '../decorators/relation.decorator'; -// need the import for exporting of a return type -// tslint:disable-next-line:no-unused-variable -import {HasManyRepository} from './relation.repository'; export namespace juggler { export import DataSource = legacy.DataSource; @@ -153,7 +150,7 @@ export class DefaultCrudRepository * orderRepository: EntityCrudRepository, * ) { * super(Customer, db); - * this.orders = this._createHasManyRepositoryFactoryFor( + * this.orders = this._createHasManyAccessorFor( * 'orders', * orderRepository, * ); @@ -162,30 +159,27 @@ export class DefaultCrudRepository * ``` * * @param relationName Name of the relation defined on the source model - * @param targetRepo Target repository instance + * @param targetRepoGetter Getter for the target repository instance */ - protected _createHasManyRepositoryFactoryFor( + protected _createHasManyAccessorFor( relationName: string, - targetRepo: Getter>, - ): HasManyRepositoryFactory { + targetRepoGetter: Getter>, + ): HasManyAccessor { const meta = this.entityClass.definition.relations[relationName]; return createHasManyRepositoryFactory( meta as HasManyDefinition, - targetRepo, + targetRepoGetter, ); } - protected _createBelongsToRepositoryFactoryFor< - Target extends Entity, - TargetId - >( + protected _createBelongsToAccessorFor( relationName: string, - targetRepo: Getter>, - ): BelongsToFactory { + targetRepoGetter: Getter>, + ): BelongsToAccessor { const meta = this.entityClass.definition.relations[relationName]; return createBelongsToFactory( meta as BelongsToDefinition, - targetRepo, + targetRepoGetter, this, ); } diff --git a/packages/repository/src/repositories/relation.factory.ts b/packages/repository/src/repositories/relation.factory.ts index 799e794d179a..893b17cf1124 100644 --- a/packages/repository/src/repositories/relation.factory.ts +++ b/packages/repository/src/repositories/relation.factory.ts @@ -23,11 +23,11 @@ const debug = require('debug')('loopback:repository:relation:factory'); const ERR_NO_BELONGSTO_META = 'no belongsTo metadata found'; const ERR_NO_ID_META = 'no id metadata found'; -export type HasManyRepositoryFactory = ( +export type HasManyAccessor = ( fkValue: ForeignKeyType, ) => HasManyRepository; -export type BelongsToFactory = ( +export type BelongsToAccessor = ( sourceId: SourceId, ) => Promise; @@ -50,8 +50,8 @@ export function createHasManyRepositoryFactory< ForeignKeyType >( relationMetadata: HasManyDefinition, - targetRepository: Getter>, -): HasManyRepositoryFactory { + targetRepoGetter: Getter>, +): HasManyAccessor { resolveHasManyMetadata(relationMetadata); debug('resolved relation metadata: %o', relationMetadata); const fkName = relationMetadata.keyTo; @@ -65,7 +65,7 @@ export function createHasManyRepositoryFactory< Target, TargetID, EntityCrudRepository - >(targetRepository, constraint as DataObject); + >(targetRepoGetter, constraint as DataObject); }; } @@ -79,9 +79,9 @@ export function createBelongsToFactory< SourceId >( belongsToMetadata: BelongsToDefinition, - targetRepository: Getter>, + targetRepoGetter: Getter>, sourceRepository: EntityCrudRepository, -): BelongsToFactory { +): BelongsToAccessor { resolveBelongsToMetadata(belongsToMetadata); const foreignKey = belongsToMetadata.keyFrom; const primaryKey = belongsToMetadata.keyTo; @@ -93,14 +93,13 @@ export function createBelongsToFactory< if (!primaryKey) { throw new Error('The primary key property name (keyTo) must be specified'); } - // NOTE change this to accept ID - return async function(sourceId: SourceId) { + return async function getTargetInstanceOfBelongsTo(sourceId: SourceId) { const sourceModel = await sourceRepository.findById(sourceId); const foreignKeyValue = sourceModel[foreignKey as keyof Source]; // tslint:disable-next-line:no-any const constraint: any = {[primaryKey]: foreignKeyValue}; const constrainedRepo = new DefaultBelongsToEntityCrudRepository( - targetRepository, + targetRepoGetter, constraint as DataObject, ); return constrainedRepo.get(); diff --git a/packages/repository/src/repositories/relation.repository.ts b/packages/repository/src/repositories/relation.repository.ts index f3ca65f03c1d..a889224cd856 100644 --- a/packages/repository/src/repositories/relation.repository.ts +++ b/packages/repository/src/repositories/relation.repository.ts @@ -74,7 +74,7 @@ export class DefaultHasManyEntityCrudRepository< > implements HasManyRepository { /** * Constructor of DefaultHasManyEntityCrudRepository - * @param getTargetRepository the related target model repository instance + * @param getTargetRepository the getter of the related target model repository instance * @param constraint the key value pair representing foreign key name to constrain * the target repository instance */ @@ -127,7 +127,7 @@ export class DefaultBelongsToEntityCrudRepository< > implements BelongsToRepository { /** * Constructor of DefaultBelongsToEntityCrudRepository - * @param getTargetRepository the related target model repository instance + * @param getTargetRepository the getter of the related target model repository instance * @param constraint the key value pair representing foreign key name to constrain * the target repository instance */ diff --git a/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts index ca53d5e65338..760779e070c2 100644 --- a/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts +++ b/packages/repository/test/acceptance/belongs-to.relation.acceptance.ts @@ -41,7 +41,7 @@ describe('HasMany relation', () => { @repository(OrderRepository) protected orderRepository: OrderRepository, ) {} - async findOwnerOfOrder(orderId: number) { + async findOwnerOfOrder(orderId: string) { return await this.orderRepository.customer(orderId); } } diff --git a/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts b/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts index 6ddc5f52239c..d9ef9685ec87 100644 --- a/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts +++ b/packages/repository/test/acceptance/has-many-without-di.relation.acceptance.ts @@ -10,7 +10,7 @@ import { DefaultCrudRepository, juggler, hasMany, - HasManyRepositoryFactory, + HasManyAccessor, EntityCrudRepository, } from '../..'; import {expect} from '@loopback/testlab'; @@ -117,7 +117,7 @@ describe('HasMany relation', () => { }) name: string; - @hasMany(Order) + @hasMany(() => Order, {keyTo: 'customerId'}) orders: Order[]; } @@ -134,17 +134,14 @@ describe('HasMany relation', () => { Customer, typeof Customer.prototype.id > { - public orders: HasManyRepositoryFactory< - Order, - typeof Customer.prototype.id - >; + public orders: HasManyAccessor; constructor( protected db: juggler.DataSource, orderRepository: EntityCrudRepository, ) { super(Customer, db); - this.orders = this._createHasManyRepositoryFactoryFor( + this.orders = this._createHasManyAccessorFor( 'orders', createGetter(orderRepository), ); diff --git a/packages/repository/test/fixtures/models/bad/category.model.ts b/packages/repository/test/fixtures/models/bad/category.model.ts deleted file mode 100644 index 82e065a3bbc4..000000000000 --- a/packages/repository/test/fixtures/models/bad/category.model.ts +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright IBM Corp. 2018. 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 {hasMany, Entity} from '../../../..'; -import {Product} from './product.model'; - -export class Category extends Entity { - @hasMany(Product) - products: Product[]; -} diff --git a/packages/repository/test/fixtures/models/bad/product.model.ts b/packages/repository/test/fixtures/models/bad/product.model.ts deleted file mode 100644 index eabe231c37a8..000000000000 --- a/packages/repository/test/fixtures/models/bad/product.model.ts +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright IBM Corp. 2018. 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 {belongsTo, Entity} from '../../../..'; -import {Category} from './category.model'; - -export class Product extends Entity { - @belongsTo(Category) - categoryId: number; -} diff --git a/packages/repository/test/fixtures/models/order.model.ts b/packages/repository/test/fixtures/models/order.model.ts index df8e20fe33cf..5c8f5ff876e4 100644 --- a/packages/repository/test/fixtures/models/order.model.ts +++ b/packages/repository/test/fixtures/models/order.model.ts @@ -9,10 +9,10 @@ import {Customer} from './customer.model'; @model() export class Order extends Entity { @property({ - type: 'number', + type: 'string', id: true, }) - id: number; + id: string; @property({ type: 'string', diff --git a/packages/repository/test/fixtures/repositories/customer.repository.ts b/packages/repository/test/fixtures/repositories/customer.repository.ts index bd7e545bd5f3..5cf83842cf15 100644 --- a/packages/repository/test/fixtures/repositories/customer.repository.ts +++ b/packages/repository/test/fixtures/repositories/customer.repository.ts @@ -7,7 +7,7 @@ import {Customer, Order} from '../models'; import {OrderRepository} from './order.repository'; import { DefaultCrudRepository, - HasManyRepositoryFactory, + HasManyAccessor, juggler, repository, } from '../../..'; @@ -17,14 +17,14 @@ export class CustomerRepository extends DefaultCrudRepository< Customer, typeof Customer.prototype.id > { - public orders: HasManyRepositoryFactory; + public orders: HasManyAccessor; constructor( @inject('datasources.db') protected db: juggler.DataSource, @repository.getter('repositories.OrderRepository') orderRepositoryGetter: Getter, ) { super(Customer, db); - this.orders = this._createHasManyRepositoryFactoryFor( + this.orders = this._createHasManyAccessorFor( 'orders', orderRepositoryGetter, ); diff --git a/packages/repository/test/fixtures/repositories/order.repository.ts b/packages/repository/test/fixtures/repositories/order.repository.ts index 4eb8079228e4..b2e04c23986e 100644 --- a/packages/repository/test/fixtures/repositories/order.repository.ts +++ b/packages/repository/test/fixtures/repositories/order.repository.ts @@ -8,7 +8,7 @@ import {CustomerRepository} from '../repositories'; import { DefaultCrudRepository, juggler, - BelongsToFactory, + BelongsToAccessor, repository, } from '../../..'; import {inject, Getter} from '@loopback/context'; @@ -17,14 +17,14 @@ export class OrderRepository extends DefaultCrudRepository< Order, typeof Order.prototype.id > { - public customer: BelongsToFactory; + public customer: BelongsToAccessor; constructor( @inject('datasources.db') protected db: juggler.DataSource, @repository.getter('repositories.CustomerRepository') customerRepositoryGetter: Getter, ) { super(Order, db); - this.customer = this._createBelongsToRepositoryFactoryFor( + this.customer = this._createBelongsToAccessorFor( 'customerId', customerRepositoryGetter, ); diff --git a/packages/repository/test/integration/repositories/relation.factory.integration.ts b/packages/repository/test/integration/repositories/relation.factory.integration.ts index 1ac1580060b3..0896beb408b5 100644 --- a/packages/repository/test/integration/repositories/relation.factory.integration.ts +++ b/packages/repository/test/integration/repositories/relation.factory.integration.ts @@ -13,7 +13,7 @@ import { ModelDefinition, createHasManyRepositoryFactory, HasManyDefinition, - HasManyRepositoryFactory, + HasManyAccessor, hasMany, belongsTo, model, @@ -30,11 +30,11 @@ let customerRepo: EntityCrudRepository; let orderRepo: EntityCrudRepository; let reviewRepo: EntityCrudRepository; let customerOrderRepo: HasManyRepository; -let customerAuthoredReviewFactoryFn: HasManyRepositoryFactory< +let customerAuthoredReviewFactoryFn: HasManyAccessor< Review, typeof Customer.prototype.id >; -let customerApprovedReviewFactoryFn: HasManyRepositoryFactory< +let customerApprovedReviewFactoryFn: HasManyAccessor< Review, typeof Customer.prototype.id >; @@ -121,17 +121,6 @@ describe('HasMany relation', () => { }); context('createHasManyRepositoryFactory', () => { - it('errors when keyTo is not available in hasMany metadata', () => { - class SomeClass extends Entity {} - const keytolessMeta: HasManyDefinition = { - type: RelationType.hasMany, - target: SomeClass, - }; - expect(() => - createHasManyRepositoryFactory(keytolessMeta, createGetter(reviewRepo)), - ).to.throw(/The foreign key property name \(keyTo\) must be specified/); - }); - it('resolves belongsTo metadata', () => { @model() class Card extends Entity { @@ -194,7 +183,7 @@ describe('belongsTo relation', () => { class SomeClass extends Entity {} const keyFromLessMeta: BelongsToDefinition = { type: RelationType.belongsTo, - target: SomeClass, + target: () => SomeClass, keyTo: 'someKey', }; expect(() => @@ -206,22 +195,6 @@ describe('belongsTo relation', () => { ).to.throw(/The foreign key property name \(keyFrom\) must be specified/); }); - it('errors when keyTo is not available from belongsTo metadata', () => { - class SomeClass extends Entity {} - const keyToLessMeta: BelongsToDefinition = { - type: RelationType.belongsTo, - target: SomeClass, - keyFrom: 'someKey', - }; - expect(() => - createBelongsToFactory( - keyToLessMeta, - createGetter(reviewRepo), - orderRepo, - ), - ).to.throw(/The primary key property name \(keyTo\) must be specified/); - }); - it('resolves property id metadata', () => { @model() class Card extends Entity { diff --git a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts index 5f0bc6352ed8..192c13553bb1 100644 --- a/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts +++ b/packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts @@ -125,7 +125,7 @@ describe('model decorator', () => { @referencesOne() profile: Profile; - @hasMany(Order) + @hasMany(() => Order) orders?: Order[]; @hasOne() @@ -260,8 +260,7 @@ describe('model decorator', () => { ) || /* istanbul ignore next */ {}; expect(meta.orders).to.eql({ type: RelationType.hasMany, - target: Order, - keyTo: 'customerId', + target: () => Order, }); }); @@ -307,7 +306,7 @@ describe('model decorator', () => { class House extends Entity { @property() name: string; - @hasMany(Person, {keyTo: 'fk'}) + @hasMany(() => Person, {keyTo: 'fk'}) person: Person[]; } @@ -320,15 +319,6 @@ describe('model decorator', () => { expect(House.definition.relations).to.containEql({person: relationMeta}); }); - it('throws when hasMany and belongsTo relations are cyclic', () => { - expect(() => { - require('../../fixtures/models/bad/category.model'); - }).to.throw(ERR_TARGET_UNDEFINED); - expect(() => { - require('../../fixtures/models/bad/product.model'); - }).to.throw(ERR_TARGET_UNDEFINED); - }); - describe('property namespace', () => { describe('array', () => { it('adds array metadata', () => { diff --git a/packages/repository/test/unit/decorator/relation.decorator.unit.ts b/packages/repository/test/unit/decorator/relation.decorator.unit.ts index 671ab1b20517..fc695af3476a 100644 --- a/packages/repository/test/unit/decorator/relation.decorator.unit.ts +++ b/packages/repository/test/unit/decorator/relation.decorator.unit.ts @@ -18,64 +18,31 @@ import {MetadataInspector} from '@loopback/context'; describe('relation decorator', () => { context('hasMany', () => { - it('throws when foreign key is not defined in the target TypeScript model', () => { - expect(() => { - @model() - class Address extends Entity { - addressId: number; - street: string; - province: string; - } - - // tslint:disable-next-line:no-unused-variable - class AddressBook extends Entity { - id: number; - - @hasMany(Address) - addresses: Address[]; - } - }).throw(/addressBookId not found on Address/); - }); - - it('takes in complex property type and infers foreign key via source model name', () => { - @model() + it('assigns it to target key', () => { class Address extends Entity { addressId: number; street: string; province: string; - @property() - addressBookId: number; } class AddressBook extends Entity { id: number; - @hasMany(Address) + @hasMany(() => Address) addresses: Address[]; } - const meta = MetadataInspector.getPropertyMetadata( RELATIONS_KEY, AddressBook.prototype, 'addresses', ); - const jugglerMeta = MetadataInspector.getPropertyMetadata( - MODEL_PROPERTIES_KEY, - AddressBook.prototype, - 'addresses', - ); expect(meta).to.eql({ type: RelationType.hasMany, - target: Address, - keyTo: 'addressBookId', - }); - expect(jugglerMeta).to.eql({ - type: Array, - itemType: Address, + target: () => Address, }); }); - it('takes in both complex property type and hasMany metadata', () => { + it('accepts explicit keyTo property', () => { class Address extends Entity { addressId: number; street: string; @@ -85,80 +52,19 @@ describe('relation decorator', () => { class AddressBook extends Entity { id: number; - @hasMany(Address, {keyTo: 'someForeignKey'}) + @hasMany(() => Address, {keyTo: 'someForeignKey'}) addresses: Address[]; } - const meta = MetadataInspector.getPropertyMetadata( RELATIONS_KEY, AddressBook.prototype, 'addresses', ); - const jugglerMeta = MetadataInspector.getPropertyMetadata( - MODEL_PROPERTIES_KEY, - AddressBook.prototype, - 'addresses', - ); expect(meta).to.eql({ type: RelationType.hasMany, - target: Address, + target: () => Address, keyTo: 'someForeignKey', }); - expect(jugglerMeta).to.eql({ - type: Array, - itemType: Address, - }); - }); - - context('when given resolver', () => { - it('assigns it to target key', () => { - class Address extends Entity { - addressId: number; - street: string; - province: string; - } - - class AddressBook extends Entity { - id: number; - - @hasMany(() => Address) - addresses: Address[]; - } - const meta = MetadataInspector.getPropertyMetadata( - RELATIONS_KEY, - AddressBook.prototype, - 'addresses', - ); - expect(meta).to.eql({ - type: RelationType.hasMany, - target: () => Address, - }); - }); - - it('accepts explicit keyTo property', () => { - class Address extends Entity { - addressId: number; - street: string; - province: string; - } - - class AddressBook extends Entity { - id: number; - - @hasMany(() => Address, {keyTo: 'someForeignKey'}) - addresses: Address[]; - } - const meta = MetadataInspector.getPropertyMetadata( - RELATIONS_KEY, - AddressBook.prototype, - 'addresses', - ); - expect(meta).to.eql({ - type: RelationType.hasMany, - target: () => Address, - keyTo: 'someForeignKey', - }); - }); }); context('when interacting with @property.array', () => { @@ -174,7 +80,7 @@ describe('relation decorator', () => { class AddressBook extends Entity { id: number; @property.array(Entity) - @hasMany(Address, { + @hasMany(() => Address, { keyTo: 'someForeignKey', }) addresses: Address[]; @@ -192,7 +98,7 @@ describe('relation decorator', () => { id: number; } class Address extends Entity { - @belongsTo(AddressBook) + @belongsTo(() => AddressBook) addressBookId: number; } const jugglerMeta = MetadataInspector.getAllPropertyMetadata( @@ -204,155 +110,76 @@ describe('relation decorator', () => { }); }); - context('when given model', () => { - it('throws when id is not defined in the target TypeScript model', () => { - expect(() => { - @model() - class AddressBook extends Entity { - addresses: Address[]; - } - - class Address extends Entity { - addressId: number; - street: string; - province: string; - @belongsTo(AddressBook) - addressBookId: number; - } - }).throw(/primary key not found on AddressBook/); - }); - - it('infers foreign key', () => { - @model() - class AddressBook extends Entity { - @property({id: true}) - id: number; - } - class Address extends Entity { - @belongsTo(AddressBook) - addressBookId: number; - @property() - someOtherKey: string; - } - const meta = MetadataInspector.getAllPropertyMetadata( - RELATIONS_KEY, - Address.prototype, - ); - expect(meta) - .to.have.property('addressBookId') - .which.containEql({keyFrom: 'addressBookId'}); - }); - - it('takes in complex property type and defines infers primary key name', () => { - @model() - class AddressBook extends Entity { - @property({id: true}) - yellowPageId: number; - addresses: Address[]; - } - - @model() - class Address extends Entity { - addressId: number; - street: string; - province: string; - @belongsTo(AddressBook) - addressBookId: number; - } - - const meta = MetadataInspector.getPropertyMetadata( - RELATIONS_KEY, - Address.prototype, - 'addressBookId', - ); - expect(meta).to.eql({ - type: RelationType.belongsTo, - target: AddressBook, - keyFrom: 'addressBookId', - keyTo: 'yellowPageId', - }); - }); - - it('takes in both complex property type and belongsTo metadata', () => { - @model() - class AddressBook extends Entity { - id: number; - addresses: Address[]; - } - - class Address extends Entity { - addressId: number; - street: string; - province: string; - @belongsTo(AddressBook, {keyTo: 'somePrimaryKey'}) - addressBookId: number; - } - - const meta = MetadataInspector.getPropertyMetadata( - RELATIONS_KEY, - Address.prototype, - 'addressBookId', - ); - expect(meta).to.eql({ - type: RelationType.belongsTo, - target: AddressBook, - keyFrom: 'addressBookId', - keyTo: 'somePrimaryKey', - }); - }); + it('infers foreign key', () => { + @model() + class AddressBook extends Entity { + @property({id: true}) + id: number; + } + class Address extends Entity { + @belongsTo(() => AddressBook) + addressBookId: number; + @property() + someOtherKey: string; + } + const meta = MetadataInspector.getAllPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + ); + expect(meta) + .to.have.property('addressBookId') + .which.containEql({keyFrom: 'addressBookId'}); }); - context('when given resolver', () => { - it('assigns it to target key', () => { - class Address extends Entity { - addressId: number; - street: string; - province: string; - @belongsTo(() => AddressBook) - addressBookId: number; - } + it('assigns it to target key', () => { + class Address extends Entity { + addressId: number; + street: string; + province: string; + @belongsTo(() => AddressBook) + addressBookId: number; + } - class AddressBook extends Entity { - id: number; - addresses: Address[]; - } + class AddressBook extends Entity { + id: number; + addresses: Address[]; + } - const meta = MetadataInspector.getPropertyMetadata( - RELATIONS_KEY, - Address.prototype, - 'addressBookId', - ); - expect(meta).to.eql({ - type: RelationType.belongsTo, - target: () => AddressBook, - keyFrom: 'addressBookId', - }); + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + 'addressBookId', + ); + expect(meta).to.eql({ + type: RelationType.belongsTo, + target: () => AddressBook, + keyFrom: 'addressBookId', }); + }); - it('accepts explicit keyTo property', () => { - class Address extends Entity { - addressId: number; - street: string; - province: string; - @belongsTo(() => AddressBook, {keyTo: 'somePrimaryKey'}) - addressBookId: number; - } + it('accepts explicit keyTo property', () => { + class Address extends Entity { + addressId: number; + street: string; + province: string; + @belongsTo(() => AddressBook, {keyTo: 'somePrimaryKey'}) + addressBookId: number; + } - class AddressBook extends Entity { - id: number; - addresses: Address[]; - } - const meta = MetadataInspector.getPropertyMetadata( - RELATIONS_KEY, - Address.prototype, - 'addressBookId', - ); - expect(meta).to.eql({ - type: RelationType.belongsTo, - target: () => AddressBook, - keyFrom: 'addressBookId', - keyTo: 'somePrimaryKey', - }); + class AddressBook extends Entity { + id: number; + addresses: Address[]; + } + const meta = MetadataInspector.getPropertyMetadata( + RELATIONS_KEY, + Address.prototype, + 'addressBookId', + ); + expect(meta).to.eql({ + type: RelationType.belongsTo, + target: () => AddressBook, + keyFrom: 'addressBookId', + keyTo: 'somePrimaryKey', }); }); }); diff --git a/packages/repository/test/unit/repositories/relation.repository.unit.ts b/packages/repository/test/unit/repositories/relation.repository.unit.ts index 38a7d1a92d0b..d57504f6a189 100644 --- a/packages/repository/test/unit/repositories/relation.repository.unit.ts +++ b/packages/repository/test/unit/repositories/relation.repository.unit.ts @@ -116,17 +116,6 @@ describe('relation repository', () => { }); context('resolveHasManyMetadata', () => { - it('retains non-resolver type', () => { - class TargetModel extends Entity {} - const meta: HasManyDefinition = { - type: RelationType.hasMany, - target: TargetModel, - }; - const result = resolveHasManyMetadata(meta); - - expect(result).to.eql(meta); - }); - it('throws if no target relation metadata is found', () => { class TargetModel extends Entity { static definition = new ModelDefinition({ @@ -205,17 +194,6 @@ describe('relation repository', () => { }); context('resolveBelongsToMetadata', () => { - it('retains non-resolver type', () => { - class TargetModel extends Entity {} - const meta: BelongsToDefinition = { - type: RelationType.belongsTo, - target: TargetModel, - }; - const result = resolveBelongsToMetadata(meta); - - expect(result).to.eql(meta); - }); - it('throws if no target definition metadata is found', () => { class TargetModel extends Entity { static definition = new ModelDefinition({