Skip to content

Commit

Permalink
fix: try using __serialize when obj implements \Serializable
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Pedro Coutinho committed Apr 20, 2022
1 parent 2086428 commit 259c34e
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 10 deletions.
8 changes: 6 additions & 2 deletions src/DataBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 6 additions & 2 deletions src/Utilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
43 changes: 40 additions & 3 deletions tests/ConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -531,7 +532,7 @@ public function testCustomObject()
$this->assertSame($expectedCustom, $actualCustom);
}

public function testCustomSerializable()
public function testDeprecatedSerializable()
{
$expectedCustom = array(
"foo" => "bar",
Expand All @@ -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
Expand All @@ -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(
Expand Down
8 changes: 5 additions & 3 deletions tests/TestHelpers/CustomSerializable.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<?php namespace Rollbar\TestHelpers;
<?php

namespace Rollbar\TestHelpers;

class CustomSerializable implements \Serializable
{
Expand All @@ -11,7 +13,7 @@ public function __construct($data)

public function serialize()
{
return $this->data;
throw new \Exception("Not implemented");
}

public function unserialize(string $data)
Expand All @@ -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
Expand Down
21 changes: 21 additions & 0 deletions tests/TestHelpers/DeprecatedSerializable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php namespace Rollbar\TestHelpers;

class DeprecatedSerializable implements \Serializable
{
public $data;

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

public function serialize()
{
return $this->data;
}

public function unserialize(string $data)
{
throw new \Exception("Not implemented");
}
}
58 changes: 58 additions & 0 deletions tests/UtilitiesTest.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
<?php namespace Rollbar;

use Rollbar\Payload\Level;
use Rollbar\TestHelpers\CustomSerializable;
use Rollbar\TestHelpers\CycleCheck\ParentCycleCheck;
use Rollbar\TestHelpers\CycleCheck\ChildCycleCheck;
use Rollbar\TestHelpers\CycleCheck\ParentCycleCheckSerializable;
use Rollbar\TestHelpers\CycleCheck\ChildCycleCheckSerializable;
use Rollbar\TestHelpers\DeprecatedSerializable;

class UtilitiesTest extends BaseRollbarTest
{
Expand Down Expand Up @@ -173,4 +176,59 @@ public function testSerializeForRollbarNestingLevels()
$this->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']);
}
}

0 comments on commit 259c34e

Please sign in to comment.