Skip to content
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

feat: use transactions when writing to the database #527

Merged
merged 18 commits into from
Dec 5, 2016

Conversation

zacharygolba
Copy link
Contributor

@zacharygolba zacharygolba commented Nov 13, 2016

This PR introduces a public transaction api to the model class. Changes are backwards
compatible with the previous model api.

Internally, all methods that modify the state of the database are wrapped in transactions.
If the transaction fails, all calls to create, save, or update will be rolled back to
the state before the transaction began.

Example:

import User from 'app/models/user';

// This internally uses a transaction.
await User.create({
  firstName: 'New',
  lastName: 'User'
});

Working With Transactions

You have the ability to manually specify the transaction that will be used for a
create, update, or save call with the static and instance method, transacting.

Example:

import { Model } from 'lux-framework';

import Profile from 'app/models/profile';

class User extends Model {
  static hasOne = {
    profile: {
      inverse: 'user'
    }
  };

  static hooks = {
    async beforeCreate(user, trx) {
      // If the transaction fails the profile instance will not be persisted.
      user.profile = await Profile
        .transacting(trx)
        .create();
    }
  };
}

You can also manually trigger create a transaction if you plan on creating many
model instances as once.

Example:

import User from 'app/models/user';

User.transaction(trx => (
  Promise.all([
    User
      .transacting(trx)
      .create({
        firstName: 'New',
        lastName: 'User'
      }),
    User
      .transacting(trx)
      .create({
        firstName: 'New',
        lastName: 'User'
      }),
    User
      .transacting(trx)
      .create({
        firstName: 'New',
        lastName: 'User'
      })
  ])
));

* @private
*/
export function tap<T>(input: T): T {
console.log(input); // eslint-disable-line no-console
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just for debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct

@codecov-io
Copy link

codecov-io commented Dec 3, 2016

Current coverage is 90.03% (diff: 95.91%)

Merging #527 into master will increase coverage by 1.11%

@@             master       #527   diff @@
==========================================
  Files           170        176     +6   
  Lines          1849       1926    +77   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1644       1734    +90   
+ Misses          205        192    -13   
  Partials          0          0          

Powered by Codecov. Last update 9e89b04...e6c80cd

@@ -299,6 +298,10 @@ export default async function initializeClass<T: Class<Model>>({
...belongsTo
});

if (!hooks) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if this could be embedded into L132?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This takes care of edge cases where hooks is null, 0, false or any other falsy value besides undefined. Those edge cases should never occur but this accounts for them just in case.

Destructed defaults only work if the value is undefined.

set(subject, 'image', image);
});

afterEach(teardown);

it('can add a record to the relationship', async () => {
console.log(image.changeSets);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's another log here, just fyi.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find!

Copy link
Contributor

@kev5873 kev5873 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else looks good to me

@zacharygolba zacharygolba merged commit 16d224b into master Dec 5, 2016
@zacharygolba zacharygolba deleted the feat-transactions branch December 5, 2016 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants