Yii2: BC break in ArrayHelper on 2.0.36

[YiiRocks]

What steps will reproduce the problem?

$list = self::find()
    ->select(['lng', 'name' => 'name_' . Yii::$app->language])
    ->orderBy('sort, name')
    ->all();
$test = ArrayHelper::map($list, 'lng', 'name');
var_dump($test);

What is the expected result?

2.0.35 result

array(9) {
  ["Icao"]=>
  string(27) "ICAO/NATO phonetic alphabet"
  ["Lapd"]=>
  string(19) "LAPD radio alphabet"
  ["NlBe"]=>
  string(15) "Dutch (Belgium)"
  ["NlNl"]=>
  string(23) "Dutch (The Netherlands)"
  ["Fr"]=>
  string(6) "French"
  ["De"]=>
  string(6) "German"
  ["It"]=>
  string(7) "Italian"
  ["Es"]=>
  string(7) "Spanish"
  ["Useless"]=>
  string(49) "The Non-Phonetic Alphabet (Use at your own risk!)"
}

What do you get instead?

2.0.36-dev result

array(9) {
  ["Icao"]=>
  NULL
  ["Lapd"]=>
  NULL
  ["NlBe"]=>
  NULL
  ["NlNl"]=>
  NULL
  ["Fr"]=>
  NULL
  ["De"]=>
  NULL
  ["It"]=>
  NULL
  ["Es"]=>
  NULL
  ["Useless"]=>
  NULL
}

Additional info

| Q | A
| ---------------- | ---
| Yii version | 2.0.36-dev
| PHP version | 7.4.6
| Operating system | Debian 9

[alex-code]

What results does the query return?

[YiiRocks]

Both times the same return (diff returned nohing).

array(9) {
  [0]=>
  object(mister42\models\tools\PhoneticAlphabet)#103 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(27) "ICAO/NATO phonetic alphabet"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "Icao"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "Icao"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [1]=>
  object(mister42\models\tools\PhoneticAlphabet)#138 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(19) "LAPD radio alphabet"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "Lapd"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "Lapd"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [2]=>
  object(mister42\models\tools\PhoneticAlphabet)#139 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(15) "Dutch (Belgium)"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "NlBe"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "NlBe"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [3]=>
  object(mister42\models\tools\PhoneticAlphabet)#140 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(23) "Dutch (The Netherlands)"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "NlNl"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(4) "NlNl"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [4]=>
  object(mister42\models\tools\PhoneticAlphabet)#141 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(6) "French"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "Fr"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "Fr"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [5]=>
  object(mister42\models\tools\PhoneticAlphabet)#142 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(6) "German"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "De"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "De"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [6]=>
  object(mister42\models\tools\PhoneticAlphabet)#143 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(7) "Italian"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "It"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "It"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [7]=>
  object(mister42\models\tools\PhoneticAlphabet)#144 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(7) "Spanish"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "Es"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(2) "Es"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
  [8]=>
  object(mister42\models\tools\PhoneticAlphabet)#145 (14) {
    ["alphabet"]=>
    NULL
    ["name"]=>
    string(49) "The Non-Phonetic Alphabet (Use at your own risk!)"
    ["numeric"]=>
    bool(true)
    ["text"]=>
    NULL
    ["_attributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(7) "Useless"
    }
    ["_oldAttributes":"yii\db\BaseActiveRecord":private]=>
    array(1) {
      ["lng"]=>
      string(7) "Useless"
    }
    ["_related":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_relationsDependencies":"yii\db\BaseActiveRecord":private]=>
    array(0) {
    }
    ["_errors":"yii\base\Model":private]=>
    NULL
    ["_validators":"yii\base\Model":private]=>
    NULL
    ["_scenario":"yii\base\Model":private]=>
    string(7) "default"
    ["_events":"yii\base\Component":private]=>
    array(0) {
    }
    ["_eventWildcards":"yii\base\Component":private]=>
    array(0) {
    }
    ["_behaviors":"yii\base\Component":private]=>
    array(0) {
    }
  }
}

