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

Fix(filter): Filter objects with circular references #6319

Closed
wants to merge 7 commits into from
Closed

Fix(filter): Filter objects with circular references #6319

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1f40a39
fix(filter): Add test for filtering circular references
brettshollenberger Feb 18, 2014
7cec2f6
fix(filter): End filtering recursion for circular references
brettshollenberger Feb 18, 2014
07d4d14
fix(filter): Add final results of filtering to expectation
brettshollenberger Feb 18, 2014
3b96b4e
fix(filter): Use stack of evaluated objects to prevent circular refer…
brettshollenberger Feb 18, 2014
ca21798
fix(filter): Only perform circular reference check on non-primitive t…
brettshollenberger Feb 18, 2014
245721e
fix(filter): Also perform circular reference check on functions; they…
brettshollenberger Feb 18, 2014
a9d814a
fix(filters): Simplify block
brettshollenberger Feb 18, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/ng/filter/filter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ function filterFilter() {
}
}

var search = function(obj, text){
var search = function(obj, text, orig){
if (typeof text == 'string' && text.charAt(0) === '!') {
return !search(obj, text.substr(1));
}
Expand All @@ -166,7 +166,8 @@ function filterFilter() {
return comparator(obj, text);
default:
for ( var objKey in obj) {
if (objKey.charAt(0) !== '$' && search(obj[objKey], text)) {
if (obj[objKey] === orig) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will skip other properties in the object

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. I'll go at this again :)

if (objKey.charAt(0) !== '$' && search(obj[objKey], text, obj)) {
return true;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

free up references with evaluatedObjects.length = 0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind, that won't do anything. ignore :)

Expand Down
10 changes: 10 additions & 0 deletions test/ng/filter/filterSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ describe('Filter: filter', function() {
expect(filter(items, 'misko').length).toBe(0);
});

it('should not re-evaluate circular references', function() {
var originalItem = {name: 'misko'};
var referencedItem = {originalItem: originalItem};
originalItem.referencedItem = referencedItem;

var items = [originalItem];
expect(function() { filter(items, 'not misko') })
.not.toThrow(new Error("Maximum call stack size exceeded"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is probably not right, better to just use not.toThrow(), to avoid issues with different VM implementations.

It should also verify that the output looks as expected

});

it('should filter on specific property', function() {
var items = [{ignore: 'a', name: 'a'}, {ignore: 'a', name: 'abc'}];
expect(filter(items, {}).length).toBe(2);
Expand Down