From d70d21aaba93769675a57569a897e307449755cb Mon Sep 17 00:00:00 2001 From: Martin Othamar Date: Sat, 25 May 2024 21:33:39 +0200 Subject: [PATCH] Remove/fix or suppress null-forgiving operator usage in `src/`, enable warning/build errors (#636) --- .editorconfig | 3 ++ Directory.Build.props | 5 --- .../Controllers/InstancesController.cs | 22 +++++++-- .../Controllers/PdfController.cs | 6 ++- .../RequestHandling/MultipartRequestReader.cs | 3 +- .../Helpers/RequestHandling/RequestPart.cs | 10 ++++- src/Altinn.App.Api/Models/SimpleInstance.cs | 9 +++- .../DefaultEFormidlingService.cs | 4 +- .../EformidlingStatusCheckEventHandler.cs | 4 +- .../EformidlingStatusCheckEventHandler2.cs | 4 +- .../Extensions/HttpContextExtensions.cs | 3 +- .../Extensions/XmlToLinqExtensions.cs | 5 ++- .../Features/Action/SigningUserAction.cs | 18 ++++---- .../Action/UniqueSignatureAuthorizer.cs | 6 +-- .../Altinn2CodeListProvider.cs | 1 + .../Features/Options/AppOptionsFileHandler.cs | 4 +- .../Processors/Nets/NetsPaymentProcessor.cs | 11 ++++- .../Payment/Services/PaymentService.cs | 9 +++- .../Default/DataAnnotationValidator.cs | 10 ++++- ...gacyIInstanceValidatorFormDataValidator.cs | 11 ++++- .../LegacyIInstanceValidatorTaskValidator.cs | 11 ++++- .../Validation/Helpers/ModelStateHelpers.cs | 2 +- src/Altinn.App.Core/Helpers/ProcessHelper.cs | 3 ++ .../Serialization/ModelDeserializer.cs | 2 +- .../Implementation/AppResourcesSI.cs | 9 ++-- .../Implementation/PrefillSI.cs | 30 ++++++++++--- .../Clients/Events/EventsClient.cs | 2 +- .../Clients/KeyVault/SecretsLocalClient.cs | 1 + .../Clients/Storage/DataClient.cs | 13 +++++- .../Clients/Storage/InstanceClient.cs | 15 +++++-- .../Clients/Storage/InstanceEventClient.cs | 4 +- .../Clients/Storage/ProcessClient.cs | 1 + .../Internal/AppModel/DefaultAppModel.cs | 7 ++- .../Expressions/ExpressionEvaluator.cs | 1 + .../Internal/Language/ApplicationLanguage.cs | 2 + .../Internal/Linq/Extensions.cs | 18 ++++++++ .../Internal/Patch/PatchService.cs | 6 +-- .../Internal/Process/ProcessEngine.cs | 28 +++++++++--- .../Process/ProcessEventHandlingDelegator.cs | 3 ++ .../Internal/Process/ProcessReader.cs | 7 +-- .../Common/ProcessTaskFinalizer.cs | 6 ++- .../ProcessTasks/PaymentProcessTask.cs | 5 ++- .../Models/ApplicationMetadata.cs | 3 +- .../Models/Layout/Components/GridComponent.cs | 10 ++--- .../Models/Layout/PageComponentConverter.cs | 45 ++++++++++++++----- .../Models/Validation/ValidationIssue.cs | 5 ++- src/Directory.Build.props | 4 ++ .../Validators/ValidationServiceTests.cs | 22 +++++++++ .../LayoutModelConverterFromObject.cs | 15 +++++-- 49 files changed, 333 insertions(+), 95 deletions(-) create mode 100644 src/Altinn.App.Core/Internal/Linq/Extensions.cs diff --git a/.editorconfig b/.editorconfig index 5fb6a86b8..5243a2812 100644 --- a/.editorconfig +++ b/.editorconfig @@ -102,6 +102,9 @@ dotnet_diagnostic.CA2254.severity = none # CA1822: Mark members as static dotnet_diagnostic.CA1822.severity = suggestion +# IDE0080: Remove unnecessary suppression operator +dotnet_diagnostic.IDE0080.severity = error + [Program.cs] dotnet_diagnostic.CA1050.severity = none dotnet_diagnostic.S1118.severity = none diff --git a/Directory.Build.props b/Directory.Build.props index 7f5799428..ad238e3a6 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -16,10 +16,5 @@ all runtime; build; native; contentfiles; analyzers - - diff --git a/src/Altinn.App.Api/Controllers/InstancesController.cs b/src/Altinn.App.Api/Controllers/InstancesController.cs index a32dfa55e..2e895eb7d 100644 --- a/src/Altinn.App.Api/Controllers/InstancesController.cs +++ b/src/Altinn.App.Api/Controllers/InstancesController.cs @@ -231,10 +231,15 @@ [FromQuery] int? instanceOwnerPartyId } else { + if (instanceOwnerPartyId is null) + { + return StatusCode(500, "Can't create minimal instance when Instance owner Party ID is null"); + } + // create minimum instance template instanceTemplate = new Instance { - InstanceOwner = new InstanceOwner { PartyId = instanceOwnerPartyId!.Value.ToString() } + InstanceOwner = new InstanceOwner { PartyId = instanceOwnerPartyId.Value.ToString() } }; } @@ -1072,6 +1077,15 @@ private async Task StorePrefillParts(Instance instance, ApplicationMetadata appI foreach (RequestPart part in parts) { + // NOTE: part.Name is nullable on the type here, but `RequestPartValidator.ValidatePart` which is called + // further up the stack will error out if it actually null, so we just sanity-check here + // and throw if it is null. + // TODO: improve the modelling of this type. + if (part.Name is null) + { + throw new InvalidOperationException("Unexpected state - part name is null"); + } + DataType? dataType = appInfo.DataTypes.Find(d => d.Id == part.Name); DataElement dataElement; @@ -1101,7 +1115,7 @@ private async Task StorePrefillParts(Instance instance, ApplicationMetadata appI throw new InvalidOperationException(deserializer.Error); } - await _prefillService.PrefillDataModel(instance.InstanceOwner.PartyId, part.Name!, data); + await _prefillService.PrefillDataModel(instance.InstanceOwner.PartyId, part.Name, data); await _instantiationProcessor.DataCreation(instance, data, null); @@ -1114,14 +1128,14 @@ private async Task StorePrefillParts(Instance instance, ApplicationMetadata appI org, app, instanceOwnerIdAsInt, - part.Name! + part.Name ); } else { dataElement = await _dataClient.InsertBinaryData( instance.Id, - part.Name!, + part.Name, part.ContentType, part.FileName, part.Stream diff --git a/src/Altinn.App.Api/Controllers/PdfController.cs b/src/Altinn.App.Api/Controllers/PdfController.cs index 97d49b650..81ea4944a 100644 --- a/src/Altinn.App.Api/Controllers/PdfController.cs +++ b/src/Altinn.App.Api/Controllers/PdfController.cs @@ -128,7 +128,9 @@ [FromRoute] Guid dataGuid LayoutSet? layoutSet = null; if (!string.IsNullOrEmpty(layoutSetsString)) { - layoutSets = JsonSerializer.Deserialize(layoutSetsString, _jsonSerializerOptions)!; + layoutSets = + JsonSerializer.Deserialize(layoutSetsString, _jsonSerializerOptions) + ?? throw new JsonException("Could not deserialize LayoutSets"); layoutSet = layoutSets.Sets?.FirstOrDefault(t => t.DataType.Equals(dataElement.DataType) && t.Tasks.Contains(taskId) ); @@ -145,7 +147,7 @@ [FromRoute] Guid dataGuid layoutSettings = JsonSerializer.Deserialize( layoutSettingsFileContent, _jsonSerializerOptions - )!; + ); } // Ensure layoutsettings are initialized in FormatPdf diff --git a/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs b/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs index e9d53242a..462a35ada 100644 --- a/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs +++ b/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs @@ -132,7 +132,8 @@ out ContentDispositionHeaderValue? contentDisposition private string GetBoundary() { MediaTypeHeaderValue mediaType = MediaTypeHeaderValue.Parse(request.ContentType); - return mediaType.Boundary.Value!.Trim('"'); + return mediaType.Boundary.Value?.Trim('"') + ?? throw new Exception("Could not retrieve boundary value from Content-Type header"); } } } diff --git a/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs b/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs index e9fe66efb..b0b37213c 100644 --- a/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs +++ b/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs @@ -8,7 +8,10 @@ public class RequestPart /// /// The stream to access this part. /// - public Stream Stream { get; set; } = default!; +#nullable disable + public Stream Stream { get; set; } + +#nullable restore /// /// The file name as given in content description. @@ -23,7 +26,10 @@ public class RequestPart /// /// The content type of the part. /// - public string ContentType { get; set; } = default!; +#nullable disable + public string ContentType { get; set; } + +#nullable restore /// /// The file size of the part, 0 if not given. diff --git a/src/Altinn.App.Api/Models/SimpleInstance.cs b/src/Altinn.App.Api/Models/SimpleInstance.cs index 839bfebe7..626a59eb9 100644 --- a/src/Altinn.App.Api/Models/SimpleInstance.cs +++ b/src/Altinn.App.Api/Models/SimpleInstance.cs @@ -8,7 +8,10 @@ public class SimpleInstance /// /// The instance identifier formated as {instanceOwner.partyId}/{instanceGuid}. /// - public string Id { get; set; } = default!; +#nullable disable + public string Id { get; set; } + +#nullable restore /// /// Presentation texts from the instance @@ -28,5 +31,7 @@ public class SimpleInstance /// /// Full name of user to last change the instance. /// - public string LastChangedBy { get; set; } = default!; +#nullable disable + public string LastChangedBy { get; set; } +#nullable restore } diff --git a/src/Altinn.App.Core/EFormidling/Implementation/DefaultEFormidlingService.cs b/src/Altinn.App.Core/EFormidling/Implementation/DefaultEFormidlingService.cs index 48f476f79..dbe3e2d61 100644 --- a/src/Altinn.App.Core/EFormidling/Implementation/DefaultEFormidlingService.cs +++ b/src/Altinn.App.Core/EFormidling/Implementation/DefaultEFormidlingService.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using Altinn.App.Core.Configuration; using Altinn.App.Core.Constants; using Altinn.App.Core.EFormidling.Interface; @@ -216,7 +217,8 @@ private async Task SendInstanceData(Instance instance, Dictionary AddCompleteConfirmation(InstanceIdentifier instanceIdentifier) + private async Task AddCompleteConfirmation(InstanceIdentifier instanceIdentifier) { string url = $"instances/{instanceIdentifier.InstanceOwnerPartyId}/{instanceIdentifier.InstanceGuid}/complete"; @@ -169,7 +169,7 @@ private async Task AddCompleteConfirmation(InstanceIdentifier instance if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); - Instance instance = JsonConvert.DeserializeObject(instanceData)!; + Instance? instance = JsonConvert.DeserializeObject(instanceData); return instance; } diff --git a/src/Altinn.App.Core/EFormidling/Implementation/EformidlingStatusCheckEventHandler2.cs b/src/Altinn.App.Core/EFormidling/Implementation/EformidlingStatusCheckEventHandler2.cs index b5b23f5c6..dae04b275 100644 --- a/src/Altinn.App.Core/EFormidling/Implementation/EformidlingStatusCheckEventHandler2.cs +++ b/src/Altinn.App.Core/EFormidling/Implementation/EformidlingStatusCheckEventHandler2.cs @@ -133,7 +133,7 @@ await response.Content.ReadAsStringAsync() /// endpoint in InstanceController. This method should be removed once we have a better /// alernative for authenticating the app/org without having a http request context with /// a logged on user/org. - private async Task AddCompleteConfirmation(InstanceIdentifier instanceIdentifier) + private async Task AddCompleteConfirmation(InstanceIdentifier instanceIdentifier) { string url = $"instances/{instanceIdentifier.InstanceOwnerPartyId}/{instanceIdentifier.InstanceGuid}/complete"; @@ -154,7 +154,7 @@ private async Task AddCompleteConfirmation(InstanceIdentifier instance if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); - Instance instance = JsonConvert.DeserializeObject(instanceData)!; + Instance? instance = JsonConvert.DeserializeObject(instanceData); return instance; } diff --git a/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs b/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs index f12393448..ce049a48e 100644 --- a/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs +++ b/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs @@ -15,7 +15,8 @@ public static class HttpContextExtensions public static StreamContent CreateContentStream(this HttpRequest request) { StreamContent content = new StreamContent(request.Body); - content.Headers.ContentType = MediaTypeHeaderValue.Parse(request.ContentType!); + ArgumentNullException.ThrowIfNull(request.ContentType); + content.Headers.ContentType = MediaTypeHeaderValue.Parse(request.ContentType); if (request.Headers.TryGetValue("Content-Disposition", out StringValues headerValues)) { diff --git a/src/Altinn.App.Core/Extensions/XmlToLinqExtensions.cs b/src/Altinn.App.Core/Extensions/XmlToLinqExtensions.cs index 00d7a7c5a..e534e4353 100644 --- a/src/Altinn.App.Core/Extensions/XmlToLinqExtensions.cs +++ b/src/Altinn.App.Core/Extensions/XmlToLinqExtensions.cs @@ -27,7 +27,10 @@ public static class XmlToLinqExtensions /// public static XElement AddAttribute(this XElement element, string attributeName, object value) { - element.Add(new XAttribute(attributeName, value.ToString()!)); + var valueStr = + value.ToString() + ?? throw new ArgumentException("String representation of parameter 'value' is null", nameof(value)); + element.Add(new XAttribute(attributeName, valueStr)); return element; } diff --git a/src/Altinn.App.Core/Features/Action/SigningUserAction.cs b/src/Altinn.App.Core/Features/Action/SigningUserAction.cs index 86444a228..f1c95b63c 100644 --- a/src/Altinn.App.Core/Features/Action/SigningUserAction.cs +++ b/src/Altinn.App.Core/Features/Action/SigningUserAction.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.Process; using Altinn.App.Core.Internal.Process.Elements; @@ -75,14 +76,16 @@ public async Task HandleAction(UserActionContext context) ?.DataTypes?.Where(d => dataTypeIds.Contains(d.Id, StringComparer.OrdinalIgnoreCase)) .ToList(); - if (ShouldSign(currentTask, context.Instance.Data, dataTypesToSign)) + if ( + GetDataTypeForSignature(currentTask, context.Instance.Data, dataTypesToSign) is string signatureDataType + ) { var dataElementSignatures = GetDataElementSignatures(context.Instance.Data, dataTypesToSign); SignatureContext signatureContext = new( new InstanceIdentifier(context.Instance), currentTask.Id, - currentTask.ExtensionElements?.TaskExtension?.SignatureConfiguration?.SignatureDataType!, + signatureDataType, await GetSignee(context.UserId.Value), dataElementSignatures ); @@ -100,24 +103,23 @@ await GetSignee(context.UserId.Value), ); } - private static bool ShouldSign( + private static string? GetDataTypeForSignature( ProcessTask currentTask, List dataElements, List? dataTypesToSign ) { - var signatureIsConfigured = - currentTask.ExtensionElements?.TaskExtension?.SignatureConfiguration?.SignatureDataType is not null; - if (dataTypesToSign is null or [] || !signatureIsConfigured) + var signatureDataType = currentTask.ExtensionElements?.TaskExtension?.SignatureConfiguration?.SignatureDataType; + if (dataTypesToSign is null or [] || signatureDataType is null) { - return false; + return null; } var dataElementMatchExists = dataElements.Any(de => dataTypesToSign.Any(dt => string.Equals(dt.Id, de.DataType, StringComparison.OrdinalIgnoreCase)) ); var allDataTypesAreOptional = dataTypesToSign.All(d => d.MinCount == 0); - return dataElementMatchExists || allDataTypesAreOptional; + return dataElementMatchExists || allDataTypesAreOptional ? signatureDataType : null; } private static List GetDataElementSignatures( diff --git a/src/Altinn.App.Core/Features/Action/UniqueSignatureAuthorizer.cs b/src/Altinn.App.Core/Features/Action/UniqueSignatureAuthorizer.cs index f4066185a..6deaf60f7 100644 --- a/src/Altinn.App.Core/Features/Action/UniqueSignatureAuthorizer.cs +++ b/src/Altinn.App.Core/Features/Action/UniqueSignatureAuthorizer.cs @@ -64,9 +64,9 @@ public async Task AuthorizeAction(UserActionAuthorizerContext context) context.InstanceIdentifier.InstanceGuid ); var dataTypes = flowElement - .ExtensionElements! - .TaskExtension! - .SignatureConfiguration! + .ExtensionElements + .TaskExtension + .SignatureConfiguration .UniqueFromSignaturesInDataTypes; var signatureDataElements = instance.Data.Where(d => dataTypes.Contains(d.DataType)).ToList(); foreach (var signatureDataElement in signatureDataElements) diff --git a/src/Altinn.App.Core/Features/Options/Altinn2Provider/Altinn2CodeListProvider.cs b/src/Altinn.App.Core/Features/Options/Altinn2Provider/Altinn2CodeListProvider.cs index 40326e05e..5a253fce1 100644 --- a/src/Altinn.App.Core/Features/Options/Altinn2Provider/Altinn2CodeListProvider.cs +++ b/src/Altinn.App.Core/Features/Options/Altinn2Provider/Altinn2CodeListProvider.cs @@ -76,6 +76,7 @@ public async Task GetRawAltinn2CodelistAsync(string? l _ => "1044", // default to norwegian bokmål }; + // ! TODO: address this is next major release, should never return null return ( await _cache.GetOrCreateAsync( $"{_metadataApiId}{langCode}{_codeListVersion}", diff --git a/src/Altinn.App.Core/Features/Options/AppOptionsFileHandler.cs b/src/Altinn.App.Core/Features/Options/AppOptionsFileHandler.cs index ca12a2036..66026c9bb 100644 --- a/src/Altinn.App.Core/Features/Options/AppOptionsFileHandler.cs +++ b/src/Altinn.App.Core/Features/Options/AppOptionsFileHandler.cs @@ -37,10 +37,10 @@ public AppOptionsFileHandler(IOptions settings) if (File.Exists(filename)) { string fileData = await File.ReadAllTextAsync(filename, Encoding.UTF8); - List options = JsonSerializer.Deserialize>( + List? options = JsonSerializer.Deserialize>( fileData, _jsonSerializerOptions - )!; + ); return options; } diff --git a/src/Altinn.App.Core/Features/Payment/Processors/Nets/NetsPaymentProcessor.cs b/src/Altinn.App.Core/Features/Payment/Processors/Nets/NetsPaymentProcessor.cs index e51c18384..3503e9812 100644 --- a/src/Altinn.App.Core/Features/Payment/Processors/Nets/NetsPaymentProcessor.cs +++ b/src/Altinn.App.Core/Features/Payment/Processors/Nets/NetsPaymentProcessor.cs @@ -146,17 +146,24 @@ public async Task TerminatePayment(Instance instance, PaymentInformation p ); } - NetsPayment payment = httpApiResult.Result.Payment!; + NetsPayment payment = + httpApiResult.Result.Payment + ?? throw new PaymentException("Payment information is null in the response from Nets"); decimal? chargedAmount = payment.Summary?.ChargedAmount; PaymentStatus status = chargedAmount > 0 ? PaymentStatus.Paid : PaymentStatus.Created; NetsPaymentDetails? paymentPaymentDetails = payment.PaymentDetails; + var checkout = + payment.Checkout ?? throw new PaymentException("Checkout information is missing in the response from Nets"); + var checkoutUrl = + checkout.Url ?? throw new PaymentException("Checkout URL is missing in the response from Nets"); + PaymentDetails paymentDetails = new() { PaymentId = paymentId, - RedirectUrl = AddLanguageQueryParam(payment.Checkout!.Url!, language), + RedirectUrl = AddLanguageQueryParam(checkoutUrl, language), Payer = NetsMapper.MapPayerDetails(payment.Consumer), PaymentType = paymentPaymentDetails?.PaymentType, PaymentMethod = paymentPaymentDetails?.PaymentMethod, diff --git a/src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs b/src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs index 3c428daa2..5735a4194 100644 --- a/src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs +++ b/src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs @@ -53,6 +53,7 @@ public PaymentService( } ValidateConfig(paymentConfiguration); + // ! TODO: restructure code to avoid assertion (it is validated above) string dataTypeId = paymentConfiguration.PaymentDataType!; (Guid dataElementId, PaymentInformation? existingPaymentInformation) = @@ -132,6 +133,7 @@ public async Task CheckAndStorePaymentStatus( ValidateConfig(paymentConfiguration); + // ! TODO: restructure code to avoid assertion (it is validated above) string dataTypeId = paymentConfiguration.PaymentDataType!; (Guid dataElementId, PaymentInformation? paymentInformation) = await _dataService.GetByType( instance, @@ -153,7 +155,6 @@ public async Task CheckAndStorePaymentStatus( }; } - PaymentDetails paymentDetails = paymentInformation.PaymentDetails!; decimal totalPriceIncVat = paymentInformation.OrderDetails.TotalPriceIncVat; string paymentProcessorId = paymentInformation.OrderDetails.PaymentProcessorId; @@ -172,6 +173,10 @@ public async Task CheckAndStorePaymentStatus( _paymentProcessors.FirstOrDefault(p => p.PaymentProcessorId == paymentProcessorId) ?? throw new PaymentException($"Payment processor with ID '{paymentProcessorId}' not found."); + PaymentDetails paymentDetails = + paymentInformation.PaymentDetails + ?? throw new PaymentException("Payment details unexpectedly missing from payment information."); + (PaymentStatus paymentStatus, PaymentDetails updatedPaymentDetails) = await paymentProcessor.GetPaymentStatus( instance, paymentDetails.PaymentId, @@ -203,6 +208,7 @@ public async Task IsPaymentCompleted(Instance instance, AltinnPaymentConfi { ValidateConfig(paymentConfiguration); + // ! TODO: restructure code to avoid assertion (it is validated above) string dataTypeId = paymentConfiguration.PaymentDataType!; (Guid _, PaymentInformation? paymentInformation) = await _dataService.GetByType( instance, @@ -225,6 +231,7 @@ AltinnPaymentConfiguration paymentConfiguration { ValidateConfig(paymentConfiguration); + // ! TODO: restructure code to avoid assertion (it is validated above) string dataTypeId = paymentConfiguration.PaymentDataType!; (Guid dataElementId, PaymentInformation? paymentInformation) = await _dataService.GetByType( instance, diff --git a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs index e082d4a64..210afb25b 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs @@ -60,13 +60,21 @@ public Task> ValidateFormData( try { var modelState = new ModelStateDictionary(); + var httpContext = _httpContextAccessor.HttpContext; + if (httpContext is null) + { + throw new Exception("Could not get HttpContext - must be in a request context to validate form data"); + } var actionContext = new ActionContext( - _httpContextAccessor.HttpContext!, + httpContext, new Microsoft.AspNetCore.Routing.RouteData(), new ActionDescriptor(), modelState ); ValidationStateDictionary validationState = new ValidationStateDictionary(); + // ! TODO: 'prefix' on the interfacee is non-nullable, but on the actual implementation + // ! TODO: it seems to be nullable, so this should be safe.. + // ! TODO: https://github.com/dotnet/aspnetcore/blob/5ff2399a2b9ea6346dcdcf2cc8ba65fba67d035a/src/Mvc/Mvc.Core/src/ModelBinding/ObjectModelValidator.cs#L41 _objectModelValidator.Validate(actionContext, validationState, null!, data); return Task.FromResult( diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs index 402202385..5533a391f 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs @@ -1,4 +1,5 @@ #pragma warning disable CS0618 // Type or member is obsolete +using System.Diagnostics; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features.Validation.Helpers; using Altinn.App.Core.Models.Validation; @@ -34,7 +35,15 @@ public LegacyIInstanceValidatorFormDataValidator( public string DataType => _instanceValidator is null ? "" : "*"; /// > - public string ValidationSource => _instanceValidator?.GetType().FullName ?? GetType().FullName!; + public string ValidationSource + { + get + { + var type = _instanceValidator?.GetType() ?? GetType(); + Debug.Assert(type.FullName is not null, "FullName does not return null on class/struct types"); + return type.FullName; + } + } /// /// Always run for incremental validation (if it exists) diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs index b22b90b12..246fb7ef7 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs @@ -1,4 +1,5 @@ #pragma warning disable CS0618 // Type or member is obsolete +using System.Diagnostics; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features.Validation.Helpers; using Altinn.App.Core.Models.Validation; @@ -35,7 +36,15 @@ public LegacyIInstanceValidatorTaskValidator( public string TaskId => "*"; /// - public string ValidationSource => _instanceValidator?.GetType().FullName ?? GetType().FullName!; + public string ValidationSource + { + get + { + var type = _instanceValidator?.GetType() ?? GetType(); + Debug.Assert(type.FullName is not null, "FullName does not return null on class/struct types"); + return type.FullName; + } + } /// public async Task> ValidateTask(Instance instance, string taskId, string? language) diff --git a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs index c491c539c..a5ab2fc19 100644 --- a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs +++ b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs @@ -49,7 +49,7 @@ string source DataElementId = dataElement.Id, Source = source, Code = severityAndMessage.Message, - Field = ModelKeyToField(modelKey, objectType)!, + Field = ModelKeyToField(modelKey, objectType), Severity = severityAndMessage.Severity, Description = severityAndMessage.Message } diff --git a/src/Altinn.App.Core/Helpers/ProcessHelper.cs b/src/Altinn.App.Core/Helpers/ProcessHelper.cs index c0e9ef1cd..786f07dde 100644 --- a/src/Altinn.App.Core/Helpers/ProcessHelper.cs +++ b/src/Altinn.App.Core/Helpers/ProcessHelper.cs @@ -17,6 +17,9 @@ public static class ProcessHelper /// List of possible start events /// Any error preventing the process from starting. /// The name of the start event or null if start event wasn't found. + // TODO: improve implementation of this so that we help out nullability flow analysis in the compiler + // i.e. startEventError is non-null when the function returns null + // this should probably also be internal... public static string? GetValidStartEventOrError( string? proposedStartEvent, List possibleStartEvents, diff --git a/src/Altinn.App.Core/Helpers/Serialization/ModelDeserializer.cs b/src/Altinn.App.Core/Helpers/Serialization/ModelDeserializer.cs index 9bc2867cc..981c8362b 100644 --- a/src/Altinn.App.Core/Helpers/Serialization/ModelDeserializer.cs +++ b/src/Altinn.App.Core/Helpers/Serialization/ModelDeserializer.cs @@ -70,7 +70,7 @@ public ModelDeserializer(ILogger logger, Type modelType) { using StreamReader reader = new StreamReader(stream, Encoding.UTF8); string content = await reader.ReadToEndAsync(); - return JsonSerializer.Deserialize(content, _modelType, _jsonSerializerOptions)!; + return JsonSerializer.Deserialize(content, _modelType, _jsonSerializerOptions); } catch (JsonException jsonReaderException) { diff --git a/src/Altinn.App.Core/Implementation/AppResourcesSI.cs b/src/Altinn.App.Core/Implementation/AppResourcesSI.cs index 6c0a3d6b4..b9328c297 100644 --- a/src/Altinn.App.Core/Implementation/AppResourcesSI.cs +++ b/src/Altinn.App.Core/Implementation/AppResourcesSI.cs @@ -83,12 +83,11 @@ public byte[] GetText(string org, string app, string textResource) using (FileStream fileStream = new(fullFileName, FileMode.Open, FileAccess.Read)) { - TextResource textResource = ( + TextResource textResource = await System.Text.Json.JsonSerializer.DeserializeAsync( fileStream, _jsonSerializerOptions - ) - )!; + ) ?? throw new System.Text.Json.JsonException("Failed to deserialize text resource"); textResource.Id = $"{org}-{app}-{language}"; textResource.Org = org; textResource.Language = language; @@ -208,6 +207,7 @@ public LayoutSettings GetLayoutSettings() if (File.Exists(filename)) { var filedata = File.ReadAllText(filename, Encoding.UTF8); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release LayoutSettings layoutSettings = JsonConvert.DeserializeObject(filedata)!; return layoutSettings; } @@ -243,6 +243,7 @@ public string GetLayouts() if (File.Exists(fileName)) { string fileData = File.ReadAllText(fileName, Encoding.UTF8); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release layouts.Add("FormLayout", JsonConvert.DeserializeObject(fileData)!); return JsonConvert.SerializeObject(layouts); } @@ -254,6 +255,7 @@ public string GetLayouts() { string data = File.ReadAllText(file, Encoding.UTF8); string name = file.Replace(layoutsPath, string.Empty).Replace(".json", string.Empty); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release layouts.Add(name, JsonConvert.DeserializeObject(data)!); } } @@ -313,6 +315,7 @@ public string GetLayoutsForSet(string layoutSetId) { string data = File.ReadAllText(file, Encoding.UTF8); string name = file.Replace(layoutsPath, string.Empty).Replace(".json", string.Empty); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release layouts.Add(name, JsonConvert.DeserializeObject(data)!); } } diff --git a/src/Altinn.App.Core/Implementation/PrefillSI.cs b/src/Altinn.App.Core/Implementation/PrefillSI.cs index 27269b4cd..1eeb85c5c 100644 --- a/src/Altinn.App.Core/Implementation/PrefillSI.cs +++ b/src/Altinn.App.Core/Implementation/PrefillSI.cs @@ -106,11 +106,18 @@ public async Task PrefillDataModel( if (profilePrefill != null) { - var userProfileDict = profilePrefill.ToObject>()!; + var userProfileDict = + profilePrefill.ToObject>() + ?? throw new Exception("Unexpectedly failed to convert profilePrefill JToken to dictionary"); if (userProfileDict.Count > 0) { - int userId = AuthenticationHelper.GetUserId(_httpContextAccessor.HttpContext!); + var httpContext = + _httpContextAccessor.HttpContext + ?? throw new Exception( + "Could not get HttpContext - must be in a request context to get current user" + ); + int userId = AuthenticationHelper.GetUserId(httpContext); UserProfile? userProfile = userId != 0 ? await _profileClient.GetUserProfile(userId) : null; if (userProfile != null) { @@ -135,7 +142,9 @@ public async Task PrefillDataModel( JToken? enhetsregisteret = prefillConfiguration.SelectToken(ER_KEY); if (enhetsregisteret != null) { - var enhetsregisterPrefill = enhetsregisteret.ToObject>()!; + var enhetsregisterPrefill = + enhetsregisteret.ToObject>() + ?? throw new Exception("Unexpectedly failed to convert enhetsregisteret JToken to dictionary"); if (enhetsregisterPrefill.Count > 0) { @@ -162,7 +171,9 @@ public async Task PrefillDataModel( JToken? folkeregisteret = prefillConfiguration.SelectToken(DSF_KEY); if (folkeregisteret != null) { - var folkeregisterPrefill = folkeregisteret.ToObject>()!; + var folkeregisterPrefill = + folkeregisteret.ToObject>() + ?? throw new Exception("Unexpectedly failed to convert folkeregisteret JToken to dictionary"); if (folkeregisterPrefill.Count > 0) { @@ -240,11 +251,18 @@ private void AssignValueToDataModel( if (propertyValue == null) { // the object does not exsist, create a new one with the property type - propertyValue = Activator.CreateInstance(property.PropertyType)!; + propertyValue = + Activator.CreateInstance(property.PropertyType) + ?? throw new Exception( + $"Could not create instance of type {property.PropertyType.Name} while prefilling" + ); property.SetValue(currentObject, propertyValue, null); } - // recurivly assign values + // recursively assign values + // TODO: handle Nullable (nullable value types), propertyValue may be null here + // due to Activator.CreateInstance above. Right now there is an exception + // but we could handle this better AssignValueToDataModel(keys, value, propertyValue, index + 1, continueOnError); } } diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Events/EventsClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Events/EventsClient.cs index f7054284e..56b48b576 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Events/EventsClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Events/EventsClient.cs @@ -88,7 +88,7 @@ public async Task AddEvent(string eventType, Instance instance) { Subject = $"/party/{instance.InstanceOwner.PartyId}", Type = eventType, - AlternativeSubject = alternativeSubject!, + AlternativeSubject = alternativeSubject, Time = DateTime.UtcNow, SpecVersion = "1.0", Source = new Uri($"{baseUrl}instances/{instance.Id}") diff --git a/src/Altinn.App.Core/Infrastructure/Clients/KeyVault/SecretsLocalClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/KeyVault/SecretsLocalClient.cs index d3eba2b58..9eed9b54e 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/KeyVault/SecretsLocalClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/KeyVault/SecretsLocalClient.cs @@ -34,6 +34,7 @@ public Task GetCertificateAsync(string certificateName) public Task GetKeyAsync(string keyName) { string token = GetTokenFromSecrets(keyName); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release JsonWebKey key = JsonSerializer.Deserialize(token)!; return Task.FromResult(key); } diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs index 16682e316..73821ac39 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs @@ -100,6 +100,7 @@ Type type if (response.IsSuccessStatusCode) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release dataElement = JsonConvert.DeserializeObject(instanceData)!; return dataElement; @@ -143,6 +144,7 @@ Guid dataId if (response.IsSuccessStatusCode) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release DataElement dataElement = JsonConvert.DeserializeObject(instanceData)!; return dataElement; } @@ -256,7 +258,9 @@ Guid instanceGuid if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); - dataList = JsonConvert.DeserializeObject(instanceData)!; + dataList = + JsonConvert.DeserializeObject(instanceData) + ?? throw new JsonException("Could not deserialize DataElementList"); ExtractAttachments(dataList.DataElements, attachmentList); @@ -370,6 +374,7 @@ HttpRequest request if (response.IsSuccessStatusCode) { string instancedata = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release dataElement = JsonConvert.DeserializeObject(instancedata)!; return dataElement; @@ -416,6 +421,7 @@ public async Task InsertBinaryData( if (response.IsSuccessStatusCode) { string instancedata = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release dataElement = JsonConvert.DeserializeObject(instancedata)!; return dataElement; @@ -449,6 +455,7 @@ HttpRequest request if (response.IsSuccessStatusCode) { string instancedata = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release DataElement dataElement = JsonConvert.DeserializeObject(instancedata)!; return dataElement; @@ -485,6 +492,7 @@ Stream stream if (response.IsSuccessStatusCode) { string instancedata = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release DataElement dataElement = JsonConvert.DeserializeObject(instancedata)!; return dataElement; @@ -509,6 +517,7 @@ public async Task Update(Instance instance, DataElement dataElement if (response.IsSuccessStatusCode) { + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release DataElement result = JsonConvert.DeserializeObject( await response.Content.ReadAsStringAsync() )!; @@ -535,6 +544,7 @@ public async Task LockDataElement(InstanceIdentifier instanceIdenti HttpResponseMessage response = await _client.PutAsync(token, apiUrl, content: null); if (response.IsSuccessStatusCode) { + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release DataElement result = JsonConvert.DeserializeObject( await response.Content.ReadAsStringAsync() )!; @@ -568,6 +578,7 @@ public async Task UnlockDataElement(InstanceIdentifier instanceIden HttpResponseMessage response = await _client.DeleteAsync(token, apiUrl); if (response.IsSuccessStatusCode) { + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release DataElement result = JsonConvert.DeserializeObject( await response.Content.ReadAsStringAsync() )!; diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs index b74107ce1..b6b8b6f88 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs @@ -79,6 +79,7 @@ public async Task GetInstance(string app, string org, int instanceOwne if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance instance = JsonConvert.DeserializeObject(instanceData)!; return instance; } @@ -135,9 +136,9 @@ private async Task> QueryInstances(string token, string if (response.StatusCode == HttpStatusCode.OK) { string responseString = await response.Content.ReadAsStringAsync(); - QueryResponse queryResponse = JsonConvert.DeserializeObject>( - responseString - )!; + QueryResponse queryResponse = + JsonConvert.DeserializeObject>(responseString) + ?? throw new JsonException("Could not deserialize Instance query response"); return queryResponse; } else @@ -167,6 +168,7 @@ public async Task UpdateProcess(Instance instance) if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance updatedInstance = JsonConvert.DeserializeObject(instanceData)!; return updatedInstance; } @@ -193,6 +195,7 @@ public async Task CreateInstance(string org, string app, Instance inst if (response.IsSuccessStatusCode) { + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance createdInstance = JsonConvert.DeserializeObject( await response.Content.ReadAsStringAsync() )!; @@ -221,6 +224,7 @@ public async Task AddCompleteConfirmation(int instanceOwnerPartyId, Gu if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance instance = JsonConvert.DeserializeObject(instanceData)!; _telemetry?.InstanceCompleted(instance); return instance; @@ -244,6 +248,7 @@ public async Task UpdateReadStatus(int instanceOwnerPartyId, Guid inst if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance instance = JsonConvert.DeserializeObject(instanceData)!; return instance; } @@ -275,6 +280,7 @@ public async Task UpdateSubstatus(int instanceOwnerPartyId, Guid insta if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance instance = JsonConvert.DeserializeObject(instanceData)!; return instance; } @@ -305,6 +311,7 @@ PresentationTexts presentationTexts if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance instance = JsonConvert.DeserializeObject(instanceData)!; return instance; } @@ -331,6 +338,7 @@ public async Task UpdateDataValues(int instanceOwnerPartyId, Guid inst if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance instance = JsonConvert.DeserializeObject(instanceData)!; return instance; } @@ -353,6 +361,7 @@ public async Task DeleteInstance(int instanceOwnerPartyId, Guid instan if (response.StatusCode == HttpStatusCode.OK) { string instanceData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release Instance instance = JsonConvert.DeserializeObject(instanceData)!; _telemetry?.InstanceDeleted(instance); return instance; diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceEventClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceEventClient.cs index 28e3d40c6..545bc0c26 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceEventClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceEventClient.cs @@ -89,7 +89,9 @@ string to if (response.IsSuccessStatusCode) { string eventData = await response.Content.ReadAsStringAsync(); - InstanceEventList instanceEvents = JsonConvert.DeserializeObject(eventData)!; + InstanceEventList instanceEvents = + JsonConvert.DeserializeObject(eventData) + ?? throw new JsonException("Could not deserialize InstanceEventList"); return instanceEvents.InstanceEvents; } diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Storage/ProcessClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Storage/ProcessClient.cs index ac52c4860..554209a9d 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Storage/ProcessClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Storage/ProcessClient.cs @@ -92,6 +92,7 @@ public async Task GetProcessHistory(string instanceGuid, str if (response.IsSuccessStatusCode) { string eventData = await response.Content.ReadAsStringAsync(); + // ! TODO: this null-forgiving operator should be fixed/removed for the next major release ProcessHistoryList processHistoryList = JsonConvert.DeserializeObject(eventData)!; return processHistoryList; diff --git a/src/Altinn.App.Core/Internal/AppModel/DefaultAppModel.cs b/src/Altinn.App.Core/Internal/AppModel/DefaultAppModel.cs index e2e780e49..42f867bfd 100644 --- a/src/Altinn.App.Core/Internal/AppModel/DefaultAppModel.cs +++ b/src/Altinn.App.Core/Internal/AppModel/DefaultAppModel.cs @@ -20,6 +20,7 @@ public DefaultAppModel(ILogger logger) public object Create(string classRef) { _logger.LogInformation($"CreateNewAppModel {classRef}"); + // ! TODO: Activator.CreateInstance only returns null for Nullable (nullable value types) return Activator.CreateInstance(GetModelType(classRef))!; } @@ -27,6 +28,10 @@ public object Create(string classRef) public Type GetModelType(string classRef) { _logger.LogInformation($"GetAppModelType {classRef}"); - return Assembly.GetEntryAssembly()!.GetType(classRef, true)!; + var assembly = + Assembly.GetEntryAssembly() + ?? throw new Exception("Could not get entry assembly while resolving model type"); + // ! TODO: need some way to handle this for the next major version + return assembly.GetType(classRef, true)!; } } diff --git a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs index 564c40a66..518348230 100644 --- a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs @@ -72,6 +72,7 @@ bool defaultReturn } var args = expr.Args.Select(a => EvaluateExpression(state, a, context, positionalArguments)).ToArray(); + // ! TODO: should find better ways to deal with nulls here for the next major version var ret = expr.Function switch { ExpressionFunction.dataModel => DataModel(args.First()?.ToString(), context, state), diff --git a/src/Altinn.App.Core/Internal/Language/ApplicationLanguage.cs b/src/Altinn.App.Core/Internal/Language/ApplicationLanguage.cs index 899eb8523..c2680393a 100644 --- a/src/Altinn.App.Core/Internal/Language/ApplicationLanguage.cs +++ b/src/Altinn.App.Core/Internal/Language/ApplicationLanguage.cs @@ -53,6 +53,8 @@ public ApplicationLanguage( { await using (FileStream fileStream = new(fileInfo.FullName, FileMode.Open, FileAccess.Read)) { + // ! TODO: find a better way to deal with deserialization errors here, rather than adding nulls to the list + // ! JSON deserialization returns null if the input is literally "null" var applicationLanguage = ( await JsonSerializer.DeserializeAsync( fileStream, diff --git a/src/Altinn.App.Core/Internal/Linq/Extensions.cs b/src/Altinn.App.Core/Internal/Linq/Extensions.cs new file mode 100644 index 000000000..76575e820 --- /dev/null +++ b/src/Altinn.App.Core/Internal/Linq/Extensions.cs @@ -0,0 +1,18 @@ +namespace System.Linq; + +internal static class Extensions +{ + internal static IEnumerable WhereNotNull(this IEnumerable source) + where T : class + { + ArgumentNullException.ThrowIfNull(source); + + foreach (var item in source) + { + if (item is not null) + { + yield return item; + } + } + } +} diff --git a/src/Altinn.App.Core/Internal/Patch/PatchService.cs b/src/Altinn.App.Core/Internal/Patch/PatchService.cs index cf08d0133..ec15c56dd 100644 --- a/src/Altinn.App.Core/Internal/Patch/PatchService.cs +++ b/src/Altinn.App.Core/Internal/Patch/PatchService.cs @@ -90,7 +90,7 @@ public async Task> ApplyPatch( if (!patchResult.IsSuccess) { - bool testOperationFailed = patchResult.Error!.Contains("is not equal to the indicated value."); + bool testOperationFailed = patchResult.Error.Contains("is not equal to the indicated value."); return new DataPatchError() { Title = testOperationFailed ? "Precondition in patch failed" : "Patch Operation Failed", @@ -106,7 +106,7 @@ public async Task> ApplyPatch( }; } - var result = DeserializeModel(oldModel.GetType(), patchResult.Result!); + var result = DeserializeModel(oldModel.GetType(), patchResult.Result); if (!result.Success) { return new DataPatchError() @@ -149,7 +149,7 @@ await _dataClient.UpdateData( return new DataPatchResult { NewDataModel = result.Ok, ValidationIssues = validationIssues }; } - private static ServiceResult DeserializeModel(Type type, JsonNode patchResult) + private static ServiceResult DeserializeModel(Type type, JsonNode? patchResult) { try { diff --git a/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs b/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs index a7c7a2166..9e92c8996 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Security.Claims; using Altinn.App.Core.Extensions; using Altinn.App.Core.Features; @@ -81,7 +82,7 @@ public async Task GenerateProcessStartEvents(ProcessStartRe _processReader.GetStartEventIds(), out ProcessError? startEventError ); - if (startEventError != null) + if (startEventError is not null) { return new ProcessChangeResult() { @@ -91,10 +92,16 @@ out ProcessError? startEventError }; } + // TODO: assert can be removed when we improve nullability annotation in GetValidStartEventOrError + Debug.Assert( + validStartElement is not null, + "validStartElement should always be nonnull when startEventError is null" + ); + // start process ProcessStateChange? startChange = await ProcessStart( processStartRequest.Instance, - validStartElement!, + validStartElement, processStartRequest.User ); InstanceEvent? startEvent = startChange?.Events?[0].CopyValues(); @@ -210,6 +217,7 @@ public async Task HandleEventsAndUpdateStorage( await GenerateProcessChangeEvent(InstanceEventType.process_StartEvent.ToString(), instance, now, user) ]; + // ! TODO: should probably improve nullability handling in the next major version return new ProcessStateChange { OldProcessState = null!, @@ -280,13 +288,14 @@ private async Task> MoveProcessToNext( } // ending process if next element is end event - if (_processReader.IsEndEvent(nextElement?.Id)) + var nextElementId = nextElement?.Id; + if (_processReader.IsEndEvent(nextElementId)) { using var activity = _telemetry?.StartProcessEndActivity(instance); currentState.CurrentTask = null; currentState.Ended = now; - currentState.EndEvent = nextElement!.Id; + currentState.EndEvent = nextElementId; events.Add( await GenerateProcessChangeEvent(InstanceEventType.process_EndEvent.ToString(), instance, now, user) @@ -295,14 +304,19 @@ await GenerateProcessChangeEvent(InstanceEventType.process_EndEvent.ToString(), // add submit event (to support Altinn2 SBL) events.Add(await GenerateProcessChangeEvent(InstanceEventType.Submited.ToString(), instance, now, user)); } - else if (_processReader.IsProcessTask(nextElement?.Id)) + else if (_processReader.IsProcessTask(nextElementId)) { + if (nextElement is null) + { + throw new Exception("Next process element was unexpectedly null"); + } + var task = nextElement as ProcessTask; currentState.CurrentTask = new ProcessElementInfo { Flow = currentState.CurrentTask?.Flow + 1, - ElementId = nextElement!.Id, - Name = nextElement!.Name, + ElementId = nextElementId, + Name = nextElement.Name, Started = now, AltinnTaskType = task?.ExtensionElements?.TaskExtension?.TaskType, FlowType = action is "reject" diff --git a/src/Altinn.App.Core/Internal/Process/ProcessEventHandlingDelegator.cs b/src/Altinn.App.Core/Internal/Process/ProcessEventHandlingDelegator.cs index 497041a0b..ac860b447 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessEventHandlingDelegator.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessEventHandlingDelegator.cs @@ -72,6 +72,7 @@ public async Task HandleEvents( case InstanceEventType.process_StartEvent: break; case InstanceEventType.process_StartTask: + // ! TODO: figure out why taskId can be null here await _startTaskEventHandler.Execute( GetProcessTaskInstance(altinnTaskType), taskId!, @@ -80,6 +81,7 @@ await _startTaskEventHandler.Execute( ); break; case InstanceEventType.process_EndTask: + // ! TODO: figure out why taskId can be null here await _endTaskEventHandler.Execute( GetProcessTaskInstance(altinnTaskType), taskId!, @@ -88,6 +90,7 @@ await _endTaskEventHandler.Execute( break; case InstanceEventType.process_AbandonTask: // InstanceEventType is set to Abandon when action performed is `Reject`. This is to keep backwards compatability with existing code that only should be run when a task is abandoned/rejected. + // ! TODO: figure out why taskId can be null here await _abandonTaskEventHandler.Execute( GetProcessTaskInstance(altinnTaskType), taskId!, diff --git a/src/Altinn.App.Core/Internal/Process/ProcessReader.cs b/src/Altinn.App.Core/Internal/Process/ProcessReader.cs index ef68b625d..f5f2aba41 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessReader.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessReader.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using System.Xml.Serialization; using Altinn.App.Core.Features; using Altinn.App.Core.Internal.Process.Elements; @@ -46,7 +47,7 @@ public List GetStartEventIds() } /// - public bool IsStartEvent(string? elementId) + public bool IsStartEvent([NotNullWhen(true)] string? elementId) { using var activity = _telemetry?.StartIsStartEventActivity(); return elementId != null && GetStartEventIds().Contains(elementId); @@ -67,7 +68,7 @@ public List GetProcessTaskIds() } /// - public bool IsProcessTask(string? elementId) + public bool IsProcessTask([NotNullWhen(true)] string? elementId) { using var activity = _telemetry?.StartIsProcessTaskActivity(); return elementId != null && GetProcessTaskIds().Contains(elementId); @@ -102,7 +103,7 @@ public List GetEndEventIds() } /// - public bool IsEndEvent(string? elementId) + public bool IsEndEvent([NotNullWhen(true)] string? elementId) { using var activity = _telemetry?.StartIsEndEventActivity(); return elementId != null && GetEndEventIds().Contains(elementId); diff --git a/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs b/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs index 7d1eb7d28..996baeba0 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs @@ -139,7 +139,11 @@ await _dataClient.InsertFormData( else { // Remove the shadow fields from the data - data = JsonSerializer.Deserialize(serializedData, modelType)!; + data = + JsonSerializer.Deserialize(serializedData, modelType) + ?? throw new JsonException( + "Could not deserialize back datamodel after removing shadow fields. Data was \"null\"" + ); } } // remove AltinnRowIds diff --git a/src/Altinn.App.Core/Internal/Process/ProcessTasks/PaymentProcessTask.cs b/src/Altinn.App.Core/Internal/Process/ProcessTasks/PaymentProcessTask.cs index d2330f85c..56b9be233 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessTasks/PaymentProcessTask.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessTasks/PaymentProcessTask.cs @@ -57,9 +57,12 @@ public async Task End(string taskId, Instance instance) 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!; + await _dataClient.InsertBinaryData( instance.Id, - paymentConfiguration.PaymentDataType!, + paymentDataType, PdfContentType, ReceiptFileName, pdfStream, diff --git a/src/Altinn.App.Core/Models/ApplicationMetadata.cs b/src/Altinn.App.Core/Models/ApplicationMetadata.cs index 0341f9cb3..2dae6f346 100644 --- a/src/Altinn.App.Core/Models/ApplicationMetadata.cs +++ b/src/Altinn.App.Core/Models/ApplicationMetadata.cs @@ -63,7 +63,8 @@ public ApplicationMetadata(string id) /// [JsonProperty(PropertyName = "altinnNugetVersion")] public string AltinnNugetVersion { get; set; } = - typeof(ApplicationMetadata).Assembly!.GetName().Version!.ToString(); + typeof(ApplicationMetadata).Assembly.GetName().Version?.ToString() + ?? throw new Exception("Assembly version is null"); /// /// Holds properties that are not mapped to other properties diff --git a/src/Altinn.App.Core/Models/Layout/Components/GridComponent.cs b/src/Altinn.App.Core/Models/Layout/Components/GridComponent.cs index d71f70544..38c921ca0 100644 --- a/src/Altinn.App.Core/Models/Layout/Components/GridComponent.cs +++ b/src/Altinn.App.Core/Models/Layout/Components/GridComponent.cs @@ -78,12 +78,12 @@ public class GridCell /// Returns the child component IDs /// /// - public List Children() + public List Children() { return this.Rows?.Where(r => r.Cells is not null) - .SelectMany(r => r.Cells!) - .Where(c => c.ComponentId is not null) - .Select(c => c.ComponentId!) - .ToList() ?? new List(); + .SelectMany(r => r.Cells ?? []) + .Select(c => c.ComponentId) + .WhereNotNull() + .ToList() ?? []; } } diff --git a/src/Altinn.App.Core/Models/Layout/PageComponentConverter.cs b/src/Altinn.App.Core/Models/Layout/PageComponentConverter.cs index c47aeff17..26b790377 100644 --- a/src/Altinn.App.Core/Models/Layout/PageComponentConverter.cs +++ b/src/Altinn.App.Core/Models/Layout/PageComponentConverter.cs @@ -52,7 +52,9 @@ public PageComponent ReadNotNull(ref Utf8JsonReader reader, string pageName, Jso { if (reader.TokenType != JsonTokenType.StartObject) { - throw new JsonException(); + throw new JsonException( + $"Unexpected JSON token type '{reader.TokenType}', expected '{nameof(JsonTokenType.StartObject)}'" + ); } PageComponent? page = null; @@ -60,10 +62,13 @@ public PageComponent ReadNotNull(ref Utf8JsonReader reader, string pageName, Jso { if (reader.TokenType != JsonTokenType.PropertyName) { - throw new JsonException(); //Think this is impossible. After a JsonTokenType.StartObject, everything should be JsonTokenType.PropertyName + // Think this is impossible. After a JsonTokenType.StartObject, everything should be JsonTokenType.PropertyName + throw new JsonException( + $"Unexpected JSON token type after StartObject: '{reader.TokenType}', expected '{nameof(JsonTokenType.PropertyName)}'" + ); } - var propertyName = reader.GetString()!; + var propertyName = reader.GetString(); reader.Read(); if (propertyName == "data") { @@ -86,7 +91,9 @@ private PageComponent ReadData(ref Utf8JsonReader reader, string pageName, JsonS { if (reader.TokenType != JsonTokenType.StartObject) { - throw new JsonException(); + throw new JsonException( + $"Unexpected JSON token type '{reader.TokenType}', expected '{nameof(JsonTokenType.StartObject)}'" + ); } List? componentListFlat = null; @@ -105,10 +112,18 @@ private PageComponent ReadData(ref Utf8JsonReader reader, string pageName, JsonS { if (reader.TokenType != JsonTokenType.PropertyName) { - throw new JsonException(); //Think this is impossible. After a JsonTokenType.StartObject, everything should be JsonTokenType.PropertyName + // Think this is impossible. After a JsonTokenType.StartObject, everything should be JsonTokenType.PropertyName + throw new JsonException( + $"Unexpected JSON token type after StartObject: '{reader.TokenType}', expected '{nameof(JsonTokenType.PropertyName)}'" + ); } - var propertyName = reader.GetString()!; + var propertyName = + reader.GetString() + ?? throw new JsonException( + $"Could not read property name from JSON token with type '{nameof(JsonTokenType.PropertyName)}'" + ); + reader.Read(); switch (propertyName.ToLowerInvariant()) { @@ -157,7 +172,7 @@ JsonSerializerOptions options while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) { - var component = ReadComponent(ref reader, options)!; + var component = ReadComponent(ref reader, options); // Add component to the collections componentListFlat.Add(component); @@ -237,7 +252,9 @@ private BaseComponent ReadComponent(ref Utf8JsonReader reader, JsonSerializerOpt { if (reader.TokenType != JsonTokenType.StartObject) { - throw new JsonException(); + throw new JsonException( + $"Unexpected JSON token type '{reader.TokenType}', expected '{nameof(JsonTokenType.StartObject)}'" + ); } string? id = null; string? type = null; @@ -264,10 +281,18 @@ private BaseComponent ReadComponent(ref Utf8JsonReader reader, JsonSerializerOpt { if (reader.TokenType != JsonTokenType.PropertyName) { - throw new JsonException(); // Not possiblie? + // Think this is impossible. After a JsonTokenType.StartObject, everything should be JsonTokenType.PropertyName + throw new JsonException( + $"Unexpected JSON token type after StartObject: '{reader.TokenType}', expected '{nameof(JsonTokenType.PropertyName)}'" + ); } - var propertyName = reader.GetString()!; + var propertyName = + reader.GetString() + ?? throw new JsonException( + $"Could not read property name from JSON token with type '{nameof(JsonTokenType.PropertyName)}'" + ); + reader.Read(); switch (propertyName.ToLowerInvariant()) { diff --git a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs index ab2af0f81..b94a6a941 100644 --- a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs +++ b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs @@ -71,7 +71,10 @@ public class ValidationIssue /// [JsonProperty(PropertyName = "source")] [JsonPropertyName("source")] - public string Source { get; set; } = default!; +#nullable disable + public string Source { get; set; } + +#nullable restore /// /// The custom text key to use for the localized text in the frontend. diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 4140f5be8..d827d60b7 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -4,6 +4,10 @@ + + all + runtime; build; native; contentfiles; analyzers + $([System.IO.Directory]::GetParent($(MSBuildThisFileDirectory)).Parent.FullName) diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs index 37da9418e..8d9899e4d 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -201,6 +201,23 @@ private void SetupFormDataValidatorReturn( .Verifiable(hasRelevantChanges is null ? Times.Never : Times.AtLeastOnce); } + private void SourcePropertyIsSet(Dictionary> result) + { + Assert.All(result.Values, SourcePropertyIsSet); + } + + private void SourcePropertyIsSet(List result) + { + Assert.All( + result, + issue => + { + Assert.NotNull(issue.Source); + Assert.NotEqual("[]", issue.Source); + } + ); + } + private void SetupDataClient(MyModel data) { _dataClientMock @@ -299,6 +316,7 @@ public async Task ValidateFormData_WithSpecificValidator() result.Should().ContainKey("specificValidator").WhoseValue.Should().HaveCount(0); result.Should().HaveCount(1); + SourcePropertyIsSet(result); } [Fact] @@ -350,6 +368,7 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsErrorsWhenNameIsKa .Which.CustomTextKey.Should() .Be("AlwaysNameNotOla"); resultData.Should().HaveCount(2); + SourcePropertyIsSet(resultData); } [Fact] @@ -435,6 +454,7 @@ List CreateIssues(string code) .Which.Severity.Should() .Be(ValidationIssueSeverity.Error); taskResult.Should().HaveCount(6); + SourcePropertyIsSet(taskResult); var elementResult = await validationService.ValidateDataElement( DefaultInstance, @@ -463,6 +483,7 @@ List CreateIssues(string code) .Which.Severity.Should() .Be(ValidationIssueSeverity.Error); elementResult.Should().HaveCount(4); + SourcePropertyIsSet(elementResult); var dataResult = await validationService.ValidateFormData( DefaultInstance, @@ -488,6 +509,7 @@ List CreateIssues(string code) .Which.Severity.Should() .Be(ValidationIssueSeverity.Error); dataResult.Should().HaveCount(2); + SourcePropertyIsSet(dataResult); } [Fact] diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/LayoutModelConverterFromObject.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/LayoutModelConverterFromObject.cs index 706802047..9ec0925e8 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/LayoutModelConverterFromObject.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/LayoutModelConverterFromObject.cs @@ -19,7 +19,9 @@ public class LayoutModelConverterFromObject : JsonConverter { if (reader.TokenType != JsonTokenType.StartObject) { - throw new JsonException(); + throw new JsonException( + $"Unexpected JSON token type '{reader.TokenType}', expected '{nameof(JsonTokenType.StartObject)}'" + ); } var componentModel = new LayoutModel(); @@ -29,10 +31,17 @@ public class LayoutModelConverterFromObject : JsonConverter { if (reader.TokenType != JsonTokenType.PropertyName) { - throw new JsonException(); // Think this is impossible. After a JsonTokenType.StartObject, everything should be JsonTokenType.PropertyName + // Think this is impossible. After a JsonTokenType.StartObject, everything should be JsonTokenType.PropertyName + throw new JsonException( + $"Unexpected JSON token type after StartObject: '{reader.TokenType}', expected '{nameof(JsonTokenType.PropertyName)}'" + ); } - var pageName = reader.GetString()!; + var pageName = + reader.GetString() + ?? throw new JsonException( + $"Could not read property name from JSON token with type '{nameof(JsonTokenType.PropertyName)}'" + ); reader.Read(); PageComponentConverter.SetAsyncLocalPageName(pageName);