From ff558401fa58e35f741f90464b88e96a8369adee Mon Sep 17 00:00:00 2001
From: Chen Yangjian <252317+cyjake@users.noreply.github.com>
Date: Sat, 22 Feb 2020 17:04:34 +0800
Subject: [PATCH] fix: opts.sequelize, opts.dialect, and opts.client (#34)

also updated error messages a bit to better align with JavaScript convention
---
 History.md                           |  6 ++++++
 index.js                             | 18 +++++++++++-------
 lib/bone.js                          | 20 ++++++++++----------
 lib/drivers/index.js                 |  9 ++++-----
 lib/spell.js                         | 28 ++++++++++++++++------------
 package.json                         |  2 +-
 test/integration/test.postgres.js    |  2 +-
 test/integration/test.sqlite.js      |  2 +-
 test/unit/adapters/test.sequelize.js |  2 +-
 test/unit/test.connect.js            |  8 ++++----
 test/unit/test.realm.js              |  6 +++---
 test/unit/test.spell.js              |  9 ++++++++-
 12 files changed, 66 insertions(+), 46 deletions(-)

diff --git a/History.md b/History.md
index 1cd01b48..c7cc52da 100644
--- a/History.md
+++ b/History.md
@@ -1,3 +1,9 @@
+0.5.3 / 2020-02-22
+==================
+
+  * Fix: `connect({ sequelize, dialect, client })` to allow mandatory sqlite client
+  * Fix: prevent queries being performed unless model is correctly connected
+
 0.5.2 / 2020-02-21
 ==================
 
diff --git a/index.js b/index.js
index 3b58379a..e5bde1b1 100644
--- a/index.js
+++ b/index.js
@@ -67,19 +67,24 @@ async function loadModels(Bone, models, opts) {
   }
 }
 
