Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 7bd2a58

Browse files
gkalpakgkalpak
committed
fix(filterFilter): correctly handle deep expression objects
Previously, trying to use a deep expression object (i.e. an object whose properties can be objects themselves) did not work correctly. This commit refactors `filterFilter`, making it simpler and adding support for filtering collections of arbitrarily deep objects. Closes #9698
1 parent e780eee commit 7bd2a58

File tree

2 files changed

+136
-96
lines changed

2 files changed

+136
-96
lines changed

‎src/ng/filter/filter.js‎

Lines changed: 68 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -119,104 +119,85 @@ function filterFilter() {
119119
return function(array, expression, comparator) {
120120
if (!isArray(array)) return array;
121121

122-
var comparatorType = typeof(comparator),
123-
predicates = [];
124-
125-
predicates.check = function(value, index) {
126-
for (var j = 0; j < predicates.length; j++) {
127-
if (!predicates[j](value, index)) {
128-
return false;
129-
}
130-
}
131-
return true;
132-
};
133-
134-
if (comparatorType !== 'function') {
135-
if (comparatorType === 'boolean' && comparator) {
136-
comparator = function(obj, text) {
137-
return angular.equals(obj, text);
138-
};
139-
} else {
140-
comparator = function(obj, text) {
141-
if (obj && text && typeof obj === 'object' && typeof text === 'object') {
142-
for (var objKey in obj) {
143-
if (objKey.charAt(0) !== '$' && hasOwnProperty.call(obj, objKey) &&
144-
comparator(obj[objKey], text[objKey])) {
145-
return true;
146-
}
147-
}
148-
return false;
149-
}
150-
text = (''+text).toLowerCase();
151-
return (''+obj).toLowerCase().indexOf(text) > -1;
152-
};
153-
}
154-
}
122+
var predicateFn;
155123

156-
var search = function(obj, text) {
157-
if (typeof text === 'string' && text.charAt(0) === '!') {
158-
return !search(obj, text.substr(1));
159-
}
160-
switch (typeof obj) {
161-
case 'boolean':
162-
case 'number':
163-
case 'string':
164-
return comparator(obj, text);
165-
case 'object':
166-
switch (typeof text) {
167-
case 'object':
168-
return comparator(obj, text);
169-
default:
170-
for (var objKey in obj) {
171-
if (objKey.charAt(0) !== '$' && search(obj[objKey], text)) {
172-
return true;
173-
}
174-
}
175-
break;
176-
}
177-
return false;
178-
case 'array':
179-
for (var i = 0; i < obj.length; i++) {
180-
if (search(obj[i], text)) {
181-
return true;
182-
}
183-
}
184-
return false;
185-
default:
186-
return false;
187-
}
188-
};
189124
switch (typeof expression) {
125+
case 'object':
126+
// Replace `{$: 'xyz'}` with `'xyz'` and fall through
127+
var keys = Object.keys(expression);
128+
if ((keys.length === 1) && (keys[0] === '$')) expression = expression.$;
129+
// jshint -W086
190130
case 'boolean':
191131
case 'number':
192132
case 'string':
193-
// Set up expression object and fall through
194-
expression = {$:expression};
195-
// jshint -W086
196-
case 'object':
197133
// jshint +W086
198-
for (var key in expression) {
199-
(function(path) {
200-
if (typeof expression[path] === 'undefined') return;
201-
predicates.push(function(value) {
202-
return search(path == '$' ? value : (value && value[path]), expression[path]);
203-
});
204-
})(key);
205-
}
134+
predicateFn = createPredicateFn(expression, comparator);
206135
break;
207136
case 'function':
208-
predicates.push(expression);
137+
predicateFn = expression;
209138
break;
210139
default:
211140
return array;
212141
}
213-
var filtered = [];
214-
for (var j = 0; j < array.length; j++) {
215-
var value = array[j];
216-
if (predicates.check(value, j)) {
217-
filtered.push(value);
218-
}
219-
}
220-
return filtered;
142+
143+
return array.filter(predicateFn);
221144
};
222145
}
146+
147+
// Helper functions for `filterFilter`
148+
function createPredicateFn(expression, comparator) {
149+
var predicateFn;
150+
151+
if (comparator === true) {
152+
comparator = equals;
153+
} else if (!isFunction(comparator)) {
154+
comparator = function(actual, expected) {
155+
actual = ('' + actual).toLowerCase();
156+
expected = ('' + expected).toLowerCase();
157+
return actual.indexOf(expected) !== -1;
158+
};
159+
}
160+
161+
predicateFn = function(item) {
162+
return deepCompare(item, expression, comparator);
163+
};
164+
165+
return predicateFn;
166+
}
167+
168+
function deepCompare(actual, expected, comparator) {
169+
var actualType = typeof actual;
170+
var expectedType = typeof expected;
171+
172+
if (expectedType === 'function') {
173+
return expected(actual);
174+
} else if ((expectedType === 'string') && (expected.charAt(0) === '!')) {
175+
return !deepCompare(actual, expected.substring(1), comparator);
176+
} else if (actualType === 'array') {
177+
return actual.some(function(item) {
178+
return deepCompare(item, expected, comparator);
179+
});
180+
}
181+
182+
switch (actualType) {
183+
case 'object':
184+
if (expectedType === 'object') {
185+
return Object.keys(expected).every(function(key) {
186+
var actualVal = (key === '$') ? actual : actual[key];
187+
var expectedVal = expected[key];
188+
return deepCompare(actualVal, expectedVal, comparator);
189+
});
190+
} else {
191+
return Object.keys(actual).some(function(key) {
192+
return (key.charAt(0) !== '$') && deepCompare(actual[key], expected, comparator);
193+
});
194+
}
195+
break;
196+
default:
197+
if (expectedType === 'object') {
198+
return true;
199+
}
200+
201+
return comparator(actual, expected);
202+
}
203+
}

