Skip to content

Commit

Permalink
Fixed C# 'required' modifier in request DTO's throwing exceptions bef…
Browse files Browse the repository at this point in the history
…ore validation step. Closes #34
  • Loading branch information
jezzsantos committed May 12, 2024
1 parent 3581cd0 commit 46719a1
Show file tree
Hide file tree
Showing 134 changed files with 1,147 additions and 351 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[![Under Construction](https://img.shields.io/badge/Status-UnderConstruction-yellow.svg)](README.md)
[![Backend API Ready](https://img.shields.io/badge/Status-BackendAPI_Ready-green.svg)](README.md)
[![Frontend WebApp Under Construction](https://img.shields.io/badge/Status-FrontendUI_UnderConstruction-yellow.svg)](README.md)
[![Build and Test](https://github.com/jezzsantos/saastack/actions/workflows/build.yml/badge.svg)](https://github.com/jezzsantos/saastack/actions/workflows/build.yml)
[![](https://img.shields.io/static/v1?label=Sponsor&message=%E2%9D%A4&logo=GitHub&color=%23fe8e86)](https://github.com/sponsors/jezzsantos)

Expand Down
45 changes: 45 additions & 0 deletions docs/decisions/0150-nullability.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Nullability

* status: accepted
* date: 2023-09-12
* deciders: jezzsantos

# Context and Problem Statement

Nullability refers to the issue of managing nulls in code.
Managing null in C# has been a long-standing issue, and the language has made several advances to reduce the amount of handling required.
However, the issue still remains, and the language has not yet provided a definitive solution. Thus many developers are still guarding against nulls and handling them as results.

Nullable value types were first introduced in C# 2.0 (circa, Nov 2005), which allowed "value types" (like `int?`, `DateTime?`, and `MyCustomEnum?` etc) to be explicitly assigned a null value.
Since C# 8 (circa, Sept 2019), we've had the ability to define nullable "reference types" (like `string?` or `MyCustomObject?`), which allow developers to annotate their code to indicate where nulls are allowed. We also had the ability to mark up code, and projects with nullable annotations to help the compiler identify potential null reference exceptions. (like `#nullable enable` or `#nullable disable`) and make whole projects nullable or not using `<Nullable>enable</Nullable>`. Which also added tooling like Roslyn rules to help identify potential null reference exceptions.

Outside of official dotnet releases, we've seen the introduction of more functional programming concepts like `Option<T>`, `OneOf<T>` and `Result<T>` types, which are used to wrap values that may or may not be null, and provide a more functional way of handling nulls.

These help the programmer manage return types much easier (since they are often implemented as `struct`) and that can lead to the eradication of `null` as a value anywhere in the code (except dealing with 3rd party library code).

## Considered Options

The options are:

1. Nullable Context - having the compiler and IDE help identify potential null reference exceptions.
2. Optional<T> - representing a value that may or may not be present.
3. Nulls - same old, same old.

## Decision Outcome

`Nullable Context + Optional<T>`

- Neither just Nullable Context nor Optional<T> alone is sufficient to completely eradicate nulls form the codebase. Both are useful in certain contexts. It largely depends on what is being expressed at the time.
- Nullable Context (in an IDE) is very useful to help the programmer recognize where nulls are allowed, and where they are not. It also helps the compiler to identify potential null reference exceptions.
- Optional<T> is very useful when dealing with return types, and when you want to express that a value may or may not be present. It is also useful when you want to express that a value may or may not be present, and you want to provide a default value if it is not present.
- {justification3}

## More Information

The dotnet team has attempted to introduce a set of functional types including `Optional<T>` in the past in the [dotNext](https://github.com/dotnet/dotNext) which many products/teams have adopted. However, there have been strong statements from the dotnet team (can't find it online) that they will NOT be adopting these types into the dotnet runtime (some statement like: "we don't want another `null`!").

However, in the interim, we have moved forward with our own slim version of `Optional<T>` and `Result<T>`types, which are based on the [dotNext](https://github.com/dotnet/dotNext) project:

- We decided to reduce the number of dependencies in this codebase
- We decided to learn more about these types, and how they could be used in our codebase.
- We decided that one day they can be (relatively easily) ripped and replaced for another implementation (at some cost), either when the language official supports them, or when we want to commit to another library.
4 changes: 2 additions & 2 deletions docs/design-principles/0020-api-framework.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ where each service operation (method above) would have a unique request DTO that
[Route("/cars/{Id}", OperationMethod.Get)]
public class GetCarRequest : IWebRequest<GetCarResponse>
{
public required string Id { get; set; }
public string? Id { get; set; }
}
```

Expand Down Expand Up @@ -267,7 +267,7 @@ Then we use Roslyn analyzers (and other tooling) to guide the author in creating
[Route("/cars/{Id}", OperationMethod.Get)]
public class GetCarRequest : IWebRequest<GetCarResponse>
{
public required string Id { get; set; }
public string? Id { get; set; }
}
```
and, in `Infrastructure.Web.Api.Interfaces/Operations/Cars/GetCarResponse.cs`:
Expand Down
2 changes: 1 addition & 1 deletion docs/design-principles/0025-modularity.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ For example,
[Authorize(Roles.Platform_Operations)]
public class GetOrganizationSettingsRequest : TenantedRequest<GetOrganizationSettingsResponse>
{
public required string Id { get; set; }
public string? Id { get; set; }
}
```

Expand Down
4 changes: 2 additions & 2 deletions docs/design-principles/0090-authentication-authorization.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ For example,
[Authorize(Roles.Tenant_Member, Features.Tenant_Basic)]
public class GetCarRequest : TenantedRequest<GetCarResponse>
{
public required string Id { get; set; }
public string? Id { get; set; }
}
```

Expand Down Expand Up @@ -198,7 +198,7 @@ For example,
[Authorize(Roles.Tenant_Member, Features.Tenant_Basic)]
public class GetCarRequest : TenantedRequest<GetCarResponse>
{
public required string Id { get; set; }
public string? Id { get; set; }
}
```

Expand Down
2 changes: 1 addition & 1 deletion src/AncillaryInfrastructure/Api/Audits/AuditsApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public async Task<ApiPostResult<bool, DeliverMessageResponse>> Deliver(DeliverAu
CancellationToken cancellationToken)
{
var delivered =
await _ancillaryApplication.DeliverAuditAsync(_callerFactory.Create(), request.Message, cancellationToken);
await _ancillaryApplication.DeliverAuditAsync(_callerFactory.Create(), request.Message!, cancellationToken);

return () => delivered.HandleApplicationResult<bool, DeliverMessageResponse>(_ =>
new PostResult<DeliverMessageResponse>(new DeliverMessageResponse { IsDelivered = true }));
Expand Down
2 changes: 1 addition & 1 deletion src/AncillaryInfrastructure/Api/Emails/EmailsApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public async Task<ApiPostResult<bool, DeliverMessageResponse>> Deliver(DeliverEm
CancellationToken cancellationToken)
{
var delivered =
await _ancillaryApplication.DeliverEmailAsync(_callerFactory.Create(), request.Message, cancellationToken);
await _ancillaryApplication.DeliverEmailAsync(_callerFactory.Create(), request.Message!, cancellationToken);

return () => delivered.HandleApplicationResult<bool, DeliverMessageResponse>(_ =>
new PostResult<DeliverMessageResponse>(new DeliverMessageResponse { IsDelivered = true }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public async Task<ApiGetResult<FeatureFlag, GetFeatureFlagResponse>> Get(GetFeat
CancellationToken cancellationToken)
{
var flag = await _featureFlagsApplication.GetFeatureFlagAsync(_callerFactory.Create(),
request.Name, request.TenantId, request.UserId, cancellationToken);
request.Name!, request.TenantId, request.UserId!, cancellationToken);

return () => flag.HandleApplicationResult(f => new GetFeatureFlagResponse { Flag = f });
}
Expand All @@ -41,7 +41,7 @@ public async Task<ApiGetResult<FeatureFlag, GetFeatureFlagResponse>> GetForCalle
CancellationToken cancellationToken)
{
var flag = await _featureFlagsApplication.GetFeatureFlagForCallerAsync(_callerFactory.Create(),
request.Name, cancellationToken);
request.Name!, cancellationToken);

return () => flag.HandleApplicationResult(f => new GetFeatureFlagResponse { Flag = f });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public async Task<ApiPostResult<bool, DeliverMessageResponse>> Notify(NotifyProv
CancellationToken cancellationToken)
{
var delivered =
await _ancillaryApplication.NotifyProvisioningAsync(_callerFactory.Create(), request.Message,
await _ancillaryApplication.NotifyProvisioningAsync(_callerFactory.Create(), request.Message!,
cancellationToken);

return () => delivered.HandleApplicationResult<bool, DeliverMessageResponse>(_ =>
Expand Down
4 changes: 2 additions & 2 deletions src/AncillaryInfrastructure/Api/Recording/RecordingApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public RecordingApi(ICallerContextFactory callerFactory, IRecordingApplication r
public async Task<ApiEmptyResult> RecordMeasurement(RecordMeasureRequest request,
CancellationToken cancellationToken)
{
var result = await _recordingApplication.RecordMeasurementAsync(_callerFactory.Create(), request.EventName,
var result = await _recordingApplication.RecordMeasurementAsync(_callerFactory.Create(), request.EventName!,
request.Additional,
cancellationToken);

Expand All @@ -31,7 +31,7 @@ public async Task<ApiEmptyResult> RecordMeasurement(RecordMeasureRequest request
public async Task<ApiEmptyResult> RecordUsage(RecordUseRequest request,
CancellationToken cancellationToken)
{
var result = await _recordingApplication.RecordUsageAsync(_callerFactory.Create(), request.EventName,
var result = await _recordingApplication.RecordUsageAsync(_callerFactory.Create(), request.EventName!,
request.Additional,
cancellationToken);

Expand Down
2 changes: 1 addition & 1 deletion src/AncillaryInfrastructure/Api/Usages/UsagesApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public async Task<ApiPostResult<bool, DeliverMessageResponse>> Deliver(DeliverUs
CancellationToken cancellationToken)
{
var delivered =
await _ancillaryApplication.DeliverUsageAsync(_callerFactory.Create(), request.Message, cancellationToken);
await _ancillaryApplication.DeliverUsageAsync(_callerFactory.Create(), request.Message!, cancellationToken);

return () => delivered.HandleApplicationResult<bool, DeliverMessageResponse>(_ =>
new PostResult<DeliverMessageResponse>(new DeliverMessageResponse { IsDelivered = true }));
Expand Down
16 changes: 13 additions & 3 deletions src/ApiHost1/Api/TestingOnly/TestingWebApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,22 @@ public async Task<ApiResult<string, StringMessageTestingOnlyResponse>> Validatio
{ Message = $"amessage{request.Id}" });
}

public async Task<ApiResult<string, StringMessageTestingOnlyResponse>> ValidationsValidated(
ValidationsValidatedTestingOnlyRequest request, CancellationToken cancellationToken)
public async Task<ApiResult<string, StringMessageTestingOnlyResponse>> ValidationsValidatedGet(
ValidationsValidatedGetTestingOnlyRequest request, CancellationToken cancellationToken)
{
await Task.CompletedTask;
return () => new Result<StringMessageTestingOnlyResponse, Error>(new StringMessageTestingOnlyResponse
{ Message = $"amessage{request.Field1}" });
{ Message = $"amessage{request.RequiredField}" });
}

public async Task<ApiPostResult<string, StringMessageTestingOnlyResponse>> ValidationsValidatedPost(
ValidationsValidatedPostTestingOnlyRequest request, CancellationToken cancellationToken)
{
await Task.CompletedTask;
return () =>
new PostResult<StringMessageTestingOnlyResponse>(
new StringMessageTestingOnlyResponse { Message = $"amessage{request.RequiredField}" },
"alocation");
}

private static IReadOnlyList<IApplicationRepository> GetRepositories(IServiceProvider services,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
#if TESTINGONLY
using Common.Extensions;
using FluentValidation;
using Infrastructure.Web.Api.Operations.Shared.TestingOnly;
using JetBrains.Annotations;

namespace ApiHost1.Api.TestingOnly;

[UsedImplicitly]
public class ValidationsValidatedTestingOnlyRequestValidator : AbstractValidator<ValidationsValidatedTestingOnlyRequest>
public class
ValidationsValidatedGetTestingOnlyRequestValidator : AbstractValidator<ValidationsValidatedGetTestingOnlyRequest>
{
public ValidationsValidatedTestingOnlyRequestValidator()
public ValidationsValidatedGetTestingOnlyRequestValidator()
{
RuleFor(req => req.Id)
.NotEmpty()
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidId);
RuleFor(req => req.Field1)
RuleFor(req => req.RequiredField)
.NotEmpty()
.Matches(@"[\d]{1,3}")
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidField1);
RuleFor(req => req.Field2)
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidRequiredField);
RuleFor(req => req.OptionalField)
.NotEmpty()
.Matches(@"[\d]{1,3}")
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidField2);
.When(req => req.OptionalField.HasValue())
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidOptionalField);
}
}
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#if TESTINGONLY
using Common.Extensions;
using FluentValidation;
using Infrastructure.Web.Api.Operations.Shared.TestingOnly;
using JetBrains.Annotations;

namespace ApiHost1.Api.TestingOnly;

[UsedImplicitly]
public class
ValidationsValidatedPostTestingOnlyRequestValidator : AbstractValidator<ValidationsValidatedPostTestingOnlyRequest>
{
public ValidationsValidatedPostTestingOnlyRequestValidator()
{
RuleFor(req => req.Id)
.NotEmpty()
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidId);
RuleFor(req => req.RequiredField)
.NotEmpty()
.Matches(@"[\d]{1,3}")
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidRequiredField);
RuleFor(req => req.OptionalField)
.NotEmpty()
.Matches(@"[\d]{1,3}")
.When(req => req.OptionalField.HasValue())
.WithMessage(Resources.GetTestingOnlyValidatedRequestValidator_InvalidOptionalField);
}
}
#endif
6 changes: 6 additions & 0 deletions src/ApiHost1/ApiHost1.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
<ProjectReference Include="..\UserProfilesInfrastructure\UserProfilesInfrastructure.csproj" />
</ItemGroup>

<ItemGroup>
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">
<_Parameter1>Infrastructure.Web.Api.IntegrationTests</_Parameter1>
</AssemblyAttribute>
</ItemGroup>

<!-- Runs the source generator (in memory) on build -->
<ItemGroup>
<ProjectReference Include="..\Tools.Generators.Web.Api\Tools.Generators.Web.Api.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
Expand Down
18 changes: 9 additions & 9 deletions src/ApiHost1/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions src/ApiHost1/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
<data name="GetTestingOnlyValidatedRequestValidator_InvalidId" xml:space="preserve">
<value>The Id was either invalid or missing</value>
</data>
<data name="GetTestingOnlyValidatedRequestValidator_InvalidField1" xml:space="preserve">
<value>The Field1 was either invalid or missing</value>
<data name="GetTestingOnlyValidatedRequestValidator_InvalidRequiredField" xml:space="preserve">
<value>The RequiredField was either invalid or missing</value>
</data>
<data name="GetTestingOnlyValidatedRequestValidator_InvalidField2" xml:space="preserve">
<value>The Field2 was either invalid or missing</value>
<data name="GetTestingOnlyValidatedRequestValidator_InvalidOptionalField" xml:space="preserve">
<value>The OptionalField1 was either invalid or missing</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public void WhenEndUtcIsBeforeStartUtc_ThenThrows()
[Fact]
public void WhenDurationIsTooShort_ThenThrows()
{
_dto.EndUtc = _dto.StartUtc.Add(Validations.Booking.MinimumBookingDuration.Subtract(TimeSpan.FromSeconds(1)));
_dto.EndUtc =
_dto.StartUtc!.Value.Add(Validations.Booking.MinimumBookingDuration.Subtract(TimeSpan.FromSeconds(1)));

_validator
.Invoking(x => x.ValidateAndThrow(_dto))
Expand All @@ -74,7 +75,7 @@ public void WhenDurationIsTooShort_ThenThrows()
public void WhenDurationIsTooLong_ThenThrows()
{
_dto.EndUtc =
_dto.StartUtc.Add(Validations.Booking.MaximumBookingDuration.Add(TimeSpan.FromSeconds(1)));
_dto.StartUtc!.Value.Add(Validations.Booking.MaximumBookingDuration.Add(TimeSpan.FromSeconds(1)));

_validator
.Invoking(x => x.ValidateAndThrow(_dto))
Expand Down
4 changes: 2 additions & 2 deletions src/BookingsInfrastructure/Api/Bookings/BookingsApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public BookingsApi(ICallerContextFactory callerFactory, IBookingsApplication boo
public async Task<ApiDeleteResult> Cancel(CancelBookingRequest request, CancellationToken cancellationToken)
{
var booking =
await _bookingsApplication.CancelBookingAsync(_callerFactory.Create(), request.OrganizationId!, request.Id,
await _bookingsApplication.CancelBookingAsync(_callerFactory.Create(), request.OrganizationId!, request.Id!,
cancellationToken);
return () => booking.HandleApplicationResult();
}
Expand All @@ -30,7 +30,7 @@ public async Task<ApiPostResult<Booking, MakeBookingResponse>> Make(MakeBookingR
CancellationToken cancellationToken)
{
var booking = await _bookingsApplication.MakeBookingAsync(_callerFactory.Create(), request.OrganizationId!,
request.CarId, request.StartUtc, request.EndUtc, cancellationToken);
request.CarId!, request.StartUtc.GetValueOrDefault(), request.EndUtc, cancellationToken);

return () => booking.HandleApplicationResult<Booking, MakeBookingResponse>(c =>
new PostResult<MakeBookingResponse>(new MakeBookingResponse { Booking = c }));
Expand Down
Loading

0 comments on commit 46719a1

Please sign in to comment.