Lucid: Lifecycle hooks are called even thought the transaction is not committed yet

[iam4x]

Hi!

We are working on a AdonisJS + Algolia integration, the package is meant to be public in the end.

We have an issue with transaction and lifecycle hooks, maybe you can give us a hint?
Here is the log of the issue in a adonis repl:

mobile.club[development] > o = await Order.find(1819)
undefined

mobile.club[development] > DB = use('Database')
DatabaseManager {
  Config:
   Config {
     _configPath: '/Users/iam4x/mobile.club/workspace/packages/api/config',
     _config:
      { algolia: [Object],
        app: [Object],
        auth: [Object],
        bodyParser: [Object],
        cors: [Object],
        database: [Object],
        drive: [Object],
        mail: [Object],
        redis: [Object] } },
  _connectionPools:
   { pg: Database { connectionClient: 'pg', knex: [Function], _globalTrx: null } } }

// Creating Transaction
mobile.club[development] > trx = await DB.beginTransaction()
undefined

// Doing stuff into the transaction
mobile.club[development] > await o.transitionTo('order_awaiting_shipment', {}, trx)

// ---->> WRONG??
// afterUpdate is called (re-index into Algolia)
afterUpdate
{
    "id": 1819,
    "customer_id": 1818,
    "country_id": 1,
    "coupon_id": null,
    "code": "MAR0100718",
    "shipping_address": {
        "zip": "XXX",
        "city": "XXX",
        "extra": "XXX",
        "street": "XXX"
    },
    "billing_address": {
        "zip": "XXX",
        "city": "XXX",
        "street": "XXX",
        "country": "XXX"
    },
    "note": null,
    "activated_at": null,
    "deleted_at": null,
    "created_at": "2018-07-04 16:22:35",
    "updated_at": "2018-08-07 16:45:18",
    "is_business_order": false,
    "company_name": null,
    "company_registration_number": null,
    "active": false,
    "recurly_subscription_uuid": "XXX",
    "algolia_object_id": "XXXX"
}
afterUpdate
{
    "id": 16,
    "customer_id": 1818,
    "created_at": "2018-07-05 18:03:19",
    "updated_at": "2018-08-07 16:45:18",
    "algolia_object_id": "XXX"
}
afterUpdate
{
    "id": 1818,
    "email": "XXX",
    "first_name": "XXX",
    "last_name": "XXX",
    "phone": "XXX",
    "created_at": "2018-07-04 16:22:35",
    "updated_at": "2018-08-07 16:45:18",
    "gender": "f",
    "accepted_marketing_at": "2018-07-05T16:05:44.111Z",
    "accepted_cgl_at": "2018-07-05T16:05:44.111Z",
    "accepted_cgu_at": "2018-07-05T16:05:44.111Z",
    "accepted_contract_at": "2018-07-05T16:05:44.111Z",
    "birthdate": "1963-11-12T23:00:00.000Z",
    "invite_sent": true,
    "algolia_object_id": "XXX",
    "is_password_set": true,
    "has_completed_order": true,
    "__meta__": {
        "has_completed_order": true
    }
}
true

// Commit the transaction actual changes saved into dabase
// No lifecycle hooks are triggered at this time
mobile.club[development] > await trx.commit()
{ sql: 'COMMIT;',
  bindings: undefined,
  response:
   Result {
     command: 'COMMIT',
     rowCount: null,
     oid: null,
     rows: [],
     fields: [],
     _parsers: [],
     RowCtor: null,
     rowAsArray: false,
     _getTypeParser: [Function: bound ] } }

So two questions:

• Is this fixable? Since knex is handling transactions and not lucid.
• If not, would it be possible to have an afterTransaction? Or to get the transaction in the afterXXX hooks so we can do our stuff after it.

Thanks again for the amazing work on AdonisJS 💘

cc @othierry my co-worker on this!

[thetutlage]

I believe this is more of a question, is current behaviour the desired behaviour or not.

Currently, Lucid hooks are executed when queries are executed and not when transaction is committed or rollback.

This is helpful in certain conditions and not helpful in some conditions.

For example: If your hook wants to perform an action on the database, it can re-use the same trx object and perform the action. It transaction get's rolled back, their action will be rolled back too.

However, if your actions in the hook are not related to the database, then will take place, regardless of whether transaction was committed or rolled back.

How about afterTransaction?

Transactions are meant to be shared. For example: 2 models can write to DB using the same transaction (which is desired too). Now, when afterTransaction hook is fired, it will fire on which model? or will it fire for all the models using transactions?

[thetutlage]

Also sequelize has this as a design concern on their repo too. Pasting here for everyone to read https://github.com/sequelize/sequelize/issues/8585

[thetutlage]

Okay another one Laravel repo and one related to their official Algolia package scout

I have some general design idea in my mind. I will do an RFC on same and will drop the link here.

[iam4x]

Thank's for the response @thetutlage 👍