+function createSpine(opts) {
+  if (opts.Bone) return opts.Bone;
+  if (opts.sequelize) return sequelize(Bone);
+  return class Spine extends Bone {};
+}
+
 class Realm {
   constructor(opts = {}) {
-    const { client, dialect, database, Bone: Osteon, ...restOpts } = {
-      client: opts.dialect || 'mysql',
+    const { client, dialect, database, ...restOpts } = {
+      dialect: 'mysql',
       database: opts.db || opts.storage,
-      Bone: class Spine extends Bone {},
       ...opts
     };
-    const Spine = dialect ? sequelize(Osteon) : Osteon;
+    const Spine = createSpine(opts);
     const models = {};
 
     // test/integration/suite/migrations.js currently depends on this behavior
-    const driver = opts.driver || new (findDriver(client))({
+    const driver = opts.driver || new (findDriver(dialect))({
       client,
       database,
       ...restOpts
@@ -135,7 +140,7 @@ class Realm {
 }
 
 /**
- * Connect models to database. Need to provide both the settings of the connection and the models, or the path of the models, to connect.
+ * Connect models to database. Need to provide both connect options and models.
  * @alias module:index.connect
  * @param {Object} opts
  * @param {string} opts.client - client name
@@ -150,7 +155,6 @@ const connect = async function connect(opts = {}) {
   return await realm.connect();
 };
 
-
 Object.assign(Realm.prototype, migrations, { DataTypes });
 Object.assign(Realm, { connect, Bone, Collection, DataTypes, sequelize });
 
diff --git a/lib/bone.js b/lib/bone.js
index 294a63ca..655a2389 100644
--- a/lib/bone.js
+++ b/lib/bone.js
@@ -126,7 +126,7 @@ class Bone {
       this.rawUnset.delete(name);
       return this;
     } else {
-      if (this.rawUnset.has(name)) throw new Error(`Unset attribute "${name}"`);
+      if (this.rawUnset.has(name)) throw new Error(`unset attribute "${name}"`);
       const value = this.raw[name];
       // make sure null is returned if value is undefined
       return value == null ? null : value;
@@ -142,7 +142,7 @@ class Bone {
    * post.attributeWas('title')  // => 'Leah'
    */
   attributeWas(name) {
-    if (this.rawUnset.has(name)) throw new Error(`Unset attribute "${name}"`);
+    if (this.rawUnset.has(name)) throw new Error(`unset attribute "${name}"`);
     const value = this.rawInitial[name];
     return value == null ? null : value;
   }
@@ -238,7 +238,7 @@ class Bone {
    */
   async save() {
     const { primaryKey } = this.constructor;
-    if (this.rawUnset.has(primaryKey)) throw new Error(`Unset primary key ${primaryKey}`);
+    if (this.rawUnset.has(primaryKey)) throw new Error(`unset primary key ${primaryKey}`);
     if (this[primaryKey] == null) {
       await this.create();
     } else if (this.attributeChanged(primaryKey)) {
@@ -331,7 +331,7 @@ class Bone {
 
     if (Object.keys(changes).length === 0) return Promise.resolve(0);
     if (this[primaryKey] == null) {
-      throw new Error(`Unset primary key ${primaryKey}`);
+      throw new Error(`unset primary key ${primaryKey}`);
     }
 
     const where = { [primaryKey]: this[primaryKey] };
@@ -397,7 +397,7 @@ class Bone {
     const { primaryKey, shardingKey } = Model;
 
     if (this[primaryKey] == null) {
-      throw new Error('The instance is not persisted yet.');
+      throw new Error('instance is not persisted yet.');
     }
 
     const condition = { [primaryKey]: this[primaryKey] };
@@ -411,7 +411,7 @@ class Bone {
     const { primaryKey, shardingKey } = Model;
 
     if (this[primaryKey] == null) {
-      throw new Error('The instance is not persisted yet.');
+      throw new Error('instance is not persisted yet.');
     }
 
     const conditions = {
@@ -546,7 +546,7 @@ class Bone {
     const { attributes, attributeMap } = this;
 
     if (attributes.hasOwnProperty(newName)) {
-      throw new Error(`Unable to override existing attribute "${newName}"`);
+      throw new Error(`unable to override existing attribute "${newName}"`);
     }
 
     if (attributes.hasOwnProperty(originalName)) {
@@ -673,11 +673,11 @@ class Bone {
    */
   static associate(name, opts = {}) {
     if (name in this.associations) {
-      throw new Error(this.name + ' has duplicated association at: ' + name);
+      throw new Error(`duplicated association "${name}" on model ${this.name}`);
     }
     const { className } = opts;
     const Model = this.models[className];
-    if (!Model) throw new Error(`Unable to find model "${className}"`);
+    if (!Model) throw new Error(`unable to find model "${className}"`);
 
     const { deletedAt } = this.timestamps;
     if (Model.attributes[deletedAt] && !opts.where) {
@@ -964,7 +964,7 @@ class Bone {
     if (this.synchronized) return;
 
     if (this.physicTables) {
-      throw new Error('Unable to sync model with custom physic tables');
+      throw new Error('unable to sync model with custom physic tables');
     }
 
     const { attributes, columns } = this;
diff --git a/lib/drivers/index.js b/lib/drivers/index.js
index ee0d1568..812f40e2 100644
--- a/lib/drivers/index.js
+++ b/lib/drivers/index.js
@@ -1,18 +1,17 @@
 'use strict';
 
-function findDriver(client) {
-  switch (client) {
+function findDriver(dialect) {
+  switch (dialect) {
     case 'mysql':
-    case 'mysql2':
       return require('./mysql');
     case 'pg':
     case 'postgres':
       return require('./postgres');
-    case '@journeyapps/sqlcipher':
+    case 'sqlite':
     case 'sqlite3':
       return require('./sqlite');
     default:
-      throw new Error(`Unsupported database ${client}`);
+      throw new Error(`Unsupported database ${dialect}`);
   }
 }
 
diff --git a/lib/spell.js b/lib/spell.js
index 56c16a23..ebd0d70e 100644
--- a/lib/spell.js
+++ b/lib/spell.js
@@ -53,7 +53,7 @@ function parseConditions(conditions, ...values) {
     return [parseExpr(conditions, ...values)];
   }
   else {
-    throw new Error(`Unsupported conditions ${conditions}`);
+    throw new Error(`unexpected conditions ${conditions}`);
   }
 }
 
@@ -123,7 +123,7 @@ function isLogicalOperator(condition) {
 
 function parseLogicalObjectConditionValue(value) {
   if (value == null || typeof value !== 'object') {
-    throw new Error(`Unexpected logical operator value ${value}`);
+    throw new Error(`unexpected logical operator value ${value}`);
   }
 
   if (Array.isArray(value)) return value;
@@ -218,7 +218,7 @@ function parseSelect(spell, ...names) {
       const qualifier = qualifiers && qualifiers[0];
       const model = qualifier && joins && (qualifier in joins) ? joins[qualifier].Model : Model;
       if (!model.attributes[value]) {
-        throw new Error(`Unable to find attribute ${value} in model ${model.name}`);
+        throw new Error(`unable to find attribute ${value} in model ${model.name}`);
       }
     });
   }
@@ -316,12 +316,12 @@ function joinAssociation(spell, BaseModel, baseName, refName, opts = {}) {
   const { joins } = spell;
   let association = BaseModel.associations[refName] || BaseModel.associations[pluralize(refName, 1)];
 
-  if (refName in joins) throw new Error(`Duplicated ${refName} in join tables`);
+  if (refName in joins) throw new Error(`duplicated ${refName} in join tables`);
   if (!association && opts.targetAssociation) {
     association = findAssociation(BaseModel.associations, { Model: opts.targetAssociation.Model });
   }
   if (!association) {
-    throw new Error(`Unable to find association ${refName} on ${BaseModel.name}`);
+    throw new Error(`unable to find association ${refName} on ${BaseModel.name}`);
   }
 
   const { through, Model: RefModel, includes } = association;
@@ -417,6 +417,10 @@ class Spell {
    * @param {Object}         opts     - Extra attributes to be set.
    */
   constructor(Model, opts) {
+    if (Model.synchronized == null) {
+      throw new Error(`model ${Model.name} is not connected yet`);
+    }
+
     const scopes = [];
     const { deletedAt } = Model.timestamps;
     if (Model.attributes[deletedAt]) {
@@ -586,9 +590,9 @@ class Spell {
   $increment(name, by = 1) {
     const { Model } = this;
 
-    if (!Number.isFinite(by)) throw new Error(`Unexpected increment value ${by}`);
+    if (!Number.isFinite(by)) throw new Error(`unexpected increment value ${by}`);
     if (!Model.attributes.hasOwnProperty(name)) {
-      throw new Error(`Undefined attribute "${name}"`);
+      throw new Error(`undefined attribute "${name}"`);
     }
 
     this.sets = {
@@ -723,7 +727,7 @@ class Spell {
    */
   $offset(skip) {
     skip = +skip;
-    if (Number.isNaN(skip)) throw new Error(`Invalid offset ${skip}`);
+    if (Number.isNaN(skip)) throw new Error(`invalid offset ${skip}`);
     this.skip = skip;
     return this;
   }
@@ -735,7 +739,7 @@ class Spell {
    */
   $limit(rowCount) {
     rowCount = +rowCount;
-    if (Number.isNaN(rowCount)) throw new Error(`Invalid limit ${rowCount}`);
+    if (Number.isNaN(rowCount)) throw new Error(`invalid limit ${rowCount}`);
     this.rowCount = rowCount;
     return this;
   }
@@ -824,7 +828,7 @@ class Spell {
     const { joins } = this;
 
     if (qualifier in joins) {
-      throw new Error(`Invalid join target. ${qualifier} already defined.`);
+      throw new Error(`invalid join target. ${qualifier} already defined.`);
     }
     joins[qualifier] = { Model, on: parseConditions(onConditions, ...values)[0] };
     return this;
@@ -858,7 +862,7 @@ class Spell {
    */
   async * batch(size = 1000) {
     const limit = parseInt(size, 10);
-    if (!(limit > 0)) throw new Error(`Invalid batch limit ${size}`);
+    if (!(limit > 0)) throw new Error(`invalid batch limit ${size}`);
     // Duplicate the spell because spell.skip gets updated while iterating over the batch.
     const spell = this.limit(limit);
     let results = await spell;
@@ -897,7 +901,7 @@ for (const aggregator in AGGREGATOR_MAP) {
     writable: true,
     value: function Spell_aggreator(name = '*') {
       if (name != '*' && parseExpr(name).type != 'id') {
-        throw new Error(`Unexpected operand ${name} for ${func.toUpperCase()}()`);
+        throw new Error(`unexpected operand ${name} for ${func.toUpperCase()}()`);
       }
       this.$select(`${func}(${name}) as ${aggregator}`);
       return this;
diff --git a/package.json b/package.json
index 4efeef23..fddb3da1 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "leoric",
-  "version": "0.5.2",
+  "version": "0.5.3",
   "description": "JavaScript Object-relational mapping alchemy",
   "main": "index.js",
   "types": "index.d.ts",
diff --git a/test/integration/test.postgres.js b/test/integration/test.postgres.js
index 6265daac..8346a112 100644
--- a/test/integration/test.postgres.js
+++ b/test/integration/test.postgres.js
@@ -7,7 +7,7 @@ const { connect } = require('../..');
 
 before(async function() {
   await connect({
-    client: 'postgres',
+    dialect: 'postgres',
     host: process.env.POSTGRES_HOST || '127.0.0.1',
     port: process.env.POSTGRES_PORT,
     user: process.env.POSTGRES_USER || '',
diff --git a/test/integration/test.sqlite.js b/test/integration/test.sqlite.js
index 3bcde8a7..7c0cc973 100644
--- a/test/integration/test.sqlite.js
+++ b/test/integration/test.sqlite.js
@@ -6,7 +6,7 @@ const { checkDefinitions } = require('./helpers');
 
 before(async function() {
   await connect({
-    client: 'sqlite3',
+    dialect: 'sqlite',
     database: '/tmp/leoric.sqlite3',
     models: path.resolve(__dirname, '../models'),
   });
diff --git a/test/unit/adapters/test.sequelize.js b/test/unit/adapters/test.sequelize.js
index aade2b19..ade0656e 100644
--- a/test/unit/adapters/test.sequelize.js
+++ b/test/unit/adapters/test.sequelize.js
@@ -21,7 +21,7 @@ describe('=> Sequelize adapter', () => {
   before(async () => {
     await connect({
       Model: Spine,
-      client: 'sqlite',
+      dialect: 'sqlite',
       database: '/tmp/leoric.sqlite3',
       models: [ Book, Post ],
     });
diff --git a/test/unit/test.connect.js b/test/unit/test.connect.js
index 8e23b0b9..cb437542 100644
--- a/test/unit/test.connect.js
+++ b/test/unit/test.connect.js
@@ -11,7 +11,7 @@ describe('connect', function() {
 
   it('rejects unsupported database', async function() {
     await assert.rejects(async () => {
-      await connect({ client: 'mongodb', models: `${__dirname}/models` });
+      await connect({ dialect: 'mongodb', models: `${__dirname}/models` });
     }, /unsupported database/i);
   });
 
@@ -23,9 +23,9 @@ describe('connect', function() {
   });
 
   it('connect with specified Bone', async function() {
-    const Osteon = await connect({ Bone: class extends Bone {} });
-    assert.ok(Osteon.prototype instanceof Bone);
-    assert.ok(Osteon.models);
+    const Spine = await connect({ Bone: class extends Bone {} });
+    assert.ok(Spine.prototype instanceof Bone);
+    assert.ok(Spine.models);
   });
 
   it('connect models passed in opts.models', async function() {
diff --git a/test/unit/test.realm.js b/test/unit/test.realm.js
index 566c77a9..d03f416b 100644
--- a/test/unit/test.realm.js
+++ b/test/unit/test.realm.js
@@ -19,8 +19,8 @@ describe('=> Realm', () => {
     assert.equal(arda.Bone, Bone);
   });
 
-  it('should subclass with sequelize api if exists opts.dialect', async () => {
-    const realm = new Realm({ dialect: 'mysql' });
+  it('should subclass with sequelize api if opts.sequelize', async () => {
+    const realm = new Realm({ sequelize: true });
     const { Bone: Spine } = realm;
     assert.ok(typeof Spine.prototype.setDataValue === 'function');
   });
@@ -31,7 +31,7 @@ describe('=> Realm', () => {
   });
 
   it('should rename opts.storage to opts.database', async () => {
-    const realm = new Realm({ client: 'sqlite3', storage: '/tmp/leoric.sqlite3' });
+    const realm = new Realm({ dialect: 'sqlite', storage: '/tmp/leoric.sqlite3' });
     assert.equal(realm.options.database, '/tmp/leoric.sqlite3');
   });
 
diff --git a/test/unit/test.spell.js b/test/unit/test.spell.js
index 94e58311..55a97a6d 100644
--- a/test/unit/test.spell.js
+++ b/test/unit/test.spell.js
@@ -3,7 +3,7 @@
 const assert = require('assert').strict;
 const path = require('path');
 
-const { connect } = require('../..');
+const { connect, Bone } = require('../..');
 const Post = require('../models/post');
 const TagMap = require('../models/tagMap');
 const Comment = require('../models/comment');
@@ -17,6 +17,13 @@ before(async function() {
   });
 });
 
+describe('=> Spell', function() {
+  it('rejects query if not connected yet', async () => {
+    class Note extends Bone {};
+    await assert.rejects(async () => await Note.all);
+  });
+});
+
 describe('=> Insert', function() {
   it('insert', function() {
     const date = new Date(2017, 11, 12);