Skip to content

Commit

Permalink
fix: calling EntityManager.insert() with an empty array of entities (#…
Browse files Browse the repository at this point in the history
…5745)

As described in issue #5734, the current behaviour seems like an oversight and inconsistent with save() and remove() which already have this handled as a special case.

Test Plan: npm test, and more specifically npm test -- --grep='#5734'

Closes: #5734
  • Loading branch information
Lovro Puzar authored May 16, 2020
1 parent 7d8a1ca commit f8c52f3
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/entity-manager/EntityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,16 @@ export class EntityManager {
*/
async insert<Entity>(target: ObjectType<Entity>|EntitySchema<Entity>|string, entity: QueryDeepPartialEntity<Entity>|(QueryDeepPartialEntity<Entity>[])): Promise<InsertResult> {

// If user passed empty array of entities then we don't need to do
// anything.
//
// Fixes GitHub issue #5734. If we were to let this through we would
// run into problems downstream, like subscribers getting invoked with
// the empty array where they expect an entity, and SQL queries with an
// empty VALUES clause.
if (Array.isArray(entity) && entity.length === 0)
return Promise.resolve(new InsertResult());

// TODO: Oracle does not support multiple values. Need to create another nice solution.
if (this.connection.driver instanceof OracleDriver && Array.isArray(entity)) {
const results = await Promise.all(entity.map(entity => this.insert(target, entity)));
Expand Down
12 changes: 12 additions & 0 deletions test/github-issues/5734/entity/Post.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {PrimaryColumn} from "../../../../src/decorator/columns/PrimaryColumn";
import {Entity} from "../../../../src/decorator/entity/Entity";

@Entity()
export class Post {
@PrimaryColumn()
id: number;

constructor(id: number) {
this.id = id;
}
}
28 changes: 28 additions & 0 deletions test/github-issues/5734/issue-5734.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import "reflect-metadata";
import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../utils/test-utils";
import {Connection} from "../../../src/connection/Connection";
import {Post} from "./entity/Post";

describe("github issues > #5734 insert([]) should not crash", () => {

let connections: Connection[];
before(async () => connections = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
subscribers: [__dirname + "/subscriber/*{.js,.ts}"],
schemaCreate: true,
dropSchema: true
}));
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should not crash on insert([])", () => Promise.all(connections.map(async connection => {
const repository = connection.getRepository(Post);
await repository.insert([]);
})));

it("should still work with a nonempty array", () => Promise.all(connections.map(async connection => {
const repository = connection.getRepository(Post);
await repository.insert([new Post(1)]);
await repository.findOneOrFail({where: {id: 1}});
})));
});
22 changes: 22 additions & 0 deletions test/github-issues/5734/subscriber/CheckValidEntitySubscriber.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {Post} from "../entity/Post";
import {EntitySubscriberInterface, EventSubscriber, InsertEvent} from "../../../../src";

/**
* Subscriber which checks the validity of the entity passed to beforeInsert().
* Tests the fix for issue #5734 in which we saw an empty array leak into
* beforeInsert().
*/
@EventSubscriber()
export class ValidEntityCheckSubscriber
implements EntitySubscriberInterface<Post> {
listenTo() {
return Post;
}

beforeInsert(event: InsertEvent<Post>) {
const entity = event.entity;
if (Array.isArray(entity) || !entity.id) {
throw new Error(`Subscriber saw invalid entity: ${JSON.stringify(entity)}`);
}
}
}

0 comments on commit f8c52f3

Please sign in to comment.