New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactions #1062

Merged
merged 98 commits into from Dec 7, 2013

Conversation

6 participants
@sdepold
Member

sdepold commented Nov 18, 2013

This pull request adds transaction support.

The transaction flow can be controlled like this:

sequelize.transaction(function(t) {
  // we just opened a new connection to the database which is transaction exclusive.
  // also we send some first transaction queries to the database.

  // do some async stuff ...

  // if everything is ok ... commit the transaction
  t.commit().success(function() {})

  // if something failed ... rollback the transaction
  t.rollback().success(function() {})

  // the commit / rollback will emit events which can be observed via:
  t.done(function() { 
    /* we will be here once the transaction 
    has been committed / reverted */ 
  })
})

In order to execute queries inside the transaction, the options parameter of DAOFactory/DAO methods accepts the transaction key:

sequelize.transaction(function(t) {
  User.create({ username: 'foo' }, { transaction: t }).success(function() {
    // this user will only be available inside the session
    User.all({ transaction: t }) // will return the user
    User.all() // will not return the user
  })
})

This PR also adds Utils.validateParameter which checks the existence of parameters and the data type integrity.

@sdepold

This comment has been minimized.

Member

sdepold commented Nov 19, 2013

Transaction method signatures:

Class methods

  • User.findOrInitialize({ username: 'foo' }, { transaction: t })
  • User.findOrCreate({ username: 'Username' }, { data: 'some data' }, { transaction: t })
  • User.create({ username: 'user' }, { transaction: t })
  • User.bulkCreate([{ username: 'foo' }, { username: 'bar' }], { transaction: t })
  • User.update({ username: 'bar' }, {}, { transaction: t })
  • User.destroy({}, { transaction: t })
  • User.find({ username: 'foo' }, { transaction: t })
  • User.findAll({ transaction: t })
  • User.findAndCountAll({ transaction: t })
  • User.all({ transaction: t })
  • User.count({ transaction: t })
  • User.min('age', { transaction: t })
  • User.min('age', { transaction: t })
  • User.where({ username: "foo" }).exec({ transaction: t })

Instance methods

  • user.increment('number', { by: 2, transaction: t })
  • user.decrement('number', { by: 2, transaction: t })
  • user.reload({ transaction: t })
  • user.save({ transaction: t })
  • user.updateAttributes({ username: 'bar' }, { transaction: t })
  • user.destroy({ transaction: t })
  • group.setUser(user, { transaction: t })
  • groups.getUser({ transaction: t })
  • articles.hasLabel(label, { transaction: t })
  • article.setLabels([ label ], { transaction: t })

That should be it.

@@ -277,7 +279,7 @@ module.exports = (function() {
if (err) {
switch(err.code) {
case 'ECONNREFUSED':
case 'ER_ACCESS_DENIED_ERROR':
case 'ER_ACCESS_D2ENIED_ERROR':

This comment has been minimized.

@janmeier

janmeier Nov 21, 2013

Member

D2ENIED :) ?

This comment has been minimized.

@sdepold

sdepold Nov 22, 2013

Member

as discussed in irc: this is on purpose

QueryInterface.prototype.setIsolationLevel = function(transaction, value) {
if (!transaction || !(transaction instanceof Transaction)) {
throw new Error('Unable to set isolatipn level for a transaction without transaction object!')

This comment has been minimized.

@janmeier

janmeier Nov 21, 2013

Member

Typo - sorry for being nitpicky :)

This comment has been minimized.

@sdepold

sdepold Nov 22, 2013

Member

fixed

"watchr": "~2.4.3",
"yuidocjs": "~0.3.36",
"chai": "~1.8.0",
"mocha": "~1.13.0",
"chai-datetime": "~1.1.1",
"sinon": "~1.7.3",
"mariasql": "git://github.com/sequelize/node-mariasql.git"
"mariasql": "git://github.com/sequelize/node-mariasql.git",

This comment has been minimized.

@janmeier

janmeier Nov 21, 2013

Member

This can be changed to the official mscdex repo (the change is not published to npm yet though)

This comment has been minimized.

@sdepold

sdepold Nov 22, 2013

Member

so we cannot change it yet?

This comment has been minimized.

@janmeier

janmeier Nov 23, 2013

Member

We can change it to "git://github.com/mscdex/node-mariasql.git", but not to "mariasql"

This comment has been minimized.

@janmeier

janmeier Nov 23, 2013

Member

Actually I just did a check, and the version in npm has the required changes, so there should be no problem changing it :)

This comment has been minimized.

@sdepold

sdepold Nov 25, 2013

Member

done!

Merge branch 'master' into features/transactions
Conflicts:
	lib/dialects/sqlite/query.js
@janmeier

This comment has been minimized.

Member

janmeier commented Nov 23, 2013

Looks good to merge from me

@alexhanh

This comment has been minimized.

alexhanh commented Nov 25, 2013

Do the transactions support connection pooling?

@sdepold

This comment has been minimized.

Member

sdepold commented Nov 26, 2013

The current implementation will bypass the pooling settings and open up a new connection per transaction.

@durango

This comment has been minimized.

Member

durango commented Nov 28, 2013

@sdepold what happens to locked rows/tables within the other connections in the pools?

@sdepold

This comment has been minimized.

Member

sdepold commented Nov 29, 2013

They should be working normally. At least that is what the tests say. Though SQLite behaves different. It won't allow you to do multiple things on the same table in parallel.

Merge branch 'master' into features/transactions
Conflicts:
	.gitignore
	lib/dao-factory.js
	lib/utils.js
	package.json
	test/dao-factory.test.js
	test/sequelize.test.js

mickhansen added a commit that referenced this pull request Dec 7, 2013

@mickhansen mickhansen merged commit 631616c into master Dec 7, 2013

@janmeier janmeier deleted the features/transactions branch Jul 18, 2014

@jbarros35

This comment has been minimized.

jbarros35 commented Jun 28, 2015

it doesnt works with express services. try yourself.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jun 28, 2015

@jbarros35 Sorry what? Express shouldn't really have any impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment