Skip to content

Commit

Permalink
Refactor mssql dialect as per pull 2do2go#26 feedback
Browse files Browse the repository at this point in the history 
This commit refactors the `mssql` dialect as per the feedback
from @artzhookov on 2do2go#26.

 - Include all test files in gulp lint tasks
 - Move mssql `value` block override to blocks.js
 - Update mssql `limit` block to use `_.isUndefined`
 - Clean up minor mssql sql letter case and format
 - Set the `mssql` `identifier` `prefix` and `suffix`

This commit also introduces two changes to the `base` dialect.
 - Refactor the `base `insert:values` `block` function to clean up
   the logic
 - Refactor the `mssql` `inserted:values` `block` function to match
   the `base` function, and also include `returning` params, as it
   is needed. (mssql puts `output` keyword before `values` keyword)
 - Update `buildTemplate` to include an edge case where a `buildBlock`
   function does not return any value. This prevents unnecessary
   whitespace in the final query string.

These changes are not strictly necessary. The `mssql` `insert:values` `block`
could use the same structure as the original `base` `block`, and unnecessary
whitespace has no issues in terms of functionality, simply ascetics.

This commit refactors the `mssql` tests as per the changes made.
Test coverage and layout is not ideal, but functional.
  • Loading branch information
@ojourmel
ojourmel committed Aug 26, 2017
1 parent 6bd2f6a commit b22ec97
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 76 deletions.
2 changes: 1 addition & 1 deletion gulpfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ gulp.task('coverage', function(callback) {
.pipe(istanbul())
.pipe(istanbul.hookRequire())
.on('finish', function() {
gulp.src(['./tests/*.js'], {read: false})
gulp.src(['./tests/**/*.js'], {read: false})
.pipe(mocha({
reporter: 'spec',
bail: true
Expand Down
17 changes: 10 additions & 7 deletions lib/dialects/base/blocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,11 @@ module.exports = function(dialect) {

dialect.blocks.add('insert:values', function(params) {
var values = params.values;
var fields = params.fields;

if (!_.isArray(values)) values = [values];

var fields = params.fields || _(values)
fields = fields || _(values)
.chain()
.map(function(row) {
return _(row).keys();
Expand All @@ -345,13 +346,15 @@ module.exports = function(dialect) {
.uniq()
.value();

values = _(values).map(function(row) {
return _(fields).map(function(field) {
return dialect.buildBlock('value', {value: row[field]});
});
});

return dialect.buildTemplate('insertValues', {
fields: fields,
values: _(values).map(function(row) {
return _(fields).map(function(field) {
return dialect.buildBlock('value', {value: row[field]});
});
})
values: values,
fields: fields
});
});

Expand Down
3 changes: 2 additions & 1 deletion lib/dialects/base/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ Dialect.prototype.buildTemplate = function(type, params) {
return '';
} else {
if (self.blocks.has(type + ':' + block)) block = type + ':' + block;
return self.buildBlock(block, params) + space;
var result = self.buildBlock(block, params);
return result === '' ? result : result + space;
}
}).trim();
};
74 changes: 51 additions & 23 deletions lib/dialects/mssql/blocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,38 @@
var _ = require('underscore');

module.exports = function(dialect) {
var parentValueBlock = dialect.blocks.get('value');

dialect.blocks.set('value', function(params) {
var value = params.value;

var result;
if (_.isUndefined(value) || _.isNull(value)) {
result = 'null';
} else if (_.isBoolean(value)) {
result = String(Number(value));
} else if (_.isNumber(value)) {
result = String(value);
} else if (_.isString(value) || _.isDate(value)) {
if (dialect.builder.options.separatedValues) {
result = dialect.builder._pushValue(value);
} else {
if (_.isDate(value)) value = value.toISOString();
result = '\'' + value + '\'';
}
} else {
result = parentValueBlock(params);
}

return result;
});

dialect.blocks.set('limit', function(params) {
return (!isNaN(params.offset)) ? '' : 'top(' + dialect.builder._pushValue(params.limit) + ')';
var result = '';
if (_.isUndefined(params.offset)) {
result = 'top(' + dialect.builder._pushValue(params.limit) + ')';
}
return result;
});

dialect.blocks.set('offset', function(params) {
Expand All @@ -13,8 +43,8 @@ module.exports = function(dialect) {
var str = pre + 'offset ' + dialect.builder._pushValue(params.offset);
str += ' rows fetch next ' + dialect.builder._pushValue(params.limit) + ' rows only';
return str;
}else {
return pre + 'OFFSET ' + dialect.builder._pushValue(params.offset) + ' rows';
} else {
return pre + 'offset ' + dialect.builder._pushValue(params.offset) + ' rows';
}
});

Expand All @@ -28,32 +58,30 @@ module.exports = function(dialect) {

dialect.blocks.set('insert:values', function(params) {
var values = params.values;
var fields = params.fields;
var returning = params.returning;

if (!_.isArray(values)) values = [values];

var fields = params.fields || _(values)
.chain()
.map(function(row) {
return _(row).keys();
})
.flatten()
.uniq()
.value();
fields = fields || _(values)
.chain()
.map(function(row) {
return _(row).keys();
})
.flatten()
.uniq()
.value();

values = _(values).map(function(row) {
return _(fields).map(function(field) {
return dialect.buildBlock('value', {value: row[field]});
});
});

return dialect.buildTemplate('insertValues', {
values: values,
fields: fields,
returning: params.returning || undefined,
values: _(values).map(function(row) {
return _(fields).map(function(field) {
return dialect.buildBlock('value', {value: row[field]});
});
})
returning: returning
});
});

dialect.blocks.add('insertValues:values', function(params) {
return _(params.values).map(function(row) {
return '(' + row.join(', ') + ')';
}).join(', ');
});
};
35 changes: 4 additions & 31 deletions lib/dialects/mssql/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,45 +7,18 @@ var templatesInit = require('./templates');
var blocksInit = require('./blocks');

var Dialect = module.exports = function(builder) {

builder._pushValue = function(value) {
if (_.isUndefined(value) || _.isNull(value)) {
return 'null';
} else if (_.isBoolean(value)) {
return String(Number(value));
} else if (_.isNumber(value)) {
return String(value);
} else if (_.isString(value) || _.isDate(value)) {
if (this.options.separatedValues) {
var placeholder = this._getPlaceholder();

if (this.options.namedValues) {
this._values[placeholder] = value;
} else {
this._values.push(value);
}

return this._wrapPlaceholder(placeholder);
} else {
if (_.isDate(value)) value = value.toISOString();

return '\'' + value + '\'';
}
} else {
throw new Error('Wrong value type "' + (typeof value) + '"');
}
};

BaseDialect.call(this, builder);

// init templates
templatesInit(this);

// init blocks
blocksInit(this);

};

util.inherits(Dialect, BaseDialect);

Dialect.prototype.config = _({}).extend(BaseDialect.prototype.config, {});
Dialect.prototype.config = _({}).extend(BaseDialect.prototype.config, {
identifierPrefix: '[',
identifierSuffix: ']'
});
10 changes: 5 additions & 5 deletions lib/dialects/mssql/templates.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ module.exports = function(dialect) {
}
});

dialect.templates.add('insert', {
dialect.templates.set('insert', {
pattern: '{with} {withRecursive} insert {or} into {table} {values} ' +
'{condition}',
'{condition}',
validate: function(type, params) {
templateChecks.onlyOneOfProps(type, params, ['with', 'withRecursive']);
templateChecks.propType(type, params, 'with', 'object');
Expand All @@ -68,7 +68,7 @@ module.exports = function(dialect) {
}
});

dialect.templates.add('insertValues', {
dialect.templates.set('insertValues', {
pattern: '({fields}) {returning} values {values}',
validate: function(type, params) {
templateChecks.requiredProp('values', params, 'fields');
Expand All @@ -83,7 +83,7 @@ module.exports = function(dialect) {
}
});

dialect.templates.add('update', {
dialect.templates.set('update', {
pattern: '{with} {withRecursive} update {or} {table} {alias} {modifier} {returning} ' +
'{condition} ',
validate: function(type, params) {
Expand All @@ -109,7 +109,7 @@ module.exports = function(dialect) {
}
});

dialect.templates.add('remove', {
dialect.templates.set('remove', {
pattern: '{with} {withRecursive} delete from {table} {returning} {alias} {condition} ',
validate: function(type, params) {
templateChecks.onlyOneOfProps(type, params, ['with', 'withRecursive']);
Expand Down
20 changes: 12 additions & 8 deletions tests/6_dialects/1_mssql.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('MSSQL dialect', function() {
'name': {$eq: 'test'}
}
});
expect(result.query).to.be.equal('select top(1) "user" from "test" where "name" = $1;');
expect(result.query).to.be.equal('select top(1) [user] from [test] where [name] = $1;');
});

it('should be ok with `limit` and `offset` properties', function() {
Expand All @@ -30,7 +30,7 @@ describe('MSSQL dialect', function() {
'name': {$eq: 'test'}
}
});
expect(result.query).to.be.equal('select "user" from "test" where "name" = $1 order by 1' +
expect(result.query).to.be.equal('select [user] from [test] where [name] = $1 order by 1' +
' offset 2 rows fetch next 4 rows only;');
});
});
Expand All @@ -39,25 +39,29 @@ describe('MSSQL dialect', function() {
var result = jsonSql.build({
type: 'remove',
table: 'test',
returning: ['DELETED.*'],
returning: [
{table: 'deleted', name: '*'}
],
condition: {
Description: {$eq: 'test'}
}
});
expect(result.query).to.be.equal('delete from "test" output "DELETED".* where ' +
'"Description" = $1;');
expect(result.query).to.be.equal('delete from [test] output [deleted].* where ' +
'[Description] = $1;');
});
it('should be ok with `insert` type', function() {
var result = jsonSql.build({
type: 'insert',
table: 'test',
returning: ['INSERTED.*'],
returning: [
{table: 'inserted', name: '*'}
],
values: {
Description: 'test',
}
});
expect(result.query).to.be.equal('insert into "test" ("Description") output ' +
'"INSERTED".* values ($1);');
expect(result.query).to.be.equal('insert into [test] ([Description]) output ' +
'[inserted].* values ($1);');
});
});
});

0 comments on commit b22ec97

Please sign in to comment.