Skip to content

Promisify 'automigrate' #630

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 16, 2015
Merged

Promisify 'automigrate' #630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions lib/datasource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,9 +775,12 @@ DataSource.prototype.automigrate = function (models, cb) {
models = undefined;
}

cb = cb || utils.createPromiseCallback();

if (!this.connector.automigrate) {
// NOOP
return cb && process.nextTick(cb);
process.nextTick(cb);
return cb.promise;
}

// First argument is a model name
Expand All @@ -791,24 +794,25 @@ DataSource.prototype.automigrate = function (models, cb) {
models = models || Object.keys(attachedModels);

if (models.length === 0) {
return cb && process.nextTick(cb);
process.nextTick(cb);
return cb.promise;
}

var invalidModels = models.filter(function (m) {
return !(m in attachedModels);
});

if (invalidModels.length) {
return process.nextTick(function () {
if (cb) {
process.nextTick(function () {
cb(new Error('Cannot migrate models not attached to this datasource: ' +
invalidModels.join(' ')));
}
});
return cb.promise;
}
}

this.connector.automigrate(models, cb);
return cb.promise;
};

/**
Expand Down
66 changes: 66 additions & 0 deletions test/memory.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,25 +463,91 @@ describe('Memory connector', function() {
});
});

it('automigrate all models - promise variant', function(done) {
ds.automigrate()
.then(function(result) {
done();
})
.catch(function(err){
done(err);
});
Copy link
Member

Choose a reason for hiding this comment

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

FYI: this can be shortened to .catch(done). For the purpose of these tests, I think it's ok to keep the longer version. It's more explicit and thus it may be easier to understand.

});

it('automigrate one model', function(done) {
ds.automigrate('m1', function(err) {
done(err);
});
});

it('automigrate one model - promise variant', function(done) {
ds.automigrate('m1')
.then(function(result) {
done();
})
.catch(function(err){
done(err);
});
});

it('automigrate one or more models in an array', function(done) {
ds.automigrate(['m1'], function(err) {
done(err);
});
});

it('automigrate one or more models in an array - promise variant', function(done) {
ds.automigrate(['m1'])
.then(function(result) {
done();
})
.catch(function(err){
done(err);
});
});

it('automigrate reports errors for models not attached', function(done) {
ds.automigrate(['m1', 'm2'], function(err) {
err.should.be.an.instanceOf(Error);
done();
});
});

it('automigrate reports errors for models not attached - promise variant', function(done) {
ds.automigrate(['m1', 'm2'])
.then(function(){
done(new Error('automigrate() should have failed'));
})
.catch(function(err){
err.should.be.an.instanceOf(Error);
done();
});
});

});

describe('automigrate when NO models are attached', function() {
var ds;
beforeEach(function() {
ds = new DataSource({
connector: 'memory'
});
Copy link
Member

Choose a reason for hiding this comment

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

I am proposing to create a describe block and move the initialisation of an empty datasource to a beforeEach hook, in order to reduce the amount of duplicated code.

describe('automigrate when NO models are attached', function() {\
  var ds;
  beforeEach(/*setup ds */);
  it('does NOT report error', /*...*/);
  it('does NOT report error - promise variant');
});

Thoughts?

});

it('automigrate does NOT report error when NO models are attached', function(done) {
ds.automigrate(function(err) {
done();
})
});

it('automigrate does NOT report error when NO models are attached - promise variant', function(done) {
ds.automigrate()
.then(done)
.catch(function(err){
done(err);
});
});
});

});

describe('Optimized connector', function() {
Expand Down