Skip to content

Commit

Permalink
fix(core): mark Reference.set() as private (#5017)
Browse files Browse the repository at this point in the history
BREAKING CHANGE:

`Reference` wrapper holds an identity—this means its instance is bound
to the underlying entity. This imposes a problem when you try to change
the wrapped entity, as the same instance of the `Reference` wrapper can
be present on other places in the entity graph. For this reason, the
`set` method is no longer public, as replacing the reference instance
should be always preferred.

```diff
-book.author.set(other);
+book.author = ref(other);
```

Closes #5003
  • Loading branch information
B4nan committed Dec 13, 2023
1 parent a477306 commit 5aebf0b
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 6 deletions.
9 changes: 9 additions & 0 deletions docs/docs/upgrading-v5-to-v6.md
Expand Up @@ -517,3 +517,12 @@ The `Reference.load()` method allowed two signatures, one to ensure the entity i
-const email = book.author.load('email');
+const email = book.author.loadProperty('email');
```

## `Reference.set()` is private

`Reference` wrapper holds an identity—this means its instance is bound to the underlying entity. This imposes a problem when you try to change the wrapped entity, as the same instance of the `Reference` wrapper can be present on other places in the entity graph. For this reason, the `set` method is no longer public, as replacing the reference instance should be always preferred.

```diff
-book.author.set(other);
+book.author = ref(other);
```
8 changes: 4 additions & 4 deletions packages/core/src/entity/Reference.ts
Expand Up @@ -42,13 +42,13 @@ export class Reference<T> {

static create<T>(entity: T | Ref<T>): Ref<T> {
const unwrapped = Reference.unwrapReference(entity);
const ref = helper(entity).toReference() as Ref<T>;
const ref = helper(entity).toReference() as Reference<T>;

if (unwrapped !== ref.unwrap()) {
ref.set(unwrapped as T);
ref.set(unwrapped);
}

return ref;
return ref as Ref<T>;
}

static createFromPK<T>(entityType: EntityClass<T>, pk: Primary<T>, options?: { schema?: string }): Ref<T> {
Expand Down Expand Up @@ -112,7 +112,7 @@ export class Reference<T> {
return this.entity as Loaded<TT, P>;
}

set<TT extends T>(entity: TT | Ref<TT>): void {
private set<TT extends T>(entity: TT | Ref<TT>): void {
this.entity = Reference.unwrapReference(entity as T & object);
delete helper(this.entity).__reference;
}
Expand Down
5 changes: 3 additions & 2 deletions tests/EntityManager.mongo.test.ts
@@ -1,3 +1,4 @@
/* eslint-disable dot-notation */
import { ObjectId } from 'bson';
import type { EntityProperty } from '@mikro-orm/core';
import {
Expand Down Expand Up @@ -1909,10 +1910,10 @@ describe('EntityManagerMongo', () => {
expect(ref2).toBe(ref3);

expect(ref3.unwrap()).toBe(author);
ref3.set(author2);
ref3['set'](author2);
expect(ref3.unwrap()).toBe(author2);
expect(ref3.id).toBe(author2.id);
ref3.set(Reference.create(author));
ref3['set'](Reference.create(author));
expect(ref3.id).toBe(author.id);

const ent = await ref.load();
Expand Down
113 changes: 113 additions & 0 deletions tests/issues/GH5003.test.ts
@@ -0,0 +1,113 @@
import {
PrimaryKey,
ManyToOne,
Ref,
Collection,
Property,
Entity,
t,
OneToMany,
MikroORM,
ref,
} from '@mikro-orm/sqlite';

@Entity()
class Assignee {

@PrimaryKey({ type: t.bigint })
readonly id!: string;

@ManyToOne(() => Slot, { ref: true })
slot!: Ref<Slot>;

@Property()
email!: string;

}

@Entity()
class Slot {

@PrimaryKey({ type: t.bigint })
readonly id!: string;

@OneToMany(() => Assignee, a => a.slot)
assignees = new Collection<Assignee>(this);

@OneToMany(() => Registration, r => r.slot)
registrations = new Collection<Registration>(this);

@Property()
name!: string;

}

@Entity()
class Registration {

@PrimaryKey({ type: t.bigint })
readonly id!: string;

@ManyToOne(() => Slot, { ref: true })
slot!: Ref<Slot>;

}

let orm: MikroORM;

beforeAll(async () => {
orm = await MikroORM.init({
entities: [Assignee, Slot, Registration],
dbName: ':memory:',
});
await orm.schema.refreshDatabase();
});

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

async function createEntities() {
await orm.schema.clearDatabase();
const slot1 = orm.em.create(Slot, { name: 'slot1' });
const slot2 = orm.em.create(Slot, { name: 'slot2' });

const assignee1 = orm.em.create(Assignee, { email: 'slot1@user.com', slot: slot1 });
const assignee2 = orm.em.create(Assignee, { email: 'slot2@user.com', slot: slot2 });

const registration = orm.em.create(Registration, { slot: slot1 });
await orm.em.flush();
orm.em.clear();
}

test('reschedule registration from slot1 to slot2 (lazy loading assignees)', async () => {
expect.assertions(2);

const registration = await orm.em.findOneOrFail(Registration, { slot: { name: 'slot1' } }, { populate: ['slot'] });
const slot2 = await orm.em.findOneOrFail(Slot, { name: 'slot2' }, { populate: ['registrations'] });

const slot1 = registration.slot.$;

registration.slot = ref(slot2);
await orm.em.persistAndFlush(registration);

expect(registration.slot.id).toEqual(slot2.id);

await orm.em.populate(registration, ['slot.assignees']);
await orm.em.populate(slot1, ['assignees']);

expect(registration.slot.id).toEqual(slot2.id);
});

test('reschedule registration from slot1 to slot2 (eager loading assignees)', async () => {
const registration = await orm.em.findOneOrFail(Registration, { slot: { name: 'slot1' } }, { populate: ['slot.assignees'] });
const slot2 = await orm.em.findOneOrFail(Slot, { name: 'slot2' }, { populate: ['registrations', 'assignees'] });

const slot1 = registration.slot.$;

registration.slot = ref(slot2);
await orm.em.persistAndFlush(registration);

expect.assertions(2);
slot1.assignees.getItems().forEach(assignee => expect(assignee.slot.id).toEqual(slot1.id));
slot2.assignees.getItems().forEach(assignee => expect(assignee.slot.id).toEqual(slot2.id));
});

0 comments on commit 5aebf0b

Please sign in to comment.