From d4c684e11cd2122423ef1b8aec4e4f41c25427f3 Mon Sep 17 00:00:00 2001 From: Ricardo Ferreira Date: Thu, 1 Jun 2023 11:18:32 +0100 Subject: [PATCH 1/6] Mail Container Test --- .../LargeLetterStrategyTests.cs | 61 ++++++++++ .../MailContainerTest.Tests.csproj | 10 ++ .../MailTransferService.cs | 81 +++++++++++++ .../MailTransferStrategyFactoryTests.cs | 37 ++++++ ....GeneratedMSBuildEditorConfig.editorconfig | 3 +- .../Abstractions/ILoggerAdapter.cs | 6 + .../Abstractions/IMailContainerDataStore.cs | 10 ++ .../IMailContainerDataStoreFactory.cs | 8 ++ .../IMailTransferService.cs | 2 +- .../Abstractions/IMailTransferStrategy.cs | 8 ++ .../IMailTransferStrategyFactory.cs | 8 ++ MailContainerTest/Abstractions/IUnitOfWork.cs | 8 ++ MailContainerTest/Adapters/LoggerAdapter.cs | 19 +++ .../Data/BackupMailContainerDataStore.cs | 7 +- .../Data/MailContainerDataStore.cs | 5 +- .../MailContainerDataStoreFactory.cs | 27 +++++ .../Factories/MailTransferStrategyFactory.cs | 18 +++ MailContainerTest/MailContainerTest.csproj | 2 + .../Options/MailContainerDataStoreOptions.cs | 6 + .../Services/MailTransferService.cs | 111 +++++++----------- .../Strategies/LargeLetterStrategy.cs | 27 +++++ .../Strategies/SmallParcelStrategy.cs | 27 +++++ .../Strategies/StandardLetterStrategy.cs | 27 +++++ MailContainerTest/Types/AllowedMailType.cs | 4 +- .../Types/MailContainerNumber.cs | 20 ++++ README.md | 7 ++ 26 files changed, 472 insertions(+), 77 deletions(-) create mode 100644 MailContainerTest.Tests/LargeLetterStrategyTests.cs create mode 100644 MailContainerTest.Tests/MailTransferService.cs create mode 100644 MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs create mode 100644 MailContainerTest/Abstractions/ILoggerAdapter.cs create mode 100644 MailContainerTest/Abstractions/IMailContainerDataStore.cs create mode 100644 MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs rename MailContainerTest/{Services => Abstractions}/IMailTransferService.cs (80%) create mode 100644 MailContainerTest/Abstractions/IMailTransferStrategy.cs create mode 100644 MailContainerTest/Abstractions/IMailTransferStrategyFactory.cs create mode 100644 MailContainerTest/Abstractions/IUnitOfWork.cs create mode 100644 MailContainerTest/Adapters/LoggerAdapter.cs create mode 100644 MailContainerTest/Factories/MailContainerDataStoreFactory.cs create mode 100644 MailContainerTest/Factories/MailTransferStrategyFactory.cs create mode 100644 MailContainerTest/Options/MailContainerDataStoreOptions.cs create mode 100644 MailContainerTest/Strategies/LargeLetterStrategy.cs create mode 100644 MailContainerTest/Strategies/SmallParcelStrategy.cs create mode 100644 MailContainerTest/Strategies/StandardLetterStrategy.cs create mode 100644 MailContainerTest/Types/MailContainerNumber.cs diff --git a/MailContainerTest.Tests/LargeLetterStrategyTests.cs b/MailContainerTest.Tests/LargeLetterStrategyTests.cs new file mode 100644 index 0000000..58d60ca --- /dev/null +++ b/MailContainerTest.Tests/LargeLetterStrategyTests.cs @@ -0,0 +1,61 @@ +using FluentAssertions; +using MailContainerTest.Strategies; +using MailContainerTest.Types; +using NSubstitute; +using NSubstitute.ReceivedExtensions; +using Xunit; + +namespace MailContainerTest.Tests; + +public sealed class LargeLetterStrategyTests +{ + [Fact] + public void IsSuccess_ShouldReturnTrue_WhenAllowedMailTypeEqual() + { + // Arrange + var strategy = new LargeLetterStrategy(); + var sourceContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter + }; + var destContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter + }; + var request = new MakeMailTransferRequest + { + MailType = MailType.LargeLetter + }; + + // Act + var result = strategy.IsSuccess(sourceContainer, destContainer, request); + + // Assert + result.Should().BeTrue(); + } + + [Fact] + public void IsSuccess_ShouldReturnFalse_WhenFAllowedMailTypeNotEqual() + { + // Arrange + var strategy = new LargeLetterStrategy(); + var sourceContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter + }; + var destContainer = new MailContainer + { + AllowedMailType = AllowedMailType.SmallParcel + }; + var request = new MakeMailTransferRequest + { + MailType = MailType.LargeLetter + }; + + // Act + var result = strategy.IsSuccess(sourceContainer, destContainer, request); + + // Assert + result.Should().BeFalse(); + } +} \ No newline at end of file diff --git a/MailContainerTest.Tests/MailContainerTest.Tests.csproj b/MailContainerTest.Tests/MailContainerTest.Tests.csproj index 132c02c..4e22f48 100644 --- a/MailContainerTest.Tests/MailContainerTest.Tests.csproj +++ b/MailContainerTest.Tests/MailContainerTest.Tests.csproj @@ -6,4 +6,14 @@ enable + + + + + + + + + + diff --git a/MailContainerTest.Tests/MailTransferService.cs b/MailContainerTest.Tests/MailTransferService.cs new file mode 100644 index 0000000..a95a25e --- /dev/null +++ b/MailContainerTest.Tests/MailTransferService.cs @@ -0,0 +1,81 @@ +using FluentAssertions; +using MailContainerTest.Abstractions; +using MailContainerTest.Data; +using MailContainerTest.Services; +using MailContainerTest.Types; +using NSubstitute; +using Xunit; + +namespace MailContainerTest.Tests; + +public sealed class MailTransferServiceTests +{ + private readonly IMailContainerDataStoreFactory _mailContainerDataStoreFactory = Substitute.For(); + private readonly IMailTransferStrategyFactory _mailTransferStrategyFactory = Substitute.For(); + private readonly IUnitOfWork _unitOfWork = Substitute.For(); + private readonly ILoggerAdapter _loggerAdapter = Substitute.For>(); + + [Fact] + public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() + { + // Arrange + _mailContainerDataStoreFactory.CreateMailContainerDataStore(Arg.Any()) + .Returns(new MailContainerDataStore()); + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(false); + + var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); + + // Act + var result = mailTransferService.MakeMailTransfer(Arg.Any()); + + // Assert + _unitOfWork.DidNotReceive().Commit(); + result.Success.Should().BeFalse(); + } + + [Fact] + public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() + { + // Arrange + _mailContainerDataStoreFactory.CreateMailContainerDataStore(Arg.Any()) + .Returns(new MailContainerDataStore()); + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); + + // Act + var result = mailTransferService.MakeMailTransfer(Arg.Any()); + + // Assert + _unitOfWork.Received(1).Commit(); + result.Success.Should().BeTrue(); + } + + [Fact] + public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() + { + // Arrange + _mailContainerDataStoreFactory.CreateMailContainerDataStore(Arg.Any()) + .Returns(new MailContainerDataStore()); + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); + + _unitOfWork.When(static x => x.Commit()).Throw(); + + // Act + var result = mailTransferService.MakeMailTransfer(Arg.Any()); + + // Assert + _unitOfWork.Received(1).Commit(); + _unitOfWork.Received(1).Rollback(); + _loggerAdapter.Received(1).LogError(Arg.Any(), "Error saving changes to containers"); + result.Success.Should().BeFalse(); + } +} \ No newline at end of file diff --git a/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs b/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs new file mode 100644 index 0000000..ebebf69 --- /dev/null +++ b/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs @@ -0,0 +1,37 @@ +using FluentAssertions; +using MailContainerTest.Factories; +using MailContainerTest.Strategies; +using MailContainerTest.Types; +using NSubstitute.ExceptionExtensions; +using Xunit; + +namespace MailContainerTest.Tests; + +public sealed class MailTransferStrategyFactoryTests +{ + [Fact] + public void CreateMakeMailTransferStrategy_ShouldReturnLargeLetterStrategy_WhenMailTypeIsLargeLetter() + { + // Arrange + var factory = new MailTransferStrategyFactory(); + + // Act + var result = factory.CreateMakeMailTransferStrategy(MailType.LargeLetter); + + // Assert + result.Should().BeOfType(); + } + + [Fact] + public void CreateMakeMailTransferStrategy_ShouldThrowArgumentOutOfRangeException_WhenMailTypeIsNotInEnumRange() + { + // Arrange + var factory = new MailTransferStrategyFactory(); + + // Act + Action result = () => factory.CreateMakeMailTransferStrategy((MailType) (-1)); + + // Assert + result.Should().Throw().WithMessage("Mail type is not in enum range"); + } +} \ No newline at end of file diff --git a/MailContainerTest.Tests/obj/Debug/net6.0/MailContainerTest.Tests.GeneratedMSBuildEditorConfig.editorconfig b/MailContainerTest.Tests/obj/Debug/net6.0/MailContainerTest.Tests.GeneratedMSBuildEditorConfig.editorconfig index ed46c72..ef93c82 100644 --- a/MailContainerTest.Tests/obj/Debug/net6.0/MailContainerTest.Tests.GeneratedMSBuildEditorConfig.editorconfig +++ b/MailContainerTest.Tests/obj/Debug/net6.0/MailContainerTest.Tests.GeneratedMSBuildEditorConfig.editorconfig @@ -5,6 +5,7 @@ build_property.UsingMicrosoftNETSdkWeb = build_property.ProjectTypeGuids = build_property.InvariantGlobalization = build_property.PlatformNeutralAssembly = +build_property.EnforceExtendedAnalyzerRules = build_property._SupportedPlatformList = Linux,macOS,Windows build_property.RootNamespace = MailContainerTest.Tests -build_property.ProjectDir = c:\repos\test\MailContainerTest\MailContainerTest\MailContainerTest.Tests\ +build_property.ProjectDir = C:\Users\ricar\source\github\MailContainerTest\MailContainerTest.Tests\ diff --git a/MailContainerTest/Abstractions/ILoggerAdapter.cs b/MailContainerTest/Abstractions/ILoggerAdapter.cs new file mode 100644 index 0000000..8ac1506 --- /dev/null +++ b/MailContainerTest/Abstractions/ILoggerAdapter.cs @@ -0,0 +1,6 @@ +namespace MailContainerTest.Abstractions; + +public interface ILoggerAdapter +{ + void LogError(Exception ex,string message); +} \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IMailContainerDataStore.cs b/MailContainerTest/Abstractions/IMailContainerDataStore.cs new file mode 100644 index 0000000..8582a0d --- /dev/null +++ b/MailContainerTest/Abstractions/IMailContainerDataStore.cs @@ -0,0 +1,10 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Abstractions; + +public interface IMailContainerDataStore +{ + MailContainer GetMailContainer(string mailContainerNumber); + + void UpdateMailContainer(MailContainer mailContainer); +} \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs b/MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs new file mode 100644 index 0000000..df98d19 --- /dev/null +++ b/MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs @@ -0,0 +1,8 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Abstractions; + +public interface IMailContainerDataStoreFactory +{ + IMailContainerDataStore CreateMailContainerDataStore(MailContainerNumber mailContainerNumber); +} \ No newline at end of file diff --git a/MailContainerTest/Services/IMailTransferService.cs b/MailContainerTest/Abstractions/IMailTransferService.cs similarity index 80% rename from MailContainerTest/Services/IMailTransferService.cs rename to MailContainerTest/Abstractions/IMailTransferService.cs index fd1b275..1607465 100644 --- a/MailContainerTest/Services/IMailTransferService.cs +++ b/MailContainerTest/Abstractions/IMailTransferService.cs @@ -1,6 +1,6 @@ using MailContainerTest.Types; -namespace MailContainerTest.Services +namespace MailContainerTest.Abstractions { public interface IMailTransferService { diff --git a/MailContainerTest/Abstractions/IMailTransferStrategy.cs b/MailContainerTest/Abstractions/IMailTransferStrategy.cs new file mode 100644 index 0000000..c22bfaa --- /dev/null +++ b/MailContainerTest/Abstractions/IMailTransferStrategy.cs @@ -0,0 +1,8 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Abstractions; + +public interface IMailTransferStrategy +{ + bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request); +} \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IMailTransferStrategyFactory.cs b/MailContainerTest/Abstractions/IMailTransferStrategyFactory.cs new file mode 100644 index 0000000..a16de75 --- /dev/null +++ b/MailContainerTest/Abstractions/IMailTransferStrategyFactory.cs @@ -0,0 +1,8 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Abstractions; + +public interface IMailTransferStrategyFactory +{ + IMailTransferStrategy CreateMakeMailTransferStrategy(MailType mailType); +} \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IUnitOfWork.cs b/MailContainerTest/Abstractions/IUnitOfWork.cs new file mode 100644 index 0000000..052b893 --- /dev/null +++ b/MailContainerTest/Abstractions/IUnitOfWork.cs @@ -0,0 +1,8 @@ +namespace MailContainerTest.Abstractions; + +public interface IUnitOfWork +{ + void Commit(); + + void Rollback(); +} \ No newline at end of file diff --git a/MailContainerTest/Adapters/LoggerAdapter.cs b/MailContainerTest/Adapters/LoggerAdapter.cs new file mode 100644 index 0000000..f960d1a --- /dev/null +++ b/MailContainerTest/Adapters/LoggerAdapter.cs @@ -0,0 +1,19 @@ +using MailContainerTest.Abstractions; +using Microsoft.Extensions.Logging; + +namespace MailContainerTest.Adapters; + +public sealed class LoggerAdapter : ILoggerAdapter where T : class +{ + private readonly ILogger _logger; + + public LoggerAdapter(ILogger logger) + { + _logger = logger; + } + + public void LogError(Exception ex, string message) + { + _logger.LogError(message, ex); + } +} \ No newline at end of file diff --git a/MailContainerTest/Data/BackupMailContainerDataStore.cs b/MailContainerTest/Data/BackupMailContainerDataStore.cs index 614b1dc..75c3364 100644 --- a/MailContainerTest/Data/BackupMailContainerDataStore.cs +++ b/MailContainerTest/Data/BackupMailContainerDataStore.cs @@ -1,9 +1,12 @@ -using MailContainerTest.Types; +using MailContainerTest.Abstractions; +using MailContainerTest.Types; namespace MailContainerTest.Data { - public class BackupMailContainerDataStore + public sealed class BackupMailContainerDataStore : IMailContainerDataStore { + public static string DataStoreType => "Backup"; + public MailContainer GetMailContainer(string mailContainerNumber) { // Access the database and return the retrieved mail container. Implementation not required for this exercise. diff --git a/MailContainerTest/Data/MailContainerDataStore.cs b/MailContainerTest/Data/MailContainerDataStore.cs index c57cf54..175b31d 100644 --- a/MailContainerTest/Data/MailContainerDataStore.cs +++ b/MailContainerTest/Data/MailContainerDataStore.cs @@ -1,8 +1,9 @@ -using MailContainerTest.Types; +using MailContainerTest.Abstractions; +using MailContainerTest.Types; namespace MailContainerTest.Data { - public class MailContainerDataStore + public sealed class MailContainerDataStore : IMailContainerDataStore { public MailContainer GetMailContainer(string mailContainerNumber) { diff --git a/MailContainerTest/Factories/MailContainerDataStoreFactory.cs b/MailContainerTest/Factories/MailContainerDataStoreFactory.cs new file mode 100644 index 0000000..884633c --- /dev/null +++ b/MailContainerTest/Factories/MailContainerDataStoreFactory.cs @@ -0,0 +1,27 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Data; +using MailContainerTest.Options; +using MailContainerTest.Types; +using Microsoft.Extensions.Options; + +namespace MailContainerTest.Factories; + +public sealed class MailContainerDataStoreFactory : IMailContainerDataStoreFactory +{ + private readonly IOptionsSnapshot _options; + + public MailContainerDataStoreFactory(IOptionsSnapshot options) + { + _options = options; + } + + public IMailContainerDataStore CreateMailContainerDataStore(MailContainerNumber mailContainerNumber) + { + if (_options.Value.Equals(BackupMailContainerDataStore.DataStoreType)) + { + return new BackupMailContainerDataStore(); + } + + return new MailContainerDataStore(); + } +} \ No newline at end of file diff --git a/MailContainerTest/Factories/MailTransferStrategyFactory.cs b/MailContainerTest/Factories/MailTransferStrategyFactory.cs new file mode 100644 index 0000000..78b2e71 --- /dev/null +++ b/MailContainerTest/Factories/MailTransferStrategyFactory.cs @@ -0,0 +1,18 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Strategies; +using MailContainerTest.Types; + +namespace MailContainerTest.Factories; + +public sealed class MailTransferStrategyFactory : IMailTransferStrategyFactory +{ + public IMailTransferStrategy CreateMakeMailTransferStrategy(MailType mailType) + { + return mailType switch { + MailType.StandardLetter => new StandardLetterStrategy(), + MailType.LargeLetter => new LargeLetterStrategy(), + MailType.SmallParcel => new SmallParcelStrategy(), + _ => throw new ArgumentOutOfRangeException(nameof(mailType), mailType, "Mail type is not in enum range") + }; + } +} \ No newline at end of file diff --git a/MailContainerTest/MailContainerTest.csproj b/MailContainerTest/MailContainerTest.csproj index 03f4d06..f5de6b3 100644 --- a/MailContainerTest/MailContainerTest.csproj +++ b/MailContainerTest/MailContainerTest.csproj @@ -8,6 +8,8 @@ + + diff --git a/MailContainerTest/Options/MailContainerDataStoreOptions.cs b/MailContainerTest/Options/MailContainerDataStoreOptions.cs new file mode 100644 index 0000000..7fd4c65 --- /dev/null +++ b/MailContainerTest/Options/MailContainerDataStoreOptions.cs @@ -0,0 +1,6 @@ +namespace MailContainerTest.Options; + +public sealed class MailContainerDataStoreOptions +{ + public string? DataStoreType { get; set; } +} \ No newline at end of file diff --git a/MailContainerTest/Services/MailTransferService.cs b/MailContainerTest/Services/MailTransferService.cs index a124501..2d2c395 100644 --- a/MailContainerTest/Services/MailTransferService.cs +++ b/MailContainerTest/Services/MailTransferService.cs @@ -1,93 +1,66 @@ using MailContainerTest.Data; using MailContainerTest.Types; using System.Configuration; +using MailContainerTest.Abstractions; +using Microsoft.Extensions.Logging; namespace MailContainerTest.Services { - public class MailTransferService : IMailTransferService + public sealed class MailTransferService : IMailTransferService { - public MakeMailTransferResult MakeMailTransfer(MakeMailTransferRequest request) - { - var dataStoreType = ConfigurationManager.AppSettings["DataStoreType"]; - - MailContainer mailContainer = null; - - if (dataStoreType == "Backup") - { - var mailContainerDataStore = new BackupMailContainerDataStore(); - mailContainer = mailContainerDataStore.GetMailContainer(request.SourceMailContainerNumber); + private readonly IMailContainerDataStoreFactory _mailContainerDataStoreFactory; + private readonly IMailTransferStrategyFactory _mailTransferStrategyFactory; + private readonly IUnitOfWork _unitOfWork; + private readonly ILoggerAdapter _loggerAdapter; - } else - { - var mailContainerDataStore = new MailContainerDataStore(); - mailContainer = mailContainerDataStore.GetMailContainer(request.SourceMailContainerNumber); - } - - var result = new MakeMailTransferResult(); - - switch (request.MailType) - { - case MailType.StandardLetter: - if (mailContainer == null) - { - result.Success = false; - } - else if (!mailContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter)) - { - result.Success = false; - } - break; + public MailTransferService(IMailContainerDataStoreFactory mailContainerDataStoreFactory, + IMailTransferStrategyFactory mailTransferStrategyFactory, + IUnitOfWork unitOfWork, + ILoggerAdapter loggerAdapter) + { + _mailContainerDataStoreFactory = mailContainerDataStoreFactory; + _mailTransferStrategyFactory = mailTransferStrategyFactory; + _unitOfWork = unitOfWork; + _loggerAdapter = loggerAdapter; + } - case MailType.LargeLetter: - if (mailContainer == null) - { - result.Success = false; - } - else if (!mailContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter)) - { - result.Success = false; - } - else if (mailContainer.Capacity < request.NumberOfMailItems) - { - result.Success = false; - } - break; + public MakeMailTransferResult MakeMailTransfer(MakeMailTransferRequest request) + { + var sourceMailContainerDataStore = _mailContainerDataStoreFactory.CreateMailContainerDataStore(request.SourceMailContainerNumber); + var sourceMailContainer = sourceMailContainerDataStore.GetMailContainer(request.SourceMailContainerNumber); - case MailType.SmallParcel: - if (mailContainer == null) - { - result.Success = false; - } - else if (!mailContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel)) - { - result.Success = false; + var destMailContainerDataStore = _mailContainerDataStoreFactory.CreateMailContainerDataStore(request.DestinationMailContainerNumber); + var destMailContainer = destMailContainerDataStore.GetMailContainer(request.DestinationMailContainerNumber); - } - else if (mailContainer.Status != MailContainerStatus.Operational) - { - result.Success = false; - } - break; - } + var result = new MakeMailTransferResult + { + Success = _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(request.MailType) + .IsSuccess(sourceMailContainer, destMailContainer, request) + }; if (result.Success) { - mailContainer.Capacity -= request.NumberOfMailItems; - - if (dataStoreType == "Backup") + try { - var mailContainerDataStore = new BackupMailContainerDataStore(); - mailContainerDataStore.UpdateMailContainer(mailContainer); + sourceMailContainer.Capacity -= request.NumberOfMailItems; + destMailContainer.Capacity += request.NumberOfMailItems; + + sourceMailContainerDataStore.UpdateMailContainer(sourceMailContainer); + destMailContainerDataStore.UpdateMailContainer(destMailContainer); + _unitOfWork.Commit(); } - else + catch (Exception ex) { - var mailContainerDataStore = new MailContainerDataStore(); - mailContainerDataStore.UpdateMailContainer(mailContainer); + result.Success = false; + + _unitOfWork.Rollback(); + + _loggerAdapter.LogError(ex, "Error saving changes to containers"); } } return result; } } -} +} \ No newline at end of file diff --git a/MailContainerTest/Strategies/LargeLetterStrategy.cs b/MailContainerTest/Strategies/LargeLetterStrategy.cs new file mode 100644 index 0000000..2301e19 --- /dev/null +++ b/MailContainerTest/Strategies/LargeLetterStrategy.cs @@ -0,0 +1,27 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Types; + +namespace MailContainerTest.Strategies; + +public sealed class LargeLetterStrategy : IMailTransferStrategy +{ + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + { + if (sourceContainer is null || destContainer is null) + { + return false; + } + + if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter) && !destContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter)) + { + return false; + } + + if (sourceContainer.Status != MailContainerStatus.Operational || destContainer.Status != MailContainerStatus.Operational) + { + return false; + } + + return true; + } +} \ No newline at end of file diff --git a/MailContainerTest/Strategies/SmallParcelStrategy.cs b/MailContainerTest/Strategies/SmallParcelStrategy.cs new file mode 100644 index 0000000..09dbc4a --- /dev/null +++ b/MailContainerTest/Strategies/SmallParcelStrategy.cs @@ -0,0 +1,27 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Types; + +namespace MailContainerTest.Strategies; + +public sealed class SmallParcelStrategy : IMailTransferStrategy +{ + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + { + if (sourceContainer is null || destContainer is null) + { + return false; + } + + if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel) && !destContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel)) + { + return false; + } + + if (sourceContainer.Status != MailContainerStatus.Operational || destContainer.Status != MailContainerStatus.Operational) + { + return false; + } + + return true; + } +} \ No newline at end of file diff --git a/MailContainerTest/Strategies/StandardLetterStrategy.cs b/MailContainerTest/Strategies/StandardLetterStrategy.cs new file mode 100644 index 0000000..083be75 --- /dev/null +++ b/MailContainerTest/Strategies/StandardLetterStrategy.cs @@ -0,0 +1,27 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Types; + +namespace MailContainerTest.Strategies; + +public sealed class StandardLetterStrategy : IMailTransferStrategy +{ + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + { + if (sourceContainer is null || destContainer is null) + { + return false; + } + + if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter) && !destContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter)) + { + return false; + } + + if (sourceContainer.Status != MailContainerStatus.Operational || destContainer.Status != MailContainerStatus.Operational) + { + return false; + } + + return true; + } +} \ No newline at end of file diff --git a/MailContainerTest/Types/AllowedMailType.cs b/MailContainerTest/Types/AllowedMailType.cs index 2c7612b..2adec11 100644 --- a/MailContainerTest/Types/AllowedMailType.cs +++ b/MailContainerTest/Types/AllowedMailType.cs @@ -2,8 +2,8 @@ { public enum AllowedMailType { - StandardLetter = 1 , + StandardLetter = 1, LargeLetter = 2, - SmallParcel = 3 + SmallParcel = 4 } } \ No newline at end of file diff --git a/MailContainerTest/Types/MailContainerNumber.cs b/MailContainerTest/Types/MailContainerNumber.cs new file mode 100644 index 0000000..8856adc --- /dev/null +++ b/MailContainerTest/Types/MailContainerNumber.cs @@ -0,0 +1,20 @@ +namespace MailContainerTest.Types; + +public readonly record struct MailContainerNumber +{ + private string Value { get; } + + private MailContainerNumber(string value) + { + if (string.IsNullOrWhiteSpace(value)) + { + throw new ArgumentNullException(); + } + + Value = value; + } + + public static implicit operator string(MailContainerNumber mailContainerNumber) => mailContainerNumber.Value; + + public static implicit operator MailContainerNumber(string value) => new(value); +} \ No newline at end of file diff --git a/README.md b/README.md index e876a4d..edab595 100644 --- a/README.md +++ b/README.md @@ -32,3 +32,10 @@ You should add suitable tests into the MailContainerTest.Test project. There are no additional constraints, use the packages and approach you feel appropriate, aim to spend no more than 2 hours. Please update the readme with specific comments on any areas that are unfinished and what you would cover given more time. + +Dev notes: + - Assumed that code logic was incomplete according to requirements details + - Took aprox 2h to complete (timeboxed according with requirements details) + - Tests not fully covered. Just some to show variety of examples + - Some implementations not completed since time was limited. Interfaces used for demonstration instead (ie: unit of work) + - Project structure kept the same for simplicity. By preference I usually go for physical partitons using the Clean Architecture design but also looking into Vertical Slice Architecture design for simplicity and higher cohesion From b23304856b4e54b53cc8e40f3040fa4673b9de21 Mon Sep 17 00:00:00 2001 From: Ricardo Ferreira Date: Thu, 1 Jun 2023 11:47:08 +0100 Subject: [PATCH 2/6] Updated CreateMailContainerDataStore as source and dest container db store seems to be read from options appsettings and not request and forgot to update code --- MailContainerTest.Tests/MailTransferService.cs | 6 +++--- .../Abstractions/IMailContainerDataStoreFactory.cs | 2 +- .../Factories/MailContainerDataStoreFactory.cs | 2 +- MailContainerTest/Services/MailTransferService.cs | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/MailContainerTest.Tests/MailTransferService.cs b/MailContainerTest.Tests/MailTransferService.cs index a95a25e..8df1057 100644 --- a/MailContainerTest.Tests/MailTransferService.cs +++ b/MailContainerTest.Tests/MailTransferService.cs @@ -19,7 +19,7 @@ public sealed class MailTransferServiceTests public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() { // Arrange - _mailContainerDataStoreFactory.CreateMailContainerDataStore(Arg.Any()) + _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) @@ -39,7 +39,7 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() { // Arrange - _mailContainerDataStoreFactory.CreateMailContainerDataStore(Arg.Any()) + _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) @@ -59,7 +59,7 @@ public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() { // Arrange - _mailContainerDataStoreFactory.CreateMailContainerDataStore(Arg.Any()) + _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) diff --git a/MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs b/MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs index df98d19..78c6914 100644 --- a/MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs +++ b/MailContainerTest/Abstractions/IMailContainerDataStoreFactory.cs @@ -4,5 +4,5 @@ namespace MailContainerTest.Abstractions; public interface IMailContainerDataStoreFactory { - IMailContainerDataStore CreateMailContainerDataStore(MailContainerNumber mailContainerNumber); + IMailContainerDataStore CreateMailContainerDataStore(); } \ No newline at end of file diff --git a/MailContainerTest/Factories/MailContainerDataStoreFactory.cs b/MailContainerTest/Factories/MailContainerDataStoreFactory.cs index 884633c..0b4d49e 100644 --- a/MailContainerTest/Factories/MailContainerDataStoreFactory.cs +++ b/MailContainerTest/Factories/MailContainerDataStoreFactory.cs @@ -15,7 +15,7 @@ public MailContainerDataStoreFactory(IOptionsSnapshot Date: Thu, 1 Jun 2023 14:39:41 +0100 Subject: [PATCH 3/6] Fixed unit tests running/execution since required by RoyalMail (was just pseudocode before). Also added some more tests for 100% coverage for the ones being tested. --- .../LargeLetterStrategyTests.cs | 44 +++++++++++++++++++ .../MailContainerTest.Tests.csproj | 11 +++++ .../MailTransferService.cs | 27 ++++++++++-- .../MailTransferStrategyFactoryTests.cs | 28 +++++++++++- .../Strategies/LargeLetterStrategy.cs | 2 +- .../Strategies/SmallParcelStrategy.cs | 2 +- .../Strategies/StandardLetterStrategy.cs | 2 +- 7 files changed, 109 insertions(+), 7 deletions(-) diff --git a/MailContainerTest.Tests/LargeLetterStrategyTests.cs b/MailContainerTest.Tests/LargeLetterStrategyTests.cs index 58d60ca..12de776 100644 --- a/MailContainerTest.Tests/LargeLetterStrategyTests.cs +++ b/MailContainerTest.Tests/LargeLetterStrategyTests.cs @@ -58,4 +58,48 @@ public void IsSuccess_ShouldReturnFalse_WhenFAllowedMailTypeNotEqual() // Assert result.Should().BeFalse(); } + + [Fact] + public void IsSuccess_ShouldReturnFalse_WhenOneOfMailContainerStatusIsNotOperational() + { + // Arrange + var strategy = new LargeLetterStrategy(); + var sourceContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter, + Status = MailContainerStatus.Operational + }; + var destContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter, + Status = MailContainerStatus.OutOfService + }; + var request = new MakeMailTransferRequest + { + MailType = MailType.LargeLetter + }; + + // Act + var result = strategy.IsSuccess(sourceContainer, destContainer, request); + + // Assert + result.Should().BeFalse(); + } + + [Fact] + public void IsSuccess_ShouldReturnFalse_WhenContainersNull() + { + // Arrange + var strategy = new LargeLetterStrategy(); + var request = new MakeMailTransferRequest + { + MailType = MailType.LargeLetter + }; + + // Act + var result = strategy.IsSuccess(null, null, request); + + // Assert + result.Should().BeFalse(); + } } \ No newline at end of file diff --git a/MailContainerTest.Tests/MailContainerTest.Tests.csproj b/MailContainerTest.Tests/MailContainerTest.Tests.csproj index 4e22f48..5c391bf 100644 --- a/MailContainerTest.Tests/MailContainerTest.Tests.csproj +++ b/MailContainerTest.Tests/MailContainerTest.Tests.csproj @@ -7,7 +7,18 @@ + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + diff --git a/MailContainerTest.Tests/MailTransferService.cs b/MailContainerTest.Tests/MailTransferService.cs index 8df1057..39fbcd7 100644 --- a/MailContainerTest.Tests/MailTransferService.cs +++ b/MailContainerTest.Tests/MailTransferService.cs @@ -19,6 +19,13 @@ public sealed class MailTransferServiceTests public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() { // Arrange + var request = new MakeMailTransferRequest + { + SourceMailContainerNumber = "1", + DestinationMailContainerNumber = "2", + MailType = MailType.LargeLetter, + NumberOfMailItems = 1 + }; _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) @@ -28,7 +35,7 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); // Act - var result = mailTransferService.MakeMailTransfer(Arg.Any()); + var result = mailTransferService.MakeMailTransfer(request); // Assert _unitOfWork.DidNotReceive().Commit(); @@ -39,6 +46,13 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() { // Arrange + var request = new MakeMailTransferRequest + { + SourceMailContainerNumber = "1", + DestinationMailContainerNumber = "2", + MailType = MailType.LargeLetter, + NumberOfMailItems = 1 + }; _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) @@ -48,7 +62,7 @@ public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); // Act - var result = mailTransferService.MakeMailTransfer(Arg.Any()); + var result = mailTransferService.MakeMailTransfer(request); // Assert _unitOfWork.Received(1).Commit(); @@ -59,6 +73,13 @@ public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() { // Arrange + var request = new MakeMailTransferRequest + { + SourceMailContainerNumber = "1", + DestinationMailContainerNumber = "2", + MailType = MailType.LargeLetter, + NumberOfMailItems = 1 + }; _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) @@ -70,7 +91,7 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() _unitOfWork.When(static x => x.Commit()).Throw(); // Act - var result = mailTransferService.MakeMailTransfer(Arg.Any()); + var result = mailTransferService.MakeMailTransfer(request); // Assert _unitOfWork.Received(1).Commit(); diff --git a/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs b/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs index ebebf69..e321d8b 100644 --- a/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs +++ b/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs @@ -9,6 +9,19 @@ namespace MailContainerTest.Tests; public sealed class MailTransferStrategyFactoryTests { + [Fact] + public void CreateMakeMailTransferStrategy_ShouldReturnStandardLetter_WhenMailTypeIsStandardLetter() + { + // Arrange + var factory = new MailTransferStrategyFactory(); + + // Act + var result = factory.CreateMakeMailTransferStrategy(MailType.StandardLetter); + + // Assert + result.Should().BeOfType(); + } + [Fact] public void CreateMakeMailTransferStrategy_ShouldReturnLargeLetterStrategy_WhenMailTypeIsLargeLetter() { @@ -21,6 +34,19 @@ public void CreateMakeMailTransferStrategy_ShouldReturnLargeLetterStrategy_WhenM // Assert result.Should().BeOfType(); } + + [Fact] + public void CreateMakeMailTransferStrategy_ShouldReturnSmallParcelStrategy_WhenMailTypeIsSmallParcel() + { + // Arrange + var factory = new MailTransferStrategyFactory(); + + // Act + var result = factory.CreateMakeMailTransferStrategy(MailType.SmallParcel); + + // Assert + result.Should().BeOfType(); + } [Fact] public void CreateMakeMailTransferStrategy_ShouldThrowArgumentOutOfRangeException_WhenMailTypeIsNotInEnumRange() @@ -32,6 +58,6 @@ public void CreateMakeMailTransferStrategy_ShouldThrowArgumentOutOfRangeExceptio Action result = () => factory.CreateMakeMailTransferStrategy((MailType) (-1)); // Assert - result.Should().Throw().WithMessage("Mail type is not in enum range"); + result.Should().Throw().WithMessage("Mail type is not in enum range*"); } } \ No newline at end of file diff --git a/MailContainerTest/Strategies/LargeLetterStrategy.cs b/MailContainerTest/Strategies/LargeLetterStrategy.cs index 2301e19..38e2812 100644 --- a/MailContainerTest/Strategies/LargeLetterStrategy.cs +++ b/MailContainerTest/Strategies/LargeLetterStrategy.cs @@ -12,7 +12,7 @@ public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContain return false; } - if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter) && !destContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter)) + if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter) || !destContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter)) { return false; } diff --git a/MailContainerTest/Strategies/SmallParcelStrategy.cs b/MailContainerTest/Strategies/SmallParcelStrategy.cs index 09dbc4a..1b3aa82 100644 --- a/MailContainerTest/Strategies/SmallParcelStrategy.cs +++ b/MailContainerTest/Strategies/SmallParcelStrategy.cs @@ -12,7 +12,7 @@ public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContain return false; } - if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel) && !destContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel)) + if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel) || !destContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel)) { return false; } diff --git a/MailContainerTest/Strategies/StandardLetterStrategy.cs b/MailContainerTest/Strategies/StandardLetterStrategy.cs index 083be75..909138f 100644 --- a/MailContainerTest/Strategies/StandardLetterStrategy.cs +++ b/MailContainerTest/Strategies/StandardLetterStrategy.cs @@ -12,7 +12,7 @@ public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContain return false; } - if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter) && !destContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter)) + if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter) || !destContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter)) { return false; } From de48ac38380602a330961f6c76998d02fbbb8641 Mon Sep 17 00:00:00 2001 From: Ricardo Ferreira Date: Mon, 5 Jun 2023 12:53:24 +0100 Subject: [PATCH 4/6] Updated MakeMailTransferRequest primitives. This (primitive obsession) solves a few things, like SRP and DRY as the object itself is now responsible for the preconditions validation (ie: null checks) --- MailContainerTest.Tests/LargeLetterStrategyTests.cs | 8 ++++---- MailContainerTest.Tests/MailTransferService.cs | 6 +++--- MailContainerTest/Abstractions/IMailContainerDataStore.cs | 2 +- MailContainerTest/Abstractions/IMailTransferStrategy.cs | 2 +- MailContainerTest/Data/BackupMailContainerDataStore.cs | 2 +- MailContainerTest/Data/MailContainerDataStore.cs | 2 +- MailContainerTest/Services/MailTransferService.cs | 2 +- MailContainerTest/Strategies/LargeLetterStrategy.cs | 2 +- MailContainerTest/Strategies/SmallParcelStrategy.cs | 2 +- MailContainerTest/Strategies/StandardLetterStrategy.cs | 2 +- MailContainerTest/Types/MakeMailTransferRequest.cs | 4 ++-- 11 files changed, 17 insertions(+), 17 deletions(-) diff --git a/MailContainerTest.Tests/LargeLetterStrategyTests.cs b/MailContainerTest.Tests/LargeLetterStrategyTests.cs index 12de776..d136bba 100644 --- a/MailContainerTest.Tests/LargeLetterStrategyTests.cs +++ b/MailContainerTest.Tests/LargeLetterStrategyTests.cs @@ -28,7 +28,7 @@ public void IsSuccess_ShouldReturnTrue_WhenAllowedMailTypeEqual() }; // Act - var result = strategy.IsSuccess(sourceContainer, destContainer, request); + var result = strategy.IsSuccess(sourceContainer, destContainer); // Assert result.Should().BeTrue(); @@ -53,7 +53,7 @@ public void IsSuccess_ShouldReturnFalse_WhenFAllowedMailTypeNotEqual() }; // Act - var result = strategy.IsSuccess(sourceContainer, destContainer, request); + var result = strategy.IsSuccess(sourceContainer, destContainer); // Assert result.Should().BeFalse(); @@ -80,7 +80,7 @@ public void IsSuccess_ShouldReturnFalse_WhenOneOfMailContainerStatusIsNotOperati }; // Act - var result = strategy.IsSuccess(sourceContainer, destContainer, request); + var result = strategy.IsSuccess(sourceContainer, destContainer); // Assert result.Should().BeFalse(); @@ -97,7 +97,7 @@ public void IsSuccess_ShouldReturnFalse_WhenContainersNull() }; // Act - var result = strategy.IsSuccess(null, null, request); + var result = strategy.IsSuccess(null, null); // Assert result.Should().BeFalse(); diff --git a/MailContainerTest.Tests/MailTransferService.cs b/MailContainerTest.Tests/MailTransferService.cs index 39fbcd7..4abbaeb 100644 --- a/MailContainerTest.Tests/MailTransferService.cs +++ b/MailContainerTest.Tests/MailTransferService.cs @@ -29,7 +29,7 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any()) .Returns(false); var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); @@ -56,7 +56,7 @@ public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any()) .Returns(true); var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); @@ -83,7 +83,7 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any()) .Returns(true); var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); diff --git a/MailContainerTest/Abstractions/IMailContainerDataStore.cs b/MailContainerTest/Abstractions/IMailContainerDataStore.cs index 8582a0d..0364a9c 100644 --- a/MailContainerTest/Abstractions/IMailContainerDataStore.cs +++ b/MailContainerTest/Abstractions/IMailContainerDataStore.cs @@ -4,7 +4,7 @@ namespace MailContainerTest.Abstractions; public interface IMailContainerDataStore { - MailContainer GetMailContainer(string mailContainerNumber); + MailContainer GetMailContainer(MailContainerNumber mailContainerNumber); void UpdateMailContainer(MailContainer mailContainer); } \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IMailTransferStrategy.cs b/MailContainerTest/Abstractions/IMailTransferStrategy.cs index c22bfaa..00e9191 100644 --- a/MailContainerTest/Abstractions/IMailTransferStrategy.cs +++ b/MailContainerTest/Abstractions/IMailTransferStrategy.cs @@ -4,5 +4,5 @@ namespace MailContainerTest.Abstractions; public interface IMailTransferStrategy { - bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request); + bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer); } \ No newline at end of file diff --git a/MailContainerTest/Data/BackupMailContainerDataStore.cs b/MailContainerTest/Data/BackupMailContainerDataStore.cs index 75c3364..a2eb878 100644 --- a/MailContainerTest/Data/BackupMailContainerDataStore.cs +++ b/MailContainerTest/Data/BackupMailContainerDataStore.cs @@ -7,7 +7,7 @@ public sealed class BackupMailContainerDataStore : IMailContainerDataStore { public static string DataStoreType => "Backup"; - public MailContainer GetMailContainer(string mailContainerNumber) + public MailContainer GetMailContainer(MailContainerNumber mailContainerNumber) { // Access the database and return the retrieved mail container. Implementation not required for this exercise. return new MailContainer(); diff --git a/MailContainerTest/Data/MailContainerDataStore.cs b/MailContainerTest/Data/MailContainerDataStore.cs index 175b31d..b8d6e6c 100644 --- a/MailContainerTest/Data/MailContainerDataStore.cs +++ b/MailContainerTest/Data/MailContainerDataStore.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Data { public sealed class MailContainerDataStore : IMailContainerDataStore { - public MailContainer GetMailContainer(string mailContainerNumber) + public MailContainer GetMailContainer(MailContainerNumber mailContainerNumber) { // Access the database and return the retrieved mail container. Implementation not required for this exercise. return new MailContainer(); diff --git a/MailContainerTest/Services/MailTransferService.cs b/MailContainerTest/Services/MailTransferService.cs index 258f8b9..aeca67b 100644 --- a/MailContainerTest/Services/MailTransferService.cs +++ b/MailContainerTest/Services/MailTransferService.cs @@ -35,7 +35,7 @@ public MakeMailTransferResult MakeMailTransfer(MakeMailTransferRequest request) var result = new MakeMailTransferResult { Success = _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(request.MailType) - .IsSuccess(sourceMailContainer, destMailContainer, request) + .IsSuccess(sourceMailContainer, destMailContainer) }; if (result.Success) diff --git a/MailContainerTest/Strategies/LargeLetterStrategy.cs b/MailContainerTest/Strategies/LargeLetterStrategy.cs index 38e2812..199630f 100644 --- a/MailContainerTest/Strategies/LargeLetterStrategy.cs +++ b/MailContainerTest/Strategies/LargeLetterStrategy.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Strategies; public sealed class LargeLetterStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer) { if (sourceContainer is null || destContainer is null) { diff --git a/MailContainerTest/Strategies/SmallParcelStrategy.cs b/MailContainerTest/Strategies/SmallParcelStrategy.cs index 1b3aa82..0f77da2 100644 --- a/MailContainerTest/Strategies/SmallParcelStrategy.cs +++ b/MailContainerTest/Strategies/SmallParcelStrategy.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Strategies; public sealed class SmallParcelStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer) { if (sourceContainer is null || destContainer is null) { diff --git a/MailContainerTest/Strategies/StandardLetterStrategy.cs b/MailContainerTest/Strategies/StandardLetterStrategy.cs index 909138f..f337f7e 100644 --- a/MailContainerTest/Strategies/StandardLetterStrategy.cs +++ b/MailContainerTest/Strategies/StandardLetterStrategy.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Strategies; public sealed class StandardLetterStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer) { if (sourceContainer is null || destContainer is null) { diff --git a/MailContainerTest/Types/MakeMailTransferRequest.cs b/MailContainerTest/Types/MakeMailTransferRequest.cs index aab1add..dfafc30 100644 --- a/MailContainerTest/Types/MakeMailTransferRequest.cs +++ b/MailContainerTest/Types/MakeMailTransferRequest.cs @@ -2,8 +2,8 @@ { public class MakeMailTransferRequest { - public string SourceMailContainerNumber { get; set; } - public string DestinationMailContainerNumber { get; set; } + public MailContainerNumber SourceMailContainerNumber { get; set; } + public MailContainerNumber DestinationMailContainerNumber { get; set; } public int NumberOfMailItems { get; set; } public DateTime TransferDate { get; set; } public MailType MailType { get; set; } From f56b65956494848ab5cfa5d644301933ef1c6485 Mon Sep 17 00:00:00 2001 From: Ricardo Ferreira Date: Wed, 7 Jun 2023 10:54:36 +0100 Subject: [PATCH 5/6] Added missing strategy check for NumberOfMailItems and Capacity with own value object and encapsulated its state (SRP) --- .../LargeLetterStrategyTests.cs | 8 +-- .../MailTransferService.cs | 54 +++++++++++++++---- .../Abstractions/IMailContainerDataStore.cs | 2 +- .../Abstractions/IMailTransferStrategy.cs | 2 +- .../Data/BackupMailContainerDataStore.cs | 2 +- .../Data/MailContainerDataStore.cs | 2 +- .../Services/MailTransferService.cs | 19 ++++--- .../Strategies/LargeLetterStrategy.cs | 7 ++- .../Strategies/SmallParcelStrategy.cs | 7 ++- .../Strategies/StandardLetterStrategy.cs | 7 ++- MailContainerTest/Types/MailContainer.cs | 35 +++++++++--- .../Types/MailContainerCapacity.cs | 20 +++++++ .../Types/MakeMailTransferRequest.cs | 12 ++--- 13 files changed, 135 insertions(+), 42 deletions(-) create mode 100644 MailContainerTest/Types/MailContainerCapacity.cs diff --git a/MailContainerTest.Tests/LargeLetterStrategyTests.cs b/MailContainerTest.Tests/LargeLetterStrategyTests.cs index d136bba..12de776 100644 --- a/MailContainerTest.Tests/LargeLetterStrategyTests.cs +++ b/MailContainerTest.Tests/LargeLetterStrategyTests.cs @@ -28,7 +28,7 @@ public void IsSuccess_ShouldReturnTrue_WhenAllowedMailTypeEqual() }; // Act - var result = strategy.IsSuccess(sourceContainer, destContainer); + var result = strategy.IsSuccess(sourceContainer, destContainer, request); // Assert result.Should().BeTrue(); @@ -53,7 +53,7 @@ public void IsSuccess_ShouldReturnFalse_WhenFAllowedMailTypeNotEqual() }; // Act - var result = strategy.IsSuccess(sourceContainer, destContainer); + var result = strategy.IsSuccess(sourceContainer, destContainer, request); // Assert result.Should().BeFalse(); @@ -80,7 +80,7 @@ public void IsSuccess_ShouldReturnFalse_WhenOneOfMailContainerStatusIsNotOperati }; // Act - var result = strategy.IsSuccess(sourceContainer, destContainer); + var result = strategy.IsSuccess(sourceContainer, destContainer, request); // Assert result.Should().BeFalse(); @@ -97,7 +97,7 @@ public void IsSuccess_ShouldReturnFalse_WhenContainersNull() }; // Act - var result = strategy.IsSuccess(null, null); + var result = strategy.IsSuccess(null, null, request); // Assert result.Should().BeFalse(); diff --git a/MailContainerTest.Tests/MailTransferService.cs b/MailContainerTest.Tests/MailTransferService.cs index 4abbaeb..8098a07 100644 --- a/MailContainerTest.Tests/MailTransferService.cs +++ b/MailContainerTest.Tests/MailTransferService.cs @@ -11,6 +11,7 @@ namespace MailContainerTest.Tests; public sealed class MailTransferServiceTests { private readonly IMailContainerDataStoreFactory _mailContainerDataStoreFactory = Substitute.For(); + private readonly IMailContainerDataStore _mailContainerDataStore = Substitute.For(); private readonly IMailTransferStrategyFactory _mailTransferStrategyFactory = Substitute.For(); private readonly IUnitOfWork _unitOfWork = Substitute.For(); private readonly ILoggerAdapter _loggerAdapter = Substitute.For>(); @@ -29,7 +30,7 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() _mailContainerDataStoreFactory.CreateMailContainerDataStore() .Returns(new MailContainerDataStore()); _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(false); var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); @@ -41,7 +42,7 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() _unitOfWork.DidNotReceive().Commit(); result.Success.Should().BeFalse(); } - + [Fact] public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() { @@ -53,10 +54,28 @@ public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() MailType = MailType.LargeLetter, NumberOfMailItems = 1 }; + var sourceContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter, + MailContainerNumber = "1", + Status = MailContainerStatus.Operational + }; + var destContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter, + MailContainerNumber = "2", + Status = MailContainerStatus.Operational + }; + sourceContainer.IncreaseCapacity(100); + destContainer.IncreaseCapacity(100); + _mailContainerDataStoreFactory.CreateMailContainerDataStore() - .Returns(new MailContainerDataStore()); + .Returns(_mailContainerDataStore); + _mailContainerDataStore.GetMailContainer(Arg.Any()) + .Returns(sourceContainer, destContainer); + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); @@ -68,7 +87,7 @@ public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() _unitOfWork.Received(1).Commit(); result.Success.Should().BeTrue(); } - + [Fact] public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() { @@ -80,16 +99,33 @@ public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() MailType = MailType.LargeLetter, NumberOfMailItems = 1 }; + var sourceContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter, + MailContainerNumber = "1", + Status = MailContainerStatus.Operational + }; + var destContainer = new MailContainer + { + AllowedMailType = AllowedMailType.LargeLetter, + MailContainerNumber = "2", + Status = MailContainerStatus.Operational + }; + sourceContainer.IncreaseCapacity(100); + destContainer.IncreaseCapacity(100); _mailContainerDataStoreFactory.CreateMailContainerDataStore() - .Returns(new MailContainerDataStore()); + .Returns(_mailContainerDataStore); + _mailContainerDataStore.GetMailContainer(Arg.Any()) + .Returns(sourceContainer, destContainer); + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any()) + .IsSuccess(sourceContainer, destContainer, Arg.Any()) .Returns(true); - + var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); _unitOfWork.When(static x => x.Commit()).Throw(); - + // Act var result = mailTransferService.MakeMailTransfer(request); diff --git a/MailContainerTest/Abstractions/IMailContainerDataStore.cs b/MailContainerTest/Abstractions/IMailContainerDataStore.cs index 0364a9c..af2ff73 100644 --- a/MailContainerTest/Abstractions/IMailContainerDataStore.cs +++ b/MailContainerTest/Abstractions/IMailContainerDataStore.cs @@ -4,7 +4,7 @@ namespace MailContainerTest.Abstractions; public interface IMailContainerDataStore { - MailContainer GetMailContainer(MailContainerNumber mailContainerNumber); + MailContainer GetMailContainer(in MailContainerNumber mailContainerNumber); void UpdateMailContainer(MailContainer mailContainer); } \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IMailTransferStrategy.cs b/MailContainerTest/Abstractions/IMailTransferStrategy.cs index 00e9191..c22bfaa 100644 --- a/MailContainerTest/Abstractions/IMailTransferStrategy.cs +++ b/MailContainerTest/Abstractions/IMailTransferStrategy.cs @@ -4,5 +4,5 @@ namespace MailContainerTest.Abstractions; public interface IMailTransferStrategy { - bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer); + bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request); } \ No newline at end of file diff --git a/MailContainerTest/Data/BackupMailContainerDataStore.cs b/MailContainerTest/Data/BackupMailContainerDataStore.cs index a2eb878..4cc69b6 100644 --- a/MailContainerTest/Data/BackupMailContainerDataStore.cs +++ b/MailContainerTest/Data/BackupMailContainerDataStore.cs @@ -7,7 +7,7 @@ public sealed class BackupMailContainerDataStore : IMailContainerDataStore { public static string DataStoreType => "Backup"; - public MailContainer GetMailContainer(MailContainerNumber mailContainerNumber) + public MailContainer GetMailContainer(in MailContainerNumber mailContainerNumber) { // Access the database and return the retrieved mail container. Implementation not required for this exercise. return new MailContainer(); diff --git a/MailContainerTest/Data/MailContainerDataStore.cs b/MailContainerTest/Data/MailContainerDataStore.cs index b8d6e6c..ff68aca 100644 --- a/MailContainerTest/Data/MailContainerDataStore.cs +++ b/MailContainerTest/Data/MailContainerDataStore.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Data { public sealed class MailContainerDataStore : IMailContainerDataStore { - public MailContainer GetMailContainer(MailContainerNumber mailContainerNumber) + public MailContainer GetMailContainer(in MailContainerNumber mailContainerNumber) { // Access the database and return the retrieved mail container. Implementation not required for this exercise. return new MailContainer(); diff --git a/MailContainerTest/Services/MailTransferService.cs b/MailContainerTest/Services/MailTransferService.cs index aeca67b..6b40938 100644 --- a/MailContainerTest/Services/MailTransferService.cs +++ b/MailContainerTest/Services/MailTransferService.cs @@ -26,27 +26,26 @@ public MailTransferService(IMailContainerDataStoreFactory mailContainerDataStore public MakeMailTransferResult MakeMailTransfer(MakeMailTransferRequest request) { - var sourceMailContainerDataStore = _mailContainerDataStoreFactory.CreateMailContainerDataStore(); - var sourceMailContainer = sourceMailContainerDataStore.GetMailContainer(request.SourceMailContainerNumber); - - var destMailContainerDataStore = _mailContainerDataStoreFactory.CreateMailContainerDataStore(); - var destMailContainer = destMailContainerDataStore.GetMailContainer(request.DestinationMailContainerNumber); + var containerDataStore = _mailContainerDataStoreFactory.CreateMailContainerDataStore(); + + var sourceMailContainer = containerDataStore.GetMailContainer(request.SourceMailContainerNumber); + var destMailContainer = containerDataStore.GetMailContainer(request.DestinationMailContainerNumber); var result = new MakeMailTransferResult { Success = _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(request.MailType) - .IsSuccess(sourceMailContainer, destMailContainer) + .IsSuccess(sourceMailContainer, destMailContainer, request) }; if (result.Success) { try { - sourceMailContainer.Capacity -= request.NumberOfMailItems; - destMailContainer.Capacity += request.NumberOfMailItems; + sourceMailContainer.DecreaseCapacity(request.NumberOfMailItems); + destMailContainer.IncreaseCapacity(request.NumberOfMailItems); - sourceMailContainerDataStore.UpdateMailContainer(sourceMailContainer); - destMailContainerDataStore.UpdateMailContainer(destMailContainer); + containerDataStore.UpdateMailContainer(sourceMailContainer); + containerDataStore.UpdateMailContainer(destMailContainer); _unitOfWork.Commit(); } diff --git a/MailContainerTest/Strategies/LargeLetterStrategy.cs b/MailContainerTest/Strategies/LargeLetterStrategy.cs index 199630f..28c5efc 100644 --- a/MailContainerTest/Strategies/LargeLetterStrategy.cs +++ b/MailContainerTest/Strategies/LargeLetterStrategy.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Strategies; public sealed class LargeLetterStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer) + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) { if (sourceContainer is null || destContainer is null) { @@ -21,6 +21,11 @@ public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContain { return false; } + + if (sourceContainer.Capacity < request.NumberOfMailItems) + { + return false; + } return true; } diff --git a/MailContainerTest/Strategies/SmallParcelStrategy.cs b/MailContainerTest/Strategies/SmallParcelStrategy.cs index 0f77da2..5f690ba 100644 --- a/MailContainerTest/Strategies/SmallParcelStrategy.cs +++ b/MailContainerTest/Strategies/SmallParcelStrategy.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Strategies; public sealed class SmallParcelStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer) + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) { if (sourceContainer is null || destContainer is null) { @@ -21,6 +21,11 @@ public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContain { return false; } + + if (sourceContainer.Capacity < request.NumberOfMailItems) + { + return false; + } return true; } diff --git a/MailContainerTest/Strategies/StandardLetterStrategy.cs b/MailContainerTest/Strategies/StandardLetterStrategy.cs index f337f7e..bd65e49 100644 --- a/MailContainerTest/Strategies/StandardLetterStrategy.cs +++ b/MailContainerTest/Strategies/StandardLetterStrategy.cs @@ -5,7 +5,7 @@ namespace MailContainerTest.Strategies; public sealed class StandardLetterStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer) + public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) { if (sourceContainer is null || destContainer is null) { @@ -22,6 +22,11 @@ public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContain return false; } + if (sourceContainer.Capacity < request.NumberOfMailItems) + { + return false; + } + return true; } } \ No newline at end of file diff --git a/MailContainerTest/Types/MailContainer.cs b/MailContainerTest/Types/MailContainer.cs index 873dbe0..d319231 100644 --- a/MailContainerTest/Types/MailContainer.cs +++ b/MailContainerTest/Types/MailContainer.cs @@ -1,11 +1,34 @@ namespace MailContainerTest.Types { - public class MailContainer + public sealed class MailContainer { - public string MailContainerNumber { get; set; } - public int Capacity { get; set; } - public MailContainerStatus Status { get; set; } - public AllowedMailType AllowedMailType { get; set; } + public MailContainerNumber MailContainerNumber { get; init; } + public MailContainerCapacity Capacity { get; private set; } + public MailContainerStatus Status { get; init; } + public AllowedMailType AllowedMailType { get; init; } + public void IncreaseCapacity(int numberOfMailItems) + { + if (numberOfMailItems < 0) + { + throw new ArgumentOutOfRangeException(nameof(numberOfMailItems), + numberOfMailItems, + $"{nameof(numberOfMailItems)} must be a positive number higher than 0."); + } + + Capacity += numberOfMailItems; + } + + public void DecreaseCapacity(int numberOfMailItems) + { + if (numberOfMailItems < 0) + { + throw new ArgumentOutOfRangeException(nameof(numberOfMailItems), + numberOfMailItems, + $"{nameof(numberOfMailItems)} must be a positive number higher than 0."); + } + + Capacity -= numberOfMailItems; + } } -} +} \ No newline at end of file diff --git a/MailContainerTest/Types/MailContainerCapacity.cs b/MailContainerTest/Types/MailContainerCapacity.cs new file mode 100644 index 0000000..b96d963 --- /dev/null +++ b/MailContainerTest/Types/MailContainerCapacity.cs @@ -0,0 +1,20 @@ +namespace MailContainerTest.Types; + +public readonly record struct MailContainerCapacity +{ + private int Value { get; } + + private MailContainerCapacity(int value) + { + if (value < 0) + { + throw new ArgumentOutOfRangeException(nameof(value), value, "Capacity can never be negative. (overflow)"); + } + + Value = value; + } + + public static implicit operator int(MailContainerCapacity mailContainerCapacity) => mailContainerCapacity.Value; + + public static implicit operator MailContainerCapacity(int value) => new(value); +} \ No newline at end of file diff --git a/MailContainerTest/Types/MakeMailTransferRequest.cs b/MailContainerTest/Types/MakeMailTransferRequest.cs index dfafc30..dc99496 100644 --- a/MailContainerTest/Types/MakeMailTransferRequest.cs +++ b/MailContainerTest/Types/MakeMailTransferRequest.cs @@ -1,11 +1,11 @@ namespace MailContainerTest.Types { - public class MakeMailTransferRequest + public record MakeMailTransferRequest { - public MailContainerNumber SourceMailContainerNumber { get; set; } - public MailContainerNumber DestinationMailContainerNumber { get; set; } - public int NumberOfMailItems { get; set; } - public DateTime TransferDate { get; set; } - public MailType MailType { get; set; } + public MailContainerNumber SourceMailContainerNumber { get; init; } + public MailContainerNumber DestinationMailContainerNumber { get; init; } + public int NumberOfMailItems { get; init; } + public DateTime TransferDate { get; init; } + public MailType MailType { get; init; } } } From e930f336b4c612e6bffa737fb1b415256e49760d Mon Sep 17 00:00:00 2001 From: Ricardo Ferreira Date: Wed, 14 Jun 2023 12:42:07 +0100 Subject: [PATCH 6/6] Just updating repo with cleaner code for future reference --- .../Builders/MailContainerBuilder.cs | 53 +++++ .../MakeMailTransferRequestBuilder.cs | 48 +++++ .../LargeLetterStrategyTests.cs | 193 ++++++++++++------ .../MailTransferService.cs | 138 ------------- .../MailTransferServiceTests.cs | 152 ++++++++++++++ .../MailTransferStrategyFactoryTests.cs | 26 ++- .../Abstractions/IAllowedMailTypeBehaviour.cs | 8 + .../Abstractions/ICapacityBehaviour.cs | 8 + .../Abstractions/IMailTransferStrategy.cs | 2 +- .../IOperationalStatusBehaviour.cs | 8 + .../Factories/MailTransferStrategyFactory.cs | 26 ++- .../Services/MailTransferService.cs | 49 +++-- .../Behaviours/AllowedMailTypeBehaviour.cs | 12 ++ .../Behaviours/CapacityBehaviour.cs | 12 ++ .../Behaviours/OperationalStatusBehaviour.cs | 12 ++ .../Strategies/LargeLetterStrategy.cs | 44 +++- .../Strategies/SmallParcelStrategy.cs | 28 +-- .../Strategies/StandardLetterStrategy.cs | 26 +-- .../Types/MakeMailTransferRequest.cs | 2 +- 19 files changed, 560 insertions(+), 287 deletions(-) create mode 100644 MailContainerTest.Tests/Builders/MailContainerBuilder.cs create mode 100644 MailContainerTest.Tests/Builders/MakeMailTransferRequestBuilder.cs delete mode 100644 MailContainerTest.Tests/MailTransferService.cs create mode 100644 MailContainerTest.Tests/MailTransferServiceTests.cs create mode 100644 MailContainerTest/Abstractions/IAllowedMailTypeBehaviour.cs create mode 100644 MailContainerTest/Abstractions/ICapacityBehaviour.cs create mode 100644 MailContainerTest/Abstractions/IOperationalStatusBehaviour.cs create mode 100644 MailContainerTest/Strategies/Behaviours/AllowedMailTypeBehaviour.cs create mode 100644 MailContainerTest/Strategies/Behaviours/CapacityBehaviour.cs create mode 100644 MailContainerTest/Strategies/Behaviours/OperationalStatusBehaviour.cs diff --git a/MailContainerTest.Tests/Builders/MailContainerBuilder.cs b/MailContainerTest.Tests/Builders/MailContainerBuilder.cs new file mode 100644 index 0000000..1602a48 --- /dev/null +++ b/MailContainerTest.Tests/Builders/MailContainerBuilder.cs @@ -0,0 +1,53 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Tests.Builders; + +public sealed class MailContainerBuilder +{ + private MailContainerNumber _mailContainerNumber; + private MailContainerCapacity _capacity; + private MailContainerStatus _status; + private AllowedMailType _allowedMailType; + + public MailContainerBuilder WithMailContainerNumber(MailContainerNumber mailContainerNumber) + { + _mailContainerNumber = mailContainerNumber; + + return this; + } + + public MailContainerBuilder WithCapacity(MailContainerCapacity capacity) + { + _capacity = capacity; + + return this; + } + + public MailContainerBuilder WithStatus(MailContainerStatus status) + { + _status = status; + + return this; + } + + public MailContainerBuilder WithAllowedMailType(AllowedMailType allowedMailType) + { + _allowedMailType = allowedMailType; + + return this; + } + + public MailContainer Build() + { + var mailContainer = new MailContainer + { + MailContainerNumber = _mailContainerNumber, + Status = _status, + AllowedMailType = _allowedMailType + }; + + mailContainer.IncreaseCapacity(_capacity); + + return mailContainer; + } +} \ No newline at end of file diff --git a/MailContainerTest.Tests/Builders/MakeMailTransferRequestBuilder.cs b/MailContainerTest.Tests/Builders/MakeMailTransferRequestBuilder.cs new file mode 100644 index 0000000..545ee60 --- /dev/null +++ b/MailContainerTest.Tests/Builders/MakeMailTransferRequestBuilder.cs @@ -0,0 +1,48 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Tests.Builders; + +public sealed class MakeMailTransferRequestBuilder +{ + private MakeMailTransferRequest _makeMailTransferRequest = new(); + + public MakeMailTransferRequestBuilder WithSourceMailContainerNumber(MailContainerNumber sourceMailContainerNumber) + { + _makeMailTransferRequest = _makeMailTransferRequest with { SourceMailContainerNumber = sourceMailContainerNumber }; + + return this; + } + + public MakeMailTransferRequestBuilder WithDestinationMailContainerNumber(MailContainerNumber destinationMailContainerNumber) + { + _makeMailTransferRequest = _makeMailTransferRequest with { DestinationMailContainerNumber = destinationMailContainerNumber }; + + return this; + } + + public MakeMailTransferRequestBuilder WithNumberOfMailItems(int numberOfMailItems) + { + _makeMailTransferRequest = _makeMailTransferRequest with { NumberOfMailItems = numberOfMailItems }; + + return this; + } + + public MakeMailTransferRequestBuilder WithTransferDate(DateTime transferDate) + { + _makeMailTransferRequest = _makeMailTransferRequest with { TransferDate = transferDate }; + + return this; + } + + public MakeMailTransferRequestBuilder WithMailType(MailType mailType) + { + _makeMailTransferRequest = _makeMailTransferRequest with { MailType = mailType }; + + return this; + } + + public MakeMailTransferRequest Build() + { + return _makeMailTransferRequest; + } +} \ No newline at end of file diff --git a/MailContainerTest.Tests/LargeLetterStrategyTests.cs b/MailContainerTest.Tests/LargeLetterStrategyTests.cs index 12de776..18fa3b3 100644 --- a/MailContainerTest.Tests/LargeLetterStrategyTests.cs +++ b/MailContainerTest.Tests/LargeLetterStrategyTests.cs @@ -1,5 +1,7 @@ using FluentAssertions; +using MailContainerTest.Abstractions; using MailContainerTest.Strategies; +using MailContainerTest.Tests.Builders; using MailContainerTest.Types; using NSubstitute; using NSubstitute.ReceivedExtensions; @@ -9,97 +11,154 @@ namespace MailContainerTest.Tests; public sealed class LargeLetterStrategyTests { - [Fact] - public void IsSuccess_ShouldReturnTrue_WhenAllowedMailTypeEqual() - { - // Arrange - var strategy = new LargeLetterStrategy(); - var sourceContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter - }; - var destContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter - }; - var request = new MakeMailTransferRequest - { - MailType = MailType.LargeLetter - }; + private readonly IAllowedMailTypeBehaviour _allowedMailTypeBehaviour = Substitute.For(); + private readonly IOperationalStatusBehaviour _operationalStatusBehaviour = Substitute.For(); + private readonly ICapacityBehaviour _capacityBehaviour = Substitute.For(); - // Act - var result = strategy.IsSuccess(sourceContainer, destContainer, request); + private readonly LargeLetterStrategy _sut; - // Assert - result.Should().BeTrue(); + public LargeLetterStrategyTests() + { + _sut = new LargeLetterStrategy(_allowedMailTypeBehaviour, _operationalStatusBehaviour, _capacityBehaviour); } - + [Fact] - public void IsSuccess_ShouldReturnFalse_WhenFAllowedMailTypeNotEqual() + public void IsSuccess_ShouldReturnTrue_WhenAllowedMailTypeIsEqualAndOperationalAndWithinCapacity() { // Arrange - var strategy = new LargeLetterStrategy(); - var sourceContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter - }; - var destContainer = new MailContainer - { - AllowedMailType = AllowedMailType.SmallParcel - }; - var request = new MakeMailTransferRequest - { - MailType = MailType.LargeLetter - }; + var sourceContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .Build(); + + var destContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.SmallParcel) + .Build(); + + var request = new MakeMailTransferRequestBuilder() + .WithMailType(MailType.LargeLetter) + .Build(); + + _allowedMailTypeBehaviour.IsAllowedMailType(Arg.Any(), Arg.Any()).Returns(true); + _operationalStatusBehaviour.IsOperationalStatus(Arg.Any(), Arg.Any()).Returns(true); + _capacityBehaviour.IsWithinCapacity(Arg.Any(), Arg.Any()).Returns(true); // Act - var result = strategy.IsSuccess(sourceContainer, destContainer, request); + var result = _sut.IsSuccess(sourceContainer, destContainer, request); // Assert - result.Should().BeFalse(); + result.Should().BeTrue(); } - + [Fact] - public void IsSuccess_ShouldReturnFalse_WhenOneOfMailContainerStatusIsNotOperational() + public void IsSuccess_ShouldReturnFalse_WhenAllowedMailTypeNotEqual() { // Arrange - var strategy = new LargeLetterStrategy(); - var sourceContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter, - Status = MailContainerStatus.Operational - }; - var destContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter, - Status = MailContainerStatus.OutOfService - }; - var request = new MakeMailTransferRequest - { - MailType = MailType.LargeLetter - }; - + var sourceContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .Build(); + + var destContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.SmallParcel) + .Build(); + + var request = new MakeMailTransferRequestBuilder() + .WithMailType(MailType.LargeLetter) + .Build(); + // Act - var result = strategy.IsSuccess(sourceContainer, destContainer, request); - + var result = _sut.IsSuccess(sourceContainer, destContainer, request); + // Assert result.Should().BeFalse(); } [Fact] - public void IsSuccess_ShouldReturnFalse_WhenContainersNull() + public void IsSuccess_ShouldReturnFalse_WhenOneOfMailContainerStatusIsNotOperational() { // Arrange - var strategy = new LargeLetterStrategy(); - var request = new MakeMailTransferRequest - { - MailType = MailType.LargeLetter - }; - + var sourceContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .WithStatus(MailContainerStatus.Operational) + .Build(); + + var destContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .WithStatus(MailContainerStatus.OutOfService) + .Build(); + + var request = new MakeMailTransferRequestBuilder() + .WithMailType(MailType.LargeLetter) + .Build(); + // Act - var result = strategy.IsSuccess(null, null, request); - + var result = _sut.IsSuccess(sourceContainer, destContainer, request); + // Assert result.Should().BeFalse(); } + + // [Theory] + // [MemberData(nameof(FailingTestData))] + // public void IsSuccess_ShouldReturnFalse_WhenAllowedMailTypeIsNotEqualAndNotOperationalAndNotWithinCapacity( + // MailContainer sourceContainer, + // MailContainer destContainer, + // MakeMailTransferRequest request) + // { + // // Arrange + // + // + // // Act + // var result = _sut.IsSuccess(sourceContainer, destContainer, request); + // + // // Assert + // result.Should().BeFalse(); + // } + // + // public static IEnumerable FailingTestData => + // new List + // { + // new object[] + // { + // new MailContainerBuilder() + // .WithAllowedMailType(AllowedMailType.LargeLetter) + // .Build(), + // new MailContainerBuilder() + // .WithAllowedMailType(AllowedMailType.SmallParcel) + // .Build(), + // new MakeMailTransferRequestBuilder() + // .WithMailType(MailType.LargeLetter) + // .Build() + // }, + // new object[] + // { + // new MailContainerBuilder() + // .WithAllowedMailType(AllowedMailType.LargeLetter) + // .WithStatus(MailContainerStatus.Operational) + // .Build(), + // new MailContainerBuilder() + // .WithAllowedMailType(AllowedMailType.LargeLetter) + // .WithStatus(MailContainerStatus.OutOfService) + // .Build(), + // new MakeMailTransferRequestBuilder() + // .WithMailType(MailType.LargeLetter) + // .Build() + // }, + // new object[] + // { + // new MailContainerBuilder() + // .WithAllowedMailType(AllowedMailType.LargeLetter) + // .WithStatus(MailContainerStatus.Operational) + // .WithCapacity(100) + // .Build(), + // new MailContainerBuilder() + // .WithAllowedMailType(AllowedMailType.LargeLetter) + // .WithStatus(MailContainerStatus.Operational) + // .WithCapacity(100) + // .Build(), + // new MakeMailTransferRequestBuilder() + // .WithMailType(MailType.LargeLetter) + // .WithNumberOfMailItems(200) + // .Build() + // } + // }; } \ No newline at end of file diff --git a/MailContainerTest.Tests/MailTransferService.cs b/MailContainerTest.Tests/MailTransferService.cs deleted file mode 100644 index 8098a07..0000000 --- a/MailContainerTest.Tests/MailTransferService.cs +++ /dev/null @@ -1,138 +0,0 @@ -using FluentAssertions; -using MailContainerTest.Abstractions; -using MailContainerTest.Data; -using MailContainerTest.Services; -using MailContainerTest.Types; -using NSubstitute; -using Xunit; - -namespace MailContainerTest.Tests; - -public sealed class MailTransferServiceTests -{ - private readonly IMailContainerDataStoreFactory _mailContainerDataStoreFactory = Substitute.For(); - private readonly IMailContainerDataStore _mailContainerDataStore = Substitute.For(); - private readonly IMailTransferStrategyFactory _mailTransferStrategyFactory = Substitute.For(); - private readonly IUnitOfWork _unitOfWork = Substitute.For(); - private readonly ILoggerAdapter _loggerAdapter = Substitute.For>(); - - [Fact] - public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() - { - // Arrange - var request = new MakeMailTransferRequest - { - SourceMailContainerNumber = "1", - DestinationMailContainerNumber = "2", - MailType = MailType.LargeLetter, - NumberOfMailItems = 1 - }; - _mailContainerDataStoreFactory.CreateMailContainerDataStore() - .Returns(new MailContainerDataStore()); - _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(false); - - var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); - - // Act - var result = mailTransferService.MakeMailTransfer(request); - - // Assert - _unitOfWork.DidNotReceive().Commit(); - result.Success.Should().BeFalse(); - } - - [Fact] - public void MakeMailTransfer_ShouldReturnTrue_WhenStrategyNotSuccessful() - { - // Arrange - var request = new MakeMailTransferRequest - { - SourceMailContainerNumber = "1", - DestinationMailContainerNumber = "2", - MailType = MailType.LargeLetter, - NumberOfMailItems = 1 - }; - var sourceContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter, - MailContainerNumber = "1", - Status = MailContainerStatus.Operational - }; - var destContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter, - MailContainerNumber = "2", - Status = MailContainerStatus.Operational - }; - sourceContainer.IncreaseCapacity(100); - destContainer.IncreaseCapacity(100); - - _mailContainerDataStoreFactory.CreateMailContainerDataStore() - .Returns(_mailContainerDataStore); - _mailContainerDataStore.GetMailContainer(Arg.Any()) - .Returns(sourceContainer, destContainer); - - _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(true); - - var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); - - // Act - var result = mailTransferService.MakeMailTransfer(request); - - // Assert - _unitOfWork.Received(1).Commit(); - result.Success.Should().BeTrue(); - } - - [Fact] - public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() - { - // Arrange - var request = new MakeMailTransferRequest - { - SourceMailContainerNumber = "1", - DestinationMailContainerNumber = "2", - MailType = MailType.LargeLetter, - NumberOfMailItems = 1 - }; - var sourceContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter, - MailContainerNumber = "1", - Status = MailContainerStatus.Operational - }; - var destContainer = new MailContainer - { - AllowedMailType = AllowedMailType.LargeLetter, - MailContainerNumber = "2", - Status = MailContainerStatus.Operational - }; - sourceContainer.IncreaseCapacity(100); - destContainer.IncreaseCapacity(100); - _mailContainerDataStoreFactory.CreateMailContainerDataStore() - .Returns(_mailContainerDataStore); - _mailContainerDataStore.GetMailContainer(Arg.Any()) - .Returns(sourceContainer, destContainer); - - _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) - .IsSuccess(sourceContainer, destContainer, Arg.Any()) - .Returns(true); - - var mailTransferService = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); - - _unitOfWork.When(static x => x.Commit()).Throw(); - - // Act - var result = mailTransferService.MakeMailTransfer(request); - - // Assert - _unitOfWork.Received(1).Commit(); - _unitOfWork.Received(1).Rollback(); - _loggerAdapter.Received(1).LogError(Arg.Any(), "Error saving changes to containers"); - result.Success.Should().BeFalse(); - } -} \ No newline at end of file diff --git a/MailContainerTest.Tests/MailTransferServiceTests.cs b/MailContainerTest.Tests/MailTransferServiceTests.cs new file mode 100644 index 0000000..5ba9ae5 --- /dev/null +++ b/MailContainerTest.Tests/MailTransferServiceTests.cs @@ -0,0 +1,152 @@ +using FluentAssertions; +using FluentAssertions.Execution; +using MailContainerTest.Abstractions; +using MailContainerTest.Data; +using MailContainerTest.Services; +using MailContainerTest.Tests.Builders; +using MailContainerTest.Types; +using NSubstitute; +using Xunit; + +namespace MailContainerTest.Tests; + +public sealed class MailTransferServiceTests +{ + private readonly IMailContainerDataStoreFactory _mailContainerDataStoreFactory = Substitute.For(); + private readonly IMailContainerDataStore _mailContainerDataStore = Substitute.For(); + private readonly IMailTransferStrategyFactory _mailTransferStrategyFactory = Substitute.For(); + private readonly IUnitOfWork _unitOfWork = Substitute.For(); + private readonly ILoggerAdapter _loggerAdapter = Substitute.For>(); + + private readonly MailTransferService _sut; + + public MailTransferServiceTests() + { + _sut = new MailTransferService(_mailContainerDataStoreFactory, _mailTransferStrategyFactory, _unitOfWork, _loggerAdapter); + } + + [Fact] + public void MakeMailTransfer_ShouldReturnFalse_WhenStrategyNotSuccessful() + { + // Arrange + var request = new MakeMailTransferRequestBuilder() + .WithSourceMailContainerNumber("1") + .WithDestinationMailContainerNumber("2") + .WithMailType(MailType.LargeLetter) + .WithNumberOfMailItems(1) + .Build(); + + _mailContainerDataStoreFactory.CreateMailContainerDataStore() + .Returns(new MailContainerDataStore()); + + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(false); + + // Act + var result = _sut.MakeMailTransfer(request); + + // Assert + using (new AssertionScope()) + { + _unitOfWork.DidNotReceive().Commit(); + result.Success.Should().BeFalse(); + } + } + + [Fact] + public void MakeMailTransfer_ShouldReturnTrue_WhenStrategySuccessful() + { + // Arrange + var request = new MakeMailTransferRequestBuilder() + .WithSourceMailContainerNumber("1") + .WithDestinationMailContainerNumber("2") + .WithMailType(MailType.LargeLetter) + .WithNumberOfMailItems(1) + .Build(); + + var sourceContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .WithMailContainerNumber("1") + .WithStatus(MailContainerStatus.Operational) + .WithCapacity(100) + .Build(); + + var destContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .WithMailContainerNumber("2") + .WithStatus(MailContainerStatus.Operational) + .WithCapacity(100) + .Build(); + + _mailContainerDataStoreFactory.CreateMailContainerDataStore() + .Returns(_mailContainerDataStore); + + _mailContainerDataStore.GetMailContainer(Arg.Any()) + .Returns(sourceContainer, destContainer); + + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) + .IsSuccess(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + // Act + var result = _sut.MakeMailTransfer(request); + + // Assert + using (new AssertionScope()) + { + _unitOfWork.Received(1).Commit(); + result.Success.Should().BeTrue(); + } + } + + [Fact] + public void MakeMailTransfer_ShouldReturnFalse_WhenCommitNotSuccessful() + { + // Arrange + var request = new MakeMailTransferRequestBuilder() + .WithSourceMailContainerNumber("1") + .WithDestinationMailContainerNumber("2") + .WithMailType(MailType.LargeLetter) + .WithNumberOfMailItems(1) + .Build(); + + var sourceContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .WithMailContainerNumber("1") + .WithStatus(MailContainerStatus.Operational) + .WithCapacity(100) + .Build(); + + var destContainer = new MailContainerBuilder() + .WithAllowedMailType(AllowedMailType.LargeLetter) + .WithMailContainerNumber("2") + .WithStatus(MailContainerStatus.Operational) + .WithCapacity(100) + .Build(); + + _mailContainerDataStoreFactory.CreateMailContainerDataStore() + .Returns(_mailContainerDataStore); + + _mailContainerDataStore.GetMailContainer(Arg.Any()) + .Returns(sourceContainer, destContainer); + + _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(Arg.Any()) + .IsSuccess(Arg.Is(sourceContainer), Arg.Is(destContainer), Arg.Any()) + .Returns(true); + + _unitOfWork.When(static x => x.Commit()).Throw(); + + // Act + var result = _sut.MakeMailTransfer(request); + + // Assert + using (new AssertionScope()) + { + _unitOfWork.Received(1).Commit(); + _unitOfWork.Received(1).Rollback(); + _loggerAdapter.Received(1).LogError(Arg.Any(), "Error saving changes to containers"); + result.Success.Should().BeFalse(); + } + } +} \ No newline at end of file diff --git a/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs b/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs index e321d8b..5cbc98f 100644 --- a/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs +++ b/MailContainerTest.Tests/MailTransferStrategyFactoryTests.cs @@ -1,48 +1,53 @@ using FluentAssertions; +using MailContainerTest.Abstractions; using MailContainerTest.Factories; using MailContainerTest.Strategies; +using MailContainerTest.Strategies.Behaviours; using MailContainerTest.Types; -using NSubstitute.ExceptionExtensions; using Xunit; namespace MailContainerTest.Tests; public sealed class MailTransferStrategyFactoryTests { + private readonly MailTransferStrategyFactory _sut; + + public MailTransferStrategyFactoryTests() + { + _sut = new MailTransferStrategyFactory(new AllowedMailTypeBehaviour(), new OperationalStatusBehaviour(), new CapacityBehaviour()); + } + [Fact] public void CreateMakeMailTransferStrategy_ShouldReturnStandardLetter_WhenMailTypeIsStandardLetter() { // Arrange - var factory = new MailTransferStrategyFactory(); // Act - var result = factory.CreateMakeMailTransferStrategy(MailType.StandardLetter); + var result = _sut.CreateMakeMailTransferStrategy(MailType.StandardLetter); // Assert result.Should().BeOfType(); } - + [Fact] public void CreateMakeMailTransferStrategy_ShouldReturnLargeLetterStrategy_WhenMailTypeIsLargeLetter() { // Arrange - var factory = new MailTransferStrategyFactory(); // Act - var result = factory.CreateMakeMailTransferStrategy(MailType.LargeLetter); + var result = _sut.CreateMakeMailTransferStrategy(MailType.LargeLetter); // Assert result.Should().BeOfType(); } - + [Fact] public void CreateMakeMailTransferStrategy_ShouldReturnSmallParcelStrategy_WhenMailTypeIsSmallParcel() { // Arrange - var factory = new MailTransferStrategyFactory(); // Act - var result = factory.CreateMakeMailTransferStrategy(MailType.SmallParcel); + var result = _sut.CreateMakeMailTransferStrategy(MailType.SmallParcel); // Assert result.Should().BeOfType(); @@ -52,10 +57,9 @@ public void CreateMakeMailTransferStrategy_ShouldReturnSmallParcelStrategy_WhenM public void CreateMakeMailTransferStrategy_ShouldThrowArgumentOutOfRangeException_WhenMailTypeIsNotInEnumRange() { // Arrange - var factory = new MailTransferStrategyFactory(); // Act - Action result = () => factory.CreateMakeMailTransferStrategy((MailType) (-1)); + Action result = () => _sut.CreateMakeMailTransferStrategy((MailType) (-1)); // Assert result.Should().Throw().WithMessage("Mail type is not in enum range*"); diff --git a/MailContainerTest/Abstractions/IAllowedMailTypeBehaviour.cs b/MailContainerTest/Abstractions/IAllowedMailTypeBehaviour.cs new file mode 100644 index 0000000..d67f16a --- /dev/null +++ b/MailContainerTest/Abstractions/IAllowedMailTypeBehaviour.cs @@ -0,0 +1,8 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Abstractions; + +public interface IAllowedMailTypeBehaviour +{ + public bool IsAllowedMailType(MailContainer sourceContainer, MailContainer destContainer); +} \ No newline at end of file diff --git a/MailContainerTest/Abstractions/ICapacityBehaviour.cs b/MailContainerTest/Abstractions/ICapacityBehaviour.cs new file mode 100644 index 0000000..f37dbea --- /dev/null +++ b/MailContainerTest/Abstractions/ICapacityBehaviour.cs @@ -0,0 +1,8 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Abstractions; + +public interface ICapacityBehaviour +{ + bool IsWithinCapacity(MailContainer sourceContainer, MakeMailTransferRequest request); +} \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IMailTransferStrategy.cs b/MailContainerTest/Abstractions/IMailTransferStrategy.cs index c22bfaa..eb357f2 100644 --- a/MailContainerTest/Abstractions/IMailTransferStrategy.cs +++ b/MailContainerTest/Abstractions/IMailTransferStrategy.cs @@ -4,5 +4,5 @@ namespace MailContainerTest.Abstractions; public interface IMailTransferStrategy { - bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request); + bool IsSuccess(MailContainer sourceContainer, MailContainer destContainer, MakeMailTransferRequest request); } \ No newline at end of file diff --git a/MailContainerTest/Abstractions/IOperationalStatusBehaviour.cs b/MailContainerTest/Abstractions/IOperationalStatusBehaviour.cs new file mode 100644 index 0000000..d664858 --- /dev/null +++ b/MailContainerTest/Abstractions/IOperationalStatusBehaviour.cs @@ -0,0 +1,8 @@ +using MailContainerTest.Types; + +namespace MailContainerTest.Abstractions; + +public interface IOperationalStatusBehaviour +{ + bool IsOperationalStatus(MailContainer sourceContainer, MailContainer destContainer); +} \ No newline at end of file diff --git a/MailContainerTest/Factories/MailTransferStrategyFactory.cs b/MailContainerTest/Factories/MailTransferStrategyFactory.cs index 78b2e71..100da14 100644 --- a/MailContainerTest/Factories/MailTransferStrategyFactory.cs +++ b/MailContainerTest/Factories/MailTransferStrategyFactory.cs @@ -6,13 +6,27 @@ namespace MailContainerTest.Factories; public sealed class MailTransferStrategyFactory : IMailTransferStrategyFactory { + private readonly IAllowedMailTypeBehaviour _allowedMailTypeBehaviour; + private readonly IOperationalStatusBehaviour _operationalStatusBehaviour; + private readonly ICapacityBehaviour _capacityBehaviour; + + public MailTransferStrategyFactory(IAllowedMailTypeBehaviour allowedMailTypeBehaviour, + IOperationalStatusBehaviour operationalStatusBehaviour, + ICapacityBehaviour capacityBehaviour) + { + _allowedMailTypeBehaviour = allowedMailTypeBehaviour; + _operationalStatusBehaviour = operationalStatusBehaviour; + _capacityBehaviour = capacityBehaviour; + } + public IMailTransferStrategy CreateMakeMailTransferStrategy(MailType mailType) { - return mailType switch { - MailType.StandardLetter => new StandardLetterStrategy(), - MailType.LargeLetter => new LargeLetterStrategy(), - MailType.SmallParcel => new SmallParcelStrategy(), - _ => throw new ArgumentOutOfRangeException(nameof(mailType), mailType, "Mail type is not in enum range") - }; + return mailType switch + { + MailType.StandardLetter => new StandardLetterStrategy(_allowedMailTypeBehaviour), + MailType.LargeLetter => new LargeLetterStrategy(_allowedMailTypeBehaviour, _operationalStatusBehaviour, _capacityBehaviour), + MailType.SmallParcel => new SmallParcelStrategy(_allowedMailTypeBehaviour, _operationalStatusBehaviour), + _ => throw new ArgumentOutOfRangeException(nameof(mailType), mailType, "Mail type is not in enum range") + }; } } \ No newline at end of file diff --git a/MailContainerTest/Services/MailTransferService.cs b/MailContainerTest/Services/MailTransferService.cs index 6b40938..460314b 100644 --- a/MailContainerTest/Services/MailTransferService.cs +++ b/MailContainerTest/Services/MailTransferService.cs @@ -27,39 +27,44 @@ public MailTransferService(IMailContainerDataStoreFactory mailContainerDataStore public MakeMailTransferResult MakeMailTransfer(MakeMailTransferRequest request) { var containerDataStore = _mailContainerDataStoreFactory.CreateMailContainerDataStore(); - + var sourceMailContainer = containerDataStore.GetMailContainer(request.SourceMailContainerNumber); var destMailContainer = containerDataStore.GetMailContainer(request.DestinationMailContainerNumber); - var result = new MakeMailTransferResult - { - Success = _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(request.MailType) - .IsSuccess(sourceMailContainer, destMailContainer, request) - }; + var mailTransfer = new MakeMailTransferResult + { + Success = _mailTransferStrategyFactory.CreateMakeMailTransferStrategy(request.MailType) + .IsSuccess(sourceMailContainer, destMailContainer, request) + }; - if (result.Success) + if (mailTransfer.Success) { - try - { - sourceMailContainer.DecreaseCapacity(request.NumberOfMailItems); - destMailContainer.IncreaseCapacity(request.NumberOfMailItems); + ApplyMailContainerChanges(request, sourceMailContainer, destMailContainer, containerDataStore, mailTransfer); + } - containerDataStore.UpdateMailContainer(sourceMailContainer); - containerDataStore.UpdateMailContainer(destMailContainer); + return mailTransfer; + } - _unitOfWork.Commit(); - } - catch (Exception ex) - { - result.Success = false; + private void ApplyMailContainerChanges(MakeMailTransferRequest request, MailContainer sourceMailContainer, MailContainer destMailContainer, IMailContainerDataStore containerDataStore, MakeMailTransferResult result) + { + try + { + sourceMailContainer.DecreaseCapacity(request.NumberOfMailItems); + destMailContainer.IncreaseCapacity(request.NumberOfMailItems); - _unitOfWork.Rollback(); + containerDataStore.UpdateMailContainer(sourceMailContainer); + containerDataStore.UpdateMailContainer(destMailContainer); - _loggerAdapter.LogError(ex, "Error saving changes to containers"); - } + _unitOfWork.Commit(); } + catch (Exception ex) + { + result.Success = false; - return result; + _unitOfWork.Rollback(); + + _loggerAdapter.LogError(ex, "Error saving changes to containers"); + } } } } \ No newline at end of file diff --git a/MailContainerTest/Strategies/Behaviours/AllowedMailTypeBehaviour.cs b/MailContainerTest/Strategies/Behaviours/AllowedMailTypeBehaviour.cs new file mode 100644 index 0000000..9fd905e --- /dev/null +++ b/MailContainerTest/Strategies/Behaviours/AllowedMailTypeBehaviour.cs @@ -0,0 +1,12 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Types; + +namespace MailContainerTest.Strategies.Behaviours; + +public sealed class AllowedMailTypeBehaviour : IAllowedMailTypeBehaviour +{ + public bool IsAllowedMailType(MailContainer sourceContainer, MailContainer destContainer) + { + return sourceContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter) && destContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter); + } +} \ No newline at end of file diff --git a/MailContainerTest/Strategies/Behaviours/CapacityBehaviour.cs b/MailContainerTest/Strategies/Behaviours/CapacityBehaviour.cs new file mode 100644 index 0000000..ba14350 --- /dev/null +++ b/MailContainerTest/Strategies/Behaviours/CapacityBehaviour.cs @@ -0,0 +1,12 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Types; + +namespace MailContainerTest.Strategies.Behaviours; + +public sealed class CapacityBehaviour : ICapacityBehaviour +{ + public bool IsWithinCapacity(MailContainer sourceContainer, MakeMailTransferRequest request) + { + return sourceContainer.Capacity < request.NumberOfMailItems; + } +} \ No newline at end of file diff --git a/MailContainerTest/Strategies/Behaviours/OperationalStatusBehaviour.cs b/MailContainerTest/Strategies/Behaviours/OperationalStatusBehaviour.cs new file mode 100644 index 0000000..3ce5d67 --- /dev/null +++ b/MailContainerTest/Strategies/Behaviours/OperationalStatusBehaviour.cs @@ -0,0 +1,12 @@ +using MailContainerTest.Abstractions; +using MailContainerTest.Types; + +namespace MailContainerTest.Strategies.Behaviours; + +public sealed class OperationalStatusBehaviour : IOperationalStatusBehaviour +{ + public bool IsOperationalStatus(MailContainer sourceContainer, MailContainer destContainer) + { + return sourceContainer.Status == MailContainerStatus.Operational && destContainer.Status == MailContainerStatus.Operational; + } +} \ No newline at end of file diff --git a/MailContainerTest/Strategies/LargeLetterStrategy.cs b/MailContainerTest/Strategies/LargeLetterStrategy.cs index 28c5efc..a612eff 100644 --- a/MailContainerTest/Strategies/LargeLetterStrategy.cs +++ b/MailContainerTest/Strategies/LargeLetterStrategy.cs @@ -5,27 +5,51 @@ namespace MailContainerTest.Strategies; public sealed class LargeLetterStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + private readonly IAllowedMailTypeBehaviour _allowedMailTypeBehaviour; + private readonly IOperationalStatusBehaviour _operationalStatusBehaviour; + private readonly ICapacityBehaviour _capacityBehaviour; + + public LargeLetterStrategy(IAllowedMailTypeBehaviour allowedMailTypeBehaviour, + IOperationalStatusBehaviour operationalStatusBehaviour, + ICapacityBehaviour capacityBehaviour) + { + _allowedMailTypeBehaviour = allowedMailTypeBehaviour; + _operationalStatusBehaviour = operationalStatusBehaviour; + _capacityBehaviour = capacityBehaviour; + } + + public bool IsSuccess(MailContainer sourceContainer, MailContainer destContainer, MakeMailTransferRequest request) { - if (sourceContainer is null || destContainer is null) + if (_allowedMailTypeBehaviour.IsAllowedMailType(sourceContainer, destContainer) is false) { return false; } - - if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter) || !destContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter)) + + if (_operationalStatusBehaviour.IsOperationalStatus(sourceContainer, destContainer) is false) { return false; } - - if (sourceContainer.Status != MailContainerStatus.Operational || destContainer.Status != MailContainerStatus.Operational) + + if (_capacityBehaviour.IsWithinCapacity(sourceContainer, request) is false) { return false; } - if (sourceContainer.Capacity < request.NumberOfMailItems) - { - return false; - } + // Unit tests for member data (also commented out in MailContainerTest.Tests\LargeLetterStrategyTests.cs): + // if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter) || !destContainer.AllowedMailType.HasFlag(AllowedMailType.LargeLetter)) + // { + // return false; + // } + // + // if (sourceContainer.Status != MailContainerStatus.Operational || destContainer.Status != MailContainerStatus.Operational) + // { + // return false; + // } + // + // if (sourceContainer.Capacity < request.NumberOfMailItems) + // { + // return false; + // } return true; } diff --git a/MailContainerTest/Strategies/SmallParcelStrategy.cs b/MailContainerTest/Strategies/SmallParcelStrategy.cs index 5f690ba..a2119f2 100644 --- a/MailContainerTest/Strategies/SmallParcelStrategy.cs +++ b/MailContainerTest/Strategies/SmallParcelStrategy.cs @@ -5,24 +5,24 @@ namespace MailContainerTest.Strategies; public sealed class SmallParcelStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) + private readonly IAllowedMailTypeBehaviour _allowedMailTypeBehaviour; + private readonly IOperationalStatusBehaviour _operationalStatusBehaviour; + + public SmallParcelStrategy(IAllowedMailTypeBehaviour allowedMailTypeBehaviour, + IOperationalStatusBehaviour operationalStatusBehaviour) { - if (sourceContainer is null || destContainer is null) - { - return false; - } - - if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel) || !destContainer.AllowedMailType.HasFlag(AllowedMailType.SmallParcel)) - { - return false; - } - - if (sourceContainer.Status != MailContainerStatus.Operational || destContainer.Status != MailContainerStatus.Operational) + _allowedMailTypeBehaviour = allowedMailTypeBehaviour; + _operationalStatusBehaviour = operationalStatusBehaviour; + } + + public bool IsSuccess(MailContainer sourceContainer, MailContainer destContainer, MakeMailTransferRequest request) + { + if (_allowedMailTypeBehaviour.IsAllowedMailType(sourceContainer, destContainer) is false) { return false; } - - if (sourceContainer.Capacity < request.NumberOfMailItems) + + if (_operationalStatusBehaviour.IsOperationalStatus(sourceContainer, destContainer) is false) { return false; } diff --git a/MailContainerTest/Strategies/StandardLetterStrategy.cs b/MailContainerTest/Strategies/StandardLetterStrategy.cs index bd65e49..c91ad23 100644 --- a/MailContainerTest/Strategies/StandardLetterStrategy.cs +++ b/MailContainerTest/Strategies/StandardLetterStrategy.cs @@ -5,24 +5,16 @@ namespace MailContainerTest.Strategies; public sealed class StandardLetterStrategy : IMailTransferStrategy { - public bool IsSuccess(MailContainer? sourceContainer, MailContainer? destContainer, MakeMailTransferRequest request) - { - if (sourceContainer is null || destContainer is null) - { - return false; - } - - if (!sourceContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter) || !destContainer.AllowedMailType.HasFlag(AllowedMailType.StandardLetter)) - { - return false; - } - - if (sourceContainer.Status != MailContainerStatus.Operational || destContainer.Status != MailContainerStatus.Operational) - { - return false; - } + private readonly IAllowedMailTypeBehaviour _allowedMailTypeBehaviour; - if (sourceContainer.Capacity < request.NumberOfMailItems) + public StandardLetterStrategy(IAllowedMailTypeBehaviour allowedMailTypeBehaviour) + { + _allowedMailTypeBehaviour = allowedMailTypeBehaviour; + } + + public bool IsSuccess(MailContainer sourceContainer, MailContainer destContainer, MakeMailTransferRequest request) + { + if (_allowedMailTypeBehaviour.IsAllowedMailType(sourceContainer, destContainer) is false) { return false; } diff --git a/MailContainerTest/Types/MakeMailTransferRequest.cs b/MailContainerTest/Types/MakeMailTransferRequest.cs index dc99496..87ca1df 100644 --- a/MailContainerTest/Types/MakeMailTransferRequest.cs +++ b/MailContainerTest/Types/MakeMailTransferRequest.cs @@ -1,6 +1,6 @@ namespace MailContainerTest.Types { - public record MakeMailTransferRequest + public sealed record MakeMailTransferRequest { public MailContainerNumber SourceMailContainerNumber { get; init; } public MailContainerNumber DestinationMailContainerNumber { get; init; }