Hash::extract() is transforming values.

[dereuromark]

I think Hash::extract() does too much, it should maybe not convert the value itself into an array, but rather return it completely.
This would allow for string casts afterwards, whereas right now it would need information on how the array structure of each transformed object looks like.

Example case: FriendsOfCake/cakephp-csvview#55

We just want the User.created DateTime object from the entity, so we can pass it to _renderRow().
But instead the array of it is returned and we can never transform it again as string without deep knowledge of the structure of this specific array.

Might be a side effect of recent changes, maybe a regression even.

[markstory]

I don't see how this is a defect. It sounds like a change in behavior that would break existing applications. There haven't been any changes to Hash since Oct 2015, so the behavior has been consistent for quite some time.

[dereuromark]

Is there an alternative to not force the objects to break into arrays upon simple Hash read?
It will blow up in the future with custom objects in the entities that supply a toString version but don't have the chance to do so.

[markstory]

What do you mean by 'force the objects to break into arrays'? Are you getting the array cast of an object instead of the object? Does the object in question implement toArray by any chance?

[dereuromark]

It is our CakePHP Time object, see the linked issue in CsvView.
There we get an array back with "date" as one key. We would have expected the Time object itself which then can safely output itself as string. The array itself cannot anymore.

[markstory]

Odd, I don't really get what is going on in that other issue, but are you attempting to extract a property out of an entity and the property's value is a Time instance?

[dereuromark]

And after extracting it is an array, not a Time instance, making it impossible to work on it anymore.

[josegonzalez]

Might be because of this line.

[dereuromark]

@josegonzalez No, it doesnt even run into the isset() check
https://github.com/FriendsOfCake/cakephp-csvview/blob/master/src/View/CsvView.php#L301 already has the array instead of the object.

[dereuromark]

The problem is

                if (is_object($item) && method_exists($item, 'toArray')) {
                    $item = $item->toArray();
                }

This should only be done for iterable items we whitelist, not for all possible objects IMO.
Or at least should only do so if the lookup key goes beyond the current level.
E.g. User.created => No need to array the created object, as this is already what we need to return. We would only need to further array it if we were looking for User.created.date.

But this cannot be changed in 3.x, unless we add an additional option to prevent this from happening to any object, e.g. 'lazyToArray' => true.

[dereuromark]

I added a small test case to show the intuitive ("late toArray transformation") expectation: 3.next...3.2-hash

So the

 Array (
    'date' => '2016-01-20 18:47:44.000000'
    'timezone_type' => 3
    'timezone' => 'UTC'
) 

transformation is only necessary for the last case, where we on purpose go deeper.

[ADmad]

For the test cases you have shown following code isn't executed:

                if (is_object($item) && method_exists($item, 'toArray')) {
                    $item = $item->toArray();
                }

This is what's executed:

        if (!preg_match('/[{\[]/', $path)) {
            return (array)static::get($data, $path);
        }

So when using Hash::extract($data, 'User.created') you get an array instead of time object because the return value is cast to an array.

[dereuromark]

Thank you, you are right. This makes it more complicated. I guess virtual field hacks are the only viable workaround I can think of so far.

We would need a new method otherwise that does not cast to array but keeps the found value as is.

[dereuromark]

Hash::get() is available and works as expected, from what I just tried.
I guess this ticket can now either be closed as works-for-me or kept for 4.0 as RFC to avoid casting to array if not needed.

[ADmad]

Probably instead of

return (array)static::get($data, $path);

it should be

return array(static::get($data, $path));

[dereuromark]

That sounds reasonable, but a too harsh BC break for a 3.x release I am afraid.

But to avoid nesting of arrays maybe:

$value = static::get($data, $path);
return is_array($value) ? $value : array($value);

[ADmad]

IMO existing behavior is a bug, but yeah people could be relying in this behavior.

[markstory]

@dereuromark Your suggestion sounds like it should work well.

[dereuromark]

Is this fixable in 3.x? Or shall this wait until 4.x?

[inoas]

Turn the "correct behavior" it on by a an option flag. Set the flag as default in 4.x?

[dereuromark]

Unfortunately it does not seem trivial to fix.. Any ideas?

[markstory]

Leave it alone? 😄

[dereuromark]

Naaaaah ;) That's still an ugly bug - as ugly as those new smileys.

[dereuromark]

@ADmad After some trying around I finally found a viable solution that can land in 3.5.0.