Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
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 @@ -54,6 +54,7 @@ namespace NServiceBus
{
protected TopicTopology(NServiceBus.Transport.AzureServiceBus.TopologyOptions options, Microsoft.Extensions.Options.IValidateOptions<NServiceBus.Transport.AzureServiceBus.TopologyOptions> optionsValidator) { }
public Microsoft.Extensions.Options.IValidateOptions<NServiceBus.Transport.AzureServiceBus.TopologyOptions> OptionsValidator { get; set; }
public bool ThrowIfUnmappedEventTypes { get; set; }
public static NServiceBus.Transport.AzureServiceBus.TopicPerEventTopology Default { get; }
protected abstract string GetPublishDestinationCore(string eventTypeFullName);
public void Validate() { }
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
23 changes: 23 additions & 0 deletions src/Tests/EventRouting/TopicPerEventTopologyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ 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 Should_self_validate()
{
Expand Down Expand Up @@ -58,5 +70,16 @@ public void Should_allow_disabling_validation()
Assert.DoesNotThrow(() => topology.Validate());
}

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

var topology = TopicTopology.FromOptions(topologyOptions);
topology.ThrowIfUnmappedEventTypes = true;

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

class MyEvent;
}
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 `PublishedEventToTopicsMap` when `ThrowIfUnmappedEventTypes` is set");
}
return topic ?? eventTypeFullName;
}

internal override SubscriptionManager CreateSubscriptionManager(
SubscriptionManagerCreationOptions creationOptions) =>
Expand Down
10 changes: 10 additions & 0 deletions src/Transport/EventRouting/TopicTopology.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ protected TopicTopology(TopologyOptions options, IValidateOptions<TopologyOption

internal TopologyOptions Options { get; }

/// <summary>
/// Determines if an exception should be thrown when attempting to publish an event not mapped in PublishedEventToTopicsMap
/// </summary>
public bool ThrowIfUnmappedEventTypes
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this property belongs here. This is a behavior that is only relevant to opt in for the TopicPerEventTopology. The migration topology already throws when types are unmapped. If you put it here, one could argue the migration topology would have to respect this flag too, which I think should never be the case by design.

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: only have it on options and explicitly force migrationoptions version to true

{
get => Options.ThrowIfUnmappedEventTypes;
set => Options.ThrowIfUnmappedEventTypes = value;
}


/// <summary>
/// Creates an instance of the topology object based on serializable state.
/// </summary>
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