Skip to content

Commit cbf62fa

Browse files
committed
fix(core): delay snapshotting of entity state to fix differences with joined strategy
Closes #3876
1 parent 2ecbca8 commit cbf62fa

File tree

2 files changed

+109
-11
lines changed

2 files changed

+109
-11
lines changed

packages/core/src/unit-of-work/UnitOfWork.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -100,21 +100,22 @@ export class UnitOfWork {
100100
wrapped.__managed = true;
101101

102102
if (data && (options?.refresh || !wrapped.__originalEntityData)) {
103-
const callback = () => {
104-
// we can't use the `data` directly here as it can contain fetch joined data, that can't be used for diffing the state
105-
wrapped.__originalEntityData = this.comparator.prepareEntity(entity);
106-
wrapped.__touched = false;
107-
};
108-
109103
Object.keys(data).forEach(key => helper(entity).__loadedProperties.add(key));
110104
this.queuedActions.delete(wrapped.__meta.className);
111-
callback();
112105

113-
// schedule re-snapshotting in case it's a reference, as there might be some updates made via propagation
114-
// we need to have in the snapshot in case the entity reference gets modified
115-
if (!wrapped.__initialized) {
116-
this.snapshotQueue.push(callback);
106+
// assign the data early, delay recompute via snapshot queue unless we refresh the state
107+
if (options?.refresh) {
108+
wrapped.__originalEntityData = this.comparator.prepareEntity(entity);
109+
} else {
110+
wrapped.__originalEntityData = data;
117111
}
112+
113+
wrapped.__touched = false;
114+
this.snapshotQueue.push(() => {
115+
// we can't use the `data` directly here as it can contain fetch joined data, that can't be used for diffing the state
116+
wrapped.__originalEntityData = this.comparator.prepareEntity(entity);
117+
wrapped.__touched = false;
118+
});
118119
}
119120

120121
return entity;

tests/issues/GH3876.test.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { MikroORM } from '@mikro-orm/postgresql';
2+
import { Collection, OneToMany, OneToOne, Rel, Entity, LoadStrategy, ManyToOne, PrimaryKey, Property } from '@mikro-orm/core';
3+
import { v4 } from 'uuid';
4+
5+
@Entity()
6+
class Book {
7+
8+
@PrimaryKey({ type: 'uuid' })
9+
id: string = v4();
10+
11+
@Property()
12+
name!: string;
13+
14+
@Property({ nullable: true })
15+
description?: string;
16+
17+
@ManyToOne({ entity: () => User })
18+
user!: Rel<User>;
19+
20+
}
21+
22+
@Entity()
23+
class ProfileInfo {
24+
25+
@PrimaryKey({ type: 'uuid' })
26+
id: string = v4();
27+
28+
@Property()
29+
email!: string;
30+
31+
@OneToOne({
32+
entity: () => User,
33+
mappedBy: user => user.profileInfo,
34+
})
35+
user!: Rel<User>;
36+
37+
}
38+
39+
@Entity()
40+
class User {
41+
42+
@PrimaryKey({ type: 'uuid' })
43+
id: string = v4();
44+
45+
@Property()
46+
name!: string;
47+
48+
@OneToOne({
49+
entity: () => ProfileInfo,
50+
nullable: true,
51+
})
52+
profileInfo?: ProfileInfo;
53+
54+
@OneToMany({
55+
entity: () => Book,
56+
mappedBy: book => book.user,
57+
})
58+
books = new Collection<Book>(this);
59+
60+
}
61+
62+
let orm: MikroORM;
63+
64+
beforeAll(async () => {
65+
orm = await MikroORM.init({
66+
loadStrategy: LoadStrategy.JOINED,
67+
dbName: 'mikro_orm_3876',
68+
entities: [Book, User, ProfileInfo],
69+
});
70+
await orm.getSchemaGenerator().refreshDatabase();
71+
});
72+
73+
afterAll(async () => {
74+
await orm.close(true);
75+
});
76+
77+
test('3876', async () => {
78+
const user = orm.em.create(User, {
79+
name: 'user1',
80+
});
81+
orm.em.create(Book, {
82+
name: 'book1',
83+
user,
84+
});
85+
orm.em.create(Book, {
86+
name: 'book2',
87+
user,
88+
});
89+
await orm.em.flush();
90+
await orm.em.clear();
91+
92+
await orm.em.find(Book, {}, { populate: ['user'] });
93+
const uow1 = orm.em.getUnitOfWork();
94+
uow1.computeChangeSets();
95+
96+
expect(uow1.getChangeSets()).toHaveLength(0);
97+
});

0 commit comments

Comments
 (0)