rebus-org / Rebus.AzureServiceBus

:bus: Azure Service Bus transport for Rebus
https://mookid.dk/category/rebus
Other
33 stars 20 forks source link

Normalizing dashes to underscores. #18

Closed codyrehm closed 5 years ago

codyrehm commented 5 years ago

We are using dashes in topic names in Azure Service Bus but the topic name normalization routine is converting these to underscores. I can't seem to override this behavior in a custom implementation of ITopicNameConvention because AsbStringExtensions.ToValidAzureServiceBusTopicName() is getting involved.

Would it be ok to submit a PR to not convert dashes to underscores?

mookid8000 commented 5 years ago

Yeah sure 😄 I actually thought dashes were illegal in topic names, but apparently that is not the case.

codyrehm commented 5 years ago

Great, thanks. I found this while manually creating a topic in the Azure Portal.

regexshot

I was thinking to validate against this regex and, if valid, just hand back the topic string as provided.

public static string ToValidAzureServiceBusTopicName(this string topic)
{
    Regex asbRegex = new Regex(
        @"^[A-Za-z0-9]$|^[A-Za-z0-9][\w-\.\/\~]*[A-Za-z0-9]$", 
        RegexOptions.None, 
        TimeSpan.FromMilliseconds(500));

    if (asbRegex.Matches(topic).OfType<Match>().Any(match => match.Length == topic.Length))
    {
        return topic;
    }

    return string.Concat(topic
        .Select(c => char.IsLetterOrDigit(c) ? char.ToLower(c) : '_'));
}

This should also handle the problems with slashes (and any others lurking out there). What do you think?

mookid8000 commented 5 years ago

It's a great idea to use the regex provided by Microsoft 😁 and you had me convinced, until I remembered WHY Rebus actually changes the topic name: It has to be able to subscribe to topics named after .NET types!

...so when you

await bus.Subscribe<SomeEvent>();

... Rebus uses the "simple assembly-qualified type name" as the topic name, which could be something like SomeAssembly.SomeNamespace.SomeEvent, SomeAssembly, which is clearly not a valid Azure Service Bus topic.

To fix this, Rebus replaces all characters that could show up in a type name like that with _, so that SomeAssembly.SomeNamespace.SomeEvent, SomeAssembly becomes a topic named someassembly_somenamespace_someevent__someassembly.

mookid8000 commented 5 years ago

but

mookid8000 commented 5 years ago

dude

mookid8000 commented 5 years ago

now I realize that Rebus 5 has an ITopicNameConvention that allows for customizing how Rebus comes up with topic names from the .NET type name ... this is the PERFECT place to add the "topic mutation logic" I described above, and then the transport should simply validate that the topic name is in fact a valid topic name....

mookid8000 commented 5 years ago

Ok I can see now that I've been pretty slow here – you even mention ITopicNameConvention in the issue text.

Oh well, I've done some stuff anyway 😄 I've published Rebus.AzureServiceBus 7.0.0-a01, which uses ITopicNameConvention to create valid topic names out of .NET types. In return, the transport does not modify topic names at all, it just validates them like you suggest.

It would be awesome if you would try 7.0.0-a01 and see if it works better for you.

NOTE (to people who come by this issue): THIS IS A BREAKING CHANGE! It chnages how topic names are created out of .NET types, which means that you should probably update all of your endpoints to Rebus.AzureService version 7 (which is currently only available as a prerelease) at once.

codyrehm commented 5 years ago

Awesome. Ill give 7.0.0-a01 a shot. Thanks for your help with this.