From 259c34e7e185d92cc49a25475a225b520caf34b2 Mon Sep 17 00:00:00 2001 From: Pedro Coutinho Date: Wed, 20 Apr 2022 17:31:55 +0100 Subject: [PATCH] fix: try using __serialize when obj implements \Serializable The Serializable interface is now being deprecated in favour of using the magic methods __serialize() and __unserialize(). Some packages decided to start throwing exceptions on their Serializable implementations if the interface methods were called. This commit tries to check if the classes that implement Serializable are already using __serialize() and if they are, it calls that method, instead of the interface one serialize(). If this is the case, we also hide the deprecation notice, since it seems the client code is already preparing to drop \Serializable when the time comes. This should fix issue #136 on rollbar/rollbar-php-laravel --- src/DataBuilder.php | 8 ++- src/Utilities.php | 8 ++- tests/ConfigTest.php | 43 ++++++++++++++- tests/TestHelpers/CustomSerializable.php | 8 ++- tests/TestHelpers/DeprecatedSerializable.php | 21 +++++++ tests/UtilitiesTest.php | 58 ++++++++++++++++++++ 6 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 tests/TestHelpers/DeprecatedSerializable.php diff --git a/src/DataBuilder.php b/src/DataBuilder.php index 80a11587..a074354b 100644 --- a/src/DataBuilder.php +++ b/src/DataBuilder.php @@ -1015,10 +1015,14 @@ protected function resolveCustomContent(mixed $custom): array { // This check is placed first because it should return a string|null, and we want to return an array. if ($custom instanceof \Serializable) { - trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED); // We don't return this value instead we run it through the rest of the checks. The same is true for the // next check. - $custom = $custom->serialize(); + if (method_exists($custom, '__serialize')) { + $custom = $custom->__serialize(); + } else { + trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED); + $custom = $custom->serialize(); + } } else { if ($custom instanceof SerializerInterface) { $custom = $custom->serialize(); diff --git a/src/Utilities.php b/src/Utilities.php index bc1ae4af..b8b0c4f9 100644 --- a/src/Utilities.php +++ b/src/Utilities.php @@ -147,9 +147,13 @@ private static function serializeObject( $serialized = self::circularReferenceLabel($obj); } else { if ($obj instanceof \Serializable) { - trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED); self::markSerialized($obj, $objectHashes); - $serialized = $obj->serialize(); + if (method_exists($obj, '__serialize')) { + $serialized = $obj->__serialize(); + } else { + trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED); + $serialized = $obj->serialize(); + } } elseif ($obj instanceof SerializerInterface) { self::markSerialized($obj, $objectHashes); $serialized = $obj->serialize(); diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index e23396f8..b4bf47d4 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -12,6 +12,7 @@ use Rollbar\Defaults; use Rollbar\TestHelpers\CustomSerializable; +use Rollbar\TestHelpers\DeprecatedSerializable; use Rollbar\TestHelpers\Exceptions\SilentExceptionSampleRate; use Rollbar\TestHelpers\Exceptions\FiftyFiftyExceptionSampleRate; use Rollbar\TestHelpers\Exceptions\FiftyFityChildExceptionSampleRate; @@ -531,7 +532,7 @@ public function testCustomObject() $this->assertSame($expectedCustom, $actualCustom); } - public function testCustomSerializable() + public function testDeprecatedSerializable() { $expectedCustom = array( "foo" => "bar", @@ -540,7 +541,7 @@ public function testCustomSerializable() $config = new Config(array( "access_token" => $this->getTestAccessToken(), "environment" => $this->env, - "custom" => new CustomSerializable($expectedCustom), + "custom" => new DeprecatedSerializable($expectedCustom), )); // New error handler to make sure we get the deprecated notice @@ -566,7 +567,43 @@ public function testCustomSerializable() $this->assertSame($expectedCustom, $actualCustom); } - + + public function testCustomSerializable() + { + $expectedCustom = array( + "foo" => "bar", + "fuzz" => "buzz" + ); + $config = new Config(array( + "access_token" => $this->getTestAccessToken(), + "environment" => $this->env, + "custom" => new CustomSerializable($expectedCustom), + )); + + // Make sure the deprecation notice is not sent if the object implements __serializable as it should + set_error_handler(function ( + int $errno, + string $errstr, + ) : bool { + $this->assertStringNotContainsString("Serializable", $errstr); + $this->assertStringNotContainsString("deprecated", $errstr); + return true; + }, E_USER_DEPRECATED); + + $result = $config->getDataBuilder()->makeData( + Level::INFO, + "Test message with custom data added dynamically.", + array(), + ); + + // Clear the handler, so it does not mess with other tests. + restore_error_handler(); + + $actualCustom = $result->getCustom(); + + $this->assertSame($expectedCustom, $actualCustom); + } + public function testMaxItems() { $config = new Config(array( diff --git a/tests/TestHelpers/CustomSerializable.php b/tests/TestHelpers/CustomSerializable.php index 6c47cc89..589ba11f 100644 --- a/tests/TestHelpers/CustomSerializable.php +++ b/tests/TestHelpers/CustomSerializable.php @@ -1,4 +1,6 @@ -data; + throw new \Exception("Not implemented"); } public function unserialize(string $data) @@ -21,7 +23,7 @@ public function unserialize(string $data) public function __serialize(): array { - throw new \Exception("Not implemented"); + return $this->data; } public function __unserialize(array $data): void diff --git a/tests/TestHelpers/DeprecatedSerializable.php b/tests/TestHelpers/DeprecatedSerializable.php new file mode 100644 index 00000000..f3890a29 --- /dev/null +++ b/tests/TestHelpers/DeprecatedSerializable.php @@ -0,0 +1,21 @@ +data = $data; + } + + public function serialize() + { + return $this->data; + } + + public function unserialize(string $data) + { + throw new \Exception("Not implemented"); + } +} diff --git a/tests/UtilitiesTest.php b/tests/UtilitiesTest.php index 384e22ca..3c8ce764 100644 --- a/tests/UtilitiesTest.php +++ b/tests/UtilitiesTest.php @@ -1,9 +1,12 @@ assertArrayHasKey('three', $result['one']['two']); $this->assertArrayHasKey('four', $result['one']['two']['three']); } + + public function testSerializationOfDeprecatedSerializable() + { + $data = ['foo' => 'bar']; + + $obj = array( + "serializedObj" => new DeprecatedSerializable($data), + ); + $objectHashes = array(); + + // Make sure the deprecation notice is sent if the object implements deprecated Serializable interface + set_error_handler(function ( + int $errno, + string $errstr, + ) : bool { + $this->assertStringContainsString("Serializable", $errstr); + $this->assertStringContainsString("deprecated", $errstr); + return true; + }, E_USER_DEPRECATED); + + $result = Utilities::serializeForRollbar($obj, null, $objectHashes); + + // Clear the handler, so it does not mess with other tests. + restore_error_handler(); + + $this->assertEquals(['foo' => 'bar'], $result['serializedObj']); + } + + public function testSerializationOfCustomSerializable() + { + $data = ['foo' => 'bar']; + + $obj = array( + "serializedObj" => new CustomSerializable($data), + ); + $objectHashes = array(); + + // Make sure the deprecation notice is NOT sent if the object implements Serializable but it's using + // __serialize and __unserialize properly + set_error_handler(function ( + int $errno, + string $errstr, + ) : bool { + $this->assertStringNotContainsString("Serializable", $errstr); + $this->assertStringNotContainsString("deprecated", $errstr); + return true; + }, E_USER_DEPRECATED); + + $result = Utilities::serializeForRollbar($obj, null, $objectHashes); + + // Clear the handler, so it does not mess with other tests. + restore_error_handler(); + + $this->assertEquals(['foo' => 'bar'], $result['serializedObj']); + } }