Refactor to remove null-forgiving operator related to `AltinnPaymentC…

…onfiguration.PaymentDataType` (#657)

src/Altinn.App.Api/Controllers/PaymentController.cs

@@ -29,15 +29,15 @@ public class PaymentController : ControllerBase
     /// Initializes a new instance of the <see cref="PaymentController"/> class.
     /// </summary>
     public PaymentController(
+        IServiceProvider serviceProvider,
         IInstanceClient instanceClient,
         IProcessReader processReader,
-        IPaymentService paymentService,
         IOrderDetailsCalculator? orderDetailsCalculator = null
     )
     {
         _instanceClient = instanceClient;
         _processReader = processReader;
-        _paymentService = paymentService;
+        _paymentService = serviceProvider.GetRequiredService<IPaymentService>();
         _orderDetailsCalculator = orderDetailsCalculator;
     }
 
@@ -71,9 +71,11 @@ public async Task<IActionResult> GetPaymentInformation(
             throw new PaymentException("Payment configuration not found in AltinnTaskExtension");
         }
 
+        var validPaymentConfiguration = paymentConfiguration.Validate();
+
         PaymentInformation paymentInformation = await _paymentService.CheckAndStorePaymentStatus(
             instance,
-            paymentConfiguration,
+            validPaymentConfiguration,
             language
         );
 

src/Altinn.App.Core/Features/Action/PaymentUserAction.cs

@@ -68,7 +68,7 @@ is not ProcessTask currentTask
 
             (PaymentInformation paymentInformation, bool alreadyPaid) = await _paymentService.StartPayment(
                 context.Instance,
-                paymentConfiguration,
+                paymentConfiguration.Validate(),
                 context.Language
             );
 

src/Altinn.App.Core/Features/Payment/Services/IPaymentService.cs

@@ -7,14 +7,14 @@ namespace Altinn.App.Core.Features.Payment.Services
     /// <summary>
     /// Service for handling payment.
     /// </summary>
-    public interface IPaymentService
+    internal interface IPaymentService
     {
         /// <summary>
         /// Start payment for an instance. Will clean up any existing non-completed payment before starting a new payment.
         /// </summary>
         Task<(PaymentInformation paymentInformation, bool alreadyPaid)> StartPayment(
             Instance instance,
-            AltinnPaymentConfiguration paymentConfiguration,
+            ValidAltinnPaymentConfiguration paymentConfiguration,
             string? language
         );
 
@@ -23,18 +23,18 @@ public interface IPaymentService
         /// </summary>
         Task<PaymentInformation> CheckAndStorePaymentStatus(
             Instance instance,
-            AltinnPaymentConfiguration paymentConfiguration,
+            ValidAltinnPaymentConfiguration paymentConfiguration,
             string? language
         );
 
         /// <summary>
         /// Check our internal state to see if payment is complete.
         /// </summary>
-        Task<bool> IsPaymentCompleted(Instance instance, AltinnPaymentConfiguration paymentConfiguration);
+        Task<bool> IsPaymentCompleted(Instance instance, ValidAltinnPaymentConfiguration paymentConfiguration);
 
         /// <summary>
         /// Cancel payment with payment processor and delete internal payment information.
         /// </summary>
-        Task CancelAndDeleteAnyExistingPayment(Instance instance, AltinnPaymentConfiguration paymentConfiguration);
+        Task CancelAndDeleteAnyExistingPayment(Instance instance, ValidAltinnPaymentConfiguration paymentConfiguration);
     }
 }

src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs

@@ -1,7 +1,6 @@
 using Altinn.App.Core.Features.Payment.Exceptions;
 using Altinn.App.Core.Features.Payment.Models;
 using Altinn.App.Core.Features.Payment.Processors;
-using Altinn.App.Core.Internal.App;
 using Altinn.App.Core.Internal.Data;
 using Altinn.App.Core.Internal.Process.Elements.AltinnExtensionProperties;
 using Altinn.App.Core.Models;
