Skip to content

Commit

Permalink
feature #28247 [Messenger] Don't make EnvelopeItemInterface extend Se…
Browse files Browse the repository at this point in the history
…rializable (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Messenger] Don't make EnvelopeItemInterface extend Serializable

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes (on experimental API)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

`Serializable` is a broken interface, see e.g. https://externals.io/message/98834
I don't think we should force ppl to implement it anywhere.
Actually, it isn't required to be able to serialize an object, and it doesn't enforce making a class serializable (as the `serialize()` method can throw).

What was the purpose of the removed logic? ping @sroze @ogizanagi

Commits
-------

2beda89 [Messenger] Don't make EnvelopeItemInterface extend Serializable
  • Loading branch information
fabpot committed Aug 27, 2018
2 parents ff1727e + 2beda89 commit 93bb665
Show file tree
Hide file tree
Showing 10 changed files with 13 additions and 60 deletions.
1 change: 1 addition & 0 deletions UPGRADE-4.2.md
Expand Up @@ -87,6 +87,7 @@ FrameworkBundle
Messenger
---------

* `EnvelopeItemInterface` doesn't extend `Serializable` anymore
* The `handle` method of the `Symfony\Component\Messenger\Middleware\ValidationMiddleware` and `Symfony\Component\Messenger\Asynchronous\Middleware\SendMessageMiddleware` middlewares now requires an `Envelope` object to be given (because they implement the `EnvelopeAwareInterface`). When using these middleware with the provided `MessageBus`, you will not have to do anything. If you use the middlewares any other way, you can use `Envelope::wrap($message)` to create an envelope for your message.

Security
Expand Down
Expand Up @@ -16,6 +16,7 @@

/**
* Marker config for a received message.
*
* This is mainly used by the `SendMessageMiddleware` middleware to identify
* a message should not be sent if it was just received.
*
Expand All @@ -25,13 +26,4 @@
*/
final class ReceivedMessage implements EnvelopeItemInterface
{
public function serialize()
{
return '';
}

public function unserialize($serialized)
{
// noop
}
}
8 changes: 8 additions & 0 deletions src/Symfony/Component/Messenger/CHANGELOG.md
@@ -0,0 +1,8 @@
CHANGELOG
=========

4.2.0
-----

* `ValidationMiddleware::handle()` and `SendMessageMiddleware::handle()` now require an `Envelope` object
* `EnvelopeItemInterface` doesn't extend `Serializable` anymore
3 changes: 2 additions & 1 deletion src/Symfony/Component/Messenger/EnvelopeItemInterface.php
Expand Up @@ -13,12 +13,13 @@

/**
* An envelope item related to a message.
*
* This item must be serializable for transport.
*
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
*
* @experimental in 4.1
*/
interface EnvelopeItemInterface extends \Serializable
interface EnvelopeItemInterface
{
}
Expand Up @@ -35,24 +35,4 @@ public function getGroups()
{
return $this->groups;
}

public function serialize()
{
$isGroupSequence = $this->groups instanceof GroupSequence;

return serialize(array(
'groups' => $isGroupSequence ? $this->groups->groups : $this->groups,
'is_group_sequence' => $isGroupSequence,
));
}

public function unserialize($serialized)
{
list(
'groups' => $groups,
'is_group_sequence' => $isGroupSequence
) = unserialize($serialized, array('allowed_classes' => false));

$this->__construct($isGroupSequence ? new GroupSequence($groups) : $groups);
}
}
Expand Up @@ -24,7 +24,6 @@ public function testSerializable()
{
$config = new SerializerConfiguration(array(ObjectNormalizer::GROUPS => array('Default', 'Extra')));

$this->assertTrue(is_subclass_of(SerializerConfiguration::class, \Serializable::class, true));
$this->assertEquals($config, unserialize(serialize($config)));
}
}
15 changes: 0 additions & 15 deletions src/Symfony/Component/Messenger/Tests/Fixtures/AnEnvelopeItem.php
Expand Up @@ -15,19 +15,4 @@

class AnEnvelopeItem implements EnvelopeItemInterface
{
/**
* {@inheritdoc}
*/
public function serialize()
{
return '';
}

/**
* {@inheritdoc}
*/
public function unserialize($serialized)
{
// noop
}
}
Expand Up @@ -31,7 +31,6 @@ public function testConfig()

public function testSerializable()
{
$this->assertTrue(is_subclass_of(ValidationConfiguration::class, \Serializable::class, true));
$this->assertEquals($config = new ValidationConfiguration(array('Default', 'Extra')), unserialize(serialize($config)));
$this->assertEquals($config = new ValidationConfiguration(new GroupSequence(array('Default', 'Then'))), unserialize(serialize($config)));
}
Expand Down
Expand Up @@ -98,6 +98,6 @@ public function testEncodedWithSerializationConfiguration()
$this->assertArrayHasKey('type', $encoded['headers']);
$this->assertEquals(DummyMessage::class, $encoded['headers']['type']);
$this->assertArrayHasKey('X-Message-Envelope-Items', $encoded['headers']);
$this->assertSame('a:2:{s:75:"Symfony\Component\Messenger\Transport\Serialization\SerializerConfiguration";C:75:"Symfony\Component\Messenger\Transport\Serialization\SerializerConfiguration":59:{a:1:{s:7:"context";a:1:{s:6:"groups";a:1:{i:0;s:3:"foo";}}}}s:76:"Symfony\Component\Messenger\Middleware\Configuration\ValidationConfiguration";C:76:"Symfony\Component\Messenger\Middleware\Configuration\ValidationConfiguration":82:{a:2:{s:6:"groups";a:2:{i:0;s:3:"foo";i:1;s:3:"bar";}s:17:"is_group_sequence";b:0;}}}', $encoded['headers']['X-Message-Envelope-Items']);
$this->assertSame('a:2:{s:75:"Symfony\Component\Messenger\Transport\Serialization\SerializerConfiguration";O:75:"Symfony\Component\Messenger\Transport\Serialization\SerializerConfiguration":1:{s:84:"'."\0".'Symfony\Component\Messenger\Transport\Serialization\SerializerConfiguration'."\0".'context";a:1:{s:6:"groups";a:1:{i:0;s:3:"foo";}}}s:76:"Symfony\Component\Messenger\Middleware\Configuration\ValidationConfiguration";O:76:"Symfony\Component\Messenger\Middleware\Configuration\ValidationConfiguration":1:{s:84:"'."\0".'Symfony\Component\Messenger\Middleware\Configuration\ValidationConfiguration'."\0".'groups";a:2:{i:0;s:3:"foo";i:1;s:3:"bar";}}}', $encoded['headers']['X-Message-Envelope-Items']);
}
}
Expand Up @@ -31,16 +31,4 @@ public function getContext(): array
{
return $this->context;
}

public function serialize()
{
return serialize(array('context' => $this->context));
}

public function unserialize($serialized)
{
list('context' => $context) = unserialize($serialized, array('allowed_classes' => false));

$this->__construct($context);
}
}

0 comments on commit 93bb665

Please sign in to comment.