8000 [Messenger] New transport.serializer configuration by nikophil · Pull Request #49256 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] New transport.serializer configuration #49256

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: 7.3
Choose a base branch
from
Open

[Messenger] New transport.serializer configuration #49256

wants to merge 1 commit into from

Conversation

nikophil
Copy link
Contributor
@nikophil nikophil commented Feb 6, 2023 •
edited
Loading
Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #49081 #49094
License MIT
Doc PR waiting for this PR approval :)

Hello,

here is a proposal for Messenger, in order to improve processing for messages incoming into the app (ie: messages produced by another app) and messages outgoing from the app (ie: messages produced by our app but intended to be consumed by other apps).

Problem with incoming messages: (see #49094)

If messages are incoming in the app under different formats (json, xml, whatever) there is no easy way to customize per transport in which format the transport will deserialize the message. Same problem occurs for serialization context. Another annoyance is that we need to create a custom serializer class to determine in which class we want to deserialize the message.

This PR proposes a new configuration to handle this:

messenger:
    transports:
        transport_for_incoming_messages:
            dsn: 'null://'

            # when used, will use a new class "IncomingMessageSerializer", along with Symfony's serializer
            incoming_message_serializer:
                # one of these are required:
                messageClass: App\Some\Message
                messageClassResolver: 'service_id' # should implement a new interface "IncomingMessageClassResolverInterface"

                format: 'json' # defaults on "messenger.serializer.symfony_serializer.format"
                context: {some: context} # if empty, defaults on "messenger.serializer.symfony_serializer.context"

                # this option could be useful when dealing with complex messages which needs specific serializer service
                serializer: 'my_fancy_serializer' # defaults on "@serializer"

Problem with outgoing messages: (see #49081)

Messages which are made to be processed by other apps should be sanitized: stamps and header type must be removed, otherwise they could create errors in other apps (ie: if the message class does not have the same FQCN in both apps, or if the bus' name in sender app does not exist in receiver app: both of these cases will trigger errors when processing messages in the receiver app).
Outgoing messages may also be required to have specific format/context, so we must be able to configure format, context and serializer service used.

messenger:
    transports:
        transport_for_outgoing_messages:
            dsn: 'null://'

            # when used, will use a new class "OutgoingMessageSerializer", along with Symfony's serializer
            # all parameters are optional
            outgoing_message_serializer:
                format: 'json' # defaults on "messenger.serializer.symfony_serializer.format"
                context: {some: context} # if empty, defaults on "messenger.serializer.symfony_serializer.context"
                serializer: 'my_fancy_serializer' # defaults on "@serializer"

IMHO, both of these cases are such common, that they should not require boilerplate in userland.

At the end, the PR also provides the ability to add serialization context/format/service per transport, for all other cases:

messenger:
    transports:
        my_transport:
            dsn: 'null://'

            # short notation "serializer: custom_transport_serializer" still works
            serializer:
                # if one of these options are provided, 
                # the transport will use "\Symfony\Component\Messenger\Transport\Serialization\Serializer"
                format: 'json' # defaults on "messenger.serializer.symfony_serializer.format"
                context: {some: context} # if empty, defaults on "messenger.serializer.symfony_serializer.context"
                serializer: 'my_fancy_serializer' # defaults on "@serializer"

                # previous options are incompatible with the following
                serializer_id: custom_transport_serializer

Options incoming_message_serializer, outgoing_message_serializer and serializer are incompatible

Another solution for the problem would have been to keep only one serializer option, along with a type option (normal|incoming|outgoing|custom). But the allowed underlying options and behavior really differ from one type to another, that I preferred using 3 different options.

@carsonbot carsonbot added this to the 6.3 milestone Feb 6, 2023
@nikophil nikophil force-pushed the messenger/transports-serializers branch from e1c135d to 9bb50fb Compare February 6, 2023 08:17
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@mdeboer
Copy link
Contributor
mdeboer commented Jun 23, 2023

I might look into this a bit more but why incoming/outgoing? Isn't that why you use a serialize/deserialize method in one serializer?

I mean we already use a single serializer that does this. I don't see why you would ever want two?

@nikophil
Copy link
Contributor Author
nikophil commented Jun 26, 2023 •
edited
Loading

hi @mdeboer

I'm afraid I don't fully understand your questions, but I'll try to answer them:

I might look into this a bit more but why incoming/outgoing?

because we don't need to do the same actions, based on if the message is coming from outside of the application ("incoming" mesage) or is going to another app ("outgoing" message). I've explained the difference in the PR description.

isn't that why you use a serialize/deserialize method in one serializer?

I don't understand this question 🤔

I mean we already use a single serializer that does this. I don't see why you would ever want two?

I saw several apps where multiple serializers are defined. Mainly for performance issues, and not having one big monster-serializer which has tons of normalizers which take time to resolve.

Anyway, being able to chose another service than @serializer is not the main purpose of this PR. Its main purpose is to:

  • allow to easily define the "serialization context" per transport (ie: format + context)
  • ease deserialization process of incoming messages, by allowing to configure the messageClass (or messageClassResolver)
  • ease serialization process of outgoing messages, by cleaning the message (ie: remove some stamps and headers which can create problems if the receiver application is a Symfony app)

the current implementation proposes these new nodes incoming_message_serializer and outgoing_message_serializer in messenger's configuration, but I'm opened to other idea to simplify the conf :)

@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature Messenger Status: Needs Review
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Messenger] Sanitize message when the receiver is another app
6 participants
@nikophil @mdeboer @fabpot @nicolas-grekas @xabbuh @carsonbot
0