From 4bfde8c102927a4296ebcec91889735fff5518f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ad=C3=A1mek?= Date: Wed, 27 Mar 2019 15:00:13 +0100 Subject: [PATCH] feat(core): implement Cascade.MERGE and Cascade.ALL This also renames UoW.addToIdentityMap() to UoW.merge(). Closes #16 --- docs/cascading.md | 49 ++++++++++++++++++++-- lib/EntityManager.ts | 31 +++++++------- lib/MikroORM.ts | 2 +- lib/decorators/ManyToMany.ts | 2 +- lib/decorators/ManyToOne.ts | 2 +- lib/decorators/OneToMany.ts | 2 +- lib/drivers/AbstractSqlDriver.ts | 2 +- lib/drivers/PostgreSqlDriver.ts | 2 +- lib/entity/EntityValidator.ts | 10 ++++- lib/entity/enums.ts | 2 + lib/metadata/JavaScriptMetadataProvider.ts | 2 +- lib/metadata/MetadataDiscovery.ts | 2 +- lib/schema/SchemaGenerator.ts | 5 ++- lib/unit-of-work/ChangeSetPersister.ts | 2 +- lib/unit-of-work/UnitOfWork.ts | 15 ++++--- lib/utils/ValidationError.ts | 4 ++ tests/EntityManager.mongo.test.ts | 2 +- tests/UnitOfWork.test.ts | 2 +- 18 files changed, 97 insertions(+), 41 deletions(-) diff --git a/docs/cascading.md b/docs/cascading.md index 2210f9487995..c56b480056f5 100644 --- a/docs/cascading.md +++ b/docs/cascading.md @@ -1,7 +1,7 @@ --- --- -# Cascading persist and remove +# Cascading persist, merge and remove When persisting or removing entity, all your references are by default cascade persisted. This means that by persisting any entity, ORM will automatically persist all of its @@ -13,25 +13,33 @@ You can control this behaviour via `cascade` attribute of `@ManyToOne`, `@ManyTo > New entities without primary key will be always persisted, regardless of `cascade` value. ```typescript -// cascade persist is default value +// cascade persist & merge is default value @OneToMany({ entity: () => Book, fk: 'author' }) books = new Collection(this); // same as previous definition -@OneToMany({ entity: () => Book, fk: 'author', cascade: [Cascade.PERSIST] }) +@OneToMany({ entity: () => Book, fk: 'author', cascade: [Cascade.PERSIST, Cascade.MERGE] }) books = new Collection(this); // only cascade remove @OneToMany({ entity: () => Book, fk: 'author', cascade: [Cascade.REMOVE] }) books = new Collection(this); -// cascade persist and remove +// cascade persist and remove (but not merge) @OneToMany({ entity: () => Book, fk: 'author', cascade: [Cascade.PERSIST, Cascade.REMOVE] }) books = new Collection(this); // no cascade @OneToMany({ entity: () => Book, fk: 'author', cascade: [] }) books = new Collection(this); + +// cascade all (persist, merge and remove) +@OneToMany({ entity: () => Book, fk: 'author', cascade: [Cascade.ALL] }) +books = new Collection(this); + +// same as previous definition +@OneToMany({ entity: () => Book, fk: 'author', cascade: [Cascade.PERSIST, Cascade.MERGE, Cascade.REMOVE] }) +books = new Collection(this); ``` ## Cascade persist @@ -49,6 +57,39 @@ await orm.em.persist(book); // all book tags and author will be persisted too > When cascade persisting collections, keep in mind only fully initialized collections > will be cascade persisted. +## Cascade merge + +When you want to merge entity and all its associations, you can use `Cascade.MERGE`. This +comes handy when you want to clear identity map (e.g. when importing large number of entities), +but you also have to keep your parent entities managed (because otherwise they would be considered +as new entities and insert-persisted, which would fail with non-unique identifier). + +In following example, without having `Author.favouriteBook` set to cascade merge, you would +get an error because it would be cascade-inserted with already taken ID. + +```typescript +const a1 = new Author(...); +a1.favouriteBook = new Book('the best', ...); +await orm.em.persistAndFlush(a1); // cascade persists favourite book as well + +for (let i = 1; i < 1000; i++) { + const book = new Book('...', a1); + orm.em.persistLater(book); + + // persist every 100 records + if (i % 100 === 0) { + await orm.em.flush(); + orm.em.clear(); // this makes both a1 and his favourite book detached + orm.em.merge(a1); // so we need to merge them to prevent cascade-inserts + + // without cascade merge, you would need to manually merge all his associations + orm.em.merge(a1.favouriteBook); // not needed with Cascade.MERGE + } +} + +await orm.em.flush(); +``` + ## Cascade remove Cascade remove works same way as cascade persist, just for removing entities. Following diff --git a/lib/EntityManager.ts b/lib/EntityManager.ts index 6f4204361086..3b6fa001fd43 100644 --- a/lib/EntityManager.ts +++ b/lib/EntityManager.ts @@ -1,11 +1,11 @@ import { Configuration, RequestContext, Utils } from './utils'; -import { EntityRepository, EntityAssigner, EntityFactory, EntityLoader, EntityValidator, ReferenceType } from './entity'; +import { EntityAssigner, EntityFactory, EntityLoader, EntityRepository, EntityValidator, ReferenceType } from './entity'; import { UnitOfWork } from './unit-of-work'; -import { FilterQuery, IDatabaseDriver } from './drivers/IDatabaseDriver'; +import { FilterQuery, IDatabaseDriver } from './drivers'; import { EntityData, EntityName, IEntity, IEntityType, IPrimaryKey } from './decorators'; import { QueryBuilder, QueryOrder, SmartQueryHelper } from './query'; import { MetadataStorage } from './metadata'; -import { Connection } from './connections/Connection'; +import { Connection } from './connections'; export class EntityManager { @@ -158,18 +158,21 @@ export class EntityManager { return this.driver.aggregate(entityName, pipeline); } - merge>(entityName: EntityName, data: EntityData): T { - entityName = Utils.className(entityName); - const meta = this.metadata[entityName]; - - if (!data || (!data[meta.primaryKey] && !data[meta.serializedPrimaryKey])) { - throw new Error('You cannot merge entity without identifier!'); + merge>(entity: T): T; + merge>(entityName: EntityName, data: EntityData): T; + merge>(entityName: EntityName | T, data?: EntityData): T { + if (Utils.isEntity(entityName)) { + return this.merge(entityName.constructor.name, entityName as EntityData); } - const entity = Utils.isEntity(data) ? data : this.getEntityFactory().create(entityName, data, true); - this.getUnitOfWork().addToIdentityMap(entity); // add to IM immediately - needed for self-references that can be part of `data` - EntityAssigner.assign(entity, data, true); - this.getUnitOfWork().addToIdentityMap(entity); // add to IM again so we have correct payload saved to change set computation + entityName = Utils.className(entityName); + this.validator.validatePrimaryKey(data!, this.metadata[entityName]); + const entity = Utils.isEntity(data) ? data : this.getEntityFactory().create(entityName, data!, true); + + // add to IM immediately - needed for self-references that can be part of `data` (and do not trigger cascade merge) + this.getUnitOfWork().merge(entity, [entity]); + EntityAssigner.assign(entity, data!, true); + this.getUnitOfWork().merge(entity); // add to IM again so we have correct payload saved to change set computation return entity; } @@ -192,7 +195,7 @@ export class EntityManager { } const entity = this.getEntityFactory().createReference(entityName, id); - this.getUnitOfWork().addToIdentityMap(entity); + this.getUnitOfWork().merge(entity); return entity; } diff --git a/lib/MikroORM.ts b/lib/MikroORM.ts index 4a5b47ba006f..b45eb5014613 100644 --- a/lib/MikroORM.ts +++ b/lib/MikroORM.ts @@ -1,5 +1,5 @@ import { EntityManager } from './EntityManager'; -import { IDatabaseDriver } from './drivers/IDatabaseDriver'; +import { IDatabaseDriver } from './drivers'; import { MetadataDiscovery } from './metadata'; import { Configuration, Logger, Options } from './utils'; import { EntityMetadata } from './decorators'; diff --git a/lib/decorators/ManyToMany.ts b/lib/decorators/ManyToMany.ts index 55e551b17382..941f7b1522af 100644 --- a/lib/decorators/ManyToMany.ts +++ b/lib/decorators/ManyToMany.ts @@ -14,7 +14,7 @@ export function ManyToMany(options: ManyToManyOptions): Function { throw new Error(`'@ManyToMany({ entity: string | Function })' is required in '${target.constructor.name}.${propertyName}'`); } - const property = { name: propertyName, reference: ReferenceType.MANY_TO_MANY, owner: !!options.inversedBy, cascade: [Cascade.PERSIST] }; + const property = { name: propertyName, reference: ReferenceType.MANY_TO_MANY, owner: !!options.inversedBy, cascade: [Cascade.PERSIST, Cascade.MERGE] }; meta.properties[propertyName] = Object.assign(property, options) as EntityProperty; }; } diff --git a/lib/decorators/ManyToOne.ts b/lib/decorators/ManyToOne.ts index 3ba79276aa8f..896f303220da 100644 --- a/lib/decorators/ManyToOne.ts +++ b/lib/decorators/ManyToOne.ts @@ -8,7 +8,7 @@ export function ManyToOne(options: ManyToOneOptions = {}): Function { return function (target: IEntity, propertyName: string) { const meta = MetadataStorage.getMetadata(target.constructor.name); Utils.lookupPathFromDecorator(meta); - const property = { name: propertyName, reference: ReferenceType.MANY_TO_ONE, cascade: [Cascade.PERSIST] }; + const property = { name: propertyName, reference: ReferenceType.MANY_TO_ONE, cascade: [Cascade.PERSIST, Cascade.MERGE] }; meta.properties[propertyName] = Object.assign(property, options) as EntityProperty; }; } diff --git a/lib/decorators/OneToMany.ts b/lib/decorators/OneToMany.ts index d622ba724c45..bd1453bf3f1c 100644 --- a/lib/decorators/OneToMany.ts +++ b/lib/decorators/OneToMany.ts @@ -14,7 +14,7 @@ export function OneToMany(options: OneToManyOptions): Function { throw new Error(`'@OneToMany({ entity: string | Function })' is required in '${target.constructor.name}.${propertyName}'`); } - const property = { name: propertyName, reference: ReferenceType.ONE_TO_MANY, cascade: [Cascade.PERSIST] }; + const property = { name: propertyName, reference: ReferenceType.ONE_TO_MANY, cascade: [Cascade.PERSIST, Cascade.MERGE] }; meta.properties[propertyName] = Object.assign(property, options) as EntityProperty; }; } diff --git a/lib/drivers/AbstractSqlDriver.ts b/lib/drivers/AbstractSqlDriver.ts index 16288d30b839..fb1f6c497cfc 100644 --- a/lib/drivers/AbstractSqlDriver.ts +++ b/lib/drivers/AbstractSqlDriver.ts @@ -1,6 +1,6 @@ import { EntityData, IEntityType, IPrimaryKey } from '../decorators'; import { DatabaseDriver } from './DatabaseDriver'; -import { Connection, QueryResult } from '../connections/Connection'; +import { Connection, QueryResult } from '../connections'; import { ReferenceType } from '../entity'; import { FilterQuery } from './IDatabaseDriver'; import { QueryBuilder, QueryOrder } from '../query'; diff --git a/lib/drivers/PostgreSqlDriver.ts b/lib/drivers/PostgreSqlDriver.ts index 79515923871c..0c1489e9297c 100644 --- a/lib/drivers/PostgreSqlDriver.ts +++ b/lib/drivers/PostgreSqlDriver.ts @@ -3,7 +3,7 @@ import { AbstractSqlDriver } from './AbstractSqlDriver'; import { EntityData, IEntityType } from '../decorators'; import { QueryType } from '../query'; import { PostgreSqlPlatform } from '../platforms/PostgreSqlPlatform'; -import { QueryResult } from '../connections/Connection'; +import { QueryResult } from '../connections'; export class PostgreSqlDriver extends AbstractSqlDriver { diff --git a/lib/entity/EntityValidator.ts b/lib/entity/EntityValidator.ts index be3f89cfd370..c9a58f99367f 100644 --- a/lib/entity/EntityValidator.ts +++ b/lib/entity/EntityValidator.ts @@ -1,5 +1,5 @@ import { SCALAR_TYPES } from './EntityFactory'; -import { EntityMetadata, EntityProperty, IEntityType } from '../decorators'; +import { EntityData, EntityMetadata, EntityProperty, IEntityType } from '../decorators'; import { Utils, ValidationError } from '../utils'; import { ReferenceType } from './enums'; @@ -66,7 +66,13 @@ export class EntityValidator { } } - private validateCollection(entity: IEntityType, prop: EntityProperty): void { + validatePrimaryKey>(entity: EntityData, meta: EntityMetadata): void { + if (!entity || (!entity[meta.primaryKey] && !entity[meta.serializedPrimaryKey])) { + throw ValidationError.fromMergeWithoutPK(meta); + } + } + + private validateCollection>(entity: T, prop: EntityProperty): void { if (!entity[prop.name as keyof T]) { throw ValidationError.fromCollectionNotInitialized(entity, prop); } diff --git a/lib/entity/enums.ts b/lib/entity/enums.ts index 5799045f3ed5..5fbe28a33218 100644 --- a/lib/entity/enums.ts +++ b/lib/entity/enums.ts @@ -7,5 +7,7 @@ export enum ReferenceType { export enum Cascade { PERSIST = 'persist', + MERGE = 'merge', REMOVE = 'remove', + ALL = 'all', } diff --git a/lib/metadata/JavaScriptMetadataProvider.ts b/lib/metadata/JavaScriptMetadataProvider.ts index 565b1e3a00b1..45c8036515fe 100644 --- a/lib/metadata/JavaScriptMetadataProvider.ts +++ b/lib/metadata/JavaScriptMetadataProvider.ts @@ -45,7 +45,7 @@ export class JavaScriptMetadataProvider extends MetadataProvider { } if (prop.reference !== ReferenceType.SCALAR && typeof prop.cascade === 'undefined') { - prop.cascade = [Cascade.PERSIST]; + prop.cascade = [Cascade.PERSIST, Cascade.MERGE]; } } diff --git a/lib/metadata/MetadataDiscovery.ts b/lib/metadata/MetadataDiscovery.ts index 957a79bcb1ad..e8c7984ed3a7 100644 --- a/lib/metadata/MetadataDiscovery.ts +++ b/lib/metadata/MetadataDiscovery.ts @@ -219,7 +219,7 @@ export class MetadataDiscovery { } private definePivotProperty(prop: EntityProperty, name: string): EntityProperty { - const ret = { name, type: name, reference: ReferenceType.MANY_TO_ONE, cascade: [Cascade.PERSIST, Cascade.REMOVE] } as EntityProperty; + const ret = { name, type: name, reference: ReferenceType.MANY_TO_ONE, cascade: [Cascade.ALL] } as EntityProperty; if (name === prop.type) { const meta = this.metadata[name]; diff --git a/lib/schema/SchemaGenerator.ts b/lib/schema/SchemaGenerator.ts index 93c2f9b1ad9c..99117c11d732 100644 --- a/lib/schema/SchemaGenerator.ts +++ b/lib/schema/SchemaGenerator.ts @@ -127,9 +127,10 @@ export class SchemaGenerator { const meta2 = this.metadata[prop.type]; const pk2 = meta2.properties[meta2.primaryKey].fieldName; let ret = `REFERENCES ${this.helper.quoteIdentifier(meta2.collection)} (${this.helper.quoteIdentifier(pk2)})`; - ret += ` ON DELETE ${prop.cascade.includes(Cascade.REMOVE) ? 'CASCADE' : 'SET NULL'}`; + const cascade = prop.cascade.includes(Cascade.REMOVE) || prop.cascade.includes(Cascade.ALL); + ret += ` ON DELETE ${cascade ? 'CASCADE' : 'SET NULL'}`; - if (prop.cascade.includes(Cascade.PERSIST)) { + if (prop.cascade.includes(Cascade.PERSIST) || prop.cascade.includes(Cascade.ALL)) { ret += ' ON UPDATE CASCADE'; } diff --git a/lib/unit-of-work/ChangeSetPersister.ts b/lib/unit-of-work/ChangeSetPersister.ts index 8567351afb87..7d71691ec69f 100644 --- a/lib/unit-of-work/ChangeSetPersister.ts +++ b/lib/unit-of-work/ChangeSetPersister.ts @@ -3,7 +3,7 @@ import { EntityMetadata, EntityProperty, IEntityType } from '../decorators'; import { EntityIdentifier } from '../entity'; import { ChangeSet, ChangeSetType } from './ChangeSet'; import { IDatabaseDriver } from '..'; -import { QueryResult } from '../connections/Connection'; +import { QueryResult } from '../connections'; export class ChangeSetPersister { diff --git a/lib/unit-of-work/UnitOfWork.ts b/lib/unit-of-work/UnitOfWork.ts index ea8e853c0325..542feb62eb1d 100644 --- a/lib/unit-of-work/UnitOfWork.ts +++ b/lib/unit-of-work/UnitOfWork.ts @@ -30,12 +30,10 @@ export class UnitOfWork { constructor(private readonly em: EntityManager) { } - addToIdentityMap(entity: IEntity, initialized = true): void { + merge>(entity: T, visited: IEntity[] = []): void { this.identityMap[`${entity.constructor.name}-${entity.__serializedPrimaryKey}`] = entity; - - if (initialized) { - this.originalEntityData[entity.__uuid] = Utils.copy(entity); - } + this.originalEntityData[entity.__uuid] = Utils.copy(entity); + this.cascade(entity, Cascade.MERGE, visited); } getById>(entityName: string, id: IPrimaryKey): T { @@ -179,7 +177,7 @@ export class UnitOfWork { await this.changeSetPersister.persistToDatabase(changeSet); if (changeSet.type !== ChangeSetType.DELETE) { - this.em.merge(changeSet.name, changeSet.entity); + this.em.merge(changeSet.entity); } await this.runHooks(`after${type}`, changeSet.entity); @@ -234,7 +232,8 @@ export class UnitOfWork { visited.push(entity); switch (type) { - case Cascade.PERSIST: this.persist(entity, visited); break; + case Cascade.PERSIST: this.persist(entity, visited); break; + case Cascade.MERGE: this.merge(entity, visited); break; case Cascade.REMOVE: this.remove(entity, visited); break; } @@ -246,7 +245,7 @@ export class UnitOfWork { } private cascadeReference>(entity: T, prop: EntityProperty, type: Cascade, visited: IEntity[]): void { - if (!prop.cascade || !prop.cascade.includes(type)) { + if (!prop.cascade || !(prop.cascade.includes(type) || prop.cascade.includes(Cascade.ALL))) { return; } diff --git a/lib/utils/ValidationError.ts b/lib/utils/ValidationError.ts index d65ee75e3f62..c88a2e85570a 100644 --- a/lib/utils/ValidationError.ts +++ b/lib/utils/ValidationError.ts @@ -62,6 +62,10 @@ export class ValidationError extends Error { return ValidationError.fromMessage(meta, prop, `needs to have one of 'owner', 'mappedBy' or 'inversedBy' attributes`); } + static fromMergeWithoutPK(meta: EntityMetadata): void { + throw new Error(`You cannot merge entity '${meta.name}' without identifier!`); + } + private static fromMessage(meta: EntityMetadata, prop: EntityProperty, message: string): ValidationError { return new ValidationError(`${meta.name}.${prop.name} ${message}`); } diff --git a/tests/EntityManager.mongo.test.ts b/tests/EntityManager.mongo.test.ts index 1e04e69c2ffe..52932969d09e 100644 --- a/tests/EntityManager.mongo.test.ts +++ b/tests/EntityManager.mongo.test.ts @@ -199,7 +199,7 @@ describe('EntityManagerMongo', () => { test('should throw when trying to merge entity without id', async () => { const author = new Author('test', 'test'); - expect(() => orm.em.merge(Author, author)).toThrowError('You cannot merge entity without identifier!'); + expect(() => orm.em.merge(author)).toThrowError(`You cannot merge entity 'Author' without identifier!`); }); test('fork', async () => { diff --git a/tests/UnitOfWork.test.ts b/tests/UnitOfWork.test.ts index f0a8bcce8c81..8e02c4e11c1d 100644 --- a/tests/UnitOfWork.test.ts +++ b/tests/UnitOfWork.test.ts @@ -76,7 +76,7 @@ describe('UnitOfWork', () => { test('changeSet is null for empty payload', async () => { const author = new Author('test', 'test'); author.id = '00000001885f0a3cc37dc9f0'; - uow.addToIdentityMap(author); // add entity to IM first + uow.merge(author); // add entity to IM first const changeSet = await computer.computeChangeSet(author); // then try to persist it again expect(changeSet).toBeNull(); expect(uow.getIdentityMap()).not.toEqual({});