feat(lucid): add incrementing flag

when primaryKey is not incrementing, one needs to define it as an explicit flag. Which will aware

lucid to setup correct value for primaryKey and make everything work as expected

Closes #89

package.json

@@ -45,7 +45,8 @@
     "shelljs": "^0.7.5",
     "sqlite3": "^3.1.8",
     "standard": "^8.5.0",
-    "test-console": "^1.0.0"
+    "test-console": "^1.0.0",
+    "uuid": "^3.0.1"
   },
   "config": {
     "commitizen": {

src/Lucid/Model/Mixins/Persistance.js

@@ -39,7 +39,13 @@ Peristance.insert = function * () {
     }
     const save = yield query.insertAttributes(values).returning(this.constructor.primaryKey)
     if (save[0]) {
-      this.$primaryKeyValue = save[0]
+      /**
+       * Since {returning} statement does not work for Sqlite3, we need to
+       * make use of the returning value only when {incrementing} is set
+       * to true.
+       */
+      this.$primaryKeyValue = this.constructor.incrementing ? save[0] : values[this.constructor.primaryKey]
+      this.exists = true
       this.original = _.clone(this.attributes)
     }
     return !!save
@@ -100,6 +106,7 @@ Peristance.delete = function * () {
     const affected = yield query.deleteAttributes(values)
     if (affected > 0) {
       _.merge(this.attributes, values)
+      this.exists = false
       this.freeze()
     }
     return affected

src/Lucid/Model/index.js

@@ -90,6 +90,7 @@ class Model {
     this.original = {}
     this.transaction = null // will be added via useTransaction
     this.relations = {}
+    this.exists = false
     this.frozen = false
     this.eagerLoad = new Relations.EagerLoad()
     if (values) {
@@ -355,6 +356,16 @@ class Model {
     return util.makeTableName(this)
   }
 
+  /**
+   * Defines whether the primary key is supposed
+   * to be incrementing or not.
+   *
+   * @return {Boolean}
+   */
+  static get incrementing () {
+    return true
+  }
+
   /**
    * Returns a custom prefix to be used for selecting the database
    * table for a given model
@@ -780,7 +791,7 @@ class Model {
    * @public
    */
   isNew () {
-    return !this.$primaryKeyValue
+    return !this.exists
   }
 
   /**

src/Lucid/Model/proxyHandler.js

@@ -11,7 +11,16 @@
 
 const proxyHandler = exports = module.exports = {}
 const CE = require('../../Exceptions')
-const targetProperties = ['$primaryKeyValue', 'original', 'attributes', 'relations', 'eagerLoad', 'frozen', 'transaction']
+const targetProperties = [
+  '$primaryKeyValue',
+  'original',
+  'attributes',
+  'relations',
+  'eagerLoad',
+  'frozen',
+  'transaction',
+  'exists'
+]
 
 /**
  * proxy handler for getting target properties.Here

src/Lucid/QueryBuilder/Serializers/Base.js

@@ -65,7 +65,9 @@ class BaseSerializer {
   }
 
   /**
-   * converts the final result set into a custom collection
+   * converts the final result set into a custom collection. This method
+   * should never be called on newly instantiated models, whereas is
+   * only used when fetching models from Database.
    *
    * @param  {Array} values      [description]
    * @param  {Array} eagerValues [description]
@@ -81,6 +83,7 @@ class BaseSerializer {
     return helpers.toCollection(values).transform((result, value, index) => {
       const modelInstance = new this.queryBuilder.HostModel()
       modelInstance.attributes = value
+      modelInstance.exists = true
       modelInstance.original = _.clone(modelInstance.attributes)
       this.queryBuilder.eagerLoad.mapRelationsToRow(eagerValues, modelInstance, value)
       result[index] = modelInstance

test/unit/fixtures/model.js

@@ -7,6 +7,7 @@ module.exports = {
     const tables = [
       knex.schema.createTable('users', function (table) {
         table.increments()
+        table.string('uuid').defaultTo(null)
         table.string('username')
         table.string('firstname')
         table.string('lastname')

test/unit/helpers/postgresConnection.js

@@ -10,7 +10,7 @@ module.exports = {
   default: {
     client: 'pg',
     connection: {
-      user: '',
+      user: 'harmindervirk',
       password: '',
       database: 'default'
     }
@@ -19,7 +19,7 @@ module.exports = {
   alternateConnection: {
     client: 'pg',
     connection: {
-      user: '',
+      user: 'harmindervirk',
       password: '',
       database: 'alternate'
     }
@@ -28,7 +28,7 @@ module.exports = {
   defaultPrefix: {
     client: 'pg',
     connection: {
-      user: '',
+      user: 'harmindervirk',
       password: '',
       database: 'default'
     },

test/unit/lucid.spec.js

@@ -13,6 +13,7 @@
 const Model = require('../../src/Lucid/Model')
 const Database = require('../../src/Database')
 const chai = require('chai')
+const uuid = require('uuid')
 const moment = require('moment')
 const path = require('path')
 const Ioc = require('adonis-fold').Ioc
@@ -1920,6 +1921,43 @@ describe('Lucid', function () {
       yield zombie.restore()
       expect(queryHelpers.formatQuery(restoreQuery.sql)).to.equal(queryHelpers.formatQuery('update "zombies" set "deleted_at" = ?, "updated_at" = ? where "zombie_id" = ?'))
     })
+
+    it('should consider model as new even when primary key is provided in advance', function () {
+      class User extends Model {}
+      const user = new User()
+      expect(user.isNew()).to.be.true
+      user.id = 10
+      expect(user.isNew()).to.be.true
+    })
+
+    it('should make use of existing primary key when primary key defined when saving model', function * () {
+      class User extends Model {}
+      const user = new User()
+      user.id = 109
+      yield user.save()
+      const getUser = yield User.find(109)
+      expect(user.id).to.equal(109).to.equal(getUser.id)
+      yield user.delete()
+    })
+
+    it('should make use of primary key which is not auto incrementing', function * () {
+      class User extends Model {
+        static get primaryKey () {
+          return 'uuid'
+        }
+
+        static get incrementing () {
+          return false
+        }
+      }
+      const user = new User()
+      const v4 = uuid.v4()
+      user.uuid = v4
+      yield user.save()
+      const getUser = yield User.find(v4)
+      expect(user.$primaryKeyValue).to.equal(v4).to.equal(getUser.uuid)
+      yield user.delete()
+    })
   })
 
   context('Model Transactions', function () {