Yes this issue is tricky, I have not enough knowledge of the Lucid itself to purpose a solution. Even thought I looked at Knex transactions and they are Promises, so something should be do-able but it won't be easy.

Will be happy to help and review the RFC you are looking to write!

Max.

[othierry]

Hey @thetutlage !

Thanks for the great work on Adonisjs.

We've been working with @iam4x on a small poc for this (afterTransaction) (https://github.com/mobile-club/adonis-lucid/pull/1). by registering hooks on commit and rollback events. We also share the transaction while its active between the model's relation so It also fixes the following issue we have encountered.

$ yarn adonis console
$ mobile.club[development] > Database = use('Database)
$ mobile.club[development] > trx = await Database.beginTransaction()
$ mobile.club[development] > parcel = await Parcel.create({ ... }, trx)
$ mobile.club[development] > stockItem = await StockItem.findBy('serial', 'XXX')
$ mobile.club[development] > stockItem.parcels().attach([parcel.id], null, trx)
$ mobile.club[development] > await parcel.stockItems().getCount() // expected: 1, actual: 0

Because now transaction is shared across models (and its relations):

• await parcels().stockItems().getCount() now correctly returns 1 (because Parcel currently lives within a transaction, its relations queries are also binded to this transaction)
• afterTransaction is called on all the object graph (so in the above example, commiting or rolling back the transaction will call both Parcel.afterTransaction and StockItem.afterTransaction)
• Once the transaction is commited or rolled back, the hook resets the model's $transaction to null.

We need to write a proper PR description & some tests but your thoughts would be more than welcome before going further !

Oli.

[thetutlage]

@othierry Nice to hear that.

I also thought something similar to what you shared, but de-coupled from models.

1. Everytime you call Database.beginTransaction, the trx instance should allow you to define lifecycle hooks to it.

trx.onCommit(callback, data)
trx.onRollback(callback, data)

Now inside models you can simply attach to these hooks.

class User extends Model {
  static boot () {
    super.boot()

    this.addHook('afterSave', function (modelInstance) {
      if (modelInstance.hasTransaction()) {
        modelInstance.getTransaction().onCommit()
        modelInstance.getTransaction().onRollback()
      }
    })
  }
}

onCommit(callback, data)

When transaction commits, the callback is executed with the data as 1st argument.

onCommit(function (data) {
  // transaction commited
}, modelInstance)

onRollback(callback, data)

When transaction rollback, the callback is executed with the data as 1st argument.

onCommit(function (data) {
  // transaction commited
}, modelInstance)

[othierry]

Hey @thetutlage

Cool! I like the idea of transaction hooks being decoupled from the Model.
Is this something you are currently working on or would you like some help to implement it ?

Few questions:

• Would Model#hasTransaction and Model#getTransaction just getters for a $transaction attribute or would they have a particular dynamic logic or side effect ? (if that's not the case we may access the attribute directly like $sideLoaded for example?)
• Should we automatically register a least one onCommit/onCallback to reset this.$transaction to null when we attach the model to a transaction ? (in _insert() ou _update() call for ex)

And finally what about my 2nd point ? (from my original post) the fact that a model should share it's current transaction (if any) with it's relations queries, which fixes the bug I have described above. So basically having this in BaseRelation's constructor:

this.relatedQuery = parentInstance.$transaction
 ? this.RelatedModel.query().transacting(parentInstance.$transaction)
 : this.RelatedModel.query()

[othierry]

Any update @thetutlage ?

[thetutlage]

Ohh I am sorry, I have no idea how I missed it twice.

Would Model#hasTransaction and Model#getTransaction just getters for a $transaction attribute or would they have a particular dynamic logic or side effect ? (if that's not the case we may access the attribute directly like $sideLoaded for example?)

I believe using just the $transaction should be fine. I was just thinking too much.

Should we automatically register a least one onCommit/onCallback to reset this.$transaction to null when we attach the model to a transaction ? (in _insert() ou _update() call for ex)
Yeah we can.

And finally what about my 2nd point ? (from my original post) the fact that a model should share it's current transaction (if any) with it's relations queries

I am not 100% sure, if sharing transaction across relation chains will be good idea or not. I just have to think about enough use cases to see if it really works.

So we can actually divide this feature into 2 PR's.

• One is for transaction hooks
• Sharing the transaction with the relations.

I am currently not working on either of them. If you want, you can pick them or I will try to get some time over the weekend to work on the transaction hooks 😄

[thetutlage]

Bit late on this, so not sure it it's helpful for you.

The newer release of Lucid (part of AdonisJs v5) allows hooking into transactions as well. This is how a typical hook lifecycle will look like.

afterSaveHookFunction (modelInstance) {
  if (modelInstance.$trx) {
    modelInstance.$trx.on('commit', async () => {
      // After save and transaction also committed
    })
  } else {
    // After save
  }
}

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.