8000 [Messenger] Very naive serialization · Issue #33368 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Messenger] Very naive serialization #33368

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

Closed
pounard opened this issue Aug 28, 2019 · 9 comments
Closed

[Messenger] Very naive serialization #33368

pounard opened this issue Aug 28, 2019 · 9 comments

Comments

@pounard
Copy link
Contributor
pounard commented Aug 28, 2019

Symfony version(s) affected: 4.2, 4.3

When I look at the code of the Symfony\Component\Messenger\Transport\Serialization\Serializer class, it seems that the serialization format (as "format" in the symfony/serializer component semantics, but I'd prefer to call that "content type") is a property of the Serializer class, and it's fixed and always the same.

This means that:

  • you cannot handle more than one serialization format in the whole application, in all your buses,
  • you cannot handle external messages that would reach your node/application with different content types,
  • you cannot target different external applications that hypothetically would await for different messages content-types in an heterogeneous environment.

I might be missing something, but I'm reading the code, looking through the issues, and for the record using the messenger since 4.2 release (which means quite enough months) and I don't see anyway to come around this.

Worst thing is that if you let the default format be, have a live application, then change it by configuration for any reason at all, then deliver your new version, all stalling unprocessed yet messages will fail because they cannot be unserialized and you will loose them for eternity.

I can see some easy solutions, and all can be implemented at the same time:

  • add an optional $contentType parameter on the decode() function so that the receiver could pass an arbitrary content type, for example, AMQP transport might get the content type from the content-type message property (which is in the spec of the 0.9 and 1.0 versions),

  • in the encode() function add a content-type header alongside the type, and use that in decode() if present instead of the default one,

  • add an additionel ContentTypeStamp, this way API users can force a content type while publishing the message, which would be either kept in the encoded message, or converted to the content-type header in the encode() function.

If the three are present and conflict while decoding, arbitrarily define an order of preference on which to use, personally I'd go for "receiver given content type" (the real one it seems), "header content type" (serializer enforced) then "ContentTypeStamp" (should not be present anymore at decoding) as a last resort.

Additionally, depending upon the transport (for example AMQP, that's the only one I actually read part the spec and tested, I'm not an expert just an amateur with it) you could have content types such application/json, text/xml or amqp+some/other, but the serializer component is totally unable to handle those, so the messenger component should carry a mimetype to serializer format conversion routine.

In all cases it's a requirement that:

  • the receiver must be able to force the format the serializer will use,
  • the publisher in the form of the business code must be able to force a format (because it might actually be creating a message for a very strict target external application),
  • in case of configuration changes, already sent messages must not fail when loaded after that change.
@Tobion
Copy link
Contributor
Tobion commented Aug 29, 2019

This issue looks like a duplicate of #31230

you cannot handle more than one serialization format in the whole application, in all your buses
you cannot handle external messages that would reach your node/application with different content types
the receiver must be able to force the format the serializer will use,

All this can be solved with a custom serializer which can be configured by yourself. We can make this easier. It's just a question how. See also #32049

@pounard
Copy link
Contributor Author
pounard commented Aug 29, 2019

@Tobion thanks for answering. Both of #31230 and #32049 seem to couple my own two issues which are this one and #33369. I did searched in the issues quite some time but I didn't found those two, so yes maybe it's a duplicate.

I think each of those two problems can be solved independently.

EDIT: actually #33369 is a duplicate of #31230 but not this one.

@pounard
Copy link
Contributor Author
pounard commented Aug 29, 2019

As for #32049 it's a meta issue, it actually exposes 3 very different aspects of plugging the messenger in an heterogeneous environment.

8000
@pounard
Copy link
Contributor Author
pounard commented Aug 29, 2019

All this can be solved with a custom serializer which can be configured by yourself. We can make this easier. It's just a question how. See also #32049

I think this is a critical feature that should be in core, as a transport developer I should not care about the serialization/deserialization process, for example, how does the AMQP transport handles the content-type property ? Actually it seem to do when sending, but not when receiving. By providing an extra optional $contentType parameter to the SerializerInterface::decode() method, it allows all receiver to propagate and externally exposed message content type without the need of overriding the serializer or writing a new one.

I agree this would cause problems with the PhpSerializer, but IMHO it seems that this implementation has never been written to allow communication with the outside world, and I think that most people using the bus won't use it as soon as they have an external broker.

@OskarStark
Copy link
Contributor

@Nyholm, maybe I am wrong here, but you at happyr created something with serializing and versioning of messages IIRC.

@Nyholm
Copy link
Member
Nyholm commented Sep 2, 2019

Correct. I created a custom serializer that decouples the application from the actual message. See https://github.com/Happyr/message-serializer

I find @pounard idea of content negotiation interesting. Ie the serializer should look at the content type header and know what to do. However, that sounds awfully complex and would rarely ever be used. I mean, you are responsible of both the client and the server, so you always know what content-type you are using.

I would recommend someone writing their own implementation of serializer. It does not have to be complex. See https://github.com/Happyr/message-serializer/blob/master/src/Serializer.php

@OskarStark
Copy link
Contributor

Thanks for your feedback 👍🏻

@pounard
Copy link
Contributor Author
pounard commented Sep 3, 2019

I find @pounard idea of content negotiation interesting

Thanks. I wouldn't call that negotiation thought, if the transport gives a content-type, there's nothing to negotiate about, it's a fact the serializer must handle, in my opinion.

that sounds awfully complex and would rarely ever be used

It's not complex, I don't think so, transport gives a content type, you give it to the serializer component, the general idea is very simple.

It's only about detecting the content-type, normalizing it if necessary (that would be the job of the serializer itself actually, but that's another matter) and sending it along with the rest of the content.

At the beginning I started to think on how to implement it as a decorator, to keep the single object responsibility principle, but that's actually not possible since the default messenger serializer has one and only one content type configured at a time and will never propagate anything to the serializer component.

I mean, you are responsible of both the client and the server

Not if you plug it on a message broker that connects multiple applications. It seems like all messenger component developers tend to think that it will never be used to communicate with non Symfony applications ?

I would recommend someone writing their own implementation of serializer. It does not have to be complex

I agree that this must remain extensible, it's a good thing, but why the messenger component should not take care of content-type ? It's one single thing to care of.

It's a common concern, I think that having this per default will avoid many people the need to write their own implementation. The more people will have to write their own, the more bugs there will be into the wild. It really is boilerplate code that I'd like to not rewrite into each project, nor add a new dependency on each project for.

@Nyholm
Copy link
Member
Nyholm commented Jan 26, 2020

The current state of the messenger component gives you some great default values. The serializer is easy to get started with and "it just works".

When you want to communicate between different applications it will be far more complex and generic solutions are not good enough. The Messenger component will give you abstraction points where you can write custom logic. The Happyr Messenger Serializer gives you an example of that.

If one feel like one have a super smart serializer that should also be shipped with the Messenger component, feel free to submit a PR or open an issue with a specific description how it will work.

Im closing this issue because it is inactive for a while and because the discussion has not shown that something should be changed from the current behaviour. (Only open questions about how a new serializer may work has been discussed)

@Nyholm Nyholm closed this as completed Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
0