fix(lucid): make bulk operations reliable by executing in sequence

operations like createMany, saveMany were wrapped inside Promise.all, which makes it harder to
reason about the creation order. Now they are executed in sequence

package.json

@@ -27,27 +27,27 @@
     "@adonisjs/generic-exceptions": "^2.0.1",
     "chance": "^1.0.16",
     "debug": "^3.1.0",
-    "knex": "^0.14.6",
+    "knex": "^0.15.1",
     "lodash": "^4.17.10",
     "moment": "^2.22.2",
     "pluralize": "^7.0.0",
     "pretty-hrtime": "^1.0.3",
-    "require-all": "^2.2.0"
+    "require-all": "^3.0.0"
   },
   "devDependencies": {
     "@adonisjs/ace": "git+https://github.com/adonisjs/ace.git#dawn",
     "@adonisjs/fold": "^4.0.8",
     "@adonisjs/sink": "^1.0.16",
-    "coveralls": "^3.0.1",
+    "coveralls": "^3.0.2",
     "cz-conventional-changelog": "^2.1.0",
     "fs-extra": "^6.0.1",
     "japa": "^1.0.6",
     "japa-cli": "^1.0.1",
     "mysql": "^2.15.0",
-    "nyc": "^12.0.1",
+    "nyc": "^12.0.2",
     "pg": "^7.4.3",
     "semver": "^5.5.0",
-    "sqlite3": "^4.0.0",
+    "sqlite3": "^4.0.1",
     "standard": "^11.0.1"
   },
   "config": {

src/Database/index.js

@@ -223,8 +223,8 @@ class Database {
    *
    * @return {void}
    */
-  rollbackGlobalTransaction () {
-    this._globalTrx.rollback()
+  async rollbackGlobalTransaction () {
+    await this._globalTrx.rollback()
     this._globalTrx = null
   }
 
@@ -235,8 +235,8 @@ class Database {
    *
    * @return {void}
    */
-  commitGlobalTransaction () {
-    this._globalTrx.commit()
+  async commitGlobalTransaction () {
+    await this._globalTrx.commit()
     this._globalTrx = null
   }
 

src/Factory/DatabaseFactory.js

@@ -191,7 +191,14 @@ class DatabaseFactory {
    * @return {Array}
    */
   async createMany (numberOfRows, data = {}) {
-    return Promise.all(_.map(_.range(numberOfRows), (index) => this.create(data, index)))
+    const rows = []
+
+    for (let index of _.range(numberOfRows)) {
+      const row = await this.create(data, index)
+      rows.push(row)
+    }
+
+    return rows
   }
 
   /**

src/Lucid/Model/index.js

@@ -392,7 +392,14 @@ class Model extends BaseModel {
         .InvalidArgumentException
         .invalidParameter(`${this.name}.createMany expects an array of values`, payloadArray)
     }
-    return Promise.all(payloadArray.map((payload) => this.create(payload, trx)))
+
+    const rows = []
+    for (let payload of payloadArray) {
+      const row = await this.create(payload, trx)
+      rows.push(row)
+    }
+
+    return rows
   }
 
   /**

src/Lucid/Relations/BelongsToMany.js

@@ -771,10 +771,17 @@ class BelongsToMany extends BaseRelation {
     await this._loadAndCachePivot(trx)
     const rows = !Array.isArray(references) ? [references] : references
 
-    return Promise.all(rows.map((row) => {
-      const pivotInstance = this._getPivotInstance(row)
-      return pivotInstance ? Promise.resolve(pivotInstance) : this._attachSingle(row, pivotCallback, trx)
-    }))
+    let attachedRows = []
+
+    for (let row of rows) {
+      let pivotInstance = this._getPivotInstance(row)
+      if (!pivotInstance) {
+        pivotInstance = await this._attachSingle(row, pivotCallback, trx)
+      }
+      attachedRows.push(pivotInstance)
+    }
+
+    return attachedRows
   }
 
   /**
@@ -922,7 +929,14 @@ class BelongsToMany extends BaseRelation {
     }
 
     await this._persistParentIfRequired()
-    return Promise.all(arrayOfRelatedInstances.map((relatedInstance) => this.save(relatedInstance, pivotCallback)))
+
+    const rows = []
+    for (let relatedInstance of arrayOfRelatedInstances) {
+      const row = await this.save(relatedInstance, pivotCallback)
+      rows.push(row)
+    }
+
+    return rows
   }
 
   /**
@@ -967,7 +981,14 @@ class BelongsToMany extends BaseRelation {
     }
 
     await this._persistParentIfRequired()
-    return Promise.all(rows.map((relatedInstance) => this.create(relatedInstance, pivotCallback)))
+
+    const savedRows = []
+    for (let relatedInstance of rows) {
+      const row = await this.create(relatedInstance, pivotCallback)
+      savedRows.push(row)
+    }
+
+    return savedRows
   }
 }
 

src/Lucid/Relations/HasMany.js

@@ -200,7 +200,14 @@ class HasMany extends BaseRelation {
     }
 
     await this._persistParentIfRequired(trx)
-    return Promise.all(arrayOfPayload.map((payload) => this.create(payload, trx)))
+
+    const savedRows = []
+    for (let payload of arrayOfPayload) {
+      const row = await this.create(payload, trx)
+      savedRows.push(row)
+    }
+
+    return savedRows
   }
 
   /**
@@ -221,7 +228,14 @@ class HasMany extends BaseRelation {
     }
 
     await this._persistParentIfRequired(trx)
-    return Promise.all(arrayOfRelatedInstances.map((relatedInstance) => this.save(relatedInstance, trx)))
+
+    const savedRows = []
+    for (let relatedInstance of arrayOfRelatedInstances) {
+      const row = await this.save(relatedInstance, trx)
+      savedRows.push(row)
+    }
+
+    return savedRows
   }
 }
 

test/unit/database.spec.js

@@ -78,7 +78,7 @@ test.group('Database | QueryBuilder', (group) => {
   test('commit transactions', async (assert) => {
     const trx = await this.database.beginTransaction()
     await trx.table('users').insert({ username: 'virk' })
-    trx.commit()
+    await trx.commit()
     const firstUser = await this.database.table('users').first()
     assert.equal(firstUser.username, 'virk')
     await this.database.truncate('users')
@@ -108,15 +108,15 @@ test.group('Database | QueryBuilder', (group) => {
     await this.database.table('users').insert({ username: 'virk' })
     const users = await this.database.table('users')
     assert.lengthOf(users, 1)
-    this.database.rollbackGlobalTransaction()
+    await this.database.rollbackGlobalTransaction()
     const usersAfterRollback = await this.database.table('users')
     assert.lengthOf(usersAfterRollback, 0)
   })
 
   test('commit global transactions', async (assert) => {
     await this.database.beginGlobalTransaction()
     await this.database.table('users').insert({ username: 'virk' })
-    this.database.commitGlobalTransaction()
+    await this.database.commitGlobalTransaction()
     const users = await this.database.table('users')
     assert.lengthOf(users, 1)
     await this.database.truncate('users')

test/unit/helpers/index.js

@@ -40,10 +40,11 @@ module.exports = {
     if (process.env.DB === 'mysql') {
       return _.cloneDeep({
         client: 'mysql',
+        version: '5.5',
         connection: {
           host: '127.0.0.1',
-          user: 'root',
-          password: '',
+          user: 'virk',
+          password: 'virk',
           database: 'testing_lucid'
         }
       })

test/unit/helpers/mysqlConnections.js

@@ -9,27 +9,30 @@
 module.exports = {
   default: {
     client: 'mysql',
+    version: '5.5',
     connection: {
-      user: 'root',
-      password: '',
+      user: 'virk',
+      password: 'virk',
       database: 'default'
     }
   },
 
   alternateConnection: {
     client: 'mysql',
+    version: '5.5',
     connection: {
-      user: 'root',
-      password: '',
+      user: 'virk',
+      password: 'virk',
       database: 'alternate'
     }
   },
 
   defaultPrefix: {
     client: 'mysql',
+    version: '5.5',
     connection: {
-      user: 'root',
-      password: '',
+      user: 'virk',
+      password: 'virk',
       database: 'default'
     },
     prefix: 'ad_'

test/unit/lucid-belongs-to-many.spec.js

@@ -1119,7 +1119,7 @@ test.group('Relations | Belongs To Many', (group) => {
     await user.save()
 
     await user.posts().attach([1, 2, 3], (pivotModel) => (pivotModel.is_published = true))
-    const pivotValues = await ioc.use('Database').table('post_user')
+    const pivotValues = await ioc.use('Database').table('post_user').orderBy('id', 'asc')
     assert.lengthOf(pivotValues, 3)
     assert.equal(pivotValues[0].user_id, 1)
     assert.equal(pivotValues[0].post_id, 1)
@@ -1156,7 +1156,8 @@ test.group('Relations | Belongs To Many', (group) => {
         pivotModel.is_published = false
       }
     })
-    const pivotValues = await ioc.use('Database').table('post_user')
+    const pivotValues = await ioc.use('Database').table('post_user').orderBy('id', 'asc')
+
     assert.lengthOf(pivotValues, 3)
     assert.equal(pivotValues[0].user_id, 1)
     assert.equal(pivotValues[0].post_id, 1)