Skip to content

Commit

Permalink
Merge pull request #1312 from tediousjs/arthur/remove-validate-bulk-l…
Browse files Browse the repository at this point in the history
…oad-parameters

feat: remove `validateBulkLoadParameters` connection option
  • Loading branch information
arthurschreiber committed Aug 5, 2021
2 parents b7af4e6 + fbe3aeb commit b649874
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 142 deletions.
26 changes: 6 additions & 20 deletions src/bulk-load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,10 @@ class RowTransform extends Transform {
const c = this.columns[i];
let value = Array.isArray(row) ? row[i] : row[c.objName];

if (this.bulkLoad.options.validateBulkLoadParameters) {
try {
value = c.type.validate(value);
} catch (error) {
return callback(error);
}
try {
value = c.type.validate(value);
} catch (error) {
return callback(error);
}

const parameter = {
Expand Down Expand Up @@ -473,23 +471,11 @@ class BulkLoad extends EventEmitter {
// write each column
if (Array.isArray(row)) {
this.rowToPacketTransform.write(this.columns.map((column, i) => {
let value = row[i];

if (this.options.validateBulkLoadParameters) {
value = column.type.validate(value);
}

return value;
return column.type.validate(row[i]);
}));
} else {
this.rowToPacketTransform.write(this.columns.map((column) => {
let value = row[column.objName];

if (this.options.validateBulkLoadParameters) {
value = column.type.validate(value);
}

return value;
return column.type.validate(row[column.objName]);
}));
}
}
Expand Down
21 changes: 0 additions & 21 deletions src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,6 @@ export interface InternalConnectionOptions {
trustServerCertificate: boolean;
useColumnNames: boolean;
useUTC: boolean;
validateBulkLoadParameters: boolean;
workstationId: undefined | string;
lowerCaseGuids: boolean;
}
Expand Down Expand Up @@ -824,13 +823,6 @@ export interface ConnectionOptions {
*/
useUTC?: boolean;

/**
* A boolean determining whether BulkLoad parameters should be validated.
*
* (default: `true`).
*/
validateBulkLoadParameters?: boolean;

/**
* The workstation ID (WSID) of the client, default os.hostname().
* Used for identifying a specific client in profiling, logging or
Expand Down Expand Up @@ -1276,7 +1268,6 @@ class Connection extends EventEmitter {
trustServerCertificate: true,
useColumnNames: false,
useUTC: true,
validateBulkLoadParameters: true,
workstationId: undefined,
lowerCaseGuids: false
}
Expand Down Expand Up @@ -1677,18 +1668,6 @@ class Connection extends EventEmitter {
this.config.options.useUTC = config.options.useUTC;
}

if (config.options.validateBulkLoadParameters !== undefined) {
if (typeof config.options.validateBulkLoadParameters !== 'boolean') {
throw new TypeError('The "config.options.validateBulkLoadParameters" property must be of type boolean.');
}

if (config.options.validateBulkLoadParameters === false) {
deprecate('Setting the "config.options.validateBulkLoadParameters" to `false` is deprecated and will no longer work in the next major version of `tedious`. Set the value to `true` and update your use of BulkLoad functionality to silence this message.');
}

this.config.options.validateBulkLoadParameters = config.options.validateBulkLoadParameters;
}

if (config.options.workstationId !== undefined) {
if (typeof config.options.workstationId !== 'string') {
throw new TypeError('The "config.options.workstationId" property must be of type string.');
Expand Down
151 changes: 60 additions & 91 deletions test/integration/bulk-load-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1536,117 +1536,86 @@ describe('BulkLoad', function() {
done();
}
});
});

describe('Bulk Loads when `config.options.validateBulkLoadParameters` is `true`', () => {
/**
* @type {Connection}
*/
let connection;

beforeEach(function(done) {
const config = getConfig();
config.options = { ...config.options, validateBulkLoadParameters: true };
connection = new Connection(config);
connection.connect(done);

if (debugMode) {
connection.on('debug', (message) => console.log(message));
connection.on('infoMessage', (info) =>
console.log('Info: ' + info.number + ' - ' + info.message)
);
connection.on('errorMessage', (error) =>
console.log('Error: ' + error.number + ' - ' + error.message)
);
}
});
describe('validation errors', function() {
beforeEach(function(done) {
const request = new Request('create table #stream_test ([value] date)', (err) => {
done(err);
});

beforeEach(function(done) {
const request = new Request('create table #stream_test ([value] date)', (err) => {
done(err);
connection.execSqlBatch(request);
});

connection.execSqlBatch(request);
});
it('should handle validation errors during streaming bulk loads', (done) => {
const bulkLoad = connection.newBulkLoad('#stream_test', completeBulkLoad);
bulkLoad.addColumn('value', TYPES.Date, { nullable: false });

afterEach(function(done) {
if (!connection.closed) {
connection.on('end', done);
connection.close();
} else {
done();
}
});
const rowStream = bulkLoad.getRowStream();
connection.execBulkLoad(bulkLoad);

it('should handle validation errors during streaming bulk loads', (done) => {
const bulkLoad = connection.newBulkLoad('#stream_test', completeBulkLoad);
bulkLoad.addColumn('value', TYPES.Date, { nullable: false });
const rowSource = Readable.from([
['invalid date']
]);

const rowStream = bulkLoad.getRowStream();
connection.execBulkLoad(bulkLoad);
pipeline(rowSource, rowStream, (err) => {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');
});

const rowSource = Readable.from([
['invalid date']
]);
/**
* @param {Error | undefined | null} err
* @param {undefined | number} rowCount
*/
function completeBulkLoad(err, rowCount) {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');

pipeline(rowSource, rowStream, (err) => {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');
done();
}
});

/**
* @param {Error | undefined | null} err
* @param {undefined | number} rowCount
*/
function completeBulkLoad(err, rowCount) {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');

done();
}
});

it('should allow reusing the connection after validation errors during streaming bulk loads', (done) => {
const bulkLoad = connection.newBulkLoad('#stream_test', completeBulkLoad);
bulkLoad.addColumn('value', TYPES.Date, { nullable: false });

const rowStream = bulkLoad.getRowStream();
connection.execBulkLoad(bulkLoad);

const rowSource = Readable.from([ ['invalid date'] ]);
it('should allow reusing the connection after validation errors during streaming bulk loads', (done) => {
const bulkLoad = connection.newBulkLoad('#stream_test', completeBulkLoad);
bulkLoad.addColumn('value', TYPES.Date, { nullable: false });

pipeline(rowSource, rowStream, (err) => {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');
});
const rowStream = bulkLoad.getRowStream();
connection.execBulkLoad(bulkLoad);

/**
* @param {Error | undefined | null} err
* @param {undefined | number} rowCount
*/
function completeBulkLoad(err, rowCount) {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');
const rowSource = Readable.from([ ['invalid date'] ]);

assert.strictEqual(rowCount, 0);
pipeline(rowSource, rowStream, (err) => {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');
});

/**
* @type {unknown[]}
* @param {Error | undefined | null} err
* @param {undefined | number} rowCount
*/
const rows = [];
const request = new Request('SELECT 1', (err) => {
assert.ifError(err);
function completeBulkLoad(err, rowCount) {
assert.instanceOf(err, TypeError);
assert.strictEqual(/** @type {TypeError} */(err).message, 'Invalid date.');

assert.deepEqual([1], rows);
assert.strictEqual(rowCount, 0);

done();
});
/**
* @type {unknown[]}
*/
const rows = [];
const request = new Request('SELECT 1', (err) => {
assert.ifError(err);

request.on('row', (row) => {
rows.push(row[0].value);
});
assert.deepEqual([1], rows);

connection.execSql(request);
}
done();
});

request.on('row', (row) => {
rows.push(row[0].value);
});

connection.execSql(request);
}
});
});
});
2 changes: 1 addition & 1 deletion test/unit/bulk-load-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('BulkLoad', function() {
});

it('throws an error when adding row with a value has the wrong data type', function() {
const request = new BulkLoad('tablename', { tdsVersion: '7_2', validateBulkLoadParameters: true }, { });
const request = new BulkLoad('tablename', { tdsVersion: '7_2' }, { });
request.addColumn('columnName', TYPES.Date, { nullable: true });
assert.throws(() => {
request.addRow({ columnName: 'Wrong Input' });
Expand Down
9 changes: 0 additions & 9 deletions test/unit/connection-config-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,4 @@ describe('Connection configuration validation', function() {
new Connection(config);
});
});

it('bad validateBulkLoadParameters value', () => {
const validateBulkLoadParametersVal = 'text';
config.options.validateBulkLoadParameters = validateBulkLoadParametersVal;
config.options.tdsVersion = '7_2';
assert.throws(() => {
new Connection(config);
});
});
});

0 comments on commit b649874

Please sign in to comment.