fix: copy ArrayObject contents through recursiveCopy
ArrayObjectFilter copied each element through the public copy(), whose first statement resets the object map. That map is what keeps one copy per source object and what guards against cycles, so an object shared inside an ArrayObject was duplicated and a graph whose cycle ran through an ArrayObject recursed until memory ran out. Reach recursiveCopy() through a bound closure, the way the sibling SplDoublyLinkedListFilter already does. DeepCopy::copy() is unchanged. ArrayObjectFilterTest stubbed the copy() call being removed, so it is rewritten against a real DeepCopy like SplDoublyLinkedListFilterTest, keeping its flags and iterator-class assertions.
This commit is contained in:
1 parent
b66e64a804
commit
380b19a6d3
3 files changed
+76
-15
No files matched your search
@@ -1,6 +1,7 @@
|
|||||||
<?php
|
<?php
|
||||||
namespace DeepCopy\TypeFilter\Spl;
|
namespace DeepCopy\TypeFilter\Spl;
|
||||||
|
|
||||||
|
use Closure;
|
||||||
use DeepCopy\DeepCopy;
|
use DeepCopy\DeepCopy;
|
||||||
use DeepCopy\TypeFilter\TypeFilter;
|
use DeepCopy\TypeFilter\TypeFilter;
|
||||||
|
|
||||||
@@ -23,11 +24,30 @@ final class ArrayObjectFilter implements TypeFilter
|
|||||||
public function apply(mixed $arrayObject)
|
public function apply(mixed $arrayObject)
|
||||||
{
|
{
|
||||||
$clone = clone $arrayObject;
|
$clone = clone $arrayObject;
|
||||||
|
$copy = $this->createCopyClosure();
|
||||||
|
|
||||||
foreach ($arrayObject->getArrayCopy() as $k => $v) {
|
foreach ($arrayObject->getArrayCopy() as $k => $v) {
|
||||||
$clone->offsetSet($k, $this->copier->copy($v));
|
$clone->offsetSet($k, $copy($v));
|
||||||
}
|
}
|
||||||
|
|
||||||
return $clone;
|
return $clone;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Copies through the recursive entry point rather than {@see DeepCopy::copy()},
|
||||||
|
* which would reset the map of already copied objects: shared objects would
|
||||||
|
* be duplicated and a cycle running through the ArrayObject would not
|
||||||
|
* terminate. Same approach as {@see SplDoublyLinkedListFilter}.
|
||||||
|
*/
|
||||||
|
private function createCopyClosure(): Closure
|
||||||
|
{
|
||||||
|
$copier = $this->copier;
|
||||||
|
|
||||||
|
$copy = function (mixed $value) use ($copier): mixed {
|
||||||
|
return $copier->recursiveCopy($value);
|
||||||
|
};
|
||||||
|
|
||||||
|
return Closure::bind($copy, null, DeepCopy::class);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -247,6 +247,35 @@ class DeepCopyTest extends TestCase
|
|||||||
$this->assertSame($copy->getProp1()->c, $copy->getProp2());
|
$this->assertSame($copy->getProp1()->c, $copy->getProp2());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_it_keeps_reference_of_objects_shared_through_an_array_object()
|
||||||
|
{
|
||||||
|
$shared = new stdClass();
|
||||||
|
|
||||||
|
$object = new stdClass();
|
||||||
|
$object->collection = new ArrayObject(['a' => $shared, 'b' => $shared]);
|
||||||
|
$object->direct = $shared;
|
||||||
|
|
||||||
|
$copy = deep_copy($object);
|
||||||
|
|
||||||
|
$this->assertEqualButNotSame($shared, $copy->collection['a']);
|
||||||
|
$this->assertSame($copy->collection['a'], $copy->collection['b']);
|
||||||
|
$this->assertSame($copy->collection['a'], $copy->direct);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_it_can_copy_graphs_whose_circular_reference_runs_through_an_array_object()
|
||||||
|
{
|
||||||
|
$parent = new stdClass();
|
||||||
|
$child = new stdClass();
|
||||||
|
|
||||||
|
$child->parent = $parent;
|
||||||
|
$parent->children = new ArrayObject([$child]);
|
||||||
|
|
||||||
|
$copy = deep_copy($parent);
|
||||||
|
|
||||||
|
$this->assertEqualButNotSame($child, $copy->children[0]);
|
||||||
|
$this->assertSame($copy, $copy->children[0]->parent);
|
||||||
|
}
|
||||||
|
|
||||||
public function test_it_can_copy_graphs_with_circular_references()
|
public function test_it_can_copy_graphs_with_circular_references()
|
||||||
{
|
{
|
||||||
$a = new stdClass();
|
$a = new stdClass();
|
||||||
|
|||||||
@@ -6,8 +6,8 @@ use ArrayObject;
|
|||||||
use DeepCopy\DeepCopy;
|
use DeepCopy\DeepCopy;
|
||||||
use DeepCopy\TypeFilter\Spl\ArrayObjectFilter;
|
use DeepCopy\TypeFilter\Spl\ArrayObjectFilter;
|
||||||
use PHPUnit\Framework\TestCase;
|
use PHPUnit\Framework\TestCase;
|
||||||
use Prophecy\Prophecy\ObjectProphecy;
|
|
||||||
use RecursiveArrayIterator;
|
use RecursiveArrayIterator;
|
||||||
|
use stdClass;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @author Dominic Tubach <dominic.tubach@to.com>
|
* @author Dominic Tubach <dominic.tubach@to.com>
|
||||||
@@ -21,28 +21,40 @@ final class ArrayObjectFilterTest extends TestCase
|
|||||||
*/
|
*/
|
||||||
private $arrayObjectFilter;
|
private $arrayObjectFilter;
|
||||||
|
|
||||||
/**
|
|
||||||
* @var DeepCopy|ObjectProphecy
|
|
||||||
*/
|
|
||||||
private $copierProphecy;
|
|
||||||
|
|
||||||
protected function setUp(): void
|
protected function setUp(): void
|
||||||
{
|
{
|
||||||
$this->copierProphecy = $this->prophesize(DeepCopy::class);
|
$this->arrayObjectFilter = new ArrayObjectFilter(new DeepCopy());
|
||||||
$this->arrayObjectFilter = new ArrayObjectFilter(
|
|
||||||
$this->copierProphecy->reveal()
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_it_deep_copies_an_array_object(): void
|
public function test_it_deep_copies_an_array_object(): void
|
||||||
{
|
{
|
||||||
$arrayObject = new ArrayObject(['foo' => 'bar'], ArrayObject::ARRAY_AS_PROPS, RecursiveArrayIterator::class);
|
$foo = new stdClass();
|
||||||
$this->copierProphecy->copy('bar')->willReturn('baz');
|
$foo->bar = 'baz';
|
||||||
|
|
||||||
/** @var \ArrayObject $newArrayObject */
|
$arrayObject = new ArrayObject(
|
||||||
|
['foo' => $foo],
|
||||||
|
ArrayObject::ARRAY_AS_PROPS,
|
||||||
|
RecursiveArrayIterator::class
|
||||||
|
);
|
||||||
|
|
||||||
|
/** @var ArrayObject $newArrayObject */
|
||||||
$newArrayObject = $this->arrayObjectFilter->apply($arrayObject);
|
$newArrayObject = $this->arrayObjectFilter->apply($arrayObject);
|
||||||
$this->assertSame(['foo' => 'baz'], $newArrayObject->getArrayCopy());
|
|
||||||
|
$this->assertNotSame($foo, $newArrayObject['foo']);
|
||||||
|
$this->assertEquals($foo, $newArrayObject['foo']);
|
||||||
$this->assertSame(ArrayObject::ARRAY_AS_PROPS, $newArrayObject->getFlags());
|
$this->assertSame(ArrayObject::ARRAY_AS_PROPS, $newArrayObject->getFlags());
|
||||||
$this->assertSame(RecursiveArrayIterator::class, $newArrayObject->getIteratorClass());
|
$this->assertSame(RecursiveArrayIterator::class, $newArrayObject->getIteratorClass());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_it_keeps_objects_shared_inside_the_array_object_shared(): void
|
||||||
|
{
|
||||||
|
$shared = new stdClass();
|
||||||
|
$arrayObject = new ArrayObject(['a' => $shared, 'b' => $shared]);
|
||||||
|
|
||||||
|
/** @var ArrayObject $newArrayObject */
|
||||||
|
$newArrayObject = $this->arrayObjectFilter->apply($arrayObject);
|
||||||
|
|
||||||
|
$this->assertNotSame($shared, $newArrayObject['a']);
|
||||||
|
$this->assertSame($newArrayObject['a'], $newArrayObject['b']);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user