{"action":"created","issue":{"url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26","labels_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26/labels{/name}","comments_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26/comments","events_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26/events","html_url":"https://github.com/postmanlabs/sails-mysql-transactions/issues/26","id":123875228,"number":26,"title":"[reflection] [idea]","user":{"login":"mikermcneil","id":618009,"avatar_url":"https://avatars.githubusercontent.com/u/618009?v=3","gravatar_id":"","url":"https://api.github.com/users/mikermcneil","html_url":"https://github.com/mikermcneil","followers_url":"https://api.github.com/users/mikermcneil/followers","following_url":"https://api.github.com/users/mikermcneil/following{/other_user}","gists_url":"https://api.github.com/users/mikermcneil/gists{/gist_id}","starred_url":"https://api.github.com/users/mikermcneil/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikermcneil/subscriptions","organizations_url":"https://api.github.com/users/mikermcneil/orgs","repos_url":"https://api.github.com/users/mikermcneil/repos","events_url":"https://api.github.com/users/mikermcneil/events{/privacy}","received_events_url":"https://api.github.com/users/mikermcneil/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2015-12-25T11:42:19Z","updated_at":"2015-12-29T04:28:10Z","closed_at":null,"body":"First of all, this module is awesome; amazing work (particularly the depth you went to w/ making instance methods do their thing when amidst a transaction).\r\n\r\nI wanted to leave a note here with some reflections on the history and future of transactions in WL core, and also share an idea for some syntactic sugar based on what I learned from WL2.\r\n\r\n### Transactions in WL core\r\n\r\nBad news first:  We will probably never have true atomic, durable, consistent, and isolated _cross-adapter transactions_ in Waterline core.  \r\n\r\n> Aside: I've always **dreamed** of having cross-adapter transactions in WL core.  I spent a bunch of time messing with this [waaaaaay back](https://github.com/balderdashy/sails/commits/master?page=126) when I was first building Waterline.  In fact, that was almost exactly 3 years ago to the day (Christmas 2012).  As it turns out, trying to implement cross-adapter transactions is a [nightmare](https://github.com/balderdashy/sails/blob/09037937366cecc222c7a53829d3c6e4f64ad822/adapter.js#L307) and basically like [writing your own database](https://github.com/mikermcneil/waterline2/blob/master/Introduction%20to%20Waterline2.pdf). Nevertheless, for some reason I spent the time to get the \"CI\" in \"ACID\" working-- but I never got around to implementing inverse operations in order to be able to roll back-- which we would need to form a truly crash-resistant (read \"durable\") [commit log](https://github.com/balderdashy/sails/blob/09037937366cecc222c7a53829d3c6e4f64ad822/collections/Transaction.js)).  Long story short: it is an interesting exercise, but ultimately a bad idea.\r\n\r\nThe good news is that we will _definitely_ implement transactions across the same connection to the same database.  The question is just when-- and you guys have done an amazing job hastening that along.\r\n\r\n\r\n### An idea re: syntax\r\n\r\nThe #1 thing I've always been worried about w/ transactions is forgetting to rollback under rare error conditions (i.e. that never get tested).  The #2 thing has always been forgetting to use the proper transaction instance and accidentally using models directly.\r\n\r\nWhen I was actively working on WL2 last summer, the simplest possible usage I could come up with which also addressed both issues (at least to some degree) was something like [this](https://github.com/mikermcneil/waterline2/blob/16ac1edf7e113eee94970ce5ba894b0d0eddbb0a/lib/ORM/ORM.prototype.transaction.js#L17):\r\n\r\n```javascript\r\norm.transaction([User, Kitten], function (User, Kitten, cb) {\r\n  Kitten.create({ name: req.param('kittenName') }).exec(function (err, babyKitten){\r\n    if (err) {\r\n      return cb(err); // << that means rollback\r\n    }\r\n    User.update({ id: req.session.userId }).set({ petKitten: babyKitten.id }).exec(function (err, updatedUsers) {\r\n      if (err)  {\r\n        return cb(err);  // << that means rollback\r\n      }\r\n      if (updatedUsers.length === 0) {\r\n        err = new Error('Consistency violation: Could no longer find currently logged in user in database!');\r\n        err.code = 'loggedInUserMissingFromDb';\r\n        return cb(err);  // << that means rollback\r\n      }\r\n      return cb(undefined, updatedUsers[0]);  // << that means commit  (so would `cb()`)\r\n      // (second arg is optional, but if you pass it in, it'll show up as the 2nd argument\r\n      //  of the callback below)\r\n    });\r\n  });\r\n}).exec(function (err, data) {\r\n  if (err) {\r\n    // if `err` is truthy, then there was an error, and the transaction was\r\n    // already rolled back by the time we make it here.\r\n    return res.negotiate(err);\r\n  }\r\n\r\n  // if not, there were no errors, and the transaction was safely committed\r\n  // by the time we make it here!\r\n  return res.json({\r\n    username: data.username,\r\n    kittenId: data.petKitten\r\n  });\r\n});\r\n```\r\n\r\nBelow, I put together a hypothetical remapping of the example in the README of this module if it was wrapped up in something like this (in _addition_ to `.start()`, not instead of it).  Please take it with a grain of salt-- I'm not saying I think it should be this way in _this_ module necessarily (it's more in-line with what we'd need in core Waterline anyway-- i.e.  triggering a usage error if the models are not part of the same database connection).  I just want to make sure and share this thinking with you guys in case its helpful or spurs any ideas for folding transaction support into Sails/Waterline core.  \r\n\r\nHere goes nothing:\r\n\r\n```javascript\r\nvar Transaction = require('sails-mysql-transactions').Transaction;\r\n\r\n// Attempt a database transaction\r\nTransaction.attempt([OneModel, AnotherModel], function (OneModel, AnotherModel, cb) {\r\n  OneModel.create(req.params.all(), function (err, someRecord) {\r\n    if (err) {\r\n      return cb(err);\r\n    }\r\n    AnotherModel.findOne(req.param('id')).exec(function (err, anotherRecord) {\r\n      if (err) {\r\n        return cb(err);\r\n      }\r\n      someRecord.someAssociation.remove(req.param('remove_id'));\r\n      someRecord.save(function (err, savedRecord){\r\n        if (err) {\r\n          return cb(err);\r\n        }\r\n        return cb({\r\n          savedRecord: savedRecord\r\n          anotherRecord: anotherRecord\r\n        });\r\n      });\r\n    });\r\n  });\r\n}).exec(function (err, data) {\r\n  if (err) {\r\n    // This means that either something explicitly passed back an error in the provided function above,\r\n    // or the function never ran because a transaction could not be started, or that the transaction was\r\n    // otherwise successful but could not be committed. If we're here, we know the transaction has\r\n    // been rolled back successfully.\r\n    return res.serverError(err);\r\n  }\r\n  \r\n  // Otherwise, we know everything worked and that the transaction was committed.\r\n  return res.json({\r\n    one: data.savedModel,\r\n    another: data.anotherInstance\r\n  });\r\n});\r\n```\r\n\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/comments/167716672","html_url":"https://github.com/postmanlabs/sails-mysql-transactions/issues/26#issuecomment-167716672","issue_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26","id":167716672,"user":{"login":"mikermcneil","id":618009,"avatar_url":"https://avatars.githubusercontent.com/u/618009?v=3","gravatar_id":"","url":"https://api.github.com/users/mikermcneil","html_url":"https://github.com/mikermcneil","followers_url":"https://api.github.com/users/mikermcneil/followers","following_url":"https://api.github.com/users/mikermcneil/following{/other_user}","gists_url":"https://api.github.com/users/mikermcneil/gists{/gist_id}","starred_url":"https://api.github.com/users/mikermcneil/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikermcneil/subscriptions","organizations_url":"https://api.github.com/users/mikermcneil/orgs","repos_url":"https://api.github.com/users/mikermcneil/repos","events_url":"https://api.github.com/users/mikermcneil/events{/privacy}","received_events_url":"https://api.github.com/users/mikermcneil/received_events","type":"User","site_admin":false},"created_at":"2015-12-29T04:28:10Z","updated_at":"2015-12-29T04:28:10Z","body":"I think so too."}}