Skip to content

Commit

Permalink
feat: throw if accessing databaseVersion before it is loaded (#15346)
Browse files Browse the repository at this point in the history
* feat: throw error if databaseVersion is called early

* feat: implemented getDatabaseVersion for getting the internally fetched database version

* tests: updated tests for sequelize.js

* fix: typings for connection manager test

* docs: updated function with jsdocs

* feat: request for version for sqlite

* fix: updated context for postgres specific unit tests

* fix: explicitly specify databaseVersion for postgres in createTable

* feat: databaseVersion defaults to minimum version for tests for all dialects

* refactor: removed the explicit assignment to db version

* refactor: revert and use comparision op for null check

* refactor: remove fetchVersion from sqlite connection manager

* chore: review changes

* feat: replaced internal database version calls with updated function

* fix: fix sqlite _connect implementation

* fix: linting issues

* fix: updated the message that is thrown

* chore: review changes
  • Loading branch information
frostzt committed Dec 9, 2022
1 parent fdbd942 commit 151a458
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 18 deletions.
2 changes: 1 addition & 1 deletion dev/check-connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ const sequelize = createSequelizeInstance();
await sequelize.authenticate();
await sequelize.close();

console.info(`Connected to ${sequelize.dialect.name} ${sequelize.options.databaseVersion} successfully`);
console.info(`Connected to ${sequelize.dialect.name} ${sequelize.getDatabaseVersion()} successfully`);
})();
14 changes: 7 additions & 7 deletions src/dialects/abstract/connection-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export class AbstractConnectionManager<TConnection extends Connection = Connecti
* @param options
*/
async getConnection(options?: GetConnectionOptions) {
await this.#initDatabaseVersion();
await this._initDatabaseVersion();

try {
const result = await this.pool.acquire(options?.type, options?.useMaster);
Expand All @@ -189,8 +189,8 @@ export class AbstractConnectionManager<TConnection extends Connection = Connecti
}
}

async #initDatabaseVersion() {
if (this.sequelize.options.databaseVersion !== 0) {
async _initDatabaseVersion(conn?: TConnection) {
if (this.sequelize.options.databaseVersion != null) {
return;
}

Expand All @@ -203,7 +203,7 @@ export class AbstractConnectionManager<TConnection extends Connection = Connecti
// TODO: move to sequelize.queryRaw instead?
this.#versionPromise = (async () => {
try {
const connection = await this._connect(this.config.replication.write || this.config);
const connection = conn ?? await this._connect(this.config.replication.write || this.config);

// connection might have set databaseVersion value at initialization,
// avoiding a useless round trip
Expand All @@ -216,15 +216,15 @@ export class AbstractConnectionManager<TConnection extends Connection = Connecti
transaction: { connection },
};

const version = await this.sequelize.databaseVersion(options);
const version = await this.sequelize.fetchDatabaseVersion(options);
const parsedVersion = semver.coerce(version)?.version || version;
this.sequelize.options.databaseVersion = semver.valid(parsedVersion)
? parsedVersion
: this.dialect.defaultVersion;

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

return await this._disconnect(connection);
Expand Down
4 changes: 4 additions & 0 deletions src/dialects/sqlite/connection-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ export class SqliteConnectionManager extends AbstractConnectionManager<SqliteCon
this.connections.set(connectionCacheKey, connectionInstance);
});

await this._initDatabaseVersion(connection);

if (this.sequelize.config.password) {
// Make it possible to define and use password for sqlite encryption plugin like sqlcipher
connection.run(`PRAGMA KEY=${this.sequelize.escape(this.sequelize.config.password)}`);
Expand All @@ -102,6 +104,8 @@ export class SqliteConnectionManager extends AbstractConnectionManager<SqliteCon
return connection;
}

async disconnect(_connection: SqliteConnection): Promise<void> {}

