Skip to content

[Messenger] Add options validation to all transports #32575

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
Closed
weaverryan opened this issue Jul 17, 2019 · 0 comments
Closed

[Messenger] Add options validation to all transports #32575

weaverryan opened this issue Jul 17, 2019 · 0 comments
Labels
Feature Messenger
Milestone
5.1

Comments

@weaverryan
Copy link
Member

weaverryan commented Jul 17, 2019 •
edited
Loading

Description
Suppose the following config:

framework:
    messenger:
        transports:
            async:
                dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
                options:
                    queue_name: high

If MESSENGER_TRANSPORT_DSN uses the doctrine transport, this is great. But if you're using a different transport (e.g. amqp) then the queue_name is an invalid option. Currently, no exception is thrown - the option is just silently ignored. I think that all transports should strictly validate the options being passed to it.

Ref: symfony/symfony-docs#11967
@weaverryan weaverryan added this to the next milestone Jul 17, 2019
@Tobion Tobion mentioned this issue Jul 19, 2019
Merged
Tobion added a commit that referenced this issue Jul 19, 2019
@Tobion
minor #32624 [Messenger] fix transport_name option not passing valida…
950306a 
…tion (Tobion)

This PR was merged into the 4.3 branch.

Discussion
----------

[Messenger] fix transport_name option not passing validation

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #32567
| License       | MIT
| Doc PR        |

the doctrine transport connection validates the options and complains about this option, so we remove it before. the other transports will probably add the validation as well: #32575
the purpose of the option is for custom transport factories anyway.

Commits
-------

48408e3 [Messenger] fix transport_name option not passing validation
@spolischook spolischook mentioned this issue Aug 5, 2019
Closed
@nikophil nikophil mentioned this issue Dec 10, 2019
Merged
@fabpot fabpot closed this as completed Feb 4, 2020
fabpot added a commit that referenced this issue Feb 4, 2020
@fabpot
feature #34925 Messenger: validate options for AMQP and Redis Connect…
981a771 
…ions (nikophil)

This PR was squashed before being merged into the 5.1-dev branch (closes #34925).

Discussion
----------

Messenger: validate options for AMQP and Redis Connections

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| Ticket | #32575
| New feature?  | yes
| Deprecations? | yes
| License       | MIT

This PR validates options for AMQP and Redis connections.

Regarding AMQP, I've merged symfony's specific options (`exchange`, `delay`...) with the ones available in the [AMQP Extension](https://github.com/pdezwart/php-amqp/blob/master/amqp_connection.c#L177-L192) and i've enhanced the phpdoc with the one found [here](https://github.com/pdezwart/php-amqp/blob/master/stubs/AMQPConnection.php#L27-L54)

Commits
-------

bc4f7d7 Messenger: validate options for AMQP and Redis Connections
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Messenger
Projects
None yet
Milestone
5.1
Development

No branches or pull requests
5 participants
@fabpot @javiereguiluz @weaverryan @nicolas-grekas @Tobion