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

fix(pool): show deprecation when engine is not supported #12218

Merged
merged 4 commits into from
May 4, 2020
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
4 changes: 2 additions & 2 deletions ENGINE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

## v6-beta
| Engine | Minimum supported version |
| ------------ | :------------: |
| :------------: | :------------: |
| Postgre | [9.5 ](https://www.postgresql.org/docs/9.5/ ) |
| MySQL | [5.7](https://dev.mysql.com/doc/refman/5.7/en/) |
| MariaDB | [10.1](https://mariadb.com/kb/en/changes-improvements-in-mariadb-101/) |
| Microsoft SQL | ? |
| Microsoft SQL | `12.0.2000` |
| SQLite | [3.0](https://www.sqlite.org/version3.html)
2 changes: 1 addition & 1 deletion lib/data-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const momentTz = require('moment-timezone');
const moment = require('moment');
const { logger } = require('./utils/logger');
const warnings = {};
const { classToInvokable } = require('./utils/classToInvokable');
const { classToInvokable } = require('./utils/class-to-invokable');
const { joinSQLFragments } = require('./utils/join-sql-fragments');

class ABSTRACT {
Expand Down
8 changes: 7 additions & 1 deletion lib/dialects/abstract/connection-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const _ = require('lodash');
const semver = require('semver');
const errors = require('../../errors');
const { logger } = require('../../utils/logger');
const deprecations = require('../../utils/deprecations');
const debug = logger.debugContext('pool');

/**
Expand Down Expand Up @@ -260,7 +261,12 @@ class ConnectionManager {
const parsedVersion = _.get(semver.coerce(version), 'version') || version;
this.sequelize.options.databaseVersion = semver.valid(parsedVersion)
? parsedVersion
: this.defaultVersion;
: this.dialect.defaultVersion;
}

if (semver.lt(this.sequelize.options.databaseVersion, this.dialect.defaultVersion)) {
deprecations.unsupportedEngine();
debug(`Unsupported database engine version ${this.sequelize.options.databaseVersion}`);
}

this.versionPromise = null;
Expand Down
4 changes: 3 additions & 1 deletion lib/dialects/mariadb/connection-manager.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const semver = require('semver');
const AbstractConnectionManager = require('../abstract/connection-manager');
const SequelizeErrors = require('../../errors');
const { logger } = require('../../utils/logger');
Expand Down Expand Up @@ -84,7 +85,8 @@ class ConnectionManager extends AbstractConnectionManager {

try {
const connection = await this.lib.createConnection(connectionConfig);
this.sequelize.options.databaseVersion = connection.serverVersion();
this.sequelize.options.databaseVersion = semver.coerce(connection.serverVersion()).version;

debug('connection acquired');
connection.on('error', error => {
switch (error.code) {
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/mariadb/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ MariadbDialect.prototype.supports = _.merge(
REGEXP: true
});

ConnectionManager.prototype.defaultVersion = '5.5.3';
MariadbDialect.prototype.defaultVersion = '10.1.44';
MariadbDialect.prototype.Query = Query;
MariadbDialect.prototype.QueryGenerator = QueryGenerator;
MariadbDialect.prototype.DataTypes = DataTypes;
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/mssql/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ MssqlDialect.prototype.supports = _.merge(_.cloneDeep(AbstractDialect.prototype.
tmpTableTrigger: true
});

ConnectionManager.prototype.defaultVersion = '12.0.2000'; // SQL Server 2014 Express
MssqlDialect.prototype.defaultVersion = '12.0.2000'; // SQL Server 2014 Express
MssqlDialect.prototype.Query = Query;
MssqlDialect.prototype.name = 'mssql';
MssqlDialect.prototype.TICK_CHAR = '"';
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/mysql/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ MysqlDialect.prototype.supports = _.merge(_.cloneDeep(AbstractDialect.prototype.
REGEXP: true
});

ConnectionManager.prototype.defaultVersion = '5.6.0';
MysqlDialect.prototype.defaultVersion = '5.7.0';
MysqlDialect.prototype.Query = Query;
MysqlDialect.prototype.QueryGenerator = QueryGenerator;
MysqlDialect.prototype.DataTypes = DataTypes;
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/postgres/connection-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class ConnectionManager extends AbstractConnectionManager {
const version = semver.coerce(message.parameterValue).version;
this.sequelize.options.databaseVersion = semver.valid(version)
? version
: this.defaultVersion;
: this.dialect.defaultVersion;
}
break;
case 'standard_conforming_strings':
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/postgres/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ PostgresDialect.prototype.supports = _.merge(_.cloneDeep(AbstractDialect.prototy
searchPath: true
});

ConnectionManager.prototype.defaultVersion = '9.4.0';
PostgresDialect.prototype.defaultVersion = '9.5.0';
PostgresDialect.prototype.Query = Query;
PostgresDialect.prototype.DataTypes = DataTypes;
PostgresDialect.prototype.name = 'postgres';
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/sqlite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ SqliteDialect.prototype.supports = _.merge(_.cloneDeep(AbstractDialect.prototype
JSON: true
});

ConnectionManager.prototype.defaultVersion = '3.8.0';
SqliteDialect.prototype.defaultVersion = '3.8.0';
SqliteDialect.prototype.Query = Query;
SqliteDialect.prototype.DataTypes = DataTypes;
SqliteDialect.prototype.name = 'sqlite';
Expand Down
2 changes: 1 addition & 1 deletion lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const operatorsSet = new Set(Object.values(operators));

let inflection = require('inflection');

exports.classToInvokable = require('./utils/classToInvokable').classToInvokable;
exports.classToInvokable = require('./utils/class-to-invokable').classToInvokable;
exports.joinSQLFragments = require('./utils/join-sql-fragments').joinSQLFragments;

function useInflection(_inflection) {
Expand Down
File renamed without changes.
1 change: 1 addition & 0 deletions lib/utils/deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ exports.noTrueLogging = deprecate(noop, 'The logging-option should be either a f
exports.noStringOperators = deprecate(noop, 'String based operators are deprecated. Please use Symbol based operators for better security, read more at https://sequelize.org/master/manual/querying.html#operators', 'SEQUELIZE0003');
exports.noBoolOperatorAliases = deprecate(noop, 'A boolean value was passed to options.operatorsAliases. This is a no-op with v5 and should be removed.', 'SEQUELIZE0004');
exports.noDoubleNestedGroup = deprecate(noop, 'Passing a double nested nested array to `group` is unsupported and will be removed in v6.', 'SEQUELIZE0005');
exports.unsupportedEngine = deprecate(noop, 'This database engine version is not supported, please update your database server. More information https://github.com/sequelize/sequelize/blob/master/ENGINE.md', 'SEQUELIZE0006');
47 changes: 38 additions & 9 deletions test/integration/dialects/abstract/connection-manager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const chai = require('chai'),
expect = chai.expect,
deprecations = require('../../../../lib/utils/deprecations'),
Support = require('../../support'),
sinon = require('sinon'),
Config = require('../../../config/config'),
Expand All @@ -15,7 +16,7 @@ const poolEntry = {
pool: {}
};

describe('Connection Manager', () => {
describe(Support.getTestDialectTeaser('Connection Manager'), () => {
let sandbox;

beforeEach(() => {
Expand All @@ -31,7 +32,7 @@ describe('Connection Manager', () => {
replication: null
};
const sequelize = Support.createSequelizeInstance(options);
const connectionManager = new ConnectionManager(Support.getTestDialect(), sequelize);
const connectionManager = new ConnectionManager(sequelize.dialect, sequelize);

connectionManager.initPools();
expect(connectionManager.pool).to.be.instanceOf(Pool);
Expand All @@ -47,7 +48,7 @@ describe('Connection Manager', () => {
}
};
const sequelize = Support.createSequelizeInstance(options);
const connectionManager = new ConnectionManager(Support.getTestDialect(), sequelize);
const connectionManager = new ConnectionManager(sequelize.dialect, sequelize);

connectionManager.initPools();
expect(connectionManager.pool.read).to.be.instanceOf(Pool);
Expand All @@ -71,15 +72,15 @@ describe('Connection Manager', () => {
}
};
const sequelize = Support.createSequelizeInstance(options);
const connectionManager = new ConnectionManager(Support.getTestDialect(), sequelize);
const connectionManager = new ConnectionManager(sequelize.dialect, sequelize);

const res = {
queryType: 'read'
};

const connectStub = sandbox.stub(connectionManager, '_connect').resolves(res);
sandbox.stub(connectionManager, '_disconnect').resolves(res);
sandbox.stub(sequelize, 'databaseVersion').resolves(res);
sandbox.stub(sequelize, 'databaseVersion').resolves(sequelize.dialect.defaultVersion);
connectionManager.initPools();

const queryOptions = {
Expand All @@ -104,6 +105,34 @@ describe('Connection Manager', () => {
});
});

it('should trigger deprecation for non supported engine version', () => {
const deprecationStub = sandbox.stub(deprecations, 'unsupportedEngine');
const sequelize = Support.createSequelizeInstance();
const connectionManager = new ConnectionManager(sequelize.dialect, sequelize);

sandbox.stub(sequelize, 'databaseVersion').resolves('0.0.1');

const res = {
queryType: 'read'
};

sandbox.stub(connectionManager, '_connect').resolves(res);
sandbox.stub(connectionManager, '_disconnect').resolves(res);
connectionManager.initPools();

const queryOptions = {
priority: 0,
type: 'SELECT',
useMaster: true
};

return connectionManager.getConnection(queryOptions)
.then(() => {
chai.expect(deprecationStub).to.have.been.calledOnce;
});
});


it('should allow forced reads from the write pool', () => {
const master = { ...poolEntry };
master.host = 'the-boss';
Expand All @@ -115,14 +144,15 @@ describe('Connection Manager', () => {
}
};
const sequelize = Support.createSequelizeInstance(options);
const connectionManager = new ConnectionManager(Support.getTestDialect(), sequelize);
const connectionManager = new ConnectionManager(sequelize.dialect, sequelize);

const res = {
queryType: 'read'
};

const connectStub = sandbox.stub(connectionManager, '_connect').resolves(res);
sandbox.stub(connectionManager, '_disconnect').resolves(res);
sandbox.stub(sequelize, 'databaseVersion').resolves(res);
sandbox.stub(sequelize, 'databaseVersion').resolves(sequelize.dialect.defaultVersion);
connectionManager.initPools();

const queryOptions = {
Expand All @@ -144,7 +174,7 @@ describe('Connection Manager', () => {
replication: null
};
const sequelize = Support.createSequelizeInstance(options);
const connectionManager = new ConnectionManager(Support.getTestDialect(), sequelize);
const connectionManager = new ConnectionManager(sequelize.dialect, sequelize);

connectionManager.initPools();

Expand All @@ -156,5 +186,4 @@ describe('Connection Manager', () => {
expect(poolClearSpy.calledOnce).to.be.true;
});
});

});