Refactor to introduce nullable types and reduce compiler warnings (do…

…tnet-architecture#801) * Introduce nullable types to Core, Infrastructure and PublicApi projects * Refactor unit tests * Fixed failing tests * Introduce Parameter object for CatalogItem update method * Refactor CataloItemDetails param object * Refactor following PR comments
DarthDobber · Oct 4, 2022 · a72dd77 · a72dd77
1 parent aa6305e
commit a72dd77
Showing 51 changed files with 168 additions and 256 deletions.
diff --git a/src/ApplicationCore/ApplicationCore.csproj b/src/ApplicationCore/ApplicationCore.csproj
@@ -3,10 +3,12 @@
 	<PropertyGroup>
 		<TargetFramework>net6.0</TargetFramework>
 		<RootNamespace>Microsoft.eShopWeb.ApplicationCore</RootNamespace>
+		<Nullable>enable</Nullable>
 	</PropertyGroup>
 
 	<ItemGroup>
 		<PackageReference Include="Ardalis.GuardClauses" Version="4.0.1" />
+		<PackageReference Include="Ardalis.Result" Version="4.1.0" />
 		<PackageReference Include="Ardalis.Specification" Version="6.1.0" />
 		<PackageReference Include="MediatR" Version="10.0.1" />
 		<PackageReference Include="System.Security.Claims" Version="4.3.0" />

diff --git a/src/ApplicationCore/CatalogSettings.cs b/src/ApplicationCore/CatalogSettings.cs
@@ -2,5 +2,5 @@
 
 public class CatalogSettings
 {
-    public string CatalogBaseUrl { get; set; }
+    public string? CatalogBaseUrl { get; set; }
 }
diff --git a/src/ApplicationCore/Entities/BasketAggregate/Basket.cs b/src/ApplicationCore/Entities/BasketAggregate/Basket.cs
@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.Linq;
+using Ardalis.GuardClauses;
 using Microsoft.eShopWeb.ApplicationCore.Interfaces;
 
 namespace Microsoft.eShopWeb.ApplicationCore.Entities.BasketAggregate;
@@ -25,7 +26,7 @@ public void AddItem(int catalogItemId, decimal unitPrice, int quantity = 1)
             _items.Add(new BasketItem(catalogItemId, quantity, unitPrice));
             return;
         }
-        var existingItem = Items.FirstOrDefault(i => i.CatalogItemId == catalogItemId);
+        var existingItem = Items.First(i => i.CatalogItemId == catalogItemId);
         existingItem.AddQuantity(quantity);
     }
 

diff --git a/src/ApplicationCore/Entities/BuyerAggregate/Buyer.cs b/src/ApplicationCore/Entities/BuyerAggregate/Buyer.cs
@@ -12,10 +12,8 @@ public class Buyer : BaseEntity, IAggregateRoot
 
     public IEnumerable<PaymentMethod> PaymentMethods => _paymentMethods.AsReadOnly();
 