[YiiRocks]

I suspect https://github.com/yiisoft/yii2/blob/35fb9c624893855317e5fe52e6a21f6518a9a31c/framework/helpers/BaseArrayHelper.php#L200 from PR #18027 / commit 35fb9c624893855317e5fe52e6a21f6518a9a31c to be the cause, as it returns as if everything given to this function is an array. The correct return is never reached.
https://github.com/yiisoft/yii2/blob/35fb9c624893855317e5fe52e6a21f6518a9a31c/framework/helpers/BaseArrayHelper.php#L214

[alex-code]

I think it could be this line https://github.com/yiisoft/yii2/blob/35fb9c624893855317e5fe52e6a21f6518a9a31c/framework/helpers/BaseArrayHelper.php#L209

If the key doesn't exist it shouldn't return $default and should continue.

[samdark]

Is it possible to put together a unit test that fails with 2.0.36 but does not with 2.0.35? Then it will be easy to find commit that broke it and do the fix.

[YiiRocks]

I can't test it right now, but this might do the trick:

    public function testGetValueArrayObjects()
    {
        $data = [
            new \ArrayObject(['foo1' => 'bar1']),
            new \ArrayObject(['foo2' => 'bar2']),
        ];
        $this->assertEquals('bar2', ArrayHelper::getValue($data, 'foo2'));
    }

Edit: Again without testing this, as I am not near my computer. It might not need the array.

    public function testGetValueObject()
    {
        $data = new \ArrayObject(['foo1' => 'bar1', 'foo2' => 'bar2']);
        $this->assertEquals('bar2', ArrayHelper::getValue($data, 'foo2'));
    }

[samdark]

First one fails. Second one pass.

[samdark]

It fails for any version from 2.0.0 to 2.0.36 so what's in the tests is not a regression.

[samdark]

public function testMapObjects()
{
    $data = [
        new DynamicModel(['name' => 'bar1', 'value' => 1]),
        new DynamicModel(['name' => 'bar2', 'value' => 2]),
    ];

    $expected = [
        'bar1' => 1,
        'bar2' => 2,
    ];

    $this->assertEquals($expected, ArrayHelper::map($data, 'name', 'value'));
}

Works as expected.

[YiiRocks]

Right. I will come back with a correct test this evening then :)

[YiiRocks]

The only difference I can find right now is that in my case name is not actually a column in my database.table, but declared as public $name; in the class. This causes it to be not listed under _attributes in the object.
If that did not help I will take a look this evening.

[alex-code]

This works for me on .35

$data = [
  new class(['value' => 1]) extends \yii\base\DynamicModel {
    public $name = 'bar1';
  },
  new class(['value' => 2]) extends \yii\base\DynamicModel {
    public $name = 'bar2';
  },
];

I just did some quick local code and removing the return $default from this line https://github.com/yiisoft/yii2/blob/35fb9c624893855317e5fe52e6a21f6518a9a31c/framework/helpers/BaseArrayHelper.php#L209 made it work on .36

I think lines 199-201 & 208-210 could be combined too.

if (static::isArrayAccess($array) && static::keyExists($key, $array)) {
  return $array[$key];
}

[samdark]

@Thoulah it fails the same way in 2.0.0. There was no regression about it.

So... is there something that worked in 2.0.35 and doesn't work in master?

[YiiRocks]

I seem to be unable to make this into a phpUnit test, but the following code will reproduce the difference in results.

TestController.php:

<?php

namespace app\controllers;

class TestController extends \yii\web\Controller
{
    public function actionTest()
    {
        $model = new \app\models\Test();
        $data = $model->getList();
        var_dump($data);exit;
    }
}

Test.php:

<?php

namespace app\models;

use yii\helpers\ArrayHelper;

class Test extends \yii\db\ActiveRecord
{
    public $key;

    public static function tableName()
    {
        return '{{%test}}';
    }

    public static function getList(): array
    {
        $list = self::find()
            ->select(['key' => 'name', 'value'])
            ->all();
        $data = ArrayHelper::map($list, 'key', 'value');
        return $data;
    }
}

