diff --git a/docs/docs/entity-helper.md b/docs/docs/entity-helper.md index 463de47d0def..e9b135adf698 100644 --- a/docs/docs/entity-helper.md +++ b/docs/docs/entity-helper.md @@ -59,22 +59,77 @@ wrap(book).assign({ meta: { foo: 4 } }); console.log(book.meta); // { foo: 4 } ``` -To get the same behavior as `mergeObjects` flag for m:1 and 1:1 references, enable the `updateNestedEntities` flag. +### Updating deep entity graph + +Since v5, `assign` allows updating deep entity graph by default. To update existing +entity, we need to provide its PK in the `data`, as well as to **load that entity first +into current context**. ```typescript -import { wrap } from '@mikro-orm/core'; +const book = await em.findOneOrFail(Book, 1, { populate: ['author'] }); + +// update existing book's author's name +wrap(book).assign({ + author: { + id: book.author.id, + name: 'New name...', + }, +}); +``` + +If we want to always update the entity, even without the entity PK being present +in `data`, we can use `updateByPrimaryKey: false`: + +```typescript +const book = await em.findOneOrFail(Book, 1, { populate: ['author'] }); + +// update existing book's author's name +wrap(book).assign({ + author: { + name: 'New name...', + }, +}, { updateByPrimaryKey: false }); +``` + +Otherwise the entity data without PK are considered as new entity, and will trigger +insert query: + +```typescript +const book = await em.findOneOrFail(Book, 1, { populate: ['author'] }); + +// creating new author for given book +wrap(book).assign({ + author: { + name: 'New name...', + }, +}); +``` + +Same applies to the case when we do not load the child entity first into the context, +e.g. when we try to assign to a relation that was not populated. Even if we provide +its PK, it will be considered as new object and trigger an insert query. + +```typescript +const book = await em.findOneOrFail(Book, 1); // author is not populated + +// creating new author for given book +wrap(book).assign({ + author: { + id: book.author.id, + name: 'New name...', + }, +}); +``` -const authorEntity = new Author('Jon2 Snow', 'snow3@wall.st'); -book.author = authorEntity; +When updating collections, we can either pass complete array of all items, or just +a single item - in such case, the new item will be appended to the existing items. -wrap(book).assign({ author: { name: 'Jon Snow2' } }, { updateNestedEntities: true }); -console.log(book.author); // { ... name: 'Jon Snow2' ... } -console.log(book.author === authorEntity) // true +```ts +// resets the addresses collection to a single item +wrap(user).assign({ addresses: [new Address(...)] }); -//this will have no influence as author is an entity -wrap(book).assign({ author: { name: 'Jon Snow2' } }, { mergeObjects: true }); -console.log(book.author); // { ... name: 'Jon Snow2' ... } -console.log(book.author === authorEntity) // false +// adds new address to the collection +wrap(user).assign({ addresses: new Address(...) }); ``` ## `WrappedEntity` and `wrap()` helper diff --git a/packages/core/src/entity/EntityAssigner.ts b/packages/core/src/entity/EntityAssigner.ts index 868cc2645a82..62d780f2a86e 100644 --- a/packages/core/src/entity/EntityAssigner.ts +++ b/packages/core/src/entity/EntityAssigner.ts @@ -1,7 +1,7 @@ import { inspect } from 'util'; import { Collection } from './Collection'; import { EntityManager } from '../EntityManager'; -import { AnyEntity, EntityData, EntityDTO, EntityMetadata, EntityProperty } from '../typings'; +import { AnyEntity, EntityData, EntityDTO, EntityMetadata, EntityProperty, Primary } from '../typings'; import { Utils } from '../utils/Utils'; import { Reference } from './Reference'; import { ReferenceType, SCALAR_TYPES } from '../enums'; @@ -12,10 +12,12 @@ const validator = new EntityValidator(false); export class EntityAssigner { - static assign>(entity: T, data: EntityData | Partial>, options?: AssignOptions): T; - static assign>(entity: T, data: EntityData | Partial>, onlyProperties?: boolean): T; - static assign>(entity: T, data: EntityData | Partial>, onlyProperties: AssignOptions | boolean = false): T { - const options = (typeof onlyProperties === 'boolean' ? { onlyProperties } : onlyProperties); + static assign>(entity: T, data: EntityData | Partial>, options: AssignOptions = {}): T { + options = { + updateNestedEntities: true, + updateByPrimaryKey: true, + ...options, + }; const wrapped = entity.__helper!; const meta = entity.__meta!; const em = options.em || wrapped.__em; @@ -45,9 +47,22 @@ export class EntityAssigner { if ([ReferenceType.MANY_TO_ONE, ReferenceType.ONE_TO_ONE].includes(props[prop]?.reference) && Utils.isDefined(value, true) && EntityAssigner.validateEM(em)) { // eslint-disable-next-line no-prototype-builtins - if (options.updateNestedEntities && entity.hasOwnProperty(prop) && (Utils.isEntity(entity[prop]) || Reference.isReference(entity[prop])) && Utils.isPlainObject(value)) { + if (options.updateNestedEntities && entity.hasOwnProperty(prop) && Utils.isEntity(entity[prop], true) && Utils.isPlainObject(value)) { const unwrappedEntity = Reference.unwrapReference(entity[prop]); + if (options.updateByPrimaryKey) { + const pk = Utils.extractPK(value, props[prop].targetMeta); + + if (pk) { + const ref = em.getReference(props[prop].type, pk as Primary); + if (ref.__helper!.isInitialized()) { + return EntityAssigner.assign(ref, value, options); + } + } + + return EntityAssigner.assignReference(entity, value, props[prop], em!, options); + } + if (wrap(unwrappedEntity).isInitialized()) { return EntityAssigner.assign(unwrappedEntity, value, options); } @@ -125,8 +140,23 @@ export class EntityAssigner { private static assignCollection, U extends AnyEntity = AnyEntity>(entity: T, collection: Collection, value: any[], prop: EntityProperty, em: EntityManager, options: AssignOptions): void { const invalid: any[] = []; const items = value.map((item: any, idx) => { + if (options.updateNestedEntities && options.updateByPrimaryKey && Utils.isPlainObject(item)) { + const pk = Utils.extractPK(item, prop.targetMeta); + + if (pk) { + const ref = em.getReference(prop.type, pk as Primary); + + /* istanbul ignore else */ + if (ref.__helper!.isInitialized()) { + return EntityAssigner.assign(ref, item as U, options); + } + } + + return this.createCollectionItem(item, em, prop, invalid, options); + } + /* istanbul ignore next */ - if (options.updateNestedEntities && collection[idx]?.__helper!.isInitialized()) { + if (options.updateNestedEntities && !options.updateByPrimaryKey && collection[idx]?.__helper!.isInitialized()) { return EntityAssigner.assign(collection[idx], item, options); } @@ -203,6 +233,7 @@ export const assign = EntityAssigner.assign; export interface AssignOptions { updateNestedEntities?: boolean; + updateByPrimaryKey?: boolean; onlyProperties?: boolean; convertCustomTypes?: boolean; mergeObjects?: boolean; diff --git a/packages/core/src/typings.ts b/packages/core/src/typings.ts index ed28c1d8d242..a4546b52544a 100644 --- a/packages/core/src/typings.ts +++ b/packages/core/src/typings.ts @@ -87,7 +87,7 @@ export interface IWrappedEntity, PK extends keyof T | unk toObject(ignoreFields?: string[]): EntityDTO; toJSON(...args: any[]): EntityDTO; toPOJO(): EntityDTO; - assign(data: any, options?: AssignOptions | boolean): T; + assign(data: EntityData | Partial>, options?: AssignOptions | boolean): T; } export interface IWrappedEntityInternal, P = keyof T> extends IWrappedEntity { @@ -136,7 +136,9 @@ export type EntityDataProp = T extends Scalar ? EntityDataNested : T extends Collection ? U | U[] | EntityDataNested | EntityDataNested[] - : EntityDataNested; + : T extends readonly (infer U)[] + ? U | U[] | EntityDataNested | EntityDataNested[] + : EntityDataNested; export type EntityDataNested = T extends undefined ? never diff --git a/tests/bootstrap.ts b/tests/bootstrap.ts index 8115a3da80c8..ec9174538090 100644 --- a/tests/bootstrap.ts +++ b/tests/bootstrap.ts @@ -1,5 +1,5 @@ import 'reflect-metadata'; -import { EntityManager, JavaScriptMetadataProvider, LoadStrategy, MikroORM, Options, Utils } from '@mikro-orm/core'; +import { EntityManager, JavaScriptMetadataProvider, LoadStrategy, Logger, LoggerNamespace, MikroORM, Options, Utils } from '@mikro-orm/core'; import { AbstractSqlDriver, SchemaGenerator, SqlEntityManager, SqlEntityRepository } from '@mikro-orm/knex'; import { SqliteDriver } from '@mikro-orm/sqlite'; import { MongoDriver } from '@mikro-orm/mongodb'; @@ -243,3 +243,11 @@ export async function wipeDatabaseSqlite2(em: SqlEntityManager) { await em.execute('pragma foreign_keys = on'); em.clear(); } + +export function mockLogger(orm: MikroORM, debugMode: LoggerNamespace[] = ['query', 'query-params']) { + const mock = jest.fn(); + const logger = new Logger(mock, debugMode); + Object.assign(orm.config, { logger }); + + return mock; +} diff --git a/tests/features/embeddables/embedded-entities.postgres.test.ts b/tests/features/embeddables/embedded-entities.postgres.test.ts index 1d2ea28ea615..f9b7c35a9cb3 100644 --- a/tests/features/embeddables/embedded-entities.postgres.test.ts +++ b/tests/features/embeddables/embedded-entities.postgres.test.ts @@ -298,7 +298,7 @@ describe('embedded entities in postgresql', () => { const user = new User(); wrap(user).assign({ address1: { street: 'Downing street 10', number: 3, postalCode: '123', city: 'London 1', country: 'UK 1' }, - address2: { street: 'Downing street 11', number: 3, city: 'London 2', country: 'UK 2' }, + address2: { street: 'Downing street 11', city: 'London 2', country: 'UK 2' }, address3: { street: 'Downing street 12', number: 3, postalCode: '789', city: 'London 3', country: 'UK 3' }, address4: { street: 'Downing street 10', number: 3, postalCode: '123', city: 'London 1', country: 'UK 1' }, }, { em: orm.em }); diff --git a/tests/EntityAssigner.mongo.test.ts b/tests/features/entity-assigner/EntityAssigner.mongo.test.ts similarity index 94% rename from tests/EntityAssigner.mongo.test.ts rename to tests/features/entity-assigner/EntityAssigner.mongo.test.ts index 6998f91393ad..08145eef67b6 100644 --- a/tests/EntityAssigner.mongo.test.ts +++ b/tests/features/entity-assigner/EntityAssigner.mongo.test.ts @@ -1,7 +1,7 @@ import { assign, EntityData, expr, MikroORM, wrap } from '@mikro-orm/core'; import { MongoDriver, ObjectId } from '@mikro-orm/mongodb'; -import { Author, Book, BookTag } from './entities'; -import { initORMMongo, wipeDatabase } from './bootstrap'; +import { Author, Book, BookTag } from '../../entities'; +import { initORMMongo, wipeDatabase } from '../../bootstrap'; describe('EntityAssignerMongo', () => { @@ -71,9 +71,9 @@ describe('EntityAssignerMongo', () => { test('#assign() should merge collection items', async () => { const jon = new Author('Jon Snow', 'snow@wall.st'); - orm.em.assign(jon, { books: [{ _id: ObjectId.createFromTime(1), title: 'b1' }] }, { merge: false }); + orm.em.assign(jon, { books: [{ _id: ObjectId.createFromTime(1), title: 'b1' }] }, { merge: false, updateNestedEntities: false }); expect(wrap(jon.books[0], true).__em).toBeUndefined(); - orm.em.assign(jon, { books: [{ _id: ObjectId.createFromTime(2), title: 'b2' }] }, { merge: true }); + orm.em.assign(jon, { books: [{ _id: ObjectId.createFromTime(2), title: 'b2' }] }, { merge: true, updateNestedEntities: false }); expect(wrap(jon.books[0], true).__em).not.toBeUndefined(); }); diff --git a/tests/EntityAssigner.mysql.test.ts b/tests/features/entity-assigner/EntityAssigner.mysql.test.ts similarity index 95% rename from tests/EntityAssigner.mysql.test.ts rename to tests/features/entity-assigner/EntityAssigner.mysql.test.ts index 2514234920a1..7bd8c957a6b0 100644 --- a/tests/EntityAssigner.mysql.test.ts +++ b/tests/features/entity-assigner/EntityAssigner.mysql.test.ts @@ -1,7 +1,7 @@ -import { EntityData, MikroORM, Reference, wrap } from '@mikro-orm/core'; +import { MikroORM, Reference, wrap } from '@mikro-orm/core'; import { MySqlDriver } from '@mikro-orm/mysql'; -import { initORMMySql, wipeDatabaseMySql } from './bootstrap'; -import { Author2, Book2, BookTag2, FooBar2, Publisher2, PublisherType } from './entities-sql'; +import { initORMMySql, wipeDatabaseMySql } from '../../bootstrap'; +import { Author2, Book2, BookTag2, FooBar2, Publisher2, PublisherType } from '../../entities-sql'; describe('EntityAssignerMySql', () => { @@ -17,6 +17,7 @@ describe('EntityAssignerMySql', () => { await orm.em.persistAndFlush(book); expect(book.title).toBe('Book2'); expect(book.author).toBe(jon); + // @ts-expect-error wrap(book).assign({ title: 'Better Book2 1', author: god, notExisting: true }); expect(book.author).toBe(god); expect((book as any).notExisting).toBe(true); @@ -30,22 +31,27 @@ describe('EntityAssignerMySql', () => { test('assign() should fix property types [mysql]', async () => { const god = new Author2('God', 'hello@heaven.god'); + // @ts-expect-error wrap(god).assign({ createdAt: '2018-01-01', termsAccepted: 1 }); expect(god.createdAt).toEqual(new Date('2018-01-01')); expect(god.termsAccepted).toBe(true); const d1 = +new Date('2018-01-01'); + // @ts-expect-error wrap(god).assign({ createdAt: '' + d1, termsAccepted: 0 }); expect(god.createdAt).toEqual(new Date('2018-01-01')); expect(god.termsAccepted).toBe(false); + // @ts-expect-error wrap(god).assign({ createdAt: d1, termsAccepted: 0 }); expect(god.createdAt).toEqual(new Date('2018-01-01')); const d2 = +new Date('2018-01-01 00:00:00.123'); + // @ts-expect-error wrap(god).assign({ createdAt: '' + d2 }); expect(god.createdAt).toEqual(new Date('2018-01-01 00:00:00.123')); + // @ts-expect-error wrap(god).assign({ createdAt: d2 }); expect(god.createdAt).toEqual(new Date('2018-01-01 00:00:00.123')); }); @@ -88,7 +94,7 @@ describe('EntityAssignerMySql', () => { expect(book1.author.email).toBeUndefined(); expect(book1.author).not.toEqual(jon); - wrap(book2).assign({ author: { name: 'Jon Snow2' } }, { updateNestedEntities: true }); + wrap(book2).assign({ author: { name: 'Jon Snow2' } }, { updateByPrimaryKey: false }); expect(book2.author.name).toEqual('Jon Snow2'); expect(book2.author.email).toEqual('snow3@wall.st'); expect(book2.author).toEqual(jon2); @@ -147,7 +153,7 @@ describe('EntityAssignerMySql', () => { const originalRef = book2.publisher!; expect(originalValue.name).toEqual('Good Books LLC'); - wrap(book2).assign({ author: { name: 'Jon Snow2' }, publisher: { name: 'Better Books LLC' } }, { updateNestedEntities: true }); + wrap(book2).assign({ author: { name: 'Jon Snow2' }, publisher: { name: 'Better Books LLC' } }, { updateByPrimaryKey: false }); // this means that the original object has been replaced, something updateNestedEntities does not do expect(book2.publisher).toEqual(originalRef); diff --git a/tests/features/entity-assigner/GH1811.test.ts b/tests/features/entity-assigner/GH1811.test.ts new file mode 100644 index 000000000000..556bbe1d44e9 --- /dev/null +++ b/tests/features/entity-assigner/GH1811.test.ts @@ -0,0 +1,204 @@ +import { Collection, Entity, ManyToMany, ManyToOne, MikroORM, OneToMany, PrimaryKey, Property, t, wrap } from '@mikro-orm/core'; +import { SqliteDriver } from '@mikro-orm/sqlite'; +import { v4 } from 'uuid'; +import { mockLogger } from '../../bootstrap'; + +@Entity() +export class Recipe { + + @PrimaryKey({ type: t.uuid }) + id: string = v4(); + + @Property() + name!: string; + + // eslint-disable-next-line @typescript-eslint/no-use-before-define + @OneToMany({ entity: () => Ingredient, mappedBy: 'recipe', orphanRemoval: true }) + ingredients = new Collection(this); + + // eslint-disable-next-line @typescript-eslint/no-use-before-define + @ManyToMany({ entity: () => User }) + authors = new Collection(this); + +} + +@Entity() +export class Ingredient { + + @PrimaryKey({ type: t.uuid }) + id: string = v4(); + + @ManyToOne({ entity: () => Recipe }) + recipe!: Recipe; + + @Property() + name!: string; + +} + +@Entity() +export class User { + + @PrimaryKey({ type: t.uuid }) + id: string = v4(); + + @Property() + name!: string; + +} + +describe('GH issue 1811', () => { + + let orm: MikroORM; + + beforeAll(async () => { + orm = await MikroORM.init({ + entities: [Ingredient, Recipe, User], + dbName: ':memory:', + type: 'sqlite', + }); + await orm.getSchemaGenerator().createSchema(); + }); + + afterAll(async () => { + await orm.close(true); + }); + + beforeEach(async () => { + await orm.em.createQueryBuilder(Ingredient).truncate().execute(); + await orm.em.createQueryBuilder(User).truncate().execute(); + await orm.em.createQueryBuilder(Recipe).truncate().execute(); + orm.em.clear(); + }); + + async function createRecipe() { + const recipe = orm.em.create(Recipe, { + id: '3ea3e69c-c221-41b1-a14e-135ef3141ea3', + name: 'Salad', + ingredients: [ + { id: '16e443a7-b527-493f-ab8a-63012514b719', name: 'Spinach' }, + { id: '1d4876ce-01de-4ba8-9d17-3c1c0d56fe73', name: 'Carrot' }, + ], + authors: [ + { id: '22222222-0000-493f-ab8a-03012514b719', name: 'Alice' }, + { id: '33333333-0000-4ba8-9d17-1c1c0d56fe73', name: 'Bob' }, + ], + }); + await orm.em.persistAndFlush(recipe); + orm.em.clear(); + + return recipe; + } + + test('assigning to 1:m with nested entities', async () => { + const r = await createRecipe(); + const recipe = await orm.em.findOneOrFail(Recipe, r, ['ingredients']); + + wrap(recipe).assign({ + ingredients: [ + { id: '16e443a7-b527-493f-ab8a-63012514b719', name: 'Spinach' }, // existing + { name: 'Onion' }, // new, should be created + // carrot should be dropped, id: '1d4876ce-01de-4ba8-9d17-3c1c0d56fe73', + ], + }); + expect(recipe.ingredients.toArray()).toEqual([ + { id: '16e443a7-b527-493f-ab8a-63012514b719', name: 'Spinach', recipe: recipe.id }, + { id: expect.stringMatching(/[\w-]{36}/), name: 'Onion', recipe: recipe.id }, + ]); + + const mock = mockLogger(orm, ['query']); + await orm.em.flush(); + + expect(mock.mock.calls[0][0]).toMatch('begin'); + expect(mock.mock.calls[1][0]).toMatch('insert into `ingredient` (`id`, `name`, `recipe_id`) values (?, ?, ?)'); + expect(mock.mock.calls[2][0]).toMatch('delete from `ingredient` where `id` in (?)'); + expect(mock.mock.calls[3][0]).toMatch('commit'); + + const r1 = await orm.em.fork().findOneOrFail(Recipe, recipe, { populate: ['ingredients'] }); + expect(r1.ingredients.toArray()).toEqual([ + { id: '16e443a7-b527-493f-ab8a-63012514b719', name: 'Spinach', recipe: recipe.id }, + { id: expect.stringMatching(/[\w-]{36}/), name: 'Onion', recipe: recipe.id }, + ]); + }); + + test('assigning to m:1 with nested entities', async () => { + const r = await createRecipe(); + const recipe = await orm.em.findOneOrFail(Recipe, r, ['ingredients']); + const onion = wrap(recipe.ingredients[1]).assign({ + name: 'Onion', + recipe: { name: 'Scrambled eggs' }, // should create new recipe entity + }); + const mock = mockLogger(orm, ['query']); + await orm.em.flush(); + + expect(mock.mock.calls[0][0]).toMatch('begin'); + expect(mock.mock.calls[1][0]).toMatch('insert into `recipe` (`id`, `name`) values (?, ?)'); + expect(mock.mock.calls[2][0]).toMatch('update `ingredient` set `recipe_id` = ?, `name` = ? where `id` = ?'); + expect(mock.mock.calls[3][0]).toMatch('commit'); + + const r2 = await orm.em.fork().find(Recipe, {}, { populate: ['ingredients'] }); + expect(r2.map(r => r.name)).toEqual(['Salad', 'Scrambled eggs']); + expect(r2[0].ingredients.toArray()).toEqual([ + { id: '16e443a7-b527-493f-ab8a-63012514b719', name: 'Spinach', recipe: recipe.id }, + ]); + expect(r2[1].ingredients.toArray()).toEqual([ + { id: onion.id, name: 'Onion', recipe: r2[1].id }, + ]); + + wrap(onion).assign({ + name: 'Spring Onion', + recipe: { id: onion.recipe.id, name: 'Poached eggs' }, // should update existing recipe entity + }); + + mock.mock.calls.length = 0; + await orm.em.flush(); + + expect(mock.mock.calls[0][0]).toMatch('begin'); + expect(mock.mock.calls[1][0]).toMatch('update `recipe` set `name` = ? where `id` = ?'); + expect(mock.mock.calls[2][0]).toMatch('update `ingredient` set `name` = ? where `id` = ?'); + expect(mock.mock.calls[3][0]).toMatch('commit'); + + const r3 = await orm.em.fork().find(Recipe, {}, { populate: ['ingredients'] }); + expect(r3.map(r => r.name)).toEqual(['Salad', 'Poached eggs']); + expect(r3[0].ingredients.toArray()).toEqual([ + { id: '16e443a7-b527-493f-ab8a-63012514b719', name: 'Spinach', recipe: recipe.id }, + ]); + expect(r3[1].ingredients.toArray()).toEqual([ + { id: onion.id, name: 'Spring Onion', recipe: r3[1].id }, + ]); + }); + + test('assigning to m:m with nested entities', async () => { + const r = await createRecipe(); + const recipe = await orm.em.findOneOrFail(Recipe, r, ['authors']); + + wrap(recipe).assign({ + authors: [ + { id: '22222222-0000-493f-ab8a-03012514b719', name: 'Malice' }, // existing + { name: 'Tom' }, // new, should be created + // Bob should be dropped + ], + }); + expect(recipe.authors.toArray()).toEqual([ + { id: '22222222-0000-493f-ab8a-03012514b719', name: 'Malice' }, + { id: expect.stringMatching(/[\w-]{36}/), name: 'Tom' }, + ]); + + const mock = mockLogger(orm, ['query']); + await orm.em.flush(); + + expect(mock.mock.calls[0][0]).toMatch('begin'); + expect(mock.mock.calls[1][0]).toMatch('insert into `user` (`id`, `name`) values (?, ?)'); + expect(mock.mock.calls[2][0]).toMatch('update `user` set `name` = ? where `id` = ?'); + expect(mock.mock.calls[3][0]).toMatch('delete from `recipe_authors` where (`user_id`) in ( values (?)) and `recipe_id` = ?'); + expect(mock.mock.calls[4][0]).toMatch('insert into `recipe_authors` (`recipe_id`, `user_id`) values (?, ?)'); + expect(mock.mock.calls[5][0]).toMatch('commit'); + + const r1 = await orm.em.fork().findOneOrFail(Recipe, recipe, { populate: ['authors'], orderBy: { authors: { name: 'asc' } } }); + expect(r1.authors.toArray()).toEqual([ + { id: '22222222-0000-493f-ab8a-03012514b719', name: 'Malice' }, + { id: expect.stringMatching(/[\w-]{36}/), name: 'Tom' }, + ]); + }); + +});