Skip to content

Commit 18ab904

Browse files
VincentMolinieVincentMolinie
authored
fix(smart field): fix filter on smart reference (#669)
1 parent 7ff4f35 commit 18ab904

File tree

4 files changed

+68
-195
lines changed

4 files changed

+68
-195
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"dependencies": {
2828
"@babel/runtime": "7.15.4",
2929
"bluebird": "2.9.25",
30-
"forest-express": "9.2.0",
30+
"forest-express": "9.2.5",
3131
"http-errors": "1.7.2",
3232
"lodash": "4.17.21",
3333
"moment": "2.24.0",

‎src/services/filters-parser.js‎

Lines changed: 12 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import _ from 'lodash';
2-
import Interface, { BaseFiltersParser, BaseOperatorDateParser } from 'forest-express';
2+
import Interface, { BaseFiltersParser, BaseOperatorDateParser, SchemaUtils } from 'forest-express';
33
import { NoMatchingOperatorError, InvalidFiltersFormatError } from './errors';
44
import utils from '../utils/schema';
55
import Flattener from './flattener';
@@ -34,57 +34,20 @@ function FiltersParser(model, timezone, options) {
3434
});
3535

3636
this.perform = async (filtersString) => BaseFiltersParser
37-
.perform(filtersString, this.formatAggregation, this.formatCondition);
37+
.perform(filtersString, this.formatAggregation, this.formatCondition, modelSchema);
3838

3939
this.formatAggregation = async (aggregator, formatedConditions) => {
4040
const aggregatorOperator = this.formatAggregatorOperator(aggregator);
4141
return { [aggregatorOperator]: formatedConditions };
4242
};
4343

44-
this._ensureIsValidCondition = (condition) => {
45-
if (_.isEmpty(condition)) {
46-
throw new InvalidFiltersFormatError('Empty condition in filter');
47-
}
48-
if (!_.isObject(condition)) {
49-
throw new InvalidFiltersFormatError('Condition cannot be a raw value');
50-
}
51-
if (_.isArray(condition)) {
52-
throw new InvalidFiltersFormatError('Filters cannot be a raw array');
53-
}
54-
if (!_.isString(condition.field)
55-
|| !_.isString(condition.operator)
56-
|| _.isUndefined(condition.value)) {
57-
throw new InvalidFiltersFormatError('Invalid condition format');
58-
}
59-
};
60-
61-
this.getSmartFieldCondition = async (condition) => {
62-
const fieldFound = modelSchema.fields.find((field) => field.field === condition.field);
63-
64-
if (!fieldFound.filter) {
65-
throw new Error(`"filter" method missing on smart field "${fieldFound.field}"`);
66-
}
67-
68-
const formattedCondition = await Promise.resolve(fieldFound
69-
.filter({
70-
where: await this.formatOperatorValue(
71-
condition.field,
72-
condition.operator,
73-
condition.value,
74-
),
75-
condition,
76-
}));
77-
if (!formattedCondition) {
78-
throw new Error(`"filter" method on smart field "${fieldFound.field}" must return a condition`);
79-
}
80-
return formattedCondition;
81-
};
82-
83-
this.formatCondition = async (condition) => {
84-
this._ensureIsValidCondition(condition);
85-
86-
if (this.isSmartField(modelSchema, condition.field)) {
87-
return this.getSmartFieldCondition(condition);
44+
this.formatCondition = async (condition, isSmartField = false) => {
45+
if (isSmartField) {
46+
return this.formatOperatorValue(
47+
condition.field,
48+
condition.operator,
49+
condition.value,
50+
);
8851
}
8952

9053
const formatedField = this.formatField(condition.field);
@@ -113,15 +76,15 @@ function FiltersParser(model, timezone, options) {
11376
const [fieldName, subfieldName] = key.split(':');
11477

11578
// NOTICE: Mongoose Aggregate don't parse the value automatically.
116-
let field = _.find(modelSchema.fields, { field: fieldName });
79+
let field = SchemaUtils.getField(modelSchema, fieldName);
11780

11881
if (!field) {
11982
throw new InvalidFiltersFormatError(`Field '${fieldName}' not found on collection '${modelSchema.name}'`);
12083
}
12184

12285
const isEmbeddedField = !!field.type.fields;
12386
if (isEmbeddedField) {
124-
field = _.find(field.type.fields, { field: subfieldName });
87+
field = SchemaUtils.getField(field.type, subfieldName);
12588
}
12689

12790
if (!field) return (val) => val;
@@ -183,11 +146,6 @@ function FiltersParser(model, timezone, options) {
183146

184147
this.formatField = (field) => field.replace(':', '.');
185148

186-
this.isSmartField = (schema, fieldName) => {
187-
const fieldFound = schema.fields.find((field) => field.field === fieldName);
188-
return !!fieldFound && !!fieldFound.isVirtual;
189-
};
190-
191149
this.getAssociations = async (filtersString) => BaseFiltersParser.getAssociations(filtersString);
192150

193151
this.formatAggregationForReferences = (aggregator, conditions) => ({ aggregator, conditions });
@@ -214,7 +172,7 @@ function FiltersParser(model, timezone, options) {
214172
}
215173

216174
// Mongoose Aggregate don't parse the value automatically.
217-
const field = _.find(modelSchema.fields, { field: fieldName });
175+
const field = SchemaUtils.getField(modelSchema, fieldName);
218176

219177
if (!field) {
220178
throw new InvalidFiltersFormatError(`Field '${fieldName}' not found on collection '${modelSchema.name}'`);

‎test/tests/services/filters-parser.test.js‎

Lines changed: 51 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import loadFixture from 'mongoose-fixture-loader';
33
import Interface from 'forest-express';
44
import FiltersParser from '../../../src/services/filters-parser';
55
import mongooseConnect from '../../utils/mongoose-connect';
6-
import { InvalidFiltersFormatError, NoMatchingOperatorError } from '../../../src/services/errors';
6+
import { NoMatchingOperatorError } from '../../../src/services/errors';
77

88
describe('service > filters-parser', () => {
99
let IslandModel;
@@ -38,9 +38,19 @@ describe('service > filters-parser', () => {
3838
comment: 'String',
3939
},
4040
},
41+
{
42+
field: 'ships',
43+
type: {
44+
fields: [{
45+
field: 'weapon',
46+
type: 'String',
47+
}],
48+
},
49+
},
4150
],
4251
};
4352

53+
4454
Interface.Schemas = {
4555
schemas: {
4656
Island: islandForestSchema,
@@ -92,6 +102,33 @@ describe('service > filters-parser', () => {
92102

93103
afterAll(() => mongoose.connection.close());
94104

105+
afterEach(() => jest.restoreAllMocks());
106+
107+
describe('getParserForField', () => {
108+
describe('with an embedded field', () => {
109+
it('should return the parser for the nested field', async () => {
110+
expect.assertions(6);
111+
112+
const fakeParser = jest.fn().mockReturnValue('parsedValue');
113+
const spy = jest.spyOn(defaultParser, 'getParserForType').mockReturnValue(fakeParser);
114+
115+
const parserForField = await defaultParser.getParserForField('ships:weapon');
116+
117+
expect(defaultParser.getParserForType).toHaveBeenCalledTimes(1);
118+
expect(defaultParser.getParserForType).toHaveBeenCalledWith('String');
119+
120+
expect(fakeParser).not.toHaveBeenCalled();
121+
122+
expect(parserForField('myValue')).toStrictEqual('parsedValue');
123+
124+
expect(fakeParser).toHaveBeenCalledTimes(1);
125+
expect(fakeParser).toHaveBeenCalledWith('myValue');
126+
127+
spy.mockRestore();
128+
});
129+
});
130+
});
131+
95132
describe('formatAggregation function', () => {
96133
describe('on aggregated conditions', () => {
97134
it('should format correctly', async () => {
@@ -134,106 +171,23 @@ describe('service > filters-parser', () => {
134171
expect(await defaultParser.formatCondition({ field: 'name', operator: 'in', value: 'Pyk, Dragonstone ' })).toStrictEqual({ name: { $in: ['Pyk', 'Dragonstone'] } });
135172
});
136173

137-
describe('on empty condition', () => {
138-
it('should throw an error', async () => {
139-
expect.assertions(2);
140-
await expect(defaultParser.formatCondition()).rejects.toThrow(InvalidFiltersFormatError);
141-
await expect(defaultParser.formatCondition({})).rejects.toThrow(InvalidFiltersFormatError);
142-
});
143-
});
144-
145-
describe('on badly formated condition', () => {
146-
it('should throw an error', async () => {
147-
expect.assertions(7);
148-
await expect(defaultParser.formatCondition({ operator: 'contains', value: 'it' })).rejects.toThrow(InvalidFiltersFormatError);
149-
await expect(defaultParser.formatCondition({ field: 'name', operator: 'contains' })).rejects.toThrow(InvalidFiltersFormatError);
150-
await expect(defaultParser.formatCondition({ field: 'name', value: 'it' })).rejects.toThrow(InvalidFiltersFormatError);
151-
await expect(defaultParser.formatCondition({ field: 'name', operator: 'con', value: 'it' })).rejects.toThrow(NoMatchingOperatorError);
152-
await expect(defaultParser.formatCondition('toto')).rejects.toThrow(InvalidFiltersFormatError);
153-
await expect(defaultParser.formatCondition(['toto'])).rejects.toThrow(InvalidFiltersFormatError);
154-
await expect(defaultParser.formatCondition({ field: 'toto', operator: 'contains', value: 'it' })).rejects.toThrow(InvalidFiltersFormatError);
155-
});
156-
});
157-
158174
describe('on a smart field', () => {
159-
describe('with filter method not defined', () => {
160-
it('should throw an error', async () => {
161-
expect.assertions(1);
162-
163-
const oldFields = islandForestSchema.fields;
164-
islandForestSchema.fields = [{
165-
field: 'smart name',
166-
type: 'String',
167-
isVirtual: true,
168-
get() {},
169-
}];
170-
171-
await expect(defaultParser.formatCondition({
172-
field: 'smart name',
173-
operator: 'present',
174-
value: null,
175-
})).rejects.toThrow('"filter" method missing on smart field "smart name"');
176-
177-
islandForestSchema.fields = oldFields;
178-
});
179-
});
180-
181-
describe('with filter method defined', () => {
182-
describe('when filter method return null or undefined', () => {
183-
it('should throw an error', async () => {
184-
expect.assertions(1);
175+
it('should call formatOperatorValue', async () => {
176+
expect.assertions(3);
185177

186-
const oldFields = islandForestSchema.fields;
187-
islandForestSchema.fields = [{
188-
field: 'smart name',
189-
type: 'String',
190-
isVirtual: true,
191-
get() {},
192-
filter() {},
193-
}];
194-
195-
await expect(defaultParser.formatCondition({
196-
field: 'smart name',
197-
operator: 'present',
198-
value: null,
199-
})).rejects.toThrow('"filter" method on smart field "smart name" must return a condition');
200-
201-
islandForestSchema.fields = oldFields;
202-
});
203-
});
178+
const formattedCondition = 'myFormattedCondition';
179+
jest.spyOn(defaultParser, 'formatOperatorValue').mockReturnValue(formattedCondition);
204180

205-
describe('when filter method return a condition', () => {
206-
it('should return the condition', async () => {
207-
expect.assertions(4);
181+
const condition = {
182+
field: 'smart name',
183+
operator: 'present',
184+
value: null,
185+
};
186+
expect(await defaultParser.formatCondition(condition, true))
187+
.toStrictEqual(formattedCondition);
208188

209-
const where = { id: 1 };
210-
const oldFields = islandForestSchema.fields;
211-
islandForestSchema.fields = [{
212-
field: 'smart name',
213-
type: 'String',
214-
isVirtual: true,
215-
get() {},
216-
filter: jest.fn(() => where),
217-
}];
218-
219-
const condition = {
220-
field: 'smart name',
221-
operator: 'present',
222-
value: null,
223-
};
224-
expect(await defaultParser.formatCondition(condition)).toStrictEqual(where);
225-
expect(islandForestSchema.fields[0].filter.mock.calls).toHaveLength(1);
226-
expect(islandForestSchema.fields[0].filter.mock.calls[0]).toHaveLength(1);
227-
expect(islandForestSchema.fields[0].filter.mock.calls[0][0]).toStrictEqual({
228-
where: {
229-
$exists: true,
230-
$ne: null,
231-
},
232-
condition,
233-
});
234-
islandForestSchema.fields = oldFields;
235-
});
236-
});
189+
expect(defaultParser.formatOperatorValue).toHaveBeenCalledTimes(1);
190+
expect(defaultParser.formatOperatorValue).toHaveBeenCalledWith('smart name', 'present', null);
237191
});
238192
});
239193
});
@@ -286,45 +240,6 @@ describe('service > filters-parser', () => {
286240
});
287241
});
288242

289-
describe('isSmartField', () => {
290-
describe('on a unknown field', () => {
291-
it('should return false', () => {
292-
expect.assertions(1);
293-
const schemaToTest = { fields: [] };
294-
295-
expect(defaultParser.isSmartField(schemaToTest, 'unknown')).toBeFalse();
296-
});
297-
});
298-
299-
describe('on a non smart field', () => {
300-
it('should return false', () => {
301-
expect.assertions(1);
302-
const schemaToTest = {
303-
fields: [{
304-
field: 'name',
305-
isVirtual: false,
306-
}],
307-
};
308-
309-
expect(defaultParser.isSmartField(schemaToTest, 'name')).toBeFalse();
310-
});
311-
});
312-
313-
describe('on a smart field', () => {
314-
it('should return true', () => {
315-
expect.assertions(1);
316-
const schemaToTest = {
317-
fields: [{
318-
field: 'name',
319-
isVirtual: true,
320-
}],
321-
};
322-
323-
expect(defaultParser.isSmartField(schemaToTest, 'name')).toBeTrue();
324-
});
325-
});
326-
});
327-
328243
describe('formatField function', () => {
329244
it('should format default field correctly', () => {
330245
expect.assertions(1);

‎yarn.lock‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4812,10 +4812,10 @@ for-in@^1.0.2:
48124812
resolved "https://registry.yarnpkg.com/for-in/-/for-in-1.0.2.tgz#81068d295a8142ec0ac726c6e2200c30fb6d5e80"
48134813
integrity sha1-gQaNKVqBQuwKxybG4iAMMPttXoA=
48144814

4815-
[email protected].0:
4816-
version "9.2.0"
4817-
resolved "https://registry.yarnpkg.com/forest-express/-/forest-express-9.2.0.tgz#c7e22b66d93536984a4197c09067b97dc20f5e8f"
4818-
integrity sha512-e9LCoplxXCsLuznLShuMT+oyxBA/+6R+Ib1sazpg4U8/Xe37AgBt2XKefsc1cyUh5EATJKQCNXm95518dij6Ew==
4815+
[email protected].5:
4816+
version "9.2.5"
4817+
resolved "https://registry.yarnpkg.com/forest-express/-/forest-express-9.2.5.tgz#3364f7f185227ec9ef200ec7e33d87c229219c9e"
4818+
integrity sha512-vxqGRs9/lL+RwOSvpfPeKn0VSqy2wCJ4MY2kzL4DjirrBaOTHz8dsV2H0qIpxMMVQ8FyUDBhHH3fWsT+drWrgg==
48194819
dependencies:
48204820
"@babel/runtime" "7.10.1"
48214821
base32-encode "1.1.1"

0 commit comments

Comments
 (0)