@@ -39,7 +38,7 @@ public PaymentService(
     /// <inheritdoc/>
     public async Task<(PaymentInformation paymentInformation, bool alreadyPaid)> StartPayment(
         Instance instance,
-        AltinnPaymentConfiguration paymentConfiguration,
+        ValidAltinnPaymentConfiguration paymentConfiguration,
         string? language
     )
     {
@@ -52,9 +51,7 @@ public PaymentService(
             );
         }
 
-        ValidateConfig(paymentConfiguration);
-        // ! TODO: restructure code to avoid assertion (it is validated above)
-        string dataTypeId = paymentConfiguration.PaymentDataType!;
+        string dataTypeId = paymentConfiguration.PaymentDataType;
 
         (Guid dataElementId, PaymentInformation? existingPaymentInformation) =
             await _dataService.GetByType<PaymentInformation>(instance, dataTypeId);
@@ -118,7 +115,7 @@ public PaymentService(
     /// <inheritdoc/>
     public async Task<PaymentInformation> CheckAndStorePaymentStatus(
         Instance instance,
-        AltinnPaymentConfiguration paymentConfiguration,
+        ValidAltinnPaymentConfiguration paymentConfiguration,
         string? language
     )
     {
@@ -131,10 +128,7 @@ public async Task<PaymentInformation> CheckAndStorePaymentStatus(
             );
         }
 
-        ValidateConfig(paymentConfiguration);
-
-        // ! TODO: restructure code to avoid assertion (it is validated above)
-        string dataTypeId = paymentConfiguration.PaymentDataType!;
+        string dataTypeId = paymentConfiguration.PaymentDataType;
         (Guid dataElementId, PaymentInformation? paymentInformation) = await _dataService.GetByType<PaymentInformation>(
             instance,
             dataTypeId
@@ -204,12 +198,9 @@ await _dataService.UpdateJsonObject(
     }
 
     /// <inheritdoc/>
-    public async Task<bool> IsPaymentCompleted(Instance instance, AltinnPaymentConfiguration paymentConfiguration)
+    public async Task<bool> IsPaymentCompleted(Instance instance, ValidAltinnPaymentConfiguration paymentConfiguration)
     {
-        ValidateConfig(paymentConfiguration);
-
-        // ! TODO: restructure code to avoid assertion (it is validated above)
-        string dataTypeId = paymentConfiguration.PaymentDataType!;
+        string dataTypeId = paymentConfiguration.PaymentDataType;
         (Guid _, PaymentInformation? paymentInformation) = await _dataService.GetByType<PaymentInformation>(
             instance,
             dataTypeId
@@ -226,13 +217,10 @@ public async Task<bool> IsPaymentCompleted(Instance instance, AltinnPaymentConfi
     /// <inheritdoc/>
     public async Task CancelAndDeleteAnyExistingPayment(
         Instance instance,
-        AltinnPaymentConfiguration paymentConfiguration
+        ValidAltinnPaymentConfiguration paymentConfiguration
     )
     {
-        ValidateConfig(paymentConfiguration);
-
-        // ! TODO: restructure code to avoid assertion (it is validated above)
-        string dataTypeId = paymentConfiguration.PaymentDataType!;
+        string dataTypeId = paymentConfiguration.PaymentDataType;
         (Guid dataElementId, PaymentInformation? paymentInformation) = await _dataService.GetByType<PaymentInformation>(
             instance,
             dataTypeId
@@ -275,21 +263,4 @@ private async Task CancelAndDelete(Instance instance, Guid dataElementId, Paymen
         await _dataService.DeleteById(new InstanceIdentifier(instance), dataElementId);
         _logger.LogDebug("Payment information for deleted for instance {InstanceId}.", instance.Id);
     }
-
-    private static void ValidateConfig(AltinnPaymentConfiguration paymentConfiguration)
-    {
-        List<string> errorMessages = [];
-
-        if (string.IsNullOrWhiteSpace(paymentConfiguration.PaymentDataType))
-        {
-            errorMessages.Add("PaymentDataType is missing.");
-        }
-
-        if (errorMessages.Count != 0)
-        {
-            throw new ApplicationConfigException(
-                "Payment process task configuration is not valid: " + string.Join(",\n", errorMessages)
-            );
-        }
-    }
 }

src/Altinn.App.Core/Internal/Process/Elements/AltinnExtensionProperties/AltinnPaymentConfiguration.cs

@@ -1,4 +1,6 @@
+using System.Diagnostics.CodeAnalysis;
 using System.Xml.Serialization;
+using Altinn.App.Core.Internal.App;
 
 namespace Altinn.App.Core.Internal.Process.Elements.AltinnExtensionProperties
 {
@@ -12,5 +14,45 @@ public class AltinnPaymentConfiguration
         /// </summary>
         [XmlElement("paymentDataType", Namespace = "http://altinn.no/process")]
         public string? PaymentDataType { get; set; }
+
+        internal ValidAltinnPaymentConfiguration Validate()
+        {
+            List<string>? errorMessages = null;
+
+            var paymentDataType = PaymentDataType;
+
+            if (paymentDataType.IsNullOrWhitespace(ref errorMessages, "PaymentDataType is missing."))
+                ThrowApplicationConfigException(errorMessages);
+
+            return new ValidAltinnPaymentConfiguration(paymentDataType);
+        }
+
+        [DoesNotReturn]
+        private static void ThrowApplicationConfigException(List<string> errorMessages)
+        {
+            throw new ApplicationConfigException(
+                "Payment process task configuration is not valid: " + string.Join(",\n", errorMessages)
+            );
+        }
+    }
+
+    internal readonly record struct ValidAltinnPaymentConfiguration(string PaymentDataType);
+
+    file static class ValidationExtensions
+    {
+        internal static bool IsNullOrWhitespace(
+            [NotNullWhen(false)] this string? value,
+            [NotNullWhen(true)] ref List<string>? errors,
+            string error
+        )
+        {
+            var result = string.IsNullOrWhiteSpace(value);
+            if (result)
+            {
+                errors ??= new List<string>(1);
+                errors.Add(error);
+            }
+            return result;
+        }
     }
 }

src/Altinn.App.Core/Internal/Process/ProcessTasks/PaymentProcessTask.cs

@@ -44,25 +44,24 @@ IPaymentService paymentService
         public async Task Start(string taskId, Instance instance)
         {
             AltinnPaymentConfiguration paymentConfiguration = GetAltinnPaymentConfiguration(taskId);
-            await _paymentService.CancelAndDeleteAnyExistingPayment(instance, paymentConfiguration);
+            await _paymentService.CancelAndDeleteAnyExistingPayment(instance, paymentConfiguration.Validate());
         }
 
         /// <inheritdoc/>
         public async Task End(string taskId, Instance instance)
         {
             AltinnPaymentConfiguration paymentConfiguration = GetAltinnPaymentConfiguration(taskId);
 
-            if (!await _paymentService.IsPaymentCompleted(instance, paymentConfiguration))
+            if (!await _paymentService.IsPaymentCompleted(instance, paymentConfiguration.Validate()))
                 throw new PaymentException("The payment is not completed.");
 
             Stream pdfStream = await _pdfService.GeneratePdf(instance, taskId, CancellationToken.None);
 
-            // ! TODO: restructure code to avoid assertion. Codepaths above have already validated this field
-            var paymentDataType = paymentConfiguration.PaymentDataType!;
+            var validatedPaymentConfiguration = paymentConfiguration.Validate();
 
             await _dataClient.InsertBinaryData(
                 instance.Id,
-                paymentDataType,
+                validatedPaymentConfiguration.PaymentDataType,
                 PdfContentType,
                 ReceiptFileName,
                 pdfStream,
@@ -74,7 +73,7 @@ await _dataClient.InsertBinaryData(
         public async Task Abandon(string taskId, Instance instance)
         {
             AltinnPaymentConfiguration paymentConfiguration = GetAltinnPaymentConfiguration(taskId);
-            await _paymentService.CancelAndDeleteAnyExistingPayment(instance, paymentConfiguration);
+            await _paymentService.CancelAndDeleteAnyExistingPayment(instance, paymentConfiguration.Validate());
         }
 
         private AltinnPaymentConfiguration GetAltinnPaymentConfiguration(string taskId)
@@ -90,12 +89,7 @@ private AltinnPaymentConfiguration GetAltinnPaymentConfiguration(string taskId)
                 );
             }
 
-            if (string.IsNullOrWhiteSpace(paymentConfiguration.PaymentDataType))
-            {
-                throw new ApplicationConfigException(
-                    "PaymentDataType is missing in the payment process task configuration."
-                );
-            }
+            _ = paymentConfiguration.Validate();
 
             return paymentConfiguration;
         }

test/Altinn.App.Core.Tests/Features/Action/PaymentUserActionTests.cs

@@ -55,7 +55,7 @@ public async Task HandleAction_returns_redirect_result_correctly()
 
         _paymentServiceMock
             .Setup(x =>
-                x.StartPayment(It.IsAny<Instance>(), It.IsAny<AltinnPaymentConfiguration>(), It.IsAny<string>())
+                x.StartPayment(It.IsAny<Instance>(), It.IsAny<ValidAltinnPaymentConfiguration>(), It.IsAny<string>())
             )
             .ReturnsAsync((paymentInformation, false));
 
@@ -94,7 +94,7 @@ public async Task HandleAction_returns_success_result_when_no_redirect_url()
 
         _paymentServiceMock
             .Setup(x =>
-                x.StartPayment(It.IsAny<Instance>(), It.IsAny<AltinnPaymentConfiguration>(), It.IsAny<string>())
+                x.StartPayment(It.IsAny<Instance>(), It.IsAny<ValidAltinnPaymentConfiguration>(), It.IsAny<string>())
             )
             .ReturnsAsync((paymentInformation, false));
 
@@ -131,7 +131,7 @@ public async Task HandleAction_returns_failure_result_when_already_paid()
 
         _paymentServiceMock
             .Setup(x =>
-                x.StartPayment(It.IsAny<Instance>(), It.IsAny<AltinnPaymentConfiguration>(), It.IsAny<string>())
+                x.StartPayment(It.IsAny<Instance>(), It.IsAny<ValidAltinnPaymentConfiguration>(), It.IsAny<string>())
             )
             .ReturnsAsync((paymentInformation, true));
 

test/Altinn.App.Core.Tests/Features/Payment/AltinnPaymentConfigurationTests.cs

@@ -0,0 +1,32 @@
+using Altinn.App.Core.Internal.App;
+using Altinn.App.Core.Internal.Process.Elements.AltinnExtensionProperties;
+using FluentAssertions;
+
+namespace Altinn.App.Core.Tests.Features.Payment;
+
+public class AltinnPaymentConfigurationTests
+{
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    [InlineData(" ")]
+    public void Validation_ThrowsException_When_PaymentDataType_Is_Invalid(string? paymentDataType)
+    {
+        AltinnPaymentConfiguration paymentConfiguration = new() { PaymentDataType = paymentDataType };
+
+        var action = () => paymentConfiguration.Validate();
+
+        action.Should().Throw<ApplicationConfigException>();
+    }
+
+    [Fact]
+    public void Validation_Succeeds()
+    {
+        var paymentDataType = "paymentDataType";
+        AltinnPaymentConfiguration paymentConfiguration = new() { PaymentDataType = paymentDataType };
+        paymentConfiguration.PaymentDataType.Should().Be(paymentDataType);
+
+        var validPaymentConfiguration = paymentConfiguration.Validate();
+        validPaymentConfiguration.PaymentDataType.Should().Be(paymentDataType);
+    }
+}