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

Add queryContext to schema and query builders #2314

Merged
merged 32 commits into from Feb 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d283df8
Merge pull request #1 from tgriesser/master
joelmukuthu Nov 8, 2017
33bd3a1
feat(query-builder): add hookContext for wrapIdentifier
joelmukuthu Nov 8, 2017
7e68241
refactor: use isUndefined
joelmukuthu Nov 8, 2017
20f57b8
test(transaction): test passing of hookContext
joelmukuthu Nov 15, 2017
b67009b
feat(runnner): pass context to postProcessResponse
joelmukuthu Nov 15, 2017
117701c
test(runner): test postProcessResponse for raw responses
joelmukuthu Nov 15, 2017
08ba2c8
test(raw): test passing of hookContext
joelmukuthu Nov 15, 2017
486e637
feat: add hookContext to Raw and SchemaBuilder
joelmukuthu Nov 15, 2017
5823c7e
test(transaction): fix test for hookContext
joelmukuthu Nov 15, 2017
4e65b1a
chore: fix lint error
joelmukuthu Nov 15, 2017
d2266e4
fix: check for hookContext before calling it
joelmukuthu Nov 15, 2017
2ce2ce3
test(transaction): fix hookContext test
joelmukuthu Nov 15, 2017
e22ff8a
Merge branch 'master' into feat/hook-context
joelmukuthu Jan 4, 2018
fdc2dfe
chore: remove whitespace
joelmukuthu Jan 4, 2018
d6b5c0e
test(hookContext): test cloning of context object
joelmukuthu Jan 8, 2018
cdb91b8
refactor: hookContext -> queryContext
Jan 9, 2018
a1b3658
minor: use more descriptive variable name
Jan 10, 2018
8ebd8e2
fix: remove unnecessary checks for query builder
Jan 10, 2018
3b4f837
fix(Raw): pass query builder to formatter
Jan 10, 2018
be317bc
fix(SchemaCompiler): pass schema builder to formatter
Jan 10, 2018
07a7a34
refactor: add addQueryContext helper
Jan 10, 2018
f22a2b1
feat: add queryContext to TableBuilder and ColumnBuilder
Jan 10, 2018
cdad1bb
fix(TableCompiler): pass table builder to formatter
Jan 10, 2018
1feb6f5
fix(ColumnCompiler): pass column builder to formatter
Jan 10, 2018
48bbbd3
fix(pushQuery): fix passing builder to formatter
joelmukuthu Jan 15, 2018
feb10e0
test(Schema|Table|ColumnCompiler): test passing queryContext
joelmukuthu Jan 15, 2018
36ce3c1
fix(SchemaCompiler): pass queryContext to TableCompiler
joelmukuthu Jan 15, 2018
b8ef521
fix(TableCompiler): pass queryContext to ColumnCompiler
joelmukuthu Jan 15, 2018
08f5467
test: add queryContext tests for all schema dialects
joelmukuthu Jan 15, 2018
9b28ae0
test(TableCompiler): test overwriting queryContext from SchemaCompiler
joelmukuthu Jan 15, 2018
09be564
test(Raw): test passing queryContext to wrapIdentifier
joelmukuthu Jan 16, 2018
11b31d8
tests: run all the tests
joelmukuthu Jan 25, 2018
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
18 changes: 9 additions & 9 deletions src/client.js
Expand Up @@ -62,8 +62,8 @@ inherits(Client, EventEmitter)

