Added TransportNameResolver for sns and sqs for automatic transport r…

[robinlehrmann]

Hello :)

This will resolve #83

I thought it might be a good idea to parse the transports mapping in the messenger.yaml and map the transport with the eventSourceARN of a sqsEvent or the EventSubscriptionArn of a snsEvent with the configured transport.

With this solution the $transportName is not required anymore and I don't need to create one lambda per sqs queue for my use case :) Would be great if you can take a look at this @mnapoli

[tyx]

Hi !

I get your point but this is clearly a BC break according to your current implementation. We may continue to support old way and add possibility to introduce your concept that would fallback on old transportName if missing (and it would be nullable by defaut) ?

[mnapoli]

Could you add documentation to help understand this? Hard for me to understand right now TBH.

Also good point, we want to avoid BC breaks 👍

[robinlehrmann]

@tyx yes, this would be a BC break :) I added the $transportName again. I also added some tests as well, to make sure both ways (automatic detection and manually set) works as expected.

In an other merge request I would refactor the SnsConsumerTest if you agree and use Prophecy as well to have only one way of creating mocks. But this is for later. Could you take a look again?

@mnapoli Yes, sure. It's in the README.md file now, let me know if this is understandable :)

~~And by the way should I implement the transport detector for EventBridge as well because I forgot that one? :)~~ It seams to be not possible to do it with EventBridge, because there is no eventSource or something like this.

[robinlehrmann]

@tyx @mnapoli when are you planning to merge this? :)

[tyx]

Hi ! I'm just a bref user like you ;)

I'm sure Matthieu will do his best to review as soon as he has some free time

[robinlehrmann]

Hello @mnapoli I don't want to pressure, but can you take a look at this, it's quite urgent 😅 would be really nice if this can be merged. Thank you!

[SinaFetrat]

Hi @mnapoli , i was wondering if there is any update or plan about merging this? because I also have the same problem and this will help a lot to get rid of many lambdas

[mnapoli]

Merging since we have now 3 different users asking for it. It's always hard to judge how actually used/useful a feature is when not using it, I've made the mistake of merging stuff for 1% of users before and regretted it 😬

Also I have a rule not to merge stuff I don't understand or can't maintain. I'm breaking that rule here, I trust you guys to have reviewed and maintain it if you face issues 😅

[mnapoli]

I have no idea why but it seems GitHub Actions didn't run on this PR? On master PHPStan fails now, any idea if it's related to this PR?

[robinlehrmann]

I will take a look on this :) @mnapoli

[robinlehrmann]

@mnapoli I ignored the error here: https://github.com/brefphp/symfony-messenger/pull/85/files is this ok for you?