There are a lot of integration tests, but we should add some unit tests for some of the more important classes.

In the same way that we have a BusClient that only wraps the generic BusClient<MessageContext>, we should have an interface IDefaultBusClient that wraps IBusClient<TMessageContext>

If not provided, these defaults should be used:
Port: 5672
Username: guest
Password: guest
VirtualHost: /

For example it should be possible to just pass localhost as a connection string which should be treated as guest:guest@localhost:5672/

Are there any thoughts about supporting DNX core 5.0?

When doing RPC calls in quick succession the consumer is removed and a new one is not created and the KeyNotFoundException is thrown.

I've pinpointed the problem to RawRabbit.Operations.Requester.cs in the GetOrCreateConsumerForType method. There is a check to verify that _typeToConsumer contains the correct response type but the _disposeConsumerTimer is triggered before it is actually used.

This may be related to Issue #36

The main purpose of this is to have a simple way to unsubscribe from a Subscribe/Respond

RawRabbit needs logging - and the future of dot.net logging lays in Microsoft.Framework.Logging.

The ambition is to keep all pre-release dependencies (like those for vNext beta8) in RawRabbit.vNext, so that's where the wire-up for the logging goes. However, RawRabbit can get a temporary interface IRawLogger that is one-to-one with the ILogger interface so that in a vNext world, you just resolve the ILogger and injects it in the IRawLogger adapter and otherwise you just have to implement the IRawLogger with you favourite logging framework and register it in BusClientFactory.

It should be simple to use Lazy Queues. Therefore we should

• Add QueueArgument for lazy queue
• Add <summary> comments on arguments
• Add section in documentation

In the Configuration section of README.md, there's some instructions on how the connectionString should look like. However, the parser is really strict and if you for example don't add a semicolon (;) efter the last broker, that broker won't be matched. The port number should also be optional and default to :5672 and the virutal host default to /. Below is a list if what should be considered valid connectionStrings:

• brokers=guest:guest@localhost (no port number or vhost)
• brokers=guest:guest@localhsot:5672/ (port number and vhost)
• brokers=user1:pass1@host/vhost,user2:pass2@host2:5672 (vhost on first, port on second)
• requestTimeout=10;brokers=guest:guest@localhost (requestTimeout before brokers)

There are three channels open on Respond. This is because consecutive calls are made to IChannelFactory.GetChannel.

In some scenarios, it makes sense to track a Message Sequence through its GlobalMessageId. It should be possible to do so with #.[globalmessageid].

In order for this to work, we need to

• Change default type of exchange to Topic to leverage wild card routing
• Add a configuration option, RouteWithGlobalId, so that we don't break compatibility with previous versions

We should give some options to the default ConsoleLogger, like adding an adapter to Serilog

The default virtual host should be /, but a different host, like /foo shouldn't actually have the slash, but just foo

Currently, in Requester.cs, there's a line of code where we expect a dictionary to contain a certain key

_requestTimerDictionary[args.BasicProperties.CorrelationId]?.Dispose();

If the response timer has expired, or adding a new entry in the dictionary failed that key may not exists. This should be handled more gracious.

Up until this point, testing involving unstable connections has been done manually. It would be neat to add this to our integration test suit. A few suggestions for test:

• Make sure that connection and topology is properly restored after connection shutdown
• Other?

There's one line of code that have been bugging me for a couple of days, where we do .Wait() on the message handler for Responder. By replacing this with a chained ContinueWith, we can achieve higher performance for Responder when under heavy work load.

Generic types like GenericType<TFirst, TSecond> have a fully qualified name like

RawRabbit.IntegrationTests.TestMessages.GenericResponse`2[[RawRabbit.IntegrationTests.TestMessages.First, RawRabbit.IntegrationTests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[RawRabbit.IntegrationTests.TestMessages.Second, RawRabbit.IntegrationTests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]

The type name is sent in the basic properties of the message, and then used to create a Type instance that will be used when deserializing the message. The problem here is that Version, Culture and PublicKeyToken are present. If the version is wrong, we might not be able to create the type instance (even though the class itself is identical between versions). However, it is enough to have type name and assembly name to create the type instance. That is, this string (should) be enough:

RawRabbit.IntegrationTests.TestMessages.GenericResponse`2[[RawRabbit.IntegrationTests.TestMessages.First, RawRabbit.IntegrationTests],[RawRabbit.IntegrationTests.TestMessages.Second, RawRabbit.IntegrationTests]]

In BasicPropertiesProvider.cs, we should have a strategy to create type such that

• It does not include version, culture and public key token for main class or generic arguments
• It should do this recursively, GenericMessage<GenericArgument<NestedGeneric<NestedAgain>>> should contain no version, culture or public key token

The tests that uses Assert.Equal often passes the arguments in the wrong order.

