-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(repository): hasOne relation #1879
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
// Copyright IBM Corp. 2017,2018. All Rights Reserved. | ||
// Node module: @loopback/example-todo | ||
// This file is licensed under the MIT License. | ||
// License text available at https://opensource.org/licenses/MIT | ||
|
||
import * as debugFactory from 'debug'; | ||
import {camelCase} from 'lodash'; | ||
import {DataObject} from '../../common-types'; | ||
import {InvalidRelationError} from '../../errors'; | ||
import {Entity} from '../../model'; | ||
import {EntityCrudRepository} from '../../repositories/repository'; | ||
import {isTypeResolver} from '../../type-resolver'; | ||
import {Getter, HasOneDefinition} from '../relation.types'; | ||
import {DefaultHasOneRepository, HasOneRepository} from './has-one.repository'; | ||
|
||
const debug = debugFactory('loopback:repository:has-many-repository-factory'); | ||
|
||
export type HasOneRepositoryFactory<Target extends Entity, ForeignKeyType> = ( | ||
fkValue: ForeignKeyType, | ||
) => HasOneRepository<Target>; | ||
|
||
/** | ||
* Enforces a constraint on a repository based on a relationship contract | ||
* between models. For example, if a Customer model is related to an Address model | ||
* via a HasOne relation, then, the relational repository returned by the | ||
* factory function would be constrained by a Customer model instance's id(s). | ||
* | ||
* @param relationMeta The relation metadata used to describe the | ||
* relationship and determine how to apply the constraint. | ||
* @param targetRepo The repository which represents the target model of a | ||
* relation attached to a datasource. | ||
* @returns The factory function which accepts a foreign key value to constrain | ||
* the given target repository | ||
*/ | ||
export function createHasOneRepositoryFactory< | ||
Target extends Entity, | ||
TargetID, | ||
ForeignKeyType | ||
>( | ||
relationMetadata: HasOneDefinition, | ||
targetRepositoryGetter: Getter<EntityCrudRepository<Target, TargetID>>, | ||
): HasOneRepositoryFactory<Target, ForeignKeyType> { | ||
const meta = resolveHasOneMetadata(relationMetadata); | ||
debug('Resolved HasOne relation metadata: %o', meta); | ||
return function(fkValue: ForeignKeyType) { | ||
// tslint:disable-next-line:no-any | ||
const constraint: any = {[meta.keyTo]: fkValue}; | ||
return new DefaultHasOneRepository< | ||
Target, | ||
TargetID, | ||
EntityCrudRepository<Target, TargetID> | ||
>(targetRepositoryGetter, constraint as DataObject<Target>); | ||
}; | ||
} | ||
|
||
type HasOneResolvedDefinition = HasOneDefinition & {keyTo: string}; | ||
|
||
/** | ||
* 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 | ||
*/ | ||
function resolveHasOneMetadata( | ||
relationMeta: HasOneDefinition, | ||
): HasOneResolvedDefinition { | ||
if (!isTypeResolver(relationMeta.target)) { | ||
const reason = 'target must be a type resolver'; | ||
throw new InvalidRelationError(reason, relationMeta); | ||
} | ||
|
||
if (relationMeta.keyTo) { | ||
// The explict cast is needed because of a limitation of type inference | ||
return relationMeta as HasOneResolvedDefinition; | ||
} | ||
|
||
const sourceModel = relationMeta.source; | ||
if (!sourceModel || !sourceModel.modelName) { | ||
const reason = 'source model must be defined'; | ||
throw new InvalidRelationError(reason, relationMeta); | ||
} | ||
|
||
const targetModel = relationMeta.target(); | ||
debug( | ||
'Resolved model %s from given metadata: %o', | ||
targetModel.modelName, | ||
targetModel, | ||
); | ||
const defaultFkName = camelCase(sourceModel.modelName + '_id'); | ||
const hasDefaultFkProperty = | ||
targetModel.definition && | ||
targetModel.definition.properties && | ||
targetModel.definition.properties[defaultFkName]; | ||
|
||
if (!hasDefaultFkProperty) { | ||
const reason = `target model ${ | ||
targetModel.name | ||
} is missing definition of foreign key ${defaultFkName}`; | ||
throw new InvalidRelationError(reason, relationMeta); | ||
} | ||
|
||
if ( | ||
!targetModel.definition.properties[defaultFkName].id === true && | ||
!targetModel.definition.properties[defaultFkName].generated === false | ||
) { | ||
// throw InvalidRelationError('property must be a generated id field') | ||
} | ||
|
||
return Object.assign(relationMeta, {keyTo: defaultFkName}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we allow override the default fk in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. I believe we should allow users to provide their own There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually: this line is executed only when if (relationMeta.keyTo) {
// The explict cast is needed because of a limitation of type inference
return relationMeta as HasOneResolvedDefinition;
} |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
// Copyright IBM Corp. 2017,2018. All Rights Reserved. | ||
// Node module: @loopback/example-todo | ||
// This file is licensed under the MIT License. | ||
// License text available at https://opensource.org/licenses/MIT | ||
|
||
import {Getter} from '@loopback/context'; | ||
import {DataObject, Options} from '../../common-types'; | ||
import {Entity} from '../../model'; | ||
import {Filter, Where} from '../../query'; | ||
import { | ||
constrainDataObject, | ||
constrainFilter, | ||
} from '../../repositories/constraint-utils'; | ||
import {EntityCrudRepository} from '../../repositories/repository'; | ||
|
||
/** | ||
* CRUD operations for a target repository of a HasMany relation | ||
*/ | ||
export interface HasOneRepository<Target extends Entity> { | ||
/** | ||
* Create a target model instance | ||
* @param targetModelData The target model data | ||
* @param options Options for the operation | ||
* @returns A promise which resolves to the newly created target model instance | ||
*/ | ||
create( | ||
targetModelData: DataObject<Target>, | ||
options?: Options, | ||
): Promise<Target>; | ||
|
||
/** | ||
* Find the only target model instance that belongs to the declaring model. | ||
* @param filter Query filter without a Where condition | ||
* @param options Options for the operations | ||
* @returns A promise of the target object or null if not found. | ||
*/ | ||
get( | ||
filter?: Exclude<Filter<Target>, Where<Target>>, | ||
options?: Options, | ||
): Promise<Target | undefined>; | ||
} | ||
|
||
export class DefaultHasOneRepository< | ||
TargetEntity extends Entity, | ||
TargetID, | ||
TargetRepository extends EntityCrudRepository<TargetEntity, TargetID> | ||
> implements HasOneRepository<TargetEntity> { | ||
/** | ||
* Constructor of DefaultHasManyEntityCrudRepository | ||
* @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 | ||
*/ | ||
constructor( | ||
public getTargetRepository: Getter<TargetRepository>, | ||
public constraint: DataObject<TargetEntity>, | ||
) {} | ||
|
||
async create( | ||
targetModelData: DataObject<TargetEntity>, | ||
options?: Options, | ||
): Promise<TargetEntity> { | ||
const targetRepository = await this.getTargetRepository(); | ||
// should we have an in memory LUT instead of a db query to increase | ||
// performance here? | ||
const found = await targetRepository.find( | ||
constrainFilter({}, this.constraint), | ||
); | ||
if (found.length > 0) { | ||
throw new Error( | ||
'HasOne relation does not allow creation of more than one target model instance', | ||
); | ||
} else { | ||
return await targetRepository.create( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is crucial to leverage an atomic implementation of Consider the following scenario: the LB4 server is handling two incoming HTTP requests to create a hasOne target and the code is executed in the following way by Node.js runtime:
I am proposing to put this pull request on hold and open a new pull request to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for explaining the need for an atomic implementation with a great example. I agree, and I agree with doing that first. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened a new issue to keep track of "findOrCreate" story, see #1956 |
||
constrainDataObject(targetModelData, this.constraint), | ||
options, | ||
); | ||
} | ||
} | ||
|
||
async get( | ||
filter?: Exclude<Filter<TargetEntity>, Where<TargetEntity>>, | ||
options?: Options, | ||
): Promise<TargetEntity | undefined> { | ||
const targetRepository = await this.getTargetRepository(); | ||
const found = await targetRepository.find( | ||
Object.assign({limit: 1}, constrainFilter(filter, this.constraint)), | ||
options, | ||
); | ||
// TODO: throw EntityNotFound error when target model instance not found | ||
return found[0]; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to define
reason
twice in one function?The first one is on https://github.com/strongloop/loopback-next/pull/1879/files#diff-a42257687a98a97022f7eb63398d2269R68
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's block-scoped.