Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/AcceptanceTests/When_loading_from_options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ await Scenario.Define<Context>()
{
QueueNameToSubscriptionNameMap = { { Conventions.EndpointNamingConvention(typeof(Publisher)), TopicName } },
PublishedEventToTopicsMap = { { typeof(Event).FullName, TopicName } },
SubscribedEventToTopicsMap = { { typeof(Event).FullName, [TopicName] } }
SubscribedEventToTopicsMap = { { typeof(Event).FullName, [TopicName] } },
ThrowIfUnmappedEventTypes = true
}, TopologyOptionsSerializationContext.Default.TopologyOptions);
var options = JsonSerializer.Deserialize(serializedOptions, TopologyOptionsSerializationContext.Default.TopologyOptions);
transport.Topology = TopicTopology.FromOptions(options);
Expand All @@ -72,7 +73,7 @@ await Scenario.Define<Context>()
SubscribedEventToRuleNameMap = { { typeof(Event).FullName, typeof(Event).FullName.Shorten() } },
TopicToPublishTo = TopicName,
TopicToSubscribeOn = TopicName,
EventsToMigrateMap = [typeof(Event).FullName]
EventsToMigrateMap = [typeof(Event).FullName],
}, TopologyOptionsSerializationContext.Default.TopologyOptions);

var options = JsonSerializer.Deserialize(serializedOptions, TopologyOptionsSerializationContext.Default.TopologyOptions);
Expand Down
3 changes: 3 additions & 0 deletions src/Tests/ApprovalFiles/APIApprovals.Approve.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ namespace NServiceBus.Transport.AzureServiceBus
public System.Collections.Generic.HashSet<string> EventsToMigrateMap { get; init; }
[NServiceBus.Transport.AzureServiceBus.AzureServiceBusRules]
public System.Collections.Generic.Dictionary<string, string> SubscribedEventToRuleNameMap { get; init; }
public new bool ThrowIfUnmappedEventTypes { get; }
[NServiceBus.Transport.AzureServiceBus.AzureServiceBusTopics]
[System.ComponentModel.DataAnnotations.Required]
public required string? TopicToPublishTo { get; init; }
Expand Down Expand Up @@ -185,6 +186,7 @@ namespace NServiceBus.Transport.AzureServiceBus
[NServiceBus.Transport.AzureServiceBus.AzureServiceBusTopics]
[System.Text.Json.Serialization.JsonConverter(typeof(NServiceBus.Transport.AzureServiceBus.SubscribedEventToTopicsMapConverter))]
public System.Collections.Generic.Dictionary<string, System.Collections.Generic.HashSet<string>> SubscribedEventToTopicsMap { get; init; }
public bool ThrowIfUnmappedEventTypes { get; set; }
}
public sealed class TopologyOptionsDisableValidationValidator : Microsoft.Extensions.Options.IValidateOptions<NServiceBus.Transport.AzureServiceBus.TopologyOptions>
{
Expand All @@ -198,6 +200,7 @@ namespace NServiceBus.Transport.AzureServiceBus
{
public TopologyOptionsSerializationContext() { }
public TopologyOptionsSerializationContext(System.Text.Json.JsonSerializerOptions options) { }
public System.Text.Json.Serialization.Metadata.JsonTypeInfo<bool> Boolean { get; }
public System.Text.Json.Serialization.Metadata.JsonTypeInfo<System.Collections.Generic.Dictionary<string, System.Collections.Generic.HashSet<string>>> DictionaryStringHashSetString { get; }
public System.Text.Json.Serialization.Metadata.JsonTypeInfo<System.Collections.Generic.Dictionary<string, string>> DictionaryStringString { get; }
protected override System.Text.Json.JsonSerializerOptions? GeneratedSerializerOptions { get; }
Expand Down
26 changes: 26 additions & 0 deletions src/Tests/EventRouting/TopicPerEventTopologyTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace NServiceBus.Transport.AzureServiceBus.Tests;

using System;
using System.ComponentModel.DataAnnotations;
using NUnit.Framework;
using Particular.Approvals;
Expand All @@ -25,6 +26,31 @@ public void PublishDestination_Should_return_mapped_topic_when_event_is_mapped()
Assert.That(result, Is.EqualTo("MyTopic"));
}

[Test]
public void PublishDestination_Should_default_topic_to_event_name()
{
var topologyOptions = new TopologyOptions();

var topology = TopicTopology.FromOptions(topologyOptions);

var result = topology.GetPublishDestination(typeof(MyEvent));

Assert.That(result, Is.EqualTo(typeof(MyEvent).FullName));
}

[Test]
public void PublishDestination_Should_throw_when_instructed_to_and_type_unmapped()
{
var topologyOptions = new TopologyOptions
{
ThrowIfUnmappedEventTypes = true
};

var topology = TopicTopology.FromOptions(topologyOptions);

Assert.Throws<Exception>(() => topology.GetPublishDestination(typeof(MyEvent)));
}

[Test]
public void Should_self_validate()
{
Expand Down
4 changes: 4 additions & 0 deletions src/Transport/EventRouting/EntityValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ static partial class EntityValidator
: ValidationResult.Success;
}

// Enforces naming according to the specification https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftservicebus
[GeneratedRegex(@"^(?=.{1,260}$)(?=^[A-Za-z0-9])(?!.*[\\?#])(?:[A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9./_-]*[A-Za-z0-9])$")]
private static partial Regex TopicNameRegex();

Expand All @@ -34,6 +35,7 @@ static partial class EntityValidator
: ValidationResult.Success;
}

