Skip to content

Commit

Permalink
fix(validation): throw when calling qb.update/delete() after `qb.wh…
Browse files Browse the repository at this point in the history
…ere()`

For more complex cases we need to know the QB type before processing the condition.

Closes #2390
  • Loading branch information
B4nan committed Nov 21, 2021
1 parent c1c4d51 commit 96893e0
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 17 deletions.
15 changes: 6 additions & 9 deletions packages/knex/src/query/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,7 @@ import type {
QueryOrderMap,
QueryResult,
} from '@mikro-orm/core';
import {
LoadStrategy,
LockMode,
QueryFlag,
QueryHelper,
ReferenceType,
Utils,
ValidationError,
} from '@mikro-orm/core';
import { LoadStrategy, LockMode, QueryFlag, QueryHelper, ReferenceType, Utils, ValidationError } from '@mikro-orm/core';
import { QueryType } from './enums';
import type { AbstractSqlDriver } from '../AbstractSqlDriver';
import { QueryBuilderHelper } from './QueryBuilderHelper';
Expand Down Expand Up @@ -677,6 +669,10 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
this.type = type;
this._aliasMap[this.alias] = this.entityName;

if ([QueryType.UPDATE, QueryType.DELETE].includes(type) && Utils.hasObjectKeys(this._cond)) {
throw new Error(`You are trying to call \`qb.where().${type.toLowerCase()}()\`. Calling \`qb.${type.toLowerCase()}()\` before \`qb.where()\` is required.`);
}

if (!this.helper.isTableNameAliasRequired(type)) {
delete this._fields;
}
Expand Down Expand Up @@ -845,6 +841,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
// https://stackoverflow.com/questions/45494/mysql-error-1093-cant-specify-target-table-for-update-in-from-clause
const subSubQuery = this.getKnex().select(this.prepareFields(meta.primaryKeys)).from(subQuery.as(this.alias));
const method = this.flags.has(QueryFlag.UPDATE_SUB_QUERY) ? 'update' : 'delete';
this._cond = {}; // otherwise we would trigger validation error

this[method](this._data as EntityData<T>).where({
[Utils.getPrimaryKeyHash(meta.primaryKeys)]: { $in: subSubQuery },
Expand Down
26 changes: 18 additions & 8 deletions tests/QueryBuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,24 @@ describe('QueryBuilder', () => {
expect(qb.getParams()).toEqual(['test 123', 123]);
});

test('trying to call qb.update/delete() after qb.where() will throw', async () => {
const err1 ='You are trying to call `qb.where().update()`. Calling `qb.update()` before `qb.where()` is required.';
expect(() => orm.em.qb(Publisher2).where({ id: 123, type: PublisherType.LOCAL }).update({ name: 'test 123', type: PublisherType.GLOBAL })).toThrowError(err1);
expect(() => orm.em.qb(Book2).where({ uuid: { $in: ['1', '2', '3'] }, author: 123 }).update({ author: 321 })).toThrowError(err1);
expect(() => orm.em.qb(FooParam2).where({ bar: { baz: 123 } }).update({ value: 'test 123' })).toThrowError(err1);

expect(() => orm.em.qb(Author2).where({
$or: [
{ email: 'value1' },
{ name: { $in: ['value2'], $ne: 'value3' } },
],
}).update({ name: '123' })).toThrowError(err1);

const qb2 = orm.em.createQueryBuilder(FooParam2);
const err2 ='You are trying to call `qb.where().delete()`. Calling `qb.delete()` before `qb.where()` is required.';
expect(() => qb2.where({ bar: { baz: 123 } }).delete()).toThrowError(err2);
});

test('update query with or condition and auto-joining', async () => {
const qb = orm.em.createQueryBuilder(Publisher2);
qb.update({ name: 'test 123', type: PublisherType.GLOBAL }).where({ $or: [{ books: { author: 123 } }, { books: { title: 'book' } }] });
Expand Down Expand Up @@ -1445,14 +1463,6 @@ describe('QueryBuilder', () => {
expect(qb.getParams()).toEqual([]);
});

test('lazy delete query', async () => {
const qb = orm.em.createQueryBuilder(Publisher2);
qb.where({ name: 'test 123', type: PublisherType.GLOBAL }).delete();
expect(qb.getQuery()).toEqual('delete from `publisher2` where `name` = ? and `type` = ?');
expect(qb.getParams()).toEqual(['test 123', PublisherType.GLOBAL]);
expect(qb.getNextAlias()).toBe('e1');
});

test('clone QB', async () => {
const qb = orm.em.createQueryBuilder(Publisher2, 'p')
.select(['p.*', 'b.*', 'a.*', 't.*'])
Expand Down

0 comments on commit 96893e0

Please sign in to comment.