Skip to content

Fix deep copying of ArrayObject in PHP 7.4 #145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/DeepCopy/DeepCopy.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@

namespace DeepCopy;

use ArrayObject;
use DateInterval;
use DateTimeInterface;
use DateTimeZone;
use DeepCopy\Exception\CloneException;
use DeepCopy\Filter\Filter;
use DeepCopy\Matcher\Matcher;
use DeepCopy\Reflection\ReflectionHelper;
use DeepCopy\TypeFilter\Date\DateIntervalFilter;
use DeepCopy\TypeFilter\Spl\ArrayObjectFilter;
use DeepCopy\TypeFilter\Spl\SplDoublyLinkedListFilter;
use DeepCopy\TypeFilter\TypeFilter;
use DeepCopy\TypeMatcher\TypeMatcher;
use ReflectionObject;
use ReflectionProperty;
use DeepCopy\Reflection\ReflectionHelper;
use SplDoublyLinkedList;

/**
Expand Down Expand Up @@ -59,6 +61,7 @@ public function __construct($useCloneMethod = false)
{
$this->useCloneMethod = $useCloneMethod;

$this->addTypeFilter(new ArrayObjectFilter($this), new TypeMatcher(ArrayObject::class));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to register it only on PHP 7.4+?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason for not using it in PHP <7.4, do you?

$this->addTypeFilter(new DateIntervalFilter(), new TypeMatcher(DateInterval::class));
$this->addTypeFilter(new SplDoublyLinkedListFilter($this), new TypeMatcher(SplDoublyLinkedList::class));
}
Expand Down
36 changes: 36 additions & 0 deletions src/DeepCopy/TypeFilter/Spl/ArrayObjectFilter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
namespace DeepCopy\TypeFilter\Spl;

use ArrayObject;
use DeepCopy\DeepCopy;
use DeepCopy\TypeFilter\TypeFilter;

/**
* In PHP 7.4 the storage of an ArrayObject isn't returned as
* ReflectionProperty. So we deep copy its array copy.
*/
final class ArrayObjectFilter implements TypeFilter
{
/**
* @var DeepCopy
*/
private $copier;

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

/**
* {@inheritdoc}
*/
public function apply($arrayObject)
{
return new ArrayObject(
$this->copier->copy($arrayObject->getArrayCopy()),
$arrayObject->getFlags(),
$arrayObject->getIteratorClass()
);
}
}

14 changes: 14 additions & 0 deletions tests/DeepCopyTest/DeepCopyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace DeepCopyTest;

use ArrayObject;
use DateInterval;
use DateTime;
use DateTimeImmutable;
Expand All @@ -24,6 +25,7 @@
use DeepCopy\TypeFilter\ShallowCopyFilter;
use DeepCopy\TypeMatcher\TypeMatcher;
use PHPUnit\Framework\TestCase;
use RecursiveArrayIterator;
use SplDoublyLinkedList;
use stdClass;
use function DeepCopy\deep_copy;
Expand Down Expand Up @@ -385,6 +387,18 @@ public function test_it_uses_the_first_filter_matching_for_copying_object_proper
$this->assertNull($copy->getAProp()->cloned);
}

public function test_it_can_deep_copy_an_array_object()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure: this test fails on PHP 7.4 if the new ArrayObjectFilter is not registered?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, without ArrayObjectFilter $foo and $copy['foo'] would be same.

{
$foo = new f003\Foo('foo');
$foo->setProp('bar');
$object = new ArrayObject(['foo' => $foo, \ArrayObject::ARRAY_AS_PROPS, \RecursiveArrayIterator::class]);

$copy = deep_copy($object);

$this->assertEqualButNotSame($object, $copy);
$this->assertEqualButNotSame($foo, $copy['foo']);
}

/**
* @ticket https://github.com/myclabs/DeepCopy/pull/49
*/
Expand Down
48 changes: 48 additions & 0 deletions tests/DeepCopyTest/TypeFilter/Spl/ArrayObjectFilterTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php declare(strict_types=1);

namespace DeepCopyTest\TypeFilter\Spl;

use ArrayObject;
use DeepCopy\DeepCopy;
use DeepCopy\TypeFilter\Spl\ArrayObjectFilter;
use PHPUnit\Framework\TestCase;
use Prophecy\Prophecy\ObjectProphecy;
use RecursiveArrayIterator;

/**
* @author Dominic Tubach <[email protected]>
*
* @covers \DeepCopy\TypeFilter\Spl\ArrayObjectFilter
*/
final class ArrayObjectFilterTest extends TestCase
{
/**
* @var ArrayObjectFilter
*/
private $arrayObjectFilter;

/**
* @var DeepCopy|ObjectProphecy
*/
private $copierProphecy;

protected function setUp(): void
{
$this->copierProphecy = $this->prophesize(DeepCopy::class);
$this->arrayObjectFilter = new ArrayObjectFilter(
$this->copierProphecy->reveal()
);
}

public function test_it_deep_copies_an_array_object(): void
{
$arrayObject = new ArrayObject(['foo' => 'bar'], ArrayObject::ARRAY_AS_PROPS, RecursiveArrayIterator::class);
$this->copierProphecy->copy(['foo' => 'bar'])->willReturn(['copy' => 'bar']);

/** @var \ArrayObject $newArrayObject */
$newArrayObject = $this->arrayObjectFilter->apply($arrayObject);
$this->assertSame(['copy' => 'bar'], $newArrayObject->getArrayCopy());
$this->assertSame(ArrayObject::ARRAY_AS_PROPS, $newArrayObject->getFlags());
$this->assertSame(RecursiveArrayIterator::class, $newArrayObject->getIteratorClass());
}
}