From 6e2f51bdd3815a2f6550098a9554f43bfdc12400 Mon Sep 17 00:00:00 2001 From: Chen Yangjian <252317+cyjake@users.noreply.github.com> Date: Wed, 21 Jul 2021 22:17:09 +0800 Subject: [PATCH] fix: subclassing data type in dialects (#145) * fix: subclassing data type in dialects exports of the abstract data types should not be wrapped with invokable by default, which made subclassing like `class Postgres_DATE extends DataTypes.DATE {}` not working as expected because `DataTypes.DATE` was actually a Proxy instance. This fix defers the invokable wrapping and adds more test cases regarding data types in dialects. * Update test/unit/drivers/postgres/attribute.test.js Co-authored-by: JimmyDaddy * test: merge test cases Co-authored-by: JimmyDaddy --- Readme.md | 2 +- index.js | 2 +- src/data_types.js | 21 ++++++--- src/drivers/abstract/attribute.js | 17 +++---- src/drivers/abstract/index.js | 4 +- src/drivers/postgres/attribute.js | 4 +- src/drivers/postgres/data_types.js | 2 +- test/integration/suite/data_types.test.js | 36 ++++++++------- test/unit/data_types.test.js | 19 +++++--- test/unit/drivers/mysql/attribute.test.js | 15 +++++++ .../{mysql.test.js => mysql/index.test.js} | 2 +- test/unit/drivers/postgres/attribute.test.js | 44 +++++++++++++++++++ .../index.test.js} | 2 +- test/unit/drivers/sqlite/attribute.test.js | 22 ++++++++++ .../{sqlite.test.js => sqlite/index.test.js} | 5 ++- 15 files changed, 146 insertions(+), 51 deletions(-) create mode 100644 test/unit/drivers/mysql/attribute.test.js rename test/unit/drivers/{mysql.test.js => mysql/index.test.js} (97%) create mode 100644 test/unit/drivers/postgres/attribute.test.js rename test/unit/drivers/{postgres.test.js => postgres/index.test.js} (98%) create mode 100644 test/unit/drivers/sqlite/attribute.test.js rename test/unit/drivers/{sqlite.test.js => sqlite/index.test.js} (94%) diff --git a/Readme.md b/Readme.md index 8d1a3adb..70f54586 100644 --- a/Readme.md +++ b/Readme.md @@ -17,7 +17,7 @@ const { Bone, connect } = require('leoric') // define model class Post extends Bone { - static describe() { + static initialize() { this.belongsTo('author', { Model: 'User' }) this.hasMany('comments') } diff --git a/index.js b/index.js index be368c2d..c10d94a9 100644 --- a/index.js +++ b/index.js @@ -7,7 +7,7 @@ const Logger = require('./src/drivers/abstract/logger'); const Spell = require('./src/spell'); const Bone = require('./src/bone'); const Collection = require('./src/collection'); -const DataTypes = require('./src/data_types'); +const { invokable: DataTypes } = require('./src/data_types'); const { findDriver } = require('./src/drivers'); const migrations = require('./src/migrations'); const sequelize = require('./src/adapters/sequelize'); diff --git a/src/data_types.js b/src/data_types.js index dc558d49..7d8c5fe2 100644 --- a/src/data_types.js +++ b/src/data_types.js @@ -202,7 +202,7 @@ class JSONB extends DataType { } } -const DataTypes = [ +const DataTypes = { STRING, INTEGER, BIGINT, @@ -212,10 +212,19 @@ const DataTypes = [ BLOB, JSON, JSONB, -]; - -for (const klass of DataTypes) { - DataType[klass.name] = invokable(klass); -} +}; + +Object.assign(DataType, DataTypes); +Object.defineProperty(DataType, 'invokable', { + get() { + return new Proxy(this, { + get(target, p) { + const value = target[p]; + if (DataTypes.hasOwnProperty(p)) return invokable(value); + return value; + } + }); + }, +}); module.exports = DataType; diff --git a/src/drivers/abstract/attribute.js b/src/drivers/abstract/attribute.js index d7b67f82..23a90309 100644 --- a/src/drivers/abstract/attribute.js +++ b/src/drivers/abstract/attribute.js @@ -2,22 +2,16 @@ const debug = require('debug')('leoric'); -const DataTypes = require('../../data_types'); const { snakeCase } = require('../../utils/string'); /** * Find the corresponding JavaScript type of the type in database. * @param {string} dataType */ -function findJsType(type, defaultDataType) { - // MySQL stores BOOLEAN as TINYINT(1) - if (type instanceof DataTypes.BOOLEAN) { - return Boolean; - } - - const dataType = defaultDataType || type.dataType; - +function findJsType(dataType) { switch (dataType.toLowerCase().split('(')[0]) { + case 'boolean': + return Boolean; case 'bigint': case 'mediumint': case 'smallint': @@ -81,6 +75,7 @@ class Attribute { * @param {boolean} opts.underscored */ constructor(name, params, { underscored } = {}) { + const { DataTypes } = this.constructor; if (params instanceof Attribute) return params; const columnName = underscored === false ? name : snakeCase(name); if (params == null) return Object.assign(this, { name, columnName }); @@ -90,7 +85,7 @@ class Attribute { if (typeof params === 'function' || params instanceof DataTypes) { params = { type: params }; } - const type = createType(this.constructor.DataTypes, params); + const type = createType(DataTypes, params); const dataType = params.dataType || type.dataType; Object.assign(this, { name, @@ -102,7 +97,7 @@ class Attribute { ...params, type, dataType, - jsType: findJsType(type, dataType), + jsType: type instanceof DataTypes.BOOLEAN ? Boolean : findJsType(dataType), }); } diff --git a/src/drivers/abstract/index.js b/src/drivers/abstract/index.js index 3350cf52..48bf4a78 100644 --- a/src/drivers/abstract/index.js +++ b/src/drivers/abstract/index.js @@ -21,10 +21,10 @@ module.exports = class AbstractDriver { * @param {Object|Date|string|Boolean} type * @returns {Object|Date|string|boolean} */ - cast(value, type) { + cast(value, jsType) { if (value == null) return value; - switch (type) { + switch (jsType) { case JSON: if (!value) return null; // type === JSONB diff --git a/src/drivers/postgres/attribute.js b/src/drivers/postgres/attribute.js index df6669cd..64323971 100644 --- a/src/drivers/postgres/attribute.js +++ b/src/drivers/postgres/attribute.js @@ -9,9 +9,7 @@ class PostgresAttribute extends Attribute { super(name, params, opts); } - static get DataTypes() { - return DataTypes; - } + static DataTypes = DataTypes.invokable; toSqlString() { const { diff --git a/src/drivers/postgres/data_types.js b/src/drivers/postgres/data_types.js index 39fb0fdb..3f648468 100644 --- a/src/drivers/postgres/data_types.js +++ b/src/drivers/postgres/data_types.js @@ -10,7 +10,7 @@ class Postgres_DATE extends DataTypes.DATE { toSqlString() { const { timezone } = this; - if (timezone) return `${super.toSqlString()} WITH TIME ZONE}`; + if (timezone) return `${super.toSqlString()} WITH TIME ZONE`; return `${super.toSqlString()} WITHOUT TIME ZONE`; } } diff --git a/test/integration/suite/data_types.test.js b/test/integration/suite/data_types.test.js index 14e8391a..a75ae659 100644 --- a/test/integration/suite/data_types.test.js +++ b/test/integration/suite/data_types.test.js @@ -7,10 +7,8 @@ const { INTEGER, STRING, DATE, TEXT, BOOLEAN, JSON, JSONB } = DataTypes; describe('=> Data types', () => { - class Note extends Bone {}; - before(async () => { - await Note.driver.dropTable('notes'); - Note.init({ + class Note extends Bone { + static attributes = { id: { type: INTEGER, primaryKey: true }, title: STRING, body: TEXT, @@ -18,7 +16,10 @@ describe('=> Data types', () => { createdAt: DATE, updatedAt: DATE, publishedAt: DATE(6), - }); + } + }; + before(async () => { + await Note.driver.dropTable('notes'); await Note.sync(); }); @@ -65,18 +66,19 @@ describe('=> Data types', () => { describe('=> Data types - JSON', () => { it('=> init', async () => { - class Note extends Bone {}; - Note.init({ - id: { type: INTEGER, primaryKey: true }, - title: STRING, - body: TEXT, - isPrivate: BOOLEAN, - createdAt: DATE, - updatedAt: DATE, - publishedAt: DATE(6), - meta: JSON, - metab: JSONB, - }); + class Note extends Bone { + static attributes = { + id: { type: INTEGER, primaryKey: true }, + title: STRING, + body: TEXT, + isPrivate: BOOLEAN, + createdAt: DATE, + updatedAt: DATE, + publishedAt: DATE(6), + meta: JSON, + metab: JSONB, + } + } await Note.sync(); await Note.create({ title: 'Leah', meta: { foo: 1, baz: 'baz' }, metab: { foo: 2, baz: 'baz1' } }); const foundNote = await Note.first; diff --git a/test/unit/data_types.test.js b/test/unit/data_types.test.js index b71c629d..feec8896 100644 --- a/test/unit/data_types.test.js +++ b/test/unit/data_types.test.js @@ -2,11 +2,12 @@ const assert = require('assert').strict; const DataTypes = require('../../src/data_types'); -const { - STRING, BOOLEAN, DATE, INTEGER, BIGINT, TEXT, JSON, JSONB -} = DataTypes; describe('=> Data Types', () => { + const { + STRING, BOOLEAN, DATE, INTEGER, BIGINT, TEXT, JSON, JSONB + } = DataTypes; + it('STRING', () => { assert.equal(new STRING().dataType, 'varchar'); assert.equal(new STRING().toSqlString(), 'VARCHAR(255)'); @@ -46,8 +47,16 @@ describe('=> Data Types', () => { }); }); -describe('findType()', () => { +describe('DataTypes.findType()', () => { + const { TEXT } = DataTypes; it('longtext => TEXT', () => { - assert.strictEqual(DataTypes.findType('longtext'), TEXT); + assert.equal(DataTypes.findType('longtext'), TEXT); + }); +}); + +describe('DataTypes.invokable', function() { + const { STRING } = DataTypes.invokable; + it('should wrap data types to support flexible invoking', async function() { + assert.equal(STRING(255).toSqlString(), 'VARCHAR(255)'); }); }); diff --git a/test/unit/drivers/mysql/attribute.test.js b/test/unit/drivers/mysql/attribute.test.js new file mode 100644 index 00000000..b8d4b300 --- /dev/null +++ b/test/unit/drivers/mysql/attribute.test.js @@ -0,0 +1,15 @@ +'use strict'; + +const assert = require('assert').strict; +const Attribute = require('../../../../src/drivers/mysql/attribute'); +const { BOOLEAN } = Attribute.DataTypes; + +describe('=> Attribute (mysql)', function() { + it('should support TINYINT(1)', async function() { + const attribute= new Attribute('has_image', { + type: BOOLEAN, + defaultValue: false, + }); + assert.equal(attribute.toSqlString(), '`has_image` TINYINT(1) DEFAULT false'); + }); +}); diff --git a/test/unit/drivers/mysql.test.js b/test/unit/drivers/mysql/index.test.js similarity index 97% rename from test/unit/drivers/mysql.test.js rename to test/unit/drivers/mysql/index.test.js index 356c35f3..3d23f426 100644 --- a/test/unit/drivers/mysql.test.js +++ b/test/unit/drivers/mysql/index.test.js @@ -1,7 +1,7 @@ 'use strict'; const assert = require('assert').strict; -const MysqlDriver = require('../../../src/drivers/mysql'); +const MysqlDriver = require('../../../../src/drivers/mysql'); const database = 'leoric'; const options = { diff --git a/test/unit/drivers/postgres/attribute.test.js b/test/unit/drivers/postgres/attribute.test.js new file mode 100644 index 00000000..ed90f80b --- /dev/null +++ b/test/unit/drivers/postgres/attribute.test.js @@ -0,0 +1,44 @@ +'use strict'; + +const assert = require('assert').strict; +const Attribute = require('../../../../src/drivers/postgres/attribute'); +const { BIGINT, INTEGER, DATE } = Attribute.DataTypes; + +describe('=> Attribute (postgres)', function() { + it('should support BIGSERIAL', async function() { + const attribute= new Attribute('id', { + type: BIGINT, + primaryKey: true, + }); + assert.equal(attribute.toSqlString(), '"id" BIGSERIAL PRIMARY KEY'); + }); + + it('should support SERIAL', async function() { + const attribute= new Attribute('id', { + type: INTEGER, + primaryKey: true, + }); + assert.equal(attribute.toSqlString(), '"id" SERIAL PRIMARY KEY'); + }); + + it('should support WITH TIME ZONE', async function() { + const attribute= new Attribute('createdAt', { + type: DATE, + }); + assert.equal(attribute.toSqlString(), '"created_at" TIMESTAMP WITH TIME ZONE'); + }); + + it('should support WITHOUT TIME ZONE', async function() { + const attribute = new Attribute('createdAt', { + type: DATE(null, false), + }); + assert.equal(attribute.toSqlString(), '"created_at" TIMESTAMP WITHOUT TIME ZONE'); + }); + + it('should support DATE(precision)', async function() { + const attribute= new Attribute('updated_at', { + type: DATE(6), + }); + assert.equal(attribute.toSqlString(), '"updated_at" TIMESTAMP(6) WITH TIME ZONE'); + }); +}); diff --git a/test/unit/drivers/postgres.test.js b/test/unit/drivers/postgres/index.test.js similarity index 98% rename from test/unit/drivers/postgres.test.js rename to test/unit/drivers/postgres/index.test.js index cdcad5ab..ae2a5868 100644 --- a/test/unit/drivers/postgres.test.js +++ b/test/unit/drivers/postgres/index.test.js @@ -1,7 +1,7 @@ 'use strict'; const assert = require('assert').strict; -const PostgresDriver = require('../../../src/drivers/postgres'); +const PostgresDriver = require('../../../../src/drivers/postgres'); const database = 'leoric'; const options = { diff --git a/test/unit/drivers/sqlite/attribute.test.js b/test/unit/drivers/sqlite/attribute.test.js new file mode 100644 index 00000000..72f0b1ff --- /dev/null +++ b/test/unit/drivers/sqlite/attribute.test.js @@ -0,0 +1,22 @@ +'use strict'; + +const assert = require('assert').strict; +const Attribute = require('../../../../src/drivers/sqlite/attribute'); +const { BIGINT, DATE } = Attribute.DataTypes; + +describe('=> Attribute (sqlite)', function() { + it('should support DATETIME', async function() { + const attribute= new Attribute('createdAt', { + type: DATE, + }); + assert.equal(attribute.toSqlString(), '"created_at" DATETIME'); + }); + + it('should support BIGINT', async function() { + const attribute= new Attribute('id', { + type: BIGINT, + primaryKey: true, + }); + assert.equal(attribute.toSqlString(), '"id" INTEGER PRIMARY KEY'); + }); +}); diff --git a/test/unit/drivers/sqlite.test.js b/test/unit/drivers/sqlite/index.test.js similarity index 94% rename from test/unit/drivers/sqlite.test.js rename to test/unit/drivers/sqlite/index.test.js index 50822c4f..1b92da19 100644 --- a/test/unit/drivers/sqlite.test.js +++ b/test/unit/drivers/sqlite/index.test.js @@ -3,8 +3,8 @@ const assert = require('assert').strict; const strftime = require('strftime'); -const { heresql } = require('../../../src/utils/string'); -const SqliteDriver = require('../../../src/drivers/sqlite'); +const { heresql } = require('../../../../src/utils/string'); +const SqliteDriver = require('../../../../src/drivers/sqlite'); const { INTEGER, BIGINT, STRING, DATE, BOOLEAN } = SqliteDriver.prototype.DataTypes; @@ -64,6 +64,7 @@ describe('=> SQLite driver', () => { await driver.createTable('notes', { id: { type: BIGINT, primaryKey: true, autoIncrement: true }, public: { type: INTEGER }, + has_image: { type: BOOLEAN, defaultValue: false }, }); });