Skip to content

Commit a04be5c

Browse files
fix(retrocompatibility): make test suite pass with [email protected] (#761)
1 parent bc7ace7 commit a04be5c

File tree

10 files changed

+415
-100
lines changed

10 files changed

+415
-100
lines changed

src/services/has-many-getter.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { pick } from 'lodash';
2-
import Operators from '../utils/operators';
3-
import QueryUtils from '../utils/query';
2+
import SequelizeCompatibility from '../utils/sequelize-compatibility';
43
import PrimaryKeysManager from './primary-keys-manager';
54
import ResourcesGetter from './resources-getter';
65

@@ -23,13 +22,12 @@ class HasManyGetter extends ResourcesGetter {
2322
}
2423

2524
async _buildQueryOptions(buildOptions = {}) {
26-
const operators = Operators.getInstance({ Sequelize: this._parentModel.sequelize.constructor });
2725
const { associationName, recordId } = this._params;
2826
const [model, options] = await super._buildQueryOptions({
2927
...buildOptions, tableAlias: associationName,
3028
});
3129

32-
const parentOptions = QueryUtils.bubbleWheresInPlace(operators, {
30+
const parentOptions = SequelizeCompatibility.postProcess(this._parentModel, {
3331
where: new PrimaryKeysManager(this._parentModel).getRecordsConditions([recordId]),
3432
include: [{
3533
model,

src/services/pie-stat-getter.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ import { Schemas } from 'forest-express';
22
import _ from 'lodash';
33
import moment from 'moment';
44
import { isMSSQL } from '../utils/database';
5-
import Orm, { isVersionLessThan4 } from '../utils/orm';
5+
import Orm, { isVersionLessThan } from '../utils/orm';
66
import QueryOptions from './query-options';
77

88
// NOTICE: These aliases are not camelcased to prevent issues with Sequelize.
99
const ALIAS_GROUP_BY = 'forest_alias_groupby';
1010
const ALIAS_AGGREGATE = 'forest_alias_aggregate';
1111

1212
function PieStatGetter(model, params, options) {
13-
const needsDateOnlyFormating = isVersionLessThan4(options.Sequelize);
13+
const needsDateOnlyFormating = isVersionLessThan(options.Sequelize, '4.0.0');
1414

1515
const schema = Schemas.schemas[model.name];
1616
let associationSplit;

src/services/query-options.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { logger, Schemas } from 'forest-express';
22
import _ from 'lodash';
3+
import { isMSSQL } from '../utils/database';
34
import Operators from '../utils/operators';
5+
import QueryUtils from '../utils/query';
6+
import SequelizeCompatibility from '../utils/sequelize-compatibility';
47
import { ErrorHTTP422 } from './errors';
58
import FiltersParser from './filters-parser';
69
import LiveQueryChecker from './live-query-checker';
710
import PrimaryKeysManager from './primary-keys-manager';
811
import QueryBuilder from './query-builder';
912
import SearchBuilder from './search-builder';
10-
import QueryUtils from '../utils/query';
11-
import { isMSSQL } from '../utils/database';
1213

1314
/**
1415
* Sequelize query options generator which is configured using forest admin concepts (filters,
@@ -30,7 +31,7 @@ class QueryOptions {
3031
options.limit = this._limit;
3132
}
3233

33-
return options;
34+
return SequelizeCompatibility.postProcess(this._model, options);
3435
}
3536

3637
/**

src/services/value-stat-getter.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ class ValueStatGetter {
5858
.unscoped()
5959
.aggregate(this._aggregateField, this._aggregateFunction, options);
6060

61-
return count ?? 0;
61+
// sequelize@4 returns NaN, while sequelize@5+ returns null
62+
return count || 0;
6263
}
6364

6465
/**

src/utils/orm.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ const getVersion = (sequelize) => {
1010
return null;
1111
};
1212

13-
const isVersionLessThan4 = (sequelize) => {
13+
const isVersionLessThan = (sequelize, target) => {
1414
try {
15-
return semver.lt(getVersion(sequelize), '4.0.0');
15+
return semver.lt(getVersion(sequelize), target);
1616
} catch (error) {
1717
return true;
1818
}
@@ -36,7 +36,7 @@ const isUUID = (DataTypes, fieldType) =>
3636
|| fieldType instanceof DataTypes.UUIDV4;
3737

3838
exports.getVersion = getVersion;
39-
exports.isVersionLessThan4 = isVersionLessThan4;
39+
exports.isVersionLessThan = isVersionLessThan;
4040
exports.findRecord = findRecord;
4141
exports.getColumnName = getColumnName;
4242
exports.isUUID = isUUID;

src/utils/query.js

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import Orm from './orm';
21
import ObjectTools from './object-tools';
2+
import Orm from './orm';
33

44
exports.getReferenceSchema = (schemas, modelSchema, associationName) => {
55
const schemaField = modelSchema.fields.find((field) => field.field === associationName);
@@ -33,34 +33,3 @@ exports.mergeWhere = (operators, ...wheres) => wheres
3333
.reduce((where1, where2) => (ObjectTools.plainObjectsShareNoKeys(where1, where2)
3434
? { ...where1, ...where2 }
3535
: { [operators.AND]: [where1, where2] }));
36-
37-
/**
38-
* Extract all where conditions along the include tree, and bubbles them up to the top.
39-
* This allows to work around a sequelize quirk that cause nested 'where' to fail when they
40-
* refer to relation fields from an intermediary include (ie '$book.id$').
41-
*
42-
* This happens when forest admin filters on relations are used.
43-
*
44-
* @see https://sequelize.org/master/manual/eager-loading.html#complex-where-clauses-at-the-top-level
45-
* @see https://github.com/ForestAdmin/forest-express-sequelize/blob/7d7ad0/src/services/filters-parser.js#L104
46-
*/
47-
exports.bubbleWheresInPlace = (operators, options) => {
48-
const parentInclude = options.include ?? [];
49-
50-
parentInclude.forEach((include) => {
51-
exports.bubbleWheresInPlace(operators, include);
52-
53-
if (include.where) {
54-
const newWhere = ObjectTools.mapKeysDeep(include.where, (key) => (
55-
key[0] === '$' && key[key.length - 1] === '$'
56-
? `$${include.as}.${key.substring(1)}`
57-
: `$${include.as}.${key}$`
58-
));
59-
60-
options.where = exports.mergeWhere(operators, options.where, newWhere);
61-
delete include.where;
62-
}
63-
});
64-
65-
return options;
66-
};
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
import ObjectTools from './object-tools';
2+
import Operators from './operators';
3+
import { isVersionLessThan } from './orm';
4+
import QueryUtils from './query';
5+
6+
/**
7+
* Extract all where conditions along the include tree, and bubbles them up to the top in-place.
8+
* This allows to work around a sequelize quirk that cause nested 'where' to fail when they
9+
* refer to relation fields from an intermediary include (ie '$book.id$').
10+
*
11+
* This happens when forest admin filters on relations are used.
12+
*
13+
* @see https://sequelize.org/master/manual/eager-loading.html#complex-where-clauses-at-the-top-level
14+
* @see https://github.com/ForestAdmin/forest-express-sequelize/blob/7d7ad0/src/services/filters-parser.js#L104
15+
*/
16+
function bubbleWheresInPlace(operators, options) {
17+
const parentIncludeList = options.include ?? [];
18+
19+
parentIncludeList.forEach((include) => {
20+
bubbleWheresInPlace(operators, include);
21+
22+
if (include.where) {
23+
const newWhere = ObjectTools.mapKeysDeep(include.where, (key) => (
24+
key[0] === '$' && key[key.length - 1] === '$'
25+
? `$${include.as}.${key.substring(1)}`
26+
: `$${include.as}.${key}$`
27+
));
28+
29+
options.where = QueryUtils.mergeWhere(operators, options.where, newWhere);
30+
delete include.where;
31+
}
32+
});
33+
}
34+
35+
/**
36+
* Includes can be expressed in different ways in sequelize, which is inconvenient to
37+
* remove duplicate associations.
38+
* This convert all valid ways to perform eager loading into [{model: X, as: 'x'}].
39+
*
40+
* This is necessary as we have no control over which way customer use when writing SmartFields
41+
* search handlers.
42+
*
43+
* Among those:
44+
* - { include: [Book] }
45+
* - { include: [{ association: 'book' }] }
46+
* - { include: ['book'] }
47+
* - { include: [[{ as: 'book' }]] }
48+
* - { include: [[{ model: Book }]] }
49+
*/
50+
function normalizeInclude(model, include) {
51+
if (include.sequelize) {
52+
return {
53+
model: include,
54+
as: Object
55+
.keys(model.associations)
56+
.find((association) => model.associations[association].target.name === include.name),
57+
};
58+
}
59+
60+
if (typeof include === 'string' && model.associations[include]) {
61+
return { as: include, model: model.associations[include].target };
62+
}
63+
64+
if (typeof include === 'object') {
65+
if (typeof include.association === 'string' && model.associations[include.association]) {
66+
include.as = include.association;
67+
delete include.association;
68+
}
69+
70+
if (typeof include.as === 'string' && !include.model && model.associations[include.as]) {
71+
const includeModel = model.associations[include.as].target;
72+
include.model = includeModel;
73+
}
74+
75+
if (include.model && !include.as) {
76+
include.as = Object
77+
.keys(model.associations)
78+
.find((association) => model.associations[association].target.name === include.model.name);
79+
}
80+
}
81+
82+
// Recurse
83+
if (include.include) {
84+
include.include = include.include.map(
85+
(childInclude) => normalizeInclude(include.model, childInclude),
86+
);
87+
}
88+
89+
return include;
90+
}
91+
92+
/**
93+
* Remove duplications in a queryOption.include array in-place.
94+
* Using multiple times the same association yields invalid SQL when using sequelize <= 4.x
95+
*/
96+
function removeDuplicateAssociations(model, includeList) {
97+
// Remove duplicates
98+
includeList.sort((include1, include2) => (include1.as < include2.as ? -1 : 1));
99+
for (let i = 1; i < includeList.length; i += 1) {
100+
if (includeList[i - 1].as === includeList[i].as) {
101+
const newInclude = { ...includeList[i - 1], ...includeList[i] };
102+
103+
if (includeList[i - 1].attributes && includeList[i].attributes) {
104+
// Keep 'attributes' only when defined on both sides.
105+
newInclude.attributes = [...new Set([
106+
...includeList[i - 1].attributes,
107+
...includeList[i].attributes,
108+
])].sort();
109+
} else {
110+
delete newInclude.attributes;
111+
}
112+
113+
if (includeList[i - 1].include || includeList[i].include) {
114+
newInclude.include = [
115+
...(includeList[i - 1].include ?? []),
116+
...(includeList[i].include ?? []),
117+
];
118+
}
119+
120+
includeList[i - 1] = newInclude;
121+
includeList.splice(i, 1);
122+
i -= 1;
123+
}
124+
}
125+
126+
// Recurse
127+
includeList.forEach((include) => {
128+
if (include.include) {
129+
const association = model.associations[include.as];
130+
removeDuplicateAssociations(association.target, include.include);
131+
}
132+
});
133+
}
134+
135+
exports.postProcess = (model, rawOptions) => {
136+
if (!rawOptions.include) return rawOptions;
137+
138+
const options = rawOptions;
139+
const operators = Operators.getInstance({ Sequelize: model.sequelize.constructor });
140+
141+
if (isVersionLessThan(model.sequelize.constructor, '5.0.0')) {
142+
options.include = options.include.map((include) => normalizeInclude(model, include));
143+
bubbleWheresInPlace(operators, options);
144+
removeDuplicateAssociations(model, options.include);
145+
} else {
146+
bubbleWheresInPlace(operators, options);
147+
}
148+
149+
return options;
150+
};

test/services/has-many-getter.test.js

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,20 @@ describe('services > HasManyGetter', () => {
1616

1717
describe('_buildQueryOptions', () => {
1818
const options = { tableAlias: 'users' };
19-
const model = {
20-
name: 'cars',
21-
unscoped: () => model,
22-
sequelize: sequelizePostgres.connection,
23-
primaryKeys: { id: {} },
24-
};
25-
const association = {
19+
const UserModel = {
2620
name: 'users',
2721
rawAttributes: [{ field: 'name', type: 'String' }],
2822
sequelize: sequelizePostgres.connection,
29-
unscoped: () => association,
23+
unscoped: () => UserModel,
3024
associations: { },
3125
};
26+
const CarModel = {
27+
name: 'cars',
28+
unscoped: () => CarModel,
29+
sequelize: sequelizePostgres.connection,
30+
primaryKeys: { id: {} },
31+
associations: { users: { target: UserModel } },
32+
};
3233
Interface.Schemas = {
3334
schemas: {
3435
users: { fields: [{ field: 'name', type: 'String', columnName: 'name' }] },
@@ -40,7 +41,7 @@ describe('services > HasManyGetter', () => {
4041
it('should build an empty where condition', async () => {
4142
expect.assertions(1);
4243

43-
const hasManyGetter = new HasManyGetter(model, association, lianaOptions, baseParams);
44+
const hasManyGetter = new HasManyGetter(CarModel, UserModel, lianaOptions, baseParams);
4445
const queryOptions = await hasManyGetter._buildQueryOptions(options);
4546

4647
expect(queryOptions.where).toStrictEqual({ id: 1 });
@@ -55,7 +56,7 @@ describe('services > HasManyGetter', () => {
5556
...baseParams,
5657
filters: '{ "field": "id", "operator": "greater_than", "value": 1 }',
5758
};
58-
const hasManyGetter = new HasManyGetter(model, association, lianaOptions, params);
59+
const hasManyGetter = new HasManyGetter(CarModel, UserModel, lianaOptions, params);
5960
const queryOptions = await hasManyGetter._buildQueryOptions(options);
6061

6162
expect(queryOptions.where).toStrictEqual({
@@ -70,7 +71,7 @@ describe('services > HasManyGetter', () => {
7071
expect.assertions(1);
7172

7273
const params = { ...baseParams, search: 'test' };
73-
const hasManyGetter = new HasManyGetter(model, association, lianaOptions, params);
74+
const hasManyGetter = new HasManyGetter(CarModel, UserModel, lianaOptions, params);
7475
const queryOptions = await hasManyGetter._buildQueryOptions(options);
7576

7677
expect(queryOptions.where).toStrictEqual({
@@ -95,7 +96,7 @@ describe('services > HasManyGetter', () => {
9596
filters: '{ "field": "id", "operator": "greater_than", "value": 1 }',
9697
search: 'test',
9798
};
98-
const hasManyGetter = new HasManyGetter(model, association, lianaOptions, params);
99+
const hasManyGetter = new HasManyGetter(CarModel, UserModel, lianaOptions, params);
99100
const queryOptions = await hasManyGetter._buildQueryOptions(options);
100101

101102
expect(queryOptions.where).toStrictEqual({

0 commit comments

Comments
 (0)