Skip to content

Commit

Permalink
Ensure that 'client' is provided in knex config object (#1822)
Browse files Browse the repository at this point in the history
* Ensure that 'client' is provided in knex config object

* sqlite3 columncompiler: Assign 'modifiers' --after-- super() call.

* oracle columncompiler: Assign 'modifiers' --after-- super() call.
  • Loading branch information
wubzz authored and elhigu committed Dec 11, 2016
1 parent 0ae4f3e commit a4bfee7
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 5 deletions.
8 changes: 8 additions & 0 deletions src/client.js
Expand Up @@ -37,6 +37,14 @@ function clientId() {
// for a dialect specific client object.
function Client(config = {}) {
this.config = config

//Client is a required field, so throw error if it's not supplied.
//If 'this.dialect' is set, then this is a 'super()' call, in which case
//'client' does not have to be set as it's already assigned on the client prototype.
if(!this.config.client && !this.dialect) {
throw new Error(`knex: Required configuration option 'client' is missing.`)
}

this.connectionSettings = cloneDeep(config.connection || {})
if (this.driverName && config.connection) {
this.initializeDriver()
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/oracle/schema/columncompiler.js
Expand Up @@ -9,8 +9,8 @@ import Trigger from './trigger';
// -------

function ColumnCompiler_Oracle() {
this.modifiers = ['defaultTo', 'checkIn', 'nullable', 'comment'];
ColumnCompiler.apply(this, arguments);
this.modifiers = ['defaultTo', 'checkIn', 'nullable', 'comment'];
}
inherits(ColumnCompiler_Oracle, ColumnCompiler);

Expand Down
2 changes: 1 addition & 1 deletion src/dialects/sqlite3/schema/columncompiler.js
Expand Up @@ -6,8 +6,8 @@ import ColumnCompiler from '../../../schema/columncompiler';
// -------

function ColumnCompiler_SQLite3() {
this.modifiers = ['nullable', 'defaultTo'];
ColumnCompiler.apply(this, arguments);
this.modifiers = ['nullable', 'defaultTo'];
}
inherits(ColumnCompiler_SQLite3, ColumnCompiler);

Expand Down
1 change: 1 addition & 0 deletions src/schema/columncompiler.js
Expand Up @@ -18,6 +18,7 @@ function ColumnCompiler(client, tableCompiler, columnBuilder) {
this.isIncrements = (this.type.indexOf('increments') !== -1);
this.formatter = client.formatter();
this.sequence = [];
this.modifiers = [];
}

ColumnCompiler.prototype.pushQuery = helpers.pushQuery
Expand Down
10 changes: 10 additions & 0 deletions test/tape/knex.js
Expand Up @@ -65,3 +65,13 @@ test('it should use knex supported dialect', function(t) {
})
knexObj.destroy()
})

test('it should throw error if client is omitted in config', function(t) {
t.plan(1);
try {
var knexObj = knex({});
t.deepEqual(true, false); //Don't reach this point
} catch(error) {
t.deepEqual(error.message, 'knex: Required configuration option \'client\' is missing.');
}
})
4 changes: 2 additions & 2 deletions test/tape/query-builder.js
Expand Up @@ -14,7 +14,7 @@ tape('accumulates multiple update calls #647', function(t) {

tape('allows for object syntax in join', function(t) {
t.plan(1)
var qb = new QueryBuilder(new Client())
var qb = new QueryBuilder(new Client({client: 'mysql'}))
var sql = qb.table('users').innerJoin('accounts', {
'accounts.id': 'users.account_id',
'accounts.owner_id': 'users.id'
Expand All @@ -24,7 +24,7 @@ tape('allows for object syntax in join', function(t) {
})

tape('clones correctly', function(t) {
var qb = new QueryBuilder(new Client())
var qb = new QueryBuilder(new Client({client: 'mysql'}))
var original = qb.table('users').debug().innerJoin('accounts', {
'accounts.id': 'users.account_id',
'accounts.owner_id': 'users.id'
Expand Down
2 changes: 1 addition & 1 deletion test/tape/raw.js
Expand Up @@ -5,7 +5,7 @@ var Client = require('../../lib/client')
var test = require('tape')
var _ = require('lodash');

var client = new Client()
var client = new Client({client: 'mysql'})
function raw(sql, bindings) {
return new Raw(client).set(sql, bindings)
}
Expand Down

2 comments on commit a4bfee7

@TimonLukas
Copy link

Choose a reason for hiding this comment

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

This is a breaking change and probably should either not have been merged, or at least should have been clarified in the documentation.

@elhigu
Copy link
Member

@elhigu elhigu commented on a4bfee7 Feb 21, 2017

Choose a reason for hiding this comment

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

discussion about it is here #1822

Please sign in to comment.