async releaseConnection(connection: SqliteConnection, force?: boolean): Promise<void> {
if (connection.filename === ':memory:' && force !== true) {
return;
Expand Down
7 changes: 6 additions & 1 deletion src/sequelize.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1012,10 +1012,15 @@ export class Sequelize extends SequelizeTypeScript {
normalizeDataType(Type: string): string;
normalizeDataType(Type: DataTypeClassOrInstance): AbstractDataType<any>;

/**
* Fetches the database version
*/
fetchDatabaseVersion(options?: QueryRawOptions): Promise<string>;

/**
* Returns the database version
*/
databaseVersion(options?: QueryRawOptions): Promise<string>;
getDatabaseVersion(): string;

/**
* Returns the installed version of Sequelize
Expand Down
27 changes: 24 additions & 3 deletions src/sequelize.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export class Sequelize extends SequelizeTypeScript {
},
transactionType: TRANSACTION_TYPES.DEFERRED,
isolationLevel: null,
databaseVersion: 0,
databaseVersion: null,
noTypeValidation: false,
benchmark: false,
minifyAliases: false,
Expand Down Expand Up @@ -1036,11 +1036,32 @@ Use Sequelize#query if you wish to use replacements.`);

}

// TODO: rename to getDatabaseVersion
async databaseVersion(options) {
/**
* Fetches the version of the database
*
* @param {object} [options] Query options
*
* @returns {Promise<string>} current version of the dialect
*/
async fetchDatabaseVersion(options) {
return await this.getQueryInterface().databaseVersion(options);
}

/**
* Throws if the database version hasn't been loaded yet. It is automatically loaded the first time Sequelize connects to your database.
*
* You can use {@link Sequelize#authenticate} to cause a first connection.
*
* @returns {string} current version of the dialect that is internally loaded
*/
getDatabaseVersion() {
if (this.options.databaseVersion == null) {
throw new Error('The current database version is unknown. Please call `sequelize.authenticate()` first to fetch it, or manually configure it through options.');
}

return this.options.databaseVersion;
}

/**
* Get the fn for random based on the dialect
*
Expand Down
2 changes: 2 additions & 0 deletions test/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export const Config: Record<Dialect, Options> = {
},
minifyAliases: Boolean(env.SEQ_PG_MINIFY_ALIASES),
},

db2: {
database: process.env.SEQ_DB2_DB || process.env.SEQ_DB || process.env.IBM_DB_DBNAME || 'testdb',
username: process.env.SEQ_DB2_USER || process.env.SEQ_USER || process.env.IBM_DB_UID || 'db2inst1',
Expand All @@ -83,6 +84,7 @@ export const Config: Record<Dialect, Options> = {
idle: Number(process.env.SEQ_DB2_POOL_IDLE || process.env.SEQ_POOL_IDLE || 3000),
},
},

ibmi: {
database: env.SEQ_IBMI_DB || env.SEQ_DB,
username: process.env.SEQ_IBMI_USER || process.env.SEQ_USER,
Expand Down
6 changes: 3 additions & 3 deletions test/integration/dialects/abstract/connection-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe(getTestDialectTeaser('Connection Manager'), () => {
const connectStub = sandbox.stub(connectionManager, '_connect').resolves(res);
// @ts-expect-error -- internal method, no typings
sandbox.stub(connectionManager, '_disconnect').resolves();
sandbox.stub(sequelize, 'databaseVersion').resolves(sequelize.dialect.defaultVersion);
sandbox.stub(sequelize, 'fetchDatabaseVersion').resolves(sequelize.dialect.defaultVersion);

const queryOptions: GetConnectionOptions = {
type: 'read',
Expand Down Expand Up @@ -111,7 +111,7 @@ describe(getTestDialectTeaser('Connection Manager'), () => {
const sequelize = createSequelizeInstance();
const connectionManager = sequelize.connectionManager;

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

const res: Connection = {};

Expand Down Expand Up @@ -162,7 +162,7 @@ describe(getTestDialectTeaser('Connection Manager'), () => {
sandbox.stub(connectionManager, '_disconnect').resolves();

sandbox
.stub(sequelize, 'databaseVersion')
.stub(sequelize, 'fetchDatabaseVersion')
.resolves(sequelize.dialect.defaultVersion);

const queryOptions: GetConnectionOptions = {
Expand Down
25 changes: 22 additions & 3 deletions test/integration/sequelize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {

it('fails with incorrect database credentials (1)', async function () {
// TODO: remove this once fixed in https://github.com/brianc/node-postgres/issues/1927 or when password is not allowed to be null in our postgres implementation
if (dialect === 'postgres' && semver.gte(this.sequelize.options.databaseVersion, '12.0.0')) {
if (dialect === 'postgres' && semver.gte(this.sequelize.getDatabaseVersion(), '12.0.0')) {
return;
}

Expand Down Expand Up @@ -825,9 +825,28 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {
}
});

describe('databaseVersion', () => {
describe('fetchDatabaseVersion', () => {
it('should database/dialect version', async function () {
const version = await this.sequelize.databaseVersion();
const version = await this.sequelize.fetchDatabaseVersion();
expect(typeof version).to.equal('string');
expect(version).to.be.ok;
});
});

describe('getDatabaseVersion', () => {
it('throws if no database version is set internally', () => {
expect(() => {
// ensures the version hasn't been loaded by another test yet
const sequelize = Support.createSequelizeInstance();
sequelize.getDatabaseVersion();
}).to.throw(
'The current database version is unknown. Please call `sequelize.authenticate()` first to fetch it, or manually configure it through options.',
);
});

it('returns the database version if loaded', async function () {
await this.sequelize.authenticate();
const version = this.sequelize.getDatabaseVersion();
expect(typeof version).to.equal('string');
expect(version).to.be.ok;
});
Expand Down

0 comments on commit 151a458

Please sign in to comment.