Adding support for BasicNack in the RabbitMQBasicConsumer

[aminsrz]

Description:

Added a BasicNack method to the RabbitMQBasicConsumer class besides and similar to the already existing BasicAck and BasicReject. Also made the methods virtual and the the Semaphore protected so the RabbitMQBasicConsumer can be more extensible.

Changes:

• Added a BasicNack method to the RabbitMQBasicConsumer class.
• BasicAck, BasicReject and BasicNack methods are now virtual.
• The Semaphore in that class is now protected so the derived classes have more flexibility to override the methods.

Affected components:

• RabbitMQBasicConsumer

Checklist:

• I have tested my changes locally
• I have added necessary documentation (if applicable)
• I have updated the relevant tests (if applicable)
• My changes follow the project's code style guidelines

[yang-xiaodong]

Can you describe the motivation for this change?

[aminsrz]

Can you describe the motivation for this change?

• The main change is adding support for BasicNack which is another way (other than BasicReject) to negatively acknowledge a message in RabbitMQ. It is explained here
• Another way this could be achieved was to override the BasicReject method of the RabbitMQBasicConsumer class but the acknowledge methods were not virtual which they are now, and also more importantly, the the semaphore was private which severely limited the extensibility of that whole class. I also made that protected.

[yang-xiaodong]

Thank you for your PR and for the time and effort you have invested in this project. After review, we are unable to accept this PR at the moment for the following reasons:

1. CAP currently does not require batch message rejection: BasicReject is sufficient for our needs, and the added BasicNack method does not currently have a use case in the project.
2. The virtual modification in RabbitMQBasicConsumer: This class is not injected into the container and is instead initialized via new in RabbitMQConsumerClient, which does not currently support extension.

We truly appreciate your contribution and look forward to more submissions from you in the future!

[aminsrz]

I have a real requirement to use nack instead of reject even if "batch message rejection" is not supported right now. But currently, to make this simple and trivial change, one would have to create the exact copy of RabbitMQBasicConsumer, RabbitMQConsumerClient and RabbitMQConsumerClientFactory with just the BasicNack method added and used, and then somehow replace the injected RabbitMQConsumerClientFactory with the new one. I am sure that you agree that this is not ideal and a maintainability nightmare.

It it your choice obviously to keep the RabbitMQ support to the minimum needed feature set, but it is definitely not sufficient for a lot of use-cases. I agree with you that a better extensibility support (maybe like the way it is in the main CAP library) can be a solution to this, but as it is right now, the CAP.RabbitMQ library is far from that state. I thought this PR is a step towards that direction with the improvements to the RabbitMQBasicConsumer class for extensibility. The PR was limited to that class and I honestly don't get why an improvements to some parts of the code should be rejected because other parts (RabbitMQConsumerClient) are not "ready".

If you are not happy with all the changes in this PR, I think at least making the semaphore protected hugely boosts the extensibility. Or if extensibility is not the goal for this library for some reason, adding a simple BasicNack method with "batch message rejection" flag as false, adds no harm or complexity, but might make the life of some of the users of CAP a little bit easier.