From d8798318b75b7707874baf786514e65147e85712 Mon Sep 17 00:00:00 2001
From: Sergey Danilchenko <s.danilchenko@ttbooking.ru>
Date: Wed, 31 Jan 2024 00:17:03 +0200
Subject: [PATCH] Fix mapping objects implementing ArrayAccess (not
 collections)

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, https://github.com/cweiske/jsonmapper/issues/175),
so a way was sought to rely on interfaces only.

Patch #197 (https://github.com/cweiske/jsonmapper/pull/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, https://github.com/cweiske/jsonmapper/pull/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: https://github.com/cweiske/jsonmapper/pull/224
---
 src/JsonMapper.php                            |  4 +-
 tests/ArrayTest.php                           | 20 +++++++-
 tests/support/JsonMapperTest/Array.php        |  5 ++
 .../JsonMapperTest/ArrayAccessCollection.php  |  8 +++-
 .../JsonMapperTest/ArrayAccessObject.php      | 47 +++++++++++++++++++
 5 files changed, 81 insertions(+), 3 deletions(-)
 create mode 100644 tests/support/JsonMapperTest/ArrayAccessObject.php

diff --git a/src/JsonMapper.php b/src/JsonMapper.php
index 260848a68..c377d4569 100644
--- a/src/JsonMapper.php
+++ b/src/JsonMapper.php
@@ -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);
                 }
             }
diff --git a/tests/ArrayTest.php b/tests/ArrayTest.php
index e4e5e3ec0..8ba6678ef 100644
--- a/tests/ArrayTest.php
+++ b/tests/ArrayTest.php
@@ -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(
@@ -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']);
     }
diff --git a/tests/support/JsonMapperTest/Array.php b/tests/support/JsonMapperTest/Array.php
index cf30a1bc9..90c6d50f9 100644
--- a/tests/support/JsonMapperTest/Array.php
+++ b/tests/support/JsonMapperTest/Array.php
@@ -81,6 +81,11 @@ class JsonMapperTest_Array
      */
     public $pArrayObjectList;
 
+    /**
+     * @var JsonMapperTest_ArrayAccessObject
+     */
+    public $pArrayAccessObject;
+
     /**
      * @var JsonMapperTest_ArrayAccessCollection
      */
diff --git a/tests/support/JsonMapperTest/ArrayAccessCollection.php b/tests/support/JsonMapperTest/ArrayAccessCollection.php
index 0330ef668..1b001cd56 100644
--- a/tests/support/JsonMapperTest/ArrayAccessCollection.php
+++ b/tests/support/JsonMapperTest/ArrayAccessCollection.php
@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-class JsonMapperTest_ArrayAccessCollection implements \ArrayAccess
+class JsonMapperTest_ArrayAccessCollection implements \ArrayAccess, \IteratorAggregate
 {
     /**
      * @var array
@@ -32,4 +32,10 @@ public function offsetUnset($offset)
     {
         unset($this->data[$offset]);
     }
+
+    #[\ReturnTypeWillChange]
+    public function getIterator()
+    {
+        return new \ArrayIterator($this->data);
+    }
 }
diff --git a/tests/support/JsonMapperTest/ArrayAccessObject.php b/tests/support/JsonMapperTest/ArrayAccessObject.php
new file mode 100644
index 000000000..fa9f6640d
--- /dev/null
+++ b/tests/support/JsonMapperTest/ArrayAccessObject.php
@@ -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);
+    }
+}