Skip to content

Commit

Permalink
Tests and fixes (#162)
Browse files Browse the repository at this point in the history 
* Decoupled the tests a bit by small refactoring. Some of the test collections do not do clean up. In addition to that some of the individual tests only do clean up when it succeeds (In the .then callback). The result is that when a test fails it causes many other tests to fail and it can be difficult to see what is breaking. This change should alleviate some of that.

* Added (breaking) test for #125

* Fix for #125

* Added tests for #161 and #125

* Fix for paging totals when calling find()
  • Loading branch information
@ryanthegiantlion @daffl
ryanthegiantlion authored and daffl committed Sep 6, 2017
1 parent 698fb0e commit d98b10e
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 41 deletions.
23 changes: 16 additions & 7 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class Service {
order,
limit: filters.$limit,
offset: filters.$skip,
raw: this.raw
raw: this.raw,
distinct: true
}, params.sequelize);

if (filters.$select) {
Expand All @@ -52,6 +53,18 @@ class Service {

let Model = this.applyScope(params);

// Until Sequelize fix all the findAndCount issues, a few 'hacks' are needed to get the total count correct

// Adding an empty include changes the way the count is done
// See: https://github.com/sequelize/sequelize/blob/7e441a6a5ca44749acd3567b59b1d6ceb06ae64b/lib/model.js#L1780-L1782
q.include = q.include || [];

// Non-raw is the default but setting it manually breaks paging
// See: https://github.com/sequelize/sequelize/issues/7931
if (q.raw === false) {
delete q.raw;
}

return Model.findAndCount(q).then(result => {
return {
total: result.count,
Expand Down Expand Up @@ -207,6 +220,7 @@ class Service {

// Force the {raw: false} option as the instance is needed to properly
// update

return this._get(id, { sequelize: { raw: false } }).then(instance => {
if (!instance) {
throw new errors.NotFound(`No record found for id '${id}'`);
Expand All @@ -221,12 +235,7 @@ class Service {
}
});

return instance.update(copy, options).then(instance => {
if (options.raw === false) {
return instance;
}
return instance.toJSON();
});
return instance.update(copy, {raw: false}).then(() => this._get(id, {sequelize: options}));
})
.then(select(params, this.id))
.catch(utils.errorHandler);
Expand Down
184 changes: 150 additions & 34 deletions test/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ const Model = sequelize.define('people', {
}
}
});
const Order = sequelize.define('orders', {
name: {
type: Sequelize.STRING,
allowNull: false
}
}, {
freezeTableName: true
});
const CustomId = sequelize.define('people-customid', {
customid: {
type: Sequelize.INTEGER,
Expand Down Expand Up @@ -105,12 +113,15 @@ const CustomGetterSetter = sequelize.define('custom-getter-setter', {
}, {
freezeTableName: true
});
Model.hasMany(Order);
Order.belongsTo(Model);

describe('Feathers Sequelize Service', () => {
before(() =>
Model.sync({ force: true })
.then(() => CustomId.sync({ force: true }))
.then(() => CustomGetterSetter.sync({ force: true }))
.then(() => Order.sync({ force: true }))
);

describe('Initialization', () => {
Expand All @@ -123,7 +134,7 @@ describe('Feathers Sequelize Service', () => {
});
});

describe('Common functionality', () => {
describe('Common Tests', () => {
const app = feathers()
.use('/people', service({
Model,
Expand All @@ -133,64 +144,169 @@ describe('Feathers Sequelize Service', () => {
Model: CustomId,
events: [ 'testing' ],
id: 'customid'
}));

base(app, errors);
base(app, errors, 'people-customid', 'customid');
});

describe('Feathers-Sequelize Specific Tests', () => {
const app = feathers()
.use('/people', service({
Model,
paginate: {
default: 10
},
events: [ 'testing' ]
}))
.use('/orders', service({
Model: Order
}))
.use('/custom-getter-setter', service({
Model: CustomGetterSetter,
events: [ 'testing' ]
}));

it('allows querying for null values (#45)', () => {
const name = 'Null test';
before(() => app.service('people')
.remove(null, { query: { $limit: 1000 } })
);

after(() => app.service('people')
.remove(null, { query: { $limit: 1000 } })
);

describe('Common functionality', () => {
const people = app.service('people');
const _ids = {};
const _data = {};

beforeEach(() =>
people.create({ name: 'Kirsten', age: 30 }).then(result => {
_data.Kirsten = result;
_ids.Kirsten = result.id;
})
);

return people.create({ name }).then(person =>
people.find({ query: { age: null } }).then(people => {
assert.equal(people.length, 1);
assert.equal(people[0].name, name);
assert.equal(people[0].age, null);
}).then(() => people.remove(person.id))
afterEach(() =>
people.remove(_ids.Kirsten).catch(() => {})
);

it('allows querying for null values (#45)', () => {
const name = 'Null test';

return people.create({ name }).then(person =>
people.find({ query: { age: null } }).then(people => {
assert.equal(people.data.length, 1);
assert.equal(people.data[0].name, name);
assert.equal(people.data[0].age, null);
})
.then(() => people.remove(person.id))
.catch((err) => { people.remove(person.id); throw (err); })
);
});

it('correctly persists updates (#125)', () => {
const updateName = 'Ryan';

return people.update(_ids.Kirsten, { name: updateName })
.then((data) => people.get(_ids.Kirsten))
.then(updatedPerson => {
assert.equal(updatedPerson.name, updateName);
});
});
});

it('calls custom getters and setters (#113)', () => {
const value = 0;
const service = app.service('custom-getter-setter');
const data = {addsOneOnGet: value, addsOneOnSet: value};
describe('Association Tests', () => {
const people = app.service('people');
const orders = app.service('orders');
const _ids = {};
const _data = {};

beforeEach(() =>
people.create({ name: 'Kirsten', age: 30 })
.then(result => {
_data.Kirsten = result;
_ids.Kirsten = result.id;
return orders.create([
{ name: 'Order 1', personId: result.id },
{ name: 'Order 2', personId: result.id },
{ name: 'Order 3', personId: result.id }
]);
})
.then(() => people.create({ name: 'Ryan', age: 30 }))
.then(result => {
_data.Ryan = result;
_ids.Ryan = result.id;
return orders.create([
{ name: 'Order 4', personId: result.id },
{ name: 'Order 5', personId: result.id },
{ name: 'Order 6', personId: result.id }
]);
})
);

afterEach(() =>
orders.remove(null, { query: { $limit: 1000 } })
.then(() => people.remove(_ids.Kirsten))
.then(() => people.remove(_ids.Ryan))
.catch(() => {})
);

return service.create(data).then(result => {
assert.equal(result.addsOneOnGet, value + 1);
assert.equal(result.addsOneOnSet, value + 1);
it('find() returns correct total when using includes for non-raw requests (#137)', () => {
const options = {sequelize: {raw: false, include: Order}};
return people.find(options).then(result => {
assert.equal(result.total, 2);
});
});

it('find() returns correct total when using includes for raw requests', () => {
const options = {sequelize: {include: Order}};
return people.find(options).then(result => {
assert.equal(result.total, 2);
});
});
});

it('can ignore custom getters and setters (#113)', () => {
const value = 0;
const service = app.service('custom-getter-setter');
const data = {addsOneOnGet: value, addsOneOnSet: value};
const IGNORE_SETTERS = {sequelize: {ignoreSetters: true}};
return service.create(data, IGNORE_SETTERS).then(result => {
assert.equal(result.addsOneOnGet, value + 1);
assert.equal(result.addsOneOnSet, value);
describe('Custom getters and setters', () => {
it('calls custom getters and setters (#113)', () => {
const value = 0;
const service = app.service('custom-getter-setter');
const data = {addsOneOnGet: value, addsOneOnSet: value};

return service.create(data).then(result => {
assert.equal(result.addsOneOnGet, value + 1);
assert.equal(result.addsOneOnSet, value + 1);
});
});

it('can ignore custom getters and setters (#113)', () => {
const value = 0;
const service = app.service('custom-getter-setter');
const data = {addsOneOnGet: value, addsOneOnSet: value};
const IGNORE_SETTERS = {sequelize: {ignoreSetters: true}};
return service.create(data, IGNORE_SETTERS).then(result => {
assert.equal(result.addsOneOnGet, value + 1);
assert.equal(result.addsOneOnSet, value);
});
});
});

it('can set the scope of an operation#130', () => {
const service = app.service('people');
const people = app.service('people');
const data = {name: 'Active', status: 'active'};
const SCOPE_TO_ACTIVE = {sequelize: {scope: 'active'}};
const SCOPE_TO_PENDING = {sequelize: {scope: 'pending'}};
return service.create(data).then(person => {
return service.find(SCOPE_TO_ACTIVE).then(people => {
assert.equal(people.length, 1);
return service.find(SCOPE_TO_PENDING).then(people => {
assert.equal(people.length, 0);
return people.create(data).then(person => {
return people.find(SCOPE_TO_ACTIVE).then(result => {
assert.equal(result.data.length, 1);
return people.find(SCOPE_TO_PENDING).then(result => {
assert.equal(result.data.length, 0);
});
}).then(() => service.remove(person.id));
})
.then(() => people.remove(person.id))
.catch((err) => { people.remove(person.id); throw (err); });
});
});

base(app, errors);
base(app, errors, 'people-customid', 'customid');
});

describe('ORM functionality', () => {
Expand Down

0 comments on commit d98b10e

Please sign in to comment.