FLEX-35031 FLEX-33058
CAUSE:
-FLEX-33058: HierarchicalCollectionViewCursor assumed, in findAny() and
findLast(), that all the items in the collection will have all the properties
that the parameter does. That seems like a safe assumption to make when
findAny() is called with current as the parameter (as it is in
collectionChangeHandler()), but even that isn't always true, as the example in
the ticket demonstrates: when the HierarchicalCollectionViewCursor works on a
HierarchicalCollectionView with a GroupingCollection2 as its source, some of
the objects it will traverse will be the anonymous objects created by
GroupingCollection2 to represent the groups (which have the "mx_internal_uid"
property seen in the RTE, and also "children" and "GroupLabel"), and others
will be the grouped items, which are often sealed class instances which we
can't expect to have those properties.
-FLEX-35031: The (other) assumption in findAny() and findLast() was that the
parameter was NOT a sealed class instance. Its properties were being inspected
using a regular for(var i:String in values) loop, which only yields anything if
values's properties are enumerable, i.e. are defined on a dynamic object. So
when 'values' was a sealed class instance (as often happens in
collectionChangeHandler(), where the parameter is current, which is often a
sealed class instance), the algorithm incorrectly assumed that there was no
difference between 'values' and 'current', because 'values' seemed to have no
properties at all.
SOLUTION:
Now we assume we have a match if and only if the objects are the same, or if
the parameter is dynamic and 'current' shares all its properties and values
(even if it contains other properties and values, it's still a match).
NOTE:
-the solution implies, as documented in the unit tests, that findAny({}) and
findLast({}) will return the first and, respectively, the last item in the
collection. This seems to be how it should work.
-we're now using non-strict equality checks, which implies, again as documented
in the unit tests, that searching by the string equivalent of a number property
will still work.
Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/94d038a6
Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/94d038a6
Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/94d038a6
Branch: refs/heads/develop
Commit: 94d038a617c889f83b72e1510692e1d9c57d1d9b
Parents: 7a0e9eb
Author: Mihai Chira <[email protected]>
Authored: Wed Feb 17 15:06:17 2016 +0100
Committer: Mihai Chira <[email protected]>
Committed: Wed Feb 17 15:06:17 2016 +0100
----------------------------------------------------------------------
.../HierarchicalCollectionViewCursor.as | 58 +++++++++---------
...rchicalCollectionViewCursor_FindAny_Tests.as | 64 ++++++++++++++++++++
2 files changed, 94 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/94d038a6/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as
----------------------------------------------------------------------
diff --git
a/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as
b/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as
index bb5a19b..a55db72 100644
---
a/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as
+++
b/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as
@@ -21,7 +21,6 @@ package mx.collections
{
import flash.events.EventDispatcher;
-import flash.utils.Dictionary;
import mx.collections.errors.ChildItemPendingError;
import mx.collections.errors.ItemPendingError;
@@ -29,6 +28,7 @@ import mx.core.mx_internal;
import mx.events.CollectionEvent;
import mx.events.CollectionEventKind;
import mx.events.FlexEvent;
+import mx.utils.ObjectUtil;
import mx.utils.UIDUtil;
use namespace mx_internal;
@@ -324,19 +324,7 @@ public class HierarchicalCollectionViewCursor extends
EventDispatcher
var done:Boolean = false;
while (!done)
{
- var currentData:Object = hierarchicalData.getData(current);
-
- var matches:Boolean = true;
- for (var property:String in valuesToMatch)
- {
- if (valuesToMatch[property] != currentData[property])
- {
- matches = false;
- break;
- }
- }
-
- if (matches)
+ if (objectMatchesValues(hierarchicalData.getData(current),
valuesToMatch))
return true;
done = !moveNext();
@@ -345,6 +333,32 @@ public class HierarchicalCollectionViewCursor extends
EventDispatcher
return false;
}
+ private static function objectMatchesValues(object:Object,
values:Object):Boolean
+ {
+ if(!object && !values)
+ return true;
+
+ if(!object || !values)
+ return false;
+
+ if(object === values)
+ return true;
+
+ var enumerableProperties:Array =
ObjectUtil.getEnumerableProperties(values);
+ var matches:Boolean = enumerableProperties.length > 0 ||
ObjectUtil.isDynamicObject(values);
+
+ for each(var property:String in enumerableProperties)
+ {
+ if (!object.hasOwnProperty(property) || object[property] !=
values[property])
+ {
+ matches = false;
+ break;
+ }
+ }
+
+ return matches;
+ }
+
/**
* @inheritDoc
*
@@ -379,19 +393,7 @@ public class HierarchicalCollectionViewCursor extends
EventDispatcher
var done:Boolean = false;
while (!done)
{
- var currentData:Object = current;
-
- var matches:Boolean = true;
- for (var property:String in valuesToMatch)
- {
- if (currentData[property] != valuesToMatch[property])
- {
- matches = false;
- break;
- }
- }
-
- if (matches)
+ if (objectMatchesValues(hierarchicalData.getData(current),
valuesToMatch))
return true;
done = !movePrevious();
@@ -412,7 +414,7 @@ public class HierarchicalCollectionViewCursor extends
EventDispatcher
public function moveNext():Boolean
{
var currentNode:Object = current;
- //if there is no currentNode then we cant move forward and must be off
the ends
+ //if there is no currentNode then we can't move forward and must be
off the ends
if (currentNode == null)
return false;
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/94d038a6/frameworks/projects/advancedgrids/tests/mx/collections/HierarchicalCollectionViewCursor_FindAny_Tests.as
----------------------------------------------------------------------
diff --git
a/frameworks/projects/advancedgrids/tests/mx/collections/HierarchicalCollectionViewCursor_FindAny_Tests.as
b/frameworks/projects/advancedgrids/tests/mx/collections/HierarchicalCollectionViewCursor_FindAny_Tests.as
index 33c1269..230fa91 100644
---
a/frameworks/projects/advancedgrids/tests/mx/collections/HierarchicalCollectionViewCursor_FindAny_Tests.as
+++
b/frameworks/projects/advancedgrids/tests/mx/collections/HierarchicalCollectionViewCursor_FindAny_Tests.as
@@ -242,6 +242,34 @@ package mx.collections {
}
[Test]
+ public function
test_findAny_finds_the_first_item_when_its_argument_is_an_empty_anonymous_object():void
+ {
+ //given
+ _utils.openAllNodes(_collectionView);
+
+ //when
+ var found:Boolean = _sut.findAny({});
+
+ //then
+ assertTrue(found);
+ assertEquals(_level0.getItemAt(0), _sut.current);
+ }
+
+ [Test]
+ public function
test_findLast_finds_the_last_item_when_its_argument_is_an_empty_anonymous_object():void
+ {
+ //given
+ _utils.openAllNodes(_collectionView);
+
+ //when
+ var found:Boolean = _sut.findLast({});
+
+ //then
+ assertTrue(found);
+ assertEquals(_employeesByID[_employeesByID.length - 1],
_sut.current);
+ }
+
+ [Test]
public function
test_findAny_finds_sealed_class_instance_via_anonymous_object_with_subset_of_properties():void
{
//given
@@ -260,6 +288,24 @@ package mx.collections {
}
[Test]
+ public function
test_findAny_finds_sealed_class_instance_via_anonymous_object_with_subset_of_properties_and_string_values_instead_of_int():void
+ {
+ //given
+ const ID_TO_FIND:int = NO_EMPLOYEES_PER_DEPARTMENT;
+ _utils.openAllNodes(_collectionView);
+
+ //when
+ var found:Boolean = _sut.findAny({department:DEPARTMENT_SALES,
uniqueID:ID_TO_FIND.toString()});
+
+ //then
+ assertTrue(found);
+ var currentEmployee:EmployeeVO = _sut.current as EmployeeVO;
+ assertNotNull(currentEmployee);
+ assertEquals(DEPARTMENT_SALES, currentEmployee.department);
+ assertEquals(ID_TO_FIND, currentEmployee.uniqueID);
+ }
+
+ [Test]
public function
test_findLast_finds_sealed_class_instance_via_anonymous_object_with_subset_of_properties():void
{
//given
@@ -278,6 +324,24 @@ package mx.collections {
}
[Test]
+ public function
test_findLast_finds_sealed_class_instance_via_anonymous_object_with_subset_of_properties_and_string_values_instead_of_int():void
+ {
+ //given
+ const ID_TO_FIND:int = 0;
+ _utils.openAllNodes(_collectionView);
+
+ //when
+ var found:Boolean =
_sut.findLast({department:DEPARTMENT_DEVELOPMENT,
uniqueID:ID_TO_FIND.toString()});
+
+ //then
+ assertTrue(found);
+ var currentEmployee:EmployeeVO = _sut.current as EmployeeVO;
+ assertNotNull(currentEmployee);
+ assertEquals(DEPARTMENT_DEVELOPMENT, currentEmployee.department);
+ assertEquals(ID_TO_FIND, currentEmployee.uniqueID);
+ }
+
+ [Test]
public function
test_findAny_finds_sealed_class_instance_via_dynamic_class_instance_with_subset_of_properties():void
{
//given