‎test/ng/filter/filterSpec.js‎

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ describe('Filter: filter', function() {
77
filter = $filter('filter');
88
}));
99

10+
1011
it('should filter by string', function() {
1112
var items = ['MIsKO', {name: 'shyam'}, ['adam'], 1234];
1213
expect(filter(items, '').length).toBe(4);
@@ -27,13 +28,15 @@ describe('Filter: filter', function() {
2728
expect(filter(items, "I don't exist").length).toBe(0);
2829
});
2930

31+
3032
it('should not read $ properties', function() {
3133
expect(''.charAt(0)).toBe(''); // assumption
3234

3335
var items = [{$name: 'misko'}];
3436
expect(filter(items, 'misko').length).toBe(0);
3537
});
3638

39+
3740
it('should filter on specific property', function() {
3841
var items = [{ignore: 'a', name: 'a'}, {ignore: 'a', name: 'abc'}];
3942
expect(filter(items, {}).length).toBe(2);
@@ -44,11 +47,13 @@ describe('Filter: filter', function() {
4447
expect(filter(items, {name: 'b'})[0].name).toBe('abc');
4548
});
4649

50+
4751
it('should take function as predicate', function() {
4852
var items = [{name: 'a'}, {name: 'abc', done: true}];
4953
expect(filter(items, function(i) {return i.done;}).length).toBe(1);
5054
});
5155

56+
5257
it('should pass the index to a function predicate', function() {
5358
var items = [0, 1, 2, 3];
5459

@@ -59,6 +64,7 @@ describe('Filter: filter', function() {
5964
expect(result).toEqual([0, 2]);
6065
});
6166

67+
6268
it('should take object as predicate', function() {
6369
var items = [{first: 'misko', last: 'hevery'},
6470
{first: 'adam', last: 'abrons'}];
@@ -92,6 +98,18 @@ describe('Filter: filter', function() {
9298
});
9399

94100

101+
it('should support deep expression objects with multiple properties', function() {
102+
var items = [{person: {name: 'John', email: 'john@example.com'}},
103+
{person: {name: 'Rita', email: 'rita@example.com'}},
104+
{person: {name: 'Billy', email: 'me@billy.com'}},
105+
{person: {name: 'Joan', email: 'joan@example.net'}}];
106+
var expr = {person: {name: 'Jo', email: '!example.com'}};
107+
108+
expect(filter(items, expr).length).toBe(1);
109+
expect(filter(items, expr)).toEqual([items[3]]);
110+
});
111+
112+
95113
it('should match any properties for given "$" property', function() {
96114
var items = [{first: 'tom', last: 'hevery'},
97115
{first: 'adam', last: 'hevery', alias: 'tom', done: false},
@@ -103,24 +121,27 @@ describe('Filter: filter', function() {
103121
expect(filter(items, {$: 'hevery'})[0]).toEqual(items[0]);
104122
});
105123

124+
106125
it('should support boolean properties', function() {
107126
var items = [{name: 'tom', current: true},
108-
{name: 'demi', current: false},
109-
{name: 'sofia'}];
127+
{name: 'demi', current: false},
128+
{name: 'sofia'}];
110129

111130
expect(filter(items, {current:true}).length).toBe(1);
112131
expect(filter(items, {current:true})[0].name).toBe('tom');
113132
expect(filter(items, {current:false}).length).toBe(1);
114133
expect(filter(items, {current:false})[0].name).toBe('demi');
115134
});
116135

136+
117137
it('should support negation operator', function() {
118138
var items = ['misko', 'adam'];
119139

120140
expect(filter(items, '!isk').length).toBe(1);
121141
expect(filter(items, '!isk')[0]).toEqual(items[1]);
122142
});
123143

144+
124145
describe('should support comparator', function() {
125146

126147
it('as equality when true', function() {
@@ -133,8 +154,8 @@ describe('Filter: filter', function() {
133154
{key: 'value1', nonkey: 1},
134155
{key: 'value2', nonkey: 2},
135156
{key: 'value12', nonkey: 3},
136-
{key: 'value1', nonkey:4},
137-
{key: 'Value1', nonkey:5}
157+
{key: 'value1', nonkey: 4},
158+
{key: 'Value1', nonkey: 5}
138159
];
139160
expr = {key: 'value1'};
140161
expect(filter(items, expr, true)).toEqual([items[0], items[3]]);
@@ -143,21 +164,22 @@ describe('Filter: filter', function() {
143164
{key: 1, nonkey: 1},
144165
{key: 2, nonkey: 2},
145166
{key: 12, nonkey: 3},
146-
{key: 1, nonkey:4}
167+
{key: 1, nonkey: 4}
147168
];
148-
expr = { key: 1 };
169+
expr = {key: 1};
149170
expect(filter(items, expr, true)).toEqual([items[0], items[3]]);
150171

151172
expr = 12;
152173
expect(filter(items, expr, true)).toEqual([items[2]]);
153174
});
154175

176+
155177
it('and use the function given to compare values', function() {
156178
var items = [
157179
{key: 1, nonkey: 1},
158180
{key: 2, nonkey: 2},
159181
{key: 12, nonkey: 3},
160-
{key: 1, nonkey:14}
182+
{key: 1, nonkey: 14}
161183
];
162184
var expr = {key: 10};
163185
var comparator = function(obj, value) {
@@ -167,10 +189,47 @@ describe('Filter: filter', function() {
167189

168190
expr = 10;
169191
expect(filter(items, expr, comparator)).toEqual([items[2], items[3]]);
170-
171192
});
172193

173194

174-
});
195+
it('and use it correctly with deep expression objects', function() {
196+
var items = [
197+
{id: 0, details: {email: 'admin@example.com', role: 'admin'}},
198+
{id: 1, details: {email: 'user1@example.com', role: 'user'}},
199+
{id: 2, details: {email: 'user2@example.com', role: 'user'}}
200+
];
201+
var expr, comp;
202+
203+
expr = {details: {email: 'user@example.com', role: 'adm'}};
204+
expect(filter(items, expr)).toEqual([]);
205+
206+
expr = {details: {email: 'admin@example.com', role: 'adm'}};
207+
expect(filter(items, expr)).toEqual([items[0]]);
208+
209+
expr = {details: {email: 'admin@example.com', role: 'adm'}};
210+
expect(filter(items, expr, true)).toEqual([]);
211+
212+
expr = {details: {email: 'admin@example.com', role: 'admin'}};
213+
expect(filter(items, expr, true)).toEqual([items[0]]);
214+
215+
expr = {details: {email: 'user', role: 'us'}};
216+
expect(filter(items, expr)).toEqual([items[1], items[2]]);
175217

218+
expr = {id: 0, details: {email: 'user', role: 'us'}};
219+
expect(filter(items, expr)).toEqual([]);
220+
221+
expr = {id: 1, details: {email: 'user', role: 'us'}};
222+
expect(filter(items, expr)).toEqual([items[1]]);
223+
224+
comp = function(actual, expected) {
225+
return isString(actual) && isString(expected) && (actual.indexOf(expected) === 0);
226+
};
227+
228+
expr = {details: {email: 'admin@example.com', role: 'admn'}};
229+
expect(filter(items, expr, comp)).toEqual([]);
230+
231+
expr = {details: {email: 'admin@example.com', role: 'adm'}};
232+
expect(filter(items, expr, comp)).toEqual([items[0]]);
233+
});
234+
});
176235
});

0 commit comments

Comments
 (0)