Skip to content

Commit

Permalink
Remove/fix or suppress null-forgiving operator usage in src/, enabl…
Browse files Browse the repository at this point in the history
…e warning/build errors (#636)
  • Loading branch information
martinothamar authored May 25, 2024
1 parent 71c56ca commit d70d21a
Show file tree
Hide file tree
Showing 49 changed files with 333 additions and 95 deletions.
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 0 additions & 5 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,5 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
<!-- TODO: enable this at some point, when we've got a handle on NRTs -->
<!-- <PackageReference Include="Nullable.Extended.Analyzer" Version="1.15.6169">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference> -->
</ItemGroup>
</Project>
22 changes: 18 additions & 4 deletions src/Altinn.App.Api/Controllers/InstancesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
};
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/Altinn.App.Api/Controllers/PdfController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ [FromRoute] Guid dataGuid
LayoutSet? layoutSet = null;
if (!string.IsNullOrEmpty(layoutSetsString))
{
layoutSets = JsonSerializer.Deserialize<LayoutSets>(layoutSetsString, _jsonSerializerOptions)!;
layoutSets =
JsonSerializer.Deserialize<LayoutSets>(layoutSetsString, _jsonSerializerOptions)
?? throw new JsonException("Could not deserialize LayoutSets");
layoutSet = layoutSets.Sets?.FirstOrDefault(t =>
t.DataType.Equals(dataElement.DataType) && t.Tasks.Contains(taskId)
);
Expand All @@ -145,7 +147,7 @@ [FromRoute] Guid dataGuid
layoutSettings = JsonSerializer.Deserialize<LayoutSettings>(
layoutSettingsFileContent,
_jsonSerializerOptions
)!;
);
}

// Ensure layoutsettings are initialized in FormatPdf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
}
10 changes: 8 additions & 2 deletions src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ public class RequestPart
/// <summary>
/// The stream to access this part.
/// </summary>
public Stream Stream { get; set; } = default!;
#nullable disable
public Stream Stream { get; set; }

#nullable restore

/// <summary>
/// The file name as given in content description.
Expand All @@ -23,7 +26,10 @@ public class RequestPart
/// <summary>
/// The content type of the part.
/// </summary>
public string ContentType { get; set; } = default!;
#nullable disable
public string ContentType { get; set; }

#nullable restore

/// <summary>
/// The file size of the part, 0 if not given.
Expand Down
9 changes: 7 additions & 2 deletions src/Altinn.App.Api/Models/SimpleInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ public class SimpleInstance
/// <summary>
/// The instance identifier formated as {instanceOwner.partyId}/{instanceGuid}.
/// </summary>
public string Id { get; set; } = default!;
#nullable disable
public string Id { get; set; }

#nullable restore

/// <summary>
/// Presentation texts from the instance
Expand All @@ -28,5 +31,7 @@ public class SimpleInstance
/// <summary>
/// Full name of user to last change the instance.
/// </summary>
public string LastChangedBy { get; set; } = default!;
#nullable disable
public string LastChangedBy { get; set; }
#nullable restore
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics;
using Altinn.App.Core.Configuration;
using Altinn.App.Core.Constants;
using Altinn.App.Core.EFormidling.Interface;
Expand Down Expand Up @@ -216,7 +217,8 @@ private async Task SendInstanceData(Instance instance, Dictionary<string, string
new Guid(dataElement.Id)
);

