Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back quoteIdentifier(s) to queryInterface to ease migration to v6 #13810

Merged
merged 4 commits into from Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/dialects/abstract/query-generator.js
Expand Up @@ -925,7 +925,7 @@ class QueryGenerator {
}

/**
* Split a list of identifiers by "." and quote each part
* Adds quotes to identifier
fzn0x marked this conversation as resolved.
Show resolved Hide resolved
*
* @param {string} identifier
* @param {boolean} force
Expand All @@ -936,6 +936,13 @@ class QueryGenerator {
throw new Error(`quoteIdentifier for Dialect "${this.dialect}" is not implemented`);
}

/**
* Split a list of identifiers by "." and quote each part.
*
* @param {string} identifiers
*
* @returns {string}
*/
quoteIdentifiers(identifiers) {
if (identifiers.includes('.')) {
identifiers = identifiers.split('.');
Expand Down
23 changes: 23 additions & 0 deletions lib/dialects/abstract/query-interface.js
Expand Up @@ -430,6 +430,29 @@ class QueryInterface {
return this.sequelize.normalizeAttribute(attribute);
}

/**
* Split a list of identifiers by "." and quote each part
*
* @param {string} identifier
* @param {boolean} force
*
* @returns {string}
*/
quoteIdentifier(identifier, force) {
return this.queryGenerator.quoteIdentifier(identifier, force);
}

/**
* Split a list of identifiers by "." and quote each part.
*
* @param {string} identifiers
*
* @returns {string}
*/
quoteIdentifiers(identifiers) {
return this.queryGenerator.quoteIdentifiers(identifiers);
}

/**
* Change a column definition
*
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/class-to-invokable.ts
Expand Up @@ -15,14 +15,14 @@ interface Invokeable<Args extends Array<any>, Instance> {
* @private
*/
export function classToInvokable<Args extends Array<any>, Instance extends object>(
Class: new (...args: Args) => Instance,
Class: new (...args: Args) => Instance
): Invokeable<Args, Instance> {
return new Proxy<Invokeable<Args, Instance>>(Class as any, {
apply(_target, _thisArg, args: Args) {
return new Class(...args);
},
construct(_target, args: Args) {
return new Class(...args);
},
}
});
}
40 changes: 40 additions & 0 deletions test/unit/dialects/abstract/query-interface.test.ts
@@ -0,0 +1,40 @@
import { expect } from 'chai';
import Support from '../../support';

const { sequelize } = Support as any;

describe('QueryInterface', () => {
describe('quoteIdentifier', () => {
// regression test which covers https://github.com/sequelize/sequelize/issues/12627
it('should quote the identifier', () => {
const identifier = 'identifier';
const quotedIdentifier = sequelize
.getQueryInterface()
.quoteIdentifier(identifier);
const expectedQuotedIdentifier = sequelize
.getQueryInterface()
.queryGenerator.quoteIdentifier(identifier);

expect(quotedIdentifier).not.to.be.undefined;
expect(expectedQuotedIdentifier).not.to.be.undefined;
expect(quotedIdentifier).to.equal(expectedQuotedIdentifier);
});
});

describe('quoteIdentifiers', () => {
// regression test which covers https://github.com/sequelize/sequelize/issues/12627
it('should quote the identifiers', () => {
const identifier = 'table.identifier';
const quotedIdentifiers = sequelize
.getQueryInterface()
.quoteIdentifiers(identifier);
const expectedQuotedIdentifiers = sequelize
.getQueryInterface()
.queryGenerator.quoteIdentifiers(identifier);

expect(quotedIdentifiers).not.to.be.undefined;
expect(expectedQuotedIdentifiers).not.to.be.undefined;
expect(quotedIdentifiers).to.equal(expectedQuotedIdentifiers);
});
});
});
15 changes: 7 additions & 8 deletions types/lib/query-interface.d.ts
Expand Up @@ -630,21 +630,20 @@ export class QueryInterface {
): Promise<void>;

/**
* Escape an identifier (e.g. a table or attribute name). If force is true, the identifier will be quoted
* even if the `quoteIdentifiers` option is false.
* Escape a table name
*/
public quoteIdentifier(identifier: string, force: boolean): string;
public quoteTable(identifier: TableName): string;
fzn0x marked this conversation as resolved.
Show resolved Hide resolved

/**
* Escape a table name
* Escape an identifier (e.g. a table or attribute name). If force is true, the identifier will be quoted
* even if the `quoteIdentifiers` option is false.
*/
public quoteTable(identifier: TableName): string;
public quoteIdentifier(identifier: string, force?: boolean): string;

/**
* Split an identifier into .-separated tokens and quote each part. If force is true, the identifier will be
* quoted even if the `quoteIdentifiers` option is false.
* Split an identifier into .-separated tokens and quote each part.
*/
public quoteIdentifiers(identifiers: string, force: boolean): string;
public quoteIdentifiers(identifiers: string): string;
fzn0x marked this conversation as resolved.
Show resolved Hide resolved

/**
* Escape a value (e.g. a string, number or date)
Expand Down
4 changes: 4 additions & 0 deletions types/test/query-interface.ts
Expand Up @@ -73,6 +73,10 @@ async function test() {

await queryInterface.quoteTable({ tableName: 'foo', delimiter: 'bar' });

queryInterface.quoteIdentifier("foo");
queryInterface.quoteIdentifier("foo", true);
queryInterface.quoteIdentifiers("table.foo");

await queryInterface.dropAllTables();

await queryInterface.renameTable('Person', 'User');
Expand Down