Fix nullability issues in saga message mapper (#6776) (#6777)

* Compile-only smoke test for API usages under nullable reference types * Change ToSaga expressions to object? * Message mappers too, otherwise messages have to be 100% non-null properties * Approve * Additional approvals and API test coverage --------- Co-authored-by: Daniel Marbach <[email protected]>
Particular · Jun 16, 2023 · 21e5b5e · 21e5b5e
1 parent 627c0be
commit 21e5b5e
Show file tree

Hide file tree

Showing 6 changed files with 205 additions and 15 deletions.
diff --git a/src/NServiceBus.Core.Tests/API/NullableApiUsages.cs b/src/NServiceBus.Core.Tests/API/NullableApiUsages.cs
@@ -0,0 +1,190 @@
+#nullable enable
+
+/**
+ * The code in this file is not meant to be executed. It only shows idiomatic usages of NServiceBus
+ * APIs, but with nullable reference types enabled, so that we can attempt to make sure that the
+ * addition of nullable annotations in our public APIs doesn't create nullability warnings on
+ * our own APIs under normal circumstances. It's sort of a mini-Snippets to provide faster
+ * feedback than having to release an alpha package and check that Snippets in docs compile.
+ */
+
+namespace NServiceBus.Core.Tests.API.NullableApiUsages
+{
+    using System;
+    using System.Threading;
+    using System.Threading.Tasks;
+    using NServiceBus.Extensibility;
+    using NServiceBus.Logging;
+    using NServiceBus.MessageMutator;
+    using NServiceBus.Persistence;
+    using NServiceBus.Pipeline;
+    using NServiceBus.Sagas;
+
+    public class TopLevelApis
+    {
+        public async Task SetupEndpoint(CancellationToken cancellationToken = default)
+        {
+            var cfg = new EndpointConfiguration("EndpointName");
+
+            cfg.Conventions()
+                .DefiningCommandsAs(t => t.Namespace?.EndsWith(".Commands") ?? false)
+                .DefiningEventsAs(t => t.Namespace?.EndsWith(".Events") ?? false)
+                .DefiningMessagesAs(t => t.Namespace?.EndsWith(".Messages") ?? false);
+
+            cfg.SendFailedMessagesTo("error");
+
+            var routing = cfg.UseTransport(new LearningTransport());
+            routing.RouteToEndpoint(typeof(Cmd), "Destination");
+
+            var persistence = cfg.UsePersistence<LearningPersistence>();
+
+            cfg.UseSerialization<SystemJsonSerializer>()
+                .Options(new System.Text.Json.JsonSerializerOptions());
+
+            // Start directly
+            await Endpoint.Start(cfg, cancellationToken);
+
+            // Or create, then start
+            var startable = await Endpoint.Create(cfg, cancellationToken);
+            var ep = await startable.Start(cancellationToken);
+
+            await ep.Send(new Cmd(), cancellationToken);
+            await ep.Publish(new Evt(), cancellationToken);
+            await ep.Publish<Evt>(cancellationToken);
+        }
+    }
+
+    public class TestHandler : IHandleMessages<Cmd>
+    {
+        ILog logger;
+
+        public TestHandler(ILog logger)
+        {
+            this.logger = logger;
+        }
+
+        public async Task Handle(Cmd message, IMessageHandlerContext context)
+        {
+            logger.Info(message.OrderId);
+            await context.Send(new Cmd());
+            await context.Publish(new Evt());
+
+            var opts = new SendOptions();
+            opts.DelayDeliveryWith(TimeSpan.FromSeconds(5));
+            opts.SetHeader("a", "1");
+        }
+    }
+
+    public class TestSaga : Saga<TestSagaData>,
+        IAmStartedByMessages<Cmd>,
+        IHandleMessages<Evt>,
+        IHandleTimeouts<TestTimeout>
+    {
+        protected override void ConfigureHowToFindSaga(SagaPropertyMapper<TestSagaData> mapper)
+        {
+            mapper.MapSaga(saga => saga.OrderId)
+                .ToMessage<Cmd>(m => m.OrderId)
+                .ToMessage<Evt>(m => m.OrderId)
+                .ToMessageHeader<Cmd>("HeaderName");
+        }
+
+        public async Task Handle(Cmd message, IMessageHandlerContext context)
+        {
+            await context.Send(new Cmd());
+            await context.Publish(new Evt());
+            Console.WriteLine(Data.OrderId);
+            await RequestTimeout<TestTimeout>(context, TimeSpan.FromMinutes(1));
+            MarkAsComplete();
+        }
+
+        public async Task Handle(Evt message, IMessageHandlerContext context)
+        {
+            await context.Send(new Cmd());
+            await context.Publish(new Evt());
+            await context.Publish<Evt>();
+        }
+
+        public Task Timeout(TestTimeout state, IMessageHandlerContext context)
+        {
+            Console.WriteLine(state.TimeoutData);
+            return Task.CompletedTask;
+        }
+    }
+
+    public class TestSagaOldMapping : Saga<TestSagaData>,
+        IAmStartedByMessages<Cmd>
+    {
+        protected override void ConfigureHowToFindSaga(SagaPropertyMapper<TestSagaData> mapper)
+        {
+            mapper.ConfigureMapping<Cmd>(m => m.OrderId).ToSaga(s => s.OrderId);
+            mapper.ConfigureHeaderMapping<Cmd>("HeaderName");
+        }
+
+        public Task Handle(Cmd message, IMessageHandlerContext context) => throw new NotImplementedException();
+    }
+
+    public class Cmd : ICommand
+    {
+        public string? OrderId { get; set; }
+    }
+
+    public class Evt : IEvent
+    {
+        public string? OrderId { get; set; }
+    }
+
+    public class TestSagaData : ContainSagaData
+    {
+        public string? OrderId { get; set; }
+    }
+
+    public class TestTimeout
+    {
+        public string? TimeoutData { get; set; }
+    }
+
+    public class NotUsedSagaFinder : ISagaFinder<TestSagaData, Cmd>
+    {
+        public async Task<TestSagaData?> FindBy(Cmd message, ISynchronizedStorageSession storageSession, IReadOnlyContextBag context, CancellationToken cancellationToken = default)
+        {
+            // Super-gross, never do this
+            await Task.Yield();
+
+            if (context.TryGet<TestSagaData>(out var result))
+            {
+                return result;
+            }
+
+            return null;
+        }
+    }
+
+    public class TestBehavior : Behavior<IIncomingLogicalMessageContext>
+    {
+        public override async Task Invoke(IIncomingLogicalMessageContext context, Func<Task> next)
+        {
+            await Task.Delay(10);
+            await next();
+        }
+    }
+
+    public class TestIncomingMutator : IMutateIncomingMessages
+    {
+        public Task MutateIncoming(MutateIncomingMessageContext context) => Task.CompletedTask;
+    }
+
+    public class TestIncomingTransportMutator : IMutateIncomingTransportMessages
+    {
+        public Task MutateIncoming(MutateIncomingTransportMessageContext context) => Task.CompletedTask;
+    }
+
+    public class TestOutgoingMutator : IMutateOutgoingMessages
+    {
+        public Task MutateOutgoing(MutateOutgoingMessageContext context) => Task.CompletedTask;
+    }
+
+    public class TestOutgoingTransportMutator : IMutateOutgoingTransportMessages
+    {
+        public Task MutateOutgoing(MutateOutgoingTransportMessageContext context) => Task.CompletedTask;
+    }
+}
diff --git a/src/NServiceBus.Core.Tests/ApprovalFiles/APIApprovals.ApproveNServiceBus.approved.txt b/src/NServiceBus.Core.Tests/ApprovalFiles/APIApprovals.ApproveNServiceBus.approved.txt
@@ -181,7 +181,7 @@ namespace NServiceBus
     public class CorrelatedSagaPropertyMapper<TSagaData>
         where TSagaData :  class, NServiceBus.IContainSagaData
     {
-        public NServiceBus.CorrelatedSagaPropertyMapper<TSagaData> ToMessage<TMessage>(System.Linq.Expressions.Expression<System.Func<TMessage, object>> messageProperty) { }
+        public NServiceBus.CorrelatedSagaPropertyMapper<TSagaData> ToMessage<TMessage>(System.Linq.Expressions.Expression<System.Func<TMessage, object?>> messageProperty) { }
         public NServiceBus.CorrelatedSagaPropertyMapper<TSagaData> ToMessageHeader<TMessage>(string headerName) { }
     }
     public class CriticalError
@@ -479,7 +479,7 @@ namespace NServiceBus
     public interface ICommand : NServiceBus.IMessage { }
     public interface IConfigureHowToFindSagaWithMessage
     {
-        void ConfigureMapping<TSagaEntity, TMessage>(System.Linq.Expressions.Expression<System.Func<TSagaEntity, object>> sagaEntityProperty, System.Linq.Expressions.Expression<System.Func<TMessage, object>> messageProperty)
+        void ConfigureMapping<TSagaEntity, TMessage>(System.Linq.Expressions.Expression<System.Func<TSagaEntity, object?>> sagaEntityProperty, System.Linq.Expressions.Expression<System.Func<TMessage, object?>> messageProperty)
             where TSagaEntity : NServiceBus.IContainSagaData
         ;
     }
@@ -1041,8 +1041,8 @@ namespace NServiceBus
         where TSagaData :  class, NServiceBus.IContainSagaData
     {
         public NServiceBus.IToSagaExpression<TSagaData> ConfigureHeaderMapping<TMessage>(string headerName) { }
-        public NServiceBus.ToSagaExpression<TSagaData, TMessage> ConfigureMapping<TMessage>(System.Linq.Expressions.Expression<System.Func<TMessage, object>> messageProperty) { }
-        public NServiceBus.CorrelatedSagaPropertyMapper<TSagaData> MapSaga(System.Linq.Expressions.Expression<System.Func<TSagaData, object>> sagaProperty) { }
+        public NServiceBus.ToSagaExpression<TSagaData, TMessage> ConfigureMapping<TMessage>(System.Linq.Expressions.Expression<System.Func<TMessage, object?>> messageProperty) { }
+        public NServiceBus.CorrelatedSagaPropertyMapper<TSagaData> MapSaga(System.Linq.Expressions.Expression<System.Func<TSagaData, object?>> sagaProperty) { }
     }
     public abstract class Saga<TSagaData> : NServiceBus.Saga
         where TSagaData :  class, NServiceBus.IContainSagaData, new ()
@@ -1219,8 +1219,8 @@ namespace NServiceBus
     public class ToSagaExpression<TSagaData, TMessage>
         where TSagaData :  class, NServiceBus.IContainSagaData
     {
-        public ToSagaExpression(NServiceBus.IConfigureHowToFindSagaWithMessage sagaMessageFindingConfiguration, System.Linq.Expressions.Expression<System.Func<TMessage, object>> messageProperty) { }
-        public void ToSaga(System.Linq.Expressions.Expression<System.Func<TSagaData, object>> sagaEntityProperty) { }
+        public ToSagaExpression(NServiceBus.IConfigureHowToFindSagaWithMessage sagaMessageFindingConfiguration, System.Linq.Expressions.Expression<System.Func<TMessage, object?>> messageProperty) { }
+        public void ToSaga(System.Linq.Expressions.Expression<System.Func<TSagaData, object?>> sagaEntityProperty) { }
     }
     [System.Obsolete("Configure the transport via the TransportDefinition instance\'s properties. Will b" +
         "e removed in version 9.0.0.", true)]

diff --git a/src/NServiceBus.Core/Sagas/CorrelatedSagaPropertyMapper.cs b/src/NServiceBus.Core/Sagas/CorrelatedSagaPropertyMapper.cs
@@ -12,9 +12,9 @@ namespace NServiceBus
     public class CorrelatedSagaPropertyMapper<TSagaData> where TSagaData : class, IContainSagaData
     {
         readonly SagaPropertyMapper<TSagaData> sagaPropertyMapper;
-        readonly Expression<Func<TSagaData, object>> sagaProperty;
+        readonly Expression<Func<TSagaData, object?>> sagaProperty;
 
-        internal CorrelatedSagaPropertyMapper(SagaPropertyMapper<TSagaData> sagaPropertyMapper, Expression<Func<TSagaData, object>> sagaProperty)
+        internal CorrelatedSagaPropertyMapper(SagaPropertyMapper<TSagaData> sagaPropertyMapper, Expression<Func<TSagaData, object?>> sagaProperty)
         {
             Guard.ThrowIfNull(sagaPropertyMapper);
             Guard.ThrowIfNull(sagaProperty);
@@ -30,7 +30,7 @@ internal CorrelatedSagaPropertyMapper(SagaPropertyMapper<TSagaData> sagaProperty
         /// <returns>
         /// The same mapper instance.
         /// </returns>
-        public CorrelatedSagaPropertyMapper<TSagaData> ToMessage<TMessage>(Expression<Func<TMessage, object>> messageProperty)
+        public CorrelatedSagaPropertyMapper<TSagaData> ToMessage<TMessage>(Expression<Func<TMessage, object?>> messageProperty)
         {
             sagaPropertyMapper.ConfigureMapping(messageProperty).ToSaga(sagaProperty);
             return this;

diff --git a/src/NServiceBus.Core/Sagas/IConfigureHowToFindSagaWithMessage.cs b/src/NServiceBus.Core/Sagas/IConfigureHowToFindSagaWithMessage.cs
@@ -17,6 +17,6 @@ public interface IConfigureHowToFindSagaWithMessage
         /// of the given type, which message property should be matched to
         /// which saga entity property in the persistent saga store.
         /// </summary>
-        void ConfigureMapping<TSagaEntity, TMessage>(Expression<Func<TSagaEntity, object>> sagaEntityProperty, Expression<Func<TMessage, object>> messageProperty) where TSagaEntity : IContainSagaData;
+        void ConfigureMapping<TSagaEntity, TMessage>(Expression<Func<TSagaEntity, object?>> sagaEntityProperty, Expression<Func<TMessage, object?>> messageProperty) where TSagaEntity : IContainSagaData;
     }
 }
diff --git a/src/NServiceBus.Core/Sagas/SagaPropertyMapper.cs b/src/NServiceBus.Core/Sagas/SagaPropertyMapper.cs
@@ -24,7 +24,7 @@ internal SagaPropertyMapper(IConfigureHowToFindSagaWithMessage sagaMessageFindin
         /// <see cref="ToSagaExpression{TSagaData,TMessage}.ToSaga" /> to link <paramref name="messageProperty" /> with
         /// <typeparamref name="TSagaData" />.
         /// </returns>
-        public ToSagaExpression<TSagaData, TMessage> ConfigureMapping<TMessage>(Expression<Func<TMessage, object>> messageProperty)
+        public ToSagaExpression<TSagaData, TMessage> ConfigureMapping<TMessage>(Expression<Func<TMessage, object?>> messageProperty)
         {
             Guard.ThrowIfNull(messageProperty);
             return new ToSagaExpression<TSagaData, TMessage>(sagaMessageFindingConfiguration, messageProperty);
@@ -61,7 +61,7 @@ public IToSagaExpression<TSagaData> ConfigureHeaderMapping<TMessage>(string head
         /// <see cref="CorrelatedSagaPropertyMapper{TSagaData}.ToMessage{TMessage}"/> to map a message type to
         /// the correlation property.
         /// </returns>
-        public CorrelatedSagaPropertyMapper<TSagaData> MapSaga(Expression<Func<TSagaData, object>> sagaProperty)
+        public CorrelatedSagaPropertyMapper<TSagaData> MapSaga(Expression<Func<TSagaData, object?>> sagaProperty)
         {
             Guard.ThrowIfNull(sagaProperty);
             return new CorrelatedSagaPropertyMapper<TSagaData>(this, sagaProperty);

diff --git a/src/NServiceBus.Core/Sagas/ToSagaExpression.cs b/src/NServiceBus.Core/Sagas/ToSagaExpression.cs
@@ -13,7 +13,7 @@ public class ToSagaExpression<TSagaData, TMessage> where TSagaData : class, ICon
         /// <summary>
         /// Initializes a new instance of <see cref="ToSagaExpression{TSagaData,TMessage}" />.
         /// </summary>
-        public ToSagaExpression(IConfigureHowToFindSagaWithMessage sagaMessageFindingConfiguration, Expression<Func<TMessage, object>> messageProperty)
+        public ToSagaExpression(IConfigureHowToFindSagaWithMessage sagaMessageFindingConfiguration, Expression<Func<TMessage, object?>> messageProperty)
         {
             Guard.ThrowIfNull(sagaMessageFindingConfiguration);
             Guard.ThrowIfNull(messageProperty);
@@ -26,13 +26,13 @@ public ToSagaExpression(IConfigureHowToFindSagaWithMessage sagaMessageFindingCon
         /// Defines the property on the saga data to which the message property should be mapped.
         /// </summary>
         /// <param name="sagaEntityProperty">The property to map.</param>
-        public void ToSaga(Expression<Func<TSagaData, object>> sagaEntityProperty)
+        public void ToSaga(Expression<Func<TSagaData, object?>> sagaEntityProperty)
         {
             Guard.ThrowIfNull(sagaEntityProperty);
             sagaMessageFindingConfiguration.ConfigureMapping(sagaEntityProperty, messageProperty);
         }
 
-        readonly Expression<Func<TMessage, object>> messageProperty;
+        readonly Expression<Func<TMessage, object?>> messageProperty;
         readonly IConfigureHowToFindSagaWithMessage sagaMessageFindingConfiguration;
     }
 }