Mysql dump:

CREATE TABLE `tbl_test` (
  `id` int(11) NOT NULL,
  `name` varchar(5) NOT NULL,
  `value` varchar(15) NOT NULL
) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4;

INSERT INTO `tbl_test` (`id`, `name`, `value`) VALUES
(1, 'name1', 'Yii2 test'),
(2, 'name2', 'Another value'),
(3, 'name3', 'Last one');

And finally the results of this code:
2.0.35: array(3) { ["name1"]=> string(9) "Yii2 test" ["name2"]=> string(13) "Another value" ["name3"]=> string(8) "Last one" }
2.0.36-dev: array(1) { [""]=> string(8) "Last one" }

[alex-code]

Ignore those commits, thought I had it but we get other issues.

The problem is if an object implements ArrayAccess you can't use ArrayHelper::getValue to access public properties anymore and still have $default return on non existent keys.

[YiiRocks]

Is that related to my issue on the forum?

[alex-code]

That's different, if you're trying to access a property that exists (as part of the object) that'll be used before deferring to magic methods.

class Test
{
  public $foo = 'bar';

  public function __get($name)
  {
    if ($name === 'foo') {
      return "I'm also bar";
    }
  }
}

(new Test)->foo; // is 'bar'

[mikk150]

mkay, so, issue is that since you want to get property off of ActiveRecord instance, and ActiveRecord instance implements \ArrayAccess interface(meaning I can ask stuff with \ArrayAccess::offsetGet()) that means pretty much, that public properties and getters cannot be accessed using ArrayHelper::getValue()

this Is a regression yes..

[mikk150]

hmm, it is actually more interesting, as it should not matter, as ActiveRecord::offsetGet() is https://github.com/yiisoft/yii2/blob/master/framework/base/Model.php#L1024

edit://

https://github.com/yiisoft/yii2/blob/master/framework/db/BaseActiveRecord.php#L334
should be the reason why, it does not check if public propery exists

[mikk150]

@samdark maybe we should fix BaseActiveRecord::__isset() to check for public properties as well, but that might be Backwards breaking for some folks out there

[mikk150]

So BaseActiveRecord::__isset() sure does not work correctly

[alex-code]

I don't think this issue is specific to ActiveRecord

Quick dirty example

$data = new class(['value' => 123]) implements \ArrayAccess {
  private $attributes = [];
  public $name = 'bar1';

  public function __construct($attributes)
  {
    $this->attributes = $attributes;
  }

  public function offsetExists($offset) { return isset($this->attributes[$offset]); }
  public function offsetGet($offset) { return $this->attributes[$offset]; }
  public function offsetSet($offset, $value) {}
  public function offsetUnset($offset) {}
};

var_dump(ArrayHelper::getValue($data, 'value')); // dumps as `123`
var_dump(ArrayHelper::getValue($data, 'name')); // dumps as `NULL`

If a class implements ArrayAccess you can't use ArrayHelper::getValue to get object properties.

[mikk150]

I don't think this issue is specific to ActiveRecord

Quick dirty example

$data = new class(['value' => 123]) implements \ArrayAccess {
  private $attributes = [];
  public $name = 'bar1';

  public function __construct($attributes)
  {
    $this->attributes = $attributes;
  }

  public function offsetExists($offset) { return isset($this->attributes[$offset]); }
  public function offsetGet($offset) { return $this->attributes[$offset]; }
  public function offsetSet($offset, $value) {}
  public function offsetUnset($offset) {}
};

var_dump(ArrayHelper::getValue($data, 'value')); // dumps as `123`
var_dump(ArrayHelper::getValue($data, 'name')); // dumps as `NULL`

If a class implements ArrayAccess you can't use ArrayHelper::getValue to get object properties.

well, yes but if this PR wasn't merged you could not get ArrayAccess offset values

[samdark]

@Thoulah is your issue https://github.com/yiisoft/yii2/issues/18086#issuecomment-649557239?

[YiiRocks]

These smart individuals came to that conclusion based on my report and code. I have never even looked at ArrayAccess to say anything useful in this matter.

