Skip to content

Commit

Permalink
fix(core): clean up removed entities from relations in identity map
Browse files Browse the repository at this point in the history
Closes #4863
  • Loading branch information
B4nan committed Oct 24, 2023
1 parent eb138ad commit 1e3bb0e
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 19 deletions.
13 changes: 13 additions & 0 deletions packages/core/src/metadata/MetadataDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export class MetadataDiscovery {

filtered.forEach(meta => Object.values(meta.properties).forEach(prop => this.initIndexes(prop)));
filtered.forEach(meta => this.autoWireBidirectionalProperties(meta));
filtered.forEach(meta => this.findReferencingProperties(meta, filtered));

for (const meta of filtered) {
discovered.push(...await this.processEntity(meta));
Expand Down Expand Up @@ -513,6 +514,18 @@ export class MetadataDiscovery {
return ret;
}

private findReferencingProperties(meta: EntityMetadata, metadata: EntityMetadata[]) {
for (const meta2 of metadata) {
const prop2 = meta2.relations.find(prop2 => {
return prop2.reference !== ReferenceType.SCALAR && prop2.type === meta.className;
});

if (prop2) {
meta.referencingProperties.push({ meta: meta2, prop: prop2 });
}
}
}

private initFactoryField<T>(meta: EntityMetadata<T>, prop: EntityProperty<T>): void {
['mappedBy', 'inversedBy', 'pivotEntity'].forEach(type => {
const value = prop[type];
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export class EntityMetadata<T = any> {
this.indexes = [];
this.uniques = [];
this.checks = [];
this.referencingProperties = [];
this.concurrencyCheckKeys = new Set();
Object.assign(this, meta);
}
Expand Down Expand Up @@ -550,6 +551,7 @@ export interface EntityMetadata<T = any> {
props: EntityProperty<T>[];
relations: EntityProperty<T>[];
bidirectionalRelations: EntityProperty<T>[];
referencingProperties: { meta: EntityMetadata<T>; prop: EntityProperty<T> }[];
comparableProps: EntityProperty<T>[]; // for EntityComparator
hydrateProps: EntityProperty<T>[]; // for Hydrator
uniqueProps: EntityProperty<T>[];
Expand Down
27 changes: 10 additions & 17 deletions packages/core/src/unit-of-work/UnitOfWork.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,23 +423,16 @@ export class UnitOfWork {
this.identityMap.delete(entity);
const wrapped = helper(entity);

for (const prop of wrapped.__meta.bidirectionalRelations) {
const inverse = prop.mappedBy || prop.inversedBy;
const rel = Reference.unwrapReference(entity[prop.name]);

if (Utils.isCollection(rel) && rel.isInitialized()) {
rel.getItems(false).forEach(item => rel.removeWithoutPropagation(item));
continue;
}

if (Utils.isCollection(rel?.[inverse]) && rel[inverse].isInitialized()) {
rel[inverse].removeWithoutPropagation(entity);
continue;
}

// there is a value, and it is still a self-reference (e.g. not replaced by user manually)
if (!Utils.isCollection(rel) && rel?.[inverse] && entity === Reference.unwrapReference(rel[inverse])) {
delete helper(rel).__data[inverse];
// remove references of this entity in all managed entities, otherwise flushing could reinsert the entity
for (const { meta, prop } of wrapped.__meta.referencingProperties) {
for (const referrer of this.identityMap.getStore(meta).values()) {
const rel = Reference.unwrapReference(referrer[prop.name] as object);

if (Utils.isCollection(rel)) {
rel.removeWithoutPropagation(entity);
} else if (rel === entity) {
delete helper(referrer).__data[prop.name];
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion tests/EntityManager.mongo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,12 @@ describe('EntityManagerMongo', () => {
expect(orm.em.getUnitOfWork().getById<Author>(Author.name, author.id)).toBeDefined();
await repo.flush();
expect(orm.em.getUnitOfWork().getById<Author>(Author.name, author.id)).toBeUndefined();
expect(orm.em.getUnitOfWork().getIdentityMap()).toEqual({ registry: new Map([[Author, new Map<string, Author>()]]) });
expect(orm.em.getUnitOfWork().getIdentityMap()).toEqual({
registry: new Map([
[Author, new Map<string, Author>()],
[Book, new Map<string, Book>()],
] as any[]),
});
});

test('removing entity will remove its FK from relations', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,6 @@ describe('composite keys in sqlite', () => {
expect(c1).toBe(u2.cars[0]);

await orm.em.remove(u2).flush();
expect(u2.cars[0]).toBeUndefined();
const o3 = await orm.em.findOne(User2, u1);
expect(o3).toBeNull();
const c2 = await orm.em.findOneOrFail(Car2, car1);
Expand Down
81 changes: 81 additions & 0 deletions tests/features/unit-of-work/GH4863.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { Entity, PrimaryKey, Property, ManyToOne } from '@mikro-orm/core';
import { MikroORM } from '@mikro-orm/sqlite';

@Entity()
class Department {

@PrimaryKey()
id!: number;

@Property()
name!: string;

}

@Entity()
class Person {

@PrimaryKey()
id!: number;

@Property()
name!: string;

}

@Entity()
class Employee {

@PrimaryKey()
id!: number;

@ManyToOne({ onDelete: 'cascade' })
department!: Department;

@ManyToOne({ onDelete: 'cascade' })
person!: Person;

}

let orm: MikroORM;

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

afterAll(() => orm.close(true));

test('insert reference object after deleting it', async () => {
const em = orm.em.fork();

const department = new Department();
department.name = 'Department';
await em.persistAndFlush(department); // here we create department

const person = new Person();
person.name = 'Person';
await em.persistAndFlush(person); // here we create person

const employee = new Employee();
employee.department = department;
employee.person = person;
await em.persistAndFlush(employee); // here we create employee
await em.removeAndFlush(department); // here we remove department and owned employee

person.name = 'New Person';
await em.persistAndFlush(person); // here we update person
await em.removeAndFlush(person); // here we remove person
orm.em.clear();

const departments = await em.find(Department, {});
const people = await em.find(Person, {});
const employees = await em.find(Employee, {});

expect(departments.length).toBe(0);
expect(people.length).toBe(0);
expect(employees.length).toBe(0);
});

0 comments on commit 1e3bb0e

Please sign in to comment.