The call should be done like this:
Assert.Equal(expected, actual)
But in most places it is like this:
Assert.Equal(actual, expected)

This is not noticed when all tests are green but it complicates debugging when tests are failing.

When publishing a lot of messages (1'000'000+) there are some flip sides, like

RabbitMQ.Client.Exceptions.OperationInterruptedException: The AMQP operation was interrupted: AMQP close-reason, initiated by Peer, code=504, text="CHANNEL_ERROR - expected 'channel.open'", classId=60, methodId=40, cause=
   at RabbitMQ.Client.Impl.SimpleBlockingRpcContinuation.GetReply(TimeSpan timeout)
   at RabbitMQ.Client.Impl.ModelBase.ModelRpc(MethodBase method, ContentHeaderBase header, Byte[] body)
   at RabbitMQ.Client.Framing.Impl.Model._Private_ChannelOpen(String outOfBand)
   at RabbitMQ.Client.Framing.Impl.AutorecoveringConnection.CreateNonRecoveringModel()
   at RabbitMQ.Client.Framing.Impl.AutorecoveringConnection.CreateModel()
   at RawRabbit.Common.ChannelFactory.<>c.<CreateChannelAsync>b__12_0(Task`1 c)

This should be fixed

This is a similar issue to #21, but concerns multiple RPC calls.

The RabbitMq team feels strongly that the QueueingBasicConsumer is over with rabbitmq/rabbitmq-tutorials#58, so it makes sense for us to remove the QueueingRawConsumer

In the pursuit of high performance, we're looking at refactoring operations so that they efficiently can handle multiple operations on different threads. Publisher.cs will be the first operation that will hold

• TopologyProvider for creating exchanges, queues and bindings
• CreateChannelAsync will be used rather than the sync method
• GetAcTask takes channel as argument rather than working on one channel at a time.

A bug was introduced in 1.8.9; in Subscriber.cs the ConsumerFactory is called without the IModel argument, which uses GetChannel (rather than CreateChannel). This is wrong

This is useful for users that have registered a custom context or want to use the AdvancedMessageContext.

var client = BusClientFactory.CreateDefault<AdvancedContext>() // optional ioc arg.

My system had a slight hick-up and the timer for publish acknowledge expired on one thread ([12]) while another thread ([27]) received an ack for the same message. This caused the application to throw a KeyNotFoundException.

This happens very seldom, but should be fixed nevertheless. See logs below

2016-01-21 09:25:47,554 [27] [Information]  Recieved ack for 1 with multiple set to 'False'
2016-01-21 09:25:48,640 [12] [Warning]  Ack for 1 has timed out.
2016-01-21 09:25:49,258 [27] [Debug]  Disposed ack timer for 1
2016-01-21 09:25:50,881 [27] [Debug]  Message with delivery tag 1 has been acknowledged by broker.
2016-01-21 09:25:52,159 [12] [Fatal]  Unhandled exception: "System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary.
   at System.Collections.Concurrent.ConcurrentDictionary`2.get_Item(TKey key)
   at RawRabbit.Common.PublishAcknowledger.<>c__DisplayClass8_0.<GetAckTask>b__0(Object state)
   at System.Threading.TimerQueueTimer.CallCallbackInContext(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.TimerQueueTimer.CallCallback()
   at System.Threading.TimerQueueTimer.Fire()
   at System.Threading.TimerQueue.FireNextTimers()
   at System.Threading.TimerQueue.AppDomainTimerCallback()"

There's a private method in NamingConvention called GetApplicationName. The algorithm today isn't to refined. Scenarios that should be handled:

• "C:\\Projects\\Temp\\My.Console.App\\bin\\Debug\\RawException.vshost.exe" (console app)
• "C:\\Projects\\Temp\\My.Application\\My.Application.Web\\bin" (hosted in IIS)
• "C:\\Application\\PRODUCTION\\My.Windows.Service\\2.2.1.916\\" (windows service, deployed with version number)

We expect the Application Name to be MyConsoleApp, MyApplicationWeb and MyWindowsService.

This method could be public and unit tested.

As of RabbitMq Client 3.6.0, the official client supports connection lists and host selection strategies (rabbitmq/rabbitmq-dotnet-client#81). This should be used instead of the IConnectionBroker. A prereq is that #9 has been completed

In ChannelFactory.cs a ConnectionFactory is created in the ctor

public ChannelFactory(RawRabbitConfiguration config, IClientPropertyProvider propsProvider)
{
    _connectionFactory = new ConnectionFactory
    {
        VirtualHost =  config.VirtualHost,
        UserName = config.Username,
        Password = config.Password,
        Port = config.Port,
        AutomaticRecoveryEnabled = config.AutomaticRecovery,
        TopologyRecoveryEnabled = config.TopologyRecovery,
        NetworkRecoveryInterval = config.RecoveryInterval,
        ClientProperties = propsProvider.GetClientProperties(config)
    };
}

The ConnectionFactory class implements the interface IConnectionFactory, so theoretically any implementation of the IConnectionFactory would suffice. Therefore, we should make sure that the ChannelFactory takes a IConnectionFactory as a constructor argument.

The ambition with RawRabbit is to provide a neat api for publish/subscribe pattern as well as request/response (a.k.a. RPC). However, it makes sense to have support for chained messages, and the RawRabbit.Extensions framework is just the right place for it.

We should add a SslOption prop on RawRabbitConfiguration and set it on the ConnectionFactory

There is a 'n' missing from 'Convetions' in NamingConvetions.

The vNext IServiceCollection extension AddRawRabbit does not seem to setup RawRabbit correctly for use in a (ASP.NET 5) web-context.

1. it registers ChannelFactory as a singleton.
2. IBusClient is registered as Transient with concrete type BaseBusClient.

Since the client is registered as transient it will be constructed and disposed of once per request for each service dependency of that request which has a dependency on IBusClient.

BaseBusClient in turn diposes of it's ChannelFactory instance.

The ChannelFactory is however, in fact, never actually disposed and will be reused for subsequent activation of another IBusClient. Calling dipose on it does render the ChannelFactory useless for this purpose, as it disposes of it's _threadChannels.

I'm not sure what the intended behavior is here, but I'm guessing ChannelFactory should actually be used/registered as a singleton.

The use case for generating a new BusClient for each service requesting it makes sense, mostly in environments which are intended for message processing. (Maybe not so much in web-based environments where the BusClient might be used to push something on the queue.) However, as soon as the first BusClient is disposed of, all other existing instances will fall flat. (Issue 1?)

Actual issue:
BaseBusClient should not dispose of it's dependencies.
I think it would be safe to delegate this to the DI container? Ie. with the default configuration, they should never be disposed.

The test projects, RawRabbit.Tests and RawRabbit.IntegrationTests should not be in the src folder, but rather be in a test folder

This means that all classes deriving from something will be serialized and deserialized into it's base class.

As raw rabbit is all about speed (?) I'd really like to see that we rely on RabbitMQ 3.4.0 or later and make use of direct RPC to avoid the overhead of creating a separate response queue per RPC-request.

There are several sections that needs to be added or updated, including:

• Building locally
• Contributing
• Examples

And possibly more. Since we're using a readthedocs wiki, we might want to reference it from the README

In the current version, the default exchange type is Direct. However, in order to use wild card routing with global message id (added in #73) we need to use Topic exchanges. Since exchanges can not be updated, but rather deleted and re-declared, it would make sense to have an extension that given an exchange

• Gathers information about bindings
• Delete the exchange and re-declare it according to configuration
• Re-bind queues to exchange

The API could look something like

client.UpdateTopologyAsync(t => t
    .ForExchange("my_exchange")
    .UseConfiguration(e => e
        .WithType(ExchangeType.Topic)
        .WithDurability(false))
);

The BasicProperty header was changed in commit 6fe8897 in order to support long typenames (i.e. generic types). However, it removed the Type header from the basic props, breaking backwards compatibility with earlier versions. This should be fixed.

The IBusClient<TContext> interface should have a method Shutdown() that

• Closes all consumers so that no new messages are consumed
• Gracefully wait until all prefetch messages have been processed then close connection

The logic used today requires the connection string to contain a ? but should allow connection strings without ? as well.

One weakness in how the MessageSequence extension is working is that it listens to messages with the same routing key as any other subscriber for that message. This means that if the message it is listening for is published a lot, it will receive it multiple times while still only being interested in a couple of those. When #73 is done, we should make sure that the extension binds for a specific message id ([routingkey].[globalmessageid). This way, only messages for the specific message sequence is routed to the queue. Once a message sequence is finished, it should remove the bindings for that sequence. As a consequence, to this we will have a slight performance hit in creating a sequence as it will have to make more roundtrips to the broker

There is an issue in the dotnet client, rabbitmq/rabbitmq-dotnet-client#153. It is tagged for release 3.6.1, but we need a fix for the issue while waiting for that version

Today, the only (optional) parameter supported in RawRabbit is requestTimeout. The connectionString should should support all the parameters available at the RawRabbitConfiguration configuration object.

A few examples of connectionStrings that should be supported

• guest:guest:localhost:5672/?autoCloseConnection=true
• guest:guest:localhost:5672/?autoCloseConnection=true&requestTimeout=10
• guest:guest:localhost:5672/?requestTimeout=10&autoCloseConnection=true

1. The order of the parameters should not matter
2. Default values should be used for parameters omitted in the connectionString

This ticket is a placeholder for refactoring activities for 1.9.0