From b7a74c06f367c0dabefba8b897f150301115bad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Baptiste=20Clavi=C3=A9?= Date: Fri, 25 Dec 2015 22:22:41 +0100 Subject: [PATCH 1/4] Remove all traces of reflection in ObjectSnapshot --- src/Snapshot/ObjectSnapshot.php | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Snapshot/ObjectSnapshot.php b/src/Snapshot/ObjectSnapshot.php index eb19122..970487d 100644 --- a/src/Snapshot/ObjectSnapshot.php +++ b/src/Snapshot/ObjectSnapshot.php @@ -41,18 +41,15 @@ public function __construct($object) $this->raw = $object; $this->oid = spl_object_hash($object); - $refl = new ReflectionObject($object); - if (method_exists('\\ReflectionObject', 'isCloneable') && $refl->isCloneable()) { - $this->raw = clone $object; - } + $export = (array) $object; + $class = get_class($object); - /** @var \ReflectionProperty $reflProperty */ - foreach ($refl->getProperties() as $reflProperty) { - $reflProperty->setAccessible(true); - $value = $reflProperty->getValue($object); + foreach ($export as $property => $value) { + $property = str_replace("\x00*\x00", '', $property); // protected property + $property = str_replace("\x00{$class}\x00", '', $property); // private property - $this->data[$reflProperty->getName()] = $value; + $this->data[$property] = $value; } parent::normalize(); From bba0b0fcde7582b7b7023b63ace2b09d664466ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Baptiste=20Clavi=C3=A9?= Date: Sat, 26 Dec 2015 00:08:27 +0100 Subject: [PATCH 2/4] Add a test that tests all objects properties are exported --- test/Snapshot/ObjectSnapshotTest.php | 40 +++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/test/Snapshot/ObjectSnapshotTest.php b/test/Snapshot/ObjectSnapshotTest.php index 3beae19..52a35c1 100644 --- a/test/Snapshot/ObjectSnapshotTest.php +++ b/test/Snapshot/ObjectSnapshotTest.php @@ -58,9 +58,43 @@ public function testDeepConstructor($value) public function deepProvider() { - return [[(object) ['bar' => 'baz']], - [['bar' => 'baz']], - ['fubar']]; + return [ + 'with a sub-object' => [(object) ['bar' => 'baz']], + 'with a sub-array' => [['bar' => 'baz']], + 'with a scalar' => ['fubar'] + ]; + } + + public function testExportAllProperties() + { + $snapshot = new ObjectSnapshot(new Foo('foo', 'bar', 'baz')); + $data = $snapshot->getComparableData(); + + $this->assertCount(3, $data); + + $this->assertArrayHasKey('foo', $data); + $this->assertArrayHasKey('bar', $data); + $this->assertArrayHasKey('baz', $data); + + $this->assertSame('foo', $data['foo']); + $this->assertSame('bar', $data['bar']); + $this->assertSame('baz', $data['baz']); } } +// todo in php7 : use anon class ! +class Foo +{ + public $foo; + protected $bar; + private $baz; + + public function __construct($foo, $bar, $baz) + { + $this->foo = $foo; + $this->bar = $bar; + $this->baz = $baz; + } +} + + From ddd1028d958226231ab9df0d4f505a1c9b435871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Baptiste=20Clavi=C3=A9?= Date: Sat, 26 Dec 2015 00:13:13 +0100 Subject: [PATCH 3/4] Add some assertions for deep constructor on object snapshots --- test/Snapshot/ObjectSnapshotTest.php | 33 ++++++++++++++-------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/test/Snapshot/ObjectSnapshotTest.php b/test/Snapshot/ObjectSnapshotTest.php index 52a35c1..fbaf2f1 100644 --- a/test/Snapshot/ObjectSnapshotTest.php +++ b/test/Snapshot/ObjectSnapshotTest.php @@ -48,23 +48,6 @@ public function testConstructWithoutObject() new ObjectSnapshot([]); } - /** - * @dataProvider deepProvider - */ - public function testDeepConstructor($value) - { - new ObjectSnapshot((object) ['foo' => $value]); - } - - public function deepProvider() - { - return [ - 'with a sub-object' => [(object) ['bar' => 'baz']], - 'with a sub-array' => [['bar' => 'baz']], - 'with a scalar' => ['fubar'] - ]; - } - public function testExportAllProperties() { $snapshot = new ObjectSnapshot(new Foo('foo', 'bar', 'baz')); @@ -80,6 +63,22 @@ public function testExportAllProperties() $this->assertSame('bar', $data['bar']); $this->assertSame('baz', $data['baz']); } + + public function testDeepConstructor() + { + $object = new Foo( + (object) ['foo' => 'bar'], // object + ['baz' => 'fubar'], // array + 'fubaz' // scalar + ); + + $snapshot = new ObjectSnapshot($object); + $data = $snapshot->getComparableData(); + + $this->assertInstanceOf('Totem\\Snapshot\\ObjectSnapshot', $data['foo']); + $this->assertInstanceOf('Totem\\Snapshot\\ArraySnapshot', $data['bar']); + $this->assertNotInstanceOf('Totem\\AbstractSnapshot', $data['baz']); + } } // todo in php7 : use anon class ! From 4c5ac9c7459678dd9eeec1cf2f9c362b9ae598bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Baptiste=20Clavi=C3=A9?= Date: Sat, 26 Dec 2015 18:34:13 +0100 Subject: [PATCH 4/4] Use an array for str_replace instead of two str_replaces Micro-optimization ! https://blackfire.io/profiles/15cd1b15-4ee4-4b71-8b5a-5e1b16c53f8b/graph https://blackfire.io/profiles/3bdf788a-5b6f-4515-8769-3cf09947236f/graph --- src/Snapshot/ObjectSnapshot.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Snapshot/ObjectSnapshot.php b/src/Snapshot/ObjectSnapshot.php index 970487d..a97b021 100644 --- a/src/Snapshot/ObjectSnapshot.php +++ b/src/Snapshot/ObjectSnapshot.php @@ -46,8 +46,7 @@ public function __construct($object) $class = get_class($object); foreach ($export as $property => $value) { - $property = str_replace("\x00*\x00", '', $property); // protected property - $property = str_replace("\x00{$class}\x00", '', $property); // private property + $property = str_replace(["\x00*\x00", "\x00{$class}\x00"], '', $property); // not accessible properties $this->data[$property] = $value; }