-    private Buyer()
-    {
-        // required by EF
-    }
+    #pragma warning disable CS8618 // Required by Entity Framework
+    private Buyer() { }
 
     public Buyer(string identity) : this()
     {

diff --git a/src/ApplicationCore/Entities/BuyerAggregate/PaymentMethod.cs b/src/ApplicationCore/Entities/BuyerAggregate/PaymentMethod.cs
@@ -2,7 +2,7 @@
 
 public class PaymentMethod : BaseEntity
 {
-    public string Alias { get; private set; }
-    public string CardId { get; private set; } // actual card data must be stored in a PCI compliant system, like Stripe
-    public string Last4 { get; private set; }
+    public string? Alias { get; private set; }
+    public string? CardId { get; private set; } // actual card data must be stored in a PCI compliant system, like Stripe
+    public string? Last4 { get; private set; }
 }
diff --git a/src/ApplicationCore/Entities/CatalogItem.cs b/src/ApplicationCore/Entities/CatalogItem.cs
@@ -11,9 +11,9 @@ public class CatalogItem : BaseEntity, IAggregateRoot
     public decimal Price { get; private set; }
     public string PictureUri { get; private set; }
     public int CatalogTypeId { get; private set; }
-    public CatalogType CatalogType { get; private set; }
+    public CatalogType? CatalogType { get; private set; }
     public int CatalogBrandId { get; private set; }
-    public CatalogBrand CatalogBrand { get; private set; }
+    public CatalogBrand? CatalogBrand { get; private set; }
 
     public CatalogItem(int catalogTypeId,
         int catalogBrandId,
@@ -30,15 +30,15 @@ public CatalogItem(int catalogTypeId,
         PictureUri = pictureUri;
     }
 
-    public void UpdateDetails(string name, string description, decimal price)
+    public void UpdateDetails(CatalogItemDetails details)
     {
-        Guard.Against.NullOrEmpty(name, nameof(name));
-        Guard.Against.NullOrEmpty(description, nameof(description));
-        Guard.Against.NegativeOrZero(price, nameof(price));
+        Guard.Against.NullOrEmpty(details.Name, nameof(details.Name));
+        Guard.Against.NullOrEmpty(details.Description, nameof(details.Description));
+        Guard.Against.NegativeOrZero(details.Price, nameof(details.Price));
 
-        Name = name;
-        Description = description;
-        Price = price;
+        Name = details.Name;
+        Description = details.Description;
+        Price = details.Price;
     }
 
     public void UpdateBrand(int catalogBrandId)
@@ -62,4 +62,18 @@ public void UpdatePictureUri(string pictureName)
         }
         PictureUri = $"images\\products\\{pictureName}?{new DateTime().Ticks}";
     }
+
+    public readonly record struct CatalogItemDetails
+    {
+        public string? Name { get; }
+        public string? Description { get; }
+        public decimal Price { get; }
+
+        public CatalogItemDetails(string? name, string? description, decimal price)
+        {
+            Name = name;
+            Description = description;
+            Price = price;
+        }
+    }
 }
diff --git a/src/ApplicationCore/Entities/OrderAggregate/Address.cs b/src/ApplicationCore/Entities/OrderAggregate/Address.cs
@@ -12,6 +12,7 @@ public class Address // ValueObject
 
     public string ZipCode { get; private set; }
 
+    #pragma warning disable CS8618 // Required by Entity Framework
     private Address() { }
 
     public Address(string street, string city, string state, string country, string zipcode)

diff --git a/src/ApplicationCore/Entities/OrderAggregate/CatalogItemOrdered.cs b/src/ApplicationCore/Entities/OrderAggregate/CatalogItemOrdered.cs
@@ -19,10 +19,8 @@ public CatalogItemOrdered(int catalogItemId, string productName, string pictureU
         PictureUri = pictureUri;
     }
 
-    private CatalogItemOrdered()
-    {
-        // required by EF
-    }
+    #pragma warning disable CS8618 // Required by Entity Framework
+    private CatalogItemOrdered() {}
 
     public int CatalogItemId { get; private set; }
     public string ProductName { get; private set; }

