Skip to content

Commit

Permalink
fix(core): fix assign on collections of unloaded entities
Browse files Browse the repository at this point in the history
Calling `em.assign(ref, { collection: [...] })` previously worked only as a side effect,
this commit implements proper handling for missing collection instance, which in turn
allows proper orphan removal functionality when using a reference.
  • Loading branch information
B4nan committed Dec 21, 2023
1 parent c848f8c commit b60e4ee
Show file tree
Hide file tree
Showing 2 changed files with 270 additions and 4 deletions.
13 changes: 9 additions & 4 deletions packages/core/src/entity/EntityAssigner.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { inspect } from 'util';
import type { Collection } from './Collection';
import { Collection } from './Collection';
import type { EntityManager } from '../EntityManager';
import type { Platform } from '../platforms/Platform';
import type { AnyEntity, Dictionary, EntityData, EntityDTO, EntityMetadata, EntityProperty, Primary, RequiredEntityData } from '../typings';
Expand Down Expand Up @@ -56,8 +56,13 @@ export class EntityAssigner {
throw new Error(`You must pass a non-${value} value to the property ${propName} of entity ${(entity as Dictionary).constructor.name}.`);
}

if (prop && Utils.isCollection(entity[propName as keyof T])) {
return EntityAssigner.assignCollection<T>(entity, entity[propName as keyof T] as unknown as Collection<AnyEntity>, value, prop, options.em, options);
// create collection instance if its missing so old items can be deleted with orphan removal
if ([ReferenceType.MANY_TO_MANY, ReferenceType.ONE_TO_MANY].includes(prop?.reference) && entity[prop.name] == null) {
entity[prop.name] = Collection.create(entity, prop.name, undefined, helper(entity).isInitialized()) as any;
}

if (prop && Utils.isCollection(entity[prop.name])) {
return EntityAssigner.assignCollection<T>(entity, entity[prop.name] as unknown as Collection<AnyEntity>, value, prop, options.em, options);
}

const customType = prop?.customType;
Expand Down Expand Up @@ -96,7 +101,7 @@ export class EntityAssigner {
}

if (prop?.reference === ReferenceType.SCALAR && SCALAR_TYPES.includes(prop.type) && (prop.setter || !prop.getter)) {
return entity[propName as keyof T] = validator.validateProperty(prop, value, entity);
return entity[prop.name] = validator.validateProperty(prop, value, entity);
}

if (prop?.reference === ReferenceType.EMBEDDED && EntityAssigner.validateEM(options.em)) {
Expand Down
261 changes: 261 additions & 0 deletions tests/issues/GHx10.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
import {
Collection,
Entity,
ManyToOne,
OneToMany,
PrimaryKey,
PrimaryKeyType,
Property,
Ref,
Unique,
} from '@mikro-orm/core';
import { MikroORM } from '@mikro-orm/sqlite';
import { v4 } from 'uuid';
import { mockLogger } from '../helpers';

@Entity()
class Organization {

@PrimaryKey({ columnType: 'uuid' })
id = v4();

}

@Entity()
class File {

@PrimaryKey({ columnType: 'uuid' })
id = v4();

@ManyToOne({ entity: () => Organization, ref: true, primary: true })
organization!: Ref<Organization>;

@ManyToOne({
entity: () => Document,
ref: true,
joinColumns: ['document_id', 'organization_id'],
})
document!: Ref<Document>;

}

@Entity()
class Document {

[PrimaryKeyType]?: [string, string];

@Unique({ name: 'document_id_unique' })
@PrimaryKey({ columnType: 'uuid' })
id = v4();

@ManyToOne({ entity: () => Organization, ref: true, primary: true })
organization!: Ref<Organization>;

@ManyToOne({
entity: () => Project,
ref: true,
joinColumns: ['project_id', 'organization_id'],
nullable: true,
})
project?: Ref<Project>;

@ManyToOne({
entity: () => ProjectUpdate,
ref: true,
joinColumns: ['project_update_id', 'organization_id'],
nullable: true,
})
projectUpdate?: Ref<ProjectUpdate>;

@OneToMany({
entity: () => File,
mappedBy: 'document',
orphanRemoval: true,
})
files = new Collection<File>(this);

}

@Entity()
class Project {

[PrimaryKeyType]?: [string, string];

@PrimaryKey({ columnType: 'uuid' })
id = v4();

@ManyToOne({ entity: () => Organization, ref: true, primary: true })
organization!: Ref<Organization>;

@Property({ length: 255 })
name!: string;

@OneToMany({
entity: () => Document,
mappedBy: 'project',
orphanRemoval: true,
})
documents = new Collection<Document>(this);

@OneToMany({
entity: () => ProjectUpdate,
mappedBy: 'project',
orphanRemoval: true,
})
projectUpdates = new Collection<ProjectUpdate>(this);

}

@Entity()
class ProjectUpdate {

[PrimaryKeyType]?: [string, string];

@PrimaryKey({ columnType: 'uuid' })
id = v4();

@ManyToOne({ entity: () => Organization, ref: true, primary: true })
organization!: Ref<Organization>;

@ManyToOne({
entity: () => Project,
ref: true,
joinColumns: ['project_id', 'organization_id'],
})
project!: Ref<Project>;

@OneToMany({
entity: () => Document,
mappedBy: 'projectUpdate',
orphanRemoval: true,
})
documents = new Collection<Document>(this);

}

let orm: MikroORM;
let org: Organization;
let project: Project;
let oldDocument: Document;

beforeAll(async () => {
orm = await MikroORM.init({
dbName: ':memory:',
entities: [Project, Organization, ProjectUpdate],
});
await orm.schema.createSchema();
});

afterAll(async () => {
await orm.close();
});

beforeEach(async () => {
await orm.schema.clearDatabase();

org = new Organization();
project = orm.em.create(Project, {
organization: org,
name: 'init',
documents: [{
organization: org,
}],
});
oldDocument = project.documents[0];
await orm.em.flush();
orm.em.clear();
});

test('orphan removal with complex FKs sharing a column (with loaded entity)', async () => {
// Loading the project does make orphanremoval work
const pr = await orm.em.findOneOrFail(Project, { organization: org.id });

const projectUpdate = new ProjectUpdate();
orm.em.assign(projectUpdate, {
organization: org.id,
project: [project.id, org.id],
});

const file = new File();
const document = new Document();
orm.em.assign(file, {
organization: org.id,
document: [document.id, org.id],
});

// We attach the same document to both the project and project update
orm.em.assign(document, {
organization: org.id,
project: [project.id, org.id],
projectUpdate: [projectUpdate.id, org.id],
files: [file],
});

orm.em.assign(pr, {
name: 'jos',
documents: [document],
projectUpdates: [projectUpdate],
});

const mock = mockLogger(orm, ['query']);
await orm.em.flush();

expect(mock.mock.calls).toHaveLength(8);
expect(mock.mock.calls[0][0]).toMatch('begin');
expect(mock.mock.calls[1][0]).toMatch('insert into `project_update` (`id`, `organization_id`, `project_id`) values (?, ?, ?)');
expect(mock.mock.calls[2][0]).toMatch('insert into `document` (`id`, `organization_id`, `project_id`, `project_update_id`) values (?, ?, ?, ?)');
expect(mock.mock.calls[3][0]).toMatch('insert into `file` (`id`, `organization_id`, `document_id`) values (?, ?, ?)');
expect(mock.mock.calls[4][0]).toMatch('update `project` set `name` = ? where `id` = ? and `organization_id` = ?');
expect(mock.mock.calls[5][0]).toMatch('delete from `document` where (`project_id`, `organization_id`) in ( values (?, ?)) and (`id`, `organization_id`) not in ( values (?, ?))');
expect(mock.mock.calls[6][0]).toMatch('delete from `project_update` where (`project_id`, `organization_id`) in ( values (?, ?)) and (`id`, `organization_id`) not in ( values (?, ?))');
expect(mock.mock.calls[7][0]).toMatch('commit');

const exists = await orm.em.count(Document, oldDocument);
expect(exists).toBe(0);
});

test('orphan removal with complex FKs sharing a column (with reference)', async () => {
const pr = orm.em.getReference(Project, [project.id, org.id]);
const projectUpdate = new ProjectUpdate();
orm.em.assign(projectUpdate, {
organization: org.id,
project: [project.id, org.id],
});

const file = new File();
const document = new Document();
orm.em.assign(file, {
organization: org.id,
document: [document.id, org.id],
});

// We attach the same document to both the project and project update
orm.em.assign(document, {
organization: org.id,
project: [project.id, org.id],
projectUpdate: [projectUpdate.id, org.id],
files: [file],
});

orm.em.assign(pr, {
name: 'jos',
documents: [document],
projectUpdates: [projectUpdate],
});

const mock = mockLogger(orm, ['query']);
await orm.em.flush();

expect(mock.mock.calls).toHaveLength(8);
expect(mock.mock.calls[0][0]).toMatch('begin');
expect(mock.mock.calls[1][0]).toMatch('insert into `project_update` (`id`, `organization_id`, `project_id`) values (?, ?, ?)');
expect(mock.mock.calls[2][0]).toMatch('insert into `document` (`id`, `organization_id`, `project_id`, `project_update_id`) values (?, ?, ?, ?)');
expect(mock.mock.calls[3][0]).toMatch('insert into `file` (`id`, `organization_id`, `document_id`) values (?, ?, ?)');
expect(mock.mock.calls[4][0]).toMatch('update `project` set `name` = ? where `id` = ? and `organization_id` = ?');
expect(mock.mock.calls[5][0]).toMatch('delete from `document` where (`project_id`, `organization_id`) in ( values (?, ?)) and (`id`, `organization_id`) not in ( values (?, ?))');
expect(mock.mock.calls[6][0]).toMatch('delete from `project_update` where (`project_id`, `organization_id`) in ( values (?, ?)) and (`id`, `organization_id`) not in ( values (?, ?))');
expect(mock.mock.calls[7][0]).toMatch('commit');

const exists = await orm.em.count(Document, oldDocument);
expect(exists).toBe(0);
});

0 comments on commit b60e4ee

Please sign in to comment.