// Enforces naming according to the specification https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftservicebus
// Note the queue pattern is the same as the topic pattern. Deliberately kept separate for future extensibility.
[GeneratedRegex(@"^(?=.{1,260}$)(?=^[A-Za-z0-9])(?!.*[\\?#])(?:[A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9./_-]*[A-Za-z0-9])$")]
private static partial Regex QueueNameRegex();
Expand All @@ -50,6 +52,7 @@ static partial class EntityValidator
: ValidationResult.Success;
}

// Enforces naming according to the specification https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftservicebus
[GeneratedRegex(@"^(?!\$)(?=.{1,50}$)(?=^[A-Za-z0-9])(?!.*[\/\\?#])[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$")]
private static partial Regex RuleNameRegex();

Expand All @@ -66,6 +69,7 @@ static partial class EntityValidator
: ValidationResult.Success;
}

//enforces naming according to the specification https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftservicebus
// Note the subscription pattern is the same as the rule pattern. Deliberately kept separate for future extensibility.
[GeneratedRegex(@"^(?!\$)(?=.{1,50}$)(?=^[A-Za-z0-9])(?!.*[\/\\?#])[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$")]
private static partial Regex SubscriptionNameRegex();
Expand Down
6 changes: 6 additions & 0 deletions src/Transport/EventRouting/MigrationTopologyOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ namespace NServiceBus.Transport.AzureServiceBus;
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Text.Json.Serialization;
using Particular.Obsoletes;

/// <summary>
Expand Down Expand Up @@ -45,4 +46,9 @@ public Dictionary<string, string> SubscribedEventToRuleNameMap
get;
init => field = value ?? [];
} = [];

// NOTE: explicitly set to true always and ignored from JSON, since MigrationTopology is already obsolete and we don't want to have a fallback naming strategy
/// <inheritdoc cref="TopologyOptions.ThrowIfUnmappedEventTypes" />
[JsonIgnore]
public new bool ThrowIfUnmappedEventTypes => true;
}
9 changes: 7 additions & 2 deletions src/Transport/EventRouting/TopicPerEventTopology.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
namespace NServiceBus.Transport.AzureServiceBus;

using System;
using System.Collections.Generic;

/// <summary>
/// A topology that uses separate topic for each event.
Expand Down Expand Up @@ -105,7 +104,13 @@ public void OverrideSubscriptionNameFor(string queueName, string subscriptionNam

/// <inheritdoc />
protected override string GetPublishDestinationCore(string eventTypeFullName)
=> Options.PublishedEventToTopicsMap.GetValueOrDefault(eventTypeFullName, eventTypeFullName);
{
if (!Options.PublishedEventToTopicsMap.TryGetValue(eventTypeFullName, out string? topic) && Options.ThrowIfUnmappedEventTypes)
{
throw new Exception($"Unmapped event type '{eventTypeFullName}'. All events must be mapped in `{nameof(TopologyOptions.PublishedEventToTopicsMap)}` when `{nameof(TopologyOptions.ThrowIfUnmappedEventTypes)}` is set");
}
return topic ?? eventTypeFullName;
}

internal override SubscriptionManager CreateSubscriptionManager(
SubscriptionManagerCreationOptions creationOptions) =>
Expand Down
5 changes: 5 additions & 0 deletions src/Transport/EventRouting/TopologyOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,9 @@ public Dictionary<string, string> QueueNameToSubscriptionNameMap
get;
init => field = value ?? [];
} = [];

/// <summary>
/// Determines if an exception should be thrown when attempting to publish an event not mapped in PublishedEventToTopicsMap
/// </summary>
public bool ThrowIfUnmappedEventTypes { get; set; } = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above. I think this is the time when we need to introduce the TopicPerEventTopologyOptions again (we killed the type because it was empty at the time and said we could always reintroduce it later).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative name suggestions: ThrowForUnmappedEventTypes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed in call: having a new subclass of TopologyOptions which now becomes the 'true' representation of options for TopicPerEvent topology introduces potential breaking changes and difficulty in obsoleting which aren't worth this change. Instead we are forcing this property to true for MigrationTopologyOptions since that's the expected behaviour there

}
Loading