Replace IReadOnlyBasicProperties, IAmqpHeader generic type constraints in IChannel.BasicPublishAsync() with a single interface constraint.

[Miguel-Rodrigues]

Is your feature request related to a problem? Please describe.

Currently it is not possible to create unit tests for code that consumes the RabbitMQ.Client.IChannelExtensions.BasicPublishAsync(...) extensions, that use the RabbitMQ.Client.Impl.EmptyBasicProperty.Empty static instance.

Since RabbitMQ.Client.IChannel.BasicPublishAsync(...) methods require the parameter TProperties basicProperties to be both IReadOnlyBasicProperties and IAmqpHeader I cannot create an argument type matcher that satisfies both interfaces requirements at the same time to the matching instance type (that I'm aware of). It needs to be a EmptyBasicProperty class, that currently is defined as a internal. Creating an interface interface IProperties : IReadOnlyBasicProperties, IAmqpHeader { } will not suffice, as it does not match the TProperties parameter type at runtime.

The only way that I'm aware of to make the code testable is to use our own copy implementation of the EmptyBasicProperty.Empty type which makes the purpose of both extensions and the Empty instance pointless.

Describe the solution you'd like

Define an interface type that implements both interfaces and use it as the constraint instead of having two constraints. Something like

public interface IProperties : IReadOnlyBasicProperties, IAmqpHeader { }

ValueTask BasicPublishAsync<TProperties>(string exchange, string routingKey,
    bool mandatory, TProperties basicProperties, ReadOnlyMemory<byte> body,
    CancellationToken cancellationToken = default)
    where TProperties : IProperties;

Describe alternatives you've considered

Alternativelly make the EmptyBasicProperty.Empty type visible to the outside, by defining it as public static sealed RabbitMQ.Client.EmptyBasicProperty.Empty. The code base can still be at RabbitMQ.Client.Impl and internal.

namespace RabbitMQ.Client
{
    public static class EmptyBasicProperty
    {
        public static EmptyBasicProperty Empty => new RabbitMQ.Client.Impl.EmptyBasicProperty();
    } 
}

Additional context

No response

[lukebakken]

Hello, thanks for using this library.

If you could provide a pull request with what you need, that is backwards-compatible with the version 7 API, and includes tests, I'd be happy to include it with version 7.2.0 of this library. Thanks.

In addition, I would be curious to see what you are trying to accomplish with the library as it now stands - one of the tests you are trying to write that doesn't work.

[Miguel-Rodrigues]

Hello @lukebakken.
To be backwards compatible I would use the latter option as it is would not affect the existing source.
Once I have some time free I can work on it.

As for the second question, I'm working on updating an example project that I used on a previous job application. And I'm still using it as boilerplate project for other work. Specifically for this context, I have a service that sends a message to the default exchange informing other subscribed services when a forex rate has been updated.

I'll be using the workaround or try finding alternatives for this as I was not able yet to fix my code after the latest major changes.
The code on the repo is not up to date yet...
https://github.com/Miguel-Rodrigues/ExchangeRateManager
https://github.com/Miguel-Rodrigues/ExchangeRateManager/blob/main/src/ExchangeRateManager.Services/RabbitMessageQueueService.cs

[lukebakken]

I took a few minutes to review that repository, as well as try to update to version 7.1.2 of this library. I'm not yet sold that any change needs to happen to this library. What I'm going to do is convert this issue to a discussion, where we can continue looking at your code.

[lukebakken]

Here's my $0.02... I don't see the point of tests like these:

https://github.com/Miguel-Rodrigues/ExchangeRateManager/blob/main/src/ExchangeRateManager.UnitTests/Services/RabbitMessageQueueServiceTests.cs

If you're going to test BasicPublishAsync (in version 7), you should be using a real connection to RabbitMQ.

So, at this point, please provide a concrete example of what you can't accomplish using version 7.1.2 of this library. Let me know when your repo has the necessary changes.

[Miguel-Rodrigues]

This test is just for coverage. The idea is not to test BasicPublishAsync, but to make sure that the service will call this method. Note that the IModel and IConnection interfaces are mocked. For actual Integration tests I'm thinking of using TestContainers for that purpose. For for other tests and development purposes I have a stub class that with that implements the same service.

Meanwhile I'll be looking for the documentation to understand the differences with version 7 to see how can I keep the same logic.

[Miguel-Rodrigues]

After checking the documentation I can just send as null.

I appreciate your time and my apologies to bother...
Best regards.

[Miguel-Rodrigues]

Although.. I'm not sure, but I think documentation may not be correct. When using the same example it gives the error of The TProperties type cannot be inferred by using null:

[lukebakken]

Please point out where the incorrect documentation is, and provide code that does not work as you expect, or at least point it out in your repository.

[Miguel-Rodrigues]

I pushed my changes and I was able to make it work by just initializing the properties with a new BasicProperties().

At https://www.rabbitmq.com/client-libraries/dotnet-api-guide#publishing, the documentation states:

To publish a message to an exchange, use IChannel.BasicPublishAsync as follows:

byte[] messageBodyBytes = System.Text.Encoding.UTF8.GetBytes("Hello, world!");
await channel.BasicPublishAsync(exchangeName, routingKey, false, null, messageBodyBytes);

On my code, the service RabbitMessageQueueService.cs, at line 27:

await channel.BasicPublishAsync(
    string.Empty, queue, false, new BasicProperties(),
    message.ToUTF8JsonByteArray(),
    cancellationToken);

If a null is used instead of the new BasicProperties() like:

await channel.BasicPublishAsync(
    string.Empty, queue, false, null,
    message.ToUTF8JsonByteArray(),
    cancellationToken);

Results in a CS0411 compiler error:

The type arguments for method 'IChannel.BasicPublishAsync(string, string, bool, TProperties, ReadOnlyMemory, CancellationToken)' cannot be inferred from the usage. Try specifying the type arguments explicitly. ExchangeRateManager.Services

[lukebakken]

Thanks! Yep, the documentation needs to be updated. I suspect that's a hold-over from version 6 of this library. Thank you for all of the feedback.

[lukebakken]

@Miguel-Rodrigues - rabbitmq/rabbitmq-website#2219