diff --git a/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj b/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj index b21ab73..83e3c10 100644 --- a/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj +++ b/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj @@ -5,6 +5,18 @@ true + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + diff --git a/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs new file mode 100644 index 0000000..61bb4d4 --- /dev/null +++ b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs @@ -0,0 +1,182 @@ +using ClearBank.DeveloperTest.Data; +using ClearBank.DeveloperTest.Services; +using ClearBank.DeveloperTest.Types; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace ClearBank.DeveloperTest.Tests +{ + public class PaymentServiceTests + { + private readonly Mock _accountDataStoreMock; + + private readonly Mock> _loggerMock; + + public PaymentServiceTests() + { + _accountDataStoreMock = new Mock(); + _loggerMock = new Mock>(); + } + + //Define special condition accounts to be used in tests + private readonly Account _accountNoBacs = new Account + { + AccountNumber = "123456", + Balance = 500, + Status = AccountStatus.Live, + AllowedPaymentSchemes = AllowedPaymentSchemes.FasterPayments | AllowedPaymentSchemes.Chaps + }; + + private readonly Account _noBalanceFasterPaymentAccount = new Account + { + AccountNumber = "111222", + Balance = 0, + Status = AccountStatus.Live, + AllowedPaymentSchemes = AllowedPaymentSchemes.FasterPayments + }; + + private readonly Account _disabledChapsAccount = new Account + { + AccountNumber = "111222", + Balance = 500, + Status = AccountStatus.Disabled, + AllowedPaymentSchemes = AllowedPaymentSchemes.Chaps + }; + + [Fact] + public void MakePayment_ShouldReturnFalse_WhenAccountIsNull() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.Bacs + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns((Account)null); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Fact] + public void MakeBacsPayment_ShouldReturnFalse_WhenAccountBacsPaymentSchemeIsDisallowed() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.Bacs + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(_accountNoBacs); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Fact] + public void MakeFasterPaymentsPayment_ShouldReturnFalse_WhenAccountHasZeroBalance() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.FasterPayments + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(_noBalanceFasterPaymentAccount); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Fact] + public void MakeChapsPayment_ShouldReturnFalse_WhenAccountAccountStatusIsDisabled() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.Chaps + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(_disabledChapsAccount); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Theory] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.Bacs)] + [InlineData(PaymentScheme.FasterPayments, AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Chaps)] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.Bacs | AllowedPaymentSchemes.Chaps | AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Chaps | AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.FasterPayments, AllowedPaymentSchemes.Bacs | AllowedPaymentSchemes.FasterPayments)] + public void MakePayment_ShouldReturnTrue_WhenAccountIsActive_AndHasCorrectPaymentScheme(PaymentScheme paymentScheme, AllowedPaymentSchemes allowedPaymentSchemes) + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); + + var account = new Account + { + AccountNumber = "12345678", + Balance = 500, + Status = AccountStatus.Live, + AllowedPaymentSchemes = allowedPaymentSchemes + }; + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = paymentScheme + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(account); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.True(result.Success); + } + + [Theory] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Bacs)] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.FasterPayments, AllowedPaymentSchemes.Chaps)] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.Chaps | AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Bacs | AllowedPaymentSchemes.FasterPayments)] + public void MakePayment_ShouldReturnFalse_WhenAccountIsActive_AndHasIncorrectPaymentScheme(PaymentScheme paymentScheme, AllowedPaymentSchemes allowedPaymentSchemes) + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); + + var account = new Account + { + AccountNumber = "12345678", + Balance = 500, + Status = AccountStatus.Live, + AllowedPaymentSchemes = allowedPaymentSchemes + }; + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = paymentScheme + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(account); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + } +} diff --git a/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj b/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj index 6aac7db..f6a33a8 100644 --- a/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj +++ b/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj @@ -1,11 +1,13 @@ - + net8.0 + enable + enable - + - \ No newline at end of file + diff --git a/ClearBank.DeveloperTest/Controllers/PaymentServiceController.cs b/ClearBank.DeveloperTest/Controllers/PaymentServiceController.cs new file mode 100644 index 0000000..20c78b0 --- /dev/null +++ b/ClearBank.DeveloperTest/Controllers/PaymentServiceController.cs @@ -0,0 +1,24 @@ +using ClearBank.DeveloperTest.Services; +using ClearBank.DeveloperTest.Types; +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +namespace ClearBank.DeveloperTest.Controllers +{ + [ApiController] + [Route("[controller]")] + public class PaymentServiceController : ControllerBase + { + private readonly IPaymentService _paymentService; + public PaymentServiceController(IPaymentService paymentService) + { + _paymentService = paymentService; + } + + [HttpPost] + public MakePaymentResult Post(MakePaymentRequest request) + { + return _paymentService.MakePayment(request); + } + } +} diff --git a/ClearBank.DeveloperTest/Data/AccountDataStore.cs b/ClearBank.DeveloperTest/Data/AccountDataStore.cs index 89e49dd..800deb0 100644 --- a/ClearBank.DeveloperTest/Data/AccountDataStore.cs +++ b/ClearBank.DeveloperTest/Data/AccountDataStore.cs @@ -2,8 +2,13 @@ namespace ClearBank.DeveloperTest.Data { - public class AccountDataStore + public class AccountDataStore : IAccountDataStore { + public AccountDataStore() + { + // I'd add here the database context, or the account service provided its an external service + } + public Account GetAccount(string accountNumber) { // Access database to retrieve account, code removed for brevity diff --git a/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs b/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs index 170362c..e1746da 100644 --- a/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs +++ b/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs @@ -2,8 +2,13 @@ namespace ClearBank.DeveloperTest.Data { - public class BackupAccountDataStore + public class BackupAccountDataStore : IAccountDataStore { + public BackupAccountDataStore() + { + // I'd add here the backup database context if needed, or the backup account service provided its an external service + } + public Account GetAccount(string accountNumber) { // Access backup data base to retrieve account, code removed for brevity diff --git a/ClearBank.DeveloperTest/Data/IAccountDataStore.cs b/ClearBank.DeveloperTest/Data/IAccountDataStore.cs new file mode 100644 index 0000000..ff14fb3 --- /dev/null +++ b/ClearBank.DeveloperTest/Data/IAccountDataStore.cs @@ -0,0 +1,11 @@ +using ClearBank.DeveloperTest.Types; + +namespace ClearBank.DeveloperTest.Data +{ + public interface IAccountDataStore + { + Account GetAccount(string accountNumber); + + void UpdateAccount(Account account); + } +} diff --git a/ClearBank.DeveloperTest/Program.cs b/ClearBank.DeveloperTest/Program.cs new file mode 100644 index 0000000..4311aa2 --- /dev/null +++ b/ClearBank.DeveloperTest/Program.cs @@ -0,0 +1,42 @@ +using ClearBank.DeveloperTest.Services; +using ClearBank.DeveloperTest.Data; +using ClearBank.DeveloperTest.Types; + +var builder = WebApplication.CreateBuilder(args); + +// Add services to the container. + +builder.Services.AddControllers(); +// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle +builder.Services.AddEndpointsApiExplorer(); +builder.Services.AddSwaggerGen(); +builder.Services.AddLogging(); + +bool useBackupDataStore = (bool)builder.Configuration.GetValue(typeof(bool), "UseBackupDataStore"); +if (useBackupDataStore) +{ + builder.Services.AddSingleton(); +} +else +{ + builder.Services.AddSingleton(); +} + +builder.Services.AddScoped(); + +var app = builder.Build(); + +// Configure the HTTP request pipeline. +if (app.Environment.IsDevelopment()) +{ + app.UseSwagger(); + app.UseSwaggerUI(); +} + +app.UseHttpsRedirection(); + +app.UseAuthorization(); + +app.MapControllers(); + +app.Run(); diff --git a/ClearBank.DeveloperTest/Services/PaymentService.cs b/ClearBank.DeveloperTest/Services/PaymentService.cs index 573713d..4ad5233 100644 --- a/ClearBank.DeveloperTest/Services/PaymentService.cs +++ b/ClearBank.DeveloperTest/Services/PaymentService.cs @@ -1,93 +1,79 @@ using ClearBank.DeveloperTest.Data; using ClearBank.DeveloperTest.Types; -using System.Configuration; +using System.Security.Principal; namespace ClearBank.DeveloperTest.Services { public class PaymentService : IPaymentService { - public MakePaymentResult MakePayment(MakePaymentRequest request) + private readonly IAccountDataStore _accountDataStore; + private readonly ILogger _logger; + + public PaymentService(IAccountDataStore accountDataStore, ILogger logger) { - var dataStoreType = ConfigurationManager.AppSettings["DataStoreType"]; - - Account account = null; + _accountDataStore = accountDataStore; + _logger = logger; + } - if (dataStoreType == "Backup") - { - var accountDataStore = new BackupAccountDataStore(); - account = accountDataStore.GetAccount(request.DebtorAccountNumber); - } - else - { - var accountDataStore = new AccountDataStore(); - account = accountDataStore.GetAccount(request.DebtorAccountNumber); - } + public MakePaymentResult MakePayment(MakePaymentRequest request) + { + Account account = _accountDataStore.GetAccount(request.DebtorAccountNumber); var result = new MakePaymentResult(); - result.Success = true; - - switch (request.PaymentScheme) + result.Success = IsAccountStatusValid(account, request); + + if (result.Success) { - case PaymentScheme.Bacs: - if (account == null) - { - result.Success = false; - } - else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.Bacs)) - { - result.Success = false; - } - break; + account.Balance -= request.Amount; - case PaymentScheme.FasterPayments: - if (account == null) - { - result.Success = false; - } - else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.FasterPayments)) - { - result.Success = false; - } - else if (account.Balance < request.Amount) - { - result.Success = false; - } - break; + _accountDataStore.UpdateAccount(account); - case PaymentScheme.Chaps: - if (account == null) - { - result.Success = false; - } - else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.Chaps)) - { - result.Success = false; - } - else if (account.Status != AccountStatus.Live) - { - result.Success = false; - } - break; + _logger.LogInformation($"Payment of {request.Amount} from account {account.AccountNumber} successful."); } - if (result.Success) + return result; + } + + private bool IsAccountStatusValid(Account account, MakePaymentRequest request) + { + if (account == null) { - account.Balance -= request.Amount; + return false; + } + else { + if (!account.AllowedPaymentSchemes.HasFlag((AllowedPaymentSchemes)request.PaymentScheme)) + { + _logger.LogWarning($"{0} payment scheme not allowed for account {1}.", request.PaymentScheme, account.AccountNumber); + return false; + } - if (dataStoreType == "Backup") + if (account.Balance < request.Amount) { - var accountDataStore = new BackupAccountDataStore(); - accountDataStore.UpdateAccount(account); + _logger.LogWarning($"Payment failed for account {account.AccountNumber} as the balance is less than the amount"); + return false; } - else + + if (account.Status != AccountStatus.Live) { - var accountDataStore = new AccountDataStore(); - accountDataStore.UpdateAccount(account); + _logger.LogWarning($"Payment failed as account {account.AccountNumber} is currently disabled"); + return false; } - } - return result; + //This follows the original logic however I couldn't help feeling that the same logic should apply to all transactions, not just Chaps or Faster Payments, + //hence why this has been commented out. Would appreciate your thoughts on this. + //if (account.Status != AccountStatus.Live && request.PaymentScheme == PaymentScheme.Chaps) + //{ + // _logger.LogWarning($"Chaps payment failed for account {account.AccountNumber} as the account status is not live"); + // return false; + //} + //else if (account.Balance < request.Amount && request.PaymentScheme == PaymentScheme.FasterPayments) + //{ + // _logger.LogWarning($"Faster Payments payment failed for account {account.AccountNumber} as the balance is less than the amount"); + // return false; + //} + } + return true; } } } diff --git a/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs b/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs index 4e582b6..b9595bd 100644 --- a/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs +++ b/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs @@ -1,17 +1,24 @@ using System; +using System.ComponentModel.DataAnnotations; namespace ClearBank.DeveloperTest.Types { public class MakePaymentRequest { + [Required(ErrorMessage = "Please provide a Creditor Account Number")] public string CreditorAccountNumber { get; set; } + [Required(ErrorMessage = "Please provide a Debtor Account Number")] public string DebtorAccountNumber { get; set; } + [Required(ErrorMessage = "Please provide a payment amount")] + [Range(0.01, double.MaxValue, ErrorMessage = "Amount must be greater than zero")] public decimal Amount { get; set; } + [Required(ErrorMessage = "Please provide a Payment Date")] public DateTime PaymentDate { get; set; } + [EnumDataType(typeof(PaymentScheme), ErrorMessage = "Please provide a valid Payment Scheme")] public PaymentScheme PaymentScheme { get; set; } } } diff --git a/ClearBank.DeveloperTest/Types/PaymentScheme.cs b/ClearBank.DeveloperTest/Types/PaymentScheme.cs index d521384..027be9c 100644 --- a/ClearBank.DeveloperTest/Types/PaymentScheme.cs +++ b/ClearBank.DeveloperTest/Types/PaymentScheme.cs @@ -1,9 +1,14 @@ -namespace ClearBank.DeveloperTest.Types +using System.ComponentModel; + +namespace ClearBank.DeveloperTest.Types { public enum PaymentScheme { - FasterPayments, - Bacs, - Chaps + [Description("Faster Payments")] + FasterPayments = 1 << 0, + [Description("Bacs")] + Bacs = 1 << 1, + [Description("Chaps")] + Chaps = 1 << 2 } } diff --git a/ClearBank.DeveloperTest/appsettings.Development.json b/ClearBank.DeveloperTest/appsettings.Development.json new file mode 100644 index 0000000..0c208ae --- /dev/null +++ b/ClearBank.DeveloperTest/appsettings.Development.json @@ -0,0 +1,8 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning" + } + } +} diff --git a/ClearBank.DeveloperTest/appsettings.json b/ClearBank.DeveloperTest/appsettings.json new file mode 100644 index 0000000..8ab1ca7 --- /dev/null +++ b/ClearBank.DeveloperTest/appsettings.json @@ -0,0 +1,10 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning" + } + }, + "AllowedHosts": "*", + "UseBackupDataStore": false +}