Skip to content

Commit

Permalink
fix(core): mark all properties as populated for new entities
Browse files Browse the repository at this point in the history 
When flushing new entity, we now mark all the relations as populated,
just like if the entity was loaded from the db. This aligns the serialized
output of `e.toJSON()` of a loaded entity and just-inserted one.

Closes #784
  • Loading branch information
@B4nan
B4nan committed Sep 2, 2020
1 parent fc2fbaa commit 5f7fb8f
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 26 deletions.
48 changes: 33 additions & 15 deletions packages/core/src/unit-of-work/ChangeSetPersister.ts
View file
Expand Up @@ -18,15 +18,15 @@ export class ChangeSetPersister {
changeSets.forEach(changeSet => this.processProperties(changeSet));

for (const changeSet of changeSets) {
await this.persistEntity(changeSet, ctx);
await this.persistNewEntity(changeSet, ctx);
}
}

async executeUpdates<T extends AnyEntity<T>>(changeSets: ChangeSet<T>[], ctx?: Transaction): Promise<void> {
changeSets.forEach(changeSet => this.processProperties(changeSet));

for (const changeSet of changeSets) {
await this.persistEntity(changeSet, ctx);
await this.persistManagedEntity(changeSet, ctx);
}
}

Expand All @@ -51,25 +51,27 @@ export class ChangeSetPersister {
}
}