[mikk150]

I could fix it with checking first with offsetExists

[samdark]

@mikk150 let's do it but I'd like to see a test that works on 2.0.35 but fails on 2.0.36 first.

[alex-code]

With my example here, https://github.com/yiisoft/yii2/issues/18086#issuecomment-649557239
_| .35 | .36
---|---|---
1st var_dump | Causes an exception, treated as a non existent object property | Result as expected
2nd var_dump | Result as expected | Result is always NULL (or whatever $default is) even though property exists

[samdark]

Added a test. Looking for a way to fix it.

[samdark]

@alex-code, @Thoulah, @mikk150 implemented a fix. Need someone to check me. Especially on better ways achieving it.

[YiiRocks]

Is it prefered to return $default from a catch?

The static function isArrayAccess is now unused.

[samdark]

Is it prefered to return $default from a catch?

In this case yes because we have no idea what's missing, an object property or ArrayAccess-accessible data. Since it's ArrayAccess object we assume that's data so it behaves according to this assumption.

[samdark]

btw., was your original issue fixed with it?

[YiiRocks]

btw., was your original issue fixed with it?

I will let you know soon, I am in the middle of a server migration (Plesk now support Debian 10). It won't take very long

[YiiRocks]

Looking good from my end!

[alex-code]

@alex-code, @Thoulah, @mikk150 implemented a fix. Need someone to check me. Especially on better ways achieving it.

Looks OK to me, I can't think of another way to do it either.

[YiiRocks]

Thank you all for your help in resolving this issue :+1:

[arkzverev]

After update to 2.0.36 my script crashed with memory overflow.
You can replace this:

if (static::keyExists($key, $array)) {

to

if (is_array($array) && static::keyExists($key, $array)) {

My object with getter-property - catchs this section, instead of getting into the section:

if (is_object($array)) {

[samdark]

How to reproduce it?

[arkzverev]

How to reproduce it?

wrote to email

[rikcage]

Hi i found a bug in
public static function filter ($ array, $ filters)
yii2 / framework / helpers / BaseArrayHelper.php file
line 947 if ($ filter [0] === '!') {https://github.com/yiisoft/yii2/blob/763a2b04eae79ce3cb4cc448ad179605c52bdec2/framework/helpers/BaseArrayHelper.php#L947}
if $ filters is one dimensional array i see

PHP Notice - yii base ErrorException

Trying to access array offset on value of type int
1.in /var/www/test_dev/vendor/yiisoft/yii2/helpers/BaseArrayHelper.phpat line 947

 * @return array Filtered array
 * @since 2.0.9
 * /
public static function filter ($ array, $ filters)
{
    $ result = [];
    $ excludeFilters = [];

    foreach ($ filters as $ filter) {
        if ($ filter [0] === '!') {
            $ excludeFilters [] = substr ($ filter, 1);
            continue;
        }

        $ nodeValue = $ array; // set $ array as root node
        $ keys = explode ('.', $ filter);
        foreach ($ keys as $ key) {
            if (! array_key_exists ($ key, $ nodeValue)) {
                continue 2; // Jump to next filter

In older versions it worked like this
$ array:

Array
(
    [0] => #
    [1] => Action Column
    [2] => User
    [3] => Subject
    [4] => Is Active
    [5] => Created At
    [6] => Updated At
)

$ filters:

Array
(
    [0] => 0
    [1] => 1
    [2] => 3
    [3] => 2
    [4] => 6
    [5] => 5
)

$ result:

Array
(
    [0] => #
    [1] => Action Column
    [3] => Subject
    [2] => User
    [6] => Updated At
    [5] => Created At
    [4] => Is Active
)

It was possible to use ArrayHelper :: filter to display the array in the desired order.
I fixed it like this

- 947 if ($ filter [0] === '!') {
+ 947 if (is_array ($ filter) && $ filter [0] === '!') {

Maybe it should be included this in the repository?

[samdark]

@rikcage please create a separate issue. Thanks. It's easy to lose track of comment in a closed issue.