fix(filterFilter): ignore function properties and account for inherited properties

gkalpak · gkalpak · commit 47a10b3d20b6 · 2014-12-01T15:15:30.000+02:00
Closes angular#9984
diff --git a/src/ng/filter/filter.js b/src/ng/filter/filter.js
@@ -122,20 +122,15 @@ function filterFilter() {
     var predicateFn;
 
     switch (typeof expression) {
-      case 'object':
-        // Replace `{$: 'xyz'}` with `'xyz'` and fall through
-        var keys = Object.keys(expression);
-        if ((keys.length === 1) && (keys[0] === '$')) expression = expression.$;
-        // jshint -W086
+      case 'function':
+        predicateFn = expression;
+        break;
       case 'boolean':
       case 'number':
+      case 'object':
       case 'string':
-        // jshint +W086
         predicateFn = createPredicateFn(expression, comparator);
         break;
-      case 'function':
-        predicateFn = expression;
-        break;
       default:
         return array;
     }
@@ -152,8 +147,8 @@ function createPredicateFn(expression, comparator) {
     comparator = equals;
   } else if (!isFunction(comparator)) {
     comparator = function(actual, expected) {
-      actual = ('' + actual).toLowerCase();
-      expected = ('' + expected).toLowerCase();
+      actual = lowercase('' + actual);
+      expected = lowercase('' + expected);
       return actual.indexOf(expected) !== -1;
     };
   }
@@ -165,32 +160,44 @@ function createPredicateFn(expression, comparator) {
   return predicateFn;
 }
 
-function deepCompare(actual, expected, comparator) {
+function deepCompare(actual, expected, comparator, keyWasDollar) {
   var actualType = typeof actual;
   var expectedType = typeof expected;
 
-  if (expectedType === 'function') {
-    return expected(actual);
-  } else if ((expectedType === 'string') && (expected.charAt(0) === '!')) {
+  if ((expectedType === 'string') && (expected.charAt(0) === '!')) {
     return !deepCompare(actual, expected.substring(1), comparator);
   } else if (actualType === 'array') {
+    // In case `actual` is an array, consider it a match
+    // if any of it's items matches `expected`
     return actual.some(function(item) {
       return deepCompare(item, expected, comparator);
     });
   }
 
   switch (actualType) {
     case 'object':
-      if (expectedType === 'object') {
-        return Object.keys(expected).every(function(key) {
-          var actualVal = (key === '$') ? actual : actual[key];
-          var expectedVal = expected[key];
-          return deepCompare(actualVal, expectedVal, comparator);
-        });
+      var key;
+      if (keyWasDollar || (expectedType !== 'object')) {
+        for (key in actual) {
+          if ((key.charAt(0) !== '$') && deepCompare(actual[key], expected, comparator)) {
+            return true;
+          }
+        }
+        return false;
       } else {
-        return Object.keys(actual).some(function(key) {
-          return (key.charAt(0) !== '$') && deepCompare(actual[key], expected, comparator);
-        });
+        for (key in expected) {
+          var expectedVal = expected[key];
+          if (isFunction(expectedVal)) {
+            continue;
+          }
+
+          var keyIsDollar = key === '$';
+          var actualVal = keyIsDollar ? actual : actual[key];
+          if (!deepCompare(actualVal, expectedVal, comparator, keyIsDollar)) {
+            return false;
+          }
+        }
+        return true;
       }
       break;
     default:
diff --git a/test/ng/filter/filterSpec.js b/test/ng/filter/filterSpec.js
@@ -136,6 +136,19 @@ describe('Filter: filter', function() {
   });
 
 
+  it('should respect the nesting level of "$"', function() {
+    var items = [{supervisor: 'me', person: {name: 'Annet', email: 'annet@example.com'}},
+                 {supervisor: 'me', person: {name: 'Billy', email: 'me@billy.com'}},
+                 {supervisor: 'me', person: {name: 'Joan', email: 'joan@example.net'}},
+                 {supervisor: 'me', person: {name: 'John', email: 'john@example.com'}},
+                 {supervisor: 'me', person: {name: 'Rita', email: 'rita@example.com'}}];
+    var expr = {$: {$: 'me'}};
+
+    expect(filter(items, expr).length).toBe(1);
+    expect(filter(items, expr)).toEqual([items[1]]);
+  });
+
+
   it('should support boolean properties', function() {
     var items = [{name: 'tom', current: true},
                  {name: 'demi', current: false},
@@ -156,23 +169,146 @@ describe('Filter: filter', function() {
   });
 
 
-  it('should not consider the expression\'s inherited properties', function() {
-    Object.prototype.noop = noop;
+  it('should ignore function properties in items', function() {
+    // Own function properties
+    var items = [
+      {text: 'hello', func: noop},
+      {text: 'goodbye'},
+      {text: 'kittens'},
+      {text: 'puppies'}
+    ];
+    var expr = {text: 'hello'};
+
+    expect(filter(items, expr).length).toBe(1);
+    expect(filter(items, expr)[0]).toBe(items[0]);
+    expect(filter(items, expr, true).length).toBe(1);
+    expect(filter(items, expr, true)[0]).toBe(items[0]);
+
+    // Inherited function proprties
+    function Item(text) {
+        this.text = text;
+    }
+    Item.prototype.func = noop;
+
+    items = [
+      new Item('hello'),
+      new Item('goodbye'),
+      new Item('kittens'),
+      new Item('puppies')
+    ];
 
+    expect(filter(items, expr).length).toBe(1);
+    expect(filter(items, expr)[0]).toBe(items[0]);
+    expect(filter(items, expr, true).length).toBe(1);
+    expect(filter(items, expr, true)[0]).toBe(items[0]);
+  });
+
+
+  it('should ignore function properties in expression', function() {
+    // Own function properties
     var items = [
       {text: 'hello'},
       {text: 'goodbye'},
       {text: 'kittens'},
       {text: 'puppies'}
     ];
+    var expr = {text: 'hello', func: noop};
+
+    expect(filter(items, expr).length).toBe(1);
+    expect(filter(items, expr)[0]).toBe(items[0]);
+    expect(filter(items, expr, true).length).toBe(1);
+    expect(filter(items, expr, true)[0]).toBe(items[0]);
+
+    // Inherited function proprties
+    function Expr(text) {
+        this.text = text;
+    }
+    Expr.prototype.func = noop;
+
+    expr = new Expr('hello');
+
+    expect(filter(items, expr).length).toBe(1);
+    expect(filter(items, expr)[0]).toBe(items[0]);
+    expect(filter(items, expr, true).length).toBe(1);
+    expect(filter(items, expr, true)[0]).toBe(items[0]);
+  });
+
+
+  it('should consider inherited properties in items', function() {
+    function Item(text) {
+      this.text = text;
+    }
+    Item.prototype.doubleL = 'maybe';
+
+    var items = [
+      new Item('hello'),
+      new Item('goodbye'),
+      new Item('kittens'),
+      new Item('puppies')
+    ];
+    var expr = {text: 'hello', doubleL: 'perhaps'};
+
+    expect(filter(items, expr).length).toBe(0);
+    expect(filter(items, expr, true).length).toBe(0);
+
+    expr = {text: 'hello', doubleL: 'maybe'};
+
+    expect(filter(items, expr).length).toBe(1);
+    expect(filter(items, expr)[0]).toBe(items[0]);
+    expect(filter(items, expr, true).length).toBe(1);
+    expect(filter(items, expr, true)[0]).toBe(items[0]);
+  });
+
+
+  it('should consider inherited properties in expression', function() {
+    function Expr(text) {
+      this.text = text;
+    }
+    Expr.prototype.doubleL = true;
+
+    var items = [
+      {text: 'hello', doubleL: true},
+      {text: 'goodbye'},
+      {text: 'kittens'},
+      {text: 'puppies'}
+    ];
+    var expr = new Expr('e');
+
+    expect(filter(items, expr).length).toBe(1);
+    expect(filter(items, expr)[0]).toBe(items[0]);
+
+    expr = new Expr('hello');
+
+    expect(filter(items, expr, true).length).toBe(1);
+    expect(filter(items, expr)[0]).toBe(items[0]);
+  });
+
+
+  it('should not be affected by `Object.prototype` when using a string expression', function() {
+    Object.prototype.someProp = 'oo';
+
+    var items = [
+      createMap(),
+      createMap(),
+      createMap(),
+      createMap()
+    ];
+    items[0].someProp = 'hello';
+    items[1].someProp = 'goodbye';
+    items[2].someProp = 'kittens';
+    items[3].someProp = 'puppies';
+
+    // Affected by `Object.prototype`
+    expect(filter(items, {}).length).toBe(1);
+    expect(filter(items, {})[0]).toBe(items[1]);
 
-    expect(filter(items, {text: 'hell'}).length).toBe(1);
-    expect(filter(items, {text: 'hell'})[0]).toBe(items[0]);
+    expect(filter(items, {$: 'll'}).length).toBe(0);
 
-    expect(filter(items, 'hell').length).toBe(1);
-    expect(filter(items, 'hell')[0]).toBe(items[0]);
+    // Not affected by `Object.prototype`
+    expect(filter(items, 'll').length).toBe(1);
+    expect(filter(items, 'll')[0]).toBe(items[0]);
 
-    delete(Object.prototype.noop);
+    delete Object.prototype.someProp;
   });
 
 