private async persistEntity<T extends AnyEntity<T>>(changeSet: ChangeSet<T>, ctx?: Transaction): Promise<void> {
let res: QueryResult | undefined;
private async persistNewEntity<T extends AnyEntity<T>>(changeSet: ChangeSet<T>, ctx?: Transaction): Promise<void> {
const meta = this.metadata.find(changeSet.name)!;
const wrapped = changeSet.entity.__helper!;
const res = await this.driver.nativeInsert(changeSet.name, changeSet.payload, ctx);
const hasPrimaryKey = Utils.isDefined(wrapped.__primaryKey, true);
this.mapReturnedValues(changeSet.entity, res, meta);

if (changeSet.type === ChangeSetType.UPDATE) {
res = await this.updateEntity(meta, changeSet, ctx);
this.mapReturnedValues(changeSet.entity, res, meta);
} else if (Utils.isDefined(wrapped.__primaryKey, true)) { // ChangeSetType.CREATE with primary key
res = await this.driver.nativeInsert(changeSet.name, changeSet.payload, ctx);
this.mapReturnedValues(changeSet.entity, res, meta);
wrapped.__initialized = true;
} else { // ChangeSetType.CREATE without primary key
res = await this.driver.nativeInsert(changeSet.name, changeSet.payload, ctx);
this.mapReturnedValues(changeSet.entity, res, meta);
if (!hasPrimaryKey) {
this.mapPrimaryKey(meta, res.insertId, changeSet);
wrapped.__initialized = true;
}

this.markAsPopulated(changeSet, meta);
wrapped.__initialized = true;
await this.processOptimisticLock(meta, changeSet, res, ctx);
changeSet.persisted = true;
}

private async persistManagedEntity<T extends AnyEntity<T>>(changeSet: ChangeSet<T>, ctx?: Transaction): Promise<void> {
const meta = this.metadata.find(changeSet.name)!;
const res = await this.updateEntity(meta, changeSet, ctx);
this.mapReturnedValues(changeSet.entity, res, meta);
await this.processOptimisticLock(meta, changeSet, res, ctx);
changeSet.persisted = true;
}
Expand All @@ -82,6 +84,22 @@ export class ChangeSetPersister {
this.identifierMap.get(wrapped.__uuid)!.setValue(changeSet.entity[prop.name] as unknown as IPrimaryKey);
}

/**
* Sets populate flag to new entities so they are serialized like if they were loaded from the db
*/
private markAsPopulated<T extends AnyEntity<T>>(changeSet: ChangeSet<T>, meta: EntityMetadata<T>) {
changeSet.entity.__helper!.populated();
Object.values(meta.properties).forEach(prop => {
const value = changeSet.entity[prop.name];

if (Utils.isEntity(value, true)) {
(value as AnyEntity).__helper!.populated();
} else if (Utils.isCollection(value)) {
value.populated();
}
});
}

private async updateEntity<T extends AnyEntity<T>>(meta: EntityMetadata<T>, changeSet: ChangeSet<T>, ctx?: Transaction): Promise<QueryResult> {
if (!meta.versionProperty || !changeSet.entity[meta.versionProperty]) {
return this.driver.nativeUpdate(changeSet.name, changeSet.entity.__helper!.__primaryKey as Dictionary, changeSet.payload, ctx);
Expand Down
17 changes: 17 additions & 0 deletions tests/EntityAssigner.mongo.test.ts
View file
Expand Up @@ -84,6 +84,23 @@ describe('EntityAssignerMongo', () => {
expect((jon as any).code).toBeUndefined();
});

test('newly created entity should be considered as populated (GH issue #784)', async () => {
const god = new Author('God', 'hello@heaven.god');
const jon = new Author('Jon Snow', 'snow@wall.st');
const book = new Book('Book2', jon);
jon.favouriteAuthor = god;
jon.books.add(book);
await orm.em.persistAndFlush(jon);
expect(jon.toObject().id).not.toBeUndefined();
expect(jon.toObject().books).toHaveLength(1);
expect(jon.toObject().books[0]).toMatchObject({
title: 'Book2',
});
expect(jon.toObject().favouriteAuthor).toMatchObject({
name: 'God',
});
});

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

});
8 changes: 4 additions & 4 deletions tests/EntityManager.mysql.test.ts
View file
Expand Up @@ -820,7 +820,7 @@ describe('EntityManagerMySql', () => {
const b1 = (await orm.em.findOne(FooBar2, { id: bar.id }, ['baz']))!;
expect(b1.baz).toBeInstanceOf(FooBaz2);
expect(b1.baz!.id).toBe(baz.id);
expect(wrap(b1).toJSON()).toMatchObject({ baz: wrap(baz).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ baz: { id: baz.id, bar: bar.id, name: 'baz' } });
});

test('populate OneToOne relation on inverse side', async () => {
Expand All @@ -846,15 +846,15 @@ describe('EntityManagerMySql', () => {
expect(mock.mock.calls[2][0]).toMatch('select `e0`.*, (select 123) as `random` from `foo_bar2` as `e0` where `e0`.`id` in (?) order by `e0`.`id` asc');
expect(b1.bar).toBeInstanceOf(FooBar2);
expect(b1.bar!.id).toBe(bar.id);
expect(wrap(b1).toJSON()).toMatchObject({ bar: wrap(bar).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ bar: { id: bar.id, baz: baz.id, name: 'bar' } });
orm.em.clear();

const b2 = (await orm.em.findOne(FooBaz2, { bar: bar.id }, ['bar']))!;
expect(mock.mock.calls[3][0]).toMatch('select `e0`.*, `e1`.`id` as `bar_id` from `foo_baz2` as `e0` left join `foo_bar2` as `e1` on `e0`.`id` = `e1`.`baz_id` where `e1`.`id` = ? limit ?');
expect(mock.mock.calls[4][0]).toMatch('select `e0`.*, (select 123) as `random` from `foo_bar2` as `e0` where `e0`.`id` in (?) order by `e0`.`id` asc');
expect(b2.bar).toBeInstanceOf(FooBar2);
expect(b2.bar!.id).toBe(bar.id);
expect(wrap(b2).toJSON()).toMatchObject({ bar: wrap(bar).toJSON() });
expect(wrap(b2).toJSON()).toMatchObject({ bar: { id: bar.id, baz: baz.id, name: 'bar' } });
});

test('populate OneToOne relation with uuid PK', async () => {
Expand All @@ -867,7 +867,7 @@ describe('EntityManagerMySql', () => {

const b1 = (await orm.em.findOne(Book2, { test: test.id }, ['test.config']))!;
expect(b1.uuid).not.toBeNull();
expect(wrap(b1).toJSON()).toMatchObject({ test: wrap(test).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ test: { id: test.id, book: test.book.uuid, name: 't' } });
});

test('populate passes nested where and orderBy', async () => {
Expand Down
8 changes: 4 additions & 4 deletions tests/EntityManager.postgre.test.ts
View file
Expand Up @@ -652,7 +652,7 @@ describe('EntityManagerPostgre', () => {
const b1 = (await orm.em.findOne(FooBar2, { id: bar.id }, { populate: ['baz'], refresh: true }))!;
expect(b1.baz).toBeInstanceOf(FooBaz2);
expect(b1.baz!.id).toBe(baz.id);
expect(wrap(b1).toJSON()).toMatchObject({ baz: wrap(baz).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ baz: { id: baz.id, bar: bar.id, name: 'baz' } });
});

test('populate OneToOne relation on inverse side', async () => {
Expand All @@ -677,15 +677,15 @@ describe('EntityManagerPostgre', () => {
expect(mock.mock.calls[2][0]).toMatch('select "e0".*, (select 123) as "random" from "foo_bar2" as "e0" where "e0"."baz_id" in ($1) order by "e0"."baz_id" asc');
expect(b1.bar).toBeInstanceOf(FooBar2);
expect(b1.bar!.id).toBe(bar.id);
expect(wrap(b1).toJSON()).toMatchObject({ bar: wrap(bar).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ bar: { id: bar.id, baz: baz.id, name: 'bar' } });
orm.em.clear();

const b2 = await orm.em.findOneOrFail(FooBaz2, { bar: bar.id }, ['bar']);
expect(mock.mock.calls[3][0]).toMatch('select "e0".*, "e1"."id" as "bar_id" from "foo_baz2" as "e0" left join "foo_bar2" as "e1" on "e0"."id" = "e1"."baz_id" where "e1"."id" = $1 limit $2');
expect(mock.mock.calls[4][0]).toMatch('select "e0".*, (select 123) as "random" from "foo_bar2" as "e0" where "e0"."baz_id" in ($1) order by "e0"."baz_id" asc');
expect(b2.bar).toBeInstanceOf(FooBar2);
expect(b2.bar!.id).toBe(bar.id);
expect(wrap(b2).toJSON()).toMatchObject({ bar: wrap(bar).toJSON() });
expect(wrap(b2).toJSON()).toMatchObject({ bar: { id: bar.id, baz: baz.id, name: 'bar' } });
});

test('populate OneToOne relation with uuid PK', async () => {
Expand All @@ -698,7 +698,7 @@ describe('EntityManagerPostgre', () => {

const b1 = (await orm.em.findOne(Book2, { test: test.id }, ['test.config']))!;
expect(b1.uuid).not.toBeNull();
expect(wrap(b1).toJSON()).toMatchObject({ test: wrap(test).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ test: { id: test.id, book: test.book.uuid, name: 't' } });
});

test('many to many relation', async () => {
Expand Down
6 changes: 3 additions & 3 deletions tests/joined-strategy.postgre.test.ts
View file
Expand Up @@ -289,7 +289,7 @@ describe('Joined loading strategy', () => {
expect(connMock).toBeCalledTimes(1);
expect(b1.baz).toBeInstanceOf(FooBaz2);
expect(b1.baz!.id).toBe(baz.id);
expect(wrap(b1).toJSON()).toMatchObject({ baz: wrap(baz).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ baz: { id: baz.id, bar: bar.id, name: 'baz' } });
});

test('populate OneToOne relation on inverse side', async () => {
Expand Down Expand Up @@ -320,7 +320,7 @@ describe('Joined loading strategy', () => {
expect(b1.bar).toBeInstanceOf(FooBar2);
expect(b1.bar!.id).toBe(bar.id);
expect(b1.bar!.random).toBe(123);
expect(wrap(b1).toJSON()).toMatchObject({ bar: wrap(bar).toJSON() });
expect(wrap(b1).toJSON()).toMatchObject({ bar: { id: bar.id, baz: baz.id, name: 'bar' } });
orm.em.clear();

const b2 = (await orm.em.findOne(FooBaz2, { bar: bar.id }, { populate: { bar: LoadStrategy.JOINED } }))!;
Expand All @@ -333,7 +333,7 @@ describe('Joined loading strategy', () => {
expect(b2.bar).toBeInstanceOf(FooBar2);
expect(b2.bar!.id).toBe(bar.id);
expect(b2.bar!.random).toBe(123);
expect(wrap(b2).toJSON()).toMatchObject({ bar: wrap(bar).toJSON() });
expect(wrap(b2).toJSON()).toMatchObject({ bar: { id: bar.id, baz: baz.id, name: 'bar' } });
});

test('nested populating', async () => {
Expand Down

0 comments on commit 5f7fb8f

Please sign in to comment.