Fix case then doted index is longer than array

[ant-hill]

Hi i would be happy if you accept my pull request, this pull request for situation then you try get doted index longer then you have,
ex:

$array = [
            'a' => [
                'b' => [
                    'c' => [
                        'd' => 'e'
                    ]
                ]
            ]
        ];
Stingray::get($arr,'a.b.c.d.e.f.g');

With old behavior we get:
array_key_exists() expects parameter 2 to be array, string given

[rwillians]

Hello, @ant-hill, I think the proposed behaviour fix for this kinda of scenario is indeed necessary. As far as I have used this piece of code, I haven't reached this behaviour necessity yet, so thank you for your contribution.
I'll take a better look at it as soon as I can, we might be able to improve this implementation.

[ant-hill]

@rwillians tnx you.

[ant-hill]

If you need any help, let me know i'll try to help

[rwillians]

@ant-hill, I believe this changes would be enough to fix your problem without dragging in new responsibilities to the class (such as handling objects).
Let me know what you think.

[rwillians]

If it does solve your case problem, please adjust your PR so I can merge your contribution.

[ant-hill]

@rwillians : I do some change in PR.
You patch is ok for most cases, but my internal test case

    public function testConfig()
    {
        $arr = [
            'a' => [
                'b' => [
                    'c' => [
                        'd' => 'e'
                    ]
                ]
            ],
            'f' => 'h'
        ];

        $config = new \Phalcon\Config($arr);
        $this->assertInstanceOf(\Phalcon\Config::class, Stingray::get($config, 'a'));
        $this->assertEquals($arr['a'], Stingray::get($config, 'a')->toArray());

        $this->assertInstanceOf(\Phalcon\Config::class, Stingray::get($config, 'a.b'));
        $this->assertEquals($arr['a']['b'], Stingray::get($config, 'a.b')->toArray());

        $this->assertInstanceOf(\Phalcon\Config::class, Stingray::get($config, 'a.b.c'));
        $this->assertEquals($arr['a']['b']['c'], Stingray::get($config, 'a.b.c')->toArray());
        $this->assertEquals($arr['a']['b']['c']['d'], Stingray::get($config, 'a.b.c.d')); // Indirect modification of overloaded element of Phalcon\Config has no effect
        $this->assertEquals($arr['f'], Stingray::get($config, 'f'));

        $this->assertNull(Stingray::get($config, 'a.b.d'));
        $this->assertNull(Stingray::get($config, 'a.b.c.d.e.f.g'));

        $this->assertEquals($arr, $config->toArray());
    }

is fail with Indirect modification of overloaded element of Phalcon\Config has no effect notice.

this commit 3cc2ab9 fix this behavior without dragging in new responsibilities to the class

[rwillians]

@ant-hill that's probably been caused by some \Phalcon\Config's behaviour. I have tested your scenario using an ArrayObject, which also implements ArrayAccess (just like I suppose Phalcon does):

public function testConfig()
{
    $arr = [
        'a' => [
            'b' => [
                'c' => [
                    'd' => 'e'
                ]
            ]
        ],
        'f' => 'h'
    ];

    $config = new \ArrayObject($arr);
    $this->assertEquals($arr['a'], Stingray::get($config, 'a'));

    $this->assertEquals($arr['a']['b'], Stingray::get($config, 'a.b'));

    $this->assertEquals($arr['a']['b']['c'], Stingray::get($config, 'a.b.c'));
    $this->assertEquals($arr['a']['b']['c']['d'], Stingray::get($config, 'a.b.c.d')); // Indirect modification of overloaded element of ArrayObject has no effect
    $this->assertEquals($arr['f'], Stingray::get($config, 'f'));

    $this->assertNull(Stingray::get($config, 'a.b.d'));
    $this->assertNull(Stingray::get($config, 'a.b.c.d.e.f.g'));

    $this->assertEquals($arr, $config->getArrayCopy());
}

Worked just fine.

Please, try running the test above. If it fails, it indicates there might be some environmental difference causing this error. If it doesn't, then it's probably due some Phalcon's config behaviour.

[ant-hill]

@rwillians ArrayObject works fine, i'll try to add test case with ArrayAccess interface as soon as I can.

[ant-hill]

@rwillians ant-hill@a9efdae

[rwillians]

@ant-hill ArrayObject do implements ArrayAccess. It means that Phalcon's Config has some additional behaviour that is causing the error you are experiencing.

[ant-hill]

@rwillians i create test case ant-hill@a9efdae without \Phalcon\Config, but with similar behavior

If you have recursive structure like:

$array = ['a'=>['b'=>'c']];

$arrayLikeObject = new ArrayLikeObject( array(
    'a' => new ArrayLikeObject( array('b'=>'c') ) 
     )
);

Then "old" algorithm work like this:

Stingray::get($arrayLikeObject,'a.b');

// Before cycle:
$node = &$arrayLikeObject; // $node instanceof ArrayLikeObject
// First iteration
$node = &$arrayLikeObject['a']; // $node instanceof ArrayLikeObject
// Second Iteration
$node = &$node['b']; // ArrayLikeObject = &string
// we have a notice Indirect modification of overloaded element of ArrayLikeObject has no effect

If you work with ArrayObject Class then after get index ArrayObject return array

$array = ['a'=>['b'=>'c']];
$config = new \ArrayObject($arr);
var_dump($config['a']);
//array(1) {
//  ["b"]=>
//  array(1) {
//    ["c"]=>
//    array(1) {
//      ["d"]=>
//      string(1) "e"
//    }
//  }
//}

var_dump(gettype($config['a'])); // string(5) "array"

[rwillians]

@ant-hill I see your point. Is your branch currently able to reproduce the "overloaded element" error using your ArrayLikeObject? I'm going to work on it tonight and I might need it.

[ant-hill]

@rwillians Yes my test case can reproduce it (but this branch include 3cc2ab9 commit )

PS: Maybe we remove pass by refference for Stingray::get?
It is simplify code base ( https://github.com/ant-hill/stingray/blob/change_signature/src/Stingray.php#L23 ) and i can't determine any memory leaks when test it.

[rwillians]

@ant-hill that reference was inherited from the original code, but since removing it will not affect the expected behaviours covered by this package (check unit tests if necessary), I'm okay with the idea of removing it.
In fact, I have extract all viable (IMO) changes we've discussed here and pushed 'em into this branch. Those changes fix your presented scenario in this PR ("array_key_exists() expects parameter 2 to be array, string given") both for arrays and ArrayAccess objects.
Please, check them out and let me know what you think.

[rwillians]

If there a more further changes needed, please adjust your PR so I can merge your contribution.

[ant-hill]

@rwillians I just check you branch with my test cases, i think it is ok.

[ant-hill]

@rwillians Will I need to modify this PR or you will merge your branch?

[rwillians]

@ant-hill please modify your branch so you can be credited for your contribution.