Skip to content

Commit b131494

Browse files
danielmarbachbording
danielmarbach
and
bording
authored
Fix nullable type annotations on ContextBag (#7391)
* Fix nullable type annotations on ContextBag * Follow through on more nullability for the users of TryGet * Fix warnings * Rename file to match type name * Tweak formatting * Clean up null-forgiving operator * Make calls to TryGet consistent * Add additional test * Undo sealing state class * Make State classes regular classes --------- Co-authored-by: Daniel Marbach <danielmarbach@users.noreply.github.com> Co-authored-by: Brandon Ording <bording@gmail.com>
1 parent ee9043c commit b131494

File tree

50 files changed

+297
-275
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+297
-275
lines changed

‎src/NServiceBus.AcceptanceTesting/AcceptanceTestingPersistence/AcceptanceTestingSynchronizedStorageSession.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public ValueTask<bool> TryOpen(IOutboxTransaction transaction, ContextBag contex
3737
public ValueTask<bool> TryOpen(TransportTransaction transportTransaction, ContextBag context,
3838
CancellationToken cancellationToken = default)
3939
{
40-
if (!transportTransaction.TryGet(out Transaction ambientTransaction))
40+
if (!transportTransaction.TryGet<Transaction>(out var ambientTransaction))
4141
{
4242
return new ValueTask<bool>(false);
4343
}

‎src/NServiceBus.AcceptanceTests/Core/Pipeline/When_extending_sendoptions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public class TestingSendOptionsExtensionBehavior : IBehavior<IOutgoingLogicalMes
6666
{
6767
public Task Invoke(IOutgoingLogicalMessageContext context, Func<IOutgoingLogicalMessageContext, Task> next)
6868
{
69-
if (context.Extensions.TryGet(out Context data))
69+
if (context.Extensions.TryGet<Context>(out var data))
7070
{
7171
context.UpdateMessage(new SendMessage
7272
{

‎src/NServiceBus.AcceptanceTests/Core/Pipeline/When_extending_the_publish_api.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public class PublishExtensionBehavior : IBehavior<IOutgoingLogicalMessageContext
6565
{
6666
public Task Invoke(IOutgoingLogicalMessageContext context, Func<IOutgoingLogicalMessageContext, Task> next)
6767
{
68-
if (context.Extensions.TryGet(out Context data))
68+
if (context.Extensions.TryGet<Context>(out var data))
6969
{
7070
Assert.That(data.SomeProperty, Is.EqualTo("ItWorks"));
7171
}

‎src/NServiceBus.Core.Tests/ApprovalFiles/APIApprovals.ApproveNServiceBus.approved.txt‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -793,11 +793,11 @@ namespace NServiceBus
793793
}
794794
public static class RoutingOptionExtensions
795795
{
796-
public static string GetDestination(this NServiceBus.ReplyOptions options) { }
797-
public static string GetDestination(this NServiceBus.SendOptions options) { }
798-
public static string GetReplyToRoute(this NServiceBus.ReplyOptions options) { }
799-
public static string GetReplyToRoute(this NServiceBus.SendOptions options) { }
800-
public static string GetRouteToSpecificInstance(this NServiceBus.SendOptions options) { }
796+
public static string? GetDestination(this NServiceBus.ReplyOptions options) { }
797+
public static string? GetDestination(this NServiceBus.SendOptions options) { }
798+
public static string? GetReplyToRoute(this NServiceBus.ReplyOptions options) { }
799+
public static string? GetReplyToRoute(this NServiceBus.SendOptions options) { }
800+
public static string? GetRouteToSpecificInstance(this NServiceBus.SendOptions options) { }
801801
public static bool IsRoutingReplyToAnyInstance(this NServiceBus.ReplyOptions options) { }
802802
public static bool IsRoutingReplyToAnyInstance(this NServiceBus.SendOptions options) { }
803803
public static bool IsRoutingReplyToThisInstance(this NServiceBus.ReplyOptions options) { }
@@ -1150,16 +1150,16 @@ namespace NServiceBus.Extensibility
11501150
public class ContextBag : NServiceBus.Extensibility.IReadOnlyContextBag
11511151
{
11521152
public ContextBag(NServiceBus.Extensibility.ContextBag? parentBag = null) { }
1153-
public T? Get<T>() { }
1154-
public T? Get<T>(string key) { }
1155-
public T? GetOrCreate<T>()
1153+
public T Get<T>() { }
1154+
public T Get<T>(string key) { }
1155+
public T GetOrCreate<T>()
11561156
where T : class, new () { }
11571157
public void Remove(string key) { }
11581158
public void Remove<T>() { }
11591159
public void Set<T>(T t) { }
11601160
public void Set<T>(string key, T t) { }
1161-
public bool TryGet<T>(out T? result) { }
1162-
public bool TryGet<T>(string key, out T? result) { }
1161+
public bool TryGet<T>([System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out T? result) { }
1162+
public bool TryGet<T>(string key, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out T? result) { }
11631163
}
11641164
public abstract class ExtendableOptions
11651165
{
@@ -1182,8 +1182,8 @@ namespace NServiceBus.Extensibility
11821182
{
11831183
T Get<T>();
11841184
T Get<T>(string key);
1185-
bool TryGet<T>(out T result);
1186-
bool TryGet<T>(string key, out T result);
1185+
bool TryGet<T>([System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out T? result);
1186+
bool TryGet<T>(string key, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out T? result);
11871187
}
11881188
}
11891189
namespace NServiceBus.Faults
@@ -1799,9 +1799,9 @@ namespace NServiceBus.Pipeline
17991799
}
18001800
public static class TransportMessageContextExtensions
18011801
{
1802-
public static bool TryGetIncomingPhysicalMessage(this NServiceBus.Pipeline.IOutgoingLogicalMessageContext context, out NServiceBus.Transport.IncomingMessage message) { }
1803-
public static bool TryGetIncomingPhysicalMessage(this NServiceBus.Pipeline.IOutgoingPhysicalMessageContext context, out NServiceBus.Transport.IncomingMessage message) { }
1804-
public static bool TryGetIncomingPhysicalMessage(this NServiceBus.Pipeline.IOutgoingReplyContext context, out NServiceBus.Transport.IncomingMessage message) { }
1802+
public static bool TryGetIncomingPhysicalMessage(this NServiceBus.Pipeline.IOutgoingLogicalMessageContext context, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out NServiceBus.Transport.IncomingMessage? message) { }
1803+
public static bool TryGetIncomingPhysicalMessage(this NServiceBus.Pipeline.IOutgoingPhysicalMessageContext context, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out NServiceBus.Transport.IncomingMessage? message) { }
1804+
public static bool TryGetIncomingPhysicalMessage(this NServiceBus.Pipeline.IOutgoingReplyContext context, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out NServiceBus.Transport.IncomingMessage? message) { }
18051805
}
18061806
}
18071807
namespace NServiceBus.Recoverability

‎src/NServiceBus.Core.Tests/ApprovalFiles/NullableAnnotations.ApproveNullableTypes.approved.txt‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ Changes that make this list longer should not be approved.
33
-----
44
NServiceBus.AutomaticSubscriptions.Config.AutoSubscribeSettings
55
NServiceBus.AutoSubscribeSettingsExtensions
6-
NServiceBus.BestPracticesOptionExtensions
76
NServiceBus.ConfigureError
87
NServiceBus.ConfigurePurging
98
NServiceBus.ConnectorContextExtensions
@@ -25,7 +24,6 @@ NServiceBus.EndpointWithExternallyManagedContainer
2524
NServiceBus.ErrorQueueSettings
2625
NServiceBus.Extensibility.ExtendableOptionsExtensions
2726
NServiceBus.Extensibility.IExtendable
28-
NServiceBus.Extensibility.IReadOnlyContextBag
2927
NServiceBus.FailedConfig
3028
NServiceBus.Faults.DelayedRetryMessage
3129
NServiceBus.Faults.FailedMessage
@@ -49,7 +47,6 @@ NServiceBus.IDistributionPolicy
4947
NServiceBus.IMessageConvention
5048
NServiceBus.IMessageHandlerContext
5149
NServiceBus.IMessageProcessingContext
52-
NServiceBus.ImmediateDispatchOptionExtensions
5350
NServiceBus.ImmediateRetriesSettings
5451
NServiceBus.ImmediateRetry
5552
NServiceBus.IPipelineContext
@@ -117,7 +114,6 @@ NServiceBus.Pipeline.PipelineTerminator`1
117114
NServiceBus.Pipeline.RegisterStep
118115
NServiceBus.Pipeline.StageConnector`2
119116
NServiceBus.Pipeline.StageForkConnector`3
120-
NServiceBus.Pipeline.TransportMessageContextExtensions
121117
NServiceBus.RateLimitSettings
122118
NServiceBus.ReceiveAddresses
123119
NServiceBus.ReceiveFeatureConfigurationContextExtensions
@@ -148,7 +144,6 @@ NServiceBus.Routing.UnicastRoute
148144
NServiceBus.Routing.UnicastRoutingStrategy
149145
NServiceBus.Routing.UnicastRoutingTable
150146
NServiceBus.RoutingFeatureSettingsExtensions
151-
NServiceBus.RoutingOptionExtensions
152147
NServiceBus.RoutingSettings
153148
NServiceBus.RoutingSettings`1
154149
NServiceBus.Sagas.ActiveSagaInstance

‎src/NServiceBus.Core.Tests/ContextBagTests.cs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace NServiceBus.Core.Tests;
1+
#nullable enable
2+
3+
namespace NServiceBus.Core.Tests;
24

35
using Extensibility;
46
using NUnit.Framework;
@@ -7,17 +9,26 @@
79
public class ContextBagTests
810
{
911
[Test]
10-
public void Should_allow_storing_null_values()
12+
public void Should_not_allow_storing_null_value()
1113
{
1214
var contextBag = new ContextBag();
1315

14-
contextBag.Set<string>("NullValue", null);
16+
Assert.Multiple(() =>
17+
{
18+
Assert.That(() => contextBag.Set<string>("NullValue", null!), Throws.ArgumentNullException);
19+
Assert.That(() => contextBag.SetOnRoot<string>("NullValue", null!), Throws.ArgumentNullException);
20+
});
21+
}
22+
23+
[Test]
24+
public void Should_not_allow_storing_null_value_even_with_nullable_generic_type()
25+
{
26+
var contextBag = new ContextBag();
1527

16-
var result = ((IReadOnlyContextBag)contextBag).TryGet("NullValue", out object theValue);
1728
Assert.Multiple(() =>
1829
{
19-
Assert.That(result, Is.True, "Should be able to retrieve a null value");
20-
Assert.That(theValue, Is.Null);
30+
Assert.That(() => contextBag.Set<string?>("NullValue", null), Throws.ArgumentNullException);
31+
Assert.That(() => contextBag.SetOnRoot<string?>("NullValue", null), Throws.ArgumentNullException);
2132
});
2233
}
2334

@@ -28,7 +39,7 @@ public void ShouldAllowMonkeyPatching()
2839

2940
contextBag.Set("MonkeyPatch", "some string");
3041

31-
((IReadOnlyContextBag)contextBag).TryGet("MonkeyPatch", out string theValue);
42+
_ = ((IReadOnlyContextBag)contextBag).TryGet("MonkeyPatch", out string? theValue);
3243
Assert.That(theValue, Is.EqualTo("some string"));
3344
}
3445

‎src/NServiceBus.Core.Tests/Fakes/FakeSynchronizedStorageSession.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public ValueTask<bool> TryOpen(IOutboxTransaction transaction, ContextBag contex
4040
public ValueTask<bool> TryOpen(TransportTransaction transportTransaction, ContextBag context,
4141
CancellationToken cancellationToken = default)
4242
{
43-
if (!transportTransaction.TryGet(out Transaction ambientTransaction))
43+
if (!transportTransaction.TryGet<Transaction>(out var ambientTransaction))
4444
{
4545
return new ValueTask<bool>(false);
4646
}

‎src/NServiceBus.Core.Tests/MessageSessionTests.cs‎

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,26 @@ public class MessageSessionTests
1414
public async Task Should_not_share_root_context_across_operations()
1515
{
1616
var readValues = new List<int>();
17-
var messageOperations = new TestableMessageOperations();
18-
messageOperations.SendPipeline.OnInvoke = context =>
17+
var messageOperations = new TestableMessageOperations
1918
{
20-
// read existing value
21-
if (context.Extensions.TryGet<int>("test", out var i))
19+
SendPipeline =
2220
{
23-
readValues.Add(i);
21+
OnInvoke = context =>
22+
{
23+
// read existing value
24+
if (context.Extensions.TryGet<int>("test", out var i))
25+
{
26+
readValues.Add(i);
27+
}
28+
29+
// set value on root
30+
context.Extensions.SetOnRoot("test", 42);
31+
}
2432
}
25-
// set value on root
26-
context.Extensions.SetOnRoot("test", 42);
2733
};
2834

29-
var session = new MessageSession(null, messageOperations, null, CancellationToken.None);
35+
var session = new MessageSession(new ThrowingServiceProvider(), messageOperations, new ThrowingPipelineCache(),
36+
CancellationToken.None);
3037

3138
await session.Send(new object());
3239
await session.Send(new object());
@@ -38,28 +45,40 @@ public async Task Should_not_share_root_context_across_operations()
3845
[Test]
3946
public void Should_propagate_endpoint_cancellation_status_to_context()
4047
{
41-
var messageOperations = new TestableMessageOperations();
42-
messageOperations.SendPipeline.OnInvoke = context =>
48+
var messageOperations = new TestableMessageOperations
4349
{
44-
context.CancellationToken.ThrowIfCancellationRequested();
50+
SendPipeline =
51+
{
52+
OnInvoke = context =>
53+
{
54+
context.CancellationToken.ThrowIfCancellationRequested();
55+
}
56+
}
4557
};
4658

47-
var session = new MessageSession(null, messageOperations, null, new CancellationToken(true));
59+
var session = new MessageSession(new ThrowingServiceProvider(), messageOperations, new ThrowingPipelineCache(), new CancellationToken(true));
4860

49-
Assert.ThrowsAsync<OperationCanceledException>(async () => await session.Send(new object(), new CancellationToken()));
61+
Assert.ThrowsAsync<OperationCanceledException>(async () =>
62+
await session.Send(new object(), CancellationToken.None));
5063
}
5164

5265
[Test]
5366
public void Should_propagate_request_cancellation_status_to_context()
5467
{
55-
var messageOperations = new TestableMessageOperations();
56-
messageOperations.SendPipeline.OnInvoke = context =>
68+
var messageOperations = new TestableMessageOperations
5769
{
58-
context.CancellationToken.ThrowIfCancellationRequested();
70+
SendPipeline =
71+
{
72+
OnInvoke = context =>
73+
{
74+
context.CancellationToken.ThrowIfCancellationRequested();
75+
}
76+
}
5977
};
6078

61-
var session = new MessageSession(null, messageOperations, null, new CancellationToken());
79+
var session = new MessageSession(new ThrowingServiceProvider(), messageOperations, new ThrowingPipelineCache(), CancellationToken.None);
6280

63-
Assert.ThrowsAsync<OperationCanceledException>(async () => await session.Send(new object(), new CancellationToken(true)));
81+
Assert.ThrowsAsync<OperationCanceledException>(async () =>
82+
await session.Send(new object(), new CancellationToken(true)));
6483
}
6584
}

‎src/NServiceBus.Core.Tests/OpenTelemetry/ActivityExtensionsTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public void TryGetRecordingPipelineActivity_should_return_false_when_value_null(
2828
ambientActivity.Start();
2929

3030
var contextBag = new ContextBag();
31-
contextBag.SetOutgoingPipelineActitvity(null);
3231

3332
Assert.Multiple(() =>
3433
{

‎src/NServiceBus.Core.Tests/OpenTelemetry/ActivityFactoryTests.cs‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace NServiceBus.Core.Tests.OpenTelemetry;
1+
#nullable enable
2+
3+
namespace NServiceBus.Core.Tests.OpenTelemetry;
24

35
using System;
46
using System.Collections.Generic;
@@ -16,21 +18,15 @@
1618
[TestFixture]
1719
public class ActivityFactoryTests
1820
{
19-
ActivityFactory activityFactory = new();
21+
readonly ActivityFactory activityFactory = new();
2022

2123
TestingActivityListener nsbActivityListener;
2224

2325
[OneTimeSetUp]
24-
public void Setup()
25-
{
26-
nsbActivityListener = TestingActivityListener.SetupNServiceBusDiagnosticListener();
27-
}
26+
public void Setup() => nsbActivityListener = TestingActivityListener.SetupNServiceBusDiagnosticListener();
2827

2928
[OneTimeTearDown]
30-
public void TearDown()
31-
{
32-
nsbActivityListener.Dispose();
33-
}
29+
public void TearDown() => nsbActivityListener.Dispose();
3430

3531
class StartIncomingActivity : ActivityFactoryTests
3632
{
@@ -61,7 +57,7 @@ public void Should_attach_to_context_activity_when_activity_on_context_and_trace
6157
var contextBag = new ContextBag();
6258
contextBag.Set(contextActivity);
6359

64-
var messageHeaders = new Dictionary<string, string> { { Headers.DiagnosticsTraceParent, sendActivity.Id } };
60+
var messageHeaders = new Dictionary<string, string> { { Headers.DiagnosticsTraceParent, sendActivity.Id! } };
6561

6662
var activity = activityFactory.StartIncomingPipelineActivity(CreateMessageContext(messageHeaders, contextBag));
6763

@@ -115,7 +111,7 @@ public void Should_attach_to_header_trace_when_no_activity_on_context_and_trace_
115111
{
116112
using var sendActivity = CreateCompletedActivity("send activity");
117113

118-
var messageHeaders = new Dictionary<string, string> { { Headers.DiagnosticsTraceParent, sendActivity.Id } };
114+
var messageHeaders = new Dictionary<string, string> { { Headers.DiagnosticsTraceParent, sendActivity.Id! } };
119115

120116
var activity = activityFactory.StartIncomingPipelineActivity(CreateMessageContext(messageHeaders));
121117

@@ -180,7 +176,7 @@ public void Should_add_native_message_id_tag()
180176

181177
var activity = activityFactory.StartIncomingPipelineActivity(messageContext);
182178

183-
Assert.That(activity.Tags.ToImmutableDictionary()["nservicebus.native_message_id"], Is.EqualTo(messageContext.NativeMessageId));
179+
Assert.That(activity!.Tags.ToImmutableDictionary()["nservicebus.native_message_id"], Is.EqualTo(messageContext.NativeMessageId));
184180
}
185181

186182
static Activity CreateCompletedActivity(string activityName, ActivityIdFormat idFormat = ActivityIdFormat.W3C)
@@ -192,16 +188,14 @@ static Activity CreateCompletedActivity(string activityName, ActivityIdFormat id
192188
return activity;
193189
}
194190

195-
static MessageContext CreateMessageContext(Dictionary<string, string> messageHeaders = null, ContextBag contextBag = null)
196-
{
197-
return new MessageContext(
191+
static MessageContext CreateMessageContext(Dictionary<string, string>? messageHeaders = null, ContextBag? contextBag = null) =>
192+
new(
198193
Guid.NewGuid().ToString(),
199194
messageHeaders ?? [],
200195
Array.Empty<byte>(),
201196
new TransportTransaction(),
202197
"receiver",
203198
contextBag ?? new ContextBag());
204-
}
205199
}
206200

207201
class StartOutgoingPipelineActivity : ActivityFactoryTests
@@ -214,7 +208,7 @@ public void Should_attach_ambient_activity()
214208

215209
var activity = activityFactory.StartOutgoingPipelineActivity("activityName", "activityDisplayName", new FakeRootContext());
216210

217-
Assert.That(activity.ParentId, Is.EqualTo(ambientActivity.Id));
211+
Assert.That(activity?.ParentId, Is.EqualTo(ambientActivity.Id));
218212
}
219213

220214
[Test]
@@ -228,16 +222,16 @@ public void Should_always_create_a_w3c_id()
228222

229223
Assert.Multiple(() =>
230224
{
231-
Assert.That(activity.ParentId, Is.EqualTo(ambientActivity.Id));
232-
Assert.That(activity.IdFormat, Is.EqualTo(ActivityIdFormat.W3C));
225+
Assert.That(activity?.ParentId, Is.EqualTo(ambientActivity.Id));
226+
Assert.That(activity?.IdFormat, Is.EqualTo(ActivityIdFormat.W3C));
233227
});
234228
}
235229

236230
[Test]
237231
public void Should_set_activity_in_context()
238232
{
239233
var context = new FakeRootContext();
240-
activityFactory.StartOutgoingPipelineActivity("activityName", "activityDisplayName", context);
234+
_ = activityFactory.StartOutgoingPipelineActivity("activityName", "activityDisplayName", context);
241235

242236
Assert.That(context.Extensions.Get<Activity>(ActivityExtensions.OutgoingActivityKey), Is.Not.Null);
243237
}

0 commit comments

Comments
 (0)