bool successful = await _eFormidlingClient!.UploadAttachment(
Debug.Assert(_eFormidlingClient is not null, "This is validated before use");
bool successful = await _eFormidlingClient.UploadAttachment(
stream,
instanceGuid.ToString(),
fileName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,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<Instance> AddCompleteConfirmation(InstanceIdentifier instanceIdentifier)
private async Task<Instance?> AddCompleteConfirmation(InstanceIdentifier instanceIdentifier)
{
string url =
$"instances/{instanceIdentifier.InstanceOwnerPartyId}/{instanceIdentifier.InstanceGuid}/complete";
Expand All @@ -169,7 +169,7 @@ private async Task<Instance> AddCompleteConfirmation(InstanceIdentifier instance
if (response.StatusCode == HttpStatusCode.OK)
{
string instanceData = await response.Content.ReadAsStringAsync();
Instance instance = JsonConvert.DeserializeObject<Instance>(instanceData)!;
Instance? instance = JsonConvert.DeserializeObject<Instance>(instanceData);
return instance;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Instance> AddCompleteConfirmation(InstanceIdentifier instanceIdentifier)
private async Task<Instance?> AddCompleteConfirmation(InstanceIdentifier instanceIdentifier)
{
string url =
$"instances/{instanceIdentifier.InstanceOwnerPartyId}/{instanceIdentifier.InstanceGuid}/complete";
Expand All @@ -154,7 +154,7 @@ private async Task<Instance> AddCompleteConfirmation(InstanceIdentifier instance
if (response.StatusCode == HttpStatusCode.OK)
{
string instanceData = await response.Content.ReadAsStringAsync();
Instance instance = JsonConvert.DeserializeObject<Instance>(instanceData)!;
Instance? instance = JsonConvert.DeserializeObject<Instance>(instanceData);
return instance;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Altinn.App.Core/Extensions/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
5 changes: 4 additions & 1 deletion src/Altinn.App.Core/Extensions/XmlToLinqExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ public static class XmlToLinqExtensions
/// </returns>
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;
}

Expand Down
18 changes: 10 additions & 8 deletions src/Altinn.App.Core/Features/Action/SigningUserAction.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -75,14 +76,16 @@ public async Task<UserActionResult> 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
);
Expand All @@ -100,24 +103,23 @@ await GetSignee(context.UserId.Value),
);
}

private static bool ShouldSign(
private static string? GetDataTypeForSignature(
ProcessTask currentTask,
List<DataElement> dataElements,
List<DataType>? 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<DataElementSignature> GetDataElementSignatures(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ public async Task<bool> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public async Task<MetadataCodelistResponse> GetRawAltinn2CodelistAsync(string? l
_ => "1044", // default to norwegian bokmål
};

// ! TODO: address this is next major release, should never return null

Check warning on line 79 in src/Altinn.App.Core/Features/Options/Altinn2Provider/Altinn2CodeListProvider.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
return (
await _cache.GetOrCreateAsync(
$"{_metadataApiId}{langCode}{_codeListVersion}",
Expand Down
4 changes: 2 additions & 2 deletions src/Altinn.App.Core/Features/Options/AppOptionsFileHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ public AppOptionsFileHandler(IOptions<AppSettings> settings)
if (File.Exists(filename))
{
string fileData = await File.ReadAllTextAsync(filename, Encoding.UTF8);
List<AppOption> options = JsonSerializer.Deserialize<List<AppOption>>(
List<AppOption>? options = JsonSerializer.Deserialize<List<AppOption>>(
fileData,
_jsonSerializerOptions
)!;
);
return options;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,24 @@ public async Task<bool> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public PaymentService(
}

ValidateConfig(paymentConfiguration);
// ! TODO: restructure code to avoid assertion (it is validated above)

Check warning on line 56 in src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
string dataTypeId = paymentConfiguration.PaymentDataType!;

(Guid dataElementId, PaymentInformation? existingPaymentInformation) =
Expand Down Expand Up @@ -132,6 +133,7 @@ public async Task<PaymentInformation> CheckAndStorePaymentStatus(

ValidateConfig(paymentConfiguration);

// ! TODO: restructure code to avoid assertion (it is validated above)

Check warning on line 136 in src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
string dataTypeId = paymentConfiguration.PaymentDataType!;
(Guid dataElementId, PaymentInformation? paymentInformation) = await _dataService.GetByType<PaymentInformation>(
instance,
Expand All @@ -153,7 +155,6 @@ public async Task<PaymentInformation> CheckAndStorePaymentStatus(
};
}

PaymentDetails paymentDetails = paymentInformation.PaymentDetails!;
decimal totalPriceIncVat = paymentInformation.OrderDetails.TotalPriceIncVat;
string paymentProcessorId = paymentInformation.OrderDetails.PaymentProcessorId;

Expand All @@ -172,6 +173,10 @@ public async Task<PaymentInformation> 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,
Expand Down Expand Up @@ -203,6 +208,7 @@ public async Task<bool> IsPaymentCompleted(Instance instance, AltinnPaymentConfi
{
ValidateConfig(paymentConfiguration);

// ! TODO: restructure code to avoid assertion (it is validated above)

Check warning on line 211 in src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
string dataTypeId = paymentConfiguration.PaymentDataType!;
(Guid _, PaymentInformation? paymentInformation) = await _dataService.GetByType<PaymentInformation>(
instance,
Expand All @@ -225,6 +231,7 @@ AltinnPaymentConfiguration paymentConfiguration
{
ValidateConfig(paymentConfiguration);

// ! TODO: restructure code to avoid assertion (it is validated above)

Check warning on line 234 in src/Altinn.App.Core/Features/Payment/Services/PaymentService.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
string dataTypeId = paymentConfiguration.PaymentDataType!;
(Guid dataElementId, PaymentInformation? paymentInformation) = await _dataService.GetByType<PaymentInformation>(
instance,
Expand Down
Loading

0 comments on commit d70d21a

Please sign in to comment.