Skip to content
This repository has been archived by the owner on Mar 4, 2019. It is now read-only.

Commit

Permalink
fix: decompose should preserve ordering of query results (#531)
Browse files Browse the repository at this point in the history
* Add failing test

* Fix: Decomposed results are not ordered correctly

The issue was caused by a behavior of Object.keys() which sorts integer keys numerically. This fix ensures that the keys are strictly treated as strings by simply prefixing a non-numeric character onto each id used as a key in the decompose mapping.

* Fix test broken by previous fix
  • Loading branch information
nireno authored and dmfay committed Feb 3, 2018
1 parent eda1955 commit d13bb2f
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 11 deletions.
19 changes: 12 additions & 7 deletions lib/util/decompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,27 @@ exports = module.exports = function (schema, data) {
return (function build (obj, objSchema) {
const id = row[objSchema.pk];

// Use a string id instead of the (potentially numerical) row id as keys
// in the mapping. This prevents Object.keys from reordering the
// mapping in the "transform" step.
const strid = '_' + id;

if (id === null) {
// null id means this entity doesn't exist (eg outer join)
return undefined;
} else if (!obj.hasOwnProperty(id)) {
} else if (!obj.hasOwnProperty(strid)) {
// this entity is new
obj[id] = {};
obj[strid] = {};
}

let mapper;

if (_.isArray(objSchema.columns)) {
// columns is just a list of field names
mapper = val => obj[id][val] = row[val];
mapper = val => obj[strid][val] = row[val];
} else {
// the columns object maps field names in the row to object key names
mapper = (val, key) => obj[id][val] = row[key];
mapper = (val, key) => obj[strid][val] = row[key];
}

_.map(objSchema.columns, mapper);
Expand All @@ -68,13 +73,13 @@ exports = module.exports = function (schema, data) {
switch (c) {
case 'pk': case 'columns': case 'array': break;
default: {
const descendant = build(obj[id][c] || {}, objSchema[c]);
const descendant = build(obj[strid][c] || {}, objSchema[c]);

if (descendant) {
obj[id][c] = descendant;
obj[strid][c] = descendant;
} else if (objSchema[c].array) {
// we always want an array if there could be multiple descendants
obj[id][c] = [];
obj[strid][c] = [];
}

break;
Expand Down
75 changes: 71 additions & 4 deletions test/queryable/decomposition.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ describe('decomposing results', function () {
id: 3,
username: 'carol',
tests: [{
id: 2,
name: 'carol\'s first test',
issues: []
}, {
id: 3,
name: 'carol\'s second test',
issues: [{
Expand All @@ -73,6 +69,10 @@ describe('decomposing results', function () {
test_id: 3,
description: 'carol\'s issue'
}]
}, {
id: 2,
name: 'carol\'s first test',
issues: []
}]
}]);
});
Expand Down Expand Up @@ -122,4 +122,71 @@ describe('decomposing results', function () {
});
});
});

it('preserves ordering of query results', function* () {
const issues = yield db.everything.find({}, {
decompose: {
pk: 'user_id',
columns: {
user_id: 'id',
username: 'username'
},
tests: {
pk: 'test_id',
columns: {
test_id: 'id',
name: 'name'
},
array: true,
issues: {
pk: 'id',
columns: {
id: 'id',
user_id: 'user_id',
test_id: 'test_id',
description: 'description'
},
array: true
}
}
},
order: 'username DESC'
});

assert.deepEqual(issues, [{
id: 3,
username: 'carol',
tests: [{
id: 3,
name: 'carol\'s second test',
issues: [{
id: 2,
user_id: 3,
test_id: 3,
description: 'carol\'s issue'
}]
}, {
id: 2,
name: 'carol\'s first test',
issues: []
}]
}, {
id: 2,
username: 'bob',
tests: []
}, {
id: 1,
username: 'alice',
tests: [{
id: 1,
name: 'alice\'s test',
issues: [{
id: 1,
user_id: 1,
test_id: 1,
description: 'alice\'s issue'
}]
}]
}]);
});
});

0 comments on commit d13bb2f

Please sign in to comment.