Skip to content

Commit

Permalink
Fix mapping objects implementing ArrayAccess (not collections)
Browse files Browse the repository at this point in the history
Only handle objects as array when they implement both
ArrayAccess and Traversable.

BC break!

----

Originally, JsonMapper handled objects extending ArrayObject as arrays.

Extending own collection classes from ArrayObject is not always feasible
(issue #175, #175),
so a way was sought to rely on interfaces only.

Patch #197 (#197) changed
the implementation to check for the ArrayAccess interface instead of
ArrayObject.

This unfortunately breaks
objects-that-allow-array-access-but-are-not-traversable-arrays
(issue #224, #224),
for example when you allow array access to properties stored
in some internal variable.

The correct solution is to check that the object implements
ArrayAcces *and* Traversable - then we can be sure the object
is intended to be used with e.g. foreach().

Resolves: #224
  • Loading branch information
Sergey Danilchenko authored and cweiske committed May 17, 2024
1 parent 10aceb9 commit d879831
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 3 deletions.
4 changes: 3 additions & 1 deletion src/JsonMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,9 @@ public function map($json, $object)
$array = array();
$subtype = $type;
} else {
if (is_a($type, 'ArrayAccess', true)) {
if (is_a($type, 'ArrayAccess', true)
&& is_a($type, 'Traversable', true)
) {
$array = $this->createInstance($type, false, $jvalue);
}
}
Expand Down
20 changes: 19 additions & 1 deletion tests/ArrayTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,24 @@ public function testMapSimpleArrayObject()
$this->assertSame(1, $sn->pSimpleArrayObject['zwei']);
}

public function testMapSimpleArrayAccess()
public function testMapArrayAccessObject()
{
$jm = new JsonMapper();
$sn = $jm->map(
json_decode(
'{"pArrayAccessObject":{"eins": 1,"zwei": "two","valueObject":{"value": 1, "publicValue": 2}}}'
),
new JsonMapperTest_Array()
);
$this->assertInstanceOf(ArrayAccess::class, $sn->pArrayAccessObject);
$this->assertSame(1, $sn->pArrayAccessObject['eins']);
$this->assertSame('two', $sn->pArrayAccessObject['zwei']);
$this->assertInstanceOf(JsonMapperTest_ValueObject::class, $sn->pArrayAccessObject['valueObject']);
$this->assertSame(2, $sn->pArrayAccessObject['valueObject']->publicValue);
}

public function testMapArrayAccessCollection()

{
$jm = new JsonMapper();
$sn = $jm->map(
Expand All @@ -203,6 +220,7 @@ public function testMapSimpleArrayAccess()
new JsonMapperTest_Array()
);
$this->assertInstanceOf(ArrayAccess::class, $sn->pArrayAccessCollection);
$this->assertInstanceOf(Traversable::class, $sn->pArrayAccessCollection);
$this->assertSame(1, $sn->pArrayAccessCollection['eins']);
$this->assertSame('two', $sn->pArrayAccessCollection['zwei']);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/support/JsonMapperTest/Array.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ class JsonMapperTest_Array
*/
public $pArrayObjectList;

/**
* @var JsonMapperTest_ArrayAccessObject
*/
public $pArrayAccessObject;

/**
* @var JsonMapperTest_ArrayAccessCollection
*/
Expand Down
8 changes: 7 additions & 1 deletion tests/support/JsonMapperTest/ArrayAccessCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

declare(strict_types=1);

class JsonMapperTest_ArrayAccessCollection implements \ArrayAccess
class JsonMapperTest_ArrayAccessCollection implements \ArrayAccess, \IteratorAggregate
{
/**
* @var array
Expand Down Expand Up @@ -32,4 +32,10 @@ public function offsetUnset($offset)
{
unset($this->data[$offset]);
}

#[\ReturnTypeWillChange]
public function getIterator()
{
return new \ArrayIterator($this->data);
}
}
47 changes: 47 additions & 0 deletions tests/support/JsonMapperTest/ArrayAccessObject.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

declare(strict_types=1);

require_once __DIR__ . '/ValueObject.php';

class JsonMapperTest_ArrayAccessObject implements \ArrayAccess
{
/**
* @var int
*/
public $eins;

/**
* @var string
*/
public $zwei;

/**
* @var JsonMapperTest_ValueObject
*/
public $valueObject;

#[\ReturnTypeWillChange]
public function offsetExists($offset)
{
return isset($this->$offset);
}

#[\ReturnTypeWillChange]
public function offsetGet($offset)
{
return $this->$offset ?? null;
}

#[\ReturnTypeWillChange]
public function offsetSet($offset, $value)
{
$this->$offset = $value;
}

#[\ReturnTypeWillChange]
public function offsetUnset($offset)
{
unset($this->$offset);
}
}

0 comments on commit d879831

Please sign in to comment.