III-4841 Fix queue routing keys #214

bertramakers · 2022-07-15T13:15:38Z

Fixed

Fixed queues being (re)declared by the consumers using the default # routing key, which resulted in all messages being sent to both the API and CLI queues.

Changed

Refactored and simplified the setup of the AMQP consumers so it is more readable (hopefully) and it was easier to inject the routing key config. Before it would have to go through multiple classes / methods, while now we can easily inject it directly.

Ticket: https://jira.uitdatabank.be/browse/III-4841

There has only been one implementation for years anyway

It was only protected because it was abstract before

… method

This reduces the number of properties and makes it easier to see at a glance what is happening in the constructor

And use the same order of methods in both the interface and the implementation

The extra check it did on the return type is no longer necessary since we got return types in PHP

The $callbacks property is technically public, but marked as internal and the phpdoc mentions to use is_consuming() instead

Don't talk to your friends' friends, and more loose coupling

It should not be used directly, only as a callback on the AMQPChannel class

We can still test it by storing the callback that is sent to AMQPChannel::basic_consume() in a property in the unit tests, and then call the callback ourselves

…ndant factory The factory should make it easy in theory to create multiple consumers based on some shared services/parameters. However it's only used in one place, where we can easily pass all of those arguments directly to EventBusForwardingConsumer itself

They were not shared anyway (add() was used instead of addShared())

It's copy-pasted from the abstract parent method

bertramakers · 2022-07-15T14:21:03Z

Since there is no one available for code review and this is a relatively urgent bugfix, I'm going to merge this without code review for now. Feel free to double-check everything when you are back @JonasVHG and/or @LucWollants , we can always make another PR for any comments.

bertramakers added 30 commits July 15, 2022 12:24

Boyscouting in AbstractConsumer

41e74e3

Boyscouting in EventBusForwardingConsumer

0f1011d

Merge AbstractConsumer into EventBusForwardingConsumer

1613364

There has only been one implementation for years anyway

Move private functions to the bottom

2cdf7df

Remove private method that's only used once

1957068

Use a default NullLogger to remove if checks

b996e95

Move message ack into try so we don't need the returns in the catch()

0a3617b

Don't split code into multiple lines if not necessary

eea61c4

Move protected function down

481860a

Make protected function private

bf82a44

It was only protected because it was abstract before

Remove public method that was only used in unit test for that method

ca49bba

Remove unused property

5332a06

Move all handle logic into handle() method

e9ff58b

Move ack logic into separate method

c5018f0

Catch any Throwable, not just Exception

601e4be

Move reject logic into separate method

686f8e3

Set context as a property so we don't need to pass it to every single…

bfce1a9

… method

Move channel setup inline in the constructor

a31fa9a

This reduces the number of properties and makes it easier to see at a glance what is happening in the constructor

Add return types to ConsumerInterface

17ed254

And use the same order of methods in both the interface and the implementation

ConsumeCommand boyscouting

e9574f3

Remove redundant getChannel() method on ConsumeCommand

fe30d3a

The extra check it did on the return type is no longer necessary since we got return types in PHP

Move private handleSignal() method inline

997b44e

Move private method down

9731cf3

Use public is_consuming() method instead of internal $callbacks property

bfd98f9

The $callbacks property is technically public, but marked as internal and the phpdoc mentions to use is_consuming() instead

Replace getChannel() with more specific methods

079a31f

Don't talk to your friends' friends, and more loose coupling

Remove consume() method from interface

d3f1e64

It should not be used directly, only as a callback on the AMQPChannel class

Make the consume() callback function private

c31600d

We can still test it by storing the callback that is sent to AMQPChannel::basic_consume() in a property in the unit tests, and then call the callback ourselves

Add types to EventBusForwardingConsumerTest

1c40d82

Move properties that can be local variables inline

7f8ffbe

Move $eventBusForwardingConsumer property to local variable

1a4049e

bertramakers added 14 commits July 15, 2022 14:52

Group creation of mocks in constructor together

ca564ec

Explain why we need to mock basic_consume() and how the tests work

5800318

Remove unused imports

9b1b589

Boyscouting™ in AmqpProvider

b9501f9

Move services that are only used in one place inline

e1cf7b8

They were not shared anyway (add() was used instead of addShared())

Make method that is only used in the class itself private

aee7002

Rename consumers() to getConsumers()

4304485

Remove useless comment

e0e0441

It's copy-pasted from the abstract parent method

Remove unused EventBusForwardingConsumerFactory and Delay classes

4927824

Make routingKey configurable on EventBusForwardingConsumer

eab6b85

Pass a routing key from the consumers config, if available

9956650

Use underscore for config parameter name, as is the convention

352ee67

Fix missing routingKey argument in tests

794b71a

bertramakers marked this pull request as ready for review July 15, 2022 14:19

bertramakers requested review from LucWollants and JonasVHG as code owners July 15, 2022 14:19

bertramakers merged commit a8eb989 into main Jul 15, 2022

bertramakers deleted the III-4841-fix-queue-routing-keys branch July 15, 2022 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

III-4841 Fix queue routing keys #214

III-4841 Fix queue routing keys #214

bertramakers commented Jul 15, 2022

bertramakers commented Jul 15, 2022

III-4841 Fix queue routing keys #214

III-4841 Fix queue routing keys #214

Conversation

bertramakers commented Jul 15, 2022

Fixed

Changed

bertramakers commented Jul 15, 2022