diff --git a/src/ApplicationCore/Entities/OrderAggregate/Order.cs b/src/ApplicationCore/Entities/OrderAggregate/Order.cs
@@ -7,16 +7,12 @@ namespace Microsoft.eShopWeb.ApplicationCore.Entities.OrderAggregate;
 
 public class Order : BaseEntity, IAggregateRoot
 {
-    private Order()
-    {
-        // required by EF
-    }
+    #pragma warning disable CS8618 // Required by Entity Framework
+    private Order() {}
 
     public Order(string buyerId, Address shipToAddress, List<OrderItem> items)
     {
         Guard.Against.NullOrEmpty(buyerId, nameof(buyerId));
-        Guard.Against.Null(shipToAddress, nameof(shipToAddress));
-        Guard.Against.Null(items, nameof(items));
 
         BuyerId = buyerId;
         ShipToAddress = shipToAddress;

diff --git a/src/ApplicationCore/Entities/OrderAggregate/OrderItem.cs b/src/ApplicationCore/Entities/OrderAggregate/OrderItem.cs
@@ -6,10 +6,8 @@ public class OrderItem : BaseEntity
     public decimal UnitPrice { get; private set; }
     public int Units { get; private set; }
 
-    private OrderItem()
-    {
-        // required by EF
-    }
+    #pragma warning disable CS8618 // Required by Entity Framework
+    private OrderItem() {}
 
     public OrderItem(CatalogItemOrdered itemOrdered, decimal unitPrice, int units)
     {

diff --git a/src/ApplicationCore/Extensions/GuardExtensions.cs b/src/ApplicationCore/Extensions/GuardExtensions.cs
@@ -7,12 +7,6 @@ namespace Ardalis.GuardClauses;
 
 public static class BasketGuards
 {
-    public static void NullBasket(this IGuardClause guardClause, int basketId, Basket basket)
-    {
-        if (basket == null)
-            throw new BasketNotFoundException(basketId);
-    }
-
     public static void EmptyBasketOnCheckout(this IGuardClause guardClause, IReadOnlyCollection<BasketItem> basketItems)
     {
         if (!basketItems.Any())

diff --git a/src/ApplicationCore/Extensions/JsonExtensions.cs b/src/ApplicationCore/Extensions/JsonExtensions.cs
@@ -9,7 +9,7 @@ public static class JsonExtensions
         PropertyNameCaseInsensitive = true
     };
 
-    public static T FromJson<T>(this string json) =>
+    public static T? FromJson<T>(this string json) =>
         JsonSerializer.Deserialize<T>(json, _jsonOptions);
 
     public static string ToJson<T>(this T obj) =>

diff --git a/src/ApplicationCore/Interfaces/IBasketService.cs b/src/ApplicationCore/Interfaces/IBasketService.cs
@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
+using Ardalis.Result;
 using Microsoft.eShopWeb.ApplicationCore.Entities.BasketAggregate;
 
 namespace Microsoft.eShopWeb.ApplicationCore.Interfaces;
@@ -8,6 +9,6 @@ public interface IBasketService
 {
     Task TransferBasketAsync(string anonymousId, string userName);
     Task<Basket> AddItemToBasket(string username, int catalogItemId, decimal price, int quantity = 1);
-    Task<Basket> SetQuantities(int basketId, Dictionary<string, int> quantities);
+    Task<Result<Basket>> SetQuantities(int basketId, Dictionary<string, int> quantities);
     Task DeleteBasketAsync(int basketId);
 }
diff --git a/src/ApplicationCore/Services/BasketService.cs b/src/ApplicationCore/Services/BasketService.cs
@@ -1,6 +1,7 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Ardalis.GuardClauses;
+using Ardalis.Result;
 using Microsoft.eShopWeb.ApplicationCore.Entities.BasketAggregate;
 using Microsoft.eShopWeb.ApplicationCore.Interfaces;
 using Microsoft.eShopWeb.ApplicationCore.Specifications;
@@ -22,7 +23,7 @@ public BasketService(IRepository<Basket> basketRepository,
     public async Task<Basket> AddItemToBasket(string username, int catalogItemId, decimal price, int quantity = 1)
     {
         var basketSpec = new BasketWithItemsSpecification(username);
-        var basket = await _basketRepository.GetBySpecAsync(basketSpec);
+        var basket = await _basketRepository.FirstOrDefaultAsync(basketSpec);
 
         if (basket == null)
         {
@@ -39,15 +40,15 @@ public async Task<Basket> AddItemToBasket(string username, int catalogItemId, de
     public async Task DeleteBasketAsync(int basketId)
     {
         var basket = await _basketRepository.GetByIdAsync(basketId);
+        Guard.Against.Null(basket, nameof(basket));
         await _basketRepository.DeleteAsync(basket);
     }
 
-    public async Task<Basket> SetQuantities(int basketId, Dictionary<string, int> quantities)
+    public async Task<Result<Basket>> SetQuantities(int basketId, Dictionary<string, int> quantities)
     {
-        Guard.Against.Null(quantities, nameof(quantities));
         var basketSpec = new BasketWithItemsSpecification(basketId);
-        var basket = await _basketRepository.GetBySpecAsync(basketSpec);
-        Guard.Against.NullBasket(basketId, basket);
+        var basket = await _basketRepository.FirstOrDefaultAsync(basketSpec);
+        if (basket == null) return Result<Basket>.NotFound();
 
         foreach (var item in basket.Items)
         {
@@ -64,13 +65,11 @@ public async Task<Basket> SetQuantities(int basketId, Dictionary<string, int> qu
 
     public async Task TransferBasketAsync(string anonymousId, string userName)
     {
-        Guard.Against.NullOrEmpty(anonymousId, nameof(anonymousId));
-        Guard.Against.NullOrEmpty(userName, nameof(userName));
         var anonymousBasketSpec = new BasketWithItemsSpecification(anonymousId);
-        var anonymousBasket = await _basketRepository.GetBySpecAsync(anonymousBasketSpec);
+        var anonymousBasket = await _basketRepository.FirstOrDefaultAsync(anonymousBasketSpec);
         if (anonymousBasket == null) return;
         var userBasketSpec = new BasketWithItemsSpecification(userName);
-        var userBasket = await _basketRepository.GetBySpecAsync(userBasketSpec);
+        var userBasket = await _basketRepository.FirstOrDefaultAsync(userBasketSpec);
         if (userBasket == null)
         {
             userBasket = new Basket(userName);

diff --git a/src/ApplicationCore/Services/OrderService.cs b/src/ApplicationCore/Services/OrderService.cs
@@ -30,9 +30,9 @@ public OrderService(IRepository<Basket> basketRepository,
     public async Task CreateOrderAsync(int basketId, Address shippingAddress)
     {
         var basketSpec = new BasketWithItemsSpecification(basketId);
-        var basket = await _basketRepository.GetBySpecAsync(basketSpec);
+        var basket = await _basketRepository.FirstOrDefaultAsync(basketSpec);
 
-        Guard.Against.NullBasket(basketId, basket);
+        Guard.Against.Null(basket, nameof(basket));
         Guard.Against.EmptyBasketOnCheckout(basket.Items);
 
         var catalogItemsSpecification = new CatalogItemsSpecification(basket.Items.Select(item => item.CatalogItemId).ToArray());

diff --git a/src/Infrastructure/Data/CatalogContext.cs b/src/Infrastructure/Data/CatalogContext.cs
@@ -8,9 +8,8 @@ namespace Microsoft.eShopWeb.Infrastructure.Data;
 
 public class CatalogContext : DbContext
 {
-    public CatalogContext(DbContextOptions<CatalogContext> options) : base(options)
-    {
-    }
+    #pragma warning disable CS8618 // Required by Entity Framework
+    public CatalogContext(DbContextOptions<CatalogContext> options) : base(options) {}
 
     public DbSet<Basket> Baskets { get; set; }
     public DbSet<CatalogItem> CatalogItems { get; set; }

diff --git a/src/Infrastructure/Data/Config/BasketConfiguration.cs b/src/Infrastructure/Data/Config/BasketConfiguration.cs
@@ -9,7 +9,7 @@ public class BasketConfiguration : IEntityTypeConfiguration<Basket>
     public void Configure(EntityTypeBuilder<Basket> builder)
     {
         var navigation = builder.Metadata.FindNavigation(nameof(Basket.Items));
-        navigation.SetPropertyAccessMode(PropertyAccessMode.Field);
+        navigation?.SetPropertyAccessMode(PropertyAccessMode.Field);
 
         builder.Property(b => b.BuyerId)
             .IsRequired()

diff --git a/src/Infrastructure/Data/Config/OrderConfiguration.cs b/src/Infrastructure/Data/Config/OrderConfiguration.cs
@@ -10,7 +10,7 @@ public void Configure(EntityTypeBuilder<Order> builder)
     {
         var navigation = builder.Metadata.FindNavigation(nameof(Order.OrderItems));
 
-        navigation.SetPropertyAccessMode(PropertyAccessMode.Field);
+        navigation?.SetPropertyAccessMode(PropertyAccessMode.Field);
 
         builder.Property(b => b.BuyerId)
             .IsRequired()

diff --git a/src/Infrastructure/Data/FileItem.cs b/src/Infrastructure/Data/FileItem.cs
@@ -2,10 +2,10 @@
 
 public class FileItem
 {
-    public string FileName { get; set; }
-    public string Url { get; set; }
+    public string? FileName { get; set; }
+    public string? Url { get; set; }
     public long Size { get; set; }
-    public string Ext { get; set; }
-    public string Type { get; set; }
-    public string DataBase64 { get; set; }
+    public string? Ext { get; set; }
+    public string? Type { get; set; }
+    public string? DataBase64 { get; set; }
 }
diff --git a/src/Infrastructure/Infrastructure.csproj b/src/Infrastructure/Infrastructure.csproj
@@ -3,6 +3,7 @@
 	<PropertyGroup>
 		<TargetFramework>net6.0</TargetFramework>
 		<RootNamespace>Microsoft.eShopWeb.Infrastructure</RootNamespace>
+		<Nullable>enable</Nullable>
 	</PropertyGroup>
 
 	<ItemGroup>

diff --git a/src/PublicApi/CatalogItemEndpoints/UpdateCatalogItemEndpoint.cs b/src/PublicApi/CatalogItemEndpoints/UpdateCatalogItemEndpoint.cs
@@ -41,8 +41,9 @@ public async Task<IResult> HandleAsync(UpdateCatalogItemRequest request)
         var response = new UpdateCatalogItemResponse(request.CorrelationId());
 
         var existingItem = await _itemRepository.GetByIdAsync(request.Id);
-
-        existingItem.UpdateDetails(request.Name, request.Description, request.Price);
+
+        CatalogItem.CatalogItemDetails details = new(request.Name, request.Description, request.Price);
+        existingItem.UpdateDetails(details);
         existingItem.UpdateBrand(request.CatalogBrandId);
         existingItem.UpdateType(request.CatalogTypeId);
 

diff --git a/src/PublicApi/PublicApi.csproj b/src/PublicApi/PublicApi.csproj
@@ -6,6 +6,7 @@
     <UserSecretsId>5b662463-1efd-4bae-bde4-befe0be3e8ff</UserSecretsId>
     <DockerDefaultTargetOS>Linux</DockerDefaultTargetOS>
     <DockerfileContext>..\..</DockerfileContext>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/src/Web/Areas/Identity/Pages/Account/Login.cshtml.cs b/src/Web/Areas/Identity/Pages/Account/Login.cshtml.cs
@@ -1,4 +1,5 @@
 using System.ComponentModel.DataAnnotations;
+using Ardalis.GuardClauses;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Identity;
@@ -73,17 +74,17 @@ public async Task<IActionResult> OnPostAsync(string? returnUrl = null)
             // This doesn't count login failures towards account lockout
             // To enable password failures to trigger account lockout, set lockoutOnFailure: true
             //var result = await _signInManager.PasswordSignInAsync(Input.Email, Input.Password, Input.RememberMe, lockoutOnFailure: true);
-            var result = await _signInManager.PasswordSignInAsync(Input.Email, Input.Password, false, true);
+            var result = await _signInManager.PasswordSignInAsync(Input?.Email, Input?.Password, false, true);
 
             if (result.Succeeded)
             {
                 _logger.LogInformation("User logged in.");
-                await TransferAnonymousBasketToUserAsync(Input.Email);
+                await TransferAnonymousBasketToUserAsync(Input?.Email);
                 return LocalRedirect(returnUrl);
             }
             if (result.RequiresTwoFactor)
             {
-                return RedirectToPage("./LoginWith2fa", new { ReturnUrl = returnUrl, RememberMe = Input.RememberMe });
+                return RedirectToPage("./LoginWith2fa", new { ReturnUrl = returnUrl, RememberMe = Input?.RememberMe });
             }
             if (result.IsLockedOut)
             {
@@ -108,6 +109,7 @@ private async Task TransferAnonymousBasketToUserAsync(string? userName)
             var anonymousId = Request.Cookies[Constants.BASKET_COOKIENAME];
             if (Guid.TryParse(anonymousId, out var _))
             {
+                Guard.Against.NullOrEmpty(userName, nameof(userName));
                 await _basketService.TransferBasketAsync(anonymousId, userName);
             }
             Response.Cookies.Delete(Constants.BASKET_COOKIENAME);