assign(Client.prototype, {

formatter() {
return new Formatter(this)
formatter(builder) {
return new Formatter(this, builder)
},

queryBuilder() {
Expand Down Expand Up @@ -163,20 +163,20 @@ assign(Client.prototype, {
return sql;
},

postProcessResponse(resp) {
postProcessResponse(resp, queryContext) {
if (this.config.postProcessResponse) {
return this.config.postProcessResponse(resp);
return this.config.postProcessResponse(resp, queryContext);
}
return resp;
},

wrapIdentifier(value) {
return this.customWrapIdentifier(value, this.wrapIdentifierImpl);
wrapIdentifier(value, queryContext) {
return this.customWrapIdentifier(value, this.wrapIdentifierImpl, queryContext);
},

customWrapIdentifier(value, origImpl) {
customWrapIdentifier(value, origImpl, queryContext) {
if (this.config.wrapIdentifier) {
return this.config.wrapIdentifier(value, origImpl);
return this.config.wrapIdentifier(value, origImpl, queryContext);
}
return origImpl(value);
},
Expand Down Expand Up @@ -232,7 +232,7 @@ assign(Client.prototype, {
.catch(err => {
// Acquire connection must never reject, because generic-pool
// will retry trying to get connection until acquireConnectionTimeout is
// reached. acquireConnectionTimeout should trigger in knex only
// reached. acquireConnectionTimeout should trigger in knex only
// in that case if aquiring connection waits because pool is full
// https://github.com/coopernurse/node-pool/pull/184
// https://github.com/tgriesser/knex/issues/2325
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/mssql/index.js
Expand Up @@ -43,7 +43,7 @@ assign(Client_MSSQL.prototype, {
},

formatter() {
return new MSSQL_Formatter(this)
return new MSSQL_Formatter(this, ...arguments)
},

transaction() {
Expand Down
4 changes: 2 additions & 2 deletions src/dialects/mssql/schema/tablecompiler.js
Expand Up @@ -74,7 +74,7 @@ assign(TableCompiler_MSSQL.prototype, {
},

dropFKRefs (runner, refs) {
const formatter = this.client.formatter();
const formatter = this.client.formatter(this.tableBuilder);
return Promise.all(refs.map(function (ref) {
const constraintName = formatter.wrap(ref.CONSTRAINT_NAME);
const tableName = formatter.wrap(ref.TABLE_NAME);
Expand All @@ -84,7 +84,7 @@ assign(TableCompiler_MSSQL.prototype, {
}));
},
createFKRefs (runner, refs) {
const formatter = this.client.formatter();
const formatter = this.client.formatter(this.tableBuilder);

return Promise.all(refs.map(function (ref) {
const tableName = formatter.wrap(ref.TABLE_NAME);
Expand Down
6 changes: 3 additions & 3 deletions src/dialects/mysql/schema/tablecompiler.js
Expand Up @@ -111,7 +111,7 @@ assign(TableCompiler_MySQL.prototype, {
},

getFKRefs (runner) {
const formatter = this.client.formatter();
const formatter = this.client.formatter(this.tableBuilder);
const sql = 'SELECT KCU.CONSTRAINT_NAME, KCU.TABLE_NAME, KCU.COLUMN_NAME, '+
' KCU.REFERENCED_TABLE_NAME, KCU.REFERENCED_COLUMN_NAME, '+
' RC.UPDATE_RULE, RC.DELETE_RULE '+
Expand All @@ -129,7 +129,7 @@ assign(TableCompiler_MySQL.prototype, {
},

dropFKRefs (runner, refs) {
const formatter = this.client.formatter();
const formatter = this.client.formatter(this.tableBuilder);

return Promise.all(refs.map(function (ref) {
const constraintName = formatter.wrap(ref.CONSTRAINT_NAME);
Expand All @@ -140,7 +140,7 @@ assign(TableCompiler_MySQL.prototype, {
}));
},
createFKRefs (runner, refs) {
const formatter = this.client.formatter();
const formatter = this.client.formatter(this.tableBuilder);

return Promise.all(refs.map(function (ref) {
const tableName = formatter.wrap(ref.TABLE_NAME);
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/oracle/index.js
Expand Up @@ -41,7 +41,7 @@ assign(Client_Oracle.prototype, {
},

formatter() {
return new Formatter(this)
return new Formatter(this, ...arguments)
},

queryCompiler() {
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/oracledb/index.js
Expand Up @@ -53,7 +53,7 @@ Client_Oracledb.prototype.columnCompiler = function() {
return new ColumnCompiler(this, ...arguments)
}
Client_Oracledb.prototype.formatter = function() {
return new Oracledb_Formatter(this)
return new Oracledb_Formatter(this, ...arguments)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't all these be

= function(builder) {
   return new Oracledb_Formatter(this, builder);
}

Thats more understandable for anyone reading the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had it like that at first but then I noticed that the convention was passing all the arguments, for example on line 53 there:https://github.com/tgriesser/knex/blob/59f6cba17839435e3c10d9e1011d5b78e4a970f0/src/dialects/oracledb/index.js#L53

Or is that because those are the generic compiler and transaction classes?

I agree though that it's more readable..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it has just been personal preference of the one who originally implemented oracledb dialect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. but then it's also the same on the mssql dialect. I don't mean to prod I'm just a sucker for conventions :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both dialects are pretty new ones and might have been used as a reference for each other. But sure you are right, since that syntax is used everywhere in those dialects, there is no reason why it couldn't be used also in this PR. So no need to change that.

}
Client_Oracledb.prototype.transaction = function() {
return new Transaction(this, ...arguments)
Expand Down
8 changes: 5 additions & 3 deletions src/formatter.js
Expand Up @@ -27,8 +27,9 @@ const operators = transform([

export default class Formatter {

constructor(client) {
constructor(client, builder) {
this.client = client
this.builder = builder
this.bindings = []
}

Expand Down Expand Up @@ -119,7 +120,8 @@ export default class Formatter {
}

wrapAsIdentifier(value) {
return this.client.wrapIdentifier((value || '').trim());
const queryContext = this.builder.queryContext();
return this.client.wrapIdentifier((value || '').trim(), queryContext);
}

alias(first, second) {
Expand Down Expand Up @@ -212,7 +214,7 @@ export default class Formatter {
if (i === 0 && segments.length > 1) {
wrapped.push(this.wrap((value || '').trim()));
} else {
wrapped.push(this.client.wrapIdentifier((value || '').trim()));
wrapped.push(this.wrapAsIdentifier(value));
}
}
return wrapped.join('.');
Expand Down
14 changes: 13 additions & 1 deletion src/helpers.js
Expand Up @@ -72,4 +72,16 @@ export function containsUndefined(mixed) {
}

return argContainsUndefined;
}
}

export function addQueryContext(Target) {
// Stores or returns (if called with no arguments) context passed to
// wrapIdentifier and postProcessResponse hooks
Target.prototype.queryContext = function(context) {
if (isUndefined(context)) {
return this._queryContext;
}
this._queryContext = context;
return this;
}
}
4 changes: 4 additions & 0 deletions src/query/builder.js
Expand Up @@ -53,6 +53,9 @@ assign(Builder.prototype, {
if (!isUndefined(this._options)) {
cloned._options = clone(this._options);
}
if (!isUndefined(this._queryContext)) {
cloned._queryContext = clone(this._queryContext);
}

return cloned;
},
Expand Down Expand Up @@ -1002,5 +1005,6 @@ Builder.prototype.del = Builder.prototype.delete

// Attach all of the top level promise methods that should be chainable.
require('../interface')(Builder);
helpers.addQueryContext(Builder);

export default Builder;
2 changes: 1 addition & 1 deletion src/query/compiler.js
Expand Up @@ -26,7 +26,7 @@ function QueryCompiler(client, builder) {
this.timeout = builder._timeout || false;
this.cancelOnTimeout = builder._cancelOnTimeout || false;
this.grouped = groupBy(builder._statements, 'grouping');
this.formatter = client.formatter()
this.formatter = client.formatter(builder)
}

const components = [
Expand Down
7 changes: 4 additions & 3 deletions src/raw.js
Expand Up @@ -14,8 +14,8 @@ import uuid from 'uuid';
const debugBindings = debug('knex:bindings')

const fakeClient = {
formatter() {
return new Formatter(fakeClient)
formatter(builder) {
return new Formatter(fakeClient, builder)
}
}

Expand Down Expand Up @@ -70,7 +70,7 @@ assign(Raw.prototype, {
// Returns the raw sql for the query.
toSQL(method, tz) {
let obj
const formatter = this.client.formatter()
const formatter = this.client.formatter(this)

if (Array.isArray(this.bindings)) {
obj = replaceRawArrBindings(this, formatter)
Expand Down Expand Up @@ -186,5 +186,6 @@ function replaceKeyBindings(raw, formatter) {
// Allow the `Raw` object to be utilized with full access to the relevant
// promise API.
require('./interface')(Raw)
helpers.addQueryContext(Raw);

export default Raw
3 changes: 2 additions & 1 deletion src/runner.js
Expand Up @@ -132,8 +132,9 @@ assign(Runner.prototype, {
return queryPromise
.then((resp) => {
const processedResponse = this.client.processResponse(resp, runner);
const queryContext = this.builder.queryContext();
const postProcessedResponse = this.client
.postProcessResponse(processedResponse);
.postProcessResponse(processedResponse, queryContext);

this.builder.emit(
'query-response',
Expand Down
2 changes: 2 additions & 0 deletions src/schema/builder.js
Expand Up @@ -2,6 +2,7 @@
import inherits from 'inherits';
import { EventEmitter } from 'events';
import { each, toArray } from 'lodash'
import { addQueryContext } from '../helpers';

// Constructor for the builder instance, typically called from
// `knex.builder`, accepting the current `knex` instance,
Expand Down Expand Up @@ -47,6 +48,7 @@ each([
})

require('../interface')(SchemaBuilder)
addQueryContext(SchemaBuilder);

SchemaBuilder.prototype.withSchema = function(schemaName) {
this._schema = schemaName;
Expand Down
5 changes: 4 additions & 1 deletion src/schema/columnbuilder.js
@@ -1,5 +1,6 @@

import { extend, each, toArray } from 'lodash'
import { extend, each, toArray } from 'lodash';
import { addQueryContext } from '../helpers';

// The chainable interface off the original "column" method.
export default function ColumnBuilder(client, tableBuilder, type, args) {
Expand Down Expand Up @@ -41,6 +42,8 @@ each(modifiers, function(method) {
};
});

addQueryContext(ColumnBuilder);

ColumnBuilder.prototype.notNull =
ColumnBuilder.prototype.notNullable = function notNullable() {
return this.nullable(false)
Expand Down
2 changes: 1 addition & 1 deletion src/schema/columncompiler.js
Expand Up @@ -16,7 +16,7 @@ function ColumnCompiler(client, tableCompiler, columnBuilder) {
this.grouped = groupBy(columnBuilder._statements, 'grouping');
this.modified = columnBuilder._modifiers;
this.isIncrements = (this.type.indexOf('increments') !== -1);
this.formatter = client.formatter();
this.formatter = client.formatter(columnBuilder);
this.sequence = [];
this.modifiers = [];
}
Expand Down
10 changes: 8 additions & 2 deletions src/schema/compiler.js
@@ -1,7 +1,7 @@

import { pushQuery, pushAdditional } from './helpers';

import { assign } from 'lodash'
import { assign, isUndefined } from 'lodash'

// The "SchemaCompiler" takes all of the query statements which have been
// gathered in the "SchemaBuilder" and turns them into an array of
Expand All @@ -10,7 +10,7 @@ function SchemaCompiler(client, builder) {
this.builder = builder
this.client = client
this.schema = builder._schema;
this.formatter = client.formatter()
this.formatter = client.formatter(builder)
this.sequence = []
}

Expand Down Expand Up @@ -60,6 +60,12 @@ function buildTable(type) {
return function(tableName, fn) {
const builder = this.client.tableBuilder(type, tableName, fn);

// pass queryContext down to tableBuilder but do not overwrite it if already set
const queryContext = this.builder.queryContext();
if (!isUndefined(queryContext) && isUndefined(builder.queryContext())) {
builder.queryContext(queryContext);
}

builder.setSchema(this.schema);
const sql = builder.toSQL();

Expand Down
15 changes: 14 additions & 1 deletion src/schema/helpers.js
@@ -1,5 +1,8 @@

import { isString, tail } from 'lodash'
import ColumnCompiler from './columncompiler'
import TableCompiler from './tablecompiler'
import SchemaCompiler from './compiler'

// Push a new query onto the compiled "sequence" stack,
// creating a new formatter, returning the compiler.
Expand All @@ -12,7 +15,17 @@ export function pushQuery(query) {
query.bindings = this.formatter.bindings;
}
this.sequence.push(query);
this.formatter = this.client.formatter();

let builder;
if (this instanceof ColumnCompiler) {
builder = this.columnBuilder;
} else if (this instanceof TableCompiler) {
builder = this.tableBuilder;
} else if (this instanceof SchemaCompiler) {
builder = this.builder;
}

this.formatter = this.client.formatter(builder);
}

// Used in cases where we need to push some additional column specific statements.
Expand Down
2 changes: 2 additions & 0 deletions src/schema/tablebuilder.js
Expand Up @@ -85,6 +85,8 @@ each(specialMethods, function(methods, dialect) {
});
});

helpers.addQueryContext(TableBuilder);

// Each of the column types that we can add, we create a new ColumnBuilder
// instance and push it onto the statements array.
const columnTypes = [
Expand Down
15 changes: 12 additions & 3 deletions src/schema/tablecompiler.js
Expand Up @@ -4,16 +4,17 @@
// -------
import { pushAdditional, pushQuery } from './helpers';
import * as helpers from '../helpers';
import { groupBy, reduce, map, first, tail, isEmpty, indexOf, isArray } from 'lodash'
import { groupBy, reduce, map, first, tail, isEmpty, indexOf, isArray, isUndefined } from 'lodash'

function TableCompiler(client, tableBuilder) {
this.client = client
this.tableBuilder = tableBuilder;
this.method = tableBuilder._method;
this.schemaNameRaw = tableBuilder._schemaName;
this.tableNameRaw = tableBuilder._tableName;
this.single = tableBuilder._single;
this.grouped = groupBy(tableBuilder._statements, 'grouping');
this.formatter = client.formatter();
this.formatter = client.formatter(tableBuilder);
this.sequence = [];
this._formatting = client.config && client.config.formatting
}
Expand Down Expand Up @@ -143,9 +144,17 @@ TableCompiler.prototype.getColumns = function (method) {
const columns = this.grouped.columns || [];
method = method || 'add';

const queryContext = this.tableBuilder.queryContext();

return columns
.filter(column => column.builder._method === method)
.map(column => this.client.columnCompiler(this, column.builder));
.map(column => {
// pass queryContext down to columnBuilder but do not overwrite it if already set
if (!isUndefined(queryContext) && isUndefined(column.builder.queryContext())) {
column.builder.queryContext(queryContext);
}
return this.client.columnCompiler(this, column.builder);
});
};

TableCompiler.prototype.tableName = function () {
Expand Down