Skip to content

Commit 3bc76c6

Browse files
daffldaffl
committed
fix(adapter-commons): Clean up semantics for adapter operators and filters
BREAKING CHANGE: filterQuery will now more stricly separate between top level filters and query property level operators.
1 parent 3b2ca0a commit 3bc76c6

File tree

6 files changed

+196
-113
lines changed

6 files changed

+196
-113
lines changed

‎packages/adapter-commons/src/declarations.ts‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
import { Query, Params, Paginated, Id, NullableId } from '@feathersjs/feathers';
22

3-
export type FilterSettings = string[]|{
4-
[key: string]: (value: any, options: any) => any
3+
export type FilterQueryOptions = {
4+
filters?: FilterSettings;
5+
operators?: string[];
6+
paginate?: PaginationParams;
7+
}
8+
9+
export type QueryFilter = (value: any, options: FilterQueryOptions) => any
10+
11+
export type FilterSettings = {
12+
[key: string]: QueryFilter|true
513
}
614

715
export interface PaginationOptions {
@@ -11,26 +19,39 @@ export interface PaginationOptions {
1119

1220
export type PaginationParams = false|PaginationOptions;
1321

14-
export type FilterQueryOptions = {
15-
filters?: FilterSettings;
16-
operators?: string[];
17-
paginate?: PaginationParams;
18-
}
19-
2022
export interface AdapterServiceOptions {
21-
events?: string[];
23+
/**
24+
* Whether to allow multiple updates for everything (`true`) or specific methods (e.g. `['create', 'remove']`)
25+
*/
2226
multi?: boolean|string[];
27+
/**
28+
* The name of the id property
29+
*/
2330
id?: string;
31+
/**
32+
* Pagination settings for this service
33+
*/
2434
paginate?: PaginationOptions
2535
/**
26-
* @deprecated renamed to `allow`.
36+
* A list of additional property query operators to allow in a query
37+
*/
38+
operators?: string[];
39+
/**
40+
* An object of additional top level query filters, e.g. `{ $populate: true }`
41+
* Can also be a converter function like `{ $ignoreCase: (value) => value === 'true' ? true : false }`
42+
*/
43+
filters?: FilterSettings;
44+
/**
45+
* @deprecated Use service `events` option when registering the service with `app.use`.
2746
*/
47+
events?: string[];
48+
/**
49+
* @deprecated renamed to `operators`.
50+
*/
2851
whitelist?: string[];
29-
allow?: string[];
30-
filters?: string[];
3152
}
3253

33-
export interface AdapterOptions<M = any> extends Pick<AdapterServiceOptions, 'multi'|'allow'|'paginate'> {
54+
export interface AdapterOptions<M = any> extends Pick<AdapterServiceOptions, 'multi'|'filters'|'operators'|'paginate'> {
3455
Model?: M;
3556
}
3657

‎packages/adapter-commons/src/filter-query.ts‎

Lines changed: 103 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -3,115 +3,135 @@ import { BadRequest } from '@feathersjs/errors';
33
import { Query } from '@feathersjs/feathers';
44
import { FilterQueryOptions, FilterSettings } from './declarations';
55

6-
function parse (number: any) {
7-
if (typeof number !== 'undefined') {
8-
return Math.abs(parseInt(number, 10));
9-
}
10-
11-
return undefined;
12-
}
6+
const parse = (value: any) => typeof value !== 'undefined' ? parseInt(value, 10) : value;
137

14-
// Returns the pagination limit and will take into account the
15-
// default and max pagination settings
16-
function getLimit (limit: any, paginate: any) {
17-
if (paginate && (paginate.default || paginate.max)) {
18-
const base = paginate.default || 0;
19-
const lower = typeof limit === 'number' && !isNaN(limit) ? limit : base;
20-
const upper = typeof paginate.max === 'number' ? paginate.max : Number.MAX_VALUE;
8+
const isPlainObject = (value: any) => _.isObject(value) && value.constructor === {}.constructor;
219

22-
return Math.min(lower, upper);
10+
const validateQueryProperty = (query: any, operators: string[] = []): Query => {
11+
if (!isPlainObject(query)) {
12+
return query;
2313
}
2414

25-
return limit;
26-
}
15+
for (const key of Object.keys(query)) {
16+
if (key.startsWith('$') && !operators.includes(key)) {
17+
throw new BadRequest(`Invalid query parameter ${key}`, query);
18+
}
2719

28-
// Makes sure that $sort order is always converted to an actual number
29-
function convertSort (sort: any) {
30-
if (typeof sort !== 'object' || Array.isArray(sort)) {
31-
return sort;
32-
}
20+
const value = query[key];
3321

34-
return Object.keys(sort).reduce((result, key) => {
35-
result[key] = typeof sort[key] === 'object'
36-
? sort[key] : parseInt(sort[key], 10);
22+
if (isPlainObject(value)) {
23+
query[key] = validateQueryProperty(value, operators);
24+
}
25+
}
3726

38-
return result;
39-
}, {} as { [key: string]: number });
27+
return {
28+
...query
29+
}
4030
}
4131

42-
function cleanQuery (query: Query, operators: any, filters: any): any {
43-
if (Array.isArray(query)) {
44-
return query.map(value => cleanQuery(value, operators, filters));
45-
} else if (_.isObject(query) && query.constructor === {}.constructor) {
46-
const result: { [key: string]: any } = {};
47-
48-
_.each(query, (value, key) => {
49-
if (key[0] === '$') {
50-
if (filters[key] !== undefined) {
51-
return;
52-
}
53-
54-
if (!operators.includes(key)) {
55-
throw new BadRequest(`Invalid query parameter ${key}`, query);
56-
}
57-
}
32+
const getFilters = (query: Query, settings: FilterQueryOptions) => {
33+
const filterNames = Object.keys(settings.filters);
5834

59-
result[key] = cleanQuery(value, operators, filters);
60-
});
35+
return filterNames.reduce((current, key) => {
36+
const queryValue = query[key];
37+
const filter = settings.filters[key];
6138

62-
Object.getOwnPropertySymbols(query).forEach(symbol => {
63-
// @ts-ignore
64-
result[symbol] = query[symbol];
65-
});
39+
if (filter) {
40+
const value = typeof filter === 'function' ? filter(queryValue, settings) : queryValue;
6641

67-
return result;
68-
}
42+
if (value !== undefined) {
43+
current[key] = value;
44+
}
45+
}
6946

70-
return query;
47+
return current;
48+
}, {} as { [key: string]: any });
7149
}
7250

73-
function assignFilters (object: any, query: Query, filters: FilterSettings, options: any): { [key: string]: any } {
74-
if (Array.isArray(filters)) {
75-
_.each(filters, (key) => {
76-
if (query[key] !== undefined) {
77-
object[key] = query[key];
78-
}
79-
});
80-
} else {
81-
_.each(filters, (converter, key) => {
82-
const converted = converter(query[key], options);
51+
const getQuery = (query: Query, settings: FilterQueryOptions) => {
52+
const keys = Object.keys(query).concat(Object.getOwnPropertySymbols(query) as any as string[]);
8353

84-
if (converted !== undefined) {
85-
object[key] = converted;
54+
return keys.reduce((result, key) => {
55+
if (typeof key === 'string' && key.startsWith('$')) {
56+
if (settings.filters[key] === undefined) {
57+
throw new BadRequest(`Invalid filter value ${key}`);
8658
}
87-
});
88-
}
59+
} else {
60+
result[key] = validateQueryProperty(query[key], settings.operators);
61+
}
8962

90-
return object;
63+
return result;
64+
}, {} as Query)
9165
}
9266

67+
export const OPERATORS = ['$in', '$nin', '$lt', '$lte', '$gt', '$gte', '$ne', '$or'];
68+
9369
export const FILTERS: FilterSettings = {
94-
$sort: (value: any) => convertSort(value),
95-
$limit: (value: any, options: any) => getLimit(parse(value), options.paginate),
9670
$skip: (value: any) => parse(value),
97-
$select: (value: any) => value
71+
$sort: (sort: any): { [key: string]: number } => {
72+
if (typeof sort !== 'object' || Array.isArray(sort)) {
73+
return sort;
74+
}
75+
76+
return Object.keys(sort).reduce((result, key) => {
77+
result[key] = typeof sort[key] === 'object'
78+
? sort[key] : parse(sort[key]);
79+
80+
return result;
81+
}, {} as { [key: string]: number });
82+
},
83+
$limit: (_limit: any, { paginate }: FilterQueryOptions) => {
84+
const limit = parse(_limit);
85+
86+
if (paginate && (paginate.default || paginate.max)) {
87+
const base = paginate.default || 0;
88+
const lower = typeof limit === 'number' && !isNaN(limit) && limit >= 0 ? limit : base;
89+
const upper = typeof paginate.max === 'number' ? paginate.max : Number.MAX_VALUE;
90+
91+
return Math.min(lower, upper);
92+
}
93+
94+
return limit;
95+
},
96+
$select: (select: any) => {
97+
if (Array.isArray(select)) {
98+
return select.map(current => `${current}`);
99+
}
100+
101+
return select;
102+
},
103+
$or: (or: any, { operators }: FilterQueryOptions) => {
104+
if (Array.isArray(or)) {
105+
return or.map(current => validateQueryProperty(current, operators));
106+
}
107+
108+
return or;
109+
}
98110
};
99111

100-
export const OPERATORS = ['$in', '$nin', '$lt', '$lte', '$gt', '$gte', '$ne', '$or'];
101-
102-
// Converts Feathers special query parameters and pagination settings
103-
// and returns them separately a `filters` and the rest of the query
104-
// as `query`
112+
/**
113+
* Converts Feathers special query parameters and pagination settings
114+
* and returns them separately as `filters` and the rest of the query
115+
* as `query`. `options` also gets passed the pagination settings and
116+
* a list of additional `operators` to allow when querying properties.
117+
*
118+
* @param query The initial query
119+
* @param options Options for filtering the query
120+
* @returns An object with `query` which contains the query without `filters`
121+
* and `filters` which contains the converted values for each filter.
122+
*/
105123
export function filterQuery (query: Query, options: FilterQueryOptions = {}) {
106-
const {
107-
filters: additionalFilters = [],
108-
operators: additionalOperators = []
109-
} = options;
110-
const baseFilters = assignFilters({}, query, FILTERS, options);
111-
const filters = assignFilters(baseFilters, query, additionalFilters, options);
124+
const settings = {
125+
...options,
126+
filters: {
127+
...FILTERS,
128+
...options.filters
129+
},
130+
operators: OPERATORS.concat(options.operators || [])
131+
}
112132

113133
return {
114-
filters,
115-
query: cleanQuery(query, OPERATORS.concat(additionalOperators), filters) as Query
134+
filters: getFilters(query, settings),
135+
query: getQuery(query, settings)
116136
}
117137
}

‎packages/adapter-commons/src/service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export class AdapterBase<O extends Partial<AdapterServiceOptions> = Partial<Adap
2929
events: [],
3030
paginate: false,
3131
multi: false,
32-
filters: [],
32+
filters: {},
3333
allow: []
3434
}, options);
3535
}
@@ -48,7 +48,7 @@ export class AdapterBase<O extends Partial<AdapterServiceOptions> = Partial<Adap
4848
: this.getOptions(params).paginate;
4949
const query: Query = { ...params.query };
5050
const options = {
51-
operators: this.options.whitelist || this.options.allow || [],
51+
operators: this.options.whitelist || this.options.operators || [],
5252
filters: this.options.filters,
5353
paginate,
5454
...opts

‎packages/adapter-commons/test/filter-query.test.ts‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe('@feathersjs/adapter-commons/filterQuery', () => {
4444
assert.ok(false, 'Should never get here');
4545
} catch (error: any) {
4646
assert.strictEqual(error.name, 'BadRequest');
47-
assert.strictEqual(error.message, 'Invalid query parameter $foo');
47+
assert.strictEqual(error.message, 'Invalid filter value $foo');
4848
}
4949
});
5050

@@ -96,16 +96,16 @@ describe('@feathersjs/adapter-commons/filterQuery', () => {
9696
describe('pagination', () => {
9797
it('limits with default pagination', () => {
9898
const { filters } = filterQuery({}, { paginate: { default: 10 } });
99+
const { filters: filtersNeg } = filterQuery({ $limit: -20 }, { paginate: { default: 5, max: 10 } });
99100

100101
assert.strictEqual(filters.$limit, 10);
102+
assert.strictEqual(filtersNeg.$limit, 5);
101103
});
102104

103105
it('limits with max pagination', () => {
104106
const { filters } = filterQuery({ $limit: 20 }, { paginate: { default: 5, max: 10 } });
105-
const { filters: filtersNeg } = filterQuery({ $limit: -20 }, { paginate: { default: 5, max: 10 } });
106107

107108
assert.strictEqual(filters.$limit, 10);
108-
assert.strictEqual(filtersNeg.$limit, 10);
109109
});
110110

111111
it('limits with default pagination when not a number', () => {
@@ -223,6 +223,16 @@ describe('@feathersjs/adapter-commons/filterQuery', () => {
223223
message: 'Invalid query parameter $exists'
224224
});
225225
});
226+
227+
it('allows default operators in $or', () => {
228+
const { filters } = filterQuery({
229+
$or: [{ value: { $gte: 10 } }]
230+
});
231+
232+
assert.deepStrictEqual(filters, {
233+
$or: [{ value: { $gte: 10 } }]
234+
});
235+
});
226236
});
227237

228238
describe('additional filters', () => {
@@ -231,13 +241,18 @@ describe('@feathersjs/adapter-commons/filterQuery', () => {
231241
filterQuery({ $select: 1, $known: 1 });
232242
assert.ok(false, 'Should never get here');
233243
} catch (error: any) {
234-
assert.strictEqual(error.message, 'Invalid query parameter $known');
244+
assert.strictEqual(error.message, 'Invalid filter value $known');
235245
}
236246
});
237247

238248
it('returns default and known additional filters (array)', () => {
239249
const query = { $select: ['a', 'b'], $known: 1, $unknown: 1 };
240-
const { filters } = filterQuery(query, { filters: [ '$known', '$unknown' ] });
250+
const { filters } = filterQuery(query, {
251+
filters: {
252+
$known: true,
253+
$unknown: true
254+
}
255+
});
241256

242257
assert.strictEqual(filters.$unknown, 1);
243258
assert.strictEqual(filters.$known, 1);
@@ -259,12 +274,18 @@ describe('@feathersjs/adapter-commons/filterQuery', () => {
259274
describe('additional operators', () => {
260275
it('returns query with default and known additional operators', () => {
261276
const { query } = filterQuery({
262-
$ne: 1, $known: 1
277+
prop: { $ne: 1, $known: 1 }
263278
}, { operators: [ '$known' ] });
264279

265-
assert.strictEqual(query.$ne, 1);
266-
assert.strictEqual(query.$known, 1);
267-
assert.strictEqual(query.$unknown, undefined);
280+
assert.deepStrictEqual(query, { prop: { '$ne': 1, '$known': 1 } });
281+
});
282+
283+
it('throws an error with unknown query operator', () => {
284+
assert.throws(() => filterQuery({
285+
prop: { $unknown: 'something' }
286+
}), {
287+
message: 'Invalid query parameter $unknown'
288+
});
268289
});
269290
});
270291
});

0 commit comments

Comments
 (0)